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

Issue:
New Octane Rule: `no-classic-classes`
Pulled from https://github.com/ember-cli/eslint-plugin-ember/issues/533 which proposes a new set of rules for Octane related best practices

Disallow "classic" classes preferring use of native JS classes

```js
// ### Good
export default class MyComponent extends Component {
  ...
}

// This mixin usage should be okay
export default class MyService extends Service.extends(Evented) {
  ...
}

// ### Bad
export default Component.extend({
  ...
});
```

If anyone would like to implement this please comment that you'll take it and work away.

Pull request:
Add new `no-classic-classes` rule
Closes #539

Patch:
diff --git a/README.md b/README.md
index dea63870..f605776b 100644
--- a/README.md
+++ b/README.md
@@ -142,6 +142,7 @@ The `--fix` option on the command line automatically fixes problems reported by
 
 |    | Rule ID | Description |
 |:---|:--------|:------------|
+|  | [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 |
 
diff --git a/docs/rules/no-classic-classes.md b/docs/rules/no-classic-classes.md
new file mode 100644
index 00000000..2d77848b
--- /dev/null
+++ b/docs/rules/no-classic-classes.md
@@ -0,0 +1,44 @@
+# Disallow "classic" classes in favor of native JS classes (no-classic-classes)
+
+Ember now allows you to use native JS classes to extend the built-in classes provided by Ember. This pattern is preferred in favor of using the "classic" style of classes that Ember has provided since before JS classes were available to use.
+
+## Rule Details
+
+This rule aims to ensure that you do not use a "classic" Ember class where a native class could be used instead. The one instance where `.extend` should still be used is for including a Mixin into your class, which does not have a native JS class alternative available.
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+// Extending an Ember class using the "classic" class pattern is not OK
+import Component from '@ember/component';
+
+export default Component.extend({});
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+// Extending using a native JS class is OK
+import Component from '@ember/component';
+
+export default class MyComponent extends Component {};
+```
+
+```javascript
+// Including a Mixin is OK
+import Component from '@ember/component';
+import Evented from '@ember/object/evented';
+
+export default class MyComponent extends Component.extend(Evented) {};
+```
+
+## When Not To Use It
+
+* If you are not ready to transition completely to native JS classes, you should not enable this rule
+
+## Further Reading
+
+* [Ember Octane Release Plan](https://blog.emberjs.com/2019/08/15/octane-release-plan.html)
+  * Includes advice on transition Components to use native, rather than classic, classes
+* [Ember.js Native Class Update - 2019 Edition](https://blog.emberjs.com/2019/01/26/emberjs-native-class-update-2019-edition.html)
+  * **Note:** some of the recommendations made in this blog post are longer relevant, such as using `.extend` when defining a computed property
diff --git a/lib/index.js b/lib/index.js
index c2047aca..11466152 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -16,6 +16,7 @@ module.exports = {
     'no-attrs-in-components': require('./rules/no-attrs-in-components'),
     'no-attrs-snapshot': require('./rules/no-attrs-snapshot'),
     'no-capital-letters-in-routes': require('./rules/no-capital-letters-in-routes'),
+    'no-classic-classes': require('./rules/no-classic-classes'),
     'no-classic-components': require('./rules/no-classic-components'),
     'no-computed-properties-in-native-classes': require('./rules/no-computed-properties-in-native-classes'),
     'no-deeply-nested-dependent-keys-with-each': require('./rules/no-deeply-nested-dependent-keys-with-each'),
diff --git a/lib/recommended-rules.js b/lib/recommended-rules.js
index 05868ea6..6bfb76f0 100644
--- a/lib/recommended-rules.js
+++ b/lib/recommended-rules.js
@@ -19,6 +19,7 @@ module.exports = {
   "ember/no-attrs-in-components": "error",
   "ember/no-attrs-snapshot": "error",
   "ember/no-capital-letters-in-routes": "error",
+  "ember/no-classic-classes": "off",
   "ember/no-classic-components": "off",
   "ember/no-computed-properties-in-native-classes": "off",
   "ember/no-deeply-nested-dependent-keys-with-each": "error",
diff --git a/lib/rules/no-classic-classes.js b/lib/rules/no-classic-classes.js
new file mode 100644
index 00000000..32813c2d
--- /dev/null
+++ b/lib/rules/no-classic-classes.js
@@ -0,0 +1,60 @@
+'use strict';
+
+const { getImportModule } = require('../utils/utils');
+const { isObjectExpression } = require('../utils/types');
+
+const ERROR_MESSAGE_NO_CLASSIC_CLASSES =
+  'Native JS classes should be used instead of classic classes';
+
+function hasNoArguments(node) {
+  return node.arguments.length === 0;
+}
+
+function hasObjectArgument(node) {
+  return node.arguments.some(isObjectExpression);
+}
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+  ERROR_MESSAGE_NO_CLASSIC_CLASSES,
+
+  meta: {
+    docs: {
+      description: 'Disallow "classic" classes in favor of native JS classes',
+      category: 'Ember Octane',
+      recommended: false,
+    },
+    fixable: null, // or "code" or "whitespace"
+  },
+
+  create(context) {
+    function getModuleForNode(node) {
+      return getImportModule(context, node);
+    }
+
+    function reportNode(node) {
+      context.report(node, ERROR_MESSAGE_NO_CLASSIC_CLASSES);
+    }
+
+    return {
+      'CallExpression > MemberExpression[property.name="extend"]'(node) {
+        const callExpression = node.parent;
+
+        // Invalid if there are no arguments because that is equivalent to nnot called `extend` at all
+        // 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);
+
+          // Only warn about classes imported from the `@ember/` namespace
+          if (classImportedFrom && classImportedFrom.startsWith('@ember/')) {
+            reportNode(callExpression);
+          }
+        }
+      },
+    };
+  },
+};
diff --git a/lib/utils/types.js b/lib/utils/types.js
index a0a9a9a1..fb60a3ab 100644
--- a/lib/utils/types.js
+++ b/lib/utils/types.js
@@ -12,6 +12,7 @@ module.exports = {
   isExpressionStatement,
   isFunctionExpression,
   isIdentifier,
+  isImportDeclaration,
   isLiteral,
   isLogicalExpression,
   isMemberExpression,
@@ -147,6 +148,16 @@ function isIdentifier(node) {
   return node !== undefined && node.type === 'Identifier';
 }
 
+/**
+ * Check whether or not a node is an ImportDeclaration.
+ *
+ * @param {Object} node The node to check.
+ * @returns {boolean} Whether or not the node is an ImportDeclaration.
+ */
+function isImportDeclaration(node) {
+  return node !== undefined && node.type === 'ImportDeclaration';
+}
+
 /**
  * Check whether or not a node is an Literal.
  *
diff --git a/lib/utils/utils.js b/lib/utils/utils.js
index f6e72459..21faef49 100644
--- a/lib/utils/utils.js
+++ b/lib/utils/utils.js
@@ -3,6 +3,7 @@
 const {
   isCallExpression,
   isIdentifier,
+  isImportDeclaration,
   isLiteral,
   isMemberExpression,
   isNewExpression,
@@ -13,6 +14,7 @@ module.exports = {
   collectObjectPatternBindings,
   findNodes,
   findUnorderedProperty,
+  getImportModule,
   getParent,
   getPropertyValue,
   getSize,
@@ -198,6 +200,25 @@ function isEmptyMethod(node) {
   return node.value.body && node.value.body.body && node.value.body.body.length <= 0;
 }
 
+/**
+ * Gets the name of the module that an identifier was imported from,
+ * if it was imported
+ *
+ * @param {Object} context the context of the ESLint rule
+ * @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) {
+  const [program] = context.getAncestors(node);
+  const importDeclaration = program.body
+    .filter(isImportDeclaration)
+    .find(importDeclaration =>
+      importDeclaration.specifiers.some(specifier => specifier.local.name === node.name)
+    );
+
+  return importDeclaration ? importDeclaration.source.value : undefined;
+}
+
 /**
  * Travels up the ancestors of a given node, if the predicate function returns
  * truthy for a given node or ancestor, return that node, otherwise return null


Test patch:
diff --git a/tests/lib/rules/no-classic-classes.js b/tests/lib/rules/no-classic-classes.js
new file mode 100644
index 00000000..8367ad15
--- /dev/null
+++ b/tests/lib/rules/no-classic-classes.js
@@ -0,0 +1,85 @@
+'use strict';
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-classic-classes');
+const RuleTester = require('eslint').RuleTester;
+
+const { ERROR_MESSAGE_NO_CLASSIC_CLASSES: ERROR_MESSAGE } = rule;
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({
+  parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+});
+
+ruleTester.run('no-classic-classes', rule, {
+  valid: [
+    `
+      import Component from '@ember/component';
+
+      export default class MyComponent extends Component {}
+    `,
+    `
+      import Component from '@ember/component';
+
+      export default class MyRoute extends Route.extend(SomeMixin) {}
+    `,
+    `
+      import SomeOtherThing from 'some-other-library';
+
+      export default SomeOtherThing.extend({});
+    `,
+  ],
+
+  invalid: [
+    {
+      code: `
+        import Component from '@ember/component';
+        export default Component.extend();
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default Component.extend({});
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        import Evented from '@ember/object/Evented';
+        export default Component.extend(Evented, {});
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 4, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default class MyComponent extends Component.extend() {};
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        export default class MyComponent extends Component.extend({}) {};
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 3, type: 'CallExpression' }],
+    },
+    {
+      code: `
+        import Component from '@ember/component';
+        import Evented from '@ember/object/evented';
+        export default class MyComponent extends Component.extend(Evented, {}) {};
+      `,
+      errors: [{ message: ERROR_MESSAGE, line: 4, type: 'CallExpression' }],
+    },
+  ],
+});