[New] Add no-import-module-exports rule

CHANGELOG.md

@@ -10,6 +10,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 - [`no-commonjs`]: Also detect require calls with expressionless template literals: ``` require(`x`) ``` ([#1958], thanks [@FloEdelmann])
 - [`no-internal-modules`]: Add `forbid` option ([#1846], thanks [@guillaumewuip])
 - add [`no-relative-packages`] ([#1860], [#966], thanks [@tapayne88] [@panrafal])
+- add [`no-import-module-exports`] rule: report import declarations with CommonJS exports ([#804], thanks [@kentcdodds] and [@ttmarek])
 
 ### Fixed
 - [`export`]/TypeScript: properly detect export specifiers as children of a TS module block ([#1889], thanks [@andreubotella])
@@ -732,6 +733,7 @@ for info on changes for earlier releases.
 [`no-duplicates`]: ./docs/rules/no-duplicates.md
 [`no-dynamic-require`]: ./docs/rules/no-dynamic-require.md
 [`no-extraneous-dependencies`]: ./docs/rules/no-extraneous-dependencies.md
+[`no-import-module-exports`]: ./docs/rules/no-import-module-exports.md
 [`no-internal-modules`]: ./docs/rules/no-internal-modules.md
 [`no-mutable-exports`]: ./docs/rules/no-mutable-exports.md
 [`no-named-as-default-member`]: ./docs/rules/no-named-as-default-member.md
@@ -908,6 +910,7 @@ for info on changes for earlier releases.
 [#871]: https://github.com/benmosher/eslint-plugin-import/pull/871
 [#858]: https://github.com/benmosher/eslint-plugin-import/pull/858
 [#843]: https://github.com/benmosher/eslint-plugin-import/pull/843
+[#804]: https://github.com/benmosher/eslint-plugin-import/pull/804
 [#797]: https://github.com/benmosher/eslint-plugin-import/pull/797
 [#794]: https://github.com/benmosher/eslint-plugin-import/pull/794
 [#744]: https://github.com/benmosher/eslint-plugin-import/pull/744
@@ -1330,4 +1333,5 @@ for info on changes for earlier releases.
 [@bicstone]: https://github.com/bicstone
 [@guillaumewuip]: https://github.com/guillaumewuip
 [@tapayne88]: https://github.com/tapayne88
-[@panrafal]: https://github.com/panrafal
+[@panrafal]: https://github.com/panrafal
+[@ttmarek]: https://github.com/ttmarek

README.md

@@ -67,11 +67,13 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 * Report CommonJS `require` calls and `module.exports` or `exports.*`. ([`no-commonjs`])
 * Report AMD `require` and `define` calls. ([`no-amd`])
 * No Node.js builtin modules. ([`no-nodejs-modules`])
+* Forbid imports with CommonJS exports ([`no-import-module-exports`])
 
 [`unambiguous`]: ./docs/rules/unambiguous.md
 [`no-commonjs`]: ./docs/rules/no-commonjs.md
 [`no-amd`]: ./docs/rules/no-amd.md
 [`no-nodejs-modules`]: ./docs/rules/no-nodejs-modules.md
+[`no-import-module-exports`]: ./docs/rules/no-import-module-exports.md
 
 
 ### Style guide

docs/rules/no-import-module-exports.md

@@ -0,0 +1,74 @@
+# no-import-module-exports
+
+Reports the use of import declarations with CommonJS exports in any module
+except for the [main module](https://docs.npmjs.com/files/package.json#main).
+
+If you have multiple entry points or are using `js:next` this rule includes an
+`exceptions` option which you can use to exclude those files from the rule.
+
+## Options
+
+#### `exceptions`
+ - An array of globs. The rule will be omitted from any file that matches a glob
+   in the options array. For example, the following setting will omit the rule
+   in the `some-file.js` file.
+
+```json
+"import/no-import-module-exports": ["error", {
+    "exceptions": ["**/*/some-file.js"]
+}]
+```
+
+## Rule Details
+
+### Fail
+
+```js
+import { stuff } from 'starwars'
+module.exports = thing
+
+import * as allThings from 'starwars'
+exports.bar = thing
+
+import thing from 'other-thing'
+exports.foo = bar
+
+import thing from 'starwars'
+const baz = module.exports = thing
+console.log(baz)
+```
+
+### Pass
+Given the following package.json:
+
+```json
+{
+  "main": "lib/index.js",
+}
+```
+
+```js
+import thing from 'other-thing'
+export default thing
+
+const thing = require('thing')
+module.exports = thing
+
+const thing = require('thing')
+exports.foo = bar
+
+import thing from 'otherthing'
+console.log(thing.module.exports)
+
+// in lib/index.js
+import foo from 'path';
+module.exports = foo;
+
+// in some-file.js
+// eslint import/no-import-module-exports: ["error", {"exceptions": ["**/*/some-file.js"]}]
+import foo from 'path';
+module.exports = foo;
+```
+
+### Further Reading
+ - [webpack issue #4039](https://github.com/webpack/webpack/issues/4039)

package.json

@@ -109,6 +109,7 @@
     "is-core-module": "^1.0.2",
     "minimatch": "^3.0.4",
     "object.values": "^1.1.1",
+    "pkg-up": "^1.0.0",
     "read-pkg-up": "^2.0.0",
     "resolve": "^1.17.0",
     "tsconfig-paths": "^3.9.0"

src/index.js

@@ -40,6 +40,7 @@ export const rules = {
   'no-unassigned-import': require('./rules/no-unassigned-import'),
   'no-useless-path-segments': require('./rules/no-useless-path-segments'),
   'dynamic-import-chunkname': require('./rules/dynamic-import-chunkname'),
+  'no-import-module-exports': require('./rules/no-import-module-exports'),
 
   // export
   'exports-last': require('./rules/exports-last'),

src/rules/no-import-module-exports.js

@@ -0,0 +1,66 @@
+import minimatch from 'minimatch';
+import path from 'path';
+import pkgUp from 'pkg-up';
+
+function getEntryPoint(context) {
+  const pkgPath = pkgUp.sync(context.getFilename());
+  return require.resolve(path.dirname(pkgPath));
+}
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description: 'Disallow import statements with module.exports',
+      category: 'Best Practices',
+      recommended: true,
+    },
+    fixable: 'code',
+    schema: [
+      {
+        'type': 'object',
+        'properties': {
+          'exceptions': { 'type': 'array' },
+        },
+        'additionalProperties': false,
+      },
+    ],
+  },
+  create(context) {
+    const importDeclarations = [];
+    const entryPoint = getEntryPoint(context);
+    const options = context.options[0] || {};
+    let alreadyReported = false;
+
+    function report(node) {
+      const fileName = context.getFilename();
+      const isEntryPoint = entryPoint === fileName;
+      const isIdentifier = node.object.type === 'Identifier';
+      const hasKeywords = (/^(module|exports)$/).test(node.object.name);
+      const isException = options.exceptions &&
+        options.exceptions.some(glob => minimatch(fileName, glob));
+
+      if (isIdentifier && hasKeywords && !isEntryPoint && !isException) {
+        importDeclarations.forEach(importDeclaration => {
+          context.report({
+            node: importDeclaration,
+            message: `Cannot use import declarations in modules that export using ` +
+              `CommonJS (module.exports = 'foo' or exports.bar = 'hi')`,
+          });
+        });
+        alreadyReported = true;
+      }
+    }
+
+    return {
+      ImportDeclaration(node) {
+        importDeclarations.push(node);
+      },
+      MemberExpression(node) {
+        if (!alreadyReported) {
+          report(node);
+        }
+      },
+    };
+  },
+};

tests/src/rules/no-import-module-exports.js

@@ -0,0 +1,101 @@
+import path from 'path';
+import { RuleTester } from 'eslint';
+
+import { test } from '../utils';
+
+const ruleTester = new RuleTester({
+  parserOptions: { ecmaVersion: 6, sourceType: 'module' },
+});
+const rule = require('rules/no-import-module-exports');
+
+const error = {
+  message: `Cannot use import declarations in modules that export using CommonJS ` +
+    `(module.exports = 'foo' or exports.bar = 'hi')`,
+  type: 'ImportDeclaration',
+};
+
+ruleTester.run('no-import-module-exports', rule, {
+  valid: [
+    test({
+      code: `
+        const thing = require('thing')
+        module.exports = thing
+      `,
+    }),
+    test({
+      code: `
+        import thing from 'otherthing'
+        console.log(thing.module.exports)
+      `,
+    }),
+    test({
+      code: `
+        import thing from 'other-thing'
+        export default thing
+      `,
+    }),
+    test({
+      code: `
+        const thing = require('thing')
+        exports.foo = bar
+      `,
+    }),
+    test({
+      code: `
+        import foo from 'path';
+        module.exports = foo;
+      `,
+      // When the file matches the entry point defined in package.json
+      // See tests/files/package.json
+      filename: path.join(process.cwd(), 'tests/files/index.js'),
+    }),
+    test({
+      code: `
+        import foo from 'path';
+        module.exports = foo;
+      `,
+      filename: path.join(process.cwd(), 'tests/files/some/other/entry-point.js'),
+      options: [{ exceptions: ['**/*/other/entry-point.js'] }],
+    }),
+  ],
+  invalid: [
+    test({
+      code: `
+        import { stuff } from 'starwars'
+        module.exports = thing
+      `,
+      errors: [error],
+    }),
+    test({
+      code: `
+        import thing from 'starwars'
+        const baz = module.exports = thing
+        console.log(baz)
+      `,
+      errors: [error],
+    }),
+    test({
+      code: `
+        import * as allThings from 'starwars'
+        exports.bar = thing
+      `,
+      errors: [error],
+    }),
+    test({
+      code: `
+        import thing from 'other-thing'
+        exports.foo = bar
+      `,
+      errors: [error],
+    }),
+    test({
+      code: `
+        import foo from 'path';
+        module.exports = foo;
+      `,
+      filename: path.join(process.cwd(), 'tests/files/some/other/entry-point.js'),
+      options: [{ exceptions: ['**/*/other/file.js'] }],
+      errors: [error],
+    }),
+  ],
+});