From fa29e978b6d1b65297eb1a9fdb26380feb6de081 Mon Sep 17 00:00:00 2001 From: Rainer Hahnekamp Date: Sun, 13 Oct 2024 21:48:13 +0200 Subject: [PATCH] feat(core): add support for barrel-less modules and checks * feat(core): add barrel-less modules and checks Introduces an option to define modules without a barrel file (e.g., index.ts). Instead, modules can encapsulate files within an internals folder. Modules are now defined via configuration, not by the presence of a barrel file. The existing tagging property is renamed to modules, with backward compatibility maintained until version 1. Both barrel-less and traditional barrel-based modules can coexist. If a module has both a barrel file and an internals folder, the barrel file takes precedence, and a console warning is shown. This warning can be disabled via config. Barrel-less mode is disabled by default and can be enabled via the `enableBarrelLess` config option. --- .../src/lib/checks/check-for-deep-imports.ts | 70 ++++--- ...check-for-deep-imports.barrel-less.spec.ts | 182 ++++++++++++++++++ .../tests/check-for-deep-imports.spec.ts | 2 +- ...heck-for-dependency-rule-violation.spec.ts | 54 +++++- .../deep-import-with-exclude-root.spec.ts | 82 ++++++++ .../core/src/lib/config/default-config.ts | 1 + .../src/lib/config/tests/parse-config.spec.ts | 4 +- .../src/lib/config/user-sheriff-config.ts | 6 + .../core/src/lib/eslint/deep-import.spec.ts | 57 ------ packages/core/src/lib/main/parse-project.ts | 13 +- .../core/src/lib/modules/create-modules.ts | 46 ++++- packages/core/src/lib/modules/file.info.ts | 4 + .../core/src/lib/modules/find-module-paths.ts | 2 +- packages/core/src/lib/modules/module.ts | 24 ++- .../lib/modules/tests/create-module.spec.ts | 85 ++++++-- .../warn-on-barrel-file-less-collision.ts | 19 ++ 16 files changed, 540 insertions(+), 111 deletions(-) create mode 100644 packages/core/src/lib/checks/tests/check-for-deep-imports.barrel-less.spec.ts create mode 100644 packages/core/src/lib/checks/tests/deep-import-with-exclude-root.spec.ts create mode 100644 packages/core/src/lib/modules/warn-on-barrel-file-less-collision.ts diff --git a/packages/core/src/lib/checks/check-for-deep-imports.ts b/packages/core/src/lib/checks/check-for-deep-imports.ts index f7c7bb9..8d6bb60 100644 --- a/packages/core/src/lib/checks/check-for-deep-imports.ts +++ b/packages/core/src/lib/checks/check-for-deep-imports.ts @@ -1,7 +1,7 @@ import { FsPath } from '../file-info/fs-path'; import { SheriffConfig } from '../config/sheriff-config'; import { ProjectInfo } from '../main/init'; -import { FileInfo } from "../modules/file.info"; +import { FileInfo } from '../modules/file.info'; /** * verifies if an existing file has deep imports which are forbidden. @@ -16,17 +16,15 @@ export function checkForDeepImports( const deepImports: string[] = []; const assignedFileInfo = getFileInfo(fsPath); - const isRootAndExcluded = createIsRootAndExcluded(rootDir, config); - const isModuleBarrel = (fileInfo: FileInfo) => - fileInfo.moduleInfo.hasBarrel && - fileInfo.moduleInfo.barrelPath === fileInfo.path; - for (const importedFileInfo of assignedFileInfo.imports) { if ( - !isModuleBarrel(importedFileInfo) && - !isRootAndExcluded(importedFileInfo.moduleInfo.path) && - importedFileInfo.moduleInfo !== assignedFileInfo.moduleInfo + isSameModule(importedFileInfo, assignedFileInfo) || + isExcludedRootModule(rootDir, config, importedFileInfo) || + accessesBarrelFileForBarrelModules(importedFileInfo) || + accessesExposedFileForBarrelLessModules(importedFileInfo, config.enableBarrelLess) ) { + // 👍 all good + } else { deepImports.push( assignedFileInfo.getRawImportForImportedFileInfo(importedFileInfo.path), ); @@ -36,22 +34,44 @@ export function checkForDeepImports( return deepImports; } -/** - * creates a function which allows a deep import, if - * `excludeRoot` in the config is `true` and the - * importedModulePath is the root module. - */ -const createIsRootAndExcluded = ( +function accessesExposedFileForBarrelLessModules(fileInfo: FileInfo, enableBarrelLess: boolean) { + if (!enableBarrelLess) { + return false; + } + + if (fileInfo.moduleInfo.hasBarrel) { + return false; + } + + const possibleEncapsulatedFolderPath = + fileInfo.moduleInfo.getEncapsulatedFolder(); + if (possibleEncapsulatedFolderPath === undefined) { + return true; + } + + return !fileInfo.path.startsWith(possibleEncapsulatedFolderPath); +} + +function accessesBarrelFileForBarrelModules(fileInfo: FileInfo) { + if (!fileInfo.moduleInfo.hasBarrel) { + return false; + } + + return fileInfo.moduleInfo.barrelPath === fileInfo.path; +} + +function isExcludedRootModule( rootDir: FsPath, - config: SheriffConfig | undefined, -) => { - let excludeRoot = false; - if (config === undefined) { - excludeRoot = false; - } else { - excludeRoot = Boolean(config.excludeRoot); + config: SheriffConfig, + importedModule: FileInfo, +) { + if (importedModule.moduleInfo.path !== rootDir) { + return false; } - return (importedModulePath: string): boolean => - excludeRoot && importedModulePath === rootDir; -}; + return config.excludeRoot; +} + +function isSameModule(importedFileInfo: FileInfo, assignedFileInfo: FileInfo) { + return importedFileInfo.moduleInfo.path === assignedFileInfo.moduleInfo.path +} diff --git a/packages/core/src/lib/checks/tests/check-for-deep-imports.barrel-less.spec.ts b/packages/core/src/lib/checks/tests/check-for-deep-imports.barrel-less.spec.ts new file mode 100644 index 0000000..95ea0d4 --- /dev/null +++ b/packages/core/src/lib/checks/tests/check-for-deep-imports.barrel-less.spec.ts @@ -0,0 +1,182 @@ +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 { UserSheriffConfig } from '../../config/user-sheriff-config'; +import { traverseFileInfo } from '../../modules/traverse-file-info'; + +describe('barrel-less', () => { + it('should check for deep imports', () => { + assertProject() + .withCustomerRoute({ + feature: { + internal: { + 'customer-sub.component.ts': [], + }, + 'customer.component.ts': [ + './internal/customer-sub.component.ts', + '../data/customer.service.ts', + ], + 'customers.component.ts': ['../data/internal/hidden.service.ts'], + }, + data: { + 'customer.service.ts': ['./internal/hidden.service.ts'], + internal: { 'hidden.service.ts': [] }, + }, + }) + .hasDeepImports({ + 'feature/customers.component.ts': [ + '../data/internal/hidden.service.ts', + ], + }); + }); + + it('should also work with nested paths inside internal', () => { + assertProject() + .withCustomerRoute({ + feature: { + internal: { + 'customer-sub.component.ts': [], + }, + 'customer.component.ts': [ + './internal/customer-sub.component.ts', + '../data/customer.service.ts', + ], + 'customers.component.ts': [ + '../data/internal/services/hidden.service.ts', + ], + }, + data: { + 'customer.service.ts': ['./internal/services/hidden.service.ts'], + internal: { services: { 'hidden.service.ts': [] } }, + }, + }) + .hasDeepImports({ + 'feature/customers.component.ts': [ + '../data/internal/services/hidden.service.ts', + ], + }); + }); + + it('should also work with nested paths outside of internal', () => { + assertProject() + .withCustomerRoute({ + feature: { + internal: { + 'customer-sub.component.ts': [], + }, + 'customers.component.ts': ['./components/customers-sub.component.ts'], + 'customer.component.ts': ['../data/services/customer.service.ts'], + components: { + 'customers-sub.component.ts': [ + '../internal/customer-sub.component.ts', + '../../data/internal/hidden.service.ts', + ], + }, + }, + data: { + services: { + 'customer.service.ts': ['../internal/hidden.service.ts'], + }, + internal: { 'hidden.service.ts': [] }, + }, + }) + .hasDeepImports({ + 'feature/components/customers-sub.component.ts': [ + '../../data/internal/hidden.service.ts', + ], + }); + }); + + it('should be able to change the name of internals', () => { + assertProject({ + encapsulatedFolderNameForBarrelLess: 'private', + }) + .withCustomerRoute({ + feature: { + 'customer.component.ts': ['../data/internal/customer.service.ts'], + 'customers.component.ts': ['../data/private/hidden.service.ts'], + }, + data: { + private: { 'hidden.service.ts': [] }, + internal: { 'customer.service.ts': ['../private/hidden.service.ts'] }, + }, + }) + .hasDeepImports({ + 'feature/customers.component.ts': ['../data/private/hidden.service.ts'], + }); + }); + + it('should always prioritize the barrel file', () => { + assertProject({ showWarningOnBarrelCollision: false }) + .withCustomerRoute({ + feature: { + 'customer.component.ts': ['../data'], + 'customers.component.ts': ['../data/open.service.ts'], + }, + data: { + 'index.ts': ['open.service.ts'], + 'open.service.ts': [], + internal: { 'hidden.service.ts': [] }, + }, + }) + .hasDeepImports({ + 'feature/customers.component.ts': ['../data/open.service.ts'], + }); + }); +}); + +function assertProject(config: Partial = {}) { + return { + withCustomerRoute(customerFileTree: FileTree) { + return { + hasDeepImports(deepImports: Record = {}) { + const projectInfo = testInit('src/main.ts', { + 'tsconfig.json': tsConfig(), + 'sheriff.config.ts': sheriffConfig({ + ...{ + tagging: { + 'src/app//': ['domain:', 'type:'], + }, + depRules: {}, + enableBarrelLess: true, + }, + ...config, + }), + src: { + 'main.ts': ['./app/app.routes'], + app: { + 'app.routes.ts': [ + './customer/feature/customer.component.ts', + './customer/feature/customers.component.ts', + ], + customer: customerFileTree, + }, + }, + }); + + for (const { fileInfo } of traverseFileInfo(projectInfo.fileInfo)) { + expect( + fileInfo.hasUnresolvedImports(), + `${fileInfo.path} has unresolved imports`, + ).toBe(false); + + const pathToLookup = fileInfo.path.replace( + '/project/src/app/customer/', + '', + ); + + const expectedDeepImports = deepImports[pathToLookup] || []; + expect + .soft( + checkForDeepImports(fileInfo.path, projectInfo), + `deep imports check failed for ${fileInfo.path}`, + ) + .toEqual(expectedDeepImports); + } + }, + }; + }, + }; +} diff --git a/packages/core/src/lib/checks/tests/check-for-deep-imports.spec.ts b/packages/core/src/lib/checks/tests/check-for-deep-imports.spec.ts index f073df8..a11a77c 100644 --- a/packages/core/src/lib/checks/tests/check-for-deep-imports.spec.ts +++ b/packages/core/src/lib/checks/tests/check-for-deep-imports.spec.ts @@ -32,7 +32,7 @@ describe('check deep imports', () => { ]) { expect( checkForDeepImports(toFsPath(`/project/${filename}`), projectInfo), - `failed deep import test for ${filename}` + `failed deep import test for ${filename}`, ).toEqual(deepImports); } }); diff --git a/packages/core/src/lib/checks/tests/check-for-dependency-rule-violation.spec.ts b/packages/core/src/lib/checks/tests/check-for-dependency-rule-violation.spec.ts index 940d2b5..66195f8 100644 --- a/packages/core/src/lib/checks/tests/check-for-dependency-rule-violation.spec.ts +++ b/packages/core/src/lib/checks/tests/check-for-dependency-rule-violation.spec.ts @@ -547,6 +547,58 @@ describe('check for dependency rule violation', () => { toFsPath('/project/src/customers/feature/customer.component.ts'), projectInfo, ); - expect(violationsForSubFolder).toHaveLength(1) + expect(violationsForSubFolder).toHaveLength(1); + }); + + describe('barrel less', () => { + const setup = (addRuleForRoot: boolean) => { + return testInit('src/main.ts', { + 'tsconfig.json': tsConfig(), + 'sheriff.config.ts': sheriffConfig({ + tagging: { + 'src/customers': ['domain:customers', 'type:domain'], + 'src/customers/': ['domain:customers', 'type:'], + }, + depRules: { + 'domain:customers': sameTag, + 'type:domain': ['type:feature'], + 'type:feature': [], + root: addRuleForRoot ? ['domain:*', 'type:feature'] : [], + }, + enableBarrelLess: true, + }), + src: { + 'main.ts': ['./customers/feature/customer.component.ts'], + customers: { + feature: { + 'customer.routes.ts': ['./customer.component.ts'], + 'customer.component.ts': ['../data/customer.service.ts'], + }, + data: { + 'customer.service.ts': [], + }, + }, + }, + }); + }; + + it('should test a default case', () => { + const projectInfo = setup(true); + const violationsForSuperFolder = checkForDependencyRuleViolation( + toFsPath('/project/src/main.ts'), + projectInfo, + ); + expect(violationsForSuperFolder).toEqual([]); + }); + + it('should fail for dependency violation', () => { + const projectInfo = setup(false); + const violations = checkForDependencyRuleViolation( + toFsPath('/project/src/main.ts'), + projectInfo, + ); + expect(violations).toHaveLength(1); + expect(violations[0]).toMatchObject({fromTag: 'root', toTags: ['domain:customers', 'type:feature']}); + }); }); }); diff --git a/packages/core/src/lib/checks/tests/deep-import-with-exclude-root.spec.ts b/packages/core/src/lib/checks/tests/deep-import-with-exclude-root.spec.ts new file mode 100644 index 0000000..2be46c8 --- /dev/null +++ b/packages/core/src/lib/checks/tests/deep-import-with-exclude-root.spec.ts @@ -0,0 +1,82 @@ +import { describe, expect, it } from 'vitest'; +import { anyTag, hasDeepImport } 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('deep imports and excludeRoot config property', () => { + for (const { excludeRoot, enableBarrelLess, hasDeepImport } of [ + { excludeRoot: true, enableBarrelLess: false, hasDeepImport: false }, + { excludeRoot: false, enableBarrelLess: false, hasDeepImport: true }, + { excludeRoot: true, enableBarrelLess: true, hasDeepImport: false }, + { excludeRoot: false, enableBarrelLess: true, hasDeepImport: false }, + ]) { + it(`should be ${ + hasDeepImport ? 'valid' : 'invalid' + } for rootExcluded: ${excludeRoot} and barrel-less: ${enableBarrelLess}`, () => { + testInit('src/main.ts', { + 'tsconfig.json': tsConfig(), + 'sheriff.config.ts': sheriffConfig({ + tagging: { 'src/shared': 'shared' }, + depRules: { '*': anyTag }, + excludeRoot, + enableBarrelLess, + }), + src: { + 'main.ts': '', + 'router.ts': ['./shared/dialog'], // always deep import + '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}, + }, + }, + }); + + assertDeepImport('/project/src/router.ts', './shared/dialog'); + assertDeepImport( + '/project/src/holidays/holidays-component.ts', + '../config', + false, + ); + + assertDeepImport( + '/project/src/shared/get.ts', + '../config', + hasDeepImport, + ); + + assertDeepImport( + '/project/src/shared/get.ts', + '../holidays/holidays-component', + hasDeepImport, + ); + }); + } +}); + +function assertDeepImport( + filename: string, + importCommand: string, + isDeepImport = true, +) { + expect( + hasDeepImport( + filename, + importCommand, + true, + getFs().readFile(toFsPath(filename)), + ), + `deep import in ${filename} from ${importCommand} should be ${isDeepImport}`, + ).toBe( + isDeepImport + ? "Deep import is not allowed. Use the module's index.ts or path." + : '', + ); +} diff --git a/packages/core/src/lib/config/default-config.ts b/packages/core/src/lib/config/default-config.ts index db7db13..5b52a4b 100644 --- a/packages/core/src/lib/config/default-config.ts +++ b/packages/core/src/lib/config/default-config.ts @@ -8,6 +8,7 @@ export const defaultConfig: SheriffConfig = { excludeRoot: false, enableBarrelLess: false, encapsulatedFolderNameForBarrelLess: 'internal', + showWarningOnBarrelCollision: true, log: false, entryFile: '', isConfigFileMissing: false, diff --git a/packages/core/src/lib/config/tests/parse-config.spec.ts b/packages/core/src/lib/config/tests/parse-config.spec.ts index cc2f5bf..58db5e0 100644 --- a/packages/core/src/lib/config/tests/parse-config.spec.ts +++ b/packages/core/src/lib/config/tests/parse-config.spec.ts @@ -28,7 +28,8 @@ describe('parse Config', () => { 'depRules', 'excludeRoot', 'enableBarrelLess', - "encapsulatedFolderNameForBarrelLess", + 'encapsulatedFolderNameForBarrelLess', + 'showWarningOnBarrelCollision', 'log', 'entryFile', 'isConfigFileMissing', @@ -69,6 +70,7 @@ export const config: SheriffConfig = { depRules: { noTag: 'noTag' }, enableBarrelLess: false, encapsulatedFolderNameForBarrelLess: 'internal', + showWarningOnBarrelCollision: true, excludeRoot: false, log: false, isConfigFileMissing: false, diff --git a/packages/core/src/lib/config/user-sheriff-config.ts b/packages/core/src/lib/config/user-sheriff-config.ts index 2d1707b..c78250c 100644 --- a/packages/core/src/lib/config/user-sheriff-config.ts +++ b/packages/core/src/lib/config/user-sheriff-config.ts @@ -132,6 +132,12 @@ export interface UserSheriffConfig { */ encapsulatedFolderNameForBarrelLess?: string; + /** + * Show warning message when a module has both a barrel file + * and an "internal" (barrel-less) folder + */ + showWarningOnBarrelCollision?: boolean + /** * enable internal logging and save it to `sheriff.log` */ diff --git a/packages/core/src/lib/eslint/deep-import.spec.ts b/packages/core/src/lib/eslint/deep-import.spec.ts index cc4085a..e407282 100644 --- a/packages/core/src/lib/eslint/deep-import.spec.ts +++ b/packages/core/src/lib/eslint/deep-import.spec.ts @@ -1,8 +1,6 @@ import { beforeAll, beforeEach, describe, expect, it } from 'vitest'; -import { FileTree, sheriffConfig } from '../test/project-configurator'; import getFs, { useVirtualFs } from '../fs/getFs'; import { hasDeepImport } from './deep-import'; -import { anyTag } from '../checks/any-tag'; import { toFsPath } from '../file-info/fs-path'; import { testInit } from '../test/test-init'; import { tsConfig } from '../test/fixtures/ts-config'; @@ -76,59 +74,4 @@ describe('deep import', () => { ), ).toBe('import ./app/app.component cannot be resolved'); }); - - describe('rootExcluded', () => { - const createFileTree = (excludeRoot?: boolean): FileTree => ({ - 'tsconfig.json': tsConfig(), - 'sheriff.config.ts': sheriffConfig({ - tagging: { 'src/shared': 'shared' }, - depRules: { '*': anyTag }, - excludeRoot, - }), - src: { - 'main.ts': '', - 'router.ts': ['./shared/dialog'], // always deep import - '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}, - }, - }, - }); - - for (const { excludeRoot, outcome } of [ - { excludeRoot: true, outcome: true }, - { excludeRoot: false, outcome: false }, - { excludeRoot: undefined, outcome: false }, - ]) { - it(`should be ${ - outcome ? 'valid' : 'invalid' - } for rootExcluded: ${excludeRoot}`, () => { - testInit('src/main.ts', createFileTree(excludeRoot)); - - assertDeepImport('/project/src/router.ts', './shared/dialog'); - assertDeepImport( - '/project/src/holidays/holidays-component.ts', - '../config', - false, - ); - - assertDeepImport( - '/project/src/shared/get.ts', - '../config', - !excludeRoot, - ); - - assertDeepImport( - '/project/src/shared/get.ts', - '../holidays/holidays-component', - !excludeRoot, - ); - }); - } - }); }); diff --git a/packages/core/src/lib/main/parse-project.ts b/packages/core/src/lib/main/parse-project.ts index 40146d5..837868c 100644 --- a/packages/core/src/lib/main/parse-project.ts +++ b/packages/core/src/lib/main/parse-project.ts @@ -43,14 +43,19 @@ export const parseProject = ( rootDir, config ); + const modules = createModules( - unassignedFileInfo, modulePaths, - rootDir, fileInfoMap, getFileInfo, - config.barrelFileName, - ); + { + entryFileInfo: unassignedFileInfo, + rootDir, + barrelFile: config.barrelFileName, + encapsulatedFolderName: config.encapsulatedFolderNameForBarrelLess, + showWarningOnBarrelFileLessCollision: config.showWarningOnBarrelCollision, + }, + ) fillFileInfoMap(fileInfoMap, modules); const fileInfo = getFileInfo(unassignedFileInfo.path); diff --git a/packages/core/src/lib/modules/create-modules.ts b/packages/core/src/lib/modules/create-modules.ts index 45a1edd..9f68211 100644 --- a/packages/core/src/lib/modules/create-modules.ts +++ b/packages/core/src/lib/modules/create-modules.ts @@ -5,20 +5,46 @@ import throwIfNull from '../util/throw-if-null'; import { FsPath, toFsPath } from '../file-info/fs-path'; import { FileInfo } from './file.info'; import { ModulePathMap } from './find-module-paths'; -import { entries, fromEntries, keys, values } from "../util/typed-object-functions"; +import { + entries, + fromEntries, + keys, + values, +} from '../util/typed-object-functions'; +import { warnOnBarrelFileLessCollision } from './warn-on-barrel-file-less-collision'; + +interface CreateModulesContext { + entryFileInfo: UnassignedFileInfo; + rootDir: FsPath; + barrelFile: string; + encapsulatedFolderName: string; + showWarningOnBarrelFileLessCollision: boolean; +} export function createModules( - entryFileInfo: UnassignedFileInfo, modulePathMap: ModulePathMap, - rootDir: FsPath, fileInfoMap: Map, getFileInfo: (path: FsPath) => FileInfo, - barrelFile: string + { + entryFileInfo, + rootDir, + barrelFile, + encapsulatedFolderName, + showWarningOnBarrelFileLessCollision, + }: CreateModulesContext, ): Module[] { const moduleMap = fromEntries( entries(modulePathMap).map(([path, hasBarrel]) => [ path, - new Module(toFsPath(path), fileInfoMap, getFileInfo, false, hasBarrel, barrelFile), + new Module( + toFsPath(path), + fileInfoMap, + getFileInfo, + false, + hasBarrel, + barrelFile, + encapsulatedFolderName, + ), ]), ); // add root module @@ -28,7 +54,8 @@ export function createModules( getFileInfo, true, false, - barrelFile + barrelFile, + encapsulatedFolderName, ); const modulePaths = keys(moduleMap); @@ -38,7 +65,12 @@ export function createModules( moduleMap[modulePath].addFileInfo(fileInfo); } - return values(moduleMap); + const modules = values(moduleMap); + if (showWarningOnBarrelFileLessCollision) { + warnOnBarrelFileLessCollision(modules, barrelFile, encapsulatedFolderName); + } + + return modules; } function findClosestModulePath(path: string, modulePaths: FsPath[]) { diff --git a/packages/core/src/lib/modules/file.info.ts b/packages/core/src/lib/modules/file.info.ts index e383d5e..6dbe490 100644 --- a/packages/core/src/lib/modules/file.info.ts +++ b/packages/core/src/lib/modules/file.info.ts @@ -43,6 +43,10 @@ export class FileInfo { return this.unassignedFileInfo.isUnresolvableImport(importCommand); } + hasUnresolvedImports() { + return this.unassignedFileInfo.hasUnresolvableImports(); + } + getExternalLibraries() { return this.unassignedFileInfo.getExternalLibraries(); } diff --git a/packages/core/src/lib/modules/find-module-paths.ts b/packages/core/src/lib/modules/find-module-paths.ts index 7ff2da7..3b70eb8 100644 --- a/packages/core/src/lib/modules/find-module-paths.ts +++ b/packages/core/src/lib/modules/find-module-paths.ts @@ -19,7 +19,7 @@ export function findModulePaths( const { tagging, enableBarrelLess, - barrelFileName, + barrelFileName } = sheriffConfig; const modulesWithoutBarrel = enableBarrelLess ? findModulePathsWithoutBarrel(tagging, rootDir, barrelFileName) diff --git a/packages/core/src/lib/modules/module.ts b/packages/core/src/lib/modules/module.ts index 5c66c7a..8994475 100644 --- a/packages/core/src/lib/modules/module.ts +++ b/packages/core/src/lib/modules/module.ts @@ -9,7 +9,8 @@ import getFs from '../fs/getFs'; * approach here. */ export class Module { - fileInfos: FileInfo[] = []; + readonly fileInfos: FileInfo[] = []; + readonly #encapsulatedFolderPath: FsPath | undefined; constructor( public readonly path: FsPath, @@ -18,7 +19,17 @@ export class Module { public readonly isRoot: boolean, public readonly hasBarrel: boolean, private readonly barrelFile: string, - ) {} + private readonly encapsulatedFolderName: string, + ) { + const fs = getFs(); + const possibleEncapsulatedFolderPath = fs.join( + path, + encapsulatedFolderName, + ); + if (fs.exists(possibleEncapsulatedFolderPath)) { + this.#encapsulatedFolderPath = possibleEncapsulatedFolderPath; + } + } addFileInfo(unassignedFileInfo: UnassignedFileInfo) { const fileInfo = new FileInfo(unassignedFileInfo, this, this.getFileInfo); @@ -29,4 +40,13 @@ export class Module { get barrelPath(): FsPath { return toFsPath(getFs().join(this.path, this.barrelFile)); } + + /** + * return the path for the encapsulated (internal) folder. + * If the module exposes everything, that folder might not exist. + * It returns even if the module is a barrel module. + */ + getEncapsulatedFolder(): FsPath | undefined { + return this.#encapsulatedFolderPath; + } } diff --git a/packages/core/src/lib/modules/tests/create-module.spec.ts b/packages/core/src/lib/modules/tests/create-module.spec.ts index 341697b..8b14d88 100644 --- a/packages/core/src/lib/modules/tests/create-module.spec.ts +++ b/packages/core/src/lib/modules/tests/create-module.spec.ts @@ -2,13 +2,26 @@ import { UnassignedFileInfo } from '../../file-info/unassigned-file-info'; import { createModules } from '../create-modules'; import findFileInfo from '../../test/find-file-info'; import { Module } from '../module'; -import { afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { + afterEach, + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vitest, +} from 'vitest'; import throwIfNull from '../../util/throw-if-null'; import getFs, { useVirtualFs } from '../../fs/getFs'; import { FsPath, toFsPath } from '../../file-info/fs-path'; import { FileInfo } from '../file.info'; import { buildFileInfo } from '../../test/build-file-info'; import { fromEntries } from '../../util/typed-object-functions'; +import { testInit } from '../../test/test-init'; +import { tsConfig } from '../../test/fixtures/ts-config'; +import { sheriffConfig } from '../../test/project-configurator'; +import { ModulePathMap } from '../find-module-paths'; interface TestParameter { fileInfo: UnassignedFileInfo; @@ -24,6 +37,7 @@ describe('createModule', () => { afterEach(() => { getFs().reset(); }); + it('should create module for simple configuration', () => { assertModule(() => ({ fileInfo: buildFileInfo('/src/app/app.component.ts', [ @@ -199,6 +213,52 @@ describe('createModule', () => { ], })); }); + + describe('warning message on barrel collision', () => { + const warnSpy = vitest.spyOn(console, 'warn'); + + beforeEach(() => { + warnSpy.mockReset(); + }); + + afterAll(() => { + warnSpy.mockRestore(); + }); + + [true, false].forEach((showWarningOnBarrelCollision) => { + it(`should${showWarningOnBarrelCollision ? ' ' : ' not '}show warnings on barrel collision`, () => { + testInit('src/main.ts', { + src: { + 'tsconfig.json': tsConfig(), + 'sheriff.config.ts': sheriffConfig({ + tagging: { customer: 'domain:customer' }, + depRules: {}, + enableBarrelLess: true, + showWarningOnBarrelCollision, + }), + 'main.ts': ['./customer'], + customer: { + 'index.ts': [], + internal: {}, + }, + }, + }); + + if (showWarningOnBarrelCollision) { + expect(warnSpy).toHaveBeenNthCalledWith( + 1, + `Module /project/src/customer has both a barrel file (index.ts) and an encapsulated folder (internal)`, + ); + expect(warnSpy).toHaveBeenNthCalledWith( + 2, + `You can disable this warning by setting the property warnOnBarrelFileLessCollision in 'sheriff.config.ts' to false`, + ); + } else { + expect(warnSpy).not.toHaveBeenCalled(); + } + }); + }); + }); }); function assertModule(createTestParams: () => TestParameter) { @@ -214,18 +274,17 @@ function assertModule(createTestParams: () => TestParameter) { barrelFiles.forEach((modulePath) => { getFs().writeFile(modulePath, ''); }); - const modules = createModules( - fileInfo, - fromEntries( - barrelFiles.map((path) => [ - toFsPath(path.replace('/index.ts', '')), - true, - ]), - ), - toFsPath('/'), - fileInfoMap, - getFileInfo, + + const modulePathMap: ModulePathMap = fromEntries( + barrelFiles.map((path) => [toFsPath(path.replace('/index.ts', '')), true]), ); + const modules = createModules(modulePathMap, fileInfoMap, getFileInfo, { + entryFileInfo: fileInfo, + rootDir: toFsPath('/'), + barrelFile: 'index.ts', + encapsulatedFolderName: 'internals', + showWarningOnBarrelFileLessCollision: true, + }); const expectedModules = testParams.expectedModules.map((mi) => { const fileInfos = mi.fileInfoPaths.map((fip) => @@ -240,6 +299,8 @@ function assertModule(createTestParams: () => TestParameter) { getFileInfo, mi.path === '/', mi.path !== '/', + 'index.ts', + 'internals', ); for (const fi of fileInfos) { module.addFileInfo(fi); diff --git a/packages/core/src/lib/modules/warn-on-barrel-file-less-collision.ts b/packages/core/src/lib/modules/warn-on-barrel-file-less-collision.ts new file mode 100644 index 0000000..38beac1 --- /dev/null +++ b/packages/core/src/lib/modules/warn-on-barrel-file-less-collision.ts @@ -0,0 +1,19 @@ +import { Module } from './module'; + +export function warnOnBarrelFileLessCollision(modules: Module[], barrelFileName: string, encapsulatedFolderName: string): void { + const barrelModulesWithEncapsulatedFolder = modules + .filter((module) => module.hasBarrel) + .filter((module) => module.getEncapsulatedFolder() !== undefined); + + for (const module of barrelModulesWithEncapsulatedFolder) { + console.warn( + `Module ${module.path} has both a barrel file (${barrelFileName}) and an encapsulated folder (${encapsulatedFolderName})`, + ); + } + + if (barrelModulesWithEncapsulatedFolder.length > 0) { + console.warn( + `You can disable this warning by setting the property warnOnBarrelFileLessCollision in 'sheriff.config.ts' to false`, + ); + } +}