Skip to content

Commit

Permalink
feat(eslint-plugin): [no-unused-vars] fork the base rule
Browse files Browse the repository at this point in the history
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.

Fixes #2714
Fixes #2648
Closes #2679
  • Loading branch information
bradzacher committed Nov 22, 2020
1 parent c2dde58 commit 8062050
Show file tree
Hide file tree
Showing 8 changed files with 1,196 additions and 222 deletions.
500 changes: 326 additions & 174 deletions packages/eslint-plugin/src/rules/no-unused-vars.ts

Large diffs are not rendered by default.

729 changes: 729 additions & 0 deletions packages/eslint-plugin/src/util/collectUnusedVariables.ts

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
@@ -1,6 +1,7 @@
import { ESLintUtils } from '@typescript-eslint/experimental-utils';

export * from './astUtils';
export * from './collectUnusedVariables';
export * from './createRule';
export * from './isTypeReadonly';
export * from './misc';
Expand Down
64 changes: 59 additions & 5 deletions packages/eslint-plugin/tests/rules/no-unused-vars.test.ts
Expand Up @@ -927,6 +927,39 @@ export declare function setAlignment(value: \`\${VerticalAlignment}-\${Horizonta
type EnthusiasticGreeting<T extends string> = \`\${Uppercase<T>} - \${Lowercase<T>} - \${Capitalize<T>} - \${Uncapitalize<T>}\`;
export type HELLO = EnthusiasticGreeting<"heLLo">;
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/2714
{
code: `
interface IItem {
title: string;
url: string;
children?: IItem[];
}
`,
// unreported because it's in a decl file, even though it's only self-referenced
filename: 'foo.d.ts',
},
// https://github.com/typescript-eslint/typescript-eslint/issues/2648
{
code: `
namespace _Foo {
export const bar = 1;
export const baz = Foo.bar;
}
`,
// ignored by pattern, even though it's only self-referenced
options: [{ varsIgnorePattern: '^_' }],
},
{
code: `
interface _Foo {
a: string;
b: Foo;
}
`,
// ignored by pattern, even though it's only self-referenced
options: [{ varsIgnorePattern: '^_' }],
},
],

invalid: [
Expand Down Expand Up @@ -1376,8 +1409,8 @@ namespace Foo {
action: 'defined',
additional: '',
},
line: 2,
column: 11,
line: 4,
column: 15,
},
],
},
Expand Down Expand Up @@ -1408,8 +1441,8 @@ namespace Foo {
action: 'defined',
additional: '',
},
line: 3,
column: 13,
line: 5,
column: 17,
},
],
},
Expand All @@ -1424,7 +1457,7 @@ interface Foo {
errors: [
{
messageId: 'unusedVar',
line: 2,
line: 4,
data: {
varName: 'Foo',
action: 'defined',
Expand Down Expand Up @@ -1575,5 +1608,26 @@ export namespace Foo {
},
],
},
{
code: `
interface Foo {
a: string;
}
interface Foo {
b: Foo;
}
`,
errors: [
{
messageId: 'unusedVar',
line: 6,
data: {
varName: 'Foo',
action: 'defined',
additional: '',
},
},
],
},
],
});
14 changes: 14 additions & 0 deletions packages/eslint-plugin/typings/eslint-rules.d.ts
Expand Up @@ -811,3 +811,17 @@ declare module 'eslint/lib/rules/space-infix-ops' {
>;
export = rule;
}

declare module 'eslint/lib/rules/utils/ast-utils' {
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';

const utils: {
getNameLocationInGlobalDirectiveComment(
sourceCode: TSESLint.SourceCode,
comment: TSESTree.Comment,
name: string,
): TSESTree.SourceLocation;
};

export = utils;
}
22 changes: 22 additions & 0 deletions packages/experimental-utils/src/ast-utils/predicates.ts
Expand Up @@ -214,6 +214,27 @@ function isAwaitKeyword(
return node?.type === AST_TOKEN_TYPES.Identifier && node.value === 'await';
}

function isLoop(
node: TSESTree.Node | undefined | null,
): node is
| TSESTree.DoWhileStatement
| TSESTree.ForStatement
| TSESTree.ForInStatement
| TSESTree.ForOfStatement
| TSESTree.WhileStatement {
if (!node) {
return false;
}

return (
node.type === AST_NODE_TYPES.DoWhileStatement ||
node.type === AST_NODE_TYPES.ForStatement ||
node.type === AST_NODE_TYPES.ForInStatement ||
node.type === AST_NODE_TYPES.ForOfStatement ||
node.type === AST_NODE_TYPES.WhileStatement
);
}

export {
isAwaitExpression,
isAwaitKeyword,
Expand All @@ -223,6 +244,7 @@ export {
isFunctionOrFunctionType,
isFunctionType,
isIdentifier,
isLoop,
isLogicalOrOperator,
isNonNullAssertionPunctuator,
isNotNonNullAssertionPunctuator,
Expand Down
75 changes: 35 additions & 40 deletions packages/experimental-utils/src/ts-eslint/Scope.ts
@@ -1,56 +1,51 @@
/* eslint-disable @typescript-eslint/no-namespace */

import * as scopeManager from '@typescript-eslint/scope-manager';
import { TSESTree } from '@typescript-eslint/types';

namespace Scope {
// ESLint defines global variables using the eslint-scope Variable class
// So a variable in the scope may be either of these
declare class ESLintScopeVariable {
public readonly defs: Definition[];
public readonly identifiers: TSESTree.Identifier[];
public readonly name: string;
public readonly references: Reference[];
public readonly scope: Scope;

/**
* Written to by ESLint.
* If this key exists, this variable is a global variable added by ESLint.
* If this is `true`, this variable can be assigned arbitrary values.
* If this is `false`, this variable is readonly.
*/
public writeable?: boolean; // note that this isn't a typo - ESlint uses this spelling here

/**
* Written to by ESLint.
* This property is undefined if there are no globals directive comments.
* The array of globals directive comments which defined this global variable in the source code file.
*/
public eslintExplicitGlobal?: boolean;

/**
* Written to by ESLint.
* The configured value in config files. This can be different from `variable.writeable` if there are globals directive comments.
*/
public eslintImplicitGlobalSetting?: 'readonly' | 'writable';

/**
* Written to by ESLint.
* If this key exists, it is a global variable added by ESLint.
* If `true`, this global variable was defined by a globals directive comment in the source code file.
*/
public eslintExplicitGlobalComments?: TSESTree.Comment[];
}

export type ScopeManager = scopeManager.ScopeManager;
export type Reference = scopeManager.Reference;
export type Variable = scopeManager.Variable | ESLintScopeVariable;
export type Variable =
| scopeManager.Variable
| scopeManager.ESLintScopeVariable;
export type Scope = scopeManager.Scope;
export const ScopeType = scopeManager.ScopeType;
// TODO - in the next major, clean this up with a breaking change
export type DefinitionType = scopeManager.Definition;
export type Definition = scopeManager.Definition;
export const DefinitionType = scopeManager.DefinitionType;

export namespace Definitions {
export type CatchClauseDefinition = scopeManager.CatchClauseDefinition;
export type ClassNameDefinition = scopeManager.ClassNameDefinition;
export type FunctionNameDefinition = scopeManager.FunctionNameDefinition;
export type ImplicitGlobalVariableDefinition = scopeManager.ImplicitGlobalVariableDefinition;
export type ImportBindingDefinition = scopeManager.ImportBindingDefinition;
export type ParameterDefinition = scopeManager.ParameterDefinition;
export type TSEnumMemberDefinition = scopeManager.TSEnumMemberDefinition;
export type TSEnumNameDefinition = scopeManager.TSEnumNameDefinition;
export type TSModuleNameDefinition = scopeManager.TSModuleNameDefinition;
export type TypeDefinition = scopeManager.TypeDefinition;
export type VariableDefinition = scopeManager.VariableDefinition;
}
export namespace Scopes {
export type BlockScope = scopeManager.BlockScope;
export type CatchScope = scopeManager.CatchScope;
export type ClassScope = scopeManager.ClassScope;
export type ConditionalTypeScope = scopeManager.ConditionalTypeScope;
export type ForScope = scopeManager.ForScope;
export type FunctionExpressionNameScope = scopeManager.FunctionExpressionNameScope;
export type FunctionScope = scopeManager.FunctionScope;
export type FunctionTypeScope = scopeManager.FunctionTypeScope;
export type GlobalScope = scopeManager.GlobalScope;
export type MappedTypeScope = scopeManager.MappedTypeScope;
export type ModuleScope = scopeManager.ModuleScope;
export type SwitchScope = scopeManager.SwitchScope;
export type TSEnumScope = scopeManager.TSEnumScope;
export type TSModuleScope = scopeManager.TSModuleScope;
export type TypeScope = scopeManager.TypeScope;
export type WithScope = scopeManager.WithScope;
}
}

export { Scope };
13 changes: 10 additions & 3 deletions packages/scope-manager/src/referencer/VisitorBase.ts
Expand Up @@ -3,6 +3,7 @@ import { visitorKeys, VisitorKeys } from '@typescript-eslint/visitor-keys';

interface VisitorOptions {
childVisitorKeys?: VisitorKeys | null;
visitChildrenEvenIfSelectorExists?: boolean;
}

function isObject(obj: unknown): obj is Record<string, unknown> {
Expand All @@ -18,8 +19,11 @@ type NodeVisitor = {

abstract class VisitorBase {
readonly #childVisitorKeys: VisitorKeys;
readonly #visitChildrenEvenIfSelectorExists: boolean;
constructor(options: VisitorOptions) {
this.#childVisitorKeys = options.childVisitorKeys ?? visitorKeys;
this.#visitChildrenEvenIfSelectorExists =
options.visitChildrenEvenIfSelectorExists ?? false;
}

/**
Expand All @@ -29,13 +33,13 @@ abstract class VisitorBase {
*/
visitChildren<T extends TSESTree.Node>(
node: T | null | undefined,
excludeArr?: (keyof T)[],
excludeArr: (keyof T)[] = [],
): void {
if (node == null || node.type == null) {
return;
}

const exclude = new Set(excludeArr) as Set<string>;
const exclude = new Set(excludeArr.concat(['parent'])) as Set<string>;
const children = this.#childVisitorKeys[node.type] ?? Object.keys(node);
for (const key of children) {
if (exclude.has(key)) {
Expand Down Expand Up @@ -69,7 +73,10 @@ abstract class VisitorBase {

const visitor = (this as NodeVisitor)[node.type];
if (visitor) {
return visitor.call(this, node);
visitor.call(this, node);
if (!this.#visitChildrenEvenIfSelectorExists) {
return;
}
}

this.visitChildren(node);
Expand Down

0 comments on commit 8062050

Please sign in to comment.