-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(eslint-plugin): add 'no-unnecessary-qualifier' rule (#231)
* feat(eslint-plugin): add no-unnecessary-qualifier rule * docs: add documentation * docs: update README and ROADMAP * test: increase code coverage * docs: add simple correct usage of qualifiers * chore: migrate to ts * refactor: use expressive selector
- Loading branch information
1 parent
b50a68b
commit cc8f906
Showing
7 changed files
with
522 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
79 changes: 79 additions & 0 deletions
79
packages/eslint-plugin/docs/rules/no-unnecessary-qualifier.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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/) |
208 changes: 208 additions & 0 deletions
208
packages/eslint-plugin/src/rules/no-unnecessary-qualifier.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| }; | ||
| } | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export type T = number; |
Oops, something went wrong.