From 6c8e322fa1765eba0c67e535b8a2207326296860 Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Tue, 14 Jan 2020 22:48:13 -0800 Subject: [PATCH] refactor(language-service): cleanup of low-hanging TODOs (#34784) Cleans up some simple TODOs. Also removes `typescript_symbols#getTypeWrapper`, which I thought I had but failed to remove in #34571. PR Close #34784 --- .../src/expression_diagnostics.ts | 3 +- .../language-service/src/expression_type.ts | 9 ++--- .../language-service/src/locate_symbol.ts | 2 - .../src/typescript_symbols.ts | 37 +++++-------------- packages/language-service/test/mocks.ts | 13 +++---- 5 files changed, 18 insertions(+), 46 deletions(-) diff --git a/packages/language-service/src/expression_diagnostics.ts b/packages/language-service/src/expression_diagnostics.ts index b5c7e43402..124f63e77a 100644 --- a/packages/language-service/src/expression_diagnostics.ts +++ b/packages/language-service/src/expression_diagnostics.ts @@ -220,8 +220,7 @@ export function getExpressionScope( class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor { private path: TemplateAstPath; - // TODO(issue/24571): remove '!'. - private directiveSummary !: CompileDirectiveSummary; + private directiveSummary: CompileDirectiveSummary|undefined; diagnostics: Diagnostic[] = []; diff --git a/packages/language-service/src/expression_type.ts b/packages/language-service/src/expression_type.ts index a6795a14c6..7073b1eee4 100644 --- a/packages/language-service/src/expression_type.ts +++ b/packages/language-service/src/expression_type.ts @@ -19,8 +19,7 @@ export class TypeDiagnostic { // AstType calculatetype of the ast given AST element. export class AstType implements AstVisitor { - // TODO(issue/24571): remove '!'. - public diagnostics !: TypeDiagnostic[]; + public diagnostics: TypeDiagnostic[] = []; constructor( private scope: SymbolTable, private query: SymbolQuery, @@ -338,8 +337,7 @@ export class AstType implements AstVisitor { return this.resolvePropertyRead(this.query.getNonNullableType(this.getType(ast.receiver)), ast); } - // TODO(issue/24571): remove '!'. - private _anyType !: Symbol; + private _anyType: Symbol|undefined; private get anyType(): Symbol { let result = this._anyType; if (!result) { @@ -348,8 +346,7 @@ export class AstType implements AstVisitor { return result; } - // TODO(issue/24571): remove '!'. - private _undefinedType !: Symbol; + private _undefinedType: Symbol|undefined; private get undefinedType(): Symbol { let result = this._undefinedType; if (!result) { diff --git a/packages/language-service/src/locate_symbol.ts b/packages/language-service/src/locate_symbol.ts index 392c043c52..7849a611b1 100644 --- a/packages/language-service/src/locate_symbol.ts +++ b/packages/language-service/src/locate_symbol.ts @@ -98,8 +98,6 @@ export function locateSymbol(info: AstResult, position: number): SymbolInfo|unde } // See if this attribute matches the selector of any directive on the element. - // TODO(ayazhafiz): Consider caching selector matches (at the expense of potentially - // very high memory usage). const attributeSelector = `[${ast.name}=${ast.value}]`; const parsedAttribute = CssSelector.parse(attributeSelector); if (!parsedAttribute.length) return; diff --git a/packages/language-service/src/typescript_symbols.ts b/packages/language-service/src/typescript_symbols.ts index 1c5b4bb0aa..ba7d088c8c 100644 --- a/packages/language-service/src/typescript_symbols.ts +++ b/packages/language-service/src/typescript_symbols.ts @@ -82,14 +82,16 @@ export function getPipesTable( class TypeScriptSymbolQuery implements SymbolQuery { private typeCache = new Map(); - // TODO(issue/24571): remove '!'. - private pipesCache !: SymbolTable; + private pipesCache: SymbolTable|undefined; constructor( private program: ts.Program, private checker: ts.TypeChecker, private source: ts.SourceFile, private fetchPipes: () => SymbolTable) {} - getTypeKind(symbol: Symbol): BuiltinType { return typeKindOf(this.getTsTypeOf(symbol)); } + getTypeKind(symbol: Symbol): BuiltinType { + const type = symbol instanceof TypeWrapper ? symbol.tsType : undefined; + return typeKindOf(type); + } getBuiltinType(kind: BuiltinType): Symbol { let result = this.typeCache.get(kind); @@ -205,21 +207,6 @@ class TypeScriptSymbolQuery implements SymbolQuery { } } } - - private getTsTypeOf(symbol: Symbol): ts.Type|undefined { - const type = getTypeWrapper(symbol); - return type && type.tsType; - } -} - -function getTypeWrapper(symbol: Symbol): TypeWrapper|undefined { - let type: TypeWrapper|undefined = undefined; - if (symbol instanceof TypeWrapper) { - type = symbol; - } else if (symbol.type instanceof TypeWrapper) { - type = symbol.type; - } - return type; } function typeCallable(type: ts.Type): boolean { @@ -290,9 +277,8 @@ class TypeWrapper implements Symbol { return selectSignature(this.tsType, this.context, types); } - indexed(argument: Symbol, value: any): Symbol|undefined { - const type = getTypeWrapper(argument); - if (!type) return; + indexed(type: Symbol, value: any): Symbol|undefined { + if (!(type instanceof TypeWrapper)) return; const typeKind = typeKindOf(type.tsType); switch (typeKind) { @@ -318,11 +304,7 @@ class TypeWrapper implements Symbol { const typeReference = (this.tsType as ts.TypeReference); let typeArguments: ReadonlyArray|undefined; - if (this.context.checker.getTypeArguments) { - typeArguments = this.context.checker.getTypeArguments(typeReference); - } else { - typeArguments = typeReference.typeArguments; - } + typeArguments = this.context.checker.getTypeArguments(typeReference); if (!typeArguments) return undefined; return typeArguments.map(ta => new TypeWrapper(ta, this.context)); } @@ -603,8 +585,7 @@ class PipesTable implements SymbolTable { const INDEX_PATTERN = /[\\/]([^\\/]+)[\\/]\1\.d\.ts$/; class PipeSymbol implements Symbol { - // TODO(issue/24571): remove '!'. - private _tsType !: ts.Type; + private _tsType: ts.Type|undefined; public readonly kind: DeclarationKind = 'pipe'; public readonly language: string = 'typescript'; public readonly container: Symbol|undefined = undefined; diff --git a/packages/language-service/test/mocks.ts b/packages/language-service/test/mocks.ts index e58327e1db..bab5996054 100644 --- a/packages/language-service/test/mocks.ts +++ b/packages/language-service/test/mocks.ts @@ -107,14 +107,11 @@ const summaryResolver = new AotSummaryResolver( staticSymbolCache); export class DiagnosticContext { - // tslint:disable - // TODO(issue/24571): remove '!'. - _analyzedModules !: NgAnalyzedModules; - _staticSymbolResolver: StaticSymbolResolver|undefined; - _reflector: StaticReflector|undefined; - _errors: {e: any, path?: string}[] = []; - _resolver: CompileMetadataResolver|undefined; - // tslint:enable + private _analyzedModules: NgAnalyzedModules|undefined; + private _staticSymbolResolver: StaticSymbolResolver|undefined; + private _reflector: StaticReflector|undefined; + private _errors: {e: any, path?: string}[] = []; + private _resolver: CompileMetadataResolver|undefined; constructor( public service: ts.LanguageService, public program: ts.Program,