feat(eslint-plugin): [no-unused-vars] fork the base rule

[bradzacher]

I wanted to avoid doing this, but not doing this restricts our logic too much - it causes problems when we want to consider reporting on things that the base rule wouldn't report on.

By forking this logic - it also opens the door to using this in naming-convention to detect unused variables.

Fixes #2782
Fixes #2714
Fixes #2648
Closes #2679

[typescript-eslint]

Thanks for the PR, @bradzacher!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

---

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

[codecov]

Codecov Report

Merging #2768 (40e40c4) into master (c2dde58) will decrease coverage by 0.03%.
The diff coverage is 91.88%.

@@            Coverage Diff             @@
##           master    #2768      +/-   ##
==========================================
- Coverage   92.80%   92.76%   -0.04%     
==========================================
  Files         300      301       +1     
  Lines        9857    10150     +293     
  Branches     2769     2870     +101     
==========================================
+ Hits         9148     9416     +268     
- Misses        332      345      +13     
- Partials      377      389      +12     

Flag	Coverage Δ
unittest	92.76% <91.88%> (-0.04%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ges/experimental-utils/src/ts-eslint/RuleTester.ts	100.00% <ø> (ø)
packages/experimental-utils/src/ts-eslint/Scope.ts	100.00% <ø> (ø)
...ges/experimental-utils/src/ast-utils/predicates.ts	36.84% <20.00%> (-1.62%)	⬇️
...ckages/scope-manager/src/referencer/VisitorBase.ts	85.29% <75.00%> (-4.00%)	⬇️
...s/eslint-plugin/src/util/collectUnusedVariables.ts	91.10% <91.10%> (ø)
packages/eslint-plugin/src/rules/no-unused-vars.ts	96.62% <98.95%> (+3.48%)	⬆️

[dentuzhik]

Hey 👋🏼

Sorry if this is not a correct channel to ask this, I can raise a separate issue as well, just let me know :)

Can you please help out to understand better the relation between this rule, new naming-convnention options and the no-unused-vars-experimental?

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars.md
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars-experimental.md

Up to 4.7.0 we had to use no-unused-vars-experimental because of the case of const foo = (_myUnusedVar, myUsedVar) => {}, but I see that experimental rule gets no development and wondering if a new unused parameter introduced in #2810 in naming-convention fixes the case.

We currently have this, but as I understand we can change it to some combo of post 4.9.0 non-experimental rules, right?

{
    // While main rules works, it reports errors in places where TS would not complain, like underscored variables e.g. _actions
    // https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars.md
    //
    // Experimental rule seems to be aligned better with TS itself
    // https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unused-vars-experimental.md
    'no-unused-vars': [OFF],
    '@typescript-eslint/no-unused-vars-experimental': [ERROR],
}

Seems like documentation for no-unused-vars could maybe be a bit more clear on if and how it is expected to be combined with naming-convention, I can help out with PR as well

[HolgerJeromin]

My understanding is this:
The no-unused-vars-experimental version was kind of a technology experiment which rebuilt the rule on a different technical base.
This no-unused-vars is another way to do it.
As the policy is to never delete a rule we now have both usable.

[bradzacher]

This is explained in #1856

[SimenB]

this line breaks using eslint@5, which is supported:

typescript-eslint/packages/eslint-plugin/package.json

Line 64 in 1ef0d64

"eslint": "^5.0.0 || ^6.0.0 || ^7.0.0"

[SimenB]

see jest-community/eslint-plugin-jest#732 for CI failure

    Cannot find module 'eslint/lib/rules/utils/ast-utils'
    Require stack:
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/@typescript-eslint/eslint-plugin/dist/rules/no-unused-vars.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/@typescript-eslint/eslint-plugin/dist/rules/index.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/@typescript-eslint/eslint-plugin/dist/index.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/eslint/lib/config/plugins.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/eslint/lib/config.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/eslint/lib/cli-engine.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/eslint/lib/api.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/jest-runner-eslint/build/runner/runESLint.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/create-jest-runner/build/createJestRunner.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/create-jest-runner/build/index.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/jest-runner-eslint/build/runner/index.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/@jest/core/build/TestScheduler.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/@jest/core/build/jest.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/jest-cli/build/cli/index.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/jest-cli/bin/jest.js
    - /home/runner/work/eslint-plugin-jest/eslint-plugin-jest/node_modules/jest/bin/jest.js