[meta] Problems with the rules that use scope analysis [no-unused-vars][no-undef][no-shadow][no-redeclare][no-use-before-define]

[bradzacher]

I wanted to merge all of the scope analysis rule issues together into a single meta-issue. So I could centrally provide some context on what the problem is, and what needs to be done.

The Problem

The no-unused-vars rule, as with many other rules in ESLint [1][2] relies upon scope analysis.

For context; an ESLint parser may additionally provide a ScopeManager alongside the AST it produces. A ScopeManager is a utility class which can be used to interrogate the variables available in a certain scope.

ESLint rules can then use this ScopeManager to gather information about the variables that are included in the current scope - i.e. what variables are defined.

Other parsers (such as babel-eslint) provide scope analysis, but most other parsers have a smaller scope of extensions to support, so they can just build on top of the existing scope manager, or they get around it via "extension" rules to either mark things as used, or to ignore things from rules entirely.

In TypeScript's case, in order for us to properly describe the scope and usage across TS's dual-scope system, it would require a large rewrite of the scope analyser (eg #1533).

This is never something we invested much time into for a number of reasons like:

• most of the rules that use the scope manager are either infrequently used, or have analogous checks/options in the TypeScript itself,
• we wanted to be careful that we wouldn't implement something that would get outdated by changes to the underlying TS language,
• we explored other options that would not require scope analysis:
  • manually marking nodes via context.markVariableAsUsed.
    • This got us a decent chunk of the way, but it's not a great way to handle all of the cases and scope boundaries.
  • no-unused-vars-experimental.
    • This was a great idea in theory, but the getSemanticDiagnostics API is too slow for the rule to be usable on large codebases (~30ms per file) ([no-unused-vars-experimental] rule is slow #1335)
• the inner workings of eslint-scope are relatively poorly documented, so implementing TS scope into it was always a large task.

The Solution

It's become very clear that we need to bite the bullet and just do this work.
We're currently building out a full scope manager (#1939), but it is not a small task, so please be patient.

[bradzacher]

On the sidebar you can see the linked pull requests - it is being worked on.
As per the OP - it's a non-trivial change to do, as evidenced by the 214,968 lines of change that have been submitted to fix it so far.

The maintainers of this project are volunteers that give their free time when they can. Big issues like this are going to take a lot of time to complete.

[bradzacher]

This has been released to NPM as part of the prerelease version for v4.

Try out the rc-v4 tag on NPM.

Please raise any bugs you find as a new issue so we can track and fix them before the full release!
We're working on finalising all the work for the v4 release so eta for full release is uncertain.

https://github.com/typescript-eslint/typescript-eslint/milestone/5
https://github.com/typescript-eslint/typescript-eslint/releases/tag/v4.0.0-rc

[hiranya911]

Great job on landing this work. I wanted to know a bit more about how this affects the performance issues discussed in #1335. Does this work address the slow execution time in the no-unused-var-experimental rule? Is that available in the v4.0.0-rc packages?

[bradzacher]

I have marked no-unused-vars-experimental as deprecated as part of this change, and we will likely remove that rule in v5.

The underlying infra changes made for this work mean that @typescript-eslint/no-unused-vars rule now just works. It works with the same perf as the core no-unused-vars rule because it has no dependency on typescript or the typechecker, and instead uses a fork(/ complete rewrite) of ESLint's scope analyser.

[vegerot]

Is this still an issue in https://github.com/typescript-eslint/typescript-eslint/blob/v4.1.0/packages/eslint-plugin/docs/rules/no-use-before-define.md ?

[foucdeg]

@vegerot I'm still having this issue with typescript-eslint 4.4.0 and TS 4.0.3.

[bradzacher]

Please raise a new issue and fill out the template.
"I'm still having this issue" is pretty useless as it gives us no information to help you.