Skip to content

Commit

Permalink
no-duplicates: Handle namespace imports when autofixing
Browse files Browse the repository at this point in the history
  • Loading branch information
lydell committed Mar 31, 2019
1 parent f47622c commit cdb7c37
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 2 deletions.
20 changes: 18 additions & 2 deletions src/rules/no-duplicates.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ function getFix(first, rest, sourceCode) {
return undefined
}

// If the first import is `import * as ns from './foo'` there's nothing we can do.
if (hasNamespace(first)) {
return undefined
}

const defaultImportNames = new Set(
[first, ...rest].map(getDefaultImportName).filter(Boolean)
)
Expand All @@ -47,11 +52,14 @@ function getFix(first, rest, sourceCode) {
}

// It's not obvious what the user wants to do with comments associated with
// duplicate imports, so skip imports with comments when autofixing.
// duplicate imports, so skip imports with comments when autofixing. Also skip
// `import * as ns from './foo'` imports, since they cannot be merged into
// another import.
const restWithoutComments = rest.filter(node => !(
hasCommentBefore(node, sourceCode) ||
hasCommentAfter(node, sourceCode) ||
hasCommentInsideNonSpecifiers(node, sourceCode)
hasCommentInsideNonSpecifiers(node, sourceCode) ||
hasNamespace(node)
))

const specifiers = restWithoutComments
Expand All @@ -75,6 +83,7 @@ function getFix(first, rest, sourceCode) {

const unnecessaryImports = restWithoutComments.filter(node =>
!hasSpecifiers(node) &&
!hasNamespace(node) &&
!specifiers.some(specifier => specifier.importNode === node)
)

Expand Down Expand Up @@ -156,6 +165,13 @@ function getDefaultImportName(node) {
return defaultSpecifier != null ? defaultSpecifier.local.name : undefined
}

// Checks whether `node` has a namespace import.
function hasNamespace(node) {
const specifiers = node.specifiers
.filter(specifier => specifier.type === 'ImportNamespaceSpecifier')
return specifiers.length > 0
}

// Checks whether `node` has any non-default specifiers.
function hasSpecifiers(node) {
const specifiers = node.specifiers
Expand Down
14 changes: 14 additions & 0 deletions tests/src/rules/no-duplicates.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,20 @@ ruleTester.run('no-duplicates', rule, {
errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
}),

test({
code: "import * as ns from './foo'; import {y} from './foo'",
// Autofix bail because first import is a namespace import.
output: "import * as ns from './foo'; import {y} from './foo'",
errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
}),

test({
code: "import {x} from './foo'; import * as ns from './foo'; import {y} from './foo'; import './foo'",
// Autofix could merge some imports, but not the namespace import.
output: "import {x,y} from './foo'; import * as ns from './foo'; ",
errors: ['\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.', '\'./foo\' imported multiple times.'],
}),

test({
code: `
import {x} from './foo'
Expand Down

0 comments on commit cdb7c37

Please sign in to comment.