diff --git a/docs/checks.md b/docs/checks.md index fdc8348..9a964f1 100644 --- a/docs/checks.md +++ b/docs/checks.md @@ -787,7 +787,12 @@ The contracts family needs a base revision, so it runs in diff mode. When `check ## Design Purpose: -- Layer boundary enforcement + +- Language-neutral architecture boundary enforcement +- Domain and data ownership protection +- Capability and public-surface encapsulation +- Production/test isolation +- Entrypoint reachability and dependency stability warnings - Separation of concerns heuristics - Clean-code naming heuristics - SOLID-oriented heuristics @@ -821,7 +826,10 @@ Config keys: ``` Current behavior: -- fails on architecture boundary violations + +- fails on configured layer, domain, data-ownership, capability, public-surface, and production/test boundary violations +- can require every module to belong to a configured layer and domain +- warns on configured unreachable modules and stability-direction inversions - Go targets keep the existing package, import-boundary, declaration-count, type-size, and interface-size heuristics - Python targets fail on public-to-private imports, direct or transitive entrypoint coupling, and internal import cycles, and warn on overly generic module names - TypeScript targets warn on overly generic module names, oversized classes, and oversized interfaces or object types using compiler-parsed AST analysis when the semantic runtime is available @@ -853,6 +861,81 @@ Language command example: } ``` +### Standalone design policy + +Large architecture policies can live outside the main config as a top-level +`DesignRulesConfig` document. Codeguard automatically looks for +`.codeguard/design_rules.yml`, then `.codeguard/design_rules.yaml`, relative to the +project/config root. When the main config is already inside `.codeguard`, the policy is +discovered beside it. A missing auto-discovered file is ignored. + +To choose another file, set `checks.design_rules_file`. Relative paths are resolved from +the directory containing the main config: + +```yaml +checks: + design: true + design_rules_file: policies/architecture.yml + design_rules: + # Optional local overrides of the shared policy. + stability: + enabled: false +``` + +An explicit file must exist and must remain within the project/config root, including +after symlinks are resolved. The external file contains the design-rule fields directly; +do not wrap them in `checks.design_rules`: + +```yaml +require_boundary_assignment: true +layers: + - name: domain + paths: ["internal/domain/**"] + may_depend_on: [contracts] + - name: contracts + paths: ["internal/contracts/**"] +``` + +The external policy is the base. Each top-level field explicitly present in inline +`checks.design_rules` replaces the corresponding external field, including explicit +`false`, `0`, and empty list values. Profile defaults fill fields that remain unspecified. +If both auto-discovery names exist, `design_rules.yml` takes precedence; keep only one to +avoid ambiguity. See the complete [standalone policy template](../examples/design_rules.yml). + +All architecture policies are opt-in and run only when `checks.design` is enabled. A +non-empty `layers`, `domains`, `capabilities`, or `public_surfaces` list enables that +policy. `production_test`, `reachability`, and `stability` become active when their +section is present unless `enabled: false` is set. `require_boundary_assignment` runs only +when explicitly true. Existing design heuristics and graph rules retain their normal +defaults. + +Paths use forward-slash, target-relative matching. A path without glob syntax matches +that path and its descendants; `*`, `?`, character classes, and `**` globs are supported. +External-import policies also match the language's import specifier. + +Architecture policy fields: + +| Policy | Fields | Behavior | +|---|---|---| +| Layer dependencies | `layers[].name`, `paths`, `may_depend_on`, `deny_depend_on`, `denied_external` | Restricts local dependencies by layer and denies selected external imports. Imports within the same layer are allowed. An empty `may_depend_on` does not create an allowlist. | +| Boundary assignment | `require_boundary_assignment` | Fails modules not covered by every configured boundary dimension: layer, domain, or both. | +| Domains and data ownership | `domains[].name`, `paths`, `public_paths`, `data_paths`, `may_depend_on` | Cross-domain imports must target an allowed domain's public path. Another domain's data paths always remain private. An empty `may_depend_on` denies cross-domain imports. | +| Capabilities | `capabilities[].name`, `imports`, `allowed_paths` | Limits database, network, filesystem, cloud SDK, or other sensitive imports to approved adapters. | +| Public surfaces | `public_surfaces[].name`, `paths`, `entrypoints` | Prevents consumers outside a component from deep-importing its private modules. Imports originating inside that surface are allowed. | +| Production/test isolation | `production_test.enabled`, `production_paths`, `test_paths` | Prevents production modules from importing test helpers, mocks, fixtures, or other test-only modules. | +| Reachability | `reachability.enabled`, `entrypoints`, `ignore_paths` | Warns when a production module cannot be reached from a configured entrypoint. `targets[].entrypoints` are included automatically. | +| Stability direction | `stability.enabled`, `minimum_fan_in`, `max_instability_delta`, `ignore_paths` | Warns when a stable, widely depended-on module imports a substantially less stable module. Defaults are fan-in `3` and instability delta `0.35`. | + +To migrate an existing inline policy: + +1. Copy the contents of `checks.design_rules` into `.codeguard/design_rules.yml`, without + the `checks` or `design_rules` wrapper. +2. Remove the copied inline fields. Auto-discovery needs no additional main-config key; + set `checks.design_rules_file` only for a different filename or location. +3. Keep small repository- or environment-specific inline fields when they should override + the shared policy, then run a full scan before enabling + `require_boundary_assignment` or reachability. + ## Security Purpose: diff --git a/docs/sdk.md b/docs/sdk.md index 9bb9517..4eede72 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -77,6 +77,24 @@ if err := codeguard.WriteReport(os.Stdout, report, "json"); err != nil { } ``` +### Loading standalone design policies + +`LoadConfigFile` also loads a standalone architecture policy. It auto-discovers +`.codeguard/design_rules.yml` or `.codeguard/design_rules.yaml`, or follows +`checks.design_rules_file` relative to the main config. The external document is loaded as +the base `DesignRulesConfig`; explicitly present inline `checks.design_rules` fields win. + +This behavior belongs to file loading. Setting `CheckConfig.DesignRulesFile` on a config +constructed entirely in memory and passing it directly to `Run` does not read the file. +For an in-memory config, populate `CheckConfig.DesignRules` directly using the exported +`DesignRulesConfig`, `DesignLayerConfig`, `DesignDomainConfig`, +`DesignCapabilityConfig`, `DesignPublicSurfaceConfig`, +`DesignProductionTestConfig`, `DesignReachabilityConfig`, and +`DesignStabilityConfig` types. + +See [the standalone policy template](../examples/design_rules.yml) for the complete YAML +schema and migration notes in the [Design checks guide](checks.md#standalone-design-policy). + ## Verified fix flow `VerifyFix` and `GenerateVerifiedFix` fail closed. They do not return a patch unless: diff --git a/examples/design_rules.yml b/examples/design_rules.yml new file mode 100644 index 0000000..be2bb3f --- /dev/null +++ b/examples/design_rules.yml @@ -0,0 +1,176 @@ +# Standalone Codeguard design policy template. +# +# Put this file at .codeguard/design_rules.yml for automatic discovery, or point +# checks.design_rules_file at it from the main Codeguard config. This document is +# the contents of DesignRulesConfig itself; it is not wrapped in `checks` or +# `design_rules`. +# +# Paths are relative to each target. Adapt these example paths to your repository +# before enabling strict assignment. + +# Start false while adopting a policy. Set true after every production module is +# covered by a configured layer and domain. +require_boundary_assignment: false + +layers: + - name: domain + paths: + - internal/domain/** + may_depend_on: + - contracts + denied_external: + - database/sql + - net/http + - github.com/aws/** + + - name: contracts + paths: + - internal/contracts/** + + - name: application + paths: + - internal/application/** + may_depend_on: + - domain + - contracts + + - name: adapters + paths: + - internal/adapters/** + may_depend_on: + - application + - domain + - contracts + deny_depend_on: + - delivery + + - name: delivery + paths: + - internal/http/** + - internal/workers/** + may_depend_on: + - application + - contracts + + - name: bootstrap + paths: + - cmd/** + - internal/bootstrap/** + may_depend_on: + - application + - adapters + - delivery + - contracts + +domains: + - name: orders + paths: + - internal/orders/** + public_paths: + - internal/orders/contracts/** + - internal/orders/events/** + data_paths: + - internal/orders/repository/** + - internal/orders/models/** + may_depend_on: + - identity + + - name: identity + paths: + - internal/identity/** + public_paths: + - internal/identity/contracts/** + data_paths: + - internal/identity/repository/** + - internal/identity/models/** + # Empty may_depend_on denies cross-domain imports for this domain. + +# Only these adapters may import the listed database or network capabilities. +# Import patterns can match local module paths or external package specifiers. +capabilities: + - name: database + imports: + - database/sql + - github.com/jackc/pgx/** + - sqlalchemy + - sqlalchemy/** + - "@prisma/client" + allowed_paths: + - internal/adapters/database/** + - internal/*/repository/** + + - name: outbound-network + imports: + - net/http + - requests + - axios + allowed_paths: + - internal/adapters/http/** + +# Consumers outside each component must import through an approved entrypoint. +public_surfaces: + - name: orders + paths: + - internal/orders/** + entrypoints: + - internal/orders/contracts/** + - internal/orders/events/** + + - name: identity + paths: + - internal/identity/** + entrypoints: + - internal/identity/contracts/** + +production_test: + enabled: true + production_paths: + - cmd/** + - internal/** + - src/** + test_paths: + - tests/** + - test/** + - "**/*_test.go" + - "**/*.test.ts" + - "**/*.spec.ts" + - "**/testdata/**" + - "**/fixtures/**" + - "**/mocks/**" + +reachability: + enabled: true + entrypoints: + - cmd/**/main.go + - src/index.ts + - src/main.py + ignore_paths: + - "**/*_test.go" + - tests/** + - "**/generated/**" + +stability: + enabled: true + minimum_fan_in: 3 + max_instability_delta: 0.35 + ignore_paths: + - "**/generated/**" + - "**/*_test.go" + +# Existing design heuristics and dependency-graph checks can live in this file +# too. Inline checks.design_rules values in the main config override the matching +# top-level fields here, including false, zero, and empty-list values. +detect_import_cycles: true +detect_god_modules: true +god_module_threshold: 24 +detect_high_impact_changes: true +high_impact_change_threshold: 20 +max_decls_per_file: 12 +max_methods_per_type: 8 +max_interface_methods: 5 +forbidden_package_names: + - util + - utils + - common + - helpers + - misc diff --git a/internal/codeguard/checks/design/design.go b/internal/codeguard/checks/design/design.go index 36930f6..b5a422d 100644 --- a/internal/codeguard/checks/design/design.go +++ b/internal/codeguard/checks/design/design.go @@ -16,6 +16,9 @@ func Run(ctx context.Context, env support.Context) core.SectionResult { if graph != nil { findings = append(findings, importCycleFindings(env, graph)...) findings = append(findings, godModuleFindings(env, graph)...) + findings = append(findings, architectureBoundaryFindings(env, target, graph)...) + findings = append(findings, encapsulationBoundaryFindings(env, target, graph)...) + findings = append(findings, graphPolicyFindings(env, target, graph)...) graphs = append(graphs, targetModuleGraph{target: target, graph: graph}) } findings = append(findings, commandFindings(ctx, env, target)...) diff --git a/internal/codeguard/checks/design/design_architecture_boundaries.go b/internal/codeguard/checks/design/design_architecture_boundaries.go new file mode 100644 index 0000000..d5f8184 --- /dev/null +++ b/internal/codeguard/checks/design/design_architecture_boundaries.go @@ -0,0 +1,212 @@ +package design + +import ( + "fmt" + "sort" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// architectureBoundaryFindings enforces the configured, language-neutral +// architecture boundaries over a target's source-level import graph. +func architectureBoundaryFindings(env support.Context, target core.TargetConfig, graph *moduleGraph) []core.Finding { + _ = target + if graph == nil { + return nil + } + rules := env.Config.Checks.DesignRules + findings := make([]core.Finding, 0, len(graph.imports)) + findings = append(findings, unassignedModuleFindings(env, graph, rules)...) + findings = append(findings, layerBoundaryFindings(env, graph, rules.Layers)...) + findings = append(findings, domainBoundaryFindings(env, graph, rules.Domains)...) + findings = append(findings, capabilityBoundaryFindings(env, graph, rules.Capabilities)...) + return deduplicateArchitectureFindings(findings) +} + +func unassignedModuleFindings(env support.Context, graph *moduleGraph, rules core.DesignRulesConfig) []core.Finding { + if rules.RequireBoundaryAssignment == nil || !*rules.RequireBoundaryAssignment || (len(rules.Layers) == 0 && len(rules.Domains) == 0) { + return nil + } + findings := make([]core.Finding, 0) + for _, module := range graph.sortedOrder() { + node := graph.modules[module] + missing := make([]string, 0, 2) + if len(rules.Layers) > 0 && layerForPath(rules.Layers, node.file) == nil { + missing = append(missing, "layer") + } + if len(rules.Domains) > 0 && domainForPath(rules.Domains, node.file) == nil { + missing = append(missing, "domain") + } + if len(missing) == 0 { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.unassigned-module", Level: "fail", Path: node.file, Line: 1, Column: 1, + Message: fmt.Sprintf("module %q is not assigned to a configured %s; add its path to the appropriate design boundary", module, strings.Join(missing, " and ")), + })) + } + return findings +} + +func layerBoundaryFindings(env support.Context, graph *moduleGraph, layers []core.DesignLayerConfig) []core.Finding { + if len(layers) == 0 { + return nil + } + findings := make([]core.Finding, 0) + for _, imported := range graph.imports { + sourceLayer := layerForPath(layers, imported.sourceFile) + if sourceLayer == nil { + continue + } + targetNode, local := graph.modules[imported.to] + if !local { + if designPathMatches(sourceLayer.DeniedExternal, imported.specifier) { + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.layer-boundary", Level: "fail", Path: imported.sourceFile, Line: imported.line, Column: 1, + Message: fmt.Sprintf("layer %q may not import external dependency %q; move the import behind an allowed adapter or remove it", sourceLayer.Name, imported.specifier), + })) + } + continue + } + targetLayer := layerForPath(layers, targetNode.file) + if targetLayer == nil || strings.EqualFold(sourceLayer.Name, targetLayer.Name) { + continue + } + denied := nameInList(sourceLayer.DenyDependOn, targetLayer.Name) + notAllowed := len(sourceLayer.MayDependOn) > 0 && !nameInList(sourceLayer.MayDependOn, targetLayer.Name) + if !denied && !notAllowed { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.layer-boundary", Level: "fail", Path: imported.sourceFile, Line: imported.line, Column: 1, + Message: fmt.Sprintf("layer %q may not depend on layer %q via %q; invert the dependency or route it through an allowed layer", sourceLayer.Name, targetLayer.Name, imported.specifier), + })) + } + return findings +} + +func domainBoundaryFindings(env support.Context, graph *moduleGraph, domains []core.DesignDomainConfig) []core.Finding { + if len(domains) == 0 { + return nil + } + findings := make([]core.Finding, 0) + for _, imported := range graph.imports { + targetNode, local := graph.modules[imported.to] + if !local { + continue + } + sourceDomain := domainForPath(domains, imported.sourceFile) + targetDomain := domainForPath(domains, targetNode.file) + if sourceDomain == nil || targetDomain == nil || strings.EqualFold(sourceDomain.Name, targetDomain.Name) { + continue + } + if designPathMatches(targetDomain.DataPaths, targetNode.file) { + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.data-ownership", Level: "fail", Path: imported.sourceFile, Line: imported.line, Column: 1, + Message: fmt.Sprintf("domain %q imports data-owned module %q from domain %q; access that data through the owning domain's public contract", sourceDomain.Name, targetNode.file, targetDomain.Name), + })) + } + allowedDomain := nameInList(sourceDomain.MayDependOn, targetDomain.Name) + publicTarget := designPathMatches(targetDomain.PublicPaths, targetNode.file) + if allowedDomain && publicTarget { + continue + } + reason := fmt.Sprintf("domain %q is not in may_depend_on", targetDomain.Name) + if allowedDomain { + reason = fmt.Sprintf("target %q is not one of domain %q's public_paths", targetNode.file, targetDomain.Name) + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.domain-boundary", Level: "fail", Path: imported.sourceFile, Line: imported.line, Column: 1, + Message: fmt.Sprintf("domain %q may not import %q: %s; use an allowed public domain contract", sourceDomain.Name, imported.specifier, reason), + })) + } + return findings +} + +func capabilityBoundaryFindings(env support.Context, graph *moduleGraph, capabilities []core.DesignCapabilityConfig) []core.Finding { + if len(capabilities) == 0 { + return nil + } + findings := make([]core.Finding, 0) + for _, imported := range graph.imports { + candidates := []string{ + imported.specifier, + strings.ReplaceAll(imported.specifier, "::", "/"), + imported.to, + } + if targetNode, ok := graph.modules[imported.to]; ok { + candidates = append(candidates, targetNode.file) + } + for _, capability := range capabilities { + if !matchesAnyCandidate(capability.Imports, candidates) || designPathMatches(capability.AllowedPaths, imported.sourceFile) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.capability-boundary", Level: "fail", Path: imported.sourceFile, Line: imported.line, Column: 1, + Message: fmt.Sprintf("capability %q is not allowed from %q (import %q); move this access into one of the capability's allowed_paths", capability.Name, imported.sourceFile, imported.specifier), + })) + } + } + return findings +} + +func layerForPath(layers []core.DesignLayerConfig, file string) *core.DesignLayerConfig { + for idx := range layers { + if designPathMatches(layers[idx].Paths, file) { + return &layers[idx] + } + } + return nil +} + +func domainForPath(domains []core.DesignDomainConfig, file string) *core.DesignDomainConfig { + for idx := range domains { + if designPathMatches(domains[idx].Paths, file) { + return &domains[idx] + } + } + return nil +} + +func matchesAnyCandidate(patterns []string, candidates []string) bool { + for _, candidate := range candidates { + if candidate != "" && designPathMatches(patterns, candidate) { + return true + } + } + return false +} + +func nameInList(values []string, want string) bool { + for _, value := range values { + if strings.EqualFold(strings.TrimSpace(value), strings.TrimSpace(want)) { + return true + } + } + return false +} + +func deduplicateArchitectureFindings(findings []core.Finding) []core.Finding { + sort.SliceStable(findings, func(i, j int) bool { + if findings[i].Path != findings[j].Path { + return findings[i].Path < findings[j].Path + } + if findings[i].Line != findings[j].Line { + return findings[i].Line < findings[j].Line + } + return findings[i].RuleID < findings[j].RuleID + }) + seen := make(map[string]bool, len(findings)) + result := make([]core.Finding, 0, len(findings)) + for _, finding := range findings { + key := fmt.Sprintf("%s:%d:%s:%s", finding.Path, finding.Line, finding.RuleID, finding.Message) + if seen[key] { + continue + } + seen[key] = true + result = append(result, finding) + } + return result +} diff --git a/internal/codeguard/checks/design/design_dependency_graph.go b/internal/codeguard/checks/design/design_dependency_graph.go index be4111c..1990b5e 100644 --- a/internal/codeguard/checks/design/design_dependency_graph.go +++ b/internal/codeguard/checks/design/design_dependency_graph.go @@ -13,6 +13,7 @@ type moduleGraph struct { modules map[string]*moduleGraphNode order []string fileToModule map[string]string + imports []moduleGraphImport } type moduleGraphNode struct { @@ -65,6 +66,22 @@ func (g *moduleGraph) addEdge(from string, to string, line int) { node.edges = append(node.edges, moduleGraphEdge{to: to, line: line}) } +func (g *moduleGraph) addImport(from string, to string, sourceFile string, specifier string, line int) { + if from == "" || sourceFile == "" || specifier == "" { + return + } + g.imports = append(g.imports, moduleGraphImport{ + from: from, + to: to, + sourceFile: sourceFile, + specifier: specifier, + line: line, + }) + if to != "" { + g.addEdge(from, to, line) + } +} + func (g *moduleGraph) addSelfEdge(module string, line int) { node, ok := g.modules[module] if !ok { diff --git a/internal/codeguard/checks/design/design_dependency_graph_import.go b/internal/codeguard/checks/design/design_dependency_graph_import.go new file mode 100644 index 0000000..509d287 --- /dev/null +++ b/internal/codeguard/checks/design/design_dependency_graph_import.go @@ -0,0 +1,12 @@ +package design + +// moduleGraphImport retains the source-level import behind a graph edge. The +// dependency graph intentionally contains local modules only, while boundary +// policies also need to inspect imports of external packages and SDKs. +type moduleGraphImport struct { + from string + to string + sourceFile string + specifier string + line int +} diff --git a/internal/codeguard/checks/design/design_encapsulation_boundaries.go b/internal/codeguard/checks/design/design_encapsulation_boundaries.go new file mode 100644 index 0000000..c0b28a4 --- /dev/null +++ b/internal/codeguard/checks/design/design_encapsulation_boundaries.go @@ -0,0 +1,99 @@ +package design + +import ( + "fmt" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +// encapsulationBoundaryFindings applies the path-based public-surface and +// production/test policies to source-level imports from every supported +// language graph. +func encapsulationBoundaryFindings(env support.Context, _ core.TargetConfig, graph *moduleGraph) []core.Finding { + if graph == nil { + return nil + } + findings := publicSurfaceFindings(env, graph) + findings = append(findings, productionTestFindings(env, graph)...) + return findings +} + +func publicSurfaceFindings(env support.Context, graph *moduleGraph) []core.Finding { + surfaces := env.Config.Checks.DesignRules.PublicSurfaces + if len(surfaces) == 0 { + return nil + } + findings := make([]core.Finding, 0) + for _, imported := range graph.imports { + targetPath, ok := resolvedImportPath(graph, imported) + if !ok { + continue + } + for _, surface := range surfaces { + if !designPathMatches(surface.Paths, targetPath) || + designPathMatches(surface.Paths, imported.sourceFile) || + designPathMatches(surface.Entrypoints, targetPath) { + continue + } + name := strings.TrimSpace(surface.Name) + if name == "" { + name = strings.Join(surface.Paths, ", ") + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.private-module-import", + Level: "fail", + Path: imported.sourceFile, + Line: positiveImportLine(imported.line), + Column: 1, + Message: fmt.Sprintf("module %q imports private module %q from public surface %q; import an approved entrypoint instead", imported.sourceFile, targetPath, name), + })) + } + } + return findings +} + +func productionTestFindings(env support.Context, graph *moduleGraph) []core.Finding { + policy := env.Config.Checks.DesignRules.ProductionTest + if policy == nil || !designToggleEnabled(policy.Enabled) { + return nil + } + findings := make([]core.Finding, 0) + for _, imported := range graph.imports { + if !designPathMatches(policy.ProductionPaths, imported.sourceFile) { + continue + } + targetPath, ok := resolvedImportPath(graph, imported) + if !ok || !designPathMatches(policy.TestPaths, targetPath) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.production-imports-test", + Level: "fail", + Path: imported.sourceFile, + Line: positiveImportLine(imported.line), + Column: 1, + Message: fmt.Sprintf("production module %q imports test-only module %q", imported.sourceFile, targetPath), + })) + } + return findings +} + +func resolvedImportPath(graph *moduleGraph, imported moduleGraphImport) (string, bool) { + if imported.to == "" { + return "", false + } + node, ok := graph.modules[imported.to] + if !ok || node.file == "" { + return "", false + } + return normalizeDesignPath(node.file), true +} + +func positiveImportLine(line int) int { + if line > 0 { + return line + } + return 1 +} diff --git a/internal/codeguard/checks/design/design_graph_cpp.go b/internal/codeguard/checks/design/design_graph_cpp.go index 773201d..53754a1 100644 --- a/internal/codeguard/checks/design/design_graph_cpp.go +++ b/internal/codeguard/checks/design/design_graph_cpp.go @@ -1,10 +1,24 @@ package design import ( + "regexp" + "strings" + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" ) +var ( + cppBoundaryModuleDeclarationPattern = regexp.MustCompile(`(?m)^[ \t]*(?:export[ \t]+)?module[ \t]+([A-Za-z_]\w*(?::[A-Za-z_]\w*)*)[ \t]*;`) + cppBoundaryModuleImportPattern = regexp.MustCompile(`(?m)^[ \t]*(?:export[ \t]+)?import[ \t]+([A-Za-z_]\w*(?::[A-Za-z_]\w*)*)[ \t]*;`) +) + +type cppBoundaryNamedImport struct { + from string + specifier string + line int +} + func buildCPPImportGraph(env support.Context, target core.TargetConfig) *moduleGraph { dependencyGraph := support.BuildCPPDependencyGraph(env, target) if dependencyGraph == nil { @@ -20,5 +34,75 @@ func buildCPPImportGraph(env support.Context, target core.TargetConfig) *moduleG graph.addEdge(id, edge.To, edge.Line) } } + namedModules := make(map[string]string) + namedImports := make([]cppBoundaryNamedImport, 0) + env.VisitTargetFiles(target, func(rel string) bool { return support.IsCPPPath(rel, true) }, func(rel string, data []byte) { + parsed := support.ParseCLike(string(data), support.CLikeCPP) + for _, imported := range parsed.Imports { + resolved := cppGraphEdgeAtLine(graph, rel, imported.Line) + if resolved == "" { + resolved = resolveCPPGraphImport(graph, rel, imported.Module) + } + graph.addImport(rel, resolved, rel, imported.Module, imported.Line) + } + if match := cppBoundaryModuleDeclarationPattern.FindStringSubmatch(parsed.Masked); len(match) == 2 { + namedModules[match[1]] = rel + } + for _, match := range cppBoundaryModuleImportPattern.FindAllStringSubmatchIndex(parsed.Masked, -1) { + specifier := strings.TrimSpace(parsed.Masked[match[2]:match[3]]) + namedImports = append(namedImports, cppBoundaryNamedImport{from: rel, specifier: specifier, line: support.LineNumberForOffset(parsed.Masked, match[0])}) + } + }) + for _, imported := range namedImports { + graph.addImport(imported.from, namedModules[imported.specifier], imported.from, imported.specifier, imported.line) + } return graph } + +func cppGraphEdgeAtLine(graph *moduleGraph, from string, line int) string { + node := graph.modules[from] + if node == nil { + return "" + } + for _, edge := range node.edges { + if edge.line == line { + return edge.to + } + } + return "" +} + +func resolveCPPGraphImport(graph *moduleGraph, from string, specifier string) string { + for _, candidate := range []string{ + pathCleanJoin(from, specifier), + strings.TrimPrefix(strings.ReplaceAll(specifier, "\\", "/"), "./"), + } { + if _, ok := graph.modules[candidate]; ok { + return candidate + } + } + return "" +} + +func pathCleanJoin(from string, specifier string) string { + from = strings.ReplaceAll(from, "\\", "/") + parts := strings.Split(from, "/") + if len(parts) > 0 { + parts = parts[:len(parts)-1] + } + specifierParts := strings.Split(strings.ReplaceAll(specifier, "\\", "/"), "/") + parts = append(parts, specifierParts...) + stack := make([]string, 0, len(parts)) + for _, part := range parts { + switch part { + case "", ".": + case "..": + if len(stack) > 0 { + stack = stack[:len(stack)-1] + } + default: + stack = append(stack, part) + } + } + return strings.Join(stack, "/") +} diff --git a/internal/codeguard/checks/design/design_graph_go.go b/internal/codeguard/checks/design/design_graph_go.go index 6f4b233..0570de6 100644 --- a/internal/codeguard/checks/design/design_graph_go.go +++ b/internal/codeguard/checks/design/design_graph_go.go @@ -24,10 +24,11 @@ func buildGoImportGraph(env support.Context, target core.TargetConfig) *moduleGr env.VisitTargetFiles(target, isGoSourceFile, func(rel string, data []byte) { pkg := path.Dir(filepath.ToSlash(rel)) graph.addModule(pkg, rel) - pending = append(pending, goImportEdges(pkg, rel, data, modulePrefix)...) + pending = append(pending, goImportEdges(pkg, rel, data)...) }) for _, edge := range pending { - graph.addEdge(edge.from, edge.to, edge.line) + local := goLocalPackageDir(edge.to, modulePrefix) + graph.addImport(edge.from, local, edge.file, edge.to, edge.line) } return graph } @@ -36,7 +37,7 @@ func isGoSourceFile(rel string) bool { return strings.HasSuffix(rel, ".go") && !strings.HasSuffix(rel, "_test.go") } -func goImportEdges(pkg string, rel string, data []byte, modulePrefix string) []pendingGraphEdge { +func goImportEdges(pkg string, rel string, data []byte) []pendingGraphEdge { fset := token.NewFileSet() parsed, err := parser.ParseFile(fset, rel, data, parser.ImportsOnly) if err != nil { @@ -45,11 +46,7 @@ func goImportEdges(pkg string, rel string, data []byte, modulePrefix string) []p edges := make([]pendingGraphEdge, 0, len(parsed.Imports)) for _, imp := range parsed.Imports { importPath := strings.Trim(imp.Path.Value, `"`) - local := goLocalPackageDir(importPath, modulePrefix) - if local == "" { - continue - } - edges = append(edges, pendingGraphEdge{from: pkg, to: local, line: fset.Position(imp.Pos()).Line}) + edges = append(edges, pendingGraphEdge{from: pkg, to: importPath, file: rel, line: fset.Position(imp.Pos()).Line}) } return edges } diff --git a/internal/codeguard/checks/design/design_graph_java.go b/internal/codeguard/checks/design/design_graph_java.go index 84cca5b..41105e6 100644 --- a/internal/codeguard/checks/design/design_graph_java.go +++ b/internal/codeguard/checks/design/design_graph_java.go @@ -35,7 +35,7 @@ func buildJavaImportGraph(env support.Context, target core.TargetConfig) *module pending[module] = append(pending[module], imports...) }) for module, imports := range pending { - addJavaImportEdges(graph, packageModules, module, imports) + addJavaImportEdges(graph, packageModules, module, graph.modules[module].file, imports) } return graph } @@ -70,17 +70,21 @@ func javaModuleName(pkg string, rel string) string { return pkg + "." + base } -func addJavaImportEdges(graph *moduleGraph, packageModules map[string][]string, module string, imports []javaImportStatement) { +func addJavaImportEdges(graph *moduleGraph, packageModules map[string][]string, module string, file string, imports []javaImportStatement) { for _, statement := range imports { if statement.wildcard { + resolved := "" for _, member := range packageModules[statement.target] { - graph.addEdge(module, member, statement.line) + graph.addImport(module, member, file, statement.target+".*", statement.line) + resolved = member + } + if resolved == "" { + graph.addImport(module, "", file, statement.target+".*", statement.line) } continue } - if resolved := resolveJavaImport(graph, statement); resolved != "" { - graph.addEdge(module, resolved, statement.line) - } + resolved := resolveJavaImport(graph, statement) + graph.addImport(module, resolved, file, statement.target, statement.line) } } diff --git a/internal/codeguard/checks/design/design_graph_policy.go b/internal/codeguard/checks/design/design_graph_policy.go new file mode 100644 index 0000000..d1c9dc3 --- /dev/null +++ b/internal/codeguard/checks/design/design_graph_policy.go @@ -0,0 +1,177 @@ +package design + +import ( + "fmt" + "sort" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +const ( + defaultStabilityMinimumFanIn = 3 + defaultMaxInstabilityDelta = 0.35 +) + +func graphPolicyFindings(env support.Context, target core.TargetConfig, graph *moduleGraph) []core.Finding { + if graph == nil { + return nil + } + findings := reachabilityFindings(env, target, graph) + findings = append(findings, stabilityDirectionFindings(env, graph)...) + return findings +} + +func reachabilityFindings(env support.Context, target core.TargetConfig, graph *moduleGraph) []core.Finding { + policy := env.Config.Checks.DesignRules.Reachability + if policy == nil || !designPolicyEnabled(policy.Enabled) { + return nil + } + entrypointPatterns := append([]string(nil), policy.Entrypoints...) + entrypointPatterns = append(entrypointPatterns, target.Entrypoints...) + if len(entrypointPatterns) == 0 { + return nil + } + + reachable := reachableModulesFromEntrypoints(graph, entrypointPatterns) + + findings := make([]core.Finding, 0) + for _, module := range graph.sortedOrder() { + node := graph.modules[module] + if reachable[module] || designPathMatches(policy.IgnorePaths, node.file) { + continue + } + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.unreachable-module", + Level: "warn", + Path: node.file, + Line: 1, + Column: 1, + Message: fmt.Sprintf("module %q is not reachable from an approved entrypoint", module), + })) + } + return findings +} + +func reachableModulesFromEntrypoints(graph *moduleGraph, entrypointPatterns []string) map[string]bool { + reachable := make(map[string]bool, len(graph.modules)) + queue := initialReachabilityQueue(graph, entrypointPatterns, reachable) + for len(queue) > 0 { + module := queue[0] + queue = queue[1:] + for _, edge := range graph.modules[module].edges { + if reachable[edge.to] { + continue + } + reachable[edge.to] = true + queue = append(queue, edge.to) + } + } + return reachable +} + +func initialReachabilityQueue(graph *moduleGraph, entrypointPatterns []string, reachable map[string]bool) []string { + queue := make([]string, 0) + for file, module := range graph.fileToModule { + if designPathMatches(entrypointPatterns, file) && !reachable[module] { + reachable[module] = true + queue = append(queue, module) + } + } + for module := range graph.modules { + if !reachable[module] && designPathMatches(entrypointPatterns, module) { + reachable[module] = true + queue = append(queue, module) + } + } + return queue +} + +func stabilityDirectionFindings(env support.Context, graph *moduleGraph) []core.Finding { + policy := env.Config.Checks.DesignRules.Stability + if policy == nil || !designPolicyEnabled(policy.Enabled) { + return nil + } + minimumFanIn := policy.MinimumFanIn + if minimumFanIn <= 0 { + minimumFanIn = defaultStabilityMinimumFanIn + } + maxDelta := policy.MaxInstabilityDelta + if maxDelta <= 0 { + maxDelta = defaultMaxInstabilityDelta + } + + fanOut, fanIn := graph.fanCounts() + instability := moduleInstability(graph, fanOut, fanIn) + stability := stabilityCheck{ + graph: graph, + policy: policy, + instability: instability, + maxDelta: maxDelta, + } + + findings := make([]core.Finding, 0) + for _, module := range graph.sortedOrder() { + node := graph.modules[module] + if fanIn[module] < minimumFanIn || designPathMatches(policy.IgnorePaths, node.file) { + continue + } + edges := append([]moduleGraphEdge(nil), node.edges...) + sort.Slice(edges, func(i, j int) bool { return edges[i].to < edges[j].to }) + for _, edge := range edges { + if !stability.violation(module, edge) { + continue + } + findingPath, findingLine := graphImportLocation(graph, module, edge) + findings = append(findings, env.NewFinding(support.FindingInput{ + RuleID: "design.stability-direction", + Level: "warn", + Path: findingPath, + Line: findingLine, + Column: 1, + Message: fmt.Sprintf("stable module %q (instability %.2f, fan-in %d) depends on less stable module %q (instability %.2f)", + module, instability[module], fanIn[module], edge.to, instability[edge.to]), + })) + } + } + return findings +} + +func moduleInstability(graph *moduleGraph, fanOut map[string]int, fanIn map[string]int) map[string]float64 { + instability := make(map[string]float64, len(graph.modules)) + for module := range graph.modules { + total := fanIn[module] + fanOut[module] + if total > 0 { + instability[module] = float64(fanOut[module]) / float64(total) + } + } + return instability +} + +type stabilityCheck struct { + graph *moduleGraph + policy *core.DesignStabilityConfig + instability map[string]float64 + maxDelta float64 +} + +func (check stabilityCheck) violation(module string, edge moduleGraphEdge) bool { + target := check.graph.modules[edge.to] + if target == nil || designPathMatches(check.policy.IgnorePaths, target.file) { + return false + } + return check.instability[edge.to]-check.instability[module] > check.maxDelta +} + +func graphImportLocation(graph *moduleGraph, from string, edge moduleGraphEdge) (string, int) { + for _, imported := range graph.imports { + if imported.from == from && imported.to == edge.to && imported.line == edge.line { + return imported.sourceFile, positiveImportLine(imported.line) + } + } + return graph.modules[from].file, positiveImportLine(edge.line) +} + +func designPolicyEnabled(enabled *bool) bool { + return enabled == nil || *enabled +} diff --git a/internal/codeguard/checks/design/design_graph_python.go b/internal/codeguard/checks/design/design_graph_python.go index 557390b..9e30523 100644 --- a/internal/codeguard/checks/design/design_graph_python.go +++ b/internal/codeguard/checks/design/design_graph_python.go @@ -7,6 +7,12 @@ func moduleGraphFromPython(src pythonImportGraph) *moduleGraph { for _, module := range src.graph.Order { graph.addModule(module, src.graph.Nodes[module].Path) } + addPythonGraphEdges(graph, src) + addPythonGraphImports(graph, src) + return graph +} + +func addPythonGraphEdges(graph *moduleGraph, src pythonImportGraph) { for _, module := range src.graph.Order { for _, edge := range src.graph.Nodes[module].Edges { if edge.To == module { @@ -16,5 +22,33 @@ func moduleGraphFromPython(src pythonImportGraph) *moduleGraph { graph.addEdge(module, edge.To, edge.Line) } } - return graph +} + +func addPythonGraphImports(graph *moduleGraph, src pythonImportGraph) { + for _, node := range src.nodes { + for _, statement := range node.statements { + if len(statement.modules) > 0 { + addPythonModuleImports(graph, src, node, statement) + continue + } + targets := pythonFromImportTargets(statement, src.nodes) + if len(targets) == 0 { + graph.addImport(node.module, "", node.file, statement.from, statement.line) + continue + } + for _, target := range targets { + graph.addImport(node.module, target, node.file, statement.from, statement.line) + } + } + } +} + +func addPythonModuleImports(graph *moduleGraph, src pythonImportGraph, node pythonGraphNode, statement pythonImportStatement) { + for _, imported := range statement.modules { + resolved := "" + if _, ok := src.nodes[imported]; ok { + resolved = imported + } + graph.addImport(node.module, resolved, node.file, imported, statement.line) + } } diff --git a/internal/codeguard/checks/design/design_graph_rust.go b/internal/codeguard/checks/design/design_graph_rust.go index fb6cc19..c762589 100644 --- a/internal/codeguard/checks/design/design_graph_rust.go +++ b/internal/codeguard/checks/design/design_graph_rust.go @@ -22,13 +22,17 @@ func buildRustImportGraph(env support.Context, target core.TargetConfig) *module }, func(rel string, data []byte) { module := rustModulePath(rel) graph.addModule(module, rel) - pending = append(pending, rustImportEdges(module, string(data))...) + for _, edge := range rustImportEdges(module, string(data)) { + edge.file = rel + pending = append(pending, edge) + } }) for _, edge := range pending { resolved := resolveRustImport(graph, edge.to) - if resolved != "" && resolved != edge.from { - graph.addEdge(edge.from, resolved, edge.line) + if resolved == edge.from { + resolved = "" } + graph.addImport(edge.from, resolved, edge.file, edge.to, edge.line) } return graph } @@ -62,10 +66,11 @@ func rustImportEdges(module string, source string) []pendingGraphEdge { } for _, used := range expandRustUseClause(strings.TrimSpace(match[1])) { absolute := resolveRustUsePath(module, used) - if absolute == "" { - continue + if absolute != "" { + edges = append(edges, pendingGraphEdge{from: module, to: absolute, line: lineNo}) + } else { + edges = append(edges, pendingGraphEdge{from: module, to: strings.TrimSpace(strings.Split(used, " as ")[0]), line: lineNo}) } - edges = append(edges, pendingGraphEdge{from: module, to: absolute, line: lineNo}) } } return edges diff --git a/internal/codeguard/checks/design/design_graph_typescript.go b/internal/codeguard/checks/design/design_graph_typescript.go index e889694..8aea3b5 100644 --- a/internal/codeguard/checks/design/design_graph_typescript.go +++ b/internal/codeguard/checks/design/design_graph_typescript.go @@ -28,6 +28,7 @@ type scriptImport struct { type pendingGraphEdge struct { from string to string + file string line int } @@ -39,14 +40,12 @@ func buildTypeScriptImportGraph(env support.Context, target core.TargetConfig) * graph.addModule(module, rel) source := strings.ReplaceAll(string(data), "\r\n", "\n") for _, imp := range typeScriptImportSpecifiers(source) { - pending = append(pending, pendingGraphEdge{from: module, to: imp.specifier, line: imp.line}) + pending = append(pending, pendingGraphEdge{from: module, to: imp.specifier, file: rel, line: imp.line}) } }) for _, edge := range pending { resolved := resolveTypeScriptImport(graph, edge.from, edge.to) - if resolved != "" { - graph.addEdge(edge.from, resolved, edge.line) - } + graph.addImport(edge.from, resolved, edge.file, edge.to, edge.line) } return graph } diff --git a/internal/codeguard/checks/design/design_policy_match.go b/internal/codeguard/checks/design/design_policy_match.go new file mode 100644 index 0000000..0472e9e --- /dev/null +++ b/internal/codeguard/checks/design/design_policy_match.go @@ -0,0 +1,89 @@ +package design + +import ( + "path" + "regexp" + "strings" +) + +func designPathMatches(patterns []string, value string) bool { + normalized := normalizeDesignPath(value) + for _, pattern := range patterns { + if designPatternMatches(pattern, normalized) { + return true + } + } + return false +} + +func designPatternMatches(pattern string, value string) bool { + pattern = normalizeDesignPath(pattern) + if pattern == "" { + return false + } + if !strings.ContainsAny(pattern, "*?[") { + return value == pattern || strings.HasPrefix(value, strings.TrimSuffix(pattern, "/")+"/") + } + if !strings.Contains(pattern, "**") { + matched, err := path.Match(pattern, value) + return err == nil && matched + } + re, err := regexp.Compile(designGlobRegex(pattern)) + return err == nil && re.MatchString(value) +} + +func normalizeDesignPath(value string) string { + trimmed := strings.ReplaceAll(strings.TrimSpace(value), "\\", "/") + if trimmed == "" { + return "" + } + return strings.TrimPrefix(path.Clean(trimmed), "./") +} + +func designGlobRegex(pattern string) string { + var out strings.Builder + out.WriteString("^") + for idx := 0; idx < len(pattern); { + switch pattern[idx] { + case '*': + if idx+1 < len(pattern) && pattern[idx+1] == '*' { + idx += 2 + if idx < len(pattern) && pattern[idx] == '/' { + out.WriteString("(?:.*/)?") + idx++ + } else { + out.WriteString(".*") + } + continue + } + out.WriteString("[^/]*") + case '?': + out.WriteString("[^/]") + case '[': + idx = writeDesignGlobClass(&out, pattern, idx) + default: + out.WriteString(regexp.QuoteMeta(string(pattern[idx]))) + } + idx++ + } + out.WriteString("$") + return out.String() +} + +func writeDesignGlobClass(out *strings.Builder, pattern string, idx int) int { + end := strings.IndexByte(pattern[idx+1:], ']') + if end < 0 { + out.WriteString(`\[`) + return idx + } + end += idx + 1 + class := pattern[idx+1 : end] + if strings.HasPrefix(class, "!") { + out.WriteByte('^') + class = strings.TrimPrefix(class, "!") + } + out.WriteByte('[') + out.WriteString(strings.ReplaceAll(class, `\`, `\\`)) + out.WriteByte(']') + return end +} diff --git a/internal/codeguard/checks/design/design_python_graph.go b/internal/codeguard/checks/design/design_python_graph.go index 3ea2657..102ee1a 100644 --- a/internal/codeguard/checks/design/design_python_graph.go +++ b/internal/codeguard/checks/design/design_python_graph.go @@ -13,6 +13,7 @@ import ( type pythonImportGraph struct { graph support.DependencyGraph entrypoints map[string]struct{} + nodes map[string]pythonGraphNode } type pythonGraphNode struct { @@ -52,6 +53,7 @@ func buildPythonImportGraph(env support.Context, target core.TargetConfig) pytho } } graph.graph = support.NewDependencyGraph(dependencyNodes) + graph.nodes = nodes if env.PutArtifact != nil { env.PutArtifact(support.NewDependencyGraphArtifact(pythonDependencyGraphArtifactID(target), "python", target.Path, graph.graph)) } diff --git a/internal/codeguard/config/io.go b/internal/codeguard/config/io.go index 0e16446..97d13b7 100644 --- a/internal/codeguard/config/io.go +++ b/internal/codeguard/config/io.go @@ -38,16 +38,27 @@ func LoadFile(path string) (core.Config, error) { } var cfg core.Config - if err := unmarshalConfig(data, resolvedPath, &cfg); err != nil { + err = unmarshalConfig(data, resolvedPath, &cfg) + if err != nil { + return core.Config{}, err + } + designOverlay, err := loadExternalDesignRules(&cfg, data, resolvedPath) + if err != nil { return core.Config{}, err } baseDir := filepath.Dir(resolvedPath) resolveRelativePaths(&cfg, baseDir) ApplyDefaults(&cfg) - if err := containConfigArtifactPaths(&cfg, baseDir); err != nil { + // Applying defaults intentionally treats numeric zero as unset. When an + // external design policy is used, however, explicitly present inline fields + // must win even when their value is zero, false, or empty. + designOverlay.apply(&cfg.Checks.DesignRules) + err = containConfigArtifactPaths(&cfg, baseDir) + if err != nil { return core.Config{}, err } - if err := Validate(cfg); err != nil { + err = Validate(cfg) + if err != nil { return core.Config{}, err } return cfg, nil diff --git a/internal/codeguard/config/io_design_rules.go b/internal/codeguard/config/io_design_rules.go new file mode 100644 index 0000000..190a668 --- /dev/null +++ b/internal/codeguard/config/io_design_rules.go @@ -0,0 +1,135 @@ +package config + +import ( + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "reflect" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" + "gopkg.in/yaml.v3" +) + +var defaultDesignRulesNames = []string{"design_rules.yml", "design_rules.yaml"} + +// designRulesOverlay retains every field explicitly present in the external +// policy or inline overrides so values such as zero and an empty slice can be +// restored after profile defaults are applied. It is inactive when no external +// policy was loaded, preserving the historical defaulting behavior of +// inline-only configurations. +type designRulesOverlay struct { + active bool + rules core.DesignRulesConfig + fields map[string]struct{} +} + +func (o designRulesOverlay) apply(dst *core.DesignRulesConfig) { + if !o.active { + return + } + copyDesignRuleFields(dst, o.rules, o.fields) +} + +func loadExternalDesignRules(cfg *core.Config, mainData []byte, configPath string) (designRulesOverlay, error) { + inlineFields, err := inlineDesignRuleFields(mainData) + if err != nil { + return designRulesOverlay{}, fmt.Errorf("inspect inline design_rules: %w", err) + } + inlineRules := cfg.Checks.DesignRules + + policyPath, found, err := resolveDesignRulesPath(configPath, cfg.Checks.DesignRulesFile) + if err != nil { + return designRulesOverlay{}, err + } + if !found { + return designRulesOverlay{}, nil + } + + data, err := readSizeCappedFile(policyPath) + if err != nil { + return designRulesOverlay{}, fmt.Errorf("read design rules file %q: %w", policyPath, err) + } + var external core.DesignRulesConfig + err = unmarshalDesignRules(data, policyPath, &external) + if err != nil { + return designRulesOverlay{}, fmt.Errorf("parse design rules file %q: %w", policyPath, err) + } + externalFields, err := topLevelDesignRuleFields(data) + if err != nil { + return designRulesOverlay{}, fmt.Errorf("inspect design rules file %q: %w", policyPath, err) + } + + cfg.Checks.DesignRules = external + copyDesignRuleFields(&cfg.Checks.DesignRules, inlineRules, inlineFields) + for name := range inlineFields { + externalFields[name] = struct{}{} + } + return designRulesOverlay{active: true, rules: cfg.Checks.DesignRules, fields: externalFields}, nil +} + +func readSizeCappedFile(path string) ([]byte, error) { + f, err := os.Open(path) //nolint:gosec // path is contained beneath the project root above + if err != nil { + return nil, err + } + defer func() { _ = f.Close() }() + + data, err := io.ReadAll(io.LimitReader(f, maxConfigFileBytes+1)) + if err != nil { + return nil, err + } + if len(data) > maxConfigFileBytes { + return nil, fmt.Errorf("file exceeds the %d-byte config size limit", maxConfigFileBytes) + } + return data, nil +} + +func unmarshalDesignRules(data []byte, path string, rules *core.DesignRulesConfig) error { + if strings.EqualFold(filepath.Ext(path), ".json") { + return json.Unmarshal(data, rules) + } + return yaml.Unmarshal(data, rules) +} + +func inlineDesignRuleFields(data []byte) (map[string]struct{}, error) { + var document struct { + Checks struct { + DesignRules map[string]yaml.Node `yaml:"design_rules"` + } `yaml:"checks"` + } + if err := yaml.Unmarshal(data, &document); err != nil { + return nil, err + } + fields := make(map[string]struct{}, len(document.Checks.DesignRules)) + for name := range document.Checks.DesignRules { + fields[name] = struct{}{} + } + return fields, nil +} + +func topLevelDesignRuleFields(data []byte) (map[string]struct{}, error) { + var document map[string]yaml.Node + if err := yaml.Unmarshal(data, &document); err != nil { + return nil, err + } + fields := make(map[string]struct{}, len(document)) + for name := range document { + fields[name] = struct{}{} + } + return fields, nil +} + +func copyDesignRuleFields(dst *core.DesignRulesConfig, src core.DesignRulesConfig, fields map[string]struct{}) { + dstValue := reflect.ValueOf(dst).Elem() + srcValue := reflect.ValueOf(src) + typ := dstValue.Type() + for i := 0; i < typ.NumField(); i++ { + name := strings.Split(typ.Field(i).Tag.Get("yaml"), ",")[0] + if _, ok := fields[name]; ok { + dstValue.Field(i).Set(srcValue.Field(i)) + } + } +} diff --git a/internal/codeguard/config/io_design_rules_paths.go b/internal/codeguard/config/io_design_rules_paths.go new file mode 100644 index 0000000..b77d404 --- /dev/null +++ b/internal/codeguard/config/io_design_rules_paths.go @@ -0,0 +1,71 @@ +package config + +import ( + "fmt" + "os" + "path/filepath" + "strings" +) + +func resolveDesignRulesPath(configPath, explicit string) (string, bool, error) { + absConfig, err := filepath.Abs(configPath) + if err != nil { + return "", false, err + } + configDir := filepath.Dir(absConfig) + projectRoot := configDir + discoveryDir := filepath.Join(projectRoot, ".codeguard") + if filepath.Base(configDir) == ".codeguard" { + projectRoot = filepath.Dir(configDir) + discoveryDir = configDir + } + + if strings.TrimSpace(explicit) != "" { + return explicitDesignRulesPath(projectRoot, configDir, explicit) + } + + return discoverDesignRulesPath(projectRoot, discoveryDir) +} + +func explicitDesignRulesPath(projectRoot, configDir, explicit string) (string, bool, error) { + candidate := explicit + if !filepath.IsAbs(candidate) { + candidate = filepath.Join(configDir, candidate) + } + contained, err := containedPath(projectRoot, candidate) + if err != nil { + return "", false, fmt.Errorf("checks.design_rules_file: %w", err) + } + if _, err := os.Stat(contained); err != nil { + return "", false, fmt.Errorf("checks.design_rules_file %q: %w", explicit, err) + } + return contained, true, nil +} + +func discoverDesignRulesPath(projectRoot, discoveryDir string) (string, bool, error) { + for _, name := range defaultDesignRulesNames { + candidate, found, err := inspectDiscoveredDesignRulesPath(projectRoot, filepath.Join(discoveryDir, name)) + if err != nil || found { + return candidate, found, err + } + } + return "", false, nil +} + +func inspectDiscoveredDesignRulesPath(projectRoot, path string) (string, bool, error) { + candidate, err := containedPath(projectRoot, path) + if err != nil { + return "", false, fmt.Errorf("discover design rules: %w", err) + } + info, err := os.Stat(candidate) + switch { + case err == nil && !info.IsDir(): + return candidate, true, nil + case err == nil: + return "", false, nil + case os.IsNotExist(err): + return "", false, nil + default: + return "", false, fmt.Errorf("discover design rules file %q: %w", candidate, err) + } +} diff --git a/internal/codeguard/config/validate.go b/internal/codeguard/config/validate.go index d048ef9..af36ce1 100644 --- a/internal/codeguard/config/validate.go +++ b/internal/codeguard/config/validate.go @@ -27,6 +27,7 @@ func Validate(cfg core.Config) error { validateCoverageDelta(cfg.Checks.QualityRules.CoverageDelta), validateCPPTooling(cfg.Checks.QualityRules.CPPTooling), validateGraphThresholds(cfg.Checks.DesignRules), + validateDesignArchitectureRules(cfg.Checks.DesignRules), validatePerformanceRules(cfg.Checks.PerformanceRules), validateSecretsRules(cfg.Checks.SecurityRules.Secrets), validateParsers(cfg.Parsers), diff --git a/internal/codeguard/config/validate_design.go b/internal/codeguard/config/validate_design.go new file mode 100644 index 0000000..9534795 --- /dev/null +++ b/internal/codeguard/config/validate_design.go @@ -0,0 +1,164 @@ +package config + +import ( + "fmt" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/core" +) + +func validateDesignArchitectureRules(rules core.DesignRulesConfig) error { + layers, err := validateDesignLayers(rules.Layers) + if err != nil { + return err + } + err = validateLayerReferences(rules.Layers, layers) + if err != nil { + return err + } + domains, err := validateDesignDomains(rules.Domains) + if err != nil { + return err + } + err = validateDomainReferences(rules.Domains, domains) + if err != nil { + return err + } + err = validateDesignCapabilities(rules.Capabilities) + if err != nil { + return err + } + err = validateDesignPublicSurfaces(rules.PublicSurfaces) + if err != nil { + return err + } + return validateDesignBoundaryToggles(rules) +} + +func validateDesignLayers(ruleLayers []core.DesignLayerConfig) (map[string]struct{}, error) { + layers := make(map[string]struct{}, len(ruleLayers)) + for idx, layer := range ruleLayers { + name := strings.TrimSpace(layer.Name) + if name == "" { + return nil, fmt.Errorf("design_rules.layers[%d].name is required", idx) + } + key := strings.ToLower(name) + if _, exists := layers[key]; exists { + return nil, fmt.Errorf("design_rules.layers contains duplicate name %q", name) + } + if !hasNonBlankDesignValue(layer.Paths) { + return nil, fmt.Errorf("design_rules.layers[%d].paths must not be empty", idx) + } + layers[key] = struct{}{} + } + return layers, nil +} + +func validateLayerReferences(ruleLayers []core.DesignLayerConfig, layers map[string]struct{}) error { + for idx, layer := range ruleLayers { + for _, dependency := range append(append([]string(nil), layer.MayDependOn...), layer.DenyDependOn...) { + if _, ok := layers[strings.ToLower(strings.TrimSpace(dependency))]; !ok { + return fmt.Errorf("design_rules.layers[%d] references unknown layer %q", idx, dependency) + } + } + } + return nil +} + +func validateDesignDomains(ruleDomains []core.DesignDomainConfig) (map[string]struct{}, error) { + domains := make(map[string]struct{}, len(ruleDomains)) + for idx, domain := range ruleDomains { + name := strings.TrimSpace(domain.Name) + if name == "" { + return nil, fmt.Errorf("design_rules.domains[%d].name is required", idx) + } + key := strings.ToLower(name) + if _, exists := domains[key]; exists { + return nil, fmt.Errorf("design_rules.domains contains duplicate name %q", name) + } + if !hasNonBlankDesignValue(domain.Paths) { + return nil, fmt.Errorf("design_rules.domains[%d].paths must not be empty", idx) + } + domains[key] = struct{}{} + } + return domains, nil +} + +func validateDomainReferences(ruleDomains []core.DesignDomainConfig, domains map[string]struct{}) error { + for idx, domain := range ruleDomains { + for _, dependency := range domain.MayDependOn { + if _, ok := domains[strings.ToLower(strings.TrimSpace(dependency))]; !ok { + return fmt.Errorf("design_rules.domains[%d] references unknown domain %q", idx, dependency) + } + } + } + return nil +} + +func validateDesignCapabilities(capabilities []core.DesignCapabilityConfig) error { + seenCapabilities := map[string]struct{}{} + for idx, capability := range capabilities { + name := strings.TrimSpace(capability.Name) + if name == "" { + return fmt.Errorf("design_rules.capabilities[%d].name is required", idx) + } + key := strings.ToLower(name) + if _, exists := seenCapabilities[key]; exists { + return fmt.Errorf("design_rules.capabilities contains duplicate name %q", name) + } + if !hasNonBlankDesignValue(capability.Imports) || !hasNonBlankDesignValue(capability.AllowedPaths) { + return fmt.Errorf("design_rules.capabilities[%d].imports and allowed_paths must not be empty", idx) + } + seenCapabilities[key] = struct{}{} + } + return nil +} + +func validateDesignPublicSurfaces(surfaces []core.DesignPublicSurfaceConfig) error { + seenSurfaces := map[string]struct{}{} + for idx, surface := range surfaces { + name := strings.TrimSpace(surface.Name) + if name == "" { + return fmt.Errorf("design_rules.public_surfaces[%d].name is required", idx) + } + key := strings.ToLower(name) + if _, exists := seenSurfaces[key]; exists { + return fmt.Errorf("design_rules.public_surfaces contains duplicate name %q", name) + } + if !hasNonBlankDesignValue(surface.Paths) || !hasNonBlankDesignValue(surface.Entrypoints) { + return fmt.Errorf("design_rules.public_surfaces[%d].paths and entrypoints must not be empty", idx) + } + seenSurfaces[key] = struct{}{} + } + return nil +} + +func validateDesignBoundaryToggles(rules core.DesignRulesConfig) error { + if policy := rules.ProductionTest; policy != nil && designConfigEnabled(policy.Enabled) { + if !hasNonBlankDesignValue(policy.ProductionPaths) || !hasNonBlankDesignValue(policy.TestPaths) { + return fmt.Errorf("design_rules.production_test.production_paths and test_paths must not be empty") + } + } + if policy := rules.Stability; policy != nil { + if policy.MinimumFanIn < 0 { + return fmt.Errorf("design_rules.stability.minimum_fan_in must not be negative") + } + if policy.MaxInstabilityDelta < 0 || policy.MaxInstabilityDelta > 1 { + return fmt.Errorf("design_rules.stability.max_instability_delta must be between 0 and 1") + } + } + return nil +} + +func designConfigEnabled(value *bool) bool { + return value == nil || *value +} + +func hasNonBlankDesignValue(values []string) bool { + for _, value := range values { + if strings.TrimSpace(value) != "" { + return true + } + } + return false +} diff --git a/internal/codeguard/core/config_design_boundary_types.go b/internal/codeguard/core/config_design_boundary_types.go new file mode 100644 index 0000000..d2b3c5a --- /dev/null +++ b/internal/codeguard/core/config_design_boundary_types.go @@ -0,0 +1,62 @@ +package core + +// DesignLayerConfig assigns source paths to an architectural layer and +// constrains the local and external dependencies that layer may use. +type DesignLayerConfig struct { + Name string `json:"name" yaml:"name"` + Paths []string `json:"paths" yaml:"paths"` + MayDependOn []string `json:"may_depend_on,omitempty" yaml:"may_depend_on,omitempty"` + DenyDependOn []string `json:"deny_depend_on,omitempty" yaml:"deny_depend_on,omitempty"` + DeniedExternal []string `json:"denied_external,omitempty" yaml:"denied_external,omitempty"` +} + +// DesignDomainConfig describes a bounded context. Imports from another domain +// must target a public path, while data paths remain private to their owner. +type DesignDomainConfig struct { + Name string `json:"name" yaml:"name"` + Paths []string `json:"paths" yaml:"paths"` + PublicPaths []string `json:"public_paths,omitempty" yaml:"public_paths,omitempty"` + DataPaths []string `json:"data_paths,omitempty" yaml:"data_paths,omitempty"` + MayDependOn []string `json:"may_depend_on,omitempty" yaml:"may_depend_on,omitempty"` +} + +// DesignCapabilityConfig limits imports that grant a technical capability +// (database, network, filesystem, cloud SDK, and similar) to approved paths. +type DesignCapabilityConfig struct { + Name string `json:"name" yaml:"name"` + Imports []string `json:"imports" yaml:"imports"` + AllowedPaths []string `json:"allowed_paths" yaml:"allowed_paths"` +} + +// DesignPublicSurfaceConfig prevents consumers outside a package or component +// from deep-importing implementation modules instead of its public entrypoints. +type DesignPublicSurfaceConfig struct { + Name string `json:"name" yaml:"name"` + Paths []string `json:"paths" yaml:"paths"` + Entrypoints []string `json:"entrypoints" yaml:"entrypoints"` +} + +// DesignProductionTestConfig keeps test-only helpers and dependencies out of +// production source paths. +type DesignProductionTestConfig struct { + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + ProductionPaths []string `json:"production_paths" yaml:"production_paths"` + TestPaths []string `json:"test_paths" yaml:"test_paths"` +} + +// DesignReachabilityConfig reports production modules that cannot be reached +// from an approved application or package entrypoint. +type DesignReachabilityConfig struct { + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + Entrypoints []string `json:"entrypoints,omitempty" yaml:"entrypoints,omitempty"` + IgnorePaths []string `json:"ignore_paths,omitempty" yaml:"ignore_paths,omitempty"` +} + +// DesignStabilityConfig warns when a stable, widely depended-on module points +// toward a substantially less stable module. +type DesignStabilityConfig struct { + Enabled *bool `json:"enabled,omitempty" yaml:"enabled,omitempty"` + MinimumFanIn int `json:"minimum_fan_in,omitempty" yaml:"minimum_fan_in,omitempty"` + MaxInstabilityDelta float64 `json:"max_instability_delta,omitempty" yaml:"max_instability_delta,omitempty"` + IgnorePaths []string `json:"ignore_paths,omitempty" yaml:"ignore_paths,omitempty"` +} diff --git a/internal/codeguard/core/config_rule_types.go b/internal/codeguard/core/config_rule_types.go index df5a139..48d25aa 100644 --- a/internal/codeguard/core/config_rule_types.go +++ b/internal/codeguard/core/config_rule_types.go @@ -130,6 +130,14 @@ type DesignRulesConfig struct { ForbiddenPackageNames []string `json:"forbidden_package_names,omitempty" yaml:"forbidden_package_names,omitempty"` LanguageCommands map[string][]CommandCheckConfig `json:"language_commands,omitempty" yaml:"language_commands,omitempty"` LanguageDiffCommands map[string][]CommandCheckConfig `json:"language_diff_commands,omitempty" yaml:"language_diff_commands,omitempty"` + RequireBoundaryAssignment *bool `json:"require_boundary_assignment,omitempty" yaml:"require_boundary_assignment,omitempty"` + Layers []DesignLayerConfig `json:"layers,omitempty" yaml:"layers,omitempty"` + Domains []DesignDomainConfig `json:"domains,omitempty" yaml:"domains,omitempty"` + Capabilities []DesignCapabilityConfig `json:"capabilities,omitempty" yaml:"capabilities,omitempty"` + PublicSurfaces []DesignPublicSurfaceConfig `json:"public_surfaces,omitempty" yaml:"public_surfaces,omitempty"` + ProductionTest *DesignProductionTestConfig `json:"production_test,omitempty" yaml:"production_test,omitempty"` + Reachability *DesignReachabilityConfig `json:"reachability,omitempty" yaml:"reachability,omitempty"` + Stability *DesignStabilityConfig `json:"stability,omitempty" yaml:"stability,omitempty"` } type PromptRulesConfig struct { diff --git a/internal/codeguard/core/config_types.go b/internal/codeguard/core/config_types.go index 8c8e161..e17c261 100644 --- a/internal/codeguard/core/config_types.go +++ b/internal/codeguard/core/config_types.go @@ -75,6 +75,7 @@ type CheckConfig struct { Context *bool `json:"context,omitempty" yaml:"context,omitempty"` QualityRules QualityRulesConfig `json:"quality_rules" yaml:"quality_rules"` PerformanceRules PerformanceRulesConfig `json:"performance_rules,omitempty" yaml:"performance_rules,omitempty"` + DesignRulesFile string `json:"design_rules_file,omitempty" yaml:"design_rules_file,omitempty"` DesignRules DesignRulesConfig `json:"design_rules" yaml:"design_rules"` PromptRules PromptRulesConfig `json:"prompt_rules" yaml:"prompt_rules"` CIRules CIRulesConfig `json:"ci_rules" yaml:"ci_rules"` diff --git a/internal/codeguard/rules/catalog.go b/internal/codeguard/rules/catalog.go index 1a0391e..563e3f6 100644 --- a/internal/codeguard/rules/catalog.go +++ b/internal/codeguard/rules/catalog.go @@ -12,6 +12,7 @@ var catalog = withSecurityOWASP(mergeRuleCatalogs( performanceMeasuredCatalog, designCatalog, designGraphCatalog, + designPolicyCatalog, securityCatalog, securityExtraCatalog, supplyChainCatalog, diff --git a/internal/codeguard/rules/catalog_design_graph.go b/internal/codeguard/rules/catalog_design_graph.go index 0552d44..936bd1e 100644 --- a/internal/codeguard/rules/catalog_design_graph.go +++ b/internal/codeguard/rules/catalog_design_graph.go @@ -3,6 +3,56 @@ package rules import "github.com/devr-tools/codeguard/internal/codeguard/core" var designGraphCatalog = map[string]core.RuleMetadata{ + "design.layer-boundary": { + ID: "design.layer-boundary", Section: "Design Patterns", DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, LanguageCoverage: designGraphLanguageCoverage(), + Title: "Layer dependency boundary", Description: "Fails when a module imports a denied architectural layer or an external dependency denied to its layer.", + HowToFix: "Invert the dependency, move the implementation to an allowed layer, or route access through an allowed contract.", + }, + "design.unassigned-module": { + ID: "design.unassigned-module", Section: "Design Patterns", DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, LanguageCoverage: designGraphLanguageCoverage(), + Title: "Unassigned architecture module", Description: "Fails when boundary assignment is required and a module is outside every configured layer or domain.", + HowToFix: "Add the module path to the appropriate configured layer and domain.", + }, + "design.domain-boundary": { + ID: "design.domain-boundary", Section: "Design Patterns", DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, LanguageCoverage: designGraphLanguageCoverage(), + Title: "Domain dependency boundary", Description: "Fails when a cross-domain import is not permitted or bypasses the target domain's public paths.", + HowToFix: "Use a public contract exposed by an allowed domain, or remove the cross-domain dependency.", + }, + "design.data-ownership": { + ID: "design.data-ownership", Section: "Design Patterns", DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, LanguageCoverage: designGraphLanguageCoverage(), + Title: "Domain data ownership", Description: "Fails when one domain directly imports another domain's data or persistence modules.", + HowToFix: "Access the data through the owning domain's public API, contract, or event.", + }, + "design.capability-boundary": { + ID: "design.capability-boundary", Section: "Design Patterns", DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, LanguageCoverage: designGraphLanguageCoverage(), + Title: "Technical capability boundary", Description: "Fails when a source module imports a sensitive technical capability outside its approved paths.", + HowToFix: "Move the import and its implementation into an allowed adapter and expose a narrow contract to callers.", + }, + "design.private-module-import": { + ID: "design.private-module-import", + Section: "Design Patterns", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: designGraphLanguageCoverage(), + Title: "Private module import", + Description: "Fails when code outside a configured public surface imports one of its implementation modules instead of an approved entrypoint.", + HowToFix: "Import the component's public entrypoint, or deliberately add the target module to the surface's configured entrypoints.", + }, + "design.production-imports-test": { + ID: "design.production-imports-test", + Section: "Design Patterns", + DefaultLevel: "fail", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: designGraphLanguageCoverage(), + Title: "Production imports test code", + Description: "Fails when a configured production module imports a test, mock, fixture, or other test-only module.", + HowToFix: "Move the shared behavior into a production module, then make both production and test code import that module.", + }, "design.typescript.import-cycle": { ID: "design.typescript.import-cycle", Section: "Design Patterns", diff --git a/internal/codeguard/rules/catalog_design_policy.go b/internal/codeguard/rules/catalog_design_policy.go new file mode 100644 index 0000000..e22f337 --- /dev/null +++ b/internal/codeguard/rules/catalog_design_policy.go @@ -0,0 +1,38 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var designPolicyCatalog = map[string]core.RuleMetadata{ + "design.unreachable-module": { + ID: "design.unreachable-module", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: designGraphLanguageCoverage(), + Title: "Module unreachable from approved entrypoints", + Description: "Warns when a production module cannot be reached from any configured application or package entrypoint.", + HowToFix: "Remove dead code, add the intended entrypoint, or connect the module through the application dependency graph.", + }, + "design.stability-direction": { + ID: "design.stability-direction", + Section: "Design Patterns", + DefaultLevel: "warn", + ExecutionModel: core.RuleExecutionModelLanguageAgnostic, + LanguageCoverage: designGraphLanguageCoverage(), + Title: "Stable module depends on volatile module", + Description: "Warns when a widely depended-on stable module imports a substantially less stable module, reversing the desired dependency direction.", + HowToFix: "Invert the dependency through a stable contract or move volatile behavior behind an adapter owned by the less stable module.", + }, +} + +func designGraphLanguageCoverage() core.RuleLanguageCoverage { + return core.FixedRuleLanguageCoverage( + core.RuleLanguageGo, + core.RuleLanguagePython, + core.RuleLanguageTypeScript, + core.RuleLanguageJavaScript, + core.RuleLanguageRust, + core.RuleLanguageJava, + core.RuleLanguageCPP, + ) +} diff --git a/internal/codeguard/rules/catalog_fix_templates.go b/internal/codeguard/rules/catalog_fix_templates.go index 53bd967..1d3c192 100644 --- a/internal/codeguard/rules/catalog_fix_templates.go +++ b/internal/codeguard/rules/catalog_fix_templates.go @@ -25,6 +25,7 @@ var fixTemplates = mergeFixTemplates( securityFixTemplates, securityLanguageFixTemplates, designFixTemplates, + designPolicyFixTemplates, miscFixTemplates, contextFixTemplates, contextReadinessFixTemplates, diff --git a/internal/codeguard/rules/catalog_fix_templates_design.go b/internal/codeguard/rules/catalog_fix_templates_design.go index 3ab5bca..93650a8 100644 --- a/internal/codeguard/rules/catalog_fix_templates_design.go +++ b/internal/codeguard/rules/catalog_fix_templates_design.go @@ -6,6 +6,13 @@ import "github.com/devr-tools/codeguard/internal/codeguard/core" // cycles, size limits, and dependency-graph concentration. All are guided — // each template shows the import restructure or extraction shape. var designFixTemplates = map[string]core.FixTemplate{ + "design.layer-boundary": {Kind: guided, Text: "Route the dependency through an allowed inward-facing contract.\n\nBefore:\n// domain imports adapters/database\n\nAfter:\n// domain owns a Store interface; adapters/database implements it"}, + "design.unassigned-module": {Kind: guided, Text: "Assign the module to the layer and domain that own its responsibility.\n\nAfter:\n// add the module's path pattern to the matching layer/domain in design_rules.yml"}, + "design.domain-boundary": {Kind: guided, Text: "Import another domain only through an allowed public contract.\n\nBefore:\n// billing imports orders/internal/calculator\n\nAfter:\n// billing imports orders/contracts/order"}, + "design.data-ownership": {Kind: guided, Text: "Keep persistence details inside the owning domain.\n\nBefore:\n// billing imports orders/data/order_record\n\nAfter:\n// billing calls an orders public API or consumes an orders event"}, + "design.capability-boundary": {Kind: guided, Text: "Move sensitive technical access to an approved adapter.\n\nBefore:\n// domain imports a database or cloud SDK\n\nAfter:\n// an allowed adapter imports the SDK and implements a domain-owned interface"}, + "design.private-module-import": {Kind: guided, Text: "Import through the component's declared public entrypoint instead of reaching into an implementation module.\n\nBefore:\nimport { parseToken } from '../auth/internal/token-parser';\n\nAfter:\nimport { parseToken } from '../auth';\n// if this API is intentionally public, export it from the configured entrypoint"}, + "design.production-imports-test": {Kind: guided, Text: "Move the needed helper into production-owned code so runtime modules do not depend on tests, mocks, or fixtures.\n\nBefore:\nimport { fakeClock } from '../test/helpers/clock';\n\nAfter:\nimport { Clock } from '../platform/clock';\n// keep fakeClock in test code as a test-only implementation of Clock"}, "design.command-check": {Kind: guided, Text: "Run the configured design command locally and fix each reported boundary violation at its source.\n\nBefore:\n$ npx depcruise src\nerror no-cli-imports: src/lib/render.ts -> src/cli/flags.ts\n\nAfter:\n// move the shared flag types from src/cli into src/lib\n// re-run until the command exits 0; adjust the command only if it does not fit the target"}, "design.diff-command-check": {Kind: guided, Text: "Restore the contract the diff command reports as broken, or update the target contract deliberately.\n\nBefore:\n$ oasdiff breaking base.yaml head.yaml\nDELETE /v1/orders removed\n\nAfter:\n// restore the removed operation and ship the replacement additively\n// or version the API and update the configured contract target in the same change"}, "design.cmd-through-internal-cli": {Kind: guided, Text: "Route the entrypoint through internal/cli instead of importing service packages directly.\n\nBefore:\n// cmd/app/main.go\nimport \"example.com/app/internal/codeguard/runner\"\n\nAfter:\n// cmd/app/main.go\nimport \"example.com/app/internal/cli\"\n\nfunc main() {\n\tos.Exit(cli.Run(os.Args[1:], os.Stdin, os.Stdout, os.Stderr))\n}"}, diff --git a/internal/codeguard/rules/catalog_fix_templates_design_policy.go b/internal/codeguard/rules/catalog_fix_templates_design_policy.go new file mode 100644 index 0000000..abd5e0e --- /dev/null +++ b/internal/codeguard/rules/catalog_fix_templates_design_policy.go @@ -0,0 +1,8 @@ +package rules + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +var designPolicyFixTemplates = map[string]core.FixTemplate{ + "design.unreachable-module": {Kind: guided, Text: "Remove the unreachable production module, connect it from an approved entrypoint, or add the missing entrypoint deliberately.\n\nBefore:\n// src/legacy.ts is reachable from no application or package entrypoint\n\nAfter:\n// delete legacy.ts if dead, or import its supported API from the configured entrypoint"}, + "design.stability-direction": {Kind: guided, Text: "Make the dependency point toward the more stable abstraction.\n\nBefore:\n// shared/core imports a volatile feature adapter\n\nAfter:\n// shared/core owns a small contract; the volatile adapter implements it at the composition root"}, +} diff --git a/pkg/codeguard/sdk_types_config_design.go b/pkg/codeguard/sdk_types_config_design.go new file mode 100644 index 0000000..fa4c7df --- /dev/null +++ b/pkg/codeguard/sdk_types_config_design.go @@ -0,0 +1,11 @@ +package codeguard + +import "github.com/devr-tools/codeguard/internal/codeguard/core" + +type DesignLayerConfig = core.DesignLayerConfig +type DesignDomainConfig = core.DesignDomainConfig +type DesignCapabilityConfig = core.DesignCapabilityConfig +type DesignPublicSurfaceConfig = core.DesignPublicSurfaceConfig +type DesignProductionTestConfig = core.DesignProductionTestConfig +type DesignReachabilityConfig = core.DesignReachabilityConfig +type DesignStabilityConfig = core.DesignStabilityConfig diff --git a/tests/checks/design_architecture_boundaries_test.go b/tests/checks/design_architecture_boundaries_test.go new file mode 100644 index 0000000..81ee009 --- /dev/null +++ b/tests/checks/design_architecture_boundaries_test.go @@ -0,0 +1,179 @@ +package checks_test + +import ( + "context" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func designFinding(t *testing.T, report codeguard.Report, ruleID string) codeguard.Finding { + t.Helper() + for _, section := range report.Sections { + if section.Name != "Design Patterns" { + continue + } + for _, finding := range section.Findings { + if finding.RuleID == ruleID { + return finding + } + } + } + t.Fatalf("finding %q not found", ruleID) + return codeguard.Finding{} +} + +func TestDesignGoLayerBoundaryUsesImportingFileAndLine(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "go.mod"), "module example.com/layers\n\ngo 1.23.0\n") + writeFile(t, filepath.Join(dir, "internal", "domain", "model.go"), "package domain\n") + writeFile(t, filepath.Join(dir, "internal", "adapters", "store.go"), "package adapters\n") + writeFile(t, filepath.Join(dir, "internal", "domain", "service.go"), "package domain\n\nimport _ \"example.com/layers/internal/adapters\"\n") + writeFile(t, filepath.Join(dir, "misc", "orphan.go"), "package misc\n") + + required := true + cfg := graphTestConfig("design-go-layer-boundary", dir, "go") + cfg.Checks.DesignRules.RequireBoundaryAssignment = &required + cfg.Checks.DesignRules.Layers = []codeguard.DesignLayerConfig{ + {Name: "domain", Paths: []string{"internal/domain/**"}, MayDependOn: []string{"domain"}, DeniedExternal: []string{"database/sql"}}, + {Name: "adapters", Paths: []string{"internal/adapters/**"}, MayDependOn: []string{"domain"}}, + } + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + boundary := designFinding(t, report, "design.layer-boundary") + if boundary.Path != "internal/domain/service.go" || boundary.Line != 3 { + t.Fatalf("layer finding location = %s:%d, want internal/domain/service.go:3", boundary.Path, boundary.Line) + } + if !strings.Contains(boundary.Message, `layer "domain"`) || !strings.Contains(boundary.Message, `layer "adapters"`) { + t.Fatalf("layer finding lacks exact boundary names: %q", boundary.Message) + } + unassigned := designFinding(t, report, "design.unassigned-module") + if unassigned.Path != "misc/orphan.go" { + t.Fatalf("unassigned finding path = %q, want misc/orphan.go", unassigned.Path) + } +} + +func TestDesignTypeScriptDomainAndDataOwnershipBoundaries(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "orders", "contracts", "order.ts"), "export interface Order { id: string }\n") + writeFile(t, filepath.Join(dir, "src", "orders", "data", "store.ts"), "export const table = 'orders';\n") + writeFile(t, filepath.Join(dir, "src", "billing", "allowed.ts"), "import type { Order } from '../orders/contracts/order';\nexport const id = (o: Order) => o.id;\n") + writeFile(t, filepath.Join(dir, "src", "billing", "forbidden.ts"), "// billing service\nimport { table } from '../orders/data/store';\nexport const source = table;\n") + + cfg := graphTestConfig("design-ts-domains", dir, "typescript") + cfg.Checks.DesignRules.Domains = []codeguard.DesignDomainConfig{ + {Name: "billing", Paths: []string{"src/billing/**"}, MayDependOn: []string{"orders"}}, + {Name: "orders", Paths: []string{"src/orders/**"}, PublicPaths: []string{"src/orders/contracts/**"}, DataPaths: []string{"src/orders/data/**"}}, + } + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + for _, ruleID := range []string{"design.domain-boundary", "design.data-ownership"} { + finding := designFinding(t, report, ruleID) + if finding.Path != "src/billing/forbidden.ts" || finding.Line != 2 { + t.Fatalf("%s location = %s:%d, want src/billing/forbidden.ts:2", ruleID, finding.Path, finding.Line) + } + } +} + +func TestDesignLayerDeniedExternalImport(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "go.mod"), "module example.com/external-layer\n\ngo 1.23.0\n") + writeFile(t, filepath.Join(dir, "internal", "domain", "query.go"), "package domain\n\nimport _ \"database/sql\"\n") + cfg := graphTestConfig("design-layer-external", dir, "go") + cfg.Checks.DesignRules.Layers = []codeguard.DesignLayerConfig{{ + Name: "domain", Paths: []string{"internal/domain/**"}, DeniedExternal: []string{"database/sql"}, + }} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := designFinding(t, report, "design.layer-boundary") + if finding.Path != "internal/domain/query.go" || finding.Line != 3 || !strings.Contains(finding.Message, "database/sql") { + t.Fatalf("external layer finding = %s:%d %q", finding.Path, finding.Line, finding.Message) + } +} + +func TestDesignCapabilityBoundaryCoversSupportedLanguageImports(t *testing.T) { + tests := []struct { + name string + language string + file string + source string + pattern string + line int + extraFiles map[string]string + }{ + {name: "go", language: "go", file: "internal/domain/service.go", source: "package domain\n\nimport _ \"database/sql\"\n", pattern: "database/sql", line: 3, extraFiles: map[string]string{"go.mod": "module example.com/capability\n\ngo 1.23.0\n"}}, + {name: "typescript", language: "typescript", file: "src/domain/service.ts", source: "import { S3Client } from '@aws-sdk/client-s3';\nexport const client = S3Client;\n", pattern: "@aws-sdk/**", line: 1}, + {name: "python", language: "python", file: "src/domain/service.py", source: "import boto3\n", pattern: "boto3", line: 1}, + {name: "rust", language: "rust", file: "src/domain/service.rs", source: "use aws_sdk_s3::Client;\n", pattern: "aws_sdk_s3/**", line: 1}, + {name: "java", language: "java", file: "src/domain/Service.java", source: "package domain;\nimport software.amazon.awssdk.services.s3.S3Client;\npublic class Service {}\n", pattern: "software.amazon.awssdk.**", line: 2}, + {name: "cpp", language: "cpp", file: "src/domain/service.cpp", source: "#include \n", pattern: "aws/**", line: 1}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, filepath.FromSlash(tt.file)), tt.source) + for file, content := range tt.extraFiles { + writeFile(t, filepath.Join(dir, file), content) + } + cfg := graphTestConfig("design-capability-"+tt.name, dir, tt.language) + cfg.Checks.DesignRules.Capabilities = []codeguard.DesignCapabilityConfig{{ + Name: "cloud-or-database", Imports: []string{tt.pattern}, AllowedPaths: []string{"src/adapters/**", "internal/adapters/**"}, + }} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := designFinding(t, report, "design.capability-boundary") + if finding.Path != filepath.ToSlash(tt.file) || finding.Line != tt.line { + t.Fatalf("capability finding location = %s:%d, want %s:%d", finding.Path, finding.Line, filepath.ToSlash(tt.file), tt.line) + } + }) + } +} + +func TestDesignRustUsesLanguageNeutralLayerPolicy(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "lib.rs"), "mod domain;\nmod adapters;\n") + writeFile(t, filepath.Join(dir, "src", "domain.rs"), "use crate::adapters::Store;\n") + writeFile(t, filepath.Join(dir, "src", "adapters.rs"), "pub struct Store;\n") + + cfg := graphTestConfig("design-rust-layer", dir, "rust") + cfg.Checks.DesignRules.Layers = []codeguard.DesignLayerConfig{ + {Name: "domain", Paths: []string{"src/domain.rs"}, DenyDependOn: []string{"adapters"}}, + {Name: "adapters", Paths: []string{"src/adapters.rs"}}, + } + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := designFinding(t, report, "design.layer-boundary") + if finding.Path != "src/domain.rs" || finding.Line != 1 { + t.Fatalf("Rust boundary location = %s:%d, want src/domain.rs:1", finding.Path, finding.Line) + } +} + +func TestDesignArchitectureRulesAreOptIn(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "service.ts"), "import '@aws-sdk/client-s3';\n") + report, err := codeguard.Run(context.Background(), graphTestConfig("design-boundaries-opt-in", dir, "typescript")) + if err != nil { + t.Fatalf("run: %v", err) + } + for _, ruleID := range []string{"design.layer-boundary", "design.unassigned-module", "design.domain-boundary", "design.data-ownership", "design.capability-boundary"} { + assertFindingRuleAbsent(t, report, "Design Patterns", ruleID) + } +} diff --git a/tests/checks/design_encapsulation_boundaries_test.go b/tests/checks/design_encapsulation_boundaries_test.go new file mode 100644 index 0000000..1158bdc --- /dev/null +++ b/tests/checks/design_encapsulation_boundaries_test.go @@ -0,0 +1,115 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestDesignPublicSurfaceRejectsTypeScriptDeepImport(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "app.ts"), "// application entrypoint\nimport { parseToken } from './auth/internal/token';\n") + writeFile(t, filepath.Join(dir, "src", "auth", "index.ts"), "export { parseToken } from './internal/token';\n") + writeFile(t, filepath.Join(dir, "src", "auth", "internal", "token.ts"), "export const parseToken = () => 'token';\n") + + cfg := graphTestConfig("design-ts-public-surface", dir, "typescript") + cfg.Checks.DesignRules.PublicSurfaces = []codeguard.DesignPublicSurfaceConfig{{ + Name: "auth", + Paths: []string{"src/auth/**"}, + Entrypoints: []string{"src/auth/index.ts"}, + }} + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + finding := findFinding(t, report, "Design Patterns", "design.private-module-import") + if finding.Path != "src/app.ts" || finding.Line != 2 { + t.Fatalf("finding location = %s:%d, want src/app.ts:2", finding.Path, finding.Line) + } +} + +func TestDesignPublicSurfaceAllowsEntrypointAndInternalImports(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "app.ts"), "import { parseToken } from './auth';\n") + writeFile(t, filepath.Join(dir, "src", "auth", "index.ts"), "export { parseToken } from './internal/token';\n") + writeFile(t, filepath.Join(dir, "src", "auth", "internal", "token.ts"), "export const parseToken = () => 'token';\n") + + cfg := graphTestConfig("design-ts-public-entrypoint", dir, "typescript") + cfg.Checks.DesignRules.PublicSurfaces = []codeguard.DesignPublicSurfaceConfig{{ + Name: "auth", + Paths: []string{"src/auth/**"}, + Entrypoints: []string{"src/auth/index.ts"}, + }} + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Design Patterns", "design.private-module-import") +} + +func TestDesignProductionCodeRejectsTypeScriptTestImport(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "app.ts"), "// runtime module\nimport { fakeClock } from '../test/helpers/clock';\n") + writeFile(t, filepath.Join(dir, "test", "helpers", "clock.ts"), "export const fakeClock = () => 0;\n") + + cfg := graphTestConfig("design-ts-production-test", dir, "typescript") + cfg.Checks.DesignRules.ProductionTest = &codeguard.DesignProductionTestConfig{ + ProductionPaths: []string{"src/**"}, + TestPaths: []string{"test/**"}, + } + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + finding := findFinding(t, report, "Design Patterns", "design.production-imports-test") + if finding.Path != "src/app.ts" || finding.Line != 2 { + t.Fatalf("finding location = %s:%d, want src/app.ts:2", finding.Path, finding.Line) + } +} + +func TestDesignProductionTestPolicyCanBeDisabled(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "app.ts"), "import { fakeClock } from '../test/helpers/clock';\n") + writeFile(t, filepath.Join(dir, "test", "helpers", "clock.ts"), "export const fakeClock = () => 0;\n") + off := false + + cfg := graphTestConfig("design-ts-production-test-disabled", dir, "typescript") + cfg.Checks.DesignRules.ProductionTest = &codeguard.DesignProductionTestConfig{ + Enabled: &off, + ProductionPaths: []string{"src/**"}, + TestPaths: []string{"test/**"}, + } + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + assertFindingRuleAbsent(t, report, "Design Patterns", "design.production-imports-test") +} + +func TestDesignEncapsulationPoliciesUseGoImportSourceLocation(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "go.mod"), "module example.com/app\n\ngo 1.22\n") + writeFile(t, filepath.Join(dir, "internal", "service", "service.go"), "package service\n\nimport \"example.com/app/internal/testsupport\"\n") + writeFile(t, filepath.Join(dir, "internal", "testsupport", "fixture.go"), "package testsupport\n") + + cfg := graphTestConfig("design-go-production-test", dir, "go") + cfg.Checks.DesignRules.ProductionTest = &codeguard.DesignProductionTestConfig{ + ProductionPaths: []string{"internal/service/**"}, + TestPaths: []string{"internal/testsupport/**"}, + } + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + + finding := findFinding(t, report, "Design Patterns", "design.production-imports-test") + if finding.Path != "internal/service/service.go" || finding.Line != 3 { + t.Fatalf("finding location = %s:%d, want internal/service/service.go:3", finding.Path, finding.Line) + } +} diff --git a/tests/checks/design_graph_policy_test.go b/tests/checks/design_graph_policy_test.go new file mode 100644 index 0000000..fbddd9e --- /dev/null +++ b/tests/checks/design_graph_policy_test.go @@ -0,0 +1,124 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestDesignReachabilityReportsModulesOutsideApprovedEntrypoints(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "main.ts"), `import "./reachable";`) + writeFile(t, filepath.Join(dir, "src", "reachable.ts"), `export const reachable = true;`) + writeFile(t, filepath.Join(dir, "src", "orphan.ts"), `export const orphan = true;`) + + cfg := designPolicyTestConfig(dir) + cfg.Targets[0].Entrypoints = []string{"src/main.ts"} + cfg.Checks.DesignRules.Reachability = &codeguard.DesignReachabilityConfig{} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := findFinding(t, report, "Design Patterns", "design.unreachable-module") + if finding.Path != "src/orphan.ts" { + t.Fatalf("unreachable finding path = %q, want src/orphan.ts", finding.Path) + } +} + +func TestDesignReachabilityMatchesAnyFileInGoEntrypointPackage(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "go.mod"), "module example.com/reachability\n\ngo 1.23.0\n") + writeFile(t, filepath.Join(dir, "cmd", "app", "a.go"), "package main\n\nimport _ \"example.com/reachability/internal/live\"\n") + writeFile(t, filepath.Join(dir, "cmd", "app", "main.go"), "package main\n\nfunc main() {}\n") + writeFile(t, filepath.Join(dir, "internal", "live", "live.go"), "package live\n") + writeFile(t, filepath.Join(dir, "internal", "orphan", "orphan.go"), "package orphan\n") + + cfg := designPolicyTestConfig(dir) + cfg.Targets[0].Language = "go" + cfg.Targets[0].Entrypoints = []string{"cmd/**/main.go"} + cfg.Checks.DesignRules.Reachability = &codeguard.DesignReachabilityConfig{} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := findFinding(t, report, "Design Patterns", "design.unreachable-module") + if finding.Path != "internal/orphan/orphan.go" { + t.Fatalf("unreachable finding path = %q, want internal/orphan/orphan.go", finding.Path) + } + for _, section := range report.Sections { + for _, candidate := range section.Findings { + if candidate.RuleID == "design.unreachable-module" && candidate.Path == "internal/live/live.go" { + t.Fatal("module imported by the Go entrypoint package was reported unreachable") + } + } + } +} + +func TestDesignStabilityReportsDependencyTowardVolatileModule(t *testing.T) { + dir := t.TempDir() + for _, importer := range []string{"one", "two", "three"} { + writeFile(t, filepath.Join(dir, importer+".ts"), `import "./stable";`) + } + writeFile(t, filepath.Join(dir, "stable.ts"), `import "./volatile";`) + writeFile(t, filepath.Join(dir, "volatile.ts"), ` +import "./leaf-one"; +import "./leaf-two"; +import "./leaf-three"; +import "./leaf-four"; +`) + for _, leaf := range []string{"leaf-one", "leaf-two", "leaf-three", "leaf-four"} { + writeFile(t, filepath.Join(dir, leaf+".ts"), `export const value = true;`) + } + + cfg := designPolicyTestConfig(dir) + cfg.Checks.DesignRules.Stability = &codeguard.DesignStabilityConfig{ + MinimumFanIn: 3, + MaxInstabilityDelta: 0.30, + } + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := findFinding(t, report, "Design Patterns", "design.stability-direction") + if finding.Path != "stable.ts" || finding.Line != 1 { + t.Fatalf("stability finding location = %s:%d, want stable.ts:1", finding.Path, finding.Line) + } +} + +func TestDesignGraphPoliciesCanBeDisabled(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "main.ts"), `export const main = true;`) + writeFile(t, filepath.Join(dir, "src", "orphan.ts"), `export const orphan = true;`) + off := false + + cfg := designPolicyTestConfig(dir) + cfg.Checks.DesignRules.Reachability = &codeguard.DesignReachabilityConfig{ + Enabled: &off, + Entrypoints: []string{"src/main.ts"}, + } + cfg.Checks.DesignRules.Stability = &codeguard.DesignStabilityConfig{Enabled: &off} + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + assertFindingRuleAbsent(t, report, "Design Patterns", "design.unreachable-module") + assertFindingRuleAbsent(t, report, "Design Patterns", "design.stability-direction") +} + +func designPolicyTestConfig(dir string) codeguard.Config { + cfg := codeguard.ExampleConfig() + cfg.Name = "design-graph-policy" + cfg.Targets = []codeguard.TargetConfig{{Name: "app", Path: dir, Language: "typescript"}} + cfg.Checks.Design = true + cfg.Checks.Quality = false + cfg.Checks.Security = false + cfg.Checks.Prompts = false + cfg.Checks.CI = false + return cfg +} diff --git a/tests/checks/design_policy_match_test.go b/tests/checks/design_policy_match_test.go new file mode 100644 index 0000000..91a7123 --- /dev/null +++ b/tests/checks/design_policy_match_test.go @@ -0,0 +1,30 @@ +package checks_test + +import ( + "context" + "path/filepath" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestDesignBoundaryPathPatternsSupportRecursiveGlobCharacterClassAndLiteralDescendants(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "src", "domain", "handler2.ts"), "import { store } from '../adapters/store';\nexport const run = store;\n") + writeFile(t, filepath.Join(dir, "src", "adapters", "store.ts"), "export const store = 1;\n") + + cfg := graphTestConfig("design-path-patterns", dir, "typescript") + cfg.Checks.DesignRules.Layers = []codeguard.DesignLayerConfig{ + {Name: "domain", Paths: []string{"src/**/handler[0-9].ts"}, DenyDependOn: []string{"adapters"}}, + {Name: "adapters", Paths: []string{"src/adapters"}}, + } + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + finding := findFinding(t, report, "Design Patterns", "design.layer-boundary") + if finding.Path != "src/domain/handler2.ts" || finding.Line != 1 { + t.Fatalf("finding location = %s:%d, want src/domain/handler2.ts:1", finding.Path, finding.Line) + } +} diff --git a/tests/codeguard/design_rules_config_io_test.go b/tests/codeguard/design_rules_config_io_test.go new file mode 100644 index 0000000..6106075 --- /dev/null +++ b/tests/codeguard/design_rules_config_io_test.go @@ -0,0 +1,238 @@ +package codeguard_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestLoadFileExplicitDesignRulesYAMLInlinePrecedence(t *testing.T) { + root := t.TempDir() + writeConfigTestFile(t, filepath.Join(root, ".codeguard", "policy.yml"), ` +max_decls_per_file: 99 +detect_import_cycles: true +forbidden_package_names: [legacy] +`) + configPath := filepath.Join(root, "codeguard.yml") + writeConfigTestFile(t, configPath, ` +name: external-policy +targets: + - name: repo + path: . + language: go +checks: + design_rules_file: .codeguard/policy.yml + design_rules: + max_decls_per_file: 0 + detect_import_cycles: false + forbidden_package_names: [] +output: + format: text +`) + + cfg, err := codeguard.LoadConfigFile(configPath) + if err != nil { + t.Fatalf("LoadFile: %v", err) + } + if cfg.Checks.DesignRules.MaxDeclsPerFile != 0 { + t.Fatalf("max_decls_per_file = %d, want explicit inline zero", cfg.Checks.DesignRules.MaxDeclsPerFile) + } + if cfg.Checks.DesignRules.DetectImportCycles == nil || *cfg.Checks.DesignRules.DetectImportCycles { + t.Fatal("detect_import_cycles did not preserve explicit inline false") + } + if cfg.Checks.DesignRules.ForbiddenPackageNames == nil || len(cfg.Checks.DesignRules.ForbiddenPackageNames) != 0 { + t.Fatalf("forbidden_package_names = %#v, want explicit empty list", cfg.Checks.DesignRules.ForbiddenPackageNames) + } +} + +func TestLoadFileExternalDesignRulesPreserveExplicitZeroAndEmptyValues(t *testing.T) { + root := t.TempDir() + writeConfigTestFile(t, filepath.Join(root, ".codeguard", "design_rules.yml"), ` +max_decls_per_file: 0 +forbidden_package_names: [] +`) + configPath := filepath.Join(root, "codeguard.yml") + writeConfigTestFile(t, configPath, ` +name: external-zero-values +profile: strict +targets: + - name: repo + path: . + language: go +checks: {} +output: + format: text +`) + + cfg, err := codeguard.LoadConfigFile(configPath) + if err != nil { + t.Fatalf("LoadFile: %v", err) + } + if cfg.Checks.DesignRules.MaxDeclsPerFile != 0 { + t.Fatalf("max_decls_per_file = %d, want explicit external zero", cfg.Checks.DesignRules.MaxDeclsPerFile) + } + if cfg.Checks.DesignRules.ForbiddenPackageNames == nil || len(cfg.Checks.DesignRules.ForbiddenPackageNames) != 0 { + t.Fatalf("forbidden_package_names = %#v, want explicit external empty list", cfg.Checks.DesignRules.ForbiddenPackageNames) + } +} + +func TestLoadFileExplicitDesignRulesJSONMainRelativeToConfigDirectory(t *testing.T) { + root := t.TempDir() + configDir := filepath.Join(root, ".codeguard") + writeConfigTestFile(t, filepath.Join(configDir, "team-policy.yml"), ` +max_methods_per_type: 31 +detect_god_modules: false +`) + configPath := filepath.Join(configDir, "codeguard.json") + writeConfigTestFile(t, configPath, `{ + "name": "json-external-policy", + "targets": [{"name": "repo", "path": "..", "language": "go"}], + "checks": { + "design_rules_file": "team-policy.yml", + "design_rules": {"max_methods_per_type": 7} + }, + "output": {"format": "json"} +}`) + + cfg, err := codeguard.LoadConfigFile(configPath) + if err != nil { + t.Fatalf("LoadFile: %v", err) + } + if got := cfg.Checks.DesignRules.MaxMethodsPerType; got != 7 { + t.Fatalf("max_methods_per_type = %d, want inline value 7", got) + } + if cfg.Checks.DesignRules.DetectGodModules == nil || *cfg.Checks.DesignRules.DetectGodModules { + t.Fatal("external detect_god_modules=false was not loaded") + } +} + +func TestLoadFileDiscoversDesignRules(t *testing.T) { + tests := []struct { + name string + configPath func(string) string + policyPath func(string) string + }{ + { + name: "project dot-codeguard directory", + configPath: func(root string) string { return filepath.Join(root, "codeguard.yml") }, + policyPath: func(root string) string { return filepath.Join(root, ".codeguard", "design_rules.yml") }, + }, + { + name: "beside config already in dot-codeguard", + configPath: func(root string) string { return filepath.Join(root, ".codeguard", "codeguard.yml") }, + policyPath: func(root string) string { return filepath.Join(root, ".codeguard", "design_rules.yaml") }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + writeConfigTestFile(t, tt.policyPath(root), "max_interface_methods: 23\n") + writeConfigTestFile(t, tt.configPath(root), ` +name: discovered-policy +targets: + - name: repo + path: . + language: go +checks: {} +output: + format: text +`) + + cfg, err := codeguard.LoadConfigFile(tt.configPath(root)) + if err != nil { + t.Fatalf("LoadFile: %v", err) + } + if got := cfg.Checks.DesignRules.MaxInterfaceMethods; got != 23 { + t.Fatalf("max_interface_methods = %d, want discovered value 23", got) + } + }) + } +} + +func TestLoadFileDesignRulesErrors(t *testing.T) { + t.Run("invalid external document", func(t *testing.T) { + root := t.TempDir() + writeConfigTestFile(t, filepath.Join(root, ".codeguard", "design_rules.yml"), "layers: [\n") + configPath := filepath.Join(root, "codeguard.yml") + writeConfigTestFile(t, configPath, minimalConfigYAML("")) + + _, err := codeguard.LoadConfigFile(configPath) + if err == nil || !strings.Contains(err.Error(), "parse design rules file") { + t.Fatalf("error = %v, want invalid design rules error", err) + } + }) + + t.Run("missing explicit file", func(t *testing.T) { + root := t.TempDir() + configPath := filepath.Join(root, "codeguard.yml") + writeConfigTestFile(t, configPath, minimalConfigYAML("missing.yml")) + + _, err := codeguard.LoadConfigFile(configPath) + if err == nil || !strings.Contains(err.Error(), "design_rules_file") { + t.Fatalf("error = %v, want missing explicit file error", err) + } + }) + + t.Run("path traversal", func(t *testing.T) { + parent := t.TempDir() + root := filepath.Join(parent, "repo") + writeConfigTestFile(t, filepath.Join(parent, "outside.yml"), "max_decls_per_file: 2\n") + configPath := filepath.Join(root, ".codeguard", "codeguard.yml") + writeConfigTestFile(t, configPath, minimalConfigYAML("../../outside.yml")) + + _, err := codeguard.LoadConfigFile(configPath) + if err == nil || !strings.Contains(err.Error(), "escapes") { + t.Fatalf("error = %v, want traversal rejection", err) + } + }) + + t.Run("symlink escape", func(t *testing.T) { + parent := t.TempDir() + root := filepath.Join(parent, "repo") + outside := filepath.Join(parent, "outside.yml") + writeConfigTestFile(t, outside, "max_decls_per_file: 2\n") + link := filepath.Join(root, ".codeguard", "linked.yml") + if err := os.MkdirAll(filepath.Dir(link), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.Symlink(outside, link); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + configPath := filepath.Join(root, ".codeguard", "codeguard.yml") + writeConfigTestFile(t, configPath, minimalConfigYAML("linked.yml")) + + _, err := codeguard.LoadConfigFile(configPath) + if err == nil || !strings.Contains(err.Error(), "symlink") { + t.Fatalf("error = %v, want symlink escape rejection", err) + } + }) +} + +func minimalConfigYAML(designRulesFile string) string { + fileSetting := "" + if designRulesFile != "" { + fileSetting = " design_rules_file: " + designRulesFile + "\n" + } + return `name: policy-errors +targets: + - name: repo + path: . + language: go +checks: +` + fileSetting + `output: + format: text +` +} + +func writeConfigTestFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", filepath.Dir(path), err) + } + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} diff --git a/tests/codeguard/design_rules_config_validate_test.go b/tests/codeguard/design_rules_config_validate_test.go new file mode 100644 index 0000000..6adaa1c --- /dev/null +++ b/tests/codeguard/design_rules_config_validate_test.go @@ -0,0 +1,105 @@ +package codeguard_test + +import ( + "strings" + "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" +) + +func TestValidateDesignArchitectureRules(t *testing.T) { + tests := []struct { + name string + rules codeguard.DesignRulesConfig + want string + }{ + { + name: "unknown layer dependency", + rules: codeguard.DesignRulesConfig{Layers: []codeguard.DesignLayerConfig{{ + Name: "domain", Paths: []string{"domain/**"}, MayDependOn: []string{"missing"}, + }}}, + want: "unknown layer", + }, + { + name: "duplicate domain", + rules: codeguard.DesignRulesConfig{Domains: []codeguard.DesignDomainConfig{ + {Name: "orders", Paths: []string{"orders/**"}}, + {Name: "orders", Paths: []string{"other/**"}}, + }}, + want: "duplicate name", + }, + { + name: "incomplete capability", + rules: codeguard.DesignRulesConfig{Capabilities: []codeguard.DesignCapabilityConfig{{ + Name: "database", Imports: []string{"database/sql"}, + }}}, + want: "allowed_paths", + }, + { + name: "incomplete public surface", + rules: codeguard.DesignRulesConfig{PublicSurfaces: []codeguard.DesignPublicSurfaceConfig{{ + Name: "billing", Paths: []string{"billing/**"}, + }}}, + want: "entrypoints", + }, + { + name: "incomplete production test policy", + rules: codeguard.DesignRulesConfig{ProductionTest: &codeguard.DesignProductionTestConfig{ + ProductionPaths: []string{"src/**"}, + }}, + want: "test_paths", + }, + { + name: "invalid stability delta", + rules: codeguard.DesignRulesConfig{Stability: &codeguard.DesignStabilityConfig{ + MaxInstabilityDelta: 1.1, + }}, + want: "between 0 and 1", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateDesignRulesConfig(tt.rules) + if err == nil || !strings.Contains(err.Error(), tt.want) { + t.Fatalf("error = %v, want containing %q", err, tt.want) + } + }) + } +} + +func TestValidateDesignArchitectureRulesRejectsCaseInsensitiveDuplicates(t *testing.T) { + rules := codeguard.DesignRulesConfig{Layers: []codeguard.DesignLayerConfig{ + {Name: "Domain", Paths: []string{"internal/domain/**"}}, + {Name: "domain", Paths: []string{"src/domain/**"}}, + }} + if err := validateDesignRulesConfig(rules); err == nil || !strings.Contains(err.Error(), "duplicate") { + t.Fatalf("error = %v, want case-insensitive duplicate rejection", err) + } +} + +func TestValidateDesignArchitectureRulesAcceptsCompletePolicy(t *testing.T) { + rules := codeguard.DesignRulesConfig{ + Layers: []codeguard.DesignLayerConfig{ + {Name: "domain", Paths: []string{"domain/**"}, MayDependOn: []string{"domain"}}, + {Name: "adapters", Paths: []string{"adapters/**"}, MayDependOn: []string{"domain"}}, + }, + Domains: []codeguard.DesignDomainConfig{ + {Name: "shared", Paths: []string{"shared/**"}, PublicPaths: []string{"shared/contracts/**"}}, + {Name: "orders", Paths: []string{"orders/**"}, MayDependOn: []string{"shared"}}, + }, + Capabilities: []codeguard.DesignCapabilityConfig{{Name: "database", Imports: []string{"database/sql"}, AllowedPaths: []string{"adapters/**"}}}, + PublicSurfaces: []codeguard.DesignPublicSurfaceConfig{{Name: "orders", Paths: []string{"orders/**"}, Entrypoints: []string{"orders/index.ts"}}}, + ProductionTest: &codeguard.DesignProductionTestConfig{ProductionPaths: []string{"src/**"}, TestPaths: []string{"test/**"}}, + Reachability: &codeguard.DesignReachabilityConfig{Entrypoints: []string{"src/main.ts"}}, + Stability: &codeguard.DesignStabilityConfig{MinimumFanIn: 3, MaxInstabilityDelta: 0.4}, + } + if err := validateDesignRulesConfig(rules); err != nil { + t.Fatalf("validate complete policy: %v", err) + } +} + +func validateDesignRulesConfig(rules codeguard.DesignRulesConfig) error { + cfg := codeguard.ExampleConfig() + cfg.Checks.DesignRules = rules + return codeguard.ValidateConfig(cfg) +}