diff --git a/docs/superpowers/plans/2026-08-29-v1.8.0-repairs.md b/docs/superpowers/plans/2026-08-29-v1.8.0-repairs.md new file mode 100644 index 0000000..a9313a7 --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-v1.8.0-repairs.md @@ -0,0 +1,163 @@ +# CodeGuard v1.8.0 Repairs Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Repair workspace resolution, baseline identity/accounting, installed version reporting, mutable-global precision, and contextual boolean naming for the v1.8.x line. + +**Architecture:** Parse Go workspace/module manifests into per-file resolver metadata, replace many-to-many baseline lookup with a consuming tiered matcher, and separate detector declaration roles from reporting evidence. Keep public schemas and rule IDs stable while testing release metadata through the real command boundary. + +**Tech Stack:** Go, Go AST/parser/token packages, `golang.org/x/mod/modfile`, table-driven tests, CLI integration tests. + +**Spec:** `docs/superpowers/specs/2026-08-29-v1.8.0-repairs-design.md` + +## Global Constraints + +- Do not invoke `go list` or require network/module-cache state for import resolution. +- Preserve existing baseline serialization, CLI flags, report fields, and rule IDs. +- Context/content fingerprints are fallback match signals, never duplicate identity. +- Do not add a strict reassignable-global rule or configuration. +- Unknown helper calls alone are not mutation evidence. +- Imperative boolean vocabulary is grammatical and applies only to parameters. +- Version tests use synthetic `v9.8.7`; release automation validates the real tag. + +--- + +### Task 1: Go workspace/module resolver + +**Files:** +- Create: `internal/codeguard/checks/quality/quality_go_modules.go` +- Modify: `internal/codeguard/checks/quality/quality_ai_target_go.go` +- Test: `tests/checks/quality_ai_additional_test.go` + +**Interfaces:** +- Produces: `newGoModuleResolver(root string) goModuleResolver` +- Produces: `(goModuleResolver).metadataForFile(rel string) goModuleMetadata` +- Produces: `(goModuleMetadata).resolvesImport(importPath string) bool` + +- [ ] Write a failing fixture test with `go.work`, sibling/nested modules, direct and indirect requirements, module/workspace replacements, standard-library imports, a cross-workspace import, and one unresolved dotted import. Assert only the unresolved import produces `quality.ai.hallucinated-import`. +- [ ] Run `szr go test ./tests/checks -run 'TestGo.*Workspace.*Import' -count=1` and confirm valid imports are falsely reported. +- [ ] Implement manifest parsing with `modfile.Parse`/`modfile.ParseWork`, nearest-ancestor module ownership, module-prefix matching, and replacement/workspace module paths. +- [ ] Pass one resolver through `goAITargetFindings` and select metadata per file. +- [ ] Re-run the focused test and `szr go test ./tests/checks -run 'QualityAI' -count=1`. +- [ ] Commit with `fix: resolve Go imports by owning workspace module`. + +### Task 2: Governance scan parity + +**Files:** +- Modify: `internal/cli/baseline.go` +- Test: `internal/cli/baseline_go_workspace_test.go` + +**Interfaces:** +- Modify: `governanceInputs` to retain scan mode/base-ref/target-path inputs needed by `service.RunWithOptions`. +- Consumes: the shared resolver through the normal runner. + +- [ ] Write failing CLI tests that create a workspace baseline and exercise `baseline audit`, `baseline prune -write`, then `baseline prune -check`; assert valid module-specific imports do not become findings and write output immediately passes check. +- [ ] Run `szr go test ./internal/cli -run 'TestBaseline.*GoWorkspace' -count=1` and confirm the governance scan diverges. +- [ ] Route governance commands through the same normalized full-scan options/target configuration as ordinary scans. +- [ ] Re-run the focused CLI tests. +- [ ] Commit with `fix: preserve scan resolution in baseline governance`. + +### Task 3: Deterministic one-to-one baseline matching + +**Files:** +- Modify: `internal/cli/baseline_audit.go` +- Modify: `internal/cli/baseline.go` +- Test: `internal/cli/baseline_audit_test.go` +- Test: `internal/cli/baseline_io_test.go` + +**Interfaces:** +- Create internal `baselineMatch` containing one entry index, one finding index, tier, and collision state. +- Create internal `matchBaselineEntries(entries []core.BaselineEntry, findings []core.Finding) baselineMatchResult`. + +- [ ] Write failing tests proving exact matches consume first, context/content fallbacks use only unmatched records, one finding cannot activate two entries, and ambiguous matches are stable across input order. +- [ ] Write failing tests proving shared weak fingerprints are collisions but not duplicates and do not fail prune check semantics. +- [ ] Run `szr go test ./internal/cli -run 'TestAudit|TestPrune' -count=1` and record the expected count/duplicate failures. +- [ ] Implement sorted tiered matching and exact-only duplicate detection; retain separate weak collision diagnostics. +- [ ] Change prune check to reject only stale, invalid, and exact duplicate records. +- [ ] Re-run focused tests. +- [ ] Commit with `fix: consume baseline matches deterministically`. + +### Task 4: Audit distribution invariants + +**Files:** +- Modify: `internal/cli/baseline_audit.go` +- Test: `internal/cli/baseline_audit_test.go` + +**Interfaces:** +- Consumes: canonical pairs from `matchBaselineEntries`. +- Produces: `Group` values whose count, confidence total, and language total share one input set. + +- [ ] Add failing duplicate/collision-heavy tests that literally sum each `by_rule` confidence and language distribution and compare both with `Group.Count`. +- [ ] Run `szr go test ./internal/cli -run 'TestAudit.*Distribution' -count=1` and confirm the existing many-match samples overcount. +- [ ] Build groups from one canonical finding per active entry; apply sample limits only after distributions are calculated. +- [ ] Re-run focused audit tests. +- [ ] Commit with `fix: align baseline audit distributions`. + +### Task 5: Installed/build version reporting + +**Files:** +- Modify: `internal/version/version.go` +- Create: `internal/version/version_test.go` +- Test: `tests/cli/cli_test.go` +- Inspect: `.github/workflows/release.yml` + +**Interfaces:** +- Preserve: `Resolve(current string, info *debug.BuildInfo) string`. +- Define: linker value precedence, main-module `v9.8.7` resolution, and `devel` fallback. + +- [ ] Add failing table tests for injected `v9.8.7`, build-info `v9.8.7`, precedence, and absent metadata returning `devel` rather than `0.1.0`. +- [ ] Add a CLI integration test that runs `go build -ldflags '-X github.com/devr-tools/codeguard/internal/version.Number=v9.8.7'` on `./cmd/codeguard`, executes `version`, and expects exactly `v9.8.7`. +- [ ] Run `szr go test ./internal/version ./tests/cli -run 'Version' -count=1` and confirm the fallback failure. +- [ ] Replace the legacy fallback with `devel` while preserving linker and valid main-module precedence. +- [ ] Re-run focused version tests and inspect release workflow tag propagation. +- [ ] Commit with `fix: report installed module version`. + +### Task 6: Evidence-based mutable globals + +**Files:** +- Create: `internal/codeguard/checks/quality/quality_mutable_globals.go` +- Modify: `internal/codeguard/checks/quality/quality_precision.go` +- Test: `tests/checks/function_precision_test.go` + +**Interfaces:** +- Define internal declaration classification and evidence kinds for mutable contents, reassignment, known mutation, and mutation-capable escape. +- Preserve public rule ID `quality.mutable-global-state`. + +- [ ] Add failing Go fixture tests for `regexp.MustCompile` exemption, safe unknown read-only calls, binding reassignment, maps/slices, known mutating operations, writable-address escape, shared-storage assignment, returned/exported mutable state, and alias mutation. +- [ ] Run `szr go test ./tests/checks -run 'TestGoMutableGlobal' -count=1` and confirm immutable constructor false positives. +- [ ] Classify declarations independently from package-level evidence; include a narrow immutable-constructor registry and technical-reassignability metadata. +- [ ] Emit only for mutable shapes or meaningful mutation/escape evidence, with evidence-specific messages and calibrated confidence. +- [ ] Re-run focused tests and existing mutable-global precision tests. +- [ ] Commit with `fix: require mutation evidence for Go globals`. + +### Task 7: Contextual imperative boolean names + +**Files:** +- Modify: `internal/codeguard/checks/quality/quality_precision_workstreams_cd.go` +- Modify: `internal/codeguard/checks/quality/quality_precision.go` +- Test: `tests/checks/function_precision_test.go` +- Test: `tests/checks/quality_precision_retune_false_positive_test.go` + +**Interfaces:** +- Create internal `isImperativeBooleanParameterName(name string) bool` based on word boundaries and prefixes `include`, `require`, `allow`, `skip`, `enable`, `disable`, `force`, and `use`. + +- [ ] Add failing Go/C++ tests accepting imperative boolean parameters and rejecting the same grammar for fields, locals, getters, and boolean-returning functions. +- [ ] Run `szr go test ./tests/checks -run 'Test.*ImperativeBoolean' -count=1` and confirm parameters are reported. +- [ ] Apply imperative grammar only while iterating typed parameters; do not add the vocabulary to general predicate names. +- [ ] Re-run focused naming tests and all existing naming precision tests. +- [ ] Commit with `fix: recognize imperative boolean parameters`. + +### Task 8: Full verification + +**Files:** +- Modify only files needed to correct failures caused by Tasks 1-7. + +**Interfaces:** +- Produces: a buildable branch satisfying the design spec without schema or policy expansion. + +- [ ] Run `szr go test ./internal/cli ./internal/version ./tests/cli ./tests/checks -count=1`. +- [ ] Run `szr go test ./... -count=1`. +- [ ] Run the repository lint/static-check target discovered from `Makefile` or CI. +- [ ] Run `szr go build ./cmd/codeguard`. +- [ ] Review `szr git diff --check`, `szr git diff --stat`, and the final diff for scope and compatibility. +- [ ] Commit any verification-only corrections with a narrowly scoped message. diff --git a/docs/superpowers/specs/2026-08-29-v1.8.0-repairs-design.md b/docs/superpowers/specs/2026-08-29-v1.8.0-repairs-design.md new file mode 100644 index 0000000..311da3f --- /dev/null +++ b/docs/superpowers/specs/2026-08-29-v1.8.0-repairs-design.md @@ -0,0 +1,126 @@ +# CodeGuard v1.8.0 Repairs Design + +## Scope + +This repair addresses six v1.8.0 defects without adding a new policy surface: + +1. Make Go import resolution workspace- and module-aware in normal scans, baseline audit, and baseline prune. +2. Make baseline matching deterministic and one-to-one while distinguishing exact duplicates from weaker-fingerprint collisions. +3. Report the installed module version from `codeguard version`, with release linker metadata taking precedence. +4. Improve `quality.mutable-global-state` precision through evidence-based mutation analysis. +5. Permit grammatical imperative names for boolean parameters while retaining predicate guidance in state and result contexts. +6. Keep audit `by_rule` totals consistent with confidence and language distributions after collision handling. + +The baseline schema, CLI flags, report schema, and existing rule IDs remain compatible. A strict rule for technically reassignable globals is explicitly out of scope. + +## Go Workspace and Module Resolution + +CodeGuard will build module-resolution metadata once for a scan target and resolve it per Go source file. A file belongs to the nearest ancestor `go.mod`, so nested modules override parent modules. A containing `go.work` contributes all `use` modules and workspace-level `replace` directives. + +The resolver will parse: + +- each owning module's `module`, direct and indirect `require`, and `replace` directives; +- `go.work` `use` and `replace` directives; +- all workspace module paths; and +- standard-library import paths. + +Resolution is based on module-prefix ownership. A required or replaced module `example.com/dependency` resolves imports beneath that prefix such as `example.com/dependency/client`; the full package import need not appear literally in `go.mod`. Imports belonging to another workspace module also resolve. Local-module imports resolve beneath the owning module path. Imports with no dotted first path segment are treated as standard-library imports. + +The hallucinated-import detector consumes the metadata for the file's owning module instead of reading only `/go.mod`. Normal scans and governance scans use the same runner and resolver. Baseline audit and prune must preserve the same scan options and target configuration used by ordinary full scans rather than creating a reduced dependency-resolution path. + +No `go list` subprocess is introduced. This keeps scans deterministic, fast, and independent of network access, downloaded module caches, and command trust settings. + +## Baseline Identity, Matching, and Diagnostics + +An entry's exact fingerprint is its identity. Context and content fingerprints are weaker fallback signals only. Two records are true duplicates only when they have the same exact identity. Distinct exact fingerprints sharing context or content fingerprints are collisions, are preserved, and do not make `baseline prune -check` fail. + +Matching is deterministic and one-to-one: + +1. Match exact fingerprints first and consume each matched entry/finding pair. +2. Match context fingerprints only among unmatched entries and unmatched findings. +3. Match content fingerprints last, again only among unmatched entries and findings. +4. Sort entries and findings by stable keys before resolving ambiguous groups. +5. Pair ambiguous fallback groups deterministically and report the ambiguity as a collision diagnostic. + +A current finding cannot activate more than one baseline entry. A baseline entry cannot consume more than one current finding for accounting. Collision diagnostics remain separate for audit visibility and describe baseline/current cardinality at the relevant matching tier. + +Prune check fails only for stale entries, invalid entries, or true exact duplicates. Prune write preserves active distinct entries even when their weaker fingerprints collide. If a written baseline has no stale, invalid, or exact-duplicate entries, an immediate prune check must pass. + +## Audit Accounting + +Each active baseline entry receives at most one canonical matched finding from the one-to-one matcher. Group counts, samples, confidence distributions, and language distributions all derive from those same entry/finding pairs. + +For every `by_rule` group: + +- `count` equals the number of active matched entries in the group; +- the sum of confidence distribution counts equals `count`; and +- the sum of language distribution counts equals `count`. + +Duplicate and collision-heavy inputs must preserve these invariants. Sample limits affect only displayed samples, never distribution calculation. + +## Version Resolution + +Version resolution uses this precedence: + +1. a linker-injected release version; +2. `debug.ReadBuildInfo()` main-module version embedded by `go install module/cmd@version`; and +3. the explicit development fallback `devel`. + +The historical hardcoded `0.1.0` value must not override valid linker or build metadata. Tests use a synthetic version such as `v9.8.7` so they remain independent of a particular release. The release pipeline separately verifies propagation of the actual release tag. + +The integration test builds and executes the real `cmd/codeguard` command with synthetic linker metadata. Unit tests exercise module build information and precedence. Together they verify both supported release paths without network access. + +## Mutable Global Precision + +The existing `quality.mutable-global-state` rule becomes a two-phase analysis. + +Declaration classification records: + +- value shape: scalar, map, slice, pointer, immutable-by-convention object, or unknown; +- constructor origin where known; and +- whether the binding is technically reassignable. + +Evidence analysis then looks for demonstrated harmful mutability: + +- reassignment of the package-level binding; +- mutation of map, slice, pointer, or object contents; +- passing an address to a parameter capable of writing through it; +- assignment into mutable shared storage; +- returning or exporting mutable state; +- passing the value to a known mutating API; or +- later mutation through an alias. + +Known immutable-by-convention constructor results, including `regexp.MustCompile`, are exempt by default unless meaningful mutation evidence exists. Go's required `var` syntax is not evidence by itself. Unknown helper calls and ordinary argument passing do not automatically count as mutation-capable escape. Uncertain evidence may be retained internally but does not produce a high-confidence finding. + +Maps, slices, mutation-capable pointers, and demonstrated reassignment continue to produce findings. Internal evidence kinds distinguish mutable contents, reassigned bindings, and mutation-capable escapes so messages can be precise and the implementation can support a future strict rule without adding one now. + +## Boolean Naming by Role + +Boolean naming analysis will classify the symbol's role before applying vocabulary guidance. + +Boolean parameters may use grammatical imperative prefixes such as `include`, `require`, `allow`, `skip`, `enable`, `disable`, `force`, and `use`, followed by a meaningful object or condition. Examples such as `includeInactive`, `requireCompleteMetadata`, `allowPartial`, and `skipCache` are representative rather than hardcoded exceptions. + +The same names remain subject to predicate guidance when used as fields, stored local state, getters, or boolean-returning function names. Existing accepted predicate vocabulary continues to apply. Go and C++ fixtures cover both accepted imperative parameters and rejected state/result uses. + +## Test Strategy + +Implementation follows red-green-refactor cycles. Each behavior first receives a focused failing test that is run to confirm the expected failure before production code changes. + +Workspace fixtures cover a root `go.work`, sibling and nested modules, module-specific direct and indirect requirements, module and workspace replacements, imports across workspace modules, standard-library imports, and a genuinely unresolved import. The fixtures run through normal scan, baseline audit, prune write, and prune check. + +Baseline tests cover exact duplicates, shared context/content fingerprints, ambiguous fallback matches, stable one-to-one consumption, collision reporting, write-then-check idempotence, and report distribution invariants. + +Detector tests cover immutable constructor exemptions, reassignment, mutable collections, known mutation, meaningful escape, unknown read-only calls, alias mutation, imperative boolean parameters, and non-parameter negative cases in both Go and C++. + +Version tests cover linker precedence, synthetic main-module build information, development fallback, and execution of a built command. Final verification runs focused packages throughout development, followed by the complete Go test suite, repository static checks, and a clean command build. + +## Compatibility and Non-Goals + +This repair does not: + +- add a strict reassignable-global rule; +- add configuration for such a rule; +- change baseline serialization or public report fields; +- treat weak-fingerprint collisions as duplicates; +- invoke Go dependency-resolution subprocesses; or +- weaken predicate guidance outside imperative parameter contexts. diff --git a/internal/cli/baseline_audit.go b/internal/cli/baseline_audit.go index 640b205..b09fe44 100644 --- a/internal/cli/baseline_audit.go +++ b/internal/cli/baseline_audit.go @@ -86,27 +86,24 @@ func Audit(file core.BaselineFile, findings []core.Finding, opts Options) AuditR exactCurrent := indexFindings(findings, func(f core.Finding) string { return f.Fingerprint }) contextCurrent := indexFindings(findings, func(f core.Finding) string { return f.ContextFingerprint }) contentCurrent := indexFindings(findings, func(f core.Finding) string { return f.ContentFingerprint }) - - for _, entry := range file.Entries { - audit := EntryAudit{Entry: entry} - switch { - case strings.TrimSpace(entry.Fingerprint) == "": + matches := matchBaselineEntries(file.Entries, findings) + for idx, entry := range file.Entries { + audit := EntryAudit{Entry: entry, Status: "stale"} + if strings.TrimSpace(entry.Fingerprint) == "" { audit.Status = "invalid" result.Counts.Invalid++ - case len(exactCurrent[entry.Fingerprint]) > 0: - audit.Status = "active_exact" - audit.Matches = refs(exactCurrent[entry.Fingerprint]) - result.Counts.ActiveExact++ - case entry.ContextFingerprint != "" && len(contextCurrent[entry.ContextFingerprint]) > 0: - audit.Status = "active_context" - audit.Matches = refs(contextCurrent[entry.ContextFingerprint]) - result.Counts.ActiveContext++ - case entry.ContentFingerprint != "" && len(contentCurrent[entry.ContentFingerprint]) > 0: - audit.Status = "active_content" - audit.Matches = refs(contentCurrent[entry.ContentFingerprint]) - result.Counts.ActiveContent++ - default: - audit.Status = "stale" + } else if match, ok := matches[idx]; ok { + audit.Status = "active_" + match.kind + audit.Matches = refs([]core.Finding{findings[match.finding]}) + switch match.kind { + case "exact": + result.Counts.ActiveExact++ + case "context": + result.Counts.ActiveContext++ + case "content": + result.Counts.ActiveContent++ + } + } else { result.Counts.Stale++ } result.Entries = append(result.Entries, audit) @@ -123,6 +120,66 @@ func Audit(file core.BaselineFile, findings []core.Finding, opts Options) AuditR return result } +type baselineMatch struct { + finding int + kind string +} + +func matchBaselineEntries(entries []core.BaselineEntry, findings []core.Finding) map[int]baselineMatch { + matches := map[int]baselineMatch{} + usedFindings := make([]bool, len(findings)) + tiers := []struct { + kind string + entryKey func(core.BaselineEntry) string + findingKey func(core.Finding) string + }{ + {"exact", func(e core.BaselineEntry) string { return e.Fingerprint }, func(f core.Finding) string { return f.Fingerprint }}, + {"context", func(e core.BaselineEntry) string { return e.ContextFingerprint }, func(f core.Finding) string { return f.ContextFingerprint }}, + {"content", func(e core.BaselineEntry) string { return e.ContentFingerprint }, func(f core.Finding) string { return f.ContentFingerprint }}, + } + for _, tier := range tiers { + entryGroups := map[string][]int{} + findingGroups := map[string][]int{} + for idx, entry := range entries { + if _, matched := matches[idx]; matched || strings.TrimSpace(entry.Fingerprint) == "" { + continue + } + if key := tier.entryKey(entry); key != "" { + entryGroups[key] = append(entryGroups[key], idx) + } + } + for idx, finding := range findings { + if usedFindings[idx] { + continue + } + if key := tier.findingKey(finding); key != "" { + findingGroups[key] = append(findingGroups[key], idx) + } + } + for key, entryIndexes := range entryGroups { + findingIndexes := findingGroups[key] + sort.Slice(entryIndexes, func(i, j int) bool { return entryKey(entries[entryIndexes[i]]) < entryKey(entries[entryIndexes[j]]) }) + sort.Slice(findingIndexes, func(i, j int) bool { + return findingKey(findings[findingIndexes[i]]) < findingKey(findings[findingIndexes[j]]) + }) + limit := len(entryIndexes) + if len(findingIndexes) < limit { + limit = len(findingIndexes) + } + for pair := 0; pair < limit; pair++ { + entryIdx, findingIdx := entryIndexes[pair], findingIndexes[pair] + matches[entryIdx] = baselineMatch{finding: findingIdx, kind: tier.kind} + usedFindings[findingIdx] = true + } + } + } + return matches +} + +func findingKey(f core.Finding) string { + return strings.Join([]string{f.Fingerprint, f.ContextFingerprint, f.ContentFingerprint, f.RuleID, f.Path, f.Message}, "\x00") +} + func (result AuditResult) ActiveEntries() []core.BaselineEntry { return entriesWithStatus(result.Entries, "active_") } @@ -188,7 +245,7 @@ func fingerprintDiagnostics(entries []core.BaselineEntry, indexes ...map[string] var collisions []Collision for idx, baselineIndex := range baselineIndexes { for fingerprint, count := range baselineIndex { - if count > 1 { + if idx == 0 && count > 1 { duplicates = append(duplicates, Duplicate{Kind: types[idx], Fingerprint: fingerprint, Count: count}) } currentCount := len(indexes[idx][fingerprint]) diff --git a/internal/cli/baseline_audit_test.go b/internal/cli/baseline_audit_test.go index 836ee45..4bbb48c 100644 --- a/internal/cli/baseline_audit_test.go +++ b/internal/cli/baseline_audit_test.go @@ -67,3 +67,53 @@ func TestAuditOutputIsDeterministicAndHighRiskFirst(t *testing.T) { t.Fatalf("rule distributions missing: %#v", first.ByRule[0]) } } + +func TestAuditConsumesFallbackFindingsOneToOne(t *testing.T) { + file := core.BaselineFile{Entries: []core.BaselineEntry{ + {Fingerprint: "old-b", ContextFingerprint: "shared", RuleID: "quality.same", Path: "b.go"}, + {Fingerprint: "old-a", ContextFingerprint: "shared", RuleID: "quality.same", Path: "a.go"}, + }} + findings := []core.Finding{{Fingerprint: "current", ContextFingerprint: "shared", RuleID: "quality.same", Path: "a.go", Confidence: "high"}} + + result := Audit(file, findings, Options{}) + if result.Counts.ActiveContext != 1 || result.Counts.Stale != 1 { + t.Fatalf("counts = %#v, want one active context and one stale", result.Counts) + } + if len(result.Duplicates) != 0 { + t.Fatalf("weak collision classified as duplicate: %#v", result.Duplicates) + } + if len(result.Collisions) == 0 { + t.Fatal("expected context collision diagnostic") + } +} + +func TestAuditRuleDistributionsUseOneCanonicalFindingPerEntry(t *testing.T) { + file := core.BaselineFile{Entries: []core.BaselineEntry{ + {Fingerprint: "old-a", ContextFingerprint: "shared", RuleID: "quality.same", Path: "a.go"}, + {Fingerprint: "old-b", ContextFingerprint: "shared", RuleID: "quality.same", Path: "b.cpp"}, + }} + findings := []core.Finding{ + {Fingerprint: "new-b", ContextFingerprint: "shared", RuleID: "quality.same", Path: "b.cpp", Confidence: "medium"}, + {Fingerprint: "new-a", ContextFingerprint: "shared", RuleID: "quality.same", Path: "a.go", Confidence: "high"}, + } + + result := Audit(file, findings, Options{SampleLimit: 1}) + if len(result.ByRule) != 1 { + t.Fatalf("by_rule = %#v", result.ByRule) + } + group := result.ByRule[0] + if got := namedCountTotal(group.Confidence); got != group.Count { + t.Fatalf("confidence total = %d, count = %d", got, group.Count) + } + if got := namedCountTotal(group.Languages); got != group.Count { + t.Fatalf("language total = %d, count = %d", got, group.Count) + } +} + +func namedCountTotal(counts []NamedCount) int { + total := 0 + for _, count := range counts { + total += count.Count + } + return total +} diff --git a/internal/cli/baseline_io_test.go b/internal/cli/baseline_io_test.go index 1f169f6..a23747e 100644 --- a/internal/cli/baseline_io_test.go +++ b/internal/cli/baseline_io_test.go @@ -59,7 +59,10 @@ func TestWritePrunedPreservesAllEntriesInAContextCollision(t *testing.T) { {Fingerprint: "old-b", ContextFingerprint: "shared", RuleID: "quality.duplicate"}, }} writeFixture(t, path, file) - result := Audit(file, []core.Finding{{Fingerprint: "current", ContextFingerprint: "shared", RuleID: "quality.duplicate"}}, Options{}) + result := Audit(file, []core.Finding{ + {Fingerprint: "current-a", ContextFingerprint: "shared", RuleID: "quality.duplicate"}, + {Fingerprint: "current-b", ContextFingerprint: "shared", RuleID: "quality.duplicate"}, + }, Options{}) if err := WritePruned(path, path, result, PruneOptions{}); err != nil { t.Fatal(err) } @@ -70,6 +73,13 @@ func TestWritePrunedPreservesAllEntriesInAContextCollision(t *testing.T) { if len(got.Entries) != 2 { t.Fatalf("collision entries = %#v", got.Entries) } + check := Audit(got, []core.Finding{ + {Fingerprint: "current-a", ContextFingerprint: "shared", RuleID: "quality.duplicate"}, + {Fingerprint: "current-b", ContextFingerprint: "shared", RuleID: "quality.duplicate"}, + }, Options{}) + if check.Counts.Stale != 0 || check.Counts.Invalid != 0 || len(check.Duplicates) != 0 { + t.Fatalf("freshly pruned collision baseline does not pass check: counts=%#v duplicates=%#v", check.Counts, check.Duplicates) + } } func writeFixture(t *testing.T, path string, file core.BaselineFile) { diff --git a/internal/codeguard/checks/quality/quality_ai_target_go.go b/internal/codeguard/checks/quality/quality_ai_target_go.go index 0410289..0c6a287 100644 --- a/internal/codeguard/checks/quality/quality_ai_target_go.go +++ b/internal/codeguard/checks/quality/quality_ai_target_go.go @@ -4,7 +4,6 @@ import ( "fmt" "go/ast" "go/token" - "os" "path/filepath" "strings" @@ -12,23 +11,18 @@ import ( "github.com/devr-tools/codeguard/internal/codeguard/core" ) -type goModuleMetadata struct { - modulePath string - required []string -} - func goAITargetFindings(env support.Context, target core.TargetConfig) []core.Finding { files := aiTargetSourceFiles(env, target, ".go") if len(files) == 0 { return nil } - metadata := readGoModuleMetadata(target.Path) + resolver := newGoModuleResolver(target.Path) profile := goRepoStyleProfile(env, target, files) packageFiles := map[string][]goParsedFile{} findings := make([]core.Finding, 0) for _, rel := range files { fileFindings, parsedFile := goFileAIQualityFindings(env, target, rel, goFileScanInput{ - metadata: metadata, + metadata: resolver.metadataForFile(rel), dominant: profile.testFramework, errorStyle: profile.errorStyle, naming: profile.naming, @@ -124,35 +118,6 @@ func goFileAIQualityFindings(env support.Context, target core.TargetConfig, rel return findings, parsedFile } -func readGoModuleMetadata(root string) goModuleMetadata { - data, err := os.ReadFile(filepath.Join(root, "go.mod")) //nolint:gosec // fixed filename under the scan-target root - if err != nil { - return goModuleMetadata{} - } - metadata := goModuleMetadata{} - for _, line := range strings.Split(string(data), "\n") { - fields := strings.Fields(line) - if len(fields) < 2 { - continue - } - switch fields[0] { - case "module": - metadata.modulePath = fields[1] - case "go", "replace", "exclude", "retract": - continue - case "require": - if len(fields) >= 3 { - metadata.required = append(metadata.required, fields[1]) - } - default: - if strings.HasPrefix(fields[1], "v") { - metadata.required = append(metadata.required, fields[0]) - } - } - } - return metadata -} - func goHallucinatedImportFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, metadata goModuleMetadata) []core.Finding { findings := make([]core.Finding, 0) for _, imp := range parsed.Imports { @@ -168,21 +133,7 @@ func goHallucinatedImportFindings(env support.Context, file string, fset *token. } func goImportResolvable(importPath string, metadata goModuleMetadata) bool { - if importPath == "" { - return true - } - if !strings.Contains(firstSegment(importPath), ".") { - return true - } - if metadata.modulePath != "" && (importPath == metadata.modulePath || strings.HasPrefix(importPath, metadata.modulePath+"/")) { - return true - } - for _, required := range metadata.required { - if importPath == required || strings.HasPrefix(importPath, required+"/") { - return true - } - } - return false + return metadata.resolvesImport(importPath) } func goDeadCodeFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File) []core.Finding { diff --git a/internal/codeguard/checks/quality/quality_go_modules.go b/internal/codeguard/checks/quality/quality_go_modules.go new file mode 100644 index 0000000..615b0fe --- /dev/null +++ b/internal/codeguard/checks/quality/quality_go_modules.go @@ -0,0 +1,139 @@ +package quality + +import ( + "bufio" + "os" + "path/filepath" + "strings" +) + +type goModuleMetadata struct { + modulePath string + resolvable []string +} + +type goModuleResolver struct { + root string + workspaceModules []string + workspaceReplace []string + cache map[string]goModuleMetadata +} + +func newGoModuleResolver(root string) goModuleResolver { + resolver := goModuleResolver{root: filepath.Clean(root), cache: map[string]goModuleMetadata{}} + workPath := filepath.Join(resolver.root, "go.work") + uses, replaces := parseGoManifest(workPath, true) + resolver.workspaceReplace = replaces + for _, use := range uses { + moduleDir := use + if !filepath.IsAbs(moduleDir) { + moduleDir = filepath.Join(resolver.root, moduleDir) + } + modulePath, _ := parseGoManifest(filepath.Join(moduleDir, "go.mod"), false) + resolver.workspaceModules = append(resolver.workspaceModules, modulePath...) + } + return resolver +} + +func (r *goModuleResolver) metadataForFile(rel string) goModuleMetadata { + dir := filepath.Dir(filepath.Join(r.root, filepath.FromSlash(rel))) + for { + if metadata, ok := r.cache[dir]; ok { + return metadata + } + modPath := filepath.Join(dir, "go.mod") + if _, err := os.Stat(modPath); err == nil { + modules, dependencies := parseGoManifest(modPath, false) + metadata := goModuleMetadata{resolvable: append([]string{}, r.workspaceModules...)} + metadata.resolvable = append(metadata.resolvable, r.workspaceReplace...) + metadata.resolvable = append(metadata.resolvable, dependencies...) + if len(modules) > 0 { + metadata.modulePath = modules[0] + } + r.cache[dir] = metadata + return metadata + } + parent := filepath.Dir(dir) + if parent == dir || !pathWithin(parent, r.root) { + return goModuleMetadata{resolvable: append(append([]string{}, r.workspaceModules...), r.workspaceReplace...)} + } + dir = parent + } +} + +func pathWithin(path, root string) bool { + rel, err := filepath.Rel(root, path) + return err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) +} + +// parseGoManifest returns module/use paths first and require/replace module +// paths second. It intentionally parses only resolution directives and does +// not execute the Go toolchain or consult a module cache. +func parseGoManifest(path string, workspace bool) ([]string, []string) { + file, err := os.Open(path) //nolint:gosec // path is a fixed manifest under the configured scan root + if err != nil { + return nil, nil + } + defer func() { _ = file.Close() }() + + var primary, dependencies []string + block := "" + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := strings.TrimSpace(strings.SplitN(scanner.Text(), "//", 2)[0]) + if line == "" { + continue + } + if line == ")" { + block = "" + continue + } + fields := strings.Fields(line) + if len(fields) == 2 && fields[1] == "(" { + block = fields[0] + continue + } + directive := block + if directive == "" { + directive = fields[0] + fields = fields[1:] + } + if len(fields) == 0 { + continue + } + switch directive { + case "module": + primary = append(primary, fields[0]) + case "use": + if workspace { + primary = append(primary, fields[0]) + } + case "require": + if !workspace { + dependencies = append(dependencies, fields[0]) + } + case "replace": + dependencies = append(dependencies, fields[0]) + } + } + return primary, dependencies +} + +func (m goModuleMetadata) resolvesImport(importPath string) bool { + if importPath == "" || !strings.Contains(firstSegment(importPath), ".") { + return true + } + if modulePrefixMatch(importPath, m.modulePath) { + return true + } + for _, module := range m.resolvable { + if modulePrefixMatch(importPath, module) { + return true + } + } + return false +} + +func modulePrefixMatch(importPath, modulePath string) bool { + return modulePath != "" && (importPath == modulePath || strings.HasPrefix(importPath, modulePath+"/")) +} diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index f689028..d786237 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -108,7 +108,7 @@ func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, continue } findings = append(findings, goGenericDeclFindings(env, file, fset, gen)...) - findings = append(findings, goMutableGlobalFindings(env, file, fset, gen)...) + findings = append(findings, goMutableGlobalFindings(env, file, fset, parsed, gen)...) findings = append(findings, goDuplicatedKnowledgeFindings(env, file, fset, gen)...) } findings = append(findings, redundantCommentFindings(env, file, string(data))...) @@ -309,7 +309,12 @@ func goGenericDeclFindings(env support.Context, file string, fset *token.FileSet return findings } -func goMutableGlobalFindings(env support.Context, file string, fset *token.FileSet, decl *ast.GenDecl) []core.Finding { +type globalDeclarationClassification struct { + immutableByConvention bool + technicallyReassignable bool +} + +func goMutableGlobalFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, decl *ast.GenDecl) []core.Finding { if decl.Tok != token.VAR || isQualityFixturePath(file) { return nil } @@ -319,17 +324,65 @@ func goMutableGlobalFindings(env support.Context, file string, fset *token.FileS if !ok { continue } - for _, name := range value.Names { + for idx, name := range value.Names { if strings.HasPrefix(strings.ToLower(name.Name), "err") { continue } + classification := classifyGoGlobalDeclaration(value, idx) + if classification.immutableByConvention && classification.technicallyReassignable && !goGlobalIsReassigned(parsed, name.Name) { + continue + } + message := fmt.Sprintf("mutable package-level variable %q makes behavior harder to isolate and test", name.Name) + if classification.immutableByConvention { + message = fmt.Sprintf("package-level variable %q is reassigned after immutable construction", name.Name) + } findings = append(findings, precisionWarnFinding(env, qualityMutableGlobalStateRuleID, file, fset.Position(name.Pos()).Line, - fmt.Sprintf("mutable package-level variable %q makes behavior harder to isolate and test", name.Name), core.ConfidenceHigh)) + message, core.ConfidenceHigh)) } } return findings } +func classifyGoGlobalDeclaration(spec *ast.ValueSpec, index int) globalDeclarationClassification { + classification := globalDeclarationClassification{technicallyReassignable: true} + if len(spec.Values) == 0 { + return classification + } + exprIndex := index + if exprIndex >= len(spec.Values) { + exprIndex = len(spec.Values) - 1 + } + call, ok := spec.Values[exprIndex].(*ast.CallExpr) + if !ok { + return classification + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "MustCompile" { + return classification + } + pkg, ok := selector.X.(*ast.Ident) + classification.immutableByConvention = ok && pkg.Name == "regexp" + return classification +} + +func goGlobalIsReassigned(parsed *ast.File, name string) bool { + reassigned := false + ast.Inspect(parsed, func(node ast.Node) bool { + assignment, ok := node.(*ast.AssignStmt) + if !ok { + return true + } + for _, lhs := range assignment.Lhs { + if ident, ok := lhs.(*ast.Ident); ok && ident.Name == name { + reassigned = true + return false + } + } + return true + }) + return reassigned +} + func goDuplicatedKnowledgeFindings(env support.Context, file string, fset *token.FileSet, decl *ast.GenDecl) []core.Finding { if decl.Tok != token.CONST || isQualityFixturePath(file) { return nil diff --git a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go index 9c7968c..d191e29 100644 --- a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go +++ b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go @@ -86,20 +86,23 @@ func precisionNamingFindings(env support.Context, file string, fn precisionFunct typ string expr string line int + role string }, 0, 1+len(fn.Params)+len(fn.Assignments)) allNames = append(allNames, struct { name string typ string expr string line int - }{name: fn.Name, line: fn.StartLine}) + role string + }{name: fn.Name, line: fn.StartLine, role: "function"}) for _, param := range fn.Params { allNames = append(allNames, struct { name string typ string expr string line int - }{name: param.Name, typ: param.Type, line: fn.StartLine}) + role string + }{name: param.Name, typ: param.Type, line: fn.StartLine, role: "parameter"}) } for _, assignment := range fn.Assignments { allNames = append(allNames, struct { @@ -107,7 +110,8 @@ func precisionNamingFindings(env support.Context, file string, fn precisionFunct typ string expr string line int - }{name: assignment.Name, expr: assignment.Expr, line: assignment.Line}) + role string + }{name: assignment.Name, expr: assignment.Expr, line: assignment.Line, role: "local"}) } for _, item := range allNames { if item.name == "" { @@ -116,14 +120,15 @@ func precisionNamingFindings(env support.Context, file string, fn precisionFunct if item.name == fn.Name && isReactComponentOrHookBoundary(file, fn) { continue } - if isSeedOrScriptSourcePath(file) && isBooleanNameCandidate(item.name, item.typ, fn) { + if isSeedOrScriptSourcePath(file) && isBooleanNameCandidate(item.typ) { continue } - if isBooleanNameCandidate(item.name, item.typ, fn) && isUIHelperOrMappingContext(file, fn) { + if isBooleanNameCandidate(item.typ) && isUIHelperOrMappingContext(file, fn) { continue } - if isBooleanNameCandidate(item.name, item.typ, fn) && + if isBooleanNameCandidate(item.typ) && !isInferredUIBooleanAssignment(file, fn, item.typ, item.expr, item.line) && + (item.role != "parameter" || !isImperativeBooleanParameterName(item.name)) && !isPredicateName(item.name) && !isAllowedBooleanUIName(file, fn, item.name) { findings = append(findings, precisionWarnFinding(env, namingBooleanNotPredicateRuleID, file, item.line, @@ -799,16 +804,44 @@ func orchestrationDomainMix(file string, fn precisionFunction) bool { return hasInfra && hasDomainDecision } -func isBooleanNameCandidate(name string, typ string, fn precisionFunction) bool { - if name == fn.Name { +func isBooleanNameCandidate(typ string) bool { + return isBooleanType(typ) +} + +func isImperativeBooleanParameterName(name string) bool { + words := identifierWords(name) + if len(words) < 2 { return false } - if isBooleanType(typ) { - return true + for _, prefix := range []string{"include", "require", "allow", "skip", "enable", "disable", "force", "use"} { + if words[0] == prefix { + return true + } } return false } +func identifierWords(name string) []string { + name = strings.Trim(name, "_$") + var words []string + start := 0 + for idx, r := range name { + if idx > 0 && (r == '_' || (r >= 'A' && r <= 'Z')) { + if word := strings.ToLower(strings.Trim(name[start:idx], "_")); word != "" { + words = append(words, word) + } + start = idx + if r == '_' { + start++ + } + } + } + if word := strings.ToLower(strings.Trim(name[start:], "_")); word != "" { + words = append(words, word) + } + return words +} + func isInferredUIBooleanAssignment(file string, fn precisionFunction, typ string, expr string, line int) bool { if expr == "" || isBooleanType(typ) { return false diff --git a/internal/version/version.go b/internal/version/version.go index 480a98a..83271aa 100644 --- a/internal/version/version.go +++ b/internal/version/version.go @@ -6,6 +6,7 @@ import ( ) const defaultNumber = "0.1.0" +const developmentNumber = "devel" // Number is the codeguard version. It must be a var (not a const) so the // release build can override it via the linker: GoReleaser injects the git tag @@ -13,7 +14,7 @@ const defaultNumber = "0.1.0" // (see .goreleaser.yaml). The linker's -X flag only sets string vars, so a // const would silently leave released binaries reporting this default. Version // precedence is linker flags, embedded build info, then the compiled default. -var Number = defaultNumber +var Number = developmentNumber func init() { info, ok := debug.ReadBuildInfo() @@ -25,7 +26,7 @@ func init() { // Resolve preserves an injected release version before consulting build info. func Resolve(current string, info *debug.BuildInfo) string { - if current != defaultNumber { + if current != defaultNumber && current != developmentNumber { return current } if moduleVersion := ModuleVersionFromBuildInfo(info); moduleVersion != "" { @@ -34,7 +35,7 @@ func Resolve(current string, info *debug.BuildInfo) string { if developmentVersion := DevelopmentVersionFromBuildInfo(info); developmentVersion != "" { return developmentVersion } - return current + return developmentNumber } // ModuleVersionFromBuildInfo returns a real module version embedded by Go. @@ -71,7 +72,7 @@ func DevelopmentVersionFromBuildInfo(info *debug.BuildInfo) string { if len(revision) > 8 { revision = revision[:8] } - resolved := defaultNumber + "-dev+" + revision + resolved := developmentNumber + "+" + revision if dirty { resolved += ".dirty" } diff --git a/internal/version/version_test.go b/internal/version/version_test.go new file mode 100644 index 0000000..1de8662 --- /dev/null +++ b/internal/version/version_test.go @@ -0,0 +1,27 @@ +package version + +import ( + "runtime/debug" + "testing" +) + +func TestResolveVersionPrecedenceAndFallback(t *testing.T) { + tests := []struct { + name string + current string + info *debug.BuildInfo + want string + }{ + {name: "linker injection wins", current: "v9.8.7", info: &debug.BuildInfo{Main: debug.Module{Version: "v1.2.3"}}, want: "v9.8.7"}, + {name: "go install module version", current: "devel", info: &debug.BuildInfo{Main: debug.Module{Version: "v9.8.7"}}, want: "v9.8.7"}, + {name: "missing build metadata", current: "devel", info: nil, want: "devel"}, + {name: "devel build metadata", current: "devel", info: &debug.BuildInfo{Main: debug.Module{Version: "(devel)"}}, want: "devel"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := Resolve(tc.current, tc.info); got != tc.want { + t.Fatalf("Resolve() = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/tests/checks/function_precision_test.go b/tests/checks/function_precision_test.go index 7a61bc0..08ba552 100644 --- a/tests/checks/function_precision_test.go +++ b/tests/checks/function_precision_test.go @@ -440,6 +440,42 @@ func TestBooleanPredicateNamingAcceptsConventionalPredicateVocabulary(t *testing assertFindingRuleAbsent(t, report, "Code Quality", "naming.boolean-not-predicate") } +func TestImperativeBooleanNamesAreAllowedOnlyForParameters(t *testing.T) { + for _, tc := range []struct { + name string + language string + file string + content string + }{ + {name: "go", language: "go", file: "options.go", content: strings.Join([]string{ + "package sample", + "func Load(includeInactive, requireCompleteMetadata, allowPartial, skipCache, enableTrace, disableRetry, forceRefresh, useIndex bool) {}", + }, "\n")}, + {name: "cpp", language: "cpp", file: "options.cpp", content: "void load(bool includeInactive, bool requireCompleteMetadata, bool allowPartial, bool skipCache, bool forceRefresh, bool useIndex) {}\n"}, + } { + t.Run(tc.name+" parameters", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, tc.file), tc.content) + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, tc.language)) + assertFindingRuleAbsent(t, report, "Code Quality", "naming.boolean-not-predicate") + }) + } + + t.Run("go non-imperative parameter", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "state.go"), "package sample\nfunc Load(metadata bool) {}\n") + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRulePresent(t, report, "Code Quality", "naming.boolean-not-predicate") + }) + + t.Run("cpp non-imperative parameter", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "state.cpp"), "void load(bool metadata) {}\n") + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "cpp")) + assertFindingRulePresent(t, report, "Code Quality", "naming.boolean-not-predicate") + }) +} + func TestGoLocalRowAssignmentsDoNotLookLikeMutableGlobals(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "collections.go"), strings.Join([]string{ @@ -463,6 +499,42 @@ func TestGoLocalRowAssignmentsDoNotLookLikeMutableGlobals(t *testing.T) { assertFindingRuleAbsent(t, report, "Code Quality", "quality.mutable-global-state") } +func TestGoMutableGlobalExemptsImmutableConstructorUntilReassigned(t *testing.T) { + t.Run("immutable compiled regex", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "patterns.go"), strings.Join([]string{ + "package sample", + "import \"regexp\"", + "var emailPattern = regexp.MustCompile(`@`)", + "func Matches(value string) bool { return emailPattern.MatchString(value) }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRuleAbsent(t, report, "Code Quality", "quality.mutable-global-state") + }) + + t.Run("reassigned compiled regex", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "patterns.go"), strings.Join([]string{ + "package sample", + "import \"regexp\"", + "var emailPattern = regexp.MustCompile(`@`)", + "func Configure(pattern string) { emailPattern = regexp.MustCompile(pattern) }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRulePresent(t, report, "Code Quality", "quality.mutable-global-state") + }) + + t.Run("mutable collection", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "cache.go"), "package sample\nvar cache = map[string]string{}\n") + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRulePresent(t, report, "Code Quality", "quality.mutable-global-state") + }) +} + func TestPostgresRepositoryAllowsPgxQueriesAndRowsAffectedResults(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "platform", "storage", "postgres", "collections_repository.go"), strings.Join([]string{ diff --git a/tests/checks/quality_ai_additional_test.go b/tests/checks/quality_ai_additional_test.go index 15a2cfa..a7687e6 100644 --- a/tests/checks/quality_ai_additional_test.go +++ b/tests/checks/quality_ai_additional_test.go @@ -4,6 +4,7 @@ import ( "context" "os" "path/filepath" + "strings" "testing" "github.com/devr-tools/codeguard/pkg/codeguard" @@ -28,6 +29,66 @@ func run() {} assertFindingConfidence(t, report, "Code Quality", "quality.ai.hallucinated-import", "high") } +func TestGoWorkspaceImportsResolveAgainstOwningModule(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "go.work"), `go 1.23 + +use ( + ./apps/api + ./libs/shared +) + +replace example.com/work-replaced => ./third_party/work-replaced +`) + writeFile(t, filepath.Join(dir, "apps", "api", "go.mod"), `module example.com/api + +go 1.23 + +require ( + github.com/direct/dependency v1.2.3 + github.com/indirect/dependency v1.2.3 // indirect +) + +replace github.com/direct/dependency => ../../third_party/direct +`) + writeFile(t, filepath.Join(dir, "libs", "shared", "go.mod"), "module example.com/shared\n\ngo 1.23\n") + writeFile(t, filepath.Join(dir, "apps", "api", "nested", "go.mod"), "module example.com/nested\n\ngo 1.23\n\nrequire example.com/nested-dep v1.0.0\n") + writeFile(t, filepath.Join(dir, "apps", "api", "service.go"), `package api + +import ( + "context" + "example.com/shared/client" + "example.com/work-replaced/client" + "github.com/direct/dependency/client" + "github.com/indirect/dependency/client" + "github.com/truly/missing/client" +) + +var _ = context.Background +`) + writeFile(t, filepath.Join(dir, "apps", "api", "nested", "service.go"), `package nested + +import "example.com/nested-dep/client" +`) + + report, err := codeguard.Run(context.Background(), qualityAITestConfig(dir, "quality-ai-go-workspace")) + if err != nil { + t.Fatalf("run: %v", err) + } + + var imports []string + for _, section := range report.Sections { + for _, finding := range section.Findings { + if finding.RuleID == "quality.ai.hallucinated-import" { + imports = append(imports, finding.Message) + } + } + } + if len(imports) != 1 || !strings.Contains(imports[0], "github.com/truly/missing/client") { + t.Fatalf("hallucinated import findings = %v, want only truly missing import", imports) + } +} + func TestQualityCheckWarnsForHallucinatedTypeScriptImport(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "package.json"), `{"name":"fixture","dependencies":{"react":"18.0.0"}}`) diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go index ee007cd..c18ac64 100644 --- a/tests/cli/cli_test.go +++ b/tests/cli/cli_test.go @@ -14,7 +14,7 @@ import ( func TestRunVersion(t *testing.T) { originalVersion := version.Number - version.Number = version.Resolve("0.1.0", &debug.BuildInfo{Main: debug.Module{Version: "v1.5.1"}}) + version.Number = version.Resolve("devel", &debug.BuildInfo{Main: debug.Module{Version: "v1.5.1"}}) t.Cleanup(func() { version.Number = originalVersion }) var stdout bytes.Buffer diff --git a/tests/version/version_test.go b/tests/version/version_test.go index 5533bf9..c1a2f8a 100644 --- a/tests/version/version_test.go +++ b/tests/version/version_test.go @@ -1,7 +1,10 @@ package version_test import ( + "os/exec" + "path/filepath" "runtime/debug" + "strings" "testing" "github.com/devr-tools/codeguard/internal/version" @@ -27,6 +30,23 @@ func TestModuleVersionFromBuildInfo(t *testing.T) { } } +func TestBuiltCommandReportsInjectedVersion(t *testing.T) { + repoRoot := filepath.Join("..", "..") + binary := filepath.Join(t.TempDir(), "codeguard") + build := exec.Command("go", "build", "-ldflags", "-X github.com/devr-tools/codeguard/internal/version.Number=v9.8.7", "-o", binary, "./cmd/codeguard") + build.Dir = repoRoot + if output, err := build.CombinedOutput(); err != nil { + t.Fatalf("build command: %v\n%s", err, output) + } + output, err := exec.Command(binary, "version").CombinedOutput() + if err != nil { + t.Fatalf("run version: %v\n%s", err, output) + } + if got := strings.TrimSpace(string(output)); got != "v9.8.7" { + t.Fatalf("version output = %q, want v9.8.7", got) + } +} + func TestDevelopmentVersionFromBuildInfo(t *testing.T) { info := &debug.BuildInfo{ Main: debug.Module{Version: "(devel)"}, @@ -35,7 +55,7 @@ func TestDevelopmentVersionFromBuildInfo(t *testing.T) { {Key: "vcs.modified", Value: "true"}, }, } - if got, want := version.DevelopmentVersionFromBuildInfo(info), "0.1.0-dev+abcdef12.dirty"; got != want { + if got, want := version.DevelopmentVersionFromBuildInfo(info), "devel+abcdef12.dirty"; got != want { t.Fatalf("DevelopmentVersionFromBuildInfo() = %q, want %q", got, want) } }