feat(linter): allow banning of deep/secondary paths (#17755)

This commit is contained in:
Miroslav Jonaš 2023-07-03 14:41:45 +02:00 committed by GitHub
parent 50d01d1567
commit 6b82a2ff59
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 129 additions and 32 deletions

View File

@ -465,7 +465,7 @@ describe('Enforce Module Boundaries (eslint)', () => {
); );
const message = const message =
'A project tagged with "api" is not allowed to import the "npm-package" package'; 'A project tagged with "api" is not allowed to import "npm-package"';
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);
@ -507,7 +507,7 @@ describe('Enforce Module Boundaries (eslint)', () => {
); );
const message = const message =
'A project tagged with "api" is not allowed to import the "npm-awesome-package" package'; 'A project tagged with "api" is not allowed to import "npm-awesome-package"';
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);
@ -549,7 +549,7 @@ describe('Enforce Module Boundaries (eslint)', () => {
); );
const message = const message =
'A project tagged with "api" is not allowed to import the "npm-package" package'; 'A project tagged with "api" is not allowed to import "npm-package"';
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);
@ -570,7 +570,57 @@ describe('Enforce Module Boundaries (eslint)', () => {
); );
const message = const message =
'A project tagged with "api" is not allowed to import the "npm-package" package'; 'A project tagged with "api" is not allowed to import "npm-package"';
expect(failures.length).toEqual(2);
expect(failures[0].message).toEqual(message);
expect(failures[1].message).toEqual(message);
});
it('should not error when importing package nested allowed route', () => {
const failures = runRule(
{
depConstraints: [
{
sourceTag: 'api',
allowedExternalImports: ['npm-package/*'],
bannedExternalImports: ['npm-package/testing'],
},
],
},
`${process.cwd()}/proj/libs/api/src/index.ts`,
`
import 'npm-package/allowed';
import('npm-package/allowed');
`,
graph,
fileMap
);
expect(failures.length).toEqual(0);
});
it('should error when importing package nested forbidden route', () => {
const failures = runRule(
{
depConstraints: [
{
sourceTag: 'api',
allowedExternalImports: ['npm-package/*'],
bannedExternalImports: ['npm-package/testing'],
},
],
},
`${process.cwd()}/proj/libs/api/src/index.ts`,
`
import 'npm-package/testing';
import('npm-package/testing');
`,
graph,
fileMap
);
const message =
'A project tagged with "api" is not allowed to import "npm-package/testing"';
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);
@ -637,7 +687,7 @@ describe('Enforce Module Boundaries (eslint)', () => {
); );
const message = (packageName) => const message = (packageName) =>
`A project tagged with "api" is not allowed to import the "${packageName}" package`; `A project tagged with "api" is not allowed to import "${packageName}"`;
expect(failures.length).toEqual(2); expect(failures.length).toEqual(2);
expect(failures[0].message).toEqual(message('npm-package')); expect(failures[0].message).toEqual(message('npm-package'));
expect(failures[1].message).toEqual(message('npm-awesome-package')); expect(failures[1].message).toEqual(message('npm-awesome-package'));

View File

@ -128,8 +128,8 @@ export default createESLintRule<Options, MessageIds>({
'Buildable libraries cannot import or export from non-buildable libraries', 'Buildable libraries cannot import or export from non-buildable libraries',
noImportsOfLazyLoadedLibraries: `Static imports of lazy-loaded libraries are forbidden.\n\nLibrary "{{targetProjectName}}" is lazy-loaded in these files:\n{{filePaths}}`, noImportsOfLazyLoadedLibraries: `Static imports of lazy-loaded libraries are forbidden.\n\nLibrary "{{targetProjectName}}" is lazy-loaded in these files:\n{{filePaths}}`,
projectWithoutTagsCannotHaveDependencies: `A project without tags matching at least one constraint cannot depend on any libraries`, projectWithoutTagsCannotHaveDependencies: `A project without tags matching at least one constraint cannot depend on any libraries`,
bannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import the "{{package}}" package`, bannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import "{{imp}}"`,
nestedBannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import the "{{package}}" package. Nested import found at {{childProjectName}}`, nestedBannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import "{{imp}}". Nested import found at {{childProjectName}}`,
noTransitiveDependencies: `Transitive dependencies are not allowed. Only packages defined in the "package.json" can be imported`, 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}}`, onlyTagsConstraintViolation: `A project tagged with "{{sourceTag}}" can only depend on libs tagged with {{tags}}`,
emptyOnlyTagsConstraintViolation: emptyOnlyTagsConstraintViolation:
@ -405,7 +405,8 @@ export default createESLintRule<Options, MessageIds>({
const constraint = hasBannedImport( const constraint = hasBannedImport(
sourceProject, sourceProject,
targetProject, targetProject,
depConstraints depConstraints,
imp
); );
if (constraint) { if (constraint) {
context.report({ context.report({
@ -415,7 +416,7 @@ export default createESLintRule<Options, MessageIds>({
sourceTag: isComboDepConstraint(constraint) sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "') ? constraint.allSourceTags.join('" and "')
: constraint.sourceTag, : constraint.sourceTag,
package: targetProject.data.packageName, imp,
}, },
}); });
} }
@ -624,19 +625,20 @@ export default createESLintRule<Options, MessageIds>({
const matches = hasBannedDependencies( const matches = hasBannedDependencies(
transitiveExternalDeps, transitiveExternalDeps,
projectGraph, projectGraph,
constraint constraint,
imp
); );
if (matches.length > 0) { if (matches.length > 0) {
matches.forEach(([target, violatingSource, constraint]) => { matches.forEach(([target, violatingSource, constraint]) => {
context.report({ context.report({
node, node,
messageId: 'bannedExternalImportsViolation', messageId: 'nestedBannedExternalImportsViolation',
data: { data: {
sourceTag: isComboDepConstraint(constraint) sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "') ? constraint.allSourceTags.join('" and "')
: constraint.sourceTag, : constraint.sourceTag,
childProjectName: violatingSource.name, childProjectName: violatingSource.name,
package: target.data.packageName, imp,
}, },
}); });
}); });

View File

@ -138,7 +138,9 @@ describe('hasBannedImport', () => {
}, },
]; ];
expect(hasBannedImport(source, target, constraints)).toBe(constraints[1]); expect(hasBannedImport(source, target, constraints, 'react-native')).toBe(
constraints[1]
);
}); });
it('should return just first DepConstraint banning given target', () => { it('should return just first DepConstraint banning given target', () => {
@ -156,7 +158,9 @@ describe('hasBannedImport', () => {
}, },
]; ];
expect(hasBannedImport(source, target, constraints)).toBe(constraints[1]); expect(hasBannedImport(source, target, constraints, 'react-native')).toBe(
constraints[1]
);
}); });
it('should return null if tag does not match', () => { it('should return null if tag does not match', () => {
@ -174,7 +178,9 @@ describe('hasBannedImport', () => {
}, },
]; ];
expect(hasBannedImport(source, target, constraints)).toBe(undefined); expect(hasBannedImport(source, target, constraints, 'react-native')).toBe(
undefined
);
}); });
it('should return null if packages does not match', () => { it('should return null if packages does not match', () => {
@ -188,7 +194,9 @@ describe('hasBannedImport', () => {
}, },
]; ];
expect(hasBannedImport(source, target, constraints)).toBe(undefined); expect(hasBannedImport(source, target, constraints, 'react-native')).toBe(
undefined
);
}); });
}); });
@ -298,12 +306,17 @@ describe('dependentsHaveBannedImport + findTransitiveExternalDependencies', () =
); );
}); });
it('should return target and constraint pair if any dependents have banned import', () => { it("should return empty array if any dependents don't have banned import", () => {
expect( expect(
hasBannedDependencies(externalDependencies.slice(1), graph, { hasBannedDependencies(
externalDependencies.slice(1),
graph,
{
sourceTag: 'a', sourceTag: 'a',
bannedExternalImports: ['angular'], bannedExternalImports: ['angular'],
}) },
'react-native'
)
).toStrictEqual([]); ).toStrictEqual([]);
}); });
@ -314,7 +327,12 @@ describe('dependentsHaveBannedImport + findTransitiveExternalDependencies', () =
}; };
expect( expect(
hasBannedDependencies(externalDependencies.slice(1), graph, constraint) hasBannedDependencies(
externalDependencies.slice(1),
graph,
constraint,
'react-native'
)
).toStrictEqual([[bannedTarget, d, constraint]]); ).toStrictEqual([[bannedTarget, d, constraint]]);
}); });
@ -325,7 +343,12 @@ describe('dependentsHaveBannedImport + findTransitiveExternalDependencies', () =
}; };
expect( expect(
hasBannedDependencies(externalDependencies.slice(1), graph, constraint) hasBannedDependencies(
externalDependencies.slice(1),
graph,
constraint,
'react'
)
).toStrictEqual([ ).toStrictEqual([
[nonBannedTarget, target, constraint], [nonBannedTarget, target, constraint],
[nonBannedTarget, c, constraint], [nonBannedTarget, c, constraint],
@ -339,8 +362,20 @@ describe('dependentsHaveBannedImport + findTransitiveExternalDependencies', () =
}; };
expect( expect(
hasBannedDependencies(externalDependencies.slice(1), graph, constraint) hasBannedDependencies(
.length externalDependencies.slice(1),
graph,
constraint,
'react-native'
).length
).toBe(0);
expect(
hasBannedDependencies(
externalDependencies.slice(1),
graph,
constraint,
'react'
).length
).toBe(0); ).toBe(0);
}); });
}); });

View File

@ -234,15 +234,20 @@ export function getSourceFilePath(sourceFileName: string, projectPath: string) {
*/ */
function isConstraintBanningProject( function isConstraintBanningProject(
externalProject: ProjectGraphExternalNode, externalProject: ProjectGraphExternalNode,
constraint: DepConstraint constraint: DepConstraint,
imp: string
): boolean { ): boolean {
const { allowedExternalImports, bannedExternalImports } = constraint; const { allowedExternalImports, bannedExternalImports } = constraint;
const { packageName } = externalProject.data; const { packageName } = externalProject.data;
if (imp !== packageName && !imp.startsWith(`${packageName}/`)) {
return false;
}
/* Check if import is banned... */ /* Check if import is banned... */
if ( if (
bannedExternalImports?.some((importDefinition) => bannedExternalImports?.some((importDefinition) =>
mapGlobToRegExp(importDefinition).test(packageName) mapGlobToRegExp(importDefinition).test(imp)
) )
) { ) {
return true; return true;
@ -250,14 +255,17 @@ function isConstraintBanningProject(
/* ... then check if there is a whitelist and if there is a match in the whitelist. */ /* ... then check if there is a whitelist and if there is a match in the whitelist. */
return allowedExternalImports?.every( return allowedExternalImports?.every(
(importDefinition) => !mapGlobToRegExp(importDefinition).test(packageName) (importDefinition) =>
!imp.startsWith(packageName) ||
!mapGlobToRegExp(importDefinition).test(imp)
); );
} }
export function hasBannedImport( export function hasBannedImport(
source: ProjectGraphProjectNode, source: ProjectGraphProjectNode,
target: ProjectGraphExternalNode, target: ProjectGraphExternalNode,
depConstraints: DepConstraint[] depConstraints: DepConstraint[],
imp: string
): DepConstraint | undefined { ): DepConstraint | undefined {
// return those constraints that match source project // return those constraints that match source project
depConstraints = depConstraints.filter((c) => { depConstraints = depConstraints.filter((c) => {
@ -271,7 +279,7 @@ export function hasBannedImport(
return tags.every((t) => hasTag(source, t)); return tags.every((t) => hasTag(source, t));
}); });
return depConstraints.find((constraint) => return depConstraints.find((constraint) =>
isConstraintBanningProject(target, constraint) isConstraintBanningProject(target, constraint, imp)
); );
} }
@ -323,7 +331,8 @@ export function findTransitiveExternalDependencies(
export function hasBannedDependencies( export function hasBannedDependencies(
externalDependencies: ProjectGraphDependency[], externalDependencies: ProjectGraphDependency[],
graph: ProjectGraph, graph: ProjectGraph,
depConstraint: DepConstraint depConstraint: DepConstraint,
imp: string
): ):
| Array<[ProjectGraphExternalNode, ProjectGraphProjectNode, DepConstraint]> | Array<[ProjectGraphExternalNode, ProjectGraphProjectNode, DepConstraint]>
| undefined { | undefined {
@ -331,7 +340,8 @@ export function hasBannedDependencies(
.filter((dependency) => .filter((dependency) =>
isConstraintBanningProject( isConstraintBanningProject(
graph.externalNodes[dependency.target], graph.externalNodes[dependency.target],
depConstraint depConstraint,
imp
) )
) )
.map((dep) => [ .map((dep) => [