Skip to content

Commit 448f68a

Browse files
committed
fix: eliminate testOnlyExports, teach dead-export scanner cross-package test usage
Remove the testOnlyExports allowlist entirely. Instead, add Phase 2.5 to TestNoDeadExports that loads packages with Tests: true and removes symbols used cross-package in test files. A symbol imported by a test in a different package is test infrastructure, not dead. Also fix parser.findParser → parser.find (stuttery name) and move to tools.go (mixed visibility). Spec: specs/ast-audit-contributor-guide.md Signed-off-by: Jose Alekhinne <jose@ctx.ist>
1 parent 03ae63a commit 448f68a

6 files changed

Lines changed: 69 additions & 47 deletions

File tree

internal/audit/dead_exports_test.go

Lines changed: 48 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,29 +26,6 @@ import (
2626
// internal and may be used via reflection or are
2727
// genuinely file-scoped helpers.
2828

29-
// DO NOT add entries here to make tests pass. New code must
30-
// conform to the check. Widening requires a dedicated PR with
31-
// justification for each entry.
32-
//
33-
// testOnlyExports lists exported symbols that exist
34-
// solely for test usage. The dead-export scanner skips
35-
// test files, so these would otherwise be false
36-
// positives. Keep this list small: prefer eliminating
37-
// the export over adding it here.
38-
var testOnlyExports = map[string]bool{
39-
"github.com/ActiveMemory/ctx/internal/config/hook.CategoryCustomizable": true,
40-
"github.com/ActiveMemory/ctx/internal/assets/hooks/messages.Hooks": true,
41-
"github.com/ActiveMemory/ctx/internal/cli/pad/core/store.EnsureGitignore": true,
42-
"github.com/ActiveMemory/ctx/internal/cli/system/core/state.SetDirForTest": true,
43-
"github.com/ActiveMemory/ctx/internal/config/asset.DirReferences": true,
44-
"github.com/ActiveMemory/ctx/internal/config/regex.Phase": true,
45-
"github.com/ActiveMemory/ctx/internal/inspect.StartsWithCtxMarker": true,
46-
"github.com/ActiveMemory/ctx/internal/journal/parser.Parser": true,
47-
"github.com/ActiveMemory/ctx/internal/mcp/proto.InitializeParams": true,
48-
"github.com/ActiveMemory/ctx/internal/mcp/proto.UnsubscribeParams": true,
49-
"github.com/ActiveMemory/ctx/internal/rc.Reset": true,
50-
}
51-
5229
// DO NOT add entries here to make tests pass. New code must
5330
// conform to the check. Widening requires a dedicated PR with
5431
// justification for each entry.
@@ -148,9 +125,30 @@ func TestNoDeadExports(t *testing.T) {
148125
}
149126
}
150127

