Skip to content

Commit

Permalink
[Fix] no-extraneous-dependencies: fix package name algorithm
Browse files Browse the repository at this point in the history
- resolve nested package.json problems (a/b/c import will check a, a/b and a/b/c)
- resolve renamed dependencies: checks the import name and the resolve package name

Fixes #2066. Fixes #2065. Fixes #2058. Fixes #2078.
  • Loading branch information
jeromeh authored and ljharb committed May 25, 2021
1 parent 81b9d24 commit da8d584
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 23 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`no-cycle`]: fix false negative when file imports a type after importing a value in Flow ([#2083], thanks [@cherryblossom000])
- [`order`]: restore default behavior unless `type` is in groups ([#2087], thanks [@grit96])
- [`no-import-module-exports`]: Don't crash if packages have no entrypoint ([#2099], thanks [@eps1lon])
- [`no-extraneous-dependencies`]: fix package name algorithm ([#2097], thanks [@paztis])

### Changed
- [Docs] Add `no-relative-packages` to list of to the list of rules ([#2075], thanks [@arvigeus])
Expand Down Expand Up @@ -796,6 +797,7 @@ for info on changes for earlier releases.
[`memo-parser`]: ./memo-parser/README.md

[#2099]: https://github.com/benmosher/eslint-plugin-import/pull/2099
[#2097]: https://github.com/benmosher/eslint-plugin-import/pull/2097
[#2090]: https://github.com/benmosher/eslint-plugin-import/pull/2090
[#2087]: https://github.com/benmosher/eslint-plugin-import/pull/2087
[#2083]: https://github.com/benmosher/eslint-plugin-import/pull/2083
Expand Down
84 changes: 63 additions & 21 deletions src/rules/no-extraneous-dependencies.js
Expand Up @@ -126,6 +126,36 @@ function getModuleRealName(resolved) {
return getFilePackageName(resolved);
}

function checkDependencyDeclaration(deps, packageName) {
// in case of sub package.json inside a module
// check the dependencies on all hierarchy
const packageHierarchy = [];
const packageNameParts = packageName.split('/');
packageNameParts.forEach((namePart, index) => {
if (!namePart.startsWith('@')) {
const ancestor = packageNameParts.slice(0, index + 1).join('/');
packageHierarchy.push(ancestor);
}
});

return packageHierarchy.reduce((result, ancestorName) => {
return {
isInDeps: result.isInDeps || deps.dependencies[ancestorName] !== undefined,
isInDevDeps: result.isInDevDeps || deps.devDependencies[ancestorName] !== undefined,
isInOptDeps: result.isInOptDeps || deps.optionalDependencies[ancestorName] !== undefined,
isInPeerDeps: result.isInPeerDeps || deps.peerDependencies[ancestorName] !== undefined,
isInBundledDeps:
result.isInBundledDeps || deps.bundledDependencies.indexOf(ancestorName) !== -1,
};
}, {
isInDeps: false,
isInDevDeps: false,
isInOptDeps: false,
isInPeerDeps: false,
isInBundledDeps: false,
});
}

function reportIfMissing(context, deps, depsOptions, node, name) {
// Do not report when importing types
if (node.importKind === 'type' || (node.parent && node.parent.importKind === 'type') || node.importKind === 'typeof') {
Expand All @@ -139,37 +169,49 @@ function reportIfMissing(context, deps, depsOptions, node, name) {
const resolved = resolve(name, context);
if (!resolved) { return; }

// get the real name from the resolved package.json
// if not aliased imports (alias/react for example) will not be correctly interpreted
// fallback on original name in case no package.json found
const packageName = getModuleRealName(resolved) || getModuleOriginalName(name);

const isInDeps = deps.dependencies[packageName] !== undefined;
const isInDevDeps = deps.devDependencies[packageName] !== undefined;
const isInOptDeps = deps.optionalDependencies[packageName] !== undefined;
const isInPeerDeps = deps.peerDependencies[packageName] !== undefined;
const isInBundledDeps = deps.bundledDependencies.indexOf(packageName) !== -1;

if (isInDeps ||
(depsOptions.allowDevDeps && isInDevDeps) ||
(depsOptions.allowPeerDeps && isInPeerDeps) ||
(depsOptions.allowOptDeps && isInOptDeps) ||
(depsOptions.allowBundledDeps && isInBundledDeps)
const importPackageName = getModuleOriginalName(name);
const importPackageNameDeclaration = checkDependencyDeclaration(deps, importPackageName);

if (importPackageNameDeclaration.isInDeps ||
(depsOptions.allowDevDeps && importPackageNameDeclaration.isInDevDeps) ||
(depsOptions.allowPeerDeps && importPackageNameDeclaration.isInPeerDeps) ||
(depsOptions.allowOptDeps && importPackageNameDeclaration.isInOptDeps) ||
(depsOptions.allowBundledDeps && importPackageNameDeclaration.isInBundledDeps)
) {
return;
}

// test the real name from the resolved package.json
// if not aliased imports (alias/react for example), importPackageName can be misinterpreted
const realPackageName = getModuleRealName(resolved);
const realPackageNameDeclaration = checkDependencyDeclaration(deps, realPackageName);

if (realPackageNameDeclaration.isInDeps ||
(depsOptions.allowDevDeps && realPackageNameDeclaration.isInDevDeps) ||
(depsOptions.allowPeerDeps && realPackageNameDeclaration.isInPeerDeps) ||
(depsOptions.allowOptDeps && realPackageNameDeclaration.isInOptDeps) ||
(depsOptions.allowBundledDeps && realPackageNameDeclaration.isInBundledDeps)
) {
return;
}

if (isInDevDeps && !depsOptions.allowDevDeps) {
context.report(node, devDepErrorMessage(packageName));
if ((
importPackageNameDeclaration.isInDevDeps ||
realPackageNameDeclaration.isInDevDeps
) && !depsOptions.allowDevDeps) {
context.report(node, devDepErrorMessage(realPackageName));
return;
}

if (isInOptDeps && !depsOptions.allowOptDeps) {
context.report(node, optDepErrorMessage(packageName));
if ((
importPackageNameDeclaration.isInOptDeps ||
realPackageNameDeclaration.isInOptDeps
) && !depsOptions.allowOptDeps) {
context.report(node, optDepErrorMessage(realPackageName));
return;
}

context.report(node, missingErrorMessage(packageName));
context.report(node, missingErrorMessage(realPackageName));
}

function testConfig(config, filename) {
Expand Down
1 change: 1 addition & 0 deletions tests/files/node_modules/rxjs/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/files/node_modules/rxjs/operators/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions tests/files/node_modules/rxjs/operators/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions tests/files/node_modules/rxjs/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion tests/files/package.json
Expand Up @@ -11,7 +11,8 @@
"@org/package": "^1.0.0",
"jquery": "^3.1.0",
"lodash.cond": "^4.3.0",
"pkg-up": "^1.0.0"
"pkg-up": "^1.0.0",
"rxjs": "^1.0.0"
},
"optionalDependencies": {
"lodash.isarray": "^4.0.0"
Expand Down
6 changes: 5 additions & 1 deletion tests/src/rules/no-extraneous-dependencies.js
Expand Up @@ -88,7 +88,7 @@ ruleTester.run('no-extraneous-dependencies', rule, {
}),
test({
code: `
// @flow
// @flow
import typeof TypeScriptModule from 'typescript';
`,
options: [{ packageDir: packageDirWithFlowTyped }],
Expand Down Expand Up @@ -150,6 +150,10 @@ ruleTester.run('no-extraneous-dependencies', rule, {
code: 'import "@generated/bar/and/sub/path"',
settings: { 'import/core-modules': ['@generated/bar'] },
}),
// check if "rxjs" dependency declaration fix the "rxjs/operators subpackage
test({
code: 'import "rxjs/operators"',
}),
],
invalid: [
test({
Expand Down

0 comments on commit da8d584

Please sign in to comment.