[FEAT] Finishes up named blocks

Finishes up named blocks by adding tests and assertions for various edge
cases and bugs. Related:

- ember-polyfills/ember-named-blocks-polyfill#3

Fixes #1106

packages/@glimmer/compiler/lib/passes/1-normalization/keywords/has-block.ts

@@ -13,7 +13,7 @@ export function assertValidHasBlockUsage(
   let positionals = call.type === 'Call' ? call.args.positional : null;
 
   if (named && !named.isEmpty()) {
-    return Err(generateSyntaxError(`${type} does not take any named arguments`, call.loc));
+    return Err(generateSyntaxError(`(${type}) does not take any named arguments`, call.loc));
   }
 
   if (!positionals || positionals.isEmpty()) {
@@ -23,9 +23,14 @@ export function assertValidHasBlockUsage(
     if (ASTv2.isLiteral(positional, 'string')) {
       return Ok(positional.toSlice());
     } else {
-      return Err(generateSyntaxError(`you can only yield to a literal value`, call.loc));
+      return Err(
+        generateSyntaxError(
+          `(${type}) can only receive a string literal as its first argument`,
+          call.loc
+        )
+      );
     }
   } else {
-    return Err(generateSyntaxError(`${type} only takes a single positional argument`, call.loc));
+    return Err(generateSyntaxError(`(${type}) only takes a single positional argument`, call.loc));
   }
 }

packages/@glimmer/integration-tests/lib/suites/emberish-components.ts

@@ -260,9 +260,9 @@ export class EmberishComponentTests extends RenderTest {
         super();
       }
     }
-    this.registerComponent('Curly', 'FooBar', 'Hello{{yield to="inverse"}}world!', FooBar);
+    this.registerComponent('Curly', 'FooBar', 'Hello{{yield "my" to="inverse"}}world!', FooBar);
 
-    this.render(`<FooBar><:else> my </:else></FooBar>`);
+    this.render(`<FooBar><:else as |value|> {{value}} </:else></FooBar>`);
 
     this.assertComponent('Hello my world!');
     this.assertStableRerender();

packages/@glimmer/integration-tests/test/syntax-errors-test.ts → packages/@glimmer/integration-tests/test/syntax/general-errors-test.ts

@@ -1,7 +1,7 @@
-import { RenderTest, jitSuite, test, preprocess, syntaxErrorFor } from '..';
+import { RenderTest, jitSuite, test, preprocess, syntaxErrorFor } from '../..';
 
 class SyntaxErrors extends RenderTest {
-  static suiteName = 'syntax errors';
+  static suiteName = 'general syntax errors';
 
   @test
   'context switching using ../ is not allowed'() {
@@ -64,6 +64,19 @@ class SyntaxErrors extends RenderTest {
       preprocess('<x-bar as |foo[bar]|></x-bar>', { meta: { moduleName: 'test-module' } });
     }, syntaxErrorFor("Invalid identifier for block parameters, 'foo[bar]'", '<x-bar as |foo[bar]|></x-bar>', 'test-module', 1, 0));
   }
+
+  @test
+  'Block params in HTML syntax - Throws an error on missing `as`'() {
+    this.assert.throws(() => {
+      preprocess('<x-bar |x|></x-bar>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Block parameters must be preceded by the `as` keyword, detected block parameters without `as`', '<x-bar |x|></x-bar>', 'test-module', 1, 0));
+
+    this.assert.throws(() => {
+      preprocess('<x-bar><:baz |x|></:baz></x-bar>', {
+        meta: { moduleName: 'test-module' },
+      });
+    }, syntaxErrorFor('Block parameters must be preceded by the `as` keyword, detected block parameters without `as`', '<:baz |x|></:baz>', 'test-module', 1, 7));
+  }
 }
 
 jitSuite(SyntaxErrors);

packages/@glimmer/integration-tests/test/keyword-errors-test.ts → packages/@glimmer/integration-tests/test/syntax/keyword-errors-test.ts

