Fix: Missing parameter properties info in constructors (fixes #143)

[weirdpattern]

Includes modifications to ast-converter and corresponding tests

[eslintbot]

Thanks for the pull request, @weirdpattern! I took a look to make sure it's ready for merging and found some changes are needed:

• The commit summary must be 72 characters or shorter. Please check out our guide for how to properly format your commit summary and update it on this pull request.

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

[weirdpattern]

Updating summary as per comments form eslintbot

[eslintbot]

LGTM

[weirdpattern]

Build is not being updated to complete, although travis completed successfully...

[soda0289]

@weirdpattern Maybe a webhook failed from traivs. No worries.

LGTM, Thanks!

[soda0289]

@JamesHenry Could you take a look at this one too. It would allow for us to implement the TSLint rule no-parameter-properties for the typescript plugin.

[JamesHenry]

@weirdpattern @soda0289 Sorry to be a pain, but could we do an initial PR which adds a test for a standard JS class with one or more parameters in its constructor?

I was surprised to see that we don't already have one in our ecmaFeatures/classes fixtures.

If we could get that merged in first, it would be a really useful reference within this PR to see a before and after. It would also make it much easier to identify any unintended issues.

[JamesHenry]

There may well be no change, based on the code

[weirdpattern]

sure, no problem... I can work on this tonight unless @soda0289 wants to do it now, I leave the decision to you 😀

[soda0289]

I can write some tests, I don't mind

[weirdpattern]

@soda0289 go for it and thank you!

[JamesHenry]

Waiting on #178

[JamesHenry]

Ready to be rebased against master! 😄

[jsf-clabot]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ weirdpattern
✅ JamesHenry
✅ soda0289
❌ ESLint Jenkins

---

ESLint Jenkins seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

[eslintbot]

LGTM

[soda0289]

@weirdpattern I think you did a merge instead of a rebase and force push

[weirdpattern]

@soda0289 I think you are right, I was sure I did a rebase, no idea what I did wrong ... what should I do now?

[soda0289]

@weirdpattern Perfrom the rebase again it should clean it up. When you perfrom the push it might warn you that the histroy has changed. Just ignore the message and do a git push --force

[eslintbot]

LGTM

[weirdpattern]

@soda0289 ok I think I got it right this time... please review

[soda0289]

@weirdpattern Yup! LGTM, Thanks!

[JamesHenry]

Otherwise LGTM! Glad you were able to figure out the rebasing

[JamesHenry]

Using filter is a bit of an odd way to check this. More semantic would be:

isReadonly: param.modifiers.some(function(modifier) {
  return modifier.kind === SyntaxKind.ReadonlyKeyword
})

[eslintbot]

Thanks for the pull request, @weirdpattern! I took a look to make sure it's ready for merging and found some changes are needed:

• The commit summary needs to begin with a tag (such as Fix: or Update:). Please check out our guide for how to properly format your commit summary and update it on this pull request.

• Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

[eslintbot]

LGTM

[weirdpattern]

@JamesHenry done