From 09480d3a9d3c541f6ac248f58c0c840477edf915 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],