Repository: ember-cli/eslint-plugin-ember
Pull request #144 (2017-09-22 06:39:02)

Add destructuring support to `new-modules-import` rule
This PR updates the `new-modules-import` rule to account for destructuring assignments. Right now when enabling this rule, it will not report on Ember properties without the `Ember.`. Below is an example:

```javascript
import Ember from 'ember';

// This does not get reported to ESLint
const { Component } = Ember;

export default Component.extend({ ... });
```
However...
```javascript
import Ember from 'ember';

// This does get reported to ESLint
export default Ember.Component.extend({ ... });
```
I'd like to take advantage of this awesome rule and turn it on for the Ember applications I'm currently working on. However, all of my applications use destructuring assignments and the current rule's state does not send/report any ESLint errors like I'd like it to.

caseywatts: Thanks for working on this @clcuevas !
+1 do want :)

Patch:
diff --git a/lib/rules/avoid-leaking-state-in-ember-objects.js b/lib/rules/avoid-leaking-state-in-ember-objects.js
index 13c0be38..d1376eb0 100644
--- a/lib/rules/avoid-leaking-state-in-ember-objects.js
+++ b/lib/rules/avoid-leaking-state-in-ember-objects.js
@@ -21,9 +21,7 @@ const isAllowed = function (property) {
     utils.isLiteral(value) ||
     utils.isIdentifier(value) ||
     utils.isCallExpression(value) ||
-    utils.isBinaryExpression(value) ||
-    utils.isTemplateLiteral(value) ||
-    utils.isTaggedTemplateExpression(value);
+    utils.isBinaryExpression(value);
 };
 
 
diff --git a/lib/rules/new-module-imports.js b/lib/rules/new-module-imports.js
index d55f5240..6b46585c 100644
--- a/lib/rules/new-module-imports.js
+++ b/lib/rules/new-module-imports.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const EMBER_NAMESPACES = ['inject.controller', 'inject.service'];
 const GLOBALS = require('ember-rfc176-data/globals.json');
 
 //------------------------------------------------------------------------------
