Merge pull request #2688 from johanrd/day_fix/template-no-attrs-in-components

Post-merge-review: Fix template-no-attrs-in-components: align detection with upstream

Author: NullVoxPopuli

lib/rules/template-no-attrs-in-components.js

@@ -1,17 +1,29 @@
+// Matches traditional, pod, ui/components, and -components/ paths.
+// Also matches Octane co-located templates (app/components/foo.hbs) via
+// /components/ — cf. ember-template-lint#1445.
+const COMPONENT_TEMPLATE_REGEX = new RegExp(
+  'templates/components|components/.*/template|ui/components|-components/|/components/'
+);
+
 /** @type {import('eslint').Rule.RuleModule} */
 module.exports = {
   meta: {
     type: 'suggestion',
     docs: {
       description: 'disallow attrs in component templates',
       category: 'Deprecations',
-
       url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-no-attrs-in-components.md',
+      templateMode: 'both',
     },
     schema: [],
     messages: {
-      noThisAttrs:
-        'Component templates should not contain `this.attrs`. Use `@arg` syntax instead.',
+      noAttrs: 'Component templates should not contain `attrs`.',
+    },
+    originallyFrom: {
+      name: 'ember-template-lint',
+      rule: 'lib/rules/no-attrs-in-components.js',
+      docs: 'docs/rule/no-attrs-in-components.md',
+      tests: 'test/unit/rules/no-attrs-in-components-test.js',
     },
     originallyFrom: {
       name: 'ember-template-lint',
@@ -21,10 +33,16 @@ module.exports = {
     },
   },
   create(context) {
+    if (!COMPONENT_TEMPLATE_REGEX.test(context.filename)) {
+      return {};
+    }
     return {
       GlimmerPathExpression(node) {
-        if (node.original?.startsWith('this.attrs.') || node.original === 'this.attrs') {
-          context.report({ node, messageId: 'noThisAttrs' });
+        // Flag `attrs.*` and `this.attrs.*` — both are pre-Octane args-leakage
+        // patterns from @ember/component. In the Glimmer AST, `this.attrs.foo`
+        // has parts[0] === 'attrs' (this is the receiver, not a part).
+        if (node.parts && node.parts[0] === 'attrs') {
+          context.report({ node, messageId: 'noAttrs' });
         }
       },
     };

tests/lib/rules/template-no-attrs-in-components.js

@@ -8,40 +8,120 @@ const ruleTester = new RuleTester({
 
 ruleTester.run('template-no-attrs-in-components', rule, {
   valid: [
-    '<template>{{@value}}</template>',
-    '<template>{{this.value}}</template>',
-    // Class component with normal this access
-    `import Component from '@glimmer/component';
-     class MyComponent extends Component {
-       <template>{{this.args.name}}</template>
-     }`,
-    // Bare attrs is not accessible without this, so it's allowed
-    '<template>{{attrs.value}}</template>',
-    '<template>{{attrs}}</template>',
-    `import Component from '@glimmer/component';
-     class MyComponent extends Component {
-       <template>{{attrs.name}}</template>
-     }`,
+    // Not a component template path: nothing is flagged, regardless of content.
+    {
+      filename: 'app/templates/application.hbs',
+      code: '<template>{{@value}}</template>',
+    },
+    {
+      filename: 'app/templates/application.hbs',
+      code: '<template>{{this.value}}</template>',
+    },
+    // `this.attrs.*` outside a component template — not flagged (path gate).
+    {
+      filename: 'app/templates/application.hbs',
+      code: '<template>{{this.attrs.foo}}</template>',
+    },
+    // Even `attrs.*` itself is only flagged inside component templates.
+    {
+      filename: 'app/templates/application.hbs',
+      code: '<template>{{attrs.value}}</template>',
+    },
+    // Inside a component template, non-attrs paths are fine.
+    {
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{@value}}</template>',
+    },
+    {
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{this.value}}</template>',
+    },
+    // Pod-style components path matches the gate, but no `attrs` usage.
+    {
+      filename: 'app/components/foo/template.hbs',
+      code: '<template>{{@value}}</template>',
+    },
+    // `-components/` path gate, no `attrs` usage.
+    {
+      filename: 'app/ui-components/foo.hbs',
+      code: '<template>{{@value}}</template>',
+    },
+    // Octane co-located template — non-attrs path is fine.
+    {
+      filename: 'app/components/foo.hbs',
+      code: '<template>{{@value}}</template>',
+    },
   ],
   invalid: [
+    // Bare `attrs.*` inside `templates/components/` — flagged.
+    {
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{attrs.foo}}</template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // Bare `attrs` (no dotted tail) inside `templates/components/` — flagged.
+    {
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{attrs}}</template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // Pod-style path `components/*/template` — flagged.
+    {
+      filename: 'app/components/foo/template.hbs',
+      code: '<template>{{attrs.name}}</template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // `ui/components` path — flagged.
+    {
+      filename: 'app/ui/components/foo.hbs',
+      code: '<template>{{attrs.name}}</template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // `-components/` path — flagged.
+    {
+      filename: 'app/ui-components/foo.hbs',
+      code: '<template>{{attrs.name}}</template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // `this.attrs.*` inside a component template — flagged (real @ember/component API).
+    {
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{this.attrs.foo}}</template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // attrs in attribute value position.
+    {
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template><div class={{attrs.foo}}></div></template>',
+      output: null,
+      errors: [{ messageId: 'noAttrs' }],
+    },
+    // attrs in block helper condition.
     {
-      code: '<template>{{this.attrs.value}}</template>',
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{#if attrs.foo}}bar{{/if}}</template>',
       output: null,
-      errors: [{ messageId: 'noThisAttrs' }],
+      errors: [{ messageId: 'noAttrs' }],
     },
+    // attrs as hash pair value.
     {
-      code: '<template>{{this.attrs}}</template>',
+      filename: 'app/templates/components/foo.hbs',
+      code: '<template>{{bar foo=attrs.foo}}</template>',
       output: null,
-      errors: [{ messageId: 'noThisAttrs' }],
+      errors: [{ messageId: 'noAttrs' }],
     },
-    // Class component using this.attrs
+    // Octane co-located template (app/components/foo.hbs) — flagged.
     {
-      code: `import Component from '@glimmer/component';
-       class MyComponent extends Component {
-         <template>{{this.attrs.name}}</template>
-       }`,
+      filename: 'app/components/foo.hbs',
+      code: '<template>{{attrs.foo}}</template>',
       output: null,
-      errors: [{ messageId: 'noThisAttrs' }],
+      errors: [{ messageId: 'noAttrs' }],
     },
   ],
 });