Repository: ember-cli/eslint-plugin-ember (js)

Issue:
New Octane Rule: `require-tagless-components`
Pulled from https://github.com/ember-cli/eslint-plugin-ember/issues/533 which proposes a new set of rules for Octane related best practices

Only allow `@glimmer/component` base class, or tagName: '' if `@ember/component` is used

```js
// ### Good
import Component from '@glimmer/component'
export default class MyComponent extends Component {
  ...
}

import Component from '@ember/component'
export default class MyComponent extends Component {
  tagName = '';
}

import Component from '@ember/component'
export default Component.extend({
  tagName: ''
});


// ### Bad
// implicit `<div>`
import Component from '@ember/component'
export default class MyComponent extends Component {
  ...
}

import Component from '@ember/component'
export default Component.extend({
  ...
});

// custom tag
import Component from '@ember/component'
export default class MyComponent extends Component {
  tagName = 'span';
}

import Component from '@ember/component'
export default Component.extend({
  tagName: 'span'
});
```

If anyone would like to implement this please comment that you'll take it and work away.

Pull request description:
Add new `require-tagless-components` rule
Introduces lint rule to ensure that components do not have tag names

Closes #541

Patch:
diff --git a/README.md b/README.md
index f605776b..1271e1a7 100644
--- a/README.md
+++ b/README.md
@@ -145,6 +145,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 |  | [no-classic-classes](./docs/rules/no-classic-classes.md) | Disallow "classic" classes in favor of native JS classes |
 |  | [no-classic-components](./docs/rules/no-classic-components.md) | Enforces Glimmer components |
 |  | [no-computed-properties-in-native-classes](./docs/rules/no-computed-properties-in-native-classes.md) | Disallows using computed properties in native classes |
+|  | [require-tagless-components](./docs/rules/require-tagless-components.md) | Disallows using the wrapper element of a Component |
 
 
 ### Ember Data
