Skip to content

Commit

Permalink
Merge 20c373c into a332f20
Browse files Browse the repository at this point in the history
  • Loading branch information
ljharb committed May 21, 2021
2 parents a332f20 + 20c373c commit 9fbdb66
Show file tree
Hide file tree
Showing 14 changed files with 455 additions and 227 deletions.
359 changes: 186 additions & 173 deletions CHANGELOG.md

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -27,6 +27,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Forbid a module from importing a module with a dependency path back to itself ([`no-cycle`])
* Prevent unnecessary path segments in import and require statements ([`no-useless-path-segments`])
* Forbid importing modules from parent directories ([`no-relative-parent-imports`])
* Prevent importing packages through relative paths ([`no-relative-packages`])

[`no-unresolved`]: ./docs/rules/no-unresolved.md
[`named`]: ./docs/rules/named.md
Expand All @@ -41,6 +42,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
[`no-cycle`]: ./docs/rules/no-cycle.md
[`no-useless-path-segments`]: ./docs/rules/no-useless-path-segments.md
[`no-relative-parent-imports`]: ./docs/rules/no-relative-parent-imports.md
[`no-relative-packages`]: ./docs/rules/no-relative-packages.md

### Helpful warnings

Expand Down
3 changes: 1 addition & 2 deletions package.json
@@ -1,6 +1,6 @@
{
"name": "eslint-plugin-import",
"version": "2.23.2",
"version": "2.23.3",
"description": "Import with sanity.",
"engines": {
"node": ">=4"
Expand Down Expand Up @@ -101,7 +101,6 @@
"dependencies": {
"array-includes": "^3.1.3",
"array.prototype.flat": "^1.2.4",
"contains-path": "^1.0.0",
"debug": "^2.6.9",
"doctrine": "^2.1.0",
"eslint-import-resolver-node": "^0.3.4",
Expand Down
11 changes: 6 additions & 5 deletions src/ExportMap.js
Expand Up @@ -495,7 +495,9 @@ ExportMap.parse = function (path, content, context) {
if (n.type === 'ImportDeclaration') {
// import type { Foo } (TS and Flow)
const declarationIsType = n.importKind === 'type';
let isOnlyImportingTypes = declarationIsType;
// import './foo' or import {} from './foo' (both 0 specifiers) is a side effect and
// shouldn't be considered to be just importing types
let specifiersOnlyImportingTypes = n.specifiers.length;
const importedSpecifiers = new Set();
n.specifiers.forEach(specifier => {
if (supportedImportTypes.has(specifier.type)) {
Expand All @@ -506,11 +508,10 @@ ExportMap.parse = function (path, content, context) {
}

// import { type Foo } (Flow)
if (!declarationIsType) {
isOnlyImportingTypes = specifier.importKind === 'type';
}
specifiersOnlyImportingTypes =
specifiersOnlyImportingTypes && specifier.importKind === 'type';
});
captureDependency(n, isOnlyImportingTypes, importedSpecifiers);
captureDependency(n, declarationIsType || specifiersOnlyImportingTypes, importedSpecifiers);

const ns = n.specifiers.find(s => s.type === 'ImportNamespaceSpecifier');
if (ns) {
Expand Down
26 changes: 17 additions & 9 deletions src/rules/no-cycle.js
Expand Up @@ -84,18 +84,26 @@ module.exports = {
traversed.add(m.path);

for (const [path, { getter, declarations }] of m.imports) {
if (path === myPath) return true;
if (traversed.has(path)) continue;
for (const { source, isOnlyImportingTypes } of declarations) {
if (ignoreModule(source.value)) continue;
const toTraverse = [...declarations].filter(({ source, isOnlyImportingTypes }) =>
!ignoreModule(source.value) &&
// Ignore only type imports
if (isOnlyImportingTypes) continue;
!isOnlyImportingTypes
);
/*
Only report as a cycle if there are any import declarations that are considered by
the rule. For example:
if (route.length + 1 < maxDepth) {
untraversed.push({
mget: getter,
route: route.concat(source),
});
a.ts:
import { foo } from './b' // should not be reported as a cycle
b.ts:
import type { Bar } from './a'
*/
if (path === myPath && toTraverse.length > 0) return true;
if (route.length + 1 < maxDepth) {
for (const { source } of toTraverse) {
untraversed.push({ mget: getter, route: route.concat(source) });
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/rules/no-restricted-paths.js
@@ -1,11 +1,15 @@
import containsPath from 'contains-path';
import path from 'path';

import resolve from 'eslint-module-utils/resolve';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
import docsUrl from '../docsUrl';
import importType from '../core/importType';

const containsPath = (filepath, target) => {
const relative = path.relative(target, filepath);
return relative === '' || !relative.startsWith('..');
};

module.exports = {
meta: {
type: 'problem',
Expand Down
10 changes: 7 additions & 3 deletions src/rules/order.js
Expand Up @@ -313,7 +313,7 @@ function computeRank(context, ranks, importEntry, excludedImportTypes) {
let rank;
if (importEntry.type === 'import:object') {
impType = 'object';
} else if (importEntry.node.importKind === 'type') {
} else if (importEntry.node.importKind === 'type' && ranks.omittedTypes.indexOf('type') === -1) {
impType = 'type';
} else {
impType = importType(importEntry.value, context);
Expand Down Expand Up @@ -382,10 +382,12 @@ function convertGroupsToRanks(groups) {
return rankObject[type] === undefined;
});

return omittedTypes.reduce(function(res, type) {
const ranks = omittedTypes.reduce(function(res, type) {
res[type] = groups.length;
return res;
}, rankObject);

return { groups: ranks, omittedTypes };
}

function convertPathGroupsForRanks(pathGroups) {
Expand Down Expand Up @@ -590,8 +592,10 @@ module.exports = {

try {
const { pathGroups, maxPosition } = convertPathGroupsForRanks(options.pathGroups || []);
const { groups, omittedTypes } = convertGroupsToRanks(options.groups || defaultGroups);
ranks = {
groups: convertGroupsToRanks(options.groups || defaultGroups),
groups,
omittedTypes,
pathGroups,
maxPosition,
};
Expand Down
@@ -0,0 +1,3 @@
// @flow

import { type FooType, type BarType } from './depth-zero';
3 changes: 3 additions & 0 deletions tests/files/cycles/flow-types-only-importing-type.js
@@ -0,0 +1,3 @@
// @flow

import type { FooType } from './depth-zero';
3 changes: 3 additions & 0 deletions tests/files/cycles/flow-types-some-type-imports.js
@@ -0,0 +1,3 @@
// @flow

import { foo, type BarType } from './depth-zero'
Empty file.
13 changes: 13 additions & 0 deletions tests/src/rules/no-cycle.js
Expand Up @@ -73,12 +73,25 @@ ruleTester.run('no-cycle', rule, {
code: 'import { bar } from "./flow-types"',
parser: require.resolve('babel-eslint'),
}),
test({
code: 'import { bar } from "./flow-types-only-importing-type"',
parser: require.resolve('babel-eslint'),
}),
test({
code: 'import { bar } from "./flow-types-only-importing-multiple-types"',
parser: require.resolve('babel-eslint'),
}),
],
invalid: [
test({
code: 'import { foo } from "./depth-one"',
errors: [error(`Dependency cycle detected.`)],
}),
test({
code: 'import { bar } from "./flow-types-some-type-imports"',
parser: require.resolve('babel-eslint'),
errors: [error(`Dependency cycle detected.`)],
}),
test({
code: 'import { foo } from "cycles/external/depth-one"',
errors: [error(`Dependency cycle detected.`)],
Expand Down
11 changes: 11 additions & 0 deletions tests/src/rules/no-restricted-paths.js
Expand Up @@ -50,6 +50,17 @@ ruleTester.run('no-restricted-paths', rule, {
} ],
} ],
}),
test({
code: 'import a from "../one/a.js"',
filename: testFilePath('./restricted-paths/server/two-new/a.js'),
options: [ {
zones: [ {
target: './tests/files/restricted-paths/server/two',
from: './tests/files/restricted-paths/server',
except: [],
} ],
} ],
}),


// irrelevant function calls
Expand Down

0 comments on commit 9fbdb66

Please sign in to comment.