@@ -17,6 +18,33 @@ module.exports = {
 
   create(context) {
     return {
+      VariableDeclarator(node) {
+        // Filter out non-Ember variable declarations
+        if (!node.init || node.init.name !== 'Ember') return;
+
+        const properties = node.id.properties;
+        // Iterate through the destructured properties and report them
+        properties.forEach((item) => {
+          // Locate nested destructuring
+          if (item.value.properties) {
+            const parent = item.key.name;
+            const props = item.value.properties;
+            reportNestedProperties(props, parent);
+          } else {
+            const key = item.key.name;
+            const match = GLOBALS[key];
+            const message = populateMessage({
+              customKey: (key !== item.value.name) ? item.value.name : null,
+              key,
+              match,
+              type: item.type
+            });
+            // Report the issue to ESLint
+            context.report(item, message);
+          }
+        });
+      },
+
       'MemberExpression > Identifier[name=Ember]': function (node) {
         // filter out "foo.Ember"
         if (node.parent.object !== node) return;
@@ -40,27 +68,61 @@ module.exports = {
 
           const key = fullName.replace(/^Ember\./, '');
           const match = GLOBALS[key];
-          if (match) {
-            // build replacement import for the warning message
-            let importSpecifier;
-            if (match[1]) {
-              importSpecifier = match[2] ? `{ ${match[1]} as ${match[2]} }` : `{ ${match[1]} }`;
-            } else {
-              importSpecifier = match[2] || key;
-            }
-
-            const replacement = `import ${importSpecifier} from '${match[0]}';`;
-
-            const message = `Use  ${replacement}  instead of using  ${fullName}`;
 
+          if (match) {
+            const message = populateMessage({ fullName, key, match });
             // report the issue to ESLint
             context.report(node, message);
-
             // exit the loop after the first match was found
             break;
           }
         }
       }
     };
+
+    function reportNestedProperties(properties, parent) {
+      properties.forEach((item) => {
+        const match = GLOBALS[`${parent}.${item.key.name}`];
+        const message = populateMessage({
+          key: item.key.name,
+          match,
+          parent,
+          type: item.type
+        });
+
+        context.report(item, message);
+      });
+    }
+
+    function populateMessage(obj) {
+      // Both the Controller module and Service module each make available the
+      // "inject" namespace. In order to make it more readable, so it's more
+      // explicit at first glance from which module the "inject" namespace
+      // belongs to or is being imported from, let's check against this so we
+      // can properly populate the import specifier to report the ESLint error.
+      // Ex: import { inject as service } from '@ember/service';
+      const isNamespace = EMBER_NAMESPACES.indexOf(`${obj.parent}.${obj.key}`) !== -1;
+
+      let importSpecifier;
+      let message;
+
+      if (obj.match[1] && !isNamespace) {
+        importSpecifier = obj.match[2] ? `{ ${obj.match[1]} as ${obj.match[2]} }` : `{ ${obj.match[1]} }`;
+      } else if (obj.match[1] && isNamespace) {
+        importSpecifier = `{ ${obj.parent} as ${obj.key} }`;
+      } else {
+        importSpecifier = obj.match[2] || obj.customKey || obj.key;
+      }
+
+      const replacement = `import ${importSpecifier} from '${obj.match[0]}';`;
+
+      if (obj.type === 'Property') {
+        message = `Use ${replacement} instead of using Ember destructuring`;
+      } else {
+        message = `Use ${replacement} instead of using ${obj.fullName}`;
+      }
+
+      return message;
+    }
   }
 };
diff --git a/lib/utils/utils.js b/lib/utils/utils.js
index b4c8d21e..5789f998 100644
--- a/lib/utils/utils.js
+++ b/lib/utils/utils.js
@@ -17,7 +17,6 @@ module.exports = {
   isCallWithFunctionExpression,
   isThisExpression,
   isConditionalExpression,
-  isTemplateLiteral,
   isTaggedTemplateExpression,
   isGlobalCallExpression,
   getSize,
@@ -226,16 +225,6 @@ function isTaggedTemplateExpression(node) {
   return node !== undefined && node.type === 'TaggedTemplateExpression';
 }
 
-/**
- * Check whether or not a node is a TemplateLiteral.
- *
- * @param {Object} node The node to check.
- * @returns {boolean} Whether or not the node is a TemplateLiteral.
- */
-function isTemplateLiteral(node) {
-  return node !== undefined && node.type === 'TemplateLiteral';
-}
-
 /**
  * Check if given call is a global call
  *
diff --git a/package.json b/package.json
index a0825a3d..9415592c 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "eslint-plugin-ember",
-  "version": "4.6.1",
+  "version": "4.6.0",
   "description": "Eslint plugin for Ember.js apps",
   "main": "lib/index.js",
   "directories": {


Test patch:
diff --git a/tests/lib/rules/avoid-leaking-state-in-ember-objects.js b/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
index 7c90d8d1..a053a5dc 100644
--- a/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
+++ b/tests/lib/rules/avoid-leaking-state-in-ember-objects.js
@@ -89,12 +89,6 @@ eslintTester.run('avoid-leaking-state-in-ember-objects', rule, {
     {
       code: "export default Foo.extend({ fullName: 'a' + 'b' });"
     },
-    {
-      code: 'export default Foo.extend({ test: hbs`lorem ipsum` });'
-    },
-    {
-      code: 'export default Foo.extend({ test: `lorem ipsum` });'
-    }
   ],
   invalid: [
     {
diff --git a/tests/lib/rules/new-module-imports.js b/tests/lib/rules/new-module-imports.js
index 9c28c86d..a5138769 100644
--- a/tests/lib/rules/new-module-imports.js
+++ b/tests/lib/rules/new-module-imports.js
@@ -16,41 +16,145 @@ eslintTester.run('new-module-imports', rule, {
     { code: 'Ember.MODEL_FACTORY_INJECTIONS = true;' },
     { code: 'console.log(Ember.VERSION);' },
     { code: 'if (Ember.testing) {}' },
+    {
+      code: `import Component from '@ember/component';
+
+        export default Component.extend({});
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    },
+    {
+      code: `
+        import Controller from '@ember/controller';
+        import { bool } from '@ember/object/computed';
+
+        export default Controller.extend({
+          isTrue: bool('')
+        });
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+    }
   ],
   invalid: [
+    {
+      code: `import Ember from 'ember';
+
+        const { Object: EmberObject } = Ember;
+
+        export default Component.extend({});
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [
+        { message: 'Use import EmberObject from \'@ember/object\'; instead of using Ember destructuring', line: 3 }
+      ]
+    },
+    {
+      code: `import Ember from 'ember';
+
+        const { $, Controller } = Ember;
+
+        export default Controller.extend({});
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [
+        { message: 'Use import $ from \'jquery\'; instead of using Ember destructuring', line: 3 },
+        { message: 'Use import Controller from \'@ember/controller\'; instead of using Ember destructuring', line: 3 }
+      ]
+    },
+    {
+      code: `import Ember from 'ember';
+
+        const { Component, String: { htmlSafe } } = Ember;
+        const TEST = 'MY TEST';
+
+        export default Component.extend({});
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [
+        { message: 'Use import Component from \'@ember/component\'; instead of using Ember destructuring', line: 3 },
+        { message: 'Use import { htmlSafe } from \'@ember/string\'; instead of using Ember destructuring', line: 3 }
+      ]
+    },
+    {
+      code: `import Ember from 'ember';
+
+        const { inject: { controller, service } } = Ember;
+
+        export default Ember.Component.extend({
+          myService: service('my-service')
+        });
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [
+        { message: 'Use import { inject as controller } from \'@ember/controller\'; instead of using Ember destructuring', line: 3 },
+        { message: 'Use import { inject as service } from \'@ember/service\'; instead of using Ember destructuring', line: 3 },
+        { message: 'Use import Component from \'@ember/component\'; instead of using Ember.Component', line: 5 }
+      ]
+    },
+    {
+      code: `
+        import Ember from 'ember';
+        import Component from '@ember/component';
+
+        const { computed: { alias, uniq } } = Ember;
+
+        export default Component.extend({});
+      `,
+      parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+      errors: [
+        { message: 'Use import { alias } from \'@ember/object/computed\'; instead of using Ember destructuring', line: 5 },
+        { message: 'Use import { uniq } from \'@ember/object/computed\'; instead of using Ember destructuring', line: 5 }
+      ]
+    },
     {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       code: 'export default Ember.Service;',
-      errors: [{ message: 'Use  import Service from \'@ember/service\';  instead of using  Ember.Service' }],
+      errors: [
+        { message: 'Use import Service from \'@ember/service\'; instead of using Ember.Service', line: 1 }
+      ],
     },
     {
       parserOptions: { ecmaVersion: 6, sourceType: 'module' },
       code: 'export default Ember.Service.extend({});',
-      errors: [{ message: 'Use  import Service from \'@ember/service\';  instead of using  Ember.Service' }],
+      errors: [
+        { message: 'Use import Service from \'@ember/service\'; instead of using Ember.Service', line: 1 }
+      ],
     },
     {
       code: 'Ember.computed();',
-      errors: [{ message: 'Use  import { computed } from \'@ember/object\';  instead of using  Ember.computed' }],
+      errors: [
+        { message: 'Use import { computed } from \'@ember/object\'; instead of using Ember.computed', line: 1 }
+      ],
     },
     {
       code: 'Ember.computed.not();',
-      errors: [{ message: 'Use  import { not } from \'@ember/object/computed\';  instead of using  Ember.computed.not' }],
+      errors: [
+        { message: 'Use import { not } from \'@ember/object/computed\'; instead of using Ember.computed.not', line: 1 }
+      ],
     },
     {
       code: 'Ember.inject.service(\'foo\');',
-      errors: [{ message: 'Use  import { inject } from \'@ember/service\';  instead of using  Ember.inject.service' }],
+      errors: [
+        { message: 'Use import { inject } from \'@ember/service\'; instead of using Ember.inject.service', line: 1 }
+      ],
     },
     {
       code: 'var Router = Ember.Router.extend({});',
-      errors: [{ message: 'Use  import EmberRouter from \'@ember/routing/router\';  instead of using  Ember.Router' }],
+      errors: [
+        { message: 'Use import EmberRouter from \'@ember/routing/router\'; instead of using Ember.Router', line: 1 }
+      ],
     },
     {
       code: 'Ember.$(\'.foo\')',
-      errors: [{ message: 'Use  import $ from \'jquery\';  instead of using  Ember.$' }],
+      errors: [
+        { message: 'Use import $ from \'jquery\'; instead of using Ember.$', line: 1 }
+      ],
     },
     {
       code: 'new Ember.RSVP.Promise();',
-      errors: [{ message: 'Use  import { Promise } from \'rsvp\';  instead of using  Ember.RSVP.Promise' }],
+      errors: [
+        { message: 'Use import { Promise } from \'rsvp\'; instead of using Ember.RSVP.Promise', line: 1 }
+      ],
     },
   ],
 });