[Fix] group-exports: Flow type export awareness

Fixes #1702.
import-js · Mar 29, 2020 · baf1a8c · baf1a8c
1 parent 6a110dd
commit baf1a8c
Show file tree

Hide file tree

Showing 5 changed files with 129 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,9 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
 
 ## [Unreleased]
 
+### Fixed
+- [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano])
+
 ## [2.20.2] - 2020-03-28
 ### Fixed
 - [`order`]: fix `isExternalModule` detect on windows ([#1651], thanks [@fisker])
@@ -660,6 +663,7 @@ for info on changes for earlier releases.
 
 [`memo-parser`]: ./memo-parser/README.md
 
+[#1702]: https://github.com/benmosher/eslint-plugin-import/issues/1702
 [#1666]: https://github.com/benmosher/eslint-plugin-import/pull/1666
 [#1664]: https://github.com/benmosher/eslint-plugin-import/pull/1664
 [#1658]: https://github.com/benmosher/eslint-plugin-import/pull/1658
@@ -988,6 +992,7 @@ for info on changes for earlier releases.
 [0.12.1]: https://github.com/benmosher/eslint-plugin-import/compare/v0.12.0...v0.12.1
 [0.12.0]: https://github.com/benmosher/eslint-plugin-import/compare/v0.11.0...v0.12.0
 [0.11.0]: https://github.com/benmosher/eslint-plugin-import/compare/v0.10.1...v0.11.0
+
 [@mathieudutour]: https://github.com/mathieudutour
 [@gausie]: https://github.com/gausie
 [@singles]: https://github.com/singles
@@ -1122,3 +1127,4 @@ for info on changes for earlier releases.
 [@fisker]: https://github.com/fisker
 [@richardxia]: https://github.com/richardxia
 [@TheCrueltySage]: https://github.com/TheCrueltySage
+[@ernestostifano]: https://github.com/ernestostifano
diff --git a/docs/rules/group-exports.md b/docs/rules/group-exports.md
@@ -60,6 +60,15 @@ test.another = true
 module.exports = test
 ```
 
+```flow js
+const first = true;
+type firstType = boolean
+
+// A single named export declaration (type exports handled separately) -> ok
+export {first}
+export type {firstType}
+```
+
 
 ### Invalid
 
@@ -94,6 +103,15 @@ module.exports.first = true
 module.exports.second = true
 ```
 
+```flow js
+type firstType = boolean
+type secondType = any
+
+// Multiple named type export statements -> not ok!
+export type {firstType}
+export type {secondType}
+```
+
 ## 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/package.json b/package.json
@@ -62,6 +62,7 @@
     "babel-plugin-istanbul": "^4.1.6",
     "babel-plugin-module-resolver": "^2.7.1",
     "babel-preset-es2015-argon": "latest",
+    "babel-preset-flow": "^6.23.0",
     "babel-register": "^6.26.0",
     "babylon": "^6.18.0",
     "chai": "^4.2.0",

diff --git a/src/rules/group-exports.js b/src/rules/group-exports.js
@@ -46,19 +46,28 @@ function accessorChain(node) {
 
 function create(context) {
   const nodes = {
-    modules: new Set(),
-    commonjs: new Set(),
-    sources: {},
+    modules: {
+      set: new Set(),
+      sources: {},
+    },
+    types: {
+      set: new Set(),
+      sources: {},
+    },
+    commonjs: {
+      set: new Set(),
+    },
   }
 
   return {
     ExportNamedDeclaration(node) {
+      let target = node.exportKind === 'type' ? nodes.types : nodes.modules
       if (!node.source) {
-        nodes.modules.add(node)
-      } else if (Array.isArray(nodes.sources[node.source.value])) {
-        nodes.sources[node.source.value].push(node)
+        target.set.add(node)
+      } else if (Array.isArray(target.sources[node.source.value])) {
+        target.sources[node.source.value].push(node)
       } else {
-        nodes.sources[node.source.value] = [node]
+        target.sources[node.source.value] = [node]
       }
     },
 
@@ -73,21 +82,21 @@ function create(context) {
       // 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)
+        nodes.commonjs.set.add(node)
         return
       }
 
       // Assignments to exports (exports.* = *)
       if (chain[0] === 'exports' && chain.length === 2) {
-        nodes.commonjs.add(node)
+        nodes.commonjs.set.add(node)
         return
       }
     },
 
     'Program:exit': function onExit() {
       // Report multiple `export` declarations (ES2015 modules)
-      if (nodes.modules.size > 1) {
-        nodes.modules.forEach(node => {
+      if (nodes.modules.set.size > 1) {
+        nodes.modules.set.forEach(node => {
           context.report({
             node,
             message: errors[node.type],
@@ -96,7 +105,27 @@ function create(context) {
       }
 
       // Report multiple `aggregated exports` from the same module (ES2015 modules)
-      flat(values(nodes.sources)
+      flat(values(nodes.modules.sources)
+        .filter(nodesWithSource => Array.isArray(nodesWithSource) && nodesWithSource.length > 1))
+        .forEach((node) => {
+          context.report({
+            node,
+            message: errors[node.type],
+          })
+        })
+
+      // Report multiple `export type` declarations (FLOW ES2015 modules)
+      if (nodes.types.set.size > 1) {
+        nodes.types.set.forEach(node => {
+          context.report({
+            node,
+            message: errors[node.type],
+          })
+        })
+      }
+
+      // Report multiple `aggregated type exports` from the same module (FLOW ES2015 modules)
+      flat(values(nodes.types.sources)
         .filter(nodesWithSource => Array.isArray(nodesWithSource) && nodesWithSource.length > 1))
         .forEach((node) => {
           context.report({
@@ -106,8 +135,8 @@ function create(context) {
         })
 
       // Report multiple `module.exports` assignments (CommonJS)
-      if (nodes.commonjs.size > 1) {
-        nodes.commonjs.forEach(node => {
+      if (nodes.commonjs.set.size > 1) {
+        nodes.commonjs.set.forEach(node => {
           context.report({
             node,
             message: errors[node.type],

diff --git a/tests/src/rules/group-exports.js b/tests/src/rules/group-exports.js
@@ -1,14 +1,25 @@
 import { test } from '../utils'
 import { RuleTester } from 'eslint'
 import rule from 'rules/group-exports'
+import {resolve} from 'path'
+import {default as babelPresetFlow} from 'babel-preset-flow'
 
 /* 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()
+const ruleTester = new RuleTester({
+  parser: resolve(__dirname, '../../../node_modules/babel-eslint'),
+  parserOptions: {
+    babelOptions: {
+      configFile: false,
+      babelrc: false,
+      presets: [babelPresetFlow],
+    },
+  },
+})
 
 ruleTester.run('group-exports', rule, {
   valid: [
@@ -103,6 +114,27 @@ ruleTester.run('group-exports', rule, {
       unrelated = 'assignment'
       module.exports.test = true
     ` }),
+    test({ code: `
+      type firstType = {
+        propType: string
+      };
+      const first = {};
+      export type { firstType };
+      export { first };
+    ` }),
+    test({ code: `
+      type firstType = {
+        propType: string
+      };
+      type secondType = {
+        propType: string
+      };
+      export type { firstType, secondType };
+    ` }),
+    test({ code: `
+      export type { type1A, type1B } from './module-1'
+      export { method1 } from './module-1'
+    ` }),
   ],
   invalid: [
     test({
@@ -231,5 +263,33 @@ ruleTester.run('group-exports', rule, {
         errors.commonjs,
       ],
     }),
+    test({
+      code: `
+        type firstType = {
+          propType: string
+        };
+        type secondType = {
+          propType: string
+        };
+        const first = {};
+        export type { firstType };
+        export type { secondType };
+        export { first };
+      `,
+      errors: [
+        errors.named,
+        errors.named,
+      ],
+    }),
+    test({
+      code: `
+        export type { type1 } from './module-1'
+        export type { type2 } from './module-1'
+      `,
+      errors: [
+        errors.named,
+        errors.named,
+      ],
+    }),
   ],
 })