Merge pull request #154 from benmosher/modules-merge

Merge no-cjs, no-define from eslint-plugin-modules

README.md

@@ -31,11 +31,13 @@ Helpful warnings:
 
 Style guide:
 
-* Report CommonJS `require` calls. ([`no-require`])
+* Report CommonJS `require` calls and `module.exports` or `exports.*`. ([`no-commonjs`])
+* Report AMD `require` and `define` calls. ([`no-amd`])
 * Ensure all imports appear before other statements ([`imports-first`])
 * Report repeated import of the same module in multiple places ([`no-duplicates`])
 
-[`no-require`]: ./docs/rules/no-require.md
+[`no-commonjs`]: ./docs/rules/no-commonjs.md
+[`no-amd`]: ./docs/rules/no-amd.md
 [`imports-first`]: ./docs/rules/imports-first.md
 [`no-duplicates`]: ./docs/rules/no-duplicates.md
 

docs/rules/no-amd.md

@@ -0,0 +1,35 @@
+# no-amd
+
+Reports `require([array], ...)` and `define([array], ...) function calls at the
+module scope. Will not report if !=2 arguments, or first argument is not a literal array.
+
+Intended for temporary use when migrating to pure ES6 modules.
+
+## Rule Details
+
+This will be reported:
+
+```js
+define(["a", "b"], function (a, b) { /* ... */ })
+
+require(["b", "c"], function (b, c) { /* ... */ })
+```
+
+CommonJS `require` is still valid.
+
+## When Not To Use It
+
+If you don't mind mixing module systems (sometimes this is useful), you probably
+don't want this rule.
+
+It is also fairly noisy if you have a larger codebase that is being transitioned
+from AMD to ES6 modules.
+
+## Contributors
+
+Special thanks to @xjamundx for donating his no-define rule as a start to this.
+
+## Further Reading
+
+- [`no-commonjs`](./no-commonjs.md): report CommonJS `require` and `exports`
+- Source: https://github.com/xjamundx/eslint-plugin-modules

docs/rules/no-commonjs.md

@@ -0,0 +1,61 @@
+# no-commonjs
+
+Reports `require([string])` function calls. Will not report if >1 argument,
+or single argument is not a literal string.
+
+Reports `module.exports` or `exports.*`, also.
+
+Intended for temporary use when migrating to pure ES6 modules.
+
+## Rule Details
+
+This will be reported:
+
+```js
+var mod = require('./mod')
+  , common = require('./common')
+  , fs = require('fs')
+  , whateverModule = require('./not-found')
+
+module.exports = { a: "b" }
+exports.c = "d"
+```
+
+If `allow-primitive-modules` is provided as an option, the following is valid:
+
+```js
+/*eslint no-commonjs: [2, "allow-primitive-modules"]*/
+
+module.exports = "foo"
+module.exports = function rule(context) { return { /* ... */ } }
+```
+
+but this is still reported:
+
+```js
+/*eslint no-commonjs: [2, "allow-primitive-modules"]*/
+
+module.exports = { x: "y" }
+exports.z = function boop() { /* ... */ }
+```
+
+This is useful for things like ESLint rule modules, which must export a function as
+the module.
+
+## When Not To Use It
+
+If you don't mind mixing module systems (sometimes this is useful), you probably
+don't want this rule.
+
+It is also fairly noisy if you have a larger codebase that is being transitioned
+from CommonJS to ES6 modules.
+
+
+## Contributors
+
+Special thanks to @xjamundx for donating the module.exports and exports.* bits.
+
+## Further Reading
+
+- [`no-amd`](./no-amd.md): report on AMD `require`, `define`
+- Source: https://github.com/xjamundx/eslint-plugin-modules

src/index.js

@@ -7,7 +7,8 @@ export const rules = {
 
   'no-named-as-default': require('./rules/no-named-as-default'),
 
-  'no-require': require('./rules/no-require'),
+  'no-commonjs': require('./rules/no-commonjs'),
+  'no-amd': require('./rules/no-amd'),
   'no-duplicates': require('./rules/no-duplicates'),
   'imports-first': require('./rules/imports-first'),
 }
@@ -21,7 +22,8 @@ export const rulesConfig = {
 
   'no-named-as-default': 0,
 
-  'no-require': 0,
+  'no-commonjs': 0,
+  'no-amd': 0,
   'no-duplicates': 0,
   'imports-first': 0,
 }

src/rules/no-amd.js

@@ -0,0 +1,33 @@
+/**
+ * @fileoverview Rule to prefer imports to AMD
+ * @author Jamund Ferguson
+ */
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function (context) {
+
+	return {
+
+		'CallExpression': function (node) {
+      if (context.getScope().type !== 'module') return
+
+      if (node.callee.type !== 'Identifier') return
+      if (node.callee.name !== 'require' &&
+          node.callee.name !== 'define') return
+
+      // todo: capture define((require, module, exports) => {}) form?
+      if (node.arguments.length !== 2) return
+
+      const modules = node.arguments[0]
+      if (modules.type !== 'ArrayExpression') return
+
+      // todo: check second arg type? (identifier or callback)
+
+			context.report(node, `Expected imports instead of AMD ${node.callee.name}().`)
+		},
+	}
+
+}

src/rules/no-commonjs.js

@@ -0,0 +1,59 @@
+/**
+ * @fileoverview Rule to prefer ES6 to CJS
+ * @author Jamund Ferguson
+ */
+
+const EXPORT_MESSAGE = 'Expected "export" or "export default"'
+    , IMPORT_MESSAGE = 'Expected "import" instead of "require()"'
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+
+module.exports = function (context) {
+
+  return {
+
+    'MemberExpression': function (node) {
+
+      // module.exports
+      if (node.object.name === 'module' && node.property.name === 'exports') {
+        if (allowPrimitive(node, context)) return
+        context.report({ node, message: EXPORT_MESSAGE })
+      }
+
+      // exports.
+      if (node.object.name === 'exports') {
+        context.report({ node, message: EXPORT_MESSAGE })
+      }
+
+    },
+    'CallExpression': function (call) {
+      if (context.getScope().type !== 'module') return
+
+      if (call.callee.type !== 'Identifier') return
+      if (call.callee.name !== 'require') return
+
+      if (call.arguments.length !== 1) return
+      var module = call.arguments[0]
+
+      if (module.type !== 'Literal') return
+      if (typeof module.value !== 'string') return
+
+      // keeping it simple: all 1-string-arg `require` calls are reported
+      context.report({
+        node: call.callee,
+        message: IMPORT_MESSAGE,
+      })
+    },
+  }
+
+}
+
+  // allow non-objects as module.exports
+function allowPrimitive(node, context) {
+  if (context.options.indexOf('allow-primitive-modules') < 0) return false
+  if (node.parent.type !== 'AssignmentExpression') return false
+  return (node.parent.right.type !== 'ObjectExpression')
+}

tests/src/rules/no-amd.js

@@ -0,0 +1,34 @@
+import { RuleTester } from 'eslint'
+
+var ruleTester = new RuleTester()
+
+ruleTester.run('no-amd', require('rules/no-amd'), {
+	valid: [
+		{ code: 'import "x";', ecmaFeatures: { modules: true } },
+		{ code: 'import x from "x"', ecmaFeatures: { modules: true } },
+		'var x = require("x")',
+    'require("x")',
+    // 2-args, not an array
+		'require("x", "y")',
+    // random other function
+    'setTimeout(foo, 100)',
+    // non-identifier callee
+    '(a || b)(1, 2, 3)',
+
+    // nested scope is fine
+    'function x() { define(["a"], function (a) {}) }',
+    'function x() { require(["a"], function (a) {}) }',
+
+    // unmatched arg types/number
+    'define(0, 1, 2)',
+    'define("a")',
+	],
+
+	invalid: [
+    { code: 'define([], function() {})', errors: [ { message: 'Expected imports instead of AMD define().' }] },
+    { code: 'define(["a"], function(a) { console.log(a); })', errors: [ { message: 'Expected imports instead of AMD define().' }] },
+
+		{ code: 'require([], function() {})', errors: [ { message: 'Expected imports instead of AMD require().' }] },
+		{ code: 'require(["a"], function(a) { console.log(a); })', errors: [ { message: 'Expected imports instead of AMD require().' }] },
+	],
+})

tests/src/rules/no-commonjs.js

@@ -0,0 +1,56 @@
+import { RuleTester } from 'eslint'
+
+const EXPORT_MESSAGE = 'Expected "export" or "export default"'
+    , IMPORT_MESSAGE = 'Expected "import" instead of "require()"'
+
+const ruleTester = new RuleTester()
+
+ruleTester.run('no-commonjs', require('rules/no-commonjs'), {
+  valid: [
+
+    // imports
+    { code: 'import "x";', ecmaFeatures: { modules: true } },
+    { code: 'import x from "x"', ecmaFeatures: { modules: true } },
+    { code: 'import x from "x"', ecmaFeatures: { modules: true } },
+    { code: 'import { x } from "x"', ecmaFeatures: { modules: true } },
+
+    // exports
+    { code: 'export default "x"', ecmaFeatures: { modules: true } },
+    { code: 'export function house() {}', ecmaFeatures: { modules: true } },
+
+    // allowed requires
+    { code: 'function a() { var x = require("y"); }' }, // nested requires allowed
+    { code: 'require.resolve("help")' }, // methods of require are allowed
+    { code: 'require.ensure([])' }, // webpack specific require.ensure is allowed
+    { code: 'require([], function(a, b, c) {})' }, // AMD require is allowed
+    { code: "var bar = require('./bar', true);" },
+    { code: "var bar = proxyquire('./bar');" },
+    { code: "var bar = require('./ba' + 'r');" },
+    { code: 'var zero = require(0);' },
+
+    { code: 'module.exports = function () {}', options: ['allow-primitive-modules'] },
+    { code: 'module.exports = "foo"', options: ['allow-primitive-modules'] },
+  ],
+
+  invalid: [
+
+    // imports
+    { code: 'var x = require("x")', errors: [ { message: IMPORT_MESSAGE }] },
+    { code: 'require("x")', errors: [ { message: IMPORT_MESSAGE }] },
+
+    // exports
+    { code: 'exports.face = "palm"', errors: [ { message: EXPORT_MESSAGE }] },
+    { code: 'module.exports.face = "palm"', errors: [ { message: EXPORT_MESSAGE }] },
+    { code: 'module.exports = face', errors: [ { message: EXPORT_MESSAGE }] },
+    { code: 'exports = module.exports = {}', errors: [ { message: EXPORT_MESSAGE }] },
+    { code: 'var x = module.exports = {}', errors: [ { message: EXPORT_MESSAGE }] },
+    { code: 'module.exports = {}',
+      options: ['allow-primitive-modules'],
+      errors: [ { message: EXPORT_MESSAGE }],
+    },
+    { code: 'var x = module.exports',
+      options: ['allow-primitive-modules'],
+      errors: [ { message: EXPORT_MESSAGE }],
+    },
+  ],
+})