From 3d07a99faa0a5fc1b44acdb43ddbfc90a5105833 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sun, 6 Sep 2020 13:20:23 -0700 Subject: [PATCH] fix(eslint-plugin): [no-unused-vars] correct detection of unused vars in a declared module with `export =` (#2505) If a `declare module` has an `export =` in its body, then TS will only export that. If it doesn't have an `export =`, then all things are ambiently exported. This adds handling to correctly detect this case. --- .../src/rules/adjacent-overload-signatures.ts | 5 ++- packages/eslint-plugin/src/rules/indent.ts | 2 +- .../eslint-plugin/src/rules/no-unused-vars.ts | 37 +++++++++++++++++-- .../tests/rules/no-unused-vars.test.ts | 32 ++++++++++++++++ .../eslint-plugin/typings/eslint-rules.d.ts | 1 - packages/scope-manager/src/scope/ScopeBase.ts | 26 +++++++++---- packages/types/src/ts-estree.ts | 18 ++++++++- 7 files changed, 105 insertions(+), 16 deletions(-) diff --git a/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts b/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts index c7cd7be84b4..05f120b4cf2 100644 --- a/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts +++ b/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts @@ -10,7 +10,10 @@ type RuleNode = | TSESTree.TSModuleBlock | TSESTree.TSTypeLiteral | TSESTree.TSInterfaceBody; -type Member = TSESTree.ClassElement | TSESTree.Statement | TSESTree.TypeElement; +type Member = + | TSESTree.ClassElement + | TSESTree.ProgramStatement + | TSESTree.TypeElement; export default util.createRule({ name: 'adjacent-overload-signatures', diff --git a/packages/eslint-plugin/src/rules/indent.ts b/packages/eslint-plugin/src/rules/indent.ts index 34ec4015179..9ac6a1593d9 100644 --- a/packages/eslint-plugin/src/rules/indent.ts +++ b/packages/eslint-plugin/src/rules/indent.ts @@ -410,7 +410,7 @@ export default util.createRule({ // transform it to a BlockStatement return rules['BlockStatement, ClassBody']({ type: AST_NODE_TYPES.BlockStatement, - body: node.body, + body: node.body as any, // location data parent: node.parent, diff --git a/packages/eslint-plugin/src/rules/no-unused-vars.ts b/packages/eslint-plugin/src/rules/no-unused-vars.ts index d3dc72049ce..03c27d40616 100644 --- a/packages/eslint-plugin/src/rules/no-unused-vars.ts +++ b/packages/eslint-plugin/src/rules/no-unused-vars.ts @@ -29,6 +29,7 @@ export default util.createRule({ create(context) { const rules = baseRule.create(context); const filename = context.getFilename(); + const MODULE_DECL_CACHE = new Map(); /** * Gets a list of TS module definitions for a specified variable. @@ -209,7 +210,7 @@ export default util.createRule({ }, // declaration file handling - [declarationSelector(AST_NODE_TYPES.Program, true)]( + [ambientDeclarationSelector(AST_NODE_TYPES.Program, true)]( node: DeclarationSelectorNode, ): void { if (!util.isDefinitionFile(filename)) { @@ -219,14 +220,44 @@ export default util.createRule({ }, // declared namespace handling - [declarationSelector( + [ambientDeclarationSelector( 'TSModuleDeclaration[declare = true] > TSModuleBlock', false, )](node: DeclarationSelectorNode): void { + const moduleDecl = util.nullThrows( + node.parent?.parent, + util.NullThrowsReasons.MissingParent, + ) as TSESTree.TSModuleDeclaration; + + // declared modules with an `export =` statement will only export that one thing + // all other statements are not automatically exported in this case + if (checkModuleDeclForExportEquals(moduleDecl)) { + return; + } + markDeclarationChildAsUsed(node); }, }; + function checkModuleDeclForExportEquals( + node: TSESTree.TSModuleDeclaration, + ): boolean { + const cached = MODULE_DECL_CACHE.get(node); + if (cached != null) { + return cached; + } + + for (const statement of node.body?.body ?? []) { + if (statement.type === AST_NODE_TYPES.TSExportAssignment) { + MODULE_DECL_CACHE.set(node, true); + return true; + } + } + + MODULE_DECL_CACHE.set(node, false); + return false; + } + type DeclarationSelectorNode = | TSESTree.TSInterfaceDeclaration | TSESTree.TSTypeAliasDeclaration @@ -236,7 +267,7 @@ export default util.createRule({ | TSESTree.TSEnumDeclaration | TSESTree.TSModuleDeclaration | TSESTree.VariableDeclaration; - function declarationSelector( + function ambientDeclarationSelector( parent: string, childDeclare: boolean, ): string { diff --git a/packages/eslint-plugin/tests/rules/no-unused-vars.test.ts b/packages/eslint-plugin/tests/rules/no-unused-vars.test.ts index 837246f4583..fbb468e375f 100644 --- a/packages/eslint-plugin/tests/rules/no-unused-vars.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unused-vars.test.ts @@ -848,6 +848,18 @@ export type Test = U extends (arg: { jsxFragmentName: 'Fragment', }, }, + ` +declare module 'foo' { + type Test = 1; +} + `, + ` +declare module 'foo' { + type Test = 1; + const x: Test = 1; + export = x; +} + `, ], invalid: [ @@ -1424,5 +1436,25 @@ export const ComponentFoo = () => { }, ], }, + { + code: ` +declare module 'foo' { + type Test = any; + const x = 1; + export = x; +} + `, + errors: [ + { + messageId: 'unusedVar', + line: 3, + data: { + varName: 'Test', + action: 'defined', + additional: '', + }, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/typings/eslint-rules.d.ts b/packages/eslint-plugin/typings/eslint-rules.d.ts index 9117f06c6c1..7cabdf8a3c9 100644 --- a/packages/eslint-plugin/typings/eslint-rules.d.ts +++ b/packages/eslint-plugin/typings/eslint-rules.d.ts @@ -48,7 +48,6 @@ declare module 'eslint/lib/rules/camelcase' { declare module 'eslint/lib/rules/indent' { import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; - type Listener = (node: TSESTree.Node) => void; type ElementList = number | 'first' | 'off'; const rule: TSESLint.RuleModule< 'wrongIndentation', diff --git a/packages/scope-manager/src/scope/ScopeBase.ts b/packages/scope-manager/src/scope/ScopeBase.ts index 9b852dcff6a..04f38e8a65f 100644 --- a/packages/scope-manager/src/scope/ScopeBase.ts +++ b/packages/scope-manager/src/scope/ScopeBase.ts @@ -15,6 +15,7 @@ import { ReferenceTypeFlag, } from '../referencer/Reference'; import { Variable } from '../variable'; +import { TSModuleScope } from './TSModuleScope'; /** * Test if scope is strict @@ -124,6 +125,14 @@ function registerScope(scopeManager: ScopeManager, scope: Scope): void { const generator = createIdGenerator(); +type VariableScope = GlobalScope | FunctionScope | ModuleScope | TSModuleScope; +const VARIABLE_SCOPE_TYPES = new Set([ + ScopeType.global, + ScopeType.function, + ScopeType.module, + ScopeType.tsModule, +]); + type AnyScope = ScopeBase; abstract class ScopeBase< TType extends ScopeType, @@ -209,11 +218,11 @@ abstract class ScopeBase< */ public readonly variables: Variable[] = []; /** - * For 'global', 'function', and 'module' scopes, this is a self-reference. + * For scopes that can contain variable declarations, this is a self-reference. * For other scope types this is the *variableScope* value of the parent scope. * @public */ - public readonly variableScope: GlobalScope | FunctionScope | ModuleScope; + public readonly variableScope: VariableScope; constructor( scopeManager: ScopeManager, @@ -228,12 +237,9 @@ abstract class ScopeBase< this.#dynamic = this.type === ScopeType.global || this.type === ScopeType.with; this.block = block; - this.variableScope = - this.type === 'global' || - this.type === 'function' || - this.type === 'module' - ? (this as AnyScope['variableScope']) - : upperScopeAsScopeBase.variableScope; + this.variableScope = this.isVariableScope() + ? this + : upperScopeAsScopeBase.variableScope; this.upper = upperScope; /** @@ -252,6 +258,10 @@ abstract class ScopeBase< registerScope(scopeManager, this as Scope); } + private isVariableScope(): this is VariableScope { + return VARIABLE_SCOPE_TYPES.has(this.type); + } + public shouldStaticallyClose(): boolean { return !this.#dynamic; } diff --git a/packages/types/src/ts-estree.ts b/packages/types/src/ts-estree.ts index 61e6bc097a5..31b051d8b9d 100644 --- a/packages/types/src/ts-estree.ts +++ b/packages/types/src/ts-estree.ts @@ -457,6 +457,20 @@ export type PrimaryExpression = | TemplateLiteral | ThisExpression | TSNullKeyword; +export type ProgramStatement = + | ClassDeclaration + | ExportAllDeclaration + | ExportDefaultDeclaration + | ExportNamedDeclaration + | ImportDeclaration + | Statement + | TSDeclareFunction + | TSEnumDeclaration + | TSExportAssignment + | TSImportEqualsDeclaration + | TSInterfaceDeclaration + | TSNamespaceExportDeclaration + | TSTypeAliasDeclaration; export type Property = PropertyComputedName | PropertyNonComputedName; export type PropertyName = PropertyNameComputed | PropertyNameNonComputed; export type PropertyNameComputed = Expression; @@ -1146,7 +1160,7 @@ export interface ObjectPattern extends BaseNode { export interface Program extends BaseNode { type: AST_NODE_TYPES.Program; - body: Statement[]; + body: ProgramStatement[]; sourceType: 'module' | 'script'; comments?: Comment[]; tokens?: Token[]; @@ -1473,7 +1487,7 @@ export interface TSMethodSignatureNonComputedName export interface TSModuleBlock extends BaseNode { type: AST_NODE_TYPES.TSModuleBlock; - body: Statement[]; + body: ProgramStatement[]; } export interface TSModuleDeclaration extends BaseNode {