From 8d550adc7e7edc16c3ee97c58dd115892a7d69e7 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Fri, 28 Apr 2017 15:10:30 -0700 Subject: [PATCH] fix(language-service): remove asserts for non-null expressions Reworked some of the code so asserts are no longer necessary. Added additional and potentially redundant checks Added checks where the null checker found real problems. --- packages/language-service/src/ast_path.ts | 4 +- packages/language-service/src/completions.ts | 155 +++++++++--------- packages/language-service/src/diagnostics.ts | 50 +++--- packages/language-service/src/expressions.ts | 18 +- .../language-service/src/language_service.ts | 28 ++-- .../language-service/src/locate_symbol.ts | 49 +++--- .../language-service/src/reflector_host.ts | 8 +- .../language-service/src/template_path.ts | 4 +- packages/language-service/src/ts_plugin.ts | 8 +- packages/language-service/src/types.ts | 4 +- .../language-service/src/typescript_host.ts | 98 +++++------ packages/language-service/src/utils.ts | 10 +- 12 files changed, 241 insertions(+), 195 deletions(-) diff --git a/packages/language-service/src/ast_path.ts b/packages/language-service/src/ast_path.ts index a1003945f9114..88e8cf5d966bd 100644 --- a/packages/language-service/src/ast_path.ts +++ b/packages/language-service/src/ast_path.ts @@ -13,7 +13,9 @@ export class AstPath { get head(): T|undefined { return this.path[0]; } get tail(): T|undefined { return this.path[this.path.length - 1]; } - parentOf(node: T): T|undefined { return this.path[this.path.indexOf(node) - 1]; } + parentOf(node: T|undefined): T|undefined { + return node && this.path[this.path.indexOf(node) - 1]; + } childOf(node: T): T|undefined { return this.path[this.path.indexOf(node) + 1]; } first(ctor: {new (...args: any[]): N}): N|undefined { diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 031268d0c3781..a60b1febd2d23 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -33,71 +33,73 @@ export function getTemplateCompletions(templateInfo: TemplateInfo): Completions| let result: Completions|undefined = undefined; let {htmlAst, templateAst, template} = templateInfo; // The templateNode starts at the delimiter character so we add 1 to skip it. - let templatePosition = templateInfo.position ! - template.span.start; - let path = new HtmlAstPath(htmlAst, templatePosition); - let mostSpecific = path.tail; - if (path.empty) { - result = elementCompletions(templateInfo, path); - } else { - let astPosition = templatePosition - mostSpecific !.sourceSpan !.start.offset; - mostSpecific !.visit( - { - visitElement(ast) { - let startTagSpan = spanOf(ast.sourceSpan); - let tagLen = ast.name.length; - if (templatePosition <= - startTagSpan !.start + tagLen + 1 /* 1 for the opening angle bracked */) { - // If we are in the tag then return the element completions. - result = elementCompletions(templateInfo, path); - } else if (templatePosition < startTagSpan !.end) { - // We are in the attribute section of the element (but not in an attribute). - // Return the attribute completions. - result = attributeCompletions(templateInfo, path); - } - }, - visitAttribute(ast) { - if (!ast.valueSpan || !inSpan(templatePosition, spanOf(ast.valueSpan))) { - // We are in the name of an attribute. Show attribute completions. - result = attributeCompletions(templateInfo, path); - } else if (ast.valueSpan && inSpan(templatePosition, spanOf(ast.valueSpan))) { - result = attributeValueCompletions(templateInfo, templatePosition, ast); - } - }, - visitText(ast) { - // Check if we are in a entity. - result = entityCompletions(getSourceText(template, spanOf(ast) !), astPosition); - if (result) return result; - result = interpolationCompletions(templateInfo, templatePosition); - if (result) return result; - let element = path.first(Element); - if (element) { - let definition = getHtmlTagDefinition(element.name); - if (definition.contentType === TagContentType.PARSABLE_DATA) { + if (templateInfo.position != null) { + let templatePosition = templateInfo.position - template.span.start; + let path = new HtmlAstPath(htmlAst, templatePosition); + let mostSpecific = path.tail; + if (path.empty || !mostSpecific) { + result = elementCompletions(templateInfo, path); + } else { + let astPosition = templatePosition - mostSpecific.sourceSpan.start.offset; + mostSpecific.visit( + { + visitElement(ast) { + let startTagSpan = spanOf(ast.sourceSpan); + let tagLen = ast.name.length; + if (templatePosition <= + startTagSpan.start + tagLen + 1 /* 1 for the opening angle bracked */) { + // If we are in the tag then return the element completions. + result = elementCompletions(templateInfo, path); + } else if (templatePosition < startTagSpan.end) { + // We are in the attribute section of the element (but not in an attribute). + // Return the attribute completions. + result = attributeCompletions(templateInfo, path); + } + }, + visitAttribute(ast) { + if (!ast.valueSpan || !inSpan(templatePosition, spanOf(ast.valueSpan))) { + // We are in the name of an attribute. Show attribute completions. + result = attributeCompletions(templateInfo, path); + } else if (ast.valueSpan && inSpan(templatePosition, spanOf(ast.valueSpan))) { + result = attributeValueCompletions(templateInfo, templatePosition, ast); + } + }, + visitText(ast) { + // Check if we are in a entity. + result = entityCompletions(getSourceText(template, spanOf(ast)), astPosition); + if (result) return result; + result = interpolationCompletions(templateInfo, templatePosition); + if (result) return result; + let element = path.first(Element); + if (element) { + let definition = getHtmlTagDefinition(element.name); + if (definition.contentType === TagContentType.PARSABLE_DATA) { + result = voidElementAttributeCompletions(templateInfo, path); + if (!result) { + // If the element can hold content Show element completions. + result = elementCompletions(templateInfo, path); + } + } + } else { + // If no element container, implies parsable data so show elements. result = voidElementAttributeCompletions(templateInfo, path); if (!result) { - // If the element can hold content Show element completions. result = elementCompletions(templateInfo, path); } } - } else { - // If no element container, implies parsable data so show elements. - result = voidElementAttributeCompletions(templateInfo, path); - if (!result) { - result = elementCompletions(templateInfo, path); - } - } + }, + visitComment(ast) {}, + visitExpansion(ast) {}, + visitExpansionCase(ast) {} }, - visitComment(ast) {}, - visitExpansion(ast) {}, - visitExpansionCase(ast) {} - }, - null); + null); + } } return result; } function attributeCompletions(info: TemplateInfo, path: HtmlAstPath): Completions|undefined { - let item = path.tail instanceof Element ? path.tail : path.parentOf(path.tail !); + let item = path.tail instanceof Element ? path.tail : path.parentOf(path.tail); if (item instanceof Element) { return attributeCompletionsForElement(info, item.name, item); } @@ -213,11 +215,12 @@ function elementCompletions(info: TemplateInfo, path: HtmlAstPath): Completions| let htmlNames = elementNames().filter(name => !(name in hiddenHtmlElements)); // Collect the elements referenced by the selectors - let directiveElements = - getSelectors(info).selectors.map(selector => selector.element).filter(name => !!name); + let directiveElements = getSelectors(info) + .selectors.map(selector => selector.element) + .filter(name => !!name) as string[]; let components = - directiveElements.map(name => ({kind: 'component', name: name !, sort: name !})); + directiveElements.map(name => ({kind: 'component', name, sort: name})); let htmlElements = htmlNames.map(name => ({kind: 'element', name: name, sort: name})); // Return components and html elements @@ -262,25 +265,25 @@ function voidElementAttributeCompletions(info: TemplateInfo, path: HtmlAstPath): undefined { let tail = path.tail; if (tail instanceof Text) { - let match = tail.value.match(/<(\w(\w|\d|-)*:)?(\w(\w|\d|-)*)\s/) !; + let match = tail.value.match(/<(\w(\w|\d|-)*:)?(\w(\w|\d|-)*)\s/); // The position must be after the match, otherwise we are still in a place where elements // are expected (such as `<|a` or `= match.index ! + match[0].length + tail.sourceSpan.start.offset) { + if (match && + path.position >= (match.index || 0) + match[0].length + tail.sourceSpan.start.offset) { return attributeCompletionsForElement(info, match[3]); } } } class ExpressionVisitor extends NullTemplateVisitor { + private getExpressionScope: () => SymbolTable; result: Completions; constructor( private info: TemplateInfo, private position: number, private attr?: Attribute, - private getExpressionScope?: () => SymbolTable) { + getExpressionScope?: () => SymbolTable) { super(); - if (!getExpressionScope) { - this.getExpressionScope = () => info.template.members; - } + this.getExpressionScope = getExpressionScope || (() => info.template.members); } visitDirectiveProperty(ast: BoundDirectivePropertyAst): void { @@ -311,7 +314,8 @@ class ExpressionVisitor extends NullTemplateVisitor { this.info.expressionParser.parseTemplateBindings(key, this.attr.value, null); // find the template binding that contains the position - const valueRelativePosition = this.position - this.attr.valueSpan !.start.offset - 1; + if (!this.attr.valueSpan) return; + const valueRelativePosition = this.position - this.attr.valueSpan.start.offset - 1; const bindings = templateBindingResult.templateBindings; const binding = bindings.find( @@ -340,10 +344,12 @@ class ExpressionVisitor extends NullTemplateVisitor { // We are after the '=' in a let clause. The valid values here are the members of the // template reference's type parameter. const directiveMetadata = selectorInfo.map.get(selector); - const contextTable = - this.info.template.query.getTemplateContext(directiveMetadata !.type.reference); - if (contextTable) { - this.result = this.symbolsToCompletions(contextTable.values()); + if (directiveMetadata) { + const contextTable = + this.info.template.query.getTemplateContext(directiveMetadata.type.reference); + if (contextTable) { + this.result = this.symbolsToCompletions(contextTable.values()); + } } } else if (binding.key && valueRelativePosition <= (binding.key.length - key.length)) { keyCompletions(); @@ -371,7 +377,7 @@ class ExpressionVisitor extends NullTemplateVisitor { const expressionPosition = this.position - ast.sourceSpan.start.offset; if (inSpan(expressionPosition, ast.value.span)) { const completions = getExpressionCompletions( - this.getExpressionScope !(), ast.value, expressionPosition, this.info.template.query); + this.getExpressionScope(), ast.value, expressionPosition, this.info.template.query); if (completions) { this.result = this.symbolsToCompletions(completions); } @@ -380,8 +386,8 @@ class ExpressionVisitor extends NullTemplateVisitor { private attributeValueCompletions(value: AST, position?: number) { const symbols = getExpressionCompletions( - this.getExpressionScope !(), value, - position == null ? this.attributeValuePosition : position, this.info.template.query); + this.getExpressionScope(), value, position == null ? this.attributeValuePosition : position, + this.info.template.query); if (symbols) { this.result = this.symbolsToCompletions(symbols); } @@ -393,12 +399,13 @@ class ExpressionVisitor extends NullTemplateVisitor { } private get attributeValuePosition() { - return this.position - this.attr !.valueSpan !.start.offset - 1; + if (this.attr && this.attr.valueSpan) { + return this.position - this.attr.valueSpan.start.offset - 1; + } + return 0; } } - - function getSourceText(template: TemplateSource, span: Span): string { return template.source.substring(span.start, span.end); } diff --git a/packages/language-service/src/diagnostics.ts b/packages/language-service/src/diagnostics.ts index fd848368629dc..bf380f0b9afa0 100644 --- a/packages/language-service/src/diagnostics.ts +++ b/packages/language-service/src/diagnostics.ts @@ -29,7 +29,7 @@ export function getTemplateDiagnostics( results.push(...ast.parseErrors.map( e => ({ kind: DiagnosticKind.Error, - span: offsetSpan(spanOf(e.span) !, template.span.start), + span: offsetSpan(spanOf(e.span), template.span.start), message: e.msg }))); } else if (ast.templateAst) { @@ -91,20 +91,24 @@ export function getDeclarationDiagnostics( function getTemplateExpressionDiagnostics( template: TemplateSource, astResult: AstResult): Diagnostics { - const info: TemplateInfo = { - template, - htmlAst: astResult.htmlAst !, - directive: astResult.directive !, - directives: astResult.directives !, - pipes: astResult.pipes !, - templateAst: astResult.templateAst !, - expressionParser: astResult.expressionParser ! - }; - const visitor = new ExpressionDiagnosticsVisitor( - info, (path: TemplateAstPath, includeEvent: boolean) => - getExpressionScope(info, path, includeEvent)); - templateVisitAll(visitor, astResult.templateAst !); - return visitor.diagnostics; + if (astResult.htmlAst && astResult.directive && astResult.directives && astResult.pipes && + astResult.templateAst && astResult.expressionParser) { + const info: TemplateInfo = { + template, + htmlAst: astResult.htmlAst, + directive: astResult.directive, + directives: astResult.directives, + pipes: astResult.pipes, + templateAst: astResult.templateAst, + expressionParser: astResult.expressionParser + }; + const visitor = new ExpressionDiagnosticsVisitor( + info, (path: TemplateAstPath, includeEvent: boolean) => + getExpressionScope(info, path, includeEvent)); + templateVisitAll(visitor, astResult.templateAst !); + return visitor.diagnostics; + } + return []; } class ExpressionDiagnosticsVisitor extends TemplateAstChildVisitor { @@ -158,11 +162,11 @@ class ExpressionDiagnosticsVisitor extends TemplateAstChildVisitor { if (context && !context.has(ast.value)) { if (ast.value === '$implicit') { this.reportError( - 'The template context does not have an implicit value', spanOf(ast.sourceSpan) !); + 'The template context does not have an implicit value', spanOf(ast.sourceSpan)); } else { this.reportError( `The template context does not defined a member called '${ast.value}'`, - spanOf(ast.sourceSpan) !); + spanOf(ast.sourceSpan)); } } } @@ -233,11 +237,13 @@ class ExpressionDiagnosticsVisitor extends TemplateAstChildVisitor { } } - private reportError(message: string, span: Span) { - this.diagnostics.push({ - span: offsetSpan(span, this.info.template.span.start), - kind: DiagnosticKind.Error, message - }); + private reportError(message: string, span: Span|undefined) { + if (span) { + this.diagnostics.push({ + span: offsetSpan(span, this.info.template.span.start), + kind: DiagnosticKind.Error, message + }); + } } private reportWarning(message: string, span: Span) { diff --git a/packages/language-service/src/expressions.ts b/packages/language-service/src/expressions.ts index 3d6b567a3fc24..ba3e585beacab 100644 --- a/packages/language-service/src/expressions.ts +++ b/packages/language-service/src/expressions.ts @@ -495,8 +495,9 @@ class AstType implements ExpressionVisitor { // The type of a method is the selected methods result type. const method = receiverType.members().get(ast.name); if (!method) return this.reportError(`Unknown method ${ast.name}`, ast); - if (!method.type !.callable) return this.reportError(`Member ${ast.name} is not callable`, ast); - const signature = method.type !.selectSignature(ast.args.map(arg => this.getType(arg))); + if (!method.type) return this.reportError(`Could not find a type for ${ast.name}`, ast); + if (!method.type.callable) return this.reportError(`Member ${ast.name} is not callable`, ast); + const signature = method.type.selectSignature(ast.args.map(arg => this.getType(arg))); if (!signature) return this.reportError(`Unable to resolve signature for call of method ${ast.name}`, ast); return signature.result; @@ -601,7 +602,9 @@ function visitChildren(ast: AST, visitor: ExpressionVisitor) { visit(ast.falseExp); }, visitFunctionCall(ast) { - visit(ast.target !); + if (ast.target) { + visit(ast.target); + } visitAll(ast.args); }, visitImplicitReceiver(ast) {}, @@ -676,7 +679,7 @@ function getReferences(info: TemplateInfo): SymbolDeclaration[] { function processReferences(references: ReferenceAst[]) { for (const reference of references) { - let type: Symbol = undefined !; + let type: Symbol|undefined = undefined; if (reference.value) { type = info.template.query.getTypeSymbol(tokenReference(reference.value)); } @@ -721,7 +724,7 @@ function getVarDeclarations(info: TemplateInfo, path: TemplateAstPath): SymbolDe .find(c => !!c); // Determine the type of the context field referenced by variable.value. - let type: Symbol = undefined !; + let type: Symbol|undefined = undefined; if (context) { const value = context.get(variable.value); if (value) { @@ -762,7 +765,10 @@ function refinedVariableType( const bindingType = new AstType(info.template.members, info.template.query, {}).getType(ngForOfBinding.value); if (bindingType) { - return info.template.query.getElementType(bindingType) !; + const result = info.template.query.getElementType(bindingType); + if (result) { + return result; + } } } } diff --git a/packages/language-service/src/language_service.ts b/packages/language-service/src/language_service.ts index 6cb1089ea1cc2..65a481e6adbf3 100644 --- a/packages/language-service/src/language_service.ts +++ b/packages/language-service/src/language_service.ts @@ -81,24 +81,25 @@ class LanguageServiceImpl implements LanguageService { let template = this.host.getTemplateAt(fileName, position); if (template) { let astResult = this.getTemplateAst(template, fileName); - if (astResult && astResult.htmlAst && astResult.templateAst) + if (astResult && astResult.htmlAst && astResult.templateAst && astResult.directive && + astResult.directives && astResult.pipes && astResult.expressionParser) return { position, fileName, template, htmlAst: astResult.htmlAst, - directive: astResult.directive !, - directives: astResult.directives !, - pipes: astResult.pipes !, + directive: astResult.directive, + directives: astResult.directives, + pipes: astResult.pipes, templateAst: astResult.templateAst, - expressionParser: astResult.expressionParser ! + expressionParser: astResult.expressionParser }; } return undefined; } getTemplateAst(template: TemplateSource, contextFile: string): AstResult { - let result: AstResult = undefined !; + let result: AstResult|undefined = undefined; try { const resolvedMetadata = this.metadataResolver.getNonNormalizedDirectiveMetadata(template.type as any); @@ -112,7 +113,7 @@ class LanguageServiceImpl implements LanguageService { config, expressionParser, new DomElementSchemaRegistry(), htmlParser, null !, []); const htmlResult = htmlParser.parse(template.source, '', true); const analyzedModules = this.host.getAnalyzedModules(); - let errors: Diagnostic[] = undefined !; + let errors: Diagnostic[]|undefined = undefined; let ngModule = analyzedModules.ngModuleByPipeOrDirective.get(template.type); if (!ngModule) { // Reported by the the declaration diagnostics. @@ -121,8 +122,7 @@ class LanguageServiceImpl implements LanguageService { if (ngModule) { const resolvedDirectives = ngModule.transitiveModule.directives.map( d => this.host.resolver.getNonNormalizedDirectiveMetadata(d.reference)); - const directives = - resolvedDirectives.filter(d => d !== null).map(d => d !.metadata.toSummary()); + const directives = removeMissing(resolvedDirectives).map(d => d.metadata.toSummary()); const pipes = ngModule.transitiveModule.pipes.map( p => this.host.resolver.getOrLoadPipeMetadata(p.reference).toSummary()); const schemas = ngModule.schemas; @@ -142,10 +142,14 @@ class LanguageServiceImpl implements LanguageService { } result = {errors: [{kind: DiagnosticKind.Error, message: e.message, span}]}; } - return result; + return result || {}; } } +function removeMissing(values: (T | null | undefined)[]): T[] { + return values.filter(e => !!e) as T[]; +} + function uniqueBySpan < T extends { span: Span; } @@ -169,8 +173,8 @@ function uniqueBySpan < T extends { } } -function findSuitableDefaultModule(modules: NgAnalyzedModules): CompileNgModuleMetadata { - let result: CompileNgModuleMetadata = undefined !; +function findSuitableDefaultModule(modules: NgAnalyzedModules): CompileNgModuleMetadata|undefined { + let result: CompileNgModuleMetadata|undefined = undefined; let resultSize = 0; for (const module of modules.ngModules) { const moduleSize = module.transitiveModule.directives.length; diff --git a/packages/language-service/src/locate_symbol.ts b/packages/language-service/src/locate_symbol.ts index 5f55f2c48bfac..3cc93669bc317 100644 --- a/packages/language-service/src/locate_symbol.ts +++ b/packages/language-service/src/locate_symbol.ts @@ -21,22 +21,25 @@ export interface SymbolInfo { } export function locateSymbol(info: TemplateInfo): SymbolInfo|undefined { - const templatePosition = info.position ! - info.template.span.start; + if (!info.position) return undefined; + const templatePosition = info.position - info.template.span.start; const path = new TemplateAstPath(info.templateAst, templatePosition); if (path.tail) { - let symbol: Symbol = undefined !; - let span: Span = undefined !; + let symbol: Symbol|undefined = undefined; + let span: Span|undefined = undefined; const attributeValueSymbol = (ast: AST, inEvent: boolean = false): boolean => { const attribute = findAttribute(info); if (attribute) { if (inSpan(templatePosition, spanOf(attribute.valueSpan))) { const scope = getExpressionScope(info, path, inEvent); - const expressionOffset = attribute.valueSpan !.start.offset + 1; - const result = getExpressionSymbol( - scope, ast, templatePosition - expressionOffset, info.template.query); - if (result) { - symbol = result.symbol; - span = offsetSpan(result.span, expressionOffset); + if (attribute.valueSpan) { + const expressionOffset = attribute.valueSpan.start.offset + 1; + const result = getExpressionSymbol( + scope, ast, templatePosition - expressionOffset, info.template.query); + if (result) { + symbol = result.symbol; + span = offsetSpan(result.span, expressionOffset); + } } return true; } @@ -52,28 +55,28 @@ export function locateSymbol(info: TemplateInfo): SymbolInfo|undefined { if (component) { symbol = info.template.query.getTypeSymbol(component.directive.type.reference); symbol = symbol && new OverrideKindSymbol(symbol, 'component'); - span = spanOf(ast) !; + span = spanOf(ast); } else { // Find a directive that matches the element name - const directive = - ast.directives.find(d => d.directive.selector !.indexOf(ast.name) >= 0); + const directive = ast.directives.find( + d => d.directive.selector != null && d.directive.selector.indexOf(ast.name) >= 0); if (directive) { symbol = info.template.query.getTypeSymbol(directive.directive.type.reference); symbol = symbol && new OverrideKindSymbol(symbol, 'directive'); - span = spanOf(ast) !; + span = spanOf(ast); } } }, visitReference(ast) { symbol = info.template.query.getTypeSymbol(tokenReference(ast.value)); - span = spanOf(ast) !; + span = spanOf(ast); }, visitVariable(ast) {}, visitEvent(ast) { if (!attributeValueSymbol(ast.handler, /* inEvent */ true)) { - symbol = findOutputBinding(info, path, ast) !; + symbol = findOutputBinding(info, path, ast); symbol = symbol && new OverrideKindSymbol(symbol, 'event'); - span = spanOf(ast) !; + span = spanOf(ast); } }, visitElementProperty(ast) { attributeValueSymbol(ast.value); }, @@ -93,12 +96,12 @@ export function locateSymbol(info: TemplateInfo): SymbolInfo|undefined { visitText(ast) {}, visitDirective(ast) { symbol = info.template.query.getTypeSymbol(ast.directive.type.reference); - span = spanOf(ast) !; + span = spanOf(ast); }, visitDirectiveProperty(ast) { if (!attributeValueSymbol(ast.value)) { - symbol = findInputBinding(info, path, ast) !; - span = spanOf(ast) !; + symbol = findInputBinding(info, path, ast); + span = spanOf(ast); } } }, @@ -110,9 +113,11 @@ export function locateSymbol(info: TemplateInfo): SymbolInfo|undefined { } function findAttribute(info: TemplateInfo): Attribute|undefined { - const templatePosition = info.position ! - info.template.span.start; - const path = new HtmlAstPath(info.htmlAst, templatePosition); - return path.first(Attribute); + if (info.position) { + const templatePosition = info.position - info.template.span.start; + const path = new HtmlAstPath(info.htmlAst, templatePosition); + return path.first(Attribute); + } } function findInputBinding( diff --git a/packages/language-service/src/reflector_host.ts b/packages/language-service/src/reflector_host.ts index 086cb48b695b7..9a84fb1e3700c 100644 --- a/packages/language-service/src/reflector_host.ts +++ b/packages/language-service/src/reflector_host.ts @@ -22,18 +22,24 @@ class ReflectorModuleModuleResolutionHost implements ts.ModuleResolutionHost { if (snapshot) { return snapshot.getText(0, snapshot.getLength()); } + + // Typescript readFile() declaration should be `readFile(fileName: string): string | undefined return undefined !; } directoryExists: (directoryName: string) => boolean; } +// This reflector host's purpose is to first set verboseInvalidExpressions to true so the +// reflector will collect errors instead of throwing, and second to all deferring the creation +// of the program until it is actually needed. export class ReflectorHost extends CompilerHost { constructor( private getProgram: () => ts.Program, serviceHost: ts.LanguageServiceHost, options: AngularCompilerOptions) { super( - null !, options, + // The ancestor value for program is overridden below so passing null here is safe. + /* program */ null !, options, new ModuleResolutionHostAdapter(new ReflectorModuleModuleResolutionHost(serviceHost)), {verboseInvalidExpression: true}); } diff --git a/packages/language-service/src/template_path.ts b/packages/language-service/src/template_path.ts index 07fca912ad722..e974e39563025 100644 --- a/packages/language-service/src/template_path.ts +++ b/packages/language-service/src/template_path.ts @@ -104,10 +104,10 @@ class TemplateAstPathBuilder extends TemplateAstChildVisitor { constructor(private position: number, private allowWidening: boolean) { super(); } visit(ast: TemplateAst, context: any): any { - let span = spanOf(ast) !; + let span = spanOf(ast); if (inSpan(this.position, span)) { const len = this.path.length; - if (!len || this.allowWidening || isNarrower(span, spanOf(this.path[len - 1]) !)) { + if (!len || this.allowWidening || isNarrower(span, spanOf(this.path[len - 1]))) { this.path.push(ast); } } else { diff --git a/packages/language-service/src/ts_plugin.ts b/packages/language-service/src/ts_plugin.ts index 3470097cb65d2..d1b58a24bbd37 100644 --- a/packages/language-service/src/ts_plugin.ts +++ b/packages/language-service/src/ts_plugin.ts @@ -25,14 +25,16 @@ export function create(info: any /* ts.server.PluginCreateInfo */): ts.LanguageS } function diagnosticToDiagnostic(d: Diagnostic, file: ts.SourceFile): ts.Diagnostic { - return { + const result = { file, start: d.span.start, length: d.span.end - d.span.start, messageText: d.message, category: ts.DiagnosticCategory.Error, - code: 0 + code: 0, + source: 'ng' }; + return result; } function tryOperation(attempting: string, callback: () => void) { @@ -78,7 +80,7 @@ export function create(info: any /* ts.server.PluginCreateInfo */): ts.LanguageS if (ours) { const displayParts: typeof base.displayParts = []; for (const part of ours.text) { - displayParts.push({kind: part.language !, text: part.text}); + displayParts.push({kind: part.language || 'angular', text: part.text}); } base = { displayParts, diff --git a/packages/language-service/src/types.ts b/packages/language-service/src/types.ts index fe29b2724746f..9e8c46403acf6 100644 --- a/packages/language-service/src/types.ts +++ b/packages/language-service/src/types.ts @@ -8,8 +8,6 @@ import {CompileDirectiveMetadata, CompileMetadataResolver, NgAnalyzedModules, StaticSymbol} from '@angular/compiler'; - - /** * The range of a span of text in a source file. * @@ -455,7 +453,7 @@ export interface LanguageServiceHost { * refers to a template file then the `position` should be ignored. If the `position` is not in a * template literal string then this method should return `undefined`. */ - getTemplateAt(fileName: string, position: number): TemplateSource /* |undefined */; + getTemplateAt(fileName: string, position: number): TemplateSource|undefined; /** * Return the template source information for all templates in `fileName` or for `fileName` if it diff --git a/packages/language-service/src/typescript_host.ts b/packages/language-service/src/typescript_host.ts index 8ea992c11f2a9..f6e249c72803c 100644 --- a/packages/language-service/src/typescript_host.ts +++ b/packages/language-service/src/typescript_host.ts @@ -74,22 +74,22 @@ export class DummyResourceLoader extends ResourceLoader { * @experimental */ export class TypeScriptServiceHost implements LanguageServiceHost { - private _resolver: CompileMetadataResolver; + private _resolver: CompileMetadataResolver|null; private _staticSymbolCache = new StaticSymbolCache(); private _summaryResolver: AotSummaryResolver; private _staticSymbolResolver: StaticSymbolResolver; - private _reflector: StaticReflector; + private _reflector: StaticReflector|null; private _reflectorHost: ReflectorHost; - private _checker: ts.TypeChecker; + private _checker: ts.TypeChecker|null; private _typeCache: Symbol[] = []; private context: string|undefined; private lastProgram: ts.Program|undefined; private modulesOutOfDate: boolean = true; - private analyzedModules: NgAnalyzedModules; + private analyzedModules: NgAnalyzedModules|null; private service: LanguageService; - private fileToComponent: Map; - private templateReferences: string[]; - private collectedErrors: Map; + private fileToComponent: Map|null; + private templateReferences: string[]|null; + private collectedErrors: Map|null; private fileVersions = new Map(); constructor(private host: ts.LanguageServiceHost, private tsService: ts.LanguageService) {} @@ -127,28 +127,28 @@ export class TypeScriptServiceHost implements LanguageServiceHost { getTemplateReferences(): string[] { this.ensureTemplateMap(); - return this.templateReferences; + return this.templateReferences || []; } - getTemplateAt(fileName: string, position: number): TemplateSource { + getTemplateAt(fileName: string, position: number): TemplateSource|undefined { let sourceFile = this.getSourceFile(fileName); if (sourceFile) { this.context = sourceFile.fileName; let node = this.findNode(sourceFile, position); if (node) { return this.getSourceFromNode( - fileName, this.host.getScriptVersion(sourceFile.fileName), node) !; + fileName, this.host.getScriptVersion(sourceFile.fileName), node); } } else { this.ensureTemplateMap(); // TODO: Cannocalize the file? - const componentType = this.fileToComponent.get(fileName); + const componentType = this.fileToComponent !.get(fileName); if (componentType) { return this.getSourceFromType( - fileName, this.host.getScriptVersion(fileName), componentType) !; + fileName, this.host.getScriptVersion(fileName), componentType); } } - return null !; + return undefined; } getAnalyzedModules(): NgAnalyzedModules { @@ -172,7 +172,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { getTemplates(fileName: string): TemplateSources { this.ensureTemplateMap(); - const componentType = this.fileToComponent.get(fileName); + const componentType = this.fileToComponent !.get(fileName); if (componentType) { const templateSource = this.getTemplateAt(fileName, 0); if (templateSource) { @@ -225,10 +225,10 @@ export class TypeScriptServiceHost implements LanguageServiceHost { updateAnalyzedModules() { this.validate(); if (this.modulesOutOfDate) { - this.analyzedModules = null !; - this._reflector = null !; - this.templateReferences = null !; - this.fileToComponent = null !; + this.analyzedModules = null; + this._reflector = null; + this.templateReferences = null; + this.fileToComponent = null; this.ensureAnalyzedModules(); this.modulesOutOfDate = false; } @@ -273,10 +273,10 @@ export class TypeScriptServiceHost implements LanguageServiceHost { } private clearCaches() { - this._checker = null !; + this._checker = null; this._typeCache = []; - this._resolver = null !; - this.collectedErrors = null !; + this._resolver = null; + this.collectedErrors = null; this.modulesOutOfDate = true; } @@ -345,7 +345,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { if (declaration && declaration.name) { const sourceFile = this.getSourceFile(fileName); return this.getSourceFromDeclaration( - fileName, version, this.stringOf(node) !, shrink(spanOf(node)), + fileName, version, this.stringOf(node) || '', shrink(spanOf(node)), this.reflector.getStaticSymbol(sourceFile.fileName, declaration.name.text), declaration, node, sourceFile); } @@ -359,11 +359,13 @@ export class TypeScriptServiceHost implements LanguageServiceHost { let result: TemplateSource|undefined = undefined; const declaration = this.getTemplateClassFromStaticSymbol(type); if (declaration) { - const snapshot = this.host.getScriptSnapshot(fileName) !; - const source = snapshot.getText(0, snapshot.getLength()); - result = this.getSourceFromDeclaration( - fileName, version, source, {start: 0, end: source.length}, type, declaration, declaration, - declaration.getSourceFile()); + const snapshot = this.host.getScriptSnapshot(fileName); + if (snapshot) { + const source = snapshot.getText(0, snapshot.getLength()); + result = this.getSourceFromDeclaration( + fileName, version, source, {start: 0, end: source.length}, type, declaration, + declaration, declaration.getSourceFile()); + } } return result; } @@ -398,17 +400,19 @@ export class TypeScriptServiceHost implements LanguageServiceHost { return result; } - private collectError(error: any, filePath: string) { - let errorMap = this.collectedErrors; - if (!errorMap) { - errorMap = this.collectedErrors = new Map(); - } - let errors = errorMap.get(filePath); - if (!errors) { - errors = []; - this.collectedErrors.set(filePath, errors); + private collectError(error: any, filePath: string|null) { + if (filePath) { + let errorMap = this.collectedErrors; + if (!errorMap || !this.collectedErrors) { + errorMap = this.collectedErrors = new Map(); + } + let errors = errorMap.get(filePath); + if (!errors) { + errors = []; + this.collectedErrors.set(filePath, errors); + } + errors.push(error); } - errors.push(error); } private get staticSymbolResolver(): StaticSymbolResolver { @@ -416,9 +420,9 @@ export class TypeScriptServiceHost implements LanguageServiceHost { if (!result) { this._summaryResolver = new AotSummaryResolver( { - loadSummary(filePath: string) { return null !; }, - isSourceFile(sourceFilePath: string) { return true !; }, - getOutputFileName(sourceFilePath: string) { return null !; } + loadSummary(filePath: string) { return null; }, + isSourceFile(sourceFilePath: string) { return true; }, + getOutputFileName(sourceFilePath: string) { return sourceFilePath; } }, this._staticSymbolCache); result = this._staticSymbolResolver = new StaticSymbolResolver( @@ -445,7 +449,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost { const declarationNode = ts.forEachChild(source, child => { if (child.kind === ts.SyntaxKind.ClassDeclaration) { const classDeclaration = child as ts.ClassDeclaration; - if (classDeclaration.name !.text === type.name) { + if (classDeclaration.name != null && classDeclaration.name.text === type.name) { return classDeclaration; } } @@ -614,7 +618,7 @@ class TypeScriptSymbolQuery implements SymbolQuery { 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 { return typeKindOf(this.getTsTypeOf(symbol)); } getBuiltinType(kind: BuiltinType): Symbol { // TODO: Replace with typeChecker API when available. @@ -1303,7 +1307,7 @@ function getTypeParameterOf(type: ts.Type, name: string): ts.Type|undefined { } } -function typeKindOf(type: ts.Type): BuiltinType { +function typeKindOf(type: ts.Type | undefined): BuiltinType { if (type) { if (type.flags & ts.TypeFlags.Any) { return BuiltinType.Any; @@ -1318,17 +1322,19 @@ function typeKindOf(type: ts.Type): BuiltinType { return BuiltinType.Null; } else if (type.flags & ts.TypeFlags.Union) { // If all the constituent types of a union are the same kind, it is also that kind. - let candidate: BuiltinType = undefined !; + let candidate: BuiltinType|null = null; const unionType = type as ts.UnionType; if (unionType.types.length > 0) { - candidate = typeKindOf(unionType.types[0]) !; + candidate = typeKindOf(unionType.types[0]); for (const subType of unionType.types) { if (candidate != typeKindOf(subType)) { return BuiltinType.Other; } } } - return candidate; + if (candidate != null) { + return candidate; + } } else if (type.flags & ts.TypeFlags.TypeParameter) { return BuiltinType.Unbound; } diff --git a/packages/language-service/src/utils.ts b/packages/language-service/src/utils.ts index 22a063af34fe6..dc751d66b5db1 100644 --- a/packages/language-service/src/utils.ts +++ b/packages/language-service/src/utils.ts @@ -21,6 +21,9 @@ export function isParseSourceSpan(value: any): value is ParseSourceSpan { return value && !!value.start; } +export function spanOf(span: SpanHolder): Span; +export function spanOf(span: ParseSourceSpan): Span; +export function spanOf(span: SpanHolder | ParseSourceSpan | undefined): Span|undefined; export function spanOf(span?: SpanHolder | ParseSourceSpan): Span|undefined { if (!span) return undefined; if (isParseSourceSpan(span)) { @@ -39,8 +42,8 @@ export function spanOf(span?: SpanHolder | ParseSourceSpan): Span|undefined { } export function inSpan(position: number, span?: Span, exclusive?: boolean): boolean { - return span && exclusive ? position >= span.start && position < span.end : - position >= span !.start && position <= span !.end; + return span != null && (exclusive ? position >= span.start && position < span.end : + position >= span.start && position <= span.end); } export function offsetSpan(span: Span, amount: number): Span { @@ -54,7 +57,8 @@ export function isNarrower(spanA: Span, spanB: Span): boolean { export function hasTemplateReference(type: CompileTypeMetadata): boolean { if (type.diDeps) { for (let diDep of type.diDeps) { - if (diDep.token !.identifier && identifierName(diDep.token !.identifier !) == 'TemplateRef') + if (diDep.token && diDep.token.identifier && + identifierName(diDep.token !.identifier !) == 'TemplateRef') return true; } }