[BUGFIX] Invoke argument-less helpers in attribute position in strict mode

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

@@ -1440,7 +1440,166 @@ class BuiltInsStrictModeTest extends RenderTest {
   }
 }
 
+class AttributeStrictModeTest extends RenderTest {
+  static suiteName = 'strict mode: attribute position';
+
+  @test
+  'Can use a helper in attribute position (without args)'() {
+    const works = defineSimpleHelper(() => 'works');
+    const Foo = defineComponent({ works }, '<div class={{works}}></div>');
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div class="works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Can use a helper in attribute interpolation (without args)'() {
+    const works = defineSimpleHelper(() => 'works');
+    const Foo = defineComponent({ works }, '<div class="it {{works}}"></div>');
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Can use a plain function in attribute position (without args)'() {
+    const works = () => 'works';
+    const Foo = defineComponent({ works }, '<div class="it {{works}}"></div>');
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Can use a helper in attribute position on a component (without args)'() {
+    const works = defineSimpleHelper(() => 'works');
+    const Foo = defineComponent({}, '<div ...attributes></div>');
+    const Bar = defineComponent({ works, Foo }, '<Foo class="it {{works}}"/>');
+
+    this.renderComponent(Bar);
+    this.assertHTML('<div class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Helpers in attribute position update when their dependencies change'() {
+    const obj = trackedObj({ value: 'red' });
+    const color = defineSimpleHelper(() => obj['value']);
+    const Foo = defineComponent({ color }, '<div class="it {{color}}"></div>');
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div class="it red"></div>');
+    this.assertStableRerender();
+
+    obj['value'] = 'blue';
+
+    this.rerender();
+    this.assertHTML('<div class="it blue"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Non-helper values in attribute position are used directly'() {
+    const value = 'works';
+    const Foo = defineComponent({ value }, '<div data-foo={{value}} class="it {{value}}"></div>');
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div data-foo="works" class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test({ skip: !DEBUG })
+  'Component definitions in attribute position are an error'() {
+    const Foo = defineComponent({}, 'Hello, world!');
+    const Bar = defineComponent({ Foo }, '<div data-foo={{Foo}}></div>');
+
+    this.assert.throws(() => {
+      this.renderComponent(Bar);
+    }, /Attempted to use a component as the value of an attribute, but only values and helpers are valid in attribute position./u);
+  }
+
+  @test
+  'Paths on lexical variables in attribute position are used as values'() {
+    const styles = { works: 'works' };
+    const Foo = defineComponent(
+      { styles },
+      '<div data-foo={{styles.works}} class="it {{styles.works}}"></div>'
+    );
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div data-foo="works" class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Can call a helper in attribute position (with args)'() {
+    const echo = defineSimpleHelper((value: string) => value);
+    const Foo = defineComponent(
+      { echo },
+      '<div data-foo={{echo "works"}} class="it {{echo "works"}}"></div>'
+    );
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div data-foo="works" class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Can call a helper in attribute position as a subexpression (without args)'() {
+    const works = defineSimpleHelper(() => 'works');
+    const Foo = defineComponent(
+      { works },
+      '<div data-foo={{(works)}} class="it {{(works)}}"></div>'
+    );
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div data-foo="works" class="it works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Can use a helper in trusting attribute position (without args)'() {
+    const works = defineSimpleHelper(() => 'works');
+    const Foo = defineComponent({ works }, '<div data-foo={{{works}}}></div>');
+
+    this.renderComponent(Foo);
+    this.assertHTML('<div data-foo="works"></div>');
+    this.assertStableRerender();
+  }
+
+  @test
+  'Helpers in argument position (without parens) are passed by value'(assert: Assert) {
+    const works = defineSimpleHelper(() => 'works');
+    const Foo = defineComponent({}, '{{this.receivedFunction}}', {
+      definition: class extends GlimmerishComponent {
+        get receivedFunction() {
+          assert.strictEqual(
+            typeof this.args['value'],
+            'function',
+            'the argument is a function, not an invoked result'
+          );
+          assert.strictEqual(
+            (this.args as { value: unknown })['value'],
+            works,
+            'the helper itself was passed as the argument'
+          );
+          return String(typeof this.args['value'] === 'function');
+        }
+      },
+    });
+    const Bar = defineComponent({ works, Foo }, '<Foo @value={{works}}/>');
+
+    this.renderComponent(Bar);
+    this.assertHTML('true');
+    this.assertStableRerender();
+  }
+}
+
 jitSuite(GeneralStrictModeTest);
 jitSuite(StaticStrictModeTest);
 jitSuite(DynamicStrictModeTest);
 jitSuite(BuiltInsStrictModeTest);
+jitSuite(AttributeStrictModeTest);

packages/@glimmer/opcode-compiler/lib/opcode-builder/helpers/vm.ts

@@ -1,8 +1,10 @@
 import type { CurriedType, NonSmallIntOperand, Nullable, WireFormat } from '@glimmer/interfaces';
+import { DEBUG } from '@glimmer/env';
 import { encodeImmediate, isSmallInt } from '@glimmer/constants/lib/immediate';
 import {
   VM_BIND_DYNAMIC_SCOPE_OP,
   VM_CAPTURE_ARGS_OP,
+  VM_CONSTANT_REFERENCE_OP,
   VM_CURRY_OP,
   VM_DUP_OP,
   VM_DYNAMIC_HELPER_OP,
@@ -16,9 +18,11 @@ import {
 } from '@glimmer/constants/lib/syscall-ops';
 import { VM_POP_FRAME_OP, VM_PUSH_FRAME_OP } from '@glimmer/constants/lib/vm-ops';
 import { $fp, $v0 } from '@glimmer/vm/lib/registers';
+import { opcodes as SexpOpcodes } from '@glimmer/wire-format/lib/opcodes';
 
 import type { PushExpressionOp, PushStatementOp } from '../../syntax/compilers';
 
+import { HighLevelResolutionOpcodes } from '../opcodes';
 import { isStrictMode, nonSmallIntOperand } from '../operands';
 import { expr } from './expr';
 import { SimpleArgs } from './shared';
@@ -105,6 +109,47 @@ export function CallDynamic(
   }
 }
 
+/**
+ * Compile an expression in attribute-value position (either an entire dynamic
+ * attribute value, or one part of an attribute interpolation).
+ *
+ * In strict mode, a bare curly referencing an in-scope (lexical) binding —
+ * e.g. `class={{foo}}` or `class="prefix {{foo}}"` — is invoked as a
+ * zero-argument helper if its value has an associated helper manager (which
+ * includes plain functions, per RFC 0756). Otherwise the value itself is
+ * used. This matches loose mode, where an attribute curly that resolves to a
+ * helper is always invoked.
+ */
+export function DynamicAttrValue(op: PushExpressionOp, expression: WireFormat.Expression): void {
+  if (
+    Array.isArray(expression) &&
+    expression[0] === SexpOpcodes.GetLexicalSymbol &&
+    expression.length === 2
+  ) {
+    op(HighLevelResolutionOpcodes.OptionalComponentOrHelper, expression, {
+      ifComponent: () => {
+        if (DEBUG) {
+          throw new Error(
+            'Attempted to use a component as the value of an attribute, but only values and helpers are valid in attribute position.'
+          );
+        }
+
+        PushPrimitiveReference(op, undefined);
+      },
+
+      ifHelper: (handle: number) => {
+        Call(op, handle, null, null);
+      },
+
+      ifValue: (handle: number) => {
+        op(VM_CONSTANT_REFERENCE_OP, handle);
+      },
+    });
+  } else {
+    expr(op, expression);
+  }
+}
+
 /**
  * Evaluate statements in the context of new dynamic scope entries. Move entries from the
  * stack into named entries in the dynamic scope, then evaluate the statements, then pop

packages/@glimmer/opcode-compiler/lib/syntax/expressions.ts

@@ -23,15 +23,21 @@ import type { PushExpressionOp } from './compilers';
 import { expr } from '../opcode-builder/helpers/expr';
 import { isGetFreeHelper } from '../opcode-builder/helpers/resolution';
 import { SimpleArgs } from '../opcode-builder/helpers/shared';
-import { Call, CallDynamic, Curry, PushPrimitiveReference } from '../opcode-builder/helpers/vm';
+import {
+  Call,
+  CallDynamic,
+  Curry,
+  DynamicAttrValue,
+  PushPrimitiveReference,
+} from '../opcode-builder/helpers/vm';
 import { HighLevelResolutionOpcodes } from '../opcode-builder/opcodes';
 import { Compilers } from './compilers';
 
 export const EXPRESSIONS = new Compilers<PushExpressionOp, ExpressionSexpOpcode>();
 
 EXPRESSIONS.add(SexpOpcodes.Concat, (op, [, parts]) => {
   for (let part of parts) {
-    expr(op, part);
+    DynamicAttrValue(op, part);
   }
 
   op(VM_CONCAT_OP, parts.length);

packages/@glimmer/opcode-compiler/lib/syntax/statements.ts

@@ -67,6 +67,7 @@ import { CompilePositional, SimpleArgs } from '../opcode-builder/helpers/shared'
 import {
   Call,
   CallDynamic,
+  DynamicAttrValue,
   DynamicScope,
   PushPrimitiveReference,
 } from '../opcode-builder/helpers/vm';
@@ -123,22 +124,22 @@ STATEMENTS.add(SexpOpcodes.StaticComponentAttr, (op, [, name, value, namespace])
 });
 
 STATEMENTS.add(SexpOpcodes.DynamicAttr, (op, [, name, value, namespace]) => {
-  expr(op, value);
+  DynamicAttrValue(op, value);
   op(VM_DYNAMIC_ATTR_OP, inflateAttrName(name), false, namespace ?? null);
 });
 
 STATEMENTS.add(SexpOpcodes.TrustingDynamicAttr, (op, [, name, value, namespace]) => {
-  expr(op, value);
+  DynamicAttrValue(op, value);
   op(VM_DYNAMIC_ATTR_OP, inflateAttrName(name), true, namespace ?? null);
 });
 
 STATEMENTS.add(SexpOpcodes.ComponentAttr, (op, [, name, value, namespace]) => {
-  expr(op, value);
+  DynamicAttrValue(op, value);
   op(VM_COMPONENT_ATTR_OP, inflateAttrName(name), false, namespace ?? null);
 });
 
 STATEMENTS.add(SexpOpcodes.TrustingComponentAttr, (op, [, name, value, namespace]) => {
-  expr(op, value);
+  DynamicAttrValue(op, value);
   op(VM_COMPONENT_ATTR_OP, inflateAttrName(name), true, namespace ?? null);
 });