diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..0e707de --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,86 @@ +# Used as inspiration: https://github.com/caddyserver/caddy/blob/master/.github/workflows/ci.yml + +name: Tests + +on: + push: + branches: + - master + pull_request: + branches: + - master + +env: + # https://github.com/actions/setup-go/issues/491 + GOTOOLCHAIN: local + +permissions: + contents: read + +jobs: + test: + strategy: + # Default is true, cancels jobs for other platforms in the matrix if one fails + fail-fast: false + matrix: + os: + - linux + go: + - '1.27' + + include: + # Set the minimum Go patch version for the given Go minor + # Usable via ${{ matrix.GO_SEMVER }} + - go: '1.27' + GO_SEMVER: '~1.27.0' + + # Set some variables per OS, usable via ${{ matrix.VAR }} + # OS_LABEL: the VM label from GitHub Actions + - os: linux + OS_LABEL: ubuntu-latest + + runs-on: ${{ matrix.OS_LABEL }} + permissions: + contents: read + pull-requests: read + + steps: + - name: Harden the runner (Audit all outbound calls) + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.4 + with: + egress-policy: audit + + - name: Checkout code + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Install Go + uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 + with: + go-version: ${{ matrix.GO_SEMVER }} + check-latest: true + + - name: Print Go version and environment + shell: bash + run: | + printf "Using go at: $(which go)\n" + printf "Go version: $(go version)\n" + printf "\n\nGo environment:\n\n" + go env + printf "\n\nSystem environment:\n\n" + env + printf "Git version: $(git version)\n\n" + + - name: Get dependencies + run: | + go get -v -t -d ./... + + - name: Run vet + run: | + go vet ./... + + # The test suite loads packages with the Go toolchain and sets + # CGO_ENABLED=0 itself where needed; the race detector requires + # cgo, so CGO_ENABLED is not forced off here. + - name: Run tests + run: | + go test -short -race ./... diff --git a/baseline_behavior_test.go b/baseline_behavior_test.go new file mode 100644 index 0000000..2bfc03f --- /dev/null +++ b/baseline_behavior_test.go @@ -0,0 +1,46 @@ +package moduledoc + +import ( + "os" + "testing" + + "golang.org/x/tools/go/packages" +) + +// TestOriginalBehaviorBaseline establishes baseline behavior for the original implementation +func TestOriginalBehaviorBaseline(t *testing.T) { + t.Run("ValidStaticModule", func(t *testing.T) { + // Test only the gizmo.go file which should work + cfg := &packages.Config{ + Dir: ".", + Mode: packagesLoadMode, + Env: append(os.Environ(), "CGO_ENABLED=0"), + } + + pkgs, err := packages.Load(cfg, "./testdata") + if err != nil { + t.Fatalf("loading testdata package: %v", err) + } + if len(pkgs) == 0 { + t.Fatal("no packages loaded") + } + + driver := New(nil) + pkg := pkgs[0] + + moduleIdents, err := driver.findCaddyModuleIdents(pkg) + if err != nil { + t.Fatalf("finding module idents: %v", err) + } + + found := false + for _, moduleID := range moduleIdents { + if moduleID == "app.namespace.gizmo" { + found = true + } + } + if !found { + t.Errorf("expected module 'app.namespace.gizmo', got: %v", moduleIdents) + } + }) +} diff --git a/concurrency_baseline_test.go b/concurrency_baseline_test.go new file mode 100644 index 0000000..751625b --- /dev/null +++ b/concurrency_baseline_test.go @@ -0,0 +1,28 @@ +package moduledoc + +import ( + "testing" +) + +// TestMemoryLeakDemo demonstrates the unbounded cache growth +func TestMemoryLeakDemo(t *testing.T) { + t.Run("UnboundedCacheGrowth", func(t *testing.T) { + driver := New(nil) + + // Simulate what happens with many package versions + for i := 0; i < 1000; i++ { + for version := 0; version < 5; version++ { + key := "test.package." + string(rune('A'+i)) + "@v1." + string(rune('0'+version)) + ".0" + driver.setDiscoveredType(key, &Value{ + Type: String, + TypeName: key, + }) + } + } + + // no size limit, TTL, or eviction: every entry is retained + if len(driver.discoveredTypes) != 5000 { + t.Errorf("expected 5000 retained entries, got %d", len(driver.discoveredTypes)) + } + }) +} diff --git a/concurrency_edge_test.go b/concurrency_edge_test.go new file mode 100644 index 0000000..c58c393 --- /dev/null +++ b/concurrency_edge_test.go @@ -0,0 +1,163 @@ +package moduledoc + +import ( + "os" + "os/exec" + "sync" + "testing" + + "golang.org/x/tools/go/packages" +) + +// localWorkspace returns a workspace anchored at the repository root so +// the local testdata package can be loaded without running 'go get'. +func localWorkspace(t *testing.T, d *Driver) workspace { + t.Helper() + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("getting working directory: %v", err) + } + return workspace{ + mu: new(sync.RWMutex), + dir: cwd, + driver: d, + goGets: map[string]struct{}{"github.com/caddyserver/moduledoc": {}}, + packagePatterns: make(map[string][]string), + parsedPackages: make(map[string]*packages.Package), + } +} + +const testdataPackagePath = "github.com/caddyserver/moduledoc/testdata" + +// runIsolated re-runs the named test in a child process with childEnv set, +// so fatal runtime errors or race reports cannot take down the whole suite. +func runIsolated(t *testing.T, testName, childEnv, failureMsg string) { + t.Helper() + cmd := exec.Command(os.Args[0], "-test.run", "^"+testName+"$", "-test.v") + cmd.Env = append(os.Environ(), childEnv+"=1") + out, err := cmd.CombinedOutput() + if err != nil { + if len(out) > 4000 { + out = out[:4000] + } + t.Fatalf("%s: %v\n%s", failureMsg, err, out) + } +} + +func TestConcurrentDiscoveredTypesAccess(t *testing.T) { + if os.Getenv("MODULEDOC_TEST_DISCOVERED_TYPES_CHILD") == "1" { + db := newMemStorage() + d := New(db) + ws := localWorkspace(t, d) + + pkgs, err := ws.getPackages(testdataPackagePath, "") + if err != nil { + t.Fatalf("loading testdata package: %v", err) + } + obj := pkgs[0].Types.Scope().Lookup("Gizmo") + if obj == nil { + t.Fatal("Gizmo type not found in testdata package") + } + + // pre-store the type so every build takes the db-hit path, + // which writes to the driver's type cache on each call + db.StoreType(testdataPackagePath, "Gizmo", "", &Value{ + Type: Struct, + TypeName: testdataPackagePath + ".Gizmo", + }) + + start := make(chan struct{}) + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + rb := ws.representationBuilder() + if _, err := rb.buildRepresentation(obj.Type()); err != nil { + t.Errorf("building representation: %v", err) + } + }() + } + close(start) + wg.Wait() + return + } + + if testing.Short() { + t.Skip("requires the Go toolchain") + } + runIsolated(t, "TestConcurrentDiscoveredTypesAccess", "MODULEDOC_TEST_DISCOVERED_TYPES_CHILD", + "concurrent representation building on a shared Driver must be safe") +} + +func TestConcurrentModuleTypeLoading(t *testing.T) { + if os.Getenv("MODULEDOC_TEST_MODULE_LOADING_CHILD") == "1" { + db := newMemStorage() + db.types[storageKey("example.com/pkg", "Handler", "")] = &Value{ + Type: Struct, + TypeName: "example.com/pkg.Handler", + Doc: "handler doc", + StructFields: []*StructField{ + {Key: "root", Value: &Value{Type: String}, Doc: "root doc"}, + }, + } + db.modules["http.handlers.x"] = []*Value{ + {SameAs: "example.com/pkg.Handler", Doc: "usage doc"}, + } + d := New(db) + + start := make(chan struct{}) + var wg sync.WaitGroup + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + if _, err := d.LoadTypesByModuleID("http.handlers.x"); err != nil { + t.Errorf("loading module types: %v", err) + } + }() + } + close(start) + wg.Wait() + return + } + + runIsolated(t, "TestConcurrentModuleTypeLoading", "MODULEDOC_TEST_MODULE_LOADING_CHILD", + "concurrent module type loading on a shared Driver must be safe") +} + +func TestConcurrentGetPackages(t *testing.T) { + if os.Getenv("MODULEDOC_TEST_GET_PACKAGES_CHILD") == "1" { + d := New(newMemStorage()) + ws := localWorkspace(t, d) + + start := make(chan struct{}) + var wg sync.WaitGroup + for i := 0; i < 4; i++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + pkgs, err := ws.getPackages(testdataPackagePath, "") + if err != nil { + t.Errorf("loading packages: %v", err) + return + } + if len(pkgs) != 1 { + t.Errorf("expected 1 package, got %d", len(pkgs)) + } + }() + } + close(start) + wg.Wait() + return + } + + if testing.Short() { + t.Skip("requires the Go toolchain") + } + runIsolated(t, "TestConcurrentGetPackages", "MODULEDOC_TEST_GET_PACKAGES_CHILD", + "concurrent package loading on a shared workspace must be safe") +} diff --git a/core_functionality_test.go b/core_functionality_test.go new file mode 100644 index 0000000..d0fbbb4 --- /dev/null +++ b/core_functionality_test.go @@ -0,0 +1,323 @@ +package moduledoc + +import ( + "go/types" + "testing" + + "golang.org/x/tools/go/packages" +) + +// TestSynthesisOriginalBehavior tests the core synthesis.go functionality +func TestSynthesisOriginalBehavior(t *testing.T) { + t.Run("BuildRepresentationBasicTypes", func(t *testing.T) { + // Test that basic Go types are handled correctly + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Test basic types that should work + basicTypes := map[string]types.BasicKind{ + "bool": types.Bool, + "int": types.Int, + "string": types.String, + "float64": types.Float64, + "complex128": types.Complex128, + } + + for typeName, kind := range basicTypes { + basicType := types.Typ[kind] + rep, err := rb.buildRepresentation(basicType) + if err != nil { + t.Errorf("Failed to build representation for %s: %v", typeName, err) + continue + } + + if rep == nil { + t.Errorf("Got nil representation for %s", typeName) + continue + } + + t.Logf("✓ Successfully built representation for %s: %v", typeName, rep.Type) + } + }) + + t.Run("BuildRepresentationInterfaces", func(t *testing.T) { + // Test interface handling - original returns empty Value + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Create a simple interface type + interfaceType := types.NewInterfaceType(nil, nil) + rep, err := rb.buildRepresentation(interfaceType) + if err != nil { + t.Errorf("Interface handling should not error, but got: %v", err) + } + + // Original implementation returns new(Value) for interfaces + if rep == nil { + t.Error("Expected non-nil representation for interface") + } else { + t.Logf("✓ Interface representation: %+v", rep) + } + }) + + t.Run("BuildRepresentationUnknownType", func(t *testing.T) { + // Test the "unknown type" error case that causes hard failures + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Create a channel type (not handled by original implementation) + chanType := types.NewChan(types.SendRecv, types.Typ[types.String]) + rep, err := rb.buildRepresentation(chanType) + + // types with no JSON representation get an empty fallback value + if err != nil { + t.Errorf("channel type should fall back gracefully, got error: %v", err) + } else if rep == nil { + t.Error("expected non-nil fallback representation for channel type") + } else { + t.Logf("✓ Graceful fallback for channel type: %+v", rep) + } + }) + + t.Run("GetDepVersionCaching", func(t *testing.T) { + // Test the version caching mechanism in getDepVersion + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Verify the version cache is initialized + if rb.versionCache == nil { + t.Error("Version cache not initialized in representationBuilder") + } + + // hierarchical lookup: a type in a subpackage resolves to its cached + // parent module version without invoking go list + rb.versionCache["example.com/mod"] = "v1.2.3" + pkg := types.NewPackage("example.com/mod/sub/pkg", "pkg") + named := types.NewNamed( + types.NewTypeName(0, pkg, "Thing", nil), + types.Typ[types.String], nil, + ) + + version, err := rb.getDepVersion(named) + if err != nil { + t.Fatalf("getDepVersion failed: %v", err) + } + if version != "v1.2.3" { + t.Errorf("expected cached parent version v1.2.3, got %q", version) + } + }) +} + +// TestWorkspaceOriginalBehavior tests workspace.go functionality +func TestWorkspaceOriginalBehavior(t *testing.T) { + t.Run("WorkspaceCreation", func(t *testing.T) { + driver := New(nil) + + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + + // Test workspace structure + if ws.dir == "" { + t.Error("Workspace directory not set") + } + + if ws.goGets == nil { + t.Error("goGets map not initialized") + } + + if ws.packagePatterns == nil { + t.Error("packagePatterns map not initialized") + } + + if ws.parsedPackages == nil { + t.Error("parsedPackages map not initialized") + } + + t.Logf("✓ Workspace created at: %s", ws.dir) + + // Test cleanup + err = ws.Close() + if err != nil { + t.Errorf("Failed to close workspace: %v", err) + } + t.Log("✓ Workspace cleaned up successfully") + }) + + t.Run("PackageCachingBehavior", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + // Test the caching mechanism described in TODOs + t.Log("Original package caching behavior:") + t.Log("- parsedPackages map stores *packages.Package by key") + t.Log("- Keys include both versioned and non-versioned forms") + t.Log("- TODO comment: 'make this cache ephemeral... not enough memory for all versions'") + t.Log("- No eviction strategy implemented") + t.Log("- packages.Visit() caches all imported packages") + + // Demonstrate unbounded cache behavior + testPkg := &packages.Package{ + ID: "test/package", + Name: "testpkg", + } + + // Simulate what happens in the actual code + ws.mu.Lock() + ws.parsedPackages["test/package"] = testPkg + ws.parsedPackages["test/package@v1.0.0"] = testPkg + ws.parsedPackages["test/package@v1.1.0"] = testPkg + ws.mu.Unlock() + + if len(ws.parsedPackages) != 3 { + t.Errorf("Expected 3 cache entries, got %d", len(ws.parsedPackages)) + } + + t.Logf("✓ Cache grew to %d entries (demonstrates unbounded growth)", len(ws.parsedPackages)) + }) + + t.Run("ConcurrentAccessPatterns", func(t *testing.T) { + // Test the mutex usage in workspace + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + t.Log("Workspace concurrency:") + t.Log("- Has sync.RWMutex for workspace operations") + t.Log("- Protects goGets map and package operations") + t.Log("- Driver.discoveredTypes is protected by its own mutex") + + // The workspace itself is properly protected, but the driver cache is not + testKey := "concurrent/test" + + // This should be safe (workspace level) + ws.mu.Lock() + ws.goGets[testKey] = struct{}{} + ws.mu.Unlock() + + ws.mu.RLock() + _, exists := ws.goGets[testKey] + ws.mu.RUnlock() + + if !exists { + t.Error("Concurrent access to goGets failed") + } else { + t.Log("✓ Workspace-level synchronization works correctly") + } + }) +} + +// TestStorageOriginalBehavior tests storage.go functionality +func TestStorageOriginalBehavior(t *testing.T) { + t.Run("DereferenceFunction", func(t *testing.T) { + // Test the dereference functionality + driver := New(nil) + + // Test with no SameAs (should be no-op) + val := &Value{ + Type: String, + TypeName: "test", + } + + result, err := driver.dereference(val) + if err != nil { + t.Errorf("Dereference of non-SameAs value failed: %v", err) + } + + if result != val { + t.Error("Dereference should return same value when SameAs is empty") + } + + t.Log("✓ Dereference no-op case works correctly") + }) + + t.Run("DeepDereferenceRecursion", func(t *testing.T) { + // Test the recursive deep dereferencing + driver := New(nil) + + // Create a nested structure to test recursion + val := &Value{ + Type: Struct, + StructFields: []*StructField{ + { + Key: "field1", + Value: &Value{ + Type: String, + TypeName: "string", + }, + }, + }, + } + + result, err := driver.deepDereference(val) + if err != nil { + t.Errorf("DeepDereference failed: %v", err) + } + + if result == nil { + t.Error("DeepDereference returned nil") + } + + if len(result.StructFields) != 1 { + t.Errorf("Expected 1 struct field, got %d", len(result.StructFields)) + } + + t.Log("✓ DeepDereference recursion works correctly") + }) + + t.Run("SplitLastDotFunction", func(t *testing.T) { + // Test the utility function that splits fully qualified type names + testCases := []struct { + input string + expectedLeft string + expectedRight string + }{ + {"github.com/caddyserver/caddy/v2.Config", "github.com/caddyserver/caddy/v2", "Config"}, + {"http.handlers.file_server", "http.handlers", "file_server"}, + {"http", "", "http"}, + {"", "", ""}, + } + + for _, tc := range testCases { + left, right := SplitLastDot(tc.input) + if left != tc.expectedLeft || right != tc.expectedRight { + t.Errorf("SplitLastDot(%q) = (%q, %q), expected (%q, %q)", + tc.input, left, right, tc.expectedLeft, tc.expectedRight) + } else { + t.Logf("✓ SplitLastDot(%q) = (%q, %q)", tc.input, left, right) + } + } + }) +} diff --git a/driver.go b/driver.go index b7f2f90..fbd0e6f 100644 --- a/driver.go +++ b/driver.go @@ -30,7 +30,7 @@ import ( type Driver struct { db Storage - // TODO: use this, there's DEFINITELY A CONFIRMED race on discoveredTypes + // protects discoveredTypes mu sync.RWMutex // a cache of type definitions we've processed, keyed @@ -38,6 +38,26 @@ type Driver struct { discoveredTypes map[string]*Value } +// discoveredType returns the cached type for key, if present. +func (d *Driver) discoveredType(key string) (*Value, bool) { + d.mu.RLock() + defer d.mu.RUnlock() + val, ok := d.discoveredTypes[key] + return val, ok +} + +func (d *Driver) setDiscoveredType(key string, val *Value) { + d.mu.Lock() + defer d.mu.Unlock() + d.discoveredTypes[key] = val +} + +func (d *Driver) deleteDiscoveredType(key string) { + d.mu.Lock() + defer d.mu.Unlock() + delete(d.discoveredTypes, key) +} + // New constructs a new documentation system. func New(database Storage) *Driver { return &Driver{ @@ -228,7 +248,10 @@ func (d *Driver) TraverseType(path string, start *Value) (val, nearestType *Valu if err != nil { return nil, nil, fmt.Errorf("loading type for module %s: %v", caddyModuleID, err) } - val = vals[0] // TODO: support multiple values (two modules with same ID)... how? if in the middle, maybe find the one that matches; if at end...? maybe return a slice of them? + if len(vals) == 0 { + return nil, nil, fmt.Errorf("no module found with ID: %s", caddyModuleID) + } + val = vals[0].clone() // TODO: support multiple values (two modules with same ID)... how? if in the middle, maybe find the one that matches; if at end...? maybe return a slice of them? val.ModuleInlineKey = moduleInlineKey case Map, Array: diff --git a/driver_edge_test.go b/driver_edge_test.go new file mode 100644 index 0000000..a33c47f --- /dev/null +++ b/driver_edge_test.go @@ -0,0 +1,138 @@ +package moduledoc + +import ( + "strings" + "testing" +) + +func TestLoadTypesByModuleID(t *testing.T) { + db := newMemStorage() + db.types[storageKey("example.com/pkg", "Handler", "")] = &Value{ + Type: Struct, + TypeName: "example.com/pkg.Handler", + Doc: "handler doc", + StructFields: []*StructField{ + {Key: "root", Value: &Value{Type: String}, Doc: "root doc"}, + }, + } + db.modules["http.handlers.x"] = []*Value{ + {SameAs: "example.com/pkg.Handler", Doc: "usage doc"}, + } + d := New(db) + + vals, err := d.LoadTypesByModuleID("http.handlers.x") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(vals) != 1 { + t.Fatalf("expected 1 value, got %d", len(vals)) + } + got := vals[0] + if got.Type != Struct || got.TypeName != "example.com/pkg.Handler" { + t.Errorf("module type not resolved, got %+v", got) + } + if !strings.Contains(got.Doc, "usage doc") || !strings.Contains(got.Doc, "handler doc") { + t.Errorf("docs should combine usage and type docs, got %q", got.Doc) + } + if len(got.StructFields) != 1 || got.StructFields[0].Value.Type != String { + t.Errorf("struct fields not deeply dereferenced: %+v", got.StructFields) + } + + // unknown IDs yield an empty result, not an error + vals, err = d.LoadTypesByModuleID("does.not.exist") + if err != nil { + t.Fatalf("unexpected error for unknown module ID: %v", err) + } + if len(vals) != 0 { + t.Errorf("expected no values for unknown module ID, got %d", len(vals)) + } +} + +func TestLoadTypeByPath(t *testing.T) { + db := newMemStorage() + db.types[storageKey(CaddyCorePackage, "Config", "")] = &Value{ + Type: Struct, + TypeName: CaddyCorePackage + ".Config", + StructFields: []*StructField{ + { + Key: "listen", + Value: &Value{SameAs: "example.com/pkg.Listen"}, + Doc: "listen field doc", + }, + }, + } + db.types[storageKey("example.com/pkg", "Listen", "")] = &Value{ + Type: String, + TypeName: "example.com/pkg.Listen", + Doc: "listen type doc", + } + d := New(db) + + exact, nearest, err := d.LoadTypeByPath("listen", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if exact == nil || exact.Type != String { + t.Errorf("expected resolved string type at path, got %+v", exact) + } + if !strings.Contains(exact.Doc, "listen field doc") { + t.Errorf("field doc should be included at target, got %q", exact.Doc) + } + if nearest == nil || nearest.TypeName == "" { + t.Errorf("expected a named nearest type, got %+v", nearest) + } + + // without a stored Config start type, the lookup must error + if _, _, err := New(newMemStorage()).LoadTypeByPath("listen", ""); err == nil { + t.Error("expected error when start type is missing") + } +} + +func TestAddTypeStdlibStruct(t *testing.T) { + if testing.Short() { + t.Skip("requires the Go toolchain") + } + db := newMemStorage() + d := New(db) + + // stdlib packages resolve offline in a fresh workspace + rep, err := d.AddType("net/url", "URL", "") + if err != nil { + t.Fatalf("AddType failed: %v", err) + } + if !strings.Contains(rep.SameAs, "net/url.URL") { + t.Errorf("expected reference to net/url.URL, got %+v", rep) + } + + stored, err := db.GetTypeByName("net/url", "URL", "") + if err != nil || stored == nil { + t.Fatalf("stored type not found: %v", err) + } + if stored.Type != Struct { + t.Errorf("expected struct, got %s", stored.Type) + } + if !strings.Contains(stored.Doc, "URL") { + t.Errorf("expected type godoc, got %q", stored.Doc) + } +} + +func TestAddTypeErrors(t *testing.T) { + if testing.Short() { + t.Skip("requires the Go toolchain") + } + d := New(newMemStorage()) + + if _, err := d.AddType("net/url", "DoesNotExist", ""); err == nil { + t.Error("expected error for unknown type name") + } +} + +func TestLoadModulesFromImportingPackageEmptyPattern(t *testing.T) { + if testing.Short() { + t.Skip("requires the Go toolchain") + } + d := New(newMemStorage()) + if _, err := d.LoadModulesFromImportingPackage("", ""); err == nil { + t.Error("expected error for empty package pattern") + } +} diff --git a/helper_test.go b/helper_test.go index b40a0b6..0dc27ce 100644 --- a/helper_test.go +++ b/helper_test.go @@ -15,8 +15,6 @@ package moduledoc import ( - "errors" - "fmt" "go/types" "sync" "testing" @@ -28,15 +26,8 @@ import ( func loadAliasesFixture(t *testing.T) *packages.Package { t.Helper() cfg := &packages.Config{ - Mode: packages.NeedName | - packages.NeedFiles | - packages.NeedImports | - packages.NeedDeps | - packages.NeedTypes | - packages.NeedTypesInfo | - packages.NeedSyntax | - packages.NeedModule, - Dir: "testdata/aliases", + Mode: packagesLoadMode, + Dir: "testdata/aliases", } pkgs, err := packages.Load(cfg, ".") if err != nil { @@ -80,38 +71,3 @@ func newTestBuilder(t *testing.T, pkg *packages.Package) representationBuilder { rb.versionCache[pkg.PkgPath] = "" return rb } - -// memStorage is an in-memory Storage implementation for tests. -type memStorage struct { - mu sync.Mutex - types map[string]*Value -} - -func newMemStorage() *memStorage { - return &memStorage{types: map[string]*Value{}} -} - -func (m *memStorage) key(pkgPath, name, version string) string { - return fmt.Sprintf("%s|%s|%s", pkgPath, name, version) -} - -func (m *memStorage) GetTypeByName(packagePath, name, version string) (*Value, error) { - m.mu.Lock() - defer m.mu.Unlock() - return m.types[m.key(packagePath, name, version)], nil -} - -func (m *memStorage) GetTypesByCaddyModuleID(caddyModuleID string) ([]*Value, error) { - return nil, errors.New("not implemented") -} - -func (m *memStorage) StoreType(packagePath, typeName, version string, rep *Value) error { - m.mu.Lock() - defer m.mu.Unlock() - m.types[m.key(packagePath, typeName, version)] = rep - return nil -} - -func (m *memStorage) SetCaddyModuleName(pkg *packages.Package, typeName, modName string) error { - return nil -} diff --git a/integration_baseline_test.go b/integration_baseline_test.go new file mode 100644 index 0000000..3c2f8f4 --- /dev/null +++ b/integration_baseline_test.go @@ -0,0 +1,220 @@ +package moduledoc + +import ( + "go/types" + "testing" +) + +// TestIntegrationOriginalBehavior tests the integration between workspace, synthesis, and storage +func TestIntegrationOriginalBehavior(t *testing.T) { + t.Run("FullWorkflowWithTestdata", func(t *testing.T) { + if testing.Short() { + t.Skip("requires the Go toolchain") + } + // the public API needs a fetchable module path, so run the same + // pipeline (load → find idents → build representations) against + // the local testdata package instead + driver := New(newMemStorage()) + ws := localWorkspace(t, driver) + + pkgs, err := ws.getPackages(testdataPackagePath, "") + if err != nil { + t.Fatalf("loading testdata package: %v", err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package, got %d", len(pkgs)) + } + + modules, err := ws.representationBuilder().loadModulesFromSinglePackage(pkgs[0]) + if err != nil { + t.Fatalf("loading modules from testdata package: %v", err) + } + + t.Logf("Found %d modules in testdata", len(modules)) + + for i, module := range modules { + t.Logf("Module %d: Name=%s", i, module.Name) + if module.Representation != nil { + t.Logf(" Type: %s", module.Representation.Type) + t.Logf(" TypeName: %s", module.Representation.TypeName) + } + } + + // Verify we found the expected static module + foundGizmo := false + for _, module := range modules { + if module.Name == "app.namespace.gizmo" { + foundGizmo = true + break + } + } + + if !foundGizmo { + t.Error("Expected to find 'app.namespace.gizmo' module") + } else { + t.Log("✓ Successfully found expected module") + } + }) + + t.Run("WorkspacePackageCacheIntegration", func(t *testing.T) { + // Test the integration between workspace and package caching + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + // Test that cachedPackages returns nil for non-cached packages + cached := ws.cachedPackages("non.existent/package") + if cached != nil { + t.Error("Expected nil for non-cached package") + } else { + t.Log("✓ cachedPackages correctly returns nil for non-cached packages") + } + + // Test that the cache behavior matches the TODO comments + t.Log("Original caching behavior verification:") + t.Log("- Cache stores packages.Package by string key") + t.Log("- Uses both versioned and non-versioned keys") + t.Log("- No size limits (TODO: 'not enough memory for all versions')") + t.Log("- No TTL expiration (TODO: 'should probably expire')") + }) + + t.Run("SynthesisStorageIntegration", func(t *testing.T) { + // Test the integration between synthesis and storage via dereference + driver := New(nil) + + // Test dereference with SameAs reference and nil storage - this will panic + // because dereference calls ds.db.GetTypeByName() and ds.db is nil + valWithSameAs := &Value{ + SameAs: "example.com/test.TestType@v1.0.0", + ModuleNamespace: stringPtr("test.namespace"), + ModuleInlineKey: stringPtr("type"), + } + + // A nil Storage is invalid usage; today dereference panics on it, + // a graceful error would also be acceptable — only require that it + // does not silently succeed + func() { + defer func() { + if r := recover(); r != nil { + t.Logf("✓ Dereference panicked with nil storage: %v", r) + } + }() + _, err := driver.dereference(valWithSameAs) + if err != nil { + t.Logf("✓ Dereference returned error with nil storage: %v", err) + } else { + t.Error("dereference with nil storage must not silently succeed") + } + }() + + // Test dereference with empty SameAs - this should work fine + valNoSameAs := &Value{ + Type: String, + TypeName: "string", + ModuleNamespace: stringPtr("test.namespace"), + ModuleInlineKey: stringPtr("type"), + } + + result, err := driver.dereference(valNoSameAs) + if err != nil { + t.Errorf("dereference failed with empty SameAs: %v", err) + } else if result != valNoSameAs { + t.Error("dereference should return same value when SameAs is empty") + } else { + t.Log("✓ Dereference works correctly with empty SameAs") + } + + // Test deepDereference recursion with no SameAs references + structVal := &Value{ + Type: Struct, + StructFields: []*StructField{ + { + Key: "field1", + Value: &Value{ + Type: String, + TypeName: "string", + Doc: "Field documentation", + }, + }, + }, + } + + deepResult, err := driver.deepDereference(structVal) + if err != nil { + t.Errorf("deepDereference failed: %v", err) + } else if deepResult == nil { + t.Error("deepDereference returned nil") + } else { + t.Log("✓ deepDereference completed successfully") + if len(deepResult.StructFields) > 0 && deepResult.StructFields[0].Doc != "" { + t.Logf(" Field doc preserved: %q", deepResult.StructFields[0].Doc) + } + } + }) + + t.Run("BuildRepresentationCacheIntegration", func(t *testing.T) { + // Test that buildRepresentation uses discoveredTypes cache + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // a type already in the driver cache is returned as a reference + // without loading its package or touching storage + pkg := types.NewPackage("example.com/fake", "fake") + named := types.NewNamed( + types.NewTypeName(0, pkg, "Thing", nil), + types.Typ[types.String], nil, + ) + + rb.versionCache["example.com/fake"] = "v1.0.0" + sameAs := "example.com/fake.Thing@v1.0.0" + driver.setDiscoveredType(sameAs, &Value{Type: String, TypeName: "example.com/fake.Thing"}) + + rep, err := rb.buildRepresentation(named) + if err != nil { + t.Fatalf("buildRepresentation failed: %v", err) + } + if rep.SameAs != sameAs { + t.Errorf("expected cache hit returning reference %q, got %+v", sameAs, rep) + } + }) +} + +// TestModuleValidationIntegration tests the strict validation behavior +func TestModuleValidationIntegration(t *testing.T) { + t.Run("OriginalStrictValidation", func(t *testing.T) { + // fully compliant module: registration + implementation + static ID + validSource := ` +package test + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(ValidModule)) +} + +type ValidModule struct{} + +func (*ValidModule) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.test.valid", + New: func() caddy.Module { return new(ValidModule) }, + } +} +` + testModuleSource(t, validSource, true, "Compliant module should be detected") + }) +} + +// Helper function to create string pointers for testing +func stringPtr(s string) *string { + return &s +} diff --git a/original_baseline_test.go b/original_baseline_test.go new file mode 100644 index 0000000..25650b9 --- /dev/null +++ b/original_baseline_test.go @@ -0,0 +1,109 @@ +// Copyright 2019 Matthew Holt +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package moduledoc + +import ( + "os" + "testing" + + "golang.org/x/tools/go/packages" +) + +// TestOriginalImplementation tests the existing functionality to establish a baseline +func TestOriginalImplementation(t *testing.T) { + t.Run("BasicModuleDetection", func(t *testing.T) { + // Test that basic module detection works with static string literals + cfg := &packages.Config{ + Dir: ".", + Mode: packagesLoadMode, + Env: append(os.Environ(), "CGO_ENABLED=0"), + } + + pkgs, err := packages.Load(cfg, "./testdata") + if err != nil { + t.Fatalf("Failed to load testdata: %v", err) + } + + if len(pkgs) == 0 { + t.Fatal("No packages loaded") + } + + driver := New(nil) + pkg := pkgs[0] + + // Test findCaddyModuleIdents with the original gizmo.go + moduleIdents, err := driver.findCaddyModuleIdents(pkg) + if err != nil { + t.Errorf("findCaddyModuleIdents failed: %v", err) + } + + // Should find the static module from gizmo.go + expectedModuleID := "app.namespace.gizmo" + found := false + for _, moduleID := range moduleIdents { + if moduleID == expectedModuleID { + found = true + break + } + } + + if !found { + t.Errorf("Expected to find module ID '%s' but got: %v", expectedModuleID, moduleIdents) + } + + t.Logf("Successfully found %d modules: %v", len(moduleIdents), moduleIdents) + }) +} + +// TestOriginalCachingBehavior tests the caching mechanisms in the original implementation +func TestOriginalCachingBehavior(t *testing.T) { + t.Run("UnboundedCacheGrowth", func(t *testing.T) { + // Test that demonstrates unbounded cache growth + driver := New(nil) + + // Simulate adding many entries to the cache + for i := 0; i < 1000; i++ { + key := "test.type." + string(rune(i)) + driver.setDiscoveredType(key, &Value{ + Type: String, + TypeName: key, + }) + } + + // no size limit, TTL, or eviction: every entry is retained + if len(driver.discoveredTypes) != 1000 { + t.Errorf("Expected 1000 cache entries, got %d", len(driver.discoveredTypes)) + } + }) +} + +// TestOriginalErrorHandling tests error handling patterns in the original code +func TestOriginalErrorHandling(t *testing.T) { + t.Run("StrictModuleValidation", func(t *testing.T) { + // registration without a local CaddyModule method fails the package + source := ` +package test + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(HalfModule)) +} + +type HalfModule struct{} +` + testModuleSource(t, source, false, "Registration without implementation should fail") + }) +} diff --git a/sourcecode.go b/sourcecode.go index 9075790..7500cd9 100644 --- a/sourcecode.go +++ b/sourcecode.go @@ -17,6 +17,7 @@ package moduledoc import ( "fmt" "go/ast" + "go/constant" "go/types" "log" "strings" @@ -102,24 +103,29 @@ func (ds *Driver) findCaddyModuleIdents(pkg *packages.Package) (map[*ast.Ident]s if !ok { continue } - if kv.Key.(*ast.Ident).Name == "ID" { - // TODO: configadapters.go in the main caddy module has an unexported helper type called - // adapterModule which implements CaddyModule interface, and its ID is computed, not static: - // `caddy.ModuleID("caddy.adapters." + am.name)` - this is obviously problematic here... - // but that's also a special case that real modules should not be having - kvValueBasicLiteral, ok := kv.Value.(*ast.BasicLit) - if !ok { - log.Printf("[WARNING] CaddyModule() method in %s returns ModuleInfo with unsupported ID value (must be a static literal value); skipping: %#v", file.Name, kv.Value) - delete(caddyModRegs, currentCaddyModuleFunc.Name) - delete(caddyModImpls, currentCaddyModuleFunc.Name) - currentCaddyModuleFunc = nil - return true + keyIdent, ok := kv.Key.(*ast.Ident) + if !ok { + continue + } + if keyIdent.Name == "ID" { + // the type checker evaluates constant expressions, so + // literals, constants, and constant concatenations all work; + // only truly runtime-computed IDs remain unsupported + if tv, ok := pkg.TypesInfo.Types[kv.Value]; ok && tv.Value != nil && tv.Value.Kind() == constant.String { + caddyModName = constant.StringVal(tv.Value) + break } - - // TODO: What if the module name is pulled out to a constant? do we need to evaluate it? - rawString := kvValueBasicLiteral.Value - caddyModName = strings.Trim(rawString, `"`) - break + // no usable type info (e.g. package has errors); still + // accept plain string literals + if lit, ok := kv.Value.(*ast.BasicLit); ok { + caddyModName = strings.Trim(lit.Value, `"`) + break + } + log.Printf("[WARNING] CaddyModule() method in %s returns ModuleInfo with unsupported ID value (must be constant); skipping: %#v", file.Name, kv.Value) + delete(caddyModRegs, currentCaddyModuleFunc.Name) + delete(caddyModImpls, currentCaddyModuleFunc.Name) + currentCaddyModuleFunc = nil + return true } } @@ -214,12 +220,24 @@ func (ds *Driver) findModuleRegistration(pkg *packages.Package, fnCall *ast.Call switch val := fnCall.Args[0].(type) { case *ast.CompositeLit: // happens with `caddy.RegisterModule(Gizmo{})` - caddyModuleIdent = val.Type.(*ast.Ident) + ident, ok := val.Type.(*ast.Ident) + if !ok { + // e.g. a qualified type from another package; it will be + // documented from its home package, so skip it here + log.Printf("[WARNING] %s() argument type is not declared in this package; skipping: %#v", registerModule, val.Type) + return nil, nil + } + caddyModuleIdent = ident case *ast.CallExpr: // happens with `caddy.RegisterModule(new(Gizmo))` if funIdent, ok := val.Fun.(*ast.Ident); ok && funIdent.Name == "new" { - caddyModuleIdent = val.Args[0].(*ast.Ident) + ident, ok := val.Args[0].(*ast.Ident) + if !ok { + log.Printf("[WARNING] %s() argument type is not declared in this package; skipping: %#v", registerModule, val.Args[0]) + return nil, nil + } + caddyModuleIdent = ident } else { return nil, fmt.Errorf("unknown function call in %s(): %#v - only support new()", registerModule, val.Fun) @@ -252,7 +270,11 @@ func (ds *Driver) findModuleImpl(fnDecl *ast.FuncDecl) (*ast.Ident, error) { case *ast.Ident: receiver = val case *ast.StarExpr: - receiver = val.X.(*ast.Ident) + ident, ok := val.X.(*ast.Ident) + if !ok { + return nil, fmt.Errorf("expected identifier for pointer receiver type, but got %#v", val.X) + } + receiver = ident default: return nil, fmt.Errorf("expected identifier or pointer for receiver type, but got %#v", fnDecl.Recv.List[0].Type) } diff --git a/sourcecode_edge_test.go b/sourcecode_edge_test.go new file mode 100644 index 0000000..feee1ff --- /dev/null +++ b/sourcecode_edge_test.go @@ -0,0 +1,115 @@ +package moduledoc + +import ( + "os" + "testing" + + "golang.org/x/tools/go/packages" +) + +// loadFixturePackage loads one local testdata package with full type info. +func loadFixturePackage(t *testing.T, pattern string) *packages.Package { + t.Helper() + if testing.Short() { + t.Skip("requires the Go toolchain") + } + cfg := &packages.Config{ + Mode: packagesLoadMode, + Env: append(os.Environ(), "CGO_ENABLED=0"), + } + pkgs, err := packages.Load(cfg, pattern) + if err != nil { + t.Fatalf("loading %s: %v", pattern, err) + } + if len(pkgs) != 1 { + t.Fatalf("expected 1 package for %s, got %d", pattern, len(pkgs)) + } + for _, e := range pkgs[0].Errors { + t.Fatalf("fixture package %s has errors: %v", pattern, e) + } + return pkgs[0] +} + +func identsByTypeName(t *testing.T, d *Driver, pkg *packages.Package) map[string]string { + t.Helper() + idents, err := d.findCaddyModuleIdents(pkg) + if err != nil { + t.Fatalf("finding module idents: %v", err) + } + byName := make(map[string]string, len(idents)) + for ident, id := range idents { + byName[ident.Name] = id + } + return byName +} + +func TestFindModuleIdentsPointerAndValuePatterns(t *testing.T) { + d := New(newMemStorage()) + + // registration via new() with pointer receiver + pkg := loadFixturePackage(t, "./testdata") + got := identsByTypeName(t, d, pkg) + if got["Gizmo"] != "app.namespace.gizmo" { + t.Errorf("expected Gizmo module, got %#v", got) + } + + // registration via composite literal with value receiver + pkg = loadFixturePackage(t, "./testdata/valuerecv") + got = identsByTypeName(t, d, pkg) + if got["Sprocket"] != "app.namespace.sprocket" { + t.Errorf("expected Sprocket module, got %#v", got) + } +} + +func TestModuleIDFromConstant(t *testing.T) { + d := New(newMemStorage()) + pkg := loadFixturePackage(t, "./testdata/constid") + got := identsByTypeName(t, d, pkg) + if got["ConstWidget"] != "app.namespace.const_widget" { + t.Errorf("module with ID declared as a package constant should be discovered, got %#v", got) + } +} + +func TestModuleIDFromConcatenation(t *testing.T) { + d := New(newMemStorage()) + pkg := loadFixturePackage(t, "./testdata/exprid") + got := identsByTypeName(t, d, pkg) + if got["ExprWidget"] != "app.namespace.expr_widget" { + t.Errorf("module with ID built from constant string concatenation should be discovered, got %#v", got) + } +} + +func TestUnregisteredModuleFailsPackage(t *testing.T) { + d := New(newMemStorage()) + pkg := loadFixturePackage(t, "./testdata/unregistered") + // one unregistered module type fails the whole package, + // including its valid sibling module (current strict contract) + if _, err := d.findCaddyModuleIdents(pkg); err == nil { + t.Error("expected error for package containing an unregistered module type") + } +} + +func TestRegistrationWithoutLocalImplementation(t *testing.T) { + d := New(newMemStorage()) + pkg := loadFixturePackage(t, "./testdata/noimpl") + if _, err := d.findCaddyModuleIdents(pkg); err == nil { + t.Error("expected error for registration whose CaddyModule method is not declared in the package") + } +} + +func TestFindModuleIdentsUnusualAST(t *testing.T) { + d := New(newMemStorage()) + pkg := loadFixturePackage(t, "./testdata/crosspkg") + + defer func() { + if r := recover(); r != nil { + t.Fatalf("registration of a qualified type from another package must not panic: %v", r) + } + }() + // either outcome is acceptable as long as it does not panic: + // an error (no local implementation) or an empty result + idents, err := d.findCaddyModuleIdents(pkg) + if err == nil && len(idents) > 0 { + t.Logf("cross-package registration produced idents: %v", idents) + } +} diff --git a/specific_functions_test.go b/specific_functions_test.go new file mode 100644 index 0000000..969121d --- /dev/null +++ b/specific_functions_test.go @@ -0,0 +1,326 @@ +package moduledoc + +import ( + "go/types" + "os" + "reflect" + "strings" + "testing" + + "golang.org/x/tools/go/packages" +) + +// TestWorkspaceSpecificFunctions tests specific functions in workspace.go +func TestWorkspaceSpecificFunctions(t *testing.T) { + t.Run("PackageKeyFunction", func(t *testing.T) { + // Test the packageKey function that creates cache keys + testCases := []struct { + name string + pkg *packages.Package + expected string + }{ + { + name: "PackageWithVersion", + pkg: &packages.Package{ + ID: "example.com/test", + Module: &packages.Module{ + Version: "v1.0.0", + }, + }, + expected: "example.com/test@v1.0.0", + }, + { + name: "PackageWithoutVersion", + pkg: &packages.Package{ + ID: "example.com/test", + }, + expected: "example.com/test", + }, + { + name: "PackageWithEmptyVersion", + pkg: &packages.Package{ + ID: "example.com/test", + Module: &packages.Module{ + Version: "", + }, + }, + expected: "example.com/test", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + result := packageKey(tc.pkg) + if result != tc.expected { + t.Errorf("packageKey() = %q, expected %q", result, tc.expected) + } else { + t.Logf("✓ packageKey() = %q", result) + } + }) + } + }) + + t.Run("AlreadyGotModuleFunction", func(t *testing.T) { + // Test the alreadyGotModule function + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + // Add some modules to the goGets cache + ws.goGets["github.com/caddyserver/caddy"] = struct{}{} + ws.goGets["example.com/test"] = struct{}{} + + testCases := []struct { + packagePath string + expected bool + description string + }{ + {"github.com/caddyserver/caddy/v2", true, "should find parent module"}, + {"github.com/caddyserver/caddy/v2/modules/http", true, "should find ancestor module"}, + {"example.com/test/subpackage", true, "should find parent module"}, + {"other.com/package", false, "should not find unrelated module"}, + {"", false, "should handle empty path"}, + } + + for _, tc := range testCases { + result := ws.alreadyGotModule(tc.packagePath) + if result != tc.expected { + t.Errorf("alreadyGotModule(%q) = %v, expected %v (%s)", + tc.packagePath, result, tc.expected, tc.description) + } else { + t.Logf("✓ alreadyGotModule(%q) = %v (%s)", + tc.packagePath, result, tc.description) + } + } + }) + + t.Run("RepresentationBuilderCreation", func(t *testing.T) { + // Test representationBuilder creation + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + if rb.ws.dir != ws.dir { + t.Error("RepresentationBuilder workspace not set correctly") + } + + if rb.versionCache == nil { + t.Error("RepresentationBuilder versionCache not initialized") + } + + if len(rb.versionCache) != 0 { + t.Error("RepresentationBuilder versionCache should start empty") + } + + t.Log("✓ RepresentationBuilder created correctly") + }) +} + +// TestSynthesisSpecificFunctions tests specific functions in synthesis.go +func TestSynthesisSpecificFunctions(t *testing.T) { + t.Run("GetStructFieldGodocs", func(t *testing.T) { + // Test getStructFieldGodocs with real testdata + cfg := &packages.Config{ + Dir: "testdata", + Mode: packagesLoadMode, + Env: append(os.Environ(), "CGO_ENABLED=0"), + } + + pkgs, err := packages.Load(cfg, "testdata") + if err != nil { + t.Logf("Package loading failed (expected if testdata has issues): %v", err) + return + } + + if len(pkgs) == 0 { + t.Skip("No packages loaded, skipping getStructFieldGodocs test") + } + + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + pkg := pkgs[0] + + // Look for the Gizmo struct type + for _, obj := range pkg.TypesInfo.Defs { + if obj != nil && obj.Name() == "Gizmo" { + if typeName, ok := obj.(*types.TypeName); ok { + namedType := typeName.Type().(*types.Named) + + // Test getStructFieldGodocs + docs, err := rb.getStructFieldGodocs(namedType) + if err != nil { + t.Errorf("getStructFieldGodocs failed: %v", err) + } else { + t.Logf("✓ getStructFieldGodocs returned %d field docs", len(docs)) + for field, doc := range docs { + t.Logf(" Field %s: %q", field, strings.TrimSpace(doc)) + } + } + break + } + } + } + }) + + t.Run("GetGodocForType", func(t *testing.T) { + // Test getGodocForType with testdata + cfg := &packages.Config{ + Dir: "testdata", + Mode: packagesLoadMode, + Env: append(os.Environ(), "CGO_ENABLED=0"), + } + + pkgs, err := packages.Load(cfg, "testdata") + if err != nil { + t.Logf("Package loading failed (expected if testdata has issues): %v", err) + return + } + + if len(pkgs) == 0 { + t.Skip("No packages loaded, skipping getGodocForType test") + } + + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + pkg := pkgs[0] + + // Look for the Gizmo struct type + for _, obj := range pkg.TypesInfo.Defs { + if obj != nil && obj.Name() == "Gizmo" { + if typeName, ok := obj.(*types.TypeName); ok { + namedType := typeName.Type().(*types.Named) + + // Test getGodocForType + doc, err := rb.getGodocForType(namedType) + if err != nil { + t.Errorf("getGodocForType failed: %v", err) + } else { + t.Logf("✓ getGodocForType returned: %q", strings.TrimSpace(doc)) + } + break + } + } + } + }) +} + +// TestStorageSpecificFunctions tests specific functions in storage.go +func TestStorageSpecificFunctions(t *testing.T) { + t.Run("ConfigPathPartsFunction", func(t *testing.T) { + // Test ConfigPathParts function + testCases := []struct { + input string + expected []string + }{ + {"/path/to/config", []string{"path", "to", "config"}}, + {"path/to/config", []string{"path", "to", "config"}}, + {"path/to/config/", []string{"path", "to", "config"}}, + {"/path", []string{"path"}}, + {"path", []string{"path"}}, + {"/", []string{""}}, + {"", []string{""}}, + } + + for _, tc := range testCases { + result := ConfigPathParts(tc.input) + if !reflect.DeepEqual(result, tc.expected) { + t.Errorf("ConfigPathParts(%q) = %v, expected %v", tc.input, result, tc.expected) + } else { + t.Logf("✓ ConfigPathParts(%q) = %v", tc.input, result) + } + } + }) + + t.Run("JsonNameFromTagFunction", func(t *testing.T) { + // Test jsonNameFromTag function + testCases := []struct { + tagStr string + expected string + ok bool + }{ + {`json:"name"`, "name", true}, + {`json:"name,omitempty"`, "name", true}, + {`json:",omitempty"`, "", true}, + {`json:"-"`, "", false}, + // per encoding/json, only exactly "-" excludes; this names the field "-" + {`json:"-,omitempty"`, "-", true}, + {`json:"name,required"`, "name", true}, + {`other:"value" json:"field"`, "field", true}, + {`other:"value"`, "", true}, + {"", "", true}, + } + + for _, tc := range testCases { + result, ok := jsonNameFromTag(tc.tagStr) + if result != tc.expected || ok != tc.ok { + t.Errorf("jsonNameFromTag(%q) = (%q, %v), expected (%q, %v)", + tc.tagStr, result, ok, tc.expected, tc.ok) + } else { + t.Logf("✓ jsonNameFromTag(%q) = (%q, %v)", tc.tagStr, result, ok) + } + } + }) + + t.Run("CaddyTagFieldsFunction", func(t *testing.T) { + // Test caddyTagFields function + testCases := []struct { + tagStr string + hasError bool + }{ + {`caddy:"namespace=http.handlers"`, false}, + {`caddy:"inline_key=type"`, false}, + {`json:"name" caddy:"namespace=app"`, false}, + {`other:"value"`, false}, + {"", false}, + } + + for _, tc := range testCases { + result, err := caddyTagFields(tc.tagStr) + if tc.hasError && err == nil { + t.Errorf("caddyTagFields(%q) expected error but got none", tc.tagStr) + } else if !tc.hasError && err != nil { + t.Errorf("caddyTagFields(%q) unexpected error: %v", tc.tagStr, err) + } else { + t.Logf("✓ caddyTagFields(%q) = %v (err: %v)", tc.tagStr, result, err) + } + } + }) + + t.Run("TypeUtilityFunctions", func(t *testing.T) { + pkg := types.NewPackage("example.com/pkg", "pkg") + named := types.NewNamed( + types.NewTypeName(0, pkg, "Thing", nil), + types.Typ[types.String], nil, + ) + + if got := fullyQualifiedTypeName(named); got != "example.com/pkg.Thing" { + t.Errorf("fullyQualifiedTypeName = %q; want %q", got, "example.com/pkg.Thing") + } + if p, n := typePackageAndName(named); p != "example.com/pkg" || n != "Thing" { + t.Errorf("typePackageAndName = (%q, %q); want (%q, %q)", p, n, "example.com/pkg", "Thing") + } + if got := localTypeName(named); got != "Thing" { + t.Errorf("localTypeName = %q; want %q", got, "Thing") + } + }) +} diff --git a/storage.go b/storage.go index 619cbdd..e9c410a 100644 --- a/storage.go +++ b/storage.go @@ -65,6 +65,10 @@ func (ds *Driver) dereference(val *Value) (*Value, error) { return nil, fmt.Errorf("dereference failed, type not found: %s@%s", fqtn, version) } + // work on a copy so the Storage-owned value is never mutated; + // otherwise context-specific info would bleed between dereferences + typ = typ.clone() + // transfer over the module namespace and inline key, since that // information is specific to the context in which the type appears, // thus the normalized stored type will not have that information; @@ -91,6 +95,21 @@ func (ds *Driver) dereference(val *Value) (*Value, error) { // As a result, the returned value information is completely // dereferenced and filled out. func (ds *Driver) deepDereference(val *Value) (*Value, error) { + // clone so the caller's (or Storage's) value is never mutated + return ds.deepDereferenceRec(val.clone(), make(map[string]struct{})) +} + +// deepDereferenceRec does the work of deepDereference, tracking the +// references on the current resolution path to detect cycles. +func (ds *Driver) deepDereferenceRec(val *Value, path map[string]struct{}) (*Value, error) { + if val.SameAs != "" { + if _, ok := path[val.SameAs]; ok { + return nil, fmt.Errorf("circular type reference: %s", val.SameAs) + } + path[val.SameAs] = struct{}{} + defer delete(path, val.SameAs) + } + var err error val, err = ds.dereference(val) if err != nil { @@ -99,7 +118,7 @@ func (ds *Driver) deepDereference(val *Value) (*Value, error) { // dereference all struct fields for _, sf := range val.StructFields { - sf.Value, err = ds.deepDereference(sf.Value) + sf.Value, err = ds.deepDereferenceRec(sf.Value, path) if err != nil { return nil, err } @@ -128,7 +147,7 @@ func (ds *Driver) deepDereference(val *Value) (*Value, error) { // dereference all map keys if val.MapKeys != nil { - val.MapKeys, err = ds.deepDereference(val.MapKeys) + val.MapKeys, err = ds.deepDereferenceRec(val.MapKeys, path) if err != nil { return nil, err } @@ -136,7 +155,7 @@ func (ds *Driver) deepDereference(val *Value) (*Value, error) { // dereference all map or array elements if val.Elems != nil { - val.Elems, err = ds.deepDereference(val.Elems) + val.Elems, err = ds.deepDereferenceRec(val.Elems, path) if err != nil { return nil, err } @@ -151,3 +170,33 @@ func (ds *Driver) getTypeByFullName(fqtn, version string) (*Value, error) { pkgName, typeName := SplitLastDot(fqtn) return ds.db.GetTypeByName(pkgName, typeName, version) } + +// clone returns a deep copy of v, so callers can freely +// mutate the copy without affecting Storage-owned values. +func (v *Value) clone() *Value { + if v == nil { + return nil + } + c := *v + c.MapKeys = v.MapKeys.clone() + c.Elems = v.Elems.clone() + if v.StructFields != nil { + c.StructFields = make([]*StructField, len(v.StructFields)) + for i, sf := range v.StructFields { + c.StructFields[i] = &StructField{ + Key: sf.Key, + Value: sf.Value.clone(), + Doc: sf.Doc, + } + } + } + if v.ModuleNamespace != nil { + ns := *v.ModuleNamespace + c.ModuleNamespace = &ns + } + if v.ModuleInlineKey != nil { + ik := *v.ModuleInlineKey + c.ModuleInlineKey = &ik + } + return &c +} diff --git a/storage_edge_test.go b/storage_edge_test.go new file mode 100644 index 0000000..9f3f9d8 --- /dev/null +++ b/storage_edge_test.go @@ -0,0 +1,244 @@ +package moduledoc + +import ( + "os" + "os/exec" + "strings" + "sync" + "testing" + + "golang.org/x/tools/go/packages" +) + +// memStorage is a minimal thread-safe in-memory Storage for unit tests. +type memStorage struct { + mu sync.Mutex + types map[string]*Value + modules map[string][]*Value +} + +func newMemStorage() *memStorage { + return &memStorage{ + types: make(map[string]*Value), + modules: make(map[string][]*Value), + } +} + +func storageKey(packagePath, name, version string) string { + return packagePath + "." + name + "@" + version +} + +func (m *memStorage) GetTypeByName(packagePath, name, version string) (*Value, error) { + m.mu.Lock() + defer m.mu.Unlock() + return m.types[storageKey(packagePath, name, version)], nil +} + +func (m *memStorage) GetTypesByCaddyModuleID(caddyModuleID string) ([]*Value, error) { + m.mu.Lock() + defer m.mu.Unlock() + // return a fresh slice, as a database-backed implementation would + return append([]*Value(nil), m.modules[caddyModuleID]...), nil +} + +func (m *memStorage) StoreType(packagePath, typeName, version string, rep *Value) error { + m.mu.Lock() + defer m.mu.Unlock() + m.types[storageKey(packagePath, typeName, version)] = rep + return nil +} + +func (m *memStorage) SetCaddyModuleName(pkg *packages.Package, typeName, modName string) error { + return nil +} + +func TestDereferenceNoOpWithoutSameAs(t *testing.T) { + d := New(newMemStorage()) + val := &Value{Type: String, Doc: "a doc"} + got, err := d.dereference(val) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != val { + t.Errorf("expected same value back when SameAs is empty") + } +} + +func TestDereferenceTypeNotFound(t *testing.T) { + d := New(newMemStorage()) + + _, err := d.dereference(&Value{SameAs: "example.com/pkg.Missing"}) + if err == nil { + t.Error("expected error for unknown referenced type, got nil") + } + + // malformed reference without any dot must error, not panic + _, err = d.dereference(&Value{SameAs: "nodot"}) + if err == nil { + t.Error("expected error for malformed type reference, got nil") + } +} + +func TestDereferenceVersionedReference(t *testing.T) { + db := newMemStorage() + db.types[storageKey("example.com/pkg", "T", "v1.2.3")] = &Value{Type: String, TypeName: "example.com/pkg.T"} + d := New(db) + + got, err := d.dereference(&Value{SameAs: "example.com/pkg.T@v1.2.3"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.Type != String { + t.Errorf("expected resolved string type, got %#v", got) + } + + // same reference without the version must not resolve to the versioned entry + if _, err := d.dereference(&Value{SameAs: "example.com/pkg.T"}); err == nil { + t.Error("expected error for unversioned reference to a versioned-only type") + } +} + +func TestDereferenceTransfersModuleInfoToInnermostElem(t *testing.T) { + db := newMemStorage() + // stored shape: array of maps of modules + db.types[storageKey("example.com/pkg", "Handlers", "")] = &Value{ + Type: Array, + TypeName: "example.com/pkg.Handlers", + Elems: &Value{ + Type: Map, + Elems: &Value{Type: Module}, + }, + } + d := New(db) + + ns := "http.handlers" + ik := "handler" + got, err := d.dereference(&Value{ + SameAs: "example.com/pkg.Handlers", + ModuleNamespace: &ns, + ModuleInlineKey: &ik, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + inner := got + for inner.Elems != nil { + inner = inner.Elems + } + if inner.ModuleNamespace == nil || *inner.ModuleNamespace != ns { + t.Errorf("module namespace not transferred to innermost element: %#v", inner) + } + if inner.ModuleInlineKey == nil || *inner.ModuleInlineKey != ik { + t.Errorf("module inline key not transferred to innermost element: %#v", inner) + } + if got.ModuleNamespace != nil && got.Elems != nil { + t.Errorf("namespace should live on the innermost element only") + } +} + +func TestDereferenceDoesNotMutateStoredType(t *testing.T) { + db := newMemStorage() + db.types[storageKey("example.com/pkg", "T", "")] = &Value{ + Type: String, + TypeName: "example.com/pkg.T", + Doc: "type doc", + } + d := New(db) + + ns := "http.handlers" + first, err := d.dereference(&Value{SameAs: "example.com/pkg.T", Doc: "field doc", ModuleNamespace: &ns}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + wantDoc := "field doc\n\ntype doc" + if first.Doc != wantDoc { + t.Errorf("first dereference doc = %q; want %q", first.Doc, wantDoc) + } + + // dereferencing the same reference again from a different context + // must not accumulate docs or leak module info between contexts + second, err := d.dereference(&Value{SameAs: "example.com/pkg.T", Doc: "field doc"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if second.Doc != wantDoc { + t.Errorf("second dereference doc = %q; want %q (docs must not accumulate)", second.Doc, wantDoc) + } + if second.ModuleNamespace != nil { + t.Errorf("second dereference has namespace %q leaked from a prior dereference", *second.ModuleNamespace) + } + if first.ModuleNamespace == nil || *first.ModuleNamespace != ns { + t.Errorf("first result's namespace was clobbered by a later dereference: %#v", first.ModuleNamespace) + } +} + +func TestDeepDereferenceResolvesNestedReferences(t *testing.T) { + db := newMemStorage() + db.types[storageKey("example.com/pkg", "Leaf", "")] = &Value{ + Type: String, + TypeName: "example.com/pkg.Leaf", + Doc: "leaf type doc", + } + db.types[storageKey("example.com/pkg", "Root", "")] = &Value{ + Type: Struct, + TypeName: "example.com/pkg.Root", + StructFields: []*StructField{ + {Key: "leaf", Value: &Value{SameAs: "example.com/pkg.Leaf"}, Doc: "field doc"}, + }, + MapKeys: &Value{SameAs: "example.com/pkg.Leaf"}, + Elems: nil, + } + d := New(db) + + got, err := d.deepDereference(&Value{SameAs: "example.com/pkg.Root"}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got.StructFields) != 1 { + t.Fatalf("expected 1 struct field, got %d", len(got.StructFields)) + } + sf := got.StructFields[0] + if sf.Value.Type != String { + t.Errorf("struct field not dereferenced: %#v", sf.Value) + } + if !strings.Contains(sf.Doc, "field doc") || !strings.Contains(sf.Doc, "leaf type doc") { + t.Errorf("field doc should combine field and type docs, got %q", sf.Doc) + } + if !strings.HasPrefix(sf.Doc, "field doc") { + t.Errorf("field doc should lead with the more specific field doc, got %q", sf.Doc) + } + if got.MapKeys == nil || got.MapKeys.Type != String { + t.Errorf("map keys not dereferenced: %#v", got.MapKeys) + } +} + +func TestDeepDereferenceCycle(t *testing.T) { + // run in a child process because unbounded recursion kills the + // whole process and would take every other test down with it + if os.Getenv("MODULEDOC_TEST_CYCLE_CHILD") == "1" { + db := newMemStorage() + db.types[storageKey("example.com/pkg", "Node", "")] = &Value{ + Type: Struct, + TypeName: "example.com/pkg.Node", + StructFields: []*StructField{ + {Key: "next", Value: &Value{SameAs: "example.com/pkg.Node"}}, + }, + } + d := New(db) + if _, err := d.deepDereference(&Value{SameAs: "example.com/pkg.Node"}); err == nil { + t.Fatal("expected error for self-referential type, got nil") + } + return + } + + cmd := exec.Command(os.Args[0], "-test.run", "^TestDeepDereferenceCycle$") + cmd.Env = append(os.Environ(), "MODULEDOC_TEST_CYCLE_CHILD=1") + out, err := cmd.CombinedOutput() + if err != nil { + if len(out) > 2000 { + out = out[:2000] + } + t.Fatalf("deep dereference of a self-referential type must fail gracefully, not crash: %v\n%s", err, out) + } +} diff --git a/synthesis.go b/synthesis.go index fb5d03a..b81c436 100644 --- a/synthesis.go +++ b/synthesis.go @@ -221,7 +221,7 @@ func (rb representationBuilder) buildRepresentation(caddyModuleType types.Type) if typeVersion != "" { sameAs += "@" + typeVersion } - if _, ok := rb.ws.driver.discoveredTypes[sameAs]; ok { + if _, ok := rb.ws.driver.discoveredType(sameAs); ok { return &Value{SameAs: sameAs}, nil } @@ -232,15 +232,28 @@ func (rb representationBuilder) buildRepresentation(caddyModuleType types.Type) return nil, err } if discoveredType != nil { - rb.ws.driver.discoveredTypes[sameAs] = discoveredType + rb.ws.driver.setDiscoveredType(sameAs, discoveredType) return &Value{SameAs: discoveredType.TypeName}, nil } // a json.RawMessage type represents a module! - if packagePath == "encoding/json" && typeName == "RawMessage" { + // (json.RawMessage is an alias for jsontext.Value as of the + // json/v2 stdlib, and aliases are unwrapped above) + if (packagePath == "encoding/json" && typeName == "RawMessage") || + (packagePath == "encoding/json/jsontext" && typeName == "Value") { return &Value{Type: Module}, nil } + // pre-register the type so self-referential fields resolve to a + // reference instead of recursing forever; removed again on failure + rb.ws.driver.setDiscoveredType(sameAs, new(Value)) + stored := false + defer func() { + if !stored { + rb.ws.driver.deleteDiscoveredType(sameAs) + } + }() + // otherwise, if this type is new, store it in the DB switch utyp := typ.Underlying().(type) { case *types.Struct: @@ -323,7 +336,8 @@ func (rb representationBuilder) buildRepresentation(caddyModuleType types.Type) rep.TypeName = fullTypeName // remember this type so we don't have to re-assemble it all later - rb.ws.driver.discoveredTypes[sameAs] = rep + rb.ws.driver.setDiscoveredType(sameAs, rep) + stored = true err = rb.ws.driver.db.StoreType(packagePath, typeName, typeVersion, rep) if err != nil { return nil, err @@ -400,7 +414,10 @@ func (rb representationBuilder) buildRepresentation(caddyModuleType types.Type) return nil, nil default: - return nil, fmt.Errorf("unknown type %s: %#v", caddyModuleType.String(), caddyModuleType) + // types with no JSON representation (chan, func, generics, ...) + // must not fail the whole containing type; represent them like + // interfaces, as an empty value + return new(Value), nil } } diff --git a/synthesis_baseline_test.go b/synthesis_baseline_test.go new file mode 100644 index 0000000..db14aae --- /dev/null +++ b/synthesis_baseline_test.go @@ -0,0 +1,310 @@ +package moduledoc + +import ( + "go/types" + "strings" + "testing" +) + +// TestSynthesisFunctionsOriginalBehavior tests the core synthesis functions +func TestSynthesisFunctionsOriginalBehavior(t *testing.T) { + t.Run("GetStructFieldGodocs", func(t *testing.T) { + _, rb, typ := kindsBuilder(t, "Widget") + + docs, err := rb.getStructFieldGodocs(typ) + if err != nil { + t.Fatalf("getStructFieldGodocs failed: %v", err) + } + if !strings.Contains(docs["Name"], "Name is the widget name") { + t.Errorf("expected field godoc for Name, got %q", docs["Name"]) + } + if _, ok := docs["DoublePtr"]; ok { + t.Error("fields without godoc should not appear in the map") + } + }) + + t.Run("GetGodocForType", func(t *testing.T) { + _, rb, typ := kindsBuilder(t, "Widget") + + doc, err := rb.getGodocForType(typ) + if err != nil { + t.Fatalf("getGodocForType failed: %v", err) + } + if !strings.Contains(doc, "exercises the field kinds") { + t.Errorf("expected type godoc, got %q", doc) + } + }) + + t.Run("BuildRepresentationBasicTypes", func(t *testing.T) { + // Test buildRepresentation with basic Go types + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Test basic types that should work without external dependencies + basicTypes := map[string]types.Type{ + "bool": types.Typ[types.Bool], + "int": types.Typ[types.Int], + "uint": types.Typ[types.Uint], + "float64": types.Typ[types.Float64], + "complex128": types.Typ[types.Complex128], + "string": types.Typ[types.String], + } + + for name, typ := range basicTypes { + t.Run(name, func(t *testing.T) { + rep, err := rb.buildRepresentation(typ) + if err != nil { + t.Errorf("buildRepresentation failed for %s: %v", name, err) + } else if rep == nil { + t.Errorf("buildRepresentation returned nil for %s", name) + } else { + t.Logf("✓ %s -> Type: %s", name, rep.Type) + } + }) + } + }) + + t.Run("BuildRepresentationInterfaces", func(t *testing.T) { + // Test buildRepresentation with interface types + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Create an interface type + interfaceType := types.NewInterfaceType(nil, nil) + + rep, err := rb.buildRepresentation(interfaceType) + if err != nil { + t.Errorf("buildRepresentation failed for interface: %v", err) + } else if rep == nil { + t.Error("expected non-nil empty representation for interface") + } else if rep.Type != "" { + t.Errorf("expected empty type for interface, got %s", rep.Type) + } + }) + + t.Run("BuildRepresentationPointers", func(t *testing.T) { + // Test buildRepresentation with pointer types + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Create pointer to string + stringType := types.Typ[types.String] + ptrType := types.NewPointer(stringType) + + rep, err := rb.buildRepresentation(ptrType) + if err != nil { + t.Errorf("buildRepresentation failed for pointer: %v", err) + } else if rep == nil { + t.Error("buildRepresentation returned nil for pointer") + } else if rep.Type != String { + t.Errorf("Expected String type for *string, got %s", rep.Type) + } else { + t.Log("✓ Pointer types dereference to underlying type") + } + }) + + t.Run("BuildRepresentationSlices", func(t *testing.T) { + // Test buildRepresentation with slice types + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Create []string slice + stringType := types.Typ[types.String] + sliceType := types.NewSlice(stringType) + + rep, err := rb.buildRepresentation(sliceType) + if err != nil { + t.Errorf("buildRepresentation failed for slice: %v", err) + } else if rep == nil { + t.Error("buildRepresentation returned nil for slice") + } else if rep.Type != Array { + t.Errorf("Expected Array type for []string, got %s", rep.Type) + } else if rep.Elems == nil { + t.Error("Expected Elems to be set for array type") + } else if rep.Elems.Type != String { + t.Errorf("Expected String element type, got %s", rep.Elems.Type) + } else { + t.Log("✓ Slice types become Array with proper element type") + } + }) + + t.Run("BuildRepresentationMaps", func(t *testing.T) { + // Test buildRepresentation with map types + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + // Create map[string]int + stringType := types.Typ[types.String] + intType := types.Typ[types.Int] + mapType := types.NewMap(stringType, intType) + + rep, err := rb.buildRepresentation(mapType) + if err != nil { + t.Errorf("buildRepresentation failed for map: %v", err) + } else if rep == nil { + t.Error("buildRepresentation returned nil for map") + } else if rep.Type != Map { + t.Errorf("Expected Map type for map[string]int, got %s", rep.Type) + } else if rep.MapKeys == nil { + t.Error("Expected MapKeys to be set") + } else if rep.MapKeys.Type != String { + t.Errorf("Expected String key type, got %s", rep.MapKeys.Type) + } else if rep.Elems == nil { + t.Error("Expected Elems to be set") + } else if rep.Elems.Type != Int { + t.Errorf("Expected Int element type, got %s", rep.Elems.Type) + } else { + t.Log("✓ Map types have proper key and element types") + } + }) + + t.Run("ModuleMapDetection", func(t *testing.T) { + // map[string]json.RawMessage must become ModuleMap; any other map + // stays a plain Map + driver := New(newMemStorage()) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + rawMsg := types.NewNamed( + types.NewTypeName(0, types.NewPackage("encoding/json", "json"), "RawMessage", nil), + types.NewSlice(types.Typ[types.Byte]), nil, + ) + moduleMap := types.NewMap(types.Typ[types.String], rawMsg) + + rep, err := rb.buildRepresentation(moduleMap) + if err != nil { + t.Fatalf("buildRepresentation failed for map[string]json.RawMessage: %v", err) + } + if rep == nil || rep.Type != ModuleMap { + t.Errorf("expected ModuleMap, got %+v", rep) + } + + regularMap := types.NewMap(types.Typ[types.String], types.Typ[types.Int]) + rep, err = rb.buildRepresentation(regularMap) + if err != nil { + t.Fatalf("buildRepresentation failed for map[string]int: %v", err) + } + if rep == nil || rep.Type != Map { + t.Errorf("expected plain Map, got %+v", rep) + } + }) + + t.Run("UnknownTypeHandling", func(t *testing.T) { + // Test handling of unknown/unsupported types + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + t.Log("Testing unknown type handling:") + t.Log("- Types with no JSON representation fall back to an empty value") + + // Create a channel type (no JSON representation) + chanType := types.NewChan(types.SendRecv, types.Typ[types.String]) + + rep, err := rb.buildRepresentation(chanType) + if err != nil { + t.Errorf("channel type should fall back gracefully, got error: %v", err) + } else if rep == nil { + t.Error("expected non-nil fallback representation for channel type") + } else { + t.Logf("✓ Graceful fallback for channel type: %+v", rep) + } + + // Create a function type (no JSON representation) + sig := types.NewSignature(nil, nil, nil, false) + funcType := types.NewSignature(nil, types.NewTuple(), types.NewTuple(), false) + _ = funcType // Avoid unused variable + _ = sig // Avoid unused variable + + t.Log("✓ Unsupported types (channels, funcs, etc.) no longer fail the containing type") + }) +} + +// TestRepresentationBuilderOriginalBehavior tests the representationBuilder struct +// TestRunGoListOriginalBehavior tests the runGoList utility function +func TestRunGoListOriginalBehavior(t *testing.T) { + t.Run("RunGoListBehavior", func(t *testing.T) { + tempDir := "/tmp" + invalidPkg := "invalid.package.name.that.does.not.exist" + + output, err := runGoList(tempDir, invalidPkg) + if err == nil { + t.Fatalf("expected error for invalid package, got: %+v", output) + } + // stderr is embedded in the error between >>>>>> markers + if !strings.Contains(err.Error(), ">>>>>>") { + t.Errorf("expected stderr markers in error, got: %v", err) + } + }) +} + +// TestSynthesisConstantsOriginalBehavior tests constants and package-level values +func TestSynthesisConstantsOriginalBehavior(t *testing.T) { + t.Run("CaddyCorePackagePath", func(t *testing.T) { + expectedPath := "github.com/caddyserver/caddy/v2" + if caddyCorePackagePath != expectedPath { + t.Errorf("Expected caddyCorePackagePath=%q, got %q", expectedPath, caddyCorePackagePath) + } + }) +} + +// TestSynthesisIntegrationOriginalBehavior tests integration between synthesis components +func TestSynthesisIntegrationOriginalBehavior(t *testing.T) { + t.Run("RepresentationBuilderIntegration", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + if rb.ws.driver != driver { + t.Error("representationBuilder should reference correct driver") + } + if rb.versionCache == nil { + t.Error("versionCache should be initialized") + } + }) +} diff --git a/synthesis_edge_test.go b/synthesis_edge_test.go new file mode 100644 index 0000000..302dc05 --- /dev/null +++ b/synthesis_edge_test.go @@ -0,0 +1,154 @@ +package moduledoc + +import ( + "go/types" + "os" + "strings" + "testing" +) + +const kindsPackagePath = "github.com/caddyserver/moduledoc/testdata/kinds" + +// kindsBuilder loads the kinds fixture package and returns a builder +// plus the named type, using an in-memory storage on the driver. +func kindsBuilder(t *testing.T, typeName string) (*memStorage, representationBuilder, types.Type) { + t.Helper() + if testing.Short() { + t.Skip("requires the Go toolchain") + } + db := newMemStorage() + d := New(db) + ws := localWorkspace(t, d) + pkgs, err := ws.getPackages(kindsPackagePath, "") + if err != nil { + t.Fatalf("loading kinds package: %v", err) + } + obj := pkgs[0].Types.Scope().Lookup(typeName) + if obj == nil { + t.Fatalf("type %s not found in kinds package", typeName) + } + return db, ws.representationBuilder(), obj.Type() +} + +func structField(rep *Value, key string) *StructField { + for _, sf := range rep.StructFields { + if sf.Key == key { + return sf + } + } + return nil +} + +func TestBuildRepresentationWidgetFields(t *testing.T) { + db, rb, typ := kindsBuilder(t, "Widget") + + ref, err := rb.buildRepresentation(typ) + if err != nil { + t.Fatalf("building representation: %v", err) + } + if ref.SameAs == "" { + t.Fatalf("expected reference to stored type, got %#v", ref) + } + rep, err := db.GetTypeByName(kindsPackagePath, "Widget", "") + if err != nil || rep == nil { + t.Fatalf("stored Widget type not found: %v", err) + } + + if rep.Type != Struct { + t.Fatalf("expected struct, got %#v", rep.Type) + } + if !strings.Contains(rep.Doc, "exercises the field kinds") { + t.Errorf("type godoc missing, got %q", rep.Doc) + } + + if sf := structField(rep, "name"); sf == nil || sf.Value.Type != String { + t.Errorf("expected string field 'name', got %#v", sf) + } else if !strings.Contains(sf.Doc, "Name is the widget name") { + t.Errorf("field godoc missing, got %q", sf.Doc) + } + + for _, absent := range []string{"Hidden", "-", "unexported"} { + if sf := structField(rep, absent); sf != nil { + t.Errorf("field %q should not be documented", absent) + } + } + + if sf := structField(rep, "double_ptr"); sf == nil || sf.Value.Type != Int { + t.Errorf("pointer-to-pointer field should unwrap to int, got %#v", sf) + } + if sf := structField(rep, "numbers"); sf == nil || sf.Value.Type != Array || sf.Value.Elems == nil || sf.Value.Elems.Type != Int { + t.Errorf("expected array of int, got %#v", sf) + } + if sf := structField(rep, "lookup"); sf == nil || sf.Value.Type != Map || + sf.Value.MapKeys == nil || sf.Value.MapKeys.Type != Int || + sf.Value.Elems == nil || sf.Value.Elems.Type != String { + t.Errorf("expected map with int keys and string values, got %#v", sf) + } + if sf := structField(rep, "anything"); sf == nil || sf.Value.Type != "" { + t.Errorf("interface field should have an empty representation, got %#v", sf) + } + + if sf := structField(rep, "raw"); sf == nil || sf.Value.Type != Module { + t.Errorf("json.RawMessage field should be a module, got %#v", sf) + } else { + if sf.Value.ModuleNamespace == nil || *sf.Value.ModuleNamespace != "widget.raw" { + t.Errorf("module namespace not applied, got %#v", sf.Value.ModuleNamespace) + } + if sf.Value.ModuleInlineKey == nil || *sf.Value.ModuleInlineKey != "kind" { + t.Errorf("module inline key not applied, got %#v", sf.Value.ModuleInlineKey) + } + } + if sf := structField(rep, "raw_map"); sf == nil || sf.Value.Type != ModuleMap { + t.Errorf("map of json.RawMessage should be a module map, got %#v", sf) + } else if sf.Value.ModuleNamespace == nil || *sf.Value.ModuleNamespace != "widget.rawmap" { + t.Errorf("module map namespace not applied, got %#v", sf.Value.ModuleNamespace) + } + + if sf := structField(rep, "extra"); sf == nil || sf.Value.Type != Bool { + t.Errorf("embedded struct fields should be promoted, got %#v", sf) + } + if sf := structField(rep, "Embedded"); sf != nil { + t.Error("embedded type itself should not appear as a field") + } + + if sf := structField(rep, "nested"); sf == nil || !strings.Contains(sf.Value.SameAs, "kinds.Nested") { + t.Errorf("named nested type should be stored by reference, got %#v", sf) + } + if nested, _ := db.GetTypeByName(kindsPackagePath, "Nested", ""); nested == nil { + t.Error("nested named type should have been stored") + } + + if sf := structField(rep, "inline"); sf == nil || sf.Value.Type != Struct || structField(sf.Value, "a") == nil { + t.Errorf("inline anonymous struct should be represented with its fields, got %#v", sf) + } +} + +func TestBuildRepresentationUnsupportedFieldTypes(t *testing.T) { + db, rb, typ := kindsBuilder(t, "Tricky") + + // function- and channel-typed fields cannot appear in JSON config, + // but they must not prevent documenting the rest of the type + if _, err := rb.buildRepresentation(typ); err != nil { + t.Fatalf("expected graceful handling of func/chan fields, got error: %v", err) + } + rep, err := db.GetTypeByName(kindsPackagePath, "Tricky", "") + if err != nil || rep == nil { + t.Fatalf("stored Tricky type not found: %v", err) + } +} + +func TestBuildRepresentationRecursiveType(t *testing.T) { + if os.Getenv("MODULEDOC_TEST_RECURSIVE_TYPE_CHILD") == "1" { + _, rb, typ := kindsBuilder(t, "Node") + // success or a clean error are both fine; a crash is not + if _, err := rb.buildRepresentation(typ); err != nil { + t.Logf("recursive type produced error: %v", err) + } + return + } + if testing.Short() { + t.Skip("requires the Go toolchain") + } + runIsolated(t, "TestBuildRepresentationRecursiveType", "MODULEDOC_TEST_RECURSIVE_TYPE_CHILD", + "a self-referential struct type must not crash representation building") +} diff --git a/synthesis_real_test.go b/synthesis_real_test.go new file mode 100644 index 0000000..2d3f51d --- /dev/null +++ b/synthesis_real_test.go @@ -0,0 +1,345 @@ +package moduledoc + +import ( + "encoding/json" + "go/types" + "os" + "path/filepath" + "strings" + "testing" +) + +// TestSynthesisRealFunctionsOriginalBehavior tests synthesis functions with actual data +func TestSynthesisRealFunctionsOriginalBehavior(t *testing.T) { + t.Run("RunGoListWithRealPackage", func(t *testing.T) { + // Create a temporary workspace directory + tempDir, err := os.MkdirTemp("", "moduledoc-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + // Create a basic go.mod file + goModContent := `module test.example +go 1.19 +` + goModPath := filepath.Join(tempDir, "go.mod") + if err := os.WriteFile(goModPath, []byte(goModContent), 0o644); err != nil { + t.Fatalf("Failed to write go.mod: %v", err) + } + + t.Log("Testing runGoList behavior:") + t.Log("- Executes 'go list -json ' in workspace") + t.Log("- Returns goListOutput struct with module information") + t.Log("- Handles standard library packages differently") + + // Test with standard library package + output, err := runGoList(tempDir, "fmt") + if err != nil { + t.Logf("runGoList failed for fmt package (expected in temp workspace): %v", err) + } else { + t.Logf("✓ fmt package info: ImportPath=%s, Standard=%v", output.ImportPath, output.Standard) + if output.ImportPath == "fmt" { + t.Log("✓ Standard library package detected correctly") + } + if output.Standard { + t.Log("✓ Standard flag set correctly") + } + } + + // Test with non-existent package + _, err = runGoList(tempDir, "non.existent.package/v999") + if err != nil { + t.Logf("✓ runGoList properly fails for non-existent package: %v", err) + if strings.Contains(err.Error(), ">>>>>>") { + t.Log("✓ Error formatting includes stderr markers") + } + } else { + t.Error("Expected error for non-existent package") + } + }) + + t.Run("GoListOutputStructValidation", func(t *testing.T) { + t.Log("Testing goListOutput struct fields and JSON unmarshaling:") + + // Test JSON unmarshaling with sample data + jsonData := `{ + "Dir": "/test/dir", + "ImportPath": "example.com/test", + "Name": "test", + "Module": { + "Path": "example.com/test", + "Version": "v1.0.0" + }, + "Standard": false, + "GoFiles": ["main.go", "test.go"] + }` + + var output goListOutput + if err := json.Unmarshal([]byte(jsonData), &output); err != nil { + t.Errorf("Failed to unmarshal test JSON: %v", err) + } else { + t.Log("✓ JSON unmarshaling works correctly") + if output.Dir == "/test/dir" { + t.Log("✓ Dir field parsed correctly") + } + if output.Module.Version == "v1.0.0" { + t.Log("✓ Nested Module.Version field parsed correctly") + } + if len(output.GoFiles) == 2 { + t.Log("✓ GoFiles slice parsed correctly") + } + } + }) + + t.Run("RepresentationBuilderVersionCache", func(t *testing.T) { + // Test the version cache behavior with real representationBuilder + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + if rb.versionCache == nil { + t.Fatal("versionCache should be initialized") + } + + // standard library types resolve to an empty version and are + // cached under their import path + stdType := types.NewNamed( + types.NewTypeName(0, types.NewPackage("encoding/json", "json"), "RawMessage", nil), + types.NewSlice(types.Typ[types.Byte]), nil, + ) + + version, err := rb.getDepVersion(stdType) + if err != nil { + t.Fatalf("getDepVersion failed for stdlib type: %v", err) + } + if version != "" { + t.Errorf("stdlib types should have empty version, got %q", version) + } + if _, ok := rb.versionCache["encoding/json"]; !ok { + t.Error("stdlib version should be cached under the import path") + } + }) +} + +// TestBuildRepresentationWithRealTypes tests buildRepresentation with real Go types +func TestBuildRepresentationWithRealTypes(t *testing.T) { + t.Run("BasicTypeRepresentations", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + t.Log("Testing buildRepresentation with basic Go types:") + + // Test all basic types + basicTests := []struct { + name string + typ types.Type + expected Type + }{ + {"bool", types.Typ[types.Bool], Bool}, + {"int", types.Typ[types.Int], Int}, + {"int8", types.Typ[types.Int8], Int}, + {"int16", types.Typ[types.Int16], Int}, + {"int32", types.Typ[types.Int32], Int}, + {"int64", types.Typ[types.Int64], Int}, + {"uint", types.Typ[types.Uint], Uint}, + {"uint8", types.Typ[types.Uint8], Uint}, + {"uint16", types.Typ[types.Uint16], Uint}, + {"uint32", types.Typ[types.Uint32], Uint}, + {"uint64", types.Typ[types.Uint64], Uint}, + {"uintptr", types.Typ[types.Uintptr], Uint}, + {"float32", types.Typ[types.Float32], Float}, + {"float64", types.Typ[types.Float64], Float}, + {"complex64", types.Typ[types.Complex64], Complex}, + {"complex128", types.Typ[types.Complex128], Complex}, + {"string", types.Typ[types.String], String}, + } + + for _, tt := range basicTests { + t.Run(tt.name, func(t *testing.T) { + rep, err := rb.buildRepresentation(tt.typ) + if err != nil { + t.Errorf("buildRepresentation failed for %s: %v", tt.name, err) + } else if rep == nil { + t.Errorf("buildRepresentation returned nil for %s", tt.name) + } else if rep.Type != tt.expected { + t.Errorf("Expected Type %s for %s, got %s", tt.expected, tt.name, rep.Type) + } else { + t.Logf("✓ %s -> %s", tt.name, rep.Type) + } + }) + } + }) + + t.Run("ComplexTypeRepresentations", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + t.Log("Testing buildRepresentation with complex Go types:") + + // Test pointer to string + stringType := types.Typ[types.String] + ptrType := types.NewPointer(stringType) + rep, err := rb.buildRepresentation(ptrType) + if err != nil { + t.Errorf("Pointer representation failed: %v", err) + } else if rep.Type != String { + t.Errorf("Expected String for *string, got %s", rep.Type) + } else { + t.Log("✓ *string -> String (pointer dereferenced)") + } + + // Test []string slice + sliceType := types.NewSlice(stringType) + rep, err = rb.buildRepresentation(sliceType) + if err != nil { + t.Errorf("Slice representation failed: %v", err) + } else if rep.Type != Array { + t.Errorf("Expected Array for []string, got %s", rep.Type) + } else if rep.Elems == nil || rep.Elems.Type != String { + t.Error("Expected String element type for []string") + } else { + t.Log("✓ []string -> Array with String elements") + } + + // Test map[string]int + intType := types.Typ[types.Int] + mapType := types.NewMap(stringType, intType) + rep, err = rb.buildRepresentation(mapType) + if err != nil { + t.Errorf("Map representation failed: %v", err) + } else if rep.Type != Map { + t.Errorf("Expected Map for map[string]int, got %s", rep.Type) + } else if rep.MapKeys == nil || rep.MapKeys.Type != String { + t.Error("Expected String key type") + } else if rep.Elems == nil || rep.Elems.Type != Int { + t.Error("Expected Int element type") + } else { + t.Log("✓ map[string]int -> Map with String keys, Int elements") + } + + // Test interface{} + emptyInterface := types.NewInterfaceType(nil, nil) + rep, err = rb.buildRepresentation(emptyInterface) + if err != nil { + t.Errorf("Interface representation failed: %v", err) + } else if rep == nil { + t.Log("✓ interface{} -> nil (original behavior)") + } else { + t.Logf("Interface representation: %+v", rep) + } + }) + + t.Run("UnsupportedTypeHandling", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + + t.Log("Testing buildRepresentation with unsupported types:") + + // Test channel type + stringType := types.Typ[types.String] + chanType := types.NewChan(types.SendRecv, stringType) + rep, err := rb.buildRepresentation(chanType) + if err != nil { + t.Errorf("channel type should fall back gracefully, got error: %v", err) + } else if rep == nil { + t.Error("expected non-nil fallback representation for channel type") + } else { + t.Log("✓ Graceful fallback for channel type") + } + + // Test function type + sig := types.NewSignature(nil, types.NewTuple(), types.NewTuple(), false) + rep, err = rb.buildRepresentation(sig) + if err != nil { + t.Errorf("function type should fall back gracefully, got error: %v", err) + } else if rep == nil { + t.Error("expected non-nil fallback representation for function type") + } else { + t.Log("✓ Graceful fallback for function type") + } + }) +} + +// TestCaddyCorePackagePathConstant tests the constant usage +func TestCaddyCorePackagePathConstant(t *testing.T) { + expectedPath := "github.com/caddyserver/caddy/v2" + if caddyCorePackagePath != expectedPath { + t.Errorf("Expected caddyCorePackagePath=%q, got %q", expectedPath, caddyCorePackagePath) + } +} + +// TestSynthesisErrorHandlingOriginalBehavior tests error conditions +func TestSynthesisErrorHandlingOriginalBehavior(t *testing.T) { + // a type whose package cannot be resolved by go list + missingType := func() *types.Named { + return types.NewNamed( + types.NewTypeName(0, types.NewPackage("invalid.example/does/not/exist", "exist"), "Ghost", nil), + types.Typ[types.String], nil, + ) + } + + t.Run("GetStructFieldGodocsErrors", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + if _, err := rb.getStructFieldGodocs(missingType()); err == nil { + t.Error("expected error for unresolvable package") + } + }) + + t.Run("GetGodocForTypeErrors", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + if _, err := rb.getGodocForType(missingType()); err == nil { + t.Error("expected error for unresolvable package") + } + }) + + t.Run("GetDepVersionErrors", func(t *testing.T) { + driver := New(nil) + ws, err := driver.openWorkspace() + if err != nil { + t.Fatalf("Failed to create workspace: %v", err) + } + defer ws.Close() + + rb := ws.representationBuilder() + if _, err := rb.getDepVersion(missingType()); err == nil { + t.Error("expected error when go list cannot resolve the package") + } + }) +} diff --git a/testdata/aliases/aliases.go b/testdata/aliases/aliases.go index b1641da..c9812bb 100644 --- a/testdata/aliases/aliases.go +++ b/testdata/aliases/aliases.go @@ -1,6 +1,12 @@ // Package aliases is a test fixture exercising type-alias handling in // moduledoc under Go 1.23+ (where go/types materialises aliases as // *types.Alias). +// +// Expected (see synthesis_test.go, utils_test.go): buildRepresentation +// unwraps aliases so Config.Nested resolves to the Settings struct, +// Config.Label resolves to a plain string, and Config.Items resolves +// to a slice of the Item struct — the alias names themselves do not +// appear as distinct types. package aliases // Settings holds nested configuration used to verify alias unwrapping. diff --git a/testdata/constid/module.go b/testdata/constid/module.go new file mode 100644 index 0000000..160af7d --- /dev/null +++ b/testdata/constid/module.go @@ -0,0 +1,29 @@ +// Package constid is a module whose ID is a package-level constant +// rather than a string literal, exercising constant-expression +// evaluation in module ID discovery. +// +// Expected: findCaddyModuleIdents discovers ConstWidget as +// "app.namespace.const_widget". +package constid + +import "github.com/caddyserver/caddy/v2" + +const widgetModuleID = "app.namespace.const_widget" + +func init() { + caddy.RegisterModule(new(ConstWidget)) +} + +type ConstWidget struct{} + +// CaddyModule implements caddy.Module +func (*ConstWidget) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: widgetModuleID, + New: func() caddy.Module { + return new(ConstWidget) + }, + } +} + +var _ caddy.Module = (*ConstWidget)(nil) diff --git a/testdata/crosspkg/module.go b/testdata/crosspkg/module.go new file mode 100644 index 0000000..8cf7e76 --- /dev/null +++ b/testdata/crosspkg/module.go @@ -0,0 +1,16 @@ +// Package crosspkg registers a type declared in another package +// (a qualified composite literal, valuerecv.Sprocket{}), exercising +// the AST handling of selector-expression registration arguments. +// +// Expected: findCaddyModuleIdents must not panic; it may return an +// error (no local CaddyModule method) or an empty result. +package crosspkg + +import ( + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/moduledoc/testdata/valuerecv" +) + +func init() { + caddy.RegisterModule(valuerecv.Sprocket{}) +} diff --git a/testdata/exprid/module.go b/testdata/exprid/module.go new file mode 100644 index 0000000..6c8744f --- /dev/null +++ b/testdata/exprid/module.go @@ -0,0 +1,29 @@ +// Package exprid is a module whose ID is built from concatenating a +// package-level constant with a string literal, exercising constant +// binary-expression evaluation in module ID discovery. +// +// Expected: findCaddyModuleIdents discovers ExprWidget as +// "app.namespace.expr_widget". +package exprid + +import "github.com/caddyserver/caddy/v2" + +const idPrefix = "app.namespace." + +func init() { + caddy.RegisterModule(new(ExprWidget)) +} + +type ExprWidget struct{} + +// CaddyModule implements caddy.Module +func (*ExprWidget) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: idPrefix + "expr_widget", + New: func() caddy.Module { + return new(ExprWidget) + }, + } +} + +var _ caddy.Module = (*ExprWidget)(nil) diff --git a/testdata/gizmo.go b/testdata/gizmo.go new file mode 100644 index 0000000..fc2a6e8 --- /dev/null +++ b/testdata/gizmo.go @@ -0,0 +1,26 @@ +// Package testdata is the canonical minimal valid Caddy module: a +// pointer-receiver type with both caddy.RegisterModule and CaddyModule, +// and a static string literal module ID. +// +// Expected: findCaddyModuleIdents discovers Gizmo as "app.namespace.gizmo". +package testdata + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(Gizmo)) +} + +type Gizmo struct{} + +// CaddyModule implements caddy.Module +func (*Gizmo) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.namespace.gizmo", + New: func() caddy.Module { + return new(Gizmo) + }, + } +} + +var _ caddy.Module = (*Gizmo)(nil) diff --git a/testdata/kinds/types.go b/testdata/kinds/types.go new file mode 100644 index 0000000..a16829e --- /dev/null +++ b/testdata/kinds/types.go @@ -0,0 +1,67 @@ +// Package kinds exercises the struct field kinds moduledoc's type +// synthesis must handle: pointer-to-pointer, unexported and json:"-" +// fields, non-string map keys, interface{}, json.RawMessage as a Caddy +// module (with namespace/inline_key tags), embedded field promotion, +// named vs. inline anonymous structs, self-referential types, and +// field kinds with no JSON representation (chan, func). +// +// Expected (see synthesis_edge_test.go): Hidden, unexported, and +// Embedded are absent from the built representation; DoublePtr +// resolves to Int; Lookup is a Map with Int keys; Anything is an +// empty Value; Raw is a Module with namespace "widget.raw" and +// inline key "kind"; RawMap is a ModuleMap; Extra is promoted onto +// the parent struct; Nested is stored and referenced by name; Inline +// is represented with its own fields; Node builds without recursing +// forever; Tricky's chan/func fields build without erroring. +package kinds + +import "encoding/json" + +// Widget exercises the field kinds the doc system should handle. +type Widget struct { + // Name is the widget name. + Name string `json:"name"` + + // Hidden is excluded from JSON. + Hidden string `json:"-"` + + unexported string + + DoublePtr **int `json:"double_ptr,omitempty"` + Numbers []int `json:"numbers,omitempty"` + Lookup map[int]string `json:"lookup,omitempty"` + Anything interface{} `json:"anything,omitempty"` + + Raw json.RawMessage `json:"raw,omitempty" caddy:"namespace=widget.raw inline_key=kind"` + RawMap map[string]json.RawMessage `json:"raw_map,omitempty" caddy:"namespace=widget.rawmap"` + + Embedded + + Nested Nested `json:"nested,omitempty"` + + Inline struct { + A string `json:"a"` + } `json:"inline,omitempty"` +} + +// Embedded is a mixin whose fields are promoted. +type Embedded struct { + // Extra adds extra behavior. + Extra bool `json:"extra,omitempty"` +} + +// Nested is a named nested type. +type Nested struct { + B int `json:"b"` +} + +// Node is a self-referential type. +type Node struct { + Next *Node `json:"next,omitempty"` +} + +// Tricky has field kinds with no JSON representation. +type Tricky struct { + Notify func() `json:"notify,omitempty"` + Events chan int `json:"events,omitempty"` +} diff --git a/testdata/noimpl/module.go b/testdata/noimpl/module.go new file mode 100644 index 0000000..8df6dbb --- /dev/null +++ b/testdata/noimpl/module.go @@ -0,0 +1,21 @@ +// Package noimpl registers a type that satisfies caddy.Module only +// through an embedded field, so the package has a registration but no +// locally declared CaddyModule method. +// +// Expected: findCaddyModuleIdents returns an error. +package noimpl + +import ( + "github.com/caddyserver/caddy/v2" + "github.com/caddyserver/moduledoc/testdata" +) + +func init() { + caddy.RegisterModule(new(Inherited)) +} + +// Inherited satisfies caddy.Module through embedding, so this +// package has a registration but no local CaddyModule method. +type Inherited struct { + testdata.Gizmo +} diff --git a/testdata/unregistered/module.go b/testdata/unregistered/module.go new file mode 100644 index 0000000..ba66b9f --- /dev/null +++ b/testdata/unregistered/module.go @@ -0,0 +1,38 @@ +// Package unregistered pairs one fully compliant module with one type +// that implements CaddyModule but is never registered, exercising the +// strict validation that fails an entire package on partial compliance. +// +// Expected: findCaddyModuleIdents returns an error for the whole +// package, even though Good is otherwise fully compliant. +package unregistered + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(Good)) +} + +type Good struct{} + +// CaddyModule implements caddy.Module +func (*Good) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.namespace.good", + New: func() caddy.Module { + return new(Good) + }, + } +} + +// Orphan implements caddy.Module but is never registered. +type Orphan struct{} + +// CaddyModule implements caddy.Module +func (*Orphan) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.namespace.orphan", + New: func() caddy.Module { + return new(Orphan) + }, + } +} diff --git a/testdata/valuerecv/module.go b/testdata/valuerecv/module.go new file mode 100644 index 0000000..8370117 --- /dev/null +++ b/testdata/valuerecv/module.go @@ -0,0 +1,27 @@ +// Package valuerecv registers a module via a value receiver and a +// composite literal (caddy.RegisterModule(Sprocket{})), rather than +// the more common pointer-receiver plus new(...) pattern. +// +// Expected: findCaddyModuleIdents discovers Sprocket as +// "app.namespace.sprocket". +package valuerecv + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(Sprocket{}) +} + +type Sprocket struct{} + +// CaddyModule implements caddy.Module +func (Sprocket) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.namespace.sprocket", + New: func() caddy.Module { + return Sprocket{} + }, + } +} + +var _ caddy.Module = Sprocket{} diff --git a/traverse_edge_test.go b/traverse_edge_test.go new file mode 100644 index 0000000..54ef1a5 --- /dev/null +++ b/traverse_edge_test.go @@ -0,0 +1,175 @@ +package moduledoc + +import ( + "strings" + "testing" +) + +func newConfigTree() (*memStorage, *Value) { + db := newMemStorage() + + ns := "http.handlers" + ik := "handler" + + root := &Value{ + Type: Struct, + TypeName: "example.com/pkg.Config", + StructFields: []*StructField{ + { + Key: "listen", + Value: &Value{ + Type: String, + }, + Doc: "listen doc", + }, + { + Key: "servers", + Value: &Value{ + Type: Map, + MapKeys: &Value{Type: String}, + Elems: &Value{ + Type: Struct, + TypeName: "example.com/pkg.Server", + StructFields: []*StructField{ + {Key: "port", Value: &Value{Type: Int}}, + }, + }, + }, + }, + { + Key: "handlers", + Value: &Value{ + Type: Array, + Elems: &Value{ + Type: Module, + ModuleNamespace: &ns, + ModuleInlineKey: &ik, + }, + }, + }, + }, + } + + db.modules["http.handlers.file_server"] = []*Value{ + { + Type: Struct, + TypeName: "example.com/fileserver.FileServer", + StructFields: []*StructField{ + {Key: "root", Value: &Value{Type: String}}, + }, + }, + } + + return db, root +} + +func TestTraverseTypeStartValidation(t *testing.T) { + d := New(newMemStorage()) + if _, _, err := d.TraverseType("a/b", &Value{}); err == nil { + t.Error("expected error when starting from an untyped value") + } +} + +func TestTraverseTypeEmptyPathReturnsStart(t *testing.T) { + db, root := newConfigTree() + d := New(db) + val, nearest, err := d.TraverseType("", root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if val != root || nearest != root { + t.Error("empty path should return the start value as both results") + } +} + +func TestTraverseTypeStructField(t *testing.T) { + db, root := newConfigTree() + d := New(db) + + val, nearest, err := d.TraverseType("listen", root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if val.Type != String { + t.Errorf("expected string value at path, got %#v", val) + } + if !strings.Contains(val.Doc, "listen doc") { + t.Errorf("struct field doc should be included at target, got %q", val.Doc) + } + if nearest.TypeName != "example.com/pkg.Config" { + t.Errorf("nearest type should be the containing config type, got %q", nearest.TypeName) + } +} + +func TestTraverseTypeUnknownStructField(t *testing.T) { + db, root := newConfigTree() + d := New(db) + if _, _, err := d.TraverseType("nope", root); err == nil { + t.Error("expected error for unknown struct field") + } +} + +func TestTraverseTypeThroughMapContainer(t *testing.T) { + db, root := newConfigTree() + d := New(db) + + // maps and arrays are transparent containers: the next path + // part applies to the element type, not a key or index + val, nearest, err := d.TraverseType("servers/port", root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if val.Type != Int { + t.Errorf("expected int value at path, got %#v", val) + } + if nearest.TypeName != "example.com/pkg.Server" { + t.Errorf("nearest type should be the map's element type, got %q", nearest.TypeName) + } +} + +func TestTraverseTypeModuleLookup(t *testing.T) { + db, root := newConfigTree() + d := New(db) + + val, _, err := d.TraverseType("handlers/file_server", root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if val.TypeName != "example.com/fileserver.FileServer" { + t.Errorf("expected module type at path, got %#v", val) + } + if val.ModuleInlineKey == nil || *val.ModuleInlineKey != "handler" { + t.Errorf("inline key should be set on module resolved at the final path part, got %#v", val.ModuleInlineKey) + } + + // traversal into the module's own fields + val, _, err = d.TraverseType("handlers/file_server/root", root) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if val.Type != String { + t.Errorf("expected string value inside module, got %#v", val) + } +} + +func TestTraverseTypeUnknownModuleID(t *testing.T) { + db, root := newConfigTree() + d := New(db) + + defer func() { + if r := recover(); r != nil { + t.Fatalf("traversal to an unknown module ID must return an error, not panic: %v", r) + } + }() + if _, _, err := d.TraverseType("handlers/does_not_exist", root); err == nil { + t.Error("expected error for unknown module ID") + } +} + +func TestTraverseTypeNotTraversable(t *testing.T) { + db, root := newConfigTree() + d := New(db) + if _, _, err := d.TraverseType("listen/deeper", root); err == nil { + t.Error("expected error when traversing into a primitive value") + } +} diff --git a/utils.go b/utils.go index f0f0ad4..961459f 100644 --- a/utils.go +++ b/utils.go @@ -26,10 +26,9 @@ import ( // If there is no dot, then before will be empty string and after // will be the input. Examples: // -// "github.com/caddyserver/caddy/v2.Config" => ("github.com/caddyserver/caddy/v2", "Config") -// "http.handlers.file_server" => ("http.handlers", "file_server") -// "http" => ("", "http") -// +// "github.com/caddyserver/caddy/v2.Config" => ("github.com/caddyserver/caddy/v2", "Config") +// "http.handlers.file_server" => ("http.handlers", "file_server") +// "http" => ("", "http") func SplitLastDot(input string) (before, after string) { lastDot := strings.LastIndex(input, ".") if lastDot < 0 { @@ -52,12 +51,14 @@ func ConfigPathParts(configPath string) []string { // by the encoding/json package), then false is returned. func jsonNameFromTag(tagStr string) (string, bool) { jsonName := reflect.StructTag(tagStr).Get("json") - if commaIdx := strings.Index(jsonName, ","); commaIdx > 0 { - jsonName = strings.TrimSpace(jsonName[:commaIdx]) - } + // per encoding/json, only a tag of exactly "-" excludes the field; + // "-," names the field "-" if jsonName == "-" { return "", false } + if commaIdx := strings.Index(jsonName, ","); commaIdx >= 0 { + jsonName = strings.TrimSpace(jsonName[:commaIdx]) + } return jsonName, true } diff --git a/utils_edge_test.go b/utils_edge_test.go new file mode 100644 index 0000000..e84dc4e --- /dev/null +++ b/utils_edge_test.go @@ -0,0 +1,128 @@ +package moduledoc + +import ( + "go/types" + "reflect" + "testing" +) + +func TestSplitLastDotEdgeCases(t *testing.T) { + for _, tc := range []struct { + input, before, after string + }{ + {"github.com/caddyserver/caddy/v2.Config", "github.com/caddyserver/caddy/v2", "Config"}, + {"http.handlers.file_server", "http.handlers", "file_server"}, + {"http", "", "http"}, + {"", "", ""}, + {".", "", ""}, + {"trailing.", "trailing", ""}, + {".leading", "", "leading"}, + {"a.b.c.d", "a.b.c", "d"}, + {"..", ".", ""}, + } { + before, after := SplitLastDot(tc.input) + if before != tc.before || after != tc.after { + t.Errorf("SplitLastDot(%q) = (%q, %q); want (%q, %q)", + tc.input, before, after, tc.before, tc.after) + } + } +} + +func TestConfigPathPartsEdgeCases(t *testing.T) { + for _, tc := range []struct { + input string + want []string + }{ + {"apps/http/servers", []string{"apps", "http", "servers"}}, + {"/apps/http/", []string{"apps", "http"}}, + {"apps", []string{"apps"}}, + // degenerate inputs collapse to a single empty part + {"", []string{""}}, + {"/", []string{""}}, + {"//", []string{""}}, + // interior empty segments are preserved + {"a//b", []string{"a", "", "b"}}, + } { + got := ConfigPathParts(tc.input) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("ConfigPathParts(%q) = %#v; want %#v", tc.input, got, tc.want) + } + } +} + +func TestJSONNameFromTagEdgeCases(t *testing.T) { + for _, tc := range []struct { + tag string + name string + include bool + }{ + {`json:"listen"`, "listen", true}, + {`json:"listen,omitempty"`, "listen", true}, + {`json:"listen, omitempty"`, "listen", true}, + {`json:"-"`, "", false}, + // "-," means the field is named "-", not excluded + {`json:"-,"`, "-", true}, + // empty name with options means no explicit JSON name + {`json:",omitempty"`, "", true}, + {`json:""`, "", true}, + {``, "", true}, + {`yaml:"listen"`, "", true}, + {`json:"a" caddy:"namespace=http"`, "a", true}, + } { + name, include := jsonNameFromTag(tc.tag) + if name != tc.name || include != tc.include { + t.Errorf("jsonNameFromTag(%q) = (%q, %v); want (%q, %v)", + tc.tag, name, include, tc.name, tc.include) + } + } +} + +func TestCaddyTagFieldsEdgeCases(t *testing.T) { + fields, err := caddyTagFields(`caddy:"namespace=http.handlers inline_key=handler"`) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fields["namespace"] != "http.handlers" || fields["inline_key"] != "handler" { + t.Errorf("unexpected fields: %#v", fields) + } + + fields, err = caddyTagFields(``) + if err != nil { + t.Fatalf("empty tag should not error: %v", err) + } + if len(fields) != 0 { + t.Errorf("empty tag should yield no fields, got %#v", fields) + } + + fields, err = caddyTagFields(`json:"a"`) + if err != nil { + t.Fatalf("tag without caddy key should not error: %v", err) + } + if len(fields) != 0 { + t.Errorf("expected no caddy fields, got %#v", fields) + } +} + +func TestTypeNameHelpersOnNonNamedTypes(t *testing.T) { + basic := types.Typ[types.String] + + if got := fullyQualifiedTypeName(basic); got != "string" { + t.Errorf("fullyQualifiedTypeName(string) = %q; want %q", got, "string") + } + if pkg, name := typePackageAndName(basic); pkg != "" || name != "" { + t.Errorf("typePackageAndName(string) = (%q, %q); want empty", pkg, name) + } + if got := localTypeName(basic); got != "" { + t.Errorf("localTypeName(string) = %q; want empty", got) + } + + ptr := types.NewPointer(basic) + if got := fullyQualifiedTypeName(ptr); got != "*string" { + t.Errorf("fullyQualifiedTypeName(*string) = %q; want %q", got, "*string") + } + + slice := types.NewSlice(basic) + if got := fullyQualifiedTypeName(slice); got != "[]string" { + t.Errorf("fullyQualifiedTypeName([]string) = %q; want %q", got, "[]string") + } +} diff --git a/validation_baseline_test.go b/validation_baseline_test.go new file mode 100644 index 0000000..d908883 --- /dev/null +++ b/validation_baseline_test.go @@ -0,0 +1,142 @@ +package moduledoc + +import ( + "go/ast" + "go/parser" + "go/token" + "go/types" + "testing" + + "golang.org/x/tools/go/packages" +) + +// TestModuleValidationOriginal tests the strict validation logic of the original implementation +func TestModuleValidationOriginal(t *testing.T) { + t.Run("ValidModulePattern", func(t *testing.T) { + // Test the valid pattern: registration + implementation + static ID + source := ` +package test + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(ValidModule)) +} + +type ValidModule struct{} + +func (*ValidModule) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.test.valid", + New: func() caddy.Module { return new(ValidModule) }, + } +} +` + + // This should work with the original implementation + testModuleSource(t, source, true, "Valid module should be detected") + }) + + t.Run("MissingRegistration", func(t *testing.T) { + // Test module with implementation but no registration + source := ` +package test + +import "github.com/caddyserver/caddy/v2" + +type UnregisteredModule struct{} + +func (*UnregisteredModule) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: "app.test.unregistered", + New: func() caddy.Module { return new(UnregisteredModule) }, + } +} +` + + // This should fail in original implementation + testModuleSource(t, source, false, "Module without registration should fail") + }) + + t.Run("MissingImplementation", func(t *testing.T) { + // Test registration without implementation + source := ` +package test + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(IncompleteModule)) +} + +type IncompleteModule struct{} +// Missing CaddyModule() method +` + + // This should fail in original implementation + testModuleSource(t, source, false, "Module without implementation should fail") + }) + + t.Run("NonStaticModuleID", func(t *testing.T) { + // Test computed module ID that should be skipped + source := ` +package test + +import "github.com/caddyserver/caddy/v2" + +func init() { + caddy.RegisterModule(new(ComputedModule)) +} + +type ComputedModule struct{} + +func (*ComputedModule) CaddyModule() caddy.ModuleInfo { + return caddy.ModuleInfo{ + ID: caddy.ModuleID("computed." + "dynamic"), + New: func() caddy.Module { return new(ComputedModule) }, + } +} +` + + // This should be skipped (not fail, but not detected) in original implementation + testModuleSource(t, source, false, "Computed module ID should be skipped") + }) +} + +// testModuleSource is a helper that tests module detection on source code +func testModuleSource(t *testing.T, source string, expectSuccess bool, message string) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "test.go", source, parser.ParseComments) + if err != nil { + t.Fatalf("Failed to parse source: %v", err) + } + + // Create a minimal package for testing + pkg := &packages.Package{ + Name: "test", + Syntax: []*ast.File{file}, + TypesInfo: &types.Info{ + Uses: make(map[*ast.Ident]types.Object), + Defs: make(map[*ast.Ident]types.Object), + }, + } + + driver := New(nil) + + moduleIdents, err := driver.findCaddyModuleIdents(pkg) + + if expectSuccess { + if err != nil { + t.Errorf("%s: Expected success but got error: %v", message, err) + } + if len(moduleIdents) == 0 { + t.Errorf("%s: Expected to find modules but found none", message) + } + t.Logf("%s: ✓ Found %d modules", message, len(moduleIdents)) + } else { + if err == nil && len(moduleIdents) > 0 { + t.Errorf("%s: Expected failure/skip but found modules: %v", message, moduleIdents) + } + t.Logf("%s: ✓ Correctly failed or skipped as expected", message) + } +} diff --git a/workspace.go b/workspace.go index c8419b2..4724408 100644 --- a/workspace.go +++ b/workspace.go @@ -46,6 +46,17 @@ type workspace struct { parsedPackages map[string]*packages.Package } +// packagesLoadMode is the packages.Config.Mode used wherever we need +// full syntax and type information to inspect Caddy modules. +const packagesLoadMode = packages.NeedName | + packages.NeedFiles | + packages.NeedSyntax | + packages.NeedImports | + packages.NeedDeps | + packages.NeedTypes | + packages.NeedTypesInfo | + packages.NeedModule + func (d *Driver) openWorkspace() (workspace, error) { tempDir, err := ioutil.TempDir("", "caddy_docsys_") if err != nil { @@ -121,13 +132,8 @@ func (ws *workspace) getPackages(packagePattern, version string) ([]*packages.Pa // finally, load and parse the package cfg := &packages.Config{ - Dir: ws.dir, - Mode: packages.NeedSyntax | - packages.NeedImports | - packages.NeedDeps | - packages.NeedTypes | - packages.NeedModule | - packages.NeedTypesInfo, + Dir: ws.dir, + Mode: packagesLoadMode, // on Linux, leaving CGO_ENABLED to the default value of 1 would // cause an error: "could not import C (no metadata for C)", but diff --git a/workspace_edge_test.go b/workspace_edge_test.go new file mode 100644 index 0000000..072b001 --- /dev/null +++ b/workspace_edge_test.go @@ -0,0 +1,133 @@ +package moduledoc + +import ( + "os" + "path/filepath" + "sync" + "testing" + + "golang.org/x/tools/go/packages" +) + +func tempDocsysDirs(t *testing.T) map[string]bool { + t.Helper() + matches, err := filepath.Glob(filepath.Join(os.TempDir(), "caddy_docsys_*")) + if err != nil { + t.Fatalf("globbing temp dir: %v", err) + } + set := make(map[string]bool, len(matches)) + for _, m := range matches { + set[m] = true + } + return set +} + +func TestWorkspaceCloseIdempotent(t *testing.T) { + if testing.Short() { + t.Skip("requires the Go toolchain") + } + d := New(newMemStorage()) + ws, err := d.openWorkspace() + if err != nil { + t.Fatalf("opening workspace: %v", err) + } + if err := ws.Close(); err != nil { + t.Fatalf("closing workspace: %v", err) + } + if _, err := os.Stat(ws.dir); !os.IsNotExist(err) { + t.Errorf("workspace dir should be removed after Close: %v", err) + } + if err := ws.Close(); err != nil { + t.Errorf("closing an already-closed workspace should not error: %v", err) + } +} + +func TestOpenWorkspaceInitFailureCleansTempDir(t *testing.T) { + before := tempDocsysDirs(t) + + // an empty PATH makes the 'go mod init' exec fail after TempDir succeeds + t.Setenv("PATH", t.TempDir()) + + d := New(newMemStorage()) + if _, err := d.openWorkspace(); err == nil { + t.Fatal("expected error when the go binary is unavailable") + } + + after := tempDocsysDirs(t) + for dir := range after { + if !before[dir] { + t.Errorf("temp workspace dir leaked after failed init: %s", dir) + } + } +} + +func TestGetPackagesEmptyPattern(t *testing.T) { + ws := workspace{mu: new(sync.RWMutex)} + if _, err := ws.getPackages("", ""); err == nil { + t.Error("expected error for empty package pattern") + } +} + +func TestCachedPackagesPartialMiss(t *testing.T) { + pkgA := &packages.Package{ID: "example.com/mod/a"} + ws := workspace{ + mu: new(sync.RWMutex), + packagePatterns: map[string][]string{ + "example.com/mod/...": {"example.com/mod/a", "example.com/mod/b"}, + }, + parsedPackages: map[string]*packages.Package{ + "example.com/mod/a": pkgA, + }, + } + + // exact key hit + if got := ws.cachedPackages("example.com/mod/a"); len(got) != 1 || got[0] != pkgA { + t.Errorf("expected cached package for exact key, got %#v", got) + } + // pattern expansion with one member missing must miss entirely + if got := ws.cachedPackages("example.com/mod/..."); got != nil { + t.Errorf("expected nil for partially-cached pattern, got %#v", got) + } + // unknown pattern + if got := ws.cachedPackages("example.com/other/..."); got != nil { + t.Errorf("expected nil for unknown pattern, got %#v", got) + } +} + +func TestPackageKeyVersionSuffix(t *testing.T) { + pkg := &packages.Package{ID: "example.com/mod/a"} + if got := packageKey(pkg); got != "example.com/mod/a" { + t.Errorf("packageKey without module = %q", got) + } + pkg.Module = &packages.Module{Path: "example.com/mod"} + if got := packageKey(pkg); got != "example.com/mod/a" { + t.Errorf("packageKey with unversioned module = %q", got) + } + pkg.Module.Version = "v1.2.3" + if got := packageKey(pkg); got != "example.com/mod/a@v1.2.3" { + t.Errorf("packageKey with versioned module = %q", got) + } +} + +func TestAlreadyGotModuleHierarchy(t *testing.T) { + ws := workspace{ + goGets: map[string]struct{}{"github.com/foo/bar": {}}, + } + for _, tc := range []struct { + path string + want bool + }{ + {"github.com/foo/bar", true}, + {"github.com/foo/bar/baz", true}, + {"github.com/foo/bar/baz/deep", true}, + // sibling with a shared string prefix is a different module + {"github.com/foo/barbaz", false}, + {"github.com/foo", false}, + {"github.com", false}, + {"", false}, + } { + if got := ws.alreadyGotModule(tc.path); got != tc.want { + t.Errorf("alreadyGotModule(%q) = %v; want %v", tc.path, got, tc.want) + } + } +}