diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index daacab20afdb..79fdeb0fb969 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -128,7 +128,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-empty-interface`](./docs/rules/no-empty-interface.md) | Disallow the declaration of empty interfaces (`no-empty-interface` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-explicit-any`](./docs/rules/no-explicit-any.md) | Disallow usage of the `any` type (`no-any` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces (`no-unnecessary-class` from TSLint) | | | -| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop (`no-for-in-array` from TSLint) | | | +| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop (`no-for-in-array` from TSLint) | | | | [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean. (`no-inferrable-types` from TSLint) | :heavy_check_mark: | :wrench: | | [`@typescript-eslint/no-misused-new`](./docs/rules/no-misused-new.md) | Enforce valid definition of `new` and `constructor`. (`no-misused-new` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-namespace`](./docs/rules/no-namespace.md) | Disallow the use of custom TypeScript modules and namespaces (`no-namespace` from TSLint) | :heavy_check_mark: | | @@ -139,6 +139,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` (`no-this-assignment` from TSLint) | | | | [`@typescript-eslint/no-triple-slash-reference`](./docs/rules/no-triple-slash-reference.md) | Disallow `/// ` comments (`no-reference` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases (`interface-over-type-literal` from TSLint) | | | +| [`@typescript-eslint/no-unnecessary-qualifier`](./docs/rules/no-unnecessary-qualifier.md) | Warns when a namespace qualifier is unnecessary (`no-unnecessary-qualifier` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unnecessary-type-assertion`](./docs/rules/no-unnecessary-type-assertion.md) | Warns if a type assertion does not change the type of an expression (`no-unnecessary-type-assertion` from TSLint) | | :wrench: | | [`@typescript-eslint/no-unused-vars`](./docs/rules/no-unused-vars.md) | Disallow unused variables (`no-unused-variable` from TSLint) | :heavy_check_mark: | | | [`@typescript-eslint/no-use-before-define`](./docs/rules/no-use-before-define.md) | Disallow the use of variables before they are defined | :heavy_check_mark: | | diff --git a/packages/eslint-plugin/ROADMAP.md b/packages/eslint-plugin/ROADMAP.md index 9b7beda941b4..b99637b9057f 100644 --- a/packages/eslint-plugin/ROADMAP.md +++ b/packages/eslint-plugin/ROADMAP.md @@ -163,7 +163,7 @@ | [`no-trailing-whitespace`] | 🌟 | [`no-trailing-spaces`][no-trailing-spaces] | | [`no-unnecessary-callback-wrapper`] | 🛑 | N/A and this might be unsafe (i.e. with `forEach`) | | [`no-unnecessary-initializer`] | 🌟 | [`no-undef-init`][no-undef-init] | -| [`no-unnecessary-qualifier`] | 🛑 | N/A | +| [`no-unnecessary-qualifier`] | ✅ | [`@typescript-eslint/no-unnecessary-qualifier`][no-unnecessary-qualifier] | | [`number-literal-format`] | 🛑 | N/A | | [`object-literal-key-quotes`] | 🌟 | [`quote-props`][quote-props] | | [`object-literal-shorthand`] | 🌟 | [`object-shorthand`][object-shorthand] | @@ -606,6 +606,7 @@ Relevant plugins: [`chai-expect-keywords`](https://github.com/gavinaiken/eslint- [`@typescript-eslint/no-array-constructor`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-array-constructor.md [`@typescript-eslint/prefer-function-type`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/prefer-function-type.md [`@typescript-eslint/no-for-in-array`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-for-in-array.md +[`@typescript-eslint/no-unnecessary-qualifier`]: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md diff --git a/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md b/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md new file mode 100644 index 000000000000..2f356e0eac3a --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md @@ -0,0 +1,79 @@ +# Warns when a namespace qualifier is unnecessary. (no-unnecessary-qualifier) + +## Rule Details + +This rule aims to let users know when a namespace or enum qualifier is unnecessary, +whether used for a type or for a value. + +Examples of **incorrect** code for this rule: + +```ts +namespace A { + export type B = number; + const x: A.B = 3; +} +``` + +```ts +namespace A { + export const x = 3; + export const y = A.x; +} +``` + +```ts +enum A { + B, + C = A.B +} +``` + +```ts +namespace A { + export namespace B { + export type T = number; + const x: A.B.T = 3; + } +} +``` + +Examples of **correct** code for this rule: + +```ts +namespace X { + export type T = number; +} + +namespace Y { + export const x: X.T = 3; +} +``` + +```ts +enum A { + X, + Y +} + +enum B { + Z = A.X +} +``` + +```ts +namespace X { + export type T = number; + namespace Y { + type T = string; + const x: X.T = 0; + } +} +``` + +## When Not To Use It + +If you don't care about having unneeded namespace or enum qualifiers, then you don't need to use this rule. + +## Further Reading + +- TSLint: [no-unnecessary-qualifier](https://palantir.github.io/tslint/rules/no-unnecessary-qualifier/) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-qualifier.ts b/packages/eslint-plugin/src/rules/no-unnecessary-qualifier.ts new file mode 100644 index 000000000000..f13be913cbb3 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-unnecessary-qualifier.ts @@ -0,0 +1,208 @@ +/** + * @fileoverview Warns when a namespace qualifier is unnecessary. + * @author Benjamin Lichtman + */ + +import { TSESTree } from '@typescript-eslint/typescript-estree'; +import ts from 'typescript'; +import * as tsutils from 'tsutils'; +import * as util from '../util'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +export default util.createRule({ + name: 'no-unnecessary-qualifier', + meta: { + docs: { + category: 'Best Practices', + description: 'Warns when a namespace qualifier is unnecessary.', + recommended: false, + tslintName: 'no-unnecessary-qualifier' + }, + fixable: 'code', + messages: { + unnecessaryQualifier: + "Qualifier is unnecessary since '{{ name }}' is in scope." + }, + schema: [], + type: 'suggestion' + }, + defaultOptions: [], + create(context) { + const namespacesInScope: ts.Node[] = []; + let currentFailedNamespaceExpression: TSESTree.Node | null = null; + const parserServices = util.getParserServices(context); + const esTreeNodeToTSNodeMap = parserServices.esTreeNodeToTSNodeMap; + const program = parserServices.program; + const checker = program.getTypeChecker(); + const sourceCode = context.getSourceCode(); + + //---------------------------------------------------------------------- + // Helpers + //---------------------------------------------------------------------- + + function tryGetAliasedSymbol( + symbol: ts.Symbol, + checker: ts.TypeChecker + ): ts.Symbol | null { + return tsutils.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias) + ? checker.getAliasedSymbol(symbol) + : null; + } + + function symbolIsNamespaceInScope(symbol: ts.Symbol): boolean { + const symbolDeclarations = symbol.getDeclarations() || []; + + if ( + symbolDeclarations.some(decl => + namespacesInScope.some(ns => ns === decl) + ) + ) { + return true; + } + + const alias = tryGetAliasedSymbol(symbol, checker); + + return alias !== null && symbolIsNamespaceInScope(alias); + } + + function getSymbolInScope( + node: ts.Node, + flags: ts.SymbolFlags, + name: string + ): ts.Symbol | undefined { + // TODO:PERF `getSymbolsInScope` gets a long list. Is there a better way? + const scope = checker.getSymbolsInScope(node, flags); + + return scope.find(scopeSymbol => scopeSymbol.name === name); + } + + function symbolsAreEqual(accessed: ts.Symbol, inScope: ts.Symbol): boolean { + return accessed === checker.getExportSymbolOfSymbol(inScope); + } + + function qualifierIsUnnecessary( + qualifier: TSESTree.Node, + name: TSESTree.Identifier + ): boolean { + const tsQualifier = esTreeNodeToTSNodeMap.get(qualifier); + const tsName = esTreeNodeToTSNodeMap.get(name); + + if (!(tsQualifier && tsName)) return false; // TODO: throw error? + + const namespaceSymbol = checker.getSymbolAtLocation(tsQualifier); + + if ( + typeof namespaceSymbol === 'undefined' || + !symbolIsNamespaceInScope(namespaceSymbol) + ) { + return false; + } + + const accessedSymbol = checker.getSymbolAtLocation(tsName); + + if (typeof accessedSymbol === 'undefined') { + return false; + } + + // If the symbol in scope is different, the qualifier is necessary. + const fromScope = getSymbolInScope( + tsQualifier, + accessedSymbol.flags, + sourceCode.getText(name) + ); + + return ( + typeof fromScope === 'undefined' || + symbolsAreEqual(accessedSymbol, fromScope) + ); + } + + function visitNamespaceAccess( + node: TSESTree.Node, + qualifier: TSESTree.Node, + name: TSESTree.Identifier + ): void { + // Only look for nested qualifier errors if we didn't already fail on the outer qualifier. + if ( + !currentFailedNamespaceExpression && + qualifierIsUnnecessary(qualifier, name) + ) { + currentFailedNamespaceExpression = node; + context.report({ + node: qualifier, + messageId: 'unnecessaryQualifier', + data: { + name: sourceCode.getText(name) + }, + fix(fixer) { + return fixer.removeRange([qualifier.range[0], name.range[0]]); + } + }); + } + } + + function enterDeclaration(node: TSESTree.Node): void { + const tsDeclaration = esTreeNodeToTSNodeMap.get(node); + if (tsDeclaration) { + namespacesInScope.push(tsDeclaration); + } + } + + function exitDeclaration(node: TSESTree.Node) { + if (esTreeNodeToTSNodeMap.has(node)) { + namespacesInScope.pop(); + } + } + + function resetCurrentNamespaceExpression(node: TSESTree.Node): void { + if (node === currentFailedNamespaceExpression) { + currentFailedNamespaceExpression = null; + } + } + + function isPropertyAccessExpression( + node: TSESTree.Node + ): node is TSESTree.MemberExpression { + return node.type === 'MemberExpression' && !node.computed; + } + + function isEntityNameExpression(node: TSESTree.Node): boolean { + return ( + node.type === 'Identifier' || + (isPropertyAccessExpression(node) && + isEntityNameExpression(node.object)) + ); + } + + //---------------------------------------------------------------------- + // Public + //---------------------------------------------------------------------- + + return { + TSModuleDeclaration: enterDeclaration, + TSEnumDeclaration: enterDeclaration, + 'ExportNamedDeclaration[declaration.type="TSModuleDeclaration"]': enterDeclaration, + 'ExportNamedDeclaration[declaration.type="TSEnumDeclaration"]': enterDeclaration, + 'TSModuleDeclaration:exit': exitDeclaration, + 'TSEnumDeclaration:exit': exitDeclaration, + 'ExportNamedDeclaration[declaration.type="TSModuleDeclaration"]:exit': exitDeclaration, + 'ExportNamedDeclaration[declaration.type="TSEnumDeclaration"]:exit': exitDeclaration, + TSQualifiedName(node: TSESTree.TSQualifiedName): void { + visitNamespaceAccess(node, node.left, node.right); + }, + 'MemberExpression[computed=false]': function( + node: TSESTree.MemberExpression + ): void { + const property = node.property as TSESTree.Identifier; + if (isEntityNameExpression(node.object)) { + visitNamespaceAccess(node, node.object, property); + } + }, + 'TSQualifiedName:exit': resetCurrentNamespaceExpression, + 'MemberExpression:exit': resetCurrentNamespaceExpression + }; + } +}); diff --git a/packages/eslint-plugin/tests/fixtures/foo.ts b/packages/eslint-plugin/tests/fixtures/foo.ts new file mode 100644 index 000000000000..fda4aa0a1c50 --- /dev/null +++ b/packages/eslint-plugin/tests/fixtures/foo.ts @@ -0,0 +1 @@ +export type T = number; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts new file mode 100644 index 000000000000..3e9587bb6c79 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-qualifier.test.ts @@ -0,0 +1,229 @@ +/** + * @fileoverview Warns when a namespace qualifier is unnecessary. + * @author Benjamin Lichtman + */ + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +import path from 'path'; +import rule from '../../src/rules/no-unnecessary-qualifier'; +import { RuleTester } from '../RuleTester'; +import { AST_NODE_TYPES } from '@typescript-eslint/typescript-estree'; + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +const messageId = 'unnecessaryQualifier'; +const rootPath = path.join(process.cwd(), 'tests/fixtures/'); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + sourceType: 'module', + ecmaVersion: 6 + } +}); + +ruleTester.run('no-unnecessary-qualifier', rule, { + valid: [ + ` +namespace X { + export type T = number; +} + +namespace Y { + export const x: X.T = 3; +}`, + ` +enum A { + X, + Y +} + +enum B { + Z = A.X +}`, + ` +namespace X { + export type T = number; + namespace Y { + type T = string; + const x: X.T = 0; + } +}`, + `const x: A.B = 3;`, + ` +namespace X { + const z = X.y; +}` + ], + + invalid: [ + { + code: ` +namespace A { + export type B = number; + const x: A.B = 3; +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.Identifier + } + ], + output: ` +namespace A { + export type B = number; + const x: B = 3; +}` + }, + { + code: ` +namespace A { + export const x = 3; + export const y = A.x; +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.Identifier + } + ], + output: ` +namespace A { + export const x = 3; + export const y = x; +}` + }, + { + code: ` +namespace A { + export type T = number; + export namespace B { + const x: A.T = 3; + } +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.Identifier + } + ], + output: ` +namespace A { + export type T = number; + export namespace B { + const x: T = 3; + } +}` + }, + { + code: ` +namespace A { + export namespace B { + export type T = number; + const x: A.B.T = 3; + } +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.TSQualifiedName + } + ], + output: ` +namespace A { + export namespace B { + export type T = number; + const x: T = 3; + } +}` + }, + { + code: ` +namespace A { + export namespace B { + export const x = 3; + const y = A.B.x; + } +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.MemberExpression + } + ], + output: ` +namespace A { + export namespace B { + export const x = 3; + const y = x; + } +}` + }, + { + code: ` +enum A { + B, + C = A.B +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.Identifier + } + ], + output: ` +enum A { + B, + C = B +}` + }, + { + code: ` +namespace Foo { + export enum A { + B, + C = Foo.A.B + } +}`, + errors: [ + { + messageId, + type: AST_NODE_TYPES.MemberExpression + } + ], + output: ` +namespace Foo { + export enum A { + B, + C = B + } +}` + }, + { + code: ` +import * as Foo from './foo'; +declare module './foo' { + const x: Foo.T = 3; +}`, + filename: path.join(rootPath, 'bar.ts'), + errors: [ + { + messageId, + type: AST_NODE_TYPES.Identifier + } + ], + output: ` +import * as Foo from './foo'; +declare module './foo' { + const x: T = 3; +}` + } + ] +}); diff --git a/packages/typescript-estree/src/parser-options.ts b/packages/typescript-estree/src/parser-options.ts index 8a30b27b89ff..9dadb456483b 100644 --- a/packages/typescript-estree/src/parser-options.ts +++ b/packages/typescript-estree/src/parser-options.ts @@ -38,6 +38,7 @@ export interface ParserOptions { export interface ParserWeakMap { get(key: TKey): TValue; + has(key: any): boolean; } export interface ParserServices {