Skip to content

Commit

Permalink
New: group-exports rule
Browse files Browse the repository at this point in the history
  • Loading branch information
robertrossmann committed Jan 17, 2017
1 parent c975742 commit d4bed97
Show file tree
Hide file tree
Showing 6 changed files with 284 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]
### Added
- Add [`group-exports`] rule: style-guide rule to report use of multiple named exports ([#721], thanks [@Alaneor])

### Changed
- [`no-extraneous-dependencies`]: use `read-pkg-up` to simplify finding + loading `package.json` ([#680], thanks [@wtgtybhertgeghgtwtg])

Expand Down Expand Up @@ -376,7 +379,9 @@ for info on changes for earlier releases.
[`no-webpack-loader-syntax`]: ./docs/rules/no-webpack-loader-syntax.md
[`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md
[`unambiguous`]: ./docs/rules/unambiguous.md
[`group-exports`]: ./docs/rules/group-exports.md

[#721]: https://github.com/benmosher/eslint-plugin-import/pull/721
[#680]: https://github.com/benmosher/eslint-plugin-import/pull/680
[#654]: https://github.com/benmosher/eslint-plugin-import/pull/654
[#639]: https://github.com/benmosher/eslint-plugin-import/pull/639
Expand Down Expand Up @@ -561,3 +566,4 @@ for info on changes for earlier releases.
[@ntdb]: https://github.com/ntdb
[@jakubsta]: https://github.com/jakubsta
[@wtgtybhertgeghgtwtg]: https://github.com/wtgtybhertgeghgtwtg
[@Alaneor]: https://github.com/Alaneor
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Limit the maximum number of dependencies a module can have ([`max-dependencies`])
* Forbid unassigned imports ([`no-unassigned-import`])
* Forbid named default exports ([`no-named-default`])
* Prefer single named export declaration ([`group-exports`])

[`first`]: ./docs/rules/first.md
[`no-duplicates`]: ./docs/rules/no-duplicates.md
Expand All @@ -87,6 +88,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
[`max-dependencies`]: ./docs/rules/max-dependencies.md
[`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md
[`no-named-default`]: ./docs/rules/no-named-default.md
[`group-exports`]: ./docs/rules/group-exports.md

## Installation

Expand Down
62 changes: 62 additions & 0 deletions docs/rules/group-exports.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# 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.

**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.

## 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.

### Valid

```js
// Default export is adjacent to named export -> ok
export default function test() {}
// A single named export -> ok
export const valid = true
```

```js
const first = true
const second = true

// A single named export -> ok
export {
first,
second,
}
```

```js
// A single exports assignment -> ok
module.exports = {
first: true,
second: true
}
```

### Invalid

```js
// Multiple named exports -> not ok!
export const first = true
export const second = true
```

```js
// Default export is not adjacent to the named export -> not ok!
export default {}
const first = true
export { first }
```

```js
// Multiple exports assignments -> not ok!
exports.first = true
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.
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
133 changes: 133 additions & 0 deletions src/rules/group-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
const meta = {}
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',
MemberExpression:
'Multiple CommonJS exports; consolidate all exports into a single assignment to ' +
'`module.exports`',
}

/**
* 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
}

/**
* Determine how many property accesses precede this node
*
* For example, `module.exports` = 1, `module.exports.something` = 2 and so on.
*
* @param {Object} node The node being visited
* @return {Number}
* @private
*/
function accessorDepth(node) {
let depth = 0

while (node.type === 'MemberExpression') {
depth++
node = node.parent
}

return depth
}

function create(context) {
const exports = {
named: new Set(),
default: null,
last: null,
}

return {
ExportDefaultDeclaration(node) {
exports.default = node
},

ExportNamedDeclaration(node) {
exports.named.add(node)
exports.last = node
},

MemberExpression(node) {
if (['MemberExpression', 'AssignmentExpression'].indexOf(node.parent.type) === -1) {
return
}

// Member expressions on the right side of the assignment do not interest us
if (node.parent.type === 'AssignmentExpression' && node.parent.left !== node) {
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) {
return
}

return void exports.named.add(node)
}

if (node.object.name === 'exports') {
// exports.exported.*: assignments this deep should not be considered as exports
if (accessorDepth(node) > 1) {
return
}

return void exports.named.add(node)
}
},

'Program:exit': function onExit() {
if (exports.named.size > 1) {
for (const node of exports.named) {
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],
})
}
}
},
}
}

export default {
meta,
create,
}
80 changes: 80 additions & 0 deletions tests/src/rules/group-exports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { test } from '../utils'
import { RuleTester } from 'eslint'
import rule from 'rules/group-exports'

const errors = {
named:
'Multiple named export declarations; consolidate all named exports into a single statement',
default:
'Default export declaration should be adjacent to named export',
commonjs:
'Multiple CommonJS exports; consolidate all exports into a single assignment to ' +
'`module.exports`',
}
const ruleTester = new RuleTester()

ruleTester.run('group-exports', rule, {
valid: [
test({ code: 'export const test = true' }),
test({ code: 'export default {}\nexport const test = true' }),
test({ code: [
'const first = true',
'const second = true',
'export { first,\nsecond }',
].join('\n') }),
test({ code: 'export default {}\n/* test */\nexport const test = true'}),
test({ code: 'export default {}\n// test\nexport const test = true'}),
test({ code: 'export const test = true\n/* test */\nexport default {}'}),
test({ code: 'export const test = true\n// test\nexport default {}'}),
test({ code: 'module.exports = {} '}),
test({ code: 'module.exports = { test: true,\nanother: false }' }),
test({ code: 'exports.test = true' }),

test({ code: 'module.exports = {}\nconst test = module.exports' }),
test({ code: 'exports.test = true\nconst test = exports.test' }),
test({ code: 'module.exports = {}\nmodule.exports.too.deep = true' }),
test({ code: 'module.exports = {}\nexports.too.deep = true' }),
],
invalid: [
test({
code: [
'export const test = true',
'export const another = true',
].join('\n'),
errors: [
errors.named,
errors.named,
],
}),
test({
code: [
'module.exports = {}',
'module.exports.test = true',
'module.exports.another = true',
].join('\n'),
errors: [
errors.commonjs,
errors.commonjs,
errors.commonjs,
],
}),
test({
code: [
'module.exports = {}',
'module.exports = {}',
].join('\n'),
errors: [
errors.commonjs,
errors.commonjs,
],
}),
test({
code: 'export default {}\nconst test = true\nexport { test }',
errors: [errors.default],
}),
test({
code: 'const test = true\nexport { test }\nconst another = true\nexport default {}',
errors: [errors.default],
}),
],
})

0 comments on commit d4bed97

Please sign in to comment.