From cbb1d9f2bb1e6a084451d73ed7d2ee8dfcca7920 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Mon, 11 Nov 2019 16:16:56 -0800 Subject: [PATCH] fix(compiler-cli): Refactor getTsTypeFromBuiltinType (#33778) This commit fixes a few issues with helper method `getBuiltInTypeFromTsType`. 1. The function is wrongly named. It should be the other way round. 2. The ts.Type returned by the function should not contain any value. This is because for some data types like Number and String, the SourceFile (context.node) is not the correct value. Value is never needed for program correctness in this case. PR Close #33778 --- .../src/typescript_symbols.ts | 67 ++++++------------- .../test/typescript_symbols_spec.ts | 6 +- 2 files changed, 24 insertions(+), 49 deletions(-) diff --git a/packages/language-service/src/typescript_symbols.ts b/packages/language-service/src/typescript_symbols.ts index 34f373c1d7..6635ee5a18 100644 --- a/packages/language-service/src/typescript_symbols.ts +++ b/packages/language-service/src/typescript_symbols.ts @@ -94,8 +94,11 @@ class TypeScriptSymbolQuery implements SymbolQuery { getBuiltinType(kind: BuiltinType): Symbol { let result = this.typeCache.get(kind); if (!result) { - const type = getBuiltinTypeFromTs( - kind, {checker: this.checker, node: this.source, program: this.program}); + const type = getTsTypeFromBuiltinType(kind, { + checker: this.checker, + node: this.source, + program: this.program, + }); result = new TypeWrapper(type, {program: this.program, checker: this.checker, node: this.source}); this.typeCache.set(kind, result); @@ -553,7 +556,7 @@ class PipeSymbol implements Symbol { resultType = getTypeParameterOf(parameterType.tsType, parameterType.name); break; default: - resultType = getBuiltinTypeFromTs(BuiltinType.Any, this.context); + resultType = getTsTypeFromBuiltinType(BuiltinType.Any, this.context); break; } break; @@ -580,7 +583,7 @@ class PipeSymbol implements Symbol { type = this._tsType = this.findTransformMethodType(classSymbol) !; } if (!type) { - type = this._tsType = getBuiltinTypeFromTs(BuiltinType.Any, this.context); + type = this._tsType = getTsTypeFromBuiltinType(BuiltinType.Any, this.context); } } return type; @@ -633,62 +636,34 @@ function isSymbolPrivate(s: ts.Symbol): boolean { return !!s.valueDeclaration && isPrivate(s.valueDeclaration); } -function getBuiltinTypeFromTs(kind: BuiltinType, context: TypeContext): ts.Type { - let type: ts.Type; - const checker = context.checker; - const node = context.node; - switch (kind) { +function getTsTypeFromBuiltinType(builtinType: BuiltinType, ctx: TypeContext): ts.Type { + let syntaxKind: ts.SyntaxKind; + switch (builtinType) { case BuiltinType.Any: - type = checker.getTypeAtLocation(setParents( - { - kind: ts.SyntaxKind.AsExpression, - expression: {kind: ts.SyntaxKind.TrueKeyword}, - type: {kind: ts.SyntaxKind.AnyKeyword} - }, - node)); + syntaxKind = ts.SyntaxKind.AnyKeyword; break; case BuiltinType.Boolean: - type = - checker.getTypeAtLocation(setParents({kind: ts.SyntaxKind.TrueKeyword}, node)); + syntaxKind = ts.SyntaxKind.BooleanKeyword; break; case BuiltinType.Null: - type = - checker.getTypeAtLocation(setParents({kind: ts.SyntaxKind.NullKeyword}, node)); + syntaxKind = ts.SyntaxKind.NullKeyword; break; case BuiltinType.Number: - const numeric = { - kind: ts.SyntaxKind.NumericLiteral, - text: node.getText(), - }; - setParents({kind: ts.SyntaxKind.ExpressionStatement, expression: numeric}, node); - type = checker.getTypeAtLocation(numeric); + syntaxKind = ts.SyntaxKind.NumberKeyword; break; case BuiltinType.String: - type = checker.getTypeAtLocation(setParents( - { - kind: ts.SyntaxKind.NoSubstitutionTemplateLiteral, - text: node.getText(), - }, - node)); + syntaxKind = ts.SyntaxKind.StringKeyword; break; case BuiltinType.Undefined: - type = checker.getTypeAtLocation(setParents( - { - kind: ts.SyntaxKind.VoidExpression, - expression: {kind: ts.SyntaxKind.NumericLiteral} - }, - node)); + syntaxKind = ts.SyntaxKind.UndefinedKeyword; break; default: - throw new Error(`Internal error, unhandled literal kind ${kind}:${BuiltinType[kind]}`); + throw new Error( + `Internal error, unhandled literal kind ${builtinType}:${BuiltinType[builtinType]}`); } - return type; -} - -function setParents(node: T, parent: ts.Node): T { - node.parent = parent; - ts.forEachChild(node, child => setParents(child, node)); - return node; + const node = ts.createNode(syntaxKind); + node.parent = ctx.node; + return ctx.checker.getTypeAtLocation(node); } function spanAt(sourceFile: ts.SourceFile, line: number, column: number): Span|undefined { diff --git a/packages/language-service/test/typescript_symbols_spec.ts b/packages/language-service/test/typescript_symbols_spec.ts index ce661491b0..aa4064949b 100644 --- a/packages/language-service/test/typescript_symbols_spec.ts +++ b/packages/language-service/test/typescript_symbols_spec.ts @@ -53,10 +53,10 @@ describe('symbol query', () => { const tests: Array<[BuiltinType, boolean, ts.TypeFlags?]> = [ // builtinType, throws, want [BuiltinType.Any, false, ts.TypeFlags.Any], - [BuiltinType.Boolean, false, ts.TypeFlags.BooleanLiteral], + [BuiltinType.Boolean, false, ts.TypeFlags.Boolean | ts.TypeFlags.Union], [BuiltinType.Null, false, ts.TypeFlags.Null], - [BuiltinType.Number, false, ts.TypeFlags.NumberLiteral], - [BuiltinType.String, false, ts.TypeFlags.StringLiteral], + [BuiltinType.Number, false, ts.TypeFlags.Number], + [BuiltinType.String, false, ts.TypeFlags.String], [BuiltinType.Undefined, false, ts.TypeFlags.Undefined], [BuiltinType.Unbound, true], [BuiltinType.Other, true],