Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

[FEAT] [no-for-in-array] Add rule #250

Closed
wants to merge 3 commits into from

Conversation

uniqueiniquity
Copy link

  • I’ve filled out the documentation, including all relevant sections and a link to the equivalent TSLint rule if available.
  • I’ve filled out the rule’s category
  • I’ve added documentation for the equivalent TSLint rule (in extraDescription)
  • I’ve added the rule documentation link via the helper from ../utils.
  • I’ve added the rule to the roadmap
  • I’ve added tests for my changes

Adding equivalent of TSLint's no-for-in-array. Depends on eslint/typescript-eslint-parser#568 for type information.

@uniqueiniquity
Copy link
Author

@armano2 How did you get your tests in #230 to pass without the typescript-eslint-parser changes being merged?

@armano2
Copy link
Contributor

armano2 commented Dec 18, 2018

@uniqueiniquity i installed package as

"typescript-eslint-parser": "git+https://github.com/uniqueiniquity/typescript-eslint-parser.git#0625f29a9721c6f10a7522de6c9d236a671bd1ac"

https://github.com/bradzacher/eslint-plugin-typescript/pull/230/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R26

@uniqueiniquity
Copy link
Author

I will probably leave this as-is and re-run the tests when the parser gets updated.

@bradzacher bradzacher added the WIP PRs that are work in progress label Dec 18, 2018
ROADMAP.md Outdated Show resolved Hide resolved
!context.parserServices.program
) {
return;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does context.parserServices.program exist when create is first called, as implied by #230? If so, could you...

  • ...return an empty object instead of one with ForInStatement?
  • ...move the checker = context...getTypechecker() call to outside this returned function?

The two would be nice as precedent for other rules that would have multiple methods using the type checker.

Apologies if this is well known; I'm very new to ESLint. 😊

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should use https://github.com/bradzacher/eslint-plugin-typescript/blob/master/lib/util.js#L114

you need access to context and context is parameter of function create(context)

!context.parserServices.program
) {
return;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TSLint will warn for each rule enabled that requires type information when it's not provided. Should there be some standard way for these to warn in a similar manner? Would that be a separate issue from this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs type info WIP PRs that are work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants