Merge pull request #1585 from glimmerjs/strict-mode-keywords

Introduce `keywords` option for `precompile`

Author: wycats

packages/@glimmer-workspace/integration-tests/lib/modes/jit/compilation-context.ts

@@ -23,8 +23,8 @@ export default class JitCompileTimeLookup implements CompileTimeResolver {
     return this.resolver.lookupComponent(name, owner);
   }
 
-  lookupBuiltInHelper(_name: string): Nullable<HelperDefinitionState> {
-    return null;
+  lookupBuiltInHelper(name: string): Nullable<HelperDefinitionState> {
+    return this.resolver.lookupHelper(`$keyword.${name}`);
   }
 
   lookupBuiltInModifier(_name: string): Nullable<ModifierDefinitionState> {

packages/@glimmer-workspace/integration-tests/lib/test-helpers/define.ts

@@ -96,6 +96,9 @@ export interface DefineComponentOptions {
 
   // defaults to true when some scopeValues are passed and false otherwise
   strictMode?: boolean;
+
+  // additional strict-mode keywords
+  keywords?: string[];
 }
 
 export function defineComponent(
@@ -110,8 +113,10 @@ export function defineComponent(
     strictMode = scopeValues !== null;
   }
 
+  let keywords = options.keywords ?? [];
+
   let definition = options.definition ?? templateOnlyComponent();
-  let templateFactory = createTemplate(templateSource, { strictMode }, scopeValues ?? {});
+  let templateFactory = createTemplate(templateSource, { strictMode, keywords }, scopeValues ?? {});
   setComponentTemplate(templateFactory, definition);
   return definition;
 }

packages/@glimmer-workspace/integration-tests/test/strict-mode-test.ts

@@ -72,12 +72,16 @@ class GeneralStrictModeTest extends RenderTest {
   }
 
   @test
-  'Implicit this lookup does not work'() {
+  'Undefined references is an error'() {
+    this.registerHelper('bar', () => 'should not resolve this helper');
+
     this.assert.throws(
       () => {
         defineComponent({}, '{{bar}}', {
           definition: class extends GlimmerishComponent {
-            bar = 'Hello, world!';
+            get bar() {
+              throw new Error('should not fallback to this.bar');
+            }
           },
         });
       },
@@ -91,6 +95,28 @@ class GeneralStrictModeTest extends RenderTest {
     );
   }
 
+  @test
+  'Non-native keyword'() {
+    this.registerHelper('bar', () => {
+      throw new Error('should not resolve this helper');
+    });
+
+    this.registerHelper('$keyword.bar', () => 'bar keyword');
+
+    const Foo = defineComponent({}, '{{bar}}', {
+      keywords: ['bar'],
+      definition: class extends GlimmerishComponent {
+        get bar() {
+          throw new Error('should not fallback to this.bar');
+        }
+      },
+    });
+
+    this.renderComponent(Foo);
+    this.assertHTML('bar keyword');
+    this.assertStableRerender();
+  }
+
   @test
   '{{component}} throws an error if a string is used in strict (append position)'() {
     this.assert.throws(() => {

packages/@glimmer/compiler/lib/builder/builder.ts

@@ -632,7 +632,7 @@ export function buildVar(
   symbols: Symbols,
   path?: PresentArray<string>
 ): Expressions.GetPath | Expressions.GetVar {
-  let op: Expressions.GetVar[0] = Op.GetSymbol;
+  let op: Expressions.GetPath[0] | Expressions.GetVar[0] = Op.GetSymbol;
   let sym: number;
   switch (head.kind) {
     case VariableKind.Free:
@@ -665,6 +665,7 @@ export function buildVar(
   if (path === undefined || path.length === 0) {
     return [op, sym];
   } else {
+    assert(op !== Op.GetStrictKeyword, '[BUG] keyword with a path');
     return [op, sym, path];
   }
 }

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

@@ -78,12 +78,6 @@ class KeywordImpl<
   }
 }
 
-export type PossibleNode =
-  | ASTv2.PathExpression
-  | ASTv2.AppendContent
-  | ASTv2.CallExpression
-  | ASTv2.InvokeBlock;
-
 export const KEYWORD_NODES = {
   Call: ['Call'],
   Block: ['InvokeBlock'],

packages/@glimmer/compiler/lib/passes/1-normalization/utils/is-node.ts

@@ -1,111 +1,4 @@
-import type { PresentArray } from '@glimmer/interfaces';
-import type { SourceSlice } from '@glimmer/syntax';
-import { ASTv2, generateSyntaxError } from '@glimmer/syntax';
-import { unreachable } from '@glimmer/util';
-
-export type HasPath<Node extends ASTv2.CallNode = ASTv2.CallNode> = Node & {
-  head: ASTv2.PathExpression;
-};
-
-export type HasArguments =
-  | {
-      params: PresentArray<ASTv2.ExpressionNode>;
-    }
-  | {
-      hash: {
-        pairs: PresentArray<ASTv2.NamedArgument>;
-      };
-    };
-
-export type HelperInvocation<Node extends ASTv2.CallNode = ASTv2.CallNode> = HasPath<Node> &
-  HasArguments;
-
-export function hasPath<N extends ASTv2.CallNode>(node: N): node is HasPath<N> {
-  return node.callee.type === 'Path';
-}
-
-export function isHelperInvocation<N extends ASTv2.CallNode>(
-  node: ASTv2.CallNode
-): node is HelperInvocation<N> {
-  if (!hasPath(node)) {
-    return false;
-  }
-
-  return !node.args.isEmpty();
-}
-
-export interface SimplePath extends ASTv2.PathExpression {
-  tail: [SourceSlice];
-  data: false;
-  this: false;
-}
-
-export type SimpleHelper<N extends HasPath> = N & {
-  path: SimplePath;
-};
-
-export function isSimplePath(path: ASTv2.ExpressionNode): path is SimplePath {
-  if (path.type === 'Path') {
-    let { ref: head, tail: parts } = path;
-
-    return head.type === 'Free' && !ASTv2.isStrictResolution(head.resolution) && parts.length === 0;
-  } else {
-    return false;
-  }
-}
-
-export function isStrictHelper(expr: HasPath): boolean {
-  if (expr.callee.type !== 'Path') {
-    return true;
-  }
-
-  if (expr.callee.ref.type !== 'Free') {
-    return true;
-  }
-
-  return ASTv2.isStrictResolution(expr.callee.ref.resolution);
-}
-
-export function assertIsValidModifier<N extends HasPath>(
-  helper: N
-): asserts helper is SimpleHelper<N> {
-  if (isStrictHelper(helper) || isSimplePath(helper.callee)) {
-    return;
-  }
-
-  throw generateSyntaxError(
-    `\`${printPath(helper.callee)}\` is not a valid name for a modifier`,
-    helper.loc
-  );
-}
-
-function printPath(path: ASTv2.ExpressionNode): string {
-  switch (path.type) {
-    case 'Literal':
-      return JSON.stringify(path.value);
-    case 'Path': {
-      let printedPath = [printPathHead(path.ref)];
-      printedPath.push(...path.tail.map((t) => t.chars));
-      return printedPath.join('.');
-    }
-    case 'Call':
-      return `(${printPath(path.callee)} ...)`;
-    case 'Interpolate':
-      throw unreachable('a concat statement cannot appear as the head of an expression');
-  }
-}
-
-function printPathHead(head: ASTv2.VariableReference): string {
-  switch (head.type) {
-    case 'Arg':
-      return head.name.chars;
-    case 'Free':
-    case 'Local':
-      return head.name;
-    case 'This':
-      return 'this';
-  }
-}
+import type { ASTv2 } from '@glimmer/syntax';
 
 /**
  * This function is checking whether an AST node is a triple-curly, which means that it's

packages/@glimmer/compiler/lib/passes/1-normalization/visitors/element/classified.ts

@@ -7,7 +7,6 @@ import { Ok, Result, ResultArray } from '../../../../shared/result';
 import { getAttrNamespace } from '../../../../utils';
 import * as mir from '../../../2-encoding/mir';
 import { MODIFIER_KEYWORDS } from '../../keywords';
-import { assertIsValidModifier, isHelperInvocation } from '../../utils/is-node';
 import { convertPathToCallIfKeyword, VISIT_EXPRS } from '../expressions';
 
 export type ValidAttr = mir.StaticAttr | mir.DynamicAttr | mir.SplatAttr;
@@ -75,10 +74,6 @@ export class ClassifiedElement {
   }
 
   private modifier(modifier: ASTv2.ElementModifier): Result<mir.Modifier> {
-    if (isHelperInvocation(modifier)) {
-      assertIsValidModifier(modifier);
-    }
-
     let translated = MODIFIER_KEYWORDS.translate(modifier, this.state);
 
     if (translated !== null) {

packages/@glimmer/compiler/lib/passes/1-normalization/visitors/expressions.ts

@@ -8,13 +8,14 @@ import type { NormalizationState } from '../context';
 import { Ok, Result, ResultArray } from '../../../shared/result';
 import * as mir from '../../2-encoding/mir';
 import { CALL_KEYWORDS } from '../keywords';
-import { hasPath } from '../utils/is-node';
 
 export class NormalizeExpressions {
   visit(node: ASTv2.ExpressionNode, state: NormalizationState): Result<mir.ExpressionNode> {
     switch (node.type) {
       case 'Literal':
         return Ok(this.Literal(node));
+      case 'Keyword':
+        return Ok(this.Keyword(node));
       case 'Interpolate':
         return this.Interpolate(node, state);
       case 'Path':
@@ -78,6 +79,10 @@ export class NormalizeExpressions {
     return literal;
   }
 
+  Keyword(keyword: ASTv2.KeywordExpression): ASTv2.KeywordExpression {
+    return keyword;
+  }
+
   Interpolate(
     expr: ASTv2.InterpolateExpression,
     state: NormalizationState
@@ -93,8 +98,8 @@ export class NormalizeExpressions {
     expr: ASTv2.CallExpression,
     state: NormalizationState
   ): Result<mir.ExpressionNode> {
-    if (!hasPath(expr)) {
-      throw new Error(`unimplemented subexpression at the head of a subexpression`);
+    if (expr.callee.type === 'Call') {
+      throw new Error(`unimplemented: subexpression at the head of a subexpression`);
     } else {
       return Result.all(
         VISIT_EXPRS.visit(expr.callee, state),

packages/@glimmer/compiler/lib/passes/1-normalization/visitors/strict-mode.ts

@@ -120,6 +120,7 @@ export default class StrictModeValidationPass {
   ): Result<null> {
     switch (expression.type) {
       case 'Literal':
+      case 'Keyword':
       case 'Missing':
       case 'This':
       case 'Arg':

packages/@glimmer/compiler/lib/passes/2-encoding/expressions.ts

@@ -1,6 +1,6 @@
 import type { PresentArray, WireFormat } from '@glimmer/interfaces';
 import type { ASTv2 } from '@glimmer/syntax';
-import { assertPresentArray, isPresentArray, mapPresentArray } from '@glimmer/util';
+import { assert, assertPresentArray, isPresentArray, mapPresentArray } from '@glimmer/util';
 import { SexpOpcodes } from '@glimmer/wire-format';
 
 import type * as mir from './mir';
@@ -14,6 +14,8 @@ export class ExpressionEncoder {
         return undefined;
       case 'Literal':
         return this.Literal(expr);
+      case 'Keyword':
+        return this.Keyword(expr);
       case 'CallExpression':
         return this.CallExpression(expr);
       case 'PathExpression':
@@ -86,9 +88,13 @@ export class ExpressionEncoder {
     return [isTemplateLocal ? SexpOpcodes.GetLexicalSymbol : SexpOpcodes.GetSymbol, symbol];
   }
 
+  Keyword({ symbol }: ASTv2.KeywordExpression): WireFormat.Expressions.GetStrictFree {
+    return [SexpOpcodes.GetStrictKeyword, symbol];
+  }
+
   PathExpression({ head, tail }: mir.PathExpression): WireFormat.Expressions.GetPath {
     let getOp = EXPR.expr(head) as WireFormat.Expressions.GetVar;
-
+    assert(getOp[0] !== SexpOpcodes.GetStrictKeyword, '[BUG] keyword in a PathExpression');
     return [...getOp, EXPR.Tail(tail)];
   }
 

packages/@glimmer/compiler/lib/passes/2-encoding/mir.ts

@@ -180,9 +180,10 @@ export class Tail extends node('Tail').fields<{ members: PresentArray<SourceSlic
 
 export type ExpressionNode =
   | ASTv2.LiteralExpression
+  | ASTv2.KeywordExpression
+  | ASTv2.VariableReference
   | Missing
   | PathExpression
-  | ASTv2.VariableReference
   | InterpolateExpression
   | CallExpression
   | Not

packages/@glimmer/compiler/lib/wire-format-debug.ts

@@ -170,7 +170,7 @@ export default class WireFormatDebugger {
           return ['concat', this.formatParams(opcode[1] as WireFormat.Core.Params)];
 
         case Op.GetStrictKeyword:
-          return ['get-strict-free', this.upvars[opcode[1]], opcode[2]];
+          return ['get-strict-free', this.upvars[opcode[1]]];
 
         case Op.GetFreeAsComponentOrHelperHead:
           return ['GetFreeAsComponentOrHelperHead', this.upvars[opcode[1]], opcode[2]];

packages/@glimmer/interfaces/lib/compile/wire-format/api.d.ts

@@ -111,7 +111,6 @@ export namespace Expressions {
 
   export type GetPathSymbol = [GetSymbolOpcode, number, Path];
   export type GetPathTemplateSymbol = [GetLexicalSymbolOpcode, number, Path];
-  export type GetPathStrictFree = [GetStrictKeywordOpcode, number, Path];
   export type GetPathFreeAsComponentOrHelperHead = [
     GetFreeAsComponentOrHelperHeadOpcode,
     number,
@@ -126,8 +125,7 @@ export namespace Expressions {
     | GetPathFreeAsHelperHead
     | GetPathFreeAsModifierHead
     | GetPathFreeAsComponentHead;
-  export type GetPathFree = GetPathStrictFree | GetPathContextualFree;
-  export type GetPath = GetPathSymbol | GetPathTemplateSymbol | GetPathFree;
+  export type GetPath = GetPathSymbol | GetPathTemplateSymbol | GetPathContextualFree;
 
   export type Get = GetVar | GetPath;
 

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

@@ -661,6 +661,22 @@ export interface TemplateIdFn {
 
 export interface PrecompileOptions extends PreprocessOptions {
   id?: TemplateIdFn;
+
+  /**
+   * Additional non-native keywords.
+   *
+   * Local variables (block params or lexical scope) always takes precedence,
+   * but otherwise, suitable free variable candidates (e.g. those are not part
+   * of a path) are matched against this list and turned into keywords.
+   *
+   * In strict mode compilation, keywords suppresses the undefined reference
+   * error and will be resolved by the runtime environment.
+   *
+   * In loose mode, keywords are currently ignored and since all free variables
+   * are already resolved by the runtime environment.
+   */
+  keywords?: readonly string[];
+
   customizeComponentName?: ((input: string) => string) | undefined;
 }