feat(linter): show files involved in circular dependency (#7113)
This commit is contained in:
parent
4fb351617d
commit
c18a40b1f0
@ -1,4 +1,4 @@
|
|||||||
import type { ProjectGraph } from '@nrwl/devkit';
|
import type { FileData, ProjectGraph } from '@nrwl/devkit';
|
||||||
import {
|
import {
|
||||||
DependencyType,
|
DependencyType,
|
||||||
ProjectType,
|
ProjectType,
|
||||||
@ -6,7 +6,6 @@ import {
|
|||||||
import { TSESLint } from '@typescript-eslint/experimental-utils';
|
import { TSESLint } from '@typescript-eslint/experimental-utils';
|
||||||
import * as parser from '@typescript-eslint/parser';
|
import * as parser from '@typescript-eslint/parser';
|
||||||
import { vol } from 'memfs';
|
import { vol } from 'memfs';
|
||||||
import { extname } from 'path';
|
|
||||||
import enforceModuleBoundaries, {
|
import enforceModuleBoundaries, {
|
||||||
RULE_NAME as enforceModuleBoundariesRuleName,
|
RULE_NAME as enforceModuleBoundariesRuleName,
|
||||||
} from '../../src/rules/enforce-module-boundaries';
|
} from '../../src/rules/enforce-module-boundaries';
|
||||||
@ -936,7 +935,7 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
architect: {},
|
architect: {},
|
||||||
files: [createFile(`libs/mylib/src/main.ts`)],
|
files: [createFile(`libs/mylib/src/main.ts`, ['anotherlibName'])],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
anotherlibName: {
|
anotherlibName: {
|
||||||
@ -947,7 +946,7 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
architect: {},
|
architect: {},
|
||||||
files: [createFile(`libs/anotherlib/src/main.ts`)],
|
files: [createFile(`libs/anotherlib/src/main.ts`, ['mylibName'])],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
myappName: {
|
myappName: {
|
||||||
@ -974,8 +973,11 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
const message =
|
const message = `Circular dependency between "anotherlibName" and "mylibName" detected: anotherlibName -> mylibName -> anotherlibName
|
||||||
'Circular dependency between "anotherlibName" and "mylibName" detected: anotherlibName -> mylibName -> anotherlibName';
|
|
||||||
|
Circular file chain:
|
||||||
|
- libs/anotherlib/src/main.ts
|
||||||
|
- libs/mylib/src/main.ts`;
|
||||||
expect(failures.length).toEqual(2);
|
expect(failures.length).toEqual(2);
|
||||||
expect(failures[0].message).toEqual(message);
|
expect(failures[0].message).toEqual(message);
|
||||||
expect(failures[1].message).toEqual(message);
|
expect(failures[1].message).toEqual(message);
|
||||||
@ -999,7 +1001,9 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
architect: {},
|
architect: {},
|
||||||
files: [createFile(`libs/mylib/src/main.ts`)],
|
files: [
|
||||||
|
createFile(`libs/mylib/src/main.ts`, ['badcirclelibName']),
|
||||||
|
],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
anotherlibName: {
|
anotherlibName: {
|
||||||
@ -1010,7 +1014,10 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
architect: {},
|
architect: {},
|
||||||
files: [createFile(`libs/anotherlib/src/main.ts`)],
|
files: [
|
||||||
|
createFile(`libs/anotherlib/src/main.ts`, ['mylibName']),
|
||||||
|
createFile(`libs/anotherlib/src/index.ts`, ['mylibName']),
|
||||||
|
],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
badcirclelibName: {
|
badcirclelibName: {
|
||||||
@ -1021,7 +1028,9 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
architect: {},
|
architect: {},
|
||||||
files: [createFile(`libs/badcirclelib/src/main.ts`)],
|
files: [
|
||||||
|
createFile(`libs/badcirclelib/src/main.ts`, ['anotherlibName']),
|
||||||
|
],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
myappName: {
|
myappName: {
|
||||||
@ -1062,8 +1071,12 @@ describe('Enforce Module Boundaries (eslint)', () => {
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
const message =
|
const message = `Circular dependency between "mylibName" and "badcirclelibName" detected: mylibName -> badcirclelibName -> anotherlibName -> mylibName
|
||||||
'Circular dependency between "mylibName" and "badcirclelibName" detected: mylibName -> badcirclelibName -> anotherlibName -> mylibName';
|
|
||||||
|
Circular file chain:
|
||||||
|
- libs/mylib/src/main.ts
|
||||||
|
- libs/badcirclelib/src/main.ts
|
||||||
|
- [libs/anotherlib/src/main.ts,libs/anotherlib/src/index.ts]`;
|
||||||
expect(failures.length).toEqual(2);
|
expect(failures.length).toEqual(2);
|
||||||
expect(failures[0].message).toEqual(message);
|
expect(failures[0].message).toEqual(message);
|
||||||
expect(failures[1].message).toEqual(message);
|
expect(failures[1].message).toEqual(message);
|
||||||
@ -1525,8 +1538,8 @@ const baseConfig = {
|
|||||||
linter.defineParser('@typescript-eslint/parser', parser);
|
linter.defineParser('@typescript-eslint/parser', parser);
|
||||||
linter.defineRule(enforceModuleBoundariesRuleName, enforceModuleBoundaries);
|
linter.defineRule(enforceModuleBoundariesRuleName, enforceModuleBoundaries);
|
||||||
|
|
||||||
function createFile(f) {
|
function createFile(f: string, deps?: string[]): FileData {
|
||||||
return { file: f, hash: '' };
|
return { file: f, hash: '', ...(deps && { deps }) };
|
||||||
}
|
}
|
||||||
|
|
||||||
function runRule(
|
function runRule(
|
||||||
@ -27,7 +27,10 @@ import {
|
|||||||
} from '@nrwl/workspace/src/core/project-graph';
|
} from '@nrwl/workspace/src/core/project-graph';
|
||||||
import { readNxJson } from '@nrwl/workspace/src/core/file-utils';
|
import { readNxJson } from '@nrwl/workspace/src/core/file-utils';
|
||||||
import { TargetProjectLocator } from '@nrwl/workspace/src/core/target-project-locator';
|
import { TargetProjectLocator } from '@nrwl/workspace/src/core/target-project-locator';
|
||||||
import { checkCircularPath } from '@nrwl/workspace/src/utils/graph-utils';
|
import {
|
||||||
|
checkCircularPath,
|
||||||
|
findFilesInCircularPath,
|
||||||
|
} from '@nrwl/workspace/src/utils/graph-utils';
|
||||||
import { isRelativePath } from '@nrwl/workspace/src/utilities/fileutils';
|
import { isRelativePath } from '@nrwl/workspace/src/utilities/fileutils';
|
||||||
|
|
||||||
type Options = [
|
type Options = [
|
||||||
@ -83,7 +86,7 @@ export default createESLintRule<Options, MessageIds>({
|
|||||||
],
|
],
|
||||||
messages: {
|
messages: {
|
||||||
noRelativeOrAbsoluteImportsAcrossLibraries: `Libraries cannot be imported by a relative or absolute path, and must begin with a npm scope`,
|
noRelativeOrAbsoluteImportsAcrossLibraries: `Libraries cannot be imported by a relative or absolute path, and must begin with a npm scope`,
|
||||||
noCircularDependencies: `Circular dependency between "{{sourceProjectName}}" and "{{targetProjectName}}" detected: {{path}}`,
|
noCircularDependencies: `Circular dependency between "{{sourceProjectName}}" and "{{targetProjectName}}" detected: {{path}}\n\nCircular file chain:\n{{filePaths}}`,
|
||||||
noSelfCircularDependencies: `Projects should use relative imports to import from other files within the same project. Use "./path/to/file" instead of import from "{{imp}}"`,
|
noSelfCircularDependencies: `Projects should use relative imports to import from other files within the same project. Use "./path/to/file" instead of import from "{{imp}}"`,
|
||||||
noImportsOfApps: 'Imports of apps are forbidden',
|
noImportsOfApps: 'Imports of apps are forbidden',
|
||||||
noImportsOfE2e: 'Imports of e2e projects are forbidden',
|
noImportsOfE2e: 'Imports of e2e projects are forbidden',
|
||||||
@ -236,11 +239,13 @@ export default createESLintRule<Options, MessageIds>({
|
|||||||
// check constraints between libs and apps
|
// check constraints between libs and apps
|
||||||
// check for circular dependency
|
// check for circular dependency
|
||||||
const circularPath = checkCircularPath(
|
const circularPath = checkCircularPath(
|
||||||
projectGraph,
|
(global as any).projectGraph,
|
||||||
sourceProject,
|
sourceProject,
|
||||||
targetProject
|
targetProject
|
||||||
);
|
);
|
||||||
if (circularPath.length !== 0) {
|
if (circularPath.length !== 0) {
|
||||||
|
const circularFilePath = findFilesInCircularPath(circularPath);
|
||||||
|
|
||||||
context.report({
|
context.report({
|
||||||
node,
|
node,
|
||||||
messageId: 'noCircularDependencies',
|
messageId: 'noCircularDependencies',
|
||||||
@ -251,6 +256,14 @@ export default createESLintRule<Options, MessageIds>({
|
|||||||
(acc, v) => `${acc} -> ${v.name}`,
|
(acc, v) => `${acc} -> ${v.name}`,
|
||||||
sourceProject.name
|
sourceProject.name
|
||||||
),
|
),
|
||||||
|
filePaths: circularFilePath
|
||||||
|
.map((files) =>
|
||||||
|
files.length > 1 ? `[${files.join(',')}]` : files[0]
|
||||||
|
)
|
||||||
|
.reduce(
|
||||||
|
(acc, files) => `${acc}\n- ${files}`,
|
||||||
|
`- ${sourceFilePath}`
|
||||||
|
),
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
return;
|
return;
|
||||||
|
|||||||
@ -856,8 +856,8 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'libs/mylib',
|
root: 'libs/mylib',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`libs/mylib/src/main.ts`)],
|
files: [createFile(`libs/mylib/src/main.ts`, ['anotherlibName'])],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
anotherlibName: {
|
anotherlibName: {
|
||||||
@ -867,8 +867,8 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'libs/anotherlib',
|
root: 'libs/anotherlib',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`libs/anotherlib/src/main.ts`)],
|
files: [createFile(`libs/anotherlib/src/main.ts`, ['mylibName'])],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
myappName: {
|
myappName: {
|
||||||
@ -878,7 +878,7 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'apps/myapp',
|
root: 'apps/myapp',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`apps/myapp/src/index.ts`)],
|
files: [createFile(`apps/myapp/src/index.ts`)],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@ -895,7 +895,11 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
expect(failures[0].getFailure()).toEqual(
|
expect(failures[0].getFailure()).toEqual(
|
||||||
'Circular dependency between "anotherlibName" and "mylibName" detected: anotherlibName -> mylibName -> anotherlibName'
|
`Circular dependency between "anotherlibName" and "mylibName" detected: anotherlibName -> mylibName -> anotherlibName
|
||||||
|
|
||||||
|
Circular file chain:
|
||||||
|
- libs/anotherlib/src/main.ts
|
||||||
|
- libs/mylib/src/main.ts`
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
@ -913,8 +917,10 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'libs/mylib',
|
root: 'libs/mylib',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`libs/mylib/src/main.ts`)],
|
files: [
|
||||||
|
createFile(`libs/mylib/src/main.ts`, ['badcirclelibName']),
|
||||||
|
],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
anotherlibName: {
|
anotherlibName: {
|
||||||
@ -924,8 +930,11 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'libs/anotherlib',
|
root: 'libs/anotherlib',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`libs/anotherlib/src/main.ts`)],
|
files: [
|
||||||
|
createFile(`libs/anotherlib/src/main.ts`, ['mylibName']),
|
||||||
|
createFile(`libs/anotherlib/src/index.ts`, ['mylibName']),
|
||||||
|
],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
badcirclelibName: {
|
badcirclelibName: {
|
||||||
@ -935,8 +944,10 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'libs/badcirclelib',
|
root: 'libs/badcirclelib',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`libs/badcirclelib/src/main.ts`)],
|
files: [
|
||||||
|
createFile(`libs/badcirclelib/src/main.ts`, ['anotherlibName']),
|
||||||
|
],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
myappName: {
|
myappName: {
|
||||||
@ -946,7 +957,7 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
root: 'apps/myapp',
|
root: 'apps/myapp',
|
||||||
tags: [],
|
tags: [],
|
||||||
implicitDependencies: [],
|
implicitDependencies: [],
|
||||||
targets: {},
|
architect: {},
|
||||||
files: [createFile(`apps/myapp/index.ts`)],
|
files: [createFile(`apps/myapp/index.ts`)],
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
@ -977,7 +988,12 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
expect(failures[0].getFailure()).toEqual(
|
expect(failures[0].getFailure()).toEqual(
|
||||||
'Circular dependency between "mylibName" and "badcirclelibName" detected: mylibName -> badcirclelibName -> anotherlibName -> mylibName'
|
`Circular dependency between "mylibName" and "badcirclelibName" detected: mylibName -> badcirclelibName -> anotherlibName -> mylibName
|
||||||
|
|
||||||
|
Circular file chain:
|
||||||
|
- libs/mylib/src/main.ts
|
||||||
|
- libs/badcirclelib/src/main.ts
|
||||||
|
- [libs/anotherlib/src/main.ts,libs/anotherlib/src/index.ts]`
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
@ -1156,8 +1172,8 @@ describe('Enforce Module Boundaries (tslint)', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
function createFile(f): FileData {
|
function createFile(f: string, deps?: string[]): FileData {
|
||||||
return { file: f, hash: '' };
|
return { file: f, hash: '', ...(deps && { deps }) };
|
||||||
}
|
}
|
||||||
|
|
||||||
function runRule(
|
function runRule(
|
||||||
|
|||||||
@ -26,7 +26,10 @@ import {
|
|||||||
import { normalize } from 'path';
|
import { normalize } from 'path';
|
||||||
import { readNxJson } from '../core/file-utils';
|
import { readNxJson } from '../core/file-utils';
|
||||||
import { TargetProjectLocator } from '../core/target-project-locator';
|
import { TargetProjectLocator } from '../core/target-project-locator';
|
||||||
import { checkCircularPath } from '../utils/graph-utils';
|
import {
|
||||||
|
checkCircularPath,
|
||||||
|
findFilesInCircularPath,
|
||||||
|
} from '../utils/graph-utils';
|
||||||
import { isRelativePath } from '../utilities/fileutils';
|
import { isRelativePath } from '../utilities/fileutils';
|
||||||
|
|
||||||
export class Rule extends Lint.Rules.AbstractRule {
|
export class Rule extends Lint.Rules.AbstractRule {
|
||||||
@ -191,7 +194,13 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
|
|||||||
(acc, v) => `${acc} -> ${v.name}`,
|
(acc, v) => `${acc} -> ${v.name}`,
|
||||||
sourceProject.name
|
sourceProject.name
|
||||||
);
|
);
|
||||||
const error = `Circular dependency between "${sourceProject.name}" and "${targetProject.name}" detected: ${path}`;
|
|
||||||
|
const circularFilePath = findFilesInCircularPath(circularPath);
|
||||||
|
const filePaths = circularFilePath
|
||||||
|
.map((files) => (files.length > 1 ? `[${files.join(',')}]` : files[0]))
|
||||||
|
.reduce((acc, files) => `${acc}\n- ${files}`, `- ${filePath}`);
|
||||||
|
|
||||||
|
const error = `Circular dependency between "${sourceProject.name}" and "${targetProject.name}" detected: ${path}\n\nCircular file chain:\n${filePaths}`;
|
||||||
this.addFailureAt(node.getStart(), node.getWidth(), error);
|
this.addFailureAt(node.getStart(), node.getWidth(), error);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
@ -111,5 +111,24 @@ export function checkCircularPath(
|
|||||||
targetProject: ProjectGraphNode
|
targetProject: ProjectGraphNode
|
||||||
): Array<ProjectGraphNode> {
|
): Array<ProjectGraphNode> {
|
||||||
if (!graph.nodes[targetProject.name]) return [];
|
if (!graph.nodes[targetProject.name]) return [];
|
||||||
|
|
||||||
return getPath(graph, targetProject.name, sourceProject.name);
|
return getPath(graph, targetProject.name, sourceProject.name);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function findFilesInCircularPath(
|
||||||
|
circularPath: ProjectGraphNode[]
|
||||||
|
): Array<string[]> {
|
||||||
|
const filePathChain = [];
|
||||||
|
|
||||||
|
for (let i = 0; i < circularPath.length - 1; i++) {
|
||||||
|
const next = circularPath[i + 1].name;
|
||||||
|
const files = circularPath[i].data.files;
|
||||||
|
filePathChain.push(
|
||||||
|
Object.keys(files)
|
||||||
|
.filter((key) => files[key].deps?.indexOf(next) !== -1)
|
||||||
|
.map((key) => files[key].file)
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
return filePathChain;
|
||||||
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user