Make unambiguous helpers opt-in, fixes #514, fixes #504

Author: lolmaus

README.md

@@ -191,6 +191,24 @@ If you would like to only convert certain component invocations to use the angle
 }
 ```
 
+### Making helper invocations unambiguous
+
+In order to make helper invocations unambiguous, use this:
+
+**config/anglebrackets-codemod-config.json**
+
+```js
+{
+  "unambiguousHelpers": true
+}
+```
+
+This will result in invocations like `{{concat "foo" "bar"}}` to be converted into `{{(concat "foo" "bar")}}`, which may be useful in strict-mode Embroider.
+
+Note that it does not work in non-Embroider Ember, as of January 2024.
+
+Note that ambiguous invocations, that cannot be statically distinguished between a helper, a property and a component, will not be modified.
+
 ## Debugging Workflow
 
 Oftentimes, you want to debug the codemod or the transform to identify issues with the code or to understand

test/fixtures/with-telemetry/input/app/templates/application.hbs

@@ -3,15 +3,15 @@
     <h3 class="sr-only">
       Components
     </h3>
-    {{#bs-nav type="pills" stacked=true as |nav|}}
+    <BsNav @type="pills" @stacked={{true}} as |nav|>
       {{#each this.model as |comp|}}
-        {{#nav.item}}
-          {{#nav.link-to route=comp.demoRoute}}
+        <nav.item>
+          <nav.link-to @route={{comp.demoRoute}}>
             {{comp.title}}
-          {{/nav.link-to}}
-        {{/nav.item}}
+          </nav.link-to>
+        </nav.item>
       {{/each}}
-    {{/bs-nav}}
+    </BsNav>
   </div>
   <div class="col-sm-8 col-sm-pull-4 col-md-9 col-md-pull-3">
     {{utils/bee-bop}}
@@ -28,13 +28,13 @@
     {{/if}}
     {{outlet}}
 
-    {{#bs-button id="openModal" onClick=(action this.addModal)}}Open{{/bs-button}}
+    {{#bs-button id="openModal" onClick=(action "addModal")}}Open{{/bs-button}}
 
     {{#if hasModal}}
       {{#bs-modal-simple open=modal onHidden=(action "removeModal") title="Dynamic Dialog"}}
         Hi there
       {{/bs-modal-simple}}
     {{/if}}
-    {{file-less foo=true}}
+    <FileLess @foo={{true}} />
   </div>
 </div>

test/fixtures/with-telemetry/input/app/templates/components/file-less.hbs

@@ -1,2 +1,2 @@
 <div>this template has no js </div>
-{{#bs-button type="primary"}}Primary{{/bs-button}}
+<BsButton @type="primary">Primary</BsButton>

test/fixtures/with-telemetry/output/app/templates/application.hbs

@@ -14,9 +14,9 @@
     </BsNav>
   </div>
   <div class="col-sm-8 col-sm-pull-4 col-md-9 col-md-pull-3">
-    {{(utils/bee-bop)}}
-    {{(-wat-wat)}}
-    {{(utils/-wat-wat)}}
+    <Utils::BeeBop />
+    {{-wat-wat}}
+    <Utils::-WatWat />
     {{#if this.isDetailPage}}
       <h1>
         {{currentComponent.title}}
@@ -28,7 +28,7 @@
     {{/if}}
     {{outlet}}
 
-    <BsButton @id="openModal" @onClick={{action this.addModal}}>Open</BsButton>
+    <BsButton @id="openModal" @onClick={{action "addModal"}}>Open</BsButton>
 
     {{#if hasModal}}
       <BsModalSimple @open={{modal}} @onHidden={{action "removeModal"}} @title="Dynamic Dialog">

test/fixtures/with-telemetry/output/app/templates/components/foo-bar.hbs

@@ -1,4 +1,4 @@
 <button>
-  {{(titleize "string helpers for ember")}}
+  {{titleize "string helpers for ember"}}
 </button>
-{{(biz-baz canConvert="no" why="helper" where="local")}}
+{{biz-baz  canConvert="no" why="helper" where="local"}}

test/fixtures/without-telemetry/input/app/templates/application.hbs

@@ -1,9 +1,9 @@
-{{site-header user=this.user class=(if this.user.isAdmin "admin")}}
+<SiteHeader @user={{this.user}} @class={{if this.user.isAdmin "admin"}} />
 
-{{#super-select selected=this.user.country as |s|}}
+<SuperSelect @selected={{this.user.country}} as |s|>
   {{#each this.availableCountries as |country|}}
-    {{#s.option value=country}}{{country.name}}{{/s.option}}
+    <s.option @value={{country}}>{{country.name}}</s.option>
   {{/each}}
-{{/super-select}}
+</SuperSelect>
 
-{{ui/button text="Click me"}}
+<Ui::Button @text="Click me" />

transforms/angle-brackets/index.js

@@ -31,6 +31,7 @@ function getOptions() {
 
     options.includeValuelessDataTestAttributes = !!config.includeValuelessDataTestAttributes;
     options.skipBuiltInComponents = !!config.skipBuiltInComponents;
+    options.unambiguousHelpers = !!config.unambiguousHelpers;
   }
 
   if (cliOptions.telemetry) {

transforms/angle-brackets/transform.js

@@ -45,12 +45,16 @@ function isBuiltInComponent(key) {
   return BUILT_IN_COMPONENTS.includes(key);
 }
 
-function isNestedComponentTagName(tagName) {
-  return (
-    tagName &&
-    tagName.includes &&
-    (tagName.includes('/') || (tagName.includes('-') && tagName.includes('.')))
-  );
+function isNestedComponentTagName(tagName, unambiguousHelpers = false) {
+  if (unambiguousHelpers) {
+    return (
+      tagName &&
+      tagName.includes &&
+      (tagName.includes('/') || (tagName.includes('-') && tagName.includes('.')))
+    );
+  }
+
+  return tagName && tagName.includes && (tagName.includes('/') || tagName.includes('-'));
 }
 
 function isWallStreet(tagName) {
@@ -341,18 +345,21 @@ function isKnownHelper(fullName, config, invokableData) {
   }
 
   if (isTelemetryData) {
-    let isComponent =
-      !config.helpers.includes(name) &&
-      [...(components || []), ...BUILT_IN_COMPONENTS].includes(name);
+    if (config.unambiguousHelpers) {
+      let isComponent =
+        !config.helpers.includes(name) &&
+        [...(components || []), ...BUILT_IN_COMPONENTS].includes(name);
 
-    if (isComponent) {
-      return false;
+      if (isComponent) {
+        return false;
+      }
     }
 
     let mergedHelpers = [...KNOWN_HELPERS, ...(helpers || [])];
     let isHelper = mergedHelpers.includes(name) || config.helpers.includes(name);
+    let isComponent = [...(components || []), ...BUILT_IN_COMPONENTS].includes(name);
     let strName = `${name}`; // coerce boolean and number to string
-    return isHelper && !strName.includes('.');
+    return (isHelper || (!config.unambiguousHelpers && !isComponent)) && !strName.includes('.');
   } else {
     return KNOWN_HELPERS.includes(name) || config.helpers.includes(name);
   }
@@ -480,15 +487,19 @@ function transformToAngleBracket(fileInfo, config, invokableData) {
       const isTagKnownHelper = isKnownHelper(tagName, config, invokableData);
 
       // Don't change attribute statements
-      const isValidMustacheComponent = node.loc.source !== '(synthetic)' && !isTagKnownHelper;
-      const isNestedComponent = isNestedComponentTagName(tagName);
+      const isValidMustacheComponent = config.unambiguousHelpers
+        ? node.loc.source !== '(synthetic)' && !isTagKnownHelper
+        : node.loc.source !== '(synthetic)' && !isKnownHelper(tagName, config, invokableData);
+
+      const isNestedComponent = isNestedComponentTagName(tagName, config.unambiguousHelpers);
 
       if (
         isValidMustacheComponent &&
         (node.hash.pairs.length > 0 || node.params.length > 0 || isNestedComponent)
       ) {
         return transformComponentNode(node, fileInfo, config);
       } else if (
+        config.unambiguousHelpers &&
         isTagKnownHelper &&
         node.path.type !== 'SubExpression' &&
         walkerPath.parent.node.type !== 'AttrNode' &&

transforms/angle-brackets/transform.test.js

@@ -476,7 +476,7 @@ test('let', () => {
         {{#let (capitalize this.person.firstName) (capitalize this.person.lastName)
           as |firstName lastName|
         }}
-          Welcome back {{(concat firstName ' ' lastName)}}
+          Welcome back {{concat firstName ' ' lastName}}
 
           Account Details:
           First Name: {{firstName}}
@@ -554,8 +554,8 @@ test('link-to-inline', () => {
         <LinkTo @route=\\"apps.segments\\" class=\\"tabs__discrete-tab\\" @activeClass=\\"o__selected\\" @current-when=\\"apps.segments\\" data-test-segment-link=\\"segments\\">Segments</LinkTo>
         <LinkTo @route={{this.dynamicPath}} class=\\"tabs__discrete-tab\\" @activeClass=\\"o__selected\\" @current-when=\\"apps.segments\\" data-test-segment-link=\\"segments\\">Segments</LinkTo>
         <LinkTo @route=\\"apps.app.companies.segments.segment\\" @model={{segment}} class=\\"t__em-link\\">{{segment.name}}</LinkTo>
-        <LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{(t \\"show\\")}}</LinkTo>
-        <LinkTo @route=\\"user\\" @model={{if linkActor event.actor.id event.user.id}}>{{(t \\"show\\")}}</LinkTo>
+        <LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{t \\"show\\"}}</LinkTo>
+        <LinkTo @route=\\"user\\" @model={{if linkActor event.actor.id event.user.id}}>{{t \\"show\\"}}</LinkTo>
         <LinkTo @route=\\"user\\" @models={{array this.first this.second}}>Show</LinkTo>
         <LinkTo @route=\\"user\\" @models={{array this.first this.second}} @query={{hash foo=\\"baz\\"}}>Show</LinkTo>
         <LinkTo @route=\\"user\\" @model={{this.first}}>Show</LinkTo>
@@ -839,7 +839,7 @@ test('t-helper', () => {
 
   expect(runTest('t-helper.hbs', input)).toMatchInlineSnapshot(`
     "
