Make helpers unambiguious with parens aka subexpressions

Author: lolmaus

transforms/angle-brackets/known-helpers.js

@@ -18,14 +18,12 @@ const KNOWN_HELPERS = [
   'loc',
   'log',
   'mut',
-  'outlet',
   'partial',
   'query-params',
   'readonly',
   'unbound',
   'unless',
   'with',
-  'yield',
 
   // Glimmer VM
   'identity', // glimmer blocks

transforms/angle-brackets/telemetry/mock-invokables.js

@@ -101,4 +101,7 @@ module.exports = {
   'app/helpers/nested/helper': {
     type: 'Helper',
   },
+  'app/helpers/fooknownhelper': {
+    type: 'Helper',
+  },
 };

transforms/angle-brackets/transform.js

@@ -326,7 +326,7 @@ function getNonDataAttributesFromParams(params) {
   return params.filter((it) => !isDataAttrPathExpression(it));
 }
 
-function shouldIgnoreMustacheStatement(fullName, config, invokableData) {
+function isKnownHelper(fullName, config, invokableData) {
   let { helpers, components } = invokableData;
   let isTelemetryData = !!(helpers || components);
 
@@ -337,11 +337,18 @@ function shouldIgnoreMustacheStatement(fullName, config, invokableData) {
   }
 
   if (isTelemetryData) {
+    let isComponent =
+      !config.helpers.includes(name) &&
+      [...(components || []), ...BUILT_IN_COMPONENTS].includes(name);
+
+    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 || !isComponent) && !strName.includes('.');
+    return isHelper && !strName.includes('.');
   } else {
     return KNOWN_HELPERS.includes(name) || config.helpers.includes(name);
   }
@@ -363,7 +370,7 @@ function nodeHasPositionalParameters(node) {
   return false;
 }
 
-function transformNode(node, fileInfo, config) {
+function transformComponentNode(node, fileInfo, config) {
   if (
     hasValuelessDataParams(node.params) &&
     shouldSkipDataTestParams(node.params, config.includeValuelessDataTestAttributes)
@@ -425,6 +432,10 @@ function transformNode(node, fileInfo, config) {
   );
 }
 
+function transformHelperNode(node) {
+  return b.mustache(b.sexpr(node.path, node.params, node.hash));
+}
+
 function subExpressionToMustacheStatement(subExpression) {
   return b.mustache(subExpression.path, subExpression.params, subExpression.hash);
 }
@@ -457,34 +468,38 @@ function transformToAngleBracket(fileInfo, config, invokableData) {
    * Transform the attributes names & values properly
    */
   return {
-    MustacheStatement(node) {
+    MustacheStatement(node, walkerPath) {
       const tagName = `${node.path && node.path.original}`;
 
       if (config.components && !config.components.includes(tagName)) return;
 
+      const isTagKnownHelper = isKnownHelper(tagName, config, invokableData);
+
       // Don't change attribute statements
-      const isValidMustache =
-        node.loc.source !== '(synthetic)' &&
-        !shouldIgnoreMustacheStatement(tagName, config, invokableData);
+      const isValidMustacheComponent = node.loc.source !== '(synthetic)' && !isTagKnownHelper;
       const isNestedComponent = isNestedComponentTagName(tagName);
 
       if (
-        isValidMustache &&
+        isValidMustacheComponent &&
         (node.hash.pairs.length > 0 || node.params.length > 0 || isNestedComponent)
       ) {
-        return transformNode(node, fileInfo, config);
+        return transformComponentNode(node, fileInfo, config);
+      } else if (
+        isTagKnownHelper &&
+        node.path.type !== 'SubExpression' &&
+        walkerPath.parent.node.type !== 'AttrNode' &&
+        walkerPath.parent.node.type !== 'ConcatStatement'
+      ) {
+        return transformHelperNode(node, walkerPath);
       }
     },
     BlockStatement(node) {
       let tagName = `${node.path.original}`;
 
       if (config.components && !config.components.includes(tagName)) return;
 
-      if (
-        !shouldIgnoreMustacheStatement(node.path.original, config, invokableData) ||
-        isWallStreet(tagName)
-      ) {
-        return transformNode(node, fileInfo, config);
+      if (!isKnownHelper(node.path.original, config, invokableData) || isWallStreet(tagName)) {
+        return transformComponentNode(node, fileInfo, config);
       }
     },
     AttrNode: {

transforms/angle-brackets/transform.test.js

@@ -288,7 +288,6 @@ test('deeply-nested-sub', () => {
       )
     }}
   `;
-
   /**
    * NOTE: An issue has been opened in `ember-template-recast` (https://github.com/ember-template-lint/ember-template-recast/issues/82)
    * regarding to create an API to allow a transform to customize the whitespace for newly created nodes.
@@ -477,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}}
@@ -555,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>
@@ -840,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)}}
       "
   `);
 });
@@ -978,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>
@@ -998,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)}}
       "
   `);
 });
@@ -1040,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>
@@ -1060,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}} />
@@ -1087,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}}
@@ -1181,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>
       "
   `);
 });
@@ -1278,18 +1277,34 @@ test('wallstreet-telemetry', () => {
   let input = `
     {{nested$helper}}
     {{nested::helper}}
-    {{nested$helper param="cool!"}}
     {{nested::helper param="yeah!"}}
     {{helper-1}}
   `;
 
   expect(runTest('wallstreet-telemetry.hbs', input)).toMatchInlineSnapshot(`
     "
         {{nested$helper}}
-        {{nested::helper}}
-        {{nested$helper param=\\"cool!\\"}}
-        {{nested::helper param=\\"yeah!\\"}}
-        {{helper-1}}
+        {{(nested::helper)}}
+        {{(nested::helper param=\\"yeah!\\")}}
+        {{(helper-1)}}
+      "
+  `);
+});
+
+test('wrapping-helpers-with-parens', () => {
+  let input = `
+    {{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\\")}}
+        {{foounknownhelper}}
       "
   `);
 });
@@ -1335,3 +1350,23 @@ test('No telemetry', () => {
       "
   `);
 });
+
+test('pipe', () => {
+  let input = `<Foo @title={{concat ">"}} as |bar|></Foo>`;
+
+  expect(runTestWithData('pipe.hbs', input, {}, {})).toMatchInlineSnapshot(
+    `"<Foo @title={{concat \\">\\"}} as |bar|></Foo>"`
+  );
+});
+
+test('outlet', () => {
+  let input = `{{outlet}}`;
+
+  expect(runTestWithData('pipe.hbs', input, {}, {})).toMatchInlineSnapshot(`"{{outlet}}"`);
+});
+
+test('yield', () => {
+  let input = `{{yield}}`;
+
+  expect(runTestWithData('pipe.hbs', input, {}, {})).toMatchInlineSnapshot(`"{{yield}}"`);
+});