diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b237134..c297953 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,6 @@ -name: CI +# CI pipeline for vibe-check: build, test, vet, and lint. +# Runs on pushes to main and pull requests targeting main. +name: CI — Build, Test, and Lint on: push: @@ -6,6 +8,10 @@ on: pull_request: branches: [main] +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + permissions: contents: read @@ -13,9 +19,9 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version-file: go.mod @@ -31,13 +37,13 @@ jobs: lint: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version-file: go.mod - name: golangci-lint - uses: golangci/golangci-lint-action@v7 + uses: golangci/golangci-lint-action@9fae48acfc02a90574d7c304a1758ef9895495fa # v7 with: - version: latest + version: v2.12.2 diff --git a/.gitignore b/.gitignore index 447fc1f..e07d218 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,5 @@ .unbound-force/ .muti-mind/ .mx-f/ +# Go coverage output (generated by `go test -coverprofile`) +coverage.out diff --git a/.uf/dewey/learnings/envoy-review-20260831T152907-jay-flowers.md b/.uf/dewey/learnings/envoy-review-20260831T152907-jay-flowers.md new file mode 100644 index 0000000..ebf1a19 --- /dev/null +++ b/.uf/dewey/learnings/envoy-review-20260831T152907-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: envoy-review +author: jay-flowers +category: pattern +created_at: 2026-08-31T15:29:07Z +identity: envoy-review-20260831T152907-jay-flowers +tier: draft +--- + +Envoy code-review of vibe-check branch opsx/go-analyze (first public capability: `vibe-check analyze` CLI, JSON metrics, distributed via `go install ...@v0.1.0`). VERDICT: REQUEST CHANGES. Anchor HIGH: first-run error messages omit the spec+design-mandated "suggested remediation" — adapter.go:76 ("no Go packages found in %s") and resolve.go:64 ("unable to determine module path") print cause only, but analyze-command/spec.md:52 and design.md:288-290 both MUST require remediation (design even gives the go.mod example). MEDIUMs: (1) terminology split — help text/CHANGELOG say the tool analyzes Go "packages" but violation strings say `VIOLATION: module %q` where %q is a package import path, and the JSON schema uses ModuleGraph/modules[]/warnings[].module; "module" is a distinct Go concept so this misleads the Go audience (VB-003). Fix = use "package" in the Go CLI's human-readable strings while keeping ModuleGraph as the documented universal schema term (do NOT rename the public contract). (2) The competitive superlative "the Martin metrics suite that no single OSS tool currently computes for Go" is still live at AGENTS.md:9 (origin universal-coupling-model/proposal.md:3) — rivals goda/go-arch-lint make it risky; per prior Envoy reviews it becomes HIGH once it hits a README/announcement/pkg.go.dev. This branch is the publish moment. (3) proposal.md:17 over-promises `analyze [packages...]` but the shipped CLI is `analyze [path]` single-directory (MaximumNArgs(1); load pattern ./... hard-coded in resolve.go:52). (4) No README exists at all for a public go install release, and the public JSON contract (schema v1.1) has no external pointer/docs. Positive: --version format matches spec exactly (cobra default template + Version field), exit codes 0/1/2 consistent across spec/design/help/code, JSON-always-to-stdout well-communicated, schema is strict+self-describing. Recurring Envoy checklist for a project's first public release: (a) README present with install+usage+JSON-contract pointer+v0.1.0 limitations, (b) error messages on failure paths carry remediation, (c) one canonical term per concept across help/CHANGELOG/violations/JSON, (d) no unverified competitive superlatives in ANY public file (AGENTS.md counts), (e) stated stability posture for v0.1.0 output schema. diff --git a/.uf/dewey/learnings/envoy-review-20260831T163518-jay-flowers.md b/.uf/dewey/learnings/envoy-review-20260831T163518-jay-flowers.md new file mode 100644 index 0000000..d313a77 --- /dev/null +++ b/.uf/dewey/learnings/envoy-review-20260831T163518-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: envoy-review +author: jay-flowers +category: decision +created_at: 2026-08-31T16:35:18Z +identity: envoy-review-20260831T163518-jay-flowers +tier: draft +--- + +Envoy VERIFICATION re-review (council iter 1) of vibe-check opsx/go-analyze — VERDICT flips REQUEST CHANGES → APPROVE. Prior blocking HIGH (non-actionable first-run errors) is RESOLVED: all four first-run paths now carry spec-mandated remediation (analyze-command/spec.md:52; design.md:290-292): adapter.go:97 'no Go packages found in %s — ensure the path is a Go module directory containing .go files and a go.mod'; adapter.go:108 total-load-failure appends 'run go build ./... … go mod download'; resolve.go:68 'verify the path is a valid Go module directory…'; resolve.go:78 'run go mod init if missing'. Prior MEDIUMs resolved: (b) absolute 'no single OSS tool' superlative removed from ALL shipping .md — AGENTS.md now 'few OSS tools compute for Go'; (c) proposal.md:17/29 corrected to analyze [path] (no [packages...] left in shipping artifacts); (d) README.md created with schema pointer at README.md:107 → metrics/modulegraph.schema.json; (a) terminology bridged via README.md:109-113 note (module==Go package in JSON). NEW residual MEDIUM (non-blocking, FA-001/FA-002): README.md:16-17 overstates the --version fallback as reporting 'a meaningful version, commit, AND date', but for `go install …@v0.1.0` (no ldflags) runtime/debug gives Main.Version only — vcs.revision/vcs.time are NOT stamped for module-cache builds, so output is 'vX (commit none, built unknown)'. CHANGELOG.md:33-34 is correctly scoped ('the reported version stays meaningful'); README + design.md:299 overstate. One-line README fix. Good foundations intact: --version format matches design.md:300 (cobra 'Name version' template + versionString ' (commit , built )'); exit codes 0/1/2 consistent; JSON-always-to-stdout written before threshold checks (analyze.go:100 before :104); schemaVersion 1.1 consistent everywhere. LOW residuals: README terminology note scoped to 'JSON output' doesn't cover stderr 'VIOLATION: module %q' strings (analyze.go:159/167/175); design.md:288 stale advice to 'scope to specific packages rather than ./...' impossible with single-path CLI (README limitation section is correct). Recurring Envoy pattern: on a first public release, verify the primary README's --version fallback claims against actual runtime/debug behavior — vcs stamping is absent for `go install pkg@version`. diff --git a/.uf/dewey/learnings/go-analyze-20260829T215310-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260829T215310-jay-flowers.md new file mode 100644 index 0000000..d3ac9b5 --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260829T215310-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: gotcha +created_at: 2026-08-29T21:53:10Z +identity: go-analyze-20260829T215310-jay-flowers +tier: draft +--- + +When implementing Ce (efferent coupling) for a Go adapter, the Martin metrics definition requires counting ALL imports — standard library, third-party, and module-internal packages. The initial implementation excluded stdlib imports (packages with Module==nil) which caused a CRITICAL spec deviation. The correct implementation is simply `len(pkg.Imports)`. Ca (afferent coupling) only counts module-internal dependents since external consumers are not observable. This asymmetry (Ce counts everything, Ca counts only internal) is fundamental to the metrics model and must be documented in both code comments and test assertions. The spec review caught this in advance but implementation still deviated, underscoring the need for the code review council to verify spec-to-code alignment. diff --git a/.uf/dewey/learnings/go-analyze-20260829T215315-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260829T215315-jay-flowers.md new file mode 100644 index 0000000..ffa6840 --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260829T215315-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: pattern +created_at: 2026-08-29T21:53:15Z +identity: go-analyze-20260829T215315-jay-flowers +tier: draft +--- + +The exitCodeError pattern is the correct way to handle process exit codes in cobra CLI applications without calling os.Exit directly in RunE handlers. Define a type `exitCodeError struct { code int; err error }` implementing Error() and Unwrap(), return it from RunE, and extract it with errors.As in main(). This preserves deferred cleanup (signal handlers, context cancellation), makes the full cobra execution path testable, and keeps os.Exit() isolated to main(). The RunAnalyze function (AP-002 testable entry point) returns the exit code in the result struct, while the cobra layer wraps it in exitCodeError for main() to process. diff --git a/.uf/dewey/learnings/go-analyze-20260829T215326-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260829T215326-jay-flowers.md new file mode 100644 index 0000000..617df59 --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260829T215326-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: context +created_at: 2026-08-29T21:53:26Z +identity: go-analyze-20260829T215326-jay-flowers +tier: draft +--- + +For the vibe-check Go adapter, the go/packages load mode must include NeedTypesInfo in addition to NeedName|NeedImports|NeedTypes|NeedSyntax|NeedModule. The NeedTypesInfo flag is required for LCOM4 computation because the types.Info.Selections map is needed to resolve field accesses in method bodies — without it, the adapter cannot reliably determine which struct fields each method accesses, which is the foundation of the connected-component LCOM4 algorithm. The environment sanitization allowlist for go/packages should include GOPATH, GOROOT, GOMODCACHE, GOPROXY, GONOSUMCHECK, GOMOD but must NOT include GOFLAGS (which enables arbitrary flag injection into go list subprocesses, including -toolexec for command execution). diff --git a/.uf/dewey/learnings/go-analyze-20260829T215336-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260829T215336-jay-flowers.md new file mode 100644 index 0000000..7eb7854 --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260829T215336-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: pattern +created_at: 2026-08-29T21:53:36Z +identity: go-analyze-20260829T215336-jay-flowers +tier: draft +--- + +The go-analyze spec review went through 2 iterations across both spec and code review phases. Key review council patterns: (1) The Guard agent's CRITICAL finding about Ce/stdlib counting was the most impactful — it caught a fundamental metrics definition error that tests had encoded as correct behavior. (2) The Adversary caught GOFLAGS in the environment allowlist as a command injection vector — an important security insight since go/packages spawns subprocesses. (3) The SRE caught unpinned golangci-lint version (version: latest) which is a common CI reproducibility failure. (4) The Tester caught shallow test assertions — errors.Is checks for context wrapping and Warning field assertions that were missing. The pattern shows that multi-persona review catches different categories of issues that a single reviewer would miss. diff --git a/.uf/dewey/learnings/go-analyze-20260831T152549-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260831T152549-jay-flowers.md new file mode 100644 index 0000000..049e560 --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260831T152549-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: gotcha +created_at: 2026-08-31T15:25:49Z +identity: go-analyze-20260831T152549-jay-flowers +tier: draft +--- + +In vibe-check's Go adapter, resolveReceiverType (internal/goadapter/lcom.go:99) has a latent correctness bug with ZERO test coverage: for generic pointer receivers `func (s *S[T]) M()`, the receiver expr is *ast.StarExpr{X: *ast.IndexExpr}, so the `case *ast.StarExpr` branch does `t.X.(*ast.Ident)` which FAILS (X is IndexExpr not Ident), falls through, and returns "" — silently dropping the method from computeLCOM4, corrupting the LCOM metric. The value-receiver `*ast.Ident` branch and the generic `*ast.IndexExpr`/`*ast.IndexListExpr` branches are also untested because every testdata fixture uses only plain pointer receivers `func (s *S)`. Gaze flagged this as CRAP 27.0 / 33.3% line coverage — the highest-CRAP gap in the branch. Fix: add a fixture (or table-driven unit test using go/parser) covering value receiver `func (t T)`, generic value `func (t T[P])`, generic pointer `func (t *T[P])`, and multi-param generic `func (t *T[P1,P2])`; then fix the StarExpr branch to recurse into IndexExpr/IndexListExpr. This is a determinism/correctness-mandate risk (AGENTS.md requires deterministic, correct metric computation). diff --git a/.uf/dewey/learnings/go-analyze-20260831T152802-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260831T152802-jay-flowers.md new file mode 100644 index 0000000..1ddc565 --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260831T152802-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: pattern +created_at: 2026-08-31T15:28:02Z +identity: go-analyze-20260831T152802-jay-flowers +tier: draft +--- + +Curator doc-gate review of vibe-check `opsx/go-analyze` (ships user-facing `vibe-check analyze` CLI, first working adapter). Key triage findings for future reviewers: (1) The content pipeline IS tracked — issues #18 (docs: README+CHANGELOG), #19 (blog), #22 (tutorial) all exist — so do NOT file duplicates; reference/refresh instead. (2) All three content issues have `labels: []`; the repo uses title prefixes (`docs:`/`blog:`/`tutorial:`) but never created the GitHub labels, so `gh issue list --label docs` returns EMPTY. Any agent following the AGENTS.md documentation gate (which searches by label) will falsely conclude no docs issue exists and file a duplicate. Fix: create+apply the labels. (3) Issue #18's README scope is STALE — it predates the CLI (from the universal-coupling-model change) and describes a LIBRARY install (`go get`), omitting `go install .../cmd/vibe-check@v0.1.0`, `vibe-check analyze`, threshold flags. CHANGELOG half of #18 is done; README half is not. (4) The competitive superlative "the Martin metrics suite that no single OSS tool computes for Go" (AGENTS.md:9, proposal Why) is unsubstantiated (rivals: goda, go-arch-lint) and will propagate into README/blog — Envoy previously flagged it as "HIGH the moment it's published". (5) go-analyze tasks.md Section 9 has NO README task — README deferred entirely to #18. Pattern: when a repo tracks content work by title-prefix instead of labels, the label-based doc-gate search silently breaks; verify labels exist AND that stale tracking issues cover the CURRENT user-facing surface, not a prior change's. diff --git a/.uf/dewey/learnings/go-analyze-20260831T163216-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260831T163216-jay-flowers.md new file mode 100644 index 0000000..77c007d --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260831T163216-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: pattern +created_at: 2026-08-31T16:32:16Z +identity: go-analyze-20260831T163216-jay-flowers +tier: draft +--- + +Curator VERIFICATION re-review (council iter 1) of vibe-check `opsx/go-analyze` (ships user-facing `vibe-check analyze` CLI). Resolution status vs prior HIGH: (1) RESOLVED — README.md now created at repo root; comprehensive and correct for the CLI: `go install github.com/zero-dot-force/vibe-check/cmd/vibe-check@v0.1.0`, `analyze [path]`, all 6 flags + `--version`, exit codes 0/1/2, JSON output with schemaVersion 1.1 + `extensions` (go.interfaceWidth/go.interfaceProximity), pointer to metrics/modulegraph.schema.json, terminology note (package↔module), Known limitations, Apache 2.0. Crucially the shipped README does NOT contain the unsubstantiated 'no OSS tool computes Martin metrics for Go' superlative — FA-001 clean. (2) RESOLVED — CHANGELOG.md has [0.1.0] 2026-08-31 (Added/Changed), Keep-a-Changelog + SemVer + compare links. (3) CONFIRMED no GoDoc regression — cmd/vibe-check main.go/root.go/analyze.go all have identifier-first GoDoc on every exported symbol (TD-007); tasks.md 8.6 checked. STILL-OPEN external process caveats (cannot fix from code tree, ADVISORY not blocking): (a) `gh issue list --label docs` STILL returns EMPTY — labels never created; #18/#19/#22 still show `labels: []` and track via title-prefix only, so the AGENTS.md label-based doc-gate will keep falsely reporting 'no docs issue' → duplicate-filing risk; I cannot create labels (only issue list/view/create allowed). (b) Issue #18 body TEXT still stale — describes `go get` LIBRARY install + metrics-package scope, predates CLI; in-tree README supersedes it but the issue should be refreshed/split/closed before merge. (c) website-doc-sync and GoReleaser/release-automation issues NOT filed (proposal lists both as pre-merge TODOs). (d) v0.1.0 tag/release NOT verifiable from tree — README install + CHANGELOG release link depend on it (README honestly hedges this). (e) tasks.md Section 9 still has NO README line item (only 9.1 AGENTS.md, 9.2 CHANGELOG) though README shipped anyway. (f) blog #19 superlative ('first OSS tool… Why No One Computes…') still lives in the issue + AGENTS.md:9 ('which few OSS tools compute') — Herald/Envoy must fact-check at authoring (rivals: goda, go-arch-lint). VERDICT: APPROVE — blocking HIGH resolved, no regressions; remaining items are advisory external-process caveats for the human. diff --git a/.uf/dewey/learnings/go-analyze-20260831T163321-jay-flowers.md b/.uf/dewey/learnings/go-analyze-20260831T163321-jay-flowers.md new file mode 100644 index 0000000..d4027ad --- /dev/null +++ b/.uf/dewey/learnings/go-analyze-20260831T163321-jay-flowers.md @@ -0,0 +1,10 @@ +--- +tag: go-analyze +author: jay-flowers +category: decision +created_at: 2026-08-31T16:33:21Z +identity: go-analyze-20260831T163321-jay-flowers +tier: draft +--- + +Adversary council re-review (iteration 1) of vibe-check branch opsx/go-analyze: prior MEDIUM (ExternalAdapter trust boundary under-enforced schema) is RESOLVED via two-layer enforcement. metrics/validate.go now enforces numeric ranges through a moduleNumber helper (instability/abstractness/distance in [0,1]; ca/ce/lcom/exportedTypes/abstractTypes >= 0, plus number-type checks), and metrics/external.go decodes the subprocess response with json.Decoder.DisallowUnknownFields() enforcing additionalProperties:false. Key insight: integer-ness (schema says ca/ce/lcom are integer) is NOT enforced by Validate() alone (moduleNumber accepts any float64), but the strict decoder rejects fractional values because Module.Ca etc. are Go int fields — so the ExternalAdapter boundary (Validate THEN Decode) is airtight, while the standalone public metrics.Validate() has a minor integer-fidelity gap for external callers (LOW). DisallowUnknownFields correctly flattens the embedded Module struct (promoted fields path/name/ca/ce/exportedTypes/abstractTypes) and does NOT restrict the extensions map[string]any (matches open-object schema). packageEnvAllowlist (resolve.go) excludes GOFLAGS with regression test TestPackageEnvAllowlist_ExcludesInjectionVectors that locks exact set+size. Remaining advisories all LOW/non-blocking: (1) binaryPath unvalidated in NewExternalAdapter but caller-supplied trusted config and not wired to CLI, (2) go/packages type-checking can execute code via cgo/-toolexec on untrusted input — undocumented but intended use is self-analysis of trusted code, (3) recursive Tarjan strongConnect has no depth guard (stack overflow only on pathological graphs). VERDICT: APPROVE. diff --git a/AGENTS.md b/AGENTS.md index 3d965ec..ef6f636 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,8 +6,8 @@ Vibe-Check is a design quality and architectural metrics toolkit for Go codebases. It computes package-level coupling metrics (afferent/ efferent coupling, instability, abstractness, distance from main sequence), cohesion analysis, and circular dependency detection — -providing the Martin metrics suite that no single OSS tool currently -computes for Go. +providing the full Martin metrics suite, which few OSS tools compute +for Go. Vibe-Check also serves as the metrics backbone for the Unbound Force ecosystem's entropy sentinel and architectural drift tracking @@ -238,6 +238,9 @@ hotfixes (retroactively documented). # Build go build ./... +# Build the CLI binary +go build ./cmd/vibe-check + # Test (with race detection) go test -race -count=1 ./... @@ -254,20 +257,33 @@ golangci-lint run ./... .opencode/ # OpenCode agent configuration, skills, packs .specify/ # Constitution and governance memory .uf/ # Unbound Force tooling configuration +cmd/vibe-check/ # CLI entry point (Layer 3) + main.go # Binary entry point with ldflags version embedding + root.go # Cobra root command with --version flag + analyze.go # analyze subcommand with threshold flags +internal/goadapter/ # Go language adapter (Layer 2) + adapter.go # Adapter struct implementing metrics.Adapter + resolve.go # Package loading via go/packages + types.go # Type classification (interfaces = abstract) + lcom.go # LCOM4 via connected-component analysis + cycles.go # Tarjan's SCC for circular dependency detection + extensions.go # go.interfaceWidth and go.interfaceProximity extensions + doc.go # Package-level GoDoc + testdata/ # Test fixtures (coupling, types, lcom, extensions, partial) metrics/ # Universal coupling metrics model (Layer 1) adapter.go # Adapter interface and Capability type compute.go # Metric computation functions cycle.go # Cycle type for circular dependency representation doc.go # Package-level GoDoc external.go # ExternalAdapter (JSON-RPC subprocess) - graph.go # ModuleGraph and ModuleResult types + graph.go # ModuleGraph and ModuleResult types (with Extensions) jsonrpc.go # JSON-RPC 2.0 protocol types module.go # Module type (universal unit of analysis) - modulegraph.schema.json # JSON Schema for ModuleGraph validation + modulegraph.schema.json # JSON Schema for ModuleGraph validation (v1.1) registry.go # Adapter registry (dependency-injected) schema.go # Embedded JSON schema access security.go # Path validation and environment sanitization - validate.go # JSON schema validation + validate.go # JSON schema validation (accepts v1.0 and v1.1) values.go # Named metric types (Instability, Abstractness, etc.) warning.go # Warning type for analysis caveats zone.go # Zone and Status types @@ -279,20 +295,32 @@ openspec/ # OpenSpec change artifacts (proposals, specs, tasks) ## Architecture -The planned architecture follows the RFC phasing: - -- **P0**: Core coupling metrics engine — Ca, Ce, Instability, - Abstractness, Distance from Main Sequence, cohesion, circular - dependency detection -- **P1**: Multi-language adapter interface, metrics command, convention - pack integration +The architecture follows a three-layer design per the RFC phasing: + +- **Layer 1** (`metrics/`): Language-agnostic universal model — Ca, Ce, + Instability, Abstractness, Distance from Main Sequence, LCOM4, + circular dependency detection, JSON schema validation, adapter + interface, and security primitives. +- **Layer 2** (`internal/goadapter/`): Go language adapter implementing + `metrics.Adapter`. Uses `golang.org/x/tools/go/packages` for + type-aware dependency resolution, AST-based type classification, + LCOM4 via connected-component analysis (Hitz & Montazeri 1995), + Tarjan's SCC for cycle detection, and Go-specific extensions + (`go.interfaceWidth`, `go.interfaceProximity`). +- **Layer 3** (`cmd/vibe-check/`): CLI entry point using cobra. Provides + `vibe-check analyze` with threshold flags (`--max-instability`, + `--max-distance`, `--max-lcom`, `--no-circular-deps`, `--timeout`) + and JSON output. + +RFC phasing status: + +- **P0**: Core coupling metrics engine (complete — `metrics/` package) +- **P0 effective**: Go adapter and CLI (complete — `internal/goadapter/` + and `cmd/vibe-check/`; classified as P1 in RFC but required for MVP) - **P2**: Python adapter, cognitive complexity, branch coverage, architectural drift tracking - **P3**: TS/JS adapter, SBOM integration, mutation testing hooks -The `metrics` package implements the P0 universal model. Language -adapters (P1+) will be added as separate packages. - ## Coding Conventions - **Formatting**: `gofmt` is the law. All code MUST be formatted with diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..89e1c3a --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,57 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [Unreleased] + +## [0.1.0] - 2026-08-31 + +Initial release: package-level design-quality and architectural metrics +for Go. + +### Added + +- `vibe-check analyze [path]` computes the Martin coupling metrics suite + for a Go module — afferent coupling (Ca), efferent coupling (Ce), + instability, abstractness, distance from the main sequence, LCOM4 + cohesion, and circular-dependency detection — and prints the results + as JSON on stdout (path defaults to `.`). +- Go language adapter (`internal/goadapter/`) implementing the + `metrics.Adapter` interface, using `golang.org/x/tools/go/packages` + for type-aware dependency resolution. +- CI gate threshold flags — `--max-instability`, `--max-distance`, + `--max-lcom`, and `--no-circular-deps` — so pipelines can fail the + build (exit code 1) when a module breaches an architectural budget. + All violations are reported before exit, and JSON is still written to + stdout. +- `--timeout` flag to bound analysis time (no timeout by default). +- `--version` reporting version, commit, and build date. Binaries + installed via `go install ...@v0.1.0` (built without ldflags) fall + back to `runtime/debug` build info, so the reported version stays + meaningful. +- Go-specific metric extensions surfaced under the JSON `extensions` + object: `go.interfaceWidth` (method count per exported interface) and + `go.interfaceProximity` (producer/consumer classification). +- `Extensions map[string]any` field on `ModuleResult` for + language-specific extension metrics. +- CI workflow (`.github/workflows/ci.yml`) running build, vet, test + (with `-race` and a coverage profile), and lint, using SHA-pinned + GitHub Actions. + +### Changed + +- Output JSON schema version bumped from `1.0` to `1.1` to carry the + optional `extensions` object. Backward-compatible; existing 1.0 + consumers need no migration. +- `metrics.Validate()` accepts both schema versions `1.0` and `1.1`, + and now enforces numeric ranges (instability, abstractness, and + distance in [0, 1]; counts non-negative). +- The external-analyzer trust boundary (`ExternalAdapter.Analyze`) now + rejects unknown fields via a strict JSON decoder, hardening it against + malformed subprocess output. + +[Unreleased]: https://github.com/zero-dot-force/vibe-check/compare/v0.1.0...HEAD +[0.1.0]: https://github.com/zero-dot-force/vibe-check/releases/tag/v0.1.0 diff --git a/README.md b/README.md new file mode 100644 index 0000000..42ae5c8 --- /dev/null +++ b/README.md @@ -0,0 +1,130 @@ +# Vibe-Check + +Vibe-Check is a design-quality and architectural-metrics tool for Go. It computes the +Martin package-coupling metrics suite — afferent coupling (Ca), efferent coupling (Ce), +instability, abstractness, and distance from the main sequence — plus LCOM4 cohesion and +circular-dependency detection, and emits the results as JSON. + +## Install + +```bash +go install github.com/zero-dot-force/vibe-check/cmd/vibe-check@v0.1.0 +``` + +This resolves against the `v0.1.0` release tag, which must be published in the repository +for module-version installs to work. When a binary is built this way (without release +`ldflags`), `--version` falls back to the toolchain's build info so it still reports a +meaningful module version (commit and build date may show as `none`/`unknown` for +module-cache installs). + +To build from a checkout instead: + +```bash +go build ./cmd/vibe-check +``` + +## Usage + +```bash +vibe-check analyze [path] +``` + +`analyze` takes a single optional path to a Go module directory and defaults to the +current directory (`.`). It writes the analysis as JSON to stdout. + +```bash +# Analyze the current module +vibe-check analyze + +# Analyze a module elsewhere, failing CI if any package is too unstable +vibe-check analyze ./myproject --max-instability 0.8 --no-circular-deps +``` + +## Flags + +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--max-instability` | float | unset (no check) | Fail if any module's instability exceeds this value. Must be in `[0.0, 1.0]`. | +| `--max-distance` | float | unset (no check) | Fail if any module's distance from the main sequence exceeds this value. Must be in `[0.0, 1.0]`. | +| `--max-lcom` | int | unset (no check) | Fail if any module's LCOM4 exceeds this value. Must be `>= 1`. | +| `--no-circular-deps` | bool | `false` | Treat any detected circular dependency as a violation. | +| `--timeout` | duration | none | Bound total analysis time (e.g., `30s`, `2m`). No timeout by default. | +| `--version` | — | — | Print version, commit, and build date, then exit. Use on the root command: `vibe-check --version`. | + +Threshold comparisons use strict greater-than: a metric exactly equal to the threshold +**passes**. Invalid flag values (for example, out of range) exit with code `2` before any +analysis runs. + +## Exit codes + +| Code | Meaning | +|------|---------| +| `0` | Success — analysis completed with no threshold violations. | +| `1` | Policy failure — analysis succeeded, but one or more thresholds were exceeded. | +| `2` | Tool failure — invalid arguments/flags, adapter error, timeout, or interrupt signal. | + +Violation messages are written to stderr; the JSON report is still written to stdout even +when violations cause a non-zero exit, so CI can capture the full results while failing the +gate. + +## Output + +The analysis is printed to stdout as pretty-printed JSON with `schemaVersion` `"1.1"`. Each +entry in `modules[]` carries `ca`, `ce`, `instability`, `abstractness`, `distance`, `lcom`, +`exportedTypes`, `abstractTypes`, `zone`, and an optional `extensions` object. For Go, the +extensions may include `go.interfaceWidth` (method count per exported interface) and +`go.interfaceProximity` (`"producer"` or `"consumer"` per interface). Detected cycles appear +in `cycles` as lexicographically-sorted sets of package paths. + +```json +{ + "schemaVersion": "1.1", + "language": "go", + "modules": [ + { + "path": "github.com/you/proj/pkg", + "name": "pkg", + "ca": 2, + "ce": 5, + "instability": 0.71, + "abstractness": 0.25, + "distance": 0.04, + "lcom": 1, + "exportedTypes": 4, + "abstractTypes": 1, + "zone": "main-sequence", + "extensions": { + "go.interfaceWidth": { "Reader": 1 }, + "go.interfaceProximity": { "Reader": "consumer" } + } + } + ], + "cycles": [], + "warnings": [], + "status": "complete" +} +``` + +The full JSON Schema is at [`metrics/modulegraph.schema.json`](metrics/modulegraph.schema.json). + +### A note on terminology + +Vibe-Check analyzes Go **packages**, but the universal output schema names each unit of +analysis a `module`. Throughout vibe-check's output — every `modules[]` entry in the JSON +and each `VIOLATION: module ...` line on stderr — one `module` corresponds to one Go +package. + +## Known limitations + +- `analyze` accepts a single path argument (defaulting to `.`); it does not take multiple + package patterns. +- There is no default timeout — set `--timeout` to bound long analyses (for example, on large + monorepos). +- Provenance metadata (producer, version, timestamp, input) is not yet emitted in the output; + it is deferred to a follow-up. +- Large-repository performance is a P2 item: `go/packages` loads full ASTs and type + information into memory, so very large monorepos may be slow. + +## License + +Apache 2.0. diff --git a/cmd/vibe-check/analyze.go b/cmd/vibe-check/analyze.go new file mode 100644 index 0000000..1d15690 --- /dev/null +++ b/cmd/vibe-check/analyze.go @@ -0,0 +1,283 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "os/signal" + "syscall" + "time" + + "github.com/spf13/cobra" + + "github.com/zero-dot-force/vibe-check/internal/goadapter" + "github.com/zero-dot-force/vibe-check/metrics" +) + +// AnalyzeOptions contains the configuration for the analyze command. +// It follows the AP-001 Options struct pattern for testable CLI commands. +type AnalyzeOptions struct { + // Stdout is the writer for JSON output. Required. + Stdout io.Writer + // Stderr is the writer for violation reports and errors. Required. + Stderr io.Writer + // Path is the project directory to analyze. + Path string + + // MaxInstability is the threshold for instability violations. + // nil means not set (no threshold check). Must be in [0.0, 1.0]. + MaxInstability *float64 + // MaxDistance is the threshold for distance-from-main-sequence violations. + // nil means not set (no threshold check). Must be in [0.0, 1.0]. + MaxDistance *float64 + // NoCircularDeps when true treats any detected cycle as a violation. + NoCircularDeps bool + // MaxLCOM is the threshold for LCOM violations. + // nil means not set (no threshold check). Must be >= 1. + MaxLCOM *int + + // Timeout is the analysis timeout duration. Zero means no timeout. + Timeout time.Duration +} + +// AnalyzeResult contains the analysis outcome. +// It follows the AP-001 Result struct pattern. +type AnalyzeResult struct { + // Graph is the computed module graph. May be nil if analysis failed. + Graph *metrics.ModuleGraph + // Violations is the list of threshold violation messages. + Violations []string + // ExitCode is the process exit code: 0 success, 1 policy failure, 2 tool failure. + ExitCode int +} + +// RunAnalyze executes the analysis and returns a result. +// This is the testable entry point per AP-002/AP-003: all business logic +// lives here, not in the cobra command layer. +// +// Exit code semantics: +// - 0: success, no violations +// - 1: analysis succeeded, threshold violations detected (policy failure) +// - 2: tool failure (invalid args, adapter error, timeout, signal) +func RunAnalyze(ctx context.Context, opts AnalyzeOptions) (*AnalyzeResult, error) { + // Step 1: Validate flags. + if err := validateFlags(opts); err != nil { + return &AnalyzeResult{ExitCode: 2}, err + } + + // Step 2: Apply timeout if configured. + if opts.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, opts.Timeout) + defer cancel() + } + + // Step 3: Run analysis via the Go adapter. + adapter := goadapter.New() + graph, err := adapter.Analyze(ctx, opts.Path) + if err != nil { + return &AnalyzeResult{ExitCode: 2}, fmt.Errorf("analyze: %w", err) + } + + // Step 4: Check context before writing output. + // Spec: "no partial JSON MUST be written to stdout" on signal/timeout. + if ctx.Err() != nil { + return &AnalyzeResult{ExitCode: 2}, fmt.Errorf("analyze: %w", ctx.Err()) + } + + // Step 5: Marshal and write JSON to stdout (always, even when violations exist). + data, err := json.MarshalIndent(graph, "", " ") + if err != nil { + return &AnalyzeResult{ExitCode: 2}, fmt.Errorf("analyze: marshal: %w", err) + } + + // Final context check before writing — prevent partial output. + if ctx.Err() != nil { + return &AnalyzeResult{ExitCode: 2}, fmt.Errorf("analyze: %w", ctx.Err()) + } + + if _, err := fmt.Fprintln(opts.Stdout, string(data)); err != nil { + return &AnalyzeResult{ExitCode: 2}, fmt.Errorf("write output: %w", err) + } + + // Step 6: Check thresholds and collect violations. + // Uses strict > comparison: metric == threshold passes. + violations := checkThresholds(graph, opts) + + // Step 7: Report violations to stderr. + for _, v := range violations { + _, _ = fmt.Fprintln(opts.Stderr, v) + } + + exitCode := 0 + if len(violations) > 0 { + exitCode = 1 + } + + return &AnalyzeResult{ + Graph: graph, + Violations: violations, + ExitCode: exitCode, + }, nil +} + +// validateFlags checks that all threshold flag values are within valid ranges. +// Returns an error describing the first invalid flag found. +func validateFlags(opts AnalyzeOptions) error { + if opts.MaxInstability != nil { + v := *opts.MaxInstability + if v < 0.0 || v > 1.0 { + return fmt.Errorf("invalid --max-instability value %.2f: must be in [0.0, 1.0]", v) + } + } + if opts.MaxDistance != nil { + v := *opts.MaxDistance + if v < 0.0 || v > 1.0 { + return fmt.Errorf("invalid --max-distance value %.2f: must be in [0.0, 1.0]", v) + } + } + if opts.MaxLCOM != nil { + v := *opts.MaxLCOM + if v < 1 { + return fmt.Errorf("invalid --max-lcom value %d: must be >= 1", v) + } + } + return nil +} + +// checkThresholds compares module metrics against configured thresholds. +// Uses strict > (greater than) comparison: if metric == threshold, it passes. +// All violations are collected — does not short-circuit on first violation. +func checkThresholds(graph *metrics.ModuleGraph, opts AnalyzeOptions) []string { + var violations []string + + for _, m := range graph.Modules { + if opts.MaxInstability != nil { + if float64(m.Instability) > *opts.MaxInstability { + violations = append(violations, fmt.Sprintf( + "VIOLATION: module %q instability %.2f exceeds threshold %.2f", + m.Path, float64(m.Instability), *opts.MaxInstability, + )) + } + } + if opts.MaxDistance != nil { + if float64(m.Distance) > *opts.MaxDistance { + violations = append(violations, fmt.Sprintf( + "VIOLATION: module %q distance %.2f exceeds threshold %.2f", + m.Path, float64(m.Distance), *opts.MaxDistance, + )) + } + } + if opts.MaxLCOM != nil { + if int(m.LCOM) > *opts.MaxLCOM { + violations = append(violations, fmt.Sprintf( + "VIOLATION: module %q lcom %d exceeds threshold %d", + m.Path, int(m.LCOM), *opts.MaxLCOM, + )) + } + } + } + + if opts.NoCircularDeps && len(graph.Cycles) > 0 { + for _, cycle := range graph.Cycles { + violations = append(violations, fmt.Sprintf( + "VIOLATION: circular dependency detected: %v", + []string(cycle), + )) + } + } + + return violations +} + +// analyzeCmd creates the cobra command for the analyze subcommand. +// It wires flag parsing and signal handling, then delegates to RunAnalyze +// per AP-002 (no business logic in the command layer). +func analyzeCmd() *cobra.Command { + var ( + maxInstability float64 + maxDistance float64 + maxLCOM int + noCircularDeps bool + timeout time.Duration + ) + + cmd := &cobra.Command{ + Use: "analyze [path]", + Short: "Analyze Go packages and compute coupling metrics", + Long: `Analyze computes package-level coupling metrics for a Go project: +afferent coupling (Ca), efferent coupling (Ce), instability, abstractness, +distance from main sequence, LCOM4 cohesion, and circular dependency detection. + +Output is JSON conforming to the ModuleGraph schema (version 1.1). + +Use threshold flags (--max-instability, --max-distance, --max-lcom, +--no-circular-deps) for CI gate enforcement. Violations cause exit code 1. +JSON output is always written to stdout, even when violations are detected.`, + Args: cobra.MaximumNArgs(1), + // SilenceUsage prevents cobra from printing usage on RunE errors. + // We handle error reporting ourselves. + SilenceUsage: true, + RunE: func(cmd *cobra.Command, args []string) error { + // Determine project path: argument or current directory. + path := "." + if len(args) > 0 { + path = args[0] + } + + // Signal handling: intercept SIGINT and SIGTERM. + ctx, stop := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) + defer stop() + + // Build options, converting explicitly-set flags to pointers. + opts := AnalyzeOptions{ + Stdout: cmd.OutOrStdout(), + Stderr: cmd.ErrOrStderr(), + Path: path, + NoCircularDeps: noCircularDeps, + Timeout: timeout, + } + + // Use cobra's Changed() to distinguish "not set" from "set to zero". + if cmd.Flags().Changed("max-instability") { + opts.MaxInstability = &maxInstability + } + if cmd.Flags().Changed("max-distance") { + opts.MaxDistance = &maxDistance + } + if cmd.Flags().Changed("max-lcom") { + opts.MaxLCOM = &maxLCOM + } + + result, err := RunAnalyze(ctx, opts) + if err != nil { + // Print error to stderr; cobra will not print usage + // because SilenceUsage is true. + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Error:", err) + return &exitCodeError{code: result.ExitCode, err: err} + } + + if result.ExitCode != 0 { + // Policy violations — error message already written to stderr + // by RunAnalyze. Return an exitCodeError so main() can set + // the correct exit code without bypassing deferred cleanup. + return &exitCodeError{ + code: result.ExitCode, + err: fmt.Errorf("threshold violations detected"), + } + } + + return nil + }, + } + + // Register flags with change-tracking callbacks. + cmd.Flags().Float64Var(&maxInstability, "max-instability", 0, "Maximum allowed instability [0.0, 1.0]") + cmd.Flags().Float64Var(&maxDistance, "max-distance", 0, "Maximum allowed distance from main sequence [0.0, 1.0]") + cmd.Flags().IntVar(&maxLCOM, "max-lcom", 0, "Maximum allowed LCOM value (>= 1)") + cmd.Flags().BoolVar(&noCircularDeps, "no-circular-deps", false, "Treat circular dependencies as violations") + cmd.Flags().DurationVar(&timeout, "timeout", 0, "Analysis timeout (e.g., 30s, 2m)") + + return cmd +} diff --git a/cmd/vibe-check/analyze_test.go b/cmd/vibe-check/analyze_test.go new file mode 100644 index 0000000..97216e9 --- /dev/null +++ b/cmd/vibe-check/analyze_test.go @@ -0,0 +1,1027 @@ +package main + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +// fixtureDir returns the absolute path to a named test fixture under +// internal/goadapter/testdata/. Uses runtime.Caller to locate the source +// tree, then navigates to the goadapter testdata directory. +func fixtureDir(t *testing.T, name string) string { + t.Helper() + _, filename, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("unable to determine test file location") + } + // Navigate from cmd/vibe-check/ up to repo root, then into internal/goadapter/testdata/. + repoRoot := filepath.Join(filepath.Dir(filename), "..", "..") + return filepath.Join(repoRoot, "internal", "goadapter", "testdata", name) +} + +// couplingFixtureDir returns the absolute path to the coupling test fixture. +func couplingFixtureDir(t *testing.T) string { + t.Helper() + return fixtureDir(t, "coupling") +} + +// float64Ptr returns a pointer to the given float64 value. +func float64Ptr(v float64) *float64 { + t := v + return &t +} + +// intPtr returns a pointer to the given int value. +func intPtr(v int) *int { + t := v + return &t +} + +// --- Task 7.5: Flag parsing and help --- + +func TestAnalyzeHelp(t *testing.T) { + t.Parallel() + + cmd := rootCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + cmd.SetArgs([]string{"analyze", "--help"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := out.String() + if !strings.Contains(output, "analyze") { + t.Error("help output does not mention 'analyze'") + } + if !strings.Contains(output, "--max-instability") { + t.Error("help output does not mention '--max-instability'") + } + if !strings.Contains(output, "--max-distance") { + t.Error("help output does not mention '--max-distance'") + } + if !strings.Contains(output, "--max-lcom") { + t.Error("help output does not mention '--max-lcom'") + } + if !strings.Contains(output, "--no-circular-deps") { + t.Error("help output does not mention '--no-circular-deps'") + } + if !strings.Contains(output, "--timeout") { + t.Error("help output does not mention '--timeout'") + } +} + +func TestVersion(t *testing.T) { + t.Parallel() + + cmd := rootCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetArgs([]string{"--version"}) + + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + output := out.String() + // Version format: "vibe-check version dev (commit none, built unknown)" + if !strings.Contains(output, "vibe-check") { + t.Errorf("version output missing 'vibe-check': %q", output) + } + if !strings.Contains(output, "dev") { + t.Errorf("version output missing default version 'dev': %q", output) + } +} + +func TestAnalyzeFlagParsing(t *testing.T) { + t.Parallel() + + // Verify the command structure accepts all expected flags without error. + cmd := analyzeCmd() + var out bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&out) + + // Parse flags without executing (just verify they're registered). + err := cmd.Flags().Parse([]string{ + "--max-instability", "0.5", + "--max-distance", "0.3", + "--max-lcom", "3", + "--no-circular-deps", + "--timeout", "30s", + }) + if err != nil { + t.Fatalf("flag parsing failed: %v", err) + } + + // Verify parsed values. + instability, err := cmd.Flags().GetFloat64("max-instability") + if err != nil { + t.Fatalf("GetFloat64 max-instability: %v", err) + } + if instability != 0.5 { + t.Errorf("max-instability: got %f, want 0.5", instability) + } + + distance, err := cmd.Flags().GetFloat64("max-distance") + if err != nil { + t.Fatalf("GetFloat64 max-distance: %v", err) + } + if distance != 0.3 { + t.Errorf("max-distance: got %f, want 0.3", distance) + } + + lcom, err := cmd.Flags().GetInt("max-lcom") + if err != nil { + t.Fatalf("GetInt max-lcom: %v", err) + } + if lcom != 3 { + t.Errorf("max-lcom: got %d, want 3", lcom) + } + + noCycles, err := cmd.Flags().GetBool("no-circular-deps") + if err != nil { + t.Fatalf("GetBool no-circular-deps: %v", err) + } + if !noCycles { + t.Error("no-circular-deps: got false, want true") + } + + timeout, err := cmd.Flags().GetDuration("timeout") + if err != nil { + t.Fatalf("GetDuration timeout: %v", err) + } + if timeout != 30*time.Second { + t.Errorf("timeout: got %v, want 30s", timeout) + } +} + +// --- Task 7.6: Threshold violations --- + +func TestRunAnalyze_ThresholdViolations(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + fixtureDir := couplingFixtureDir(t) + + tests := []struct { + name string + opts AnalyzeOptions + wantExitCode int + wantViolations int + wantContains string + }{ + { + name: "instability_violation", + opts: AnalyzeOptions{ + Path: fixtureDir, + MaxInstability: float64Ptr(0.0), // pkga and pkgc have I=1.0, exceeds 0.0 + }, + wantExitCode: 1, + wantViolations: 2, // pkga (I=1.0) and pkgc (I=1.0) + wantContains: "instability", + }, + { + name: "instability_passes", + opts: AnalyzeOptions{ + Path: fixtureDir, + MaxInstability: float64Ptr(1.0), // all modules have I <= 1.0 + }, + wantExitCode: 0, + wantViolations: 0, + }, + { + name: "distance_violation", + opts: AnalyzeOptions{ + Path: fixtureDir, + MaxDistance: float64Ptr(0.0), // most modules will exceed 0.0 + }, + wantExitCode: 1, + wantContains: "distance", + }, + { + name: "distance_passes", + opts: AnalyzeOptions{ + Path: fixtureDir, + MaxDistance: float64Ptr(1.0), // all distances are <= 1.0 + }, + wantExitCode: 0, + wantViolations: 0, + }, + { + name: "lcom_passes", + opts: AnalyzeOptions{ + Path: fixtureDir, + MaxLCOM: intPtr(100), // generous threshold + }, + wantExitCode: 0, + wantViolations: 0, + }, + { + name: "no_circular_deps_passes", + opts: AnalyzeOptions{ + Path: fixtureDir, + NoCircularDeps: true, // coupling fixture has no cycles + }, + wantExitCode: 0, + wantViolations: 0, + }, + { + name: "multiple_violations", + opts: AnalyzeOptions{ + Path: fixtureDir, + MaxInstability: float64Ptr(0.0), + MaxDistance: float64Ptr(0.0), + }, + wantExitCode: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + tt.opts.Stdout = &stdout + tt.opts.Stderr = &stderr + + result, err := RunAnalyze(context.Background(), tt.opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + + if result.ExitCode != tt.wantExitCode { + t.Errorf("ExitCode: got %d, want %d\nstderr: %s", result.ExitCode, tt.wantExitCode, stderr.String()) + } + + if tt.wantViolations > 0 && len(result.Violations) != tt.wantViolations { + t.Errorf("Violations count: got %d, want %d\nviolations: %v", len(result.Violations), tt.wantViolations, result.Violations) + } + + if tt.wantContains != "" { + found := false + for _, v := range result.Violations { + if strings.Contains(v, tt.wantContains) { + found = true + break + } + } + if !found { + t.Errorf("no violation contains %q\nviolations: %v", tt.wantContains, result.Violations) + } + } + + // JSON is always written to stdout, even when violations exist. + if stdout.Len() == 0 { + t.Error("stdout is empty, expected JSON output") + } + }) + } +} + +func TestRunAnalyze_LCOMViolation(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + // The lcom fixture has noncohesive package with LCOM=2. + // Setting threshold to 1 should trigger a violation for noncohesive. + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: fixtureDir(t, "lcom"), + MaxLCOM: intPtr(1), + } + + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + + if result.ExitCode != 1 { + t.Errorf("ExitCode: got %d, want 1", result.ExitCode) + } + + if len(result.Violations) == 0 { + t.Fatal("expected at least one LCOM violation") + } + + found := false + for _, v := range result.Violations { + if strings.Contains(v, "lcom") { + found = true + break + } + } + if !found { + t.Errorf("no violation contains 'lcom'\nviolations: %v", result.Violations) + } + + // JSON should still be written to stdout. + if stdout.Len() == 0 { + t.Error("stdout is empty, expected JSON output") + } +} + +func TestRunAnalyze_JSONWrittenOnViolation(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + MaxInstability: float64Ptr(0.0), // Will cause violations + } + + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + + // Verify exit code 1 (violations detected). + if result.ExitCode != 1 { + t.Errorf("ExitCode: got %d, want 1", result.ExitCode) + } + + // Verify JSON was still written to stdout. + if stdout.Len() == 0 { + t.Fatal("stdout is empty, expected JSON output even with violations") + } + + // Verify the JSON is valid and passes schema validation. + if err := metrics.Validate(stdout.Bytes()); err != nil { + t.Errorf("JSON output failed validation: %v", err) + } + + // Verify violations were written to stderr. + if stderr.Len() == 0 { + t.Error("stderr is empty, expected violation messages") + } +} + +// --- Task 7.7: Flag validation and exit codes --- + +func TestRunAnalyze_FlagValidation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opts AnalyzeOptions + wantErr string + }{ + { + name: "max_instability_too_high", + opts: AnalyzeOptions{MaxInstability: float64Ptr(1.5)}, + wantErr: "--max-instability", + }, + { + name: "max_instability_negative", + opts: AnalyzeOptions{MaxInstability: float64Ptr(-0.1)}, + wantErr: "--max-instability", + }, + { + name: "max_distance_too_high", + opts: AnalyzeOptions{MaxDistance: float64Ptr(1.5)}, + wantErr: "--max-distance", + }, + { + name: "max_distance_negative", + opts: AnalyzeOptions{MaxDistance: float64Ptr(-0.5)}, + wantErr: "--max-distance", + }, + { + name: "max_lcom_zero", + opts: AnalyzeOptions{MaxLCOM: intPtr(0)}, + wantErr: "--max-lcom", + }, + { + name: "max_lcom_negative", + opts: AnalyzeOptions{MaxLCOM: intPtr(-1)}, + wantErr: "--max-lcom", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + tt.opts.Stdout = &stdout + tt.opts.Stderr = &stderr + tt.opts.Path = "/nonexistent" // Won't reach adapter + + result, err := RunAnalyze(context.Background(), tt.opts) + if err == nil { + t.Fatal("expected error for invalid flag, got nil") + } + if result.ExitCode != 2 { + t.Errorf("ExitCode: got %d, want 2", result.ExitCode) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("error %q does not contain %q", err.Error(), tt.wantErr) + } + }) + } +} + +func TestRunAnalyze_AdapterError(t *testing.T) { + t.Parallel() + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: "/nonexistent/path/that/does/not/exist", + } + + result, err := RunAnalyze(context.Background(), opts) + if err == nil { + t.Fatal("expected error for nonexistent path, got nil") + } + if result.ExitCode != 2 { + t.Errorf("ExitCode: got %d, want 2", result.ExitCode) + } +} + +func TestRunAnalyze_ContextCancellation(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately. + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + } + + result, err := RunAnalyze(ctx, opts) + if err == nil { + t.Fatal("expected error for cancelled context, got nil") + } + if result.ExitCode != 2 { + t.Errorf("ExitCode: got %d, want 2", result.ExitCode) + } + + // No partial JSON should be written. + if stdout.Len() > 0 { + t.Errorf("stdout should be empty on cancellation, got %d bytes", stdout.Len()) + } +} + +func TestRunAnalyze_TimeoutCreatesDeadline(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + // Use a generous timeout that should not expire. + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + Timeout: 60 * time.Second, + } + + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + if result.ExitCode != 0 { + t.Errorf("ExitCode: got %d, want 0", result.ExitCode) + } + + // Verify JSON was produced. + if stdout.Len() == 0 { + t.Error("stdout is empty, expected JSON output") + } +} + +func TestRunAnalyze_BoundaryValues(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + // In the coupling fixture: + // pkga: I=1.0 (Ce=1, Ca=0), pkgb: I=0.0 (Ce=0, Ca=2), pkgc: I=1.0 (Ce=1, Ca=0) + // Setting threshold to exactly the metric value should PASS (strict >). + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + MaxInstability: float64Ptr(1.0), // pkga and pkgc have I=1.0, which equals threshold → passes + } + + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + if result.ExitCode != 0 { + t.Errorf("ExitCode: got %d, want 0 (metric == threshold should pass)\nviolations: %v", result.ExitCode, result.Violations) + } +} + +func TestRunAnalyze_ExitCodeDistinction(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + // Exit 0: no violations. + t.Run("exit_0_no_violations", func(t *testing.T) { + t.Parallel() + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + } + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + if result.ExitCode != 0 { + t.Errorf("ExitCode: got %d, want 0", result.ExitCode) + } + }) + + // Exit 1: threshold violations. + t.Run("exit_1_violations", func(t *testing.T) { + t.Parallel() + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + MaxInstability: float64Ptr(0.0), + } + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + if result.ExitCode != 1 { + t.Errorf("ExitCode: got %d, want 1", result.ExitCode) + } + }) + + // Exit 2: tool failure (invalid flag). + t.Run("exit_2_invalid_flag", func(t *testing.T) { + t.Parallel() + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: "/nonexistent", + MaxInstability: float64Ptr(2.0), + } + result, err := RunAnalyze(context.Background(), opts) + if err == nil { + t.Fatal("expected error, got nil") + } + if result.ExitCode != 2 { + t.Errorf("ExitCode: got %d, want 2", result.ExitCode) + } + }) + + // Exit 2: adapter error. + t.Run("exit_2_adapter_error", func(t *testing.T) { + t.Parallel() + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: "/nonexistent/path", + } + result, err := RunAnalyze(context.Background(), opts) + if err == nil { + t.Fatal("expected error, got nil") + } + if result.ExitCode != 2 { + t.Errorf("ExitCode: got %d, want 2", result.ExitCode) + } + }) +} + +// --- Task 7.8: JSON output validation --- + +func TestRunAnalyze_JSONOutputValidation(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + } + + _, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + + // Verify JSON passes schema validation. + data := stdout.Bytes() + if err := metrics.Validate(data); err != nil { + t.Errorf("JSON output failed schema validation: %v", err) + } +} + +func TestRunAnalyze_JSONOutputPrettyPrinted(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + } + + _, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + + output := stdout.String() + + // Pretty-printed JSON should contain newlines and indentation. + if !strings.Contains(output, "\n") { + t.Error("JSON output is not pretty-printed: no newlines found") + } + if !strings.Contains(output, " ") { + t.Error("JSON output is not pretty-printed: no indentation found") + } + + // Verify it's valid JSON by unmarshaling. + var graph metrics.ModuleGraph + if err := json.Unmarshal([]byte(output), &graph); err != nil { + t.Errorf("JSON output is not valid JSON: %v", err) + } + + // Verify expected fields are present. + if graph.SchemaVersion != "1.1" { + t.Errorf("SchemaVersion: got %q, want %q", graph.SchemaVersion, "1.1") + } + if graph.Language != "go" { + t.Errorf("Language: got %q, want %q", graph.Language, "go") + } + if len(graph.Modules) == 0 { + t.Error("Modules is empty, expected at least one module") + } +} + +func TestRunAnalyze_JSONOutputContainsModules(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + var stdout, stderr bytes.Buffer + opts := AnalyzeOptions{ + Stdout: &stdout, + Stderr: &stderr, + Path: couplingFixtureDir(t), + } + + result, err := RunAnalyze(context.Background(), opts) + if err != nil { + t.Fatalf("RunAnalyze returned error: %v", err) + } + + // Verify the graph contains the expected coupling fixture packages. + if result.Graph == nil { + t.Fatal("Graph is nil") + } + + byName := make(map[string]metrics.ModuleResult) + for _, m := range result.Graph.Modules { + byName[m.Name] = m + } + + expectedPkgs := []string{"pkga", "pkgb", "pkgc"} + for _, name := range expectedPkgs { + if _, ok := byName[name]; !ok { + t.Errorf("expected package %q not found in results", name) + } + } +} + +// --- Command-level tests: exercise analyzeCmd RunE via Execute() --- + +// TestAnalyzeCommand_ExitCodes drives the analyze subcommand through the cobra +// command (RunE), rather than calling RunAnalyze directly, and asserts the +// *exitCodeError code returned by Execute(). This covers the RunE wiring: +// policy violations (exit 1), tool failures (exit 2), and — critically — the +// Flags().Changed()→pointer conversion that distinguishes an unset threshold +// from one explicitly set to zero. +func TestAnalyzeCommand_ExitCodes(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + coupling := couplingFixtureDir(t) + + tests := []struct { + name string + args []string + wantErr bool + wantCode int // asserted only when wantErr is true + }{ + { + // Explicit zero instability threshold: pkga and pkgc have I=1.0 > 0, + // so this is a policy violation. If the flag were treated as unset, + // there would be no violation and Execute would return nil. + name: "policy_violation_instability_zero", + args: []string{"analyze", "--max-instability", "0", coupling}, + wantErr: true, + wantCode: 1, + }, + { + // Explicit zero distance threshold is honored: pkgb has D=1.0 > 0. + // Pairs with no_thresholds_success below to prove the + // Changed()→pointer wiring distinguishes "set to 0" from "not set". + name: "explicit_zero_distance_honored", + args: []string{"analyze", "--max-distance", "0", coupling}, + wantErr: true, + wantCode: 1, + }, + { + // No thresholds set: analysis succeeds with no violations (exit 0). + // Proves the "not set" path does not spuriously violate. + name: "no_thresholds_success", + args: []string{"analyze", coupling}, + wantErr: false, + }, + { + // Tool failure: the target directory does not exist (exit 2). + name: "tool_failure_nonexistent_dir", + args: []string{"analyze", "/nonexistent/path/that/does/not/exist"}, + wantErr: true, + wantCode: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + cmd := rootCmd() + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + cmd.SetArgs(tt.args) + + err := cmd.Execute() + + if !tt.wantErr { + if err != nil { + t.Fatalf("Execute: unexpected error: %v\nstderr: %s", err, errOut.String()) + } + return + } + + if err == nil { + t.Fatal("Execute: expected error, got nil") + } + + var ece *exitCodeError + if !errors.As(err, &ece) { + t.Fatalf("Execute: error is not *exitCodeError: %T (%v)", err, err) + } + if ece.code != tt.wantCode { + t.Errorf("exit code: got %d, want %d", ece.code, tt.wantCode) + } + }) + } +} + +// --- validateFlags unit tests (pure function, no integration) --- + +func TestValidateFlags(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + opts AnalyzeOptions + wantErr bool + }{ + { + name: "all_nil_passes", + opts: AnalyzeOptions{}, + }, + { + name: "valid_instability", + opts: AnalyzeOptions{MaxInstability: float64Ptr(0.5)}, + }, + { + name: "valid_instability_zero", + opts: AnalyzeOptions{MaxInstability: float64Ptr(0.0)}, + }, + { + name: "valid_instability_one", + opts: AnalyzeOptions{MaxInstability: float64Ptr(1.0)}, + }, + { + name: "invalid_instability_above", + opts: AnalyzeOptions{MaxInstability: float64Ptr(1.1)}, + wantErr: true, + }, + { + name: "invalid_instability_below", + opts: AnalyzeOptions{MaxInstability: float64Ptr(-0.1)}, + wantErr: true, + }, + { + name: "valid_distance", + opts: AnalyzeOptions{MaxDistance: float64Ptr(0.5)}, + }, + { + name: "invalid_distance_above", + opts: AnalyzeOptions{MaxDistance: float64Ptr(1.5)}, + wantErr: true, + }, + { + name: "invalid_distance_below", + opts: AnalyzeOptions{MaxDistance: float64Ptr(-0.5)}, + wantErr: true, + }, + { + name: "valid_lcom", + opts: AnalyzeOptions{MaxLCOM: intPtr(1)}, + }, + { + name: "valid_lcom_large", + opts: AnalyzeOptions{MaxLCOM: intPtr(100)}, + }, + { + name: "invalid_lcom_zero", + opts: AnalyzeOptions{MaxLCOM: intPtr(0)}, + wantErr: true, + }, + { + name: "invalid_lcom_negative", + opts: AnalyzeOptions{MaxLCOM: intPtr(-1)}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := validateFlags(tt.opts) + if tt.wantErr && err == nil { + t.Error("expected error, got nil") + } + if !tt.wantErr && err != nil { + t.Errorf("unexpected error: %v", err) + } + }) + } +} + +// --- checkThresholds unit tests (pure function, no integration) --- + +func TestCheckThresholds(t *testing.T) { + t.Parallel() + + // Build a synthetic graph for threshold testing. + graph := &metrics.ModuleGraph{ + Modules: []metrics.ModuleResult{ + { + Module: metrics.Module{Path: "example.com/foo", Name: "foo"}, + Instability: 0.75, + Distance: 0.50, + LCOM: 3, + Abstractness: 0.0, + Zone: metrics.ZoneNormal, + }, + { + Module: metrics.Module{Path: "example.com/bar", Name: "bar"}, + Instability: 0.25, + Distance: 0.10, + LCOM: 1, + Abstractness: 0.0, + Zone: metrics.ZoneNormal, + }, + }, + Cycles: []metrics.Cycle{ + {"example.com/a", "example.com/b"}, + }, + } + + tests := []struct { + name string + opts AnalyzeOptions + wantCount int + wantSubstr string + }{ + { + name: "no_thresholds", + opts: AnalyzeOptions{}, + wantCount: 0, + }, + { + name: "instability_one_violation", + opts: AnalyzeOptions{MaxInstability: float64Ptr(0.50)}, + wantCount: 1, // foo (0.75 > 0.50), bar passes (0.25 <= 0.50) + wantSubstr: "foo", + }, + { + name: "instability_boundary_passes", + opts: AnalyzeOptions{MaxInstability: float64Ptr(0.75)}, + wantCount: 0, // foo (0.75 == 0.75) passes with strict > + }, + { + name: "distance_one_violation", + opts: AnalyzeOptions{MaxDistance: float64Ptr(0.30)}, + wantCount: 1, // foo (0.50 > 0.30) + wantSubstr: "distance", + }, + { + name: "lcom_one_violation", + opts: AnalyzeOptions{MaxLCOM: intPtr(2)}, + wantCount: 1, // foo (3 > 2) + wantSubstr: "lcom", + }, + { + name: "lcom_boundary_passes", + opts: AnalyzeOptions{MaxLCOM: intPtr(3)}, + wantCount: 0, // foo (3 == 3) passes with strict > + }, + { + name: "circular_deps_violation", + opts: AnalyzeOptions{NoCircularDeps: true}, + wantCount: 1, + wantSubstr: "circular dependency", + }, + { + name: "circular_deps_not_checked", + opts: AnalyzeOptions{NoCircularDeps: false}, + wantCount: 0, + }, + { + name: "multiple_thresholds", + opts: AnalyzeOptions{ + MaxInstability: float64Ptr(0.50), + MaxDistance: float64Ptr(0.30), + MaxLCOM: intPtr(2), + NoCircularDeps: true, + }, + wantCount: 4, // foo: instability + distance + lcom, plus 1 cycle + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + violations := checkThresholds(graph, tt.opts) + if len(violations) != tt.wantCount { + t.Errorf("violation count: got %d, want %d\nviolations: %v", len(violations), tt.wantCount, violations) + } + if tt.wantSubstr != "" && len(violations) > 0 { + found := false + for _, v := range violations { + if strings.Contains(strings.ToLower(v), tt.wantSubstr) { + found = true + break + } + } + if !found { + t.Errorf("no violation contains %q\nviolations: %v", tt.wantSubstr, violations) + } + } + }) + } +} diff --git a/cmd/vibe-check/main.go b/cmd/vibe-check/main.go new file mode 100644 index 0000000..ad63d60 --- /dev/null +++ b/cmd/vibe-check/main.go @@ -0,0 +1,68 @@ +// Package main provides the vibe-check CLI tool for computing design quality +// and architectural metrics for Go codebases. +// +// Usage: +// +// vibe-check analyze [path] Analyze Go packages and compute coupling metrics +// vibe-check --version Print version information +// +// The analyze command produces JSON output conforming to the ModuleGraph schema +// (version 1.1) and supports CI gate flags for threshold enforcement. +package main + +import ( + "errors" + "fmt" + "io" + "os" +) + +// version, commit, and date are set at build time via ldflags. +var ( + version = "dev" + commit = "none" + date = "unknown" +) + +// exitCodeError wraps an error with a specific process exit code. +// This allows RunE functions to communicate exit codes to main() +// without calling os.Exit directly, preserving deferred cleanup. +type exitCodeError struct { + code int + err error +} + +func (e *exitCodeError) Error() string { return e.err.Error() } + +func (e *exitCodeError) Unwrap() error { return e.err } + +func main() { + os.Exit(run()) +} + +// run executes the root command and returns the process exit code. It is +// separated from main so the exit-code mapping is unit-testable; main itself +// only calls os.Exit. +func run() int { + return exitCode(rootCmd().Execute(), os.Stderr) +} + +// exitCode maps a command error to a process exit code: +// +// nil error → 0 (success) +// *exitCodeError → its carried code (e.g. 1 for policy failures) +// any other error → 2 (tool failure), with the error written to stderr +// +// Errors carried by *exitCodeError are already reported to stderr by the +// command layer, so they are not re-printed here. +func exitCode(err error, stderr io.Writer) int { + if err == nil { + return 0 + } + var ece *exitCodeError + if errors.As(err, &ece) { + return ece.code + } + _, _ = fmt.Fprintln(stderr, err) + return 2 +} diff --git a/cmd/vibe-check/main_test.go b/cmd/vibe-check/main_test.go new file mode 100644 index 0000000..ab39c02 --- /dev/null +++ b/cmd/vibe-check/main_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "bytes" + "errors" + "os" + "testing" +) + +// TestExitCode_Mapping verifies the error → process-exit-code mapping used by +// run(): success is 0, an *exitCodeError passes its code through unchanged, and +// any other error falls back to 2 with the message written to stderr. +func TestExitCode_Mapping(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + err error + wantCode int + wantMsg bool // whether the error should be written to stderr + }{ + { + name: "success", + err: nil, + wantCode: 0, + wantMsg: false, + }, + { + name: "policy_failure_passthrough", + err: &exitCodeError{code: 1, err: errors.New("threshold violations detected")}, + wantCode: 1, + wantMsg: false, // command layer already reported it + }, + { + name: "tool_failure_passthrough", + err: &exitCodeError{code: 2, err: errors.New("analyze failed")}, + wantCode: 2, + wantMsg: false, + }, + { + name: "fallback_plain_error", + err: errors.New("boom"), + wantCode: 2, + wantMsg: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var stderr bytes.Buffer + got := exitCode(tt.err, &stderr) + if got != tt.wantCode { + t.Errorf("exitCode: got %d, want %d", got, tt.wantCode) + } + if tt.wantMsg && stderr.Len() == 0 { + t.Error("expected error written to stderr, got none") + } + if !tt.wantMsg && stderr.Len() != 0 { + t.Errorf("expected no stderr output, got %q", stderr.String()) + } + }) + } +} + +// TestRun_VersionExitsZero exercises run() end-to-end for the success path via +// the --version flag, which prints and returns a nil error (exit 0). +func TestRun_VersionExitsZero(t *testing.T) { + // Not parallel: mutates the global os.Args, which run() reads via cobra. + oldArgs := os.Args + defer func() { os.Args = oldArgs }() + os.Args = []string{"vibe-check", "--version"} + + if code := run(); code != 0 { + t.Errorf("run() with --version: got %d, want 0", code) + } +} diff --git a/cmd/vibe-check/root.go b/cmd/vibe-check/root.go new file mode 100644 index 0000000..7f1114c --- /dev/null +++ b/cmd/vibe-check/root.go @@ -0,0 +1,55 @@ +package main + +import ( + "fmt" + "runtime/debug" + + "github.com/spf13/cobra" +) + +// rootCmd creates the root cobra command for vibe-check. +func rootCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "vibe-check", + Short: "Design quality and architectural metrics for Go codebases", + Long: `vibe-check computes package-level coupling metrics (afferent/efferent coupling, +instability, abstractness, distance from main sequence), cohesion analysis, +and circular dependency detection for Go codebases.`, + Version: versionString(), + } + + cmd.AddCommand(analyzeCmd()) + + return cmd +} + +// versionString builds the --version output value in the form +// " (commit , built )". When the ldflags-injected version +// is empty or the default "dev" (e.g., the binary was installed via +// `go install github.com/zero-dot-force/vibe-check/cmd/vibe-check@vX`), it falls +// back to build information embedded by the Go toolchain: the main module +// version and the vcs.revision / vcs.time build settings. This ensures +// --version reports meaningful data even without explicit ldflags. +func versionString() string { + v, c, d := version, commit, date + if v == "" || v == "dev" { + if info, ok := debug.ReadBuildInfo(); ok { + if mv := info.Main.Version; mv != "" && mv != "(devel)" { + v = mv + } + for _, s := range info.Settings { + switch s.Key { + case "vcs.revision": + if s.Value != "" { + c = s.Value + } + case "vcs.time": + if s.Value != "" { + d = s.Value + } + } + } + } + } + return fmt.Sprintf("%s (commit %s, built %s)", v, c, d) +} diff --git a/go.mod b/go.mod index 8e6a947..86da0fb 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,15 @@ module github.com/zero-dot-force/vibe-check go 1.25.7 + +require ( + github.com/spf13/cobra v1.10.2 + golang.org/x/tools v0.49.0 +) + +require ( + github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/spf13/pflag v1.0.9 // indirect + golang.org/x/mod v0.39.0 // indirect + golang.org/x/sync v0.22.0 // indirect +) diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..b9e1ed1 --- /dev/null +++ b/go.sum @@ -0,0 +1,18 @@ +github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= +github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= +github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= +github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= +github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= +github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +golang.org/x/mod v0.39.0 h1:UF5zwQdCRRUpHfyPwr7d4UrGiVeldIsogtzWVnczL74= +golang.org/x/mod v0.39.0/go.mod h1:bvIbwjQ0HUFFf5AKukeeYQG4ZBUG9yxQbR9aEweIwYY= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= +golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/goadapter/adapter.go b/internal/goadapter/adapter.go new file mode 100644 index 0000000..6693f6a --- /dev/null +++ b/internal/goadapter/adapter.go @@ -0,0 +1,197 @@ +package goadapter + +import ( + "context" + "errors" + "fmt" + "sort" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +// errTotalLoadFailure indicates that packages were found in the target +// directory but none could be type-checked (every package has load/type +// errors or nil type information). It is returned — wrapped with actionable +// remediation via %w — instead of a graph of all-zeroed modules, per the +// go-adapter total-load-failure scenario. Emitting all-zero metrics as if +// real, when nothing could be analyzed, would violate Metric Fidelity. +// Callers may test for it with errors.Is. +var errTotalLoadFailure = errors.New("total load failure") + +// Compile-time interface compliance check. +var _ metrics.Adapter = (*Adapter)(nil) + +// Adapter implements [metrics.Adapter] for Go codebases. It uses +// [golang.org/x/tools/go/packages] to load and analyze Go packages, +// computing the full Martin metrics suite plus Go-specific extensions. +type Adapter struct{} + +// New creates a new Go language adapter. +func New() *Adapter { + return &Adapter{} +} + +// Language returns "go" as the language identifier. +func (a *Adapter) Language() string { + return "go" +} + +// Capabilities returns all universal metrics this adapter can compute. +// The Go adapter supports the complete metrics suite. Go-specific extension +// capabilities are reported separately by [Adapter.ExtensionCapabilities]. +func (a *Adapter) Capabilities() []metrics.Capability { + return []metrics.Capability{ + metrics.CapAfferentCoupling, + metrics.CapEfferentCoupling, + metrics.CapInstability, + metrics.CapAbstractness, + metrics.CapDistance, + metrics.CapLCOM, + metrics.CapCircularDeps, + } +} + +// ExtensionCapabilities returns the optional Go-specific extension capabilities +// this adapter populates in each ModuleResult's Extensions map. These are +// declared separately from the universal [Adapter.Capabilities] because they +// are language-specific and namespaced under the "go." prefix. The returned +// identifiers double as the Extensions map keys. +func (a *Adapter) ExtensionCapabilities() []string { + return []string{CapInterfaceWidth, CapInterfaceProximity} +} + +// Analyze performs a complete analysis of the Go project at projectPath. +// It loads all packages in the module, computes coupling metrics (Ca, Ce), +// type classification (exported/abstract), LCOM4 cohesion, derived metrics +// (instability, abstractness, distance, zone), and detects circular dependencies. +// +// The returned [metrics.ModuleGraph] conforms to schema version "1.1". +// Status is [metrics.StatusComplete] when all packages load without errors, +// or [metrics.StatusPartial] when some packages have errors but analysis +// can still proceed. +// +// Returns an error if: +// - projectPath fails validation +// - the context is cancelled or expired +// - no Go packages are found in the project +// - the module path cannot be determined +// - no package can be type-checked (total load failure) +func (a *Adapter) Analyze(ctx context.Context, projectPath string) (*metrics.ModuleGraph, error) { + // Step 1: Validate project path. + if err := metrics.ValidateProjectPath(projectPath); err != nil { + return nil, fmt.Errorf("analyze: %w", err) + } + + // Step 2: Check context before expensive operations. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("analyze: %w", err) + } + + // Step 3: Load and resolve packages. + pkgs, imports, warnings, err := resolvePackages(ctx, projectPath) + if err != nil { + return nil, fmt.Errorf("analyze: %w", err) + } + + if len(pkgs) == 0 { + return nil, fmt.Errorf("analyze: no Go packages found in %s — ensure the path is a Go module directory containing .go files and a go.mod", projectPath) + } + + // Per the go-adapter total-load-failure scenario: packages were found, but + // if NONE could be type-checked (every package has load/type errors or nil + // type information) the load failed entirely. Returning a graph of + // all-zeroed modules would present fabricated metrics as if real, violating + // Metric Fidelity — return an error instead. The partial case (at least one + // package type-checks) is handled below by emitting zeroed ModuleResults + // plus warnings for the individual errored packages. + if !anyPackageTypeChecked(pkgs) { + return nil, fmt.Errorf("analyze: %w: none of the %d package(s) in %s could be type-checked — run 'go build ./...' to see the underlying errors, then ensure all dependencies are available (e.g. run 'go mod download')", errTotalLoadFailure, len(pkgs), projectPath) + } + + // Build set of module-internal package paths for cycle detection. + modulePkgs := make(map[string]bool, len(pkgs)) + for _, pkg := range pkgs { + modulePkgs[pkg.PkgPath] = true + } + + // Step 4: Compute metrics for each package. + // + // Per the go-adapter partial-build scenario, packages that fail to load or + // type-check are NOT dropped. We emit a zeroed ModuleResult for them + // (ExportedTypes=0, AbstractTypes=0, LCOM=0) so they still appear in the + // graph, while resolvePackages records a corresponding warning. Coupling + // (Ca/Ce) is still computed from the import graph, which is safe even when + // type-checking is incomplete. + var modules []metrics.ModuleResult + for _, pkg := range pkgs { + // Coupling metrics are always safe to compute from the import graph. + ca := countCa(pkg.PkgPath, imports) + ce := countCe(pkg) + + // Type classification and LCOM require complete type information. + // Packages with load/type errors or nil types get zeroed values; a + // warning for them has already been recorded in resolvePackages. + var exportedTypes, abstractTypes int + var lcom metrics.LCOM + var extensions map[string]any + if len(pkg.Errors) == 0 && pkg.Types != nil { + exportedTypes, abstractTypes = countTypes(pkg) + lcom = computeLCOM4(pkg) + extensions = computeExtensions(pkg) + } + + // Derived metrics via metrics.Compute* functions. + instability := metrics.ComputeInstability(ca, ce) + abstractness := metrics.ComputeAbstractness(abstractTypes, exportedTypes) + distance := metrics.ComputeDistance(abstractness, instability) + zone := metrics.ComputeZone(abstractness, instability, distance) + + modules = append(modules, metrics.ModuleResult{ + Module: metrics.Module{ + Path: pkg.PkgPath, + Name: pkg.Name, + Ca: ca, + Ce: ce, + ExportedTypes: exportedTypes, + AbstractTypes: abstractTypes, + }, + Instability: instability, + Abstractness: abstractness, + Distance: distance, + LCOM: lcom, + Zone: zone, + Extensions: extensions, + }) + } + + // Step 5: Detect circular dependencies. + cycles := detectCycles(imports, modulePkgs) + + // Step 6: Sort modules by Path for deterministic output. + sort.Slice(modules, func(i, j int) bool { + return modules[i].Path < modules[j].Path + }) + + // Ensure non-nil slices per ModuleGraph contract. + if warnings == nil { + warnings = []metrics.Warning{} + } + if modules == nil { + modules = []metrics.ModuleResult{} + } + + // Step 7: Determine status. + status := metrics.StatusComplete + if len(warnings) > 0 { + status = metrics.StatusPartial + } + + return &metrics.ModuleGraph{ + SchemaVersion: metrics.SchemaVersionCurrent, + Language: "go", + Modules: modules, + Cycles: cycles, + Warnings: warnings, + Status: status, + }, nil +} diff --git a/internal/goadapter/adapter_test.go b/internal/goadapter/adapter_test.go new file mode 100644 index 0000000..007a3f2 --- /dev/null +++ b/internal/goadapter/adapter_test.go @@ -0,0 +1,434 @@ +package goadapter + +import ( + "context" + "encoding/json" + "errors" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +// fixtureDir returns the absolute path to a test fixture directory. +func fixtureDir(t *testing.T, name string) string { + t.Helper() + return filepath.Join(testdataDir(t), name) +} + +func TestAdapter_CouplingMetrics(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + graph, err := adapter.Analyze(context.Background(), fixtureDir(t, "coupling")) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + // Build lookup by package name for easier assertions. + byName := make(map[string]metrics.ModuleResult) + for _, m := range graph.Modules { + byName[m.Name] = m + } + + tests := []struct { + name string + wantCa int + wantCe int + }{ + {"pkga", 0, 2}, // Ce=2: imports fmt (stdlib) + pkgb (internal) + {"pkgb", 2, 0}, // Ce=0: no imports + {"pkgc", 0, 1}, // Ce=1: imports pkgb (internal only) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + m, ok := byName[tt.name] + if !ok { + t.Fatalf("package %q not found in results", tt.name) + } + if m.Ca != tt.wantCa { + t.Errorf("Ca: got %d, want %d", m.Ca, tt.wantCa) + } + if m.Ce != tt.wantCe { + t.Errorf("Ce: got %d, want %d", m.Ce, tt.wantCe) + } + }) + } +} + +func TestAdapter_StdlibExclusion(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + graph, err := adapter.Analyze(context.Background(), fixtureDir(t, "coupling")) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + // Verify no stdlib packages appear in the module list. + for _, m := range graph.Modules { + if m.Path == "fmt" || m.Path == "io" || m.Path == "strings" { + t.Errorf("stdlib package %q should not appear in modules", m.Path) + } + } + + // pkga imports fmt (stdlib) + pkgb (internal). + // Ce counts all imports including stdlib per Martin's definition. + for _, m := range graph.Modules { + if m.Name == "pkga" { + if m.Ce != 2 { + t.Errorf("pkga Ce: got %d, want 2 (should count fmt + pkgb)", m.Ce) + } + } + } +} + +func TestAdapter_ExternalExclusion(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + graph, err := adapter.Analyze(context.Background(), fixtureDir(t, "coupling")) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + // All modules should have paths starting with the module prefix. + for _, m := range graph.Modules { + if !strings.HasPrefix(m.Path, "example.com/coupling") { + t.Errorf("external package %q should not appear in modules", m.Path) + } + } +} + +func TestAdapter_Integration(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + graph, err := adapter.Analyze(context.Background(), fixtureDir(t, "coupling")) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + // Verify schema version. + if graph.SchemaVersion != "1.1" { + t.Errorf("SchemaVersion: got %q, want %q", graph.SchemaVersion, "1.1") + } + + // Verify language. + if graph.Language != "go" { + t.Errorf("Language: got %q, want %q", graph.Language, "go") + } + + // Verify status. + if graph.Status != metrics.StatusComplete { + t.Errorf("Status: got %q, want %q", graph.Status, metrics.StatusComplete) + } + + // Verify non-nil slices. + if graph.Modules == nil { + t.Error("Modules is nil, want non-nil") + } + if graph.Cycles == nil { + t.Error("Cycles is nil, want non-nil") + } + if graph.Warnings == nil { + t.Error("Warnings is nil, want non-nil") + } + + // Verify output passes schema validation via JSON round-trip. + data, err := json.Marshal(graph) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + if err := metrics.Validate(data); err != nil { + t.Errorf("Validate: %v", err) + } +} + +func TestAdapter_Determinism(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + // Verify byte-identical JSON output across repeated runs over multiple + // fixtures. The coupling fixture exercises module ordering; the partial + // fixture additionally exercises stable warning ordering (it yields + // warnings for its errored package). + for _, fixture := range []string{"coupling", "partial"} { + t.Run(fixture, func(t *testing.T) { + t.Parallel() + assertDeterministicAnalyze(t, fixture, 10) + }) + } +} + +// assertDeterministicAnalyze runs Analyze over the named fixture n times and +// fails if any run's marshaled JSON differs from the first run's output. +func assertDeterministicAnalyze(t *testing.T, fixture string, n int) { + t.Helper() + + adapter := New() + ctx := context.Background() + dir := fixtureDir(t, fixture) + + var reference []byte + for i := 0; i < n; i++ { + graph, err := adapter.Analyze(ctx, dir) + if err != nil { + t.Fatalf("run %d: Analyze: %v", i, err) + } + + data, err := json.Marshal(graph) + if err != nil { + t.Fatalf("run %d: marshal: %v", i, err) + } + + if i == 0 { + reference = data + continue + } + + if string(data) != string(reference) { + t.Errorf("run %d: output differs from reference\ngot: %s\nwant: %s", i, data, reference) + } + } +} + +func TestAdapter_ContextCancellation(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + ctx, cancel := context.WithCancel(context.Background()) + cancel() // Cancel immediately. + + _, err := adapter.Analyze(ctx, fixtureDir(t, "coupling")) + if err == nil { + t.Fatal("expected error for cancelled context, got nil") + } + if !errors.Is(err, context.Canceled) { + t.Errorf("expected error wrapping context.Canceled, got: %v", err) + } +} + +func TestAdapter_ContextDeadline(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + + _, err := adapter.Analyze(ctx, fixtureDir(t, "coupling")) + if err == nil { + t.Fatal("expected error for expired deadline, got nil") + } + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("expected error wrapping context.DeadlineExceeded, got: %v", err) + } +} + +func TestAdapter_PathValidation(t *testing.T) { + t.Parallel() + + adapter := New() + ctx := context.Background() + + tests := []struct { + name string + path string + }{ + {"traversal", "/tmp/../etc/passwd"}, + {"non-existent", "/nonexistent/path/that/does/not/exist"}, + {"empty", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + _, err := adapter.Analyze(ctx, tt.path) + if err == nil { + t.Errorf("expected error for path %q, got nil", tt.path) + } + }) + } +} + +func TestAdapter_EmptyDirectory(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + + // The empty fixture has only .gitkeep, no Go files. + _, err := adapter.Analyze(context.Background(), fixtureDir(t, "empty")) + if err == nil { + t.Fatal("expected error for empty directory, got nil") + } +} + +func TestAdapter_PartialBuild(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + graph, err := adapter.Analyze(context.Background(), fixtureDir(t, "partial")) + if err != nil { + t.Fatalf("Analyze: %v", err) + } + + // Should have partial status due to the bad package. + if graph.Status != metrics.StatusPartial { + t.Errorf("Status: got %q, want %q", graph.Status, metrics.StatusPartial) + } + + // Should have at least one warning about the bad package. + if len(graph.Warnings) == 0 { + t.Fatal("expected warnings for partial build, got none") + } + + // Verify warning format per spec: Code, Module, and Message fields. + w := graph.Warnings[0] + if w.Code != "load-error" { + t.Errorf("Warning.Code: got %q, want %q", w.Code, "load-error") + } + if w.Module == "" { + t.Error("Warning.Module is empty, want affected package path") + } + if w.Message == "" { + t.Error("Warning.Message is empty, want error description") + } + // Warning.Module should contain the partial package path. + if !strings.Contains(w.Module, "bad") { + t.Errorf("Warning.Module %q should reference the bad package", w.Module) + } + + byName := make(map[string]metrics.ModuleResult) + for _, m := range graph.Modules { + byName[m.Name] = m + } + + // The good package should still be in the results. + if _, ok := byName["good"]; !ok { + t.Error("good package not found in partial build results") + } + + // Per the go-adapter partial-build scenario, the errored package MUST appear + // as a zeroed ModuleResult rather than being silently dropped. + bad, ok := byName["bad"] + if !ok { + t.Fatal(`errored package "bad" not found in results; it must appear as a zeroed ModuleResult`) + } + if bad.ExportedTypes != 0 { + t.Errorf("bad.ExportedTypes: got %d, want 0", bad.ExportedTypes) + } + if bad.AbstractTypes != 0 { + t.Errorf("bad.AbstractTypes: got %d, want 0", bad.AbstractTypes) + } + if bad.LCOM != 0 { + t.Errorf("bad.LCOM: got %d, want 0", bad.LCOM) + } + + // A warning MUST exist naming the errored package. + foundWarning := false + for _, w := range graph.Warnings { + if w.Module == bad.Path { + foundWarning = true + break + } + } + if !foundWarning { + t.Errorf("no warning found for errored package %q", bad.Path) + } +} + +// TestAdapter_TotalLoadFailure verifies the go-adapter total-load-failure +// scenario: when packages are found but NONE can be type-checked, Analyze must +// return an error rather than a graph of all-zeroed modules. Emitting all-zero +// metrics as if real, when nothing could be analyzed, would violate Metric +// Fidelity. This is distinct from the partial build (TestAdapter_PartialBuild), +// where at least one package type-checks and the graph is returned with +// warnings for the errored packages. +func TestAdapter_TotalLoadFailure(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip("skipping integration test") + } + + adapter := New() + // The allbad fixture contains only packages that import non-existent + // packages, so every package fails to type-check. + graph, err := adapter.Analyze(context.Background(), fixtureDir(t, "allbad")) + if err == nil { + t.Fatal("expected error when no package can be type-checked, got nil") + } + if !errors.Is(err, errTotalLoadFailure) { + t.Errorf("expected error wrapping errTotalLoadFailure, got: %v", err) + } + // A total load failure MUST NOT yield a graph of all-zeroed modules. + if graph != nil { + t.Errorf("expected nil graph on total load failure, got %d modules", len(graph.Modules)) + } +} + +func TestAdapter_Language(t *testing.T) { + t.Parallel() + + adapter := New() + if got := adapter.Language(); got != "go" { + t.Errorf("Language: got %q, want %q", got, "go") + } +} + +func TestAdapter_Capabilities(t *testing.T) { + t.Parallel() + + adapter := New() + caps := adapter.Capabilities() + + if len(caps) != 7 { + t.Fatalf("Capabilities count: got %d, want 7", len(caps)) + } + + expected := map[metrics.Capability]bool{ + metrics.CapAfferentCoupling: true, + metrics.CapEfferentCoupling: true, + metrics.CapInstability: true, + metrics.CapAbstractness: true, + metrics.CapDistance: true, + metrics.CapLCOM: true, + metrics.CapCircularDeps: true, + } + + for _, cap := range caps { + if !expected[cap] { + t.Errorf("unexpected capability: %q", cap) + } + } +} diff --git a/internal/goadapter/cycles.go b/internal/goadapter/cycles.go new file mode 100644 index 0000000..33b000b --- /dev/null +++ b/internal/goadapter/cycles.go @@ -0,0 +1,120 @@ +package goadapter + +import ( + "sort" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +// detectCycles runs Tarjan's SCC algorithm on the module-internal import graph +// and returns cycles with canonical ordering. Only module-internal edges are +// considered (edges to packages not in modulePkgs are ignored). +// +// Each cycle's members are reported as a lexicographically-sorted set of +// package paths, per the [metrics.Cycle] contract; the ordering does not encode +// traversal order. The returned slice of cycles is itself sorted by first +// element. Returns an empty (non-nil) slice if no cycles exist. +func detectCycles(imports map[string][]string, modulePkgs map[string]bool) []metrics.Cycle { + t := &tarjan{ + imports: imports, + modulePkgs: modulePkgs, + index: make(map[string]int), + lowlink: make(map[string]int), + onStack: make(map[string]bool), + counter: 0, + } + + // Run Tarjan's on all module-internal nodes. + for pkg := range modulePkgs { + if _, visited := t.index[pkg]; !visited { + t.strongConnect(pkg) + } + } + + // Filter SCCs to only those with size > 1 (actual cycles). + var cycles []metrics.Cycle + for _, scc := range t.sccs { + if len(scc) <= 1 { + continue + } + cycles = append(cycles, canonicalizeCycle(scc)) + } + + // Sort cycles by first element for deterministic output. + sort.Slice(cycles, func(i, j int) bool { + return cycles[i][0] < cycles[j][0] + }) + + // Ensure non-nil slice per ModuleGraph contract. + if cycles == nil { + cycles = []metrics.Cycle{} + } + + return cycles +} + +// tarjan holds the state for Tarjan's SCC algorithm. +type tarjan struct { + imports map[string][]string + modulePkgs map[string]bool + index map[string]int + lowlink map[string]int + onStack map[string]bool + stack []string + counter int + sccs [][]string +} + +// strongConnect is the recursive Tarjan's SCC procedure. +func (t *tarjan) strongConnect(v string) { + t.index[v] = t.counter + t.lowlink[v] = t.counter + t.counter++ + t.stack = append(t.stack, v) + t.onStack[v] = true + + // Consider successors: only module-internal edges. + for _, w := range t.imports[v] { + if !t.modulePkgs[w] { + continue + } + if _, visited := t.index[w]; !visited { + t.strongConnect(w) + if t.lowlink[w] < t.lowlink[v] { + t.lowlink[v] = t.lowlink[w] + } + } else if t.onStack[w] { + if t.index[w] < t.lowlink[v] { + t.lowlink[v] = t.index[w] + } + } + } + + // If v is a root node, pop the SCC. + if t.lowlink[v] == t.index[v] { + var scc []string + for { + w := t.stack[len(t.stack)-1] + t.stack = t.stack[:len(t.stack)-1] + t.onStack[w] = false + scc = append(scc, w) + if w == v { + break + } + } + t.sccs = append(t.sccs, scc) + } +} + +// canonicalizeCycle returns the SCC members as a deterministic, fully +// lexicographically-sorted set. Tarjan's algorithm yields SCC members in an +// order that depends on graph traversal; sorting produces a stable +// representation independent of traversal order, matching the [metrics.Cycle] +// contract. The result is the sorted membership set of the cycle, not a +// traversal path. +func canonicalizeCycle(scc []string) metrics.Cycle { + sorted := make([]string, len(scc)) + copy(sorted, scc) + sort.Strings(sorted) + return metrics.Cycle(sorted) +} diff --git a/internal/goadapter/cycles_test.go b/internal/goadapter/cycles_test.go new file mode 100644 index 0000000..cb479b3 --- /dev/null +++ b/internal/goadapter/cycles_test.go @@ -0,0 +1,176 @@ +package goadapter + +import ( + "encoding/json" + "testing" +) + +func TestDetectCycles_Acyclic(t *testing.T) { + t.Parallel() + + imports := map[string][]string{ + "example.com/foo/a": {"example.com/foo/b"}, + "example.com/foo/b": {"example.com/foo/c"}, + "example.com/foo/c": {}, + } + modulePkgs := map[string]bool{ + "example.com/foo/a": true, + "example.com/foo/b": true, + "example.com/foo/c": true, + } + + cycles := detectCycles(imports, modulePkgs) + + if cycles == nil { + t.Fatal("cycles slice is nil, want non-nil empty slice") + } + if len(cycles) != 0 { + t.Errorf("got %d cycles, want 0", len(cycles)) + } +} + +func TestDetectCycles_ConstructedCycle(t *testing.T) { + t.Parallel() + + // A→B→C→A forms a cycle. + imports := map[string][]string{ + "example.com/foo/a": {"example.com/foo/b"}, + "example.com/foo/b": {"example.com/foo/c"}, + "example.com/foo/c": {"example.com/foo/a"}, + } + modulePkgs := map[string]bool{ + "example.com/foo/a": true, + "example.com/foo/b": true, + "example.com/foo/c": true, + } + + cycles := detectCycles(imports, modulePkgs) + + if len(cycles) != 1 { + t.Fatalf("got %d cycles, want 1", len(cycles)) + } + if len(cycles[0]) != 3 { + t.Fatalf("cycle length: got %d, want 3", len(cycles[0])) + } +} + +func TestDetectCycles_CanonicalOrdering(t *testing.T) { + t.Parallel() + + // Cycle: C→A→B→C. Canonical ordering should start with A. + imports := map[string][]string{ + "example.com/foo/c": {"example.com/foo/a"}, + "example.com/foo/a": {"example.com/foo/b"}, + "example.com/foo/b": {"example.com/foo/c"}, + } + modulePkgs := map[string]bool{ + "example.com/foo/a": true, + "example.com/foo/b": true, + "example.com/foo/c": true, + } + + cycles := detectCycles(imports, modulePkgs) + + if len(cycles) != 1 { + t.Fatalf("got %d cycles, want 1", len(cycles)) + } + + cycle := cycles[0] + if cycle[0] != "example.com/foo/a" { + t.Errorf("first element: got %q, want %q", cycle[0], "example.com/foo/a") + } + if cycle[1] != "example.com/foo/b" { + t.Errorf("second element: got %q, want %q", cycle[1], "example.com/foo/b") + } + if cycle[2] != "example.com/foo/c" { + t.Errorf("third element: got %q, want %q", cycle[2], "example.com/foo/c") + } +} + +// TestDetectCycles_SortedSetSemantics pins the [metrics.Cycle] contract: +// cycles are reported as the fully lexicographically-sorted member set, not a +// rotated traversal path. The import graph B→A→C→B has a traversal order that +// differs from the sorted set: a rotation-to-smallest-first scheme would yield +// [A, C, B], whereas sorted-set semantics yield [A, B, C]. Asserting the fully +// sorted order distinguishes the two schemes non-coincidentally. +func TestDetectCycles_SortedSetSemantics(t *testing.T) { + t.Parallel() + + // Cycle: B→A→C→B. + imports := map[string][]string{ + "example.com/foo/b": {"example.com/foo/a"}, + "example.com/foo/a": {"example.com/foo/c"}, + "example.com/foo/c": {"example.com/foo/b"}, + } + modulePkgs := map[string]bool{ + "example.com/foo/a": true, + "example.com/foo/b": true, + "example.com/foo/c": true, + } + + cycles := detectCycles(imports, modulePkgs) + + if len(cycles) != 1 { + t.Fatalf("got %d cycles, want 1", len(cycles)) + } + + want := []string{"example.com/foo/a", "example.com/foo/b", "example.com/foo/c"} + got := []string(cycles[0]) + if len(got) != len(want) { + t.Fatalf("cycle length: got %d, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("cycle[%d]: got %q, want %q (canonical form is the fully-sorted member set)", + i, got[i], want[i]) + } + } +} + +// TestDetectCycles_Determinism verifies that cycle detection produces +// byte-identical, stably-ordered output across repeated runs. detectCycles +// seeds Tarjan's algorithm by ranging over a map (whose iteration order is +// randomized by the runtime), so this test guards against nondeterministic +// output leaking through. +func TestDetectCycles_Determinism(t *testing.T) { + t.Parallel() + + // Two independent cycles (a→b→c→a and x→y→x) plus an acyclic node z. + imports := map[string][]string{ + "example.com/m/a": {"example.com/m/b"}, + "example.com/m/b": {"example.com/m/c"}, + "example.com/m/c": {"example.com/m/a"}, + "example.com/m/x": {"example.com/m/y"}, + "example.com/m/y": {"example.com/m/x"}, + "example.com/m/z": {"example.com/m/a"}, + } + modulePkgs := map[string]bool{ + "example.com/m/a": true, + "example.com/m/b": true, + "example.com/m/c": true, + "example.com/m/x": true, + "example.com/m/y": true, + "example.com/m/z": true, + } + + var reference string + for i := 0; i < 20; i++ { + cycles := detectCycles(imports, modulePkgs) + data, err := json.Marshal(cycles) + if err != nil { + t.Fatalf("run %d: marshal: %v", i, err) + } + if i == 0 { + reference = string(data) + continue + } + if string(data) != reference { + t.Errorf("run %d: cycle output differs from reference\ngot: %s\nwant: %s", i, data, reference) + } + } + + // Sanity check: exactly two cycles are detected. + if got := detectCycles(imports, modulePkgs); len(got) != 2 { + t.Fatalf("got %d cycles, want 2", len(got)) + } +} diff --git a/internal/goadapter/doc.go b/internal/goadapter/doc.go new file mode 100644 index 0000000..2b37002 --- /dev/null +++ b/internal/goadapter/doc.go @@ -0,0 +1,42 @@ +// Package goadapter implements a Go-native language adapter for the vibe-check +// metrics toolkit. It analyzes Go packages using [golang.org/x/tools/go/packages] +// to compute coupling metrics (Ca, Ce), abstractness, instability, distance from +// main sequence, LCOM4 cohesion, and circular dependency detection. +// +// The adapter implements the [metrics.Adapter] interface and produces a +// [metrics.ModuleGraph] that conforms to schema version "1.1". Each Go package +// within the target module is treated as one [metrics.Module]. +// +// # Type Classification +// +// An exported type is classified as abstract when its underlying type is an +// interface — this covers interface declarations and defined types whose +// underlying type is an interface (e.g., type Z SomeInterface). All other +// exported types (structs, other named types, and type aliases, including an +// alias to an interface) are classified as concrete. Unexported types are +// excluded from both counts. +// +// # LCOM4 Computation +// +// LCOM4 uses the Hitz & Montazeri (1995) connected-component variant. +// Nodes are exported methods (functions with a receiver of an exported type). +// Edges connect methods that access at least one common struct field. +// Package-level functions (no receiver) are excluded from the graph. +// +// # Extensions +// +// The adapter populates language-specific extensions under the "go." namespace: +// - go.interfaceWidth: method count per exported interface (map[string]int) +// - go.interfaceProximity: "consumer" or "producer" per interface (map[string]string) +// +// Use [InterfaceWidths] and [InterfaceProximities] to safely extract typed +// extension values after JSON round-trip. +// +// # Security +// +// Analysis loads packages with [golang.org/x/tools/go/packages], which invokes +// the Go toolchain and may execute code (e.g., cgo preprocessing) from the +// analyzed module. Run vibe-check only on trusted, self-owned code. The adapter +// sanitizes the subprocess environment via [metrics.SanitizeEnvironment] and +// excludes GOFLAGS to remove the -toolexec command-execution vector. +package goadapter diff --git a/internal/goadapter/extensions.go b/internal/goadapter/extensions.go new file mode 100644 index 0000000..3adf8b1 --- /dev/null +++ b/internal/goadapter/extensions.go @@ -0,0 +1,192 @@ +package goadapter + +import ( + "fmt" + "go/types" + + "golang.org/x/tools/go/packages" +) + +// Extension capability identifiers for Go-specific metrics. These are optional +// capabilities beyond the universal metrics.Cap* set defined in the metrics +// package. They live in the adapter package because they are language-specific, +// and are surfaced via [Adapter.ExtensionCapabilities] rather than the core +// [Adapter.Capabilities]. Each constant also serves as the key under which its +// value is stored in a ModuleResult's Extensions map. +const ( + // CapInterfaceWidth is the extension capability and Extensions map key for + // per-interface flattened method counts (map[string]int). + CapInterfaceWidth = "go.interfaceWidth" + // CapInterfaceProximity is the extension capability and Extensions map key + // for per-interface "producer"/"consumer" classification (map[string]string). + CapInterfaceProximity = "go.interfaceProximity" +) + +// computeExtensions computes Go-specific extension metrics for a package. +// It populates "go.interfaceWidth" (method count per exported interface) and +// "go.interfaceProximity" ("producer" or "consumer" per interface). +// +// Returns nil if no exported interfaces exist in the package, which causes +// the extensions field to be omitted from JSON output (omitempty). +func computeExtensions(pkg *packages.Package) map[string]any { + if pkg.Types == nil { + return nil + } + + scope := pkg.Types.Scope() + widths := make(map[string]int) + proximities := make(map[string]string) + + // Collect all exported interfaces and their flattened method counts. + var ifaceTypes []*types.Interface + var ifaceNames []string + + for _, name := range scope.Names() { + obj := scope.Lookup(name) + tn, ok := obj.(*types.TypeName) + if !ok || !tn.Exported() { + continue + } + + iface, ok := tn.Type().Underlying().(*types.Interface) + if !ok { + continue + } + + // Flattened method count includes methods from embedded interfaces. + widths[name] = iface.NumMethods() + ifaceTypes = append(ifaceTypes, iface) + ifaceNames = append(ifaceNames, name) + } + + if len(widths) == 0 { + return nil + } + + // Collect all concrete types in the package for proximity analysis. + var concreteTypes []types.Type + for _, name := range scope.Names() { + obj := scope.Lookup(name) + tn, ok := obj.(*types.TypeName) + if !ok || !tn.Exported() { + continue + } + + // Skip interfaces — we want concrete types only. + if _, isIface := tn.Type().Underlying().(*types.Interface); isIface { + continue + } + + // Check both the type and its pointer variant. + concreteTypes = append(concreteTypes, tn.Type()) + } + + // Determine proximity for each interface. + for i, iface := range ifaceTypes { + name := ifaceNames[i] + proximities[name] = computeProximity(iface, concreteTypes) + } + + return map[string]any{ + CapInterfaceWidth: widths, + CapInterfaceProximity: proximities, + } +} + +// computeProximity determines whether an interface is a "producer" or "consumer" +// in the context of the package where it is declared. +// +// An interface is a "producer" if any concrete type in the same package implements +// it (the package produces implementations). Otherwise it is a "consumer" (the +// package consumes implementations provided by other packages). +func computeProximity(iface *types.Interface, concreteTypes []types.Type) string { + for _, ct := range concreteTypes { + // Check if the concrete type or its pointer implements the interface. + if types.Implements(ct, iface) { + return "producer" + } + ptr := types.NewPointer(ct) + if types.Implements(ptr, iface) { + return "producer" + } + } + return "consumer" +} + +// InterfaceWidths extracts the "go.interfaceWidth" extension from a +// [metrics.ModuleResult]'s Extensions map. It handles the JSON round-trip +// conversion where int values become float64 after unmarshaling. +// +// Returns an error if the key is missing or has an unexpected type. +func InterfaceWidths(extensions map[string]any) (map[string]int, error) { + raw, ok := extensions[CapInterfaceWidth] + if !ok { + return nil, fmt.Errorf("extension key %q not found", CapInterfaceWidth) + } + + // Before JSON round-trip: map[string]int. + if typed, ok := raw.(map[string]int); ok { + result := make(map[string]int, len(typed)) + for k, v := range typed { + result[k] = v + } + return result, nil + } + + // After JSON round-trip: map[string]interface{} with float64 values. + rawMap, ok := raw.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("extension key %q: expected map, got %T", CapInterfaceWidth, raw) + } + + result := make(map[string]int, len(rawMap)) + for k, v := range rawMap { + switch n := v.(type) { + case float64: + result[k] = int(n) + case int: + result[k] = n + default: + return nil, fmt.Errorf("extension key %q: value for %q has unexpected type %T", CapInterfaceWidth, k, v) + } + } + + return result, nil +} + +// InterfaceProximities extracts the "go.interfaceProximity" extension from a +// [metrics.ModuleResult]'s Extensions map. +// +// Returns an error if the key is missing or has an unexpected type. +func InterfaceProximities(extensions map[string]any) (map[string]string, error) { + raw, ok := extensions[CapInterfaceProximity] + if !ok { + return nil, fmt.Errorf("extension key %q not found", CapInterfaceProximity) + } + + // Before JSON round-trip: map[string]string. + if typed, ok := raw.(map[string]string); ok { + result := make(map[string]string, len(typed)) + for k, v := range typed { + result[k] = v + } + return result, nil + } + + // After JSON round-trip: map[string]interface{} with string values. + rawMap, ok := raw.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("extension key %q: expected map, got %T", CapInterfaceProximity, raw) + } + + result := make(map[string]string, len(rawMap)) + for k, v := range rawMap { + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("extension key %q: value for %q has unexpected type %T", CapInterfaceProximity, k, v) + } + result[k] = s + } + + return result, nil +} diff --git a/internal/goadapter/extensions_test.go b/internal/goadapter/extensions_test.go new file mode 100644 index 0000000..17a58e8 --- /dev/null +++ b/internal/goadapter/extensions_test.go @@ -0,0 +1,267 @@ +package goadapter + +import ( + "encoding/json" + "strings" + "testing" +) + +func TestExtensions_InterfaceWidth(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "extensions", "ifaces") + ext := computeExtensions(pkg) + + if ext == nil { + t.Fatal("extensions is nil, want non-nil") + } + + widths, err := InterfaceWidths(ext) + if err != nil { + t.Fatalf("InterfaceWidths: %v", err) + } + + cases := map[string]int{ + "Reader": 1, + "Processor": 3, + "Embedder": 2, + } + for name, want := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + got, ok := widths[name] + if !ok { + t.Fatalf("width for %q not found", name) + } + if got != want { + t.Errorf("width for %q: got %d, want %d", name, got, want) + } + }) + } +} + +func TestExtensions_InterfaceProximity(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "extensions", "ifaces") + ext := computeExtensions(pkg) + + if ext == nil { + t.Fatal("extensions is nil, want non-nil") + } + + proximities, err := InterfaceProximities(ext) + if err != nil { + t.Fatalf("InterfaceProximities: %v", err) + } + + cases := map[string]string{ + "Reader": "producer", + "Processor": "consumer", + "Embedder": "producer", + } + for name, want := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + got, ok := proximities[name] + if !ok { + t.Fatalf("proximity for %q not found", name) + } + if got != want { + t.Errorf("proximity for %q: got %q, want %q", name, got, want) + } + }) + } +} + +func TestInterfaceWidths_RoundTrip(t *testing.T) { + t.Parallel() + + // Simulate JSON round-trip: int values become float64. + original := map[string]any{ + "go.interfaceWidth": map[string]int{ + "Reader": 1, + "Writer": 2, + }, + } + + data, err := json.Marshal(original) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var roundTripped map[string]any + if err := json.Unmarshal(data, &roundTripped); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + widths, err := InterfaceWidths(roundTripped) + if err != nil { + t.Fatalf("InterfaceWidths after round-trip: %v", err) + } + + if widths["Reader"] != 1 { + t.Errorf("Reader width: got %d, want %d", widths["Reader"], 1) + } + if widths["Writer"] != 2 { + t.Errorf("Writer width: got %d, want %d", widths["Writer"], 2) + } +} + +func TestInterfaceProximities_RoundTrip(t *testing.T) { + t.Parallel() + + original := map[string]any{ + "go.interfaceProximity": map[string]string{ + "Reader": "producer", + "Writer": "consumer", + }, + } + + data, err := json.Marshal(original) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + var roundTripped map[string]any + if err := json.Unmarshal(data, &roundTripped); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + proximities, err := InterfaceProximities(roundTripped) + if err != nil { + t.Fatalf("InterfaceProximities after round-trip: %v", err) + } + + if proximities["Reader"] != "producer" { + t.Errorf("Reader proximity: got %q, want %q", proximities["Reader"], "producer") + } + if proximities["Writer"] != "consumer" { + t.Errorf("Writer proximity: got %q, want %q", proximities["Writer"], "consumer") + } +} + +func TestInterfaceWidths_MissingKey(t *testing.T) { + t.Parallel() + + ext := map[string]any{ + "other.key": "value", + } + + _, err := InterfaceWidths(ext) + if err == nil { + t.Error("expected error for missing key, got nil") + } +} + +func TestInterfaceProximities_MissingKey(t *testing.T) { + t.Parallel() + + ext := map[string]any{ + "other.key": "value", + } + + _, err := InterfaceProximities(ext) + if err == nil { + t.Error("expected error for missing key, got nil") + } +} + +// TestAdapter_ExtensionCapabilities verifies the exported extension capability +// constants and the accessor that surfaces them. These are separate from the +// seven universal capabilities returned by Capabilities. +func TestAdapter_ExtensionCapabilities(t *testing.T) { + t.Parallel() + + if CapInterfaceWidth != "go.interfaceWidth" { + t.Errorf("CapInterfaceWidth: got %q, want %q", CapInterfaceWidth, "go.interfaceWidth") + } + if CapInterfaceProximity != "go.interfaceProximity" { + t.Errorf("CapInterfaceProximity: got %q, want %q", CapInterfaceProximity, "go.interfaceProximity") + } + + adapter := New() + caps := adapter.ExtensionCapabilities() + if len(caps) != 2 { + t.Fatalf("ExtensionCapabilities count: got %d, want 2", len(caps)) + } + + want := map[string]bool{CapInterfaceWidth: true, CapInterfaceProximity: true} + for _, c := range caps { + if !want[c] { + t.Errorf("unexpected extension capability: %q", c) + } + delete(want, c) + } + if len(want) != 0 { + t.Errorf("missing extension capabilities: %v", want) + } +} + +// TestInterfaceWidths_WrongContainerType verifies that a non-map value under +// the interfaceWidth key produces the "expected map" error. +func TestInterfaceWidths_WrongContainerType(t *testing.T) { + t.Parallel() + + ext := map[string]any{CapInterfaceWidth: "not-a-map"} + + _, err := InterfaceWidths(ext) + if err == nil { + t.Fatal("expected error for wrong container type, got nil") + } + if !strings.Contains(err.Error(), "expected map") { + t.Errorf("error %q does not contain %q", err.Error(), "expected map") + } +} + +// TestInterfaceWidths_WrongValueType verifies that a map value whose element is +// neither float64 nor int produces the "unexpected type" error. +func TestInterfaceWidths_WrongValueType(t *testing.T) { + t.Parallel() + + ext := map[string]any{ + CapInterfaceWidth: map[string]interface{}{"Reader": "not-a-number"}, + } + + _, err := InterfaceWidths(ext) + if err == nil { + t.Fatal("expected error for wrong value type, got nil") + } + if !strings.Contains(err.Error(), "unexpected type") { + t.Errorf("error %q does not contain %q", err.Error(), "unexpected type") + } +} + +// TestInterfaceProximities_WrongContainerType verifies that a non-map value +// under the interfaceProximity key produces the "expected map" error. +func TestInterfaceProximities_WrongContainerType(t *testing.T) { + t.Parallel() + + ext := map[string]any{CapInterfaceProximity: 42} + + _, err := InterfaceProximities(ext) + if err == nil { + t.Fatal("expected error for wrong container type, got nil") + } + if !strings.Contains(err.Error(), "expected map") { + t.Errorf("error %q does not contain %q", err.Error(), "expected map") + } +} + +// TestInterfaceProximities_WrongValueType verifies that a map value whose +// element is not a string produces the "unexpected type" error. +func TestInterfaceProximities_WrongValueType(t *testing.T) { + t.Parallel() + + ext := map[string]any{ + CapInterfaceProximity: map[string]interface{}{"Reader": 123}, + } + + _, err := InterfaceProximities(ext) + if err == nil { + t.Fatal("expected error for wrong value type, got nil") + } + if !strings.Contains(err.Error(), "unexpected type") { + t.Errorf("error %q does not contain %q", err.Error(), "unexpected type") + } +} diff --git a/internal/goadapter/lcom.go b/internal/goadapter/lcom.go new file mode 100644 index 0000000..3ec7b7a --- /dev/null +++ b/internal/goadapter/lcom.go @@ -0,0 +1,256 @@ +package goadapter + +import ( + "go/ast" + "go/types" + + "golang.org/x/tools/go/packages" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +// computeLCOM4 computes the LCOM4 metric for a package using the +// Hitz & Montazeri (1995) connected-component variant. +// +// Algorithm: +// 1. Identify exported methods (functions with a receiver of an exported type). +// 2. For each method, walk the AST body to find struct field accesses (s.field). +// 3. Build a graph where methods are nodes and edges connect methods that +// access at least one common struct field. +// 4. Count connected components using union-find. +// +// Returns 0 if no exported methods exist (trivially cohesive). +// Package-level functions (no receiver) are excluded. +func computeLCOM4(pkg *packages.Package) metrics.LCOM { + if pkg.TypesInfo == nil || len(pkg.Syntax) == 0 { + return 0 + } + + // Phase 1: Collect exported methods and their field accesses. + methods := collectExportedMethods(pkg) + if len(methods) == 0 { + return 0 + } + + // Phase 2: Build union-find and merge methods sharing fields. + uf := newUnionFind(len(methods)) + + // For each pair of methods, check if they share any field. + // O(n^2 * f) where n = methods, f = avg fields per method. + // Acceptable for typical package sizes. + for i := 0; i < len(methods); i++ { + for j := i + 1; j < len(methods); j++ { + if sharesField(methods[i].fields, methods[j].fields) { + uf.union(i, j) + } + } + } + + // Phase 3: Count connected components. + roots := make(map[int]bool) + for i := range methods { + roots[uf.find(i)] = true + } + + return metrics.LCOM(len(roots)) +} + +// methodInfo captures an exported method's qualified name and the set of struct +// field keys it accesses. Each methodInfo is a node in the LCOM4 graph; nodes +// are connected when their field sets overlap. +type methodInfo struct { + name string + fields map[string]bool +} + +// collectExportedMethods walks the package AST and returns one methodInfo per +// exported method declared on an exported receiver type. It resolves the +// receiver's base type name (handling value, pointer, and generic receiver +// forms via resolveReceiverType) and records the struct fields each method +// body accesses. Package-level functions (no receiver) and methods on +// unexported types are excluded. +func collectExportedMethods(pkg *packages.Package) []methodInfo { + var methods []methodInfo + + for _, file := range pkg.Syntax { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv == nil || fn.Body == nil { + continue + } + + // Must have a receiver (i.e., be a method, not a function). + if len(fn.Recv.List) == 0 { + continue + } + + // Method must be exported. + if !fn.Name.IsExported() { + continue + } + + // Receiver type must be exported. + recvType := resolveReceiverType(fn.Recv.List[0].Type) + if recvType == "" || !ast.IsExported(recvType) { + continue + } + + // Walk the method body to find field accesses. + fields := collectFieldAccesses(fn.Body, pkg.TypesInfo) + methods = append(methods, methodInfo{ + name: recvType + "." + fn.Name.Name, + fields: fields, + }) + } + } + + return methods +} + +// resolveReceiverType extracts the base type name from a method receiver +// expression. It supports every Go receiver form: +// +// func (t T) Ident → "T" +// func (t *T) StarExpr{Ident} → "T" +// func (t T[P]) IndexExpr{X:Ident} → "T" +// func (t T[P1,P2]) IndexListExpr{X:Ident} → "T" +// func (t *T[P]) StarExpr{IndexExpr{X:Ident}} → "T" +// func (t *T[P1,P2]) StarExpr{IndexListExpr{X:Ident}} → "T" +// +// A single pointer indirection is unwrapped first, then the underlying value +// form is resolved. It returns "" only for genuinely unknown forms. +func resolveReceiverType(expr ast.Expr) string { + // Unwrap a single pointer indirection: *T, *T[P], *T[P1, P2]. + if star, ok := expr.(*ast.StarExpr); ok { + expr = star.X + } + return baseTypeName(expr) +} + +// baseTypeName resolves a value-form receiver type expression to its base +// identifier name. It handles plain identifiers (T) and generic instantiations +// (T[P] and T[P1, P2]) by extracting the underlying generic type identifier. +// It returns "" for any other expression form. +func baseTypeName(expr ast.Expr) string { + switch t := expr.(type) { + case *ast.Ident: + // Value receiver: T. + return t.Name + case *ast.IndexExpr: + // Single type parameter: T[P] — extract T. + if ident, ok := t.X.(*ast.Ident); ok { + return ident.Name + } + case *ast.IndexListExpr: + // Multiple type parameters: T[P1, P2] — extract T. + if ident, ok := t.X.(*ast.Ident); ok { + return ident.Name + } + } + return "" +} + +// collectFieldAccesses walks an AST node and returns the set of struct field +// keys accessed within it. A field key is "TypeName.FieldName" to distinguish +// fields on different types. +func collectFieldAccesses(node ast.Node, info *types.Info) map[string]bool { + fields := make(map[string]bool) + + ast.Inspect(node, func(n ast.Node) bool { + sel, ok := n.(*ast.SelectorExpr) + if !ok { + return true + } + + // Resolve the selection to a types.Object. + obj, ok := info.Selections[sel] + if !ok { + return true + } + + // Only consider direct field accesses (not method calls). + if obj.Kind() != types.FieldVal { + return true + } + + // Build a key from the receiver type and field name. + recv := obj.Recv() + if recv == nil { + return true + } + + // Dereference pointer types to get the underlying named type. + underlying := recv + if ptr, ok := underlying.(*types.Pointer); ok { + underlying = ptr.Elem() + } + + named, ok := underlying.(*types.Named) + if !ok { + return true + } + + typeName := named.Obj().Name() + fieldName := sel.Sel.Name + fields[typeName+"."+fieldName] = true + + return true + }) + + return fields +} + +// sharesField returns true if two field sets have at least one common element. +func sharesField(a, b map[string]bool) bool { + // Iterate over the smaller set for efficiency. + if len(a) > len(b) { + a, b = b, a + } + for field := range a { + if b[field] { + return true + } + } + return false +} + +// unionFind implements a disjoint-set data structure with path compression +// and union by rank for efficient connected component tracking. +type unionFind struct { + parent []int + rank []int +} + +// newUnionFind creates a union-find with n elements, each in its own set. +func newUnionFind(n int) *unionFind { + parent := make([]int, n) + rank := make([]int, n) + for i := range parent { + parent[i] = i + } + return &unionFind{parent: parent, rank: rank} +} + +// find returns the root representative of the set containing x, +// applying path compression. +func (uf *unionFind) find(x int) int { + if uf.parent[x] != x { + uf.parent[x] = uf.find(uf.parent[x]) + } + return uf.parent[x] +} + +// union merges the sets containing x and y using union by rank. +func (uf *unionFind) union(x, y int) { + rx, ry := uf.find(x), uf.find(y) + if rx == ry { + return + } + if uf.rank[rx] < uf.rank[ry] { + rx, ry = ry, rx + } + uf.parent[ry] = rx + if uf.rank[rx] == uf.rank[ry] { + uf.rank[rx]++ + } +} diff --git a/internal/goadapter/lcom_test.go b/internal/goadapter/lcom_test.go new file mode 100644 index 0000000..51aa639 --- /dev/null +++ b/internal/goadapter/lcom_test.go @@ -0,0 +1,127 @@ +package goadapter + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +func TestLCOM4_Cohesive(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "lcom", "cohesive") + got := computeLCOM4(pkg) + + if got != metrics.LCOM(1) { + t.Errorf("LCOM4: got %d, want %d", got, 1) + } +} + +func TestLCOM4_NonCohesive(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "lcom", "noncohesive") + got := computeLCOM4(pkg) + + if got != metrics.LCOM(2) { + t.Errorf("LCOM4: got %d, want %d", got, 2) + } +} + +func TestLCOM4_NoMethods(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "lcom", "nomethods") + got := computeLCOM4(pkg) + + if got != metrics.LCOM(0) { + t.Errorf("LCOM4: got %d, want %d", got, 0) + } +} + +// TestLCOM4_GenericPointerReceiver proves that methods on a generic pointer +// receiver (*Box[T]) are counted, not dropped. Both Get and Set share field +// val, so LCOM4 must be 1. Before the receiver-resolution fix the generic +// pointer receiver resolved to "", the methods were dropped, and LCOM4 was 0. +func TestLCOM4_GenericPointerReceiver(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "lcom", "generic") + got := computeLCOM4(pkg) + + if got != metrics.LCOM(1) { + t.Errorf("LCOM4: got %d, want %d (generic pointer-receiver methods must not be dropped)", got, 1) + } +} + +// parseReceiverType parses a single method declaration and returns its +// receiver type expression for direct testing of resolveReceiverType. +func parseReceiverType(t *testing.T, methodSrc string) ast.Expr { + t.Helper() + + src := "package p\n" + methodSrc + "\n" + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "recv.go", src, 0) + if err != nil { + t.Fatalf("parse %q: %v", methodSrc, err) + } + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv == nil || len(fn.Recv.List) == 0 { + continue + } + return fn.Recv.List[0].Type + } + t.Fatalf("no method receiver found in %q", methodSrc) + return nil +} + +// TestResolveReceiverType_AllForms verifies that every supported Go receiver +// form resolves to the base type name "T". +func TestResolveReceiverType_AllForms(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + src string + want string + }{ + {"value", "func (t T) M() {}", "T"}, + {"pointer", "func (t *T) M() {}", "T"}, + {"generic_value_single", "func (t T[P]) M() {}", "T"}, + {"generic_value_multi", "func (t T[P1, P2]) M() {}", "T"}, + {"generic_pointer_single", "func (t *T[P]) M() {}", "T"}, + {"generic_pointer_multi", "func (t *T[P1, P2]) M() {}", "T"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + recv := parseReceiverType(t, tt.src) + if got := resolveReceiverType(recv); got != tt.want { + t.Errorf("resolveReceiverType(%s): got %q, want %q", tt.src, got, tt.want) + } + }) + } +} + +// TestResolveReceiverType_UnknownForm verifies that unrecognized receiver +// expressions resolve to "" rather than guessing a name. +func TestResolveReceiverType_UnknownForm(t *testing.T) { + t.Parallel() + + // A selector expression (pkg.T) is not a valid receiver form. + unknown := &ast.SelectorExpr{X: ast.NewIdent("pkg"), Sel: ast.NewIdent("T")} + if got := resolveReceiverType(unknown); got != "" { + t.Errorf("resolveReceiverType(SelectorExpr): got %q, want empty string", got) + } + + // A pointer to an unknown form must also resolve to "". + ptrUnknown := &ast.StarExpr{X: unknown} + if got := resolveReceiverType(ptrUnknown); got != "" { + t.Errorf("resolveReceiverType(*SelectorExpr): got %q, want empty string", got) + } +} diff --git a/internal/goadapter/resolve.go b/internal/goadapter/resolve.go new file mode 100644 index 0000000..ee13002 --- /dev/null +++ b/internal/goadapter/resolve.go @@ -0,0 +1,193 @@ +package goadapter + +import ( + "context" + "fmt" + "strings" + + "golang.org/x/tools/go/packages" + + "github.com/zero-dot-force/vibe-check/metrics" +) + +// loadFlags defines the information requested from go/packages. +// NeedName: package name and path. +// NeedImports: import map for coupling analysis. +// NeedTypes: type information for abstractness and LCOM4. +// NeedSyntax: AST for LCOM4 field-access analysis. +// NeedTypesInfo: resolved type info for identifier resolution. +// NeedModule: module metadata for stdlib detection and module path. +const loadFlags = packages.NeedName | + packages.NeedImports | + packages.NeedTypes | + packages.NeedSyntax | + packages.NeedTypesInfo | + packages.NeedModule + +// packageEnvAllowlist enumerates the environment variables passed through to +// go/packages (and any subprocess it spawns) via metrics.SanitizeEnvironment. +// Only variables required to resolve modules and locate the build cache are +// included. Credential-bearing and command-injection vectors — notably +// GOFLAGS — are deliberately excluded. This is an immutable package-level +// slice, not mutable global state. +var packageEnvAllowlist = []string{ + "GOPATH", + "GOROOT", + "GOMODCACHE", + "GOPROXY", + "GONOSUMCHECK", + "GOMOD", +} + +// resolvePackages loads Go packages from projectPath using go/packages and +// builds the module-internal import adjacency map used to compute Ca/Ce. It +// filters results to module-internal packages and records warnings for any +// package that fails to load or type-check. +// +// Returns: +// - pkgs: loaded module-internal packages (may include packages with errors) +// - imports: adjacency map of module-internal import edges (pkg → its module-internal imports) +// - warnings: any non-fatal issues encountered during loading +// - err: fatal error if package loading fails entirely +func resolvePackages(ctx context.Context, projectPath string) ( + pkgs []*packages.Package, + imports map[string][]string, + warnings []metrics.Warning, + err error, +) { + cfg := &packages.Config{ + Mode: loadFlags, + Dir: projectPath, + Context: ctx, + Env: metrics.SanitizeEnvironment(packageEnvAllowlist), + Tests: false, + } + + allPkgs, err := packages.Load(cfg, "./...") + if err != nil { + return nil, nil, nil, fmt.Errorf("resolve packages: %w — verify the path is a valid Go module directory containing go.mod and .go files", err) + } + + if len(allPkgs) == 0 { + return nil, nil, nil, nil + } + + // Determine module path from the first package with module info. + modulePath := detectModulePath(allPkgs) + if modulePath == "" { + return nil, nil, nil, fmt.Errorf("resolve packages: unable to determine module path — ensure the target directory contains a go.mod (run 'go mod init' if missing)") + } + + // Filter to module-internal packages and collect warnings for errored packages. + var internal []*packages.Package + internalSet := make(map[string]bool) + + for _, pkg := range allPkgs { + if !strings.HasPrefix(pkg.PkgPath, modulePath) { + continue + } + internalSet[pkg.PkgPath] = true + internal = append(internal, pkg) + + relPath := relativeModulePath(pkg.PkgPath, modulePath) + + // Collect load/type errors as warnings rather than failing. + for _, pkgErr := range pkg.Errors { + warnings = append(warnings, metrics.Warning{ + Code: "load-error", + Message: fmt.Sprintf("package %s: %s", relPath, pkgErr.Msg), + Module: pkg.PkgPath, + }) + } + + // A package with no reported errors but nil type information could not + // be type-checked. Record a warning so its zeroed type metrics (emitted + // by the adapter per the go-adapter partial-build scenario) are + // explained to consumers. + if len(pkg.Errors) == 0 && pkg.Types == nil { + warnings = append(warnings, metrics.Warning{ + Code: "load-error", + Message: fmt.Sprintf("package %s: type information unavailable (type-checking did not complete)", relPath), + Module: pkg.PkgPath, + }) + } + } + + // Build import adjacency map: for each internal package, record which + // other internal packages it imports (module-internal edges only). + imports = make(map[string][]string, len(internal)) + for _, pkg := range internal { + var internalImports []string + for impPath := range pkg.Imports { + if internalSet[impPath] { + internalImports = append(internalImports, impPath) + } + } + imports[pkg.PkgPath] = internalImports + } + + return internal, imports, warnings, nil +} + +// anyPackageTypeChecked reports whether at least one package in pkgs was +// successfully type-checked — that is, it has no load/type errors and non-nil +// type information. It distinguishes a partial build (at least one usable +// package, so analysis proceeds with warnings for the rest) from a total load +// failure (no usable package, so Analyze must return an error rather than a +// graph of all-zeroed modules). The predicate mirrors the per-package guard in +// Adapter.Analyze that gates type classification and LCOM computation. +func anyPackageTypeChecked(pkgs []*packages.Package) bool { + for _, pkg := range pkgs { + if len(pkg.Errors) == 0 && pkg.Types != nil { + return true + } + } + return false +} + +// countCe counts the efferent coupling for a package: the number of distinct +// imports including standard library, third-party, and module-internal packages. +// Per Robert C. Martin's definition, Ce counts all outgoing dependencies +// regardless of their origin. Stdlib and third-party packages are excluded +// from the module list but still contribute to Ce counts. +func countCe(pkg *packages.Package) int { + return len(pkg.Imports) +} + +// countCa counts the afferent coupling for a package: the number of +// module-internal packages that import it. +func countCa(pkgPath string, imports map[string][]string) int { + count := 0 + for _, deps := range imports { + for _, dep := range deps { + if dep == pkgPath { + count++ + break + } + } + } + return count +} + +// detectModulePath extracts the module path from loaded packages. +// It uses the Module field from the first package that has one. +func detectModulePath(pkgs []*packages.Package) string { + for _, pkg := range pkgs { + if pkg.Module != nil { + return pkg.Module.Path + } + } + return "" +} + +// relativeModulePath returns the package path relative to the module root. +// For example, "example.com/foo/bar/baz" with module "example.com/foo" returns "bar/baz". +// If the package is the module root, returns ".". +func relativeModulePath(pkgPath, modulePath string) string { + rel := strings.TrimPrefix(pkgPath, modulePath) + rel = strings.TrimPrefix(rel, "/") + if rel == "" { + return "." + } + return rel +} diff --git a/internal/goadapter/resolve_test.go b/internal/goadapter/resolve_test.go new file mode 100644 index 0000000..93a4163 --- /dev/null +++ b/internal/goadapter/resolve_test.go @@ -0,0 +1,36 @@ +package goadapter + +import "testing" + +// TestPackageEnvAllowlist_ExcludesInjectionVectors locks the go/packages +// environment allowlist against command-injection. GOFLAGS must never be +// present (it can inject arbitrary build flags into the subprocess that +// go/packages spawns), while the variables required for module resolution and +// build-cache location must be. Changing this set requires consciously updating +// this test. +func TestPackageEnvAllowlist_ExcludesInjectionVectors(t *testing.T) { + t.Parallel() + + present := make(map[string]bool, len(packageEnvAllowlist)) + for _, v := range packageEnvAllowlist { + present[v] = true + } + + // Must NOT contain the command-injection vector. + if present["GOFLAGS"] { + t.Error(`packageEnvAllowlist must not contain "GOFLAGS" (command-injection vector)`) + } + + // Must contain exactly the expected module-resolution variables. + expected := []string{"GOPATH", "GOROOT", "GOMODCACHE", "GOPROXY", "GONOSUMCHECK", "GOMOD"} + for _, want := range expected { + if !present[want] { + t.Errorf("packageEnvAllowlist missing required entry %q", want) + } + } + + if len(packageEnvAllowlist) != len(expected) { + t.Errorf("packageEnvAllowlist size: got %d, want %d (unexpected entries lock out review)", + len(packageEnvAllowlist), len(expected)) + } +} diff --git a/internal/goadapter/testdata/allbad/broken1/broken1.go b/internal/goadapter/testdata/allbad/broken1/broken1.go new file mode 100644 index 0000000..382ca06 --- /dev/null +++ b/internal/goadapter/testdata/allbad/broken1/broken1.go @@ -0,0 +1,13 @@ +// Package broken1 deliberately imports a non-existent package so that it +// fails to type-check. Alongside broken2 it ensures every package in the +// allbad fixture module is un-analyzable, exercising the go-adapter +// total-load-failure scenario in which no package can be type-checked and +// Analyze must return an error rather than a graph of all-zeroed modules. +package broken1 + +import "example.com/doesnotexist" //nolint:all // deliberate missing import + +// Broken references the non-existent package. +func Broken() string { + return doesnotexist.Value() //nolint:all // deliberate reference to missing package +} diff --git a/internal/goadapter/testdata/allbad/broken2/broken2.go b/internal/goadapter/testdata/allbad/broken2/broken2.go new file mode 100644 index 0000000..746e4b4 --- /dev/null +++ b/internal/goadapter/testdata/allbad/broken2/broken2.go @@ -0,0 +1,12 @@ +// Package broken2 deliberately imports a non-existent package so that it +// fails to type-check. Together with broken1 it guarantees the allbad +// fixture module contains no analyzable package, so Analyze exercises the +// total-load-failure path. +package broken2 + +import "example.com/alsomissing" //nolint:all // deliberate missing import + +// AlsoBroken references the non-existent package. +func AlsoBroken() string { + return alsomissing.Value() //nolint:all // deliberate reference to missing package +} diff --git a/internal/goadapter/testdata/allbad/go.mod b/internal/goadapter/testdata/allbad/go.mod new file mode 100644 index 0000000..ff38c53 --- /dev/null +++ b/internal/goadapter/testdata/allbad/go.mod @@ -0,0 +1,3 @@ +module example.com/allbad + +go 1.25 diff --git a/internal/goadapter/testdata/coupling/go.mod b/internal/goadapter/testdata/coupling/go.mod new file mode 100644 index 0000000..3a1448a --- /dev/null +++ b/internal/goadapter/testdata/coupling/go.mod @@ -0,0 +1,3 @@ +module example.com/coupling + +go 1.25 diff --git a/internal/goadapter/testdata/coupling/pkga/pkga.go b/internal/goadapter/testdata/coupling/pkga/pkga.go new file mode 100644 index 0000000..7038bd7 --- /dev/null +++ b/internal/goadapter/testdata/coupling/pkga/pkga.go @@ -0,0 +1,20 @@ +// Package pkga demonstrates efferent coupling. +// +// pkga imports pkgb (module-internal) and fmt (stdlib). +// +// Expected metrics: +// +// Ca = 0 (no module-internal package imports pkga) +// Ce = 2 (pkga imports fmt + pkgb) +package pkga + +import ( + "fmt" + + "example.com/coupling/pkgb" +) + +// Greet formats a greeting using pkgb's Name function. +func Greet() string { + return fmt.Sprintf("Hello, %s!", pkgb.Name()) +} diff --git a/internal/goadapter/testdata/coupling/pkgb/pkgb.go b/internal/goadapter/testdata/coupling/pkgb/pkgb.go new file mode 100644 index 0000000..2f1652d --- /dev/null +++ b/internal/goadapter/testdata/coupling/pkgb/pkgb.go @@ -0,0 +1,19 @@ +// Package pkgb is a shared dependency with high afferent coupling. +// +// pkgb is imported by both pkga and pkgc but imports no module-internal packages. +// +// Expected metrics: +// +// Ca = 2 (pkga and pkgc both import pkgb) +// Ce = 0 (pkgb imports no module-internal packages) +package pkgb + +// Name returns a fixed name string. +func Name() string { + return "world" +} + +// Value returns a fixed integer value. +func Value() int { + return 42 +} diff --git a/internal/goadapter/testdata/coupling/pkgc/pkgc.go b/internal/goadapter/testdata/coupling/pkgc/pkgc.go new file mode 100644 index 0000000..3bdcf1c --- /dev/null +++ b/internal/goadapter/testdata/coupling/pkgc/pkgc.go @@ -0,0 +1,16 @@ +// Package pkgc demonstrates efferent coupling to pkgb. +// +// pkgc imports pkgb (module-internal) and no stdlib packages. +// +// Expected metrics: +// +// Ca = 0 (no module-internal package imports pkgc) +// Ce = 1 (pkgc imports pkgb) +package pkgc + +import "example.com/coupling/pkgb" + +// DoubleValue returns twice pkgb's Value. +func DoubleValue() int { + return pkgb.Value() * 2 +} diff --git a/internal/goadapter/testdata/empty/.gitkeep b/internal/goadapter/testdata/empty/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/internal/goadapter/testdata/extensions/go.mod b/internal/goadapter/testdata/extensions/go.mod new file mode 100644 index 0000000..48ad71e --- /dev/null +++ b/internal/goadapter/testdata/extensions/go.mod @@ -0,0 +1,3 @@ +module example.com/extensions + +go 1.25 diff --git a/internal/goadapter/testdata/extensions/ifaces/ifaces.go b/internal/goadapter/testdata/extensions/ifaces/ifaces.go new file mode 100644 index 0000000..34ce731 --- /dev/null +++ b/internal/goadapter/testdata/extensions/ifaces/ifaces.go @@ -0,0 +1,69 @@ +// Package ifaces provides interface declarations for testing the go.interfaceWidth +// and go.interfaceProximity extension metrics. +// +// Expected extensions: +// +// go.interfaceWidth: +// Reader: 1 (one method: Read) +// Processor: 3 (three methods: Process, Validate, Reset) +// Embedder: 2 (embeds Reader + adds Close; flattened = Read + Close) +// +// go.interfaceProximity: +// Reader: "producer" (implemented by FileReader in this package) +// Processor: "consumer" (no implementation in this package) +// Embedder: "producer" (implemented by FileReader in this package) +package ifaces + +import "io" + +// Reader is a single-method interface for reading data. +// Width = 1. +type Reader interface { + // Read reads data into p. + Read(p []byte) (int, error) +} + +// Processor is a multi-method interface for data processing. +// Width = 3. No implementation exists in this package (consumer proximity). +type Processor interface { + // Process processes the input data. + Process(data []byte) ([]byte, error) + // Validate checks whether the data is valid. + Validate(data []byte) error + // Reset resets the processor to its initial state. + Reset() +} + +// Embedder embeds Reader and adds one method. +// Flattened width = 2 (Read from Reader + Close). +type Embedder interface { + Reader + // Close releases resources. + Close() error +} + +// FileReader implements both Reader and Embedder, making them "producer" +// interfaces (implemented in the same package where they are declared). +type FileReader struct { + path string //nolint:unused // used by methods + reader io.Reader //nolint:unused // used by methods +} + +// Read implements Reader.Read and Embedder (via Reader embedding). +func (f *FileReader) Read(p []byte) (int, error) { + if f.reader == nil { + return 0, io.EOF + } + return f.reader.Read(p) +} + +// Close implements Embedder.Close. +func (f *FileReader) Close() error { + f.reader = nil + return nil +} + +// NewFileReader constructs a FileReader with the given path. +func NewFileReader(path string, r io.Reader) *FileReader { + return &FileReader{path: path, reader: r} +} diff --git a/internal/goadapter/testdata/lcom/cohesive/cohesive.go b/internal/goadapter/testdata/lcom/cohesive/cohesive.go new file mode 100644 index 0000000..1039f83 --- /dev/null +++ b/internal/goadapter/testdata/lcom/cohesive/cohesive.go @@ -0,0 +1,33 @@ +// Package cohesive demonstrates a fully cohesive struct where all methods +// access the same field, forming a single connected component. +// +// Expected LCOM4 = 1 (one connected component). +// +// Method-field graph: +// +// Get → x +// Set → x +// Inc → x +// +// All three methods share field x, so they form one connected component. +package cohesive + +// S is a struct with a single field accessed by all methods. +type S struct { + x int //nolint:unused // accessed via methods +} + +// Get returns the current value of x. +func (s *S) Get() int { + return s.x +} + +// Set assigns a new value to x. +func (s *S) Set(v int) { + s.x = v +} + +// Inc increments x by one. +func (s *S) Inc() { + s.x++ +} diff --git a/internal/goadapter/testdata/lcom/generic/generic.go b/internal/goadapter/testdata/lcom/generic/generic.go new file mode 100644 index 0000000..78f2d0d --- /dev/null +++ b/internal/goadapter/testdata/lcom/generic/generic.go @@ -0,0 +1,25 @@ +// Package generic demonstrates a generic type whose methods use a pointer +// receiver and share a field. It is a regression fixture for the receiver +// resolution bug: before the fix, a generic pointer receiver *Box[T] +// (StarExpr wrapping an IndexExpr) resolved to "" and its methods were +// silently dropped from the LCOM4 graph, corrupting the metric. +// +// Expected LCOM4 = 1: both Get and Set access field val, forming a single +// connected component. If the generic pointer receiver were dropped, LCOM4 +// would incorrectly be 0. +package generic + +// Box is a generic container with methods declared on a pointer receiver. +type Box[T any] struct { + val T //nolint:unused // accessed via Get and Set +} + +// Get returns the stored value. +func (b *Box[T]) Get() T { + return b.val +} + +// Set stores a new value. +func (b *Box[T]) Set(v T) { + b.val = v +} diff --git a/internal/goadapter/testdata/lcom/go.mod b/internal/goadapter/testdata/lcom/go.mod new file mode 100644 index 0000000..8a6c3d9 --- /dev/null +++ b/internal/goadapter/testdata/lcom/go.mod @@ -0,0 +1,3 @@ +module example.com/lcom + +go 1.25 diff --git a/internal/goadapter/testdata/lcom/nomethods/nomethods.go b/internal/goadapter/testdata/lcom/nomethods/nomethods.go new file mode 100644 index 0000000..d2e1b61 --- /dev/null +++ b/internal/goadapter/testdata/lcom/nomethods/nomethods.go @@ -0,0 +1,22 @@ +// Package nomethods contains only package-level functions with no receivers. +// +// Expected LCOM4 = 0 (no methods, trivially cohesive). +// +// LCOM4 only considers exported methods on exported types. Package-level +// functions are excluded from the method-field graph entirely. +package nomethods + +// Add returns the sum of two integers. +func Add(a, b int) int { + return a + b +} + +// Multiply returns the product of two integers. +func Multiply(a, b int) int { + return a * b +} + +// Negate returns the negation of an integer. +func Negate(a int) int { + return -a +} diff --git a/internal/goadapter/testdata/lcom/noncohesive/noncohesive.go b/internal/goadapter/testdata/lcom/noncohesive/noncohesive.go new file mode 100644 index 0000000..85bb9d0 --- /dev/null +++ b/internal/goadapter/testdata/lcom/noncohesive/noncohesive.go @@ -0,0 +1,45 @@ +// Package noncohesive demonstrates a struct with two disconnected groups +// of methods, each accessing a disjoint set of fields. +// +// Expected LCOM4 = 2 (two connected components). +// +// Method-field graph: +// +// MethodA → x, y +// MethodB → x, y +// MethodC → z, w +// MethodD → z, w +// +// Component 1: {MethodA, MethodB} share fields {x, y} +// Component 2: {MethodC, MethodD} share fields {z, w} +package noncohesive + +// S is a struct with four fields split across two method groups. +type S struct { + x int //nolint:unused // accessed via MethodA, MethodB + y int //nolint:unused // accessed via MethodA, MethodB + z int //nolint:unused // accessed via MethodC, MethodD + w int //nolint:unused // accessed via MethodC, MethodD +} + +// MethodA accesses fields x and y. +func (s *S) MethodA() int { + return s.x + s.y +} + +// MethodB accesses fields x and y. +func (s *S) MethodB(v int) { + s.x = v + s.y = v +} + +// MethodC accesses fields z and w. +func (s *S) MethodC() int { + return s.z + s.w +} + +// MethodD accesses fields z and w. +func (s *S) MethodD(v int) { + s.z = v + s.w = v +} diff --git a/internal/goadapter/testdata/partial/bad/bad.go b/internal/goadapter/testdata/partial/bad/bad.go new file mode 100644 index 0000000..982e07e --- /dev/null +++ b/internal/goadapter/testdata/partial/bad/bad.go @@ -0,0 +1,11 @@ +// Package bad deliberately imports a non-existent package to test +// partial-build error handling. The adapter should report warnings +// for this package while still analyzing valid sibling packages. +package bad + +import "example.com/doesnotexist" //nolint:all // deliberate missing import + +// Broken attempts to use the non-existent package. +func Broken() string { + return doesnotexist.Value() //nolint:all // deliberate reference to missing package +} diff --git a/internal/goadapter/testdata/partial/go.mod b/internal/goadapter/testdata/partial/go.mod new file mode 100644 index 0000000..21ff262 --- /dev/null +++ b/internal/goadapter/testdata/partial/go.mod @@ -0,0 +1,3 @@ +module example.com/partial + +go 1.25 diff --git a/internal/goadapter/testdata/partial/good/good.go b/internal/goadapter/testdata/partial/good/good.go new file mode 100644 index 0000000..f3374d1 --- /dev/null +++ b/internal/goadapter/testdata/partial/good/good.go @@ -0,0 +1,9 @@ +// Package good is a valid package for testing partial-build scenarios. +// When analyzed alongside a broken sibling package, this package should +// still produce valid metrics. +package good + +// Hello returns a greeting string. +func Hello() string { + return "hello" +} diff --git a/internal/goadapter/testdata/types/empty/empty.go b/internal/goadapter/testdata/types/empty/empty.go new file mode 100644 index 0000000..237e058 --- /dev/null +++ b/internal/goadapter/testdata/types/empty/empty.go @@ -0,0 +1,11 @@ +// Package empty contains no type declarations. +// +// Expected metrics: +// +// ExportedTypes = 0 +// AbstractTypes = 0 +package empty + +// Version is a package-level constant. Constants are not type declarations +// and do not contribute to ExportedTypes or AbstractTypes counts. +const Version = "1.0.0" diff --git a/internal/goadapter/testdata/types/go.mod b/internal/goadapter/testdata/types/go.mod new file mode 100644 index 0000000..9270a54 --- /dev/null +++ b/internal/goadapter/testdata/types/go.mod @@ -0,0 +1,3 @@ +module example.com/types + +go 1.25 diff --git a/internal/goadapter/testdata/types/mixed/mixed.go b/internal/goadapter/testdata/types/mixed/mixed.go new file mode 100644 index 0000000..6cb589f --- /dev/null +++ b/internal/goadapter/testdata/types/mixed/mixed.go @@ -0,0 +1,67 @@ +// Package mixed provides a variety of type declarations for testing +// abstractness and type-counting metrics. +// +// Expected metrics: +// +// ExportedTypes = 7 (Reader, Writer, Point, Config, Pair, Alias, IReader) +// AbstractTypes = 2 (Reader, Writer — interfaces) +// +// IReader is an alias to an interface but is classified as CONCRETE per the +// go-adapter spec: aliases never contribute to AbstractTypes. +// +// Unexported types (internal) are excluded from both counts. +package mixed + +// Reader is an abstract type for reading bytes. +type Reader interface { + // Read reads up to len(p) bytes into p. + Read(p []byte) (n int, err error) +} + +// Writer is an abstract type for writing bytes. +type Writer interface { + // Write writes len(p) bytes from p. + Write(p []byte) (n int, err error) +} + +// Point is a concrete type representing a 2D coordinate. +type Point struct { + // X is the horizontal coordinate. + X float64 + // Y is the vertical coordinate. + Y float64 +} + +// Config is a concrete type holding configuration values. +type Config struct { + // Name is the configuration name. + Name string + // Debug enables debug mode. + Debug bool +} + +// Pair is a concrete type holding two related values. +type Pair struct { + // First is the first element. + First string + // Second is the second element. + Second string +} + +// Alias is a concrete type alias for string. +type Alias = string + +// IReader is a type alias to the exported Reader interface. Although it aliases +// an interface, it MUST be classified as concrete: aliases introduce no new +// abstract type and never contribute to AbstractTypes. +type IReader = Reader + +// internal is an unexported struct excluded from exported type counts. +type internal struct { //nolint:unused // exists to test unexported exclusion + value int +} + +// Origin returns a Point at the origin. +func Origin() Point { + return Point{X: 0, Y: 0} +} diff --git a/internal/goadapter/types.go b/internal/goadapter/types.go new file mode 100644 index 0000000..67ef3c3 --- /dev/null +++ b/internal/goadapter/types.go @@ -0,0 +1,57 @@ +package goadapter + +import ( + "go/types" + + "golang.org/x/tools/go/packages" +) + +// countTypes counts exported type declarations in a package, classifying +// each as abstract (interface) or concrete (struct, named type, alias). +// Unexported types are excluded from both counts. +// +// Returns (0, 0) if pkg.Types is nil (type-checking failed for this package). +func countTypes(pkg *packages.Package) (exportedTypes, abstractTypes int) { + if pkg.Types == nil { + return 0, 0 + } + + scope := pkg.Types.Scope() + for _, name := range scope.Names() { + obj := scope.Lookup(name) + + // Only count type names (not vars, funcs, consts). + tn, ok := obj.(*types.TypeName) + if !ok { + continue + } + + // Only count exported types. + if !tn.Exported() { + continue + } + + exportedTypes++ + + // Type aliases are always classified as concrete per the go-adapter + // spec, even when they alias an interface (e.g., + // `type IReader = SomeInterface`). An alias introduces no new abstract + // type — it is merely a concrete name for an existing type — so it + // counts toward exportedTypes (incremented above) but never + // abstractTypes. + if tn.IsAlias() { + continue + } + + // Named types: check if the underlying type is an interface. + named, ok := tn.Type().(*types.Named) + if !ok { + continue + } + if types.IsInterface(named.Underlying()) { + abstractTypes++ + } + } + + return exportedTypes, abstractTypes +} diff --git a/internal/goadapter/types_test.go b/internal/goadapter/types_test.go new file mode 100644 index 0000000..a8e22fb --- /dev/null +++ b/internal/goadapter/types_test.go @@ -0,0 +1,74 @@ +package goadapter + +import ( + "path/filepath" + "runtime" + "testing" + + "golang.org/x/tools/go/packages" +) + +// loadTestPackage loads a single package from the testdata directory for +// unit-level testing of internal functions. It uses the same load flags +// as the production code. +func loadTestPackage(t *testing.T, fixture, pkg string) *packages.Package { + t.Helper() + + dir := filepath.Join(testdataDir(t), fixture) + cfg := &packages.Config{ + Mode: loadFlags, + Dir: dir, + } + + pattern := "./" + pkg + pkgs, err := packages.Load(cfg, pattern) + if err != nil { + t.Fatalf("load test package %s/%s: %v", fixture, pkg, err) + } + if len(pkgs) == 0 { + t.Fatalf("load test package %s/%s: no packages loaded", fixture, pkg) + } + + return pkgs[0] +} + +// testdataDir returns the absolute path to the testdata directory. +func testdataDir(t *testing.T) string { + t.Helper() + _, filename, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("unable to determine test file location") + } + return filepath.Join(filepath.Dir(filename), "testdata") +} + +func TestCountTypes_Mixed(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "types", "mixed") + exported, abstract := countTypes(pkg) + + // The mixed fixture declares 7 exported types (Reader, Writer, Point, + // Config, Pair, Alias, IReader). Only Reader and Writer are abstract. + // IReader aliases an interface but MUST be counted concrete per spec. + if exported != 7 { + t.Errorf("ExportedTypes: got %d, want %d", exported, 7) + } + if abstract != 2 { + t.Errorf("AbstractTypes: got %d, want %d (alias-to-interface must NOT be abstract)", abstract, 2) + } +} + +func TestCountTypes_Empty(t *testing.T) { + t.Parallel() + + pkg := loadTestPackage(t, "types", "empty") + exported, abstract := countTypes(pkg) + + if exported != 0 { + t.Errorf("ExportedTypes: got %d, want %d", exported, 0) + } + if abstract != 0 { + t.Errorf("AbstractTypes: got %d, want %d", abstract, 0) + } +} diff --git a/metrics/cycle.go b/metrics/cycle.go index d39b660..93483ad 100644 --- a/metrics/cycle.go +++ b/metrics/cycle.go @@ -1,13 +1,14 @@ package metrics -// Cycle represents a circular dependency between modules as an ordered list -// of module paths. The cycle starts from the lexicographically smallest module -// path and does not repeat the start node. +// Cycle represents a circular dependency between modules as the set of module +// paths that participate in the cycle. The members are reported as a +// deterministic, lexicographically-sorted list of package paths. The ordering +// carries no traversal meaning — it is the sorted membership set — and each +// member appears exactly once. // -// Example: if modules A→B→C→A form a cycle, it is represented as ["A", "B", "C"]. +// Example: modules that form a cycle among A, B, and C are represented as +// ["A", "B", "C"] regardless of the direction in which the cycle was traversed. // -// Canonical ordering: when a cycle is detected (e.g., C→A→B→C), it is rotated -// so that the lexicographically smallest module path appears first, yielding -// ["A", "B", "C"]. Multiple cycles in a result set are sorted lexicographically -// by their first element. +// Multiple cycles in a result set are themselves sorted lexicographically by +// their first (smallest) element. type Cycle []string diff --git a/metrics/cycle_test.go b/metrics/cycle_test.go index b99db68..bbc23e8 100644 --- a/metrics/cycle_test.go +++ b/metrics/cycle_test.go @@ -26,14 +26,15 @@ func TestCycle_Construction(t *testing.T) { func TestCycle_CanonicalOrdering(t *testing.T) { t.Parallel() - // Per the spec, a cycle C→A→B→C is represented starting from the - // lexicographically smallest path: ["A", "B", "C"]. + // Per the Cycle contract, a cycle's members are reported as a + // deterministic, lexicographically-sorted set of package paths: + // ["A", "B", "C"]. The ordering carries no traversal meaning. // This test verifies that a correctly constructed Cycle follows - // the canonical ordering convention (smallest path first, no - // repeated start node). + // the sorted-set convention (smallest element first, no repeated + // start node). // - // Note: Canonical ordering enforcement (rotation of detected cycles) - // is the responsibility of the cycle detection algorithm in language + // Note: producing the sorted set from detected cycles is the + // responsibility of the cycle detection algorithm in language // adapters, not the Cycle type itself. This test validates the // convention on a pre-constructed cycle. cycle := Cycle{"A", "B", "C"} diff --git a/metrics/external.go b/metrics/external.go index 15690ef..a5e6f3b 100644 --- a/metrics/external.go +++ b/metrics/external.go @@ -219,9 +219,14 @@ func (a *ExternalAdapter) Analyze(ctx context.Context, projectPath string) (*Mod return nil, fmt.Errorf("external analyze: response validation: %w", err) } - // Step 4: Unmarshal into ModuleGraph. + // Step 4: Unmarshal into ModuleGraph using a strict decoder. Rejecting + // unknown fields enforces the schema's additionalProperties:false at the + // untrusted subprocess boundary: an analyzer that emits unexpected fields is + // rejected rather than having them silently ignored. var graph ModuleGraph - if err := json.Unmarshal(analyzeResp.Result, &graph); err != nil { + dec := json.NewDecoder(bytes.NewReader(analyzeResp.Result)) + dec.DisallowUnknownFields() + if err := dec.Decode(&graph); err != nil { return nil, fmt.Errorf("external analyze: unmarshal response: %w", err) } diff --git a/metrics/external_test.go b/metrics/external_test.go index d2dd4d0..2ce6ea8 100644 --- a/metrics/external_test.go +++ b/metrics/external_test.go @@ -33,6 +33,8 @@ func TestHelperProcess(_ *testing.T) { helperOversized() case "env_check": helperEnvCheck() + case "unknown_field": + helperUnknownField() default: fmt.Fprintf(os.Stderr, "unknown HELPER_MODE: %s\n", mode) os.Exit(2) @@ -220,6 +222,45 @@ func helperEnvCheck() { } } +// helperUnknownField responds to analyze with a schema-valid ModuleGraph that +// carries an unexpected extra field. The response passes Validate() (which does +// not enforce additionalProperties) but MUST be rejected by the strict decoder +// (DisallowUnknownFields) at the trust boundary. +func helperUnknownField() { + scanner := bufio.NewScanner(os.Stdin) + for scanner.Scan() { + line := scanner.Bytes() + var req JSONRPCRequest + if err := json.Unmarshal(line, &req); err != nil { + os.Exit(1) + } + + switch req.Method { + case "capabilities": + sendResponse(req.ID, CapabilitiesResult{ + Language: "test", + ProtocolVersion: "1.0", + Metrics: []string{"ca"}, + }) + + case "analyze": + // Required ModuleGraph fields plus an unexpected top-level key. + sendResponse(req.ID, map[string]any{ + "schemaVersion": "1.1", + "language": "test", + "modules": []any{}, + "cycles": []any{}, + "warnings": []any{}, + "status": "complete", + "unexpectedField": true, + }) + + case "shutdown": + os.Exit(0) + } + } +} + // sendResponse marshals a result and writes a JSON-RPC response to stdout. func sendResponse(id *int, result any) { resultData, err := json.Marshal(result) @@ -293,6 +334,33 @@ func TestExternalAdapter_SuccessfulAnalysis(t *testing.T) { } } +func TestExternalAdapter_RejectsUnknownFields(t *testing.T) { + t.Parallel() + + projectDir := t.TempDir() + + adapter := NewExternalAdapter(os.Args[0], "test", + WithAnalyzeTimeout(10*time.Second), + WithCapabilitiesTimeout(5*time.Second), + WithShutdownTimeout(2*time.Second), + ) + adapter.binaryPath = os.Args[0] + adapter.env = []string{ + "GO_WANT_HELPER_PROCESS=1", + "HELPER_MODE=unknown_field", + "PATH=" + os.Getenv("PATH"), + "HOME=" + os.Getenv("HOME"), + } + + _, err := adapter.Analyze(context.Background(), projectDir) + if err == nil { + t.Fatal("Analyze() accepted a response with unknown fields, want error") + } + if !strings.Contains(err.Error(), "unknown field") { + t.Errorf("Analyze() error = %q, want error mentioning 'unknown field'", err.Error()) + } +} + func TestExternalAdapter_Timeout(t *testing.T) { t.Parallel() diff --git a/metrics/graph.go b/metrics/graph.go index e8228d3..0b395d3 100644 --- a/metrics/graph.go +++ b/metrics/graph.go @@ -4,13 +4,13 @@ package metrics // Consumers should check this value before processing to detect incompatible changes. // Version changes follow semantic versioning: minor versions are backward-compatible, // major versions may contain breaking changes. -const SchemaVersionCurrent = "1.0" +const SchemaVersionCurrent = "1.1" // ModuleGraph represents the complete analysis result for a project. // It contains all modules with their computed metrics, detected circular // dependencies, and any warnings produced during analysis. type ModuleGraph struct { - // SchemaVersion is the version of the output schema (e.g., "1.0"). + // SchemaVersion is the version of the output schema (e.g., "1.1"). // Consumers use this to detect breaking changes in the JSON structure. SchemaVersion string `json:"schemaVersion"` // Language is the lowercase language identifier (e.g., "go", "python"). @@ -40,4 +40,11 @@ type ModuleResult struct { LCOM LCOM `json:"lcom"` // Zone is the classification of the module's position relative to the main sequence. Zone Zone `json:"zone"` + // Extensions contains language-specific metric extensions namespaced by language. + // Keys use the format "language.metricName" (e.g., "go.interfaceWidth"). + // Extensions are not schema-enforced beyond being a valid JSON object. + // Use language-specific typed accessor functions for safe extraction after + // JSON round-trip (JSON unmarshaling converts int to float64, nested maps to + // map[string]interface{}). + Extensions map[string]any `json:"extensions,omitempty"` } diff --git a/metrics/modulegraph.schema.json b/metrics/modulegraph.schema.json index 5fa9995..9f9848f 100644 --- a/metrics/modulegraph.schema.json +++ b/metrics/modulegraph.schema.json @@ -5,31 +5,90 @@ "type": "object", "required": ["schemaVersion", "language", "modules", "cycles", "warnings", "status"], "properties": { - "schemaVersion": { "type": "string" }, - "language": { "type": "string", "minLength": 1 }, + "schemaVersion": { + "type": "string", + "enum": ["1.0", "1.1"], + "description": "Version of the output schema. Consumers check this to detect breaking changes in the JSON structure." + }, + "language": { + "type": "string", + "minLength": 1, + "description": "Lowercase language identifier (e.g., \"go\", \"python\")." + }, "modules": { "type": "array", + "description": "Analysis results for each module in the project.", "items": { "type": "object", "required": ["path", "name", "ca", "ce", "instability", "abstractness", "distance", "lcom", "exportedTypes", "abstractTypes", "zone"], "properties": { - "path": { "type": "string" }, - "name": { "type": "string" }, - "ca": { "type": "integer", "minimum": 0 }, - "ce": { "type": "integer", "minimum": 0 }, - "instability": { "type": "number", "minimum": 0, "maximum": 1 }, - "abstractness": { "type": "number", "minimum": 0, "maximum": 1 }, - "distance": { "type": "number", "minimum": 0, "maximum": 1 }, - "lcom": { "type": "integer", "minimum": 0 }, - "exportedTypes": { "type": "integer", "minimum": 0 }, - "abstractTypes": { "type": "integer", "minimum": 0 }, - "zone": { "type": "string", "enum": ["main-sequence", "zone-of-pain", "zone-of-uselessness", "normal"] } + "path": { + "type": "string", + "description": "Unique identifier for this module (e.g., \"github.com/foo/bar\")." + }, + "name": { + "type": "string", + "description": "Human-readable short name (e.g., \"bar\")." + }, + "ca": { + "type": "integer", + "minimum": 0, + "description": "Afferent coupling — the number of modules that depend on this module." + }, + "ce": { + "type": "integer", + "minimum": 0, + "description": "Efferent coupling — the number of modules this module depends on." + }, + "instability": { + "type": "number", + "minimum": 0, + "maximum": 1, + "description": "Instability metric I = Ce / (Ca + Ce), in [0, 1]." + }, + "abstractness": { + "type": "number", + "minimum": 0, + "maximum": 1, + "description": "Abstractness A = abstractTypes / totalExported, in [0, 1]." + }, + "distance": { + "type": "number", + "minimum": 0, + "maximum": 1, + "description": "Distance from main sequence D = |A + I - 1|, in [0, 1]." + }, + "lcom": { + "type": "integer", + "minimum": 0, + "description": "Lack of Cohesion of Methods (LCOM4 variant); the number of connected components." + }, + "exportedTypes": { + "type": "integer", + "minimum": 0, + "description": "Total count of exported types in this module." + }, + "abstractTypes": { + "type": "integer", + "minimum": 0, + "description": "Count of abstract types (e.g., Go interfaces, Python ABCs)." + }, + "zone": { + "type": "string", + "enum": ["main-sequence", "zone-of-pain", "zone-of-uselessness", "normal"], + "description": "Classification of the module's position relative to the main sequence." + }, + "extensions": { + "type": "object", + "description": "Language-specific metric extensions namespaced by language (e.g., \"go.interfaceWidth\")." + } }, "additionalProperties": false } }, "cycles": { "type": "array", + "description": "Detected circular dependencies between modules, each reported as a lexicographically-sorted set of module paths.", "items": { "type": "array", "items": { "type": "string" } @@ -37,18 +96,32 @@ }, "warnings": { "type": "array", + "description": "Language-specific caveats about metric accuracy. Always present (empty when there are none).", "items": { "type": "object", "required": ["code", "message"], "properties": { - "code": { "type": "string" }, - "message": { "type": "string" }, - "module": { "type": "string" } + "code": { + "type": "string", + "description": "Machine-readable warning identifier (e.g., \"load-error\")." + }, + "message": { + "type": "string", + "description": "Human-readable description of the warning." + }, + "module": { + "type": "string", + "description": "Path of the affected module (empty if the warning applies globally)." + } }, "additionalProperties": false } }, - "status": { "type": "string", "enum": ["complete", "partial", "error"] } + "status": { + "type": "string", + "enum": ["complete", "partial", "error"], + "description": "Overall analysis outcome." + } }, "additionalProperties": false } diff --git a/metrics/validate.go b/metrics/validate.go index 08aa53e..26eb635 100644 --- a/metrics/validate.go +++ b/metrics/validate.go @@ -45,12 +45,13 @@ func validateTopLevel(raw map[string]interface{}) error { } // Validate schemaVersion is a supported value. + // Accept both "1.0" (no extensions) and "1.1" (with extensions) for backward compatibility. version, ok := raw["schemaVersion"].(string) if !ok { return fmt.Errorf("validate: field \"schemaVersion\" must be a string") } - if version != SchemaVersionCurrent { - return fmt.Errorf("validate: unsupported schema version %q (supported: %q)", version, SchemaVersionCurrent) + if version != "1.0" && version != "1.1" { + return fmt.Errorf("validate: unsupported schema version %q (supported: \"1.0\", \"1.1\")", version) } // Validate language is a non-empty string. @@ -157,9 +158,55 @@ func validateModule(v interface{}, index int) error { return fmt.Errorf("modules[%d]: %w", index, err) } + // Validate extensions field if present: must be a JSON object (not primitive or array). + if ext, exists := m["extensions"]; exists { + if _, ok := ext.(map[string]interface{}); !ok { + return fmt.Errorf("modules[%d]: field \"extensions\" must be a JSON object", index) + } + } + + // Enforce numeric ranges matching modulegraph.schema.json. This hardens the + // validator at the trust boundary against out-of-range values from an + // untrusted external analyzer. Ratio metrics are bounded to [0, 1]; raw + // counts must be non-negative. + for _, field := range []string{"instability", "abstractness", "distance"} { + val, err := moduleNumber(m, field, index) + if err != nil { + return err + } + if val < 0.0 || val > 1.0 { + return fmt.Errorf("modules[%d]: field %q value %g out of range [0, 1]", index, field, val) + } + } + for _, field := range []string{"ca", "ce", "lcom", "exportedTypes", "abstractTypes"} { + val, err := moduleNumber(m, field, index) + if err != nil { + return err + } + if val < 0 { + return fmt.Errorf("modules[%d]: field %q value %g must be >= 0", index, field, val) + } + } + return nil } +// moduleNumber extracts a numeric module field as a float64. JSON unmarshaling +// represents all numbers as float64, so both integer and ratio fields are read +// through this helper. It returns an error if the field is missing or is not a +// JSON number. +func moduleNumber(m map[string]interface{}, field string, index int) (float64, error) { + raw, ok := m[field] + if !ok { + return 0, fmt.Errorf("modules[%d]: missing required field %q", index, field) + } + num, ok := raw.(float64) + if !ok { + return 0, fmt.Errorf("modules[%d]: field %q must be a number", index, field) + } + return num, nil +} + // validateWarning checks that a warning element has the required code and message fields. func validateWarning(v interface{}, index int) error { w, ok := v.(map[string]interface{}) diff --git a/metrics/validate_test.go b/metrics/validate_test.go index fe9f88e..231c7d3 100644 --- a/metrics/validate_test.go +++ b/metrics/validate_test.go @@ -155,6 +155,148 @@ func TestValidate_ZeroMetricsSerialized(t *testing.T) { } } +func TestValidate_ExtensionsRoundTrip(t *testing.T) { + t.Parallel() + + original := ModuleGraph{ + SchemaVersion: "1.1", + Language: "go", + Modules: []ModuleResult{ + { + Module: Module{ + Path: "github.com/example/ext", + Name: "ext", + Ca: 1, + Ce: 2, + ExportedTypes: 3, + AbstractTypes: 1, + }, + Instability: 0.666666, + Abstractness: 0.333333, + Distance: 0.0, + LCOM: 1, + Zone: ZoneMainSequence, + Extensions: map[string]any{ + "go.interfaceWidth": map[string]int{"Reader": 1, "Writer": 2}, + "go.interfaceProximity": map[string]string{"Reader": "producer", "Writer": "consumer"}, + }, + }, + }, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + + data, err := json.Marshal(original) + if err != nil { + t.Fatalf("json.Marshal failed: %v", err) + } + + // Validate passes with extensions present. + if err := Validate(data); err != nil { + t.Fatalf("Validate returned error for valid input with extensions: %v", err) + } + + // Verify JSON round-trip type behavior: int becomes float64, nested maps + // become map[string]interface{}. + var decoded ModuleGraph + if err := json.Unmarshal(data, &decoded); err != nil { + t.Fatalf("json.Unmarshal failed: %v", err) + } + + ext := decoded.Modules[0].Extensions + if ext == nil { + t.Fatal("Extensions is nil after round-trip") + } + + // After JSON round-trip, map[string]int becomes map[string]interface{} with float64 values. + widthsRaw, ok := ext["go.interfaceWidth"] + if !ok { + t.Fatal("go.interfaceWidth missing from extensions") + } + widths, ok := widthsRaw.(map[string]interface{}) + if !ok { + t.Fatalf("go.interfaceWidth: got type %T, want map[string]interface{}", widthsRaw) + } + if got, want := widths["Reader"], float64(1); got != want { + t.Errorf("go.interfaceWidth[Reader]: got %v (%T), want %v (%T)", got, got, want, want) + } + if got, want := widths["Writer"], float64(2); got != want { + t.Errorf("go.interfaceWidth[Writer]: got %v (%T), want %v (%T)", got, got, want, want) + } + + // map[string]string round-trips to map[string]interface{} with string values. + proxRaw, ok := ext["go.interfaceProximity"] + if !ok { + t.Fatal("go.interfaceProximity missing from extensions") + } + prox, ok := proxRaw.(map[string]interface{}) + if !ok { + t.Fatalf("go.interfaceProximity: got type %T, want map[string]interface{}", proxRaw) + } + if got, want := prox["Reader"], "producer"; got != want { + t.Errorf("go.interfaceProximity[Reader]: got %v, want %v", got, want) + } +} + +func TestValidate_ExtensionsOmitted(t *testing.T) { + t.Parallel() + + // ModuleResult without extensions should serialize without the extensions field + // (omitempty) and still pass validation. + g := ModuleGraph{ + SchemaVersion: "1.1", + Language: "go", + Modules: []ModuleResult{ + { + Module: Module{ + Path: "github.com/example/noext", + Name: "noext", + Ca: 0, + Ce: 0, + ExportedTypes: 0, + AbstractTypes: 0, + }, + Zone: ZoneNormal, + }, + }, + Cycles: []Cycle{}, + Warnings: []Warning{}, + Status: StatusComplete, + } + + data, err := json.Marshal(g) + if err != nil { + t.Fatalf("json.Marshal failed: %v", err) + } + + if err := Validate(data); err != nil { + t.Fatalf("Validate returned error for input without extensions: %v", err) + } + + // Verify extensions field is not present in JSON. + if strings.Contains(string(data), `"extensions"`) { + t.Error("extensions field should be omitted from JSON when nil") + } +} + +func TestValidate_SchemaVersion11(t *testing.T) { + t.Parallel() + + // Validate accepts schema version "1.1". + data := []byte(`{ + "schemaVersion": "1.1", + "language": "go", + "modules": [], + "cycles": [], + "warnings": [], + "status": "complete" + }`) + if err := Validate(data); err != nil { + t.Fatalf("Validate rejected schema version 1.1: %v", err) + } +} + func TestValidate_InvalidInputs(t *testing.T) { t.Parallel() @@ -280,6 +422,66 @@ func TestValidate_InvalidInputs(t *testing.T) { }`, wantErr: "invalid zone", }, + { + name: "extensions is a string (invalid)", + data: `{ + "schemaVersion": "1.1", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, "ce": 0, + "instability": 0, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal", + "extensions": "not-an-object" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "\"extensions\" must be a JSON object", + }, + { + name: "extensions is an array (invalid)", + data: `{ + "schemaVersion": "1.1", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, "ce": 0, + "instability": 0, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal", + "extensions": [1, 2, 3] + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "\"extensions\" must be a JSON object", + }, + { + name: "extensions is a valid object (passes)", + data: `{ + "schemaVersion": "1.1", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, "ce": 0, + "instability": 0, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal", + "extensions": {"go.interfaceWidth": {"Foo": 1}} + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "", + }, { name: "warning missing code field", data: `{ @@ -328,6 +530,63 @@ func TestValidate_InvalidInputs(t *testing.T) { }`, wantErr: "", }, + { + name: "instability above 1 (out of range)", + data: `{ + "schemaVersion": "1.1", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, "ce": 0, + "instability": 1.5, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "out of range", + }, + { + name: "distance below 0 (out of range)", + data: `{ + "schemaVersion": "1.1", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": 0, "ce": 0, + "instability": 0, "abstractness": 0, "distance": -0.5, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "out of range", + }, + { + name: "negative ca (must be >= 0)", + data: `{ + "schemaVersion": "1.1", + "language": "go", + "modules": [{ + "path": "foo", + "name": "foo", + "ca": -1, "ce": 0, + "instability": 0, "abstractness": 0, "distance": 0, "lcom": 0, + "exportedTypes": 0, "abstractTypes": 0, + "zone": "normal" + }], + "cycles": [], + "warnings": [], + "status": "complete" + }`, + wantErr: "must be >= 0", + }, } for _, tt := range tests { diff --git a/openspec/changes/go-analyze/.openspec.yaml b/openspec/changes/go-analyze/.openspec.yaml new file mode 100644 index 0000000..50adc91 --- /dev/null +++ b/openspec/changes/go-analyze/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-08-29 diff --git a/openspec/changes/go-analyze/design.md b/openspec/changes/go-analyze/design.md new file mode 100644 index 0000000..561752f --- /dev/null +++ b/openspec/changes/go-analyze/design.md @@ -0,0 +1,342 @@ +## Context + +Vibe-Check's `metrics/` package provides a universal coupling metrics model (Layer 1) +with the `Adapter` interface, `Module`/`ModuleResult`/`ModuleGraph` types, and +deterministic compute functions. No language adapter or CLI exists yet. This design +covers the Go-native adapter (Layer 2) and the `vibe-check analyze` CLI command — the +first language adapter that makes the toolkit usable. + +The existing `metrics.Adapter` interface requires: +- `Analyze(ctx context.Context, projectPath string) (*ModuleGraph, error)` +- `Language() string` +- `Capabilities() []Capability` + +The adapter must populate `Module` fields (Path, Name, Ca, Ce, ExportedTypes, +AbstractTypes) and compute derived metrics using `metrics.Compute*` functions. The +result must pass `metrics.Validate()`. + +## Goals / Non-Goals + +**Goals:** + +- Implement a Go-native adapter in `internal/goadapter/` that resolves package + dependencies via `golang.org/x/tools/go/packages` and computes all seven metrics. +- Implement a `vibe-check analyze` CLI command using cobra with JSON output and CI + gate threshold flags. +- Produce deterministic, schema-valid `ModuleGraph` output. +- Support analyzing arbitrary Go module paths (defaulting to `./...`). +- Propagate `context.Context` for cancellation and timeout support. +- Add an extensions mechanism to `ModuleResult` for language-specific metrics. +- Populate Go-specific extensions: interface width (Pike metric) and interface + proximity (consumer vs. producer declaration). + +**Non-Goals:** + +- Multi-language support (Python, TS/JS adapters are P2/P3). +- Non-JSON output formats (table, SARIF — future work). +- Incremental/cached analysis (full analysis each invocation). +- IDE integration or watch mode. +- LCOM4 computation for non-Go languages. +- Provenance metadata in `ModuleGraph` schema (Constitution III gap — tracked as + follow-up issue to add `producer`, `version`, `timestamp`, `commit` fields). +- CI gate flags for extension metrics (interface width, proximity — can be added later). +- Schema enforcement on extension contents (validated as proper JSON objects only). + +## Decisions + +### D1: Package-level analysis granularity + +**Decision**: Analyze at the Go package level — each Go package is one `metrics.Module`. + +**Rationale**: Go packages are the natural unit of encapsulation and dependency. Import +relationships are explicit and resolved by the compiler. Package-level analysis aligns +with how Go developers reason about coupling. + +**Alternatives considered**: +- File-level: Too granular, misaligns with Go's package-centric model. +- Module-level: Too coarse, hides internal coupling problems. + +### D2: `golang.org/x/tools/go/packages` for dependency resolution + +**Decision**: Use `packages.Load` with `NeedName | NeedImports | NeedTypes | NeedSyntax | +NeedTypesInfo | NeedModule` mode flags to get type-aware dependency and type information. `NeedTypesInfo` +provides the `types.Info` mapping from AST nodes to resolved types, which is required for +accurate LCOM4 field-access resolution in method bodies. + +**Rationale**: The `go/packages` API is the canonical way to load Go packages with full +type information. It handles build constraints, modules, vendoring, and CGo correctly. +Raw `go list` output lacks type information needed for abstractness and LCOM computation. + +**Alternatives considered**: +- `go list -json`: Missing type info (exported/abstract type counts, method-field graphs). +- `go/build`: not module-aware; lacks type info. +- `go/ast` + manual resolution: Reimplements what `go/packages` already provides. + +### D3: Abstractness via type-checker scope classification + +**Decision**: Inspect each package's type-checker scope (`pkg.Types.Scope()`) to count +exported type declarations and classify types whose underlying type is an interface as +abstract. Every exported type declaration is counted. A type is abstract when its +underlying type is an interface (an interface declaration, or a defined type such as +`type Z SomeInterface`); structs, other named types, and type aliases (including an +alias to an interface) are concrete. + +**Rationale**: Go has a single mechanism for abstraction — interfaces. Unlike Java/C# +there are no abstract classes. This makes classification unambiguous: +`ExportedTypes` = count of all exported type declarations, +`AbstractTypes` = count of exported non-alias types whose underlying type is an interface. + +### D4: LCOM4 via method-field connected components + +**Decision**: Compute LCOM4 per package using a graph where nodes are exported methods +and edges connect methods that share field access. LCOM4 = number of connected +components in this graph. + +**Rationale**: LCOM4 (Hitz & Montazeri 1995) uses connected-component semantics which +are well-defined and deterministic. For Go, "fields" are struct fields accessed by +methods. Methods sharing no state form separate components, +indicating the package should potentially be split. + +**Scope boundary**: LCOM is computed per package. Methods are functions with a receiver +belonging to an exported type. Package-level functions without receivers are excluded +from the LCOM graph (they don't indicate type cohesion). + +### D5: Circular dependency detection via Tarjan's SCC + +**Decision**: Use Tarjan's strongly connected components algorithm on the package +import graph to detect cycles. Each SCC with more than one node is a cycle. + +**Rationale**: Tarjan's algorithm is O(V+E), well-understood, and produces canonical +cycle representations. Go technically prohibits import cycles at compile time, but +the adapter should still detect them to (a) report on near-cycles when analyzing +partial builds and (b) maintain consistency with multi-language adapters where cycles +are possible. + +### D6: Scope filtering — module boundary + +**Decision**: Only analyze packages within the target module. Standard library and +third-party packages contribute to Ca/Ce counts but are not themselves analyzed as +modules in the output. + +**Rationale**: Users want to understand the coupling characteristics of _their_ code. +External dependencies are relevant as coupling sources but not as analysis targets. +This also bounds the analysis to a tractable scope. The adapter relies on `go/packages` +module resolution for scope filtering — packages resolved by `go/packages` that fall +within the target module's import path prefix are in scope regardless of their physical +location. + +### D7: CLI structure with cobra + +**Decision**: Use cobra for CLI with a root `vibe-check` command and an `analyze` +subcommand. Flags on the `analyze` command: +- `--max-instability ` — fail if any module exceeds this instability +- `--max-distance ` — fail if any module exceeds this distance +- `--no-circular-deps` — fail if any cycles detected +- `--max-lcom ` — fail if any module's LCOM exceeds this value +- `--timeout ` — maximum analysis time (e.g., `5m`, `30s`); no timeout by default +- (No `--json` flag: JSON is the sole output format for P0; a format selector is intentionally deferred to future work.) + +Exit codes: +- 0: success, no threshold violations +- 1: threshold violations detected (policy failure) +- 2: analysis error, invalid arguments, or missing Go toolchain (tool failure) + +Violations printed to stderr, JSON output to stdout. + +**Rationale**: cobra is adopted per convention CS-009 to maintain consistency with the +broader Go ecosystem and to support future subcommands (e.g., `vibe-check report`, +`vibe-check drift`) without refactoring the CLI layer. It provides subcommand routing, +automatic help/usage generation, shell completion, and flag validation that `flag` alone +does not offer. The `--max-lcom` flag name matches the metric name (LCOM) and follows +the `--max-*` pattern established by `--max-instability` and `--max-distance`, avoiding +semantic inversion. + +**Testable CLI pattern**: The analyze command MUST follow AP-002/AP-003. A +`RunAnalyze(opts AnalyzeOptions) (*AnalyzeResult, error)` function handles all logic. +The cobra command's `RunE` delegates to `RunAnalyze` with `io.Writer` fields for +stdout/stderr, enabling unit testing without subprocess execution. + +### D8: Package layout + +**Decision**: +- `cmd/vibe-check/main.go` — entry point, minimal (cobra Execute) +- `cmd/vibe-check/analyze.go` — analyze command definition and flag handling +- `internal/goadapter/adapter.go` — `Adapter` struct implementing `metrics.Adapter` +- `internal/goadapter/resolve.go` — package dependency resolution via `go/packages` +- `internal/goadapter/types.go` — type counting (exported, abstract) +- `internal/goadapter/lcom.go` — LCOM4 computation +- `internal/goadapter/cycles.go` — Tarjan's SCC for cycle detection +- `internal/goadapter/extensions.go` — Go-specific extensions (interface width, proximity) +- `internal/goadapter/doc.go` — package-level GoDoc + +**Rationale**: `internal/goadapter/` prevents external import (this is an implementation +detail) and avoids using `go` as a package name (which is a Go keyword). Separating +concerns across files keeps each file focused. The `cmd/` directory follows Go project +conventions. + +### D9: Context propagation + +**Decision**: The adapter MUST propagate the provided `context.Context` to +`packages.Config.Context` for cancellation support. The CLI MUST create a context with +signal handling (SIGINT/SIGTERM) and pass it through to the adapter. When SIGINT is +received during analysis, no partial JSON MUST be written to stdout — the command +MUST exit cleanly with a non-zero status and an error message to stderr. + +**Rationale**: `go/packages.Load` can take minutes on large codebases (acknowledged in +R1). Without context propagation, analysis could hang indefinitely in CI environments. +The `packages.Config` struct accepts a `Context` field specifically for this purpose. + +### D10: Registry integration + +**Decision**: The CLI directly instantiates the Go adapter rather than going through the +`metrics.Registry`. The registry pattern is designed for multi-adapter scenarios where +adapter selection is dynamic. With a single adapter, direct instantiation is simpler and +avoids unnecessary indirection. + +**Rationale**: The registry exists for future multi-language support where a dispatcher +would select adapters by language. The P0 CLI knows it wants the Go adapter and gains +nothing from registry lookup. When multi-language support is added (P2+), the CLI will +be refactored to use the registry. + +### D11: Language-specific extensions mechanism + +**Decision**: Add an optional `Extensions map[string]any` field to `ModuleResult` +(with `json:"extensions,omitempty"`) and a corresponding `extensions` object in the +JSON schema. Add `"extensions": { "type": "object" }` to the module item's `properties` +list while keeping `additionalProperties: false` — this preserves schema strictness +while allowing only the extensions field. Bump `SchemaVersion` from `"1.0"` to `"1.1"` +to signal the schema evolution (backward-compatible addition per semver). The core +model does not interpret extensions — adapters populate them, consumers opt-in to +reading them. + +**Type safety**: Extension values undergo type coercion during JSON round-trip +(`int` → `float64`, `map[string]int` → `map[string]interface{}`). The adapter +package MUST provide typed accessor functions (e.g., +`InterfaceWidths(extensions map[string]any) (map[string]int, error)`) that safely +extract and validate extension values after JSON unmarshaling. This prevents +consumers from needing unchecked type assertions. + +**Rationale**: Go-specific metrics like interface width and interface proximity are +valuable for Go codebases but do not belong in the universal model. An extensions +mechanism allows language adapters to carry additional metrics without schema changes +for each new language. Keys are namespaced by language (e.g., `go.interfaceWidth`) +to prevent collisions between adapters. + +**Alternatives considered**: +- Separate output field: Adds complexity to `ModuleGraph` and requires schema changes. +- Adapter-specific output structs: Breaks the unified `ModuleGraph` pipeline. + +### D12: Interface Width (Go Pike Metric) + +**Decision**: The Go adapter populates `go.interfaceWidth` in extensions as a +`map[string]int` mapping exported interface names to their method count. Go idiom +favors narrow interfaces (1-2 methods); wider interfaces signal abstraction problems. + +**Rationale**: Rob Pike / Go Proverbs recommend small interfaces. Measuring interface +width surfaces packages with overly broad abstractions. This metric is language-specific +(Go's implicit interface satisfaction makes width semantics different from Java/C#). + +**Scope**: Only exported interfaces in analyzed packages. Standard library and +third-party interfaces are excluded. + +### D13: Interface-to-Implementation Proximity + +**Decision**: The Go adapter populates `go.interfaceProximity` in extensions as a +`map[string]string` mapping exported interface names to `"consumer"` or `"producer"`. +An interface is `"consumer"` if it is declared in a different package from its primary +implementation; `"producer"` if declared in the same package. + +**Rationale**: Consumer-side interface declaration is a Go best practice (per Go FAQ, +Effective Go). Measuring proximity surfaces packages where interfaces are declared +on the producer side, which can lead to unnecessary coupling. + +**Heuristic**: For each exported interface, check whether any type in the same package +implements it. If yes → `"producer"`. If no implementation found in the same package → +`"consumer"`. This is a heuristic — the "primary" implementation is not always in the +same module. + +## Coverage Strategy + +**Unit tests**: Each `internal/goadapter/*.go` file has a corresponding `*_test.go`. +Tests use constructed adjacency maps, AST fixtures, and `testdata/` Go modules with +known characteristics. Coverage target: >= 80% line coverage for `internal/goadapter/`. + +**Integration test**: Analyze a dedicated `testdata/` Go module (not self-analysis) with +known structure. Assert specific metric values and verify output passes +`metrics.Validate()`. Guard with `testing.Short()` per TC-011. + +**CLI tests**: Use the testable CLI pattern (AP-003) — inject `bytes.Buffer` for +stdout/stderr. Test flag parsing, threshold violation logic, exit codes, and JSON output +validity. No subprocess execution in tests. + +**Determinism verification**: Call `Analyze()` on the same testdata module multiple times +and assert JSON output is byte-identical, following the pattern in +`metrics/compute_test.go`. + +**CLI tests coverage target**: >= 80% line coverage for `cmd/vibe-check/`. + +**Coverage profile**: CI generates a coverage profile (`go test -race -count=1 +-coverprofile=coverage.out ./...`) but does not yet enforce a coverage ratchet or a +minimum-threshold gate. Ratchet/threshold enforcement is a follow-up. + +**Module ordering**: The adapter MUST sort `Modules` by `Module.Path` in lexicographic +order before returning the `ModuleGraph` to ensure deterministic output across runs. + +## Operational Notes + +**Memory consumption**: `go/packages` with `NeedSyntax | NeedTypes` loads full ASTs and +type information into memory for all requested packages simultaneously. For large +codebases (1000+ packages), expect approximately 1-2 MB per package. Users analyzing +large monorepos should expect higher memory use; pointing `analyze` at a specific +subdirectory (rather than the repository root) reduces the package set loaded, and +`--timeout` bounds runtime. + +**Error messages**: Error messages MUST include the operation that failed, the underlying +cause, and a suggested remediation when determinable (e.g., "failed to load packages: +go.mod not found in /path — ensure the target directory contains a Go module"). + +**Version embedding**: The root command supports `--version` via cobra's built-in version +flag. Version, commit hash, and build date are embedded at build time via ldflags +(`-ldflags "-X main.version=... -X main.commit=... -X main.date=..."`). When ldflags are +absent (e.g., `go install github.com/zero-dot-force/vibe-check/cmd/vibe-check@vX`), the +command falls back to `runtime/debug.ReadBuildInfo()`, reporting the module version from +build info (plus the `vcs.revision`/`vcs.time` build settings when present, e.g. for VCS +builds), so `--version` still reports a meaningful version. +The version output format MUST be: `vibe-check version (commit , built )`. + +**Environment sanitization**: The adapter MUST set `packages.Config.Env` to a sanitized +environment using `metrics.SanitizeEnvironment` to prevent credential leakage to +subprocesses spawned by `go/packages` during package loading. + +**Warning content**: Warning messages for partial builds MUST contain the affected +package's import path and the underlying error message using relative paths (relative +to the project root) rather than absolute paths. Warning code MUST be a machine-readable +identifier (e.g., `"load-error"`, `"type-check-error"`). + +**Performance baseline**: Analysis of the vibe-check module itself (currently ~1 package) +MUST complete within 30 seconds on a standard CI runner. Large-codebase performance +optimization is deferred to P2. + +## Risks / Trade-offs + +**[R1] `go/packages` load time on large codebases** → The `NeedSyntax` mode flag +triggers full parsing. For very large monorepos (1000+ packages), initial load may be +slow. **Mitigation**: Accept for P0. Context cancellation provides timeout escape hatch. +Incremental analysis (P2+) will address performance. + +**[R2] LCOM4 accuracy for Go idioms** → Go's implicit interface satisfaction and +package-level functions don't map perfectly to OOP LCOM models. **Mitigation**: Document +the adaptation in GoDoc. Emit a warning when a package has only package-level functions +(no receivers) since LCOM is not meaningful in that case. + +**[R3] Build errors in analyzed code** → If the target codebase has compilation errors, +`go/packages` may return partial results. **Mitigation**: Check `packages.Package.Errors` +and set `Status: StatusPartial` with warnings listing the affected packages. When a +package's type information is unavailable (nil `pkg.Types`), set ExportedTypes and +AbstractTypes to 0, set LCOM to 0, and add a warning. + +**[R4] New dependencies increase attack surface** → Adding cobra and x/tools introduces +transitive dependencies. `golang.org/x/tools` is large but only `go/packages` (and its +transitive deps within x/tools) is imported. cobra brings `pflag` and `mousetrap` as +transitive deps. **Mitigation**: Both are widely-used, well-maintained Go ecosystem +projects. Pin versions in `go.mod`. Run `go mod tidy` to include only necessary deps. diff --git a/openspec/changes/go-analyze/proposal.md b/openspec/changes/go-analyze/proposal.md new file mode 100644 index 0000000..05db5c3 --- /dev/null +++ b/openspec/changes/go-analyze/proposal.md @@ -0,0 +1,78 @@ +## Why + +Vibe-Check has a universal coupling metrics model (`metrics/` package) but no way to +actually analyze Go code. Without a Go-native adapter and CLI entry point, the toolkit +cannot compute any metrics. This is the first language adapter (P1 per the RFC phasing) +and the highest-priority remaining work — it makes the toolkit usable and unblocks all +downstream features (CI gating, multi-language support, architectural drift tracking). + +Note: While AGENTS.md classifies language adapters as P1, the Go adapter is effectively +the minimum viable product. The P0 universal model (`metrics/`) is complete but produces +no value without at least one adapter. AGENTS.md will be updated to reflect this. + +## What Changes + +- Add a Go-native adapter (`internal/goadapter/`) that implements `metrics.Adapter` using + `golang.org/x/tools/go/packages` for type-aware dependency resolution. +- Add a `vibe-check analyze [path]` CLI command (`cmd/vibe-check/`) using cobra + that invokes the Go adapter, produces JSON output conforming to `metrics.ModuleGraph`, + and supports CI gate flags for threshold enforcement. +- Add new external dependencies: `golang.org/x/tools/go/packages`, `github.com/spf13/cobra`. + +## Capabilities + +### New Capabilities + +- `go-adapter`: Go-native language adapter implementing `metrics.Adapter` — resolves + package dependencies via `golang.org/x/tools/go/packages`, counts exported/abstract + types, computes LCOM4 cohesion, and detects circular dependencies. +- `analyze-command`: CLI command `vibe-check analyze` — orchestrates adapter invocation, + formats JSON output, validates against schema, and enforces CI gate thresholds + (`--max-instability`, `--max-distance`, `--no-circular-deps`, `--max-lcom`). +- `extensions-mechanism`: Language-specific extensions field on `ModuleResult` — + enables adapters to carry metrics beyond the universal model (e.g., Go interface + width, interface proximity) without modifying the core schema. Keys are namespaced + by language (e.g., `go.interfaceWidth`). + +### Modified Capabilities + +- `metrics-schema`: Add optional `extensions` object field to module items in + `modulegraph.schema.json` while keeping `additionalProperties: false` (extensions + is an explicitly allowed property). Bump `SchemaVersion` from `"1.0"` to `"1.1"`. + Top-level `ModuleGraph` and `Warning` schemas remain strict. +- `metrics-model`: Add `Extensions map[string]any` field to `ModuleResult` with + `json:"extensions,omitempty"` tag. Update `metrics.Validate` to accept the + extensions field. + +## Impact + +- **New packages**: `internal/goadapter/` (adapter), `cmd/vibe-check/` (CLI entry point) +- **Dependencies**: `golang.org/x/tools` (Go analysis), `github.com/spf13/cobra` (CLI) +- **Build artifacts**: `vibe-check` binary +- **APIs**: `ModuleResult` gains an `Extensions` field (`map[string]any`, omitempty). + JSON schema updated to allow `extensions` on module items. Existing consumers are + unaffected (field is optional and omitted when empty). +- **CI**: New binary build target; threshold flags enable CI gating in downstream pipelines +- **Distribution**: `go install github.com/zero-dot-force/vibe-check/cmd/vibe-check@v0.1.0` + for initial distribution. An initial `v0.1.0` release tag is required for `go install` + to resolve. +- **Follow-up obligations** (issues to file or refresh before PR merge): + - Documentation: refresh the scope of the existing docs issue (#18) to cover the new + `vibe-check analyze` CLI (usage, flags, exit codes) — it predates this command. + - Content: existing blog (#19) and tutorial (#22) issues track ecosystem write-ups. + - GoReleaser / release-automation: not yet tracked — file an issue before PR merge. + - Website documentation sync: not yet tracked — file an issue before PR merge. + - Provenance metadata (Constitution III gap): tracked as a follow-up to emit + `producer`, `version`, `timestamp`, and input fields in the `ModuleGraph`. + +## Constitution Alignment + +| Principle | Assessment | +|-----------|------------| +| I. Autonomous Collaboration | PASS — Adapter produces self-describing JSON output | +| II. Composability First | PASS — Implements existing `metrics.Adapter` interface | +| III. Observable Quality | PARTIAL — JSON output with schema validation and `--version` embedding are in place, but provenance metadata (producer, version, timestamp, input) is NOT yet emitted in the `ModuleGraph`. It is deferred to a tracked follow-up issue. | +| IV. Testability | PASS — Coverage strategy defined in design.md | +| V. Security by Default | PASS — Path validation via `metrics.ValidateProjectPath`; context cancellation support | +| VI. Metric Fidelity | PASS — Uses canonical `metrics.Compute*` functions; LCOM4 (Hitz & Montazeri) variant cited. LCOM4 generic pointer-receiver handling was corrected (e.g., `func (t *T[P]) M()`). | +| VII. Language Agnosticism | PASS — Adapter pattern; extensions mechanism is language-agnostic (any adapter can use it) | diff --git a/openspec/changes/go-analyze/specs/analyze-command/spec.md b/openspec/changes/go-analyze/specs/analyze-command/spec.md new file mode 100644 index 0000000..f37967f --- /dev/null +++ b/openspec/changes/go-analyze/specs/analyze-command/spec.md @@ -0,0 +1,274 @@ +## ADDED Requirements + +### Requirement: CLI entry point + +The `vibe-check` binary MUST be buildable from `cmd/vibe-check/main.go` using +`go build ./cmd/vibe-check`. The binary MUST use cobra for command-line parsing +with a root command and an `analyze` subcommand. The root command MUST support a +`--version` flag that prints the tool version, build commit, and build date +(embedded at build time via ldflags). + +#### Scenario: Binary builds successfully + +- **GIVEN** the project source code +- **WHEN** `go build ./cmd/vibe-check` is executed +- **THEN** a `vibe-check` binary MUST be produced without errors + +#### Scenario: Help output + +- **GIVEN** a built `vibe-check` binary +- **WHEN** `vibe-check --help` is executed +- **THEN** the output MUST list the `analyze` subcommand with a brief description + +#### Scenario: Version output + +- **GIVEN** a built `vibe-check` binary with version info embedded +- **WHEN** `vibe-check --version` is executed +- **THEN** the output MUST match the format `vibe-check version (commit , built )` + +### Requirement: Analyze command invokes Go adapter + +The `analyze` subcommand MUST create a Go adapter instance, invoke `Analyze` with +the target project path, and output the resulting `ModuleGraph` as JSON to stdout. +The command MUST follow the testable CLI pattern (AP-002/AP-003): a `RunAnalyze` +function accepts a params struct with `io.Writer` fields for stdout/stderr. + +#### Scenario: Default analysis target + +- **GIVEN** a Go module directory as the current working directory +- **WHEN** `vibe-check analyze` is invoked with no positional arguments +- **THEN** the adapter MUST analyze the current directory (`.`) + +#### Scenario: Explicit path argument + +- **GIVEN** a Go module at `/path/to/project` +- **WHEN** `vibe-check analyze /path/to/project` is invoked +- **THEN** the adapter MUST analyze the specified path + +#### Scenario: Adapter error propagation + +- **GIVEN** a target path that causes the Go adapter to return an error +- **WHEN** `vibe-check analyze` is invoked +- **THEN** the command MUST print an actionable error to stderr (including the operation that failed, the cause, and suggested remediation) and exit with status 2 + +### Requirement: JSON output to stdout + +The analyze command MUST output the `ModuleGraph` as JSON to stdout. The output +MUST be valid JSON that passes `metrics.Validate`. The JSON MUST be +pretty-printed (indented) by default. + +#### Scenario: Valid JSON on stdout + +- **GIVEN** a valid Go module +- **WHEN** analysis completes successfully +- **THEN** stdout MUST contain a JSON object conforming to the `ModuleGraph` schema + +#### Scenario: Stderr separation + +- **GIVEN** analysis that completes with threshold violations or warnings +- **WHEN** the command exits +- **THEN** violation messages MUST be written to stderr, and valid JSON MUST still be written to stdout + +### Requirement: Max instability threshold flag + +The analyze command MUST accept a `--max-instability` flag with a float64 value in +the range [0.0, 1.0]. When set, if any module's Instability exceeds the threshold, +the command MUST report the violation to stderr and exit with status 1. + +#### Scenario: Instability within threshold + +- **GIVEN** all modules have Instability <= 0.8 +- **WHEN** `--max-instability 0.8` is set +- **THEN** the command MUST exit with status 0 + +#### Scenario: Instability exceeds threshold + +- **GIVEN** a module with Instability 0.7 +- **WHEN** `--max-instability 0.5` is set +- **THEN** the command MUST print the violating module and its instability to stderr and exit with status 1 + +### Requirement: Max distance threshold flag + +The analyze command MUST accept a `--max-distance` flag with a float64 value in +the range [0.0, 1.0]. When set, if any module's Distance exceeds the threshold, +the command MUST report the violation to stderr and exit with status 1. + +#### Scenario: Distance within threshold + +- **GIVEN** all modules have Distance <= 0.5 +- **WHEN** `--max-distance 0.5` is set +- **THEN** the command MUST exit with status 0 + +#### Scenario: Distance exceeds threshold + +- **GIVEN** a module with Distance 0.6 +- **WHEN** `--max-distance 0.3` is set +- **THEN** the command MUST print the violating module and its distance to stderr and exit with status 1 + +### Requirement: No circular deps flag + +The analyze command MUST accept a `--no-circular-deps` boolean flag. When set, +if any cycles are detected in the `ModuleGraph`, the command MUST report each +cycle to stderr and exit with status 1. + +#### Scenario: No cycles detected + +- **GIVEN** the `Cycles` slice is empty +- **WHEN** `--no-circular-deps` is set +- **THEN** the command MUST exit with status 0 + +#### Scenario: Cycles detected + +- **GIVEN** cycles exist in the analysis result +- **WHEN** `--no-circular-deps` is set +- **THEN** the command MUST print each cycle to stderr and exit with status 1 + +### Requirement: Max LCOM threshold flag + +The analyze command MUST accept a `--max-lcom` flag with an integer value >= 1. +When set, if any module's LCOM exceeds the threshold (higher LCOM = worse cohesion), +the command MUST report the violation to stderr and exit with status 1. + +#### Scenario: LCOM within threshold + +- **GIVEN** all modules have LCOM <= 3 +- **WHEN** `--max-lcom 3` is set +- **THEN** the command MUST exit with status 0 + +#### Scenario: LCOM exceeds threshold + +- **GIVEN** a module with LCOM 4 +- **WHEN** `--max-lcom 2` is set +- **THEN** the command MUST print the violating module and its LCOM to stderr and exit with status 1 + +### Requirement: Flag value validation + +The command MUST validate all flag values before running analysis. Invalid flag +values MUST prevent analysis from running and exit with status 2. + +#### Scenario: Max instability out of range + +- **GIVEN** `--max-instability 1.5` (value > 1.0) +- **WHEN** the command is invoked +- **THEN** the command MUST print a validation error to stderr and exit with status 2 without running analysis + +#### Scenario: Max distance negative + +- **GIVEN** `--max-distance -0.5` (negative value) +- **WHEN** the command is invoked +- **THEN** the command MUST print a validation error to stderr and exit with status 2 without running analysis + +#### Scenario: Max LCOM less than 1 + +- **GIVEN** `--max-lcom 0` (value < 1) +- **WHEN** the command is invoked +- **THEN** the command MUST print a validation error to stderr and exit with status 2 without running analysis + +### Requirement: Multiple threshold violations + +When multiple threshold flags are set and multiple violations occur, the command +MUST report ALL violations before exiting. The command MUST NOT exit on the +first violation. + +#### Scenario: Multiple flags with multiple violations + +- **GIVEN** violations exist for both instability and distance thresholds +- **WHEN** `--max-instability 0.5 --max-distance 0.3` is set +- **THEN** all violations MUST be reported to stderr and the command MUST exit with status 1 + +### Requirement: JSON output produced regardless of violations + +The analyze command MUST always produce JSON output to stdout, even when threshold +violations cause a non-zero exit code. This enables CI pipelines to capture the full +analysis results while still failing the gate. + +#### Scenario: JSON output with violations + +- **GIVEN** threshold violations are detected +- **WHEN** the command exits with status 1 +- **THEN** the complete `ModuleGraph` JSON MUST still be written to stdout before the command exits + +### Requirement: Exit code semantics + +The command MUST use distinct exit codes to differentiate failure types: +- Exit 0: analysis succeeded, no threshold violations +- Exit 1: analysis succeeded but threshold violations detected (policy failure) +- Exit 2: analysis itself failed or invalid arguments (tool failure) + +#### Scenario: Tool failure exit code + +- **GIVEN** the target path is not a Go module +- **WHEN** `vibe-check analyze /not/a/module` is invoked +- **THEN** the command MUST exit with status 2 + +#### Scenario: Policy failure exit code + +- **GIVEN** a threshold violation exists +- **WHEN** analysis completes successfully but a threshold is exceeded +- **THEN** the command MUST exit with status 1 + +#### Scenario: Success exit code + +- **GIVEN** no threshold flags are set or all thresholds pass +- **WHEN** analysis completes successfully +- **THEN** the command MUST exit with status 0 + +### Requirement: Timeout flag + +The analyze command MUST accept a `--timeout ` flag (e.g., `5m`, `30s`). +When set, the command MUST create a `context.WithTimeout` wrapping the signal-handling +context. When the timeout is exceeded, the command MUST exit with status 2 and an +error message indicating timeout. The default is no timeout. + +#### Scenario: Timeout exceeded + +- **GIVEN** `--timeout 1ms` is set and analysis takes longer than 1ms +- **WHEN** the analysis exceeds the timeout +- **THEN** the command MUST exit with status 2 and print a timeout error to stderr + +#### Scenario: No timeout by default + +- **GIVEN** no `--timeout` flag is set +- **WHEN** analysis is invoked +- **THEN** no timeout MUST be applied (analysis runs until completion or signal) + +### Requirement: Signal handling and graceful shutdown + +The CLI MUST create a context with signal handling for SIGINT and SIGTERM. When a +signal is received during analysis, no partial JSON MUST be written to stdout. The +command MUST exit with a non-zero status and an error message to stderr. + +#### Scenario: Signal suppresses partial output + +- **GIVEN** analysis is in progress +- **WHEN** SIGINT is received +- **THEN** no partial JSON MUST be written to stdout, and the command MUST exit with status 2 + +#### Scenario: Context timeout propagated to adapter + +- **GIVEN** `--timeout 10s` is set +- **WHEN** the deadline is exceeded during analysis +- **THEN** the command MUST exit with status 2 and print a timeout error to stderr + +### Requirement: Threshold boundary semantics + +Threshold comparison MUST use strict greater-than (`>`) for all threshold flags. +A module whose metric value exactly equals the threshold MUST pass (not violate). + +#### Scenario: Instability at exact boundary + +- **GIVEN** a module with Instability exactly 0.5 +- **WHEN** `--max-instability 0.5` is set +- **THEN** the command MUST exit with status 0 (boundary value passes) + +#### Scenario: Distance at exact boundary + +- **GIVEN** a module with Distance exactly 0.3 +- **WHEN** `--max-distance 0.3` is set +- **THEN** the command MUST exit with status 0 (boundary value passes) + +#### Scenario: LCOM at exact boundary + +- **GIVEN** a module with LCOM exactly 3 +- **WHEN** `--max-lcom 3` is set +- **THEN** the command MUST exit with status 0 (boundary value passes) diff --git a/openspec/changes/go-analyze/specs/go-adapter/spec.md b/openspec/changes/go-analyze/specs/go-adapter/spec.md new file mode 100644 index 0000000..ab246f3 --- /dev/null +++ b/openspec/changes/go-analyze/specs/go-adapter/spec.md @@ -0,0 +1,358 @@ +## ADDED Requirements + +### Requirement: Adapter implements metrics.Adapter interface + +The Go adapter MUST implement the `metrics.Adapter` interface from the `metrics` +package. `Language()` MUST return `"go"`. `Capabilities()` MUST return all seven +capabilities: `CapAfferentCoupling`, `CapEfferentCoupling`, `CapInstability`, +`CapAbstractness`, `CapDistance`, `CapLCOM`, `CapCircularDeps`. + +#### Scenario: Language identifier + +- **GIVEN** a Go adapter instance +- **WHEN** `Language()` is called +- **THEN** the return value MUST be `"go"` + +#### Scenario: Full capability set + +- **GIVEN** a Go adapter instance +- **WHEN** `Capabilities()` is called +- **THEN** the returned slice MUST contain exactly seven capabilities matching all `Cap*` constants defined in the `metrics` package + +### Requirement: Adapter resolves Go package dependencies + +The adapter MUST use `golang.org/x/tools/go/packages` to load packages from the +target project path. Each Go package within the analyzed module MUST be represented +as one `metrics.Module`. The adapter MUST resolve import relationships to compute +afferent coupling (Ca) and efferent coupling (Ce) for each package. + +#### Scenario: Single module analysis + +- **GIVEN** a Go module containing packages A, B, C where A imports B and C imports B +- **WHEN** `Analyze` is called with the module path +- **THEN** the result MUST contain three `ModuleResult` entries, and package B MUST have `Ca == 2` (A and C depend on it). Ce counts all distinct import targets including standard library and third-party packages. + +#### Scenario: Standard library exclusion + +- **GIVEN** a package that imports standard library packages (e.g., `fmt`, `os`) +- **WHEN** `Analyze` is called +- **THEN** standard library packages MUST NOT appear as modules in the output, but MUST be counted in the importing package's Ce + +#### Scenario: Third-party dependency exclusion + +- **GIVEN** a package that imports packages outside the target module +- **WHEN** `Analyze` is called +- **THEN** external packages MUST NOT appear as modules in the output, but MUST be counted in the importing package's Ce + +### Requirement: Adapter counts exported and abstract types + +The adapter MUST inspect the type-checker scope of each package to count exported type +declarations. An exported type MUST be counted as abstract when its underlying type is an +interface (an interface declaration, or a defined type whose underlying type is an +interface). All other exported types — structs, other named types, and type aliases +(including an alias to an interface) — MUST be counted as concrete. + +#### Scenario: Interface counted as abstract + +- **GIVEN** a package that declares `type Foo interface { Bar() }` as an exported type +- **WHEN** `Analyze` is called +- **THEN** the module's `AbstractTypes` MUST include this type in its count and `ExportedTypes` MUST include it as well + +#### Scenario: Struct counted as concrete + +- **GIVEN** a package that declares `type Baz struct { X int }` as an exported type +- **WHEN** `Analyze` is called +- **THEN** the module's `ExportedTypes` MUST include this type but `AbstractTypes` MUST NOT + +#### Scenario: Unexported types excluded + +- **GIVEN** a package that declares `type internal interface { foo() }` +- **WHEN** `Analyze` is called +- **THEN** neither `ExportedTypes` nor `AbstractTypes` MUST count this type + +#### Scenario: Type alias counted as concrete + +- **GIVEN** a package that declares `type Alias = SomeOtherType` as an exported type alias +- **WHEN** `Analyze` is called +- **THEN** `ExportedTypes` MUST include this type and `AbstractTypes` MUST NOT + +#### Scenario: Empty package (no type declarations) + +- **GIVEN** a package with no type declarations (only constants, variables, or functions) +- **WHEN** `Analyze` is called +- **THEN** the module's `ExportedTypes` MUST be 0 and `AbstractTypes` MUST be 0 + +### Requirement: Adapter computes derived metrics + +The adapter MUST use `metrics.ComputeInstability`, `metrics.ComputeAbstractness`, +`metrics.ComputeDistance`, and `metrics.ComputeZone` to populate computed fields on +each `ModuleResult`. The adapter MUST NOT reimplement these computations. + +#### Scenario: Derived metrics use canonical compute functions + +- **GIVEN** a module with Ca=3, Ce=2, ExportedTypes=10, AbstractTypes=2 +- **WHEN** `Analyze` is called +- **THEN** the `ModuleResult` MUST have Instability equal to `metrics.ComputeInstability(3, 2)`, Abstractness equal to `metrics.ComputeAbstractness(2, 10)`, Distance equal to `metrics.ComputeDistance(A, I)`, and Zone equal to `metrics.ComputeZone(A, I, D)` + +### Requirement: Adapter computes LCOM4 + +The adapter MUST compute LCOM4 (Hitz & Montazeri 1995) for each package. LCOM4 +MUST be calculated as the number of connected components in a graph where nodes are +exported methods (functions with a receiver of an exported type) and edges connect +methods that access at least one common struct field. + +#### Scenario: Fully cohesive package + +- **GIVEN** a package with a struct `type S struct { x int }` and three exported methods `func (s *S) A() { _ = s.x }`, `func (s *S) B() { _ = s.x }`, `func (s *S) C() { _ = s.x }` that all access field `x` +- **WHEN** `Analyze` is called +- **THEN** the module's LCOM MUST be 1 + +#### Scenario: Non-cohesive package + +- **GIVEN** a package with `type S struct { x, y, z, w int }` and four exported methods where `A()` and `B()` access fields `x, y` and `C()` and `D()` access fields `z, w` with no overlap +- **WHEN** `Analyze` is called +- **THEN** the module's LCOM MUST be 2 + +#### Scenario: Package with no exported methods + +- **GIVEN** a package that has no exported methods (only package-level functions or unexported methods) +- **WHEN** `Analyze` is called +- **THEN** the module's LCOM MUST be 0 + +#### Scenario: Package with only package-level functions (no receivers) + +- **GIVEN** a package that has exported functions but no functions with a receiver +- **WHEN** `Analyze` is called +- **THEN** the module's LCOM MUST be 0 (package-level functions are excluded from the LCOM graph) + +### Requirement: Adapter detects circular dependencies + +The adapter MUST detect circular dependencies in the package import graph using +Tarjan's strongly connected components algorithm. Each SCC with more than one package +MUST be reported as a `metrics.Cycle` — a deterministic, lexicographically-sorted set of +the member package paths (the ordering carries no traversal meaning). The slice of +cycles is itself sorted by first element. + +#### Scenario: No cycles in valid Go code + +- **GIVEN** a Go module with no import cycles +- **WHEN** `Analyze` is called +- **THEN** the `Cycles` field of `ModuleGraph` MUST be an empty slice (not nil) + +#### Scenario: Cycle detection in partial builds + +- **GIVEN** analyzed code with partial build errors that prevent full resolution +- **WHEN** `Analyze` is called +- **THEN** the adapter MUST still attempt cycle detection on the resolvable portion and set `Status` to `StatusPartial` + +### Requirement: Adapter produces schema-valid output + +The `ModuleGraph` returned by `Analyze` MUST pass `metrics.Validate()` when +serialized to JSON. `SchemaVersion` MUST be `"1.1"` (reflecting the extensions addition). +`Language` MUST be `"go"`. + +#### Scenario: Valid JSON output + +- **GIVEN** a valid Go module +- **WHEN** `Analyze` completes successfully +- **THEN** marshaling the returned `ModuleGraph` to JSON and passing it to `metrics.Validate` MUST return nil error + +#### Scenario: Complete status on success + +- **GIVEN** all packages load without errors +- **WHEN** `Analyze` is called +- **THEN** `Status` MUST be `metrics.StatusComplete` + +#### Scenario: Partial status on load errors + +- **GIVEN** some packages fail to load due to build errors +- **WHEN** `Analyze` is called +- **THEN** `Status` MUST be `metrics.StatusPartial` and `Warnings` MUST contain entries with: (1) the affected package's import path in `Module`, (2) a machine-readable `Code` (e.g., `"load-error"`), and (3) a `Message` containing the underlying error description using relative paths + +### Requirement: Adapter validates project path + +The adapter MUST validate the project path before analysis using +`metrics.ValidateProjectPath`. Invalid paths (empty, path traversal, non-existent, +non-directory) MUST result in an error returned from `Analyze`. + +#### Scenario: Path traversal rejected + +- **GIVEN** a path containing `..` components +- **WHEN** `Analyze` is called with that path +- **THEN** the adapter MUST return an error without loading any packages + +#### Scenario: Non-existent path rejected + +- **GIVEN** a path that does not exist +- **WHEN** `Analyze` is called with that path +- **THEN** the adapter MUST return an error + +### Requirement: Adapter uses sanitized environment + +The adapter MUST set `packages.Config.Env` to a sanitized environment using +`metrics.SanitizeEnvironment` to prevent credential leakage to subprocesses +spawned by `go/packages` during package loading. + +#### Scenario: Credential environment variables not leaked + +- **GIVEN** the process environment contains `GITHUB_TOKEN=secret` +- **WHEN** `Analyze` is called +- **THEN** the `packages.Config.Env` MUST NOT contain `GITHUB_TOKEN` + +### Requirement: Adapter returns deterministically ordered output + +The adapter MUST sort the `Modules` slice in the returned `ModuleGraph` by +`Module.Path` in lexicographic order to ensure deterministic output across runs. + +#### Scenario: Module ordering is deterministic + +- **GIVEN** a Go module with packages A, B, C +- **WHEN** `Analyze` is called +- **THEN** the `Modules` slice MUST be ordered by `Path` lexicographically + +### Requirement: Adapter propagates context for cancellation + +The adapter MUST propagate the provided `context.Context` to `packages.Config.Context`. +When the context is cancelled or its deadline is exceeded, `Analyze` MUST return an +error wrapping the context error. Partial results MUST NOT be returned on cancellation. + +#### Scenario: Context cancellation during analysis + +- **GIVEN** a Go module with a non-trivial number of packages +- **WHEN** `Analyze` is called and the context is cancelled during `packages.Load` +- **THEN** the adapter MUST return an error wrapping `context.Canceled` + +#### Scenario: Context deadline exceeded + +- **GIVEN** a Go module +- **WHEN** `Analyze` is called with a context that has a deadline, and the deadline is exceeded +- **THEN** the adapter MUST return an error wrapping `context.DeadlineExceeded` + +### Requirement: Adapter handles total load failure + +When `packages.Load` returns zero packages, or returns packages where every package has +load/type errors OR nil type information (i.e., none can be type-checked), the adapter +MUST return an appropriate error rather than an empty or all-zeroed `ModuleGraph`. +In a partial build — where at least one package type-checks — individual packages that +have nil type information (`pkg.Types == nil`) MUST instead get ExportedTypes=0, +AbstractTypes=0, LCOM=0, and a warning. Note: when a single package with nil type +information is the only package, this yields the total-load-failure error rather than a +single zeroed-module graph. + +#### Scenario: No Go files in target directory + +- **GIVEN** a directory that exists but contains no Go files +- **WHEN** `Analyze` is called with that directory +- **THEN** the adapter MUST return an error + +#### Scenario: Package with nil type information + +- **GIVEN** a multi-package module in which one package fails to type-check (e.g., missing dependency) while at least one other package type-checks +- **WHEN** `Analyze` is called +- **THEN** the adapter MUST set ExportedTypes=0, AbstractTypes=0, LCOM=0 for the failing package, and add a warning with the package path + +### Requirement: Adapter populates Go-specific extensions + +The adapter MUST populate the `Extensions` field on each `ModuleResult` with +Go-specific metrics namespaced under the `go.` prefix. Extensions are language-specific +and do not modify the universal model. The adapter MUST declare the extension +capabilities as package-level constants — `CapInterfaceWidth` (`"go.interfaceWidth"`) and +`CapInterfaceProximity` (`"go.interfaceProximity"`) — and expose them via the +`ExtensionCapabilities() []string` accessor, separately from the universal +`Capabilities()` method. These constants live in the adapter package, not in the +universal `metrics` package. + +#### Scenario: Extensions present in output + +- **GIVEN** a Go module with packages that contain exported interfaces +- **WHEN** `Analyze` is called +- **THEN** each `ModuleResult` for packages with exported interfaces MUST have an `Extensions` map containing `go.interfaceWidth` and `go.interfaceProximity` keys + +#### Scenario: No extensions for packages without interfaces + +- **GIVEN** a package with no exported interfaces +- **WHEN** `Analyze` is called +- **THEN** the `Extensions` field for that module MAY be nil or omitted (no `go.interfaceWidth` or `go.interfaceProximity` keys) + +### Requirement: Interface Width (Pike metric) + +The adapter MUST compute interface width for all exported interfaces in each analyzed +package. Interface width is the method count of the interface. The result MUST be +stored in `Extensions["go.interfaceWidth"]` as a `map[string]int` mapping interface +name to method count. + +#### Scenario: Single-method interface + +- **GIVEN** a package with `type Reader interface { Read(p []byte) (n int, err error) }` +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceWidth"]` MUST contain `{"Reader": 1}` + +#### Scenario: Multi-method interface + +- **GIVEN** a package with `type ReadWriter interface { Read(p []byte) (n int, err error); Write(p []byte) (n int, err error) }` +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceWidth"]` MUST contain `{"ReadWriter": 2}` + +#### Scenario: Embedded interface method counting + +- **GIVEN** a package with `type Closer interface { Close() error }` and `type ReadCloser interface { Reader; Closer }` +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceWidth"]` MUST count the total flattened method set (e.g., `{"Closer": 1, "ReadCloser": 2}`) + +#### Scenario: No exported interfaces + +- **GIVEN** a package with no exported interfaces (only structs, functions, etc.) +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceWidth"]` MUST NOT be present (or the `Extensions` map itself may be nil) + +### Requirement: Interface-to-Implementation Proximity + +The adapter MUST compute interface proximity for all exported interfaces in each +analyzed package. Proximity indicates whether the interface is declared on the +consumer side (`"consumer"`) or producer side (`"producer"`). An interface is +`"producer"` if any type in the same package implements it; otherwise it is +`"consumer"`. The result MUST be stored in `Extensions["go.interfaceProximity"]` +as a `map[string]string`. + +#### Scenario: Producer-side interface + +- **GIVEN** a package with `type Saver interface { Save() error }` and `type FileSaver struct{}` with `func (f *FileSaver) Save() error { ... }` in the same package +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceProximity"]` MUST contain `{"Saver": "producer"}` + +#### Scenario: Consumer-side interface + +- **GIVEN** a package with `type Logger interface { Log(msg string) }` where no type in the same package implements `Logger` +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceProximity"]` MUST contain `{"Logger": "consumer"}` + +### Requirement: Typed extension accessors for JSON round-trip safety + +The adapter package MUST provide typed accessor functions for extracting extension +values after JSON round-trip. JSON unmarshaling converts `int` to `float64` and +`map[string]int` to `map[string]interface{}`. Consumers MUST NOT need unchecked +type assertions. + +#### Scenario: Interface widths extracted after JSON round-trip + +- **GIVEN** a `ModuleResult` with `Extensions["go.interfaceWidth"]` populated as `map[string]int` +- **WHEN** the `ModuleResult` is marshaled to JSON and unmarshaled back +- **THEN** calling the typed accessor function (e.g., `InterfaceWidths(extensions)`) MUST return the correct `map[string]int` values without panic + +#### Scenario: Interface proximity extracted after JSON round-trip + +- **GIVEN** a `ModuleResult` with `Extensions["go.interfaceProximity"]` populated as `map[string]string` +- **WHEN** the `ModuleResult` is marshaled to JSON and unmarshaled back +- **THEN** calling the typed accessor function (e.g., `InterfaceProximities(extensions)`) MUST return the correct `map[string]string` values without panic + +#### Scenario: Accessor returns error on missing extension key + +- **GIVEN** a `ModuleResult` with nil or empty `Extensions` +- **WHEN** the typed accessor function is called +- **THEN** it MUST return a zero-value map and an error (not panic) + +#### Scenario: Mixed proximity + +- **GIVEN** a package with two exported interfaces where one has an in-package implementation and the other does not +- **WHEN** `Analyze` is called +- **THEN** `Extensions["go.interfaceProximity"]` MUST contain entries with mixed `"producer"` and `"consumer"` values diff --git a/openspec/changes/go-analyze/tasks.md b/openspec/changes/go-analyze/tasks.md new file mode 100644 index 0000000..35a03e9 --- /dev/null +++ b/openspec/changes/go-analyze/tasks.md @@ -0,0 +1,86 @@ +## 1. Project Setup + +- [x] 1.1 Add dependencies: `golang.org/x/tools` and `github.com/spf13/cobra` to `go.mod` +- [x] 1.2 Create package scaffolding: `internal/goadapter/doc.go`, `cmd/vibe-check/main.go` with package-level GoDoc +- [x] 1.3 Create `testdata/` fixture modules: (a) multi-package module with known import graph for Ca/Ce testing, (b) packages with specific type mixes (interfaces, structs, aliases, empty) for abstractness testing, (c) packages with specific method-field patterns for LCOM testing, (d) packages with exported interfaces of varying widths and consumer/producer proximity for extensions testing, (e) a fixture module with intentional compilation errors (missing import, syntax error in one package) for partial-build testing, (f) an empty directory (no `.go` files) for total-failure testing. Each fixture must have a valid `go.mod` and produce deterministic metric values. + +## 2. Go Adapter — Dependency Resolution + +- [x] 2.1 Implement `internal/goadapter/adapter.go`: `Adapter` struct with `Language()`, `Capabilities()`, and `Analyze()` method skeleton that validates project path via `metrics.ValidateProjectPath` and propagates `context.Context` to `packages.Config.Context` +- [x] 2.2 Implement `internal/goadapter/resolve.go`: load packages via `packages.Load` with `NeedName|NeedImports|NeedTypes|NeedSyntax|NeedTypesInfo|NeedModule`, filter to module-internal packages, build import adjacency map. Set `packages.Config.Env` to `metrics.SanitizeEnvironment(packageEnvAllowlist)` — an explicit allowlist (`GOPATH`, `GOROOT`, `GOMODCACHE`, `GOPROXY`, `GONOSUMCHECK`, `GOMOD`) that deliberately excludes credential-bearing and injection-prone variables such as `GOFLAGS` — to prevent credential leakage. +- [x] 2.3 Compute Ca/Ce: Ce = `len(pkg.Imports)` (all imports, including standard library and third-party, per Martin's definition); Ca = number of module-internal packages that import this package (only Ca uses the internal import adjacency map) +- [x] 2.4 Write tests for dependency resolution: use `testdata/` fixture module with known import graph, verify Ca/Ce values, verify stdlib/external exclusion from module list + +## 3. Go Adapter — Type Analysis + +- [x] 3.1 Implement `internal/goadapter/types.go`: walk AST to count exported type declarations per package, classify interfaces as abstract. Handle edge cases: type aliases (concrete), empty packages (ExportedTypes=0), packages with nil type info (ExportedTypes=0, add warning) +- [x] 3.2 Write tests for type counting: packages with mixed exported/unexported types, interfaces vs structs, type aliases, empty packages (no type declarations) + +## 4. Go Adapter — LCOM4 Cohesion + +- [x] 4.1 Implement `internal/goadapter/lcom.go`: build method-field access graph for exported methods, compute connected components via union-find, return LCOM4 value. Package-level functions (no receiver) are excluded from the graph. +- [x] 4.2 Write tests for LCOM4: (a) fully cohesive package — struct with 3 methods all accessing same field (LCOM=1), (b) non-cohesive package — struct with 4 methods split into 2 groups accessing disjoint fields (LCOM=2), (c) package with no exported methods (LCOM=0), (d) package with only package-level functions (LCOM=0). Use concrete Go source in `testdata/` fixtures. + +## 5. Go Adapter — Cycle Detection + +- [x] 5.1 Implement `internal/goadapter/cycles.go`: Tarjan's SCC on the package import graph, convert SCCs with >1 node to `metrics.Cycle` with canonical ordering +- [x] 5.2 Write tests for cycle detection: (a) acyclic graph produces empty cycles slice (not nil), (b) test Tarjan's algorithm directly with constructed adjacency maps since Go import cycles are compile errors, (c) verify canonical ordering of cycle output + +## 5a. Metrics Model — Extensions Mechanism + +- [x] 5a.1 Add `Extensions map[string]any` field to `ModuleResult` in `metrics/graph.go` with `json:"extensions,omitempty"` tag and GoDoc comment explaining language-specific extensions namespaced by language +- [x] 5a.2 Update `metrics/modulegraph.schema.json`: add `"extensions": { "type": "object" }` to the module item's `properties` list while keeping `additionalProperties: false`. Bump `SchemaVersion` from `"1.0"` to `"1.1"` in `metrics/graph.go`. +- [x] 5a.3 Update `metrics/validate.go`: accept the `extensions` field as a valid JSON object on module items. If `extensions` is present, verify it is a JSON object (not a primitive or array). Accept both `SchemaVersion` `"1.0"` (no extensions) and `"1.1"` (with extensions) for backward compatibility. +- [x] 5a.4 Write tests for extensions: verify `ModuleResult` with extensions marshals/unmarshals correctly (including JSON round-trip type fidelity — `int` becomes `float64`), verify `metrics.Validate` passes with extensions present, verify `metrics.Validate` passes with extensions absent (omitempty), verify `metrics.Validate` rejects non-object extensions (e.g., string, array), verify existing tests still pass + +## 5b. Go Adapter — Extensions (Interface Width and Proximity) + +- [x] 5b.1 Implement `internal/goadapter/extensions.go`: compute interface width (method count per exported interface, including flattened embedded methods) and interface proximity (consumer vs. producer based on same-package implementation check). Populate `go.interfaceWidth` (map[string]int) and `go.interfaceProximity` (map[string]string) in `ModuleResult.Extensions`. Provide typed accessor functions (`InterfaceWidths(extensions map[string]any) (map[string]int, error)` and `InterfaceProximities(extensions map[string]any) (map[string]string, error)`) for safe extraction after JSON round-trip. +- [x] 5b.2 Write tests for extensions: (a) single-method interface (width=1), (b) multi-method interface (width=2), (c) embedded interface flattening, (d) no exported interfaces (no extensions), (e) producer-side interface (implementation in same package), (f) consumer-side interface (no implementation in same package), (g) mixed proximity, (h) typed accessor round-trip test (marshal → unmarshal → accessor → verify values), (i) accessor returns error on missing/nil extensions. Use `testdata/` fixtures. + +## 6. Go Adapter — Assembly and Integration + +- [x] 6.1 Complete `Analyze()`: wire resolve → types → LCOM → cycles → extensions → compute derived metrics via `metrics.Compute*` → assemble `ModuleGraph` with SchemaVersion `"1.1"`, Language, Status, Warnings. Sort `Modules` by `Module.Path` lexicographically for deterministic output. Populate `Extensions` on each `ModuleResult` via the extensions module. Return error on context cancellation. Handle total load failure (zero packages = error). Use relative paths in warning messages. +- [x] 6.2 Handle partial builds: check `packages.Package.Errors`, handle nil `pkg.Types` (set ExportedTypes=0, AbstractTypes=0, LCOM=0, add warning), set `StatusPartial` with warnings for failed packages +- [x] 6.3 Write integration test: analyze a dedicated `testdata/` fixture module with known structure, assert specific metric values, verify output passes `metrics.Validate()`. Guard with `if testing.Short() { t.Skip() }`. +- [x] 6.4 Write determinism test: call `Analyze()` on the same `testdata/` module 10 times, assert JSON output is byte-identical across runs +- [x] 6.5 Write context cancellation test: call `Analyze()` with an already-cancelled context, verify error wraps `context.Canceled` +- [x] 6.6 Write path validation tests: path traversal rejection (`../foo`), non-existent path, empty path — each MUST return error without loading packages +- [x] 6.7 Write context deadline test: call `Analyze()` with a context that has an expired deadline, verify error wraps `context.DeadlineExceeded` +- [x] 6.8 Write error-path tests: total load failure returns error (empty dir fixture), nil type info produces zero metrics with warning (partial-build fixture), partial build sets `StatusPartial` with warning containing package path and error description + +## 7. CLI — Analyze Command + +- [x] 7.1 Implement `cmd/vibe-check/main.go`: cobra root command with version info (version, commit, date embedded via ldflags), `--version` flag +- [x] 7.2 Implement `cmd/vibe-check/analyze.go`: `RunAnalyze(opts AnalyzeOptions) (*AnalyzeResult, error)` function per AP-002/AP-003 pattern. AnalyzeOptions includes `io.Writer` fields for stdout/stderr. Cobra `RunE` delegates to `RunAnalyze`. Create Go adapter, invoke `Analyze`, marshal result as indented JSON to stdout. +- [x] 7.3 Add threshold flags: `--max-instability`, `--max-distance`, `--no-circular-deps`, `--max-lcom` with validation logic. Reject `--max-instability` and `--max-distance` outside [0.0, 1.0]. Reject `--max-lcom` < 1. Invalid flags exit with status 2. Threshold comparison uses strict `>` (boundary value passes). Add `--timeout ` flag — create `context.WithTimeout` when set, default no timeout. +- [x] 7.4 Implement threshold checking and signal handling: create context with `signal.NotifyContext` for SIGINT/SIGTERM, wrap with `context.WithTimeout` if `--timeout` is set. Iterate `ModuleGraph` results, collect ALL violations, print to stderr, exit 1 if any violations, always emit JSON to stdout. Use exit code 2 for tool errors (invalid args, adapter failure, timeout, signal). If signal received before JSON output, suppress partial JSON. +- [x] 7.5 Write CLI tests — flag parsing and help: test `--help` output lists analyze command, test `--version` output, test flag parsing for all threshold flags +- [x] 7.6 Write CLI tests — threshold violations: test each threshold flag individually (pass/fail), test multiple simultaneous violations reported, test JSON still emitted on violations. Use `bytes.Buffer` for stdout/stderr per AP-003. +- [x] 7.7 Write CLI tests — flag validation and exit codes: test invalid flag values (out of range, negative), test exit code 1 vs 2 distinction, test adapter error produces exit code 2, test `--timeout` creates deadline context, test boundary-value scenarios (metric exactly at threshold passes) +- [x] 7.8 Write CLI tests — JSON output validity: test stdout output passes `metrics.Validate`, test pretty-printed (indented) output + +## 8. Validation and CI + +- [x] 8.1 Create `.github/workflows/ci.yml` per CI convention pack: + - Trigger: push to `main` and PRs targeting `main` (CI-014) + - Go version: match `go.mod` (currently 1.25.7) + - Steps: `go build ./...`, `go test -race -count=1 -coverprofile=coverage.out ./...`, `go vet ./...`, `golangci-lint run ./...` + - Pin all actions by 40-character commit SHA (CI-001/CI-002): `actions/checkout`, `actions/setup-go`, `golangci/golangci-lint-action` + - Add concurrency group per CI-012 to cancel stale runs + - Set `permissions: contents: read` per CI-020/CI-021 (least privilege) + - Descriptive workflow name per CI-010/CI-011 + - Add header comment with workflow purpose per CI-031 +- [x] 8.2 Verify `go build ./...` passes +- [x] 8.3 Verify `go test -race -count=1 ./...` passes +- [x] 8.4 Verify `go vet ./...` passes +- [x] 8.5 Run `golangci-lint run ./...` if configured, fix any findings +- [x] 8.6 Verify all exported symbols have GoDoc comments + +## 9. Documentation + +- [x] 9.1 Update AGENTS.md: add `internal/goadapter/` and `cmd/vibe-check/` to Project Structure section, update Architecture section to reflect Go adapter implementation and extensions mechanism, add `go build ./cmd/vibe-check` to Build & Test Commands +- [x] 9.2 Add CHANGELOG.md entry for the go-analyze change + + +