Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(language-service): remove asserts for non-null expressions #16422

Closed
wants to merge 1 commit into from
Closed

fix(language-service): remove asserts for non-null expressions #16422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/language-service/src/ast_path.ts
View file
Open in desktop
Expand Up @@ -13,7 +13,9 @@ export class AstPath<T> {
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<N extends T>(ctor: {new (...args: any[]): N}): N|undefined {
Expand Down
155 changes: 81 additions & 74 deletions packages/language-service/src/completions.ts
View file
Open in desktop
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<Completion>(name => ({kind: 'component', name: name !, sort: name !}));
directiveElements.map<Completion>(name => ({kind: 'component', name, sort: name}));
let htmlElements = htmlNames.map<Completion>(name => ({kind: 'element', name: name, sort: name}));

// Return components and html elements
Expand Down Expand Up @@ -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 `<a|`; we only want attributes for `<a |` or after).
if (match && path.position >= 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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down
50 changes: 28 additions & 22 deletions packages/language-service/src/diagnostics.ts
View file
Open in desktop
Expand Up @@ -29,7 +29,7 @@ export function getTemplateDiagnostics(
results.push(...ast.parseErrors.map<Diagnostic>(
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) {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down
18 changes: 12 additions & 6 deletions packages/language-service/src/expressions.ts
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {},
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down