Skip to content

Commit

Permalink
feat(linter): use graph utils to calculate dependencies with tags
Browse files Browse the repository at this point in the history
  • Loading branch information
meeroslav committed Mar 7, 2022
1 parent 8c93ae9 commit 2e6eda6
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 54 deletions.
Expand Up @@ -36,6 +36,7 @@ const tsconfig = {
'@mycompany/mylibName': ['libs/mylibName/src/index.ts'],
'@mycompany/public': ['libs/public/src/index.ts'],
'@mycompany/dependsOnPrivate': ['libs/dependsOnPrivate/src/index.ts'],
'@mycompany/dependsOnPrivate2': ['libs/dependsOnPrivate2/src/index.ts'],
'@mycompany/private': ['libs/private/src/index.ts'],
'@mycompany/anotherlibName': ['libs/anotherlibName/src/index.ts'],
'@mycompany/badcirclelib': ['libs/badcirclelib/src/index.ts'],
Expand Down Expand Up @@ -83,6 +84,7 @@ const fileSys = {
'./libs/nonBuildableLib/src/main.ts': '',
'./libs/public/src/index.ts': '',
'./libs/dependsOnPrivate/src/index.ts': '',
'./libs/dependsOnPrivate2/src/index.ts': '',
'./libs/private/src/index.ts': '',
'./tsconfig.base.json': JSON.stringify(tsconfig),
'./package.json': JSON.stringify(packageJson),
Expand Down Expand Up @@ -279,6 +281,21 @@ describe('Enforce Module Boundaries (eslint)', () => {
],
},
},
dependsOnPrivateName2: {
name: 'dependsOnPrivateName2',
type: ProjectType.lib,
data: {
root: 'libs/dependsOnPrivate2',
tags: [],
implicitDependencies: [],
architect: {},
files: [
createFile(`libs/dependsOnPrivate2/src/index.ts`, [
'privateName',
]),
],
},
},
privateName: {
name: 'privateName',
type: ProjectType.lib,
Expand Down Expand Up @@ -348,6 +365,13 @@ describe('Enforce Module Boundaries (eslint)', () => {
type: DependencyType.static,
},
],
dependsOnPrivateName2: [
{
source: 'dependsOnPrivateName2',
target: 'privateName',
type: DependencyType.static,
},
],
},
};

Expand Down Expand Up @@ -509,7 +533,19 @@ describe('Enforce Module Boundaries (eslint)', () => {
import '@mycompany/private';
import('@mycompany/private');
`,
graph
{
...graph,
dependencies: {
...graph.dependencies,
publicName: [
{
source: 'publicName',
target: 'privateName',
type: DependencyType.static,
},
],
},
}
);

const message = `A project tagged with "public" can not depend on libs tagged with "private"
Expand All @@ -527,19 +563,45 @@ Violation detected in:
`${process.cwd()}/proj/libs/public/src/index.ts`,
`
import '@mycompany/dependsOnPrivate';
import('@mycompany/dependsOnPrivate');
import '@mycompany/dependsOnPrivate2';
import '@mycompany/private';
`,
graph
{
...graph,
dependencies: {
...graph.dependencies,
publicName: [
{
source: 'publicName',
target: 'dependsOnPrivateName',
type: DependencyType.static,
},
{
source: 'publicName',
target: 'dependsOnPrivateName2',
type: DependencyType.static,
},
{
source: 'publicName',
target: 'privateName',
type: DependencyType.static,
},
],
},
}
);

expect(failures.length).toEqual(2);
expect(failures.length).toEqual(3);
// TODO: Add project dependency path to message
const message = `A project tagged with "public" can not depend on libs tagged with "private"
const message = (
prefix
) => `A project tagged with "public" can not depend on libs tagged with "private"
Violation detected in:
- dependsOnPrivateName -> privateName`;
expect(failures[0].message).toEqual(message);
expect(failures[1].message).toEqual(message);
- ${prefix}privateName`;
expect(failures[0].message).toEqual(message('dependsOnPrivateName -> '));
expect(failures[1].message).toEqual(message('dependsOnPrivateName2 -> '));
expect(failures[2].message).toEqual(message(''));
});