151-
// Phase 3: remove test-only allowlist entries.
152-
for key := range testOnlyExports {
153-
delete(defs, key)
128+
// Phase 2.5: remove symbols used cross-package in
129+
// test files. If a test in package B imports a
130+
// symbol from package A, the symbol is test
131+
// infrastructure — not dead. Same-package test
132+
// usage does not count (those should be unexported).
133+
testPkgs := loadTestPackages(t)
134+
for _, pkg := range testPkgs {
135+
for ident, obj := range pkg.TypesInfo.Uses {
136+
if obj == nil || obj.Pkg() == nil {
137+
continue
138+
}
139+
pos := pkg.Fset.Position(ident.Pos())
140+
if !isTestFile(pos.Filename) {
141+
continue
142+
}
143+
// Cross-package: the test's package path
144+
// differs from the symbol's defining package.
145+
if pkg.PkgPath == obj.Pkg().Path() {
146+
continue
147+
}
148+
key := obj.Pkg().Path() + "." +
149+
obj.Name()
150+
delete(defs, key)
151+
}
154152
}
155153

156154
// Phase 3b: remove Linux-only exports (used from
@@ -212,6 +210,30 @@ func loadCmdPackages(t *testing.T) []*packages.Package {
212210
return pkgs
213211
}
214212

213+
// loadTestPackages loads internal/ packages WITH test
214+
// files for cross-package test usage detection.
215+
func loadTestPackages(
216+
t *testing.T,
217+
) []*packages.Package {
218+
t.Helper()
219+
cfg := &packages.Config{
220+
Mode: packages.NeedName |
221+
packages.NeedFiles |
222+
packages.NeedSyntax |
223+
packages.NeedTypes |
224+
packages.NeedTypesInfo,
225+
Tests: true,
226+
}
227+
pkgs, loadErr := packages.Load(
228+
cfg,
229+
"github.com/ActiveMemory/ctx/internal/...",
230+
)
231+
if loadErr != nil {
232+
t.Fatalf("packages.Load tests: %v", loadErr)
233+
}
234+
return pkgs
235+
}
236+
215237
// isExported reports whether name starts with an
216238
// uppercase letter.
217239
func isExported(name string) bool {

internal/journal/parser/markdown_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ func TestRegisteredTools_IncludesMarkdown(t *testing.T) {
422422
}
423423

424424
func TestGetParser_Markdown(t *testing.T) {
425-
p := Parser(session.ToolMarkdown)
425+
p := find(session.ToolMarkdown)
426426
if p == nil {
427427
t.Fatalf("expected parser for %q", session.ToolMarkdown)
428428
}

internal/journal/parser/parser.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -194,19 +194,3 @@ func FindSessionsForCWD(
194194
return s.CWD == cwd
195195
}, additionalDirs...)
196196
}
197-
198-
// Parser returns a parser for the specified tool.
199-
//
200-
// Parameters:
201-
// - tool: Tool identifier (e.g., "claude-code")
202-
//
203-
// Returns:
204-
// - Session: The parser for the tool, or nil if not found
205-
func Parser(tool string) Session {
206-
for _, p := range registeredParsers {
207-
if p.Tool() == tool {
208-
return p
209-
}
210-
}
211-
return nil
212-
}

internal/journal/parser/parser_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,15 +340,15 @@ func TestRegisteredTools(t *testing.T) {
340340
}
341341

342342
func TestGetParser(t *testing.T) {
343-
parser := Parser("claude-code")
343+
parser := find("claude-code")
344344
if parser == nil {
345345
t.Error("expected parser for 'claude-code'")
346346
}
347347
if parser.Tool() != "claude-code" {
348348
t.Errorf("expected tool 'claude-code', got '%s'", parser.Tool())
349349
}
350350

351-
unknown := Parser("unknown-tool")
351+
unknown := find("unknown-tool")
352352
if unknown != nil {
353353
t.Error("expected nil for unknown tool")
354354
}

internal/journal/parser/tools.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@
66

77
package parser
88

9+
// find returns a parser for the specified tool.
10+
//
11+
// Parameters:
12+
// - tool: Tool identifier (e.g., "claude-code")
13+
//
14+
// Returns:
15+
// - Session: The parser, or nil if not found
16+
func find(tool string) Session {
17+
for _, p := range registeredParsers {
18+
if p.Tool() == tool {
19+
return p
20+
}
21+
}
22+
return nil
23+
}
24+
925
// registeredTools returns the list of supported tools.
1026
//
1127
// Returns:

internal/rc/rc.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,8 @@ func OverrideContextDir(ctxDir string) {
441441
rcOverrideDir = ctxDir
442442
}
443443

444-
// Reset clears the cached configuration, forcing reload on the next access.
445-
// This is primarily useful for testing.
444+
// Reset clears the cached configuration, forcing
445+
// reload on the next access.
446446
func Reset() {
447447
rcMu.Lock()
448448
defer rcMu.Unlock()

0 commit comments

Comments
 (0)