fix(compiler): report errors for missing binding names (#34595)

Currently, would-be binding attributes that are missing binding names
are not parsed as bindings, and fall through as regular attributes. In
some cases, this can lead to a runtime error; trying to assign `#` as a
DOM attribute in an element like in `<div #></div>` fails because `#` is
not a valid attribute name.

Attributes composed of binding prefixes but not defining a binding
should be considered invalid, as this almost certainly indicates an
unintentional elision of a binding by the developer. This commit
introduces error reporting for attributes with a binding name prefix but
no actual binding name.

Closes https://github.com/angular/vscode-ng-language-service/issues/293.

PR Close #34595

packages/compiler/src/render3/r3_template_transform.ts

@@ -21,7 +21,7 @@ import * as t from './r3_ast';
 import {I18N_ICU_VAR_PREFIX, isI18nRootNode} from './view/i18n/util';
 
 const BIND_NAME_REGEXP =
-    /^(?:(?:(?:(bind-)|(let-)|(ref-|#)|(on-)|(bindon-)|(@))(.+))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/;
+    /^(?:(?:(?:(bind-)|(let-)|(ref-|#)|(on-)|(bindon-)|(@))(.*))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/;
 
 // Group 1 = "bind-"
 const KW_BIND_IDX = 1;
@@ -399,7 +399,10 @@ class HtmlAstToIvyAst implements html.Visitor {
       valueSpan: ParseSourceSpan|undefined, variables: t.Variable[]) {
     if (identifier.indexOf('-') > -1) {
       this.reportError(`"-" is not allowed in variable names`, sourceSpan);
+    } else if (identifier.length === 0) {
+      this.reportError(`Variable does not have a name`, sourceSpan);
     }
+
     variables.push(new t.Variable(identifier, value, sourceSpan, valueSpan));
   }
 
@@ -408,6 +411,8 @@ class HtmlAstToIvyAst implements html.Visitor {
       valueSpan: ParseSourceSpan|undefined, references: t.Reference[]) {
     if (identifier.indexOf('-') > -1) {
       this.reportError(`"-" is not allowed in reference names`, sourceSpan);
+    } else if (identifier.length === 0) {
+      this.reportError(`Reference does not have a name`, sourceSpan);
     }
 
     references.push(new t.Reference(identifier, value, sourceSpan, valueSpan));

packages/compiler/src/template_parser/binding_parser.ts

@@ -203,6 +203,10 @@ export class BindingParser {
       name: string, expression: string, isHost: boolean, sourceSpan: ParseSourceSpan,
       absoluteOffset: number, valueSpan: ParseSourceSpan|undefined,
       targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) {
+    if (name.length === 0) {
+      this._reportError(`Property name is missing in binding`, sourceSpan);
+    }
+
     let isAnimationProp = false;
     if (name.startsWith(ANIMATE_PROP_PREFIX)) {
       isAnimationProp = true;
@@ -248,6 +252,10 @@ export class BindingParser {
       name: string, expression: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number,
       valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
       targetProps: ParsedProperty[]) {
+    if (name.length === 0) {
+      this._reportError('Animation trigger is missing', sourceSpan);
+    }
+
     // This will occur when a @trigger is not paired with an expression.
     // For animations it is valid to not have an expression since */void
     // states will be applied by angular when the element is attached/detached
@@ -343,6 +351,10 @@ export class BindingParser {
   parseEvent(
       name: string, expression: string, sourceSpan: ParseSourceSpan, handlerSpan: ParseSourceSpan,
       targetMatchableAttrs: string[][], targetEvents: ParsedEvent[]) {
+    if (name.length === 0) {
+      this._reportError(`Event name is missing in binding`, sourceSpan);
+    }
+
     if (isAnimationLabel(name)) {
       name = name.substr(1);
       this._parseAnimationEvent(name, expression, sourceSpan, handlerSpan, targetEvents);

packages/compiler/src/template_parser/template_parser.ts

@@ -31,7 +31,7 @@ import * as t from './template_ast';
 import {PreparsedElementType, preparseElement} from './template_preparser';
 
 const BIND_NAME_REGEXP =
-    /^(?:(?:(?:(bind-)|(let-)|(ref-|#)|(on-)|(bindon-)|(@))(.+))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/;
+    /^(?:(?:(?:(bind-)|(let-)|(ref-|#)|(on-)|(bindon-)|(@))(.*))|\[\(([^\)]+)\)\]|\[([^\]]+)\]|\(([^\)]+)\))$/;
 
 // Group 1 = "bind-"
 const KW_BIND_IDX = 1;
@@ -501,6 +501,8 @@ class TemplateParseVisitor implements html.Visitor {
       identifier: string, value: string, sourceSpan: ParseSourceSpan, targetVars: t.VariableAst[]) {
     if (identifier.indexOf('-') > -1) {
       this._reportError(`"-" is not allowed in variable names`, sourceSpan);
+    } else if (identifier.length === 0) {
+      this._reportError(`Variable does not have a name`, sourceSpan);
     }
 
     targetVars.push(new t.VariableAst(identifier, value, sourceSpan));
@@ -511,6 +513,8 @@ class TemplateParseVisitor implements html.Visitor {
       targetRefs: ElementOrDirectiveRef[]) {
     if (identifier.indexOf('-') > -1) {
       this._reportError(`"-" is not allowed in reference names`, sourceSpan);
+    } else if (identifier.length === 0) {
+      this._reportError(`Reference does not have a name`, sourceSpan);
     }
 
     targetRefs.push(new ElementOrDirectiveRef(identifier, value, sourceSpan));

packages/compiler/test/render3/r3_template_transform_spec.ts

@@ -167,6 +167,10 @@ describe('R3 template transform', () => {
       ]);
     });
 
+    it('should report missing property names in bind- syntax', () => {
+      expect(() => parse('<div bind-></div>')).toThrowError(/Property name is missing in binding/);
+    });
+
     it('should parse bound properties via {{...}}', () => {
       expectFromHtml('<div prop="{{v}}"></div>').toEqual([
         ['Element', 'div'],
@@ -339,6 +343,10 @@ describe('R3 template transform', () => {
       ]);
     });
 
+    it('should report missing event names in on- syntax', () => {
+      expect(() => parse('<div on-></div>')).toThrowError(/Event name is missing in binding/);
+    });
+
     it('should parse bound events and properties via [(...)]', () => {
       expectFromHtml('<div [(prop)]="v"></div>').toEqual([
         ['Element', 'div'],
@@ -355,12 +363,29 @@ describe('R3 template transform', () => {
       ]);
     });
 
+    it('should report missing property names in bindon- syntax', () => {
+      expect(() => parse('<div bindon-></div>'))
+          .toThrowError(/Property name is missing in binding/);
+    });
+
     it('should report an error on empty expression', () => {
       expect(() => parse('<div (event)="">')).toThrowError(/Empty expressions are not allowed/);
       expect(() => parse('<div (event)="   ">')).toThrowError(/Empty expressions are not allowed/);
     });
   });
 
+  describe('variables', () => {
+    it('should report variables not on template elements', () => {
+      expect(() => parse('<div let-a-name="b"></div>'))
+          .toThrowError(/"let-" is only supported on ng-template elements./);
+    });
+
+    it('should report missing variable names', () => {
+      expect(() => parse('<ng-template let-><ng-template>'))
+          .toThrowError(/Variable does not have a name/);
+    });
+  });
+
   describe('references', () => {
     it('should parse references via #...', () => {
       expectFromHtml('<div #a></div>').toEqual([
@@ -382,6 +407,20 @@ describe('R3 template transform', () => {
         ['Reference', 'someA', ''],
       ]);
     });
+
+    it('should report invalid reference names', () => {
+      expect(() => parse('<div #a-b></div>')).toThrowError(/"-" is not allowed in reference names/);
+    });
+
+    it('should report missing reference names', () => {
+      expect(() => parse('<div #></div>')).toThrowError(/Reference does not have a name/);
+    });
+  });
+
+  describe('literal attribute', () => {
+    it('should report missing animation trigger in @ syntax', () => {
+      expect(() => parse('<div @></div>')).toThrowError(/Animation trigger is missing/);
+    });
   });
 
   describe('ng-content', () => {

packages/compiler/test/template_parser/template_parser_spec.ts

@@ -658,6 +658,11 @@ Binding to attribute 'onEvent' is disallowed for security reasons ("<my-componen
           ]);
         });
 
+        it('should report missing property names in bind- syntax', () => {
+          expect(() => parse('<div bind-></div>', [])).toThrowError(`Template parse errors:
+Property name is missing in binding ("<div [ERROR ->]bind-></div>"): TestComp@0:5`);
+        });
+
         it('should parse bound properties via {{...}} and not report them as attributes', () => {
           expect(humanizeTplAst(parse('<div prop="{{v}}">', []))).toEqual([
             [ElementAst, 'div'],
@@ -684,6 +689,11 @@ Binding to attribute 'onEvent' is disallowed for security reasons ("<my-componen
                      /Assigning animation triggers via @prop="exp" attributes with an expression is invalid. Use property bindings \(e.g. \[@prop\]="exp"\) or use an attribute without a value \(e.g. @prop\) instead. \("<div \[ERROR ->\]@someAnimation="value2">"\): TestComp@0:5/);
            });
 
+        it('should report missing animation trigger in @ syntax', () => {
+          expect(() => parse('<div @></div>', [])).toThrowError(`Template parse errors:
+Animation trigger is missing ("<div [ERROR ->]@></div>"): TestComp@0:5`);
+        });
+
         it('should not issue a warning when host attributes contain a valid property-bound animation trigger',
            () => {
              const animationEntries = ['prop'];
@@ -804,6 +814,11 @@ Binding to attribute 'onEvent' is disallowed for security reasons ("<my-componen
           ]))).toEqual([[ElementAst, 'div'], [BoundEventAst, 'event', null, 'v']]);
         });
 
+        it('should report missing event names in on- syntax', () => {
+          expect(() => parse('<div on-></div>', []))
+              .toThrowError(/Event name is missing in binding/);
+        });
+
         it('should allow events on explicit embedded templates that are emitted by a directive',
            () => {
              const dirA =
@@ -840,6 +855,10 @@ Binding to attribute 'onEvent' is disallowed for security reasons ("<my-componen
              ]);
            });
 
+        it('should report missing property names in bindon- syntax', () => {
+          expect(() => parse('<div bindon-></div>', []))
+              .toThrowError(/Property name is missing in binding/);
+        });
       });
 
       describe('directives', () => {
@@ -1372,11 +1391,22 @@ There is no directive with "exportAs" set to "dirA" ("<div [ERROR ->]#a="dirA"><
 "-" is not allowed in reference names ("<div [ERROR ->]#a-b></div>"): TestComp@0:5`);
         });
 
+        it('should report missing reference names', () => {
+          expect(() => parse('<div #></div>', [])).toThrowError(`Template parse errors:
+Reference does not have a name ("<div [ERROR ->]#></div>"): TestComp@0:5`);
+        });
+
         it('should report variables as errors', () => {
           expect(() => parse('<div let-a></div>', [])).toThrowError(`Template parse errors:
 "let-" is only supported on ng-template elements. ("<div [ERROR ->]let-a></div>"): TestComp@0:5`);
         });
 
+        it('should report missing variable names', () => {
+          expect(() => parse('<ng-template let-></ng-template>', []))
+              .toThrowError(`Template parse errors:
+Variable does not have a name ("<ng-template [ERROR ->]let-></ng-template>"): TestComp@0:13`);
+        });
+
         it('should report duplicate reference names', () => {
           expect(() => parse('<div #a></div><div #a></div>', []))
               .toThrowError(`Template parse errors: