From 02a3137a52c705cf62683b7f17bfadceec0f39b3 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 00:10:51 +0100 Subject: [PATCH 1/8] fix(ts-estree): improve types in fix export --- packages/typescript-estree/src/node-utils.ts | 18 +++++++++--------- .../src/ts-estree/ts-estree.ts | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/typescript-estree/src/node-utils.ts b/packages/typescript-estree/src/node-utils.ts index 9ff8a5c29c1a..af9c41960b7d 100644 --- a/packages/typescript-estree/src/node-utils.ts +++ b/packages/typescript-estree/src/node-utils.ts @@ -454,7 +454,7 @@ export function isOptional(node: { * @param ast the AST * @returns the ESTreeNode with fixed exports */ -export function fixExports( +export function fixExports( node: ts.Node, result: T, ast: ts.SourceFile @@ -470,22 +470,22 @@ export function fixExports( ? findNextToken(nextModifier, ast, ast) : findNextToken(exportKeyword, ast, ast); - result.range![0] = varToken!.getStart(ast); - result.loc = getLocFor(result.range![0], result.range![1], ast); + result.range[0] = varToken!.getStart(ast); + result.loc = getLocFor(result.range[0], result.range[1], ast); if (declarationIsDefault) { return { type: AST_NODE_TYPES.ExportDefaultDeclaration, - declaration: result as any, - range: [exportKeyword.getStart(ast), result.range![1]], - loc: getLocFor(exportKeyword.getStart(ast), result.range![1], ast) + declaration: result, + range: [exportKeyword.getStart(ast), result.range[1]], + loc: getLocFor(exportKeyword.getStart(ast), result.range[1], ast) }; } else { return { type: AST_NODE_TYPES.ExportNamedDeclaration, - declaration: result as any, - range: [exportKeyword.getStart(ast), result.range![1]], - loc: getLocFor(exportKeyword.getStart(ast), result.range![1], ast), + declaration: result, + range: [exportKeyword.getStart(ast), result.range[1]], + loc: getLocFor(exportKeyword.getStart(ast), result.range[1], ast), specifiers: [], source: null }; diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index 57cc7c9016a8..5142842113df 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -273,7 +273,7 @@ export type ExportDeclaration = | TSInterfaceDeclaration | TSModuleDeclaration | TSTypeAliasDeclaration - | VariableDeclarator; + | VariableDeclaration; export type Expression = | ArrowFunctionExpression | AssignmentExpression From 533c03c863d8462cdf4a0204bf26dfe5d7833c64 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 00:49:27 +0100 Subject: [PATCH 2/8] fix(ts-estree): fix issue with registering in maps exported nodes --- .../rules/promise-function-async.test.ts | 5 +- packages/typescript-estree/src/convert.ts | 86 ++++++++++++++----- packages/typescript-estree/src/node-utils.ts | 48 ----------- .../src/ts-estree/ts-estree.ts | 1 + 4 files changed, 69 insertions(+), 71 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index 71f93d669bf3..9fc7399c20a1 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -45,7 +45,10 @@ class Test { return new Promise(); } } - ` + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/227 + `export function valid(n: number) { return n; }`, + `export default function invalid(n: number) { return n; }` ], invalid: [ { diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 38abe7f3272d..b9a63713f4bb 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -10,7 +10,6 @@ import { canContainDirective, createError, findNextToken, - fixExports, getBinaryExpressionType, getDeclarationKind, getLastModifier, @@ -111,27 +110,74 @@ export class Converter { this.allowPattern = allowPattern; } - let result: TSESTree.BaseNode | null = this.convertNode( + let result = this.convertNode( node as TSNode, parent || node.parent ); + this.registerNodeInMaps(node, result); + + this.inTypeMode = typeMode; + this.allowPattern = pattern; + return result; + } + + /** + * Fixes the exports of the given ts.Node + * @param node the ts.Node + * @param result result + * @returns the ESTreeNode with fixed exports + */ + private fixExports( + node: ts.Node, + result: T + ): TSESTree.ExportDefaultDeclaration | TSESTree.ExportNamedDeclaration | T { + // check for exports + if (node.modifiers && node.modifiers[0].kind === SyntaxKind.ExportKeyword) { + this.registerNodeInMaps(node, result); + + const exportKeyword = node.modifiers[0]; + const nextModifier = node.modifiers[1]; + const declarationIsDefault = + nextModifier && nextModifier.kind === SyntaxKind.DefaultKeyword; + + const varToken = declarationIsDefault + ? findNextToken(nextModifier, this.ast, this.ast) + : findNextToken(exportKeyword, this.ast, this.ast); + + result.range[0] = varToken!.getStart(this.ast); + result.loc = getLocFor(result.range[0], result.range[1], this.ast); + + if (declarationIsDefault) { + return this.createNode(node, { + type: AST_NODE_TYPES.ExportDefaultDeclaration, + declaration: result, + range: [exportKeyword.getStart(this.ast), result.range[1]] + }); + } else { + return this.createNode(node, { + type: AST_NODE_TYPES.ExportNamedDeclaration, + declaration: result, + specifiers: [], + source: null, + range: [exportKeyword.getStart(this.ast), result.range[1]] + }); + } + } + + return result; + } + + private registerNodeInMaps(node: ts.Node, result: TSESTree.BaseNode | null) { if (result && this.options.shouldProvideParserServices) { this.tsNodeToESTreeNodeMap.set(node, result); - if ( - node.kind !== SyntaxKind.ParenthesizedExpression && - node.kind !== SyntaxKind.ComputedPropertyName - ) { + if (!this.esTreeNodeToTSNodeMap.has(result)) { // Parenthesized expressions and computed property names do not have individual nodes in ESTree. // Therefore, result.type will never "match" node.kind if it is a ParenthesizedExpression // or a ComputedPropertyName and, furthermore, will overwrite the "matching" node this.esTreeNodeToTSNodeMap.set(result, node); } } - - this.inTypeMode = typeMode; - this.allowPattern = pattern; - return result; } /** @@ -410,11 +456,7 @@ export class Converter { break; } - if (result && this.options.shouldProvideParserServices) { - this.tsNodeToESTreeNodeMap.set(node, result); - this.esTreeNodeToTSNodeMap.set(result, node); - } - + this.registerNodeInMaps(node, result); return result; } @@ -715,7 +757,7 @@ export class Converter { } // check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } case SyntaxKind.VariableDeclaration: { @@ -764,7 +806,7 @@ export class Converter { } // check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } // mostly for for-of, for-in @@ -1431,7 +1473,7 @@ export class Converter { } // check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } // Modules @@ -2098,7 +2140,7 @@ export class Converter { } // check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } case SyntaxKind.MethodSignature: { @@ -2313,7 +2355,7 @@ export class Converter { result.declare = true; } // check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } case SyntaxKind.FirstTypeNode: { @@ -2359,7 +2401,7 @@ export class Converter { result.decorators = node.decorators.map(el => this.convertChild(el)); } // ...then check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } case SyntaxKind.EnumMember: { @@ -2387,7 +2429,7 @@ export class Converter { result.global = true; } // ...then check for exports - return fixExports(node, result, this.ast); + return this.fixExports(node, result); } // TypeScript specific types diff --git a/packages/typescript-estree/src/node-utils.ts b/packages/typescript-estree/src/node-utils.ts index af9c41960b7d..bb03b99344a6 100644 --- a/packages/typescript-estree/src/node-utils.ts +++ b/packages/typescript-estree/src/node-utils.ts @@ -447,54 +447,6 @@ export function isOptional(node: { : false; } -/** - * Fixes the exports of the given ts.Node - * @param node the ts.Node - * @param result result - * @param ast the AST - * @returns the ESTreeNode with fixed exports - */ -export function fixExports( - node: ts.Node, - result: T, - ast: ts.SourceFile -): TSESTree.ExportDefaultDeclaration | TSESTree.ExportNamedDeclaration | T { - // check for exports - if (node.modifiers && node.modifiers[0].kind === SyntaxKind.ExportKeyword) { - const exportKeyword = node.modifiers[0]; - const nextModifier = node.modifiers[1]; - const declarationIsDefault = - nextModifier && nextModifier.kind === SyntaxKind.DefaultKeyword; - - const varToken = declarationIsDefault - ? findNextToken(nextModifier, ast, ast) - : findNextToken(exportKeyword, ast, ast); - - result.range[0] = varToken!.getStart(ast); - result.loc = getLocFor(result.range[0], result.range[1], ast); - - if (declarationIsDefault) { - return { - type: AST_NODE_TYPES.ExportDefaultDeclaration, - declaration: result, - range: [exportKeyword.getStart(ast), result.range[1]], - loc: getLocFor(exportKeyword.getStart(ast), result.range[1], ast) - }; - } else { - return { - type: AST_NODE_TYPES.ExportNamedDeclaration, - declaration: result, - range: [exportKeyword.getStart(ast), result.range[1]], - loc: getLocFor(exportKeyword.getStart(ast), result.range[1], ast), - specifiers: [], - source: null - }; - } - } - - return result; -} - /** * Returns the type of a given ts.Token * @param token the ts.Token diff --git a/packages/typescript-estree/src/ts-estree/ts-estree.ts b/packages/typescript-estree/src/ts-estree/ts-estree.ts index 5142842113df..4a9c7b91ae10 100644 --- a/packages/typescript-estree/src/ts-estree/ts-estree.ts +++ b/packages/typescript-estree/src/ts-estree/ts-estree.ts @@ -127,6 +127,7 @@ export type Node = | JSXOpeningFragment | JSXSpreadAttribute | JSXSpreadChild + | JSXMemberExpression | JSXText | LabeledStatement | Literal From e6cfa76a526c1f8ee71626f97aa55acb581e451c Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 01:14:04 +0100 Subject: [PATCH 3/8] fix(ts-estree): register node to tsNodeToESTreeNodeMap only once --- packages/typescript-estree/src/convert.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index b9a63713f4bb..2c73d75294bc 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -110,10 +110,7 @@ export class Converter { this.allowPattern = allowPattern; } - let result = this.convertNode( - node as TSNode, - parent || node.parent - ); + let result = this.convertNode(node as TSNode, parent || node.parent); this.registerNodeInMaps(node, result); @@ -170,7 +167,9 @@ export class Converter { private registerNodeInMaps(node: ts.Node, result: TSESTree.BaseNode | null) { if (result && this.options.shouldProvideParserServices) { - this.tsNodeToESTreeNodeMap.set(node, result); + if (!this.tsNodeToESTreeNodeMap.has(result)) { + this.tsNodeToESTreeNodeMap.set(node, result); + } if (!this.esTreeNodeToTSNodeMap.has(result)) { // Parenthesized expressions and computed property names do not have individual nodes in ESTree. // Therefore, result.type will never "match" node.kind if it is a ParenthesizedExpression From bbc4b21599a7a38375ca90e57c5bf95be2dc9f63 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 01:14:57 +0100 Subject: [PATCH 4/8] fix(eslint-plugin): fix crash in promise-function-async --- packages/eslint-plugin/src/rules/promise-function-async.ts | 7 +++++-- .../tests/rules/promise-function-async.test.ts | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts index e2e65cfd1b6e..cf78f6c36ff3 100644 --- a/packages/eslint-plugin/src/rules/promise-function-async.ts +++ b/packages/eslint-plugin/src/rules/promise-function-async.ts @@ -92,10 +92,13 @@ export default util.createRule({ function validateNode(node: TSESTree.Node) { const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); - const [callSignature] = checker + const signatures = checker .getTypeAtLocation(originalNode) .getCallSignatures(); - const returnType = checker.getReturnTypeOfSignature(callSignature); + if (!signatures.length) { + return; + } + const returnType = checker.getReturnTypeOfSignature(signatures[0]); if (!util.containsTypeByName(returnType, allAllowedPromiseNames)) { return; diff --git a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts index 9fc7399c20a1..fd1f5a90bfb1 100644 --- a/packages/eslint-plugin/tests/rules/promise-function-async.test.ts +++ b/packages/eslint-plugin/tests/rules/promise-function-async.test.ts @@ -48,7 +48,8 @@ class Test { `, // https://github.com/typescript-eslint/typescript-eslint/issues/227 `export function valid(n: number) { return n; }`, - `export default function invalid(n: number) { return n; }` + `export default function invalid(n: number) { return n; }`, + `class Foo { constructor() { } }` ], invalid: [ { From cbb98585bdcee39466df89caaf8fec7464f482c9 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 01:20:34 +0100 Subject: [PATCH 5/8] fix(ts-estree): make sure that every node can be translated to tsNode --- packages/typescript-estree/src/convert.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 2c73d75294bc..4f31a0151913 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -167,15 +167,9 @@ export class Converter { private registerNodeInMaps(node: ts.Node, result: TSESTree.BaseNode | null) { if (result && this.options.shouldProvideParserServices) { - if (!this.tsNodeToESTreeNodeMap.has(result)) { + if (!this.tsNodeToESTreeNodeMap.has(node)) { this.tsNodeToESTreeNodeMap.set(node, result); } - if (!this.esTreeNodeToTSNodeMap.has(result)) { - // Parenthesized expressions and computed property names do not have individual nodes in ESTree. - // Therefore, result.type will never "match" node.kind if it is a ParenthesizedExpression - // or a ComputedPropertyName and, furthermore, will overwrite the "matching" node - this.esTreeNodeToTSNodeMap.set(result, node); - } } } @@ -221,6 +215,9 @@ export class Converter { result.loc = getLocFor(result.range[0], result.range[1], this.ast); } + if (result && this.options.shouldProvideParserServices) { + this.esTreeNodeToTSNodeMap.set(result, node); + } return result as T; } From 1867b9d2e5e12391b6994737f28719e8fda66090 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 01:33:08 +0100 Subject: [PATCH 6/8] chore(ts-estree): add missing function description --- packages/typescript-estree/src/convert.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 4f31a0151913..8a0389233f79 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -112,7 +112,7 @@ export class Converter { let result = this.convertNode(node as TSNode, parent || node.parent); - this.registerNodeInMaps(node, result); + this.registerTSNodeInNodeMap(node, result); this.inTypeMode = typeMode; this.allowPattern = pattern; @@ -131,7 +131,10 @@ export class Converter { ): TSESTree.ExportDefaultDeclaration | TSESTree.ExportNamedDeclaration | T { // check for exports if (node.modifiers && node.modifiers[0].kind === SyntaxKind.ExportKeyword) { - this.registerNodeInMaps(node, result); + /** + * Make sure that original node is registered instead of export + */ + this.registerTSNodeInNodeMap(node, result); const exportKeyword = node.modifiers[0]; const nextModifier = node.modifiers[1]; @@ -165,7 +168,10 @@ export class Converter { return result; } - private registerNodeInMaps(node: ts.Node, result: TSESTree.BaseNode | null) { + /** + * Register specific TypeScript node into map with first ESTree node provided + */ + private registerTSNodeInNodeMap(node: ts.Node, result: TSESTree.BaseNode | null) { if (result && this.options.shouldProvideParserServices) { if (!this.tsNodeToESTreeNodeMap.has(node)) { this.tsNodeToESTreeNodeMap.set(node, result); @@ -452,7 +458,7 @@ export class Converter { break; } - this.registerNodeInMaps(node, result); + this.registerTSNodeInNodeMap(node, result); return result; } From c4cd0af66aa113a020dd3bd4be36a9448defa63f Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 01:44:36 +0100 Subject: [PATCH 7/8] test(ts-estree): add test case for esTreeNodeToTSNodeMap --- .../typescript-estree/tests/lib/convert.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/typescript-estree/tests/lib/convert.ts b/packages/typescript-estree/tests/lib/convert.ts index 6fb1077a4714..ce9cf35fa0e8 100644 --- a/packages/typescript-estree/tests/lib/convert.ts +++ b/packages/typescript-estree/tests/lib/convert.ts @@ -149,4 +149,37 @@ describe('convert', () => { ); checkMaps(ast); }); + + it('nodeMaps should contain export node', () => { + const ast = convertCode(`export function foo () {}`); + + const instance = new Converter(ast, { + errorOnUnknownASTType: false, + useJSXTextNode: false, + shouldProvideParserServices: true + }); + const program = instance.convertProgram(); + const maps = instance.getASTMaps(); + + function checkMaps(child: any) { + child.forEachChild((node: any) => { + if (node.kind !== ts.SyntaxKind.EndOfFileToken) { + expect(ast).toBe( + maps.esTreeNodeToTSNodeMap.get(maps.tsNodeToESTreeNodeMap.get(ast)) + ); + } + checkMaps(node); + }); + } + + expect(ast).toBe( + maps.esTreeNodeToTSNodeMap.get(maps.tsNodeToESTreeNodeMap.get(ast)) + ); + + expect(maps.esTreeNodeToTSNodeMap.get(program.body[0])).toBeDefined(); + expect(program.body[0]).not.toBe( + maps.tsNodeToESTreeNodeMap.get(ast.statements[0]) + ); + checkMaps(ast); + }); }); From 69970f79799b055e10076e34b2555c2a18ecbba2 Mon Sep 17 00:00:00 2001 From: Armano Date: Fri, 15 Feb 2019 01:52:48 +0100 Subject: [PATCH 8/8] chore(ts-estree): fix code formatting --- packages/typescript-estree/src/convert.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 8a0389233f79..e457884f3e44 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -171,7 +171,10 @@ export class Converter { /** * Register specific TypeScript node into map with first ESTree node provided */ - private registerTSNodeInNodeMap(node: ts.Node, result: TSESTree.BaseNode | null) { + private registerTSNodeInNodeMap( + node: ts.Node, + result: TSESTree.BaseNode | null + ) { if (result && this.options.shouldProvideParserServices) { if (!this.tsNodeToESTreeNodeMap.has(node)) { this.tsNodeToESTreeNodeMap.set(node, result);