From 580eceb83183ddc7a4798eb5bc55a36f71dcda89 Mon Sep 17 00:00:00 2001 From: Ben Lichtman Date: Mon, 19 Aug 2019 01:50:35 -0700 Subject: [PATCH] fix(eslint-plugin): [unified-signatures] type comparison and exported nodes (#839) --- .gitignore | 3 + .../src/rules/unified-signatures.ts | 38 +++++++++--- .../tests/rules/unified-signatures.test.ts | 58 +++++++++++++++++++ .../src/ts-estree/ts-estree.ts | 1 + 4 files changed, 93 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 55599f0c184..3a06baccf6f 100644 --- a/.gitignore +++ b/.gitignore @@ -60,3 +60,6 @@ jspm_packages/ .DS_Store .idea dist + +# Editor-specific metadata folders +.vs diff --git a/packages/eslint-plugin/src/rules/unified-signatures.ts b/packages/eslint-plugin/src/rules/unified-signatures.ts index 1c3f747bc51..c4c7f9bf427 100644 --- a/packages/eslint-plugin/src/rules/unified-signatures.ts +++ b/packages/eslint-plugin/src/rules/unified-signatures.ts @@ -35,6 +35,9 @@ type ScopeNode = | TSESTree.TSTypeLiteral; type OverloadNode = MethodDefinition | SignatureDefinition; +type ContainingNode = + | TSESTree.ExportNamedDeclaration + | TSESTree.ExportDefaultDeclaration; type SignatureDefinition = | TSESTree.FunctionExpression @@ -424,7 +427,8 @@ export default util.createRule({ a === b || (a !== undefined && b !== undefined && - a.typeAnnotation.type === b.typeAnnotation.type) + sourceCode.getText(a.typeAnnotation) === + sourceCode.getText(b.typeAnnotation)) ); } @@ -495,9 +499,16 @@ export default util.createRule({ currentScope = scopes.pop()!; } - function addOverload(signature: OverloadNode, key?: string): void { + function addOverload( + signature: OverloadNode, + key?: string, + containingNode?: ContainingNode, + ): void { key = key || getOverloadKey(signature); - if (currentScope && signature.parent === currentScope.parent && key) { + if ( + currentScope && + (containingNode || signature).parent === currentScope.parent + ) { const overloads = currentScope.overloads.get(key); if (overloads !== undefined) { overloads.push(signature); @@ -521,11 +532,10 @@ export default util.createRule({ createScope(node.body, node.typeParameters); }, TSTypeLiteral: createScope, + // collect overloads TSDeclareFunction(node): void { - if (node.id && !node.body) { - addOverload(node, node.id.name); - } + addOverload(node, node.id.name, getExportingNode(node)); }, TSCallSignatureDeclaration: addOverload, TSConstructSignatureDeclaration: addOverload, @@ -540,6 +550,7 @@ export default util.createRule({ addOverload(node); } }, + // validate scopes 'Program:exit': checkScope, 'TSModuleBlock:exit': checkScope, @@ -550,7 +561,20 @@ export default util.createRule({ }, }); -function getOverloadKey(node: OverloadNode): string | undefined { +function getExportingNode( + node: TSESTree.TSDeclareFunction, +): + | TSESTree.ExportNamedDeclaration + | TSESTree.ExportDefaultDeclaration + | undefined { + return node.parent && + (node.parent.type === AST_NODE_TYPES.ExportNamedDeclaration || + node.parent.type === AST_NODE_TYPES.ExportDefaultDeclaration) + ? node.parent + : undefined; +} + +function getOverloadKey(node: OverloadNode): string { const info = getOverloadInfo(node); return ( diff --git a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts index 5d65e2f1801..c8f9740662d 100644 --- a/packages/eslint-plugin/tests/rules/unified-signatures.test.ts +++ b/packages/eslint-plugin/tests/rules/unified-signatures.test.ts @@ -105,6 +105,30 @@ interface I { function f(x: T[]): void; function f(x: T): void; `, + // Same name, different scopes + ` +declare function foo(n: number): number; + +declare module "hello" { + function foo(n: number, s: string): number; +} +`, + // children of block not checked to match TSLint + ` +{ + function block(): number; + function block(n: number): number; + function block(n?: number): number { + return 3; + } +} +`, + ` +export interface Foo { + bar(baz: string): number[]; + bar(): string[]; +} +`, ], invalid: [ { @@ -591,5 +615,39 @@ class Foo { }, ], }, + { + code: ` +export function foo(line: number): number; +export function foo(line: number, character?: number): number; +`, + errors: [ + { + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, + line: 3, + column: 35, + }, + ], + }, + { + code: ` +declare function foo(line: number): number; +export function foo(line: number, character?: number): number; +`, + errors: [ + { + messageId: 'omittingSingleParameter', + data: { + failureStringStart: + 'These overloads can be combined into one signature', + }, + line: 3, + column: 35, + }, + ], + }, ], }); diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index f079fd68d47..7276f8d93e3 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -1040,6 +1040,7 @@ export interface TSConstructSignatureDeclaration extends FunctionSignatureBase { } export interface TSDeclareFunction extends FunctionDeclarationBase { + id: Identifier; type: AST_NODE_TYPES.TSDeclareFunction; }