diff --git a/CHANGELOG.md b/CHANGELOG.md index 224ba699..22bbcf9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,25 @@ packages, and tooling contracts may change before a stable release. candidate function stays silent so the 501-stub workflow is unaffected; strict production builds still fail closed via `backend_binding_required`. This is the first slice of #328. +- Backend handler binding no longer hides failures behind silent fallbacks: a + handler declared in both same-package Go and an inline `go {}` block is + reported as `ambiguous_backend_handler` instead of silently preferring one + source; a sibling Go package that fails to compile keeps a "could not be + inspected" binding instead of falling back to an inline block and reporting a + misleading bound handler (the compile error is reported by `go_package_error`); + a failing `go list` for a same-package build function now surfaces its real + cause (for example a missing `go.mod`) rather than a generic "requires a + buildable Go package" message; and a component-script resolution error during + build now fails the build instead of silently omitting the page's component + scripts. The contract scanner's `go list -deps -export` failures now surface + their stderr instead of reaching the type checker as an opaque "exit status 1", + and an unknown build-data interpolation reference now reports whether the + missing name was a build-data field or a route param instead of always saying + "unknown route param". Generated-app module resolution no longer silently + drops the app module's `require`/`replace` when `go list -m` fails: if the + generated app imports app-owned packages it now fails with the real `go list` + error instead of producing a go.mod that fails to build with an opaque + "cannot find package". ### Known Gaps diff --git a/cmd/gowdk/go_bindings_report.go b/cmd/gowdk/go_bindings_report.go index a3e57339..2db4ebef 100644 --- a/cmd/gowdk/go_bindings_report.go +++ b/cmd/gowdk/go_bindings_report.go @@ -2,6 +2,7 @@ package main import ( "encoding/json" + "errors" "fmt" "go/ast" "go/parser" @@ -309,7 +310,13 @@ func inspectSamePackageImportPath(sourcePath string) (string, error) { command.Dir = dir output, err := command.Output() if err != nil { - return "", fmt.Errorf("same-package build data function requires a buildable Go package for %s", dir) + var exit *exec.ExitError + if errors.As(err, &exit) { + if stderr := strings.TrimSpace(string(exit.Stderr)); stderr != "" { + return "", fmt.Errorf("same-package build data function requires a buildable Go package for %s: %w\n%s", dir, err, stderr) + } + } + return "", fmt.Errorf("same-package build data function requires a buildable Go package for %s: %w", dir, err) } var info struct { ImportPath string diff --git a/docs/reference/diagnostic-codes.md b/docs/reference/diagnostic-codes.md index 80dd7a73..db9aea52 100644 --- a/docs/reference/diagnostic-codes.md +++ b/docs/reference/diagnostic-codes.md @@ -123,7 +123,8 @@ Parser diagnostics emit stable codes for common unsupported syntax and keep `invalid_go_endpoint_handler`, `malformed_go_endpoint_comment`, `go_endpoint_parse_error`, `duplicate_go_endpoint_comment`, `unsupported_action_method`, `backend_binding_required`, - `unsupported_backend_signature`, `unexported_backend_handler`. + `unsupported_backend_signature`, `unexported_backend_handler`, + `ambiguous_backend_handler`. - Layouts, CSS, and cache: `duplicate_layout_id`, `unknown_layout_id`, `invalid_css_selection`, `duplicate_css_selection`, `revalidate_requires_cache`, `duplicate_revalidate_policy`. diff --git a/docs/reference/go-interop.md b/docs/reference/go-interop.md index bdd6792d..65f03e02 100644 --- a/docs/reference/go-interop.md +++ b/docs/reference/go-interop.md @@ -80,11 +80,19 @@ the JSON report or running a strict production build: - `unexported_backend_handler` — a same-named Go function exists but is not exported, so binding cannot see it (for example `func submit` when the block expects `Submit`). +- `ambiguous_backend_handler` — the same handler is declared in both + same-package Go and an inline `go {}` block. (When both live in the same + compiled package, Go's own redeclaration error surfaces first.) A handler with no candidate function stays silent because the default workflow generates 501 stubs for not-yet-implemented handlers; strict production builds still fail closed through `backend_binding_required`. +When the sibling Go package fails to compile, binding does not fall back to an +inline `go {}` block and report a misleading bound handler: the load/action/API +binding stays "could not be inspected" and the package error itself is reported +by `go_package_error`. + ## Load Functions Request-time pages with `load {}` bind same-package functions named diff --git a/internal/appgen/appgen_test.go b/internal/appgen/appgen_test.go index 5c4b45a8..e48c925e 100644 --- a/internal/appgen/appgen_test.go +++ b/internal/appgen/appgen_test.go @@ -3085,6 +3085,46 @@ func HomeTitle() string { } } +func TestIsLocalModuleImportPath(t *testing.T) { + cases := map[string]bool{ + "context": false, + "net/http": false, + "github.com/cssbruno/gowdk": false, + "github.com/cssbruno/gowdk/runtime/response": false, + "example.com/site/content": true, + "github.com/acme/app/pages": true, + "": false, + } + for path, want := range cases { + if got := isLocalModuleImportPath(path); got != want { + t.Fatalf("isLocalModuleImportPath(%q) = %v, want %v", path, got, want) + } + } +} + +func TestAppHasLocalModuleImportsFromInlineGoBlock(t *testing.T) { + ir := gwdkir.Program{ + Pages: []gwdkir.Page{{ + Package: "pages", + ID: "home", + Route: "/", + Blocks: gwdkir.Blocks{GoBlocks: []gwdkir.GoBlock{{ + Body: `import local "example.com/site/local" + +func HomeTitle() string { return local.Suffix() }`, + }}}, + }}, + } + if !appHasLocalModuleImports(Options{IR: &ir}) { + t.Fatal("expected an app importing example.com/site/local to need the local module") + } + + empty := gwdkir.Program{Pages: []gwdkir.Page{{Package: "pages", ID: "home", Route: "/"}}} + if appHasLocalModuleImports(Options{IR: &empty}) { + t.Fatal("expected an app with no app-owned imports not to need the local module") + } +} + func TestGenerateWritesAddonGoBlockConsumerFiles(t *testing.T) { root := t.TempDir() outputDir := filepath.Join(root, "dist") diff --git a/internal/appgen/module.go b/internal/appgen/module.go index a4c34862..c56bee92 100644 --- a/internal/appgen/module.go +++ b/internal/appgen/module.go @@ -2,6 +2,7 @@ package appgen import ( "encoding/json" + "errors" "fmt" "go/ast" "os" @@ -39,7 +40,17 @@ func moduleSource(options Options) (string, error) { } lines = append(lines, "", "replace "+gowdkRuntimeModulePath+" => "+filepath.ToSlash(root)) } - if appModule, ok := currentAppModule(); ok && appModule.Path != gowdkRuntimeModulePath && optionsUsesModuleImports(options, appModule.Path) { + appModule, err := currentAppModule() + if err != nil { + // A missing/broken main module is only fatal when the generated app + // imports app-owned packages: without the module path we cannot add the + // require/replace, so the generated app would otherwise fail to build + // later with an opaque "cannot find package" error. When the app imports + // nothing app-owned, the module is not needed and the failure is ignored. + if appHasLocalModuleImports(options) { + return "", fmt.Errorf("cannot determine the app Go module for generated app imports: %w", err) + } + } else if appModule.Path != gowdkRuntimeModulePath && optionsUsesModuleImports(options, appModule.Path) { lines = append(lines, "", "require "+appModule.Path+" v0.0.0", @@ -55,20 +66,58 @@ type appModuleInfo struct { Dir string } -func currentAppModule() (appModuleInfo, bool) { +func currentAppModule() (appModuleInfo, error) { command := exec.Command("go", "list", "-m", "-json") output, err := command.Output() if err != nil { - return appModuleInfo{}, false + return appModuleInfo{}, goListModuleError(err) } var info appModuleInfo if err := json.Unmarshal(output, &info); err != nil { - return appModuleInfo{}, false + return appModuleInfo{}, fmt.Errorf("parse go list -m output: %w", err) } if strings.TrimSpace(info.Path) == "" || strings.TrimSpace(info.Dir) == "" { - return appModuleInfo{}, false + return appModuleInfo{}, fmt.Errorf("go list -m did not report a main module path and directory") + } + return info, nil +} + +// goListModuleError surfaces the underlying go list -m failure, including its +// stderr (e.g. a missing go.mod), instead of an opaque exit status. +func goListModuleError(err error) error { + var exit *exec.ExitError + if errors.As(err, &exit) { + if stderr := strings.TrimSpace(string(exit.Stderr)); stderr != "" { + return fmt.Errorf("%w\n%s", err, stderr) + } + } + return err +} + +// appHasLocalModuleImports reports whether the generated app imports any +// app-owned package (a module-path import that is not stdlib and not the GOWDK +// runtime module), which is exactly the case that needs the main module's +// require/replace lines. +func appHasLocalModuleImports(options Options) bool { + for path := range appBackendImportPaths(options) { + if isLocalModuleImportPath(path) { + return true + } + } + return false +} + +func isLocalModuleImportPath(path string) bool { + path = strings.TrimSpace(path) + if path == "" || path == gowdkRuntimeModulePath || strings.HasPrefix(path, gowdkRuntimeModulePath+"/") { + return false + } + first := path + if index := strings.Index(path, "/"); index >= 0 { + first = path[:index] } - return info, true + // Standard-library import paths have no dot in their first segment. + return strings.Contains(first, ".") } func optionsUsesModuleImports(options Options, modulePath string) bool { @@ -76,22 +125,29 @@ func optionsUsesModuleImports(options Options, modulePath string) bool { if modulePath == "" { return false } - for importPath := range backendImports(options.Actions, options.APIs, options.Fragments, options.SSR) { + for importPath := range appBackendImportPaths(options) { if importPath == modulePath || strings.HasPrefix(importPath, modulePath+"/") { return true } } + return false +} + +// appBackendImportPaths collects every Go import the generated app's backend +// glue pulls in: request-time handlers, contract exposures, and inline go {} +// blocks. +func appBackendImportPaths(options Options) map[string]bool { + paths := map[string]bool{} + for importPath := range backendImports(options.Actions, options.APIs, options.Fragments, options.SSR) { + paths[importPath] = true + } for importPath := range backendContractImports(executableContractExposures(backendAdapterIR(options).ContractExposures)) { - if importPath == modulePath || strings.HasPrefix(importPath, modulePath+"/") { - return true - } + paths[importPath] = true } for importPath := range inlineGoBlockImports(options.IR) { - if importPath == modulePath || strings.HasPrefix(importPath, modulePath+"/") { - return true - } + paths[importPath] = true } - return false + return paths } func inlineGoBlockImports(ir *gwdkir.Program) map[string]bool { diff --git a/internal/buildgen/build_data_interpolate.go b/internal/buildgen/build_data_interpolate.go index abc18f22..853ca8a0 100644 --- a/internal/buildgen/build_data_interpolate.go +++ b/internal/buildgen/build_data_interpolate.go @@ -33,15 +33,20 @@ func interpolateBuildValue(value string, routeParams map[string]string, data map value = value[end+1:] continue } + wasField := false if field, ok := buildFieldExpression(name); ok { name = field + wasField = true } resolved, ok := data[name] if !ok { resolved, ok = routeParams[name] } if !ok { - return "", fmt.Errorf("unknown route param %q", name) + if wasField { + return "", fmt.Errorf("unknown build data field %q", name) + } + return "", fmt.Errorf("unknown build data field or route param %q", name) } parts = append(parts, resolved) value = value[end+1:] diff --git a/internal/buildgen/build_data_routes_test.go b/internal/buildgen/build_data_routes_test.go index 3c5c08ea..6fd4ce0a 100644 --- a/internal/buildgen/build_data_routes_test.go +++ b/internal/buildgen/build_data_routes_test.go @@ -12,6 +12,27 @@ import ( "github.com/cssbruno/gowdk/internal/gwdkir" ) +func TestSamePackageImportPathSurfacesGoListError(t *testing.T) { + // A temp dir outside any Go module makes `go list` fail with a clear reason + // (no go.mod), which must be surfaced rather than collapsed into a generic + // "requires a buildable Go package" message. + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "page.go"), []byte("package app\n"), 0o644); err != nil { + t.Fatal(err) + } + + _, err := samePackageImportPath(filepath.Join(root, "home.page.gwdk")) + if err == nil { + t.Fatal("expected a same-package build data error outside a Go module") + } + if !strings.Contains(err.Error(), "buildable Go package") { + t.Fatalf("expected the user-facing message, got: %v", err) + } + if !strings.Contains(err.Error(), "go.mod") { + t.Fatalf("expected the underlying go list error (go.mod not found) to be surfaced, got: %v", err) + } +} + func TestBuildRendersLiteralBuildData(t *testing.T) { outputDir := t.TempDir() app := gwdkanalysis.Sources{ @@ -497,13 +518,32 @@ func TestBuildRejectsUnknownRouteParamInBuildDataValue(t *testing.T) { _, err := Build(gowdk.Config{}, app, outputDir) if err == nil { - t.Fatal("expected unknown route param error") + t.Fatal("expected unknown interpolation reference error") } - if !strings.Contains(err.Error(), `build field title: unknown route param "missing"`) { + if !strings.Contains(err.Error(), `build field title: unknown build data field or route param "missing"`) { t.Fatalf("unexpected error: %v", err) } } +func TestInterpolateBuildValueReportsAccurateUnknownReference(t *testing.T) { + data := map[string]string{"title": "Hi"} + params := map[string]string{"slug": "x"} + cases := []struct { + value string + want string + }{ + {`{field("missing")}`, `unknown build data field "missing"`}, + {`{missing}`, `unknown build data field or route param "missing"`}, + {`{param("missing")}`, `unknown route param "missing"`}, + } + for _, tc := range cases { + _, err := interpolateBuildValue(tc.value, params, data) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Fatalf("interpolateBuildValue(%q): want error containing %q, got %v", tc.value, tc.want, err) + } + } +} + func TestBuildRendersExplicitRouteParamReferences(t *testing.T) { outputDir := t.TempDir() app := gwdkanalysis.Sources{Pages: []gwdkir.Page{{ diff --git a/internal/buildgen/build_data_runner.go b/internal/buildgen/build_data_runner.go index 0289cb83..7b2df052 100644 --- a/internal/buildgen/build_data_runner.go +++ b/internal/buildgen/build_data_runner.go @@ -3,6 +3,7 @@ package buildgen import ( "bytes" "encoding/json" + "errors" "fmt" "go/ast" "go/format" @@ -222,7 +223,10 @@ func inlineBuildDataMainDecl(function string, returnsError bool) ast.Decl { func samePackageImportPath(source string) (string, error) { dir := sourceDir(source) - info := goListDir(dir) + info, err := goListDir(dir) + if err != nil { + return "", fmt.Errorf("same-package build data function requires a buildable Go package for %s: %w", dir, err) + } if strings.TrimSpace(info.ImportPath) == "" { return "", fmt.Errorf("same-package build data function requires a buildable Go package for %s", dir) } @@ -233,18 +237,31 @@ type goListDirInfo struct { ImportPath string } -func goListDir(dir string) goListDirInfo { +func goListDir(dir string) (goListDirInfo, error) { command := exec.Command("go", "list", "-json", ".") command.Dir = dir output, err := command.Output() if err != nil { - return goListDirInfo{} + return goListDirInfo{}, goListError(err) } var info goListDirInfo if err := json.Unmarshal(output, &info); err != nil { - return goListDirInfo{} + return goListDirInfo{}, fmt.Errorf("parse go list output: %w", err) + } + return info, nil +} + +// goListError surfaces the underlying go list failure, including its stderr, +// instead of collapsing it into a generic message that hides the cause (for +// example a sibling package compile error or a missing go.mod). +func goListError(err error) error { + var exit *exec.ExitError + if errors.As(err, &exit) { + if stderr := strings.TrimSpace(string(exit.Stderr)); stderr != "" { + return fmt.Errorf("%w\n%s", err, stderr) + } } - return info + return err } func sourceDir(source string) string { diff --git a/internal/buildgen/render.go b/internal/buildgen/render.go index 3321e38d..39c6d440 100644 --- a/internal/buildgen/render.go +++ b/internal/buildgen/render.go @@ -184,7 +184,11 @@ func actionRoutes(page gwdkir.Page, data map[string]string) map[string]string { func pageScripts(config gowdk.Config, page gwdkir.Page, viewSource string, components map[string]view.Component, policy renderModePolicy) ([]gowdk.Script, error) { scripts := append([]gowdk.Script{}, nonEmptyScripts(config.Build.Scripts)...) - for _, href := range scopedScriptHrefs(page, viewSource, components) { + hrefs, err := scopedScriptHrefs(page, viewSource, components) + if err != nil { + return nil, err + } + for _, href := range hrefs { scripts = append(scripts, gowdk.Script{Src: href, Type: "module"}) } if policy != renderModeSPA { diff --git a/internal/buildgen/scoped_scripts.go b/internal/buildgen/scoped_scripts.go index 8ae9dc10..6a551b31 100644 --- a/internal/buildgen/scoped_scripts.go +++ b/internal/buildgen/scoped_scripts.go @@ -188,7 +188,7 @@ func safeScriptAssetName(scriptPath string) string { return name } -func scopedScriptHrefs(page gwdkir.Page, viewSource string, components map[string]view.Component) []string { +func scopedScriptHrefs(page gwdkir.Page, viewSource string, components map[string]view.Component) ([]string, error) { seen := map[string]bool{} var scripts []string add := func(href string) { @@ -208,17 +208,20 @@ func scopedScriptHrefs(page gwdkir.Page, viewSource string, components map[strin } add("/" + pageScopedJSLogicalPath(page.ID, name)) } - componentHrefs := scopedComponentScriptHrefs(page, viewSource, components) + componentHrefs, err := scopedComponentScriptHrefs(page, viewSource, components) + if err != nil { + return nil, err + } for _, href := range componentHrefs { add(href) } - return scripts + return scripts, nil } -func scopedComponentScriptHrefs(page gwdkir.Page, viewSource string, components map[string]view.Component) []string { +func scopedComponentScriptHrefs(page gwdkir.Page, viewSource string, components map[string]view.Component) ([]string, error) { usages, err := recursiveViewComponentCallUsages(viewSource, components, page.Package, componentUses(page.Uses)) if err != nil { - return nil + return nil, err } seen := map[string]bool{} var scripts []string @@ -246,5 +249,5 @@ func scopedComponentScriptHrefs(page gwdkir.Page, viewSource string, components } } sort.Strings(scripts) - return scripts + return scripts, nil } diff --git a/internal/compiler/backend_binding_diagnostics.go b/internal/compiler/backend_binding_diagnostics.go index 4cfecb5e..460f8ddc 100644 --- a/internal/compiler/backend_binding_diagnostics.go +++ b/internal/compiler/backend_binding_diagnostics.go @@ -9,9 +9,12 @@ import ( // visible during gowdk check and build instead of only in inspect go-bindings // or a strict production build. // -// It intentionally warns only when there is positive evidence of a near-miss -// the author almost certainly meant to bind: +// It intentionally warns only when there is positive evidence of a problem the +// author almost certainly cares about: // +// - ambiguous_backend_handler: the same handler is declared in both +// same-package Go and an inline go {} block, so the chosen source is +// ambiguous. // - unsupported_backend_signature: a same-named Go function exists but has a // signature GOWDK does not support. // - unexported_backend_handler: a same-named Go function exists but is not @@ -25,6 +28,8 @@ func BackendBindingDiagnostics(bindings []source.BackendBinding) []ValidationErr var diagnostics []ValidationError for _, binding := range bindings { switch { + case binding.Ambiguous: + diagnostics = append(diagnostics, backendBindingDiagnostic("ambiguous_backend_handler", binding)) case binding.Status == source.BackendBindingUnsupportedSignature: diagnostics = append(diagnostics, backendBindingDiagnostic("unsupported_backend_signature", binding)) case binding.Status == source.BackendBindingMissing && binding.UnexportedCandidate: diff --git a/internal/compiler/backend_binding_diagnostics_test.go b/internal/compiler/backend_binding_diagnostics_test.go index e398937e..118c2217 100644 --- a/internal/compiler/backend_binding_diagnostics_test.go +++ b/internal/compiler/backend_binding_diagnostics_test.go @@ -1,6 +1,7 @@ package compiler import ( + "path/filepath" "strings" "testing" @@ -9,13 +10,21 @@ import ( "github.com/cssbruno/gowdk/internal/source" ) +// emptyPackageSource returns a page source path inside a fresh empty directory. +// inspectFeaturePackage reports the directory as having no Go files (no +// LoadError, no functions), so binding falls through to the inline go {} block. +func emptyPackageSource(t *testing.T, name string) string { + t.Helper() + return filepath.Join(t.TempDir(), name) +} + func fragmentPolicyProgram(t *testing.T, goBlockBody string) gwdkir.Program { t.Helper() return gwdkir.Program{ Pages: []gwdkir.Page{{ ID: "patients", Package: "pages", - Source: "no-such-dir/patients.page.gwdk", + Source: emptyPackageSource(t, "patients.page.gwdk"), Route: "/patients", Blocks: gwdkir.Blocks{ Fragments: []gwdkir.FragmentEndpoint{{Name: "Summary", Method: "GET", Route: "/patients/summary", Target: "#summary"}}, @@ -60,12 +69,12 @@ func findBindingByName(bindings []source.BackendBinding, kind, name string) (sou } func TestComputeBackendBindingsFlagsUnexportedInlineActionHandler(t *testing.T) { - // Same-package directory does not exist, so the only candidate is the + // The same-package directory has no Go files, so the only candidate is the // inline default go {} block, which declares an unexported near-miss. page := gwdkir.Page{ ID: "signup", Package: "pages", - Source: "no-such-dir/signup.page.gwdk", + Source: emptyPackageSource(t, "signup.page.gwdk"), Route: "/signup", Blocks: gwdkir.Blocks{ Actions: []gwdkir.Action{{Name: "Submit", Method: "POST", Route: "/signup"}}, @@ -90,7 +99,7 @@ func TestComputeBackendBindingsFlagsUnexportedInlineFragmentHandler(t *testing.T page := gwdkir.Page{ ID: "patients", Package: "pages", - Source: "no-such-dir/patients.page.gwdk", + Source: emptyPackageSource(t, "patients.page.gwdk"), Route: "/patients", Blocks: gwdkir.Blocks{ Fragments: []gwdkir.FragmentEndpoint{{Name: "Summary", Method: "GET", Route: "/patients/summary", Target: "#summary"}}, @@ -111,11 +120,95 @@ func TestComputeBackendBindingsFlagsUnexportedInlineFragmentHandler(t *testing.T } } +const inlineSubmitGoBlock = `import ( + "context" + + "github.com/cssbruno/gowdk/runtime/response" +) + +func Submit(context.Context) (response.Response, error) { + return response.Response{}, nil +}` + +func TestComputeBackendBindingsFlagsAmbiguousHandler(t *testing.T) { + root := t.TempDir() + writeCompilerTestModule(t, root) + writeCompilerTestFile(t, filepath.Join(root, "handlers.go"), `package pages + +import ( + "context" + + "github.com/cssbruno/gowdk/runtime/response" +) + +func Submit(context.Context) (response.Response, error) { + return response.Response{}, nil +} +`) + page := gwdkir.Page{ + ID: "signup", + Package: "pages", + Source: filepath.Join(root, "signup.page.gwdk"), + Route: "/signup", + Blocks: gwdkir.Blocks{ + Actions: []gwdkir.Action{{Name: "Submit", Method: "POST", Route: "/signup"}}, + GoBlocks: []gwdkir.GoBlock{{Body: inlineSubmitGoBlock}}, + }, + } + bindings := computeBackendBindings(gwdkir.Program{Pages: []gwdkir.Page{page}}) + binding, ok := findBindingByName(bindings, "action", "Submit") + if !ok || !binding.Ambiguous { + t.Fatalf("expected an ambiguous Submit binding, got %#v", binding) + } + if !strings.Contains(binding.Message, "declared in both") { + t.Fatalf("expected ambiguity message, got %q", binding.Message) + } + diagnostics := BackendBindingDiagnostics(bindings) + if len(diagnostics) != 1 || diagnostics[0].Code != "ambiguous_backend_handler" { + t.Fatalf("expected one ambiguous_backend_handler diagnostic, got %#v", diagnostics) + } +} + +func TestComputeBackendBindingsDoesNotMaskSamePackageCompileError(t *testing.T) { + root := t.TempDir() + writeCompilerTestModule(t, root) + // A sibling Go file that fails to type-check, so the same-package package + // cannot be inspected. + writeCompilerTestFile(t, filepath.Join(root, "broken.go"), `package pages + +func Broken() int { return "not an int" } +`) + page := gwdkir.Page{ + ID: "signup", + Package: "pages", + Source: filepath.Join(root, "signup.page.gwdk"), + Route: "/signup", + Blocks: gwdkir.Blocks{ + Actions: []gwdkir.Action{{Name: "Submit", Method: "POST", Route: "/signup"}}, + GoBlocks: []gwdkir.GoBlock{{Body: inlineSubmitGoBlock}}, + }, + } + bindings := computeBackendBindings(gwdkir.Program{Pages: []gwdkir.Page{page}}) + binding, ok := findBindingByName(bindings, "action", "Submit") + if !ok { + t.Fatalf("expected a Submit binding, got %#v", bindings) + } + // Even though the inline go {} block declares a valid Submit, a broken + // same-package package must not be masked by reporting the inline handler as + // bound. The compile error itself is reported separately by go_package_error. + if binding.Status == source.BackendBindingBound { + t.Fatalf("expected the broken same-package package not to report Submit as bound via inline fallback, got %#v", binding) + } + if !strings.Contains(binding.Message, "could not be inspected") { + t.Fatalf("expected a could-not-inspect message surfacing the compile error, got %q", binding.Message) + } +} + func TestComputeBackendBindingsStaysSilentForFragmentWithoutCandidate(t *testing.T) { page := gwdkir.Page{ ID: "patients", Package: "pages", - Source: "no-such-dir/patients.page.gwdk", + Source: emptyPackageSource(t, "patients.page.gwdk"), Route: "/patients", Blocks: gwdkir.Blocks{ Fragments: []gwdkir.FragmentEndpoint{{Name: "Summary", Method: "GET", Route: "/patients/summary", Target: "#summary"}}, diff --git a/internal/compiler/backend_bindings.go b/internal/compiler/backend_bindings.go index 8ac51026..72f6fc7b 100644 --- a/internal/compiler/backend_bindings.go +++ b/internal/compiler/backend_bindings.go @@ -53,25 +53,23 @@ func computeBackendBindings(ir gwdkir.Program) []source.BackendBinding { bindings = append(bindings, bindLoad(page, pkg)) } for _, action := range page.Blocks.Actions { - binding := bindAction(page, action, pkg) - if binding.Status == source.BackendBindingMissing { - binding = preferInlineBinding(binding, bindAction(page, action, defaultInlinePkg())) - } - bindings = append(bindings, binding) + inlinePkg := defaultInlinePkg() + bindings = append(bindings, resolveBackendBinding( + bindAction(page, action, pkg), + bindAction(page, action, inlinePkg), + pkg, inlinePkg, action.Name, + )) } for _, api := range page.Blocks.APIs { - binding := bindAPI(page, api, pkg) - if binding.Status == source.BackendBindingMissing { - binding = preferInlineBinding(binding, bindAPI(page, api, defaultInlinePkg())) - } - bindings = append(bindings, binding) + inlinePkg := defaultInlinePkg() + bindings = append(bindings, resolveBackendBinding( + bindAPI(page, api, pkg), + bindAPI(page, api, inlinePkg), + pkg, inlinePkg, api.Name, + )) } for _, fragment := range page.Blocks.Fragments { - if binding, ok := bindFragment(page, fragment, pkg); ok { - bindings = append(bindings, binding) - } else if binding, ok := bindFragment(page, fragment, defaultInlinePkg()); ok { - bindings = append(bindings, binding) - } else if binding, ok := bindMissingFragmentCandidate(page, fragment, pkg, defaultInlinePkg()); ok { + if binding, ok := resolveFragmentBinding(page, fragment, pkg, defaultInlinePkg()); ok { bindings = append(bindings, binding) } } @@ -104,37 +102,51 @@ func computeBackendBindings(ir gwdkir.Program) []source.BackendBinding { func bindLoad(page gwdkir.Page, pkg featurePackage) source.BackendBinding { functionName := loadFunctionName(page.ID) - if function, ok := pkg.Functions[functionName]; ok { + // A broken same-package Go package cannot be inspected: surface that instead + // of falling back to an inline go ssr {} block and reporting a misleading + // bound status. The broken package itself is reported by go_package_error. + if pkg.LoadError != "" { binding := baseBackendBinding(page, loadHandlerKind, functionName, "GET", page.Route, pkg) - if !function.Load() { - binding.Status = source.BackendBindingUnsupportedSignature - binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s must have signature func(ssr.LoadContext) map[string]any or func(ssr.LoadContext) (map[string]any, error)", bindingPackageLabel(binding, pkg), functionName) - return binding - } - binding.Signature = function.Signature - binding.Status = source.BackendBindingBound + binding.Status = source.BackendBindingMissing + binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s could not be inspected: %s", bindingPackageLabel(binding, pkg), functionName, pkg.LoadError) return binding } inlinePkg := inspectInlineScriptFeaturePackage(page, "ssr") - if function, ok := inlinePkg.Functions[functionName]; ok { - binding := baseBackendBinding(page, loadHandlerKind, functionName, "GET", page.Route, inlinePkg) - if !function.Load() { - binding.Status = source.BackendBindingUnsupportedSignature - binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s must have signature func(ssr.LoadContext) map[string]any or func(ssr.LoadContext) (map[string]any, error)", bindingPackageLabel(binding, inlinePkg), functionName) - return binding + _, inSame := pkg.Functions[functionName] + _, inInline := inlinePkg.Functions[functionName] + switch { + case inSame && inInline: + binding := bindLoadFromPackage(page, functionName, pkg) + binding.Ambiguous = true + binding.Message = ambiguousHandlerMessage(loadHandlerKind, functionName) + return binding + case inSame: + return bindLoadFromPackage(page, functionName, pkg) + case inInline: + return bindLoadFromPackage(page, functionName, inlinePkg) + default: + binding := baseBackendBinding(page, loadHandlerKind, functionName, "GET", page.Route, pkg) + binding.Status = source.BackendBindingMissing + binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s is not implemented", bindingPackageLabel(binding, pkg), functionName) + binding = markUnexportedCandidate(binding, pkg) + if !binding.UnexportedCandidate { + binding = markUnexportedCandidate(binding, inlinePkg) } - binding.Signature = function.Signature - binding.Status = source.BackendBindingBound return binding } +} + +func bindLoadFromPackage(page gwdkir.Page, functionName string, pkg featurePackage) source.BackendBinding { binding := baseBackendBinding(page, loadHandlerKind, functionName, "GET", page.Route, pkg) - binding.Status = source.BackendBindingMissing - if pkg.LoadError != "" { - binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s could not be inspected: %s", bindingPackageLabel(binding, pkg), functionName, pkg.LoadError) + function := pkg.Functions[functionName] + if !function.Load() { + binding.Status = source.BackendBindingUnsupportedSignature + binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s must have signature func(ssr.LoadContext) map[string]any or func(ssr.LoadContext) (map[string]any, error)", bindingPackageLabel(binding, pkg), functionName) return binding } - binding.Message = fmt.Sprintf("GOWDK SSR load handler %s.%s is not implemented", bindingPackageLabel(binding, pkg), functionName) - return markUnexportedCandidate(binding, pkg) + binding.Signature = function.Signature + binding.Status = source.BackendBindingBound + return binding } func bindStandaloneAction(endpoint gwdkir.GoEndpoint, pkg featurePackage) source.BackendBinding { @@ -300,6 +312,74 @@ func baseStandaloneBackendBinding(endpoint gwdkir.GoEndpoint, kind, method strin } } +// resolveBackendBinding chooses the binding for an action/api block across its +// two possible Go sources — sibling same-package code and the inline default +// go {} block — without masking failures: +// +// - When the same-package package failed to compile (LoadError), the +// could-not-inspect binding is kept instead of falling back to the inline +// result, so a broken package never looks bound. The compile error itself is +// reported by go_package_error. +// - When the handler is declared in BOTH sources it is ambiguous: the binding +// is flagged so tooling reports the conflict instead of silently preferring +// one source. +// - Otherwise the source that actually declares the handler wins, and when +// neither does, an unexported near-miss in either source is surfaced. +func resolveBackendBinding(samePackage, inline source.BackendBinding, samePkg, inlinePkg featurePackage, name string) source.BackendBinding { + if samePkg.LoadError != "" { + return samePackage + } + inSame := packageHasFunction(samePkg, name) + inInline := packageHasFunction(inlinePkg, name) + switch { + case inSame && inInline: + out := samePackage + out.Ambiguous = true + out.Message = ambiguousHandlerMessage(out.Kind, name) + return out + case inSame: + return samePackage + case inInline: + return inline + default: + return preferInlineBinding(samePackage, inline) + } +} + +// resolveFragmentBinding mirrors resolveBackendBinding for fragments, which +// never require a Go handler: a fragment with no bound handler renders static +// .gwdk output. It returns ok=false (no binding, static) unless a handler is +// declared, ambiguous, or only present as an unexported near-miss. +func resolveFragmentBinding(page gwdkir.Page, fragment gwdkir.FragmentEndpoint, samePkg, inlinePkg featurePackage) (source.BackendBinding, bool) { + if samePkg.LoadError != "" { + return source.BackendBinding{}, false + } + inSame := packageHasFunction(samePkg, fragment.Name) + inInline := packageHasFunction(inlinePkg, fragment.Name) + switch { + case inSame && inInline: + binding, _ := bindFragment(page, fragment, samePkg) + binding.Ambiguous = true + binding.Message = ambiguousHandlerMessage(fragmentHandlerKind, fragment.Name) + return binding, true + case inSame: + return bindFragment(page, fragment, samePkg) + case inInline: + return bindFragment(page, fragment, inlinePkg) + default: + return bindMissingFragmentCandidate(page, fragment, samePkg, inlinePkg) + } +} + +func packageHasFunction(pkg featurePackage, name string) bool { + _, ok := pkg.Functions[name] + return ok +} + +func ambiguousHandlerMessage(kind, name string) string { + return fmt.Sprintf("GOWDK %s handler %s is declared in both same-package Go and an inline go {} block; declare it in exactly one source", kind, name) +} + // preferInlineBinding resolves an action/api block that did not bind in its // same-package Go code by consulting the inline default go {} block. It prefers // the inline result when the inline block actually binds (bound or unsupported diff --git a/internal/contractscan/types.go b/internal/contractscan/types.go index 95fe0252..ce30dda1 100644 --- a/internal/contractscan/types.go +++ b/internal/contractscan/types.go @@ -2,6 +2,7 @@ package contractscan import ( "encoding/json" + "errors" "fmt" "go/ast" "go/importer" @@ -193,7 +194,7 @@ func scanGoListExportFiles(packageDir string, importPaths []string) (map[string] command.Dir = packageDir output, err := command.Output() if err != nil { - return nil, err + return nil, goListExportError(err) } decoder := json.NewDecoder(strings.NewReader(string(output))) exports := map[string]string{} @@ -215,3 +216,16 @@ func scanGoListExportFiles(packageDir string, importPaths []string) (map[string] } return exports, nil } + +// goListExportError surfaces the underlying go list failure, including its +// stderr, instead of letting the bare exit status reach the type checker as an +// opaque "load export data for X: exit status 1" with no cause. +func goListExportError(err error) error { + var exit *exec.ExitError + if errors.As(err, &exit) { + if stderr := strings.TrimSpace(string(exit.Stderr)); stderr != "" { + return fmt.Errorf("%w\n%s", err, stderr) + } + } + return err +} diff --git a/internal/contractscan/types_test.go b/internal/contractscan/types_test.go new file mode 100644 index 00000000..7501c9dd --- /dev/null +++ b/internal/contractscan/types_test.go @@ -0,0 +1,19 @@ +package contractscan + +import ( + "strings" + "testing" +) + +func TestScanGoListExportFilesSurfacesStderr(t *testing.T) { + // A temp dir outside any Go module makes `go list` fail with a clear reason + // (no go.mod), which must reach the caller instead of an opaque exit status. + dir := t.TempDir() + _, err := scanGoListExportFiles(dir, []string{"example.com/does-not-exist"}) + if err == nil { + t.Fatal("expected go list to fail outside a Go module") + } + if !strings.Contains(err.Error(), "go.mod") { + t.Fatalf("expected the underlying go list stderr (go.mod not found) to be surfaced, got: %v", err) + } +} diff --git a/internal/diagnostics/registry.go b/internal/diagnostics/registry.go index 80a0af53..ed551822 100644 --- a/internal/diagnostics/registry.go +++ b/internal/diagnostics/registry.go @@ -58,6 +58,7 @@ var missingUseFix = &Fix{ // compiler, build generator, contract scanner, and language tooling. var Registry = []Code{ {Code: "addon_go_block_diagnostic", Area: "go-block", Stability: StabilityAddon, Severity: SeverityError, Summary: "addon-provided go block diagnostic without a custom code"}, + {Code: "ambiguous_backend_handler", Area: "backend", Stability: StabilityStable, Severity: SeverityWarning, Summary: "a backend handler is declared in both same-package Go and an inline go block"}, {Code: "ambiguous_dynamic_route", Area: "routing", Stability: StabilityStable, Severity: SeverityError, Summary: "dynamic page route overlaps another dynamic page route"}, {Code: "backend_binding_required", Area: "backend", Stability: StabilityStable, Severity: SeverityError, Summary: "strict builds require a supported backend handler binding"}, {Code: "client_go_block_wasm_build_error", Area: "wasm", Stability: StabilityExperimental, Severity: SeverityError, Summary: "page go client WASM build failed"}, diff --git a/internal/source/source.go b/internal/source/source.go index f5b77287..ae6afca6 100644 --- a/internal/source/source.go +++ b/internal/source/source.go @@ -231,6 +231,10 @@ type BackendBinding struct { // unexported Go function exists in the inspected package, so tooling can // explain that the function is present but not exported. UnexportedCandidate bool + // Ambiguous is set when the same handler name is declared in more than one + // Go source (sibling same-package code and an inline go {} block), so tooling + // reports the conflict instead of silently preferring one. + Ambiguous bool } // ErrorPagePath returns a clean generated-output-relative error page path.