diff --git a/docs/shared/monorepo-tags.md b/docs/shared/monorepo-tags.md index beb0e3e84acd3..5c563f2b709b9 100644 --- a/docs/shared/monorepo-tags.md +++ b/docs/shared/monorepo-tags.md @@ -155,6 +155,49 @@ If you try to violate the constrains, you will get an error: A project tagged with "scope:admin" can only depend on projects tagged with "scoped:shared" or "scope:admin". ``` +### Explicitly banning tags + +Specifying which tags is project allowed to depend on can sometimes lead to a long list of possible options: + +```jsonc +{ + "sourceTag": "scope:client", + // we actually want to say it cannot depend on `scope:admin` + "onlyDependOnLibsWithTags": [ + "scope:shared", + "scope:utils", + "scope:core", + "scope:client" + ] +} +``` + +The property `notDependOnLibsWithTags` is used to invert this condition by explicitly specifying which tag(s) it cannot depend on: + +```jsonc +{ + "sourceTag": "scope:client", + // we accept any tag except for `scope:admin` + "notDependOnLibsWithTags": ["scope:admin"] +} +``` + +> In contrast to `onlyDependOnLibsWithTags`, the `notDependOnLibsWithTags` will also follow down the _entire dependency tree_ to make sure there are no sub-dependencies that violate this rule. You can also use a combination of these two rules to restrict certain types of projects to be imported: + +```jsonc +{ + "sourceTag": "type:react", + "onlyDependOnLibsWithTags": [ + "type:react", + "type:utils", + "type:animation", + "type:model" + ], + // make sure no `angular` code ends up being referenced by react projects + "notDependOnLibsWithTags": ["type:angular"] +} +``` + ### Exceptions The `"allow": []` are the list of imports that won't fail linting. 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 0411310a5503c..4624fb50e59fb 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 @@ -34,6 +34,10 @@ const tsconfig = { '@mycompany/myapp-e2e': ['apps/myapp-e2e/src/index.ts'], '@mycompany/mylib': ['libs/mylib/src/index.ts'], '@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'], '@mycompany/domain1': ['libs/domain1/src/index.ts'], @@ -78,6 +82,10 @@ const fileSys = { './libs/domain2/src/index.ts': '', './libs/buildableLib/src/main.ts': '', './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), }; @@ -249,6 +257,60 @@ describe('Enforce Module Boundaries (eslint)', () => { files: [createFile(`libs/impl/src/index.ts`)], }, }, + publicName: { + name: 'publicName', + type: ProjectType.lib, + data: { + root: 'libs/public', + tags: ['public'], + implicitDependencies: [], + architect: {}, + files: [createFile(`libs/public/src/index.ts`)], + }, + }, + dependsOnPrivateName: { + name: 'dependsOnPrivateName', + type: ProjectType.lib, + data: { + root: 'libs/dependsOnPrivate', + tags: [], + implicitDependencies: [], + architect: {}, + files: [ + createFile(`libs/dependsOnPrivate/src/index.ts`, ['privateName']), + ], + }, + }, + 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, + data: { + root: 'libs/private', + tags: ['private'], + implicitDependencies: [], + architect: {}, + files: [ + createFile( + `libs/private/src/index.tslibs/private/src/index.tslibs/private/src/index.ts` + ), + ], + }, + }, untaggedName: { name: 'untaggedName', type: ProjectType.lib, @@ -295,7 +357,22 @@ describe('Enforce Module Boundaries (eslint)', () => { }, }, }, - dependencies: {}, + dependencies: { + dependsOnPrivateName: [ + { + source: 'dependsOnPrivateName', + target: 'privateName', + type: DependencyType.static, + }, + ], + dependsOnPrivateName2: [ + { + source: 'dependsOnPrivateName2', + target: 'privateName', + type: DependencyType.static, + }, + ], + }, }; const depConstraints = { @@ -304,6 +381,7 @@ describe('Enforce Module Boundaries (eslint)', () => { { sourceTag: 'impl', onlyDependOnLibsWithTags: ['api', 'impl'] }, { sourceTag: 'domain1', onlyDependOnLibsWithTags: ['domain1'] }, { sourceTag: 'domain2', onlyDependOnLibsWithTags: ['domain2'] }, + { sourceTag: 'public', notDependOnLibsWithTags: ['private'] }, ], }; @@ -447,6 +525,85 @@ describe('Enforce Module Boundaries (eslint)', () => { expect(failures[1].message).toEqual(message); }); + it('should error when the target library has a disallowed tag', () => { + const failures = runRule( + depConstraints, + `${process.cwd()}/proj/libs/public/src/index.ts`, + ` + import '@mycompany/private'; + import('@mycompany/private'); + `, + { + ...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" + +Violation detected in: +- privateName`; + expect(failures.length).toEqual(2); + expect(failures[0].message).toEqual(message); + expect(failures[1].message).toEqual(message); + }); + + it('should error when there is a disallowed tag in the dependency tree', () => { + const failures = runRule( + depConstraints, + `${process.cwd()}/proj/libs/public/src/index.ts`, + ` + import '@mycompany/dependsOnPrivate'; + import '@mycompany/dependsOnPrivate2'; + import '@mycompany/private'; + `, + { + ...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(3); + // TODO: Add project dependency path to message + const message = ( + prefix + ) => `A project tagged with "public" can not depend on libs tagged with "private" + +Violation detected in: +- ${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', () => { const failures = runRule( depConstraints, 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 2ab49c2af3423..7cd40c65e8525 100644 --- a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts +++ b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts @@ -6,7 +6,7 @@ import { findSourceProject, getSourceFilePath, hasBuildExecutor, - hasNoneOfTheseTags, + findDependenciesWithTags, isAbsoluteImportIntoAnotherProject, isRelativeImportIntoAnotherProject, mapProjectGraphFiles, @@ -16,6 +16,8 @@ import { hasBannedImport, isDirectDependency, isTerminalRun, + stringifyTags, + hasNoneOfTheseTags, } from '@nrwl/workspace/src/utils/runtime-lint-utils'; import { AST_NODE_TYPES, @@ -55,9 +57,10 @@ export type MessageIds = | 'noImportOfNonBuildableLibraries' | 'noImportsOfLazyLoadedLibraries' | 'projectWithoutTagsCannotHaveDependencies' - | 'tagConstraintViolation' | 'bannedExternalImportsViolation' - | 'noTransitiveDependencies'; + | 'noTransitiveDependencies' + | 'onlyTagsConstraintViolation' + | 'notTagsConstraintViolation'; export const RULE_NAME = 'enforce-module-boundaries'; export default createESLintRule({ @@ -84,6 +87,7 @@ export default createESLintRule({ sourceTag: { type: 'string' }, onlyDependOnLibsWithTags: [{ type: 'string' }], bannedExternalImports: [{ type: 'string' }], + notDependOnLibsWithTags: [{ type: 'string' }], }, additionalProperties: false, }, @@ -102,9 +106,10 @@ export default createESLintRule({ 'Buildable libraries cannot import or export from non-buildable libraries', noImportsOfLazyLoadedLibraries: `Imports of lazy-loaded libraries are forbidden`, projectWithoutTagsCannotHaveDependencies: `A project without tags matching at least one constraint cannot depend on any libraries`, - tagConstraintViolation: `A project tagged with "{{sourceTag}}" can only depend on libs tagged with {{allowedTags}}`, 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}}\n\nViolation detected in:\n{{projects}}`, }, }, defaultOptions: [ @@ -304,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, @@ -319,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( @@ -400,24 +407,49 @@ export default createESLintRule({ for (let constraint of constraints) { if ( constraint.onlyDependOnLibsWithTags && + constraint.onlyDependOnLibsWithTags.length && hasNoneOfTheseTags( targetProject, constraint.onlyDependOnLibsWithTags ) ) { - const allowedTags = constraint.onlyDependOnLibsWithTags - .map((s) => `"${s}"`) - .join(', '); context.report({ node, - messageId: 'tagConstraintViolation', + messageId: 'onlyTagsConstraintViolation', data: { sourceTag: constraint.sourceTag, - allowedTags, + tags: stringifyTags(constraint.onlyDependOnLibsWithTags), }, }); return; } + if ( + constraint.notDependOnLibsWithTags && + constraint.notDependOnLibsWithTags.length + ) { + const projectPaths = findDependenciesWithTags( + targetProject, + constraint.notDependOnLibsWithTags, + projectGraph + ); + if (projectPaths.length > 0) { + context.report({ + node, + messageId: 'notTagsConstraintViolation', + data: { + sourceTag: constraint.sourceTag, + tags: stringifyTags(constraint.notDependOnLibsWithTags), + projects: projectPaths + .map( + (projectPath) => + `- ${projectPath.map((p) => p.name).join(' -> ')}` + ) + .join('\n'), + }, + }); + return; + } + } } } } diff --git a/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts b/packages/workspace/src/tslint/nxEnforceModuleBoundariesRule.ts index 5d6ca05c27061..97e9805267897 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,41 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker { for (let constraint of constraints) { if ( - hasNoneOfTheseTags( - targetProject, - constraint.onlyDependOnLibsWithTags || [] - ) + constraint.onlyDependOnLibsWithTags && + hasNoneOfTheseTags(targetProject, constraint.onlyDependOnLibsWithTags) ) { - const allowedTags = constraint.onlyDependOnLibsWithTags - .map((s) => `"${s}"`) - .join(', '); - const error = `A project tagged with "${constraint.sourceTag}" can only depend on libs tagged with ${allowedTags}`; + 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 projectPaths = findDependenciesWithTags( + targetProject, + constraint.notDependOnLibsWithTags, + this.projectGraph + ); + 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${projectPaths + .map( + (projectPath) => + `- ${projectPath.map((p) => p.name).join(' -> ')}` + ) + .join('\n')}`; + this.addFailureAt(node.getStart(), node.getWidth(), error); + return; + } + } } } diff --git a/packages/workspace/src/utils/graph-utils.ts b/packages/workspace/src/utils/graph-utils.ts index dafc137b31906..92aa4b191dc33 100644 --- a/packages/workspace/src/utils/graph-utils.ts +++ b/packages/workspace/src/utils/graph-utils.ts @@ -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) { @@ -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]) { @@ -52,8 +52,8 @@ function buildMatrix(graph: ProjectGraph) { } }; - nodes.forEach((v, i) => { - traverse(nodes[i], nodes[i]); + nodes.forEach((v) => { + traverse(v, v); }); return { @@ -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[] { if (!graph.nodes[targetProject.name]) return []; return getPath(graph, targetProject.name, sourceProject.name); diff --git a/packages/workspace/src/utils/runtime-lint-utils.ts b/packages/workspace/src/utils/runtime-lint-utils.ts index 2c5c66f719b8d..f529afb4f9274 100644 --- a/packages/workspace/src/utils/runtime-lint-utils.ts +++ b/packages/workspace/src/utils/runtime-lint-utils.ts @@ -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 = ProjectGraphProjectNode & { data: { @@ -26,16 +27,48 @@ export type Deps = { [projectName: string]: ProjectGraphDependency[] }; export type DepConstraint = { sourceTag: string; onlyDependOnLibsWithTags: string[]; + notDependOnLibsWithTags: string[]; bannedExternalImports?: string[]; }; +export function stringifyTags(tags: string[]): string { + return tags.map((t) => `"${t}"`).join(', '); +} + export function hasNoneOfTheseTags( - proj: ProjectGraphProjectNode, + proj: ProjectGraphProjectNode, tags: string[] -) { +): boolean { return tags.filter((tag) => hasTag(proj, tag)).length === 0; } +/** + * Check if any of the given tags is included in the project + * @param proj ProjectGraphProjectNode + * @param tags + * @returns + */ +export function findDependenciesWithTags( + targetProject: ProjectGraphProjectNode, + tags: string[], + 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) => + pathExists(graph, targetProject.name, projectName) && + tags.some((tag) => hasTag(graph.nodes[projectName], tag)) + ); + + // 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) { return tag === '*' || (proj.data.tags || []).indexOf(tag) > -1; }