Skip to content

Commit

Permalink
feat(eslint): supersede deep-import rules with encapsulation rule
Browse files Browse the repository at this point in the history
The renaming is necessary since the concept of a deep import does not exist in
barrel-less modules.

The configs for ESLint have been changed. To use the old `deep-import` rule,
you need to switch to the config `barrelModulesOnly`.
  • Loading branch information
rainerhahnekamp authored Oct 19, 2024
1 parent 86cf9f6 commit 3a4f132
Show file tree
Hide file tree
Showing 25 changed files with 515 additions and 274 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { hasDeepImport } from './lib/eslint/deep-import';
export { violatesEncapsulationRule } from './lib/eslint/violates-encapsulation-rule';
export { violatesDependencyRule } from './lib/eslint/violates-dependency-rule';
export { anyTag } from './lib/checks/any-tag';
export { sameTag } from './lib/checks/same-tag';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@ import { ProjectInfo } from '../main/init';
import { FileInfo } from '../modules/file.info';

/**
* verifies if an existing file has deep imports which are forbidden.
* verifies if an existing file has imports which break
* the other module's encapsulation.
*
* Unresolvable imports are skipped.
*
* It is up to the caller to decide.
*/
export function checkForDeepImports(
export function hasEncapsulationViolations(
fsPath: FsPath,
{ rootDir, config, getFileInfo }: ProjectInfo,
): string[] {
const deepImports: string[] = [];
): Record<string, FileInfo> {
const encapsulationViolations: Record<string, FileInfo> = {};
const assignedFileInfo = getFileInfo(fsPath);

for (const importedFileInfo of assignedFileInfo.imports) {
Expand All @@ -25,13 +27,12 @@ export function checkForDeepImports(
) {
// 👍 all good
} else {
deepImports.push(
assignedFileInfo.getRawImportForImportedFileInfo(importedFileInfo.path),
);
const rawImport = assignedFileInfo.getRawImportForImportedFileInfo(importedFileInfo.path);
encapsulationViolations[rawImport] = importedFileInfo;
}
}

return deepImports;
return encapsulationViolations;
}

function accessesExposedFileForBarrelLessModules(fileInfo: FileInfo, enableBarrelLess: boolean) {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { describe, it, expect } from 'vitest';
import { testInit } from '../../test/test-init';
import { tsConfig } from '../../test/fixtures/ts-config';
import { FileTree, sheriffConfig } from '../../test/project-configurator';
import { checkForDeepImports } from '../check-for-deep-imports';
import { hasEncapsulationViolations } from '../has-encapsulation-violations';
import { UserSheriffConfig } from '../../config/user-sheriff-config';
import { traverseFileInfo } from '../../modules/traverse-file-info';

Expand All @@ -25,7 +25,7 @@ describe('barrel-less', () => {
internal: { 'hidden.service.ts': [] },
},
})
.hasDeepImports({
.hasEncapsulationViolations({
'feature/customers.component.ts': [
'../data/internal/hidden.service.ts',
],
Expand All @@ -52,7 +52,7 @@ describe('barrel-less', () => {
internal: { services: { 'hidden.service.ts': [] } },
},
})
.hasDeepImports({
.hasEncapsulationViolations({
'feature/customers.component.ts': [
'../data/internal/services/hidden.service.ts',
],
Expand Down Expand Up @@ -82,7 +82,7 @@ describe('barrel-less', () => {
internal: { 'hidden.service.ts': [] },
},
})
.hasDeepImports({
.hasEncapsulationViolations({
'feature/components/customers-sub.component.ts': [
'../../data/internal/hidden.service.ts',
],
Expand All @@ -103,7 +103,7 @@ describe('barrel-less', () => {
internal: { 'customer.service.ts': ['../private/hidden.service.ts'] },
},
})
.hasDeepImports({
.hasEncapsulationViolations({
'feature/customers.component.ts': ['../data/private/hidden.service.ts'],
});
});
Expand All @@ -121,7 +121,7 @@ describe('barrel-less', () => {
internal: { 'hidden.service.ts': [] },
},
})
.hasDeepImports({
.hasEncapsulationViolations({
'feature/customers.component.ts': ['../data/open.service.ts'],
});
});
Expand All @@ -131,7 +131,7 @@ function assertProject(config: Partial<UserSheriffConfig> = {}) {
return {
withCustomerRoute(customerFileTree: FileTree) {
return {
hasDeepImports(deepImports: Record<string, string[]> = {}) {
hasEncapsulationViolations(encapsulationViolations: Record<string, string[]> = {}) {
const projectInfo = testInit('src/main.ts', {
'tsconfig.json': tsConfig(),
'sheriff.config.ts': sheriffConfig({
Expand Down Expand Up @@ -167,10 +167,12 @@ function assertProject(config: Partial<UserSheriffConfig> = {}) {
'',
);

const expectedDeepImports = deepImports[pathToLookup] || [];
const expectedDeepImports = encapsulationViolations[pathToLookup] || [];
const violations = hasEncapsulationViolations(fileInfo.path, projectInfo);
const violatedImports = Object.keys(violations);
expect
.soft(
checkForDeepImports(fileInfo.path, projectInfo),
violatedImports,
`deep imports check failed for ${fileInfo.path}`,
)
.toEqual(expectedDeepImports);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { describe, expect, it } from 'vitest';
import { anyTag, violatesEncapsulationRule } from '@softarc/sheriff-core';
import getFs from '../../fs/getFs';
import { toFsPath } from '../../file-info/fs-path';
import { testInit } from '../../test/test-init';
import { sheriffConfig } from '../../test/project-configurator';
import { tsConfig } from '../../test/fixtures/ts-config';

describe('encapsulation and excludeRoot config property', () => {
for (const { excludeRoot, enableBarrelLess, hasViolation } of [
{ excludeRoot: true, enableBarrelLess: false, hasViolation: false },
{ excludeRoot: false, enableBarrelLess: false, hasViolation: true },
{ excludeRoot: true, enableBarrelLess: true, hasViolation: false },
{ excludeRoot: false, enableBarrelLess: true, hasViolation: false },
]) {
it(`should be ${
hasViolation ? 'valid' : 'invalid'
} for rootExcluded: ${excludeRoot} and barrel-less: ${enableBarrelLess}`, () => {
testInit('src/main.ts', {
'tsconfig.json': tsConfig(),
'sheriff.config.ts': sheriffConfig({
modules: { 'src/shared': 'shared' },
depRules: { '*': anyTag },
excludeRoot,
enableBarrelLess,
}),
src: {
'main.ts': '',
'router.ts': ['./shared/dialog'], // always violation
'config.ts': ['./shared/index'],
shared: {
'get.ts': ['../config', '../holidays/holidays-component'], // depends on `excludeRoot`
'dialog.ts': '',
'index.ts': '',
},
holidays: {
'holidays-component.ts': ['../config'], // always valid},
},
},
});

assertViolation('/project/src/router.ts', './shared/dialog', false, true);

assertViolation(
'/project/src/holidays/holidays-component.ts',
'../config',
true,
false,
);

assertViolation(
'/project/src/shared/get.ts',
'../config',
true,
hasViolation,
);

assertViolation(
'/project/src/shared/get.ts',
'../holidays/holidays-component',
true,
hasViolation,
);
});
}
});

function assertViolation(
filename: string,
importCommand: string,
isImportToBarrelLess: boolean,
hasViolation: boolean,
) {
expect(
violatesEncapsulationRule(
filename,
importCommand,
true,
getFs().readFile(toFsPath(filename)),
false,
),
`import in ${filename} to ${importCommand} should violate encapsulation: ${hasViolation}`,
).toBe(getMessage(hasViolation, isImportToBarrelLess, importCommand));
}

function getMessage(
hasViolation: boolean,
isImportToBarrelLess: boolean,
importCommand: string,
) {
if (!hasViolation) {
return '';
}
return isImportToBarrelLess
? `'${importCommand}' cannot be imported. It is encapsulated.`
: `'${importCommand}' is a deep import from a barrel module. Use the module's barrel file (index.ts) instead.`;
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { describe, expect, it } from 'vitest';
import { checkForDeepImports } from '../check-for-deep-imports';
import { hasEncapsulationViolations } from '../has-encapsulation-violations';
import { toFsPath } from '../../file-info/fs-path';
import { testInit } from '../../test/test-init';
import { tsConfig } from '../../test/fixtures/ts-config';

describe('check deep imports', () => {
describe('check encapsulation', () => {
it('should check for deep imports', () => {
const projectInfo = testInit('src/main.ts', {
'tsconfig.json': tsConfig(),
Expand All @@ -31,7 +31,7 @@ describe('check deep imports', () => {
['src/app/customers/index.ts', []],
]) {
expect(
checkForDeepImports(toFsPath(`/project/${filename}`), projectInfo),
Object.keys(hasEncapsulationViolations(toFsPath(`/project/${filename}`), projectInfo)),
`failed deep import test for ${filename}`,
).toEqual(deepImports);
}
Expand All @@ -53,7 +53,7 @@ describe('check deep imports', () => {
['src/app/app.component.ts', []],
]) {
expect(
checkForDeepImports(toFsPath(`/project/${filename}`), projectInfo),
Object.keys(hasEncapsulationViolations(toFsPath(`/project/${filename}`), projectInfo)),
).toEqual(deepImports);
}
});
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/lib/cli/verify.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { checkForDeepImports } from '../checks/check-for-deep-imports';
import { hasEncapsulationViolations } from '../checks/has-encapsulation-violations';
import { traverseFileInfo } from '../modules/traverse-file-info';
import { checkForDependencyRuleViolation } from '../checks/check-for-dependency-rule-violation';
import getFs from '../fs/getFs';
Expand All @@ -21,16 +21,17 @@ export function verify(args: string[]) {
const projectInfo = getEntryFromCliOrConfig(args[0]);

for (const { fileInfo } of traverseFileInfo(projectInfo.fileInfo)) {
const deepImports = checkForDeepImports(fileInfo.path, projectInfo);
const violations = Object.keys(hasEncapsulationViolations(fileInfo.path, projectInfo));

const dependencyRuleViolations = checkForDependencyRuleViolation(
fileInfo.path,
projectInfo,
);

if (deepImports.length > 0 || dependencyRuleViolations.length > 0) {
if (violations.length > 0 || dependencyRuleViolations.length > 0) {
hasError = true;
filesCount++;
deepImportsCount += deepImports.length;
deepImportsCount += violations.length;
dependencyRulesCount += dependencyRuleViolations.length;

const dependencyRules = dependencyRuleViolations.map(
Expand All @@ -39,7 +40,7 @@ export function verify(args: string[]) {
);

validationsMap[fs.relativeTo(fs.cwd(), fileInfo.path)] = {
deepImports,
deepImports: violations,
dependencyRules,
};
}
Expand Down
Loading

0 comments on commit 3a4f132

Please sign in to comment.