-        {{(t \\"some.string\\" param=\\"string\\" another=1)}}
+        {{t \\"some.string\\" param=\\"string\\" another=1}}
       "
   `);
 });
@@ -878,7 +878,7 @@ test('tilde', () => {
   expect(runTest('tilde.hbs', input)).toMatchInlineSnapshot(`
     "
         {{#if foo~}}
-          {{some-component}}
+          <SomeComponent />
         {{/if}}
       "
   `);
@@ -977,7 +977,7 @@ test('skip-default-helpers', () => {
   expect(runTest('skip-default-helpers.hbs', input, options)).toMatchInlineSnapshot(`
     "
         <div id=\\"main-container\\">
-          {{(liquid-outlet)}}
+          {{liquid-outlet}}
         </div>
         <button {{action \\"toggle\\" \\"showA\\"}}>
           Toggle A/B</button>
@@ -997,11 +997,11 @@ test('skip-default-helpers', () => {
             Two
           </div>
         {{/liquid-if}}
-        {{(moment '12-25-1995' 'MM-DD-YYYY')}}
-        {{(moment-from '1995-12-25' '2995-12-25' hideAffix=true)}}
+        {{moment '12-25-1995' 'MM-DD-YYYY'}}
+        {{moment-from '1995-12-25' '2995-12-25' hideAffix=true}}
         <SomeComponent @foo={{true}} />
-        {{(some-helper1 foo=true)}}
-        {{(some-helper2 foo=true)}}
+        {{some-helper1 foo=true}}
+        {{some-helper2 foo=true}}
       "
   `);
 });
@@ -1039,7 +1039,7 @@ test('skip-default-helpers (no-config)', () => {
   expect(runTest('skip-default-helpers.hbs', input)).toMatchInlineSnapshot(`
     "
         <div id=\\"main-container\\">
-          {{(liquid-outlet)}}
+          {{liquid-outlet}}
         </div>
         <button {{action \\"toggle\\" \\"showA\\"}}>
           Toggle A/B</button>
@@ -1059,8 +1059,8 @@ test('skip-default-helpers (no-config)', () => {
             Two
           </div>
         {{/liquid-if}}
-        {{(moment '12-25-1995' 'MM-DD-YYYY')}}
-        {{(moment-from '1995-12-25' '2995-12-25' hideAffix=true)}}
+        {{moment '12-25-1995' 'MM-DD-YYYY'}}
+        {{moment-from '1995-12-25' '2995-12-25' hideAffix=true}}
         <SomeComponent @foo={{true}} />
         <SomeHelper1 @foo={{true}} />
         <SomeHelper2 @foo={{true}} />
@@ -1086,8 +1086,8 @@ test('custom-options', () => {
   expect(runTest('custom-options.hbs', input, options)).toMatchInlineSnapshot(`
     "
         <SomeComponent @foo={{true}} />
-        {{(some-helper1 foo=true)}}
-        {{(some-helper2 foo=true)}}
+        {{some-helper1 foo=true}}
+        {{some-helper2 foo=true}}
         {{link-to \\"Title\\" \\"some.route\\"}}
         {{textarea value=this.model.body}}
         {{input type=\\"checkbox\\" name=\\"email-opt-in\\" checked=this.model.emailPreference}}
@@ -1180,7 +1180,7 @@ test('preserve arguments', () => {
     "
         <FooBar @class=\\"baz\\" />
         {{foo-bar data-baz class=\\"baz\\"}}
-        <LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{(t \\"show\\")}}</LinkTo>
+        <LinkTo @route=\\"flight\\" @model={{event.flight.id}} class=\\"btn btn-default btn-sm pull-right\\">{{t \\"show\\"}}</LinkTo>
       "
   `);
 });
@@ -1284,26 +1284,26 @@ test('wallstreet-telemetry', () => {
   expect(runTest('wallstreet-telemetry.hbs', input)).toMatchInlineSnapshot(`
     "
         {{nested$helper}}
-        {{(nested::helper)}}
-        {{(nested::helper param=\\"yeah!\\")}}
-        {{(helper-1)}}
+        {{nested::helper}}
+        {{nested::helper param=\\"yeah!\\"}}
+        {{helper-1}}
       "
   `);
 });
 
 test('wrapping-helpers-with-parens', () => {
   let input = `
     {{fooknownhelper}}
-    {{(fooknownhelper)}}
+    {{fooknownhelper}}
     {{fooknownhelper data-test-foo foo="bar"}}
     {{foounknownhelper}}
   `;
 
   expect(runTest('wrapping-helpers-with-parens.hbs', input)).toMatchInlineSnapshot(`
     "
-        {{(fooknownhelper)}}
-        {{(fooknownhelper)}}
-        {{(fooknownhelper data-test-foo foo=\\"bar\\")}}
+        {{fooknownhelper}}
+        {{fooknownhelper}}
+        {{fooknownhelper data-test-foo foo=\\"bar\\"}}
         {{foounknownhelper}}
       "
   `);
@@ -1382,7 +1382,24 @@ test('unknown helper with args', () => {
     "
         <ApiReference @component={{this.currentComponent}} />
         {{api-reference someArg}}
-        {{api-reference}}
+        <ApiReference />
+      "
+  `);
+});
+
+test('unambiguousHelpers: true', () => {
+  let input = `
+    {{concat}}
+    {{unknown}}
+    {{t "some.string" param="string" another=1}}
+  `;
+
+  expect(runTest('unambiguousHelpers: true', input, { unambiguousHelpers: true }))
+    .toMatchInlineSnapshot(`
+    "
+        {{(concat)}}
+        {{unknown}}
+        {{(t \\"some.string\\" param=\\"string\\" another=1)}}
       "
   `);
 });