Funnel all v1 AST node construction through parser builders

The parser builders are the defacto constructors for the v1 AST
nodes. As the nodes get increasingly more eloborate for backwards
compatibility and such, it's important that we share the logic for
constructing these nodes as much as possible so we don't miss any
cases.

Also standardize the signatures to use the object argument style,
you should be able to clone a node by passing it back into the same
builder method which it originated from.

Author: chancancode

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

@@ -11,7 +11,7 @@ import type * as ASTv1 from './v1/api';
 import type * as HBS from './v1/handlebars-ast';
 
 export type ParserNodeBuilder<N extends { loc: src.SourceSpan }> = Omit<N, 'loc'> & {
-  loc: src.SourceOffset;
+  start: src.SourceOffset;
 };
 
 export interface Tag<T extends 'StartTag' | 'EndTag'> {
@@ -42,7 +42,7 @@ export abstract class Parser {
   public currentNode: Nullable<
     Readonly<
       | ParserNodeBuilder<ASTv1.CommentStatement>
-      | ASTv1.TextNode
+      | ParserNodeBuilder<ASTv1.TextNode>
       | ParserNodeBuilder<Tag<'StartTag'>>
       | ParserNodeBuilder<Tag<'EndTag'>>
     >
@@ -70,7 +70,7 @@ export abstract class Parser {
 
   finish<T extends { loc: src.SourceSpan }>(node: ParserNodeBuilder<T>): T {
     return assign({}, node, {
-      loc: node.loc.until(this.offset()),
+      loc: node.start.until(this.offset()),
     } as const) as unknown as T;
 
     // node.loc = node.loc.withEnd(end);
@@ -143,7 +143,7 @@ export abstract class Parser {
     return node;
   }
 
-  get currentData(): ASTv1.TextNode {
+  get currentData(): ParserNodeBuilder<ASTv1.TextNode> {
     let node = this.currentNode;
     assert(node && node.type === 'TextNode', 'expected a text node');
     return node;

packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts

@@ -10,7 +10,6 @@ import { Parser } from '../parser';
 import { NON_EXISTENT_LOCATION } from '../source/location';
 import { generateSyntaxError } from '../syntax-error';
 import { appendChild, isHBSLiteral, printLiteral } from '../utils';
-import { buildLegacyPath } from '../v1/legacy-interop';
 import b from '../v1/parser-builders';
 
 const BEFORE_ATTRIBUTE_NAME = 'beforeAttributeName' as TokenizerState;
@@ -129,7 +128,7 @@ export abstract class HandlebarsNodeVisitors extends Parser {
       mustache = b.mustache({
         path: this.acceptNode<ASTv1.Literal>(rawMustache.path),
         params: [],
-        hash: b.hash([], this.source.spanFor(rawMustache.path.loc).collapse('end')),
+        hash: b.hash({ pairs: [], loc: this.source.spanFor(rawMustache.path.loc).collapse('end') }),
         trusting: !escaped,
         loc: this.source.spanFor(loc),
         strip,
@@ -229,7 +228,7 @@ export abstract class HandlebarsNodeVisitors extends Parser {
     }
 
     const { value, loc } = rawComment;
-    const comment = b.mustacheComment(value, this.source.spanFor(loc));
+    const comment = b.mustacheComment({ value, loc: this.source.spanFor(loc) });
 
     switch (tokenizer.state) {
       case 'beforeAttributeName':
@@ -336,13 +335,12 @@ export abstract class HandlebarsNodeVisitors extends Parser {
 
     let pathHead: ASTv1.PathHead;
     if (thisHead) {
-      pathHead = {
-        type: 'ThisHead',
+      pathHead = b.this({
         loc: this.source.spanFor({
           start: path.loc.start,
           end: { line: path.loc.start.line, column: path.loc.start.column + 4 },
         }),
-      };
+      });
     } else if (path.data) {
       const head = parts.shift();
 
@@ -353,14 +351,13 @@ export abstract class HandlebarsNodeVisitors extends Parser {
         );
       }
 
-      pathHead = {
-        type: 'AtHead',
+      pathHead = b.atName({
         name: `@${head}`,
         loc: this.source.spanFor({
           start: path.loc.start,
           end: { line: path.loc.start.line, column: path.loc.start.column + head.length + 1 },
         }),
-      };
+      });
     } else {
       const head = parts.shift();
 
@@ -371,17 +368,16 @@ export abstract class HandlebarsNodeVisitors extends Parser {
         );
       }
 
-      pathHead = {
-        type: 'VarHead',
+      pathHead = b.var({
         name: head,
         loc: this.source.spanFor({
           start: path.loc.start,
           end: { line: path.loc.start.line, column: path.loc.start.column + head.length },
         }),
-      };
+      });
     }
 
-    return buildLegacyPath({
+    return b.path({
       head: pathHead,
       tail: parts,
       loc: this.source.spanFor(path.loc),
@@ -397,7 +393,7 @@ export abstract class HandlebarsNodeVisitors extends Parser {
       })
     );
 
-    return b.hash(pairs, this.source.spanFor(hash.loc));
+    return b.hash({ pairs, loc: this.source.spanFor(hash.loc) });
   }
 
   StringLiteral(string: HBS.StringLiteral): ASTv1.StringLiteral {
@@ -502,38 +498,43 @@ function acceptCallNodes(
   params: ASTv1.Expression[];
   hash: ASTv1.Hash;
 } {
-  if (node.path.type.endsWith('Literal')) {
-    const path = node.path as unknown as
-      | HBS.StringLiteral
-      | HBS.UndefinedLiteral
-      | HBS.NullLiteral
-      | HBS.NumberLiteral
-      | HBS.BooleanLiteral;
-
-    let value = '';
-    if (path.type === 'BooleanLiteral') {
-      value = path.original.toString();
-    } else if (path.type === 'StringLiteral') {
-      value = `"${path.original}"`;
-    } else if (path.type === 'NullLiteral') {
-      value = 'null';
-    } else if (path.type === 'NumberLiteral') {
-      value = path.value.toString();
-    } else {
-      value = 'undefined';
+  let path: ASTv1.PathExpression | ASTv1.SubExpression;
+
+  switch (node.path.type) {
+    case 'PathExpression':
+      path = compiler.PathExpression(node.path);
+      break;
+
+    case 'SubExpression':
+      path = compiler.SubExpression(node.path);
+      break;
+
+    case 'StringLiteral':
+    case 'UndefinedLiteral':
+    case 'NullLiteral':
+    case 'NumberLiteral':
+    case 'BooleanLiteral': {
+      let value: string;
+      if (node.path.type === 'BooleanLiteral') {
+        value = node.path.original.toString();
+      } else if (node.path.type === 'StringLiteral') {
+        value = `"${node.path.original}"`;
+      } else if (node.path.type === 'NullLiteral') {
+        value = 'null';
+      } else if (node.path.type === 'NumberLiteral') {
+        value = node.path.value.toString();
+      } else {
+        value = 'undefined';
+      }
+      throw generateSyntaxError(
+        `${node.path.type} "${
+          node.path.type === 'StringLiteral' ? node.path.original : value
+        }" cannot be called as a sub-expression, replace (${value}) with ${value}`,
+        compiler.source.spanFor(node.path.loc)
+      );
     }
-    throw generateSyntaxError(
-      `${path.type} "${
-        path.type === 'StringLiteral' ? path.original : value
-      }" cannot be called as a sub-expression, replace (${value}) with ${value}`,
-      compiler.source.spanFor(path.loc)
-    );
   }
 
-  const path =
-    node.path.type === 'PathExpression'
-      ? compiler.PathExpression(node.path)
-      : compiler.SubExpression(node.path as unknown as HBS.SubExpression);
   const params = node.params
     ? node.params.map((e) => compiler.acceptNode<ASTv1.Expression>(e))
     : [];
@@ -544,11 +545,10 @@ function acceptCallNodes(
 
   const hash = node.hash
     ? compiler.Hash(node.hash)
-    : ({
-        type: 'Hash',
-        pairs: [] as ASTv1.HashPair[],
+    : b.hash({
+        pairs: [],
         loc: compiler.source.spanFor(end).collapse('end'),
-      } as const);
+      });
 
   return { path, params, hash };
 }

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

@@ -30,34 +30,37 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
   // Comment
 
   beginComment(): void {
-    this.currentNode = b.comment('', this.source.offsetFor(this.tagOpenLine, this.tagOpenColumn));
+    this.currentNode = {
+      type: 'CommentStatement',
+      value: '',
+      start: this.source.offsetFor(this.tagOpenLine, this.tagOpenColumn),
+    };
   }
 
   appendToCommentData(char: string): void {
     this.currentComment.value += char;
   }
 
   finishComment(): void {
-    appendChild(this.currentElement(), this.finish(this.currentComment));
+    appendChild(this.currentElement(), b.comment(this.finish(this.currentComment)));
   }
 
   // Data
 
   beginData(): void {
-    this.currentNode = b.text({
+    this.currentNode = {
+      type: 'TextNode',
       chars: '',
-      loc: this.offset().collapsed(),
-    });
+      start: this.offset(),
+    };
   }
 
   appendToData(char: string): void {
     this.currentData.chars += char;
   }
 
   finishData(): void {
-    this.currentData.loc = this.currentData.loc.withEnd(this.offset());
-
-    appendChild(this.currentElement(), this.currentData);
+    appendChild(this.currentElement(), b.text(this.finish(this.currentData)));
   }
 
   // Tags - basic
@@ -75,7 +78,7 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
       modifiers: [],
       comments: [],
       selfClosing: false,
-      loc: this.source.offsetFor(this.tagOpenLine, this.tagOpenColumn),
+      start: this.source.offsetFor(this.tagOpenLine, this.tagOpenColumn),
     };
   }
 
@@ -87,7 +90,7 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
       modifiers: [],
       comments: [],
       selfClosing: false,
-      loc: this.source.offsetFor(this.tagOpenLine, this.tagOpenColumn),
+      start: this.source.offsetFor(this.tagOpenLine, this.tagOpenColumn),
     };
   }
 
@@ -101,7 +104,7 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
         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(),
+            start: this.currentTag.start.toJSON(),
             end: this.offset().toJSON(),
           })
         );
@@ -116,19 +119,14 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
   }
 
   finishStartTag(): void {
-    let {
-      name,
-      attributes: attrs,
-      modifiers,
-      comments,
-      selfClosing,
-      loc,
-    } = this.finish(this.currentStartTag);
+    let { name, attributes, modifiers, comments, selfClosing, loc } = this.finish(
+      this.currentStartTag
+    );
 
     let element = b.element({
       tag: name,
       selfClosing,
-      attrs,
+      attributes,
       modifiers,
       comments,
       children: [],
@@ -148,7 +146,7 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
 
     element.loc = element.loc.withEnd(this.offset());
     parseElementBlockParams(element);
-    appendChild(parent, element);
+    appendChild(parent, b.element(element));
   }
 
   markTagAsSelfClosing(): void {
@@ -222,7 +220,7 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
     if (tag.type === 'EndTag') {
       throw generateSyntaxError(
         `Invalid end tag: closing tag must not have attributes`,
-        this.source.spanFor({ start: tag.loc.toJSON(), end: tokenizerPos.toJSON() })
+        this.source.spanFor({ start: tag.start.toJSON(), end: tokenizerPos.toJSON() })
       );
     }
 
@@ -256,7 +254,10 @@ export class TokenizerEventHandlers extends HandlebarsNodeVisitors {
     let first = getFirst(parts);
     let last = getLast(parts);
 
-    return b.concat(parts, this.source.spanFor(first.loc).extend(this.source.spanFor(last.loc)));
+    return b.concat({
+      parts,
+      loc: this.source.spanFor(first.loc).extend(this.source.spanFor(last.loc)),
+    });
   }
 
   validateEndTag(

packages/@glimmer/syntax/lib/v1/handlebars-ast.ts

@@ -86,9 +86,9 @@ export interface CommonBlock extends CommonNode {
   hash: Hash;
   program: Program;
   inverse: Program;
-  openStrip: StripFlags;
-  inverseStrip: StripFlags;
-  closeStrip: StripFlags;
+  openStrip?: StripFlags;
+  inverseStrip?: StripFlags;
+  closeStrip?: StripFlags;
 }
 
 export interface BlockStatement extends CommonBlock {