From 2b90cd532f00f996271fc574799b6b67a867788c Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Mon, 12 Dec 2016 15:59:12 -0800 Subject: [PATCH] fix(compiler): narrow the span reported for invalid pipes fixes #13326 closes #13411 --- .../compiler/src/expression_parser/parser.ts | 2 +- modules/@angular/compiler/src/parse_util.ts | 33 +++++++++++++++++++ .../src/template_parser/binding_parser.ts | 11 ++++--- .../template_parser/template_parser_spec.ts | 2 +- .../language-service/test/diagnostics_spec.ts | 12 +++++++ 5 files changed, 54 insertions(+), 6 deletions(-) diff --git a/modules/@angular/compiler/src/expression_parser/parser.ts b/modules/@angular/compiler/src/expression_parser/parser.ts index 423e0e7a7b..a4264b1df0 100644 --- a/modules/@angular/compiler/src/expression_parser/parser.ts +++ b/modules/@angular/compiler/src/expression_parser/parser.ts @@ -344,7 +344,7 @@ export class _ParseAST { while (this.optionalCharacter(chars.$COLON)) { args.push(this.parseExpression()); } - result = new BindingPipe(this.span(result.span.start - this.offset), result, name, args); + result = new BindingPipe(this.span(result.span.start), result, name, args); } while (this.optionalOperator('|')); } diff --git a/modules/@angular/compiler/src/parse_util.ts b/modules/@angular/compiler/src/parse_util.ts index 50f01e5de8..59e6b321da 100644 --- a/modules/@angular/compiler/src/parse_util.ts +++ b/modules/@angular/compiler/src/parse_util.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import * as chars from './chars'; import {isPresent} from './facade/lang'; export class ParseLocation { @@ -15,6 +16,38 @@ export class ParseLocation { toString(): string { return isPresent(this.offset) ? `${this.file.url}@${this.line}:${this.col}` : this.file.url; } + + moveBy(delta: number): ParseLocation { + const source = this.file.content; + const len = source.length; + let offset = this.offset; + let line = this.line; + let col = this.col; + while (offset > 0 && delta < 0) { + offset--; + delta++; + const ch = source.charCodeAt(offset); + if (ch == chars.$LF) { + line--; + const priorLine = source.substr(0, offset - 1).lastIndexOf(String.fromCharCode(chars.$LF)); + col = priorLine > 0 ? offset - priorLine : offset; + } else { + col--; + } + } + while (offset < len && delta > 0) { + const ch = source.charCodeAt(offset); + offset++; + delta--; + if (ch == chars.$LF) { + line++; + col = 0; + } else { + col++; + } + } + return new ParseLocation(this.file, offset, line, col); + } } export class ParseSourceFile { diff --git a/modules/@angular/compiler/src/template_parser/binding_parser.ts b/modules/@angular/compiler/src/template_parser/binding_parser.ts index 07b47770a4..7b51ac387f 100644 --- a/modules/@angular/compiler/src/template_parser/binding_parser.ts +++ b/modules/@angular/compiler/src/template_parser/binding_parser.ts @@ -377,9 +377,12 @@ export class BindingParser { if (isPresent(ast)) { const collector = new PipeCollector(); ast.visit(collector); - collector.pipes.forEach((pipeName) => { + collector.pipes.forEach((ast, pipeName) => { if (!this.pipesByName.has(pipeName)) { - this._reportError(`The pipe '${pipeName}' could not be found`, sourceSpan); + this._reportError( + `The pipe '${pipeName}' could not be found`, + new ParseSourceSpan( + sourceSpan.start.moveBy(ast.span.start), sourceSpan.start.moveBy(ast.span.end))); } }); } @@ -402,9 +405,9 @@ export class BindingParser { } export class PipeCollector extends RecursiveAstVisitor { - pipes = new Set(); + pipes = new Map(); visitPipe(ast: BindingPipe, context: any): any { - this.pipes.add(ast.name); + this.pipes.set(ast.name, ast); ast.exp.visit(this); this.visitAll(ast.args, context); return null; diff --git a/modules/@angular/compiler/test/template_parser/template_parser_spec.ts b/modules/@angular/compiler/test/template_parser/template_parser_spec.ts index 6810318baf..635a8788b6 100644 --- a/modules/@angular/compiler/test/template_parser/template_parser_spec.ts +++ b/modules/@angular/compiler/test/template_parser/template_parser_spec.ts @@ -1839,7 +1839,7 @@ Property binding a not used by any directive on an embedded template. Make sure it('should report pipes as error that have not been defined as dependencies', () => { expect(() => parse('{{a | test}}', [])).toThrowError(`Template parse errors: -The pipe 'test' could not be found ("[ERROR ->]{{a | test}}"): TestComp@0:0`); +The pipe 'test' could not be found ("{{[ERROR ->]a | test}}"): TestComp@0:2`); }); }); diff --git a/modules/@angular/language-service/test/diagnostics_spec.ts b/modules/@angular/language-service/test/diagnostics_spec.ts index 73e616c52f..7385cc4e93 100644 --- a/modules/@angular/language-service/test/diagnostics_spec.ts +++ b/modules/@angular/language-service/test/diagnostics_spec.ts @@ -151,6 +151,18 @@ describe('diagnostics', () => { }); }); + // Issue #13326 + it('should report a narrow span for invalid pipes', () => { + const code = + ` @Component({template: '

Using an invalid pipe {{data | dat}}

'}) export class MyComponent { data = 'some data'; }`; + addCode(code, fileName => { + const diagnostic = + ngService.getDiagnostics(fileName).filter(d => d.message.indexOf('pipe') > 0)[0]; + expect(diagnostic).not.toBeUndefined(); + expect(diagnostic.span.end - diagnostic.span.start).toBeLessThan(11); + }); + }); + function addCode(code: string, cb: (fileName: string, content?: string) => void) { const fileName = '/app/app.component.ts'; const originalContent = mockHost.getFileContent(fileName);