@@ -1,4 +1,4 @@
-import { RenderTest, jitSuite, test, preprocess } from '..';
+import { RenderTest, jitSuite, test, preprocess } from '../..';
 import { KEYWORDS_TYPES } from '@glimmer/syntax';
 
 const KEYWORDS = Object.keys(KEYWORDS_TYPES);
@@ -13,7 +13,7 @@ const MODIFIER_KEYWORDS = KEYWORDS.filter((key) => KEYWORDS_TYPES[key].indexOf('
 
 for (let keyword of KEYWORDS) {
   class KeywordSyntaxErrors extends RenderTest {
-    static suiteName = `\`${keyword}\` keyword errors`;
+    static suiteName = `\`${keyword}\` keyword syntax errors`;
 
     @test
     'keyword can be used as a value in non-strict mode'() {

packages/@glimmer/integration-tests/test/syntax/named-blocks-test.ts

@@ -0,0 +1,83 @@
+import { RenderTest, jitSuite, test, preprocess, syntaxErrorFor } from '../..';
+
+class NamedBlocksSyntaxErrors extends RenderTest {
+  static suiteName = 'named blocks syntax errors';
+
+  @test
+  'Defining block params on a component which has named blocks'() {
+    this.assert.throws(() => {
+      preprocess('<Foo as |bar|><:foo></:foo></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Unexpected block params list on <Foo> component invocation: when passing named blocks, the invocation tag cannot take block params', '<Foo as |bar|><:foo></:foo></Foo>', 'test-module', 1, 0));
+  }
+
+  @test
+  'Defining named blocks on a plain element is not allowed'() {
+    this.assert.throws(() => {
+      preprocess('<div><:foo></:foo></div>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Unexpected named block <:foo> inside <div> HTML element', '<div><:foo></:foo></div>', 'test-module', 1, 0));
+  }
+
+  @test
+  'Defining top level named blocks is not allowed'() {
+    this.assert.throws(() => {
+      preprocess('<:foo></:foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Unexpected named block at the top-level of a template', '<:foo></:foo>', 'test-module', 1, 0));
+
+    this.assert.throws(() => {
+      preprocess('{{#if}}<:foo></:foo>{{/if}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Unexpected named block nested in a normal block', '<:foo></:foo>', 'test-module', 1, 7));
+  }
+
+  @test
+  'Passing multiple of the same named block throws an error'() {
+    this.assert.throws(() => {
+      preprocess('<Foo><:foo></:foo><:foo></:foo></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Component had two named blocks with the same name, `<:foo>`. Only one block with a given name may be passed', '<Foo><:foo></:foo><:foo></:foo></Foo>', 'test-module', 1, 0));
+  }
+
+  @test
+  'Throws an error if there is content outside of the blocks'() {
+    this.assert.throws(() => {
+      preprocess('<Foo>Hello!<:foo></:foo></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Unexpected content inside <Foo> component invocation: when using named blocks, the tag cannot contain other content', '<Foo>Hello!<:foo></:foo></Foo>', 'test-module', 1, 0));
+
+    this.assert.throws(() => {
+      preprocess('<Foo><:foo></:foo>Hello!</Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Unexpected content inside <Foo> component invocation: when using named blocks, the tag cannot contain other content', '<Foo><:foo></:foo>Hello!</Foo>', 'test-module', 1, 0));
+  }
+
+  @test
+  'Cannot pass self closing named block'() {
+    this.assert.throws(() => {
+      preprocess('<Foo><:foo/></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('<:foo/> is not a valid named block: named blocks cannot be self-closing', '<:foo/>', 'test-module', 1, 5));
+  }
+
+  @test
+  'Named blocks must start with a lower case letter'() {
+    this.assert.throws(() => {
+      preprocess('<Foo><:Bar></:Bar></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('<:Bar> is not a valid named block, and named blocks must begin with a lowercase letter', '<:Bar></:Bar>', 'test-module', 1, 5));
+
+    this.assert.throws(() => {
+      preprocess('<Foo><:1bar><:/1bar></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('Invalid named block named detected, you may have created a named block without a name, or you may have began your name with a number. Named blocks must have names that are at least one character long, and begin with a lower case letter', '<:/1bar>', 'test-module', 1, 12));
+  }
+
+  @test
+  'Named blocks cannot have arguments, attributes, or modifiers'() {
+    this.assert.throws(() => {
+      preprocess('<Foo><:bar attr="baz"></:bar></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('named block <:bar> cannot have attributes, arguments, or modifiers', '<:bar attr="baz"></:bar>', 'test-module', 1, 5));
+
+    this.assert.throws(() => {
+      preprocess('<Foo><:bar @arg="baz"></:bar></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('named block <:bar> cannot have attributes, arguments, or modifiers', '<:bar @arg="baz"></:bar>', 'test-module', 1, 5));
+
+    this.assert.throws(() => {
+      preprocess('<Foo><:bar {{modifier}}></:bar></Foo>', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('named block <:bar> cannot have attributes, arguments, or modifiers', '<:bar {{modifier}}></:bar>', 'test-module', 1, 5));
+  }
+}
+
+jitSuite(NamedBlocksSyntaxErrors);

packages/@glimmer/integration-tests/test/syntax/yield-keywords-test.ts

@@ -0,0 +1,63 @@
+import { RenderTest, jitSuite, test, preprocess, syntaxErrorFor } from '../..';
+
+class NamedBlocksSyntaxErrors extends RenderTest {
+  static suiteName = 'yield keywords syntax errors';
+
+  @test
+  'yield throws if receiving any named args besides `to`'() {
+    this.assert.throws(() => {
+      preprocess('{{yield foo="bar"}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor("yield only takes a single named argument: 'to'", 'foo="bar"', 'test-module', 1, 8));
+  }
+
+  @test
+  'you can only yield to a literal string value'() {
+    this.assert.throws(() => {
+      preprocess('{{yield to=this.bar}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('you can only yield to a literal string value', 'this.bar', 'test-module', 1, 11));
+  }
+
+  @test
+  'has-block throws if receiving any named args'() {
+    this.assert.throws(() => {
+      preprocess('{{has-block foo="bar"}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('(has-block) does not take any named arguments', '{{has-block foo="bar"}}', 'test-module', 1, 0));
+  }
+
+  @test
+  'has-block throws if receiving multiple positional'() {
+    this.assert.throws(() => {
+      preprocess('{{has-block "foo" "bar"}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('(has-block) only takes a single positional argument', '{{has-block "foo" "bar"}}', 'test-module', 1, 0));
+  }
+
+  @test
+  'has-block throws if receiving a value besides a string'() {
+    this.assert.throws(() => {
+      preprocess('{{has-block this.bar}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('(has-block) can only receive a string literal as its first argument', '{{has-block this.bar}}', 'test-module', 1, 0));
+  }
+
+  @test
+  'has-block-params throws if receiving any named args'() {
+    this.assert.throws(() => {
+      preprocess('{{has-block-params foo="bar"}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('(has-block-params) does not take any named arguments', '{{has-block-params foo="bar"}}', 'test-module', 1, 0));
+  }
+
+  @test
+  'has-block-params throws if receiving multiple positional'() {
+    this.assert.throws(() => {
+      preprocess('{{has-block-params "foo" "bar"}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('(has-block-params) only takes a single positional argument', '{{has-block-params "foo" "bar"}}', 'test-module', 1, 0));
+  }
+
+  @test
+  'has-block-params throws if receiving a value besides a string'() {
+    this.assert.throws(() => {
+      preprocess('{{has-block-params this.bar}}', { meta: { moduleName: 'test-module' } });
+    }, syntaxErrorFor('(has-block-params) can only receive a string literal as its first argument', '{{has-block-params this.bar}}', 'test-module', 1, 0));
+  }
+}
+
+jitSuite(NamedBlocksSyntaxErrors);

packages/@glimmer/syntax/lib/parser/tokenizer-event-handlers.ts

@@ -97,6 +97,16 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
     if (tag.type === 'StartTag') {
       this.finishStartTag();
 
+      if (tag.name === ':') {
+        throw generateSyntaxError(
+          'Invalid named block named detected, you may have created a named block without a name, or you may have began your name with a number. Named blocks must have names that are at least one character long, and begin with a lower case letter',
+          this.source.spanFor({
+            start: this.currentTag.loc.toJSON(),
+            end: this.offset().toJSON(),
+          })
+        );
+      }
+
       if (voidMap[tag.name] || tag.selfClosing) {
         this.finishEndTag(true);
       }

packages/@glimmer/syntax/lib/utils.ts

@@ -29,6 +29,13 @@ function parseBlockParams(element: ASTv1.ElementNode): Option<string[]> {
 
   let asIndex = attrNames.indexOf('as');
 
+  if (asIndex === -1 && attrNames.length > 0 && attrNames[attrNames.length - 1].charAt(0) === '|') {
+    throw generateSyntaxError(
+      'Block parameters must be preceded by the `as` keyword, detected block parameters without `as`',
+      element.loc
+    );
+  }
+
   if (asIndex !== -1 && l > asIndex && attrNames[asIndex + 1].charAt(0) === '|') {
     // Some basic validation, since we're doing the parsing ourselves
     let paramsString = attrNames.slice(asIndex).join(' ');

packages/@glimmer/syntax/lib/v2-a/normalize.ts

@@ -741,21 +741,32 @@ class ElementChildren extends Children {
   assertNamedBlock(name: SourceSlice, table: BlockSymbolTable): ASTv2.NamedBlock {
     if (this.el.base.selfClosing) {
       throw generateSyntaxError(
-        `<:${name}> is not a valid named block: named blocks cannot be self-closing`,
+        `<:${name.chars}/> is not a valid named block: named blocks cannot be self-closing`,
         this.loc
       );
     }
 
     if (isPresent(this.namedBlocks)) {
       throw generateSyntaxError(
-        `Unexpected named block inside <:${name}> named block: named blocks cannot contain nested named blocks`,
+        `Unexpected named block inside <:${name.chars}> named block: named blocks cannot contain nested named blocks`,
         this.loc
       );
     }
 
     if (!isLowerCase(name.chars)) {
       throw generateSyntaxError(
-        `<:${name}> is not a valid named block: \`${name}\` is uppercase, and named blocks must be lowercase`,
+        `<:${name.chars}> is not a valid named block, and named blocks must begin with a lowercase letter`,
+        this.loc
+      );
+    }
+
+    if (
+      this.el.base.attrs.length > 0 ||
+      this.el.base.componentArgs.length > 0 ||
+      this.el.base.modifiers.length > 0
+    ) {
+      throw generateSyntaxError(
+        `named block <:${name.chars}> cannot have attributes, arguments, or modifiers`,
         this.loc
       );
     }
@@ -782,13 +793,13 @@ class ElementChildren extends Children {
 
       if (names.length === 1) {
         throw generateSyntaxError(
-          `Syntax Error: Unexpected named block <:foo> inside <${name}> HTML element`,
+          `Unexpected named block <:foo> inside <${name.chars}> HTML element`,
           this.loc
         );
       } else {
         let printedNames = names.map((n) => `<:${n.chars}>`).join(', ');
         throw generateSyntaxError(
-          `Syntax Error: Unexpected named blocks inside <${name}> HTML element (${printedNames})`,
+          `Unexpected named blocks inside <${name.chars}> HTML element (${printedNames})`,
           this.loc
         );
       }
@@ -816,6 +827,22 @@ class ElementChildren extends Children {
           this.loc
         );
       }
+
+      let seenNames = new Set<string>();
+
+      for (let block of this.namedBlocks) {
+        let name = block.name.chars;
+
+        if (seenNames.has(name)) {
+          throw generateSyntaxError(
+            `Component had two named blocks with the same name, \`<:${name}>\`. Only one block with a given name may be passed`,
+            this.loc
+          );
+        }
+
+        seenNames.add(name);
+      }
+
       return this.namedBlocks;
     } else {
       return [