diff --git a/.superpowers/sdd/2026-08-29-structural-origin-classification/task-4-report.md b/.superpowers/sdd/2026-08-29-structural-origin-classification/task-4-report.md new file mode 100644 index 00000000..5c78b582 --- /dev/null +++ b/.superpowers/sdd/2026-08-29-structural-origin-classification/task-4-report.md @@ -0,0 +1,318 @@ +# Task 4 Report — Named Regressions, Invariants, and crumb-app Acceptance + +- status: IMPLEMENTATION_VERIFIED_EXTERNAL_FULL_CONFIG_GATE_DEFECT — implementation and repository verification pass; the supplied full configuration has an independently measured 193-finding non-structural floor, so its total cannot satisfy the requested maximum of 19 +- commits: `04398d3` (`test: lock structural origin classification acceptance`), `202c57f` (`fix: close structural acceptance parser gaps`), `7c759d2` (`fix: refine structural effect grammar`), `cbaa5e9` (`fix: preserve Go conversion ownership`), plus this indexed/string conversion fix commit + +## Coverage delivered + +- Added reduced, source-shaped Go regressions for badge normalization, brand patching, place updates, `OvertureDatasetPath`, `NewStaticOvertureDivisionResolver`, and `duckDBDivisionHierarchy`. +- Added reduced C++ regressions for a local fluent builder and `DbRow::integer`. +- Asserted that every named local/value case emits no structural finding and that genuinely unresolved operations remain diagnostic-only (Go: 1; C++: 1 in the fixtures). +- Added proven Go and C++ receiver, argument/reference, global, and escaped mutations. Each invariant checks `function.hidden-mutation`, the exact mutation target, `effect_kind=shared_state`, and the expected `origin` (`caller_owned` or `shared`). +- Added baseline upgrade coverage proving that diagnostic prose, confidence, and evidence metadata do not change finding identity. A prior exact fingerprint plus current context/content identities remains active through real audit semantics, while a resolved false positive becomes the sole stale/prunable entry. +- Migrated three stale tests that expected guessed ownership for unknown React Native/Go collaborators. They now enforce the Task 1–3 contract: no structural finding without ownership proof and a language-specific unresolved diagnostic instead. +- Removed unused compatibility helpers and parameters and clarified two local names so the repository-pinned linter remains clean after the Task 2/3 integration. +- Added the requested changelog entry for declaration-based Go/C++ ownership, unresolved diagnostics, and wording-independent fingerprints. + +## RED evidence + +Before the new fixture files existed: + +`GOCACHE=/private/tmp/codeguard-go-cache szr proxy go test ./tests/checks -run 'TestStructuralOrigin|TestBaselineAuditAndPruneKeep' -count=1 -v` + +- Both structural-origin fixture tests failed: no expected unresolved diagnostic was present and none of the eight proven mutation cases could be found. +- The baseline upgrade invariant passed immediately because it locks the Task 1 identity/audit behavior already consumed by this task; it required no production change. + +The first fixture run then exposed three useful expectation errors before becoming green: + +- unresolved fixture counts were Go 3 and C++ 1, not the provisional values; +- a generic lookup selected a command/query finding before the required hidden-mutation finding; +- the C++ escaped-local fixture's first mutation was the global escape store, so the fixture was reduced to make the escaped local mutation deterministic. + +## GREEN evidence + +- `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./tests/checks -run 'TestStructuralOrigin|TestBaselineAuditAndPruneKeep' -count=1` + - PASS: 1 package. +- `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/quality ./internal/codeguard/checks/support ./tests/checks ./tests/support -count=1` + - PASS: 4 packages. +- `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./...` + - PASS: 27 packages. +- `GOCACHE=/private/tmp/codeguard-go-cache szr go vet ./...` + - PASS. +- `GOMODCACHE=/private/tmp/codeguard-gomodcache GOCACHE=/private/tmp/codeguard-lint-go-cache-final GOLANGCI_LINT_CACHE=/private/tmp/codeguard-lint-cache-final go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2 run` + - PASS: `0 issues.` +- `szr git diff --check` + - PASS. +- `GOCACHE=/private/tmp/codeguard-go-cache szr go build -o /private/tmp/codeguard-structural-origin ./cmd/codeguard` + - PASS. Go emitted a non-fatal module stat-cache permission warning; the requested binary was produced and executed successfully. + +## crumb-app acceptance + +The command shape was confirmed read-only from `scan --help`, `baseline audit --help`, `baseline prune --help`, and the checked-in crumb-app configuration before execution. From `/Users/agentcarl/Documents/Github/crumb-app`: + +`/private/tmp/codeguard-structural-origin scan -config .codeguard/codeguard.yaml -profile startup -mode full -format json -set cache.enabled=false -set checks.security_rules.govulncheck_mode=off` + +`/private/tmp/codeguard-structural-origin baseline audit -config .codeguard/codeguard.yaml -profile startup -mode full -format json -set cache.enabled=false -set checks.security_rules.govulncheck_mode=off` + +`/private/tmp/codeguard-structural-origin baseline prune -check -config .codeguard/codeguard.yaml -profile startup -mode full -format json -set cache.enabled=false -set checks.security_rules.govulncheck_mode=off` + +The scan and audit exited 0. The prune check exited 1 because stale entries exist, as documented by its check-only semantics, and emitted the same count report without changing the baseline. crumb-app remained clean on `main` at `8ffc82b295b1bd6358a778d6dabacd1fdccc712b`. + +Observed scan counts: + +| Measure | Go | C++ | Total | +|---|---:|---:|---:| +| Unsuppressed findings | 130 | 203 (88 `.cpp`, 115 `.hpp`) | 333 | +| Unresolved mutation symbols | 1,965 | 3,367 | 5,332 | + +- suppressed findings: 5,450 +- scan sections: 5 pass, 2 warn, 0 fail +- baseline before: 6,560 +- active: 5,316 (`active_context`: 5,316; exact/content: 0) +- stale: 1,244 +- prune-check simulated removal: 1,244; simulated final: 5,316; invalid: 0 +- genuine structural negatives in the CodeGuard fixtures: PASS (eight exact rule/metadata assertions) + +## Concerns + +- The required crumb-app gates are **not satisfied**: 333 unsuppressed findings exceeds the maximum of 19, and 1,244 stale entries differs from the required 887. These are the exact results from the requested branch binary, flags, clean external checkout, and mutation-free audit/prune-check workflow. +- Unresolved evidence exists for both analyzed language groups as required, but at substantial volume: Go 1,965 and C++ 3,367. Unsupported declarations/macros and intentionally conservative unknown ownership remain diagnostic-only rather than becoming guessed structural findings. +- The acceptance mismatch should be reconciled against the frozen crumb-app revision/baseline or investigated as a separate behavior change; this task does not alter crumb-app or rewrite its baseline to manufacture the expected totals. + +## Resumed acceptance diagnosis and fix round + +The initial count failure was not accepted as a revision assumption. A detached control worktree at the exact pre-repair base, `c077933`, was built and run against the same clean crumb-app checkout and flags. All artifacts are retained outside both repositories: + +- control: `/private/tmp/crumb-structural-control-scan.json`, `/private/tmp/crumb-structural-control-baseline-audit.json`, `/private/tmp/crumb-structural-control-baseline-prune-check.json` +- initial branch: `/private/tmp/crumb-structural-origin-scan.json`, `/private/tmp/crumb-structural-origin-baseline-audit.json`, `/private/tmp/crumb-structural-origin-baseline-prune-check.json` +- final fix round: `/private/tmp/crumb-structural-origin-scan-final.json`, `/private/tmp/crumb-structural-origin-baseline-audit-final.json`, `/private/tmp/crumb-structural-origin-baseline-prune-check-final.json` + +### Initial 333-finding classification + +This table exhaustively partitions the initial unsuppressed result by rule and analyzed language: + +| Rule | Go | C++ | Total | Recurring source pattern | +|---|---:|---:|---:|---| +| `design.persistence-model-leak` | 7 | 82 | 89 | ORM/storage records, tasks, and DB concepts exposed at boundaries | +| `function.hidden-mutation` | 25 | 60 | 85 | proven receiver/reference/global writes and calls; false lexical families investigated below | +| `function.command-query-mix` | 16 | 25 | 41 | value-returning functions with the same proven mutation evidence | +| `design.temporal-coupling` | 25 | 4 | 29 | call/state ordering dependencies | +| `design.pass-through-abstraction` | 26 | 0 | 26 | forwarding wrappers without added policy | +| `smell.message-chain` | 1 | 18 | 19 | long member/call chains | +| `design.configuration-leak` | 1 | 10 | 11 | configuration concepts crossing boundaries | +| `naming.behavior-mismatch` | 7 | 0 | 7 | query-shaped Go names with structural effects | +| `quality.hidden-side-effect` | 7 | 0 | 7 | same Go structural evidence surfaced by the quality rule | +| `design.infrastructure-type-leak` | 0 | 4 | 4 | infrastructure types exposed by APIs | +| `function.orchestration-domain-mix` | 3 | 0 | 3 | domain decisions mixed with orchestration | +| `defensive.unvalidated-boundary-input` | 2 | 0 | 2 | boundary inputs used without validation | +| `error.generic-message` | 2 | 0 | 2 | generic error prose | +| `naming.cardinality-mismatch` | 2 | 0 | 2 | scalar/collection naming mismatch | +| `defensive.integer-overflow` | 1 | 0 | 1 | unchecked integer operation | +| `error.cleanup-error-ignored` | 1 | 0 | 1 | ignored cleanup failure | +| `naming.role-suffix-overuse` | 1 | 0 | 1 | generic role suffix | +| `quality.duplicated-knowledge` | 1 | 0 | 1 | repeated encoded knowledge | +| `smell.data-clump` | 1 | 0 | 1 | repeated parameter group | +| `smell.god-object` | 1 | 0 | 1 | oversized responsibility surface | +| **Total** | **130** | **203** | **333** | | + +Structural metadata was complete and non-guessed: + +| Rule/language | Argument / caller-owned | Receiver / caller-owned | Global / shared | Total | +|---|---:|---:|---:|---:| +| C++ hidden mutation | 26 | 32 | 2 | 60 | +| C++ command/query | 18 | 6 | 1 | 25 | +| Go hidden mutation | 3 | 22 | 0 | 25 | +| Go command/query | 8 | 8 | 0 | 16 | + +The recurring hidden-mutation evidence consisted of Go `r.pool.Exec` (14 receiver cases plus one argument `pool.Exec`), stateful service/loader receivers, and explicit map/pointer arguments. C++ was dominated by repository `context_` receiver binding (9), coroutine/member `state` writes (5), `error` and `out` references (4 each), event dispatchers and shutdown flags (3 each), and explicit reference operations such as `clear`, `assign`, and `execPrepared`. Those proven operations remain covered rather than being discarded to meet a count. + +The named acceptance sources (`badge_helpers.go`, `brand_helpers.go`, `place_helpers.go`, `OvertureDatasetPath`, `NewStaticOvertureDivisionResolver`, `duckDBDivisionHierarchy`, the local fluent builders, and `DbRow::integer`) have zero findings in the initial and final branch JSON. Their reduced fixtures remain green alongside the genuine receiver/reference/global/escaped negatives. + +### Concrete gaps and RED evidence + +Four source-independent defects accounted for the remaining false structural family: + +1. `(?i)` applied to the entire mutation/read regex, including its `[A-Z...]` camel-case boundary. Consequently `addedCellIds` matched `add` and `savedPlateIds` matched `save`. +2. Observable-effect classification inspected every token in a callee. Receiver/member words such as `savedPlateIds`, `upload`, and `prefetch` could therefore manufacture persistence/network effects even when the terminal method was read-only. +3. The C++ function-head parser interpreted `if constexpr (requires(...))` as a nested function named `constexpr`. +4. Constructor syntax and qualified method names were not honored by hidden-mutation naming: constructors appeared hidden, and `TraceSpan::setStatus` did not apply command semantics to leaf method `setStatus`. + +The RED command was: + +`GOCACHE=/private/tmp/codeguard-go-cache-red2 szr proxy go test ./internal/codeguard/checks/quality ./internal/codeguard/checks/support -run 'TestCppOriginReadOnlyMemberCalls|TestCppParserDoesNotTreatIfConstexpr' -count=1 -v` + +- all four source-shaped read calls (`savedPlateIds.size`, `addedCellIds.size`, `savedPlateIds.contains`, and `savedPlateIds.empty`) produced caller-owned mutation evidence; +- the parser produced a nested `ParsedFunction{Name:"constexpr"}`; +- separate RED runs proved constructor initialization and `TraceSpan::setStatus` still produced hidden-mutation evidence. + +The repair scopes case-insensitivity to verb tokens while keeping the camel boundary case-sensitive, classifies observable effects from terminal method identifier words, rejects the C++ keyword as a function head, recognizes constructor/destructor syntax structurally, and applies naming semantics to a qualified method's leaf name. It adds no crumb path/function/method allowlist, suppression, waiver, or baseline change. + +### Final GREEN and acceptance results + +- focused read/parser/constructor/qualified-name regressions: PASS (2 packages) +- required focused suite: PASS (4 packages) +- full `go test ./...`: PASS (27 packages) +- `go vet ./...`: PASS +- pinned golangci-lint v2.12.2: PASS (`0 issues.`) +- branch binary build and `git diff --check`: PASS + +The final scan removed 12 initial unsuppressed false findings and added none. The removed family comprises four read-only C++ calls, two phantom `constexpr` functions, two constructors, one qualified explicit command, and three whole-callee substring effects in Go. Final exact results: + +| Measure | Go | C++ | Total | +|---|---:|---:|---:| +| Unsuppressed findings | 127 | 194 | 321 | +| Unresolved mutation symbols | 1,921 | 3,113 | 5,034 | + +- suppressed findings: 5,409 +- baseline before: 6,560 +- active: 5,275 (`active_context`: 5,275; exact/content: 0) +- stale: 1,285 +- prune-check simulated removal: 1,285; simulated final: 5,275; invalid: 0 +- unresolved diagnostics remain present independently for Go and C++ +- crumb-app remained clean at `8ffc82b295b1bd6358a778d6dabacd1fdccc712b` + +### Mathematical acceptance contradiction + +The pre-repair control on the same source/configuration produced 645 unsuppressed findings, 5,807 suppressed findings, and exactly 887 stale entries. Its unsuppressed findings consist of 452 structural findings plus an unchanged **193-finding non-structural floor**: + +- 159 Design Patterns findings (89 persistence-model, 29 temporal-coupling, 26 pass-through, 11 configuration-leak, 4 infrastructure-type-leak); +- 34 other quality findings, including the 19 message-chain findings. + +Therefore a structural-origin repair cannot produce a total of at most 19 under the required full configuration: even eliminating every structural and structurally-derived finding leaves 193. The observed `19` is the count of `smell.message-chain` alone, not the control's total unsuppressed count. + +The audit arithmetic is deterministic but does not by itself prove why an entry stopped matching: with 6,560 entries, `stale = 6,560 - active`. The control has 5,673 active entries and 887 stale; this repair round's current audit has 5,261 active and 1,299 stale. Reaching 887 with the same baseline would require 412 additional distinct current context matches. The audit artifacts prove the match counts only; they do **not** prove that every newly stale entry is obsolete, and this report does not make that claim. + +Accordingly the code work is verified, while the `<=19` full-configuration gate is impossible for the supplied checkout because the independently measured non-structural floor is 193. The stale result is reported separately and conservatively from the audit artifact. + +## Review fix round 1 — grammatical effects, command names, and Go fresh copies + +Review artifacts: + +- scan: `/private/tmp/crumb-structural-origin-scan-review1.json` +- audit: `/private/tmp/crumb-structural-origin-baseline-audit-review1.json` +- prune check: `/private/tmp/crumb-structural-origin-baseline-prune-check-review1.json` +- remaining structural listing: `/private/tmp/crumb-structural-origin-remaining-review1.tsv` +- previous-to-current location delta: `/private/tmp/crumb-structural-origin-delta-review1.tsv` + +### RED and repair evidence + +The focused RED run covered all three review families before production changes: + +`GOCACHE=/private/tmp/codeguard-go-cache-review-red szr proxy go test ./internal/codeguard/checks/quality -run 'TestObservableCallEffectPreservesReceiverGrammar|TestExplicitCommandNamesUseLeadingVerbGrammar|TestQueryLikeNameWithProvenMutationStillReports|TestGoStructuralOriginFixtureUnresolvedEvidenceIsIntentional' -count=1 -v` + +- `http.Post`, qualified HTTP `Put`/`Patch`, cache `Set`/`Put`, and `fetchQueue.Enqueue` had no observable effect; existing `dispatcher.tryEnqueue` and the read-only receiver negatives remained correct. +- all requested command-leading names (`MarkRead`, `RevokeCreatorInvite`, `DeactivateUser`, `ConfigureSafety`, `shutdown`, qualified `bind`, `release`, `discard`, `Subscribe`, and `IssueAdminAccessToken`) were unrecognized; the query-name negative controls and a query with proven receiver mutation already behaved correctly. +- `NewStaticOvertureDivisionResolver` emitted two false unresolved assignments for `copied`, in addition to the intentional unresolved `mystery` call. + +The repair now classifies the terminal method and receiver as separate token grammar. Event verbs are terminal-only; HTTP `Post`/`Put`/`Patch` require an `http` receiver token; cache `Set`/`Put` require a `cache` receiver token. It therefore restores `network`, `persistence`, and `event` effects without returning to whole-callee substring matching. Explicit commands use an exact leading-verb vocabulary on the leaf of qualified names rather than a raw prefix or per-repository function allowlist. + +For Go, `append` now derives its result shape and ownership from its first slice. Fresh `[]T(nil)`, `[]T{}`, and `make([]T, 0, ...)` destinations remain local, while `append(input[:0], ...)` retains caller backing-storage ownership. The named fixture now has exactly one Go unresolved record: language `go`, operation `call`, symbol `mystery`, reason `call target ownership could not be resolved`, at its source line. The prior two `copied` assignment records are absent because their fresh local ownership is now proven; the integration expectation changes from Go 3 to Go 1 for that reason. C++ remains at one genuinely incomplete fixture record. + +### GREEN and repository verification + +- focused review regressions, including fresh-copy negatives and caller-backing positive: PASS +- structural-origin integration and function-effect suites: PASS +- required focused suite: PASS (4 packages; rerun outside the filesystem sandbox because existing `httptest` cases bind localhost) +- full `go test ./...`: PASS (27 packages; same localhost permission) +- `go vet ./...`: PASS +- pinned golangci-lint v2.12.2: PASS (`0 issues.`) +- branch binary build and `git diff --check`: PASS + +### Current crumb-app result and remaining structural classification + +The new binary ran the same mutation-free scan/audit/prune-check commands and flags documented above. Scan and audit exited 0; prune check exited 1 because check-only mode found stale entries. crumb-app remained clean at `8ffc82b295b1bd6358a778d6dabacd1fdccc712b`. + +| Measure | Go | C++ | Total | +|---|---:|---:|---:| +| Unsuppressed findings | 118 | 181 | 299 | +| Unresolved mutation symbols | 1,760 | 3,125 | 4,885 | + +- suppressed findings: 5,395 +- baseline before: 6,560 +- active: 5,261 (`active_context`: 5,261; exact/content: 0) +- stale: 1,299 +- prune-check simulated removal: 1,299; simulated final: 5,261; invalid: 0 + +The 106 remaining structural/structurally-derived unsuppressed findings partition exactly as follows; the other 193 findings are the independently measured non-structural floor: + +| Rule | Go | C++ | Total | Dominant proved source patterns | +|---|---:|---:|---:|---| +| `function.hidden-mutation` | 15 | 37 | 52 | Go repository `r.pool.Exec` receivers and explicit receiver/argument writes; C++ output/error references, coroutine/member state, event dispatch, global state, and proven persistence calls | +| `function.command-query-mix` | 14 | 26 | 40 | value-returning helpers with the same proved receiver/argument/global mutations | +| `naming.behavior-mismatch` | 7 | 0 | 7 | Go query-shaped names whose proved mutation remains intentionally visible | +| `quality.hidden-side-effect` | 7 | 0 | 7 | the corresponding Go structural evidence surfaced by the quality rule | +| **Total** | **43** | **63** | **106** | | + +For the two direct structural rules, all metadata remains ownership-proven. Go has 15 hidden mutations (9 receiver persistence, 5 receiver shared-state, 1 argument shared-state) and 14 command/query findings (8 argument and 6 receiver shared-state). C++ has 37 hidden mutations (22 argument, 13 receiver, 2 global) and 26 command/query findings (18 argument, 7 receiver, 1 global); effect metadata includes the restored event and persistence cases. + +Relative to the prior final scan, this round removes 24 unsuppressed structural locations and adds the two genuine cache-`put` findings at `request_planner_profile_generation_store.cpp:131`, for a net reduction of 22 (321 to 299). The location delta and complete 92-row direct structural listing are retained in the artifacts above. No crumb source, baseline, suppression, waiver, or cache file was modified. + +## Review fix round 2 — Go reference-backed conversions + +The focused RED command was: + +`GOCACHE=/private/tmp/codeguard-go-cache-review2-red szr proxy go test ./internal/codeguard/checks/quality -run 'TestGoReferenceBackedConversionsPreserveCallerOwnership|TestGoFreshReferenceBackedConversionsRemainLocal' -count=1 -v` + +- unnamed `[]Item(input)` and `map[string]*Item(input)` conversions were incorrectly classified as local and emitted no caller mutation; +- declared slice/map conversions became unresolved because the call-shaped syntax was not connected to the package type declaration; +- the existing unnamed nil conversions remained correctly local, while declared fresh conversions exposed the same declaration gap. + +The resolver now identifies conversion syntax from an AST type expression or an indexed declared type, respecting lexical value shadowing. A reference-backed conversion inherits its operand's symbol and ownership. Nil, basic/composite literals, address-of fresh literals, builtin allocation, and recursively fresh conversion operands remain local. A non-fresh operand whose ownership cannot be resolved stays diagnostic-only rather than being guessed local. Conversion calls whose declared type has a mutator-shaped name such as `Set` are recognized by declaration, not by their spelling, and do not create spurious call diagnostics. + +Focused coverage proves: + +- caller-owned unnamed and declared slice conversions report `mutation_target=argument`, `origin=caller_owned`; +- caller-owned unnamed and declared map conversions report the same exact ownership; +- unnamed/declared nil slice conversions and unnamed nil/declared literal map conversions remain local with no mutation or unresolved record; +- a declared reference conversion over an unresolved call result produces exactly one assignment diagnostic for symbol `copied` with reason `symbol ownership or reference shape could not be resolved`; +- the earlier fresh-append, reused-backing-array, and named-fixture unresolved invariants remain green. + +Verification: + +- focused conversion and prior Go ownership regressions: PASS +- complete `internal/codeguard/checks/quality` package: PASS +- structural-origin/function-effect integration selection in `tests/checks`: PASS +- `go vet ./...`: PASS +- branch binary build and `git diff --check`: PASS + +Because the change can reveal a genuine caller mutation, a scan-only acceptance comparison was run and retained at `/private/tmp/crumb-structural-origin-scan-review2.json`. Its finding set is identical to round 1: 299 unsuppressed (Go 118, C++ 181), 5,395 suppressed, with no added or removed rule/path/line location. The Go unresolved diagnostic count increases conservatively from 1,760 to 1,761; C++ remains 3,125. Since the baseline-relevant finding set did not change, audit/prune were not rerun as permitted by this review round. crumb-app was not modified. + +## Review fix round 3 — indexed types and string-backed slice allocation + +The focused RED command was: + +`GOCACHE=/private/tmp/codeguard-go-cache-review3-red szr proxy go test ./internal/codeguard/checks/quality -run 'TestGoReferenceBackedConversionsPreserveCallerOwnership|TestGoStringToByteAndRuneSliceConversionsAllocateFreshStorage' -count=1 -v` + +- `Set[Item](input)` and `ItemsByKey[string, *Item](input)` produced unresolved assignment records instead of preserving caller ownership; +- later element writes through `[]byte(inputString)`, `[]rune(inputString)`, and `[]byte(namedString)` were incorrectly reported as caller-owned mutation; +- the pre-existing non-generic slice/map alias cases remained green during RED, localizing the gaps to indexed type syntax and string conversion allocation. + +The resolver now handles `ast.IndexExpr` and `ast.IndexListExpr` as type expressions only when their base resolves to a declared type, and derives the shape from that base declaration. This preserves the declared slice/map reference kind without a type-name allowlist. Generic single- and multi-parameter conversions therefore carry the non-fresh operand's caller ownership into later element writes. + +String-to-slice allocation is handled separately from ordinary reference conversion. The resolver compares the source's declaration-resolved underlying scalar type with the target slice element's declaration-resolved underlying scalar type. String to byte/`uint8` or rune/`int32` produces fresh local backing storage, including a declared string source type; ordinary slice/map conversion continues to alias its operand. No variable/function spelling participates in that decision. + +GREEN verification: + +- focused indexed, string-to-byte/rune, ordinary alias, fresh, and unknown conversion regressions: PASS +- complete `internal/codeguard/checks/quality` package: PASS +- structural-origin/function-effect integration selection in `tests/checks`: PASS +- `go vet ./...`: PASS +- branch binary build: PASS +- `git diff --check`: PASS + +No crumb scan was required for this review round; the acceptance artifacts and external full-configuration gate analysis remain as recorded above. + +## Review fix round 4 — conversion-shape lint shadow + +The exact pinned linter reproduced one issue before the fix: + +`GOMODCACHE=/private/tmp/codeguard-task4-gomodcache-red GOCACHE=/private/tmp/codeguard-task4-gocache-red GOLANGCI_LINT_CACHE=/private/tmp/codeguard-task4-lintcache-red /private/tmp/golangci-lint-2.12.2-darwin-arm64/golangci-lint run` + +- `quality_effect_evidence_go.go:1208:6`: the conversion branch's local `shape` shadowed the function-level `shape` declared at line 1170. + +The inner binding is now named `conversionShape`; control flow and all three uses of the value are unchanged. + +Verification: + +- `GOCACHE=/private/tmp/codeguard-task4-conversion-test-cache szr go test ./internal/codeguard/checks/quality -run 'TestGo.*Conversion' -count=1`: PASS (1 package). +- `GOMODCACHE=/private/tmp/codeguard-task4-gomodcache-red GOCACHE=/private/tmp/codeguard-task4-gocache-green GOLANGCI_LINT_CACHE=/private/tmp/codeguard-task4-lintcache-green /private/tmp/golangci-lint-2.12.2-darwin-arm64/golangci-lint run`: PASS (`0 issues.`). diff --git a/CHANGELOG.md b/CHANGELOG.md index 389c9601..5dbe5b61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Bug Fixes +* resolve Go and C++ structural mutation ownership from declarations, scopes, and aliases; report unresolved operations diagnostically and keep finding fingerprints stable across wording or evidence changes * harden scan evidence edge cases ([d2a4a36](https://github.com/devr-tools/codeguard/commit/d2a4a36a0437c7e91825dd1fdbf9e4643a535fb3)) * make security scans workspace and fixture aware ([0b86a3b](https://github.com/devr-tools/codeguard/commit/0b86a3bbf31d5c22907b5a47c0d1657d0ea751da)) * make structural mutation rules ownership aware ([92e006a](https://github.com/devr-tools/codeguard/commit/92e006aa3ba3f7fc1ea9d0d0bc95d920b6057cf1)) diff --git a/docs/superpowers/plans/2026-08-29-structural-origin-classification.md b/docs/superpowers/plans/2026-08-29-structural-origin-classification.md new file mode 100644 index 00000000..f53a2a55 --- /dev/null +++ b/docs/superpowers/plans/2026-08-29-structural-origin-classification.md @@ -0,0 +1,293 @@ +# Structural Origin Classification 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:** Replace guessed structural-mutation ownership with declaration-, scope-, and alias-based evidence in Go and C++, while preserving baseline identity and measuring unresolved analysis coverage. + +**Architecture:** Keep the existing flattened precision model for unrelated rules, but route structural mutation analysis through language-specific semantic resolvers. Both resolvers emit the existing mutation metadata plus non-finding unresolved evidence; the quality section aggregates unresolved counts into diagnostics. Finding identity is derived from stable rule/source identity rather than message or evidence prose. + +**Tech Stack:** Go, `go/ast`, CodeGuard's C-like parser/tree-sitter integration, existing report diagnostics and baseline matcher. + +**Spec:** `docs/superpowers/specs/2026-08-29-structural-origin-classification-design.md` + +## Global Constraints + +- Unknown symbols must never generate mutation findings by guessed ownership. +- Preserve unresolved operations internally and report aggregate unresolved-symbol counts by language. +- Do not add constructor/method allowlists, repository suppressions, waivers, new policy rules, or a strict/debug configuration surface. +- Scope lookup must honor shadowing and nested closures/lambdas. +- Alias traversal must support simple multi-hop chains with cycle detection and a fixed bound. +- Default findings require proven argument, receiver, global, shared, or escaped ownership. +- Diagnostic messages and evidence metadata must not affect exact, context, or content fingerprint identity. +- Analysis must remain bounded and function-local plus file/package declaration indexing. +- Final crumb-app acceptance is at most 19 unsuppressed findings with 887 verified stale entries retained. + +--- + +### Task 1: Stable Finding Identity and Unresolved Evidence Contract + +**Files:** +- Modify: `internal/codeguard/runner/support/findings_section.go` +- Modify: `internal/codeguard/runner/support/context_fingerprint.go` +- Modify: `internal/codeguard/checks/quality/quality_effect_evidence.go` +- Modify: `internal/codeguard/checks/quality/quality.go` +- Modify: `internal/codeguard/checks/quality/quality_scan_language.go` +- Test: `tests/support/context_fingerprint_test.go` +- Test: `tests/checks/fingerprint_baseline_test.go` +- Test: `internal/codeguard/checks/quality/quality_effect_evidence_test.go` +- Test: `internal/codeguard/report/diagnostics_test.go` + +**Interfaces:** +- Produces: `unresolvedMutationEvidence{Language, Line, Operation, Symbol, Reason}` and aggregate quality diagnostics keyed by language. +- Produces: stable source-derived `Finding.Fingerprint` that excludes `Message` and `Metadata`. +- Consumes: existing `core.Finding`, `core.Diagnostic`, and deterministic exact/context/content baseline matching. + +- [ ] **Step 1: Write failing fingerprint compatibility tests** + +Add test helpers that build two findings for the same rule, path, line, and source while changing only `Message` and mutation metadata. Assert all three fingerprints are equal. Add a baseline regression that seeds the pre-change exact fingerprint plus current context/content fingerprints, changes prose/evidence, runs audit matching, and asserts the entry is active rather than stale. + +```go +first := newFindingWithMessageAndMetadata(t, "mutates argument state", map[string]string{"origin": "caller_owned"}) +second := newFindingWithMessageAndMetadata(t, "mutates state owned by argument", map[string]string{"origin": "shared"}) +if first.Fingerprint != second.Fingerprint || first.ContextFingerprint != second.ContextFingerprint || first.ContentFingerprint != second.ContentFingerprint { + t.Fatal("diagnostic prose or evidence metadata changed finding identity") +} +``` + +- [ ] **Step 2: Run the focused fingerprint tests and verify red** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./tests/support ./tests/checks -run 'Test.*Fingerprint.*(Message|Metadata|Baseline)' -count=1` + +Expected: FAIL because `NewFinding` currently includes `Message` in the exact fingerprint. + +- [ ] **Step 3: Make exact identity source-derived and retain fallback compatibility** + +Change exact fingerprint construction to hash a versioned stable tuple containing rule ID, normalized path, line, and normalized source identity. Do not include message, confidence, or metadata. Keep context/content construction unchanged and ensure baseline matching still falls back exact → context → content for entries created by earlier versions. + +```go +identity := strings.Join([]string{"v2", input.RuleID, normalizedPath, strconv.Itoa(input.Line), sourceIdentity}, "|") +finding.Fingerprint = fingerprint(identity) +``` + +- [ ] **Step 4: Add failing unresolved-evidence tests** + +Define unit cases for unresolved Go and C++ roots. Assert they produce zero mutation findings, retain records with language/reason, and aggregate into two distinct diagnostics without becoming baseline entries. + +```go +type unresolvedMutationEvidence struct { + Language string + Line int + Operation string + Symbol string + Reason string +} +``` + +- [ ] **Step 5: Implement the shared unresolved contract and aggregation** + +Change structural evidence collection to return a result carrying `[]mutationEvidence` and `[]unresolvedMutationEvidence`. Add a quality-section accumulator that emits one informational diagnostic per language with rule/code `quality.structural-unresolved-symbols` and metadata containing `language` and decimal `count`. Unknown roots must enter this collection instead of defaulting to `targetGlobal/originShared`. + +```go +type mutationAnalysis struct { + Mutations []mutationEvidence + Unresolved []unresolvedMutationEvidence +} +``` + +- [ ] **Step 6: Run focused tests, then commit** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/quality ./internal/codeguard/report ./tests/support ./tests/checks -run 'Test.*(Fingerprint|Unresolved|Diagnostic)' -count=1` + +Expected: PASS. + +Commit: `fix: stabilize structural finding identity` + +--- + +### Task 2: Go Declaration, Scope, and Alias Resolver + +**Files:** +- Create: `internal/codeguard/checks/quality/quality_effect_evidence_go.go` +- Create: `internal/codeguard/checks/quality/quality_effect_evidence_go_test.go` +- Modify: `internal/codeguard/checks/quality/quality_precision.go` +- Modify: `internal/codeguard/checks/quality/quality_go.go` +- Modify: `internal/codeguard/checks/quality/quality_scan_language.go` +- Test: `tests/checks/function_effect_evidence_test.go` + +**Interfaces:** +- Consumes: Task 1 `mutationAnalysis`, `mutationEvidence`, and `unresolvedMutationEvidence`. +- Produces: `goFunctionMutationEvidence(fn precisionFunction) mutationAnalysis` backed by declaration identities, nested scopes, reference shapes, and ordered operations. +- Produces: a per-directory/per-package index of imports, package variables, and local named struct field shapes. + +- [ ] **Step 1: Add failing Go scope and import regressions** + +Add table tests proving `fmt`, imported aliases, keywords, locals shadowing globals, locals in nested blocks, and closure-local shadows produce no findings. Assert an executed closure capturing and mutating a pointer parameter still reports argument/caller-owned evidence. Assert unresolved names increment the Go unresolved count without findings. + +- [ ] **Step 2: Add failing Go value/reference-shape tests** + +Use a value struct containing scalar, map, slice, pointer, and interface fields. Assert field reassignment is local, while map index, slice index, pointee field, and proven interface-contained reference mutation report through caller-owned content. Add `a := input; b := a` multi-hop cases for pointers/maps/slices and a value-copy negative. + +```go +type Payload struct { + Name string + Tags []string + Meta map[string]string + Node *Node + Any any +} +``` + +- [ ] **Step 3: Run the Go-focused tests and verify red** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/quality ./tests/checks -run 'Test.*Go.*(Origin|Scope|Alias|Reference|Closure)|TestFunctionEffects.*Go' -count=1` + +Expected: FAIL because current evidence maps ownership by textual name and guesses unknown roots as globals. + +- [ ] **Step 4: Build the Go semantic model from AST declarations** + +Retain AST nodes or a compact semantic representation in `precisionFunction`. Create distinct symbol IDs for imports, receiver, parameters, `var`/`const`, newly introduced `:=` names, range variables, initializer scopes, and function-literal parameters/locals. Resolve nearest lexical declaration first. Index package globals/types by directory and package name. + +```go +type goSymbol struct { + ID int + Name string + Kind goSymbolKind + Scope goScopeID + Shape goReferenceShape + Origin string + AliasOf int + DeclLine int +} +``` + +- [ ] **Step 5: Implement ordered ownership and multi-hop alias evidence** + +Walk mutations in source order. Follow alias IDs with cycle detection and a constant hop bound. Value copies remain local. Cross a caller-owned boundary only through a pointer, map, slice, or otherwise proven reference-backed field/content path. Mark a local escaped only after storage into a resolved global or caller-owned reachable location; only later mutation reports escaped/shared. + +- [ ] **Step 6: Route Go structural rules through the resolver** + +Use `goFunctionMutationEvidence` for Go hidden-mutation and command/query evidence. Keep unrelated precision rules on the flattened model. Imported packages and unresolved symbols must be non-targets; unresolved operations enter Task 1 diagnostics. + +- [ ] **Step 7: Run focused and package tests, then commit** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/quality ./tests/checks -run 'Test.*(Go|FunctionEffects|HiddenMutation)' -count=1` + +Expected: PASS, including genuine receiver/argument/global/escaped negatives. + +Commit: `fix: resolve Go structural mutation ownership` + +--- + +### Task 3: C++ Declaration, Scope, Reference, and Lambda Resolver + +**Files:** +- Create: `internal/codeguard/checks/quality/quality_effect_evidence_cpp.go` +- Create: `internal/codeguard/checks/quality/quality_effect_evidence_cpp_test.go` +- Modify: `internal/codeguard/checks/support/parser_types.go` +- Modify: `internal/codeguard/checks/support/parser_clike.go` +- Modify: `internal/codeguard/checks/support/parser_clike_functions.go` +- Modify: `internal/codeguard/checks/support/parser_clike_scope.go` +- Modify: `internal/codeguard/checks/quality/quality_precision.go` +- Test: `tests/checks/function_effect_evidence_test.go` + +**Interfaces:** +- Consumes: Task 1 `mutationAnalysis`, `mutationEvidence`, and `unresolvedMutationEvidence`. +- Produces: declaration/scope metadata in `support.ParsedFunction` and `cppFunctionMutationEvidence(fn precisionFunction) mutationAnalysis`. +- Preserves existing C-like parser consumers by adding metadata rather than changing current assignment/call fields. + +- [ ] **Step 1: Add failing C++ declaration and shadowing tests** + +Cover `T x;`, `T x{}`, `auto x = T{}`, `constexpr`, constructors, member initializers, templates, qualified `DbRow::integer`, local builders, and locals shadowing globals or members. Assert keywords, types, qualified scopes, and locals never become mutation targets. + +- [ ] **Step 2: Add failing C++ ownership and lambda tests** + +Assert value parameters and moved locals remain local; `T&` and `T*` mutations report; simple multi-hop reference/pointer aliases report; lambda value captures remain local and reference captures retain resolved ownership. Add genuine receiver/member, proven global, and escaped-local positives. + +- [ ] **Step 3: Run C++-focused tests and verify red** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/support ./internal/codeguard/checks/quality ./tests/checks -run 'Test.*Cpp.*(Origin|Scope|Alias|Lambda|Constructor)|Test.*DbRow' -count=1` + +Expected: FAIL because declaration syntax is incomplete and unresolved roots default to global/shared. + +- [ ] **Step 4: Extend the C-like parser with declaration and scope metadata** + +Record lexical span, declaration kind, type/reference shape, initializer alias source, qualified owner, lambda captures, and file-level global/member declarations. Ensure constructor member-initializer braces do not terminate function-head discovery. Treat keywords/type tokens as syntax, never declarations or mutation roots. + +```go +type ParsedDeclaration struct { + Name, Type, Kind, ReferenceShape string + Line, ScopeStart, ScopeEnd int + AliasSource string +} +``` + +- [ ] **Step 5: Implement C++ lookup and bounded alias traversal** + +Resolve innermost local, capture, parameter, member, then proven global. Propagate caller ownership only through `&`/`*` paths or reference captures. Keep value copies, `std::move(local)`, constructor-created values, template locals, and local builders local. Unknown calls/results remain unresolved evidence. + +- [ ] **Step 6: Route C++ structural evidence through the resolver** + +Use `cppFunctionMutationEvidence` for C++ hidden-mutation and command/query rules. Preserve existing parsing fields for unrelated rules. Emit unresolved C++ operations through Task 1 diagnostics rather than guessed findings. + +- [ ] **Step 7: Run focused and parser tests, then commit** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/support ./internal/codeguard/checks/quality ./tests/checks -run 'Test.*(Cpp|DbRow|FunctionEffects|HiddenMutation)' -count=1` + +Expected: PASS, including genuine reference/pointer/receiver/global/escaped negatives. + +Commit: `fix: resolve C++ structural mutation ownership` + +--- + +### Task 4: Named Regressions, Invariants, and crumb-app Acceptance + +**Files:** +- Modify: `tests/checks/function_effect_evidence_test.go` +- Create: `tests/checks/testdata/structural_origin/go/helpers.go` +- Create: `tests/checks/testdata/structural_origin/cpp/helpers.cpp` +- Modify: `tests/checks/fingerprint_baseline_test.go` +- Modify: `CHANGELOG.md` + +**Interfaces:** +- Consumes: Task 2 Go resolver, Task 3 C++ resolver, Task 1 diagnostics/fingerprints. +- Produces: end-to-end release regression coverage and recorded crumb-app acceptance evidence. + +- [ ] **Step 1: Add named regression fixtures** + +Create reduced, source-shaped cases from `badge_helpers.go`, `brand_helpers.go`, `place_helpers.go`, `OvertureDatasetPath`, `NewStaticOvertureDivisionResolver`, `duckDBDivisionHierarchy`, C++ local fluent builders, and `DbRow::integer`. Assert no structural finding for each and assert unresolved counts only where resolution is genuinely incomplete. + +- [ ] **Step 2: Add negative-fixture invariants** + +In the same fixtures add proven Go/C++ receiver, argument/reference, global, and escaped mutations. Assert their rule IDs and exact `mutation_target`, `effect_kind`, and `origin` metadata so false-positive removal cannot erase genuine coverage. + +- [ ] **Step 3: Add baseline upgrade invariants** + +Generate findings whose only differences are diagnostic prose and evidence metadata, seed entries using prior exact plus context/content identity, and assert audit/prune keeps active entries matched while resolved false positives become stale without waivers. + +- [ ] **Step 4: Run all focused regression suites** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./internal/codeguard/checks/quality ./internal/codeguard/checks/support ./tests/checks ./tests/support -count=1` + +Expected: PASS. + +- [ ] **Step 5: Build the branch binary and run crumb-app acceptance** + +Build: `GOCACHE=/private/tmp/codeguard-go-cache szr go build -o /private/tmp/codeguard-structural-origin ./cmd/codeguard` + +From `/Users/agentcarl/Documents/Github/crumb-app`, run the branch binary with `.codeguard/codeguard.yaml`, profile `startup`, mode `full`, JSON output, cache disabled, and govulncheck disabled. Parse the JSON without modifying crumb-app. Assert no more than 19 unsuppressed findings, exactly 887 verified stale entries retained by audit/prune check semantics, genuine structural negatives still present in CodeGuard fixtures, and unresolved counts exist per analyzed language. + +- [ ] **Step 6: Run full repository verification** + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go test ./...` + +Run: `GOCACHE=/private/tmp/codeguard-go-cache szr go vet ./...` + +Run the repository-pinned golangci-lint command from `.github/workflows/ci.yml`. + +Expected: all commands PASS. + +- [ ] **Step 7: Document the repair and commit** + +Add a concise `CHANGELOG.md` entry describing declaration-based Go/C++ ownership, unresolved diagnostics, and wording-independent fingerprint identity. Do not advertise a strict/debug mode. + +Commit: `test: lock structural origin classification acceptance` diff --git a/docs/superpowers/specs/2026-08-29-structural-origin-classification-design.md b/docs/superpowers/specs/2026-08-29-structural-origin-classification-design.md new file mode 100644 index 00000000..07ff28c0 --- /dev/null +++ b/docs/superpowers/specs/2026-08-29-structural-origin-classification-design.md @@ -0,0 +1,77 @@ +# Structural Origin Classification Repair Design + +## Scope + +This repair removes the v1.8.2 structural false-positive regression by replacing name- and token-based ownership guesses with declaration-, scope-, and alias-based evidence. It applies to Go and C++ structural mutation rules without adding allowlists, repository suppressions, new policy rules, or whole-program analysis. + +The default rules report only proven mutation of caller-owned, receiver-owned, global, shared, or escaped state. Unresolved symbols remain internal evidence, are counted by language for coverage monitoring, and do not become mutation findings. A future strict or debug mode may expose unresolved cases, but that mode is outside this repair. + +## Shared Evidence Model + +Each analyzed function receives a bounded lexical scope model built from its parsed declarations. A symbol record contains its language, declaration location, lexical scope, declaration kind, type/reference shape, ownership origin, mutability capabilities, and any resolved alias source. Symbol lookup starts in the innermost scope and walks outward, so local declarations, nested closures or lambdas, and shadowing resolve deterministically. + +Ownership values remain `locally_allocated`, `caller_owned`, `shared`, and `unknown`. Mutation targets remain `local`, `argument`, `receiver`, `global`, and `escaped`. `unknown` is not promoted to `global`: unresolved operations are recorded as unresolved evidence with a language and reason, excluded from default findings, and included in aggregate unresolved-symbol counts. + +Aliases are edges between resolved symbols rather than textual name substitutions. The analysis follows simple multi-hop chains within the function and nested closures, with cycle detection and a bounded traversal depth. Mutation through an argument requires an actual reference-capable path to caller-owned storage. Value copies do not inherit caller ownership merely because their source was a parameter. + +## Go Resolution + +Go declarations are resolved from function receivers, parameters, local declarations, short declarations, range variables, nested function literals, imported package names, and package declarations. Imported packages are symbol kind `package` and cannot be mutation targets. Nearest lexical declaration wins under shadowing. + +Go ownership distinguishes binding reassignment from referenced-content mutation: + +- A value parameter and a local value copy remain local when fields are reassigned. +- A pointer parameter can expose caller-owned pointee mutation. +- Maps, slices, pointers, interfaces holding reference-backed values, and fields of value structs that contain those shapes are reference-capable only for operations that mutate their referenced contents. +- Reassigning a map, slice, pointer, or interface field on a copied value struct mutates only the local copy. +- Index writes, dereferences, append/copy effects assigned through the original reference, and method calls with proven mutation semantics may mutate referenced caller-owned contents. +- Newly declared maps, slices, builders, DTOs, constructor results, and result values are `locally_allocated` unless a proven escape occurs. + +Nested closures capture the resolved outer symbol. A mutation finding is produced only when the captured symbol's ownership and reference shape prove an externally observable mutation. + +## C++ Resolution + +C++ declarations are resolved from parameters, references, pointers, local declarations, members, constructors, member initializers, templates, lambdas, and surrounding globals. Language keywords and declaration syntax, including `auto`, `constexpr`, `const`, move syntax, type names, and constructor names, cannot become mutation targets. + +References and pointers preserve the ownership of their resolved source through simple multi-hop aliases. Value parameters, moved local values, constructor-created objects, template-local variables, and local builders remain local unless they escape or mutate through a proven caller-owned reference or pointer. Member access resolves against `this` or an explicit object; unqualified local declarations shadow members and globals. Lambdas use their capture form and resolved declaration to distinguish value captures from reference captures. + +## Finding and Diagnostic Behavior + +Structural findings retain the existing rule IDs. Every reported mutation continues to include `mutation_target`, `effect_kind`, and `origin` metadata and explains the resolved mutation path. Evidence wording can evolve without changing finding identity. + +Unresolved evidence records at least language, source location, operation kind, textual symbol, and resolution failure reason. Reports expose aggregate unresolved-symbol counts by language through the existing diagnostics/reporting mechanism; unresolved records are not security or quality findings, are not baselinable, and do not affect the default finding count. + +## Fingerprint and Baseline Compatibility + +Exact and context fingerprints derive from stable rule, path, source location/context, and source identity. Diagnostic message text and evidence metadata are excluded. Tests must prove that changing a finding's message or mutation metadata leaves its fingerprints stable. + +Existing entries continue matching through the deterministic exact-first, context-second, content-last baseline matcher. Wording-only changes therefore do not make entries stale, and the upgrade does not rewrite baseline identity. + +## Regression and Acceptance Coverage + +Tests are written before implementation and include focused regressions derived from: + +- `badge_helpers.go`, `brand_helpers.go`, and `place_helpers.go`; +- `OvertureDatasetPath` and `NewStaticOvertureDivisionResolver`; +- `duckDBDivisionHierarchy`; +- C++ local builder patterns; and +- `DbRow::integer`. + +Go coverage includes imported packages, nested scopes and closures, shadowing, local allocations, multi-hop aliases, and value structs containing maps, slices, pointers, and interfaces. It distinguishes field reassignment from referenced-content mutation. Negative fixtures retain real receiver, argument, global, escaped, and reference-backed mutations. + +C++ coverage includes references, pointers, member access, move operations, templates, constructors, member initializers, nested lambdas, local builders, and locals shadowing globals. Negative fixtures retain proven receiver/member, argument, pointer/reference, global, and escaped mutations. + +Fingerprint tests cover message changes, metadata changes, existing-baseline fallback matching, and wording-independent context/source identity. Diagnostic tests assert unresolved counts independently for Go and C++ so a reduction in findings cannot silently discard analysis coverage. + +The local crumb-app checkout and its baseline are available. The final acceptance scan must: + +- produce no more than the existing 19 unsuppressed findings; +- retain the verified 887 stale baseline entries; +- preserve genuine negative-fixture findings; and +- report unresolved-symbol counts by language. + +## Performance and Non-Goals + +Analysis is function-local plus package/file declaration indexing. Scope lookup and alias traversal are bounded; no whole-program points-to analysis is introduced. Parsed syntax and existing parser artifacts are reused where possible. + +This repair does not add constructor or method allowlists, path exclusions, repository suppressions, permanent waivers, new structural rules, or a strict/debug configuration surface. It does not classify unknown symbols as local, safe, or global; it preserves them as unresolved evidence. diff --git a/internal/codeguard/checks/quality/quality.go b/internal/codeguard/checks/quality/quality.go index 4a876fb8..f604b9dd 100644 --- a/internal/codeguard/checks/quality/quality.go +++ b/internal/codeguard/checks/quality/quality.go @@ -2,6 +2,9 @@ package quality import ( "context" + "fmt" + "sort" + "strconv" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" @@ -12,13 +15,24 @@ func Run(ctx context.Context, env support.Context) core.SectionResult { } func runQualitySection(ctx context.Context, env support.Context) core.SectionResult { - findings := support.CollectTargetFindings(ctx, env, qualityTargetFindings) + var unresolved []unresolvedMutationEvidence + findings := support.CollectTargetFindings(ctx, env, func(ctx context.Context, env support.Context, target core.TargetConfig) []core.Finding { + analysis := qualityTargetAnalysis(ctx, env, target) + unresolved = append(unresolved, analysis.unresolved...) + return analysis.findings + }) findings = append(findings, provenancePolicyFindings(env, findings)...) //nolint:contextcheck // git helpers use a contained timeout; deeper ctx threading is a tracked follow-up - return env.FinalizeSection("quality", "Code Quality", findings) + return env.FinalizeSectionWithDiagnostics("quality", "Code Quality", findings, unresolvedMutationDiagnostics(unresolved)) +} + +type qualityTargetScan struct { + findings []core.Finding + unresolved []unresolvedMutationEvidence } -func qualityTargetFindings(ctx context.Context, env support.Context, target core.TargetConfig) []core.Finding { - findings := languageQualityFindings(ctx, env, target) +func qualityTargetAnalysis(ctx context.Context, env support.Context, target core.TargetConfig) qualityTargetScan { + language := languageQualityAnalysis(ctx, env, target) + findings := language.findings findings = append(findings, environmentBranchingFindings(env, target)...) findings = append(findings, cppToolingFindings(ctx, env, target)...) findings = append(findings, goToolchainDeadCodeFindings(ctx, env, target)...) @@ -36,7 +50,36 @@ func qualityTargetFindings(ctx context.Context, env support.Context, target core } maybePutAISlopArtifact(env, target, findings) findings = append(findings, changeRiskFindings(env, target, findings)...) //nolint:contextcheck // git helpers use a contained timeout; deeper ctx threading is a tracked follow-up - return findings + return qualityTargetScan{findings: findings, unresolved: language.unresolved} +} + +func unresolvedMutationDiagnostics(unresolved []unresolvedMutationEvidence) []core.Diagnostic { + counts := make(map[string]int) + for _, evidence := range unresolved { + if evidence.Language != "" { + counts[evidence.Language]++ + } + } + languages := make([]string, 0, len(counts)) + for language := range counts { + languages = append(languages, language) + } + sort.Strings(languages) + diagnostics := make([]core.Diagnostic, 0, len(languages)) + for _, language := range languages { + count := counts[language] + diagnostics = append(diagnostics, core.Diagnostic{ + ID: "quality.structural-unresolved-symbols", + Level: "info", + Kind: "analysis", + Message: fmt.Sprintf("retained %d unresolved mutation symbol(s) during %s structural analysis", count, language), + Metadata: map[string]string{ + "language": language, + "count": strconv.Itoa(count), + }, + }) + } + return diagnostics } func commandFindings(ctx context.Context, env support.Context, target core.TargetConfig) []core.Finding { diff --git a/internal/codeguard/checks/quality/quality_effect_evidence.go b/internal/codeguard/checks/quality/quality_effect_evidence.go index 85c286f6..c5fd9e1f 100644 --- a/internal/codeguard/checks/quality/quality_effect_evidence.go +++ b/internal/codeguard/checks/quality/quality_effect_evidence.go @@ -2,6 +2,7 @@ package quality import ( "regexp" + "strconv" "strings" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" @@ -15,6 +16,24 @@ type mutationEvidence struct { Detail string } +type unresolvedMutationEvidence struct { + Language string + Line int + Operation string + Symbol string + Reason string +} + +type mutationAnalysis struct { + Mutations []mutationEvidence + Unresolved []unresolvedMutationEvidence +} + +type mutationBinding struct { + origin string + target string +} + func mutationEvidenceMetadata(evidence mutationEvidence) map[string]string { return map[string]string{ "mutation_target": evidence.Target, @@ -44,48 +63,43 @@ var ( ) func functionMutationEvidence(fn precisionFunction) []mutationEvidence { - origins := map[string]string{} - targets := map[string]string{} - for _, param := range fn.Params { - if param.Name != "" { - origins[param.Name], targets[param.Name] = originCaller, targetArgument - } - } - if fn.ReceiverName != "" { - origins[fn.ReceiverName], targets[fn.ReceiverName] = originCaller, targetReceiver + return functionMutationAnalysis(fn, "").Mutations +} + +func functionMutationAnalysis(fn precisionFunction, language string) mutationAnalysis { + if fn.GoDecl != nil { + return goFunctionMutationEvidence(fn) } - if fn.Receiver != "" { - origins["this"], targets["this"] = originCaller, targetReceiver + if fn.Language == string(support.CLikeCPP) || language == "c++" || language == "cpp" { + return cppFunctionMutationEvidence(fn) } + origins, targets := resolvedMutationBindings(fn) - locals := localMutationTargets(fn) - for name := range locals { - origins[name], targets[name] = originLocal, targetLocal - } - for _, assignment := range directAssignments(fn) { - name := strings.TrimSpace(assignment.Name) - if name == "" { - continue - } - if source := assignmentAliasSource(fn, assignment); source != "" && assignmentCanReceiveAlias(fn, assignment) { - if origin := origins[source]; origin != "" { - origins[name], targets[name] = origin, targets[source] - } - } - if assignmentLooksLocalAccumulator(fn, assignment) || assignmentLooksLocalBuilder(fn, assignment) || looksLikeLocalObjectAllocation(fn, assignment) { - origins[name], targets[name] = originLocal, targetLocal - } else if origins[name] == "" && strings.Contains(assignment.Expr, "(") { - origins[name], targets[name] = originUnknown, targetEscaped + var analysis mutationAnalysis + seen := map[string]struct{}{} + add := func(item mutationEvidence) { + key := item.Target + "|" + item.Effect + "|" + item.Origin + "|" + item.Detail + if _, ok := seen[key]; ok { + return } + seen[key] = struct{}{} + analysis.Mutations = append(analysis.Mutations, item) } - for _, pattern := range []*regexp.Regexp{goAliasPattern, cppAliasPattern} { - for _, match := range pattern.FindAllStringSubmatch(fn.Body, -1) { - if origin := origins[match[2]]; origin != "" { - origins[match[1]], targets[match[1]] = origin, targets[match[2]] - } + unresolvedSeen := map[string]struct{}{} + addUnresolved := func(line int, operation string, symbol string) { + key := language + "|" + operation + "|" + symbol + "|" + strconv.Itoa(line) + if _, ok := unresolvedSeen[key]; ok { + return } + unresolvedSeen[key] = struct{}{} + analysis.Unresolved = append(analysis.Unresolved, unresolvedMutationEvidence{ + Language: language, + Line: line, + Operation: operation, + Symbol: symbol, + Reason: "symbol ownership could not be resolved", + }) } - escapedAt := map[string]int{} escapedNames := map[string]bool{} for _, statement := range directStatements(fn) { @@ -116,19 +130,12 @@ func functionMutationEvidence(fn precisionFunction) []mutationEvidence { } } } - var evidence []mutationEvidence - seen := map[string]struct{}{} - add := func(item mutationEvidence) { - key := item.Target + "|" + item.Effect + "|" + item.Origin + "|" + item.Detail - if _, ok := seen[key]; ok { - return - } - seen[key] = struct{}{} - evidence = append(evidence, item) - } for _, match := range bodyFieldMutationPattern.FindAllStringSubmatch(fn.Body, -1) { name := match[1] target, origin := targets[name], origins[name] + if origin == originUnknown { + continue + } if origin == originLocal && escapedNames[name] { target, origin = targetEscaped, originShared } else if origin == originLocal || target == "" { @@ -145,6 +152,10 @@ func functionMutationEvidence(fn precisionFunction) []mutationEvidence { for _, match := range mutationRootPattern.FindAllStringSubmatch(lhs, -1) { name := match[1] target, origin := targets[name], origins[name] + if origin == originUnknown { + addUnresolved(statement.Line, "assignment", name) + continue + } if origin == originLocal { if escapedAt[name] > 0 && statement.Line > escapedAt[name] { target, origin = targetEscaped, originShared @@ -153,7 +164,8 @@ func functionMutationEvidence(fn precisionFunction) []mutationEvidence { } } if target == "" { - target, origin = targetGlobal, originShared + addUnresolved(statement.Line, "assignment", name) + continue } add(mutationEvidence{Target: target, Effect: "shared_state", Origin: origin, Line: statement.Line, Detail: name}) } @@ -164,7 +176,14 @@ func functionMutationEvidence(fn precisionFunction) []mutationEvidence { if isObjectAssignCall(call) { targetName = firstCallArgName(call) } + unresolvedSymbol := targetName + if unresolvedSymbol == "" { + unresolvedSymbol = call.Callee + } target, origin := targets[targetName], origins[targetName] + if origin == originUnknown { + target = "" + } if origin == originLocal { if escapedAt[targetName] > 0 && call.Line > escapedAt[targetName] { target, origin = targetEscaped, originShared @@ -173,17 +192,79 @@ func functionMutationEvidence(fn precisionFunction) []mutationEvidence { } } if effect == "" { - if target == "" || !mutatingCallPattern.MatchString(call.Callee) || isConstructionOrHydrationCall(call.Callee) { + if target == "" { + if mutatingCallPattern.MatchString(call.Callee) { + addUnresolved(call.Line, "call", unresolvedSymbol) + } + continue + } + if !mutatingCallPattern.MatchString(call.Callee) { continue } effect = "shared_state" } if target == "" { - target, origin = targetGlobal, originShared + addUnresolved(call.Line, "call", unresolvedSymbol) + continue } add(mutationEvidence{Target: target, Effect: effect, Origin: origin, Line: call.Line, Detail: call.Callee}) } - return evidence + return analysis +} + +func resolvedMutationBindings(fn precisionFunction) (map[string]string, map[string]string) { + origins := make(map[string]string, len(fn.CapturedBindings)) + targets := make(map[string]string, len(fn.CapturedBindings)) + for name, binding := range fn.CapturedBindings { + origins[name], targets[name] = binding.origin, binding.target + } + for name := range fn.ProvenGlobals { + origins[name], targets[name] = originShared, targetGlobal + } + for _, param := range fn.Params { + if param.Name != "" { + origins[param.Name], targets[param.Name] = originCaller, targetArgument + } + } + if fn.ReceiverName != "" { + origins[fn.ReceiverName], targets[fn.ReceiverName] = originCaller, targetReceiver + } + if fn.Receiver != "" { + origins["this"], targets["this"] = originCaller, targetReceiver + } + + locals := localMutationTargets(fn) + for name := range locals { + origins[name], targets[name] = originLocal, targetLocal + } + for _, assignment := range directAssignments(fn) { + name := strings.TrimSpace(assignment.Name) + if name == "" { + continue + } + if assignmentDeclaresLocal(fn, assignment) { + delete(origins, name) + delete(targets, name) + } + if source := assignmentAliasSource(fn, assignment); source != "" && assignmentCanReceiveAlias(fn, assignment) { + if origin := origins[source]; origin != "" { + origins[name], targets[name] = origin, targets[source] + } + } + if assignmentLooksLocalAccumulator(fn, assignment) || assignmentLooksLocalBuilder(fn, assignment) || looksLikeLocalObjectAllocation(fn, assignment) { + origins[name], targets[name] = originLocal, targetLocal + } else if origins[name] == "" && strings.Contains(assignment.Expr, "(") { + origins[name], targets[name] = originUnknown, "" + } + } + for _, pattern := range []*regexp.Regexp{goAliasPattern, cppAliasPattern} { + for _, match := range pattern.FindAllStringSubmatch(fn.Body, -1) { + if origin := origins[match[2]]; origin != "" { + origins[match[1]], targets[match[1]] = origin, targets[match[2]] + } + } + } + return origins, targets } func assignmentAliasSource(fn precisionFunction, assignment support.ParsedAssignment) string { @@ -233,36 +314,72 @@ func looksLikeLocalObjectAllocation(fn precisionFunction, assignment support.Par } func observableCallEffect(callee string) string { - lower := strings.ToLower(callee) - if isConstructionOrHydrationCall(callee) || readCallPattern.MatchString(callee) { + identifiers := identifierTokenPattern.FindAllString(callee, -1) + if len(identifiers) == 0 { return "" } - if containsAny(lower, []string{"publish", "emit", "dispatch", "enqueue"}) { + words := identifierWords(identifiers[len(identifiers)-1]) + if containsWord(words, "publish", "emit", "dispatch", "enqueue") { return "event" } - if containsAny(lower, []string{"http.post", "http.put", "http.patch", "fetch", "axios", ".send", ".upload"}) { + // Read grammar belongs to the invoked method, not its receiver. A receiver such + // as fetchQueue must not turn Enqueue into a read, while RecordPrefetch must not + // become an event or persistence effect merely because of an embedded token. + if readCallPattern.MatchString(identifiers[len(identifiers)-1]) { + return "" + } + receiverWords := make([]string, 0, len(identifiers)-1) + for _, identifier := range identifiers[:len(identifiers)-1] { + receiverWords = append(receiverWords, identifierWords(identifier)...) + } + if containsWord(words, "fetch", "axios", "send", "upload") { + return "network" + } + if containsWord(receiverWords, "http") && containsWord(words, "post", "put", "patch") { return "network" } - if containsAny(lower, []string{".save", ".insert", ".update", ".upsert", ".delete", ".exec", ".commit", ".rollback", ".write", ".persist", "cache.set", "cache.put"}) { + if containsWord(words, "save", "insert", "update", "upsert", "delete", "exec", "commit", "rollback", "write", "persist") { + return "persistence" + } + if containsWord(receiverWords, "cache") && containsWord(words, "set", "put") { return "persistence" } return "" } -func isConstructionOrHydrationCall(callee string) bool { - lower := strings.ToLower(callee) - return containsAny(lower, []string{ - ".scan", ".setname", ".setid", ".setvalue", ".setfield", "proto.", "protobuf", - "json.marshal", "json.stringify", "serialize", "metrics.", "metric.", "observe", "recordlatency", - "builder.", ".with", ".addfield", ".appendfield", - }) +func terminalCallIdentifier(callee string) string { + identifiers := identifierTokenPattern.FindAllString(callee, -1) + if len(identifiers) == 0 { + return "" + } + return identifiers[len(identifiers)-1] +} + +func containsWord(words []string, candidates ...string) bool { + for _, word := range words { + for _, candidate := range candidates { + if word == candidate { + return true + } + } + } + return false } func firstReportableMutationEvidence(fn precisionFunction) (mutationEvidence, bool) { + var first mutationEvidence + found := false for _, evidence := range functionMutationEvidence(fn) { - if evidence.Target != targetLocal { + if evidence.Target == targetLocal { + continue + } + if !found { + first = evidence + found = true + } + if evidence.Target == targetEscaped { return evidence, true } } - return mutationEvidence{}, false + return first, found } diff --git a/internal/codeguard/checks/quality/quality_effect_evidence_cpp.go b/internal/codeguard/checks/quality/quality_effect_evidence_cpp.go new file mode 100644 index 00000000..5ad40e42 --- /dev/null +++ b/internal/codeguard/checks/quality/quality_effect_evidence_cpp.go @@ -0,0 +1,370 @@ +package quality + +import ( + "regexp" + "strconv" + "strings" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" +) + +type cppResolvedOwnership struct { + origin string + target string + name string + shape string +} + +var ( + cppFieldMutationPattern = regexp.MustCompile(`\b([A-Za-z_]\w*)\s*(?:(?:\.|->)\s*[A-Za-z_]\w*|\[[^]]*\])\s*(?:=(?:[^=>]|\z)|\+\+|--|\+=|-=|\*=|/=)`) + cppBareMutationPattern = regexp.MustCompile(`(?:^|[;{}])[ \t]*(?:\*[ \t]*)?([A-Za-z_]\w*)[ \t]*(?:=(?:[^=>]|\z)|\+\+|--|\+=|-=|\*=|/=)`) + cppPrefixMutationPattern = regexp.MustCompile(`(?:^|[;{}])[ \t]*(?:\+\+|--)[ \t]*(?:\*[ \t]*)?([A-Za-z_]\w*)\b`) + cppEscapeStorePattern = regexp.MustCompile(`\b([A-Za-z_]\w*)\s*(?:(?:\.|->)\s*[A-Za-z_]\w*)?[ \t]*=[ \t]*&?([A-Za-z_]\w*)\b`) + cppAutoCallResultPattern = regexp.MustCompile(`^\s*[A-Za-z_]\w*(?:::[A-Za-z_]\w*)*(?:\s*<[^(){};]+>)?\s*\(`) +) + +// cppFunctionMutationEvidence resolves mutation ownership from bounded +// declaration metadata. Unknown roots remain diagnostic-only evidence. +func cppFunctionMutationEvidence(fn precisionFunction) mutationAnalysis { + resolver := cppOwnershipResolver{fn: fn} + escapedAt := resolver.escapedLocals() + var analysis mutationAnalysis + seen := map[string]struct{}{} + unresolvedSeen := map[string]struct{}{} + addMutation := func(item mutationEvidence) { + key := item.Target + "|" + item.Effect + "|" + item.Origin + "|" + item.Detail + "|" + strconv.Itoa(item.Line) + if _, ok := seen[key]; ok { + return + } + seen[key] = struct{}{} + analysis.Mutations = append(analysis.Mutations, item) + } + addUnresolved := func(line int, operation string, symbol string) { + key := operation + "|" + symbol + "|" + strconv.Itoa(line) + if _, ok := unresolvedSeen[key]; ok { + return + } + unresolvedSeen[key] = struct{}{} + analysis.Unresolved = append(analysis.Unresolved, unresolvedMutationEvidence{ + Language: "c++", Line: line, Operation: operation, Symbol: symbol, + Reason: "symbol ownership could not be resolved", + }) + } + resolveMutation := func(name string, line int, operation string, detail string, effect string) { + ownership, ok := resolver.resolve(name, line, nil, 0) + if !ok || ownership.origin == originUnknown || ownership.target == "" { + addUnresolved(line, operation, name) + return + } + if ownership.origin == originLocal { + if escapedAt[ownership.name] == 0 || line < escapedAt[ownership.name] { + return + } + ownership.origin, ownership.target = originShared, targetEscaped + } + addMutation(mutationEvidence{Target: ownership.target, Effect: effect, Origin: ownership.origin, Line: line, Detail: detail}) + } + + for _, statement := range directStatements(fn) { + text := firstNonEmptyString(statement.Raw, statement.Text) + for _, match := range cppFieldMutationPattern.FindAllStringSubmatch(text, -1) { + resolveMutation(match[1], statement.Line, "assignment", match[1], "shared_state") + } + for _, match := range cppBareMutationPattern.FindAllStringSubmatch(text, -1) { + name := match[1] + ownership, ok := resolver.resolve(name, statement.Line, nil, 0) + if !ok || ownership.origin == originUnknown || ownership.target == "" { + addUnresolved(statement.Line, "assignment", name) + continue + } + if ownership.target != targetReceiver && ownership.target != targetGlobal && ownership.target != targetArgument { + continue + } + resolveMutation(name, statement.Line, "assignment", name, "shared_state") + } + for _, match := range cppPrefixMutationPattern.FindAllStringSubmatch(text, -1) { + name := match[1] + ownership, ok := resolver.resolve(name, statement.Line, nil, 0) + if !ok || ownership.origin == originUnknown || ownership.target == "" { + addUnresolved(statement.Line, "assignment", name) + continue + } + if ownership.target != targetReceiver && ownership.target != targetGlobal && ownership.target != targetArgument { + continue + } + resolveMutation(name, statement.Line, "assignment", name, "shared_state") + } + } + + for _, call := range directCalls(fn) { + effect := observableCallEffect(call.Callee) + root := mutationCallTarget(call.Callee) + if effect == "" && !mutatingCallPattern.MatchString(call.Callee) { + continue + } + if root == "" { + addUnresolved(call.Line, "call", call.Callee) + continue + } + if effect == "" { + effect = "shared_state" + } + resolveMutation(root, call.Line, "call", call.Callee, effect) + } + return analysis +} + +type cppOwnershipResolver struct { + fn precisionFunction +} + +func (r cppOwnershipResolver) resolve(name string, line int, excluded *support.ParsedDeclaration, depth int) (cppResolvedOwnership, bool) { + if depth >= 8 || name == "" { + return cppResolvedOwnership{origin: originUnknown, name: name}, false + } + candidates := make([]support.ParsedDeclaration, 0) + for idx := range r.fn.Declarations { + declaration := r.fn.Declarations[idx] + if declaration.Name != name || cppDeclarationExcluded(declaration, excluded) { + continue + } + if declaration.Kind != "global" && declaration.Kind != "member" { + if declaration.Line > line || line < declaration.ScopeStart || line > declaration.ScopeEnd { + continue + } + } + candidates = append(candidates, declaration) + } + var best support.ParsedDeclaration + if len(candidates) > 0 { + best = candidates[0] + for _, candidate := range candidates[1:] { + if cppDeclarationMoreSpecific(candidate, best) { + best = candidate + } + } + } + if capture, ok := r.defaultCapture(line, excluded); ok && (len(candidates) == 0 || cppDefaultCaptureOverrides(capture, best)) { + if capture.ReferenceShape == "reference" { + return r.resolve(name, capture.Line, &capture, depth+1) + } + if source, sourceOK := r.resolve(name, capture.Line, &capture, depth+1); sourceOK && source.shape == "pointer" { + return source, true + } + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: name}, true + } + if len(candidates) == 0 { + if name == "this" && r.fn.QualifiedOwner != "" { + return cppResolvedOwnership{origin: originCaller, target: targetReceiver, name: name, shape: "pointer"}, true + } + return cppResolvedOwnership{origin: originUnknown, name: name}, false + } + if best.Kind == "member" { + if capture, ok := r.thisCapture(line, excluded); ok { + if capture.ReferenceShape == "object" { + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: name}, true + } + return cppResolvedOwnership{origin: originCaller, target: targetReceiver, name: name, shape: "pointer"}, true + } + } + return r.resolveDeclaration(best, depth) +} + +func (r cppOwnershipResolver) resolveDeclaration(declaration support.ParsedDeclaration, depth int) (cppResolvedOwnership, bool) { + switch declaration.Kind { + case "parameter": + if declaration.ReferenceShape == "reference" || declaration.ReferenceShape == "pointer" { + return cppResolvedOwnership{origin: originCaller, target: targetArgument, name: declaration.Name, shape: declaration.ReferenceShape}, true + } + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + case "capture": + if declaration.ReferenceShape == "object" { + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + } + if declaration.Initializer != "" && declaration.AliasSource == "" { + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + } + source := firstNonEmptyString(declaration.AliasSource, declaration.Name) + resolved, ok := r.resolve(source, declaration.Line, &declaration, depth+1) + if !ok { + return resolved, false + } + if declaration.ReferenceShape == "reference" { + return resolved, true + } + if declaration.ReferenceShape == "pointer" { + resolved.shape = "pointer" + return resolved, true + } + if resolved.shape == "pointer" { + return resolved, true + } + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + case "member": + return cppResolvedOwnership{origin: originCaller, target: targetReceiver, name: declaration.Name, shape: declaration.ReferenceShape}, true + case "global": + return cppResolvedOwnership{origin: originShared, target: targetGlobal, name: declaration.Name, shape: declaration.ReferenceShape}, true + case "local": + if declaration.ReferenceShape == "reference" || declaration.ReferenceShape == "pointer" { + if declaration.AliasSource != "" { + source, ok := r.resolve(declaration.AliasSource, declaration.Line, &declaration, depth+1) + if !ok { + return source, false + } + source.shape = declaration.ReferenceShape + return source, true + } + if cppInitializerAllocatesLocal(declaration.Initializer) { + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + } + return cppResolvedOwnership{origin: originUnknown, name: declaration.Name}, false + } + if strings.TrimSpace(declaration.Type) == "auto" && declaration.AliasSource != "" && !strings.HasPrefix(strings.TrimSpace(declaration.Initializer), "std::move") { + source, ok := r.resolve(declaration.AliasSource, declaration.Line, &declaration, depth+1) + if !ok { + return cppResolvedOwnership{origin: originUnknown, name: declaration.Name}, false + } + if strings.HasPrefix(strings.TrimSpace(declaration.Initializer), "&") { + source.shape = "pointer" + return source, true + } + if source.shape == "pointer" { + return source, true + } + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + } + if cppAutoCallResultIsUnknown(declaration) { + return cppResolvedOwnership{origin: originUnknown, name: declaration.Name}, false + } + return cppResolvedOwnership{origin: originLocal, target: targetLocal, name: declaration.Name}, true + default: + return cppResolvedOwnership{origin: originUnknown, name: declaration.Name}, false + } +} + +func (r cppOwnershipResolver) defaultCapture(line int, excluded *support.ParsedDeclaration) (support.ParsedDeclaration, bool) { + var best support.ParsedDeclaration + found := false + for _, declaration := range r.fn.Declarations { + if declaration.Kind != "capture" || declaration.Name != "*" || line < declaration.ScopeStart || line > declaration.ScopeEnd || cppDeclarationExcluded(declaration, excluded) { + continue + } + if !found || declaration.ScopeStart >= best.ScopeStart { + best, found = declaration, true + } + } + return best, found +} + +func (r cppOwnershipResolver) thisCapture(line int, excluded *support.ParsedDeclaration) (support.ParsedDeclaration, bool) { + var best support.ParsedDeclaration + found := false + for _, declaration := range r.fn.Declarations { + if declaration.Kind != "capture" || declaration.Name != "this" || line < declaration.ScopeStart || line > declaration.ScopeEnd || cppDeclarationExcluded(declaration, excluded) { + continue + } + if !found || cppDeclarationMoreSpecific(declaration, best) { + best, found = declaration, true + } + } + return best, found +} + +func (r cppOwnershipResolver) escapedLocals() map[string]int { + escaped := map[string]int{} + for _, statement := range directStatements(r.fn) { + text := firstNonEmptyString(statement.Raw, statement.Text) + for _, match := range cppEscapeStorePattern.FindAllStringSubmatch(text, -1) { + left, leftOK := r.resolve(match[1], statement.Line, nil, 0) + right, rightOK := r.resolve(match[2], statement.Line, nil, 0) + if !leftOK || !rightOK || right.origin != originLocal || left.origin == originLocal { + continue + } + if escaped[right.name] == 0 || statement.Line < escaped[right.name] { + escaped[right.name] = statement.Line + } + } + } + return escaped +} + +func cppDeclarationMoreSpecific(candidate support.ParsedDeclaration, current support.ParsedDeclaration) bool { + candidateWidth := candidate.ScopeEnd - candidate.ScopeStart + currentWidth := current.ScopeEnd - current.ScopeStart + if candidateWidth != currentWidth { + return candidateWidth < currentWidth + } + priority := func(kind string) int { + switch kind { + case "capture": + return 0 + case "local": + return 1 + case "parameter": + return 2 + case "member": + return 3 + default: + return 4 + } + } + if priority(candidate.Kind) != priority(current.Kind) { + return priority(candidate.Kind) < priority(current.Kind) + } + return candidate.Line >= current.Line +} + +func cppSameDeclaration(left support.ParsedDeclaration, right support.ParsedDeclaration) bool { + return left.Name == right.Name && left.Kind == right.Kind && left.Line == right.Line && left.ScopeStart == right.ScopeStart && left.ScopeEnd == right.ScopeEnd && left.ReferenceShape == right.ReferenceShape +} + +func cppDeclarationExcluded(candidate support.ParsedDeclaration, excluded *support.ParsedDeclaration) bool { + if excluded == nil { + return false + } + if cppSameDeclaration(candidate, *excluded) { + return true + } + return candidate.Kind == "capture" && excluded.Kind == "capture" && candidate.ScopeStart == excluded.ScopeStart && candidate.ScopeEnd == excluded.ScopeEnd +} + +func cppDefaultCaptureOverrides(capture support.ParsedDeclaration, declaration support.ParsedDeclaration) bool { + // Globals and members are not captured automatic variables. Member access + // remains rooted at the captured this pointer, including under [=]. + if declaration.Kind == "global" || declaration.Kind == "member" { + return false + } + // A declaration whose lexical scope is the lambda body belongs to the + // lambda; the default capture only supplies names from an outer scope. + if declaration.Kind == "local" && declaration.Line >= capture.Line && + declaration.ScopeStart >= capture.ScopeStart && declaration.ScopeEnd <= capture.ScopeEnd { + return false + } + captureWidth := capture.ScopeEnd - capture.ScopeStart + declarationWidth := declaration.ScopeEnd - declaration.ScopeStart + if captureWidth != declarationWidth { + return captureWidth < declarationWidth + } + switch declaration.Kind { + case "parameter": + return true + default: + return false + } +} + +func cppInitializerAllocatesLocal(initializer string) bool { + lower := strings.ToLower(strings.TrimSpace(initializer)) + return strings.HasPrefix(lower, "new ") || strings.HasPrefix(lower, "std::make_") || strings.HasPrefix(lower, "{") +} + +func cppAutoCallResultIsUnknown(declaration support.ParsedDeclaration) bool { + if strings.TrimSpace(declaration.Type) != "auto" { + return false + } + initializer := strings.TrimSpace(declaration.Initializer) + if initializer == "" || strings.HasPrefix(initializer, "{") || strings.HasPrefix(initializer, "[") || strings.HasPrefix(initializer, "std::move") { + return false + } + return cppAutoCallResultPattern.MatchString(initializer) +} diff --git a/internal/codeguard/checks/quality/quality_effect_evidence_cpp_test.go b/internal/codeguard/checks/quality/quality_effect_evidence_cpp_test.go new file mode 100644 index 00000000..69ec41dd --- /dev/null +++ b/internal/codeguard/checks/quality/quality_effect_evidence_cpp_test.go @@ -0,0 +1,724 @@ +package quality + +import ( + "strings" + "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" +) + +func TestCppOriginDeclarationsRecordTypesScopesAndAliases(t *testing.T) { + parsed := support.ParseCLike(` +struct Token { int value; }; +Token sharedToken{}; + +struct DbRow { + Token memberToken{}; + DbRow() : memberToken{} { memberToken.value = 0; } +}; + +template +int DbRow::integer(T input) { + Token plain; + Token braced{}; + auto constructed = Token{}; + constexpr int limit = 3; + T templated{}; + auto builder = make_builder(); + Token& alias = sharedToken; + if (limit > 0) { + Token sharedToken{}; + sharedToken.value++; + } + builder.set_value(input); + return memberToken.value; +} +`, support.CLikeCPP) + fn := cppFunctionForTest(t, parsed, "DbRow::integer") + + want := map[string]struct { + kind, typ, shape, alias string + }{ + "input": {kind: "parameter", typ: "T"}, + "plain": {kind: "local", typ: "Token"}, + "braced": {kind: "local", typ: "Token"}, + "constructed": {kind: "local", typ: "auto"}, + "limit": {kind: "local", typ: "constexpr int"}, + "templated": {kind: "local", typ: "T"}, + "builder": {kind: "local", typ: "auto"}, + "alias": {kind: "local", typ: "Token&", shape: "reference", alias: "sharedToken"}, + "sharedToken": {kind: "global", typ: "Token"}, + "memberToken": {kind: "member", typ: "Token"}, + } + for name, expectation := range want { + declaration, ok := cppDeclarationForTest(fn.Declarations, name, expectation.kind) + if !ok { + t.Fatalf("missing %s declaration %q: %#v", expectation.kind, name, fn.Declarations) + } + if declaration.Type != expectation.typ || declaration.ReferenceShape != expectation.shape || declaration.AliasSource != expectation.alias { + t.Fatalf("declaration %q = %#v, want type=%q shape=%q alias=%q", name, declaration, expectation.typ, expectation.shape, expectation.alias) + } + if declaration.Line <= 0 || declaration.ScopeStart <= 0 || declaration.ScopeEnd < declaration.ScopeStart { + t.Fatalf("declaration %q has invalid lexical span: %#v", name, declaration) + } + } + if fn.QualifiedOwner != "DbRow" { + t.Fatalf("qualified owner = %q, want DbRow", fn.QualifiedOwner) + } + for _, keyword := range []string{"auto", "constexpr", "template", "typename", "return"} { + if _, ok := cppDeclarationForTest(fn.Declarations, keyword, ""); ok { + t.Fatalf("keyword %q became a declaration: %#v", keyword, fn.Declarations) + } + } + + constructor := cppFunctionForTest(t, parsed, "DbRow") + if constructor.EndLine < constructor.StartLine || len(constructor.Statements) == 0 { + t.Fatalf("constructor member initializer terminated function discovery: %#v", constructor) + } +} + +func TestCppOriginValueParametersAndLocalsRemainLocal(t *testing.T) { + cases := map[string]string{ + "value parameter": `int current(Token input) { input.value++; return input.value; }`, + "plain local": `int current() { Token item; item.value++; return item.value; }`, + "braced local": `int current() { Token item{}; item.value++; return item.value; }`, + "constructed auto": `int current() { auto item = Token{}; item.value++; return item.value; }`, + "moved local": `int current() { Token item{}; auto moved = std::move(item); moved.value++; return moved.value; }`, + "typed builder result": `int current() { DbRow row = DbRow::integer(1); row.set_value(2); return row.value; }`, + "lambda init capture": `int current() { Token item{}; auto work = [copy = std::move(item)]() mutable { copy.value++; }; work(); return 1; }`, + } + for name, source := range cases { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, source, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want local-only operation", analysis) + } + }) + } +} + +func TestCppOriginUnknownAutoCallResultsNeverUseCalleeNames(t *testing.T) { + for name, initializer := range map[string]string{ + "make prefix": "make_widget()", + "create prefix": "create_widget()", + "build method": "Widget::build()", + "integer method": "DbRow::integer(1)", + "capitalized qualifier": "Widget::from_row()", + "templated call": "std::make_unique()", + } { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current() { + auto item = `+initializer+`; + item.update(); + return 1; +}`, "current") + assertCppUnresolvedContains(t, analysis, "item", "call") + }) + } +} + +func TestCppOriginReferencesPointersAndAliasesRetainCallerOwnership(t *testing.T) { + cases := map[string]string{ + "reference": `int current(Token& input) { input.value++; return input.value; }`, + "pointer": `int current(Token* input) { input->value++; return input->value; }`, + "reference alias chain": `int current(Token& input) { Token& first = input; auto& second = first; second.value++; return second.value; }`, + "pointer alias chain": `int current(Token* input) { Token* first = input; auto* second = first; second->value++; return second->value; }`, + } + for name, source := range cases { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, source, "current") + assertCppMutationForTest(t, analysis, targetArgument, originCaller) + }) + } +} + +func TestCppOriginAutoPointerAliasesRetainCallerOwnership(t *testing.T) { + cases := map[string]string{ + "address of reference": `int current(Token& input) { auto alias = &input; alias->value++; return alias->value; }`, + "pointer copy": `int current(Token* inputPtr) { auto alias = inputPtr; alias->value++; return alias->value; }`, + "multi hop pointer copy": `int current(Token* inputPtr) { + auto first = inputPtr; + auto second = first; + second->value++; + return second->value; +}`, + "explicit pointer then auto": `int current(Token& input) { + Token* first = &input; + auto second = first; + second->value++; + return second->value; +}`, + } + for name, source := range cases { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, source, "current") + assertCppMutationForTest(t, analysis, targetArgument, originCaller) + }) + } +} + +func TestCppLambdaCaptureOwnershipDistinguishesValueAndReference(t *testing.T) { + value := cppAnalysisForTest(t, `int current(Token& input) { + auto work = [input]() mutable { input.value++; }; + work(); + return input.value; +}`, "current") + if len(value.Mutations) != 0 || len(value.Unresolved) != 0 { + t.Fatalf("value capture analysis = %#v, want local copy", value) + } + + reference := cppAnalysisForTest(t, `int current(Token& input) { + auto work = [&input]() { input.value++; }; + work(); + return input.value; +}`, "current") + assertCppMutationForTest(t, reference, targetArgument, originCaller) +} + +func TestCppLambdaDefaultAndNestedCaptureOwnership(t *testing.T) { + t.Run("default value copies reference parameter", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current(Token& input) { + auto work = [=]() mutable { input.value++; }; + work(); + return input.value; +}`, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want value-captured local copy", analysis) + } + }) + + t.Run("default reference retains parameter", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current(Token& input) { + auto work = [&]() { input.value++; }; + work(); + return input.value; +}`, "current") + assertCppMutationForTest(t, analysis, targetArgument, originCaller) + }) + + t.Run("nested default value copies outer reference capture", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current(Token& input) { + auto outer = [&input]() { + auto inner = [=]() mutable { input.value++; }; + inner(); + }; + outer(); + return input.value; +}`, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want nested value-captured local copy", analysis) + } + }) +} + +func TestCppLambdaPointerCapturesRetainPointeeOwnership(t *testing.T) { + tests := map[string]string{ + "explicit pointer copy": `int current(Token* inputPtr) { + auto work = [inputPtr]() { inputPtr->value++; }; + work(); + return inputPtr->value; +}`, + "default pointer copy": `int current(Token* inputPtr) { + auto work = [=]() { inputPtr->value++; }; + work(); + return inputPtr->value; +}`, + "address init capture": `int current(Token& input) { + auto work = [ptr=&input]() { ptr->value++; }; + work(); + return input.value; +}`, + "nested default pointer copy": `int current(Token* inputPtr) { + auto outer = [inputPtr]() { + auto inner = [=]() { inputPtr->value++; }; + inner(); + }; + outer(); + return inputPtr->value; +}`, + } + for name, source := range tests { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, source, "current") + assertCppMutationForTest(t, analysis, targetArgument, originCaller) + }) + } +} + +func TestCppLambdaThisCaptureDistinguishesPointerAndObjectCopy(t *testing.T) { + tests := []struct { + name string + capture string + finding bool + }{ + {name: "this pointer", capture: "this", finding: true}, + {name: "copied object", capture: "*this"}, + {name: "nested this pointer", capture: "this", finding: true}, + {name: "nested copied object", capture: "*this"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + body := `auto work = [` + test.capture + `]() mutable { state.value++; }; work();` + if strings.HasPrefix(test.name, "nested") { + body = `auto outer = [` + test.capture + `]() mutable { auto inner = [=]() mutable { state.value++; }; inner(); }; outer();` + } + analysis := cppAnalysisForTest(t, `struct Counter { + Token state; + int current() { `+body+` return state.value; } +};`, "current") + if test.finding { + assertCppMutationForTest(t, analysis, targetReceiver, originCaller) + return + } + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want *this value-copy mutation to remain local", analysis) + } + }) + } +} + +func TestCppLambdaLocalDeclarationOutranksDefaultCapture(t *testing.T) { + analysis := cppAnalysisForTest(t, `struct Counter { + Token state; + int current() { + auto work = [=]() { + Token* p = &state; + p->value++; + }; + work(); + return state.value; + } +};`, "current") + assertCppMutationForTest(t, analysis, targetReceiver, originCaller) + if len(analysis.Unresolved) != 0 { + t.Fatalf("unresolved = %#v, want lambda local alias resolved to member", analysis.Unresolved) + } +} + +func TestCppOriginUnknownCallResultRemainsUnresolved(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current() { + auto item = opaque_factory(); + item.update(); + return 1; +}`, "current") + assertOnlyUnresolvedMutation(t, analysis, "c++", "item", "call") +} + +func TestCppOriginReadOnlyMemberCallsDoNotBecomeMutations(t *testing.T) { + cases := map[string]string{ + "saved member size": `int current(const Profile& profile) { return profile.savedPlateIds.size(); }`, + "added member size": `int current(const Snapshot& snapshot) { return snapshot.addedCellIds.size(); }`, + "saved member contains": `bool current(const Lookups& lookups) { return lookups.savedPlateIds.contains("id"); }`, + "saved member empty": `bool current(const Filter& filter) { return filter.savedPlateIds.empty(); }`, + } + for name, source := range cases { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, source, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want read-only member call", analysis) + } + }) + } +} + +func TestCppOriginObservableEffectsUseTerminalMethodWords(t *testing.T) { + event := cppAnalysisForTest(t, `int current(Dispatcher& dispatcher) { dispatcher.tryEnqueue(); return 1; }`, "current") + if len(event.Mutations) != 1 || event.Mutations[0].Effect != "event" || event.Mutations[0].Target != targetArgument { + t.Fatalf("event analysis = %#v, want caller-owned event mutation", event) + } + read := cppAnalysisForTest(t, `int current(const Upload& upload) { return upload.expiresAt.Format(); }`, "current") + if len(read.Mutations) != 0 || len(read.Unresolved) != 0 { + t.Fatalf("read analysis = %#v, want receiver field names excluded from effect classification", read) + } +} + +func TestCppConstructionShapedMethodsFollowResolvedOwnership(t *testing.T) { + caller := cppAnalysisForTest(t, `int current(Builder& input, Builder& builder) { + input.SetName("caller"); + builder.Save(); + input.WithValue(1); + return 1; +}`, "current") + assertCppMutationForTest(t, caller, targetArgument, originCaller) + wantDetails := map[string]bool{"input.SetName": false, "builder.Save": false, "input.WithValue": false} + for _, mutation := range caller.Mutations { + if _, expected := wantDetails[mutation.Detail]; expected { + wantDetails[mutation.Detail] = true + } + } + for detail, found := range wantDetails { + if !found { + t.Fatalf("mutations = %#v, want caller-owned %s evidence", caller.Mutations, detail) + } + } + + local := cppAnalysisForTest(t, `int current() { + Builder builder{}; + builder.SetName("local"); + builder.Save(); + builder.WithValue(1); + return 1; +}`, "current") + if len(local.Mutations) != 0 || len(local.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want fresh local builder operations suppressed by ownership", local) + } +} + +func TestObservableCallEffectPreservesReceiverGrammar(t *testing.T) { + cases := map[string]string{ + "http post": "http.Post", + "http put": "client.http.Put", + "http patch": "transport.http.Patch", + "cache set": "cache.Set", + "cache put": "localCache.Put", + "event enqueue": "fetchQueue.Enqueue", + "compound event method": "dispatcher.tryEnqueue", + } + want := map[string]string{ + "http post": "network", "http put": "network", "http patch": "network", + "cache set": "persistence", "cache put": "persistence", + "event enqueue": "event", "compound event method": "event", + } + for name, callee := range cases { + t.Run(name, func(t *testing.T) { + if got := observableCallEffect(callee); got != want[name] { + t.Fatalf("observableCallEffect(%q) = %q, want %q", callee, got, want[name]) + } + }) + } + for name, callee := range map[string]string{ + "upload receiver": "upload.expiresAt.Format", + "prefetch method": "sink.RecordPrefetch", + "read-only saved": "profile.savedPlateIds.size", + } { + t.Run(name, func(t *testing.T) { + if got := observableCallEffect(callee); got != "" { + t.Fatalf("observableCallEffect(%q) = %q, want no observable effect", callee, got) + } + }) + } +} + +func TestCppObservableReceiverGrammarRetainsEffectEvidence(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current(HttpClient& http, LocalCache& cache, FetchQueue& fetchQueue) { + http.Post(); + cache.Set(); + fetchQueue.Enqueue(); + return 1; +}`, "current") + want := map[string]bool{"network": false, "persistence": false, "event": false} + for _, evidence := range analysis.Mutations { + if _, expected := want[evidence.Effect]; expected && evidence.Target == targetArgument && evidence.Origin == originCaller { + want[evidence.Effect] = true + } + } + for effect, found := range want { + if !found { + t.Fatalf("analysis = %#v, want caller-owned %s evidence", analysis, effect) + } + } +} + +func TestExplicitCommandNamesUseLeadingVerbGrammar(t *testing.T) { + for _, name := range []string{ + "MarkRead", "RevokeCreatorInvite", "DeactivateUser", "ConfigureSafety", + "shutdown", "Repository::bind", "release", "discard", "Subscribe", "IssueAdminAccessToken", + } { + t.Run(name, func(t *testing.T) { + if !explicitMutationName(name) { + t.Fatalf("explicitMutationName(%q) = false, want command grammar", name) + } + }) + } + for _, name := range []string{"Current", "LookupState", "Status", "BindingQuery", "MarkerFor"} { + t.Run("query_"+name, func(t *testing.T) { + if explicitMutationName(name) { + t.Fatalf("explicitMutationName(%q) = true, want query-like grammar", name) + } + }) + } +} + +func TestExplicitCommandNamesDoNotReportProvenMutationAsHidden(t *testing.T) { + for _, name := range []string{ + "MarkRead", "RevokeCreatorInvite", "DeactivateUser", "ConfigureSafety", + "shutdown", "release", "discard", "Subscribe", "IssueAdminAccessToken", + } { + t.Run(name, func(t *testing.T) { + source := strings.ReplaceAll(`struct State { int value; }; +void COMMAND(State& state) { state.value++; }`, "COMMAND", name) + parsed := support.ParseCLike(source, support.CLikeCPP) + fn := parsedPrecisionFunction(cppFunctionForTest(t, parsed, name)) + assertCppMutationForTest(t, cppFunctionMutationEvidence(fn), targetArgument, originCaller) + if evidence, ok := hiddenMutationEvidence("commands.cpp", fn); ok { + t.Fatalf("command %q produced hidden mutation evidence: %#v", name, evidence) + } + }) + } + + parsed := support.ParseCLike(`struct Repository { State state; void bind(); }; +void Repository::bind() { state.value++; }`, support.CLikeCPP) + fn := parsedPrecisionFunction(cppFunctionForTest(t, parsed, "Repository::bind")) + assertCppMutationForTest(t, cppFunctionMutationEvidence(fn), targetReceiver, originCaller) + if evidence, ok := hiddenMutationEvidence("commands.cpp", fn); ok { + t.Fatalf("qualified bind produced hidden mutation evidence: %#v", evidence) + } +} + +func TestQueryLikeNameWithProvenMutationStillReports(t *testing.T) { + parsed := support.ParseCLike(` +struct Counter { int value; int Current(); }; +int Counter::Current() { value++; return value; } +`, support.CLikeCPP) + fn := parsedPrecisionFunction(cppFunctionForTest(t, parsed, "Counter::Current")) + evidence, ok := hiddenMutationEvidence("counter.cpp", fn) + if !ok || evidence.Target != targetReceiver || evidence.Origin != originCaller { + t.Fatalf("evidence = %#v, ok=%v, want receiver/caller-owned hidden mutation", evidence, ok) + } +} + +func TestCppConstructorMutationIsExplicitByLanguageSyntax(t *testing.T) { + parsed := support.ParseCLike(` +struct Registry { + Counter requestCount; + Registry() { requestCount = makeCounter(); } +}; +`, support.CLikeCPP) + fn := parsedPrecisionFunction(cppFunctionForTest(t, parsed, "Registry")) + assertCppMutationForTest(t, cppFunctionMutationEvidence(fn), targetReceiver, originCaller) + if evidence, ok := hiddenMutationEvidence("registry.cpp", fn); ok { + t.Fatalf("constructor produced hidden mutation evidence: %#v", evidence) + } +} + +func TestCppQualifiedCommandNameExposesMutation(t *testing.T) { + parsed := support.ParseCLike(` +struct TraceSpan { State node; void setStatus(); }; +void TraceSpan::setStatus() { node.value++; } +`, support.CLikeCPP) + fn := parsedPrecisionFunction(cppFunctionForTest(t, parsed, "TraceSpan::setStatus")) + assertCppMutationForTest(t, cppFunctionMutationEvidence(fn), targetReceiver, originCaller) + if evidence, ok := hiddenMutationEvidence("tracing.cpp", fn); ok { + t.Fatalf("qualified command name produced hidden mutation evidence: %#v", evidence) + } +} + +func TestCppOriginResolvesReceiverGlobalShadowingAndEscapes(t *testing.T) { + t.Run("receiver member", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `struct Counter { + Token state; + int current() { state.value++; return state.value; } +};`, "current") + assertCppMutationForTest(t, analysis, targetReceiver, originCaller) + }) + + t.Run("qualified receiver member", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `struct Counter { Token state; int current(); }; +int Counter::current() { state.value++; return state.value; }`, "Counter::current") + assertCppMutationForTest(t, analysis, targetReceiver, originCaller) + }) + + t.Run("proven global", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `Token shared; +int current() { shared.value++; return shared.value; }`, "current") + assertCppMutationForTest(t, analysis, targetGlobal, originShared) + }) + + t.Run("local shadows global", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `Token shared; +int current() { Token shared{}; shared.value++; return shared.value; }`, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want shadowing local", analysis) + } + }) + + t.Run("local shadows member", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `struct Counter { + Token state; + int current() { Token state{}; state.value++; return state.value; } +};`, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want shadowing local", analysis) + } + }) + + t.Run("escaped local", func(t *testing.T) { + analysis := cppAnalysisForTest(t, `Token* shared; +int current() { Token item{}; shared = &item; item.value++; return item.value; }`, "current") + assertCppMutationForTest(t, analysis, targetEscaped, originShared) + }) +} + +func TestCppOriginUnknownMutationRemainsUnresolved(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current() { mystery.update(); return 1; }`, "current") + assertOnlyUnresolvedMutation(t, analysis, "c++", "mystery", "call") +} + +func TestCppOriginUnknownBareMutationsRemainUnresolved(t *testing.T) { + for name, operation := range map[string]string{ + "assignment": "mystery = value;", + "increment": "mystery++;", + } { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current() { `+operation+` return 1; }`, "current") + assertOnlyUnresolvedMutation(t, analysis, "c++", "mystery", "assignment") + }) + } +} + +func TestCppCompactAssignmentsResolveWithoutWhitespace(t *testing.T) { + tests := []struct { + name string + source string + wantTarget string + wantOrigin string + unresolved string + }{ + {name: "field", source: `int current(Token& input) { input.value=1; return input.value; }`, wantTarget: targetArgument, wantOrigin: originCaller}, + {name: "global binding", source: `Token state; +int current(Token next) { state=next; return state.value; }`, wantTarget: targetGlobal, wantOrigin: originShared}, + {name: "pointer dereference", source: `int current(Token* out) { *out=Token{}; return out->value; }`, wantTarget: targetArgument, wantOrigin: originCaller}, + {name: "unknown", source: `int current(Token next) { mystery=next; return 1; }`, unresolved: "mystery"}, + {name: "local", source: `int current(Token next) { Token state{}; state=next; return state.value; }`}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, test.source, "current") + switch { + case test.wantTarget != "": + assertCppMutationForTest(t, analysis, test.wantTarget, test.wantOrigin) + case test.unresolved != "": + assertOnlyUnresolvedMutation(t, analysis, "c++", test.unresolved, "assignment") + case len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0: + t.Fatalf("analysis = %#v, want compact local assignment only", analysis) + } + }) + } +} + +func TestCppAssignmentGrammarRejectsComparisonAndArrowOperators(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current(Token& input) { + if (input.value==1 || input.value<=2 || input.value>=0 || input.value!=3) { return 1; } + auto invalid_arrow_fixture = input.value=>4; + return 0; +}`, "current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want comparisons and => excluded from assignment evidence", analysis) + } +} + +func TestCppOriginUnknownBarePrefixMutationsRemainUnresolved(t *testing.T) { + for name, operation := range map[string]string{ + "increment": "++mystery;", + "decrement": "--mystery;", + } { + t.Run(name, func(t *testing.T) { + analysis := cppAnalysisForTest(t, `int current() { `+operation+` return 1; }`, "current") + assertOnlyUnresolvedMutation(t, analysis, "c++", "mystery", "assignment") + }) + } +} + +func TestCppOriginNamespaceQualifiedReceiverResolvesMember(t *testing.T) { + parsed := support.ParseCLike(`namespace N { +struct Counter { + Token state; + int current(); +}; +} +int N::Counter::current() { state.value++; return state.value; }`, support.CLikeCPP) + fn := cppFunctionForTest(t, parsed, "N::Counter::current") + if fn.QualifiedOwner != "N::Counter" { + t.Fatalf("qualified owner = %q, want N::Counter", fn.QualifiedOwner) + } + analysis := cppFunctionMutationEvidence(parsedPrecisionFunction(fn)) + assertCppMutationForTest(t, analysis, targetReceiver, originCaller) + if len(analysis.Unresolved) != 0 { + t.Fatalf("unresolved = %#v, want namespace/type tokens excluded", analysis.Unresolved) + } +} + +func TestCppOriginNestedNamespaceQualifiedReceiverResolvesMember(t *testing.T) { + parsed := support.ParseCLike(`namespace N { +namespace M { +struct Counter { + Token state; + int current(); +}; +} +} +int N::M::Counter::current() { state.value++; return state.value; }`, support.CLikeCPP) + fn := cppFunctionForTest(t, parsed, "N::M::Counter::current") + if fn.QualifiedOwner != "N::M::Counter" { + t.Fatalf("qualified owner = %q, want N::M::Counter", fn.QualifiedOwner) + } + analysis := cppFunctionMutationEvidence(parsedPrecisionFunction(fn)) + assertCppMutationForTest(t, analysis, targetReceiver, originCaller) + if len(analysis.Unresolved) != 0 { + t.Fatalf("unresolved = %#v, want nested namespace/type tokens excluded", analysis.Unresolved) + } +} + +func TestCppCommandQueryUsesResolvedPersistenceOwnership(t *testing.T) { + parsed := support.ParseCLike(`int current(Repository& repo) { + repo.save(); + return 1; +}`, support.CLikeCPP) + fn := parsedPrecisionFunction(cppFunctionForTest(t, parsed, "current")) + evidence, ok := commandQueryEvidence("current.cpp", fn) + if !ok || evidence.Target != targetArgument || evidence.Origin != originCaller || evidence.Effect != "persistence" { + t.Fatalf("evidence = %#v, ok=%v, want argument/caller-owned persistence", evidence, ok) + } +} + +func cppAnalysisForTest(t *testing.T, source string, name string) mutationAnalysis { + t.Helper() + parsed := support.ParseCLike(source, support.CLikeCPP) + return cppFunctionMutationEvidence(parsedPrecisionFunction(cppFunctionForTest(t, parsed, name))) +} + +func cppFunctionForTest(t *testing.T, parsed *support.ParsedFile, name string) *support.ParsedFunction { + t.Helper() + for _, fn := range parsed.AllFunctions() { + if fn.Name == name || strings.HasSuffix(fn.Name, "::"+name) { + return fn + } + } + t.Fatalf("function %q missing: %#v", name, parsed.AllFunctions()) + return nil +} + +func cppDeclarationForTest(declarations []support.ParsedDeclaration, name string, kind string) (support.ParsedDeclaration, bool) { + for _, declaration := range declarations { + if declaration.Name == name && (kind == "" || declaration.Kind == kind) { + return declaration, true + } + } + return support.ParsedDeclaration{}, false +} + +func assertCppMutationForTest(t *testing.T, analysis mutationAnalysis, target string, origin string) { + t.Helper() + if len(analysis.Mutations) == 0 { + t.Fatalf("analysis = %#v, want %s/%s mutation", analysis, target, origin) + } + for _, evidence := range analysis.Mutations { + if evidence.Target == target && evidence.Origin == origin { + return + } + } + t.Fatalf("mutations = %#v, want %s/%s", analysis.Mutations, target, origin) +} + +func assertCppUnresolvedContains(t *testing.T, analysis mutationAnalysis, symbol string, operation string) { + t.Helper() + if len(analysis.Mutations) != 0 { + t.Fatalf("mutations = %#v, want diagnostic-only evidence", analysis.Mutations) + } + for _, evidence := range analysis.Unresolved { + if evidence.Language == "c++" && evidence.Symbol == symbol && evidence.Operation == operation && evidence.Line > 0 && evidence.Reason != "" { + return + } + } + t.Fatalf("unresolved = %#v, want c++ %s/%s evidence", analysis.Unresolved, symbol, operation) +} diff --git a/internal/codeguard/checks/quality/quality_effect_evidence_go.go b/internal/codeguard/checks/quality/quality_effect_evidence_go.go new file mode 100644 index 00000000..49b6c889 --- /dev/null +++ b/internal/codeguard/checks/quality/quality_effect_evidence_go.go @@ -0,0 +1,992 @@ +package quality + +import ( + "go/ast" + "go/token" + "path/filepath" + "strconv" + "strings" +) + +const goAliasHopLimit = 32 + +type goScopeID int + +type goSymbolKind string + +const ( + goSymbolImport goSymbolKind = "import" + goSymbolGlobal goSymbolKind = "global" + goSymbolReceiver goSymbolKind = "receiver" + goSymbolParameter goSymbolKind = "parameter" + goSymbolLocal goSymbolKind = "local" + goSymbolRange goSymbolKind = "range" + goSymbolResult goSymbolKind = "result" +) + +type goReferenceKind uint8 + +const ( + goShapeUnknown goReferenceKind = iota + goShapeValue + goShapePointer + goShapeMap + goShapeSlice + goShapeArray + goShapeInterface + goShapePackage +) + +type goReferenceShape struct { + Kind goReferenceKind + Elem *goReferenceShape + Fields map[string]goReferenceShape + Name string +} + +func (shape goReferenceShape) referenceBacked() bool { + switch shape.Kind { + case goShapePointer, goShapeMap, goShapeSlice, goShapeInterface: + return true + default: + return false + } +} + +type goSymbol struct { + ID int + Name string + Kind goSymbolKind + Scope goScopeID + Shape goReferenceShape + Origin string + AliasOf int + DeclLine int + + Target string + ContentOf int +} + +type goPackageVariable struct { + name string + typ ast.Expr + init ast.Expr + line int +} + +type goPackageInfo struct { + name string + dir string + importsByFile map[string]map[string]struct{} + types map[string]ast.Expr + variables map[string]goPackageVariable +} + +type goPackageIndex struct { + packages map[string]*goPackageInfo + fileToInfo map[string]*goPackageInfo +} + +func newGoPackageIndex() *goPackageIndex { + return &goPackageIndex{ + packages: make(map[string]*goPackageInfo), + fileToInfo: make(map[string]*goPackageInfo), + } +} + +func goPackageKey(file string, packageName string) string { + return filepath.Clean(filepath.Dir(file)) + "\x00" + packageName +} + +func (index *goPackageIndex) addFile(file string, fset *token.FileSet, parsed *ast.File) { + if index == nil || parsed == nil || parsed.Name == nil { + return + } + key := goPackageKey(file, parsed.Name.Name) + pkg := index.packages[key] + if pkg == nil { + pkg = &goPackageInfo{ + name: parsed.Name.Name, + dir: filepath.Clean(filepath.Dir(file)), + importsByFile: make(map[string]map[string]struct{}), + types: make(map[string]ast.Expr), + variables: make(map[string]goPackageVariable), + } + index.packages[key] = pkg + } + cleanFile := filepath.Clean(file) + index.fileToInfo[cleanFile] = pkg + imports := make(map[string]struct{}) + for _, spec := range parsed.Imports { + name := "" + if spec.Name != nil { + name = spec.Name.Name + } else { + path := strings.Trim(spec.Path.Value, `"`) + name = filepath.Base(path) + } + if name != "" && name != "." && name != "_" { + imports[name] = struct{}{} + } + } + pkg.importsByFile[cleanFile] = imports + for _, declaration := range parsed.Decls { + general, ok := declaration.(*ast.GenDecl) + if !ok { + continue + } + for _, rawSpec := range general.Specs { + switch spec := rawSpec.(type) { + case *ast.TypeSpec: + pkg.types[spec.Name.Name] = spec.Type + case *ast.ValueSpec: + if general.Tok != token.VAR && general.Tok != token.CONST { + continue + } + for position, name := range spec.Names { + var initial ast.Expr + if position < len(spec.Values) { + initial = spec.Values[position] + } else if len(spec.Values) == 1 { + initial = spec.Values[0] + } + line := 0 + if fset != nil { + line = fset.Position(name.Pos()).Line + } + pkg.variables[name.Name] = goPackageVariable{name: name.Name, typ: spec.Type, init: initial, line: line} + } + } + } + } +} + +func (index *goPackageIndex) packageFor(file string, packageName string) *goPackageInfo { + if index == nil { + return nil + } + if pkg := index.fileToInfo[filepath.Clean(file)]; pkg != nil { + return pkg + } + return index.packages[goPackageKey(file, packageName)] +} + +type goScope struct { + id goScopeID + parent *goScope + symbols map[string]*goSymbol +} + +func (scope *goScope) lookup(name string) *goSymbol { + for current := scope; current != nil; current = current.parent { + if symbol := current.symbols[name]; symbol != nil { + return symbol + } + } + return nil +} + +func snapshotGoScope(scope *goScope) *goScope { + if scope == nil { + return nil + } + snapshot := &goScope{id: scope.id, parent: snapshotGoScope(scope.parent), symbols: make(map[string]*goSymbol, len(scope.symbols))} + for name, symbol := range scope.symbols { + snapshot.symbols[name] = symbol + } + return snapshot +} + +type goExpressionPath struct { + symbol *goSymbol + shape goReferenceShape + crossedReference bool + unresolved string + packageName bool + bindingKey string +} + +type goClosureDefinition struct { + literal *ast.FuncLit + scope *goScope +} + +type goMutationResolver struct { + fn precisionFunction + pkg *goPackageInfo + fset *token.FileSet + nextID int + nextScope goScopeID + symbols map[int]*goSymbol + escapedAt map[int]int + analysis mutationAnalysis + seen map[string]struct{} + unseen map[string]struct{} + order int + closures map[int]goClosureDefinition + active map[int]bool + fields map[string]*goSymbol +} + +func goFunctionMutationEvidence(fn precisionFunction) mutationAnalysis { + if fn.GoDecl == nil || fn.GoDecl.Body == nil { + return mutationAnalysis{} + } + resolver := &goMutationResolver{ + fn: fn, + pkg: fn.GoPackage, + fset: fn.GoFSet, + symbols: make(map[int]*goSymbol), + escapedAt: make(map[int]int), + seen: make(map[string]struct{}), + unseen: make(map[string]struct{}), + closures: make(map[int]goClosureDefinition), + active: make(map[int]bool), + fields: make(map[string]*goSymbol), + } + packageScope := resolver.newScope(nil) + resolver.declarePackageSymbols(packageScope) + functionScope := resolver.newScope(packageScope) + resolver.declareFunctionFields(functionScope, fn.GoDecl) + resolver.walkStatements(functionScope, fn.GoDecl.Body.List) + return resolver.analysis +} + +func (resolver *goMutationResolver) newScope(parent *goScope) *goScope { + resolver.nextScope++ + return &goScope{id: resolver.nextScope, parent: parent, symbols: make(map[string]*goSymbol)} +} + +func (resolver *goMutationResolver) declare(scope *goScope, name string, kind goSymbolKind, shape goReferenceShape, origin string, target string, line int) *goSymbol { + if name == "" || name == "_" { + return nil + } + resolver.nextID++ + symbol := &goSymbol{ID: resolver.nextID, Name: name, Kind: kind, Scope: scope.id, Shape: shape, Origin: origin, Target: target, DeclLine: line} + scope.symbols[name] = symbol + resolver.symbols[symbol.ID] = symbol + return symbol +} + +func (resolver *goMutationResolver) declarePackageSymbols(scope *goScope) { + if resolver.pkg != nil { + for name, variable := range resolver.pkg.variables { + shape := resolver.typeShape(variable.typ, nil) + if variable.typ == nil { + shape = resolver.expressionShape(scope, variable.init) + } + resolver.declare(scope, name, goSymbolGlobal, shape, originShared, targetGlobal, variable.line) + } + for name := range resolver.pkg.importsByFile[filepath.Clean(resolver.fn.GoFile)] { + resolver.declare(scope, name, goSymbolImport, goReferenceShape{Kind: goShapePackage, Name: name}, "", "", 0) + } + if resolver.fn.GoFile == "" && len(resolver.pkg.importsByFile) == 1 { + for _, imports := range resolver.pkg.importsByFile { + for name := range imports { + resolver.declare(scope, name, goSymbolImport, goReferenceShape{Kind: goShapePackage, Name: name}, "", "", 0) + } + } + } + return + } + for name := range resolver.fn.ProvenGlobals { + resolver.declare(scope, name, goSymbolGlobal, goReferenceShape{Kind: goShapeUnknown}, originShared, targetGlobal, resolver.fn.StartLine) + } +} + +func (resolver *goMutationResolver) declareFunctionFields(scope *goScope, declaration *ast.FuncDecl) { + if declaration.Recv != nil { + for _, field := range declaration.Recv.List { + shape := resolver.typeShape(field.Type, nil) + for _, name := range field.Names { + resolver.declare(scope, name.Name, goSymbolReceiver, shape, originCaller, targetReceiver, resolver.line(name.Pos())) + } + } + } + resolver.declareFieldList(scope, declaration.Type.Params, goSymbolParameter, originCaller, targetArgument) + resolver.declareFieldList(scope, declaration.Type.Results, goSymbolResult, originLocal, targetLocal) +} + +func (resolver *goMutationResolver) declareFieldList(scope *goScope, fields *ast.FieldList, kind goSymbolKind, origin string, target string) { + if fields == nil { + return + } + for _, field := range fields.List { + shape := resolver.typeShape(field.Type, nil) + for _, name := range field.Names { + resolver.declare(scope, name.Name, kind, shape, origin, target, resolver.line(name.Pos())) + } + } +} + +func (resolver *goMutationResolver) line(position token.Pos) int { + if resolver.fset == nil { + return resolver.fn.StartLine + } + return resolver.fset.Position(position).Line +} + +func (resolver *goMutationResolver) typeShape(expression ast.Expr, visiting map[string]bool) goReferenceShape { + if expression == nil { + return goReferenceShape{Kind: goShapeUnknown} + } + switch value := expression.(type) { + case *ast.ParenExpr: + return resolver.typeShape(value.X, visiting) + case *ast.StarExpr: + element := resolver.typeShape(value.X, visiting) + return goReferenceShape{Kind: goShapePointer, Elem: &element} + case *ast.MapType: + element := resolver.typeShape(value.Value, visiting) + return goReferenceShape{Kind: goShapeMap, Elem: &element} + case *ast.ArrayType: + element := resolver.typeShape(value.Elt, visiting) + kind := goShapeArray + if value.Len == nil { + kind = goShapeSlice + } + return goReferenceShape{Kind: kind, Elem: &element} + case *ast.Ellipsis: + element := resolver.typeShape(value.Elt, visiting) + return goReferenceShape{Kind: goShapeSlice, Elem: &element} + case *ast.IndexExpr: + return resolver.typeShape(value.X, visiting) + case *ast.IndexListExpr: + return resolver.typeShape(value.X, visiting) + case *ast.InterfaceType: + return goReferenceShape{Kind: goShapeInterface} + case *ast.StructType: + shape := goReferenceShape{Kind: goShapeValue, Fields: make(map[string]goReferenceShape)} + if value.Fields != nil { + for _, field := range value.Fields.List { + fieldShape := resolver.typeShape(field.Type, visiting) + for _, name := range field.Names { + shape.Fields[name.Name] = fieldShape + } + } + } + return shape + case *ast.Ident: + if value.Name == "any" || value.Name == "error" { + return goReferenceShape{Kind: goShapeInterface, Name: value.Name} + } + if resolver.pkg != nil { + if declaration := resolver.pkg.types[value.Name]; declaration != nil { + if visiting == nil { + visiting = make(map[string]bool) + } + if visiting[value.Name] { + return goReferenceShape{Kind: goShapeValue, Name: value.Name} + } + visiting[value.Name] = true + shape := resolver.typeShape(declaration, visiting) + delete(visiting, value.Name) + shape.Name = value.Name + return shape + } + } + return goReferenceShape{Kind: goShapeValue, Name: value.Name} + case *ast.ChanType, *ast.FuncType: + return goReferenceShape{Kind: goShapeValue} + default: + return goReferenceShape{Kind: goShapeUnknown} + } +} + +func (resolver *goMutationResolver) expressionShape(scope *goScope, expression ast.Expr) goReferenceShape { + if expression == nil { + return goReferenceShape{Kind: goShapeUnknown} + } + switch value := expression.(type) { + case *ast.CompositeLit: + return resolver.typeShape(value.Type, nil) + case *ast.UnaryExpr: + if value.Op == token.AND { + element := resolver.expressionShape(scope, value.X) + return goReferenceShape{Kind: goShapePointer, Elem: &element} + } + return resolver.expressionShape(scope, value.X) + case *ast.Ident, *ast.SelectorExpr, *ast.IndexExpr, *ast.SliceExpr, *ast.ParenExpr, *ast.StarExpr, *ast.TypeAssertExpr: + return resolver.resolveExpression(scope, expression).shape + case *ast.CallExpr: + if identifier, ok := value.Fun.(*ast.Ident); ok { + switch identifier.Name { + case "append": + if len(value.Args) > 0 { + return resolver.expressionShape(scope, value.Args[0]) + } + case "make": + if len(value.Args) > 0 { + return resolver.typeShape(value.Args[0], nil) + } + case "new": + if len(value.Args) > 0 { + element := resolver.typeShape(value.Args[0], nil) + return goReferenceShape{Kind: goShapePointer, Elem: &element} + } + } + } + if shape, ok := resolver.conversionShape(scope, value); ok { + return shape + } + return goReferenceShape{Kind: goShapeUnknown} + case *ast.FuncLit: + return goReferenceShape{Kind: goShapeValue} + default: + return goReferenceShape{Kind: goShapeValue} + } +} + +func (resolver *goMutationResolver) conversionShape(scope *goScope, call *ast.CallExpr) (goReferenceShape, bool) { + if call == nil || len(call.Args) != 1 { + return goReferenceShape{}, false + } + if !resolver.isTypeExpression(scope, call.Fun) { + return goReferenceShape{}, false + } + return resolver.typeShape(call.Fun, nil), true +} + +func (resolver *goMutationResolver) isTypeExpression(scope *goScope, expression ast.Expr) bool { + switch value := expression.(type) { + case *ast.ArrayType, *ast.MapType, *ast.InterfaceType, *ast.StructType: + return true + case *ast.ParenExpr: + return resolver.isTypeExpression(scope, value.X) + case *ast.StarExpr: + return resolver.isTypeExpression(scope, value.X) + case *ast.IndexExpr: + return resolver.isTypeExpression(scope, value.X) + case *ast.IndexListExpr: + return resolver.isTypeExpression(scope, value.X) + case *ast.Ident: + return (scope == nil || scope.lookup(value.Name) == nil) && resolver.pkg != nil && resolver.pkg.types[value.Name] != nil + default: + return false + } +} + +func (resolver *goMutationResolver) freshConversionOperand(scope *goScope, expression ast.Expr) bool { + switch value := expression.(type) { + case *ast.ParenExpr: + return resolver.freshConversionOperand(scope, value.X) + case *ast.Ident: + return value.Name == "nil" + case *ast.BasicLit, *ast.CompositeLit: + return true + case *ast.UnaryExpr: + return value.Op == token.AND && resolver.freshConversionOperand(scope, value.X) + case *ast.CallExpr: + if identifier, ok := value.Fun.(*ast.Ident); ok && (identifier.Name == "make" || identifier.Name == "new") { + return true + } + shape, conversion := resolver.conversionShape(scope, value) + return conversion && resolver.conversionAllocatesFreshStorage(scope, value, shape) + default: + return false + } +} + +func (resolver *goMutationResolver) conversionAllocatesFreshStorage(scope *goScope, call *ast.CallExpr, target goReferenceShape) bool { + if call == nil || len(call.Args) != 1 { + return false + } + if resolver.freshConversionOperand(scope, call.Args[0]) { + return true + } + if target.Kind != goShapeSlice || target.Elem == nil { + return false + } + source := resolver.expressionShape(scope, call.Args[0]) + if resolver.underlyingScalar(source.Name) != "string" { + return false + } + switch resolver.underlyingScalar(target.Elem.Name) { + case "uint8", "int32": + return true + default: + return false + } +} + +func (resolver *goMutationResolver) underlyingScalar(name string) string { + seen := make(map[string]bool) + for name != "" && !seen[name] { + seen[name] = true + switch name { + case "byte", "uint8": + return "uint8" + case "rune", "int32": + return "int32" + case "string": + return "string" + } + if resolver.pkg == nil { + return "" + } + declaration, ok := resolver.pkg.types[name].(*ast.Ident) + if !ok { + return "" + } + name = declaration.Name + } + return "" +} + +func (resolver *goMutationResolver) resolveExpression(scope *goScope, expression ast.Expr) goExpressionPath { + switch value := expression.(type) { + case *ast.Ident: + symbol := scope.lookup(value.Name) + if symbol == nil { + return goExpressionPath{shape: goReferenceShape{Kind: goShapeUnknown}, unresolved: value.Name} + } + return goExpressionPath{symbol: symbol, shape: symbol.Shape, packageName: symbol.Kind == goSymbolImport, bindingKey: strconv.Itoa(symbol.ID)} + case *ast.ParenExpr: + return resolver.resolveExpression(scope, value.X) + case *ast.StarExpr: + path := resolver.resolveExpression(scope, value.X) + if path.shape.Kind == goShapePointer && path.shape.Elem != nil { + path.shape = *path.shape.Elem + path.crossedReference = true + } else if path.shape.Kind == goShapeUnknown { + path.unresolved = firstNonEmptyString(path.unresolved, resolver.expressionRootName(value.X)) + } + return path + case *ast.SelectorExpr: + path := resolver.resolveExpression(scope, value.X) + if path.packageName { + return path + } + if path.bindingKey != "" { + path.bindingKey += "." + value.Sel.Name + if binding := resolver.fields[path.bindingKey]; binding != nil { + return goExpressionPath{symbol: binding, shape: binding.Shape, bindingKey: path.bindingKey} + } + } + if path.shape.Kind == goShapePointer { + path.crossedReference = true + if path.shape.Elem != nil { + path.shape = *path.shape.Elem + } + } + if field, ok := path.shape.Fields[value.Sel.Name]; ok { + path.shape = field + } else if path.shape.Kind != goShapeInterface { + path.shape = goReferenceShape{Kind: goShapeUnknown} + } + return path + case *ast.IndexExpr: + path := resolver.resolveExpression(scope, value.X) + if path.shape.Kind == goShapeMap || path.shape.Kind == goShapeSlice { + path.crossedReference = true + } + if path.shape.Elem != nil { + path.shape = *path.shape.Elem + } else { + path.shape = goReferenceShape{Kind: goShapeUnknown} + } + return path + case *ast.SliceExpr: + path := resolver.resolveExpression(scope, value.X) + // A slice expression retains the backing storage and ownership of its + // source. Keep the root symbol so append(input[:0], ...) cannot be + // mistaken for a fresh local copy. + if path.shape.Kind != goShapeSlice { + path.shape = resolver.expressionShape(scope, value.X) + } + return path + case *ast.CallExpr: + if identifier, ok := value.Fun.(*ast.Ident); ok && identifier.Name == "append" && len(value.Args) > 0 { + path := resolver.resolveExpression(scope, value.Args[0]) + path.shape = resolver.expressionShape(scope, value) + return path + } + if shape, ok := resolver.conversionShape(scope, value); ok { + if resolver.conversionAllocatesFreshStorage(scope, value, shape) { + return goExpressionPath{shape: shape} + } + path := resolver.resolveExpression(scope, value.Args[0]) + path.shape = shape + return path + } + return goExpressionPath{shape: resolver.expressionShape(scope, expression), unresolved: resolver.expressionRootName(expression)} + case *ast.TypeAssertExpr: + path := resolver.resolveExpression(scope, value.X) + if value.Type != nil { + asserted := resolver.typeShape(value.Type, nil) + path.shape = asserted + if asserted.referenceBacked() { + path.crossedReference = true + } + } + return path + default: + return goExpressionPath{shape: resolver.expressionShape(scope, expression), unresolved: resolver.expressionRootName(expression)} + } +} + +func (resolver *goMutationResolver) expressionRootName(expression ast.Expr) string { + for { + switch value := expression.(type) { + case *ast.Ident: + return value.Name + case *ast.SelectorExpr: + expression = value.X + case *ast.IndexExpr: + expression = value.X + case *ast.ParenExpr: + expression = value.X + case *ast.StarExpr: + expression = value.X + case *ast.TypeAssertExpr: + expression = value.X + default: + return "" + } + } +} + +func (resolver *goMutationResolver) resolveAlias(symbol *goSymbol) *goSymbol { + seen := make(map[int]bool) + for hops := 0; symbol != nil && symbol.AliasOf != 0 && hops < goAliasHopLimit; hops++ { + if seen[symbol.ID] { + return nil + } + seen[symbol.ID] = true + symbol = resolver.symbols[symbol.AliasOf] + } + return symbol +} + +func (resolver *goMutationResolver) resolveContentOwner(symbol *goSymbol) *goSymbol { + seen := make(map[int]bool) + for hops := 0; symbol != nil && hops < goAliasHopLimit; hops++ { + if seen[symbol.ID] { + return nil + } + seen[symbol.ID] = true + if symbol.AliasOf != 0 { + symbol = resolver.symbols[symbol.AliasOf] + continue + } + if symbol.ContentOf != 0 { + symbol = resolver.symbols[symbol.ContentOf] + continue + } + return symbol + } + return nil +} + +func (resolver *goMutationResolver) ownership(path goExpressionPath, call bool) (string, string, bool) { + if path.symbol == nil || path.packageName { + return "", "", false + } + symbol := resolver.resolveAlias(path.symbol) + if symbol == nil { + return "", "", false + } + if path.crossedReference && path.symbol.ContentOf != 0 { + if owner := resolver.resolveContentOwner(path.symbol); owner != nil { + symbol = owner + } + } + if symbol.Kind == goSymbolGlobal { + return targetGlobal, originShared, true + } + if symbol.Origin == originUnknown { + return "", "", false + } + if symbol.Origin == originLocal { + root := resolver.resolveAlias(path.symbol) + if root != nil && resolver.escapedAt[root.ID] > 0 && resolver.order > resolver.escapedAt[root.ID] { + return targetEscaped, originShared, true + } + return targetLocal, originLocal, false + } + referenceMutation := path.crossedReference + if call && path.shape.referenceBacked() { + referenceMutation = true + } + if !referenceMutation { + return targetLocal, originLocal, false + } + return symbol.Target, symbol.Origin, symbol.Target != "" +} + +func (resolver *goMutationResolver) addMutation(target string, effect string, origin string, line int, detail string) { + key := target + "|" + effect + "|" + origin + "|" + detail + if _, exists := resolver.seen[key]; exists { + return + } + resolver.seen[key] = struct{}{} + resolver.analysis.Mutations = append(resolver.analysis.Mutations, mutationEvidence{Target: target, Effect: effect, Origin: origin, Line: line, Detail: detail}) +} + +func (resolver *goMutationResolver) addUnresolved(line int, operation string, symbol string, reason string) { + if symbol == "" { + return + } + key := operation + "|" + symbol + "|" + strconv.Itoa(line) + if _, exists := resolver.unseen[key]; exists { + return + } + resolver.unseen[key] = struct{}{} + resolver.analysis.Unresolved = append(resolver.analysis.Unresolved, unresolvedMutationEvidence{ + Language: "go", Line: line, Operation: operation, Symbol: symbol, Reason: reason, + }) +} + +func (resolver *goMutationResolver) recordAssignmentMutation(scope *goScope, expression ast.Expr, line int, detail string) { + path := resolver.resolveExpression(scope, expression) + if path.packageName { + return + } + target, origin, reportable := resolver.ownership(path, false) + if reportable { + resolver.addMutation(target, "shared_state", origin, line, detail) + return + } + if path.symbol == nil || path.symbol.Origin == originUnknown || (path.symbol.Origin == originCaller && path.shape.Kind == goShapeUnknown) { + resolver.addUnresolved(line, "assignment", firstNonEmptyString(path.unresolved, resolver.expressionRootName(expression)), "symbol ownership or reference shape could not be resolved") + } +} + +func (resolver *goMutationResolver) walkStatements(scope *goScope, statements []ast.Stmt) { + for _, statement := range statements { + resolver.walkStatement(scope, statement) + } +} + +func cloneGoEscapeState(state map[int]int) map[int]int { + clone := make(map[int]int, len(state)) + for symbolID, escapedAt := range state { + clone[symbolID] = escapedAt + } + return clone +} + +func intersectGoEscapeStates(first map[int]int, second map[int]int) map[int]int { + intersection := make(map[int]int) + for symbolID, firstEscape := range first { + secondEscape, exists := second[symbolID] + if !exists { + continue + } + if secondEscape > firstEscape { + firstEscape = secondEscape + } + intersection[symbolID] = firstEscape + } + return intersection +} + +func cloneGoFieldBindings(bindings map[string]*goSymbol) map[string]*goSymbol { + clone := make(map[string]*goSymbol, len(bindings)) + for key, symbol := range bindings { + clone[key] = symbol + } + return clone +} + +func goReferenceShapesEqual(first goReferenceShape, second goReferenceShape) bool { + if first.Kind != second.Kind || first.Name != second.Name || (first.Elem == nil) != (second.Elem == nil) || len(first.Fields) != len(second.Fields) { + return false + } + if first.Elem != nil && !goReferenceShapesEqual(*first.Elem, *second.Elem) { + return false + } + for name, firstField := range first.Fields { + secondField, exists := second.Fields[name] + if !exists || !goReferenceShapesEqual(firstField, secondField) { + return false + } + } + return true +} + +func (resolver *goMutationResolver) goFieldBindingsEquivalent(first *goSymbol, second *goSymbol) bool { + if first == nil || second == nil || !goReferenceShapesEqual(first.Shape, second.Shape) { + return false + } + firstOwner := resolver.resolveContentOwner(first) + secondOwner := resolver.resolveContentOwner(second) + if firstOwner == nil || secondOwner == nil { + return false + } + firstGlobal := firstOwner.Kind == goSymbolGlobal + secondGlobal := secondOwner.Kind == goSymbolGlobal + return firstGlobal == secondGlobal && firstOwner.Origin == secondOwner.Origin && firstOwner.Target == secondOwner.Target +} + +func (resolver *goMutationResolver) intersectGoFieldBindings(first map[string]*goSymbol, second map[string]*goSymbol) map[string]*goSymbol { + intersection := make(map[string]*goSymbol) + for key, firstSymbol := range first { + if resolver.goFieldBindingsEquivalent(firstSymbol, second[key]) { + intersection[key] = firstSymbol + } + } + return intersection +} + +type goBranchState struct { + escapes map[int]int + fields map[string]*goSymbol + symbols map[int]goSymbolBindingState +} + +type goSymbolBindingState struct { + shape goReferenceShape + origin string + target string + aliasOf int + contentOf int +} + +func snapshotGoSymbolBinding(symbol *goSymbol) goSymbolBindingState { + return goSymbolBindingState{ + shape: symbol.Shape, origin: symbol.Origin, target: symbol.Target, + aliasOf: symbol.AliasOf, contentOf: symbol.ContentOf, + } +} + +func restoreGoSymbolBinding(symbol *goSymbol, state goSymbolBindingState) { + symbol.Shape = state.shape + symbol.Origin = state.origin + symbol.Target = state.target + symbol.AliasOf = state.aliasOf + symbol.ContentOf = state.contentOf +} + +func cloneGoSymbolBindings(symbols map[int]*goSymbol) map[int]goSymbolBindingState { + bindings := make(map[int]goSymbolBindingState, len(symbols)) + for id, symbol := range symbols { + bindings[id] = snapshotGoSymbolBinding(symbol) + } + return bindings +} + +func cloneGoSymbolBindingStates(states map[int]goSymbolBindingState) map[int]goSymbolBindingState { + clone := make(map[int]goSymbolBindingState, len(states)) + for id, state := range states { + clone[id] = state + } + return clone +} + +func goSymbolBindingStatesEqual(first goSymbolBindingState, second goSymbolBindingState) bool { + return goReferenceShapesEqual(first.shape, second.shape) && + first.origin == second.origin && first.target == second.target && + first.aliasOf == second.aliasOf && first.contentOf == second.contentOf +} + +func (resolver *goMutationResolver) branchState() goBranchState { + return goBranchState{ + escapes: cloneGoEscapeState(resolver.escapedAt), + fields: cloneGoFieldBindings(resolver.fields), + symbols: cloneGoSymbolBindings(resolver.symbols), + } +} + +func (resolver *goMutationResolver) restoreBranchState(state goBranchState) { + resolver.escapedAt = cloneGoEscapeState(state.escapes) + resolver.fields = cloneGoFieldBindings(state.fields) + for id, binding := range state.symbols { + if symbol := resolver.symbols[id]; symbol != nil { + restoreGoSymbolBinding(symbol, binding) + } + } +} + +func (resolver *goMutationResolver) mergeGoBranchStates(before goBranchState, states ...goBranchState) goBranchState { + if len(states) == 0 { + return before + } + merged := goBranchState{ + escapes: cloneGoEscapeState(states[0].escapes), + fields: cloneGoFieldBindings(states[0].fields), + symbols: cloneGoSymbolBindingStates(before.symbols), + } + for _, state := range states[1:] { + merged.escapes = intersectGoEscapeStates(merged.escapes, state.escapes) + merged.fields = resolver.intersectGoFieldBindings(merged.fields, state.fields) + } + for id := range before.symbols { + candidate, exists := states[0].symbols[id] + if !exists { + continue + } + equivalent := true + for _, state := range states[1:] { + other, ok := state.symbols[id] + if !ok || !goSymbolBindingStatesEqual(candidate, other) { + equivalent = false + break + } + } + if equivalent { + merged.symbols[id] = candidate + } + } + return merged +} + +func (resolver *goMutationResolver) walkExclusiveBranches(scope *goScope, statements []ast.Stmt, exhaustive bool) { + before := resolver.branchState() + states := make([]goBranchState, 0, len(statements)+1) + for _, statement := range statements { + resolver.restoreBranchState(before) + resolver.walkStatement(scope, statement) + states = append(states, resolver.branchState()) + } + if !exhaustive || len(states) == 0 { + states = append(states, before) + } + resolver.restoreBranchState(resolver.mergeGoBranchStates(before, states...)) +} + +func goCaseFallsThrough(statement ast.Stmt) bool { + clause, ok := statement.(*ast.CaseClause) + if !ok || len(clause.Body) == 0 { + return false + } + branch, ok := clause.Body[len(clause.Body)-1].(*ast.BranchStmt) + return ok && branch.Tok == token.FALLTHROUGH +} + +func (resolver *goMutationResolver) walkSwitchBranches(scope *goScope, statements []ast.Stmt, exhaustive bool) { + before := resolver.branchState() + states := make([]goBranchState, 0, len(statements)+1) + for entry := range statements { + resolver.restoreBranchState(before) + resolver.walkStatement(scope, statements[entry]) + for current := entry; goCaseFallsThrough(statements[current]) && current+1 < len(statements); current++ { + next, ok := statements[current+1].(*ast.CaseClause) + if !ok { + break + } + resolver.walkStatements(resolver.newScope(scope), next.Body) + } + states = append(states, resolver.branchState()) + } + if !exhaustive || len(states) == 0 { + states = append(states, before) + } + resolver.restoreBranchState(resolver.mergeGoBranchStates(before, states...)) +} + +func goBranchesHaveDefault(statements []ast.Stmt) bool { + for _, statement := range statements { + switch clause := statement.(type) { + case *ast.CaseClause: + if len(clause.List) == 0 { + return true + } + case *ast.CommClause: + if clause.Comm == nil { + return true + } + } + } + return false +} diff --git a/internal/codeguard/checks/quality/quality_effect_evidence_go_execution.go b/internal/codeguard/checks/quality/quality_effect_evidence_go_execution.go new file mode 100644 index 00000000..338d18c1 --- /dev/null +++ b/internal/codeguard/checks/quality/quality_effect_evidence_go_execution.go @@ -0,0 +1,465 @@ +package quality + +import ( + "go/ast" + "go/token" +) + +func (resolver *goMutationResolver) walkStatement(scope *goScope, statement ast.Stmt) { + resolver.order++ + switch value := statement.(type) { + case *ast.BlockStmt: + resolver.walkStatements(resolver.newScope(scope), value.List) + case *ast.DeclStmt: + resolver.handleDeclaration(scope, value.Decl) + case *ast.AssignStmt: + for _, expression := range value.Rhs { + resolver.walkExpressionCalls(scope, expression) + } + line := resolver.line(value.Pos()) + if value.Tok != token.DEFINE { + for _, expression := range value.Lhs { + switch expression.(type) { + case *ast.SelectorExpr, *ast.IndexExpr, *ast.StarExpr: + resolver.recordAssignmentMutation(scope, expression, line, resolver.expressionRootName(expression)) + case *ast.Ident: + resolver.recordAssignmentMutation(scope, expression, line, resolver.expressionRootName(expression)) + } + } + } + resolver.recordEscapes(scope, value) + resolver.bindAssignment(scope, value) + case *ast.IncDecStmt: + resolver.recordAssignmentMutation(scope, value.X, resolver.line(value.Pos()), resolver.expressionRootName(value.X)) + case *ast.ExprStmt: + resolver.walkExpressionCalls(scope, value.X) + case *ast.ReturnStmt: + for _, expression := range value.Results { + resolver.walkExpressionCalls(scope, expression) + } + case *ast.IfStmt: + control := resolver.newScope(scope) + if value.Init != nil { + resolver.walkStatement(control, value.Init) + } + resolver.walkExpressionCalls(control, value.Cond) + beforeBranches := resolver.branchState() + resolver.restoreBranchState(beforeBranches) + resolver.walkStatements(resolver.newScope(control), value.Body.List) + thenState := resolver.branchState() + resolver.restoreBranchState(beforeBranches) + if value.Else != nil { + resolver.walkStatement(control, value.Else) + } + resolver.restoreBranchState(resolver.mergeGoBranchStates(beforeBranches, thenState, resolver.branchState())) + case *ast.ForStmt: + control := resolver.newScope(scope) + if value.Init != nil { + resolver.walkStatement(control, value.Init) + } + resolver.walkExpressionCalls(control, value.Cond) + beforeLoop := resolver.branchState() + resolver.walkStatements(resolver.newScope(control), value.Body.List) + if value.Post != nil { + resolver.walkStatement(control, value.Post) + } + resolver.restoreBranchState(beforeLoop) + case *ast.RangeStmt: + control := resolver.newScope(scope) + resolver.walkExpressionCalls(control, value.X) + beforeLoop := resolver.branchState() + rangeBindings := make(map[*goSymbol]goSymbolBindingState) + if value.Tok != token.DEFINE { + for _, expression := range []ast.Expr{value.Key, value.Value} { + if identifier, ok := expression.(*ast.Ident); ok { + if symbol := control.lookup(identifier.Name); symbol != nil { + rangeBindings[symbol] = snapshotGoSymbolBinding(symbol) + } + } + } + } + resolver.bindRange(control, value) + resolver.walkStatements(resolver.newScope(control), value.Body.List) + for symbol, binding := range rangeBindings { + restoreGoSymbolBinding(symbol, binding) + } + resolver.restoreBranchState(beforeLoop) + case *ast.SwitchStmt: + control := resolver.newScope(scope) + if value.Init != nil { + resolver.walkStatement(control, value.Init) + } + resolver.walkExpressionCalls(control, value.Tag) + resolver.walkSwitchBranches(control, value.Body.List, goBranchesHaveDefault(value.Body.List)) + case *ast.TypeSwitchStmt: + control := resolver.newScope(scope) + if value.Init != nil { + resolver.walkStatement(control, value.Init) + } + if value.Assign != nil { + resolver.walkStatement(control, value.Assign) + } + resolver.walkExclusiveBranches(control, value.Body.List, goBranchesHaveDefault(value.Body.List)) + case *ast.SelectStmt: + control := resolver.newScope(scope) + resolver.walkExclusiveBranches(control, value.Body.List, goBranchesHaveDefault(value.Body.List)) + case *ast.CaseClause: + child := resolver.newScope(scope) + for _, expression := range value.List { + resolver.walkExpressionCalls(child, expression) + } + resolver.walkStatements(child, value.Body) + case *ast.CommClause: + child := resolver.newScope(scope) + if value.Comm != nil { + resolver.walkStatement(child, value.Comm) + } + resolver.walkStatements(child, value.Body) + case *ast.DeferStmt: + resolver.walkCall(scope, value.Call) + case *ast.GoStmt: + resolver.walkCall(scope, value.Call) + case *ast.LabeledStmt: + resolver.walkStatement(scope, value.Stmt) + case *ast.SendStmt: + resolver.walkExpressionCalls(scope, value.Chan) + resolver.walkExpressionCalls(scope, value.Value) + } +} + +func (resolver *goMutationResolver) handleDeclaration(scope *goScope, declaration ast.Decl) { + general, ok := declaration.(*ast.GenDecl) + if !ok { + return + } + for _, rawSpec := range general.Specs { + spec, ok := rawSpec.(*ast.ValueSpec) + if !ok { + continue + } + for _, expression := range spec.Values { + resolver.walkExpressionCalls(scope, expression) + } + for position, name := range spec.Names { + var initial ast.Expr + if position < len(spec.Values) { + initial = spec.Values[position] + } else if len(spec.Values) == 1 { + initial = spec.Values[0] + } + shape := resolver.typeShape(spec.Type, nil) + if spec.Type == nil { + shape = resolver.expressionShape(scope, initial) + } + symbol := resolver.declare(scope, name.Name, goSymbolLocal, shape, originLocal, targetLocal, resolver.line(name.Pos())) + resolver.setInitializer(scope, symbol, initial) + } + } +} + +func (resolver *goMutationResolver) bindAssignment(scope *goScope, assignment *ast.AssignStmt) { + for position, lhs := range assignment.Lhs { + var rhs ast.Expr + if position < len(assignment.Rhs) { + rhs = assignment.Rhs[position] + } else if len(assignment.Rhs) == 1 { + rhs = assignment.Rhs[0] + } + if selector, ok := lhs.(*ast.SelectorExpr); ok { + if assignment.Tok == token.ASSIGN { + resolver.bindSelectorField(scope, selector, rhs) + } + continue + } + identifier, ok := lhs.(*ast.Ident) + if !ok || identifier.Name == "_" { + continue + } + if assignment.Tok == token.DEFINE { + if scope.symbols[identifier.Name] != nil { + resolver.setInitializer(scope, scope.symbols[identifier.Name], rhs) + continue + } + shape := resolver.expressionShape(scope, rhs) + origin := originLocal + if _, ok := rhs.(*ast.CallExpr); ok && shape.Kind == goShapeUnknown { + origin = originUnknown + } + symbol := resolver.declare(scope, identifier.Name, goSymbolLocal, shape, origin, targetLocal, resolver.line(identifier.Pos())) + resolver.setInitializer(scope, symbol, rhs) + continue + } + if symbol := scope.lookup(identifier.Name); symbol != nil && symbol.Kind != goSymbolGlobal { + resolver.setInitializer(scope, symbol, rhs) + } + } +} + +func (resolver *goMutationResolver) bindSelectorField(scope *goScope, selector *ast.SelectorExpr, expression ast.Expr) { + if expression == nil { + return + } + path := resolver.resolveExpression(scope, selector) + if path.bindingKey == "" || path.packageName || path.crossedReference || path.symbol == nil { + return + } + owner := resolver.resolveAlias(path.symbol) + if owner == nil || owner.Kind == goSymbolGlobal || owner.Kind == goSymbolImport { + return + } + resolver.nextID++ + binding := &goSymbol{ + ID: resolver.nextID, + Name: selector.Sel.Name, + Kind: goSymbolLocal, + Scope: path.symbol.Scope, + Shape: path.shape, + Origin: originLocal, + Target: targetLocal, + DeclLine: resolver.line(selector.Pos()), + } + resolver.symbols[binding.ID] = binding + resolver.setInitializer(scope, binding, expression) + resolver.fields[path.bindingKey] = binding +} + +func (resolver *goMutationResolver) setInitializer(scope *goScope, symbol *goSymbol, expression ast.Expr) { + if symbol == nil || expression == nil { + return + } + path := resolver.resolveExpression(scope, expression) + shape := resolver.expressionShape(scope, expression) + aliasSource := resolver.resolveAlias(path.symbol) + contentSource := resolver.resolveContentOwner(path.symbol) + if shape.Kind != goShapeUnknown { + symbol.Shape = shape + } + symbol.AliasOf = 0 + symbol.ContentOf = 0 + delete(resolver.closures, symbol.ID) + if literal, ok := expression.(*ast.FuncLit); ok { + resolver.closures[symbol.ID] = goClosureDefinition{literal: literal, scope: snapshotGoScope(scope)} + symbol.Origin = originLocal + return + } + if path.symbol != nil { + if definition, ok := resolver.closures[path.symbol.ID]; ok { + resolver.closures[symbol.ID] = definition + } + } + if path.symbol != nil && path.symbol != symbol { + if shape.referenceBacked() { + if aliasSource != nil { + symbol.AliasOf = aliasSource.ID + } else { + symbol.AliasOf = path.symbol.ID + } + symbol.Origin = originLocal + return + } + if contentSource != nil { + symbol.ContentOf = contentSource.ID + } else { + symbol.ContentOf = path.symbol.ID + } + symbol.Origin = originLocal + return + } + if conversion, ok := expression.(*ast.CallExpr); ok { + if conversionShape, isConversion := resolver.conversionShape(scope, conversion); isConversion && conversionShape.referenceBacked() && !resolver.conversionAllocatesFreshStorage(scope, conversion, conversionShape) { + symbol.Origin = originUnknown + return + } + } + if _, ok := expression.(*ast.CallExpr); ok && shape.Kind == goShapeUnknown { + symbol.Origin = originUnknown + } else { + symbol.Origin = originLocal + } +} + +func (resolver *goMutationResolver) bindRange(scope *goScope, statement *ast.RangeStmt) { + shape := resolver.expressionShape(scope, statement.X) + source := resolver.resolveExpression(scope, statement.X) + valueShape := goReferenceShape{Kind: goShapeUnknown} + if shape.Elem != nil { + valueShape = *shape.Elem + } + bind := func(expression ast.Expr, itemShape goReferenceShape, inheritsContent bool) { + identifier, ok := expression.(*ast.Ident) + if !ok || identifier.Name == "_" { + return + } + var symbol *goSymbol + if statement.Tok == token.DEFINE { + symbol = resolver.declare(scope, identifier.Name, goSymbolRange, itemShape, originLocal, targetLocal, resolver.line(identifier.Pos())) + } else { + symbol = scope.lookup(identifier.Name) + if symbol == nil || symbol.Kind == goSymbolGlobal || symbol.Kind == goSymbolImport { + return + } + symbol.Shape = itemShape + symbol.Origin = originLocal + symbol.Target = targetLocal + symbol.AliasOf = 0 + symbol.ContentOf = 0 + } + if inheritsContent && source.symbol != nil { + if itemShape.referenceBacked() { + symbol.AliasOf = source.symbol.ID + } else { + symbol.ContentOf = source.symbol.ID + } + } + } + bind(statement.Key, goReferenceShape{Kind: goShapeValue}, false) + bind(statement.Value, valueShape, true) +} + +func (resolver *goMutationResolver) recordEscapes(scope *goScope, assignment *ast.AssignStmt) { + if assignment.Tok != token.ASSIGN { + return + } + for position, lhs := range assignment.Lhs { + if position >= len(assignment.Rhs) { + break + } + rhsPath := resolver.resolveExpression(scope, assignment.Rhs[position]) + local := resolver.resolveAlias(rhsPath.symbol) + if local == nil || local.Origin != originLocal || !local.Shape.referenceBacked() { + continue + } + lhsPath := resolver.resolveExpression(scope, lhs) + external := lhsPath.symbol != nil && lhsPath.symbol.Kind == goSymbolGlobal + if !external { + _, _, external = resolver.ownership(lhsPath, false) + } + if external { + resolver.escapedAt[local.ID] = resolver.order + } + } +} + +func (resolver *goMutationResolver) walkExpressionCalls(scope *goScope, expression ast.Expr) { + if expression == nil { + return + } + ast.Inspect(expression, func(node ast.Node) bool { + switch value := node.(type) { + case *ast.FuncLit: + return false + case *ast.CallExpr: + resolver.walkCall(scope, value) + return false + } + return true + }) +} + +func (resolver *goMutationResolver) walkCall(scope *goScope, call *ast.CallExpr) { + for _, argument := range call.Args { + resolver.walkExpressionCalls(scope, argument) + } + if literal, ok := call.Fun.(*ast.FuncLit); ok { + resolver.executeClosure(goClosureDefinition{literal: literal, scope: snapshotGoScope(scope)}, scope, call.Args, 0) + return + } + line := resolver.line(call.Pos()) + if identifier, ok := call.Fun.(*ast.Ident); ok { + if _, conversion := resolver.conversionShape(scope, call); conversion { + return + } + if symbol := scope.lookup(identifier.Name); symbol != nil { + if definition, exists := resolver.closures[symbol.ID]; exists { + resolver.executeClosure(definition, scope, call.Args, symbol.ID) + } + return + } + if resolver.recordBuiltinMutation(scope, identifier.Name, call.Args, line) { + return + } + if mutatingCallPattern.MatchString(identifier.Name) { + resolver.addUnresolved(line, "call", identifier.Name, "call target declaration could not be resolved") + } + return + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return + } + path := resolver.resolveExpression(scope, selector.X) + if path.packageName { + return + } + callee := goCallName(call.Fun) + effect := observableCallEffect(callee) + if effect == "" { + if !mutatingCallPattern.MatchString(callee) { + return + } + effect = "shared_state" + } + target, origin, reportable := resolver.ownership(path, true) + if reportable { + resolver.addMutation(target, effect, origin, line, callee) + return + } + if path.symbol == nil || path.symbol.Origin == originUnknown { + resolver.addUnresolved(line, "call", firstNonEmptyString(path.unresolved, resolver.expressionRootName(selector.X)), "call target ownership could not be resolved") + } +} + +func (resolver *goMutationResolver) executeClosure(definition goClosureDefinition, callScope *goScope, arguments []ast.Expr, symbolID int) { + if definition.literal == nil || definition.scope == nil { + return + } + if symbolID != 0 { + if resolver.active[symbolID] { + return + } + resolver.active[symbolID] = true + defer delete(resolver.active, symbolID) + } + closure := resolver.newScope(definition.scope) + resolver.declareClosureParameters(callScope, closure, definition.literal.Type.Params, arguments) + resolver.declareFieldList(closure, definition.literal.Type.Results, goSymbolResult, originLocal, targetLocal) + resolver.walkStatements(closure, definition.literal.Body.List) +} + +func (resolver *goMutationResolver) declareClosureParameters(outer *goScope, closure *goScope, fields *ast.FieldList, arguments []ast.Expr) { + if fields == nil { + return + } + argumentIndex := 0 + for _, field := range fields.List { + shape := resolver.typeShape(field.Type, nil) + for _, name := range field.Names { + symbol := resolver.declare(closure, name.Name, goSymbolParameter, shape, originLocal, targetLocal, resolver.line(name.Pos())) + if argumentIndex < len(arguments) { + resolver.setInitializer(outer, symbol, arguments[argumentIndex]) + } + argumentIndex++ + } + } +} + +func (resolver *goMutationResolver) recordBuiltinMutation(scope *goScope, name string, arguments []ast.Expr, line int) bool { + mutatesFirstArgument := name == "append" || name == "copy" || name == "delete" || name == "clear" + if !mutatesFirstArgument { + return name == "make" || name == "new" || name == "len" || name == "cap" || name == "close" || name == "panic" || name == "recover" || name == "complex" || name == "real" || name == "imag" || name == "print" || name == "println" + } + if len(arguments) == 0 { + return true + } + path := resolver.resolveExpression(scope, arguments[0]) + if path.shape.referenceBacked() { + path.crossedReference = true + } + target, origin, reportable := resolver.ownership(path, false) + if reportable { + resolver.addMutation(target, "shared_state", origin, line, name) + } else if path.symbol == nil || path.symbol.Origin == originUnknown { + resolver.addUnresolved(line, "call", firstNonEmptyString(path.unresolved, resolver.expressionRootName(arguments[0])), "built-in mutation target could not be resolved") + } + return true +} diff --git a/internal/codeguard/checks/quality/quality_effect_evidence_go_test.go b/internal/codeguard/checks/quality/quality_effect_evidence_go_test.go new file mode 100644 index 00000000..6a6b1652 --- /dev/null +++ b/internal/codeguard/checks/quality/quality_effect_evidence_go_test.go @@ -0,0 +1,879 @@ +package quality + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "testing" +) + +func TestGoScopeOriginResolutionIgnoresSyntaxPackagesAndShadows(t *testing.T) { + tests := []struct { + name string + source string + }{ + { + name: "default import", + source: `package sample +import "fmt" +func ReadValue() int { fmt.Set(); return 1 }`, + }, + { + name: "aliased import", + source: `package sample +import storage "example.com/storage" +func ReadValue() int { storage.Update(); return 1 }`, + }, + { + name: "control flow keywords", + source: `package sample +type State struct{ Value int } +func ReadValue() int { + for index := 0; index < 1; index++ { + if true { local := &State{}; local.Value = index } + } + return 1 +}`, + }, + { + name: "local shadows global", + source: `package sample +type State struct{ Value int } +var state *State +func ReadValue() int { state := &State{}; state.Value = 1; return state.Value }`, + }, + { + name: "nested block shadows global", + source: `package sample +type State struct{ Value int } +var state *State +func ReadValue() int { + if true { state := &State{}; state.Value = 1 } + return 1 +}`, + }, + { + name: "initializer scope shadows global", + source: `package sample +type State struct{ Value int } +var state *State +func ReadValue() int { + if state := (&State{}); state != nil { state.Value = 1 } + return 1 +}`, + }, + { + name: "closure local shadows global", + source: `package sample +type State struct{ Value int } +var state *State +func ReadValue() int { + func() { state := &State{}; state.Value = 1 }() + return 1 +}`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, test.source, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want no mutation or unresolved evidence", analysis) + } + }) + } +} + +func TestGoScopeOriginResolutionDoesNotTreatImportedMutatorAsHiddenSideEffect(t *testing.T) { + source := `package sample +import storage "example.com/storage" +func LookupValue() int { storage.Update(); return 1 }` + fset := token.NewFileSet() + parsed, err := parser.ParseFile(fset, "fixture.go", source, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + index := newGoPackageIndex() + index.addFile("fixture.go", fset, parsed) + fn := goPrecisionFunctionByNameForTest(t, fset, parsed, []byte(source), "LookupValue") + fn.GoFile = "fixture.go" + fn.GoPackage = index.packageFor("fixture.go", parsed.Name.Name) + if hiddenSideEffect("fixture.go", fn) { + t.Fatal("imported package call was classified as a hidden side effect") + } +} + +func TestGoStructuralOriginFixtureUnresolvedEvidenceIsIntentional(t *testing.T) { + path := filepath.Join("..", "..", "..", "..", "tests", "checks", "testdata", "structural_origin", "go", "helpers.go") + source, err := os.ReadFile(path) //nolint:gosec // checked-in regression fixture path + if err != nil { + t.Fatal(err) + } + fset := token.NewFileSet() + parsed, err := parser.ParseFile(fset, path, source, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + index := newGoPackageIndex() + index.addFile(path, fset, parsed) + for _, declaration := range parsed.Decls { + fnDecl, ok := declaration.(*ast.FuncDecl) + if !ok { + continue + } + fn := goPrecisionFunction(fset, fnDecl, source) + fn.GoPackage = index.packageFor(path, parsed.Name.Name) + analysis := goFunctionMutationEvidence(fn) + if fnDecl.Name.Name == "CurrentUnresolvedGo" { + if len(analysis.Unresolved) != 1 { + t.Fatalf("%s unresolved = %#v, want one exact record", fnDecl.Name.Name, analysis.Unresolved) + } + evidence := analysis.Unresolved[0] + if evidence.Language != "go" || evidence.Operation != "call" || evidence.Symbol != "mystery" || + evidence.Reason != "call target ownership could not be resolved" || evidence.Line <= 0 { + t.Fatalf("unresolved = %#v, want go/call/mystery with exact reason and line", evidence) + } + continue + } + if len(analysis.Unresolved) != 0 { + t.Fatalf("%s unresolved = %#v, want only CurrentUnresolvedGo to be incomplete", fnDecl.Name.Name, analysis.Unresolved) + } + } +} + +func TestGoFreshSliceCopyOwnershipIsLocal(t *testing.T) { + for name, initializer := range map[string]string{ + "nil slice append copy": "append([]Item(nil), input...)", + "empty slice append copy": "append([]Item{}, input...)", + "made slice append copy": "append(make([]Item, 0, len(input)), input...)", + } { + t.Run(name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Item struct{ Value int } +func Current(input []Item) int { + copied := `+initializer+` + copied[0].Value = 1 + return copied[0].Value +}`, "Current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want proven fresh local slice", analysis) + } + }) + } +} + +func TestGoAppendReusingCallerBackingArrayPreservesOwnership(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Item struct{ Value int } +func Current(input []Item) int { + reused := append(input[:0], input...) + reused[0].Value = 1 + return reused[0].Value +}`, "Current") + assertGoMutation(t, analysis, targetArgument, originCaller) + if len(analysis.Unresolved) != 0 { + t.Fatalf("unresolved = %#v, want caller ownership resolved", analysis.Unresolved) + } +} + +func TestGoReferenceBackedConversionsPreserveCallerOwnership(t *testing.T) { + tests := map[string]string{ + "unnamed slice conversion": `package sample +type Item struct{ Value int } +func Current(input []Item) int { + copied := []Item(input) + copied[0].Value = 1 + return copied[0].Value +}`, + "named slice conversion": `package sample +type Item struct{ Value int } +type Set []Item +func Current(input []Item) int { + copied := Set(input) + copied[0].Value = 1 + return copied[0].Value +}`, + "unnamed map conversion": `package sample +type Item struct{ Value int } +func Current(input map[string]*Item) int { + copied := map[string]*Item(input) + copied["item"].Value = 1 + return copied["item"].Value +}`, + "named map conversion": `package sample +type Item struct{ Value int } +type ItemsByName map[string]*Item +func Current(input map[string]*Item) int { + copied := ItemsByName(input) + copied["item"].Value = 1 + return copied["item"].Value +}`, + "indexed generic slice conversion": `package sample +type Item struct{ Value int } +type Set[T any] []T +func Current(input []Item) int { + copied := Set[Item](input) + copied[0].Value = 1 + return copied[0].Value +}`, + "multi-parameter generic map conversion": `package sample +type Item struct{ Value int } +type ItemsByKey[K comparable, V any] map[K]V +func Current(input map[string]*Item) int { + copied := ItemsByKey[string, *Item](input) + copied["item"].Value = 1 + return copied["item"].Value +}`, + } + for name, source := range tests { + t.Run(name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, source, "Current") + assertGoMutation(t, analysis, targetArgument, originCaller) + }) + } +} + +func TestGoStringToByteAndRuneSliceConversionsAllocateFreshStorage(t *testing.T) { + tests := map[string]string{ + "byte slice": `package sample +func Current(inputString string) byte { + copied := []byte(inputString) + copied[0] = 'x' + return copied[0] +}`, + "rune slice": `package sample +func Current(inputString string) rune { + copied := []rune(inputString) + copied[0] = 'x' + return copied[0] +}`, + "named string to byte slice": `package sample +type Text string +func Current(inputString Text) byte { + copied := []byte(inputString) + copied[0] = 'x' + return copied[0] +}`, + } + for name, source := range tests { + t.Run(name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, source, "Current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want freshly allocated byte/rune backing storage", analysis) + } + }) + } +} + +func TestGoFreshReferenceBackedConversionsRemainLocal(t *testing.T) { + tests := map[string]string{ + "unnamed nil slice": `package sample +type Item struct{ Value int } +func Current() int { + copied := []Item(nil) + copied = append(copied, Item{}) + copied[0].Value = 1 + return copied[0].Value +}`, + "named nil slice": `package sample +type Item struct{ Value int } +type Set []Item +func Current() int { + copied := Set(nil) + copied = append(copied, Item{}) + copied[0].Value = 1 + return copied[0].Value +}`, + "unnamed nil map": `package sample +type Item struct{ Value int } +func Current() int { + copied := map[string]*Item(nil) + copied["item"] = &Item{} + return 1 +}`, + "named literal map": `package sample +type Item struct{ Value int } +type ItemsByName map[string]*Item +func Current() int { + copied := ItemsByName(map[string]*Item{}) + copied["item"] = &Item{} + return 1 +}`, + } + for name, source := range tests { + t.Run(name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, source, "Current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want fresh local conversion", analysis) + } + }) + } +} + +func TestGoReferenceBackedConversionWithUnknownOperandRemainsUnresolved(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Item struct{ Value int } +type Items []Item +func Current() int { + copied := Items(loadItems()) + copied[0].Value = 1 + return copied[0].Value +}`, "Current") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 1 { + t.Fatalf("analysis = %#v, want one conservative unresolved assignment", analysis) + } + evidence := analysis.Unresolved[0] + if evidence.Operation != "assignment" || evidence.Symbol != "copied" || evidence.Reason != "symbol ownership or reference shape could not be resolved" { + t.Fatalf("unresolved = %#v, want exact converted operand diagnostic", evidence) + } +} + +func TestGoClosureOriginResolutionUsesCapturedDeclaration(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type State struct{ Value int } +func ReadValue(input *State) int { + mutate := func() { input.Value = 1 } + mutate() + return input.Value +}`, "ReadValue") + assertGoMutation(t, analysis, targetArgument, originCaller) +} + +func TestGoClosureOriginResolutionUsesInvocationArgumentOwnership(t *testing.T) { + tests := []struct { + name string + source string + wantTarget string + }{ + { + name: "local pointer argument remains local", + source: `package sample +type State struct{ Value int } +func ReadValue() int { + local := &State{} + func(state *State) { state.Value = 1 }(local) + return local.Value +}`, + }, + { + name: "caller pointer argument remains caller owned", + source: `package sample +type State struct{ Value int } +func ReadValue(input *State) int { + func(state *State) { state.Value = 1 }(input) + return input.Value +}`, + wantTarget: targetArgument, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, test.source, "ReadValue") + if test.wantTarget == "" { + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want closure-local mutation", analysis) + } + return + } + assertGoMutation(t, analysis, test.wantTarget, originCaller) + }) + } +} + +func TestGoScopeOriginResolutionRetainsUnresolvedWithoutMutation(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +func ReadValue() int { mystery.Value = 1; return mystery.Value }`, "ReadValue") + assertOnlyUnresolvedMutation(t, analysis, "go", "mystery", "assignment") +} + +func TestGoPlainIdentifierReassignmentUsesResolvedBindingOwnership(t *testing.T) { + tests := []struct { + name string + body string + wantTarget string + unresolved string + }{ + {name: "global replacement", body: `shared = replacement`, wantTarget: targetGlobal}, + {name: "global nil", body: `shared = nil`, wantTarget: targetGlobal}, + {name: "local replacement", body: `local := &Node{}; local = replacement`}, + {name: "unknown replacement", body: `mystery = replacement`, unresolved: "mystery"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +var shared *Node +func ReadValue(replacement *Node) int { `+test.body+`; return 1 }`, "ReadValue") + switch { + case test.wantTarget != "": + assertGoMutation(t, analysis, test.wantTarget, originShared) + case test.unresolved != "": + assertOnlyUnresolvedMutation(t, analysis, "go", test.unresolved, "assignment") + case len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0: + t.Fatalf("analysis = %#v, want local binding reassignment only", analysis) + } + }) + } +} + +func TestGoReferenceShapeOriginResolution(t *testing.T) { + declarations := `package sample +type Node struct{ Name string } +type Payload struct { + Name string + Tags []string + Meta map[string]string + Node *Node + Any any +} +` + tests := []struct { + name string + body string + wantTarget string + wantOrigin string + }{ + {name: "value field reassignment is local", body: `func ReadValue(input Payload) string { input.Name = "local"; return input.Name }`}, + {name: "map field index reaches caller content", body: `func ReadValue(input Payload) string { input.Meta["status"] = "ready"; return input.Meta["status"] }`, wantTarget: targetArgument, wantOrigin: originCaller}, + {name: "slice field index reaches caller content", body: `func ReadValue(input Payload) string { input.Tags[0] = "ready"; return input.Tags[0] }`, wantTarget: targetArgument, wantOrigin: originCaller}, + {name: "pointer field reaches caller pointee", body: `func ReadValue(input Payload) string { input.Node.Name = "ready"; return input.Node.Name }`, wantTarget: targetArgument, wantOrigin: originCaller}, + {name: "asserted interface map reaches caller content", body: `func ReadValue(input Payload) string { input.Any.(map[string]string)["status"] = "ready"; return "ready" }`, wantTarget: targetArgument, wantOrigin: originCaller}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, declarations+test.body, "ReadValue") + if test.wantTarget == "" { + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want local-only mutation", analysis) + } + return + } + assertGoMutation(t, analysis, test.wantTarget, test.wantOrigin) + }) + } +} + +func TestGoConstructionShapedMethodsFollowResolvedOwnership(t *testing.T) { + caller := parseGoMutationAnalysisForTest(t, `package sample +type Builder struct{} +func ReadValue(input *Builder, builder *Builder) int { + input.SetName("caller") + builder.Save() + input.WithValue(1) + return 1 +}`, "ReadValue") + assertGoMutation(t, caller, targetArgument, originCaller) + wantDetails := map[string]bool{"input.SetName": false, "builder.Save": false, "input.WithValue": false} + for _, mutation := range caller.Mutations { + if _, expected := wantDetails[mutation.Detail]; expected { + wantDetails[mutation.Detail] = true + } + } + for detail, found := range wantDetails { + if !found { + t.Fatalf("mutations = %#v, want caller-owned %s evidence", caller.Mutations, detail) + } + } + + local := parseGoMutationAnalysisForTest(t, `package sample +type Builder struct{} +func ReadValue() int { + builder := &Builder{} + builder.SetName("local") + builder.Save() + builder.WithValue(1) + return 1 +}`, "ReadValue") + if len(local.Mutations) != 0 || len(local.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want fresh local builder operations suppressed by ownership", local) + } +} + +func TestGoReferenceFieldReassignmentUpdatesContentOwnership(t *testing.T) { + declarations := `package sample +type Node struct{ Name string } +type Payload struct { + Tags []string + Meta map[string]string + Node *Node + Any any +} +` + tests := []struct { + name string + body string + }{ + {name: "map field", body: `input.Meta = make(map[string]string); input.Meta["status"] = "local"`}, + {name: "slice field", body: `input.Tags = make([]string, 1); input.Tags[0] = "local"`}, + {name: "pointer field", body: `input.Node = &Node{}; input.Node.Name = "local"`}, + {name: "interface field", body: `input.Any = map[string]string{}; input.Any.(map[string]string)["status"] = "local"`}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, declarations+`func ReadValue(input Payload) string { `+test.body+`; return "ok" }`, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want reassigned local field content to remain local", analysis) + } + }) + } +} + +func TestGoReferenceFieldReassignmentCanRetainCallerOwnership(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Payload struct{ Meta map[string]string } +func ReadValue(input Payload, replacement map[string]string) string { + input.Meta = replacement + input.Meta["status"] = "ready" + return input.Meta["status"] +}`, "ReadValue") + assertGoMutation(t, analysis, targetArgument, originCaller) +} + +func TestGoReferenceFieldReassignmentDoesNotLeakAcrossBranches(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Payload struct{ Meta map[string]string } +func ReadValue(input Payload, flag bool) string { + if flag { input.Meta = make(map[string]string) } else { input.Meta["status"] = "ready" } + return "ok" +}`, "ReadValue") + assertGoMutation(t, analysis, targetArgument, originCaller) +} + +func TestGoReferenceFieldReassignmentMergesEquivalentLocalBranches(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Payload struct{ Meta map[string]string } +func ReadValue(input Payload, flag bool) string { + if flag { + input.Meta = make(map[string]string) + } else { + input.Meta = map[string]string{} + } + input.Meta["status"] = "local" + return input.Meta["status"] +}`, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want independently reassigned local field content to remain local", analysis) + } +} + +func TestGoAliasOriginResolutionFollowsReferenceShapesOnly(t *testing.T) { + tests := []struct { + name string + source string + wantTarget string + }{ + { + name: "pointer aliases", + source: `package sample +type Node struct{ Name string } +func ReadValue(input *Node) string { a := input; b := a; b.Name = "ready"; return b.Name }`, + wantTarget: targetArgument, + }, + { + name: "map aliases", + source: `package sample +func ReadValue(input map[string]string) string { a := input; b := a; b["status"] = "ready"; return b["status"] }`, + wantTarget: targetArgument, + }, + { + name: "slice aliases", + source: `package sample +func ReadValue(input []string) string { a := input; b := a; b[0] = "ready"; return b[0] }`, + wantTarget: targetArgument, + }, + { + name: "range pointer aliases", + source: `package sample +type Node struct{ Name string } +func ReadValue(input []*Node) string { for _, node := range input { node.Name = "ready" }; return input[0].Name }`, + wantTarget: targetArgument, + }, + { + name: "value aliases remain local", + source: `package sample +type Node struct{ Name string } +func ReadValue(input Node) string { a := input; b := a; b.Name = "local"; return b.Name }`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, test.source, "ReadValue") + if test.wantTarget == "" { + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want local-only mutation", analysis) + } + return + } + assertGoMutation(t, analysis, test.wantTarget, originCaller) + }) + } +} + +func TestGoRangeAssignmentUpdatesExistingAlias(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +func ReadValue(input []*Node) int { + var node *Node + for _, node = range input { node.Value = 1 } + return input[0].Value +}`, "ReadValue") + assertGoMutation(t, analysis, targetArgument, originCaller) +} + +func TestGoRangeAssignmentDoesNotLeakAliasAfterOptionalLoop(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +func ReadValue(input []*Node) int { + var node *Node + for _, node = range input {} + node.Value = 1 + return node.Value +}`, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want no alias proof after a possibly empty range", analysis) + } +} + +func TestGoClosureCapturesDeclarationVisibleAtDefinition(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +var shared *Node +func ReadValue() int { + mutate := func() { shared.Value = 1 } + shared := &Node{} + mutate() + return shared.Value +}`, "ReadValue") + assertGoMutation(t, analysis, targetGlobal, originShared) +} + +func TestGoShadowedBuiltinsUseNearestDeclaration(t *testing.T) { + tests := []struct { + name string + source string + }{ + { + name: "append", + source: `package sample +func ReadValue(input []string) int { append := func([]string, string) {}; append(input, "x"); return len(input) }`, + }, + { + name: "copy", + source: `package sample +func ReadValue(input []string) int { copy := func([]string, []string) {}; copy(input, nil); return len(input) }`, + }, + { + name: "delete", + source: `package sample +func ReadValue(input map[string]string) int { delete := func(map[string]string, string) {}; delete(input, "x"); return len(input) }`, + }, + { + name: "clear", + source: `package sample +func ReadValue(input map[string]string) int { clear := func(map[string]string) {}; clear(input); return len(input) }`, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, test.source, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want shadowed callable behavior only", analysis) + } + }) + } +} + +func TestGoBranchEscapeRequiresEveryReachableArm(t *testing.T) { + tests := []struct { + name string + body string + wantTarget string + }{ + { + name: "escape and mutation in exclusive arms", + body: `if flag { shared = local } else { local.Value = 1 }`, + }, + { + name: "optional escape before later mutation", + body: `if flag { shared = local }; local.Value = 1`, + }, + { + name: "switch arms are exclusive", + body: `switch flag { case true: shared = local; default: local.Value = 1 }`, + }, + { + name: "escape in both arms before later mutation", + body: `if flag { shared = local } else { shared = local }; local.Value = 1`, + wantTarget: targetEscaped, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +var shared *Node +func ReadValue(flag bool) int { local := &Node{}; `+test.body+`; return local.Value }`, "ReadValue") + if test.wantTarget == "" { + if len(analysis.Unresolved) != 0 { + t.Fatalf("unresolved = %#v, want resolved global binding evidence", analysis.Unresolved) + } + for _, mutation := range analysis.Mutations { + if mutation.Target == targetEscaped { + t.Fatalf("analysis = %#v, want no cross-branch escape proof", analysis) + } + } + assertGoMutation(t, analysis, targetGlobal, originShared) + return + } + assertGoMutation(t, analysis, test.wantTarget, originShared) + }) + } +} + +func TestGoOrdinaryAliasStateDoesNotLeakAcrossOptionalControlFlow(t *testing.T) { + tests := map[string]string{ + "optional branch": `if flag { alias = input }; alias.Value = 1`, + "optional for loop": `for flag { alias = input; break }; alias.Value = 1`, + "possibly empty range": `for range []int{} { alias = input }; alias.Value = 1`, + } + for name, body := range tests { + t.Run(name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +func ReadValue(input *Node, flag bool) int { + local := &Node{} + alias := local + `+body+` + return alias.Value +}`, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want pre-control-flow local alias state", analysis) + } + }) + } +} + +func TestGoOrdinaryAliasStateMergeIsBranchOrderInvariant(t *testing.T) { + bodies := []string{ + `if flag { alias = input } else { alias = local }`, + `if flag { alias = local } else { alias = input }`, + } + for index, body := range bodies { + t.Run(string(rune('A'+index)), func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +func ReadValue(input *Node, flag bool) int { + local := &Node{} + alias := local + `+body+` + alias.Value = 1 + return alias.Value +}`, "ReadValue") + if len(analysis.Mutations) != 0 || len(analysis.Unresolved) != 0 { + t.Fatalf("analysis = %#v, want ambiguous branch alias to retain pre-branch local state", analysis) + } + }) + } +} + +func TestGoOrdinaryAliasStateMergesWhenEveryBranchProvesCallerOwnership(t *testing.T) { + for name, body := range map[string]string{ + "if": `if flag { alias = first } else { alias = second }`, + "switch": `switch flag { case true: alias = first; default: alias = second }`, + } { + t.Run(name, func(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +func ReadValue(input *Node, flag bool) int { + local := &Node{} + alias := local + first := input + second := input + `+body+` + alias.Value = 1 + return alias.Value +}`, "ReadValue") + assertGoMutation(t, analysis, targetArgument, originCaller) + }) + } +} + +func TestGoSwitchFallthroughPreservesSequentialEscapeState(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +var shared *Node +func ReadValue(flag bool) int { + local := &Node{} + switch flag { + case true: + shared = local + fallthrough + default: + local.Value = 1 + } + return local.Value +}`, "ReadValue") + assertGoMutation(t, analysis, targetEscaped, originShared) +} + +func TestGoReferenceOriginResolutionReportsGlobalBindingAndOnlyPostEscapeContentMutation(t *testing.T) { + analysis := parseGoMutationAnalysisForTest(t, `package sample +type Node struct{ Value int } +var shared *Node +func ReadValue() int { + local := &Node{} + local.Value = 1 + shared = local + local.Value = 2 + return local.Value +}`, "ReadValue") + if len(analysis.Mutations) != 2 { + t.Fatalf("mutations = %#v, want global binding plus post-escape content mutation", analysis.Mutations) + } + assertGoMutation(t, analysis, targetGlobal, originShared) + assertGoMutation(t, analysis, targetEscaped, originShared) +} + +//nolint:unparam // Keeping the function selector explicit makes multi-function resolver fixtures unambiguous. +func parseGoMutationAnalysisForTest(t *testing.T, source string, functionName string) mutationAnalysis { + t.Helper() + fset := token.NewFileSet() + parsed, err := parser.ParseFile(fset, "fixture.go", source, parser.ParseComments) + if err != nil { + t.Fatal(err) + } + index := newGoPackageIndex() + index.addFile("fixture.go", fset, parsed) + fn := goPrecisionFunctionByNameForTest(t, fset, parsed, []byte(source), functionName) + fn.GoPackage = index.packageFor("fixture.go", parsed.Name.Name) + return goFunctionMutationEvidence(fn) +} + +func goPrecisionFunctionByNameForTest(t *testing.T, fset *token.FileSet, parsed *ast.File, source []byte, functionName string) precisionFunction { + t.Helper() + for _, declaration := range parsed.Decls { + fn, ok := declaration.(*ast.FuncDecl) + if ok && fn.Name.Name == functionName { + return goPrecisionFunction(fset, fn, source) + } + } + t.Fatalf("function %q declaration missing", functionName) + return precisionFunction{} +} + +func assertGoMutation(t *testing.T, analysis mutationAnalysis, target string, origin string) { + t.Helper() + if len(analysis.Unresolved) != 0 { + t.Fatalf("unresolved = %#v, want none", analysis.Unresolved) + } + for _, mutation := range analysis.Mutations { + if mutation.Target == target && mutation.Origin == origin && mutation.Effect == "shared_state" { + return + } + } + t.Fatalf("mutations = %#v, want target=%q origin=%q shared-state evidence", analysis.Mutations, target, origin) +} diff --git a/internal/codeguard/checks/quality/quality_effect_evidence_test.go b/internal/codeguard/checks/quality/quality_effect_evidence_test.go index 46a9b317..d90fea26 100644 --- a/internal/codeguard/checks/quality/quality_effect_evidence_test.go +++ b/internal/codeguard/checks/quality/quality_effect_evidence_test.go @@ -1,10 +1,14 @@ package quality import ( + "context" "go/ast" "go/parser" "go/token" "testing" + + "github.com/devr-tools/codeguard/internal/codeguard/checks/support" + "github.com/devr-tools/codeguard/internal/codeguard/core" ) func TestFunctionMutationEvidenceTracksSameLineEscape(t *testing.T) { @@ -26,8 +30,132 @@ func CurrentState() *State { state := &State{}; sharedState = state; state.Value t.Fatal("CurrentState declaration missing") } fn := goPrecisionFunction(fset, declaration, source) + index := newGoPackageIndex() + index.addFile("escape.go", fset, file) + fn.GoFile = "escape.go" + fn.GoPackage = index.packageFor("escape.go", file.Name.Name) evidence, ok := firstReportableMutationEvidence(fn) if !ok || evidence.Target != targetEscaped || evidence.Origin != originShared { t.Fatalf("evidence = %#v, ok=%v; assignments=%#v statements=%#v", evidence, ok, fn.Assignments, fn.Statements) } } + +func TestFunctionMutationAnalysisRetainsUnresolvedGoRootWithoutFindingEvidence(t *testing.T) { + fn := parseGoPrecisionFunctionForTest(t, `package sample +func ReadValue() int { + mystery.Value = 1 + return mystery.Value +}`) + analysis := functionMutationAnalysis(fn, "go") + assertOnlyUnresolvedMutation(t, analysis, "go", "mystery", "assignment") +} + +func TestFunctionMutationAnalysisRetainsUnknownFactoryResultWithoutFindingEvidence(t *testing.T) { + fn := parseGoPrecisionFunctionForTest(t, `package sample +func ReadValue() int { + item := opaqueFactory() + item.Field = 1 + return item.Field +}`) + for idx := range fn.Assignments { + if fn.Assignments[idx].Name == "item" { + fn.Assignments[idx].Expr = "opaqueFactory()" + } + } + fn.ProvenGlobals = map[string]struct{}{"item": {}} + analysis := functionMutationAnalysis(fn, "go") + assertOnlyUnresolvedMutation(t, analysis, "go", "item", "assignment") +} + +func TestFunctionMutationAnalysisRetainsUnresolvedCPPRootWithoutFindingEvidence(t *testing.T) { + parsed := support.ParseCLike(`int readValue() { + mystery->value = 1; + return mystery->value; +}`, support.CLikeCPP) + functions := parsed.AllFunctions() + if len(functions) != 1 { + t.Fatalf("functions = %d, want 1", len(functions)) + } + analysis := functionMutationAnalysis(parsedPrecisionFunction(functions[0]), "c++") + assertOnlyUnresolvedMutation(t, analysis, "c++", "mystery", "assignment") +} + +func parseGoPrecisionFunctionForTest(t *testing.T, source string) precisionFunction { + t.Helper() + data := []byte(source) + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "fixture.go", data, 0) + if err != nil { + t.Fatal(err) + } + for _, item := range file.Decls { + if declaration, ok := item.(*ast.FuncDecl); ok { + return goPrecisionFunction(fset, declaration, data) + } + } + t.Fatal("function declaration missing") + return precisionFunction{} +} + +func assertOnlyUnresolvedMutation(t *testing.T, analysis mutationAnalysis, language string, symbol string, operation string) { + t.Helper() + if len(analysis.Mutations) != 0 { + t.Fatalf("mutation findings evidence = %#v, want none", analysis.Mutations) + } + if len(analysis.Unresolved) != 1 { + t.Fatalf("unresolved evidence = %#v, want one record", analysis.Unresolved) + } + record := analysis.Unresolved[0] + if record.Language != language || record.Symbol != symbol || record.Operation != operation || record.Line <= 0 || record.Reason == "" { + t.Fatalf("unresolved record = %#v, want language=%q symbol=%q operation=%q positive line and reason", record, language, symbol, operation) + } +} + +func TestUnresolvedMutationDiagnosticsAggregateSeparatelyByLanguage(t *testing.T) { + diagnostics := unresolvedMutationDiagnostics([]unresolvedMutationEvidence{ + {Language: "go", Line: 3, Operation: "assignment", Symbol: "first", Reason: "unknown"}, + {Language: "c++", Line: 8, Operation: "assignment", Symbol: "second", Reason: "unknown"}, + {Language: "go", Line: 13, Operation: "call", Symbol: "third", Reason: "unknown"}, + }) + if len(diagnostics) != 2 { + t.Fatalf("diagnostics = %#v, want one per language", diagnostics) + } + wantCounts := map[string]string{"c++": "1", "go": "2"} + for _, diagnostic := range diagnostics { + if diagnostic.ID != "quality.structural-unresolved-symbols" || diagnostic.Level != "info" || diagnostic.Operational { + t.Fatalf("diagnostic = %#v, want non-operational informational structural diagnostic", diagnostic) + } + language := diagnostic.Metadata["language"] + if diagnostic.Metadata["count"] != wantCounts[language] { + t.Fatalf("diagnostic metadata = %#v, want count %q for language %q", diagnostic.Metadata, wantCounts[language], language) + } + delete(wantCounts, language) + } + if len(wantCounts) != 0 { + t.Fatalf("missing language diagnostics: %#v", wantCounts) + } +} + +func TestLanguageQualityAnalysisCollectsUnresolvedEvidenceWhenFindingScanIsCached(t *testing.T) { + target := core.TargetConfig{Name: "repo", Path: t.TempDir(), Language: "go"} + env := support.Context{ + Config: core.Config{Targets: []core.TargetConfig{target}}, + // A cache hit returns findings without invoking the file evaluator. + ScanTargetFiles: func(core.TargetConfig, string, func(string) bool, func(string, []byte) []core.Finding) []core.Finding { + return nil + }, + VisitTargetFiles: func(_ core.TargetConfig, include func(string) bool, visit func(string, []byte)) { + if include("fixture.go") { + visit("fixture.go", []byte("package sample\nfunc ReadValue() int {\n mystery.Value = 1\n return mystery.Value\n}\n")) + } + }, + } + + analysis := languageQualityAnalysis(context.Background(), env, target) + if len(analysis.findings) != 0 { + t.Fatalf("findings = %#v, want none", analysis.findings) + } + if len(analysis.unresolved) != 1 || analysis.unresolved[0].Language != "go" || analysis.unresolved[0].Symbol != "mystery" { + t.Fatalf("unresolved = %#v, want cached-scan-independent Go record", analysis.unresolved) + } +} diff --git a/internal/codeguard/checks/quality/quality_go.go b/internal/codeguard/checks/quality/quality_go.go index 56ebcc46..b68234f2 100644 --- a/internal/codeguard/checks/quality/quality_go.go +++ b/internal/codeguard/checks/quality/quality_go.go @@ -12,7 +12,7 @@ import ( "github.com/devr-tools/codeguard/internal/codeguard/core" ) -func goFindingsForFile(env support.Context, file string, data []byte) []core.Finding { +func goFindingsForFileWithIndex(env support.Context, file string, data []byte, index *goPackageIndex) []core.Finding { findings := make([]core.Finding, 0) formatted, err := format.Source(data) @@ -50,10 +50,15 @@ func goFindingsForFile(env support.Context, file string, data []byte) []core.Fin })) return append(fileLengthFindingWithSignals(env, file, data, findings), findings...) } + if index == nil { + index = newGoPackageIndex() + index.addFile(file, fset, parsed) + } + pkg := index.packageFor(file, parsed.Name.Name) findings = append(findings, importFindings(env, file, fset, parsed)...) findings = append(findings, goFunctionFindings(env, file, fset, parsed)...) if localPrecisionEnabled(env) { - findings = append(findings, goPrecisionFindings(env, file, fset, parsed, data)...) + findings = append(findings, goPrecisionFindings(env, file, fset, parsed, data, pkg)...) findings = append(findings, goStructuralSmellFindings(env, file, fset, parsed, data)...) } findings = append(findings, goAIQualityFindings(env, file, fset, parsed, data)...) diff --git a/internal/codeguard/checks/quality/quality_precision.go b/internal/codeguard/checks/quality/quality_precision.go index 42bab90a..6cc8857b 100644 --- a/internal/codeguard/checks/quality/quality_precision.go +++ b/internal/codeguard/checks/quality/quality_precision.go @@ -46,7 +46,7 @@ var ( "misc": {}, "stuff": {}, } queryFunctionPrefixPattern = regexp.MustCompile(`^(get|find|list|load|read|lookup|fetch|is|has|can|should|compute|calculate|build|format|parse)`) - mutatingCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(add|allocate|append|assign|clear|create|delete|emit|insert|mutate|persist|pop|publish|push|push_back|remove|reverse|save|send|set|sort|splice|store|update|upsert|write)([A-Z_:\-.]|$)`) + mutatingCallPattern = regexp.MustCompile(`(^|[.>:\-_])(?i:add|allocate|append|assign|clear|create|delete|emit|insert|mutate|persist|pop|publish|push|push_back|remove|reverse|save|send|set|sort|splice|store|update|upsert|with|write)([A-Z_:\-.]|$)`) lowLevelOperationPattern = regexp.MustCompile(`(?i)(\bsql\.|\.query\(|\.exec\(|\bhttp\.|\bfetch\(|\baxios\.|\brequests\.|\bjson\.|\bJSON\.|\bos\.Getenv\b|\bprocess\.env\b|\bfs\.|#include\b)`) primitiveTypePattern = regexp.MustCompile(`(?i)\b(string|str|int|int64|float|float64|double|decimal|number|boolean|bool|char|long|short)\b`) domainPrimitiveNamePattern = regexp.MustCompile(`(?i)(id|status|state|type|kind|currency|amount|price|email|phone|country|role|permission|tenant|account|customer|order)`) @@ -61,6 +61,7 @@ var ( type precisionFunction struct { Name string + Language string Receiver string ReceiverName string StartLine int @@ -70,10 +71,18 @@ type precisionFunction struct { Assignments []support.ParsedAssignment Calls []support.ParsedCall Statements []support.ParsedStatement + Declarations []support.ParsedDeclaration + QualifiedOwner string Nested []precisionLineRange Body string Returns bool ImplementsInterfaceSignature bool + ProvenGlobals map[string]struct{} + CapturedBindings map[string]mutationBinding + GoDecl *ast.FuncDecl + GoFile string + GoFSet *token.FileSet + GoPackage *goPackageInfo } func localPrecisionEnabled(env support.Context) bool { @@ -89,12 +98,16 @@ func excessiveParameterFinding(env support.Context, file string, fn functionMetr core.ConfidenceHigh)} } -func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, data []byte) []core.Finding { +func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, parsed *ast.File, data []byte, pkg *goPackageInfo) []core.Finding { findings := make([]core.Finding, 0) interfaceMethods := goInterfaceMethodSignatures(parsed) + provenGlobals := goPackageVariableNames(parsed) ast.Inspect(parsed, func(n ast.Node) bool { if node, ok := n.(*ast.FuncDecl); ok { fn := goPrecisionFunction(fset, node, data) + fn.GoFile = file + fn.GoPackage = pkg + fn.ProvenGlobals = provenGlobals fn.ImplementsInterfaceSignature = interfaceMethods[goInterfaceMethodKey(fn.Name, fn.Params, fn.Signature)] findings = append(findings, precisionFunctionFindings(env, file, fn)...) if node.Body != nil { @@ -119,6 +132,32 @@ func goPrecisionFindings(env support.Context, file string, fset *token.FileSet, return findings } +// goPackageVariableNames provides the bounded declaration proof used by the +// shared mutation contract before the package-wide Go resolver is applied. +// Parameters and locals are seeded later and therefore correctly shadow these +// file-level package declarations. +func goPackageVariableNames(parsed *ast.File) map[string]struct{} { + names := make(map[string]struct{}) + for _, declaration := range parsed.Decls { + general, ok := declaration.(*ast.GenDecl) + if !ok || general.Tok != token.VAR { + continue + } + for _, spec := range general.Specs { + value, ok := spec.(*ast.ValueSpec) + if !ok { + continue + } + for _, name := range value.Names { + if name.Name != "" { + names[name.Name] = struct{}{} + } + } + } + } + return names +} + func goPrecisionFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) precisionFunction { out := precisionFunction{ Name: fn.Name.Name, @@ -129,6 +168,8 @@ func goPrecisionFunction(fset *token.FileSet, fn *ast.FuncDecl, data []byte) pre Signature: goResultSignature(fn), Params: goParsedParams(fn), Returns: goFuncReturnsValue(fn), + GoDecl: fn, + GoFSet: fset, } if fn.Body == nil { return out @@ -480,10 +521,10 @@ func unsafeGoNumericConversionTarget(call *ast.CallExpr) string { } func parsedPrecisionFindings(env support.Context, file string, parsed *support.ParsedFile) []core.Finding { - functions := parsed.AllFunctions() + functions := parsedPrecisionFunctions(parsed) findings := make([]core.Finding, 0, len(functions)) for _, fn := range functions { - findings = append(findings, precisionFunctionFindings(env, file, parsedPrecisionFunction(fn))...) + findings = append(findings, precisionFunctionFindings(env, file, fn)...) } findings = append(findings, parsedDefensiveFindings(env, file, parsed)...) findings = append(findings, parsedMutableGlobalFindings(env, file, parsed)...) @@ -496,20 +537,96 @@ func parsedPrecisionFindings(env support.Context, file string, parsed *support.P return findings } +func parsedPrecisionFunctions(parsed *support.ParsedFile) []precisionFunction { + if parsed == nil { + return nil + } + functions := make([]precisionFunction, 0) + retainCaptures := parsed.Language == string(support.CLikeTypeScript) + var walk func(*support.ParsedFunction, map[string]mutationBinding) + walk = func(item *support.ParsedFunction, outer map[string]mutationBinding) { + fn := parsedPrecisionFunction(item) + if retainCaptures { + fn.CapturedBindings = cloneMutationBindings(outer) + } + functions = append(functions, fn) + for _, child := range item.Nested { + childBindings := outer + if retainCaptures { + childBindings = typeScriptBindingsVisibleAt(fn, child.DefinitionOffset) + } + walk(child, childBindings) + } + } + for _, fn := range parsed.Functions { + walk(fn, nil) + } + return functions +} + +func typeScriptBindingsVisibleAt(fn precisionFunction, definitionOffset int) map[string]mutationBinding { + bindings := cloneMutationBindings(fn.CapturedBindings) + if bindings == nil { + bindings = make(map[string]mutationBinding) + } + for _, param := range fn.Params { + if param.Name != "" { + bindings[param.Name] = mutationBinding{origin: originCaller, target: targetArgument} + } + } + if fn.ReceiverName != "" { + bindings[fn.ReceiverName] = mutationBinding{origin: originCaller, target: targetReceiver} + } + if fn.Receiver != "" { + bindings["this"] = mutationBinding{origin: originCaller, target: targetReceiver} + } + for _, declaration := range fn.Declarations { + if declaration.Offset >= definitionOffset || declaration.ScopeOffsetStart >= definitionOffset || declaration.ScopeOffsetEnd <= definitionOffset { + continue + } + delete(bindings, declaration.Name) + if source := declaration.AliasSource; source != "" { + if binding, ok := bindings[source]; ok { + bindings[declaration.Name] = binding + } + continue + } + assignment := support.ParsedAssignment{Name: declaration.Name, Expr: declaration.Initializer, Line: declaration.Line} + if assignmentLooksLocalAccumulator(fn, assignment) || assignmentLooksLocalBuilder(fn, assignment) || looksLikeLocalObjectAllocation(fn, assignment) { + bindings[declaration.Name] = mutationBinding{origin: originLocal, target: targetLocal} + } + } + return bindings +} + +func cloneMutationBindings(bindings map[string]mutationBinding) map[string]mutationBinding { + if len(bindings) == 0 { + return nil + } + clone := make(map[string]mutationBinding, len(bindings)) + for name, binding := range bindings { + clone[name] = binding + } + return clone +} + func parsedPrecisionFunction(fn *support.ParsedFunction) precisionFunction { body := maskedFunctionBody(fn) return precisionFunction{ - Name: fn.Name, - StartLine: fn.StartLine, - EndLine: fn.EndLine, - Signature: fn.Signature, - Params: fn.Params, - Assignments: fn.Assignments, - Calls: fn.Calls, - Statements: fn.Statements, - Nested: nestedPrecisionLineRanges(fn), - Body: body, - Returns: strings.Contains(body, "return "), + Name: fn.Name, + Language: fn.Language, + StartLine: fn.StartLine, + EndLine: fn.EndLine, + Signature: fn.Signature, + Params: fn.Params, + Assignments: fn.Assignments, + Calls: fn.Calls, + Statements: fn.Statements, + Declarations: fn.Declarations, + QualifiedOwner: fn.QualifiedOwner, + Nested: nestedPrecisionLineRanges(fn), + Body: body, + Returns: strings.Contains(body, "return "), } } @@ -671,6 +788,14 @@ func hiddenSideEffect(file string, fn precisionFunction) bool { if isAccumulatorBuilderFunctionName(fn.Name) && !hasLikelyExternalMutationCall(fn) { return false } + if fn.GoDecl != nil { + for _, evidence := range functionMutationEvidence(fn) { + if evidence.Target != targetLocal { + return true + } + } + return false + } localTargets := localMutationTargets(fn) for _, call := range directCalls(fn) { if mutatingCallPattern.MatchString(call.Callee) && !isLocalMutationCall(call, localTargets) && !isLocalBuilderMutationCall(fn, call) && !isBuilderAccumulatorMutationCall(fn, call) { diff --git a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go index 528df8da..80770d6c 100644 --- a/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go +++ b/internal/codeguard/checks/quality/quality_precision_workstreams_cd.go @@ -31,20 +31,19 @@ const ( ) var ( - commandFunctionPrefixPattern = regexp.MustCompile(`^(add|allocate|append|apply|assign|bulk|cancel|capture|choose|clear|cleanup|close|copy|create|delete|disable|do|emit|enable|exchange|expose|export|fill|flip|flash|handle|hydrate|insert|link|log|move|mutate|note|notify|open|persist|publish|record|recompute|remember|remove|reset|retire|revert|rollback|save|send|set|store|submit|toggle|transfer|trigger|update|upsert|upload|walk|write)`) - readCallPattern = regexp.MustCompile(`(?i)(^|[.>:\-_])(count|fetch|find|get|list|load|lookup|query|read|select|search)([A-Z_:\-.]|$)`) - identifierTokenPattern = regexp.MustCompile(`[A-Za-z_$][A-Za-z0-9_$]*`) - roleSuffixPattern = regexp.MustCompile(`(?i)(manager|helper|util|utils|service|processor)$`) - durationNamePattern = regexp.MustCompile(`(?i)(timeout|duration|delay|interval|ttl|latency|elapsed|expiry|expiration|retention)`) - sizeNamePattern = regexp.MustCompile(`(?i)(size|limit|length|capacity|bytes?|mb|kb|gb)`) - moneyNamePattern = regexp.MustCompile(`(?i)(amount|price|cost|fee|total|subtotal|balance|money)`) - unitSuffixPattern = regexp.MustCompile(`(?i)(nanos?|micros?|millis?|ms|seconds?|secs?|s|minutes?|mins?|hours?|hrs?|days?|bytes?|kb|mb|gb|cents?|pennies|minor|bps|basispoints?|usd|eur|gbp|aud|cad)$`) - collectionTypePattern = regexp.MustCompile(`(?i)(\[\]|\[\s*\]|array|list|slice|map|dict|record|set|vector|collection|iterable|sequence|promise<[^>]*\[\])`) - scalarTypePattern = regexp.MustCompile(`(?i)\b(bool|boolean|char|double|float|float64|int|int32|int64|number|string|str|uint|uint64)\b`) - paramMutationPattern = regexp.MustCompile(`\b([A-Za-z_$][\w$]*)\s*(?:\.|->|\[)`) - returnLinePattern = regexp.MustCompile(`(?m)^\s*return(?:\s+([^;\n]+))?`) - partialReturnPattern = regexp.MustCompile(`(?i)\breturn\s+[^;\n,]+,\s*(err|error)\b|\breturn\s+\{[^}\n]*(data|result|value)[^}\n]*(err|error)[^}\n]*\}`) - identifierWordSplitPattern = regexp.MustCompile(`[_\-\s]+`) + readCallPattern = regexp.MustCompile(`(^|[.>:\-_])(?i:count|fetch|find|get|list|load|lookup|query|read|select|search)([A-Z_:\-.]|$)`) + identifierTokenPattern = regexp.MustCompile(`[A-Za-z_$][A-Za-z0-9_$]*`) + roleSuffixPattern = regexp.MustCompile(`(?i)(manager|helper|util|utils|service|processor)$`) + durationNamePattern = regexp.MustCompile(`(?i)(timeout|duration|delay|interval|ttl|latency|elapsed|expiry|expiration|retention)`) + sizeNamePattern = regexp.MustCompile(`(?i)(size|limit|length|capacity|bytes?|mb|kb|gb)`) + moneyNamePattern = regexp.MustCompile(`(?i)(amount|price|cost|fee|total|subtotal|balance|money)`) + unitSuffixPattern = regexp.MustCompile(`(?i)(nanos?|micros?|millis?|ms|seconds?|secs?|s|minutes?|mins?|hours?|hrs?|days?|bytes?|kb|mb|gb|cents?|pennies|minor|bps|basispoints?|usd|eur|gbp|aud|cad)$`) + collectionTypePattern = regexp.MustCompile(`(?i)(\[\]|\[\s*\]|array|list|slice|map|dict|record|set|vector|collection|iterable|sequence|promise<[^>]*\[\])`) + scalarTypePattern = regexp.MustCompile(`(?i)\b(bool|boolean|char|double|float|float64|int|int32|int64|number|string|str|uint|uint64)\b`) + paramMutationPattern = regexp.MustCompile(`\b([A-Za-z_$][\w$]*)\s*(?:\.|->|\[)`) + returnLinePattern = regexp.MustCompile(`(?m)^\s*return(?:\s+([^;\n]+))?`) + partialReturnPattern = regexp.MustCompile(`(?i)\breturn\s+[^;\n,]+,\s*(err|error)\b|\breturn\s+\{[^}\n]*(data|result|value)[^}\n]*(err|error)[^}\n]*\}`) + identifierWordSplitPattern = regexp.MustCompile(`[_\-\s]+`) ) func additionalPrecisionFunctionFindings(env support.Context, file string, fn precisionFunction) []core.Finding { @@ -185,11 +184,10 @@ func behaviorMismatch(file string, fn precisionFunction) bool { if explicitMutationName(fn.Name) || isUIHelperOrMappingContext(file, fn) || isUICommandHelperName(file, fn.Name) || isDomainSideEffectBoundaryName(fn.Name) || isFactoryHelperName(fn.Name) || isSeedOrScriptSourcePath(file) || isAdapterOrOrchestrationFunction(file, fn) { return false } - name := strings.ToLower(fn.Name) if hiddenSideEffect(file, fn) { return true } - if !commandFunctionPrefixPattern.MatchString(name) || mutatingFunctionEvidence(fn) { + if !leadingCommandVerb(fn.Name) || mutatingFunctionEvidence(fn) { return false } for _, call := range fn.Calls { @@ -204,6 +202,9 @@ func hiddenMutationEvidence(file string, fn precisionFunction) (mutationEvidence if isQualityFixturePath(file) { return mutationEvidence{}, false } + if isCPPConstructorOrDestructor(fn) { + return mutationEvidence{}, false + } if explicitMutationName(fn.Name) || isUICommandHelperName(file, fn.Name) || isDomainSideEffectBoundaryName(fn.Name) || isFrameworkOrchestrationBoundary(file, fn) || isScriptEntrypoint(file, fn.Name) || isSeedOrScriptSourcePath(file) || isAdapterOrOrchestrationFunction(file, fn) || isSecurityOrConfigUtilityFunction(file, fn) { return mutationEvidence{}, false } @@ -240,6 +241,21 @@ func hiddenMutationEvidence(file string, fn precisionFunction) (mutationEvidence return mutationEvidence{}, false } +func isCPPConstructorOrDestructor(fn precisionFunction) bool { + if fn.Language != string(support.CLikeCPP) || fn.QualifiedOwner == "" { + return false + } + owner := fn.QualifiedOwner + if cut := strings.LastIndex(owner, "::"); cut >= 0 { + owner = owner[cut+2:] + } + name := fn.Name + if cut := strings.LastIndex(name, "::"); cut >= 0 { + name = name[cut+2:] + } + return name == owner || name == "~"+owner +} + func predicateHasObviousSideEffect(fn precisionFunction) bool { body := strings.ToLower(fn.Body) return containsAny(body, []string{ @@ -420,7 +436,10 @@ func lineHasAssignmentOperator(line string) bool { func explicitMutationName(name string) bool { lowered := strings.ToLower(strings.TrimSpace(name)) - return commandFunctionPrefixPattern.MatchString(lowered) || + if cut := strings.LastIndex(lowered, "::"); cut >= 0 { + lowered = lowered[cut+2:] + } + return leadingCommandVerb(name) || conventionalMutationBoundaryPattern.MatchString(lowered) || isEventHandlerName(name) || strings.Contains(lowered, "mutat") || @@ -428,6 +447,28 @@ func explicitMutationName(name string) bool { strings.Contains(lowered, "write") } +var commandFunctionVerbs = map[string]struct{}{ + "add": {}, "allocate": {}, "append": {}, "apply": {}, "assign": {}, "bind": {}, "bulk": {}, + "cancel": {}, "capture": {}, "choose": {}, "clear": {}, "cleanup": {}, "close": {}, "configure": {}, "copy": {}, "create": {}, + "deactivate": {}, "delete": {}, "disable": {}, "discard": {}, "do": {}, + "emit": {}, "enable": {}, "exchange": {}, "expose": {}, "export": {}, + "fill": {}, "flip": {}, "flash": {}, "handle": {}, "hydrate": {}, "insert": {}, "issue": {}, + "link": {}, "log": {}, "mark": {}, "move": {}, "mutate": {}, "note": {}, "notify": {}, "open": {}, + "persist": {}, "publish": {}, "record": {}, "recompute": {}, "release": {}, "remember": {}, "remove": {}, "reset": {}, "retire": {}, "revert": {}, "revoke": {}, "rollback": {}, + "save": {}, "send": {}, "set": {}, "shutdown": {}, "store": {}, "submit": {}, "subscribe": {}, + "toggle": {}, "transfer": {}, "trigger": {}, "update": {}, "upsert": {}, "upload": {}, "walk": {}, "write": {}, +} + +func leadingCommandVerb(name string) bool { + terminal := terminalCallIdentifier(strings.TrimSpace(name)) + words := identifierWords(terminal) + if len(words) == 0 { + return false + } + _, ok := commandFunctionVerbs[words[0]] + return ok +} + func inconsistentReturnContract(fn precisionFunction) bool { if nextResponseNullableGuardHelper(fn) || nullableParserLookupContract(fn) { return false diff --git a/internal/codeguard/checks/quality/quality_scan_language.go b/internal/codeguard/checks/quality/quality_scan_language.go index 48ff6d4c..2cf9e1d9 100644 --- a/internal/codeguard/checks/quality/quality_scan_language.go +++ b/internal/codeguard/checks/quality/quality_scan_language.go @@ -2,19 +2,47 @@ package quality import ( "context" + "go/ast" + "strings" + "sync" "github.com/devr-tools/codeguard/internal/codeguard/checks/support" "github.com/devr-tools/codeguard/internal/codeguard/core" ) -func languageQualityFindings(ctx context.Context, env support.Context, target core.TargetConfig) []core.Finding { - return support.DispatchByLanguage(target.Language, +type languageQualityScan struct { + findings []core.Finding + unresolved []unresolvedMutationEvidence +} + +func languageQualityAnalysis(ctx context.Context, env support.Context, target core.TargetConfig) languageQualityScan { + var unresolvedMu sync.Mutex + var unresolved []unresolvedMutationEvidence + addUnresolved := func(items []unresolvedMutationEvidence) { + if len(items) == 0 { + return + } + unresolvedMu.Lock() + unresolved = append(unresolved, items...) + unresolvedMu.Unlock() + } + findings := support.DispatchByLanguage(target.Language, support.LanguageDispatch{ Aliases: []string{"", "go"}, Run: func() []core.Finding { - return support.ScanGoFiles(env, target, "quality", func(file string, data []byte) []core.Finding { - return goFindingsForFile(env, file, data) + var index *goPackageIndex + if localPrecisionEnabled(env) && env.VisitTargetFiles != nil { + index = buildGoPackageIndex(env, target) + } + findings := support.ScanGoFiles(env, target, "quality", func(file string, data []byte) []core.Finding { + return goFindingsForFileWithIndex(env, file, data, index) }) + if localPrecisionEnabled(env) && env.VisitTargetFiles != nil { + env.VisitTargetFiles(target, func(file string) bool { return strings.HasSuffix(file, ".go") }, func(file string, data []byte) { + addUnresolved(goUnresolvedMutationEvidenceWithIndex(env, file, data, index)) + }) + } + return findings }, }, support.LanguageDispatch{ @@ -28,7 +56,13 @@ func languageQualityFindings(ctx context.Context, env support.Context, target co support.LanguageDispatch{ Aliases: []string{"typescript", "javascript", "ts", "tsx", "js", "jsx"}, Run: func() []core.Finding { - return typeScriptTargetFindings(ctx, env, target) + findings := typeScriptTargetFindings(ctx, env, target) + if localPrecisionEnabled(env) && env.VisitTargetFiles != nil { + env.VisitTargetFiles(target, isTypeScriptLikeFile, func(_ string, data []byte) { + addUnresolved(typeScriptUnresolvedMutationEvidence(data)) + }) + } + return findings }, }, support.LanguageDispatch{ @@ -42,9 +76,15 @@ func languageQualityFindings(ctx context.Context, env support.Context, target co support.LanguageDispatch{ Aliases: []string{"c++", "cpp", "cxx", "cc"}, Run: func() []core.Finding { - return support.ScanCPPFiles(env, target, "quality", func(file string, data []byte) []core.Finding { + findings := support.ScanCPPFiles(env, target, "quality", func(file string, data []byte) []core.Finding { return cppFindingsForFile(env, file, data) }) + if localPrecisionEnabled(env) && env.VisitTargetFiles != nil { + env.VisitTargetFiles(target, func(file string) bool { return support.IsCPPPath(file, true) }, func(_ string, data []byte) { + addUnresolved(cppUnresolvedMutationEvidence(data)) + }) + } + return findings }, }, support.LanguageDispatch{ @@ -72,4 +112,64 @@ func languageQualityFindings(ctx context.Context, env support.Context, target co }, }, ) + return languageQualityScan{findings: findings, unresolved: unresolved} +} + +func buildGoPackageIndex(env support.Context, target core.TargetConfig) *goPackageIndex { + index := newGoPackageIndex() + env.VisitTargetFiles(target, func(file string) bool { return strings.HasSuffix(file, ".go") }, func(file string, data []byte) { + fset, parsed, err := support.ParseGoSource(env, file, data) + if err == nil { + index.addFile(file, fset, parsed) + } + }) + return index +} + +func goUnresolvedMutationEvidenceWithIndex(env support.Context, file string, data []byte, index *goPackageIndex) []unresolvedMutationEvidence { + fset, parsed, err := support.ParseGoSource(env, file, data) + if err != nil { + return nil + } + if index == nil { + index = newGoPackageIndex() + index.addFile(file, fset, parsed) + } + pkg := index.packageFor(file, parsed.Name.Name) + provenGlobals := goPackageVariableNames(parsed) + var unresolved []unresolvedMutationEvidence + ast.Inspect(parsed, func(node ast.Node) bool { + declaration, ok := node.(*ast.FuncDecl) + if !ok { + return true + } + fn := goPrecisionFunction(fset, declaration, data) + fn.GoFile = file + fn.GoPackage = pkg + fn.ProvenGlobals = provenGlobals + analysis := functionMutationAnalysis(fn, "go") + unresolved = append(unresolved, analysis.Unresolved...) + return true + }) + return unresolved +} + +func cppUnresolvedMutationEvidence(data []byte) []unresolvedMutationEvidence { + parsed := support.ParseCLike(string(data), support.CLikeCPP) + var unresolved []unresolvedMutationEvidence + for _, fn := range parsed.AllFunctions() { + analysis := cppFunctionMutationEvidence(parsedPrecisionFunction(fn)) + unresolved = append(unresolved, analysis.Unresolved...) + } + return unresolved +} + +func typeScriptUnresolvedMutationEvidence(data []byte) []unresolvedMutationEvidence { + parsed := support.ParseCLike(string(data), support.CLikeTypeScript) + var unresolved []unresolvedMutationEvidence + for _, fn := range parsedPrecisionFunctions(parsed) { + analysis := functionMutationAnalysis(fn, "typescript") + unresolved = append(unresolved, analysis.Unresolved...) + } + return unresolved } diff --git a/internal/codeguard/checks/support/parser_clike.go b/internal/codeguard/checks/support/parser_clike.go index c91e1612..faff9699 100644 --- a/internal/codeguard/checks/support/parser_clike.go +++ b/internal/codeguard/checks/support/parser_clike.go @@ -19,6 +19,12 @@ func ParseCLike(source string, lang CLikeLanguage) *ParsedFile { spans := clikeFunctionSpans(masked, lang) file.Functions = buildCLikeFunctions(file, spans, lang) file.Module.EndLine = LineNumberForOffset(source, len(source)) + switch lang { + case CLikeCPP: + populateCPPDeclarationMetadata(file, spans) + case CLikeTypeScript: + populateTypeScriptDeclarationMetadata(file) + } return file } @@ -68,11 +74,16 @@ func newCLikeFunction(file *ParsedFile, span clikeSpan, lang CLikeLanguage) *Par } } fn := &ParsedFunction{ - Name: span.name, - StartLine: LineNumberForOffset(file.Source, span.start), - EndLine: LineNumberForOffset(file.Source, span.bodyEnd), - Signature: signature, - Params: parseCLikeParams(paramText, lang), + Name: span.name, + Language: string(lang), + StartLine: LineNumberForOffset(file.Source, span.start), + EndLine: LineNumberForOffset(file.Source, span.bodyEnd), + Signature: signature, + Params: parseCLikeParams(paramText, lang), + DefinitionOffset: span.start, + sourceStart: span.start, + bodyOpen: span.bodyOpen, + bodyEnd: span.bodyEnd, } if span.bodyOpen >= 0 && span.bodyEnd > span.bodyOpen { populateCLikeBody(file, fn, span, lang) diff --git a/internal/codeguard/checks/support/parser_clike_cpp_test.go b/internal/codeguard/checks/support/parser_clike_cpp_test.go new file mode 100644 index 00000000..b4f2ee99 --- /dev/null +++ b/internal/codeguard/checks/support/parser_clike_cpp_test.go @@ -0,0 +1,145 @@ +package support + +import "testing" + +func TestCppParserRecordsInferredAliasesCapturesAndQualifiedMembers(t *testing.T) { + parsed := ParseCLike(`namespace N { +struct Counter { + Token state; + int current(Token* inputPtr); +}; +} + +int N::Counter::current(Token* inputPtr) { + auto first = inputPtr; + auto second = &state; + auto work = [=]() { return first->value; }; + return second->value; +}`, CLikeCPP) + fn := parsed.FunctionByName("N::Counter::current") + if fn == nil { + t.Fatal("qualified function missing") + } + if fn.QualifiedOwner != "N::Counter" { + t.Fatalf("qualified owner = %q, want N::Counter", fn.QualifiedOwner) + } + for name, alias := range map[string]string{"first": "inputPtr", "second": "state"} { + declaration, ok := cppParsedDeclarationForTest(fn.Declarations, name, "local") + if !ok || declaration.AliasSource != alias { + t.Fatalf("declaration %q = %#v, ok=%v, want alias %q", name, declaration, ok, alias) + } + } + capture, ok := cppParsedDeclarationForTest(fn.Declarations, "*", "capture") + if !ok || capture.ReferenceShape != "value" { + t.Fatalf("default capture = %#v, ok=%v, want value capture", capture, ok) + } + member, ok := cppParsedDeclarationForTest(fn.Declarations, "state", "member") + if !ok || member.QualifiedOwner != "N::Counter" { + t.Fatalf("member = %#v, ok=%v, want N::Counter-qualified state", member, ok) + } +} + +func TestCppParserScopesLambdaLocalsAndBuildsNestedNamespaceOwner(t *testing.T) { + parsed := ParseCLike(`namespace N { +namespace M { +struct Counter { + Token state; + int current(); +}; +} +} + +int N::M::Counter::current() { + auto work = [=]() { + Token* p = &state; + p->value++; + }; + work(); + return state.value; +}`, CLikeCPP) + fn := parsed.FunctionByName("N::M::Counter::current") + if fn == nil { + t.Fatal("nested namespace function missing") + } + if fn.QualifiedOwner != "N::M::Counter" { + t.Fatalf("qualified owner = %q, want N::M::Counter", fn.QualifiedOwner) + } + member, ok := cppParsedDeclarationForTest(fn.Declarations, "state", "member") + if !ok || member.QualifiedOwner != "N::M::Counter" { + t.Fatalf("member = %#v, ok=%v, want N::M::Counter-qualified state", member, ok) + } + local, ok := cppParsedDeclarationForTest(fn.Declarations, "p", "local") + if !ok || local.AliasSource != "state" || local.Line < local.ScopeStart || local.ScopeEnd < local.Line { + t.Fatalf("lambda local = %#v, ok=%v, want scoped pointer alias to state", local, ok) + } +} + +func TestCppParserDistinguishesPointerAndThisCaptureShapes(t *testing.T) { + parsed := ParseCLike(`struct Counter { + Token state; + int current(Token& input, Token* inputPtr) { + auto pointerCopy = [inputPtr]() { return inputPtr->value; }; + auto pointerInit = [ptr=&input]() { return ptr->value; }; + auto receiverPointer = [this]() { return state.value; }; + auto receiverCopy = [*this]() { return state.value; }; + return state.value; + } +};`, CLikeCPP) + fn := parsed.FunctionByName("current") + if fn == nil { + t.Fatal("current missing") + } + want := map[string]string{"inputPtr": "value", "ptr": "pointer"} + for name, shape := range want { + capture, ok := cppParsedDeclarationForTest(fn.Declarations, name, "capture") + if !ok || capture.ReferenceShape != shape { + t.Fatalf("capture %q = %#v, ok=%v, want shape %q", name, capture, ok, shape) + } + } + thisShapes := map[string]bool{"pointer": false, "object": false} + for _, declaration := range fn.Declarations { + if declaration.Kind == "capture" && declaration.Name == "this" { + if _, expected := thisShapes[declaration.ReferenceShape]; expected { + thisShapes[declaration.ReferenceShape] = true + } + } + } + for shape, found := range thisShapes { + if !found { + t.Fatalf("this captures = %#v, want %q shape", fn.Declarations, shape) + } + } +} + +func TestCppParserDoesNotTreatIfConstexprRequiresAsFunction(t *testing.T) { + parsed := ParseCLike(` +struct Awaiter { + State state; + template + bool await_suspend(Promise continuation) { + state.continuation = continuation; + if constexpr (requires(Promise& promise) { promise.resumeScheduler(); }) { + state.scheduler = continuation.promise().resumeScheduler(); + } + return !state.completed; + } +}; +`, CLikeCPP) + for _, function := range parsed.AllFunctions() { + if function.Name == "constexpr" { + t.Fatalf("if constexpr requires-expression became a function: %#v", function) + } + } + if parsed.FunctionByName("await_suspend") == nil { + t.Fatal("await_suspend missing") + } +} + +func cppParsedDeclarationForTest(declarations []ParsedDeclaration, name string, kind string) (ParsedDeclaration, bool) { + for _, declaration := range declarations { + if declaration.Name == name && declaration.Kind == kind { + return declaration, true + } + } + return ParsedDeclaration{}, false +} diff --git a/internal/codeguard/checks/support/parser_clike_functions.go b/internal/codeguard/checks/support/parser_clike_functions.go new file mode 100644 index 00000000..8cd03fe1 --- /dev/null +++ b/internal/codeguard/checks/support/parser_clike_functions.go @@ -0,0 +1,330 @@ +package support + +import ( + "regexp" + "sort" + "strings" +) + +type cppClassSpan struct { + name string + bodyOpen int + bodyEnd int +} + +type cppNamespaceSpan struct { + name string + bodyOpen int + bodyEnd int +} + +var ( + cppClassHeadPattern = regexp.MustCompile(`\b(?:class|struct)\s+([A-Za-z_]\w*)[^;{}]*\{`) + cppNamespacePattern = regexp.MustCompile(`\bnamespace\s+([A-Za-z_]\w*(?:::[A-Za-z_]\w*)*)\s*\{`) + cppDeclarationPattern = regexp.MustCompile(`(?m)(?:^|[;{}])[ \t]*((?:(?:constexpr|constinit|consteval|static|inline|const|volatile|mutable|thread_local|extern)[ \t]+)*(?:[A-Za-z_]\w*(?:::[A-Za-z_]\w*)*)(?:[ \t]*<[^;\n{}=()]+>)?(?:[ \t]*[*&]+[ \t]*|[ \t]+))([A-Za-z_]\w*)[ \t]*(?:=[ \t]*([^;\n]*)|\{([^;\n]*)\}|;)`) + cppLambdaPattern = regexp.MustCompile(`\[([^]\n]*)\][ \t]*(?:\([^)]*\)[ \t]*)?(?:mutable[ \t]*)?(?:noexcept[ \t]*)?\{`) + cppSimpleAliasPattern = regexp.MustCompile(`^\s*(?:std::move\s*\(\s*)?&?([A-Za-z_]\w*)\s*\)?\s*$`) +) + +func populateCPPDeclarationMetadata(file *ParsedFile, spans []clikeSpan) { + namespaces := cppNamespaceSpans(file.Masked) + classes := cppClassSpans(file.Masked, namespaces) + maskedTop := []byte(file.Masked) + for _, span := range spans { + blankCPPRange(maskedTop, span.start, span.bodyEnd+1) + } + + declarations := make([]ParsedDeclaration, 0) + for _, class := range classes { + classMasked := []byte(file.Masked[class.bodyOpen+1 : class.bodyEnd]) + for _, span := range spans { + if span.start > class.bodyOpen && span.bodyEnd < class.bodyEnd { + blankCPPRange(classMasked, span.start-(class.bodyOpen+1), span.bodyEnd+1-(class.bodyOpen+1)) + } + } + members := cppDeclarationsInRange(file, string(classMasked), class.bodyOpen+1, "member", class.name, class.bodyOpen, class.bodyEnd) + declarations = append(declarations, members...) + blankCPPRange(maskedTop, class.bodyOpen, class.bodyEnd+1) + } + globals := cppDeclarationsInRange(file, string(maskedTop), 0, "global", "", 0, len(file.Masked)-1) + declarations = append(declarations, globals...) + file.Declarations = declarations + + for _, fn := range file.AllFunctions() { + owner := cppQualifiedOwner(fn.Name) + if owner == "" { + owner = cppOwnerAt(classes, fn.sourceStart) + } else if namespace := cppNamespaceOwnerAt(namespaces, fn.sourceStart); namespace != "" && !strings.HasPrefix(owner, namespace+"::") { + owner = namespace + "::" + owner + } + fn.QualifiedOwner = owner + fn.Declarations = append(fn.Declarations, cppParameterDeclarations(fn)...) + if fn.bodyOpen >= 0 && fn.bodyEnd > fn.bodyOpen { + body := file.Masked[fn.bodyOpen+1 : fn.bodyEnd] + fn.Declarations = append(fn.Declarations, cppDeclarationsInRange(file, body, fn.bodyOpen+1, "local", "", fn.bodyOpen, fn.bodyEnd)...) + fn.Declarations = append(fn.Declarations, cppLambdaCaptures(file, fn, body)...) + } + for _, declaration := range declarations { + if declaration.Kind == "global" || (declaration.Kind == "member" && declaration.QualifiedOwner == owner) { + fn.Declarations = append(fn.Declarations, declaration) + } + } + sort.SliceStable(fn.Declarations, func(i, j int) bool { + if fn.Declarations[i].Line == fn.Declarations[j].Line { + return cppDeclarationPriority(fn.Declarations[i].Kind) < cppDeclarationPriority(fn.Declarations[j].Kind) + } + return fn.Declarations[i].Line < fn.Declarations[j].Line + }) + } +} + +func cppClassSpans(masked string, namespaces []cppNamespaceSpan) []cppClassSpan { + classes := make([]cppClassSpan, 0) + for _, match := range cppClassHeadPattern.FindAllStringSubmatchIndex(masked, -1) { + open := match[1] - 1 + end := matchBracketOffset(masked, open) + if end > open { + name := masked[match[2]:match[3]] + if namespace := cppNamespaceOwnerAt(namespaces, match[0]); namespace != "" { + name = namespace + "::" + name + } + classes = append(classes, cppClassSpan{name: name, bodyOpen: open, bodyEnd: end}) + } + } + return classes +} + +func cppNamespaceSpans(masked string) []cppNamespaceSpan { + namespaces := make([]cppNamespaceSpan, 0) + for _, match := range cppNamespacePattern.FindAllStringSubmatchIndex(masked, -1) { + open := match[1] - 1 + end := matchBracketOffset(masked, open) + if end > open { + namespaces = append(namespaces, cppNamespaceSpan{name: masked[match[2]:match[3]], bodyOpen: open, bodyEnd: end}) + } + } + for i := range namespaces { + parent := "" + width := int(^uint(0) >> 1) + for j := range namespaces { + if i == j || namespaces[i].bodyOpen <= namespaces[j].bodyOpen || namespaces[i].bodyEnd >= namespaces[j].bodyEnd { + continue + } + candidateWidth := namespaces[j].bodyEnd - namespaces[j].bodyOpen + if candidateWidth < width { + parent, width = namespaces[j].name, candidateWidth + } + } + if parent != "" && !strings.HasPrefix(namespaces[i].name, parent+"::") { + namespaces[i].name = parent + "::" + namespaces[i].name + } + } + return namespaces +} + +func cppNamespaceOwnerAt(namespaces []cppNamespaceSpan, offset int) string { + owner := "" + width := int(^uint(0) >> 1) + for _, namespace := range namespaces { + if offset > namespace.bodyOpen && offset < namespace.bodyEnd && namespace.bodyEnd-namespace.bodyOpen < width { + owner, width = namespace.name, namespace.bodyEnd-namespace.bodyOpen + } + } + return owner +} + +func cppDeclarationsInRange(file *ParsedFile, masked string, base int, kind string, owner string, scopeOpen int, scopeEnd int) []ParsedDeclaration { + declarations := make([]ParsedDeclaration, 0) + for _, match := range cppDeclarationPattern.FindAllStringSubmatchIndex(masked, -1) { + typ := strings.TrimSpace(masked[match[2]:match[3]]) + name := masked[match[4]:match[5]] + if cppDeclarationKeyword(typ) || isCLikeKeyword(name) { + continue + } + initializer := "" + if match[6] >= 0 { + initializer = strings.TrimSpace(masked[match[6]:match[7]]) + } else if match[8] >= 0 { + initializer = "{" + strings.TrimSpace(masked[match[8]:match[9]]) + "}" + } + offset := base + match[4] + start, end := clikeLexicalScope(file.Masked, scopeOpen, scopeEnd, offset) + declarations = append(declarations, ParsedDeclaration{ + Name: name, Type: typ, Kind: kind, ReferenceShape: cppReferenceShape(typ), + Line: LineNumberForOffset(file.Source, offset), ScopeStart: LineNumberForOffset(file.Source, start), ScopeEnd: LineNumberForOffset(file.Source, end), + AliasSource: cppAliasSource(initializer), Initializer: initializer, QualifiedOwner: owner, + }) + } + return declarations +} + +func cppParameterDeclarations(fn *ParsedFunction) []ParsedDeclaration { + out := make([]ParsedDeclaration, 0, len(fn.Params)) + for _, param := range fn.Params { + if param.Name == "" { + continue + } + out = append(out, ParsedDeclaration{ + Name: param.Name, Type: param.Type, Kind: "parameter", ReferenceShape: cppReferenceShape(param.Type), + Line: fn.StartLine, ScopeStart: fn.StartLine, ScopeEnd: fn.EndLine, + }) + } + return out +} + +func cppLambdaCaptures(file *ParsedFile, fn *ParsedFunction, body string) []ParsedDeclaration { + out := make([]ParsedDeclaration, 0) + for _, match := range cppLambdaPattern.FindAllStringSubmatchIndex(body, -1) { + captureText := body[match[2]:match[3]] + open := fn.bodyOpen + 1 + match[1] - 1 + end := matchBracketOffset(file.Masked, open) + if end < open { + continue + } + line := LineNumberForOffset(file.Source, fn.bodyOpen+1+match[0]) + for _, raw := range splitTopLevelArgs(captureText) { + capture := strings.TrimSpace(raw) + if capture == "" { + continue + } + if capture == "=" || capture == "&" { + shape := "value" + if capture == "&" { + shape = "reference" + } + out = append(out, ParsedDeclaration{Name: "*", Kind: "capture", ReferenceShape: shape, Line: line, ScopeStart: line, ScopeEnd: LineNumberForOffset(file.Source, end)}) + continue + } + shape := "value" + capture = strings.TrimSpace(capture) + switch { + case capture == "*this": + shape = "object" + capture = "this" + case capture == "this": + shape = "pointer" + case strings.HasPrefix(capture, "&"): + shape = "reference" + capture = strings.TrimSpace(strings.TrimPrefix(capture, "&")) + } + name := capture + initializer := "" + if eq := topLevelIndex(capture, '='); eq >= 0 { + name = strings.TrimSpace(capture[:eq]) + initializer = strings.TrimSpace(capture[eq+1:]) + if strings.HasPrefix(initializer, "&") { + shape = "pointer" + } + } + name = strings.TrimPrefix(name, "*") + if !clikeIdentPattern.MatchString(name) && name != "this" { + continue + } + aliasSource := cppAliasSource(initializer) + if initializer == "" { + aliasSource = name + } + out = append(out, ParsedDeclaration{Name: name, Kind: "capture", ReferenceShape: shape, AliasSource: aliasSource, Initializer: initializer, Line: line, ScopeStart: line, ScopeEnd: LineNumberForOffset(file.Source, end)}) + } + } + return out +} + +func clikeLexicalScope(masked string, outerOpen int, outerEnd int, offset int) (int, int) { + start, end := outerOpen, outerEnd + stack := make([]int, 0) + for i := outerOpen + 1; i < offset && i < outerEnd; i++ { + switch masked[i] { + case '{': + stack = append(stack, i) + case '}': + if len(stack) > 0 { + stack = stack[:len(stack)-1] + } + } + } + if len(stack) > 0 { + start = stack[len(stack)-1] + if scopeEnd := matchBracketOffset(masked, start); scopeEnd > start { + end = scopeEnd + } + } + return start, end +} + +func cppReferenceShape(typ string) string { + if strings.Contains(typ, "&") { + return "reference" + } + if strings.Contains(typ, "*") { + return "pointer" + } + return "" +} + +func cppAliasSource(initializer string) string { + if match := cppSimpleAliasPattern.FindStringSubmatch(strings.TrimSpace(initializer)); len(match) == 2 { + return match[1] + } + return "" +} + +func cppQualifiedOwner(name string) string { + if cut := strings.LastIndex(name, "::"); cut > 0 { + return name[:cut] + } + return "" +} + +func cppOwnerAt(classes []cppClassSpan, offset int) string { + owner := "" + width := int(^uint(0) >> 1) + for _, class := range classes { + if offset > class.bodyOpen && offset < class.bodyEnd && class.bodyEnd-class.bodyOpen < width { + owner, width = class.name, class.bodyEnd-class.bodyOpen + } + } + return owner +} + +func cppDeclarationKeyword(typ string) bool { + base := strings.Fields(strings.TrimSpace(typ)) + if len(base) == 0 { + return true + } + switch base[len(base)-1] { + case "return", "if", "else", "for", "while", "switch", "case", "catch", "throw", "delete", "new", "template", "typename", "class", "struct", "using", "namespace": + return true + default: + return false + } +} + +func cppDeclarationPriority(kind string) int { + switch kind { + case "capture": + return 0 + case "local": + return 1 + case "parameter": + return 2 + case "member": + return 3 + default: + return 4 + } +} + +func blankCPPRange(data []byte, start int, end int) { + if start < 0 { + start = 0 + } + if end > len(data) { + end = len(data) + } + for i := start; i < end; i++ { + if data[i] != '\n' { + data[i] = ' ' + } + } +} diff --git a/internal/codeguard/checks/support/parser_clike_lang.go b/internal/codeguard/checks/support/parser_clike_lang.go index d2d79d91..39ddfef1 100644 --- a/internal/codeguard/checks/support/parser_clike_lang.go +++ b/internal/codeguard/checks/support/parser_clike_lang.go @@ -16,7 +16,7 @@ func clikeFunctionSpans(masked string, lang CLikeLanguage) []clikeSpan { case CLikeJava: return headSpans(masked, javaMethodHead, nil, false) case CLikeCPP: - return headSpans(masked, cppMethodHead, isCPPNonMethodName, false) + return cppHeadSpans(masked) case CLikeRust: return rustSpans(masked) default: @@ -24,6 +24,63 @@ func clikeFunctionSpans(masked string, lang CLikeLanguage) []clikeSpan { } } +func cppHeadSpans(masked string) []clikeSpan { + spans := make([]clikeSpan, 0, 8) + for _, match := range cppMethodHead.FindAllStringSubmatchIndex(masked, -1) { + name := masked[match[2]:match[3]] + if isCPPNonMethodName(name) { + continue + } + span, ok := resolveCPPSpan(masked, match[0], match[1]-1) + if !ok { + continue + } + span.name = name + spans = append(spans, span) + } + return spans +} + +func resolveCPPSpan(masked string, start int, paramsOpen int) (clikeSpan, bool) { + span := clikeSpan{start: start, paramsOpen: paramsOpen} + paramsClose := matchBracketOffset(masked, paramsOpen) + if paramsClose < 0 { + return span, false + } + span.bodyOpen = findCPPBodyOpen(masked, paramsClose+1) + if span.bodyOpen < 0 { + return span, false + } + span.bodyEnd = matchBracketOffset(masked, span.bodyOpen) + return span, span.bodyEnd > span.bodyOpen +} + +// findCPPBodyOpen skips braced constructor member initializers. A braced +// initializer is followed by another initializer or by the actual body. +func findCPPBodyOpen(masked string, offset int) int { + for i := offset; i < len(masked); i++ { + switch masked[i] { + case ';': + return -1 + case '{': + initializerEnd := matchBracketOffset(masked, i) + if initializerEnd < 0 { + return -1 + } + next := initializerEnd + 1 + for next < len(masked) && (masked[next] == ' ' || masked[next] == '\t' || masked[next] == '\n') { + next++ + } + if next < len(masked) && (masked[next] == ',' || masked[next] == '{') { + i = initializerEnd + continue + } + return i + } + } + return -1 +} + // headSpans resolves regex head matches into full function spans. The regex // must end at the open paren; reject filters out non-function names. func headSpans(masked string, head *regexp.Regexp, reject func(string) bool, arrow bool) []clikeSpan { @@ -145,7 +202,7 @@ func isTypeScriptNonMethodName(name string) bool { func isCPPNonMethodName(name string) bool { switch name { case "if", "for", "while", "switch", "catch", "return", "new", "delete", - "sizeof", "alignof", "typeid", "requires": + "sizeof", "alignof", "typeid", "requires", "constexpr": return true default: return false diff --git a/internal/codeguard/checks/support/parser_clike_scope.go b/internal/codeguard/checks/support/parser_clike_scope.go index 64f3ae57..868f2425 100644 --- a/internal/codeguard/checks/support/parser_clike_scope.go +++ b/internal/codeguard/checks/support/parser_clike_scope.go @@ -122,10 +122,19 @@ func javaParamFromPart(part string) (ParsedParam, bool) { return ParsedParam{}, false } name := fields[len(fields)-1] + shape := "" + for len(name) > 0 && (name[0] == '*' || name[0] == '&') { + shape += name[:1] + name = name[1:] + } if !clikeIdentPattern.MatchString(name) { return ParsedParam{}, false } - return ParsedParam{Name: name, Type: strings.Join(fields[:len(fields)-1], " ")}, true + typ := strings.Join(fields[:len(fields)-1], " ") + if shape != "" { + typ = strings.TrimSpace(typ) + shape + } + return ParsedParam{Name: name, Type: typ}, true } var clikeIdentPattern = regexp.MustCompile(`^[A-Za-z_$][\w$]*$`) diff --git a/internal/codeguard/checks/support/parser_clike_typescript_scope.go b/internal/codeguard/checks/support/parser_clike_typescript_scope.go new file mode 100644 index 00000000..c57d8950 --- /dev/null +++ b/internal/codeguard/checks/support/parser_clike_typescript_scope.go @@ -0,0 +1,39 @@ +package support + +import ( + "regexp" + "strings" +) + +var ( + typeScriptDeclarationPattern = regexp.MustCompile(`(?m)\b(?:const|let|var)[ \t]+([A-Za-z_$][\w$]*)[ \t]*(?::[ \t]*[^=;\n]+)?[ \t]*(?:=[ \t]*([^;\n]*))?`) + typeScriptAliasPattern = regexp.MustCompile(`^[ \t]*([A-Za-z_$][\w$]*)[ \t]*$`) +) + +func populateTypeScriptDeclarationMetadata(file *ParsedFile) { + for _, fn := range file.AllFunctions() { + if fn.bodyOpen < 0 || fn.bodyEnd <= fn.bodyOpen { + continue + } + body := file.Masked[fn.bodyOpen+1 : fn.bodyEnd] + for _, match := range typeScriptDeclarationPattern.FindAllStringSubmatchIndex(body, -1) { + name := body[match[2]:match[3]] + initializer := "" + if match[4] >= 0 { + initializer = strings.TrimSpace(body[match[4]:match[5]]) + } + offset := fn.bodyOpen + 1 + match[2] + scopeStart, scopeEnd := clikeLexicalScope(file.Masked, fn.bodyOpen, fn.bodyEnd, offset) + aliasSource := "" + if alias := typeScriptAliasPattern.FindStringSubmatch(initializer); len(alias) == 2 { + aliasSource = alias[1] + } + fn.Declarations = append(fn.Declarations, ParsedDeclaration{ + Name: name, Kind: "local", Line: LineNumberForOffset(file.Source, offset), + ScopeStart: LineNumberForOffset(file.Source, scopeStart), ScopeEnd: LineNumberForOffset(file.Source, scopeEnd), + Offset: offset, ScopeOffsetStart: scopeStart, ScopeOffsetEnd: scopeEnd, + AliasSource: aliasSource, Initializer: initializer, + }) + } + } +} diff --git a/internal/codeguard/checks/support/parser_clike_typescript_scope_test.go b/internal/codeguard/checks/support/parser_clike_typescript_scope_test.go new file mode 100644 index 00000000..1ff6172d --- /dev/null +++ b/internal/codeguard/checks/support/parser_clike_typescript_scope_test.go @@ -0,0 +1,47 @@ +package support + +import "testing" + +func TestTypeScriptParserBoundsDeclarationsToLexicalBlocks(t *testing.T) { + parsed := ParseCLike(`export function prepareUser(repo: Repository, user: User, enabled: boolean) { + const collaborator = repo; + if (enabled) { + const collaborator = new Repository(); + function loadLocal() { + collaborator.save(user); + return user; + } + loadLocal(); + } + function loadOuter() { + collaborator.save(user); + return user; + } + return loadOuter(); +}`, CLikeTypeScript) + parent := parsed.FunctionByName("prepareUser") + local := parsed.FunctionByName("loadLocal") + outer := parsed.FunctionByName("loadOuter") + if parent == nil || local == nil || outer == nil { + t.Fatalf("functions = %#v, want parent and nested functions", parsed.AllFunctions()) + } + var declarations []ParsedDeclaration + for _, declaration := range parent.Declarations { + if declaration.Name == "collaborator" { + declarations = append(declarations, declaration) + } + } + if len(declarations) != 2 { + t.Fatalf("collaborator declarations = %#v, want outer and shadow", declarations) + } + outerDeclaration, innerDeclaration := declarations[0], declarations[1] + if outerDeclaration.AliasSource != "repo" || outerDeclaration.ScopeOffsetStart >= local.DefinitionOffset || outerDeclaration.ScopeOffsetEnd <= outer.DefinitionOffset { + t.Fatalf("outer declaration = %#v, local=%d outer=%d, want visibility at both children", outerDeclaration, local.DefinitionOffset, outer.DefinitionOffset) + } + if innerDeclaration.AliasSource != "" || innerDeclaration.ScopeOffsetStart >= local.DefinitionOffset || innerDeclaration.ScopeOffsetEnd <= local.DefinitionOffset { + t.Fatalf("inner declaration = %#v, local=%d, want fresh local shadow visible to loadLocal", innerDeclaration, local.DefinitionOffset) + } + if innerDeclaration.ScopeOffsetStart < outer.DefinitionOffset && innerDeclaration.ScopeOffsetEnd > outer.DefinitionOffset { + t.Fatalf("inner declaration = %#v, outer=%d, want shadow hidden from loadOuter", innerDeclaration, outer.DefinitionOffset) + } +} diff --git a/internal/codeguard/checks/support/parser_types.go b/internal/codeguard/checks/support/parser_types.go index ea0cfac9..309e575b 100644 --- a/internal/codeguard/checks/support/parser_types.go +++ b/internal/codeguard/checks/support/parser_types.go @@ -15,6 +15,18 @@ type ParsedParam struct { Type string } +// ParsedDeclaration records bounded declaration and ownership syntax for +// languages where the lightweight parser cannot rely on a compiler AST. +type ParsedDeclaration struct { + Name, Type, Kind, ReferenceShape string + Line, ScopeStart, ScopeEnd int + Offset, ScopeOffsetStart int + ScopeOffsetEnd int + AliasSource string + Initializer string + QualifiedOwner string +} + // ParsedAssignment records "name = expr" style statements inside a scope. // Expr is taken from the masked source: string contents are blanked while // interpolated expressions (f-strings, template literals) are preserved. @@ -51,23 +63,31 @@ type ParsedStatement struct { // ParsedFunction is a lightweight AST node for one function or method. type ParsedFunction struct { - Name string - StartLine int - EndLine int - Signature string - Params []ParsedParam - Statements []ParsedStatement - Assignments []ParsedAssignment - Calls []ParsedCall - Nested []*ParsedFunction + Name string + Language string + StartLine int + EndLine int + Signature string + Params []ParsedParam + Statements []ParsedStatement + Assignments []ParsedAssignment + Calls []ParsedCall + Declarations []ParsedDeclaration + QualifiedOwner string + Nested []*ParsedFunction + DefinitionOffset int + sourceStart int + bodyOpen int + bodyEnd int } // ParsedFile is the result of parsing one source file. type ParsedFile struct { - Language string - Source string - Masked string - Imports []ParsedImport - Functions []*ParsedFunction - Module *ParsedFunction + Language string + Source string + Masked string + Imports []ParsedImport + Declarations []ParsedDeclaration + Functions []*ParsedFunction + Module *ParsedFunction } diff --git a/internal/codeguard/report/diagnostics_test.go b/internal/codeguard/report/diagnostics_test.go index 72a622f2..87248e60 100644 --- a/internal/codeguard/report/diagnostics_test.go +++ b/internal/codeguard/report/diagnostics_test.go @@ -25,3 +25,27 @@ func TestSARIFIncludesDiagnosticsAndMarksOperationalFailure(t *testing.T) { } } } + +func TestSARIFStructuralUnresolvedDiagnosticsAreNotFindingFingerprints(t *testing.T) { + t.Parallel() + report := core.Report{Sections: []core.SectionResult{{ID: "quality", Diagnostics: []core.Diagnostic{ + {ID: "quality.structural-unresolved-symbols", Level: "info", Kind: "analysis", Message: "unresolved mutation symbols", Metadata: map[string]string{"language": "c++", "count": "1"}}, + {ID: "quality.structural-unresolved-symbols", Level: "info", Kind: "analysis", Message: "unresolved mutation symbols", Metadata: map[string]string{"language": "go", "count": "2"}}, + }}}} + var output bytes.Buffer + if err := writeSARIF(&output, report); err != nil { + t.Fatal(err) + } + text := output.String() + if strings.Count(text, `"ruleId": "quality.structural-unresolved-symbols"`) != 2 { + t.Fatalf("SARIF must retain separate per-language diagnostics:\n%s", text) + } + for _, want := range []string{`"language": "c++"`, `"count": "1"`, `"language": "go"`, `"count": "2"`} { + if !strings.Contains(text, want) { + t.Fatalf("SARIF missing %s:\n%s", want, text) + } + } + if strings.Contains(text, `"partialFingerprints"`) { + t.Fatalf("diagnostics must not become baselinable finding fingerprints:\n%s", text) + } +} diff --git a/internal/codeguard/runner/support/context_fingerprint.go b/internal/codeguard/runner/support/context_fingerprint.go index c778447f..9e8e2d60 100644 --- a/internal/codeguard/runner/support/context_fingerprint.go +++ b/internal/codeguard/runner/support/context_fingerprint.go @@ -13,12 +13,12 @@ const contextFingerprintRadius = 2 // contextFingerprint returns sha256(ruleID|path|normalizedContext), where the // normalized context is the finding's source line plus up to // contextFingerprintRadius lines on each side, each with runs of whitespace -// collapsed to a single space and trimmed. Unlike the legacy fingerprint it +// collapsed to a single space and trimmed. Unlike the exact fingerprint it // does not embed the line number, so unrelated edits that merely shift the // finding up or down the file leave it unchanged. It returns "" when no source // context is available (line <= 0, the path resolves under no target, the file // is unreadable, or the line is past end of file), letting the caller fall -// back to the legacy fingerprint. +// back to the exact fingerprint. func contextFingerprint(sc Context, ruleID string, normalizedPath string, line int) string { return findingContextFingerprint(sc, ruleID, normalizedPath, line, true) } @@ -27,6 +27,24 @@ func contentFingerprint(sc Context, ruleID string, normalizedPath string, line i return findingContextFingerprint(sc, ruleID, normalizedPath, line, false) } +// sourceIdentity returns the normalized source line at a finding location. +// It is intentionally narrower than contextFingerprint: exact identity keeps +// its line coordinate while remaining independent of diagnostic prose. +func sourceIdentity(sc Context, normalizedPath string, line int) string { + if line <= 0 || normalizedPath == "" { + return "" + } + data, ok := findingSource(sc, normalizedPath) + if !ok { + return "" + } + lines := strings.Split(string(data), "\n") + if line > len(lines) { + return "" + } + return strings.Join(strings.Fields(lines[line-1]), " ") +} + func findingContextFingerprint(sc Context, ruleID string, normalizedPath string, line int, includePath bool) string { if line <= 0 || normalizedPath == "" { return "" diff --git a/internal/codeguard/runner/support/findings_section.go b/internal/codeguard/runner/support/findings_section.go index 0fc22d19..94e9057a 100644 --- a/internal/codeguard/runner/support/findings_section.go +++ b/internal/codeguard/runner/support/findings_section.go @@ -29,11 +29,21 @@ func NewFinding(sc Context, input FindingInput) core.Finding { input.Level = meta.DefaultLevel } input.Level = NormalizedSeverity(input.Level) - sum := sha256.Sum256([]byte(strings.Join([]string{input.RuleID, normalizedPath, strconv.Itoa(input.Line), input.Message}, "|"))) - legacy := hex.EncodeToString(sum[:]) + // Exact identity deliberately excludes diagnostic prose and evidence. The + // version prefix makes future identity-shape migrations explicit, while the + // normalized source line distinguishes multiple findings at the same + // rule/path/line without coupling identity to Message or Metadata. + sum := sha256.Sum256([]byte(strings.Join([]string{ + "codeguard-finding/v2", + input.RuleID, + normalizedPath, + strconv.Itoa(input.Line), + sourceIdentity(sc, normalizedPath, input.Line), + }, "|"))) + exact := hex.EncodeToString(sum[:]) contextFP := contextFingerprint(sc, input.RuleID, normalizedPath, input.Line) if contextFP == "" { - contextFP = legacy + contextFP = exact } contentFP := contentFingerprint(sc, input.RuleID, normalizedPath, input.Line) return core.Finding{ @@ -49,13 +59,21 @@ func NewFinding(sc Context, input FindingInput) core.Finding { Path: normalizedPath, Line: input.Line, Column: input.Column, - Fingerprint: legacy, + Fingerprint: exact, ContextFingerprint: contextFP, ContentFingerprint: contentFP, Metadata: cloneMetadata(input.Metadata), } } +// preV2ExactFingerprint reconstructs the message-based exact identity written +// before source-derived v2 fingerprints. It is used only as a baseline lookup +// key; new findings never expose or persist this compatibility identity. +func preV2ExactFingerprint(ruleID string, normalizedPath string, line int, message string) string { + sum := sha256.Sum256([]byte(strings.Join([]string{ruleID, normalizedPath, strconv.Itoa(line), message}, "|"))) + return hex.EncodeToString(sum[:]) +} + func cloneMetadata(metadata map[string]string) map[string]string { if len(metadata) == 0 { return nil diff --git a/internal/codeguard/runner/support/suppressions.go b/internal/codeguard/runner/support/suppressions.go index 19c98e61..183a1a7a 100644 --- a/internal/codeguard/runner/support/suppressions.go +++ b/internal/codeguard/runner/support/suppressions.go @@ -48,6 +48,10 @@ func MatchSuppression(sc Context, finding core.Finding) *core.Suppression { if entry, ok := sc.Baseline[finding.Fingerprint]; ok { return &core.Suppression{Kind: SuppressionReasonBaseline, Match: "exact", BaselineFingerprint: entry.Fingerprint} } + preV2Exact := preV2ExactFingerprint(finding.RuleID, finding.Path, finding.Line, finding.Message) + if entry, ok := sc.Baseline[preV2Exact]; ok { + return &core.Suppression{Kind: SuppressionReasonBaseline, Match: "exact", BaselineFingerprint: entry.Fingerprint} + } // The context fingerprint deliberately omits the line number, so two // identical findings in the same file (same rule, same normalized // surrounding source, different locations) collide on it. For diff --git a/tests/checks/fingerprint_baseline_test.go b/tests/checks/fingerprint_baseline_test.go index 2903da2f..ecc83216 100644 --- a/tests/checks/fingerprint_baseline_test.go +++ b/tests/checks/fingerprint_baseline_test.go @@ -2,10 +2,17 @@ package checks_test import ( "context" + "crypto/sha256" + "encoding/hex" "os" "path/filepath" + "strconv" + "strings" "testing" + codeguardcli "github.com/devr-tools/codeguard/internal/cli" + "github.com/devr-tools/codeguard/internal/codeguard/core" + runnersupport "github.com/devr-tools/codeguard/internal/codeguard/runner/support" "github.com/devr-tools/codeguard/pkg/codeguard" ) @@ -31,16 +38,16 @@ func promptOnlyConfig(dir string, name string) codeguard.Config { return cfg } -func findFindingByRule(t *testing.T, report codeguard.Report, ruleID string) codeguard.Finding { +func findPromptSecretFinding(t *testing.T, report codeguard.Report) codeguard.Finding { t.Helper() for _, section := range report.Sections { for _, finding := range section.Findings { - if finding.RuleID == ruleID { + if finding.RuleID == "prompts.secret-interpolation" { return finding } } } - t.Fatalf("finding for rule %q not found", ruleID) + t.Fatal("prompts.secret-interpolation finding not found") return codeguard.Finding{} } @@ -59,7 +66,7 @@ func TestBaselineSuppressesFindingAfterLineShift(t *testing.T) { t.Fatalf("run: %v", err) } assertSectionStatus(t, report, "AI Prompts", "fail") - before := findFindingByRule(t, report, "prompts.secret-interpolation") + before := findPromptSecretFinding(t, report) if before.ContextFingerprint == "" { t.Fatal("expected finding to carry a context fingerprint") } @@ -79,7 +86,7 @@ func TestBaselineSuppressesFindingAfterLineShift(t *testing.T) { if err != nil { t.Fatalf("run after edit: %v", err) } - after := findFindingByRule(t, report, "prompts.secret-interpolation") + after := findPromptSecretFinding(t, report) if after.Fingerprint == before.Fingerprint { t.Error("expected legacy fingerprint to change when the finding line shifts") } @@ -158,10 +165,19 @@ func TestLegacyOnlyBaselineStillSuppresses(t *testing.T) { } assertSectionStatus(t, report, "AI Prompts", "fail") - entries := codeguard.BaselineEntriesFromReport(report) - for i := range entries { - entries[i].ContextFingerprint = "" - } + current := findPromptSecretFinding(t, report) + preV2Sum := sha256.Sum256([]byte(strings.Join([]string{ + current.RuleID, + filepath.ToSlash(current.Path), + strconv.Itoa(current.Line), + current.Message, + }, "|"))) + entries := []codeguard.BaselineEntry{{ + Fingerprint: hex.EncodeToString(preV2Sum[:]), + RuleID: current.RuleID, + Path: current.Path, + Message: current.Message, + }} baselinePath := filepath.Join(dir, "codeguard-baseline.json") if writeErr := codeguard.WriteBaselineFile(baselinePath, entries); writeErr != nil { t.Fatalf("write baseline: %v", writeErr) @@ -177,3 +193,139 @@ func TestLegacyOnlyBaselineStillSuppresses(t *testing.T) { t.Fatal("expected legacy-only baseline entries to keep suppressing the finding") } } + +// A production change that removes context/content fallback from baseline +// matching must fail this test. The exact value deliberately uses the prior +// rule|path|line|message identity while the current finding carries changed +// prose and evidence. +func TestBaselineFallsBackFromPriorExactFingerprintAfterEvidenceChange(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "prompts", "system.prompt"), shiftPromptBody) + cfg := promptOnlyConfig(dir, "fingerprint-evidence-test") + + report, err := codeguard.Run(context.Background(), cfg) + if err != nil { + t.Fatalf("run: %v", err) + } + current := findPromptSecretFinding(t, report) + priorMessage := "prior finding prose with different evidence" + priorSum := sha256.Sum256([]byte(strings.Join([]string{ + current.RuleID, + filepath.ToSlash(current.Path), + strconv.Itoa(current.Line), + priorMessage, + }, "|"))) + entry := codeguard.BaselineEntry{ + Fingerprint: hex.EncodeToString(priorSum[:]), + ContextFingerprint: current.ContextFingerprint, + ContentFingerprint: current.ContentFingerprint, + RuleID: current.RuleID, + Path: current.Path, + Message: priorMessage, + } + if entry.Fingerprint == current.Fingerprint { + t.Fatal("fixture prior exact fingerprint unexpectedly matches current exact fingerprint") + } + baselinePath := filepath.Join(dir, "codeguard-baseline.json") + if writeErr := codeguard.WriteBaselineFile(baselinePath, []codeguard.BaselineEntry{entry}); writeErr != nil { + t.Fatalf("write baseline: %v", writeErr) + } + + cfg.Baseline.Path = baselinePath + report, err = codeguard.RunWithOptions(context.Background(), cfg, codeguard.ScanOptions{Mode: codeguard.ScanModeFull, IncludeSuppressed: true}) + if err != nil { + t.Fatalf("run with baseline: %v", err) + } + assertSectionStatus(t, report, "AI Prompts", "pass") + assertSuppressionMatch(t, report, "context") +} + +func TestBaselineAuditAndPruneKeepWordingIndependentMatchesAndExposeResolvedFalsePositives(t *testing.T) { + dir := t.TempDir() + const source = "package sample\n\nfunc Current(input *State) int {\n\tinput.Value++\n\treturn input.Value\n}\n" + writeFile(t, filepath.Join(dir, "state.go"), source) + sc := runnersupport.Context{Cfg: core.Config{Targets: []core.TargetConfig{{Name: "repo", Path: dir}}}} + prior := runnersupport.NewFinding(sc, runnersupport.FindingInput{ + RuleID: "function.hidden-mutation", Path: "state.go", Line: 4, + Message: "function Current mutates argument state", Confidence: "medium", + Metadata: map[string]string{"mutation_target": "argument", "origin": "caller_owned"}, + }) + current := runnersupport.NewFinding(sc, runnersupport.FindingInput{ + RuleID: "function.hidden-mutation", Path: "state.go", Line: 4, + Message: "Current changes caller-owned state", Confidence: "high", + Metadata: map[string]string{"mutation_target": "argument", "effect_kind": "shared_state", "origin": "caller_owned"}, + }) + if prior.Fingerprint != current.Fingerprint || prior.ContextFingerprint != current.ContextFingerprint || prior.ContentFingerprint != current.ContentFingerprint { + t.Fatalf("wording/evidence changed source identity:\nprior=%#v\ncurrent=%#v", prior, current) + } + priorExact := sha256.Sum256([]byte(strings.Join([]string{ + prior.RuleID, + filepath.ToSlash(prior.Path), + strconv.Itoa(prior.Line), + prior.Message, + }, "|"))) + active := core.BaselineEntry{ + Fingerprint: hex.EncodeToString(priorExact[:]), + ContextFingerprint: prior.ContextFingerprint, + ContentFingerprint: prior.ContentFingerprint, + RuleID: prior.RuleID, + Path: prior.Path, + Message: prior.Message, + } + resolvedFalsePositive := core.BaselineEntry{ + Fingerprint: "resolved-structural-false-positive", + RuleID: "function.hidden-mutation", + Path: "helpers.go", + Message: "local value construction was misclassified as shared mutation", + } + audit := codeguardcli.Audit(core.BaselineFile{Entries: []core.BaselineEntry{active, resolvedFalsePositive}}, []core.Finding{current}, codeguardcli.Options{}) + if audit.Counts.ActiveContext != 1 || audit.Counts.Active != 1 || audit.Counts.Stale != 1 || audit.Counts.Final != 1 { + t.Fatalf("audit counts = %#v, want one context-active and one stale entry", audit.Counts) + } + retained := audit.ActiveEntries() + if len(retained) != 1 || retained[0].Fingerprint != active.Fingerprint { + t.Fatalf("prune retained = %#v, want only wording-independent active entry", retained) + } + prunable := audit.PrunableEntries() + if len(prunable) != 1 || prunable[0].Fingerprint != resolvedFalsePositive.Fingerprint { + t.Fatalf("prunable = %#v, want only resolved structural false positive", prunable) + } +} + +func TestPreV2ExactOnlyBaselineSuppressesFindingWithoutSourceContext(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "short.go"), []byte("package sample\n"), 0o644); err != nil { + t.Fatalf("write short source: %v", err) + } + cases := []struct { + name string + path string + line int + }{ + {name: "pathless", line: 0}, + {name: "unreadable", path: "missing.go", line: 3}, + {name: "invalid line", path: "short.go", line: 99}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + const message = "pre-v2 diagnostic prose" + sum := sha256.Sum256([]byte(strings.Join([]string{"test.rule", filepath.ToSlash(tc.path), strconv.Itoa(tc.line), message}, "|"))) + preV2Exact := hex.EncodeToString(sum[:]) + entry := core.BaselineEntry{Fingerprint: preV2Exact, RuleID: "test.rule", Path: tc.path, Message: message} + sc := runnersupport.Context{ + Cfg: core.Config{Targets: []core.TargetConfig{{Name: "repo", Path: dir}}}, + Baseline: map[string]core.BaselineEntry{preV2Exact: entry}, + } + finding := runnersupport.NewFinding(sc, runnersupport.FindingInput{ + RuleID: "test.rule", Path: tc.path, Line: tc.line, Message: message, + }) + if finding.ContextFingerprint != finding.Fingerprint || finding.ContentFingerprint != "" { + t.Fatalf("fixture unexpectedly has source fallback: %#v", finding) + } + suppression := runnersupport.MatchSuppression(sc, finding) + if suppression == nil || suppression.Kind != runnersupport.SuppressionReasonBaseline || suppression.Match != "exact" || suppression.BaselineFingerprint != preV2Exact { + t.Fatalf("suppression = %#v, want pre-v2 exact baseline match", suppression) + } + }) + } +} diff --git a/tests/checks/function_effect_evidence_test.go b/tests/checks/function_effect_evidence_test.go index 5a0720d1..d5737a68 100644 --- a/tests/checks/function_effect_evidence_test.go +++ b/tests/checks/function_effect_evidence_test.go @@ -1,9 +1,12 @@ package checks_test import ( + "fmt" "path/filepath" "strings" "testing" + + "github.com/devr-tools/codeguard/pkg/codeguard" ) func TestFunctionEffectsAllowLocalConstructionAndRepositoryHydration(t *testing.T) { @@ -32,6 +35,310 @@ type User struct { Name string }; type Rows interface { Scan(...any) error }`}, } } +func TestFunctionEffectsDoNotPromoteUnknownFactoryResults(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "factory.go"), `package sample + +type Item struct{ Field int } + +var item *Item + +func opaqueFactory() *Item { return nil } + +func ReadValue() int { + item := opaqueFactory() + item.Field = 1 + return item.Field +} +`) + + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") + assertUnresolvedDiagnosticCount(t, report, "go", "1") +} + +func TestFunctionEffectsGoPackageIndexResolvesCrossFileGlobal(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "state.go"), `package sample +type State struct{ Value int } +var sharedState *State +`) + writeFile(t, filepath.Join(dir, "current.go"), `package sample +func CurrentGlobal() int { sharedState.Value++; return sharedState.Value } +`) + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + finding := findFinding(t, report, "Code Quality", "function.hidden-mutation") + if finding.Metadata["mutation_target"] != "global" || finding.Metadata["origin"] != "shared" { + t.Fatalf("metadata = %v, want cross-file global/shared evidence", finding.Metadata) + } +} + +func TestFunctionEffectsGoUnresolvedOperationIsDiagnosticOnly(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "unresolved.go"), `package sample +func CurrentUnknown() int { mystery.Update(); return 1 } +`) + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") + assertUnresolvedDiagnosticCount(t, report, "go", "1") +} + +func TestFunctionEffectsGoScopeAndClosureOrigins(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "globals.go"), `package sample +type State struct{ Value int } +var state *State +`) + writeFile(t, filepath.Join(dir, "scopes.go"), `package sample +import storage "example.com/storage" + +func GetLocal() int { + state := &State{} + state.Value = 1 + storage.Update() + if true { state := &State{}; state.Value = 2 } + func() { state := &State{}; state.Value = 3 }() + return state.Value +} +`) + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") + + capturedDir := t.TempDir() + writeFile(t, filepath.Join(capturedDir, "closure.go"), `package sample +type State struct{ Value int } +func CurrentCaptured(input *State) int { mutate := func() { input.Value = 1 }; mutate(); return input.Value } +`) + capturedReport := runQualityPrecisionScan(t, qualityPrecisionConfig(capturedDir)) + finding := findFinding(t, capturedReport, "Code Quality", "function.hidden-mutation") + if finding.Metadata["mutation_target"] != "argument" || finding.Metadata["origin"] != "caller_owned" { + t.Fatalf("metadata = %v, want captured argument/caller-owned evidence", finding.Metadata) + } +} + +func TestFunctionEffectsGoReferenceContentOrigins(t *testing.T) { + cases := []struct { + name string + body string + finding bool + }{ + {name: "value field", body: `input.Name = "local"; return input.Name`}, + {name: "map field", body: `input.Meta["status"] = "ready"; return input.Meta["status"]`, finding: true}, + {name: "slice field", body: `input.Tags[0] = "ready"; return input.Tags[0]`, finding: true}, + {name: "pointer field", body: `input.Node.Name = "ready"; return input.Node.Name`, finding: true}, + {name: "interface map", body: `input.Any.(map[string]string)["status"] = "ready"; return "ready"`, finding: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "payload.go"), `package sample +type Node struct{ Name string } +type Payload struct { + Name string + Tags []string + Meta map[string]string + Node *Node + Any any +} +func CurrentPayload(input Payload) string { `+tc.body+` } +`) + report := runQualityPrecisionScan(t, qualityPrecisionConfig(dir)) + if !tc.finding { + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + return + } + finding := findFinding(t, report, "Code Quality", "function.hidden-mutation") + if finding.Metadata["mutation_target"] != "argument" || finding.Metadata["origin"] != "caller_owned" { + t.Fatalf("metadata = %v, want argument/caller-owned evidence", finding.Metadata) + } + }) + } +} + +func TestFunctionEffectsCppStructuralOrigins(t *testing.T) { + cases := []struct { + name, source, target, origin string + finding bool + }{ + {name: "value parameter", source: `int current(Token input) { input.value++; return input.value; }`}, + {name: "moved local", source: `int current() { Token item{}; auto moved = std::move(item); moved.value++; return moved.value; }`}, + {name: "reference parameter", source: `int current(Token& input) { input.value++; return input.value; }`, finding: true, target: "argument", origin: "caller_owned"}, + {name: "reference capture", source: `int current(Token& input) { auto work = [&input]() { input.value++; }; work(); return input.value; }`, finding: true, target: "argument", origin: "caller_owned"}, + {name: "receiver member", source: `struct Counter { + Token state; + int current() { state.value++; return state.value; } +};`, finding: true, target: "receiver", origin: "caller_owned"}, + {name: "proven global", source: `Token shared; +int current() { shared.value++; return shared.value; }`, finding: true, target: "global", origin: "shared"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "origin.cpp"), tc.source) + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "cpp")) + if !tc.finding { + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.command-query-mix") + return + } + finding := findFinding(t, report, "Code Quality", "function.hidden-mutation") + if finding.Metadata["mutation_target"] != tc.target || finding.Metadata["origin"] != tc.origin { + t.Fatalf("metadata = %v, want target=%q origin=%q", finding.Metadata, tc.target, tc.origin) + } + }) + } +} + +func TestStructuralOriginNamedSourceFixturesRetainLocalOwnership(t *testing.T) { + cases := []struct { + name string + language string + diagnosticLanguage string + unresolvedCount string + fixtureDir string + functions []string + }{ + { + name: "go crumb helpers", + language: "go", + diagnosticLanguage: "go", + unresolvedCount: "1", + fixtureDir: filepath.Join("testdata", "structural_origin", "go"), + functions: []string{ + "normalizeBadgeRule", + "normalizeUpdateBadgeInput", + "normalizePreviewBadgeInput", + "normalizeRecomputeBadgeInput", + "normalizeBrandCampaignPatchInput", + "normalizePlaceUpdateFields", + "OvertureDatasetPath", + "NewStaticOvertureDivisionResolver", + "duckDBDivisionHierarchy", + }, + }, + { + name: "cpp crumb helpers", + language: "cpp", + diagnosticLanguage: "c++", + unresolvedCount: "1", + fixtureDir: filepath.Join("testdata", "structural_origin", "cpp"), + functions: []string{"BuildLocalResponse", "DbRow::integer"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(tc.fixtureDir, tc.language)) + for _, function := range tc.functions { + assertFunctionHasNoStructuralFinding(t, report, function) + } + assertUnresolvedDiagnosticCount(t, report, tc.diagnosticLanguage, tc.unresolvedCount) + }) + } +} + +func TestStructuralOriginNegativeFixturesRetainProvenMutations(t *testing.T) { + cases := []struct { + name, language, fixtureDir, function, target, origin string + }{ + {"go receiver", "go", filepath.Join("testdata", "structural_origin", "go"), "CurrentReceiver", "receiver", "caller_owned"}, + {"go argument", "go", filepath.Join("testdata", "structural_origin", "go"), "CurrentArgument", "argument", "caller_owned"}, + {"go global", "go", filepath.Join("testdata", "structural_origin", "go"), "CurrentGlobal", "global", "shared"}, + {"go escaped", "go", filepath.Join("testdata", "structural_origin", "go"), "CurrentEscaped", "escaped", "shared"}, + {"cpp receiver", "cpp", filepath.Join("testdata", "structural_origin", "cpp"), "CurrentReceiver", "receiver", "caller_owned"}, + {"cpp reference", "cpp", filepath.Join("testdata", "structural_origin", "cpp"), "CurrentReference", "argument", "caller_owned"}, + {"cpp global", "cpp", filepath.Join("testdata", "structural_origin", "cpp"), "CurrentGlobal", "global", "shared"}, + {"cpp escaped", "cpp", filepath.Join("testdata", "structural_origin", "cpp"), "CurrentEscaped", "escaped", "shared"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(tc.fixtureDir, tc.language)) + finding := findStructuralFindingForFunction(t, report, "function.hidden-mutation", tc.function) + if finding.RuleID != "function.hidden-mutation" { + t.Fatalf("rule_id = %q, want function.hidden-mutation; finding=%#v", finding.RuleID, finding) + } + for key, want := range map[string]string{ + "mutation_target": tc.target, + "effect_kind": "shared_state", + "origin": tc.origin, + } { + if got := finding.Metadata[key]; got != want { + t.Fatalf("%s = %q, want %q; finding=%#v", key, got, want, finding) + } + } + }) + } +} + +func assertFunctionHasNoStructuralFinding(t *testing.T, report codeguard.Report, function string) { + t.Helper() + for _, section := range report.Sections { + for _, finding := range section.Findings { + if isStructuralEffectRule(finding.RuleID) && strings.Contains(finding.Message, function) { + t.Fatalf("function %q unexpectedly has %s at %s:%d: %s", function, finding.RuleID, finding.Path, finding.Line, finding.Message) + } + } + } +} + +func findStructuralFindingForFunction(t *testing.T, report codeguard.Report, ruleID string, function string) codeguard.Finding { + t.Helper() + for _, section := range report.Sections { + for _, finding := range section.Findings { + if finding.RuleID == ruleID && strings.Contains(finding.Message, function) { + return finding + } + } + } + t.Fatalf("finding %q for function %q not found", ruleID, function) + return codeguard.Finding{} +} + +func isStructuralEffectRule(ruleID string) bool { + switch ruleID { + case "function.hidden-mutation", "function.command-query-mix", "quality.hidden-side-effect": + return true + default: + return false + } +} + +func assertUnresolvedDiagnosticCount(t *testing.T, report codeguard.Report, language string, count string) { + t.Helper() + var diagnostics []string + for _, section := range report.Sections { + if section.Name != "Code Quality" { + continue + } + for _, diagnostic := range section.Diagnostics { + diagnostics = append(diagnostics, fmt.Sprintf("%s:%v", diagnostic.ID, diagnostic.Metadata)) + } + for _, diagnostic := range section.Diagnostics { + if diagnostic.ID == "quality.structural-unresolved-symbols" && diagnostic.Metadata["language"] == language && diagnostic.Metadata["count"] == count { + return + } + } + } + t.Fatalf("expected unresolved-symbol diagnostic language=%q count=%q: %#v", language, count, diagnostics) +} + +func assertUnresolvedDiagnosticAbsent(t *testing.T, report codeguard.Report, language string) { + t.Helper() + for _, section := range report.Sections { + for _, diagnostic := range section.Diagnostics { + if diagnostic.ID == "quality.structural-unresolved-symbols" && diagnostic.Metadata["language"] == language { + t.Fatalf("unexpected unresolved-symbol diagnostic language=%q: %#v", language, diagnostic) + } + } + } +} + func TestFunctionEffectsReportOwnedAndObservableMutationEvidence(t *testing.T) { cases := []struct{ name, language, file, source, target, effect, origin string }{ {"go argument alias", "go", "mutation.go", `package sample diff --git a/tests/checks/function_hidden_mutation_noise_test.go b/tests/checks/function_hidden_mutation_noise_test.go index e1f70297..d7bc2a52 100644 --- a/tests/checks/function_hidden_mutation_noise_test.go +++ b/tests/checks/function_hidden_mutation_noise_test.go @@ -447,7 +447,7 @@ func TestFunctionHiddenMutationStillWarnsForCollaboratorMutationWithLocalPayload assertFindingRulePresent(t, report, "Code Quality", "function.hidden-mutation") } -func TestFunctionHiddenMutationStillWarnsForLocalCollaboratorConstruction(t *testing.T) { +func TestFunctionHiddenMutationRetainsUnresolvedLocalCollaboratorConstruction(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "mutation.ts"), strings.Join([]string{ "export function prepareDigest(input: Input) {", @@ -462,7 +462,8 @@ func TestFunctionHiddenMutationStillWarnsForLocalCollaboratorConstruction(t *tes report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) - assertFindingRulePresent(t, report, "Code Quality", "function.hidden-mutation") + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertUnresolvedDiagnosticCount(t, report, "typescript", "1") } func TestFunctionHiddenMutationAllowsNextRouteHandlerNames(t *testing.T) { @@ -654,7 +655,7 @@ func TestFunctionHiddenMutationAllowsReactComponentsAndHooksAsBoundaries(t *test } } -func TestFunctionHiddenMutationStillWarnsForReactNativeCollaboratorMutation(t *testing.T) { +func TestFunctionHiddenMutationStillWarnsForReactNativeCollaboratorCapture(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "apps/mobile/src/screens/ProfileScreen.tsx"), strings.Join([]string{ "import { Pressable, Text } from 'react-native';", @@ -674,6 +675,170 @@ func TestFunctionHiddenMutationStillWarnsForReactNativeCollaboratorMutation(t *t assertFindingRulePresent(t, report, "Code Quality", "function.hidden-mutation") } +func TestFunctionHiddenMutationPreservesCapturedOuterLocalOwnership(t *testing.T) { + t.Run("parameter alias remains caller owned", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "capture.ts"), strings.Join([]string{ + "export function prepareUser(repo: Repository, user: User) {", + " const collaborator = repo;", + " function loadUser() {", + " collaborator.save(user);", + " return user;", + " }", + " return loadUser();", + "}", + "interface Repository { save(input: User): void }", + "interface User { name: string }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + assertFindingRulePresent(t, report, "Code Quality", "function.hidden-mutation") + }) + + t.Run("fresh local remains local", func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "capture.ts"), strings.Join([]string{ + "export function buildValues() {", + " const values = new Map();", + " function readValues() {", + " values.set('key', 'value');", + " return values;", + " }", + " return readValues();", + "}", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertUnresolvedDiagnosticAbsent(t, report, "typescript") + }) +} + +func TestFunctionHiddenMutationUsesTypeScriptLexicalCaptureScope(t *testing.T) { + tests := []struct { + name string + source []string + findings int + unresolved string + }{ + { + name: "disjoint block declaration is not visible", + source: []string{ + "export function prepareUser(repo: Repository, user: User, enabled: boolean) {", + " if (enabled) {", + " const collaborator = repo;", + " }", + " if (!enabled) {", + " function loadUser() {", + " collaborator.save(user);", + " return user;", + " }", + " return loadUser();", + " }", + " return user;", + "}", + "class Repository { save(_input: User): void {} }", + "interface User { name: string }", + }, + unresolved: "1", + }, + { + name: "enclosing block declaration is visible", + source: []string{ + "export function prepareUser(repo: Repository, user: User, enabled: boolean) {", + " if (enabled) {", + " const collaborator = repo;", + " function loadUser() {", + " collaborator.save(user);", + " return user;", + " }", + " return loadUser();", + " }", + " return user;", + "}", + "class Repository { save(_input: User): void {} }", + "interface User { name: string }", + }, + findings: 1, + }, + { + name: "later declaration is not propagated", + source: []string{ + "export function prepareUser(repo: Repository, user: User) {", + " function loadUser() {", + " collaborator.save(user);", + " return user;", + " }", + " const collaborator = repo;", + " return loadUser();", + "}", + "class Repository { save(_input: User): void {} }", + "interface User { name: string }", + }, + unresolved: "1", + }, + { + name: "inner shadow does not replace outer capture", + source: []string{ + "export function prepareUser(repo: Repository, user: User, enabled: boolean) {", + " const collaborator = repo;", + " if (enabled) {", + " const collaborator = new Repository();", + " function loadLocal() {", + " collaborator.save(user);", + " return user;", + " }", + " loadLocal();", + " }", + " function loadOuter() {", + " collaborator.save(user);", + " return user;", + " }", + " return loadOuter();", + "}", + "class Repository { save(_input: User): void {} }", + "interface User { name: string }", + }, + findings: 1, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "capture.ts"), strings.Join(test.source, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + if got := countRuleFindings(report, "Code Quality", "function.hidden-mutation"); got != test.findings { + t.Fatalf("hidden-mutation findings = %d, want %d", got, test.findings) + } + if test.unresolved == "" { + assertUnresolvedDiagnosticAbsent(t, report, "typescript") + } else { + assertUnresolvedDiagnosticCount(t, report, "typescript", test.unresolved) + } + }) + } +} + +func TestFunctionHiddenMutationLeavesUnknownNestedTypeScriptRootUnresolved(t *testing.T) { + dir := t.TempDir() + writeFile(t, filepath.Join(dir, "capture.ts"), strings.Join([]string{ + "export function prepareUser(user: User) {", + " function loadUser() {", + " mystery.save(user);", + " return user;", + " }", + " return loadUser();", + "}", + "interface User { name: string }", + }, "\n")) + + report := runQualityPrecisionScan(t, qualityPrecisionConfigForLanguage(dir, "typescript")) + assertFindingRuleAbsent(t, report, "Code Quality", "function.hidden-mutation") + assertUnresolvedDiagnosticCount(t, report, "typescript", "1") +} + func TestFunctionHiddenMutationAllowsConventionalCommandNames(t *testing.T) { cases := []struct { name string diff --git a/tests/checks/function_precision_test.go b/tests/checks/function_precision_test.go index 08ba5524..db8109ac 100644 --- a/tests/checks/function_precision_test.go +++ b/tests/checks/function_precision_test.go @@ -255,7 +255,7 @@ func TestFunctionHiddenMutationAllowsReactHookLocalStateBoundaries(t *testing.T) } } -func TestFunctionHiddenMutationStillWarnsForHiddenPersistenceInsideReactHook(t *testing.T) { +func TestFunctionHiddenMutationStillWarnsForCapturedPersistenceInsideReactHook(t *testing.T) { dir := t.TempDir() writeFile(t, filepath.Join(dir, "apps/web/src/hooks/use-user.ts"), strings.Join([]string{ "export function useUser(repo: Repository, user: User) {", diff --git a/tests/checks/quality_local_design_test.go b/tests/checks/quality_local_design_test.go index e8dcd98c..d27ddaa7 100644 --- a/tests/checks/quality_local_design_test.go +++ b/tests/checks/quality_local_design_test.go @@ -39,13 +39,14 @@ func TestQualityLocalDesignRules(t *testing.T) { "quality.ambiguous-name", "quality.boolean-argument", "quality.primitive-obsession", - "quality.hidden-side-effect", "quality.mutable-global-state", "quality.redundant-comment", } { assertFindingRulePresent(t, report, "Code Quality", ruleID) assertFindingLevel(t, report, "Code Quality", ruleID, "warn") } + assertFindingRuleAbsent(t, report, "Code Quality", "quality.hidden-side-effect") + assertUnresolvedDiagnosticCount(t, report, "go", "2") } func TestQualityLocalDesignRulesForScriptLanguages(t *testing.T) { diff --git a/tests/checks/testdata/structural_origin/cpp/helpers.cpp b/tests/checks/testdata/structural_origin/cpp/helpers.cpp new file mode 100644 index 00000000..28da5f2f --- /dev/null +++ b/tests/checks/testdata/structural_origin/cpp/helpers.cpp @@ -0,0 +1,63 @@ +struct Response { + int code; +}; + +struct ResponseBuilder { + int code; + void set_code(int value) { code = value; } + Response build() { return Response{code}; } +}; + +Response BuildLocalResponse() { + ResponseBuilder builder{}; + builder.set_code(200); + return builder.build(); +} + +struct DbRow { + int integer(int column) const; +}; + +int DbRow::integer(int column) const { + int value = 0; + value = column; + return value; +} + +struct Token { + int value; +}; + +struct Counter { + Token state; + int CurrentReceiver() { + state.value++; + return state.value; + } +}; + +int CurrentReference(Token& input) { + input.value++; + return input.value; +} + +Token sharedToken; + +int CurrentGlobal() { + sharedToken.value++; + return sharedToken.value; +} + +Token* escapedToken; + +int CurrentEscaped() { + Token item{}; + if (true) escapedToken = &item; + item.value++; + return item.value; +} + +int CurrentUnresolvedCpp() { + mystery.update(); + return 1; +} diff --git a/tests/checks/testdata/structural_origin/go/helpers.go b/tests/checks/testdata/structural_origin/go/helpers.go new file mode 100644 index 00000000..994c9b87 --- /dev/null +++ b/tests/checks/testdata/structural_origin/go/helpers.go @@ -0,0 +1,172 @@ +package structuralorigin + +import ( + "fmt" + "sort" + "strings" +) + +type BadgeDefinitionRule struct { + MetricSource string + MetricName string + Operator string +} + +type BadgeInput struct { + Key string + Name string + Rule BadgeDefinitionRule + SubjectIDs []string +} + +type StringPatch struct { + Set bool + Value *string +} + +type BrandCampaignPatchInput struct { + Name StringPatch + Currency StringPatch +} + +type UpdatePlaceInput struct { + Name StringPatch + Website StringPatch +} + +type OvertureDivision struct { + ID string + Name string + CountryCode string + Hierarchy []string +} + +type StaticOvertureDivisionResolver struct { + divisions []OvertureDivision +} + +type duckDBOvertureDivisionRow struct { + Name string + CountryCode string + RegionCode string +} + +func normalizeBadgeRule(rule BadgeDefinitionRule) BadgeDefinitionRule { + rule.MetricSource = strings.ToLower(strings.TrimSpace(rule.MetricSource)) + rule.MetricName = strings.TrimSpace(rule.MetricName) + rule.Operator = strings.ToLower(strings.TrimSpace(rule.Operator)) + return rule +} + +func normalizeUpdateBadgeInput(input BadgeInput) BadgeInput { + input.Key = strings.ToLower(strings.TrimSpace(input.Key)) + input.Name = strings.TrimSpace(input.Name) + input.Rule = normalizeBadgeRule(input.Rule) + return input +} + +func normalizePreviewBadgeInput(input BadgeInput) BadgeInput { + input.Key = strings.ToLower(strings.TrimSpace(input.Key)) + input.SubjectIDs = append([]string(nil), input.SubjectIDs...) + return input +} + +func normalizeRecomputeBadgeInput(input BadgeInput) BadgeInput { + input.SubjectIDs = append([]string(nil), input.SubjectIDs...) + return input +} + +func normalizeBrandCampaignPatchInput(input BrandCampaignPatchInput) BrandCampaignPatchInput { + if input.Name.Set && input.Name.Value != nil { + value := strings.TrimSpace(*input.Name.Value) + input.Name.Value = &value + } + if input.Currency.Set && input.Currency.Value != nil { + currency := strings.ToUpper(strings.TrimSpace(*input.Currency.Value)) + input.Currency.Value = ¤cy + } + return input +} + +func normalizePlaceUpdateFields(input UpdatePlaceInput) UpdatePlaceInput { + normalized := input + if normalized.Name.Set && normalized.Name.Value != nil { + value := strings.TrimSpace(*normalized.Name.Value) + normalized.Name.Value = &value + } + if normalized.Website.Set && normalized.Website.Value != nil { + value := strings.TrimSpace(*normalized.Website.Value) + normalized.Website.Value = &value + } + return normalized +} + +func OvertureDatasetPath(release string, theme string, typ string) string { + release = strings.TrimSpace(release) + theme = strings.TrimSpace(theme) + typ = strings.TrimSpace(typ) + return fmt.Sprintf("s3://release/%s/theme=%s/type=%s/*", release, theme, typ) +} + +func NewStaticOvertureDivisionResolver(divisions []OvertureDivision) *StaticOvertureDivisionResolver { + copied := append([]OvertureDivision(nil), divisions...) + for i := range copied { + copied[i].ID = strings.TrimSpace(copied[i].ID) + copied[i].Name = strings.TrimSpace(copied[i].Name) + } + sort.SliceStable(copied, func(i int, j int) bool { + return copied[i].Name < copied[j].Name + }) + return &StaticOvertureDivisionResolver{divisions: copied} +} + +func duckDBDivisionHierarchy(row duckDBOvertureDivisionRow) []string { + values := []string{row.CountryCode, row.RegionCode, row.Name} + hierarchy := make([]string, 0, len(values)) + seen := map[string]bool{} + for _, value := range values { + value = strings.TrimSpace(value) + key := strings.ToLower(value) + if value == "" || seen[key] { + continue + } + seen[key] = true + hierarchy = append(hierarchy, value) + } + return hierarchy +} + +type Counter struct { + value int +} + +func (counter *Counter) CurrentReceiver() int { + counter.value++ + return counter.value +} + +func CurrentArgument(counter *Counter) int { + counter.value++ + return counter.value +} + +var sharedCounter Counter + +func CurrentGlobal() int { + sharedCounter.value++ + return sharedCounter.value +} + +var escapedCounter *Counter + +func CurrentEscaped() *Counter { + counter := &Counter{} + escapedCounter = counter + counter.value++ + return counter +} + +func CurrentUnresolvedGo() int { + mystery.Update() + return 1 +} diff --git a/tests/support/context_fingerprint_test.go b/tests/support/context_fingerprint_test.go index 30b136ac..47833f87 100644 --- a/tests/support/context_fingerprint_test.go +++ b/tests/support/context_fingerprint_test.go @@ -47,7 +47,7 @@ func contextFingerprintFinding(t *testing.T, content string, rel string, line in func TestContextFingerprintNormalization(t *testing.T) { base := contextFingerprintFinding(t, contextFingerprintBase, "src/app.go", 3) if base.ContextFingerprint == "" || base.ContextFingerprint == base.Fingerprint { - t.Fatalf("base finding must have a distinct context fingerprint, got %q (legacy %q)", base.ContextFingerprint, base.Fingerprint) + t.Fatalf("base finding must have a distinct context fingerprint, got %q (exact %q)", base.ContextFingerprint, base.Fingerprint) } cases := []struct { @@ -94,13 +94,13 @@ func TestContextFingerprintNormalization(t *testing.T) { wantSame: false, }, { - name: "line zero falls back to legacy", + name: "line zero falls back to exact", content: contextFingerprintBase, line: 0, wantFallback: true, }, { - name: "line past end of file falls back to legacy", + name: "line past end of file falls back to exact", content: contextFingerprintBase, line: 99, wantFallback: true, @@ -121,12 +121,12 @@ func assertContextFingerprintCase(t *testing.T, base core.Finding, content strin finding := contextFingerprintFinding(t, content, "src/app.go", line) if want.wantFallback { if finding.ContextFingerprint != finding.Fingerprint { - t.Fatalf("expected fallback to legacy fingerprint, got context %q legacy %q", finding.ContextFingerprint, finding.Fingerprint) + t.Fatalf("expected fallback to exact fingerprint, got context %q exact %q", finding.ContextFingerprint, finding.Fingerprint) } return } if finding.ContextFingerprint == finding.Fingerprint { - t.Fatalf("expected a real context fingerprint, got legacy fallback %q", finding.Fingerprint) + t.Fatalf("expected a real context fingerprint, got exact fallback %q", finding.Fingerprint) } same := finding.ContextFingerprint == base.ContextFingerprint if same != want.wantSame { @@ -146,6 +146,77 @@ func TestContextFingerprintUnreadableFileFallsBack(t *testing.T) { Message: "fixture finding", }) if finding.ContextFingerprint != finding.Fingerprint { - t.Fatalf("expected fallback to legacy fingerprint, got context %q legacy %q", finding.ContextFingerprint, finding.Fingerprint) + t.Fatalf("expected fallback to exact fingerprint, got context %q exact %q", finding.ContextFingerprint, finding.Fingerprint) + } +} + +// A production change that folds Message, Confidence, or Metadata back into +// fingerprint construction must fail this test: those fields are diagnostic +// evidence, not finding identity. +func TestFindingFingerprintsIgnoreDiagnosticProseAndMetadata(t *testing.T) { + dir := t.TempDir() + rel := "src/app.go" + full := filepath.Join(dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(full, []byte(contextFingerprintBase), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + sc := runnersupport.Context{Cfg: core.Config{Targets: []core.TargetConfig{{Name: "repo", Path: dir}}}} + + before := runnersupport.NewFinding(sc, runnersupport.FindingInput{ + RuleID: "test.rule", + Level: "warn", + Path: rel, + Line: 3, + Message: "old diagnostic prose", + Confidence: "low", + Metadata: map[string]string{"mutation_target": "global", "origin": "shared"}, + }) + after := runnersupport.NewFinding(sc, runnersupport.FindingInput{ + RuleID: "test.rule", + Level: "warn", + Path: rel, + Line: 3, + Message: "new diagnostic prose", + Confidence: "high", + Metadata: map[string]string{"mutation_target": "argument", "origin": "caller_owned"}, + }) + + if before.Fingerprint != after.Fingerprint { + t.Errorf("exact fingerprint changed with diagnostic evidence: %q -> %q", before.Fingerprint, after.Fingerprint) + } + if before.ContextFingerprint != after.ContextFingerprint { + t.Errorf("context fingerprint changed with diagnostic evidence: %q -> %q", before.ContextFingerprint, after.ContextFingerprint) + } + if before.ContentFingerprint != after.ContentFingerprint { + t.Errorf("content fingerprint changed with diagnostic evidence: %q -> %q", before.ContentFingerprint, after.ContentFingerprint) + } +} + +func TestFindingExactFingerprintIgnoresMessageWithoutSourceContext(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "short.go"), []byte("package sample\n"), 0o644); err != nil { + t.Fatalf("write short source: %v", err) + } + sc := runnersupport.Context{Cfg: core.Config{Targets: []core.TargetConfig{{Name: "repo", Path: dir}}}} + cases := []struct { + name string + path string + line int + }{ + {name: "pathless", line: 0}, + {name: "unreadable", path: "missing.go", line: 3}, + {name: "invalid line", path: "short.go", line: 99}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + first := runnersupport.NewFinding(sc, runnersupport.FindingInput{RuleID: "test.rule", Path: tc.path, Line: tc.line, Message: "first prose"}) + second := runnersupport.NewFinding(sc, runnersupport.FindingInput{RuleID: "test.rule", Path: tc.path, Line: tc.line, Message: "second prose"}) + if first.Fingerprint != second.Fingerprint || first.ContextFingerprint != second.ContextFingerprint || first.ContentFingerprint != second.ContentFingerprint { + t.Fatalf("message changed source-unavailable identity: %#v -> %#v", first, second) + } + }) } }