Skip to content

Commit

Permalink
Fix: ignore unmergable imports when checking no-duplicate-imports (fixes
Browse files Browse the repository at this point in the history 
 eslint#13180) & Fix: Ignore re-export all in no-duplicate-imports (fixes eslint#12760)
  • Loading branch information
@boutahlilsoufiane
boutahlilsoufiane committed Mar 21, 2021
1 parent 8984c91 commit 5b9ef18
Show file tree
Hide file tree
Showing 3 changed files with 396 additions and 73 deletions.
23 changes: 22 additions & 1 deletion docs/rules/no-duplicate-imports.md
View file
Expand Up @@ -12,7 +12,9 @@ import { find } from 'module';

## Rule Details

This rule requires that all imports from a single module exists in a single `import` statement.
An import that can be merged with another is a duplicate of that other.

This rule requires that all imports from a single module that can be merged exists in a single `import` statement.

Example of **incorrect** code for this rule:

Expand All @@ -33,6 +35,14 @@ import { merge, find } from 'module';
import something from 'another-module';
```

Example of **correct** code for this rule:

```js
// not mergable, as they would require new nodes to be created.
import { merge } from 'module';
import * as something from 'module';
```

## Options

This rule takes one optional argument, an object with a single key, `includeExports` which is a `boolean`. It defaults to `false`.
Expand All @@ -58,3 +68,14 @@ import { merge, find } from 'module';

export { find };
```

There is a special case even the export is duplicate we ignore it, because it can't be merged with another import/export from the same source, it's when we have export with type export *.

Example of **correct** code for this rule with the `{ "includeExports": true }` option:

```js

import { merge, find } from 'module';

export * from 'module';
```
235 changes: 200 additions & 35 deletions lib/rules/no-duplicate-imports.js
View file
Expand Up @@ -8,6 +8,42 @@
// Rule Definition
//------------------------------------------------------------------------------

const EXPORT_ALL_DECLARATION = 'ExportAllDeclaration'
const IMPORT_NAME_SPACE_SPECIFIER = 'ImportNamespaceSpecifier';
const IMPORT_SPECIFIER = 'ImportSpecifier';
const CONTRADICTORY_IMPORT_TYPES = [
IMPORT_NAME_SPACE_SPECIFIER,
IMPORT_SPECIFIER,
];

/**
* Return the type of import.
* @param {ASTNode} node A node to get.
* @returns {string} the type of the import.
*/
function getImportType(node) {
if (
node &&
node.specifiers &&
node.specifiers[0] &&
node.specifiers[0].type
) {
const index = node.specifiers.findIndex((specifier) =>
CONTRADICTORY_IMPORT_TYPES.includes(
specifier.type
)
);
if (index > -1) {
return node.specifiers[index].type;
} else {
return node.specifiers[0].type;
}
} else if (node && node.type) {
return node.type;
}
return "";
}

/**
* Returns the name of the module imported or re-exported.
* @param {ASTNode} node A node to get.
Expand All @@ -21,24 +57,69 @@ function getValue(node) {
return "";
}


/**
* Returns a boolean if we should report contradictory import type.
* @param {string[]} importTypes An array contain import types of a module.
* @param {string} importType An contradictory import type to check if we should report it or not.
* @returns {boolean} true if the contradictory import type should be reported.
*/
function shouldReportContradictoryImportType(importTypes, importType) {
if (importTypes.indexOf(importType) > -1) {
return true;
} else {
return (
importTypes.findIndex((importTypeItem) => {
return CONTRADICTORY_IMPORT_TYPES.includes(importTypeItem);
}) === -1
);
}
}

/**
* Checks if the name of the import or export exists in the given array, and reports if so.
* @param {RuleContext} context The ESLint rule context object.
* @param {ASTNode} node A node to get.
* @param {string} value The name of the imported or exported module.
* @param {string[]} array The array containing other imports or exports in the file.
* @param {string} messageId A messageId to be reported after the name of the module
* @param {{}} modulesWithImportTypes The object containing the name of unique modules with their first import type [specificImport, nameSpaceImport].
* @param {string} type the name of import type.
* @param {string[]} ExportAllDeclarationsInFile The array containing ExportAllDeclarations in the file.
*
* @returns {void} No return value
*/
function checkAndReport(context, node, value, array, messageId) {
if (array.indexOf(value) !== -1) {
function checkAndReport(
context,
node,
value,
array,
messageId,
modulesWithImportTypes,
type,
ExportAllDeclarationsInFile
) {
let isDuplicate = false;
if (type === EXPORT_ALL_DECLARATION) {
if (ExportAllDeclarationsInFile.indexOf(value) !== -1) {
isDuplicate = true;
}
} else if (array.indexOf(value) !== -1) {
isDuplicate = true;
if (CONTRADICTORY_IMPORT_TYPES.includes(type)) {
isDuplicate = shouldReportContradictoryImportType(
modulesWithImportTypes[value],
type
);
}
}
if (isDuplicate) {
context.report({
node,
messageId,
data: {
module: value
}
module: value,
},
});
}
}
Expand All @@ -54,21 +135,55 @@ function checkAndReport(context, node, value, array, messageId) {
* @param {boolean} includeExports Whether or not to check for exports in addition to imports.
* @param {string[]} importsInFile The array containing other imports in the file.
* @param {string[]} exportsInFile The array containing other exports in the file.
* @param {{}} modulesWithImportTypes The object containing the name of unique modules with their first import type [specificImport, nameSpaceImport].
* @param {string[]} ExportAllDeclarationsInFile The array containing ExportAllDeclarations in the file.
*
* @returns {nodeCallback} A function passed to ESLint to handle the statement.
*/
function handleImports(context, includeExports, importsInFile, exportsInFile) {
return function(node) {
function handleImports(
context,
includeExports,
importsInFile,
exportsInFile,
modulesWithImportTypes,
ExportAllDeclarationsInFile
) {
return function (node) {
const value = getValue(node);
const type = getImportType(node);

if (value) {
checkAndReport(context, node, value, importsInFile, "import");
checkAndReport(
context,
node,
value,
importsInFile,
"import",
modulesWithImportTypes,
type,
ExportAllDeclarationsInFile
);

if (includeExports) {
checkAndReport(context, node, value, exportsInFile, "importAs");
checkAndReport(
context,
node,
value,
exportsInFile,
"importAs",
modulesWithImportTypes,
type,
ExportAllDeclarationsInFile
);
}

importsInFile.push(value);
if (modulesWithImportTypes[value]) {
modulesWithImportTypes[value] = modulesWithImportTypes[
value
].concat(type);
} else {
modulesWithImportTypes[value] = [type];
}
}
};
}
Expand All @@ -78,18 +193,47 @@ function handleImports(context, includeExports, importsInFile, exportsInFile) {
* @param {RuleContext} context The ESLint rule context object.
* @param {string[]} importsInFile The array containing other imports in the file.
* @param {string[]} exportsInFile The array containing other exports in the file.
* @param {{}} modulesWithImportTypes The object containing the name of unique modules with their first import type [specificImport, nameSpaceImport].
* @param {string[]} ExportAllDeclarationsInFile The array containing ExportAllDeclarations in the file.
*
* @returns {nodeCallback} A function passed to ESLint to handle the statement.
*/
function handleExports(context, importsInFile, exportsInFile) {
return function(node) {
function handleExports(
context,
importsInFile,
exportsInFile,
modulesWithImportTypes,
ExportAllDeclarationsInFile
) {
return function (node) {
const value = getValue(node);

const type = getImportType(node);
if (value) {
checkAndReport(context, node, value, exportsInFile, "export");
checkAndReport(context, node, value, importsInFile, "exportAs");

exportsInFile.push(value);
checkAndReport(
context,
node,
value,
exportsInFile,
"export",
modulesWithImportTypes,
type,
ExportAllDeclarationsInFile
);
if (type === EXPORT_ALL_DECLARATION) {
ExportAllDeclarationsInFile.push(value);
} else {
checkAndReport(
context,
node,
value,
importsInFile,
"exportAs",
modulesWithImportTypes,
type,
ExportAllDeclarationsInFile
);
exportsInFile.push(value);
}
}
};
}
Expand All @@ -102,41 +246,62 @@ module.exports = {
description: "disallow duplicate module imports",
category: "ECMAScript 6",
recommended: false,
url: "https://eslint.org/docs/rules/no-duplicate-imports"
url: "https://eslint.org/docs/rules/no-duplicate-imports",
},

schema: [{
type: "object",
properties: {
includeExports: {
type: "boolean",
default: false
}
schema: [
{
type: "object",
properties: {
includeExports: {
type: "boolean",
default: false,
},
},
additionalProperties: false,
},
additionalProperties: false
}],
],
messages: {
import: "'{{module}}' import is duplicated.",
importAs: "'{{module}}' import is duplicated as export.",
export: "'{{module}}' export is duplicated.",
exportAs: "'{{module}}' export is duplicated as import."
}
exportAs: "'{{module}}' export is duplicated as import.",
},
},

create(context) {
const includeExports = (context.options[0] || {}).includeExports,
importsInFile = [],
exportsInFile = [];

modulesWithImportTypes = {},
exportsInFile = [],
ExportAllDeclarationsInFile = [];
const handlers = {
ImportDeclaration: handleImports(context, includeExports, importsInFile, exportsInFile)
ImportDeclaration: handleImports(
context,
includeExports,
importsInFile,
exportsInFile,
modulesWithImportTypes,
ExportAllDeclarationsInFile
),
};

if (includeExports) {
handlers.ExportNamedDeclaration = handleExports(context, importsInFile, exportsInFile);
handlers.ExportAllDeclaration = handleExports(context, importsInFile, exportsInFile);
handlers.ExportNamedDeclaration = handleExports(
context,
importsInFile,
exportsInFile,
modulesWithImportTypes,
ExportAllDeclarationsInFile
);
handlers.ExportAllDeclaration = handleExports(
context,
importsInFile,
exportsInFile,
modulesWithImportTypes,
ExportAllDeclarationsInFile
);
}

return handlers;
}
},
};

0 comments on commit 5b9ef18

Please sign in to comment.