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.

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

feat(language-service): warn when a method isn't called in an event #13437

Merged
merged 1 commit into from Dec 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 7 additions & 6 deletions modules/@angular/language-service/src/diagnostics.ts
Expand Up @@ -208,12 +208,13 @@ class ExpressionDiagnosticsVisitor extends TemplateAstChildVisitor {
private diagnoseExpression(ast: AST, offset: number, includeEvent: boolean) {
const scope = this.getExpressionScope(this.path, includeEvent);
this.diagnostics.push(
...getExpressionDiagnostics(scope, ast, this.info.template.query)
.map(d => ({
span: offsetSpan(d.ast.span, offset + this.info.template.span.start),
kind: d.kind,
message: d.message
})));
...getExpressionDiagnostics(scope, ast, this.info.template.query, {
event: includeEvent
}).map(d => ({
span: offsetSpan(d.ast.span, offset + this.info.template.span.start),
kind: d.kind,
message: d.message
})));
}

private push(ast: TemplateAst) { this.path.push(ast); }
Expand Down
22 changes: 15 additions & 7 deletions modules/@angular/language-service/src/expressions.ts
Expand Up @@ -16,9 +16,12 @@ import {TemplateAstChildVisitor, TemplateAstPath} from './template_path';
import {BuiltinType, CompletionKind, Definition, DiagnosticKind, Signature, Span, Symbol, SymbolDeclaration, SymbolQuery, SymbolTable} from './types';
import {inSpan, spanOf} from './utils';

export interface ExpressionDiagnosticsContext { event?: boolean; }

export function getExpressionDiagnostics(
scope: SymbolTable, ast: AST, query: SymbolQuery): TypeDiagnostic[] {
const analyzer = new AstType(scope, query);
scope: SymbolTable, ast: AST, query: SymbolQuery,
context: ExpressionDiagnosticsContext = {}): TypeDiagnostic[] {
const analyzer = new AstType(scope, query, context);
analyzer.getDiagnostics(ast);
return analyzer.diagnostics;
}
Expand All @@ -30,7 +33,7 @@ export function getExpressionCompletions(
const tail = path.tail;
let result: SymbolTable|undefined = scope;

function getType(ast: AST): Symbol { return new AstType(scope, query).getType(ast); }
function getType(ast: AST): Symbol { return new AstType(scope, query, {}).getType(ast); }

// If the completion request is in a not in a pipe or property access then the global scope
// (that is the scope of the implicit receiver) is the right scope as the user is typing the
Expand Down Expand Up @@ -88,7 +91,7 @@ export function getExpressionSymbol(
if (path.empty) return undefined;
const tail = path.tail;

function getType(ast: AST): Symbol { return new AstType(scope, query).getType(ast); }
function getType(ast: AST): Symbol { return new AstType(scope, query, {}).getType(ast); }

let symbol: Symbol = undefined;
let span: Span = undefined;
Expand Down Expand Up @@ -189,13 +192,18 @@ export class TypeDiagnostic {
class AstType implements ExpressionVisitor {
public diagnostics: TypeDiagnostic[];

constructor(private scope: SymbolTable, private query: SymbolQuery) {}
constructor(
private scope: SymbolTable, private query: SymbolQuery,
private context: ExpressionDiagnosticsContext) {}

getType(ast: AST): Symbol { return ast.visit(this); }

getDiagnostics(ast: AST): TypeDiagnostic[] {
this.diagnostics = [];
ast.visit(this);
const type: Symbol = ast.visit(this);
if (this.context.event && type.callable) {
this.reportWarning('Unexpected callable expression. Expected a method call', ast);
}
return this.diagnostics;
}

Expand Down Expand Up @@ -746,7 +754,7 @@ function refinedVariableType(
const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf');
if (ngForOfBinding) {
const bindingType =
new AstType(info.template.members, info.template.query).getType(ngForOfBinding.value);
new AstType(info.template.members, info.template.query, {}).getType(ngForOfBinding.value);
if (bindingType) {
return info.template.query.getElementType(bindingType);
}
Expand Down
11 changes: 11 additions & 0 deletions modules/@angular/language-service/test/diagnostics_spec.ts
Expand Up @@ -130,6 +130,17 @@ describe('diagnostics', () => {
});
});

it('should report a warning if an event results in a callable expression', () => {
const code =
` @Component({template: \`<div (click)="onClick"></div>\`}) export class MyComponent { onClick() { } }`;
addCode(code, (fileName, content) => {
const diagnostics = ngService.getDiagnostics(fileName);
includeDiagnostic(
diagnostics, 'Unexpected callable expression. Expected a method call', 'onClick',
content);
});
});

function addCode(code: string, cb: (fileName: string, content?: string) => void) {
const fileName = '/app/app.component.ts';
const originalContent = mockHost.getFileContent(fileName);
Expand Down