Repository: import-js/eslint-plugin-import
Pull request #1222 (2018-10-25 08:50:41)

[New] `named`: add commonjs option
Related: #1145

Adds an option `commonjs` to `named` rule, which will check a destructured `require` statement.

Invalid:

```javascript
// ./foo.js
export const foo = 'bar';

// ./bar.js
const {baz} = require('./foo');
```

valid:

```javascript
// ./bar.js
export const bar = 'foo';
export const foo = 'baz';

// ./baz.js
const {bar} = require('./bar');


// ./foo2.js
const {bar, foo} = require('./bar');

// ./foo3.js
let {bar} = require('./bar'), {foo} = require('./bar');
```


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 41799fb6..493c7094 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ### Added
 - [`no-dynamic-require`]: add option `esmodule` ([#1223], thanks [@vikr01])
+- [`named`]: add `commonjs` option ([#1222], thanks [@vikr01])
 
 ### Fixed
 - [`no-duplicates`]: ensure autofix avoids excessive newlines ([#2028], thanks [@ertrzyiks])
@@ -891,7 +892,6 @@ for info on changes for earlier releases.
 [#1619]: https://github.com/benmosher/eslint-plugin-import/pull/1619
 [#1612]: https://github.com/benmosher/eslint-plugin-import/pull/1612
 [#1611]: https://github.com/benmosher/eslint-plugin-import/pull/1611
-[#1223]: https://github.com/benmosher/eslint-plugin-import/pull/1223
 [#1605]: https://github.com/benmosher/eslint-plugin-import/pull/1605
 [#1586]: https://github.com/benmosher/eslint-plugin-import/pull/1586
 [#1572]: https://github.com/benmosher/eslint-plugin-import/pull/1572
@@ -963,6 +963,8 @@ for info on changes for earlier releases.
 [#1235]: https://github.com/benmosher/eslint-plugin-import/pull/1235
 [#1234]: https://github.com/benmosher/eslint-plugin-import/pull/1234
 [#1232]: https://github.com/benmosher/eslint-plugin-import/pull/1232
+[#1223]: https://github.com/benmosher/eslint-plugin-import/pull/1223
+[#1222]: https://github.com/benmosher/eslint-plugin-import/pull/1222
 [#1218]: https://github.com/benmosher/eslint-plugin-import/pull/1218
 [#1176]: https://github.com/benmosher/eslint-plugin-import/pull/1176
 [#1163]: https://github.com/benmosher/eslint-plugin-import/pull/1163
diff --git a/src/rules/named.js b/src/rules/named.js
index 7dee06ce..2770d51f 100644
--- a/src/rules/named.js
+++ b/src/rules/named.js
@@ -8,24 +8,41 @@ module.exports = {
     docs: {
       url: docsUrl('named'),
     },
-    schema: [],
+    schema: [
+      {
+        type: 'object',
+        properties: {
+          commonjs: {
+            type: 'boolean',
+          },
+        },
+        additionalProperties: false,
+      },
+    ],
   },
 
   create: function (context) {
+    const options = context.options[0] || {};
+
     function checkSpecifiers(key, type, node) {
       // ignore local exports and type imports/exports
-      if (node.source == null || node.importKind === 'type' ||
-          node.importKind === 'typeof'  || node.exportKind === 'type') {
+      if (
+        node.source == null
+        || node.importKind === 'type'
+        || node.importKind === 'typeof'
+        || node.exportKind === 'type'
+      ) {
         return;
       }
 
-      if (!node.specifiers
-        .some(function (im) { return im.type === type; })) {
+      if (!node.specifiers.some((im) => im.type === type)) {
         return; // no named imports/exports
       }
 
       const imports = Exports.get(node.source.value, context);
-      if (imports == null) return;
+      if (imports == null) {
+        return;
+      }
 
       if (imports.errors.length) {
         imports.reportErrors(context, node);
@@ -33,10 +50,13 @@ module.exports = {
       }
 
       node.specifiers.forEach(function (im) {
-        if (im.type !== type) return;
-
-        // ignore type imports
-        if (im.importKind === 'type' || im.importKind === 'typeof') return;
+        if (
+          im.type !== type
+          // ignore type imports
+          || im.importKind === 'type' || im.importKind === 'typeof'
+        ) {
+          return;
+        }
 
         const deepLookup = imports.hasDeep(im[key].name);
 
@@ -46,27 +66,73 @@ module.exports = {
               .map(i => path.relative(path.dirname(context.getPhysicalFilename ? context.getPhysicalFilename() : context.getFilename()), i.path))
               .join(' -> ');
 
-            context.report(im[key],
-              `${im[key].name} not found via ${deepPath}`);
+            context.report(im[key], `${im[key].name} not found via ${deepPath}`);
+          } else {
+            context.report(im[key], im[key].name + ' not found in \'' + node.source.value + '\'');
+          }
+        }
+      });
+    }
+
+    function checkRequire(node) {
+      if (
+        !options.commonjs
+        || node.type !== 'VariableDeclarator'
+        // return if it's not an object destructure or it's an empty object destructure
+        || !node.id || node.id.type !== 'ObjectPattern' || node.id.properties.length === 0
+        // return if there is no call expression on the right side
+        || !node.init || node.init.type !== 'CallExpression'
+      ) {
+        return;
+      }
+
+      const call = node.init;
+      const [source] = call.arguments;
+      const variableImports = node.id.properties;
+      const variableExports = Exports.get(source.value, context);
+
+      if (
+        // return if it's not a commonjs require statement
+        call.callee.type !== 'Identifier' || call.callee.name !== 'require' || call.arguments.length !== 1
+        // return if it's not a string source
+        || source.type !== 'Literal'
+        || variableExports == null
+      ) {
+        return;
+      }
+
+      if (variableExports.errors.length) {
+        variableExports.reportErrors(context, node);
+        return;
+      }
+
+      variableImports.forEach(function (im) {
+        if (im.type !== 'Property' || !im.key || im.key.type !== 'Identifier') {
+          return;
+        }
+
+        const deepLookup = variableExports.hasDeep(im.key.name);
+
+        if (!deepLookup.found) {
+          if (deepLookup.path.length > 1) {
+            const deepPath = deepLookup.path
+              .map(i => path.relative(path.dirname(context.getFilename()), i.path))
+              .join(' -> ');
+
+            context.report(im.key, `${im.key.name} not found via ${deepPath}`);
           } else {
-            context.report(im[key],
-              im[key].name + ' not found in \'' + node.source.value + '\'');
+            context.report(im.key, im.key.name + ' not found in \'' + source.value + '\'');
           }
         }
       });
     }
 
     return {
-      'ImportDeclaration': checkSpecifiers.bind( null
-        , 'imported'
-        , 'ImportSpecifier'
-      ),
-
-      'ExportNamedDeclaration': checkSpecifiers.bind( null
-        , 'local'
-        , 'ExportSpecifier'
-      ),
-    };
+      ImportDeclaration: checkSpecifiers.bind(null, 'imported', 'ImportSpecifier'),
 
+      ExportNamedDeclaration: checkSpecifiers.bind(null, 'local', 'ExportSpecifier'),
+
+      VariableDeclarator: checkRequire,
+    };
   },
 };


Test patch:
diff --git a/tests/src/rules/named.js b/tests/src/rules/named.js
index f09ee205..57e40c91 100644
--- a/tests/src/rules/named.js
+++ b/tests/src/rules/named.js
@@ -146,6 +146,41 @@ ruleTester.run('named', rule, {
       code: 'import { common } from "./re-export-default"',
     }),
 
+    // destructured requires with commonjs option
+    test({
+      code: 'const { destructuredProp } = require("./named-exports")',
+      options: [{ commonjs: true }],
+    }),
+    test({
+      code: 'let { arrayKeyProp } = require("./named-exports")',
+      options: [{ commonjs: true }],
+    }),
+    test({
+      code: 'const { deepProp } = require("./named-exports")',
+      options: [{ commonjs: true }],
+    }),
+
+    test({
+      code: 'const { foo, bar } = require("./re-export-names")',
+      options: [{ commonjs: true }],
+    }),
+
+    test({
+      code: 'const { baz } = require("./bar")',
+      errors: [error('baz', './bar')],
+    }),
+
+    test({
+      code: 'const { baz } = require("./bar")',
+      errors: [error('baz', './bar')],
+      options: [{ commonjs: false }],
+    }),
+
+    test({
+      code: 'const { default: defExport } = require("./bar")',
+      options: [{ commonjs: true }],
+    }),
+
     ...SYNTAX_CASES,
   ],
 
@@ -201,6 +236,30 @@ ruleTester.run('named', rule, {
       errors: ['baz not found via broken-trampoline.js -> named-exports.js'],
     }),
 
+    test({
+      code: 'const { baz } = require("./bar")',
+      errors: [error('baz', './bar')],
+      options: [{ commonjs: true }],
+    }),
+
+    test({
+      code: 'let { baz } = require("./bar")',
+      errors: [error('baz', './bar')],
+      options: [{ commonjs: true }],
+    }),
+
+    test({
+      code: 'const { baz: bar, bop } = require("./bar"), { a } = require("./re-export-names")',
+      errors: [error('baz', './bar'), error('bop', './bar'), error('a', './re-export-names')],
+      options: [{ commonjs: true }],
+    }),
+
+    test({
+      code: 'const { default: defExport } = require("./named-exports")',
+      errors: [error('default', './named-exports')],
+      options: [{ commonjs: true }],
+    }),
+
     // parse errors
     // test({
     //   code: "import { a } from './test.coffee';",