Merge pull request #691 from nlfurniss/no-mixins-rule

Add new rule `no-mixins`

Author: bmish

README.md

@@ -73,6 +73,7 @@ Each rule has emojis denoting what configuration it belongs to and/or a :wrench:
 | :car::wrench: | [no-get](./docs/rules/no-get.md) | require using ES5 getters instead of Ember's `get` / `getProperties` functions |
 | :white_check_mark: | [no-global-jquery](./docs/rules/no-global-jquery.md) | disallow usage of global jQuery object |
 | :car: | [no-jquery](./docs/rules/no-jquery.md) | disallow any usage of jQuery |
+|  | [no-mixins](./docs/rules/no-mixins.md) | disallow the usage of mixins |
 | :white_check_mark: | [no-new-mixins](./docs/rules/no-new-mixins.md) | disallow the creation of new mixins |
 | :white_check_mark: | [no-observers](./docs/rules/no-observers.md) | disallow usage of observers |
 | :white_check_mark::wrench: | [no-old-shims](./docs/rules/no-old-shims.md) | disallow usage of old shims for modules |

docs/rules/no-mixins.md

@@ -0,0 +1,67 @@
+# no-mixins
+
+Using mixins to share code appears easy at first. But they add significant complexity as a project grows. Furthermore, the [Octane programming model](https://guides.emberjs.com/release/upgrading/current-edition/) eliminates the need to use them in favor of native class semantics and other primitives.
+
+For practical strategies on removing mixins see [this discourse thread](https://discuss.emberjs.com/t/best-way-to-replace-mixins/17395/2).
+
+## Rule Details
+
+This rule disallows importing a mixin. This is different than [no-new-mixins](no-new-mixins.md), which disallows creating a mixin (e.g. `Mixin.create({})`). Both should be used in an app that wants to disallow the use of mixins.
+
+## Examples
+
+Examples of **incorrect** code for this rule:
+
+```javascript
+// my-octane-component.js
+import Component from '@ember/component';
+import FooMixin from '../utils/mixins/foo';
+
+export default class FooComponent extends Component.extend(FooMixin) {
+  // ...
+}
+```
+
+```javascript
+// my-component.js
+import myMixin from 'my-mixin';
+
+export default Component.extend(myMixin, {
+  aComputedProperty: computed('obj', function() {
+    return this.isValidClassName(obj.className);
+  })
+});
+```
+
+Examples of **correct** code for this rule:
+
+```javascript
+// my-utils.js
+export function isValidClassName(classname) {
+  return Boolean(className.match('-class'));
+}
+
+export function hideModal(obj, value) {
+  set(obj, 'isHidden', value);
+}
+```
+
+```javascript
+// my-component.js
+import { isValidClassName } from 'my-utils';
+
+export default Component.extend({
+  aComputedProperty: computed('obj', function() {
+    return isValidClassName(get(obj, 'className'));
+  })
+});
+```
+
+## References
+
+* [Mixins Considered Harmful](https://reactjs.org/blog/2016/07/13/mixins-considered-harmful.html)
+* [Why Are Mixins Considered Harmful?](http://raganwald.com/2016/07/16/why-are-mixins-considered-harmful.html)
+
+## Related Rules
+
+* [no-new-mixins](no-new-mixins.md)

docs/rules/no-new-mixins.md

@@ -60,3 +60,7 @@ export default Component.extend({
   })
 });
 ```
+
+## Related Rules
+
+* [no-mixins](no-mixins.md)

lib/index.js

@@ -35,6 +35,7 @@ module.exports = {
     'no-invalid-debug-function-arguments': require('./rules/no-invalid-debug-function-arguments'),
     'no-jquery': require('./rules/no-jquery'),
     'no-legacy-test-waiters': require('./rules/no-legacy-test-waiters'),
+    'no-mixins': require('./rules/no-mixins'),
     'no-new-mixins': require('./rules/no-new-mixins'),
     'no-observers': require('./rules/no-observers'),
     'no-old-shims': require('./rules/no-old-shims'),

lib/recommended-rules.js

@@ -38,6 +38,7 @@ module.exports = {
   "ember/no-invalid-debug-function-arguments": "error",
   "ember/no-jquery": "off",
   "ember/no-legacy-test-waiters": "off",
+  "ember/no-mixins": "off",
   "ember/no-new-mixins": "error",
   "ember/no-observers": "error",
   "ember/no-old-shims": "error",

lib/rules/no-mixins.js

@@ -0,0 +1,35 @@
+'use strict';
+
+//------------------------------------------------------------------------------
+// General rule - Don't use mixins
+//------------------------------------------------------------------------------
+
+const ERROR_MESSAGE = "Don't use a mixin";
+const mixinPathRegex = /\/mixins\//;
+
+module.exports = {
+  meta: {
+    type: 'suggestion',
+    docs: {
+      description: 'disallow the usage of mixins',
+      category: 'Best Practices',
+      recommended: false,
+      url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/no-mixins.md',
+    },
+    fixable: null,
+    schema: [],
+  },
+
+  ERROR_MESSAGE,
+
+  create(context) {
+    return {
+      ImportDeclaration(node) {
+        const importPath = node.source.value;
+        if (importPath.match(mixinPathRegex)) {
+          context.report(node, ERROR_MESSAGE);
+        }
+      },
+    };
+  },
+};

tests/lib/rules/no-mixins.js

@@ -0,0 +1,70 @@
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-mixins');
+const RuleTester = require('eslint').RuleTester;
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+const { ERROR_MESSAGE } = rule;
+const eslintTester = new RuleTester({
+  parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+});
+eslintTester.run('no-mixins', rule, {
+  valid: [
+    `
+    import NoMixinRule from "some/addon/mixin";
+    export default mixin;
+    `,
+    `
+    import NameIsMixin from "some/addon";
+    export default Component.extend({});
+  `,
+    `
+    import * as Mixins from "some/addon/mixins";
+    export default Component.extend({});
+  `,
+    `
+    import Mixin from '@ember/object/mixin';
+    const newMixin = Mixin.create({});
+    `,
+  ],
+  invalid: [
+    {
+      code: `
+        import BadCode from "my-addon/mixins/bad-code";
+
+        export default Component.extend(BadCode, {});
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }],
+    },
+    {
+      code: `
+      import EmberObject from '@ember/object';
+      import EditableMixin from '../mixins/editable';
+
+      const Comment = EmberObject.extend(EditableMixin, {
+        post: null
+      });
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }],
+    },
+    {
+      code: `
+      import Component from '@ember/component';
+      import FooMixin from '../utils/mixins/foo';
+
+      export default class FooComponent extends Component.extend(FooMixin) {
+        // ...
+      }
+      `,
+      output: null,
+      errors: [{ message: ERROR_MESSAGE, type: 'ImportDeclaration' }],
+    },
+  ],
+});