Skip to content

fix(ts-estree): make sure that every node can be converted to tsNode #287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Feb 19, 2019
Merged
7 changes: 5 additions & 2 deletions packages/eslint-plugin/src/rules/promise-function-async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,13 @@ export default util.createRule<Options, MessageIds>({

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ class Test {
return new Promise<void>();
}
}
`
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/227
`export function valid(n: number) { return n; }`,
`export default function invalid(n: number) { return n; }`,
`class Foo { constructor() { } }`
],
invalid: [
{
Expand Down
105 changes: 76 additions & 29 deletions packages/typescript-estree/src/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
canContainDirective,
createError,
findNextToken,
fixExports,
getBinaryExpressionType,
getDeclarationKind,
getLastModifier,
Expand Down Expand Up @@ -111,29 +110,78 @@ export class Converter {
this.allowPattern = allowPattern;
}

let result: TSESTree.BaseNode | null = this.convertNode(
node as TSNode,
parent || node.parent
);
let result = this.convertNode(node as TSNode, parent || node.parent);

if (result && this.options.shouldProvideParserServices) {
this.tsNodeToESTreeNodeMap.set(node, result);
if (
node.kind !== SyntaxKind.ParenthesizedExpression &&
node.kind !== SyntaxKind.ComputedPropertyName
) {
// 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.registerTSNodeInNodeMap(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<T extends TSESTree.ExportDeclaration>(
node: ts.Node,
result: T
): TSESTree.ExportDefaultDeclaration | TSESTree.ExportNamedDeclaration | T {
// check for exports
if (node.modifiers && node.modifiers[0].kind === SyntaxKind.ExportKeyword) {
/**
* Make sure that original node is registered instead of export
*/
this.registerTSNodeInNodeMap(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<TSESTree.ExportDefaultDeclaration>(node, {
type: AST_NODE_TYPES.ExportDefaultDeclaration,
declaration: result,
range: [exportKeyword.getStart(this.ast), result.range[1]]
});
} else {
return this.createNode<TSESTree.ExportNamedDeclaration>(node, {
type: AST_NODE_TYPES.ExportNamedDeclaration,
declaration: result,
specifiers: [],
source: null,
range: [exportKeyword.getStart(this.ast), result.range[1]]
});
}
}

return result;
}

/**
* 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);
}
}
}

/**
* Converts a TypeScript node into an ESTree node.
* @param child the child ts.Node
Expand Down Expand Up @@ -176,6 +224,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;
}

Expand Down Expand Up @@ -410,11 +461,7 @@ export class Converter {
break;
}

if (result && this.options.shouldProvideParserServices) {
this.tsNodeToESTreeNodeMap.set(node, result);
this.esTreeNodeToTSNodeMap.set(result, node);
}

this.registerTSNodeInNodeMap(node, result);
return result;
}

Expand Down Expand Up @@ -715,7 +762,7 @@ export class Converter {
}

// check for exports
return fixExports(node, result, this.ast);
return this.fixExports(node, result);
}

case SyntaxKind.VariableDeclaration: {
Expand Down Expand Up @@ -764,7 +811,7 @@ export class Converter {
}

// check for exports
return fixExports(node, result, this.ast);
return this.fixExports(node, result);
}

// mostly for for-of, for-in
Expand Down Expand Up @@ -1431,7 +1478,7 @@ export class Converter {
}

// check for exports
return fixExports(node, result, this.ast);
return this.fixExports(node, result);
}

// Modules
Expand Down Expand Up @@ -2095,7 +2142,7 @@ export class Converter {
}

// check for exports
return fixExports(node, result, this.ast);
return this.fixExports(node, result);
}

case SyntaxKind.MethodSignature: {
Expand Down Expand Up @@ -2310,7 +2357,7 @@ export class Converter {
result.declare = true;
}
// check for exports
return fixExports(node, result, this.ast);
return this.fixExports(node, result);
}

case SyntaxKind.FirstTypeNode: {
Expand Down Expand Up @@ -2356,7 +2403,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: {
Expand Down Expand Up @@ -2384,7 +2431,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
Expand Down
48 changes: 0 additions & 48 deletions packages/typescript-estree/src/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends TSESTree.BaseNode>(
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 as any,
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),
specifiers: [],
source: null
};
}
}

return result;
}

/**
* Returns the type of a given ts.Token
* @param token the ts.Token
Expand Down
3 changes: 2 additions & 1 deletion packages/typescript-estree/src/ts-estree/ts-estree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ export type Node =
| JSXOpeningFragment
| JSXSpreadAttribute
| JSXSpreadChild
| JSXMemberExpression
| JSXText
| LabeledStatement
| Literal
Expand Down Expand Up @@ -273,7 +274,7 @@ export type ExportDeclaration =
| TSInterfaceDeclaration
| TSModuleDeclaration
| TSTypeAliasDeclaration
| VariableDeclarator;
| VariableDeclaration;
export type Expression =
| ArrowFunctionExpression
| AssignmentExpression
Expand Down
33 changes: 33 additions & 0 deletions packages/typescript-estree/tests/lib/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});