Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-named-default rule #596

Merged
merged 11 commits into from
Nov 2, 2016
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
This change log adheres to standards from [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]
- Add [`no-named-default`] rule: style-guide rule to report use of unnecessarily named default imports


## [2.0.0]! - 2016-09-30
Expand Down
6 changes: 4 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,9 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Enforce a convention in module import order ([`order`])
* Enforce a newline after import statements ([`newline-after-import`])
* Prefer a default export if module exports a single name ([`prefer-default-export`])
* Limit the maximum number of dependencies a module can have. ([`max-dependencies`])
* Forbid unassigned imports. ([`no-unassigned-import`])
* 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`])

[`first`]: ./docs/rules/first.md
[`no-duplicates`]: ./docs/rules/no-duplicates.md
Expand All @@ -85,6 +86,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
[`prefer-default-export`]: ./docs/rules/prefer-default-export.md
[`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

## Installation

Expand Down
30 changes: 30 additions & 0 deletions docs/rules/no-named-default.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# no-named-default

Reports use of a default export as a locally named import.

Rationale: the syntax exists to import default exports expressively, let's use it

- *misleading*: consistent syntax usage is less likely to confuse new developers
- *a mistake*: meant to import a named export and unintentionally specified `default`

## Rule Details

Given:
```js
// foo.js
export default 'foo';
export const bar = 'baz';
```

...these would be valid:
```js
import foo from './foo.js';
import foo, { bar } from './foo.js';
```

...and these would be reported:
```js
// message: Using exported name 'bar' as identifier for default export.
import { default as foo } from './foo.js';
import { default as foo, bar } from './foo.js';
```
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const rules = {
'no-restricted-paths': require('./rules/no-restricted-paths'),
'no-internal-modules': require('./rules/no-internal-modules'),

'no-named-default': require('./rules/no-named-default'),
'no-named-as-default': require('./rules/no-named-as-default'),
'no-named-as-default-member': require('./rules/no-named-as-default-member'),

Expand Down
22 changes: 22 additions & 0 deletions src/rules/no-named-default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module.exports = {
meta: {
docs: {},
},

create: function (context) {
return {
'ImportDeclaration': function (node) {
if (node.source == null) return
Copy link
Collaborator

@jfmengels jfmengels Oct 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious, is there ever a case where the source is not defined? ❓
We can leave it in case you're not sure, but I don't when this would happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, this is here because other rules included it (named, no-deprecated). It now seems likely that it's not needed here, will remove.


node.specifiers.forEach(function (im) {
if (im.type === 'ImportSpecifier' && im.imported.name === 'default') {
context.report({
node: im.local,
message: 'Use default import syntax to ' +
Copy link
Collaborator

@jfmengels jfmengels Oct 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use template literal syntax here by the way, makes it a bit simpler (and I think you're allowed to put this all on one line, but let me know if you aren't)

`Use default import syntax to import '${im.local.name}'`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thought! I'm used to an 80-char line length, this is definitely under the project's max of 99. :)

'import \'' + im.local.name + '\'.' })
}
})
},
}
},
}
39 changes: 39 additions & 0 deletions tests/src/rules/no-named-default.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { test, SYNTAX_CASES } from '../utils'
import { RuleTester } from 'eslint'

const ruleTester = new RuleTester()
, rule = require('rules/no-named-default')

ruleTester.run('no-named-default', rule, {
valid: [
test({code: 'import bar from "./bar";'}),
test({code: 'import bar, { foo } from "./bar";'}),

...SYNTAX_CASES,
],

invalid: [
test({
code: 'import { default } from "./bar";',
errors: [{
message: 'Use default import syntax to import \'default\'.',
type: 'Identifier',
}],
parser: 'babel-eslint',
}),
test({
code: 'import { default as bar } from "./bar";',
errors: [{
message: 'Use default import syntax to import \'bar\'.',
type: 'Identifier',
}],
}),
test({
code: 'import { foo, default as bar } from "./bar";',
errors: [{
message: 'Use default import syntax to import \'bar\'.',
type: 'Identifier',
}],
}),
],
})