it('should error when the source library is untagged', () => {
Expand Down
11 changes: 8 additions & 3 deletions packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts
Expand Up @@ -427,19 +427,24 @@ export default createESLintRule<Options, MessageIds>({
constraint.notDependOnLibsWithTags &&
constraint.notDependOnLibsWithTags.length
) {
const projects = findDependenciesWithTags(
const projectPaths = findDependenciesWithTags(
targetProject,
constraint.notDependOnLibsWithTags,
projectGraph
);
if (projects.length > 0) {
if (projectPaths.length > 0) {
context.report({
node,
messageId: 'notTagsConstraintViolation',
data: {
sourceTag: constraint.sourceTag,
tags: stringifyTags(constraint.notDependOnLibsWithTags),
projects: `- ${projects.map((p) => p.name).join(' -> ')}`,
projects: projectPaths
.map(
(projectPath) =>
`- ${projectPath.map((p) => p.name).join(' -> ')}`
)
.join('\n'),
},
});
return;
Expand Down
13 changes: 8 additions & 5 deletions packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts
Expand Up @@ -289,19 +289,22 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
constraint.notDependOnLibsWithTags &&
constraint.notDependOnLibsWithTags.length
) {
const projects = findDependenciesWithTags(
const projectPaths = findDependenciesWithTags(
targetProject,
constraint.notDependOnLibsWithTags,
this.projectGraph
);
if (projects.length > 0) {
if (projectPaths.length > 0) {
const error = `A project tagged with "${
constraint.sourceTag
}" can not depend on libs tagged with ${stringifyTags(
constraint.notDependOnLibsWithTags
)}\n\nViolation detected in:\n${projects
.map((p) => p.name)
.join(' -> ')}`;
)}\n\nViolation detected in:\n${projectPaths
.map(
(projectPath) =>
`- ${projectPath.map((p) => p.name).join(' -> ')}`
)
.join('\n')}`;
this.addFailureAt(node.getStart(), node.getWidth(), error);
return;
}
Expand Down
31 changes: 24 additions & 7 deletions packages/workspace/src/utils/graph-utils.ts
Expand Up @@ -29,9 +29,9 @@ function buildMatrix(graph: ProjectGraph) {
};
}, {});

nodes.forEach((v, i) => {
adjList[nodes[i]] = [];
matrix[nodes[i]] = { ...initMatrixValues };
nodes.forEach((v) => {
adjList[v] = [];
matrix[v] = { ...initMatrixValues };
});

for (let proj in dependencies) {
Expand All @@ -42,7 +42,7 @@ function buildMatrix(graph: ProjectGraph) {
}
}

const traverse = (s, v) => {
const traverse = (s: string, v: string) => {
matrix[s][v] = true;

for (let adj of adjList[v]) {
Expand All @@ -52,8 +52,8 @@ function buildMatrix(graph: ProjectGraph) {
}
};

nodes.forEach((v, i) => {
traverse(nodes[i], nodes[i]);
nodes.forEach((v) => {
traverse(v, v);
});

return {
Expand Down Expand Up @@ -106,11 +106,28 @@ export function getPath(
}
}

export function pathExists(
graph: ProjectGraph,
sourceProjectName: string,
targetProjectName: string
): boolean {
if (sourceProjectName === targetProjectName) return true;

if (reach.graph !== graph) {
const { matrix, adjList } = buildMatrix(graph);
reach.graph = graph;
reach.matrix = matrix;
reach.adjList = adjList;
}

return reach.matrix[sourceProjectName][targetProjectName];
}

export function checkCircularPath(
graph: ProjectGraph,
sourceProject: ProjectGraphProjectNode,
targetProject: ProjectGraphProjectNode
): Array<ProjectGraphProjectNode> {
): ProjectGraphProjectNode[] {
if (!graph.nodes[targetProject.name]) return [];

return getPath(graph, targetProject.name, sourceProject.name);
Expand Down
48 changes: 17 additions & 31 deletions packages/workspace/src/utils/runtime-lint-utils.ts
Expand Up @@ -12,6 +12,7 @@ import {
import { TargetProjectLocator } from '../core/target-project-locator';
import { join } from 'path';
import { appRootPath } from './app-root';
import { getPath, pathExists } from './graph-utils';

export type MappedProjectGraphNode<T = any> = ProjectGraphProjectNode<T> & {
data: {
Expand Down Expand Up @@ -48,39 +49,24 @@ export function hasNoneOfTheseTags(
* @returns
*/
export function findDependenciesWithTags(
proj: ProjectGraphProjectNode,
targetProject: ProjectGraphProjectNode,
tags: string[],
graph: ProjectGraph,
foundPath: Array<ProjectGraphProjectNode> = [],
visited: string[] = []
): Array<ProjectGraphProjectNode> {
if (!hasNoneOfTheseTags(proj, tags)) {
return [...foundPath, proj];
}
if (!graph.dependencies[proj.name]) {
return foundPath;
}
graph: ProjectGraph
): ProjectGraphProjectNode[][] {
// find all reachable projects that have one of the tags and
// are reacheable from the targetProject (including self)
const allReachableProjects = Object.keys(graph.nodes).filter(
(projectName) =>
!hasNoneOfTheseTags(graph.nodes[projectName], tags) &&
pathExists(graph, targetProject.name, projectName)
);

for (let d of graph.dependencies[proj.name]) {
if (visited.indexOf(d.target) > -1) {
continue;
}
visited.push(d.target);
if (graph.nodes[d.target]) {
const tempPath = [...foundPath, proj];
const newPath = findDependenciesWithTags(
graph.nodes[d.target],
tags,
graph,
tempPath,
visited
);
if (newPath !== tempPath) {
return newPath;
}
}
}
return foundPath;
// return path from targetProject to reachable project
return allReachableProjects.map((project) =>
targetProject.name === project
? [targetProject]
: getPath(graph, targetProject.name, project)
);
}

function hasTag(proj: ProjectGraphProjectNode, tag: string) {
Expand Down

0 comments on commit 2e6eda6

Please sign in to comment.