fix(ivy): don't crash on unknown pipe (#33454)
Previously the compiler would crash if a pipe was encountered which did not match any pipe in the scope of a template. This commit introduces a new diagnostic error for unknown pipes instead. PR Close #33454
This commit is contained in:
parent
9db59d010d
commit
38758d856a
@ -89,6 +89,11 @@ export enum ErrorCode {
|
|||||||
* No matching directive was found for a `#ref="target"` expression.
|
* No matching directive was found for a `#ref="target"` expression.
|
||||||
*/
|
*/
|
||||||
MISSING_REFERENCE_TARGET = 8003,
|
MISSING_REFERENCE_TARGET = 8003,
|
||||||
|
|
||||||
|
/**
|
||||||
|
* No matching pipe was found for a
|
||||||
|
*/
|
||||||
|
MISSING_PIPE = 8004,
|
||||||
}
|
}
|
||||||
|
|
||||||
export function ngErrorCode(code: ErrorCode): number {
|
export function ngErrorCode(code: ErrorCode): number {
|
||||||
|
@ -5,7 +5,7 @@
|
|||||||
* Use of this source code is governed by an MIT-style license that can be
|
* 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
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
import {ParseSourceSpan, ParseSpan, Position} from '@angular/compiler';
|
import {AbsoluteSourceSpan, ParseSourceSpan, ParseSpan, Position} from '@angular/compiler';
|
||||||
import * as ts from 'typescript';
|
import * as ts from 'typescript';
|
||||||
|
|
||||||
import {getTokenAtPosition} from '../../util/src/typescript';
|
import {getTokenAtPosition} from '../../util/src/typescript';
|
||||||
@ -55,6 +55,11 @@ export function toAbsoluteSpan(span: ParseSpan, sourceSpan: ParseSourceSpan): Ab
|
|||||||
return <AbsoluteSpan>{start: span.start + offset, end: span.end + offset};
|
return <AbsoluteSpan>{start: span.start + offset, end: span.end + offset};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function absoluteSourceSpanToSourceLocation(
|
||||||
|
id: string, span: AbsoluteSourceSpan): SourceLocation {
|
||||||
|
return {id, ...span};
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Wraps the node in parenthesis such that inserted span comments become attached to the proper
|
* Wraps the node in parenthesis such that inserted span comments become attached to the proper
|
||||||
* node. This is an alias for `ts.createParen` with the benefit that it signifies that the
|
* node. This is an alias for `ts.createParen` with the benefit that it signifies that the
|
||||||
|
@ -6,12 +6,14 @@
|
|||||||
* found in the LICENSE file at https://angular.io/license
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import {TmplAstReference} from '@angular/compiler';
|
import {AbsoluteSourceSpan, BindingPipe, TmplAstReference} from '@angular/compiler';
|
||||||
import * as ts from 'typescript';
|
import * as ts from 'typescript';
|
||||||
|
|
||||||
import {ErrorCode, ngErrorCode} from '../../diagnostics';
|
import {ErrorCode, ngErrorCode} from '../../diagnostics';
|
||||||
|
|
||||||
import {TcbSourceResolver, makeTemplateDiagnostic} from './diagnostics';
|
import {TcbSourceResolver, absoluteSourceSpanToSourceLocation, makeTemplateDiagnostic} from './diagnostics';
|
||||||
|
|
||||||
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Collects `ts.Diagnostic`s on problems which occur in the template which aren't directly sourced
|
* Collects `ts.Diagnostic`s on problems which occur in the template which aren't directly sourced
|
||||||
@ -34,6 +36,19 @@ export interface OutOfBandDiagnosticRecorder {
|
|||||||
* @param ref the `TmplAstReference` which could not be matched to a directive.
|
* @param ref the `TmplAstReference` which could not be matched to a directive.
|
||||||
*/
|
*/
|
||||||
missingReferenceTarget(templateId: string, ref: TmplAstReference): void;
|
missingReferenceTarget(templateId: string, ref: TmplAstReference): void;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Reports usage of a `| pipe` expression in the template for which the named pipe could not be
|
||||||
|
* found.
|
||||||
|
*
|
||||||
|
* @param templateId the template type-checking ID of the template which contains the unknown
|
||||||
|
* pipe.
|
||||||
|
* @param ast the `BindingPipe` invocation of the pipe which could not be found.
|
||||||
|
* @param sourceSpan the `AbsoluteSourceSpan` of the pipe invocation (ideally, this should be the
|
||||||
|
* source span of the pipe's name). This depends on the source span of the `BindingPipe` itself
|
||||||
|
* plus span of the larger expression context.
|
||||||
|
*/
|
||||||
|
missingPipe(templateId: string, ast: BindingPipe, sourceSpan: AbsoluteSourceSpan): void;
|
||||||
}
|
}
|
||||||
|
|
||||||
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
|
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
|
||||||
@ -52,4 +67,19 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
|
|||||||
mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error,
|
mapping, ref.valueSpan || ref.sourceSpan, ts.DiagnosticCategory.Error,
|
||||||
ngErrorCode(ErrorCode.MISSING_REFERENCE_TARGET), errorMsg));
|
ngErrorCode(ErrorCode.MISSING_REFERENCE_TARGET), errorMsg));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
missingPipe(templateId: string, ast: BindingPipe, absSpan: AbsoluteSourceSpan): void {
|
||||||
|
const mapping = this.resolver.getSourceMapping(templateId);
|
||||||
|
const errorMsg = `No pipe found with name '${ast.name}'.`;
|
||||||
|
|
||||||
|
const location = absoluteSourceSpanToSourceLocation(templateId, absSpan);
|
||||||
|
const sourceSpan = this.resolver.sourceLocationToSpan(location);
|
||||||
|
if (sourceSpan === null) {
|
||||||
|
throw new Error(
|
||||||
|
`Assertion failure: no SourceLocation found for usage of pipe '${ast.name}'.`);
|
||||||
|
}
|
||||||
|
this._diagnostics.push(makeTemplateDiagnostic(
|
||||||
|
mapping, sourceSpan, ts.DiagnosticCategory.Error, ngErrorCode(ErrorCode.MISSING_PIPE),
|
||||||
|
errorMsg));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
@ -589,9 +589,9 @@ export class Context {
|
|||||||
*/
|
*/
|
||||||
allocateId(): ts.Identifier { return ts.createIdentifier(`_t${this.nextId++}`); }
|
allocateId(): ts.Identifier { return ts.createIdentifier(`_t${this.nextId++}`); }
|
||||||
|
|
||||||
getPipeByName(name: string): ts.Expression {
|
getPipeByName(name: string): ts.Expression|null {
|
||||||
if (!this.pipes.has(name)) {
|
if (!this.pipes.has(name)) {
|
||||||
throw new Error(`Missing pipe: ${name}`);
|
return null;
|
||||||
}
|
}
|
||||||
return this.env.pipeInst(this.pipes.get(name) !);
|
return this.env.pipeInst(this.pipes.get(name) !);
|
||||||
}
|
}
|
||||||
@ -988,9 +988,17 @@ class TcbExpressionTranslator {
|
|||||||
return ts.createIdentifier('ctx');
|
return ts.createIdentifier('ctx');
|
||||||
} else if (ast instanceof BindingPipe) {
|
} else if (ast instanceof BindingPipe) {
|
||||||
const expr = this.translate(ast.exp);
|
const expr = this.translate(ast.exp);
|
||||||
let pipe: ts.Expression;
|
let pipe: ts.Expression|null;
|
||||||
if (this.tcb.env.config.checkTypeOfPipes) {
|
if (this.tcb.env.config.checkTypeOfPipes) {
|
||||||
pipe = this.tcb.getPipeByName(ast.name);
|
pipe = this.tcb.getPipeByName(ast.name);
|
||||||
|
if (pipe === null) {
|
||||||
|
// No pipe by that name exists in scope. Record this as an error.
|
||||||
|
const nameAbsoluteSpan = toAbsoluteSpan(ast.nameSpan, this.sourceSpan);
|
||||||
|
this.tcb.oobRecorder.missingPipe(this.tcb.id, ast, nameAbsoluteSpan);
|
||||||
|
|
||||||
|
// Return an 'any' value to at least allow the rest of the expression to be checked.
|
||||||
|
pipe = NULL_AS_ANY;
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
pipe = ts.createParen(ts.createAsExpression(
|
pipe = ts.createParen(ts.createAsExpression(
|
||||||
ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)));
|
ts.createNull(), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)));
|
||||||
|
@ -387,4 +387,5 @@ export class NoopSchemaChecker implements DomSchemaChecker {
|
|||||||
export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
|
export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
|
||||||
get diagnostics(): ReadonlyArray<ts.Diagnostic> { return []; }
|
get diagnostics(): ReadonlyArray<ts.Diagnostic> { return []; }
|
||||||
missingReferenceTarget(): void {}
|
missingReferenceTarget(): void {}
|
||||||
|
missingPipe(): void {}
|
||||||
}
|
}
|
||||||
|
@ -735,6 +735,29 @@ export declare class AnimationEvent {
|
|||||||
expect(getSourceCodeForDiagnostic(diags[0])).toBe('unknownTarget');
|
expect(getSourceCodeForDiagnostic(diags[0])).toBe('unknownTarget');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should report an error with an unknown pipe', () => {
|
||||||
|
env.write('test.ts', `
|
||||||
|
import {Component, NgModule} from '@angular/core';
|
||||||
|
|
||||||
|
@Component({
|
||||||
|
selector: 'test',
|
||||||
|
template: '{{expr | unknown}}',
|
||||||
|
})
|
||||||
|
class TestCmp {
|
||||||
|
expr = 3;
|
||||||
|
}
|
||||||
|
|
||||||
|
@NgModule({
|
||||||
|
declarations: [TestCmp],
|
||||||
|
})
|
||||||
|
class Module {}
|
||||||
|
`);
|
||||||
|
const diags = env.driveDiagnostics();
|
||||||
|
expect(diags.length).toBe(1);
|
||||||
|
expect(diags[0].messageText).toBe(`No pipe found with name 'unknown'.`);
|
||||||
|
expect(getSourceCodeForDiagnostic(diags[0])).toBe('unknown');
|
||||||
|
});
|
||||||
|
|
||||||
it('should report an error with pipe bindings', () => {
|
it('should report an error with pipe bindings', () => {
|
||||||
env.write('test.ts', `
|
env.write('test.ts', `
|
||||||
import {CommonModule} from '@angular/common';
|
import {CommonModule} from '@angular/common';
|
||||||
|
@ -145,7 +145,7 @@ export class KeyedWrite extends AST {
|
|||||||
export class BindingPipe extends AST {
|
export class BindingPipe extends AST {
|
||||||
constructor(
|
constructor(
|
||||||
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string,
|
span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public exp: AST, public name: string,
|
||||||
public args: any[]) {
|
public args: any[], public nameSpan: ParseSpan) {
|
||||||
super(span, sourceSpan);
|
super(span, sourceSpan);
|
||||||
}
|
}
|
||||||
visit(visitor: AstVisitor, context: any = null): any { return visitor.visitPipe(this, context); }
|
visit(visitor: AstVisitor, context: any = null): any { return visitor.visitPipe(this, context); }
|
||||||
@ -484,7 +484,8 @@ export class AstTransformer implements AstVisitor {
|
|||||||
|
|
||||||
visitPipe(ast: BindingPipe, context: any): AST {
|
visitPipe(ast: BindingPipe, context: any): AST {
|
||||||
return new BindingPipe(
|
return new BindingPipe(
|
||||||
ast.span, ast.sourceSpan, ast.exp.visit(this), ast.name, this.visitAll(ast.args));
|
ast.span, ast.sourceSpan, ast.exp.visit(this), ast.name, this.visitAll(ast.args),
|
||||||
|
ast.nameSpan);
|
||||||
}
|
}
|
||||||
|
|
||||||
visitKeyedRead(ast: KeyedRead, context: any): AST {
|
visitKeyedRead(ast: KeyedRead, context: any): AST {
|
||||||
@ -635,7 +636,7 @@ export class AstMemoryEfficientTransformer implements AstVisitor {
|
|||||||
const exp = ast.exp.visit(this);
|
const exp = ast.exp.visit(this);
|
||||||
const args = this.visitAll(ast.args);
|
const args = this.visitAll(ast.args);
|
||||||
if (exp !== ast.exp || args !== ast.args) {
|
if (exp !== ast.exp || args !== ast.args) {
|
||||||
return new BindingPipe(ast.span, ast.sourceSpan, exp, ast.name, args);
|
return new BindingPipe(ast.span, ast.sourceSpan, exp, ast.name, args, ast.nameSpan);
|
||||||
}
|
}
|
||||||
return ast;
|
return ast;
|
||||||
}
|
}
|
||||||
|
@ -348,13 +348,16 @@ export class _ParseAST {
|
|||||||
}
|
}
|
||||||
|
|
||||||
do {
|
do {
|
||||||
|
const nameStart = this.inputIndex;
|
||||||
const name = this.expectIdentifierOrKeyword();
|
const name = this.expectIdentifierOrKeyword();
|
||||||
|
const nameSpan = this.span(nameStart);
|
||||||
const args: AST[] = [];
|
const args: AST[] = [];
|
||||||
while (this.optionalCharacter(chars.$COLON)) {
|
while (this.optionalCharacter(chars.$COLON)) {
|
||||||
args.push(this.parseExpression());
|
args.push(this.parseExpression());
|
||||||
}
|
}
|
||||||
const {start} = result.span;
|
const {start} = result.span;
|
||||||
result = new BindingPipe(this.span(start), this.sourceSpan(start), result, name, args);
|
result =
|
||||||
|
new BindingPipe(this.span(start), this.sourceSpan(start), result, name, args, nameSpan);
|
||||||
} while (this.optionalOperator('|'));
|
} while (this.optionalOperator('|'));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user