diff --git a/docs/rules/require-tagless-components.md b/docs/rules/require-tagless-components.md
new file mode 100644
index 00000000..128a5681
--- /dev/null
+++ b/docs/rules/require-tagless-components.md
@@ -0,0 +1,110 @@
+# Disallows using the wrapper element of a Component (require-tagless-components)
+
+Ember allows you to disable the wrapper element on a component (turning it into a "tagless" component). This is now the preferred style and the _only_ style allowed with Glimmer components.
+
+## Rule Details
+
+Instead of having the wrapper element implicitly defined by the component, all DOM elements should be represented in the component's template.
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+// "Classic"-class Ember components that have the default `tagName` of `div`
+import Component from '@ember/component';
+
+export default Component.extend({});
+```
+
+```javascript
+// "Classic"-class Ember components that have a `tagName` configured to something besides `''`
+import Component from '@ember/component';
+
+export default Component.extend({
+  tagName: 'span'
+});
+```
+
+```javascript
+// "Native"-class Ember components that have the default `tagName` of `div`
+import Component from '@ember/component';
+
+export default class MyComponent extends Component {}
+```
+
+```javascript
+// "Native"-class Ember components that have a `tagName` configured to something besides `''`
+import Component from '@ember/component';
+
+export default class MyComponent extends Component {
+  tagName = 'span'
+}
+```
+
+```javascript
+// "Native"-class Ember components that use the `@tagName` decorator configured to something besides `''`
+import Component from '@ember/component';
+import { tagName } from '@ember-decorators/component';
+
+@tagName('span')
+export default class MyComponent extends Component {}
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+// "Class"-class Ember components that have a `tagName` configured to `''`
+import Component from '@ember/component';
+
+export default Component.extend({
+  tagName: ''
+});
+```
+
+```javascript
+// "Native"-class Ember components that have a `tagName` configured to `''`
+import Component from '@ember/component';
+
+export default class MyComponent extends Component {
+  tagName = ''
+}
+```
+
+```javascript
+// "Native"-class Ember components that use the `@tagName` decorator configured `''`
+import Component from '@ember/component';
+import { tagName } from '@ember-decorators/component';
+
+@tagName('')
+export default class MyComponent extends Component {}
+```
+
+```javascript
+// Glimmer components never have a `tagName` and are always valid
+import Component from '@glimmer/component';
+
+export default class MyComponent extends Component {}
+```
+
+### Caveats
+
+* Rule does not catch cases where a Mixin is included to configure the `tagName` property.
+* Rule does not catch cases where a decorator is applied to configure the `tagName` property.
+
+## Fixing This Rule
+
+You can take two approaches to fixing this rule:
+
+1. Convert to a Glimmer component
+2. Set the `tagName` in your Ember component definition to `''` (an empty string)
+
+Note that you might want to wrap the component template in an additional element, if the usage of the component expects the wrapping element to exist. Classes and attributes should be applied to that wrapper element, as well as forwarding any attributes assigned to the component through `...attributes`.
+
+## When Not To Use It
+
+* If you are not prepared to convert your components to be tag-less, wrapping the associated template in a tag and applying `...attributes` where appropriate, you should not enable this rule.
+
+## Further Reading
+
+* [Glimmer Components](https://glimmerjs.com/guides/components-and-actions)
+* [RFC #311 "Angle Bracket Invocation" (HTML Attributes section)](https://emberjs.github.io/rfcs/0311-angle-bracket-invocation.html#html-attributes)
+  * Discusses forwarding attributes on a component to a DOM element using `...attributes`
diff --git a/lib/index.js b/lib/index.js
index 11466152..1b932a8b 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -52,6 +52,7 @@ module.exports = {
     'require-computed-property-dependencies': require('./rules/require-computed-property-dependencies'),
     'require-return-from-computed': require('./rules/require-return-from-computed'),
     'require-super-in-init': require('./rules/require-super-in-init'),
+    'require-tagless-components': require('./rules/require-tagless-components'),
     'route-path-style': require('./rules/route-path-style'),
     'routes-segments-snake-case': require('./rules/routes-segments-snake-case'),
     'use-brace-expansion': require('./rules/use-brace-expansion'),
diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js
index 6bfb76f0..82b38c2c 100644
--- a/lib/recommended-rules.js
+++ b/lib/recommended-rules.js
@@ -55,6 +55,7 @@ module.exports = {
   "ember/require-computed-property-dependencies": "off",
   "ember/require-return-from-computed": "error",
   "ember/require-super-in-init": "error",
+  "ember/require-tagless-components": "off",
   "ember/route-path-style": "off",
   "ember/routes-segments-snake-case": "error",
   "ember/use-brace-expansion": "error",
diff --git a/lib/rules/new-module-imports.js b/lib/rules/new-module-imports.js
index 08daa6b9..f07279f9 100644
--- a/lib/rules/new-module-imports.js
+++ b/lib/rules/new-module-imports.js
@@ -1,12 +1,8 @@
 'use strict';
 
 const MAPPING = require('ember-rfc176-data');
-const {
-  buildMessage,
-  getFullNames,
-  isDestructuring,
-  isIdentifierImportedFrom,
-} = require('../utils/new-module');
+const { buildMessage, getFullNames, isDestructuring } = require('../utils/new-module');
+const { getSourceModuleNameForIdentifier } = require('../utils/utils');
 
 const GLOBALS = MAPPING.reduce((memo, exportDefinition) => {
   if (exportDefinition.deprecated) {
@@ -58,7 +54,10 @@ module.exports = {
 
     return {
       VariableDeclarator(node) {
-        if (!isDestructuring(node) || !isIdentifierImportedFrom(node, 'ember')) {
+        if (
+          !isDestructuring(node) ||
+          getSourceModuleNameForIdentifier(context, node.init) !== 'ember'
+        ) {
           return;
         }
 
diff --git a/lib/rules/no-classic-classes.js b/lib/rules/no-classic-classes.js
index 32813c2d..d5031bf1 100644
--- a/lib/rules/no-classic-classes.js
+++ b/lib/rules/no-classic-classes.js
@@ -1,6 +1,6 @@
 'use strict';
 
-const { getImportModule } = require('../utils/utils');
+const { getSourceModuleNameForIdentifier } = require('../utils/utils');
 const { isObjectExpression } = require('../utils/types');
 
 const ERROR_MESSAGE_NO_CLASSIC_CLASSES =
@@ -31,10 +31,6 @@ module.exports = {
   },
 
   create(context) {
-    function getModuleForNode(node) {
-      return getImportModule(context, node);
-    }
-
     function reportNode(node) {
       context.report(node, ERROR_MESSAGE_NO_CLASSIC_CLASSES);
     }
@@ -47,7 +43,7 @@ module.exports = {
         // Invalid with an object as an argument because that logic needsconversion to a native class
         // Still allows `.extend` if some other identifier is passed, like a Mixin
         if (hasNoArguments(callExpression) || hasObjectArgument(callExpression)) {
-          const classImportedFrom = getModuleForNode(node.object);
+          const classImportedFrom = getSourceModuleNameForIdentifier(context, node.object);
 
           // Only warn about classes imported from the `@ember/` namespace
           if (classImportedFrom && classImportedFrom.startsWith('@ember/')) {
diff --git a/lib/rules/require-tagless-components.js b/lib/rules/require-tagless-components.js
new file mode 100644
index 00000000..a30d5269
--- /dev/null
+++ b/lib/rules/require-tagless-components.js
@@ -0,0 +1,155 @@
+'use strict';
+
+const { isEmberComponent } = require('../utils/ember');
+const { getSourceModuleNameForIdentifier } = require('../utils/utils');
+const {
+  isCallExpression,
+  isIdentifier,
+  isObjectExpression,
+  isStringLiteral,
+} = require('../utils/types');
+
+const ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS =
+  "Please switch to a tagless component by setting `tagName: ''` or converting to a Glimmer component";
+
+/**
+ * Whether or not the node represents an identifier called `tagName`
+ *
+ * @param {object} node
+ * @param {boolean}
+ */
+function isTagNameIdentifier(node) {
+  return isIdentifier(node) && node.name === 'tagName';
+}
+
+/**
+ * Whether or not the node represents a property called `tagName`
+ *
+ * @param {object} node
+ * @return {boolean}
+ */
+function isTagNameProperty(node) {
+  return isTagNameIdentifier(node.key);
+}
+
+/**
+ * @param {Property|ClassProperty} node
+ * @return {boolean}
+ */
+function isNonEmptyTagNameProperty(node) {
+  return isTagNameProperty(node) && isStringLiteral(node.value) && node.value.value !== '';
+}
+
+/**
+ * @param {ObjectExpression} node
+ * @return {Property=}
+ */
+function getNonEmptyTagNameInObjectExpression(node) {
+  return node.properties.find(isNonEmptyTagNameProperty);
+}
+
+/**
+ * @param {ClassBody} node
+ * @return {ClassProperty=}
+ */
+function getNonEmptyTagNameInClassBody(node) {
+  return node.body.find(isNonEmptyTagNameProperty);
+}
+
+/**
+ * @param {ObjectExpression} node
+ * @return {boolean}
+ */
+function hasNoTagNameInObjectExpression(node) {
+  return !node.properties.some(isTagNameProperty);
+}
+
+/**
+ * @param {ClassBody} node
+ * @return {boolean}
+ */
+function hasNoTagNameInClassBody(node) {
+  return !node.body.some(isTagNameProperty);
+}
+
+/**
+ * @param {ClassBody} node
+ * @param {string} name
+ * @return {Decorator | undefined}
+ */
+function getDecoratorCallExpressionWithName(node, name) {
+  return (
+    node.decorators &&
+    node.decorators.find(d => isCallExpression(d.expression) && d.expression.callee.name === name)
+  );
+}
+
+module.exports = {
+  ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS,
+
+  meta: {
+    docs: {
+      description: 'Disallows using the wrapper element of a Component',
+      category: 'Ember Octane',
+      recommended: false,
+      url:
+        'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/require-tagless-components.md',
+    },
+    fixable: null, // or "code" or "whitespace"
+  },
+
+  create(context) {
+    const filePath = context.getFilename();
+
+    return {
+      // Handle classic components
+      'CallExpression > MemberExpression[property.name="extend"]'(node) {
+        const callExpression = node.parent;
+
+        if (!isEmberComponent(callExpression, filePath)) {
+          return;
+        }
+
+        // Handle `.extend` being called with no arguments
+        if (callExpression.arguments.length === 0) {
+          context.report(callExpression, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
+        }
+
+        for (const arg of callExpression.arguments) {
+          // Ignore anything other than an object literal, since Mixins can be in here too
+          if (!isObjectExpression(arg)) {
+            continue;
+          }
+
+          let tagNameNode;
+
+          if ((tagNameNode = getNonEmptyTagNameInObjectExpression(arg))) {
+            context.report(tagNameNode, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
+          } else if (hasNoTagNameInObjectExpression(arg)) {
+            context.report(callExpression, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
+          }
+        }
+      },
+
+      // Handle ES class components
+      'ClassDeclaration[superClass]'(node) {
+        if (getSourceModuleNameForIdentifier(context, node.superClass) === '@ember/component') {
+          let tagNameNode;
+          let decorator;
+
+          if ((tagNameNode = getNonEmptyTagNameInClassBody(node.body))) {
+            context.report(tagNameNode, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
+          } else if ((decorator = getDecoratorCallExpressionWithName(node, 'tagName'))) {
+            const tagNameArg = decorator.expression.arguments[0];
+
+            if (isStringLiteral(tagNameArg) && tagNameArg.value !== '') {
+              context.report(decorator, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
+            }
+          } else if (hasNoTagNameInClassBody(node.body)) {
+            context.report(node.body, ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS);
+          }
+        }
+      },
+    };
+  },
+};
diff --git a/lib/rules/use-ember-data-rfc-395-imports.js b/lib/rules/use-ember-data-rfc-395-imports.js
index 1d1296b4..be6e978f 100644
--- a/lib/rules/use-ember-data-rfc-395-imports.js
+++ b/lib/rules/use-ember-data-rfc-395-imports.js
@@ -1,13 +1,8 @@
 'use strict';
 
 const MAPPINGS = require('@ember-data/rfc395-data');
-const {
-  buildFix,
-  buildMessage,
-  getFullNames,
-  isDestructuring,
-  isIdentifierImportedFrom,
-} = require('../utils/new-module');
+const { buildFix, buildMessage, getFullNames, isDestructuring } = require('../utils/new-module');
+const { getSourceModuleNameForIdentifier } = require('../utils/utils');
 
 /**
  * This function returns an object like this:
@@ -116,7 +111,10 @@ module.exports = {
        */
       VariableDeclarator(node) {
         // Filter out non-ember-data variable declarations
-        if (!isDestructuring(node) || !isIdentifierImportedFrom(node, 'ember-data')) {
+        if (
+          !isDestructuring(node) ||
+          getSourceModuleNameForIdentifier(context, node.init) !== 'ember-data'
+        ) {
           return;
         }
 
diff --git a/lib/utils/new-module.js b/lib/utils/new-module.js
index bef47468..029d6abf 100644
--- a/lib/utils/new-module.js
+++ b/lib/utils/new-module.js
@@ -165,35 +165,9 @@ function isDestructuring(node) {
   );
 }
 
-function isIdentifierImportedFrom(node, module) {
-  const { name } = node.init;
-  const pp = node.parent.parent;
-
-  // `import` may only appear at the top level, ie. in a `Program` node
-  // also, a `Program` node will always have a `body` attribute
-  if (!pp || pp.type !== 'Program') {
-    return false;
-  }
-
-  return pp.body.some(n => {
-    function containsInitSpecifier(specifiers) {
-      return specifiers.some(s => {
-        return s.local.name === name;
-      });
-    }
-
-    return (
-      n.type === 'ImportDeclaration' &&
-      n.source.value === module &&
-      containsInitSpecifier(n.specifiers)
-    );
-  });
-}
-
 module.exports = {
   buildFix,
   buildMessage,
   getFullNames,
   isDestructuring,
-  isIdentifierImportedFrom,
 };
diff --git a/lib/utils/types.js b/lib/utils/types.js
index fb60a3ab..da4c3083 100644
--- a/lib/utils/types.js
+++ b/lib/utils/types.js
@@ -85,8 +85,8 @@ function isCallExpression(node) {
  * as first argument, eg.:
  * tSomeAction: mysteriousFnc(function(){})
  *
- * @param  {[type]}  node [description]
- * @return {Boolean}      [description]
+ * @param  {Object} node The node to check
+ * @return {boolean} Whether or not the node is a call with a function expression as the first argument
  */
 function isCallWithFunctionExpression(node) {
   const callObj = isMemberExpression(node.callee) ? node.callee.object : node;
diff --git a/lib/utils/utils.js b/lib/utils/utils.js
index 21faef49..b84032f9 100644
--- a/lib/utils/utils.js
+++ b/lib/utils/utils.js
@@ -14,10 +14,10 @@ module.exports = {
   collectObjectPatternBindings,
   findNodes,
   findUnorderedProperty,
-  getImportModule,
   getParent,
   getPropertyValue,
   getSize,
+  getSourceModuleNameForIdentifier,
   isEmptyMethod,
   isGlobalCallExpression,
   parseArgs,
@@ -208,7 +208,7 @@ function isEmptyMethod(node) {
  * @param {node} node the Idenfifier to find an import for
  * @returns {string | undefined} The name of the module the identifier was imported from, if it was imported
  */
-function getImportModule(context, node) {
+function getSourceModuleNameForIdentifier(context, node) {
   const [program] = context.getAncestors(node);
   const importDeclaration = program.body
     .filter(isImportDeclaration)


Test patch:
diff --git a/tests/lib/rules/require-tagless-components.js b/tests/lib/rules/require-tagless-components.js
new file mode 100644
index 00000000..d16f008b
--- /dev/null
+++ b/tests/lib/rules/require-tagless-components.js
@@ -0,0 +1,124 @@
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/require-tagless-components');
+const RuleTester = require('eslint').RuleTester;
+
+const { ERROR_MESSAGE_REQUIRE_TAGLESS_COMPONENTS: ERROR_MESSAGE } = rule;
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('babel-eslint'),
+  parserOptions: {
+    ecmaVersion: 6,
+    sourceType: 'module',
+    ecmaFeatures: {
+      legacyDecorators: true,
+    },
+  },
+});
+
+ruleTester.run('require-tagless-components', rule, {
+  valid: [
+    `
+      import Component from '@ember/component';
+      export default Component.extend({ tagName: '' });
+    `,
+    `
+      import Component from '@ember/component';
+      export default class MyComponent extends Component {
+        tagName = ''
+      }
+    `,
+    `
+      import Component from '@ember/component';
+      import { tagName } from '@ember-decorators/component';
+      @tagName('')
+      export default class MyComponent extends Component {}
+    `,
+    `
+      import Component from '@glimmer/component';
+      export default class MyComponent extends Component {}
+    `,
+    `
+      import SomeOtherThing from 'some-other-module';
+      export default SomeOtherThing.extend({
+        tagName: ''
+      });
+    `,
+    `
+      import SomeOtherThing from 'some-other-module';
+      export default class MyThing extends SomeOtherThing {
+        tagName = '';
+      }
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+        import Component from '@ember/component';
+        export default Component.extend({});
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default Component.extend();
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default Component.extend(SomeMixin, {});
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default Component.extend({
+          tagName: 'span'
+        });
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 4, type: 'Property' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default class MyComponent extends Component {}
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'ClassBody' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default class MyComponent extends Component {
+          tagName = 'span'
+        }
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 4, type: 'ClassProperty' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        import { tagName } from '@ember-decorators/component';
+        @tagName('span')
+        export default class MyComponent extends Component {}
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, line: 4, type: 'Decorator' }],
+    },
+  ],
+});
diff --git a/tests/lib/utils/utils/get-source-module-name-for-identifier-test.js b/tests/lib/utils/utils/get-source-module-name-for-identifier-test.js
new file mode 100644
index 00000000..3678556b
--- /dev/null
+++ b/tests/lib/utils/utils/get-source-module-name-for-identifier-test.js
@@ -0,0 +1,72 @@
+'use strict';
+
+const babelEslint = require('babel-eslint');
+const { getSourceModuleNameForIdentifier } = require('../../../../lib/utils/utils');
+
+/**
+ * Builds a fake ESLint context object that's enough to satisfy the contract
+ * expected by `getSourceModuleNameForIdentifier`
+ */
+class FauxContext {
+  constructor(code) {
+    const { ast } = babelEslint.parseForESLint(code);
+
+    this.ast = ast;
+  }
+
+  /**
+   * Does not build the full tree of "parents" for the identifier, but
+   * we only care about the first one; the Program node
+   */
+  getAncestors() {
+    return [this.ast];
+  }
+}
+
+test('when the identifier is not imported', () => {
+  const context = new FauxContext(`
+    Foo;
+  `);
+
+  const node = { name: 'Foo' };
+
+  expect(getSourceModuleNameForIdentifier(context, node)).toEqual(undefined);
+});
+
+describe('when the identifier is imported', () => {
+  test('as a default export', () => {
+    const context = new FauxContext(`
+      import Foo from 'bar';
+
+      Foo;
+    `);
+
+    const node = { name: 'Foo' };
+
+    expect(getSourceModuleNameForIdentifier(context, node)).toEqual('bar');
+  });
+
+  test('as a named export', () => {
+    const context = new FauxContext(`
+      import { Foo } from 'bar';
+
+      Foo;
+    `);
+
+    const node = { name: 'Foo' };
+
+    expect(getSourceModuleNameForIdentifier(context, node)).toEqual('bar');
+  });
+
+  test('when aliasing a named export', () => {
+    const context = new FauxContext(`
+      import { SomeOtherThing as Foo } from 'bar';
+
+      Foo;
+    `);
+
+    const node = { name: 'Foo' };
+
+    expect(getSourceModuleNameForIdentifier(context, node)).toEqual('bar');
+  });
+});