Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat(linter): add notDependOnLibsWithTags constraint option to enforc…
…e-module-boundaries rule (#8633)

Co-authored-by: Miroslav Jonas <missing.manual@gmail.com>
  • Loading branch information
jaytavares and meeroslav committed Mar 8, 2022
1 parent 445cd59 commit 0a17a61
Show file tree
Hide file tree
Showing 6 changed files with 337 additions and 30 deletions.
43 changes: 43 additions & 0 deletions docs/shared/monorepo-tags.md
Expand Up @@ -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.
Expand Down
159 changes: 158 additions & 1 deletion packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts
Expand Up @@ -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'],
Expand Down Expand Up @@ -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),
};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 = {
Expand All @@ -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'] },
],
};

Expand Down Expand Up @@ -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,
Expand Down
54 changes: 43 additions & 11 deletions packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts
Expand Up @@ -6,7 +6,7 @@ import {
findSourceProject,
getSourceFilePath,
hasBuildExecutor,
hasNoneOfTheseTags,
findDependenciesWithTags,
isAbsoluteImportIntoAnotherProject,
isRelativeImportIntoAnotherProject,
mapProjectGraphFiles,
Expand All @@ -16,6 +16,8 @@ import {
hasBannedImport,
isDirectDependency,
isTerminalRun,
stringifyTags,
hasNoneOfTheseTags,
} from '@nrwl/workspace/src/utils/runtime-lint-utils';
import {
AST_NODE_TYPES,
Expand Down Expand Up @@ -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<Options, MessageIds>({
Expand All @@ -84,6 +87,7 @@ export default createESLintRule<Options, MessageIds>({
sourceTag: { type: 'string' },
onlyDependOnLibsWithTags: [{ type: 'string' }],
bannedExternalImports: [{ type: 'string' }],
notDependOnLibsWithTags: [{ type: 'string' }],
},
additionalProperties: false,
},
Expand All @@ -102,9 +106,10 @@ export default createESLintRule<Options, MessageIds>({
'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: [
Expand Down Expand Up @@ -304,7 +309,7 @@ export default createESLintRule<Options, MessageIds>({

// 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,
Expand All @@ -319,7 +324,9 @@ export default createESLintRule<Options, MessageIds>({
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(
Expand Down Expand Up @@ -400,24 +407,49 @@ export default createESLintRule<Options, MessageIds>({
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;
}
}
}
}
}
Expand Down

1 comment on commit 0a17a61

@vercel
Copy link

@vercel vercel bot commented on 0a17a61 Mar 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.