From 95e34cc5d8db22a0b468d23a9ce06c5719d56814 Mon Sep 17 00:00:00 2001 From: Bruno Carvalho Date: Sat, 13 Jun 2026 08:16:34 -0300 Subject: [PATCH 1/3] fix: make Go handler binding fail loudly instead of masking errors Removes silent fallbacks in backend handler binding and build-data resolution that hid the real cause of a failure: - Compile-error masking (A): when a sibling Go package fails to compile, binding no longer falls back to an inline go {} block and reports a misleading bound handler. The action/API/load binding stays 'could not be inspected'; the compile error is reported by go_package_error. - Ambiguous handlers (C): a handler declared in both same-package Go and an inline go {} block now emits the new ambiguous_backend_handler warning instead of silently preferring one source. - Swallowed go list errors (B): samePackageImportPath and the go-bindings report now surface the real 'go list' stderr (e.g. a missing go.mod) instead of a generic 'requires a buildable Go package' message. - Component-script masking (D): a component-script resolution error during build now fails the build instead of silently omitting a page's component scripts. Adds source.BackendBinding.Ambiguous, resolveBackendBinding / resolveFragmentBinding, and the ambiguous_backend_handler diagnostic code. Tests cover ambiguity, compile-error non-masking, inline/fragment unexported near-misses, production-policy fragment behaviour, and the surfaced go list error. The remaining swallow-style sites surfaced by an audit are not masking: malformed g:for is reported by validate_component_lists; the render-time view re-parse is guarded by view_parse_error; contractscan strconv.Unquote runs on parser-guaranteed quoted literals; and the LSP feature paths publish parse errors via CheckSource while gracefully degrading. --- CHANGELOG.md | 11 ++ cmd/gowdk/go_bindings_report.go | 9 +- docs/reference/diagnostic-codes.md | 3 +- docs/reference/go-interop.md | 8 + internal/buildgen/build_data_routes_test.go | 21 +++ internal/buildgen/build_data_runner.go | 27 +++- internal/buildgen/render.go | 6 +- internal/buildgen/scoped_scripts.go | 15 +- .../compiler/backend_binding_diagnostics.go | 9 +- .../backend_binding_diagnostics_test.go | 103 +++++++++++- internal/compiler/backend_bindings.go | 152 +++++++++++++----- internal/diagnostics/registry.go | 1 + internal/source/source.go | 4 + 13 files changed, 312 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 224ba699..21240f7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,17 @@ 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. ### 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/buildgen/build_data_routes_test.go b/internal/buildgen/build_data_routes_test.go index 3c5c08ea..965df032 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{ 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/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. From 9484e4fe0e8fecb5cea40ca07c9bae6d2b8c88f9 Mon Sep 17 00:00:00 2001 From: Bruno Carvalho Date: Sat, 13 Jun 2026 08:31:58 -0300 Subject: [PATCH 2/3] fix: surface go list stderr in contract scanner; accurate build-data interpolation error Two more silent-fallback fixes of the same class: - internal/contractscan/types.go: scanGoListExportFiles used .Output() and returned the bare error, so a failing 'go list -deps -export' reached the type checker as an opaque 'load export data for X: exit status 1'. It now surfaces the go list stderr (the real cause). - internal/buildgen/build_data_interpolate.go: a missing interpolation reference always reported 'unknown route param', even for an explicit field("...") build-data field. It now reports 'unknown build data field' for field() expressions and 'unknown build data field or route param' for bare {name} references. Audited siblings that were NOT changed because they are not masking: currentAppModule's go list -m only fails outside a module (where the build can't work anyway); validate_packages goListExportFiles falls back to importer.Default and go_package_error catches real failures; cssClassesFromViewBody / view re-parses run only after view_parse_error validation; layout/component scope-precedence resolution is intentional shadowing, not error masking; stateValueString json.Marshal can't fail on parsed .gwdk literals. --- CHANGELOG.md | 6 +++++- internal/buildgen/build_data_interpolate.go | 7 ++++++- internal/buildgen/build_data_routes_test.go | 23 +++++++++++++++++++-- internal/contractscan/types.go | 16 +++++++++++++- internal/contractscan/types_test.go | 19 +++++++++++++++++ 5 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 internal/contractscan/types_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 21240f7b..f12922c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,7 +45,11 @@ packages, and tooling contracts may change before a stable release. 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. + 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". ### Known Gaps 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 965df032..6fd4ce0a 100644 --- a/internal/buildgen/build_data_routes_test.go +++ b/internal/buildgen/build_data_routes_test.go @@ -518,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/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) + } +} From d0329cd61f88b91dcd1f743e5ae91eaf35052d46 Mon Sep 17 00:00:00 2001 From: Bruno Carvalho Date: Sat, 13 Jun 2026 08:56:55 -0300 Subject: [PATCH 3/3] fix(appgen): surface go list -m failure instead of dropping the app module replace currentAppModule swallowed any 'go list -m -json' error and returned false, so a broken/missing main module silently produced a generated go.mod without the app module's require/replace. When the generated app imports app-owned packages, that go.mod then fails to build with an opaque 'cannot find package'. currentAppModule now returns an error (with the go list stderr), and moduleSource fails loudly with that cause when the app actually imports app-owned packages (appHasLocalModuleImports / isLocalModuleImportPath). When nothing app-owned is imported, the main module is not needed and the failure is still ignored, so stdlib/gowdk-only apps are unaffected. Tests: isLocalModuleImportPath heuristic and appHasLocalModuleImports. Completes a full re-audit for this bug class. The other audited sites are not masking and were intentionally left: - validate_packages goListExportFiles falls back to importer.Default and real type errors still surface via go_package_error; - defaultCSSInputs already errors on a missing configured input, and cssOutputExcludePattern returns "" only when output is outside the CSS root (filepath.Abs effectively cannot fail); - layout/component same-package-then-global resolution is intentional scope shadowing, not error masking; - strconv.Unquote on AST import literals and stateValueString json.Marshal on parsed .gwdk literals cannot fail; - LSP feature paths degrade gracefully while publishing parse errors via CheckSource. --- CHANGELOG.md | 6 ++- internal/appgen/appgen_test.go | 40 ++++++++++++++++ internal/appgen/module.go | 84 ++++++++++++++++++++++++++++------ 3 files changed, 115 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f12922c1..22bbcf9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,11 @@ packages, and tooling contracts may change before a stable release. 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". + "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/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 {