From f27e4c40dc03834f879a94d59e499cfaec00152e Mon Sep 17 00:00:00 2001 From: chwoerz Date: Sun, 10 Sep 2023 21:44:10 +0200 Subject: [PATCH] added custom message possibility --- .../lib/checks/is-dependency-allowed.spec.ts | 37 ++--- .../src/lib/checks/is-dependency-allowed.ts | 98 +++++++++---- .../src/lib/config/dependency-rules-config.ts | 9 +- .../lib/eslint/violates-dependency-rule.ts | 131 +++++++++--------- packages/core/src/lib/util/to-array.ts | 3 + 5 files changed, 172 insertions(+), 106 deletions(-) create mode 100644 packages/core/src/lib/util/to-array.ts diff --git a/packages/core/src/lib/checks/is-dependency-allowed.spec.ts b/packages/core/src/lib/checks/is-dependency-allowed.spec.ts index d939fcd4..d7921637 100644 --- a/packages/core/src/lib/checks/is-dependency-allowed.spec.ts +++ b/packages/core/src/lib/checks/is-dependency-allowed.spec.ts @@ -1,4 +1,4 @@ -import { describe, expect, test, it } from 'vitest'; +import { describe, expect, it, test } from 'vitest'; import { DependencyCheckContext, DependencyRulesConfig, @@ -25,23 +25,26 @@ describe('check dependency rules', () => { expect( isDependencyAllowed(['type:feature'], 'type:ui', config, dummyContext) - ).toBe(true); + ).toEqual({ allowed: true }); expect( isDependencyAllowed(['type:ui'], 'type:feature', config, dummyContext) - ).toBe(false); + ).toEqual({ allowed: false }); }); test('multiple string rules', () => { const config: DependencyRulesConfig = { - 'type:feature': ['type:data', 'type:ui'], + 'type:feature': { + matcher: ['type:data', 'type:ui'], + message: (to) => `not allowed for ${to}`, + }, }; expect( isDependencyAllowed(['type:feature'], 'type:ui', config, dummyContext) - ).toBe(true); + ).toEqual({ allowed: true }); expect( isDependencyAllowed(['type:feature'], 'domain:abc', config, dummyContext) - ).toBe(false); + ).toEqual({ allowed: false, customMessage: 'not allowed for domain:abc' }); }); for (const [to, isAllowed] of [ @@ -57,7 +60,7 @@ describe('check dependency rules', () => { expect( isDependencyAllowed(['type:feature'], to, config, dummyContext) - ).toBe(isAllowed); + ).toEqual({ allowed: isAllowed }); }); } @@ -72,7 +75,7 @@ describe('check dependency rules', () => { expect( isDependencyAllowed(['type:feature'], to, config, dummyContext) - ).toBe(isAllowed); + ).toEqual({ allowed: isAllowed }); }); } @@ -118,7 +121,7 @@ describe('check dependency rules', () => { }; expect( isDependencyAllowed(['domain:customers'], to, config, dummyContext) - ).toBe(isAllowed); + ).toEqual({ allowed: isAllowed }); }); } @@ -137,7 +140,7 @@ describe('check dependency rules', () => { }; expect( isDependencyAllowed(['domain:customers'], to, config, dummyContext) - ).toBe(isAllowed); + ).toEqual({ allowed: isAllowed }); }); } @@ -154,7 +157,7 @@ describe('check dependency rules', () => { config, dummyContext ) - ).toBe(true); + ).toEqual({ allowed: true }); }); it('should have access to a module when of the tags allow it', () => { @@ -170,7 +173,7 @@ describe('check dependency rules', () => { config, dummyContext ) - ).toBe(true); + ).toEqual({ allowed: true }); }); it('should allow wildcard in rule values as well', () => { @@ -180,7 +183,7 @@ describe('check dependency rules', () => { expect( isDependencyAllowed(['type:feature'], 'shared:ui', config, dummyContext) - ).toBe(true); + ).toEqual({ allowed: true }); }); for (const [to, from, isAllowed] of [ @@ -194,9 +197,9 @@ describe('check dependency rules', () => { 'domain:*': sameTag, }; - expect(isDependencyAllowed([from], to, config, dummyContext)).toBe( - isAllowed - ); + expect(isDependencyAllowed([from], to, config, dummyContext)).toEqual({ + allowed: isAllowed, + }); }); } @@ -209,7 +212,7 @@ describe('check dependency rules', () => { expect( isDependencyAllowed(['type:model'], toTag, config, dummyContext) - ).toBe(false); + ).toEqual({ allowed: false }); } ); }); diff --git a/packages/core/src/lib/checks/is-dependency-allowed.ts b/packages/core/src/lib/checks/is-dependency-allowed.ts index 7c428f00..534be702 100644 --- a/packages/core/src/lib/checks/is-dependency-allowed.ts +++ b/packages/core/src/lib/checks/is-dependency-allowed.ts @@ -1,43 +1,91 @@ import { DependencyCheckContext, DependencyRulesConfig, + Rule, + RuleMatcher, + RuleMatcherFn, + RuleWithCustomMessage, } from '../config/dependency-rules-config'; import { wildcardToRegex } from '../util/wildcard-to-regex'; +import toArray from '../util/to-array'; + +interface AllowedDependencyResponse { + allowed: boolean; + customMessage?: string; +} export const isDependencyAllowed = ( froms: string[], to: string, config: DependencyRulesConfig, context: DependencyCheckContext -): boolean => { - let isAllowed: boolean | undefined; +): AllowedDependencyResponse => { + let result: AllowedDependencyResponse | undefined = undefined; for (const from of froms) { - isAllowed = undefined; - for (const tag in config) { - if (from.match(wildcardToRegex(tag))) { - const value = config[tag]; - const matchers = Array.isArray(value) ? value : [value]; - for (const matcher of matchers) { - if ( - typeof matcher === 'string' && - to.match(wildcardToRegex(matcher)) - ) { - return true; - } else if ( - typeof matcher === 'function' && - matcher({ from, to, ...context }) - ) { - return true; - } - } - isAllowed = false; - } - } + const resultForFrom = findDependencyRuleForFrom(from, to, context, config); - if (isAllowed === undefined) { + if (resultForFrom === undefined) { throw new Error(`cannot find any dependency rule for tag ${from}`); } + result = resultForFrom; + if (resultForFrom?.allowed) { + return resultForFrom; + } + } + + return result as AllowedDependencyResponse; +}; + +const findDependencyRuleForFrom = ( + from: string, + to: string, + context: DependencyCheckContext, + config: DependencyRulesConfig +): AllowedDependencyResponse | undefined => { + let isAllowed: AllowedDependencyResponse | undefined; + + for (const [tag, rule] of Object.entries(config)) { + if (from.match(wildcardToRegex(tag))) { + const ruleWithCustomMessage = toRuleWithCustomMessage(rule); + const matchers = toArray(ruleWithCustomMessage.matcher); + + const foundMatcher = matchers.find(testMatch(from, to, context)); + if (foundMatcher) { + return { allowed: true }; + } + // the current matcher does not allow it. + // But we may have other matchers which might fit. So dont return immediately + isAllowed = { + allowed: false, + customMessage: ruleWithCustomMessage.message(to), + }; + } } + return isAllowed; +}; - return false; +const testMatch = + (from: string, to: string, context: DependencyCheckContext) => + (matcher: RuleMatcher) => + (isRuleMatcher(matcher) && to.match(wildcardToRegex(matcher))) || + (isRuleMatcherFn(matcher) && matcher({ from, to, ...context })); + +const isRuleMatcherFn = (matcher: Rule): matcher is RuleMatcherFn => + typeof matcher === 'function'; +const isRuleMatcher = (matcher: Rule): matcher is string => + typeof matcher === 'string'; + +const isRuleWithCustomMessage = ( + matcher: Rule +): matcher is RuleWithCustomMessage => + !!(matcher as RuleWithCustomMessage).matcher; + +const toRuleWithCustomMessage = (matcher: Rule): RuleWithCustomMessage => { + if (isRuleWithCustomMessage(matcher)) { + return matcher; + } + return { + matcher, + message: () => undefined, + }; }; diff --git a/packages/core/src/lib/config/dependency-rules-config.ts b/packages/core/src/lib/config/dependency-rules-config.ts index bc0d4eb5..0b8cc73e 100644 --- a/packages/core/src/lib/config/dependency-rules-config.ts +++ b/packages/core/src/lib/config/dependency-rules-config.ts @@ -13,5 +13,12 @@ export type RuleMatcherFn = ( to: string; } & DependencyCheckContext ) => boolean; +type CustomMessageFn = (to: string) => string | undefined; + +export interface RuleWithCustomMessage { + message: CustomMessageFn; + matcher: RuleMatcher | RuleMatcher[]; +} +export type Rule = RuleWithCustomMessage | RuleMatcher[] | RuleMatcher; export type RuleMatcher = string | null | RuleMatcherFn; -export type DependencyRulesConfig = Record; +export type DependencyRulesConfig = Record; diff --git a/packages/core/src/lib/eslint/violates-dependency-rule.ts b/packages/core/src/lib/eslint/violates-dependency-rule.ts index b648ed84..fe9b86ec 100644 --- a/packages/core/src/lib/eslint/violates-dependency-rule.ts +++ b/packages/core/src/lib/eslint/violates-dependency-rule.ts @@ -21,81 +21,86 @@ export const violatesDependencyRule = ( if (isFirstRun) { cache.clear(); } - if (!cache.has(importCommand)) { - const { fileInfo, rootDir } = generateFileInfoAndGetRootDir( - toFsPath(filename), - true - ); - const configFile = findConfig(rootDir); - if (configFile === undefined) { - log('Dependency Rules', 'no sheriff.config.ts present in ' + rootDir); - return ''; - } + if (cache.has(importCommand)) { + return cache.get(importCommand) || ''; + } + const { fileInfo, rootDir } = generateFileInfoAndGetRootDir( + toFsPath(filename), + true + ); + const configFile = findConfig(rootDir); + if (configFile === undefined) { + log('Dependency Rules', 'no sheriff.config.ts present in ' + rootDir); + return ''; + } - const config = parseConfig(configFile); + const config = parseConfig(configFile); - const projectDirs = getProjectDirsFromFileInfo(fileInfo, rootDir); + const projectDirs = getProjectDirsFromFileInfo(fileInfo, rootDir); - const modulePaths = findModulePaths(projectDirs); - const modules = createModules(fileInfo, modulePaths, rootDir); - const afiMap = getAssignedFileInfoMap(modules); + const modulePaths = findModulePaths(projectDirs); + const modules = createModules(fileInfo, modulePaths, rootDir); + const afiMap = getAssignedFileInfoMap(modules); - const getAfi = (path: FsPath) => - throwIfNull(afiMap.get(path), `cannot find AssignedFileInfo for ${path}`); + const getAfi = (path: FsPath) => + throwIfNull(afiMap.get(path), `cannot find AssignedFileInfo for ${path}`); - const assignedFileInfo = getAfi(fileInfo.path); - const importedModulePathsWithRawImport = assignedFileInfo.imports - .filter((importedFi) => modulePaths.has(importedFi.path)) - .map((importedFi) => getAfi(importedFi.path)) - .map((iafi) => [ - iafi.moduleInfo.directory, - assignedFileInfo.getRawImportForImportedFileInfo(iafi.path), - ]); - const fromModule = toFsPath( - getAfi(toFsPath(filename)).moduleInfo.directory + const assignedFileInfo = getAfi(fileInfo.path); + const importedModulePathsWithRawImport = assignedFileInfo.imports + .filter((importedFi) => modulePaths.has(importedFi.path)) + .map((importedFi) => getAfi(importedFi.path)) + .map((iafi) => [ + iafi.moduleInfo.directory, + assignedFileInfo.getRawImportForImportedFileInfo(iafi.path), + ]); + const fromModulePath = toFsPath( + getAfi(toFsPath(filename)).moduleInfo.directory + ); + const fromTags = calcTagsForModule(fromModulePath, rootDir, config.tagging); + const removeRoot = (module: string) => module.substring(rootDir.length); + const fromTagString = fromTags.join(', '); + for (const [ + importedModulePath, + rawImport, + ] of importedModulePathsWithRawImport) { + const importedModuleFsPath = toFsPath(importedModulePath); + const toTags: string[] = calcTagsForModule( + importedModuleFsPath, + rootDir, + config.tagging ); - const fromTags = calcTagsForModule(fromModule, rootDir, config.tagging); - for (const [ - importedModulePath, - rawImport, - ] of importedModulePathsWithRawImport) { - const toTags: string[] = calcTagsForModule( - toFsPath(importedModulePath), - rootDir, - config.tagging - ); + log( + 'Dependency Rules', + `Checking for from tags of ${fromTagString} to ${toTags.join(',')}` + ); - log( - 'Dependency Rules', - `Checking for from tags of ${fromTags.join(',')} to ${toTags.join(',')}` + for (const toTag of toTags) { + const { allowed, customMessage } = isDependencyAllowed( + fromTags, + toTag, + config.depRules, + { + fromModulePath, + toModulePath: importedModuleFsPath, + fromFilePath: toFsPath(filename), + toFilePath: importedModuleFsPath, + } ); + if (!allowed) { + cache.set( + rawImport, + `module ${removeRoot(fromModulePath)} cannot access ${removeRoot( + importedModulePath + )}. Tags [${fromTagString}] have no clearance for ${toTag}${customMessage}` + ); - for (const toTag of toTags) { - if ( - !isDependencyAllowed(fromTags, toTag, config.depRules, { - fromModulePath: fromModule, - toModulePath: toFsPath(importedModulePath), - fromFilePath: toFsPath(filename), - toFilePath: toFsPath(importedModulePath), - }) - ) { - cache.set( - rawImport, - `module ${fromModule.substring( - rootDir.length - )} cannot access ${importedModulePath.substring( - rootDir.length - )}. Tags [${fromTags.join(',')}] have no clearance for ${toTag}` - ); - - break; - } + break; } + } - if (!cache.has(importCommand)) { - cache.set(importCommand, ''); - } + if (!cache.has(importCommand)) { + cache.set(importCommand, ''); } } diff --git a/packages/core/src/lib/util/to-array.ts b/packages/core/src/lib/util/to-array.ts new file mode 100644 index 00000000..bb84355a --- /dev/null +++ b/packages/core/src/lib/util/to-array.ts @@ -0,0 +1,3 @@ +export default function toArray(input: T | T[]) { + return Array.isArray(input) ? input : [input]; +}