From baf1a8c697d14ffa229e24455e84cc7192287d64 Mon Sep 17 00:00:00 2001 From: Ernesto Stifano Date: Sun, 29 Mar 2020 16:44:53 +0200 Subject: [PATCH] [Fix] `group-exports`: Flow type export awareness Fixes #1702. --- CHANGELOG.md | 6 ++++ docs/rules/group-exports.md | 18 ++++++++++ package.json | 1 + src/rules/group-exports.js | 57 +++++++++++++++++++++-------- tests/src/rules/group-exports.js | 62 +++++++++++++++++++++++++++++++- 5 files changed, 129 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b76324d4..44c62ee56 100644 --- 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 index f61ff5306..e6b9887b2 100644 --- 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 index d6c4c95a0..90e4ac8c3 100644 --- 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 index cd7fc992d..8abeb3d23 100644 --- 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 index 9a0c2c1ba..0766a325e 100644 --- a/tests/src/rules/group-exports.js +++ b/tests/src/rules/group-exports.js @@ -1,6 +1,8 @@ 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 = { @@ -8,7 +10,16 @@ const errors = { 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, + ], + }), ], })