Skip to content

Commit

Permalink
fix(compiler-cli): report error when a reference target is missing in…
Browse files Browse the repository at this point in the history
…stead of crashing (angular#39805)

If a template declares a reference to a missing target then referring to
that reference from elsewhere in the template would crash the template
type checker, due to a regression introduced in angular#38618. This commit
fixes the crash by ensuring that the invalid reference will resolve to
a variable of type any.

Fixes angular#39744

PR Close angular#39805
  • Loading branch information
JoostK authored and AndrewKushnir committed Nov 24, 2020
1 parent b5c0f9d commit 453b32f
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 4 deletions.
30 changes: 26 additions & 4 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Expand Up @@ -458,6 +458,26 @@ class TcbReferenceOp extends TcbOp {
}
}

/**
* A `TcbOp` which is used when the target of a reference is missing. This operation generates a
* variable of type any for usages of the invalid reference to resolve to. The invalid reference
* itself is recorded out-of-band.
*/
class TcbInvalidReferenceOp extends TcbOp {
constructor(private readonly tcb: Context, private readonly scope: Scope) {
super();
}

// The declaration of a missing reference is only needed when the reference is resolved.
readonly optional = true;

execute(): ts.Identifier {
const id = this.tcb.allocateId();
this.scope.addStatement(tsCreateVariable(id, NULL_AS_ANY));
return id;
}
}

/**
* A `TcbOp` which constructs an instance of a directive with types inferred from its inputs. The
* inputs themselves are not checked here; checking of inputs is achieved in `TcbDirectiveInputsOp`.
Expand Down Expand Up @@ -1353,13 +1373,15 @@ class Scope {
private checkAndAppendReferencesOfNode(node: TmplAstElement|TmplAstTemplate): void {
for (const ref of node.references) {
const target = this.tcb.boundTarget.getReferenceTarget(ref);

let ctxIndex: number;
if (target === null) {
// The reference is invalid if it doesn't have a target, so report it as an error.
this.tcb.oobRecorder.missingReferenceTarget(this.tcb.id, ref);
continue;
}

let ctxIndex: number;
if (target instanceof TmplAstTemplate || target instanceof TmplAstElement) {
// Any usages of the invalid reference will be resolved to a variable of type any.
ctxIndex = this.opQueue.push(new TcbInvalidReferenceOp(this.tcb, this)) - 1;
} else if (target instanceof TmplAstTemplate || target instanceof TmplAstElement) {
ctxIndex = this.opQueue.push(new TcbReferenceOp(this.tcb, this, ref, node, target)) - 1;
} else {
ctxIndex =
Expand Down
23 changes: 23 additions & 0 deletions packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts
Expand Up @@ -1080,6 +1080,29 @@ export declare class AnimationEvent {
expect(getSourceCodeForDiagnostic(diags[0])).toBe('unknownTarget');
});

it('should treat an unknown local ref target as type any', () => {
env.write('test.ts', `
import {Component, NgModule} from '@angular/core';
@Component({
selector: 'test',
template: '<div #ref="unknownTarget">{{ use(ref) }}</div>',
})
class TestCmp {
use(ref: string): string { return ref; }
}
@NgModule({
declarations: [TestCmp],
})
class Module {}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText).toBe(`No directive found with exportAs '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';
Expand Down

0 comments on commit 453b32f

Please sign in to comment.