Updates based on PR feedback

README.md

@@ -77,7 +77,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 single named export declaration ([`group-exports`])
+* Prefer named exports to be grouped together in a single export declaration ([`group-exports`])
 
 [`first`]: ./docs/rules/first.md
 [`no-duplicates`]: ./docs/rules/no-duplicates.md

docs/rules/group-exports.md

@@ -1,27 +1,25 @@
 # group-exports
 
-Reports when multiple named exports or CommonJS assignments are present in a single file and when the default export is not adjacent to the named export.
+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 along with the default export being placed next to the named export, your exports will remain at one place, making it easier to see what exports a module provides.
+**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 exports or assignments to `module.exports` (or `exports`) and when the default export is not adjacent to the named export.
+This rule warns whenever a single file contains multiple named export declarations or multiple assignments to `module.exports` (or `exports`).
 
 ### Valid
 
 ```js
-// Default export statement is adjacent to named export -> ok
-export default function test() {}
-// A single named export statement -> ok
+// A single named export declaration -> ok
 export const valid = true
 ```
 
 ```js
 const first = true
 const second = true
 
-// A single named export statement -> ok
+// A single named export declaration -> ok
 export {
   first,
   second,
@@ -36,6 +34,24 @@ module.exports = {
 }
 ```
 
+```js
+const first = true
+const second = true
+
+// A single exports assignment -> ok
+module.exports = {
+  first,
+  second,
+}
+```
+
+```js
+// "Default" export is not an object literal -> ok (ignore)
+module.exports = () => {}
+module.exports.first = true
+module.exports.second = true
+```
+
 ### Invalid
 
 ```js
@@ -45,16 +61,15 @@ export const second = true
 ```
 
 ```js
-// Default export is not adjacent to the named export statement -> not ok!
-export default {}
-const first = true
-export { first }
+// Multiple exports assignments -> not ok!
+exports.first = true
+exports.second = true
 ```
 
 ```js
 // Multiple exports assignments -> not ok!
-exports.first = true
-exports.second = true
+module.exports = {}
+module.exports.first = true
 ```
 
 ## When Not To Use It

src/rules/group-exports.js

@@ -1,34 +1,14 @@
 const meta = {}
 /* eslint-disable max-len */
 const errors = {
-  ExportNamedDeclaration: 'Multiple named export declarations; consolidate all named exports into a single statement',
-  ExportDefaultDeclaration: 'Default export declaration should be adjacent to named export',
+  ExportNamedDeclaration: 'Multiple named export declarations; consolidate all named exports into a single export declaration',
   MemberExpression: 'Multiple CommonJS exports; consolidate all exports into a single assignment to `module.exports`',
 }
 /* eslint-enable max-len */
-
-/**
- * Check if two nodes are adjacent (only whitespace between them)
- *
- * The two nodes do not have to be sorted in the order they appear in the code.
- *
- * @param     {Object}    opts            Options for the check
- * @param     {Object}    opts.context    The context of the nodes
- * @param     {Object}    opts.first      The first node
- * @param     {Object}    opts.second     The second node
- * @return    {Boolean}
- * @private
- */
-function isAdjacent(opts = {}) {
-  const sourceCode = opts.context.getSourceCode()
-
-  if (sourceCode.getTokensBetween(opts.first, opts.second).length ||
-    sourceCode.getTokensBetween(opts.second, opts.first).length) {
-    return false
-  }
-
-  return true
-}
+const parents = [
+  'AssignmentExpression',
+  'MemberExpression',
+]
 
 /**
  * Determine how many property accesses precede this node
@@ -51,78 +31,73 @@ function accessorDepth(node) {
 }
 
 function create(context) {
-  const exports = {
-    named: new Set(),
-    default: null,
-    last: null,
+  const named = new Set()
+  const commonjs = {
+    defaultExportIsObject: false,
   }
 
   return {
-    ExportDefaultDeclaration(node) {
-      exports.default = node
-    },
-
     ExportNamedDeclaration(node) {
-      exports.named.add(node)
-      exports.last = node
+      named.add(node)
     },
 
     MemberExpression(node) {
-      if (['MemberExpression', 'AssignmentExpression'].indexOf(node.parent.type) === -1) {
-        return
-      }
+      const parent = node.parent
 
-      // Member expressions on the right side of the assignment do not interest us
-      if (node.parent.type === 'AssignmentExpression' && node.parent.left !== node) {
+      // These are not the parents you are looking for
+      if (parents.indexOf(parent.type) === -1) {
         return
       }
 
-      if (node.object.name === 'module' && node.property.name === 'exports') {
-        // module.exports.exported.*: assignments this deep should not be considered as exports
-        if (accessorDepth(node) > 2) {
+      if (parent.type === 'AssignmentExpression') {
+        // Member expressions on the right side of the assignment do not interest us
+        if (parent.left !== node) {
           return
         }
 
-        exports.named.add(node)
-        return
-      }
-
-      if (node.object.name === 'exports') {
-        // exports.exported.*: assignments this deep should not be considered as exports
-        if (accessorDepth(node) > 1) {
+        // Special case: when assigning an object literal to `module.exports`, treat it as a named
+        // export declaration
+        if (parent.right.type === 'ObjectExpression') {
+          commonjs.defaultExportIsObject = true
+          named.add(node)
           return
         }
+      }
+
+      const depth = accessorDepth(node)
+
+      // Assignments to module.exports
+      // Only treat assignments to `module.exports` object as named exports, ie.
+      // module.exports.named = true
+      // And ignore direct assignments and assignments more deep, ie.
+      // module.exports = {}
+      // module.exports.named.property = true
+      if (node.object.name === 'module' && node.property.name === 'exports' && depth === 2) {
+        named.add(node)
+        return
+      }
 
-        exports.named.add(node)
+      // Assignments to exports
+      if (node.object.name === 'exports' && depth === 1) {
+        named.add(node)
         return
       }
     },
 
     'Program:exit': function onExit() {
-      if (exports.named.size > 1) {
-        for (const node of exports.named) {
+      if (named.size > 1) {
+        for (const node of named) {
+          // If the "default" CommonJS export was not an object literal, do not report anything
+          if (node.type !== 'ExportNamedDeclaration' && !commonjs.defaultExportIsObject) {
+            continue
+          }
+
           context.report({
             node,
             message: errors[node.type],
           })
         }
       }
-
-      // There is exactly one named export and a default export -> check if they are adjacent
-      if (exports.default && exports.last && exports.named.size === 1) {
-        const adjacent = isAdjacent({
-          context,
-          first: exports.default,
-          second: exports.last,
-        })
-
-        if (!adjacent) {
-          context.report({
-            node: exports.default,
-            message: errors[exports.default.type],
-          })
-        }
-      }
     },
   }
 }