Merge pull request #2508 from NullVoxPopuli/nvp/template-lint-extract-rule-template-deprecated-inline-view-helper

Fix locals tracking for deprecated-inline-view

Author: NullVoxPopuli

README.md

@@ -289,7 +289,7 @@ rules in templates can be disabled with eslint directives with mustache or html
 | [no-observers](docs/rules/no-observers.md)                                                       | disallow usage of observers                               | ✅  |    |    |
 | [no-old-shims](docs/rules/no-old-shims.md)                                                       | disallow usage of old shims for modules                   | ✅  | 🔧 |    |
 | [no-string-prototype-extensions](docs/rules/no-string-prototype-extensions.md)                   | disallow usage of `String` prototype extensions           | ✅  |    |    |
-| [template-deprecated-inline-view-helper](docs/rules/template-deprecated-inline-view-helper.md)   | disallow inline {{view}} helper                           |    |    |    |
+| [template-deprecated-inline-view-helper](docs/rules/template-deprecated-inline-view-helper.md)   | disallow inline {{view}} helper                           |    | 🔧 |    |
 | [template-no-action](docs/rules/template-no-action.md)                                           | disallow {{action}} helper                                |    |    |    |
 | [template-no-attrs-in-components](docs/rules/template-no-attrs-in-components.md)                 | disallow attrs in component templates                     |    |    |    |
 | [template-no-link-to-positional-params](docs/rules/template-no-link-to-positional-params.md)     | disallow positional params in LinkTo component            |    |    |    |

docs/rules/template-deprecated-inline-view-helper.md

@@ -1,5 +1,7 @@
 # ember/template-deprecated-inline-view-helper
 
+🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
+
 > **HBS Only**: This rule applies to classic `.hbs` template files only (loose mode). It is not relevant for `gjs`/`gts` files (strict mode), where these patterns cannot occur.
 
 <!-- end auto-generated rule header -->

lib/rules/template-deprecated-inline-view-helper.js

@@ -1,14 +1,3 @@
-function isViewPath(pathNode) {
-  return (
-    pathNode &&
-    pathNode.type === 'GlimmerPathExpression' &&
-    pathNode.original &&
-    pathNode.original.startsWith('view.') &&
-    pathNode.head?.type !== 'ThisHead' &&
-    pathNode.head?.type !== 'AtHead'
-  );
-}
-
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
@@ -19,7 +8,7 @@ module.exports = {
       url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-deprecated-inline-view-helper.md',
       templateMode: 'loose',
     },
