Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): [no-unused-vars] correct detection of unused vars in a declared module with export = #2505

Merged
merged 1 commit into from Sep 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/rules/indent.ts
Expand Up @@ -410,7 +410,7 @@ export default util.createRule<Options, MessageIds>({
// 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,
Expand Down
37 changes: 34 additions & 3 deletions packages/eslint-plugin/src/rules/no-unused-vars.ts
Expand Up @@ -29,6 +29,7 @@ export default util.createRule<Options, MessageIds>({
create(context) {
const rules = baseRule.create(context);
const filename = context.getFilename();
const MODULE_DECL_CACHE = new Map<TSESTree.TSModuleDeclaration, boolean>();

/**
* Gets a list of TS module definitions for a specified variable.
Expand Down Expand Up @@ -209,7 +210,7 @@ export default util.createRule<Options, MessageIds>({
},

// declaration file handling
[declarationSelector(AST_NODE_TYPES.Program, true)](
[ambientDeclarationSelector(AST_NODE_TYPES.Program, true)](
node: DeclarationSelectorNode,
): void {
if (!util.isDefinitionFile(filename)) {
Expand All @@ -219,14 +220,44 @@ export default util.createRule<Options, MessageIds>({
},

// 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
Expand All @@ -236,7 +267,7 @@ export default util.createRule<Options, MessageIds>({
| TSESTree.TSEnumDeclaration
| TSESTree.TSModuleDeclaration
| TSESTree.VariableDeclaration;
function declarationSelector(
function ambientDeclarationSelector(
parent: string,
childDeclare: boolean,
): string {
Expand Down
32 changes: 32 additions & 0 deletions packages/eslint-plugin/tests/rules/no-unused-vars.test.ts
Expand Up @@ -848,6 +848,18 @@ export type Test<U> = U extends (arg: {
jsxFragmentName: 'Fragment',
},
},
`
declare module 'foo' {
type Test = 1;
}
`,
`
declare module 'foo' {
type Test = 1;
const x: Test = 1;
export = x;
}
`,
],

invalid: [
Expand Down Expand Up @@ -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: '',
},
},
],
},
],
});
1 change: 0 additions & 1 deletion packages/eslint-plugin/typings/eslint-rules.d.ts
Expand Up @@ -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',
Expand Down
26 changes: 18 additions & 8 deletions packages/scope-manager/src/scope/ScopeBase.ts
Expand Up @@ -15,6 +15,7 @@ import {
ReferenceTypeFlag,
} from '../referencer/Reference';
import { Variable } from '../variable';
import { TSModuleScope } from './TSModuleScope';

/**
* Test if scope is strict
Expand Down Expand Up @@ -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<ScopeType, TSESTree.Node, Scope | null>;
abstract class ScopeBase<
TType extends ScopeType,
Expand Down Expand Up @@ -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,
Expand All @@ -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;

/**
Expand All @@ -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;
}
Expand Down
18 changes: 16 additions & 2 deletions packages/types/src/ts-estree.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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[];
Expand Down Expand Up @@ -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 {
Expand Down