Skip to content

Commit

Permalink
fix(core): dependency rules - from tags
Browse files Browse the repository at this point in the history
Each from tag must match one of the to tags of its dependency.
The old implementation did it the opposite.

It iterated over all to tags and at least one from tag had to
match.

That meant, that from tags could end up having no match at all.

Example:

Module customer-feature: ['domain:holidays', 'type:feature']
Module shared-ui: ['shared']

Dependency Rules:
{
  'domain:*': sameTag,
  'type:feature': ['shared']
}

Since shared-ui had only one tag and type:feature had clearance, the dependency worked.

Not it would fail because 'domain:holidays' would also have to have access 'shared'.
  • Loading branch information
rainerhahnekamp committed Sep 2, 2024
1 parent f3d7e30 commit f9399ed
Show file tree
Hide file tree
Showing 42 changed files with 757 additions and 376 deletions.
35 changes: 20 additions & 15 deletions packages/core/src/lib/checks/check-for-dependency-rule-violation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ export type DependencyRuleViolation = {
rawImport: string;
fromModulePath: FsPath;
toModulePath: FsPath;
fromTags: string[];
toTag: string;
fromTag: string;
toTags: string[];
};

export function checkForDependencyRuleViolation(
Expand Down Expand Up @@ -42,27 +42,32 @@ export function checkForDependencyRuleViolation(
importedModulePath,
rawImport,
] of importedModulePathsWithRawImport) {
const toTags: string[] = calcTagsForModule(
toFsPath(importedModulePath),
rootDir,
config.tagging,
config.autoTagging,
);
for (const toTag of toTags) {
if (
!isDependencyAllowed(fromTags, toTag, config.depRules, {
for (const fromTag of fromTags) {
const toTags: string[] = calcTagsForModule(
toFsPath(importedModulePath),
rootDir,
config.tagging,
config.autoTagging,
);
const isViolation = !isDependencyAllowed(
fromTag,
toTags,
config.depRules,
{
fromModulePath: fromModule,
toModulePath: toFsPath(importedModulePath),
fromFilePath: fsPath,
toFilePath: toFsPath(importedModulePath),
})
) {
},
);

if (isViolation) {
violations.push({
rawImport,
fromModulePath: fromModule,
toModulePath: toFsPath(importedModulePath),
fromTags,
toTag,
fromTag,
toTags,
});

break;
Expand Down
232 changes: 0 additions & 232 deletions packages/core/src/lib/checks/is-dependency-allowed.spec.ts

This file was deleted.

20 changes: 10 additions & 10 deletions packages/core/src/lib/checks/is-dependency-allowed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ import { wildcardToRegex } from '../util/wildcard-to-regex';
import { NoDependencyRuleForTagError } from '../error/user-error';

export const isDependencyAllowed = (
froms: string[],
to: string,
from: string,
tos: string[],
config: DependencyRulesConfig,
context: DependencyCheckContext,
): boolean => {
let isAllowed: boolean | undefined;
for (const from of froms) {
isAllowed = undefined;
for (const tag in config) {
if (from.match(wildcardToRegex(tag))) {
isAllowed = undefined;
for (const tag in config) {
if (from.match(wildcardToRegex(tag))) {
for (const to of tos) {
const value = config[tag];
const matchers = Array.isArray(value) ? value : [value];
for (const matcher of matchers) {
Expand All @@ -31,13 +31,13 @@ export const isDependencyAllowed = (
return true;
}
}
isAllowed = false;
}
isAllowed = false;
}
}

if (isAllowed === undefined) {
throw new NoDependencyRuleForTagError(from);
}
if (isAllowed === undefined) {
throw new NoDependencyRuleForTagError(from);
}

return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { describe, expect, it } from 'vitest';
import { checkForDeepImports } from './check-for-deep-imports';
import { toFsPath } from '../file-info/fs-path';
import { testInit } from '../test/test-init';
import { tsConfig } from '../test/fixtures/ts-config';
import { checkForDeepImports } from '../check-for-deep-imports';
import { toFsPath } from '../../file-info/fs-path';
import { testInit } from '../../test/test-init';
import { tsConfig } from '../../test/fixtures/ts-config';

describe('check deep imports', () => {
it('should check for deep imports', () => {
Expand Down Expand Up @@ -36,7 +36,7 @@ describe('check deep imports', () => {
}
});

it('should ignore unresolable imports', () => {
it('should ignore unresolvable imports', () => {
const projectInfo = testInit('src/main.ts', {
'tsconfig.json': tsConfig(),
src: {
Expand Down
Loading

0 comments on commit f9399ed

Please sign in to comment.