From 3abccbf3524750a5ae3f5c7e94e0e9dd3c203518 Mon Sep 17 00:00:00 2001
From: Stanley Stuart <sstuart@salesforce.com>
Date: Mon, 18 May 2020 09:03:56 -0700
Subject: [PATCH] disallow @arguments on regular HTML nodes

Adds an error message in the compile phase to inform the developer that
`@arguments` are not allowed on HTML nodes. This can often come from
copy/pasting refactoring errors.

For example, given the following template:

Before, the developer would get a vague error at runtime ([Example taken from
ember.js#18951](https://github.com/emberjs/ember.js/issues/18951)):

```
jquery.js:3827 Uncaught TypeError: func is not a function
    at StatementCompilers.compile (VM32 ember.debug.js:43121)
    at compileStatements (VM32 ember.debug.js:44323)
    at maybeCompile (VM32 ember.debug.js:44312)
    at CompilableTemplateImpl.compile (VM32 ember.debug.js:44292)
    at Object.evaluate (VM32 ember.debug.js:51011)
    at AppendOpcodes.evaluate (VM32 ember.debug.js:49382)
    at LowLevelVM.evaluateSyscall (VM32 ember.debug.js:52284)
    at LowLevelVM.evaluateInner (VM32 ember.debug.js:52240)
    at LowLevelVM.evaluateOuter (VM32 ember.debug.js:52232)
    at JitVM.next (VM32 ember.debug.js:53175)
```

Now, they get an error message that looks like:

```
SyntaxError: ${attribute.name} is not a valid attribute name. @arguments
are only allowed on components, but the tag for this element
(\`${elementNode.tag}\`) is a regular, non-component HTML element. (location
line and column)
```

Given that there's no special behavior glimmer adds for `@arguments` on regular
HTML nodes, and it seems invalid to put `@` in attribute names in regular HTML
(validator.w3c.org gives ` Attribute @onclick is not serializable as XML 1.0`
error message for the example template below), it seems helpful for the glimmer
compiler to fail at build time to prevent copy/paste errors and mistakes.

Fixes https://github.com/emberjs/ember.js/issues/18951
---
 .../compiler/lib/template-compiler.ts         | 20 ++++++++++++++++---
 .../@glimmer/compiler/test/compiler-test.ts   | 13 ++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/packages/@glimmer/compiler/lib/template-compiler.ts b/packages/@glimmer/compiler/lib/template-compiler.ts
index 5c86a787f4..7f31d76fab 100644
--- a/packages/@glimmer/compiler/lib/template-compiler.ts
+++ b/packages/@glimmer/compiler/lib/template-compiler.ts
@@ -134,11 +134,11 @@ export default class TemplateCompiler implements Processor<InputOps> {
           typeAttr = attrs[i];
           continue;
         }
-        this.attribute([attrs[i]], !simple || actionIsComponent);
+        this.attribute([attrs[i]], !simple || actionIsComponent, action);
       }
 
       if (typeAttr) {
-        this.attribute([typeAttr], !simple || actionIsComponent);
+        this.attribute([typeAttr], !simple || actionIsComponent, action);
       }
 
       for (let i = 0; i < action.modifiers.length; i++) {
@@ -161,7 +161,8 @@ export default class TemplateCompiler implements Processor<InputOps> {
     }
   }
 
-  attribute([action]: [AST.AttrNode], isComponent: boolean) {
+  attribute([action]: [AST.AttrNode], isComponent: boolean, elementNode: AST.ElementNode) {
+    assertValidArgumentName(action, isComponent, elementNode);
     let { name, value } = action;
 
     let namespace = getAttrNamespace(name);
@@ -690,6 +691,19 @@ function assertIsSimplePath(path: AST.Expression, loc: AST.SourceLocation, conte
   }
 }
 
+function assertValidArgumentName(
+  attribute: AST.AttrNode,
+  isComponent: boolean,
+  elementNode: AST.ElementNode
+) {
+  if (!isComponent && attribute.name.startsWith('@')) {
+    throw new SyntaxError(
+      `${attribute.name} is not a valid attribute name. @arguments are only allowed on components, but the tag for this element (\`${elementNode.tag}\`) is a regular, non-component HTML element.`,
+      attribute.loc
+    );
+  }
+}
+
 function assertValidYield(statement: AST.MustacheStatement): string {
   let { pairs } = statement.hash;
 
diff --git a/packages/@glimmer/compiler/test/compiler-test.ts b/packages/@glimmer/compiler/test/compiler-test.ts
index bfd11a534b..d7fadc6e04 100644
--- a/packages/@glimmer/compiler/test/compiler-test.ts
+++ b/packages/@glimmer/compiler/test/compiler-test.ts
@@ -55,6 +55,19 @@ function test(desc: string, template: string, ...expectedStatements: BuilderStat
 const Append = Builder.Append;
 const Concat = Builder.Concat;
 
+QUnit.test(
+  '@arguments are on regular non-component/regular HTML nodes throws syntax error',
+  function(assert) {
+    const template = strip`
+    <a @onClick={{action "hi"}}>Link</a>
+  `;
+    assert.throws(
+      () => compile(template),
+      /@onClick is not a valid attribute name. @arguments are only allowed on components, but the tag for this element \(`a`\) is a regular, non-component HTML element/
+    );
+  }
+);
+
 test('HTML text content', 'content', s`content`);
 
 test('Text curlies', '<div>{{title}}<span>{{title}}</span></div>', [