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

fix(compiler): throw an error if variable with the same name is already defined. #7209

Merged
merged 5 commits into from May 26, 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
31 changes: 28 additions & 3 deletions modules/@angular/compiler/src/template_parser.ts
Expand Up @@ -140,21 +140,24 @@ export class TemplateParser {
directives: CompileDirectiveMetadata[], pipes: CompilePipeMetadata[],
templateUrl: string): TemplateParseResult {
var htmlAstWithErrors = this._htmlParser.parse(template, templateUrl);
var errors: ParseError[] = htmlAstWithErrors.errors;
var errors:ParseError[] = htmlAstWithErrors.errors;
var result;
if (htmlAstWithErrors.rootNodes.length > 0) {
var uniqDirectives = <CompileDirectiveMetadata[]>removeDuplicates(directives);
var uniqPipes = <CompilePipeMetadata[]>removeDuplicates(pipes);
var providerViewContext =
new ProviderViewContext(component, htmlAstWithErrors.rootNodes[0].sourceSpan);
new ProviderViewContext(component, htmlAstWithErrors.rootNodes[0].sourceSpan);
var parseVisitor = new TemplateParseVisitor(providerViewContext, uniqDirectives, uniqPipes,
this._exprParser, this._schemaRegistry);
this._exprParser, this._schemaRegistry);

result = htmlVisitAll(parseVisitor, htmlAstWithErrors.rootNodes, EMPTY_ELEMENT_CONTEXT);
errors = errors.concat(parseVisitor.errors).concat(providerViewContext.errors);
} else {
result = [];
}

this._assertNoReferenceDuplicationOnTemplate(result, errors);

if (errors.length > 0) {
return new TemplateParseResult(result, errors);
}
Expand All @@ -164,6 +167,25 @@ export class TemplateParser {
}
return new TemplateParseResult(result, errors);
}

_assertNoReferenceDuplicationOnTemplate(result:any[], errors:TemplateParseError[]):void {
const existingReferences = [];
result
.filter(element => !!element.references)
.forEach(element => element.references.forEach(reference=> {
const name = reference.name;
if (existingReferences.indexOf(name) < 0) {
existingReferences.push(name);
}
else {
const error = new TemplateParseError(
`Reference "#${name}" is defined several times`,
reference.sourceSpan,
ParseErrorLevel.FATAL);
errors.push(error);
}
}));
}
}

class TemplateParseVisitor implements HtmlAstVisitor {
Expand Down Expand Up @@ -522,9 +544,11 @@ class TemplateParseVisitor implements HtmlAstVisitor {

private _parseVariable(identifier: string, value: string, sourceSpan: ParseSourceSpan,
targetVars: VariableAst[]) {

if (identifier.indexOf('-') > -1) {
this._reportError(`"-" is not allowed in variable names`, sourceSpan);
}

targetVars.push(new VariableAst(identifier, value, sourceSpan));
}

Expand All @@ -533,6 +557,7 @@ class TemplateParseVisitor implements HtmlAstVisitor {
if (identifier.indexOf('-') > -1) {
this._reportError(`"-" is not allowed in reference names`, sourceSpan);
}

targetRefs.push(new ElementOrDirectiveRef(identifier, value, sourceSpan));
}

Expand Down
10 changes: 10 additions & 0 deletions modules/@angular/compiler/test/template_parser_spec.ts
Expand Up @@ -822,6 +822,16 @@ There is no directive with "exportAs" set to "dirA" ("<div [ERROR ->]#a="dirA"><
"let-" is only supported on template elements. ("<div [ERROR ->]let-a></div>"): TestComp@0:5`);
});

it('should report duplicate reference names', () => {

expect(() => parse('<div #a></div><div #a></div>', []))

.toThrowError(`Template parse errors:
Reference "#a" is defined several times ("<div #a></div><div [ERROR ->]#a></div>"): TestComp@0:19`);

});




 it('should not throw error when there is same reference name in different templates', () => {

expect(() => parse('<div #a><template #a><span>OK</span></template></div>', [])).not.toThrowError();

});

it('should assign references with empty value to components', () => {
var dirA = CompileDirectiveMetadata.create({
selector: '[a]',
Expand Down
4 changes: 2 additions & 2 deletions modules/@angular/core/test/linker/query_integration_spec.ts
Expand Up @@ -1059,11 +1059,11 @@ class NeedsContentChildWithRead {

@Component({
selector: 'needs-view-children-read',
template: '<div #q text="va"></div><div #q text="vb"></div>',
template: '<div #q text="va"></div><div #w text="vb"></div>',
directives: [TextDirective]
})
class NeedsViewChildrenWithRead {
@ViewChildren('q', {read: TextDirective}) textDirChildren: QueryList<TextDirective>;
@ViewChildren('q,w', {read: TextDirective}) textDirChildren: QueryList<TextDirective>;
@ViewChildren('nonExisting', {read: TextDirective}) nonExistingVar: QueryList<TextDirective>;
}

Expand Down