fix(compiler): type-checking error for duplicate variables in templates (#35674)

It's an error to declare a variable twice on a specific template:

```html
<div *ngFor="let i of items; let i = index">
</div>
```

This commit introduces a template type-checking error which helps to detect
and diagnose this problem.

Fixes #35186

PR Close #35674
This commit is contained in:
Alex Rickabaugh
2020-02-25 14:48:30 -08:00
committed by atscott
parent 1f8a243b67
commit 2c41bb8490
6 changed files with 83 additions and 0 deletions
@@ -122,6 +122,16 @@ export enum ErrorCode {
*/
WRITE_TO_READ_ONLY_VARIABLE = 8005,
/**
* A template variable was declared twice. For example:
*
* ```html
* <div *ngFor="let i of items; let i = index">
* </div>
* ```
*/
DUPLICATE_VARIABLE_DECLARATION = 8006,
/**
* An injectable already has a `ɵprov` property.
*/
@@ -50,6 +50,17 @@ export interface OutOfBandDiagnosticRecorder {
illegalAssignmentToTemplateVar(
templateId: TemplateId, assignment: PropertyWrite, target: TmplAstVariable): void;
/**
* Reports a duplicate declaration of a template variable.
*
* @param templateId the template type-checking ID of the template which contains the duplicate
* declaration.
* @param variable the `TmplAstVariable` which duplicates a previously declared variable.
* @param firstDecl the first variable declaration which uses the same name as `variable`.
*/
duplicateTemplateVar(
templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void;
}
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
@@ -100,4 +111,23 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
span: target.valueSpan || target.sourceSpan,
}));
}
duplicateTemplateVar(
templateId: TemplateId, variable: TmplAstVariable, firstDecl: TmplAstVariable): void {
const mapping = this.resolver.getSourceMapping(templateId);
const errorMsg =
`Cannot redeclare variable '${variable.name}' as it was previously declared elsewhere for the same template.`;
// The allocation of the error here is pretty useless for variables declared in microsyntax,
// since the sourceSpan refers to the entire microsyntax property, not a span for the specific
// variable in question.
//
// TODO(alxhub): allocate to a tighter span once one is available.
this._diagnostics.push(makeTemplateDiagnostic(
mapping, variable.sourceSpan, ts.DiagnosticCategory.Error,
ngErrorCode(ErrorCode.DUPLICATE_VARIABLE_DECLARATION), errorMsg, {
text: `The variable '${firstDecl.name}' was first declared here.`,
span: firstDecl.sourceSpan,
}));
}
}
@@ -738,7 +738,17 @@ class Scope {
// has.
if (templateOrNodes instanceof TmplAstTemplate) {
// The template's variable declarations need to be added as `TcbVariableOp`s.
const varMap = new Map<string, TmplAstVariable>();
for (const v of templateOrNodes.variables) {
// Validate that variables on the `TmplAstTemplate` are only declared once.
if (!varMap.has(v.name)) {
varMap.set(v.name, v);
} else {
const firstDecl = varMap.get(v.name) !;
tcb.oobRecorder.duplicateTemplateVar(tcb.id, v, firstDecl);
}
const opIndex = scope.opQueue.push(new TcbVariableOp(tcb, scope, templateOrNodes, v)) - 1;
scope.varMap.set(v, opIndex);
}
@@ -400,4 +400,5 @@ export class NoopOobRecorder implements OutOfBandDiagnosticRecorder {
missingReferenceTarget(): void {}
missingPipe(): void {}
illegalAssignmentToTemplateVar(): void {}
duplicateTemplateVar(): void {}
}