-    fixable: null,
+    fixable: 'code',
     schema: [],
     messages: {
       deprecated:
@@ -34,13 +23,52 @@ module.exports = {
   },
 
   create(context) {
+    const sourceCode = context.sourceCode;
+
+    // Track block param names to avoid false positives on locals like:
+    //   {{#each items as |view|}} {{view.name}} {{/each}}
+    const localScopes = [];
+
+    function pushLocals(params) {
+      localScopes.push(new Set(params || []));
+    }
+
+    function popLocals() {
+      localScopes.pop();
+    }
+
+    function isLocal(name) {
+      for (const scope of localScopes) {
+        if (scope.has(name)) {
+          return true;
+        }
+      }
+      return false;
+    }
+
+    function isViewPath(pathNode) {
+      return (
+        pathNode &&
+        pathNode.type === 'GlimmerPathExpression' &&
+        pathNode.original &&
+        pathNode.original.startsWith('view.') &&
+        pathNode.head?.type !== 'ThisHead' &&
+        pathNode.head?.type !== 'AtHead' &&
+        !isLocal('view')
+      );
+    }
+
     function checkHashForViewPaths(node) {
       if (node.hash && node.hash.pairs) {
         for (const pair of node.hash.pairs) {
           if (isViewPath(pair.value)) {
+            const strippedValue = pair.value.original.replace('view.', '');
             context.report({
               node,
               messageId: 'deprecated',
+              fix(fixer) {
+                return fixer.replaceText(pair.value, strippedValue);
+              },
             });
             return true;
           }
@@ -52,11 +80,20 @@ module.exports = {
     function checkForView(node) {
       if (node.path && node.path.type === 'GlimmerPathExpression') {
         // Check for {{view ...}} with params or hash pairs
-        if (node.path.original === 'view') {
-          if (
-            (node.hash && node.hash.pairs && node.hash.pairs.length > 0) ||
-            (node.params && node.params.length > 0)
-          ) {
+        if (node.path.original === 'view' && !isLocal('view')) {
+          if (node.params && node.params.length > 0) {
+            // {{view 'component-name'}} with a single string param is fixable
+            const firstParam = node.params[0];
+            const isFixable =
+              node.params.length === 1 && firstParam.type === 'GlimmerStringLiteral';
+            context.report({
+              node,
+              messageId: 'deprecated',
+              fix: isFixable ? (fixer) => fixer.replaceText(node, `{{${firstParam.value}}}`) : null,
+            });
+            return;
+          }
+          if (node.hash && node.hash.pairs && node.hash.pairs.length > 0) {
             context.report({
               node,
               messageId: 'deprecated',
@@ -66,9 +103,13 @@ module.exports = {
         }
         // Check for {{view.something}} paths
         if (isViewPath(node.path)) {
+          const strippedPath = node.path.original.replace('view.', '');
           context.report({
             node,
             messageId: 'deprecated',
+            fix(fixer) {
+              return fixer.replaceText(node.path, strippedPath);
+            },
           });
           return;
         }
@@ -78,11 +119,33 @@ module.exports = {
     }
 
     return {
-      GlimmerMustacheStatement(node) {
+      GlimmerBlockStatement(node) {
+        // Push this block's params into scope so children can detect locals,
+        // but check the block's own path/hash first (params aren't in scope yet
+        // for the block's own arguments in real Handlebars semantics).
         checkForView(node);
+        if (node.program && node.program.blockParams) {
+          pushLocals(node.program.blockParams);
+        }
+      },
+      'GlimmerBlockStatement:exit'(node) {
+        if (node.program && node.program.blockParams) {
+          popLocals();
+        }
       },
 
-      GlimmerBlockStatement(node) {
+      GlimmerElementNode(node) {
+        if (node.blockParams && node.blockParams.length > 0) {
+          pushLocals(node.blockParams);
+        }
+      },
+      'GlimmerElementNode:exit'(node) {
+        if (node.blockParams && node.blockParams.length > 0) {
+          popLocals();
+        }
+      },
+
+      GlimmerMustacheStatement(node) {
         checkForView(node);
       },
     };

tests/lib/rules/template-deprecated-inline-view-helper.js

@@ -24,42 +24,44 @@ ruleTester.run('template-deprecated-inline-view-helper', rule, {
     '<template>{{this.view}}</template>',
     '<template>{{@view}}</template>',
     '<template>{{#let this.prop as |view|}} {{view}} {{/let}}</template>',
+    // isLocal: view is a block param, view.name should not be flagged
+    '<template>{{#each items as |view|}} {{view.name}} {{/each}}</template>',
   ],
   invalid: [
     {
       code: '<template>{{view class="foo"}}</template>',
       output: null,
       errors: [{ messageId: 'deprecated' }],
     },
-
     {
       code: "<template>{{view 'awful-fishsticks'}}</template>",
-      output: null,
+      output: '<template>{{awful-fishsticks}}</template>',
       errors: [{ messageId: 'deprecated' }],
     },
     {
       code: '<template>{{view.bad-fishsticks}}</template>',
-      output: null,
+      output: '<template>{{bad-fishsticks}}</template>',
       errors: [{ messageId: 'deprecated' }],
     },
     {
       code: '<template>{{view.terrible.fishsticks}}</template>',
-      output: null,
+      output: '<template>{{terrible.fishsticks}}</template>',
       errors: [{ messageId: 'deprecated' }],
     },
     {
       code: '<template>{{foo-bar bab=good baz=view.qux.qaz boo=okay}}</template>',
-      output: null,
+      output: '<template>{{foo-bar bab=good baz=qux.qaz boo=okay}}</template>',
       errors: [{ messageId: 'deprecated' }],
     },
     {
       code: '<template><div class="whatever-class" data-foo={{view.hallo}} sure=thing></div></template>',
-      output: null,
+      output:
+        '<template><div class="whatever-class" data-foo={{hallo}} sure=thing></div></template>',
       errors: [{ messageId: 'deprecated' }],
     },
     {
       code: '<template>{{#foo-bar derp=view.whoops thing=whatever}}{{/foo-bar}}</template>',
-      output: null,
+      output: '<template>{{#foo-bar derp=whoops thing=whatever}}{{/foo-bar}}</template>',
       errors: [{ messageId: 'deprecated' }],
     },
   ],
@@ -89,11 +91,13 @@ hbsRuleTester.run('template-deprecated-inline-view-helper', rule, {
     '{{this.view}}',
     '{{@view}}',
     '{{#let this.prop as |view|}} {{view}} {{/let}}',
+    // isLocal: view is a block param, view.name should not be flagged
+    '{{#each items as |view|}} {{view.name}} {{/each}}',
   ],
   invalid: [
     {
       code: "{{view 'awful-fishsticks'}}",
-      output: null,
+      output: '{{awful-fishsticks}}',
       errors: [
         {
           message:
@@ -103,7 +107,7 @@ hbsRuleTester.run('template-deprecated-inline-view-helper', rule, {
     },
     {
       code: '{{view.bad-fishsticks}}',
-      output: null,
+      output: '{{bad-fishsticks}}',
       errors: [
         {
           message:
@@ -113,7 +117,7 @@ hbsRuleTester.run('template-deprecated-inline-view-helper', rule, {
     },
     {
       code: '{{view.terrible.fishsticks}}',
-      output: null,
+      output: '{{terrible.fishsticks}}',
       errors: [
         {
           message:
@@ -123,7 +127,7 @@ hbsRuleTester.run('template-deprecated-inline-view-helper', rule, {
     },
     {
       code: '{{foo-bar bab=good baz=view.qux.qaz boo=okay}}',
-      output: null,
+      output: '{{foo-bar bab=good baz=qux.qaz boo=okay}}',
       errors: [
         {
           message:
@@ -133,7 +137,7 @@ hbsRuleTester.run('template-deprecated-inline-view-helper', rule, {
     },
     {
       code: '<div class="whatever-class" data-foo={{view.hallo}} sure=thing></div>',
-      output: null,
+      output: '<div class="whatever-class" data-foo={{hallo}} sure=thing></div>',
       errors: [
         {
           message:
@@ -143,7 +147,7 @@ hbsRuleTester.run('template-deprecated-inline-view-helper', rule, {
     },
     {
       code: '{{#foo-bar derp=view.whoops thing=whatever}}{{/foo-bar}}',
-      output: null,
+      output: '{{#foo-bar derp=whoops thing=whatever}}{{/foo-bar}}',
       errors: [
         {
           message: