From 8c93ae9952f6679d9006c76188a80b33afa041f5 Mon Sep 17 00:00:00 2001 From: Miroslav Jonas Date: Mon, 7 Mar 2022 15:34:23 +0100 Subject: [PATCH] feat(linter): add project chain mapping for dependency tree --- .../rules/enforce-module-boundaries.spec.ts | 12 ++-- .../src/rules/enforce-module-boundaries.ts | 47 +++++++++------- .../tslint/nxEnforceModuleBoundariesRule.ts | 40 ++++++++++--- .../workspace/src/utils/runtime-lint-utils.ts | 56 +++++++++++-------- 4 files changed, 99 insertions(+), 56 deletions(-) diff --git a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts index 2f31316487b9d..2eed692ca377c 100644 --- a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts +++ b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts @@ -512,8 +512,10 @@ describe('Enforce Module Boundaries (eslint)', () => { graph ); - const message = - 'A project tagged with "public" can not depend on libs tagged with "private"'; + const message = `A project tagged with "public" can not depend on libs tagged with "private" + +Violation detected in: +- privateName`; expect(failures.length).toEqual(2); expect(failures[0].message).toEqual(message); expect(failures[1].message).toEqual(message); @@ -532,8 +534,10 @@ describe('Enforce Module Boundaries (eslint)', () => { expect(failures.length).toEqual(2); // TODO: Add project dependency path to message - const message = - 'A project tagged with "public" can not depend on libs tagged with "private"'; + const message = `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); }); diff --git a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts index 0c80bc96a3636..b7e6eac91ae24 100644 --- a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts +++ b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts @@ -6,8 +6,7 @@ import { findSourceProject, getSourceFilePath, hasBuildExecutor, - hasNoneOfTheseTags, - hasAnyOfTheseTags, + findDependenciesWithTags, isAbsoluteImportIntoAnotherProject, isRelativeImportIntoAnotherProject, mapProjectGraphFiles, @@ -18,6 +17,7 @@ import { isDirectDependency, isTerminalRun, stringifyTags, + hasNoneOfTheseTags, } from '@nrwl/workspace/src/utils/runtime-lint-utils'; import { AST_NODE_TYPES, @@ -109,7 +109,7 @@ export default createESLintRule({ bannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import the "{{package}}" package`, noTransitiveDependencies: `Transitive dependencies are not allowed. Only packages defined in the "package.json" can be imported`, onlyTagsConstraintViolation: `A project tagged with "{{sourceTag}}" can only depend on libs tagged with {{tags}}`, - notTagsConstraintViolation: `A project tagged with "{{sourceTag}}" can not depend on libs tagged with {{tags}}`, + notTagsConstraintViolation: `A project tagged with "{{sourceTag}}" can not depend on libs tagged with {{tags}}\n\nViolation detected in:\n{{projects}}`, }, }, defaultOptions: [ @@ -309,7 +309,7 @@ export default createESLintRule({ // spacer text used for indirect dependencies when printing one line per file. // without this, we can end up with a very long line that does not display well in the terminal. - const spacer = '\n '; + const spacer = ' '; context.report({ node, @@ -324,7 +324,9 @@ export default createESLintRule({ filePaths: circularFilePath .map((files) => files.length > 1 - ? `[${spacer}${files.join(`,${spacer}`)}\n ]` + ? `[${files + .map((f) => `\n${spacer}${spacer}${f}`) + .join(',')}\n${spacer}]` : files[0] ) .reduce( @@ -404,6 +406,8 @@ export default createESLintRule({ for (let constraint of constraints) { if ( + constraint.onlyDependOnLibsWithTags && + constraint.onlyDependOnLibsWithTags.length && hasNoneOfTheseTags( targetProject, constraint.onlyDependOnLibsWithTags @@ -420,21 +424,26 @@ export default createESLintRule({ return; } if ( - hasAnyOfTheseTags( - projectGraph, - targetProject.name, - constraint.notDependOnLibsWithTags - ) + constraint.notDependOnLibsWithTags && + constraint.notDependOnLibsWithTags.length ) { - context.report({ - node, - messageId: 'notTagsConstraintViolation', - data: { - sourceTag: constraint.sourceTag, - tags: stringifyTags(constraint.notDependOnLibsWithTags), - }, - }); - return; + const projects = findDependenciesWithTags( + targetProject, + constraint.notDependOnLibsWithTags, + projectGraph + ); + if (projects.length > 0) { + context.report({ + node, + messageId: 'notTagsConstraintViolation', + data: { + sourceTag: constraint.sourceTag, + tags: stringifyTags(constraint.notDependOnLibsWithTags), + projects: `- ${projects.map((p) => p.name).join(' -> ')}`, + }, + }); + return; + } } } } diff --git a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts index 967d7964fa9fd..c91f28aa7da0e 100644 --- a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts +++ b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts @@ -11,13 +11,15 @@ import { findSourceProject, getSourceFilePath, hasBuildExecutor, - hasNoneOfTheseTags, + findDependenciesWithTags, isAbsoluteImportIntoAnotherProject, isRelativeImportIntoAnotherProject, MappedProjectGraph, mapProjectGraphFiles, matchImportWithWildcard, onlyLoadChildren, + stringifyTags, + hasNoneOfTheseTags, } from '../utils/runtime-lint-utils'; import { normalize } from 'path'; import { readNxJson } from '../core/file-utils'; @@ -272,18 +274,38 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker { for (let constraint of constraints) { if ( - hasNoneOfTheseTags( - targetProject, - constraint.onlyDependOnLibsWithTags || [] - ) + constraint.onlyDependOnLibsWithTags && + hasNoneOfTheseTags(targetProject, constraint.onlyDependOnLibsWithTags) ) { - const tags = constraint.onlyDependOnLibsWithTags - .map((s) => `"${s}"`) - .join(', '); - const error = `A project tagged with "${constraint.sourceTag}" can only depend on libs tagged with ${tags}`; + const error = `A project tagged with "${ + constraint.sourceTag + }" can only depend on libs tagged with ${stringifyTags( + constraint.onlyDependOnLibsWithTags + )}`; this.addFailureAt(node.getStart(), node.getWidth(), error); return; } + if ( + constraint.notDependOnLibsWithTags && + constraint.notDependOnLibsWithTags.length + ) { + const projects = findDependenciesWithTags( + targetProject, + constraint.notDependOnLibsWithTags, + this.projectGraph + ); + if (projects.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(' -> ')}`; + this.addFailureAt(node.getStart(), node.getWidth(), error); + return; + } + } } } diff --git a/packages/workspace/src/utils/runtime-lint-utils.ts b/packages/workspace/src/utils/runtime-lint-utils.ts index ed601130071c4..ddd70e34caf02 100644 --- a/packages/workspace/src/utils/runtime-lint-utils.ts +++ b/packages/workspace/src/utils/runtime-lint-utils.ts @@ -35,44 +35,52 @@ export function stringifyTags(tags: string[]): string { } export function hasNoneOfTheseTags( - proj: ProjectGraphProjectNode, - tags?: string[] + proj: ProjectGraphProjectNode, + tags: string[] ): boolean { - if (!tags) { - return false; - } return tags.filter((tag) => hasTag(proj, tag)).length === 0; } -export function hasAnyOfTheseTags( +/** + * Check if any of the given tags is included in the project + * @param proj ProjectGraphProjectNode + * @param tags + * @returns + */ +export function findDependenciesWithTags( + proj: ProjectGraphProjectNode, + tags: string[], graph: ProjectGraph, - projectName: string, - tags?: string[], - visited?: string[] -): boolean { - if (!tags) { - return false; + foundPath: Array = [], + visited: string[] = [] +): Array { + if (!hasNoneOfTheseTags(proj, tags)) { + return [...foundPath, proj]; } - const found = - tags.filter((tag) => hasTag(graph.nodes[projectName], tag)).length !== 0; - - if (found) { - return true; + if (!graph.dependencies[proj.name]) { + return foundPath; } - visited = visited ?? []; - - for (let d of graph.dependencies[projectName] || []) { + for (let d of graph.dependencies[proj.name]) { if (visited.indexOf(d.target) > -1) { continue; } visited.push(d.target); - if (hasAnyOfTheseTags(graph, d.target, tags, visited)) { - return true; + 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 false; + return foundPath; } function hasTag(proj: ProjectGraphProjectNode, tag: string) {