From 365dfbb8c4be69cc87523d44b218bfe3a6a4b465 Mon Sep 17 00:00:00 2001
From: Robert Rossmann <rr.rossmann@me.com>
Date: Mon, 16 Jan 2017 23:29:13 +0100
Subject: [PATCH] New: `group-exports` rule

---
 CHANGELOG.md                     |   4 +
 README.md                        |   2 +
 docs/rules/group-exports.md      |  87 ++++++++++++
 src/index.js                     |   1 +
 src/rules/group-exports.js       |  98 ++++++++++++++
 tests/src/rules/group-exports.js | 221 +++++++++++++++++++++++++++++++
 6 files changed, 413 insertions(+)
 create mode 100644 docs/rules/group-exports.md
 create mode 100644 src/rules/group-exports.js
 create mode 100644 tests/src/rules/group-exports.js

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 327c7f059..bd96d8abe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## [Unreleased]
 
+- Add [`group-exports`] rule: style-guide rule to report use of multiple named exports ([#721], thanks [@robertrossmann])
 
 ## [2.8.0] - 2017-10-18
 ### Added
@@ -431,6 +432,7 @@ for info on changes for earlier releases.
 [`unambiguous`]: ./docs/rules/unambiguous.md
 [`no-anonymous-default-export`]: ./docs/rules/no-anonymous-default-export.md
 [`exports-last`]: ./docs/rules/exports-last.md
+[`group-exports`]: ./docs/rules/group-exports.md
 
 [`memo-parser`]: ./memo-parser/README.md
 
@@ -442,6 +444,7 @@ for info on changes for earlier releases.
 [#744]: https://github.com/benmosher/eslint-plugin-import/pull/744
 [#742]: https://github.com/benmosher/eslint-plugin-import/pull/742
 [#737]: https://github.com/benmosher/eslint-plugin-import/pull/737
+[#721]: https://github.com/benmosher/eslint-plugin-import/pull/721
 [#712]: https://github.com/benmosher/eslint-plugin-import/pull/712
 [#696]: https://github.com/benmosher/eslint-plugin-import/pull/696
 [#685]: https://github.com/benmosher/eslint-plugin-import/pull/685
@@ -661,3 +664,4 @@ for info on changes for earlier releases.
 [@mplewis]: https://github.com/mplewis
 [@rosswarren]: https://github.com/rosswarren
 [@alexgorbatchev]: https://github.com/alexgorbatchev
+[@robertrossmann]: https://github.com/robertrossmann
diff --git a/README.md b/README.md
index 11a779cfe..cfe360e4f 100644
--- a/README.md
+++ b/README.md
@@ -78,6 +78,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 * Forbid unassigned imports ([`no-unassigned-import`])
 * Forbid named default exports ([`no-named-default`])
 * Forbid anonymous values as default exports ([`no-anonymous-default-export`])
+* Prefer named exports to be grouped together in a single export declaration ([`group-exports`])
 
 [`first`]: ./docs/rules/first.md
 [`exports-last`]: ./docs/rules/exports-last.md
@@ -91,6 +92,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
 [`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md
 [`no-named-default`]: ./docs/rules/no-named-default.md
 [`no-anonymous-default-export`]: ./docs/rules/no-anonymous-default-export.md
+[`group-exports`]: ./docs/rules/group-exports.md
 
 ## Installation
 
diff --git a/docs/rules/group-exports.md b/docs/rules/group-exports.md
new file mode 100644
index 000000000..d7abe721b
--- /dev/null
+++ b/docs/rules/group-exports.md
@@ -0,0 +1,87 @@
+# group-exports
+
+Reports when named exports are not grouped together in a single `export` declaration or when multiple assignments to CommonJS `module.exports` or `exports` object are present in a single file.
+
+**Rationale:** An `export` declaration or `module.exports` assignment can appear anywhere in the code. By requiring a single export declaration all your exports will remain at one place, making it easier to see what exports a module provides.
+
+## Rule Details
+
+This rule warns whenever a single file contains multiple named export declarations or multiple assignments to `module.exports` (or `exports`).
+
+### Valid
+
+```js
+// A single named export declaration -> ok
+export const valid = true
+```
+
+```js
+const first = true
+const second = true
+
+// A single named export declaration -> ok
+export {
+  first,
+  second,
+}
+```
+
+```js
+// A single exports assignment -> ok
+module.exports = {
+  first: true,
+  second: true
+}
+```
+
+```js
+const first = true
+const second = true
+
+// A single exports assignment -> ok
+module.exports = {
+  first,
+  second,
+}
+```
+
+```js
+function test() {}
+test.property = true
+test.another = true
+
+// A single exports assignment -> ok
+module.exports = test
+```
+
+
+### Invalid
+
+```js
+// Multiple named export statements -> not ok!
+export const first = true
+export const second = true
+```
+
+```js
+// Multiple exports assignments -> not ok!
+exports.first = true
+exports.second = true
+```
+
+```js
+// Multiple exports assignments -> not ok!
+module.exports = {}
+module.exports.first = true
+```
+
+```js
+// Multiple exports assignments -> not ok!
+module.exports = () => {}
+module.exports.first = true
+module.exports.second = true
+```
+
+## When Not To Use It
+
+If you do not mind having your exports spread across the file, you can safely turn this rule off.
diff --git a/src/index.js b/src/index.js
index e5b36b8f5..a92489eef 100644
--- a/src/index.js
+++ b/src/index.js
@@ -9,6 +9,7 @@ export const rules = {
   'extensions': require('./rules/extensions'),
   'no-restricted-paths': require('./rules/no-restricted-paths'),
   'no-internal-modules': require('./rules/no-internal-modules'),
+  'group-exports': require('./rules/group-exports'),
 
   'no-named-default': require('./rules/no-named-default'),
   'no-named-as-default': require('./rules/no-named-as-default'),
diff --git a/src/rules/group-exports.js b/src/rules/group-exports.js
new file mode 100644
index 000000000..ed2e96ed7
--- /dev/null
+++ b/src/rules/group-exports.js
@@ -0,0 +1,98 @@
+const meta = {}
+/* eslint-disable max-len */
+const errors = {
+  ExportNamedDeclaration: 'Multiple named export declarations; consolidate all named exports into a single export declaration',
+  AssignmentExpression: 'Multiple CommonJS exports; consolidate all exports into a single assignment to `module.exports`',
+}
+/* eslint-enable max-len */
+
+/**
+ * Returns an array with names of the properties in the accessor chain for MemberExpression nodes
+ *
+ * Example:
+ *
+ * `module.exports = {}` => ['module', 'exports']
+ * `module.exports.property = true` => ['module', 'exports', 'property']
+ *
+ * @param     {Node}    node    AST Node (MemberExpression)
+ * @return    {Array}           Array with the property names in the chain
+ * @private
+ */
+function accessorChain(node) {
+  const chain = []
+
+  do {
+    chain.unshift(node.property.name)
+
+    if (node.object.type === 'Identifier') {
+      chain.unshift(node.object.name)
+      break
+    }
+
+    node = node.object
+  } while (node.type === 'MemberExpression')
+
+  return chain
+}
+
+function create(context) {
+  const nodes = {
+    modules: new Set(),
+    commonjs: new Set(),
+  }
+
+  return {
+    ExportNamedDeclaration(node) {
+      nodes.modules.add(node)
+    },
+
+    AssignmentExpression(node) {
+      if (node.left.type !== 'MemberExpression') {
+        return
+      }
+
+      const chain = accessorChain(node.left)
+
+      // Assignments to module.exports
+      // Deeper assignments are ignored since they just modify what's already being exported
+      // (ie. module.exports.exported.prop = true is ignored)
+      if (chain[0] === 'module' && chain[1] === 'exports' && chain.length <= 3) {
+        nodes.commonjs.add(node)
+        return
+      }
+
+      // Assignments to exports (exports.* = *)
+      if (chain[0] === 'exports' && chain.length === 2) {
+        nodes.commonjs.add(node)
+        return
+      }
+    },
+
+    'Program:exit': function onExit() {
+      // Report multiple `export` declarations (ES2015 modules)
+      if (nodes.modules.size > 1) {
+        nodes.modules.forEach(node => {
+          context.report({
+            node,
+            message: errors[node.type],
+          })
+        })
+      }
+
+      // Report multiple `module.exports` assignments (CommonJS)
+      if (nodes.commonjs.size > 1) {
+        nodes.commonjs.forEach(node => {
+          context.report({
+            node,
+            message: errors[node.type],
+          })
+        })
+      }
+    },
+  }
+}
+
+export default {
+  meta,
+  create,
+}
diff --git a/tests/src/rules/group-exports.js b/tests/src/rules/group-exports.js
new file mode 100644
index 000000000..3b08997e3
--- /dev/null
+++ b/tests/src/rules/group-exports.js
@@ -0,0 +1,221 @@
+import { test } from '../utils'
+import { RuleTester } from 'eslint'
+import rule from 'rules/group-exports'
+
+/* eslint-disable max-len */
+const errors = {
+  named: 'Multiple named export declarations; consolidate all named exports into a single export declaration',
+  commonjs: 'Multiple CommonJS exports; consolidate all exports into a single assignment to `module.exports`',
+}
+/* eslint-enable max-len */
+const ruleTester = new RuleTester()
+
+ruleTester.run('group-exports', rule, {
+  valid: [
+    test({ code: 'export const test = true' }),
+    test({ code: `
+      export default {}
+      export const test = true
+    ` }),
+    test({ code: `
+      const first = true
+      const second = true
+      export {
+        first,
+        second
+      }
+    ` }),
+    test({ code: `
+      export default {}
+      /* test */
+      export const test = true
+    ` }),
+    test({ code: `
+      export default {}
+      // test
+      export const test = true
+    ` }),
+    test({ code: `
+      export const test = true
+      /* test */
+      export default {}
+    ` }),
+    test({ code: `
+      export const test = true
+      // test
+      export default {}
+    ` }),
+    test({ code: 'module.exports = {} '}),
+    test({ code: `
+      module.exports = { test: true,
+        another: false }
+    ` }),
+    test({ code: 'exports.test = true' }),
+
+    test({ code: `
+      module.exports = {}
+      const test = module.exports
+    ` }),
+    test({ code: `
+      exports.test = true
+      const test = exports.test
+    ` }),
+    test({ code: `
+      module.exports = {}
+      module.exports.too.deep = true
+    ` }),
+    test({ code: `
+      module.exports.deep.first = true
+      module.exports.deep.second = true
+    ` }),
+    test({ code: `
+      module.exports = {}
+      exports.too.deep = true
+    ` }),
+    test({ code: `
+      export default {}
+      const test = true
+      export { test }
+    ` }),
+    test({ code: `
+      const test = true
+      export { test }
+      const another = true
+      export default {}
+    ` }),
+    test({ code: `
+      module.something.else = true
+      module.something.different = true
+    ` }),
+    test({ code: `
+      module.exports.test = true
+      module.something.different = true
+    ` }),
+    test({ code: `
+      exports.test = true
+      module.something.different = true
+    ` }),
+    test({ code: `
+      unrelated = 'assignment'
+      module.exports.test = true
+    ` }),
+  ],
+  invalid: [
+    test({
+      code: `
+        export const test = true
+        export const another = true
+      `,
+      errors: [
+        errors.named,
+        errors.named,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = {}
+        module.exports.test = true
+        module.exports.another = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = {}
+        module.exports.test = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = { test: true }
+        module.exports.another = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports.test = true
+        module.exports.another = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        exports.test = true
+        module.exports.another = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = () => {}
+        module.exports.attached = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = function test() {}
+        module.exports.attached = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = () => {}
+        exports.test = true
+        exports.another = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = "non-object"
+        module.exports.attached = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+    test({
+      code: `
+        module.exports = "non-object"
+        module.exports.attached = true
+        module.exports.another = true
+      `,
+      errors: [
+        errors.commonjs,
+        errors.commonjs,
+        errors.commonjs,
+      ],
+    }),
+  ],
+})