noUnusedVariableRule should apply the ignorePattern to imports

[brandf]

I have a import that I need to ignore (custom reactNamespace), however ignore-pattern is not applied to imports.

PR checklist

• Addresses an existing issue: no-unused-variable: ignore-pattern doesn't apply to imports #3186
• New feature, bugfix, or enhancement
  • Includes tests
• Documentation update

Overview of change:

applies the importPattern to importNames (in addition to the existing behavior)

Is there anything you'd like reviewers to focus on?

not sure if importName.text is the correct thing to test against, but it seems to work.

[palantirtech]

Thanks for your interest in palantir/tslint, @brandf! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[brandf]

I'm happy to add tests/doc changes, but will need some direction on getting setup to run them. I just did a quick 'fork and edit this file' on github.

[ajafff]

ignorePattern may be undefined. That needs to be handled.
The easier fix would be moving the block at line 134 above line 119.

Regarding documentation: docs are generated from the metadata in the rule. That means you can simply replace line 41 with

Variable names and imports that match the pattern will be ignored.

Regarding tests: you can add a test case in test/rules/no-unused-variable/ignore-pattern/var.ts.lint

To test this locally, you need to

• yarn install inside your local clone of the repository (or npm install if you don't have yarn installed)
• edit the rule and the tests
• npm run verify will compile, run the linter and the tests afterwards
• if everything is fine, commit and push as usual

[brandf]

Sorry for the delay on this, I will follow up with ajafff's suggestions soon. In laws are in town, so not a lot of time atm ;-)

[brandf]

@ajafff, apologies again for the delay on this. The latest includes the easier fix, docs updated, test case, and tests passing.

[brandf]

Container 3 in circleci is failing - I can repro it locally with:

npm run clean && yarn compile && yarn add typescript@next && yarn test

...but I'm not sure how to fix it. It looks like the error message is different in typescript@next:

                                           ~~~~              ['args' is declared but its value is never read.]
                                           ~~~~              ['args' is declared but never used.]

[brandf]

After looking at other PR's, the CI failure appears to be unrelated to my change. It's just a coincidence that it was failing in the rule I modified :-/

For example, same failure here:
#3238

[ajafff]

Please merge master into your branch to fix the CI failures

[brandf]

Should be all set - green CI after rebasing

[brandf]

@adidahiya, you can remove the 'waiting on author' tag. Should be good to merge.

[ajafff]

Thanks @brandf