diff --git a/CHANGELOG.md b/CHANGELOG.md index 1616969..95007b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,46 @@ All notable changes to this project are documented here. This project follows (Principle VI), output changes that are *strictly more accurate* are MINOR, not breaking. +## [Unreleased] — Control-Flow Graph Foundation + +### Added — more accurate conditional analysis (MINOR) + +The per-function control-flow graph (previously used only to annotate branches) +is now retained as a compact, queryable reachability + dominance model, and the +conditional-analysis consumers resolve over it instead of a source-text-position +heuristic. This sharpens several cases: + +- **Conditional status codes** (the #39 / #50 / #57 pain) are computed + structurally: a status contributes iff its assignment can *reach* the response + write and is not overwritten on every path by a later, call-dominating + assignment. Mutually-exclusive `if`/`else` (and `switch`) arms fan out; an + early-`return` before the write is excluded; an unconditional overwrite shadows + earlier assignments. **A value assigned inside a loop body now reaches a write + after the loop** (the analysis terminates across the back-edge). +- **Helper-internal type-switch binding**: when a handler funnels a value into a + shared responder that `switch v.(type)`s on it, the call-site argument is bound + to the matching arm — `Respond(w, &NotFound{})` fans out only that arm's status, + not every arm. When the argument's concrete type is not statically known (a bare + `error`/`any`), the analyzer degrades to the unconditionally-reachable result + and emits a warning rather than over-approximating. +- **Method dispatch via `if r.Method == http.MethodPost`** now splits into one + operation per method, the same as a `switch r.Method` already did. +- **Branch-dependent response bodies** are attributed to the status on which they + are written (e.g. `FullUser`/200 vs `ErrorBody`/404), never merged. + +Each behavior ships with its own targeted fixture (`cfg_helper_typeswitch`, +`cfg_loop_status`, `cfg_method_if_dispatch`, `cfg_branch_bodies`); the existing +framework golden corpus does not exercise these constructs, so it — and the +determinism suite — stays byte-identical. + +### Changed — internals + +- The conditional-status fan-out's source-position heuristic (`positionAfter`, + `positionLineCol`, and the "last unconditional index") was **removed** in favor + of the structural reachability predicate. When control flow cannot be modeled, + the analyzer falls back to the unconditionally-reachable (single-path) result + and warns — it never guesses a conditional set. + ## [Unreleased] — TypeRef Metadata Integration (Phase 2) ### Added — more accurate schema output (MINOR) diff --git a/internal/engine/engine_e2e_test.go b/internal/engine/engine_e2e_test.go index b0463ff..01e9baa 100644 --- a/internal/engine/engine_e2e_test.go +++ b/internal/engine/engine_e2e_test.go @@ -62,6 +62,52 @@ func allFrameworks(t *testing.T) []frameworkTestCase { {name: "writejson_helper", inputDir: "../../testdata/writejson_helper", configFn: spec.DefaultHTTPConfig}, {name: "error_switch_minimal", inputDir: "../../testdata/error_switch_minimal", configFn: spec.DefaultChiConfig}, {name: "error_switch_file_service", inputDir: "../../testdata/error_switch_file_service", configFn: spec.DefaultChiConfig}, + // spec 009: a response helper that type-switches on its argument. The + // concrete-type route fans out only the matched arm; the imprecise route + // degrades to the default arm + warns (FR-011/FR-012). + {name: "cfg_helper_typeswitch", inputDir: "../../testdata/cfg_helper_typeswitch", configFn: spec.DefaultHTTPConfig}, + // spec 009: a status assigned inside a loop body still reaches the response + // write (FR-010 — the loop back-edge terminates and the value contributes). + {name: "cfg_loop_status", inputDir: "../../testdata/cfg_loop_status", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: an `if r.Method == …` dispatch splits into one operation per + // method, the same as a `switch r.Method` (FR-003). + {name: "cfg_method_if_dispatch", inputDir: "../../testdata/cfg_method_if_dispatch", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2 (FR-003): the switch-form mirror of cfg_method_if_dispatch, with + // net/http CONSTANT cases (`case http.MethodGet:`). Must split identically — + // extractCaseValues resolves the constants the same way extractMethodGuard does. + {name: "cfg_method_switch_dispatch", inputDir: "../../testdata/cfg_method_switch_dispatch", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: a method dispatch combined with an INDEPENDENT pre-dispatch + // conditional — the independent 500 is carried onto every method operation + // (CFG: orthogonal to the dispatch), not dropped as the pre-CFG split did. + {name: "cfg_method_if_independent", inputDir: "../../testdata/cfg_method_if_independent", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: cross-function guard — a method arm that splits AND calls a + // helper writing a conditional response. The helper response's branch is in the + // HELPER's CFG, so the classifier must not reason about it against the handler's + // CFG (that would leak it onto the other method); it is conservatively excluded. + {name: "cfg_method_helper_response", inputDir: "../../testdata/cfg_method_helper_response", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: a `fallthrough` into a `switch r.Method` default — the 405 is + // recognised structurally as the dispatch fallback and excluded, NOT leaked onto + // GET/POST despite the fallthrough edge making it reachable from the POST arm. + {name: "cfg_method_switch_fallthrough", inputDir: "../../testdata/cfg_method_switch_fallthrough", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: TWO `switch r.Method` dispatches with an independent 401 between + // them — the dispatch root is scoped to one dispatch's arms, so the 401 is shared + // onto GET+POST, not over-excluded by a root spanning both dispatches. + {name: "cfg_method_two_dispatch", inputDir: "../../testdata/cfg_method_two_dispatch", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: a COMBINED case (`case GET, POST:`) + a `default` — the combined + // arm lowers to one block dominated by itself, so the dispatch root must come from + // the recorded group (all arms incl. default); the 405 stays the fallback and is + // not leaked onto the GET/POST operations the combined case splits into. + {name: "cfg_method_combined_default", inputDir: "../../testdata/cfg_method_combined_default", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: responses split across an `if r.Method ==` arm AND a `switch r.Method` + // with a default — distinct dispatch groups; the root spans BOTH contributing groups so + // the switch default 405 is excluded, not leaked onto GET (if-arm) and POST (switch-arm). + {name: "cfg_method_if_switch_default", inputDir: "../../testdata/cfg_method_if_switch_default", configFn: spec.DefaultHTTPConfig}, + // spec 009 US2: a `switch` over a COPY of r.Method (`m := r.Method; switch m`) — recognised + // by its method-named case values (not the tag), so the default 405 is excluded. + {name: "cfg_method_switch_copy", inputDir: "../../testdata/cfg_method_switch_copy", configFn: spec.DefaultHTTPConfig}, + // spec 009 US3: branch-dependent response bodies are attributed to the status + // on which they are written — 200/FullUser vs 404/ErrorBody, not merged (FR-005). + {name: "cfg_branch_bodies", inputDir: "../../testdata/cfg_branch_bodies", configFn: spec.DefaultHTTPConfig}, {name: "bodyless_status", inputDir: "../../testdata/bodyless_status", configFn: spec.DefaultHTTPConfig}, {name: "wrapped_response", inputDir: "../../testdata/wrapped_response", configFn: spec.DefaultHTTPConfig}, {name: "echo_handler_factory", inputDir: "../../testdata/echo_handler_factory", configFn: spec.DefaultEchoConfig}, diff --git a/internal/metadata/cfg.go b/internal/metadata/cfg.go index e19d4de..bf91443 100644 --- a/internal/metadata/cfg.go +++ b/internal/metadata/cfg.go @@ -17,25 +17,33 @@ package metadata import ( "go/ast" "go/token" + "go/types" + "strings" "golang.org/x/tools/go/cfg" ) // BuildFunctionCFGs builds control-flow graphs for the given function -// declarations and annotates existing CallGraphEdge and Assignment entries -// in the metadata with BranchContext information. +// declarations, annotates existing CallGraphEdge and Assignment entries with +// BranchContext, and retains a compact per-function reachability model in +// meta.FunctionCFGs (spec 009, FR-001). declInfo provides each function's +// *types.Info for type-switch case-type capture (nil-tolerant). // -// This is additive — edges/assignments without branch context (unconditional -// code) keep Branch == nil. Only statements inside if/else/switch branches -// get annotated. -func BuildFunctionCFGs(funcDecls []*ast.FuncDecl, fset *token.FileSet, meta *Metadata) { +// The raw *cfg.CFG is local and dropped after build; only the compact model and +// the BranchContext annotations survive. +func BuildFunctionCFGs(funcDecls []*ast.FuncDecl, declInfo map[*ast.FuncDecl]*types.Info, fset *token.FileSet, meta *Metadata) { if meta == nil || len(funcDecls) == 0 { return } - // Build position→edge and position→assignment indexes for fast lookup edgesByPos := buildEdgePositionIndex(meta, fset) assignsByPos := buildAssignmentPositionIndex(meta, fset) + if meta.FunctionCFGs == nil { + meta.FunctionCFGs = make(map[string]*FunctionCFG, len(funcDecls)) + } + if meta.cfgPosToFn == nil { + meta.cfgPosToFn = make(map[string]string) + } for _, decl := range funcDecls { if decl.Body == nil { @@ -43,8 +51,77 @@ func BuildFunctionCFGs(funcDecls []*ast.FuncDecl, fset *token.FileSet, meta *Met } // Build CFG for this function. Conservative mayReturn: always true. graph := cfg.New(decl.Body, func(*ast.CallExpr) bool { return true }) - annotateBranches(graph, fset, meta, edgesByPos, assignsByPos) + fnKey := fset.Position(decl.Body.Pos()).String() // unique per function body + tsOperands := typeSwitchOperands(decl.Body) + tsDefaults := typeSwitchDefaultArms(decl.Body) + dispatchGroups := methodDispatchGroups(decl.Body, declInfo[decl]) + annotateBranches(graph, fset, declInfo[decl], meta, edgesByPos, assignsByPos, fnKey, tsOperands, tsDefaults, dispatchGroups) + } +} + +// methodDispatchGroups maps the source position of each method-dispatch ARM +// statement (a method-`switch` case clause INCLUDING the default, or each +// `if r.Method ==` chain IfStmt) to a group id — the dispatch statement's own +// position — so every arm of ONE dispatch shares an id. A switch counts when any of its +// case values names an HTTP method (switchHasMethodCase) — the SAME test the route +// splitter uses to attribute a case to a method, so `switch r.Method`, `switch m := +// r.Method`, and a method-named value-switch are all grouped exactly as they are split, +// and the group includes the `default` arm. String-literal cases need no type info; +// `http.MethodXxx` constant cases resolve via it. (The if path stays type-gated inside +// extractMethodGuard, matching how if-arm CaseValues are attributed.) +func methodDispatchGroups(body *ast.BlockStmt, info *types.Info) map[token.Pos]int { + groups := make(map[token.Pos]int) + ast.Inspect(body, func(n ast.Node) bool { + switch s := n.(type) { + case *ast.SwitchStmt: + if s.Body != nil && switchHasMethodCase(s, info) { + gid := int(s.Pos()) + for _, cc := range s.Body.List { + if clause, ok := cc.(*ast.CaseClause); ok { + groups[clause.Pos()] = gid + } + } + } + case *ast.IfStmt: + if _, done := groups[s.Pos()]; done { + return true // already linked as the else-if of a method chain + } + if len(extractMethodGuard(s, info)) > 0 { + gid := int(s.Pos()) + for cur := s; cur != nil; { // walk the else-if chain into one group + groups[cur.Pos()] = gid + if elseIf, ok := cur.Else.(*ast.IfStmt); ok { + cur = elseIf + } else { + cur = nil + } + } + } + } + return true + }) + return groups +} + +// switchHasMethodCase reports whether any case clause of s carries a value that names +// an HTTP method — using extractCaseValues + isHTTPMethodName, the exact pair the route +// splitter applies (via BranchContext.CaseValues) to decide a case is a method arm. So a +// switch is grouped as a method dispatch iff its cases are the ones the splitter fans +// out per method — independent of the switch TAG, which the splitter never inspects. +// This intentionally also matches a value-switch whose cases name methods: the splitter +// already fans those out (a pre-existing limitation), so grouping them keeps the +// dispatch model consistent with the split rather than letting their `default` leak. +func switchHasMethodCase(s *ast.SwitchStmt, info *types.Info) bool { + for _, cc := range s.Body.List { + if clause, ok := cc.(*ast.CaseClause); ok { + for _, v := range extractCaseValues(clause, info) { + if isHTTPMethodName(strings.ToUpper(v)) { + return true + } + } + } } + return false } // buildEdgePositionIndex creates a map from source position string to @@ -116,49 +193,148 @@ func buildAssignmentPositionIndex(meta *Metadata, _ *token.FileSet) map[string][ return index } -// annotateBranches walks the CFG blocks and tags edges/assignments with -// BranchContext based on the block's Kind and parent statement. -func annotateBranches(graph *cfg.CFG, fset *token.FileSet, meta *Metadata, - edgesByPos map[string]*CallGraphEdge, assignsByPos map[string][]*Assignment) { +// annotateBranches walks the CFG blocks: it (1) builds the compact FunctionCFG +// for fnKey — successor adjacency + a position→block map over ALL live blocks +// (research R3: the response call usually sits in the post-if merge block, which +// the BranchContext gate below skips) + dominators — and (2) tags edges/assignments +// with BranchContext (including type-switch case types). +func annotateBranches(graph *cfg.CFG, fset *token.FileSet, info *types.Info, meta *Metadata, //nolint:gocyclo // single CFG walk doing reachability capture + branch annotation + edgesByPos map[string]*CallGraphEdge, assignsByPos map[string][]*Assignment, fnKey string, + tsOperands map[token.Pos]string, tsDefaults []tsDefaultArm, dispatchGroups map[token.Pos]int) { + nb := len(graph.Blocks) + fc := &FunctionCFG{ + Blocks: make([]BlockInfo, nb), + Succs: make([][]int32, nb), + PosToBlock: make(map[string]BlockLoc), + DispatchArms: make(map[int][]int32), + } + for i := range fc.Blocks { + fc.Blocks[i] = BlockInfo{Index: int32(i)} //nolint:gosec // block counts are small + } + + // recordPos stores a position → block location in both the raw and + // repo-root-stripped forms (consumers carry the stripped form — #27). + // + // First-write-wins over blocks iterated in index order. A position go/cfg emits + // in two live blocks (e.g. a `select` comm-clause receive ident that appears in + // both the dispatch block and the SelectCaseBody) is therefore claimed by the + // lower-indexed block. This is a rare imprecision (no panic; the status consumer + // would treat a select-conditional value as unconditional) — left as-is rather + // than tracked because no realistic handler shape triggers it. + recordPos := func(p string, loc BlockLoc) { + if _, exists := fc.PosToBlock[p]; !exists { + fc.PosToBlock[p] = loc + } + meta.cfgPosToFn[p] = fnKey + if repoRoot != "" { + if stripped := strings.TrimPrefix(p, repoRoot); stripped != p { + if _, exists := fc.PosToBlock[stripped]; !exists { + fc.PosToBlock[stripped] = loc + } + meta.cfgPosToFn[stripped] = fnKey + } + } + } + for _, block := range graph.Blocks { - if !block.Live { + if !block.Live || int(block.Index) >= nb { continue } - - // Determine branch kind from block's Kind + bi := block.Index branchKind := mapBlockKind(block.Kind) - if branchKind == "" { - continue // Unconditional block — no annotation needed + + succs := make([]int32, 0, len(block.Succs)) + for _, s := range block.Succs { + if s != nil { + succs = append(succs, s.Index) + } } + fc.Blocks[bi] = BlockInfo{Index: bi, Kind: branchKind, NodeCount: int32(len(block.Nodes))} //nolint:gosec // small counts + fc.Succs[bi] = succs - // Get parent statement position for context + // (1) Reachability capture: ALL live blocks, every node → (block, node index). + // The same walk annotates type-switch default-arm writes (go/cfg folds the + // default body into the post-switch block, which the branch pass below skips). + // + // Do NOT descend into nested function literals: go/cfg builds no CFG for a + // closure body, so mapping a closure's inner positions to THIS function's + // block would place them in the wrong control flow and let a closure's + // statuses bleed into the enclosing handler's reachability. Closures get no + // model and their consumers degrade cleanly (BlockFor → not found). + for nodeIdx, node := range block.Nodes { + loc := BlockLoc{Block: bi, Node: int32(nodeIdx)} //nolint:gosec // small counts + ast.Inspect(node, func(nn ast.Node) bool { + if nn == nil { + return false + } + if _, isFuncLit := nn.(*ast.FuncLit); isFuncLit { + return false // a closure has its own scope; don't fold it into this block + } + posStr := fset.Position(nn.Pos()).String() + recordPos(posStr, loc) + annotateDefaultArm(nn.Pos(), posStr, tsDefaults, edgesByPos, assignsByPos) + return true + }) + } + + // (2) BranchContext annotation — only for if/else/switch/select body blocks. + if branchKind == "" { + continue + } var parentStmtPos int if block.Stmt != nil { - parentStmtPos = meta.StringPool.Get(fset.Position(block.Stmt.Pos()).String()) + stmtPos := fset.Position(block.Stmt.Pos()).String() + parentStmtPos = meta.StringPool.Get(stmtPos) + // Register the parent-statement position → fnKey so a consumer holding only + // a BranchContext (splitByConditionalMethods) can resolve it back to this + // function via FnKeyForPos, then query dominance between branch blocks to + // classify non-method conditionals. cfgPosToFn is transient (not serialized); + // double-key raw + repo-root-stripped to mirror recordPos (#27). + meta.cfgPosToFn[stmtPos] = fnKey + if repoRoot != "" { + if stripped := strings.TrimPrefix(stmtPos, repoRoot); stripped != stmtPos { + meta.cfgPosToFn[stripped] = fnKey + } + } } - ctx := &BranchContext{ - BlockIndex: block.Index, + BlockIndex: bi, BlockKind: branchKind, ParentStmtPos: parentStmtPos, } - - // For switch-case blocks, extract case clause literal values if branchKind == "switch-case" && block.Stmt != nil { - ctx.CaseValues = extractCaseValues(block.Stmt) + ctx.CaseValues = extractCaseValues(block.Stmt, info) + ctx.CaseTypeRefs = extractCaseTypeRefs(block.Stmt, info) + ctx.SwitchOperand = tsOperands[block.Stmt.Pos()] + } + // An `if r.Method == ` guard makes its then-arm method-conditional, the + // same way a `switch r.Method` case is (spec 009, US2/FR-003). Recording the + // method in CaseValues lets splitByConditionalMethods fan the handler out per + // method whether it branches via switch or if. + if branchKind == "if-then" && block.Stmt != nil { + ctx.CaseValues = extractMethodGuard(block.Stmt, info) + } + // Record this arm under its method-dispatch group (a method `switch` or a chained + // `if r.Method ==`), INCLUDING the switch `default`, so a consumer can recover the + // exact dispatch — its tag (the common dominator of the group's arms) and every arm, + // even one whose response was lost to an early-return. methodDispatchGroups detects a + // dispatch switch by its case values (switchHasMethodCase) — the same test the route + // splitter uses — so grouping stays consistent with the split (spec 009, US2). + if block.Stmt != nil { + if g, ok := dispatchGroups[block.Stmt.Pos()]; ok { + ctx.DispatchGroup = g + fc.DispatchArms[g] = append(fc.DispatchArms[g], bi) + } } - - // Walk all AST nodes in this block and tag matching edges/assignments. - // block.Nodes are statement-level; the position of an AssignStmt like - // `_, _ = w.Write(...)` is the underscore, while the call edge is - // indexed at the inner CallExpr's position. Descend into every node - // so each inner CallExpr/AssignStmt gets a chance to match. for _, node := range block.Nodes { - ast.Inspect(node, func(n ast.Node) bool { - if n == nil { + ast.Inspect(node, func(nn ast.Node) bool { + if nn == nil { return false } - pos := fset.Position(n.Pos()).String() + if _, isFuncLit := nn.(*ast.FuncLit); isFuncLit { + return false // a closure's edges belong to its own scope, not this branch + } + pos := fset.Position(nn.Pos()).String() if edge, ok := edgesByPos[pos]; ok { edge.Branch = ctx } @@ -169,29 +345,287 @@ func annotateBranches(graph *cfg.CFG, fset *token.FileSet, meta *Metadata, }) } } + + fc.Dominators = computeDominators(fc.Succs) + meta.FunctionCFGs[fnKey] = fc } -// extractCaseValues extracts string literal values from a case clause statement. -// For example, `case "GET", "HEAD":` returns ["GET", "HEAD"]. -func extractCaseValues(stmt ast.Stmt) []string { +// extractCaseValues extracts the case values of a case clause. String literals are +// returned verbatim (`case "GET", "active":` → ["GET", "active"]); an `http.MethodXxx` +// selector constant is resolved to its method name (`case http.MethodGet:` → "GET"), +// type-gated to net/http via httpMethodName. So a `switch r.Method` dispatch splits +// the same whether its cases are string literals or net/http constants — matching the +// `if r.Method == http.MethodGet` guard (spec 009, US2/FR-003). Needs type info to +// resolve constants; without it only string literals are returned. +func extractCaseValues(stmt ast.Stmt, info *types.Info) []string { cc, ok := stmt.(*ast.CaseClause) if !ok || cc == nil { return nil } var values []string for _, expr := range cc.List { - if lit, ok := expr.(*ast.BasicLit); ok { - // Strip quotes from string literals - val := lit.Value + switch v := expr.(type) { + case *ast.BasicLit: + // Strip quotes from string literals. + val := v.Value if len(val) >= 2 && val[0] == '"' && val[len(val)-1] == '"' { val = val[1 : len(val)-1] } values = append(values, val) + case *ast.SelectorExpr: + if m := httpMethodName(v, info); m != "" { + values = append(values, m) + } } } return values } +// extractMethodGuard returns the HTTP method named by an `if .Method == ` +// (or ` == .Method`) condition — "POST" for both `== "POST"` and +// `== http.MethodPost` — so a method-conditional `if` dispatch fans out per method +// the same way a `switch r.Method` does (spec 009, US2/FR-003). +// +// It is gated on TYPE INFO: the `.Method` operand must resolve to +// net/http.Request.Method, and a `MethodXxx` constant operand must resolve from the +// net/http package. This prevents unrelated business logic (`s.Method == "GET"`, +// `foo.MethodPost == r.Method`) from being misread as route method dispatch. +// Returns nil for any other condition (and when type info is unavailable). +func extractMethodGuard(stmt ast.Stmt, info *types.Info) []string { + ifStmt, ok := stmt.(*ast.IfStmt) + if !ok || ifStmt.Cond == nil { + return nil + } + bin, ok := ifStmt.Cond.(*ast.BinaryExpr) + if !ok || bin.Op != token.EQL { + return nil + } + if isHTTPRequestMethod(bin.X, info) { + if m := httpMethodName(bin.Y, info); m != "" { + return []string{m} + } + } + if isHTTPRequestMethod(bin.Y, info) { + if m := httpMethodName(bin.X, info); m != "" { + return []string{m} + } + } + return nil +} + +// isHTTPRequestMethod reports whether e is a `.Method` selector whose receiver +// type-resolves to net/http.Request (value or pointer). Requires type info; without +// it (or for any non-Request `.Method`) returns false — conservative, so a business +// type's `.Method` field is never mistaken for the HTTP method. +func isHTTPRequestMethod(e ast.Expr, info *types.Info) bool { + sel, ok := e.(*ast.SelectorExpr) + if !ok || sel.Sel == nil || sel.Sel.Name != "Method" || info == nil { + return false + } + return isNetHTTPNamed(info.TypeOf(sel.X), "Request") +} + +// isNetHTTPNamed reports whether t (after one optional pointer deref) is the named +// type `name` declared in package net/http. Type aliases are resolved: under Go's +// default gotypesalias=1 an alias like `type Req = http.Request` is a *types.Alias, +// not a *types.Named, so Unalias is needed before/after the deref to recognise an +// aliased (or pointer-to-aliased) request type. +func isNetHTTPNamed(t types.Type, name string) bool { + if t == nil { + return false + } + t = types.Unalias(t) + if p, ok := t.(*types.Pointer); ok { + t = types.Unalias(p.Elem()) + } + named, ok := t.(*types.Named) + if !ok { + return false + } + obj := named.Obj() + return obj != nil && obj.Pkg() != nil && obj.Pkg().Path() == "net/http" && obj.Name() == name +} + +// httpMethodName resolves an HTTP-method expression to its canonical method string: +// a string literal ("post"), or an `http.MethodXxx` selector that type info confirms +// resolves from net/http (MethodPost → POST). Returns "" otherwise. +func httpMethodName(e ast.Expr, info *types.Info) string { + switch v := e.(type) { + case *ast.BasicLit: + if v.Kind == token.STRING { + if s := strings.ToUpper(strings.Trim(v.Value, `"`)); isHTTPMethodName(s) { + return s + } + } + case *ast.SelectorExpr: + if v.Sel == nil || info == nil { + return "" + } + // Only a constant declared in net/http (http.MethodXxx) counts. + obj := info.ObjectOf(v.Sel) + if obj == nil || obj.Pkg() == nil || obj.Pkg().Path() != "net/http" { + return "" + } + if strings.HasPrefix(v.Sel.Name, "Method") && len(v.Sel.Name) > len("Method") { + if m := strings.ToUpper(strings.TrimPrefix(v.Sel.Name, "Method")); isHTTPMethodName(m) { + return m + } + } + } + return "" +} + +// isHTTPMethodName reports whether s (already upper-cased) is an HTTP method the +// analyzer splits operations on. It is deliberately kept in sync with the spec-side +// consumer (isValidHTTPMethodStr in internal/spec): capturing a method here that the +// consumer would later drop would silently no-op, so both sides list the same seven +// methods (CONNECT/TRACE are intentionally excluded — they are not REST operations). +func isHTTPMethodName(s string) bool { + switch s { + case "GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "HEAD": + return true + } + return false +} + +// extractCaseTypeRefs extracts the case TYPES from a type-switch case clause +// (`switch x.(type) { case *T, S: ... }`) as TypeRefs, using go/types to confirm +// each case expression is a type (not a value, which would be a value-switch). +// Returns nil for value-switches, for `case nil:`/`default:` (the unconditional +// arm), and when type info is unavailable (spec 009, FR-011/R5). +func extractCaseTypeRefs(stmt ast.Stmt, info *types.Info) []*TypeRef { + cc, ok := stmt.(*ast.CaseClause) + if !ok || cc == nil || info == nil { + return nil + } + var refs []*TypeRef + for _, expr := range cc.List { + tv, known := info.Types[expr] + if !known || !tv.IsType() { + continue // a value expression (value-switch) or untyped nil — not a type case + } + if ref := TypeRefFromExpr(expr, info); ref != nil { + refs = append(refs, ref) + } + } + return refs +} + +// typeSwitchOperands maps each type-switch CaseClause (by its Pos) to the name of +// the variable being switched on — `switch x := v.(type)` and `switch v.(type)` +// both yield "v". This lets the per-case BranchContext record which parameter the +// switch discriminates, so a consumer can bind it to the call-site argument via +// ParamArgMap (spec 009, FR-011). Only simple ident operands are recorded; a +// selector/index operand yields "" (the consumer then degrades). +func typeSwitchOperands(body *ast.BlockStmt) map[token.Pos]string { + out := make(map[token.Pos]string) + ast.Inspect(body, func(n ast.Node) bool { + ts, ok := n.(*ast.TypeSwitchStmt) + if !ok || ts.Body == nil { + return true + } + operand := typeSwitchOperandName(ts) + if operand == "" { + return true + } + for _, stmt := range ts.Body.List { + if cc, ok := stmt.(*ast.CaseClause); ok { + out[cc.Pos()] = operand + } + } + return true + }) + return out +} + +// tsDefaultArm is the source-position span of a type-switch `default:` clause body +// plus the switched operand, used to annotate default-arm writes that go/cfg folds +// into the post-switch block (see annotateDefaultArm). +type tsDefaultArm struct { + lo, hi token.Pos + operand string +} + +// typeSwitchDefaultArms returns the body spans of every type-switch `default:` +// clause in body, each paired with its switched operand. Value/expression switch +// defaults are NOT included (only *ast.TypeSwitchStmt). +func typeSwitchDefaultArms(body *ast.BlockStmt) []tsDefaultArm { + var out []tsDefaultArm + ast.Inspect(body, func(n ast.Node) bool { + ts, ok := n.(*ast.TypeSwitchStmt) + if !ok || ts.Body == nil { + return true + } + operand := typeSwitchOperandName(ts) + if operand == "" { + return true // a non-ident operand cannot be bound to a call-site argument + } + for _, stmt := range ts.Body.List { + if cc, ok := stmt.(*ast.CaseClause); ok && cc.List == nil { // nil List ⇒ default clause + out = append(out, tsDefaultArm{lo: cc.Pos(), hi: cc.End(), operand: operand}) + } + } + return true + }) + return out +} + +// annotateDefaultArm tags an edge/assignment inside a type-switch `default:` clause +// with a switch-case BranchContext carrying the operand and NO case types (the +// default marker), so a consumer can distinguish the default arm from code outside +// the switch (spec 009, FR-011). It only sets a Branch that is still nil, never +// overriding a real case annotation. When type-switch defaults nest, the TIGHTEST +// enclosing span wins, so an inner default binds to the inner operand rather than +// the lexically-enclosing outer one. +func annotateDefaultArm(pos token.Pos, posStr string, defaults []tsDefaultArm, + edgesByPos map[string]*CallGraphEdge, assignsByPos map[string][]*Assignment) { + best := -1 + var bestWidth token.Pos + for i := range defaults { + if pos < defaults[i].lo || pos >= defaults[i].hi { + continue + } + if w := defaults[i].hi - defaults[i].lo; best == -1 || w < bestWidth { + best, bestWidth = i, w + } + } + if best == -1 { + return + } + ctx := &BranchContext{BlockKind: "switch-case", SwitchOperand: defaults[best].operand} + if edge, ok := edgesByPos[posStr]; ok && edge.Branch == nil { + edge.Branch = ctx + } + for _, assign := range assignsByPos[posStr] { + if assign.Branch == nil { + assign.Branch = ctx + } + } +} + +// typeSwitchOperandName returns the operand identifier of a type switch — the X in +// `X.(type)`, whether wrapped in an assignment (`x := X.(type)`) or a bare guard +// (`X.(type)`). Returns "" when X is not a simple ident. +func typeSwitchOperandName(ts *ast.TypeSwitchStmt) string { + var assertX ast.Expr + switch a := ts.Assign.(type) { + case *ast.AssignStmt: + if len(a.Rhs) == 1 { + if ta, ok := a.Rhs[0].(*ast.TypeAssertExpr); ok { + assertX = ta.X + } + } + case *ast.ExprStmt: + if ta, ok := a.X.(*ast.TypeAssertExpr); ok { + assertX = ta.X + } + } + if id, ok := assertX.(*ast.Ident); ok { + return id.Name + } + return "" +} + // mapBlockKind converts a cfg.BlockKind to a human-readable branch kind string. // Returns "" for unconditional blocks (no annotation needed). func mapBlockKind(kind cfg.BlockKind) string { diff --git a/internal/metadata/cfg_test.go b/internal/metadata/cfg_test.go index 52a3b34..426423e 100644 --- a/internal/metadata/cfg_test.go +++ b/internal/metadata/cfg_test.go @@ -16,18 +16,118 @@ package metadata import ( "go/ast" + "go/importer" "go/parser" "go/token" + "go/types" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// typeCheckSrc parses and type-checks a single-file source, returning the file and +// its types.Info (best-effort; type errors are ignored so partial info still +// populates Types). +func typeCheckSrc(t *testing.T, src string) (*ast.File, *types.Info) { + t.Helper() + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "test.go", src, 0) + require.NoError(t, err) + info := &types.Info{ + Types: make(map[ast.Expr]types.TypeAndValue), + Defs: make(map[*ast.Ident]types.Object), + Uses: make(map[*ast.Ident]types.Object), + } + conf := types.Config{Importer: importer.Default(), Error: func(error) {}} + _, _ = conf.Check("test", fset, []*ast.File{file}, info) + return file, info +} + +func typeSwitchCases(t *testing.T, file *ast.File) []*ast.CaseClause { + t.Helper() + var cases []*ast.CaseClause + ast.Inspect(file, func(n ast.Node) bool { + if ts, ok := n.(*ast.TypeSwitchStmt); ok && ts.Body != nil { + for _, stmt := range ts.Body.List { + if cc, ok := stmt.(*ast.CaseClause); ok { + cases = append(cases, cc) + } + } + } + return true + }) + return cases +} + +func TestExtractCaseTypeRefs(t *testing.T) { + file, info := typeCheckSrc(t, `package main + +type NotFound struct{} +type Conflict struct{} + +func classify(err error) int { + switch err.(type) { + case *NotFound: + return 404 + case *Conflict: + return 409 + case nil: + return 200 + default: + return 500 + } +} +`) + cases := typeSwitchCases(t, file) + require.Len(t, cases, 4) + + // case *NotFound: / case *Conflict: → one type ref each. + nf := extractCaseTypeRefs(cases[0], info) + require.Len(t, nf, 1) + assert.Contains(t, nf[0].String(), "NotFound") + cf := extractCaseTypeRefs(cases[1], info) + require.Len(t, cf, 1) + assert.Contains(t, cf[0].String(), "Conflict") + + // case nil: and default: are the unconditional arm — no type captured. + assert.Empty(t, extractCaseTypeRefs(cases[2], info)) + assert.Empty(t, extractCaseTypeRefs(cases[3], info)) + + // Without type info we cannot tell type cases from value cases → nil. + assert.Nil(t, extractCaseTypeRefs(cases[0], nil)) +} + +func TestExtractCaseTypeRefs_ValueSwitchIsNotAType(t *testing.T) { + file, info := typeCheckSrc(t, `package main + +func pick(s string) int { + switch s { + case "GET": + return 1 + default: + return 0 + } +} +`) + // A value-switch case is NOT a TypeSwitchStmt, so typeSwitchCases finds none and + // extractCaseTypeRefs (called on the value case clause) yields no type refs. + assert.Empty(t, typeSwitchCases(t, file)) + var valueCases []*ast.CaseClause + ast.Inspect(file, func(n ast.Node) bool { + if cc, ok := n.(*ast.CaseClause); ok { + valueCases = append(valueCases, cc) + } + return true + }) + require.NotEmpty(t, valueCases) + assert.Empty(t, extractCaseTypeRefs(valueCases[0], info)) // "GET" is a value, not a type +} + func TestBuildFunctionCFGs_NilMetadata(_ *testing.T) { // Should not panic - BuildFunctionCFGs(nil, nil, nil) - BuildFunctionCFGs([]*ast.FuncDecl{}, nil, nil) + BuildFunctionCFGs(nil, nil, nil, nil) + BuildFunctionCFGs([]*ast.FuncDecl{}, nil, nil, nil) } func TestBuildFunctionCFGs_IfElseBranches(t *testing.T) { @@ -63,7 +163,64 @@ func handler(w http.ResponseWriter, r *http.Request) { } // Should not panic even with empty call graph - BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, nil, fset, meta) +} + +func TestBuildFunctionCFGs_SkipsClosureBodies(t *testing.T) { + // A nested function literal has its own scope and no CFG of its own, so its + // inner positions must NOT be folded into the enclosing function's model — + // otherwise a closure's statuses bleed into the handler's reachability. + src := `package main + +import "net/http" + +func h(w http.ResponseWriter) { + cb := func() { w.WriteHeader(500) } + _ = cb + w.WriteHeader(200) +} +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "test.go", src, 0) + require.NoError(t, err) + + var fn *ast.FuncDecl + for _, d := range file.Decls { + if f, ok := d.(*ast.FuncDecl); ok && f.Name.Name == "h" { + fn = f + } + } + require.NotNil(t, fn) + + // Locate the outer WriteHeader(200) call and the closure-body WriteHeader(500). + var outerCall, innerCall *ast.CallExpr + ast.Inspect(fn, func(n ast.Node) bool { + switch v := n.(type) { + case *ast.FuncLit: + ast.Inspect(v.Body, func(m ast.Node) bool { + if c, ok := m.(*ast.CallExpr); ok { + innerCall = c + } + return true + }) + return false // don't let outerCall pick up the closure's call + case *ast.CallExpr: + outerCall = v + } + return true + }) + require.NotNil(t, outerCall) + require.NotNil(t, innerCall) + + meta := &Metadata{StringPool: NewStringPool(), CallGraph: []CallGraphEdge{}} + BuildFunctionCFGs([]*ast.FuncDecl{fn}, nil, fset, meta) + fc := meta.FunctionCFGs[fset.Position(fn.Body.Pos()).String()] + require.NotNil(t, fc) + + _, hasOuter := fc.PosToBlock[fset.Position(outerCall.Pos()).String()] + _, hasInner := fc.PosToBlock[fset.Position(innerCall.Pos()).String()] + assert.True(t, hasOuter, "the enclosing function's own call is captured") + assert.False(t, hasInner, "a closure-body position must not be folded into the enclosing function") } func TestBuildFunctionCFGs_SwitchCaseBranches(t *testing.T) { @@ -98,7 +255,7 @@ func dispatch(method string) { CallGraph: []CallGraphEdge{}, } - BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, nil, fset, meta) } func TestBuildFunctionCFGs_UnconditionalCode(t *testing.T) { @@ -128,12 +285,105 @@ func simple() { } // Unconditional code: no edges should get Branch annotations - BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, nil, fset, meta) for _, edge := range meta.CallGraph { assert.Nil(t, edge.Branch, "unconditional code should have nil Branch") } } +// TestBuildFunctionCFGs_DispatchArms drives the full build with real type info over a +// `switch r.Method` handler, asserting the per-function CFG records every arm of the +// method dispatch (incl. the default) under one group — the data contributingDispatchArms +// / commonDominator consume to scope a method split. +func TestBuildFunctionCFGs_DispatchArms(t *testing.T) { + src := `package main +import "net/http" +func handler(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "GET": + w.WriteHeader(200) + case "POST": + w.WriteHeader(201) + default: + w.WriteHeader(405) + } +} +` + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "test.go", src, 0) + require.NoError(t, err) + info := &types.Info{ + Types: make(map[ast.Expr]types.TypeAndValue), + Defs: make(map[*ast.Ident]types.Object), + Uses: make(map[*ast.Ident]types.Object), + } + conf := types.Config{Importer: importer.Default(), Error: func(error) {}} + _, _ = conf.Check("test", fset, []*ast.File{file}, info) + + var funcDecl *ast.FuncDecl + for _, decl := range file.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Name.Name == "handler" { + funcDecl = fn + break + } + } + require.NotNil(t, funcDecl) + + meta := &Metadata{StringPool: NewStringPool(), CallGraph: []CallGraphEdge{}} + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, map[*ast.FuncDecl]*types.Info{funcDecl: info}, fset, meta) + + require.Len(t, meta.FunctionCFGs, 1) + var fc *FunctionCFG + for _, c := range meta.FunctionCFGs { + fc = c + } + require.NotNil(t, fc) + require.Len(t, fc.DispatchArms, 1, "one method-dispatch group") + + // lcd = lowest common dominator of two blocks, walking the idom tree. + lcd := func(a, b int32) int32 { + anc := map[int32]bool{a: true} + for x := a; fc.Dominators[x] >= 0; { + x = fc.Dominators[x] + anc[x] = true + } + y := b + for !anc[y] && fc.Dominators[y] >= 0 { + y = fc.Dominators[y] + } + return y + } + + for _, arms := range fc.DispatchArms { + require.Len(t, arms, 3, "GET, POST, and the default arm all recorded") + // The arms are mutually-exclusive siblings of ONE switch: no arm reaches another. + // This is what tells the real default arm apart from the post-switch merge block + // (which every arm WOULD reach) — a regression that recorded the merge in place of + // the default would keep len==3 but be caught here. + for _, a := range arms { + for _, b := range arms { + if a != b { + assert.False(t, fc.blockReaches(a, b), + "arm %d must not reach sibling arm %d (else it is not a switch arm)", a, b) + } + } + } + // Their common dominator is the switch TAG — a block that dominates every arm + // (incl. the default) and is itself no arm. This is exactly the value + // commonDominator turns the group into to scope the dispatch; the combined-case + // fix depends on the default being in the group so this LCA is the tag, not an arm. + tag := arms[0] + for _, a := range arms[1:] { + tag = lcd(tag, a) + } + for _, arm := range arms { + assert.True(t, blockDominates(fc.Dominators, tag, arm), + "dispatch tag %d dominates arm %d", tag, arm) + } + assert.NotContains(t, arms, tag, "the dispatch tag is a common dominator, not one of the arms") + } +} + func TestMapBlockKind(t *testing.T) { tests := []struct { name string @@ -168,3 +418,216 @@ func TestBranchContext_Struct(t *testing.T) { assert.Equal(t, "if-then", ctx.BlockKind) assert.Equal(t, 42, ctx.ParentStmtPos) } + +// --- spec 009: type-switch operand + default-arm capture ------------------- + +func tsFindSwitch(file *ast.File) *ast.TypeSwitchStmt { + var ts *ast.TypeSwitchStmt + ast.Inspect(file, func(n ast.Node) bool { + if t, ok := n.(*ast.TypeSwitchStmt); ok && ts == nil { + ts = t + } + return true + }) + return ts +} + +func tsFuncBody(file *ast.File, name string) *ast.BlockStmt { + for _, d := range file.Decls { + if fn, ok := d.(*ast.FuncDecl); ok && fn.Name.Name == name { + return fn.Body + } + } + return nil +} + +func TestTypeSwitchOperandName(t *testing.T) { + // assignment form: switch x := v.(type) + f1, _ := typeCheckSrc(t, "package p\nfunc h(v any) { switch x := v.(type) { case int: _ = x; default: } }") + assert.Equal(t, "v", typeSwitchOperandName(tsFindSwitch(f1))) + + // bare expression form: switch v.(type) + f2, _ := typeCheckSrc(t, "package p\nfunc h(v any) { switch v.(type) { case int: } }") + assert.Equal(t, "v", typeSwitchOperandName(tsFindSwitch(f2))) + + // non-ident operand (selector) → no operand + f3, _ := typeCheckSrc(t, "package p\ntype S struct{ E any }\nfunc h(s S) { switch s.E.(type) { case int: } }") + assert.Equal(t, "", typeSwitchOperandName(tsFindSwitch(f3))) +} + +func TestTypeSwitchOperandsAndDefaults(t *testing.T) { + f, _ := typeCheckSrc(t, "package p\nfunc h(v any) {\n\tswitch x := v.(type) {\n\tcase *int:\n\t\t_ = x\n\tdefault:\n\t\t_ = x\n\t}\n}") + body := tsFuncBody(f, "h") + + ops := typeSwitchOperands(body) + assert.NotEmpty(t, ops) + for _, op := range ops { + assert.Equal(t, "v", op) + } + + defs := typeSwitchDefaultArms(body) + require.Len(t, defs, 1) + assert.Equal(t, "v", defs[0].operand) + assert.Less(t, int(defs[0].lo), int(defs[0].hi)) + + // A type switch with no default clause yields no default arm. + f2, _ := typeCheckSrc(t, "package p\nfunc h(v any) { switch v.(type) { case int: } }") + assert.Empty(t, typeSwitchDefaultArms(tsFuncBody(f2, "h"))) + + // A switch on a non-ident operand (selector) records no operands/defaults. + f3, _ := typeCheckSrc(t, "package p\ntype S struct{ E any }\nfunc h(s S) { switch s.E.(type) { case int: default: } }") + assert.Empty(t, typeSwitchOperands(tsFuncBody(f3, "h"))) + assert.Empty(t, typeSwitchDefaultArms(tsFuncBody(f3, "h"))) +} + +func TestAnnotateDefaultArm(t *testing.T) { + meta := &Metadata{StringPool: NewStringPool()} + defaults := []tsDefaultArm{{lo: 10, hi: 20, operand: "v"}} + + // In-range: the edge and the assignment both get the default-arm Branch. + edge := &CallGraphEdge{Position: meta.StringPool.Get("p.go:5:3")} + assign := &Assignment{} + annotateDefaultArm(token.Pos(15), "p.go:5:3", defaults, + map[string]*CallGraphEdge{"p.go:5:3": edge}, map[string][]*Assignment{"p.go:5:3": {assign}}) + require.NotNil(t, edge.Branch) + assert.Equal(t, "switch-case", edge.Branch.BlockKind) + assert.Equal(t, "v", edge.Branch.SwitchOperand) + assert.Empty(t, edge.Branch.CaseTypeRefs) + require.NotNil(t, assign.Branch) + + // Out of range: untouched. + edge2 := &CallGraphEdge{} + annotateDefaultArm(token.Pos(25), "q.go:1:1", defaults, map[string]*CallGraphEdge{"q.go:1:1": edge2}, nil) + assert.Nil(t, edge2.Branch) + + // Already annotated: not overridden. + edge3 := &CallGraphEdge{Branch: &BranchContext{BlockKind: "if-then"}} + annotateDefaultArm(token.Pos(15), "p.go:5:3", defaults, map[string]*CallGraphEdge{"p.go:5:3": edge3}, nil) + assert.Equal(t, "if-then", edge3.Branch.BlockKind) + + // Nested defaults: a write inside the TIGHTER (inner) span binds the inner + // operand, not the lexically-enclosing outer one. + nested := []tsDefaultArm{ + {lo: 10, hi: 40, operand: "outer"}, + {lo: 20, hi: 30, operand: "inner"}, + } + edge4 := &CallGraphEdge{} + annotateDefaultArm(token.Pos(25), "n.go:1:1", nested, map[string]*CallGraphEdge{"n.go:1:1": edge4}, nil) + require.NotNil(t, edge4.Branch) + assert.Equal(t, "inner", edge4.Branch.SwitchOperand) +} + +// --- spec 009 US2: if r.Method == … method-guard capture -------------------- + +func tsFindIf(file *ast.File) *ast.IfStmt { + var ifs *ast.IfStmt + ast.Inspect(file, func(n ast.Node) bool { + if s, ok := n.(*ast.IfStmt); ok && ifs == nil { + ifs = s + } + return true + }) + return ifs +} + +func TestExtractMethodGuard(t *testing.T) { + cases := []struct { + name string + cond string + want []string + }{ + // Genuine net/http.Request.Method dispatch. + {"selector", "r.Method == http.MethodPost", []string{"POST"}}, + {"string literal", `r.Method == "GET"`, []string{"GET"}}, + {"reversed", "http.MethodPut == r.Method", []string{"PUT"}}, + {"lowercase literal", `r.Method == "delete"`, []string{"DELETE"}}, + {"value receiver", `rv.Method == "PATCH"`, []string{"PATCH"}}, // http.Request by value + {"alias receiver", "ra.Method == http.MethodGet", []string{"GET"}}, // type Req = http.Request → *Req + {"alias-to-pointer receiver", "rp.Method == http.MethodHead", []string{"HEAD"}}, // type ReqP = *http.Request → ReqP + {"not equality", "r.Method != http.MethodPost", nil}, + {"both sides non-method", "a == b", nil}, + // Type-gated false-positive rejections (CodeRabbit): a `.Method` field on a + // non-Request type, or a `MethodXxx` constant not from net/http. + {"non-request .Method field", `s.Method == "GET"`, nil}, + {"non-http method const", "foo.MethodPost == r.Method", nil}, + {"non-http method field rhs", "r.Method == foo.MethodPost", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + src := "package p\nimport \"net/http\"\ntype biz struct{ Method string; MethodPost string }\ntype Req = http.Request\ntype ReqP = *http.Request\n" + + "func h(r *http.Request, rv http.Request, ra *Req, rp ReqP, s biz, foo biz) {\n\tvar a, b int\n\t_ = a\n\t_ = b\n\tif " + tc.cond + " {\n\t}\n}" + f, info := typeCheckSrc(t, src) + got := extractMethodGuard(tsFindIf(f), info) + assert.Equal(t, tc.want, got) + }) + } + // Without type info, no method guard is detected (conservative). + fNo, _ := typeCheckSrc(t, "package p\nimport \"net/http\"\nfunc h(r *http.Request) { if r.Method == http.MethodPost {} }") + assert.Nil(t, extractMethodGuard(tsFindIf(fNo), nil)) + // A non-if statement yields nothing. + assert.Nil(t, extractMethodGuard(&ast.ExprStmt{}, nil)) +} + +// distinctVals collapses a group map to its set of group ids. +func distinctVals(m map[token.Pos]int) map[int]bool { + s := make(map[int]bool, len(m)) + for _, v := range m { + s[v] = true + } + return s +} + +func TestMethodDispatchGroups(t *testing.T) { + src := "package p\nimport \"net/http\"\n" + + // string-literal `switch r.Method`: a genuine method dispatch (needs no type info). + "func sw(r *http.Request) {\n\tswitch r.Method {\n\tcase \"GET\":\n\tcase \"POST\":\n\tdefault:\n\t}\n}\n" + + // http.MethodXxx constant cases — resolvable only with type info. + "func swConst(r *http.Request) {\n\tswitch r.Method {\n\tcase http.MethodGet:\n\tcase http.MethodPost:\n\t}\n}\n" + + // switch over a COPY of r.Method (tag is `m`, not the bare selector) — grouped by + // its method-named cases regardless of the tag. + "func swCopy(r *http.Request) {\n\tm := r.Method\n\tswitch m {\n\tcase \"GET\":\n\tcase \"POST\":\n\tdefault:\n\t}\n}\n" + + // value switch whose cases name methods — grouped exactly as the splitter fans it out. + "func vsw(action string) {\n\tswitch action {\n\tcase \"GET\":\n\tcase \"POST\":\n\t}\n}\n" + + // switch whose cases name NO method — not a method dispatch. + "func nonmethod(x string) {\n\tswitch x {\n\tcase \"foo\":\n\tcase \"bar\":\n\t}\n}\n" + + // if r.Method == … else-if chain. + "func chain(r *http.Request) {\n\tif r.Method == \"GET\" {\n\t} else if r.Method == \"POST\" {\n\t} else {\n\t}\n}\n" + + // tagless switch (boolean cases) is not a method dispatch. + "func tagless(r *http.Request) {\n\tswitch {\n\tcase r.Method == \"GET\":\n\t}\n}\n" + + // plain non-method if. + "func plain(x int) {\n\tif x > 0 {\n\t}\n}\n" + f, info := typeCheckSrc(t, src) + + // string-literal `switch r.Method`: every case clause INCLUDING the default shares one id. + sw := methodDispatchGroups(tsFuncBody(f, "sw"), info) + require.Len(t, sw, 3) // GET, POST, default + assert.Len(t, distinctVals(sw), 1, "all arms of one switch share a group") + // string-literal cases need no type info — the dispatch is still recognised (this is the + // no-type-info default-leak the case-value detection guards against). + assert.Len(t, methodDispatchGroups(tsFuncBody(f, "sw"), nil), 3) + + // switch over a COPY of r.Method: grouped by its method-named cases — the regression + // guard for `m := r.Method; switch m { …; default }` (a non-bare-selector tag). + require.Len(t, methodDispatchGroups(tsFuncBody(f, "swCopy"), info), 3) + + // value switch naming methods: grouped consistently with how the splitter fans it out. + vsw := methodDispatchGroups(tsFuncBody(f, "vsw"), info) + require.Len(t, vsw, 2) + assert.Len(t, distinctVals(vsw), 1) + + // else-if chain: both IfStmts share one id; the bare else (not an IfStmt) is not + // recorded, and the done-short-circuit fires when Inspect re-visits the else-if. + ch := methodDispatchGroups(tsFuncBody(f, "chain"), info) + require.Len(t, ch, 2) + assert.Len(t, distinctVals(ch), 1) + + // a method-less switch, a tagless boolean switch, and a plain non-method if record nothing. + assert.Empty(t, methodDispatchGroups(tsFuncBody(f, "nonmethod"), info)) + assert.Empty(t, methodDispatchGroups(tsFuncBody(f, "tagless"), info)) + assert.Empty(t, methodDispatchGroups(tsFuncBody(f, "plain"), info)) + + // http.MethodXxx constant cases need type info: with it the switch is recognised, + // without it the constants don't resolve so the switch is conservatively skipped. + assert.Len(t, methodDispatchGroups(tsFuncBody(f, "swConst"), info), 2) + assert.Empty(t, methodDispatchGroups(tsFuncBody(f, "swConst"), nil)) +} diff --git a/internal/metadata/dominators.go b/internal/metadata/dominators.go new file mode 100644 index 0000000..ded6cb6 --- /dev/null +++ b/internal/metadata/dominators.go @@ -0,0 +1,151 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 metadata + +// computeDominators returns the immediate-dominator array for a CFG given its +// successor adjacency (succs[i] = successor block indices of block i), with the +// entry at index 0. idom[0] (the entry) is -1 (it has no dominator); for every +// other block reachable from the entry, idom[b] is its immediate dominator. +// Blocks unreachable from the entry get idom = -1. +// +// Implements the Cooper-Harvey-Kennedy "A Simple, Fast Dominance Algorithm" +// iterative data-flow formulation. It terminates on cyclic graphs (loops / +// back-edges) — FR-010, spec 009 — because the data-flow lattice is finite and +// monotone. O(blocks·edges) on the tiny per-function CFGs this analyzer builds. +func computeDominators(succs [][]int32) []int32 { //nolint:gocyclo // cohesive Cooper-Harvey-Kennedy dominator fixpoint + n := len(succs) + idom := make([]int32, n) + for i := range idom { + idom[i] = -1 + } + if n == 0 { + return idom + } + + // Predecessors. + preds := make([][]int32, n) + for b := range succs { + for _, s := range succs[b] { + if int(s) >= 0 && int(s) < n { + preds[s] = append(preds[s], int32(b)) + } + } + } + + // Iterative DFS postorder from the entry. post[b] = postorder index (entry + // gets the highest); -1 for blocks unreachable from the entry. + post := make([]int32, n) + for i := range post { + post[i] = -1 + } + order := make([]int32, 0, n) + visited := make([]bool, n) + type frame struct { + b int32 + next int + } + stack := []frame{{b: 0}} + visited[0] = true + for len(stack) > 0 { + top := &stack[len(stack)-1] + if top.next < len(succs[top.b]) { + s := succs[top.b][top.next] + top.next++ + if int(s) >= 0 && int(s) < n && !visited[s] { + visited[s] = true + stack = append(stack, frame{b: s}) + } + } else { + post[top.b] = int32(len(order)) //nolint:gosec // CFG block count is small (tens); no int32 overflow + order = append(order, top.b) + stack = stack[:len(stack)-1] + } + } + + // Reverse postorder (excluding entry) is the worklist order. + rpo := make([]int32, 0, len(order)) + for i := len(order) - 1; i >= 0; i-- { + if order[i] != 0 { + rpo = append(rpo, order[i]) + } + } + + doms := make([]int32, n) + for i := range doms { + doms[i] = -1 + } + doms[0] = 0 // entry dominates itself during the fixpoint; reset to -1 at the end + + intersect := func(b1, b2 int32) int32 { + for b1 != b2 { + for post[b1] < post[b2] { + b1 = doms[b1] + } + for post[b2] < post[b1] { + b2 = doms[b2] + } + } + return b1 + } + + for changed := true; changed; { + changed = false + for _, b := range rpo { + var newIdom int32 = -1 + for _, p := range preds[b] { + if doms[p] == -1 { + continue // predecessor not yet processed (or unreachable) + } + if newIdom == -1 { + newIdom = p + } else { + newIdom = intersect(p, newIdom) + } + } + if newIdom != -1 && doms[b] != newIdom { + doms[b] = newIdom + changed = true + } + } + } + + copy(idom, doms) + idom[0] = -1 // entry has no dominator + return idom +} + +// blockDominates reports whether block a dominates block b, given the immediate- +// dominator array from computeDominators (a is on b's idom chain, inclusive of b +// and the entry). Returns false for out-of-range or unreachable blocks. +func blockDominates(idom []int32, a, b int32) bool { + // Out-of-range indices dominate nothing — reject them before the reflexive + // a == b check, so a stale/invalid BlockLoc can't be reported as dominating. + if int(a) < 0 || int(a) >= len(idom) || int(b) < 0 || int(b) >= len(idom) { + return false + } + for x := b; ; { + if x == a { + return true + } + if int(x) < 0 || int(x) >= len(idom) { + return false + } + p := idom[x] + if p < 0 || p == x { + return false // reached the entry without finding a + } + x = p + } +} diff --git a/internal/metadata/expression_coverage_test.go b/internal/metadata/expression_coverage_test.go index 417a05f..274a5ca 100644 --- a/internal/metadata/expression_coverage_test.go +++ b/internal/metadata/expression_coverage_test.go @@ -1063,11 +1063,11 @@ func TestMapBlockKind_AllKnownKinds(t *testing.T) { func TestExtractCaseValues_NonCaseClause(t *testing.T) { // Not a CaseClause stmt := &ast.ExprStmt{X: &ast.Ident{Name: "x"}} - assert.Nil(t, extractCaseValues(stmt)) + assert.Nil(t, extractCaseValues(stmt, nil)) } func TestExtractCaseValues_NilStmt(t *testing.T) { - assert.Nil(t, extractCaseValues(nil)) + assert.Nil(t, extractCaseValues(nil, nil)) } func TestExtractCaseValues_IntLiterals(t *testing.T) { @@ -1077,7 +1077,7 @@ func TestExtractCaseValues_IntLiterals(t *testing.T) { &ast.BasicLit{Kind: token.INT, Value: "99"}, }, } - values := extractCaseValues(cc) + values := extractCaseValues(cc, nil) assert.Equal(t, []string{"42", "99"}, values) } @@ -1088,26 +1088,28 @@ func TestExtractCaseValues_StringLiterals(t *testing.T) { &ast.BasicLit{Kind: token.STRING, Value: `"POST"`}, }, } - values := extractCaseValues(cc) + values := extractCaseValues(cc, nil) assert.Equal(t, []string{"GET", "POST"}, values) } func TestExtractCaseValues_MixedExprs(t *testing.T) { - // Non-BasicLit expressions should be skipped + // A bare Ident is skipped; a SelectorExpr without type info resolves to nothing + // (httpMethodName needs *types.Info to confirm an http.MethodXxx constant). cc := &ast.CaseClause{ List: []ast.Expr{ &ast.BasicLit{Kind: token.STRING, Value: `"A"`}, - &ast.Ident{Name: "SomeConst"}, // not a BasicLit + &ast.Ident{Name: "SomeConst"}, + &ast.SelectorExpr{X: &ast.Ident{Name: "http"}, Sel: &ast.Ident{Name: "MethodGet"}}, }, } - values := extractCaseValues(cc) + values := extractCaseValues(cc, nil) assert.Equal(t, []string{"A"}, values) } func TestExtractCaseValues_DefaultCase(t *testing.T) { // Default case has nil List cc := &ast.CaseClause{List: nil} - values := extractCaseValues(cc) + values := extractCaseValues(cc, nil) assert.Nil(t, values) } @@ -1121,7 +1123,7 @@ func TestBuildFunctionCFGs_NilBody(_ *testing.T) { Body: nil, } // Should not panic - BuildFunctionCFGs([]*ast.FuncDecl{decl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{decl}, nil, fset, meta) } func TestBuildFunctionCFGs_WithMatchingEdges(t *testing.T) { @@ -1158,7 +1160,7 @@ func handler(method string) { }, } - BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, nil, fset, meta) // Just verify it doesn't panic and runs correctly } @@ -1187,7 +1189,7 @@ func dispatch(method string) { require.NotNil(t, funcDecl) meta := newTestMeta() - BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, nil, fset, meta) } // ------------------------------------------------------------ @@ -1546,7 +1548,7 @@ func f(x int) int { }, } - BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, fset, meta) + BuildFunctionCFGs([]*ast.FuncDecl{funcDecl}, nil, fset, meta) } // Test ExprToCallArgument dispatches IndexListExpr via the switch diff --git a/internal/metadata/metadata.go b/internal/metadata/metadata.go index 24deee1..6683ee0 100644 --- a/internal/metadata/metadata.go +++ b/internal/metadata/metadata.go @@ -429,12 +429,25 @@ func GenerateMetadataWithLogger(pkgs map[string]map[string]*ast.File, fileToInfo // Finalize string pool metadata.StringPool.Finalize() - // Build CFG for all functions and annotate edges/assignments with branch context + // Build CFG for all functions and annotate edges/assignments with branch context, + // plus the retained reachability model (spec 009). declInfo gives each function's + // *types.Info (for type-switch case-type capture), resolved via its file. allFuncDecls := make([]*ast.FuncDecl, 0, len(funcMap)) for _, fn := range funcMap { allFuncDecls = append(allFuncDecls, fn) } - BuildFunctionCFGs(allFuncDecls, fset, metadata) + declInfo := make(map[*ast.FuncDecl]*types.Info) + for _, files := range pkgs { + for _, file := range files { + info := fileToInfo[file] + for _, d := range file.Decls { + if fn, ok := d.(*ast.FuncDecl); ok { + declInfo[fn] = info + } + } + } + } + BuildFunctionCFGs(allFuncDecls, declInfo, fset, metadata) if logger != nil { logger.Println("process assignment Count:", processAssignmentCount) diff --git a/internal/metadata/reachability.go b/internal/metadata/reachability.go new file mode 100644 index 0000000..d0dbe33 --- /dev/null +++ b/internal/metadata/reachability.go @@ -0,0 +1,174 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 metadata + +// The reachability query layer over the compact per-function control-flow model +// (spec 009, FR-001). Consumers (the conditional-analysis migration) resolve a +// metadata position to a block with BlockFor, then ask Reaches / Dominates. + +// InstallFunctionCFGForTest installs a per-function CFG (from a successor adjacency +// and a position→block map, computing dominators) and registers its positions in the +// cfgPosToFn index, so cross-package tests of the reachability consumers can model a +// control-flow scenario without parsing real source. Test-only. +func (m *Metadata) InstallFunctionCFGForTest(fnKey string, succs [][]int32, posBlocks map[string]BlockLoc) { + if m.FunctionCFGs == nil { + m.FunctionCFGs = make(map[string]*FunctionCFG) + } + if m.cfgPosToFn == nil { + m.cfgPosToFn = make(map[string]string) + } + blocks := make([]BlockInfo, len(succs)) + for i := range blocks { + blocks[i] = BlockInfo{Index: int32(i)} //nolint:gosec // test block counts are small + } + m.FunctionCFGs[fnKey] = &FunctionCFG{ + Blocks: blocks, + Succs: succs, + Dominators: computeDominators(succs), + PosToBlock: posBlocks, + } + for pos := range posBlocks { + m.cfgPosToFn[pos] = fnKey + } +} + +// FnKeyForPos returns the FunctionCFGs key of the function containing the given +// source position (raw or repo-root-stripped form), or "" if unknown. A consumer +// holding only a position (a CallGraphEdge.Position) uses this to find the function +// whose model to query. +func (m *Metadata) FnKeyForPos(pos string) string { + if m == nil || m.cfgPosToFn == nil { + return "" + } + return m.cfgPosToFn[pos] +} + +// fnCFG returns the compact control-flow model for the given function key, or nil. +func (m *Metadata) fnCFG(fnKey string) *FunctionCFG { + if m == nil || m.FunctionCFGs == nil { + return nil + } + return m.FunctionCFGs[fnKey] +} + +// BlockFor resolves a metadata position string (a CallGraphEdge.Position or +// Assignment.Position) to its location within fnKey's CFG. ok=false when the +// function has no model or the position is in no live block — the caller then +// degrades to the single-path result (FR-008), never panicking. +func (m *Metadata) BlockFor(fnKey, pos string) (BlockLoc, bool) { + fc := m.fnCFG(fnKey) + if fc == nil || pos == "" || fc.PosToBlock == nil { + return BlockLoc{}, false + } + loc, ok := fc.PosToBlock[pos] + return loc, ok +} + +// Reaches reports whether `to` is reachable from `from` along some control-flow +// path within fnKey. Within one block, reachability follows node order +// (from.Node <= to.Node). Across blocks it is the transitive closure of the +// successor graph, which terminates on loops/back-edges via a visited set. +// +// Intra-block reachability deliberately ignores a block's own back-edge: in a +// single-block loop (go/cfg emits a self-successor block) a node later in the +// block is NOT reported as reaching an earlier one. This conservatively +// UNDER-approximates (a loop-carried value can be missed), which is the safe +// direction for the status consumer — admitting the back-edge here would make the +// dominator-based kill predicate mutually overwrite both ends of the loop body and +// drop BOTH statuses. A reaching-definitions pass would resolve it precisely. +func (m *Metadata) Reaches(fnKey string, from, to BlockLoc) bool { + fc := m.fnCFG(fnKey) + if fc == nil { + return false + } + n := int32(len(fc.Succs)) //nolint:gosec // CFG block count is small (tens); no int32 overflow + if from.Block < 0 || from.Block >= n || to.Block < 0 || to.Block >= n { + return false // a non-existent block reaches nothing + } + if from.Block == to.Block { + return from.Node <= to.Node + } + return fc.blockReaches(from.Block, to.Block) +} + +// Dominates reports whether block a dominates block b within fnKey (a is on every +// control-flow path from the entry to b). +func (m *Metadata) Dominates(fnKey string, a, b int32) bool { + fc := m.fnCFG(fnKey) + if fc == nil { + return false + } + return blockDominates(fc.Dominators, a, b) +} + +// DispatchArms returns the block indices of every arm of the method-dispatch group +// `group` within fnKey (every method-`switch` case INCLUDING the default, or every +// `if r.Method ==` chain arm), or nil. The common dominator of the returned blocks is +// that dispatch's tag, so a consumer can scope the dispatch region exactly — per +// dispatch, including arms whose response was lost, with no combined-case collapse. +// +// The returned slice aliases the stored model — treat it as READ-ONLY; do not sort or +// append in place (the sole consumer, contributingDispatchArms, only copies elements out). +func (m *Metadata) DispatchArms(fnKey string, group int) []int32 { + fc := m.fnCFG(fnKey) + if fc == nil { + return nil + } + return fc.DispatchArms[group] +} + +// IDom returns the immediate dominator of block b within fnKey (the branch point +// that routes to b) and ok=true. Returns (-1, false) when the function has no +// model, b is out of range, or b is the entry / unreachable (idom = -1). A +// consumer uses it to group a `switch r.Method` / `if r.Method ==` dispatch's arms +// by their shared branch point, so a fallback arm (switch default / bare else) can +// be told apart from an independent conditional with a different branch point. +func (m *Metadata) IDom(fnKey string, b int32) (int32, bool) { + fc := m.fnCFG(fnKey) + if fc == nil || int(b) < 0 || int(b) >= len(fc.Dominators) { + return -1, false + } + d := fc.Dominators[b] + if d < 0 { + return -1, false + } + return d, true +} + +// blockReaches does a breadth-first search over the successor graph with a visited +// set, so cyclic graphs (loops) terminate. +func (fc *FunctionCFG) blockReaches(from, to int32) bool { + n := int32(len(fc.Succs)) //nolint:gosec // CFG block count is small (tens); no int32 overflow + if from < 0 || from >= n { + return false + } + visited := make([]bool, n) + visited[from] = true + queue := []int32{from} + for len(queue) > 0 { + b := queue[0] + queue = queue[1:] + for _, s := range fc.Succs[b] { + if s == to { + return true + } + if s >= 0 && s < n && !visited[s] { + visited[s] = true + queue = append(queue, s) + } + } + } + return false +} diff --git a/internal/metadata/reachability_test.go b/internal/metadata/reachability_test.go new file mode 100644 index 0000000..eb8b813 --- /dev/null +++ b/internal/metadata/reachability_test.go @@ -0,0 +1,166 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 metadata + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// newFnCFG builds a FunctionCFG from a successor adjacency, computing dominators +// (so computeDominators is exercised). Block 0 is the entry. +func newFnCFG(succs [][]int32) *FunctionCFG { + blocks := make([]BlockInfo, len(succs)) + for i := range succs { + blocks[i] = BlockInfo{Index: int32(i)} + } + return &FunctionCFG{ + Blocks: blocks, + Succs: succs, + Dominators: computeDominators(succs), + PosToBlock: map[string]BlockLoc{}, + } +} + +func metaWith(fc *FunctionCFG) *Metadata { + return &Metadata{FunctionCFGs: map[string]*FunctionCFG{"f": fc}} +} + +func TestReachability_StraightLine(t *testing.T) { + m := metaWith(newFnCFG([][]int32{{1}, {2}, {}})) + assert.True(t, m.Reaches("f", BlockLoc{Block: 0}, BlockLoc{Block: 2})) + assert.False(t, m.Reaches("f", BlockLoc{Block: 2}, BlockLoc{Block: 0})) + assert.True(t, m.Dominates("f", 0, 2)) + assert.True(t, m.Dominates("f", 1, 2)) +} + +func TestReachability_IfElseSiblings(t *testing.T) { + // 0 -> {1,2}; 1 -> 3; 2 -> 3 (merge) + m := metaWith(newFnCFG([][]int32{{1, 2}, {3}, {3}, {}})) + // both arms reach the merge → fan-out + assert.True(t, m.Reaches("f", BlockLoc{Block: 1}, BlockLoc{Block: 3})) + assert.True(t, m.Reaches("f", BlockLoc{Block: 2}, BlockLoc{Block: 3})) + // siblings do not reach each other (no shadow) + assert.False(t, m.Reaches("f", BlockLoc{Block: 1}, BlockLoc{Block: 2})) + assert.False(t, m.Reaches("f", BlockLoc{Block: 2}, BlockLoc{Block: 1})) + // neither arm dominates the merge; the entry does + assert.False(t, m.Dominates("f", 1, 3)) + assert.False(t, m.Dominates("f", 2, 3)) + assert.True(t, m.Dominates("f", 0, 3)) +} + +func TestReachability_UnconditionalKill(t *testing.T) { + // 0 -> 1 -> 2 ; an unconditional store in block 1 dominates the use in block 2. + m := metaWith(newFnCFG([][]int32{{1}, {2}, {}})) + assert.True(t, m.Dominates("f", 1, 2)) // later unconditional store kills earlier +} + +func TestReachability_Loop(t *testing.T) { + // 0 -> 1 ; 1 -> {2,3} ; 2 -> 1 (back-edge) ; 3 exit — FR-010: terminates on the cycle. + m := metaWith(newFnCFG([][]int32{{1}, {2, 3}, {1}, {}})) + assert.True(t, m.Reaches("f", BlockLoc{Block: 1}, BlockLoc{Block: 3})) + assert.True(t, m.Reaches("f", BlockLoc{Block: 2}, BlockLoc{Block: 1})) // back-edge + assert.True(t, m.Dominates("f", 1, 3)) // loop header dominates exit +} + +func TestReachability_EarlyReturn(t *testing.T) { + // 0 -> {1,2} ; 1 returns (no succ) ; 2 -> 3 + m := metaWith(newFnCFG([][]int32{{1, 2}, {}, {3}, {}})) + assert.False(t, m.Reaches("f", BlockLoc{Block: 1}, BlockLoc{Block: 3})) // returns before 3 + assert.True(t, m.Reaches("f", BlockLoc{Block: 2}, BlockLoc{Block: 3})) +} + +func TestReachability_IntraBlockNodeOrder(t *testing.T) { + m := metaWith(newFnCFG([][]int32{{}})) + assert.True(t, m.Reaches("f", BlockLoc{Block: 0, Node: 1}, BlockLoc{Block: 0, Node: 3})) + assert.False(t, m.Reaches("f", BlockLoc{Block: 0, Node: 3}, BlockLoc{Block: 0, Node: 1})) +} + +func TestReachability_BlockForAndDegrade(t *testing.T) { + fc := newFnCFG([][]int32{{}}) + fc.PosToBlock["f.go:10:2"] = BlockLoc{Block: 0, Node: 1} + m := metaWith(fc) + + loc, ok := m.BlockFor("f", "f.go:10:2") + assert.True(t, ok) + assert.Equal(t, int32(0), loc.Block) + + _, ok = m.BlockFor("f", "missing") + assert.False(t, ok) + + // Absent/nil model degrades to false, never panics (FR-008). + _, ok = m.BlockFor("absent", "f.go:10:2") + assert.False(t, ok) + assert.False(t, m.Reaches("absent", BlockLoc{}, BlockLoc{})) + assert.False(t, m.Dominates("absent", 0, 1)) + var nilM *Metadata + assert.Nil(t, nilM.fnCFG("x")) + _, ok = nilM.BlockFor("x", "y") + assert.False(t, ok) +} + +func TestComputeDominators_Diamond(t *testing.T) { + doms := computeDominators([][]int32{{1, 2}, {3}, {3}, {}}) + assert.Equal(t, int32(-1), doms[0]) // entry has no dominator + assert.Equal(t, int32(0), doms[1]) + assert.Equal(t, int32(0), doms[2]) + assert.Equal(t, int32(0), doms[3]) // merge dominated by entry, not by either arm + assert.True(t, blockDominates(doms, 0, 3)) + assert.True(t, blockDominates(doms, 3, 3)) // reflexive + assert.False(t, blockDominates(doms, 1, 3)) + assert.False(t, blockDominates(doms, 1, 2)) +} + +func TestComputeDominators_Empty(t *testing.T) { + assert.Empty(t, computeDominators(nil)) +} + +func TestReachability_OutOfRange(t *testing.T) { + m := metaWith(newFnCFG([][]int32{{1}, {}})) + // A from-block out of range degrades to false, never panics. + assert.False(t, m.Reaches("f", BlockLoc{Block: 9}, BlockLoc{Block: 0})) + // Dominance with an out-of-range target block degrades to false. + fc := m.fnCFG("f") + assert.False(t, blockDominates(fc.Dominators, 0, 9)) +} + +func TestDispatchArms(t *testing.T) { + m := metaWith(newFnCFG([][]int32{{1, 2}, {}, {}})) + m.FunctionCFGs["f"].DispatchArms = map[int][]int32{7: {1, 2}} + assert.Equal(t, []int32{1, 2}, m.DispatchArms("f", 7)) + assert.Nil(t, m.DispatchArms("f", 99), "unknown group → nil") + assert.Nil(t, m.DispatchArms("absent", 7), "no model → nil") +} + +func TestIDom(t *testing.T) { + // Diamond: 0 → {1,2} → 3; every non-entry block's immediate dominator is 0. + m := metaWith(newFnCFG([][]int32{{1, 2}, {3}, {3}, {}})) + + d, ok := m.IDom("f", 1) + assert.True(t, ok) + assert.Equal(t, int32(0), d) + + _, ok = m.IDom("f", 0) + assert.False(t, ok, "the entry has no immediate dominator") + + _, ok = m.IDom("f", 99) + assert.False(t, ok, "out of range") + _, ok = m.IDom("f", -1) + assert.False(t, ok, "negative index") + + _, ok = m.IDom("absent", 1) + assert.False(t, ok, "no model → not ok") +} diff --git a/internal/metadata/types.go b/internal/metadata/types.go index 9af1a27..77814a6 100644 --- a/internal/metadata/types.go +++ b/internal/metadata/types.go @@ -135,6 +135,19 @@ type Metadata struct { Packages map[string]*Package `yaml:"packages,omitempty"` CallGraph []CallGraphEdge `yaml:"call_graph,omitempty"` + // FunctionCFGs holds the per-function compact control-flow model (reachability + + // dominance), keyed by function identity (the function body's position string). + // Built by BuildFunctionCFGs (spec 009, FR-001). Transient: it is an in-memory + // analysis structure consumed during extraction and rebuilt on each run, so it is + // NOT serialized — persisting it (without its companion cfgPosToFn index, which is + // also transient) would bloat the metadata dump with a model unusable after load. + FunctionCFGs map[string]*FunctionCFG `yaml:"-"` + // cfgPosToFn maps a source position (both raw and repo-root-stripped forms, like + // the edge/assignment position indexes — #27) to the FunctionCFGs key of the + // function that position lives in, so a consumer holding only a position can find + // its function's model. Transient (rebuilt with FunctionCFGs). + cfgPosToFn map[string]string `yaml:"-"` + Callers map[string][]*CallGraphEdge `yaml:"-"` ParentFunctions map[string][]*CallGraphEdge `yaml:"-"` Callees map[string][]*CallGraphEdge `yaml:"-"` @@ -519,6 +532,55 @@ type BranchContext struct { BlockKind string `yaml:"block_kind,omitempty"` // "if-then", "if-else", "switch-case", "" ParentStmtPos int `yaml:"parent_stmt_pos,omitempty"` CaseValues []string `yaml:"case_values,omitempty"` // For switch-case: literal values from case clause (e.g., "GET", "POST") + // CaseTypeRefs holds the case TYPES for a type-switch arm (switch x.(type) { case *T: ... }). + // Disjoint from CaseValues (which carries string/method literals); empty for value-switches + // and for case nil:/default: (the unconditional arm). Populated by extractCaseTypeRefs (spec 009). + CaseTypeRefs []*TypeRef `yaml:"case_type_refs,omitempty"` + // SwitchOperand is the variable being type-switched (`switch x := v.(type)` → "v") + // for a type-switch arm. It lets a consumer bind the switched parameter to the + // call-site argument via CallGraphEdge.ParamArgMap (spec 009, FR-011). Empty when + // the operand is not a simple ident, or for non-type-switch blocks. + SwitchOperand string `yaml:"switch_operand,omitempty"` + // DispatchGroup identifies the method dispatch (one `switch r.Method`, or one + // `if r.Method ==` chain) this arm belongs to — every case of a switch (INCLUDING its + // `default`), or every `if`/`else if` of a chain, shares it; 0 for a branch that is + // not a method-dispatch arm. (A chain's bare `else` is not recorded as an arm — it is + // recognised as a fallback structurally at split time.) It lets a consumer recover the + // EXACT dispatch without reconstructing it from dominance heuristics. The id is the + // dispatch statement's source position. + DispatchGroup int `yaml:"dispatch_group,omitempty"` +} + +// FunctionCFG is the compact, retained per-function control-flow model built by +// BuildFunctionCFGs (spec 009, FR-001). The raw *cfg.CFG is dropped after build; +// this int-only form answers reachability/dominance queries and is serializable. +type FunctionCFG struct { + Blocks []BlockInfo `yaml:"blocks,omitempty"` + Succs [][]int32 `yaml:"succs,omitempty"` // Succs[i] = successor block indices of block i + Dominators []int32 `yaml:"dominators,omitempty"` // immediate dominator idom[i]; entry idom = -1 + PosToBlock map[string]BlockLoc `yaml:"pos_to_block,omitempty"` + // DispatchArms maps each method-dispatch group id (see BranchContext.DispatchGroup) + // to the block indices of ALL its arms — every `switch r.Method` case INCLUDING the + // `default`, or every `if`/`else if` of an `if r.Method ==` chain (a chain's bare + // `else` is not an arm — it is a fallback recognised structurally). The common + // dominator of one group's arms is that dispatch's tag, so a consumer can scope the + // dispatch region exactly (per dispatch, incl. arms whose response was lost) with no + // cross-dispatch inflation and no combined-case collapse (US2). + DispatchArms map[int][]int32 `yaml:"dispatch_arms,omitempty"` +} + +// BlockInfo is one CFG basic block in the compact model. +type BlockInfo struct { + Index int32 `yaml:"index"` + Kind string `yaml:"kind,omitempty"` + NodeCount int32 `yaml:"node_count,omitempty"` +} + +// BlockLoc locates a statement/call within the CFG: its block, and its node index +// within that block (node order disambiguates defs/uses inside one block). +type BlockLoc struct { + Block int32 `yaml:"block"` + Node int32 `yaml:"node"` } type Assignment struct { diff --git a/internal/spec/branch_bodies_test.go b/internal/spec/branch_bodies_test.go new file mode 100644 index 0000000..b527041 --- /dev/null +++ b/internal/spec/branch_bodies_test.go @@ -0,0 +1,57 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 spec + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestAddResponse_BranchBodiesAttributedPerStatus is the unit-level guard for US3 +// (FR-005): bodies written on different branches land under their own status code, +// never merged into one shape. +func TestAddResponse_BranchBodiesAttributedPerStatus(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + route := &RouteInfo{Response: map[string]*ResponseInfo{}} + + ext.addResponse(route, &ResponseInfo{StatusCode: 200, Schema: &Schema{Ref: "#/components/schemas/FullUser"}}) + ext.addResponse(route, &ResponseInfo{StatusCode: 404, Schema: &Schema{Ref: "#/components/schemas/ErrorBody"}}) + + require.Len(t, route.Response, 2) + require.NotNil(t, route.Response["200"].Schema) + require.NotNil(t, route.Response["404"].Schema) + assert.Equal(t, "#/components/schemas/FullUser", route.Response["200"].Schema.Ref) + assert.Equal(t, "#/components/schemas/ErrorBody", route.Response["404"].Schema.Ref) +} + +// TestAddResponse_SameStatusDistinctBodies records two distinct bodies under the +// same status as alternatives rather than dropping one. +func TestAddResponse_SameStatusDistinctBodies(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + route := &RouteInfo{Response: map[string]*ResponseInfo{}} + + ext.addResponse(route, &ResponseInfo{StatusCode: 200, Schema: &Schema{Ref: "A"}}) + ext.addResponse(route, &ResponseInfo{StatusCode: 200, Schema: &Schema{Ref: "B"}}) + ext.addResponse(route, &ResponseInfo{StatusCode: 200, Schema: &Schema{Ref: "A"}}) // duplicate ignored + + require.Len(t, route.Response, 1) + assert.Equal(t, "A", route.Response["200"].Schema.Ref) + require.Len(t, route.Response["200"].AlternativeSchemas, 1) + assert.Equal(t, "B", route.Response["200"].AlternativeSchemas[0].Ref) +} diff --git a/internal/spec/conditional_status_test.go b/internal/spec/conditional_status_test.go index f36d635..df3a212 100644 --- a/internal/spec/conditional_status_test.go +++ b/internal/spec/conditional_status_test.go @@ -15,6 +15,8 @@ package spec import ( + "bytes" + "fmt" "sort" "testing" @@ -25,8 +27,7 @@ import ( ) // csNewErrorCall builds the RHS of `err = NewError(msg, http.)`: -// a KindCall whose args are an opaque message ident and an http status -// selector. +// a KindCall whose args are an opaque message ident and an http status selector. func csNewErrorCall(meta *metadata.Metadata, statusName string) metadata.CallArgument { call := metadata.NewCallArgument(meta) call.SetKind(metadata.KindCall) @@ -41,24 +42,44 @@ func csNewErrorCall(meta *metadata.Metadata, statusName string) metadata.CallArg return *call } -// csMatcher builds a response matcher whose status arg is an opaque error -// (RespondWithError(w, err) -> StatusArgIndex 1) and registers a handler whose -// `err` variable is reassigned across branches. -func csMatcher(t *testing.T, branchStatuses []string) (*ResponsePatternMatcherImpl, *TrackerNode) { +// csSpec describes one `err = NewError(msg, http.)` assignment placed in a +// CFG block. +type csSpec struct { + status string + block int32 + uncond bool // Branch == nil (else a conditional if/else arm) +} + +// csScenario builds a handler whose `err` variable is reassigned per the specs and +// then passed to RespondWithError(w, err) at callBlock, installs a FunctionCFG from +// the successor adjacency, and returns the matcher + the call edge. If installCFG is +// false the model is NOT installed (exercising the FR-008 degrade path). +func csScenario(t *testing.T, specs []csSpec, callBlock int32, succs [][]int32, installCFG bool) (*ResponsePatternMatcherImpl, *metadata.CallGraphEdge) { t.Helper() meta := pmcTestMeta() sp := meta.StringPool - assigns := make([]metadata.Assignment, 0, len(branchStatuses)) - for _, name := range branchStatuses { - // These mirror `err = NewError(msg, http.Status…)` in distinct if/else - // branches, so each carries a (conditional) BranchContext — sibling - // branches don't shadow one another, so the fan-out keeps every status. + posBlocks := map[string]metadata.BlockLoc{} + assigns := make([]metadata.Assignment, 0, len(specs)) + for i, s := range specs { + pos := fmt.Sprintf("h.go:%d:3", 10+i) + var br *metadata.BranchContext + if !s.uncond { + br = &metadata.BranchContext{BlockKind: "if-else"} + } assigns = append(assigns, metadata.Assignment{ - Value: csNewErrorCall(meta, name), - Branch: &metadata.BranchContext{BlockKind: "if-else"}, + Value: csNewErrorCall(meta, s.status), + Branch: br, + Position: sp.Get(pos), }) + posBlocks[pos] = metadata.BlockLoc{Block: s.block} + } + callPos := "h.go:50:3" + posBlocks[callPos] = metadata.BlockLoc{Block: callBlock} + if installCFG { + meta.InstallFunctionCFGForTest("handler", succs, posBlocks) } + meta.Packages["app"] = &metadata.Package{ Files: map[string]*metadata.File{ "h.go": {Functions: map[string]*metadata.Function{ @@ -70,60 +91,145 @@ func csMatcher(t *testing.T, branchStatuses []string) (*ResponsePatternMatcherIm }}, }, } - edge := buildCallGraphEdge(meta, "handler", "app", "RespondWithError", "app", []*metadata.CallArgument{buildIdentArg(meta, "w", "app"), buildIdentArg(meta, "err", "app")}) + edge.Position = sp.Get(callPos) - cp := NewContextProvider(meta) matcher := &ResponsePatternMatcherImpl{ - BasePatternMatcher: &BasePatternMatcher{cfg: pmcTestCfg(), contextProvider: cp, schemaMapper: NewSchemaMapper(pmcTestCfg())}, + BasePatternMatcher: &BasePatternMatcher{cfg: pmcTestCfg(), contextProvider: NewContextProvider(meta), schemaMapper: NewSchemaMapper(pmcTestCfg())}, pattern: ResponsePattern{StatusFromArg: true, StatusArgIndex: 1}, } - return matcher, buildTrackerNode(edge) + return matcher, edge } -func TestExpandStatusesFromIdent(t *testing.T) { - matcher, node := csMatcher(t, []string{"StatusUnauthorized", "StatusNotFound", "StatusInternalServerError"}) - edge := node.GetEdge() - errArg := edge.Args[1] +// siblingSuccs builds an n-way branch CFG: entry(0) → blocks 1..n → merge(n+1). +func siblingSuccs(n int) (succs [][]int32, merge int32) { + succs = make([][]int32, n+2) + entry := make([]int32, 0, n) + for i := 1; i <= n; i++ { + entry = append(entry, int32(i)) //nolint:gosec // small + succs[i] = []int32{int32(n + 1)} //nolint:gosec // small + } + succs[0] = entry + return succs, int32(n + 1) //nolint:gosec // small +} - got := matcher.expandStatusesFromIdent(errArg, edge) - assert.Equal(t, []int{401, 404, 500}, got) +func sortedStatuses(in []int) []int { s := append([]int(nil), in...); sort.Ints(s); return s } - // Guards. - assert.Nil(t, matcher.expandStatusesFromIdent(nil, edge), "nil arg") - assert.Nil(t, matcher.expandStatusesFromIdent(buildLiteralArg(matcherMeta(matcher), "x"), edge), "non-ident arg") - assert.Nil(t, matcher.expandStatusesFromIdent(errArg, nil), "nil edge") +func TestExpandStatusesFromIdent_SiblingFanOut(t *testing.T) { + // Three mutually-exclusive sibling branches each reach the call → fan out all. + succs, merge := siblingSuccs(3) + matcher, edge := csScenario(t, []csSpec{ + {"StatusUnauthorized", 1, false}, {"StatusNotFound", 2, false}, {"StatusInternalServerError", 3, false}, + }, merge, succs, true) + assert.Equal(t, []int{401, 404, 500}, sortedStatuses(matcher.expandStatusesFromIdent(edge.Args[1], edge))) +} - // Unknown variable (no assignments) -> nil. +func TestExpandStatusesFromIdent_Guards(t *testing.T) { + succs, merge := siblingSuccs(2) + matcher, edge := csScenario(t, []csSpec{{"StatusNotFound", 1, false}, {"StatusForbidden", 2, false}}, merge, succs, true) + assert.Nil(t, matcher.expandStatusesFromIdent(nil, edge), "nil arg") meta := matcherMeta(matcher) - assert.Nil(t, matcher.expandStatusesFromIdent(buildIdentArg(meta, "ghost", "app"), edge)) + assert.Nil(t, matcher.expandStatusesFromIdent(buildLiteralArg(meta, "x"), edge), "non-ident arg") + assert.Nil(t, matcher.expandStatusesFromIdent(edge.Args[1], nil), "nil edge") + assert.Nil(t, matcher.expandStatusesFromIdent(buildIdentArg(meta, "ghost", "app"), edge), "unknown variable") } func TestExpandStatusesFromIdent_SingleAssignmentUntouched(t *testing.T) { - // A single assignment must NOT fan out (latest-wins is preserved upstream). - matcher, node := csMatcher(t, []string{"StatusNotFound"}) - assert.Nil(t, matcher.expandStatusesFromIdent(node.GetEdge().Args[1], node.GetEdge())) + succs, merge := siblingSuccs(1) + matcher, edge := csScenario(t, []csSpec{{"StatusNotFound", 1, false}}, merge, succs, true) + assert.Nil(t, matcher.expandStatusesFromIdent(edge.Args[1], edge), "single assignment must not fan out") } func TestExpandStatusesFromIdent_DedupAndNonCall(t *testing.T) { - matcher, node := csMatcher(t, []string{"StatusNotFound", "StatusNotFound"}) - // Two assignments, same status -> deduped to one (and so no fan-out). - assert.Equal(t, []int{404}, matcher.expandStatusesFromIdent(node.GetEdge().Args[1], node.GetEdge())) + // Two sibling assignments of the SAME status dedup to one. + succs, merge := siblingSuccs(2) + matcher, edge := csScenario(t, []csSpec{{"StatusNotFound", 1, false}, {"StatusNotFound", 2, false}}, merge, succs, true) + assert.Equal(t, []int{404}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) + + // A non-call assignment is skipped (still ≥2 entries, only one is a call). + meta := matcherMeta(matcher) + fn := findFunction(meta, "app", "handler") + keep := fn.AssignmentMap["err"][0] // the 404 call at block 1 + fn.AssignmentMap["err"] = []metadata.Assignment{keep, {Value: *buildIdentArg(meta, "plain", "app")}} + assert.Equal(t, []int{404}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) +} - // An assignment whose value is not a call is skipped. +func TestExpandStatusesFromIdent_CallWithoutStatusSkipped(t *testing.T) { + // A call-valued assignment whose arguments carry no HTTP status contributes + // nothing; only the status-bearing sibling survives. + succs, merge := siblingSuccs(2) + matcher, edge := csScenario(t, []csSpec{{"StatusNotFound", 1, false}, {"StatusBadRequest", 2, false}}, merge, succs, true) meta := matcherMeta(matcher) fn := findFunction(meta, "app", "handler") - fn.AssignmentMap["err"] = []metadata.Assignment{ - csAssign(csNewErrorCall(meta, "StatusForbidden")), - {Value: *buildIdentArg(meta, "plain", "app")}, // not a call -> skipped + noStatus := metadata.NewCallArgument(meta) + noStatus.SetKind(metadata.KindCall) + noStatus.Fun = buildIdentArg(meta, "Wrap", "app") + noStatus.Args = []*metadata.CallArgument{buildIdentArg(meta, "msg", "app")} + fn.AssignmentMap["err"][1].Value = *noStatus + assert.Equal(t, []int{404}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) +} + +func TestExpandStatusesFromIdent_AfterCallReassignSameBlockKept(t *testing.T) { + // One straight-line block where the call sits BETWEEN two assignments: + // err=400 (node 0) → RespondWithError (node 1) → err=500 (node 2). The 500 store + // executes AFTER the response write, in the same block, so it must NOT shadow the + // 400 the call actually reads. Expect [400] (a regression guard for the kill + // predicate's "killer must reach the call" clause). + meta := pmcTestMeta() + sp := meta.StringPool + pos400, posCall, pos500 := "h.go:10:3", "h.go:11:3", "h.go:12:3" + assigns := []metadata.Assignment{ + {Value: csNewErrorCall(meta, "StatusBadRequest"), Position: sp.Get(pos400)}, + {Value: csNewErrorCall(meta, "StatusInternalServerError"), Position: sp.Get(pos500)}, + } + meta.InstallFunctionCFGForTest("handler", [][]int32{{}}, map[string]metadata.BlockLoc{ + pos400: {Block: 0, Node: 0}, + posCall: {Block: 0, Node: 1}, + pos500: {Block: 0, Node: 2}, + }) + meta.Packages["app"] = &metadata.Package{Files: map[string]*metadata.File{ + "h.go": {Functions: map[string]*metadata.Function{ + "handler": {Name: sp.Get("handler"), Pkg: sp.Get("app"), AssignmentMap: map[string][]metadata.Assignment{"err": assigns}}, + }}, + }} + edge := buildCallGraphEdge(meta, "handler", "app", "RespondWithError", "app", + []*metadata.CallArgument{buildIdentArg(meta, "w", "app"), buildIdentArg(meta, "err", "app")}) + edge.Position = sp.Get(posCall) + matcher := &ResponsePatternMatcherImpl{ + BasePatternMatcher: &BasePatternMatcher{cfg: pmcTestCfg(), contextProvider: NewContextProvider(meta), schemaMapper: NewSchemaMapper(pmcTestCfg())}, + pattern: ResponsePattern{StatusFromArg: true, StatusArgIndex: 1}, } - assert.Equal(t, []int{403}, matcher.expandStatusesFromIdent(node.GetEdge().Args[1], node.GetEdge())) + assert.Equal(t, []int{400}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) +} + +func TestExpandStatusesFromIdent_UnconditionalShadow(t *testing.T) { + // Straight line: 500 (block1) → 400 (block2) → call (block3). 400 dominates the + // call and 500 reaches it, so 500 is overwritten — only 400 survives. + succs := [][]int32{{1}, {2}, {3}, {}} + matcher, edge := csScenario(t, []csSpec{{"StatusInternalServerError", 1, true}, {"StatusBadRequest", 2, true}}, 3, succs, true) + assert.Equal(t, []int{400}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) +} + +func TestExpandStatusesFromIdent_AfterCallExcluded(t *testing.T) { + // 400 (block1) → call (block2) → 500 (block3). 500 is after the call and cannot + // reach it, so only 400 contributes. + succs := [][]int32{{1}, {2}, {3}, {}} + matcher, edge := csScenario(t, []csSpec{{"StatusBadRequest", 1, false}, {"StatusInternalServerError", 3, false}}, 2, succs, true) + assert.Equal(t, []int{400}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) +} + +func TestExpandStatusesFromIdent_DegradeAndWarn(t *testing.T) { + // No CFG installed → the call cannot be placed → degrade to the unconditionally- + // reachable statuses (the Branch==nil one) and warn. + matcher, edge := csScenario(t, []csSpec{{"StatusInternalServerError", 0, true}, {"StatusBadRequest", 1, false}}, 0, nil, false) + var buf bytes.Buffer + matcher.warnings = &WarningSink{out: &buf} + assert.Equal(t, []int{500}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) + assert.Contains(t, buf.String(), "warning:") } func TestExpandStatusesFromIdent_NoMetadata(t *testing.T) { - // A matcher whose context provider isn't a *ContextProviderImpl yields no - // metadata, so expansion is skipped. meta := pmcTestMeta() edge := buildCallGraphEdge(meta, "handler", "app", "RespondWithError", "app", []*metadata.CallArgument{buildIdentArg(meta, "err", "app")}) @@ -134,94 +240,25 @@ func TestExpandStatusesFromIdent_NoMetadata(t *testing.T) { assert.Nil(t, matcher.expandStatusesFromIdent(edge.Args[0], edge)) } -// TestExtractResponse_ConditionalStatusFanOut drives the full ExtractResponse -// path: a RespondWithError(w, err) whose err is branch-assigned to distinct -// status constructors fans out to one response per status. +// TestExtractResponse_ConditionalStatusFanOut drives the full ExtractResponse path. func TestExtractResponse_ConditionalStatusFanOut(t *testing.T) { - matcher, node := csMatcher(t, []string{"StatusUnauthorized", "StatusNotFound", "StatusInternalServerError"}) + succs, merge := siblingSuccs(3) + matcher, edge := csScenario(t, []csSpec{ + {"StatusUnauthorized", 1, false}, {"StatusNotFound", 2, false}, {"StatusInternalServerError", 3, false}, + }, merge, succs, true) route := &RouteInfo{Response: map[string]*ResponseInfo{}, UsedTypes: map[string]*Schema{}} - - infos := matcher.ExtractResponse(node, route) + infos := matcher.ExtractResponse(buildTrackerNode(edge), route) require.Len(t, infos, 3, "one response per distinct branch status") - got := []int{infos[0].StatusCode, infos[1].StatusCode, infos[2].StatusCode} - sort.Ints(got) - assert.Equal(t, []int{401, 404, 500}, got) + assert.Equal(t, []int{401, 404, 500}, sortedStatuses([]int{infos[0].StatusCode, infos[1].StatusCode, infos[2].StatusCode})) } -func csAssign(v metadata.CallArgument) metadata.Assignment { return metadata.Assignment{Value: v} } - func matcherMeta(m *ResponsePatternMatcherImpl) *metadata.Metadata { return metadataFromContextProvider(m.contextProvider) } -func TestPositionAfter(t *testing.T) { - cases := []struct { - a, b string - want bool - }{ - {"f.go:20:1", "f.go:10:1", true}, // later line - {"f.go:10:5", "f.go:10:2", true}, // same line, later column - {"f.go:10:1", "f.go:20:1", false}, // earlier line - {"f.go:10:1", "f.go:10:1", false}, // equal - {"", "f.go:10:1", false}, // missing a - {"f.go:10:1", "", false}, // missing b - {"bad", "f.go:10:1", false}, // unparseable a - {"f.go:x:1", "f.go:10:1", false}, // non-numeric line - } - for _, c := range cases { - assert.Equal(t, c.want, positionAfter(c.a, c.b), "%q vs %q", c.a, c.b) - } -} - -func TestExpandStatusesFromIdent_Reachability(t *testing.T) { - // Two assignments (400 then 500); the 500 is positioned AFTER the response - // call site, so only 400 reaches it (issue #50). - matcher, node := csMatcher(t, []string{"StatusBadRequest", "StatusInternalServerError"}) - meta := matcherMeta(matcher) - sp := meta.StringPool - fn := findFunction(meta, "app", "handler") - fn.AssignmentMap["err"][0].Position = sp.Get("h.go:10:3") // before the call - fn.AssignmentMap["err"][1].Position = sp.Get("h.go:20:3") // after the call - - edge := node.GetEdge() - edge.Position = sp.Get("h.go:15:3") // call site between the two assignments - - assert.Equal(t, []int{400}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) -} - -func TestExpandStatusesFromIdent_UnconditionalShadow(t *testing.T) { - // code=500 (unconditional) then code=400 (unconditional) before the call: - // the later unconditional assignment overwrites the earlier on every path, - // so only 400 reaches the call (issue #50, the reused-variable case). - matcher, node := csMatcher(t, []string{"StatusInternalServerError", "StatusBadRequest"}) - meta := matcherMeta(matcher) - sp := meta.StringPool - fn := findFunction(meta, "app", "handler") - fn.AssignmentMap["err"][0].Position = sp.Get("h.go:10:3") // 500, earlier - fn.AssignmentMap["err"][0].Branch = nil // unconditional - fn.AssignmentMap["err"][1].Position = sp.Get("h.go:12:3") // 400, later - fn.AssignmentMap["err"][1].Branch = nil // unconditional (shadows 500) - edge := node.GetEdge() - edge.Position = sp.Get("h.go:15:3") - assert.Equal(t, []int{400}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) -} - -func TestExpandStatusesFromIdent_SiblingBranchesNotShadowed(t *testing.T) { - // Two conditional (if-then / if-else) assignments before the call don't - // shadow each other — both reach, so the intended fan-out is preserved. - matcher, node := csMatcher(t, []string{"StatusBadRequest", "StatusNotFound"}) - meta := matcherMeta(matcher) - sp := meta.StringPool - fn := findFunction(meta, "app", "handler") - fn.AssignmentMap["err"][0].Position = sp.Get("h.go:10:3") - fn.AssignmentMap["err"][0].Branch = &metadata.BranchContext{BlockKind: "if-then"} - fn.AssignmentMap["err"][1].Position = sp.Get("h.go:12:3") - fn.AssignmentMap["err"][1].Branch = &metadata.BranchContext{BlockKind: "if-else"} - edge := node.GetEdge() - edge.Position = sp.Get("h.go:15:3") - got := matcher.expandStatusesFromIdent(edge.Args[1], edge) - sort.Ints(got) - assert.Equal(t, []int{400, 404}, got) +func TestDedupInts(t *testing.T) { + assert.Equal(t, []int{1, 2, 3}, dedupInts([]int{1, 2, 2, 3, 1, 3})) + assert.Empty(t, dedupInts(nil)) } func TestStatusFromCallArgs_NoStatus(t *testing.T) { @@ -235,21 +272,3 @@ func TestStatusFromCallArgs_NoStatus(t *testing.T) { _, ok := statusFromCallArgs(matcher, call) assert.False(t, ok) } - -func TestExpandStatusesFromIdent_ShadowRobustToUnparseablePosition(t *testing.T) { - // Copilot (#57): the LATER unconditional assignment — the real shadow - // boundary — has an unparseable position. The index-based boundary must - // still shadow the earlier 500, leaving only 400 (a position-based boundary - // would have stayed pinned to 500 and leaked it). - matcher, node := csMatcher(t, []string{"StatusInternalServerError", "StatusBadRequest"}) - meta := matcherMeta(matcher) - sp := meta.StringPool - fn := findFunction(meta, "app", "handler") - fn.AssignmentMap["err"][0].Position = sp.Get("h.go:10:3") // 500, parseable - fn.AssignmentMap["err"][0].Branch = nil - fn.AssignmentMap["err"][1].Position = sp.Get("???") // 400, UNPARSEABLE - fn.AssignmentMap["err"][1].Branch = nil - edge := node.GetEdge() - edge.Position = sp.Get("h.go:15:3") - assert.Equal(t, []int{400}, matcher.expandStatusesFromIdent(edge.Args[1], edge)) -} diff --git a/internal/spec/extractor.go b/internal/spec/extractor.go index b2cc5d4..3898d18 100644 --- a/internal/spec/extractor.go +++ b/internal/spec/extractor.go @@ -19,6 +19,7 @@ import ( "maps" "regexp" "slices" + "sort" "strconv" "strings" "sync" @@ -263,6 +264,20 @@ type Extractor struct { requestMatchers []RequestPatternMatcher responseMatchers []ResponsePatternMatcher paramMatchers []ParamPatternMatcher + + warnings *WarningSink // non-fatal analysis warnings → stderr (spec 009, FR-008/FR-012) +} + +// warn records a non-fatal analysis warning (lazily creating a stderr sink). Used by +// the helper-binding degrade path (FR-012). +func (e *Extractor) warn(pos, msg string) { + if e == nil { + return + } + if e.warnings == nil { + e.warnings = NewWarningSink() + } + e.warnings.Warn(pos, msg) } // isLikelyMediaType checks if a string looks like a valid MIME type (type/subtype). @@ -938,15 +953,31 @@ func (n *callGraphEdgeNode) GetArgContext() string func (n *callGraphEdgeNode) GetRootAssignmentMap() map[string][]metadata.Assignment { return nil } // splitByConditionalMethods checks if a route's responses have CFG branch -// context with HTTP method case values (e.g., switch r.Method case "GET"). -// If so, returns separate RouteInfo entries per method. Returns nil if no -// conditional methods are detected. +// context with HTTP method case values — a `switch r.Method { case "GET": … }` +// OR an `if r.Method == http.MethodPost { … }` guard, both of which record the +// method in CaseValues (spec 009, US2). If so, returns separate RouteInfo entries +// per method. Returns nil if no conditional methods are detected. +// +// Every other response is attributed to the per-method operations using the +// per-function CFG (spec 009): an unconditional response, or an *independent* +// conditional (an `if cond { … }` orthogonal to the method dispatch), is reachable +// whatever the method ran and so is carried onto EVERY operation; a conditional +// *nested inside* a method arm (e.g. a 404 in the `case GET:` body) is carried onto +// that one method; and the dispatch FALLBACK arm — a `switch r.Method` default or a +// bare `else` of an `if r.Method ==`, whose branch point is shared with the method +// arms — is excluded from the handled methods (a 405 there is not a response of GET +// or POST). When the handler has no CFG model, non-method conditionals are +// conservatively excluded (the pre-009 behavior). +// +// Known limitation: the dispatch fallback arm is not emitted as its OWN operation +// (it has no single HTTP method to attach to). Chain explicit arms +// (`else if r.Method == http.MethodGet`) to get an operation for each method. func (e *Extractor) splitByConditionalMethods(route *RouteInfo) []*RouteInfo { // Collect HTTP methods from response branch contexts methodResponses := make(map[string]map[string]*ResponseInfo) // method → statusCode → response for statusCode, resp := range route.Response { - if resp.Branch == nil || resp.Branch.BlockKind != "switch-case" || len(resp.Branch.CaseValues) == 0 { + if resp.Branch == nil || len(resp.Branch.CaseValues) == 0 { continue } for _, val := range resp.Branch.CaseValues { @@ -965,8 +996,89 @@ func (e *Extractor) splitByConditionalMethods(route *RouteInfo) []*RouteInfo { return nil // Not enough methods to split } - var result []*RouteInfo - for method, responses := range methodResponses { + // Classify every response NOT already attributed to a method, using the + // per-function CFG (spec 009). See the doc comment for the cases. fnKey == "" + // (no model) degrades to excluding non-method conditionals, never leaking a + // fallback onto a method. + meta := e.tree.GetMetadata() + fnKey := primaryDispatchFn(meta, methodResponses) + armBlocks, armBlockToMethods := dispatchArms(methodResponses) + // dispatchRoot is the common dominator of the arms of EVERY dispatch in fnKey that + // CONTRIBUTED a method response (recorded per dispatch at annotation time — every + // `switch` case incl. the default, every `if r.Method ==` chain arm). One contributing + // dispatch → its exact tag (covering a combined `case "GET","POST"` whose default is in + // the same group, and an arm whose success was lost to an early-return). Responses split + // across an `if r.Method` AND a `switch r.Method` → the dominator spanning both, so the + // second dispatch's default is still excluded. A dispatch that wrote no response (a + // per-method header switch ahead of the responding one) is left out, so the independent + // conditional in the cfg_method_two_dispatch shape stays shared. + // + // Limitation (pre-existing, == the pre-CFG behavior; needs reaching-defs to fix): when + // TWO dispatches both respond, their union's common dominator can be the function entry, + // which dominates everything — so an independent conditional sequenced between them that + // is mutually exclusive with all arms (e.g. it early-returns) is over-excluded. + dispatchRoot, haveRoot := commonDominator(meta, fnKey, contributingDispatchArms(meta, fnKey, methodResponses)) + + shared := make(map[string]*ResponseInfo) + for statusCode, resp := range route.Response { + if resp.Branch == nil { + shared[statusCode] = resp // unconditional → on every method + continue + } + if branchNamesMethod(resp.Branch.CaseValues) { + continue // already attributed to its method(s) above + } + if fnKey == "" { + continue // no CFG model: conservatively exclude (pre-009 behavior) + } + // The branch's BlockIndex is meaningful only in its OWN function's CFG. A + // response inferred from a HELPER carries the helper's block index; querying it + // against the handler's CFG is unsound — it could alias an unrelated handler + // block and leak onto a method it does not belong to. Exclude such foreign-branch + // responses conservatively (pre-CFG behavior); per-method attribution of an + // interprocedural conditional needs call-site context (a separate concern). + if meta.FnKeyForPos(meta.StringPool.GetString(resp.Branch.ParentStmtPos)) != fnKey { + continue + } + rb := resp.Branch.BlockIndex + if methods, ok := dominatingMethods(meta, fnKey, rb, armBlocks, armBlockToMethods); ok { + for _, m := range methods { // a combined `case "GET", "HEAD"` arm owns several + if methodResponses[m] != nil { + methodResponses[m][statusCode] = resp // nested inside that arm + } + } + continue + } + if isDispatchFallback(meta, fnKey, resp.Branch, dispatchRoot, haveRoot, armBlocks) { + continue // a `switch` default / `if r.Method ==` bare else → "any other method" + } + shared[statusCode] = resp // independent conditional → on every method + } + + // One route per method, emitted in METHOD order (map iteration is not deterministic; + // the caller appends to the route list without re-sorting). Each route owns a COPY of + // every ResponseInfo it carries, so the split routes share no *ResponseInfo: a per-route + // ApplyOverrides (e.g. line ~589) that mutates BodyType/BodyTypeRef in place on a shared + // (or combined-case) response can no longer leak that override onto another method's + // operation. + methods := make([]string, 0, len(methodResponses)) + for method := range methodResponses { + methods = append(methods, method) + } + sort.Strings(methods) + + result := make([]*RouteInfo, 0, len(methods)) + for _, method := range methods { + responses := methodResponses[method] + merged := make(map[string]*ResponseInfo, len(shared)+len(responses)) + for s, r := range shared { + rc := *r + merged[s] = &rc + } + for s, r := range responses { // method-specific wins on any status overlap + rc := *r + merged[s] = &rc + } mr := &RouteInfo{ Path: route.Path, MountPath: route.MountPath, @@ -979,7 +1091,7 @@ func (e *Extractor) splitByConditionalMethods(route *RouteInfo) []*RouteInfo { Description: route.Description, Tags: route.Tags, Request: route.Request, - Response: responses, + Response: merged, Params: route.Params, UsedTypes: route.UsedTypes, Metadata: route.Metadata, @@ -991,6 +1103,190 @@ func (e *Extractor) splitByConditionalMethods(route *RouteInfo) []*RouteInfo { return result } +// primaryDispatchFn returns the function key that the most method responses resolve to +// (via cfgPosToFn), deterministically with a lexicographic tiebreak — so a dispatch whose +// arms span more than one function (e.g. a sub-dispatch in a helper) does not make the +// choice depend on map-iteration order. Returns "" when no method branch resolves to a +// function — the caller then excludes non-method conditionals (pre-009 behavior). +func primaryDispatchFn(meta *metadata.Metadata, methodResponses map[string]map[string]*ResponseInfo) string { + counts := make(map[string]int) + for _, resps := range methodResponses { + for _, r := range resps { + if r.Branch == nil { + continue + } + if fn := meta.FnKeyForPos(meta.StringPool.GetString(r.Branch.ParentStmtPos)); fn != "" { + counts[fn]++ + } + } + } + best, bestN := "", 0 + for fn, n := range counts { + if n > bestN || (n == bestN && fn < best) { + best, bestN = fn, n + } + } + return best +} + +// contributingDispatchArms returns the block indices of every arm of every method +// dispatch in fnKey that CONTRIBUTED a method response — the union across dispatch groups. +// The common dominator of this union is the dispatch root: for one contributing dispatch +// it is that dispatch's tag; for responses split across an `if r.Method` and a +// `switch r.Method` it spans both, so the second dispatch's `default` is still dominated +// (and excluded) rather than leaked. A dispatch that wrote NO response (its group never +// appears in methodResponses — e.g. a per-method header switch ahead of the responding one) +// is left out, so it does not widen the root (the cfg_method_two_dispatch independent stays +// shared). When two dispatches BOTH respond, the union root can be the function entry — an +// independent between them that is mutually exclusive with all arms is then over-excluded, a +// pre-existing limitation (see splitByConditionalMethods). Group 0 (a response in a +// post-dispatch merge block, not an arm) and foreign-function branches are skipped. +func contributingDispatchArms(meta *metadata.Metadata, fnKey string, methodResponses map[string]map[string]*ResponseInfo) []int32 { + seen := make(map[int]bool) + var arms []int32 + for _, resps := range methodResponses { + for _, r := range resps { + if r.Branch == nil || r.Branch.DispatchGroup == 0 { + continue + } + if meta.FnKeyForPos(meta.StringPool.GetString(r.Branch.ParentStmtPos)) != fnKey { + continue + } + if g := r.Branch.DispatchGroup; !seen[g] { + seen[g] = true + arms = append(arms, meta.DispatchArms(fnKey, g)...) + } + } + } + return arms +} + +// dispatchArms returns the sorted method-arm branch blocks and each block → the +// HTTP method(s) it serves. A combined `case "GET", "HEAD":` lowers to ONE block +// that serves both methods, so the value is a slice (mapping it to a single method +// would be last-writer-wins over map order — nondeterministic). Both the block list +// and each method list are sorted so downstream attribution is deterministic. +func dispatchArms(methodResponses map[string]map[string]*ResponseInfo) ([]int32, map[int32][]string) { + armBlockToMethods := make(map[int32][]string) + for method, resps := range methodResponses { + for _, r := range resps { + if r.Branch == nil { + continue + } + b := r.Branch.BlockIndex + if !slices.Contains(armBlockToMethods[b], method) { + armBlockToMethods[b] = append(armBlockToMethods[b], method) + } + } + } + blocks := make([]int32, 0, len(armBlockToMethods)) + for b := range armBlockToMethods { + blocks = append(blocks, b) + } + sort.Slice(blocks, func(i, j int) bool { return blocks[i] < blocks[j] }) + for _, ms := range armBlockToMethods { + sort.Strings(ms) + } + return blocks, armBlockToMethods +} + +// commonDominator returns the lowest common dominator of the given blocks — the +// "dispatch root" that every method arm descends from (the `switch r.Method` tag or +// the first `if r.Method ==` condition). ok=false when there is no CFG model. go/cfg +// lowers a switch into a chain of test blocks, so the arms' immediate dominators +// differ; their common dominator is the stable root shared by every arm INCLUDING +// the default. Walks immediate-dominator chains (Cooper-Harvey-Kennedy idom tree). +func commonDominator(meta *metadata.Metadata, fnKey string, blocks []int32) (int32, bool) { + if fnKey == "" || len(blocks) == 0 { + return -1, false + } + cur := blocks[0] + for _, b := range blocks[1:] { + anc := make(map[int32]bool) + for x := cur; ; { + anc[x] = true + p, ok := meta.IDom(fnKey, x) + if !ok { + break // reached the entry (it dominates everything) + } + x = p + } + y := b + for !anc[y] { + p, ok := meta.IDom(fnKey, y) + if !ok { + break + } + y = p + } + cur = y + } + return cur, true +} + +// isDispatchFallback reports whether a non-method conditional response (branch br) +// must be EXCLUDED from the handled methods: either a `switch` `default:` arm +// (recognised structurally by its empty case values, so a stray `fallthrough` into +// it cannot leak its 405 onto a method), or a fallback that descends from the +// dispatch root yet shares no control-flow path with any arm (the bare `else` of an +// `if r.Method ==` chain). A conditional outside the dispatch (root does not +// dominate it, e.g. a pre-dispatch `if bad { … return }`) or one reachable together +// with the arms is NOT a fallback — it is independent and shared. +// +// Limitation: the structural `default:` rule also matches the default arm of an +// UNRELATED switch dominated by the dispatch root (e.g. an orthogonal +// `switch mode { default: … }` after the dispatch); such a response is excluded +// rather than shared. This is conservative, not a regression — the pre-CFG split +// dropped every non-unconditional response here regardless. +func isDispatchFallback(meta *metadata.Metadata, fnKey string, br *metadata.BranchContext, dispatchRoot int32, haveRoot bool, armBlocks []int32) bool { + if !haveRoot || !meta.Dominates(fnKey, dispatchRoot, br.BlockIndex) { + return false + } + isSwitchDefault := br.BlockKind == "switch-case" && len(br.CaseValues) == 0 + return isSwitchDefault || mutuallyExclusiveWithArms(meta, fnKey, br.BlockIndex, armBlocks) +} + +// mutuallyExclusiveWithArms reports whether block rb shares NO control-flow path +// with any method arm (neither reaches the other) — true for a dispatch fallback +// arm (a sibling of the method arms), false for a conditional that falls through +// to or from the arms (an independent before/after the dispatch). +func mutuallyExclusiveWithArms(meta *metadata.Metadata, fnKey string, rb int32, armBlocks []int32) bool { + for _, a := range armBlocks { + if a == rb { + return false + } + if meta.Reaches(fnKey, metadata.BlockLoc{Block: a}, metadata.BlockLoc{Block: rb}) || + meta.Reaches(fnKey, metadata.BlockLoc{Block: rb}, metadata.BlockLoc{Block: a}) { + return false + } + } + return true +} + +// dominatingMethods returns the HTTP method(s) whose dispatch arm dominates block +// rb (rb lies inside that arm's region, e.g. a conditional nested in `case GET:`), +// or ok=false if no method arm dominates it. armBlocks is sorted, so when several +// arms dominate rb (unusual nested/fallthrough shapes) the choice is deterministic. +func dominatingMethods(meta *metadata.Metadata, fnKey string, rb int32, armBlocks []int32, armBlockToMethods map[int32][]string) ([]string, bool) { + for _, armBlock := range armBlocks { + if armBlock != rb && meta.Dominates(fnKey, armBlock, rb) { + return armBlockToMethods[armBlock], true + } + } + return nil, false +} + +// branchNamesMethod reports whether any of a branch's case values name an HTTP +// method (so its responses are already attributed per method). +func branchNamesMethod(caseValues []string) bool { + for _, v := range caseValues { + if isValidHTTPMethodStr(strings.ToUpper(v)) { + return true + } + } + return false +} + func isValidHTTPMethodStr(s string) bool { switch s { case "GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS", "HEAD": @@ -1312,71 +1608,403 @@ func (e *Extractor) helperFallbackEdges(routeNode TrackerNodeInterface) map[stri var visit func(node TrackerNodeInterface, isRoot bool) visit = func(node TrackerNodeInterface, isRoot bool) { - children := node.GetChildren() - // A node represents a USER-DEFINED helper invocation when: - // 1. it is not the route node itself (whose children are the - // handler's body — branches there are legitimate control flow, - // not internal fallback logic), AND - // 2. its edge carries a ParamArgMap (the call passed bound arguments - // through to the callee's parameters), AND - // 3. the call itself is not a response-pattern primitive (Status, - // JSON, WriteHeader, …). For chained calls like - // `c.Status(400).JSON(map)`, the Status node may have the JSON - // node as a child; treating Status as a helper would - // mis-classify legitimate handler branches as fallbacks. - isHelperInvocation := false - if !isRoot { - if edge := node.GetEdge(); edge != nil && len(edge.ParamArgMap) > 0 { - nodeIsResponsePrimitive := false - for _, m := range e.responseMatchers { - if m.MatchNode(node) { - nodeIsResponsePrimitive = true - break - } - } - if !nodeIsResponsePrimitive { - isHelperInvocation = true + if e.isHelperInvocation(node, isRoot) { + hw := e.classifyHelperWrites(node) + // #27: a defensive branch only contaminates the caller when the SAME + // helper also has an unconditional (primary) write to compare against. + if hw.hasUnconditional() { + for _, child := range hw.conditional { + fallback[child.GetEdge().Callee.ID()] = true } } } + for _, child := range node.GetChildren() { + visit(child, false) + } + } + visit(routeNode, true) + return fallback +} - if isHelperInvocation { - var unconditional bool - var conditionalIDs []string - for _, child := range children { - childEdge := child.GetEdge() - if childEdge == nil { - continue - } - matched := false - for _, m := range e.responseMatchers { - if m.MatchNode(child) { - matched = true - break +// helperWrites partitions a helper invocation's response-writing child edges by +// whether each is reachable on the helper's unconditional (primary) path or only +// under an internal branch. It is the reusable core shared by the #27 fallback +// filter (helperFallbackEdges) and the US1 helper-internal type-switch binding / +// degrade (FR-011/FR-012). +type helperWrites struct { + unconditional []TrackerNodeInterface // Branch == nil — the helper's primary path + conditional []TrackerNodeInterface // Branch != nil — guarded by an internal branch +} + +func (h helperWrites) hasUnconditional() bool { return len(h.unconditional) > 0 } + +// isHelperInvocation reports whether node represents a USER-DEFINED helper call +// (as opposed to the route node or a response primitive). A node is a helper +// invocation when: +// 1. it is not the route node itself (whose children are the handler's body — +// branches there are legitimate control flow, not internal fallback logic), +// 2. its edge carries a ParamArgMap (the call passed bound arguments through to +// the callee's parameters), and +// 3. the call itself is not a response-pattern primitive (Status, JSON, +// WriteHeader, …) — for chained calls like `c.Status(400).JSON(map)` the +// Status node may parent the JSON node, and treating Status as a helper would +// mis-classify legitimate handler branches. +func (e *Extractor) isHelperInvocation(node TrackerNodeInterface, isRoot bool) bool { + if isRoot { + return false + } + edge := node.GetEdge() + return edge != nil && len(edge.ParamArgMap) > 0 && !e.matchesAnyResponse(node) +} + +// classifyHelperWrites returns the response-writing DIRECT children of a helper- +// invocation node, partitioned by whether each is reachable unconditionally or only +// under an internal if-branch. Non-response children are ignored. +// +// It scans direct children only — deliberately NOT the whole subtree (CodeRabbit +// suggested recursing to catch nested writes; that was evaluated and rejected — see +// below). The #27 defensive-fallback shape it serves is +// `WriteHeader(status); if err { http.Error }`, where both the unconditional primary +// write and the if-then fallback are DIRECT children. A SUB-handler reached through a +// wrapper, by contrast, writes its success body via a nested +// `json.NewEncoder(w).Encode(v)` (a grandchild under NewEncoder) and its real error +// via a direct `if err { http.Error(4xx) }`. Recursing would treat that nested +// success write as the helper's "unconditional primary" and then wrongly filter the +// sub-handler's genuine 4xx as a defensive fallback (regression observed on the +// enum_validation fixture: POST drops its 400). Direct-children scanning is correct +// for both shapes. +func (e *Extractor) classifyHelperWrites(node TrackerNodeInterface) helperWrites { + var hw helperWrites + for _, child := range node.GetChildren() { + if child.GetEdge() == nil || !e.matchesAnyResponse(child) { + continue + } + br := child.GetEdge().Branch + switch { + case br == nil: + hw.unconditional = append(hw.unconditional, child) + case br.BlockKind == "if-then" || br.BlockKind == "if-else": + // Only an if-guarded write is a #27-style defensive fallback. A + // switch-case/select-case write is a type-switch (or method) arm owned by + // the type-switch-binding pass, so it must NOT be filtered here as a + // fallback — doing so would drop a precisely-bound arm. + hw.conditional = append(hw.conditional, child) + } + } + return hw +} + +// matchesAnyResponse reports whether node matches any registered response pattern. +func (e *Extractor) matchesAnyResponse(node TrackerNodeInterface) bool { + for _, m := range e.responseMatchers { + if m.MatchNode(node) { + return true + } + } + return false +} + +// helperTypeSwitchEdges resolves helper-internal type-switches against the +// call-site argument (FR-011/FR-012, US1 #5/#6) and returns the set of case-write +// edge IDs to FILTER OUT. For each helper invocation whose internal branches are +// type-switch arms, it binds the switched parameter to the call-site argument (via +// the case's SwitchOperand + the edge's ParamArgMap) and resolves its concrete +// type: +// - precise concrete binding → keep only the matched arm (or the default arm +// when the concrete type matches no case); filter the rest, no warning; +// - imprecise binding (interface/error/any, or the operand cannot be resolved) → +// keep only the default/unconditional path, filter all typed arms, and warn — +// never fanning out every arm. +// +// Returned set is keyed by edge ID (the same ID used for visitedEdges), so it +// composes with helperFallbackEdges. +// +// Arm-write edge IDs are keyed by the helper's INTERNAL call position, which is +// identical across multiple call sites of the same helper in one route. A handler +// that invokes the same type-switching helper twice with different concrete +// arguments must therefore INTERSECT, not union, the per-site decisions: an arm +// kept by ANY site is reachable and must survive. We collect both the filtered and +// the kept sets and return filtered MINUS kept. +func (e *Extractor) helperTypeSwitchEdges(routeNode TrackerNodeInterface) map[string]bool { + ae := armEdges{filtered: map[string]bool{}, kept: map[string]bool{}} + if routeNode == nil { + return ae.filtered + } + var visit func(node TrackerNodeInterface, isRoot bool) + visit = func(node TrackerNodeInterface, isRoot bool) { + if e.isHelperInvocation(node, isRoot) { + e.bindHelperTypeSwitch(node, ae) + } + for _, child := range node.GetChildren() { + visit(child, false) + } + } + visit(routeNode, true) + for id := range ae.kept { + delete(ae.filtered, id) + } + return ae.filtered +} + +// armEdges accumulates, across every type-switch binding in one route, the arm-write +// edge IDs to drop (filtered) and the ones an actual binding keeps (kept). The final +// drop set is filtered − kept, so a write reachable from any call site survives. +type armEdges struct { + filtered map[string]bool + kept map[string]bool +} + +// switchArm holds the response-writing edges of one type-switch's arms, gathered +// from a helper's subtree: typed arms (with case types) and the default arm. +type switchArm struct { + typed []TrackerNodeInterface // arms with non-empty CaseTypeRefs + dflt []TrackerNodeInterface // the default arm (no case types) +} + +// bindHelperTypeSwitch performs the per-invocation binding for helperTypeSwitchEdges, +// adding the filtered-out arm-write edge IDs to filtered. A single helper may host +// more than one type-switch (on different parameters), so arms are grouped by the +// switched operand and each group whose operand is one of this node's callee +// parameters is bound independently. +func (e *Extractor) bindHelperTypeSwitch(node TrackerNodeInterface, ae armEdges) { + edge := node.GetEdge() + if edge == nil || len(edge.ParamArgMap) == 0 { + return + } + arms, escaped := e.collectSwitchArms(node) + if escaped { + // The helper has a conditional (if/else) response write that is not a clean + // type-switch arm — typically a `case` body with nested control flow, whose + // writes go/cfg annotates with the inner branch's context rather than the + // arm's, but also any other if-guarded write in the helper. In either case + // the arms are not fully captured, so we keep ALL arms (the pre-binding + // over-approximation) rather than risk dropping or leaking a status + // (FR-012: never mis-bind; over-approximate when uncertain). + return + } + for _, operand := range slices.Sorted(maps.Keys(arms)) { + arm := arms[operand] + if len(arm.typed) == 0 { + continue + } + arg, isHost := edge.ParamArgMap[operand] + if !isHost { + // The operand is not a parameter of this node's callee — this node only + // parents the arm writes in the tracker tree (e.g. an inner + // json.NewEncoder call), so it is not the switch's binding site. + continue + } + argRef, precise := e.boundArgRef(&arg, node) + e.applyTypeSwitchBinding(arm, argRef, precise, edge, ae) + } +} + +// collectSwitchArms gathers, from node's subtree, every response-writing edge whose +// BranchContext is a type-switch arm DECLARED IN THE HOST'S OWN FUNCTION, grouped by +// the switched operand. It recurses through the whole subtree because an arm's +// writes (WriteHeader, Encode, …) may be nested under intermediate calls (e.g. +// json.NewEncoder(w).Encode) rather than direct children of the helper invocation; +// the `sameFunc` scope check confines collection to the host callee's body so a +// NESTED helper's type-switch (whose operand may shadow an outer parameter name) is +// left to its own binding pass instead of being mis-bound against this call site. +// escaped is true when the host function has any response write under an +// if-then/if-else branch. The common cause is a type-switch arm with nested +// control flow (its writes are attributed to the inner branch, not the arm), but +// any if-guarded response write in the helper trips it. The caller then declines +// to filter (over-approximates safely) because the arms are not fully captured. +func (e *Extractor) collectSwitchArms(node TrackerNodeInterface) (arms map[string]*switchArm, escaped bool) { + arms = make(map[string]*switchArm) + host := node.GetEdge() + if host == nil { + return arms, false + } + var walk func(n TrackerNodeInterface) + walk = func(n TrackerNodeInterface) { + for _, child := range n.GetChildren() { + ce := child.GetEdge() + if ce != nil && ce.Branch != nil && sameFunc(ce.Caller, host.Callee) && e.matchesAnyResponse(child) { + // A "clean" type-switch arm write is a switch-case block carrying the + // switched operand. ANY other conditional response write in the helper + // — an if/else, a select case, a nested or unrelated value-switch + // (switch-case with no operand) — means an arm contains control flow + // go/cfg attributes elsewhere, so the arms are not cleanly captured. + if ce.Branch.BlockKind == "switch-case" && ce.Branch.SwitchOperand != "" { + arm := arms[ce.Branch.SwitchOperand] + if arm == nil { + arm = &switchArm{} + arms[ce.Branch.SwitchOperand] = arm + } + if len(ce.Branch.CaseTypeRefs) > 0 { + arm.typed = append(arm.typed, child) + } else { + arm.dflt = append(arm.dflt, child) } - } - if !matched { - continue - } - if childEdge.Branch == nil { - unconditional = true } else { - conditionalIDs = append(conditionalIDs, childEdge.Callee.ID()) + escaped = true } } - if unconditional { - for _, id := range conditionalIDs { - fallback[id] = true + walk(child) + } + } + walk(node) + return arms, escaped +} + +// sameFunc reports whether two Call references identify the same function (by name, +// package, and receiver type — all string-pool indices in one Metadata). +func sameFunc(a, b metadata.Call) bool { + return a.Name == b.Name && a.Pkg == b.Pkg && a.RecvType == b.RecvType +} + +// applyTypeSwitchBinding decides which arm writes to filter for one type-switch, +// given the bound call-site argument leaf: +// - precise concrete match → keep the matched arms, filter the other typed arms +// AND the default (the concrete type selects a specific arm; default does not run); +// - precise but no match (a concrete type that hits the default) → keep the default, +// filter the typed arms, no warning; +// - imprecise binding → keep the default, filter the typed arms, and warn (FR-012). +func (e *Extractor) applyTypeSwitchBinding(arm *switchArm, argRef *metadata.TypeRef, precise bool, edge *metadata.CallGraphEdge, ae armEdges) { + var selected []TrackerNodeInterface + if precise && argRef != nil { + for _, c := range arm.typed { + if refMatchesAnyCase(c.GetEdge().Branch.CaseTypeRefs, argRef) { + selected = append(selected, c) + } + } + } + + if len(selected) > 0 { + // Precise match: KEEP the matched arms; filter the other typed arms AND the + // default (the concrete type selects a specific arm; default does not run). + keep := make(map[string]bool, len(selected)) + for _, c := range selected { + id := c.GetEdge().Callee.ID() + keep[id] = true + ae.kept[id] = true + } + mark := func(nodes []TrackerNodeInterface) { + for _, c := range nodes { + if id := c.GetEdge().Callee.ID(); !keep[id] { + ae.filtered[id] = true } } } + mark(arm.typed) + mark(arm.dflt) + return + } - for _, child := range children { - visit(child, false) + // No typed arm selected. When the binding is IMPRECISE and there is no default + // arm to fall back to, the runtime value will still hit one of the typed arms — + // we cannot tell which — so KEEP them all (over-approximate) and warn, rather + // than drop every response. (A precise concrete type that matches no case and + // has no default genuinely produces nothing, which is Go-correct.) + if !precise && len(arm.dflt) == 0 { + for _, c := range arm.typed { + ae.kept[c.GetEdge().Callee.ID()] = true } + e.warn(e.contextProvider.GetString(edge.Position), + "helper type-switch: call-site argument type not statically known; emitting all arms") + return + } + + // Otherwise KEEP the default arm, filter every typed arm. + for _, c := range arm.dflt { + ae.kept[c.GetEdge().Callee.ID()] = true + } + for _, c := range arm.typed { + ae.filtered[c.GetEdge().Callee.ID()] = true + } + if !precise { + e.warn(e.contextProvider.GetString(edge.Position), + "helper type-switch: call-site argument type not statically known; emitting unconditional result only") } - visit(routeNode, true) - return fallback +} + +// boundArgRef resolves a call-site argument to its STATIC type ref. precise is +// false when that type is an interface/error/any that does not pin a concrete type +// at the call site (FR-012 degrade), in which case the ref is returned as nil. +// +// The argument's structured TypeRef is the static type of the argument expression — +// exactly what a Go type-switch discriminates, and conservative per FR-012: an +// `any`/`error`-typed argument stays a RefInterface (imprecise) even if a concrete +// value flowed into it, so we never over-resolve across an interface boundary. The +// string-based origin is the fallback only when the structured ref is absent (a +// deserialized/hand-built argument). +func (e *Extractor) boundArgRef(arg *metadata.CallArgument, node TrackerNodeInterface) (ref *metadata.TypeRef, precise bool) { + ref = arg.TypeRef + if ref == nil { + _, ref = sharedResolveTypeOrigin(arg, node, e.contextProvider.GetArgumentInfo(arg), e.contextProvider, false) + } + if isImpreciseLeaf(ref.NamedLeaf()) { + return nil, false + } + return ref, true +} + +// isImpreciseLeaf reports whether a resolved named leaf fails to pin a concrete +// type — a nil/interface leaf, or the built-in dynamic names error/any/interface{} +// (FR-012). +// +// Known limitation: a NAMED interface (e.g. a domain `Animal`, or io.Reader) whose +// concrete value is unknown is carried as RefNamed, not RefInterface, so it is not +// recognised as imprecise here. If such a value is the type-switch operand AND the +// switch has a same-named interface case, the binding will pick that interface arm +// instead of degrading. Recognising named interfaces needs interface-aware type +// metadata at TypeRef-construction time (a non-trivial cross-cutting change); the +// pattern — a value of named-interface static type matched against its own +// interface case — is uncommon, so it is left as a documented gap. +func isImpreciseLeaf(leaf *metadata.TypeRef) bool { + if leaf == nil || leaf.Kind == metadata.RefInterface { + return true + } + switch leaf.Name { + case "", "error", "any", "interface{}": + return true + } + return false +} + +// typeRefShapeEqual reports whether two type refs denote the SAME Go type, +// structurally: same Kind, and for named types same package+name, recursing +// through pointer/slice/array/map elements, map keys, array length, and generic +// instantiation arguments. This is the exact-type identity a Go type-switch tests +// — it distinguishes T from *T, []T, and Box[A] from Box[B] — so the binding never +// over- or under-matches across the value/pointer/slice/generic distinctions +// (FR-012). It is immune to package-qualifier string-format differences (it +// compares the structured fields, not String()). +func typeRefShapeEqual(a, b *metadata.TypeRef) bool { + if a == nil || b == nil { + return a == b + } + if a.Kind != b.Kind || a.Name != b.Name || a.Pkg != b.Pkg || a.Len != b.Len { + return false + } + if !typeRefShapeEqual(a.Elem, b.Elem) || !typeRefShapeEqual(a.Key, b.Key) { + return false + } + if len(a.Args) != len(b.Args) { + return false + } + for i := range a.Args { + if !typeRefShapeEqual(a.Args[i], b.Args[i]) { + return false + } + } + return true +} + +// refMatchesAnyCase reports whether the argument's static type is exactly one of a +// case clause's types (a `case A, B:` clause carries several). The match is the +// structural type identity a Go type-switch performs, so a `*T`/`[]T`/`Box[User]` +// argument binds only the case of that same shape — never a sibling arm. +func refMatchesAnyCase(caseRefs []*metadata.TypeRef, argRef *metadata.TypeRef) bool { + for _, cr := range caseRefs { + if typeRefShapeEqual(cr, argRef) { + return true + } + } + return false } // extractRouteChildren extracts request, response, and params from children nodes @@ -1387,6 +2015,12 @@ func (e *Extractor) extractRouteChildren(routeNode TrackerNodeInterface, route * // must not contribute to the caller's response schema — see issue #27 and // helperFallbackEdges for the exact rule. fallbackEdges := e.helperFallbackEdges(routeNode) + // Also filter the type-switch arms of a response helper that the call-site + // argument does not bind to (spec 009, FR-011/FR-012). Both sets gate the same + // visitedEdges check, so merge them. + for id := range e.helperTypeSwitchEdges(routeNode) { + fallbackEdges[id] = true + } callbacks := []ExtractionCallback{ // Route-in-route detection @@ -2054,55 +2688,124 @@ func (r *ResponsePatternMatcherImpl) expandStatusesFromIdent(arg *metadata.CallA if !ok || len(assigns) < 2 { return nil } - // Reachability filter (issue #50): keep only the assignments whose value can - // reach this response call site, in two steps: - // - // 1. Drop assignments positioned textually after the call — they cannot - // supply the value at the call. - // 2. Among the survivors, an *unconditional* assignment (Branch == nil) - // overwrites every earlier assignment on every path, so it shadows them: - // keep only the assignments from the last unconditional one onward. - // Sibling if/else branch assignments (Branch != nil) don't shadow each - // other, so the intended fan-out — both branches reassigning before one - // trailing RespondWithError — is preserved. - // - // The shadow boundary is the *index* of the last unconditional survivor, not - // its source position: assignments are recorded in source order, so the - // index is a reliable boundary even when a position is missing/unparseable - // (a position-based boundary could be pinned to an earlier assignment and - // leak a shadowed status). Positions are only used — conservatively — for - // the textual after-call filter. + // Reachability filter (spec 009, FR-002/FR-006): keep the assignments whose + // value can reach this response call along some control-flow path AND that are + // not overwritten on every path to the call by a later, call-dominating + // assignment. Computed structurally from the CFG reachability model — replacing + // the former source-position + last-unconditional-index heuristic. Mutually + // exclusive sibling branches never reach each other, so they do not shadow one + // another (the if/else fan-out is preserved); an unconditional overwrite that + // dominates the call kills earlier assignments. callPos := meta.StringPool.GetString(edge.Position) + fnKey := meta.FnKeyForPos(callPos) + callLoc, callOK := meta.BlockFor(fnKey, callPos) + cands := r.collectStatusCands(meta, fnKey, assigns) - statuses := make([]int, 0, len(assigns)) - lastUncond := -1 + // FR-008: when the CFG cannot place this call (an unmodelled construct), degrade + // to the unconditionally-reachable statuses + warn, rather than guessing. + if !callOK || fnKey == "" { + return r.degradeToUnconditional(callPos, cands) + } + return contributingStatuses(meta, fnKey, callLoc, cands) +} + +// statusCand is a status code paired with its control-flow location, used by the +// reachability filter in expandStatusesFromIdent. +type statusCand struct { + status int + loc metadata.BlockLoc + hasLoc bool // the assignment was placed in a CFG block + uncond bool // the assignment is unconditional (Branch == nil) +} + +// collectStatusCands turns each call-valued assignment into a status candidate +// located in fnKey's CFG. Non-call assignments, and calls with no status-code +// argument, are skipped. +func (r *ResponsePatternMatcherImpl) collectStatusCands(meta *metadata.Metadata, fnKey string, assigns []metadata.Assignment) []statusCand { + cands := make([]statusCand, 0, len(assigns)) for i := range assigns { if assigns[i].Value.GetKind() != metadata.KindCall { continue } - if positionAfter(meta.StringPool.GetString(assigns[i].Position), callPos) { + status, okStatus := statusFromCallArgs(r, &assigns[i].Value) + if !okStatus { continue } - status, ok := statusFromCallArgs(r, &assigns[i].Value) - if !ok { + loc, hasLoc := meta.BlockFor(fnKey, meta.StringPool.GetString(assigns[i].Position)) + cands = append(cands, statusCand{status: status, loc: loc, hasLoc: hasLoc, uncond: assigns[i].Branch == nil}) + } + return cands +} + +// degradeToUnconditional returns the unconditionally-reachable statuses, warning +// when that drops any conditional candidate (FR-008). Used when the CFG cannot +// place the response call. +func (r *ResponsePatternMatcherImpl) degradeToUnconditional(callPos string, cands []statusCand) []int { + uncond := make([]int, 0, len(cands)) + for i := range cands { + if cands[i].uncond { + uncond = append(uncond, cands[i].status) + } + } + if len(uncond) < len(cands) { + r.warn(callPos, "conditional status fan-out: control flow not modelled; using unconditional statuses") + } + return dedupInts(uncond) +} + +// contributingStatuses applies the reachability + kill predicate (FR-002/FR-006): +// a candidate contributes iff it reaches the call and is not overwritten on every +// path by a later assignment whose block dominates the call. +// +// The kill test is a single-dominator approximation of "overwritten on every +// path": it does not recognise that a set of mutually-exclusive sibling +// reassignments can jointly cover every path (e.g. an unconditional default that +// BOTH arms of an if/else overwrite), so such a dead default still contributes a +// phantom status. This is a long-standing accuracy limitation shared with the +// pre-CFG heuristic — output is unchanged for it, so it is not introduced here; a +// full reaching-definitions pass would be the principled fix. +func contributingStatuses(meta *metadata.Metadata, fnKey string, callLoc metadata.BlockLoc, cands []statusCand) []int { + out := make([]int, 0, len(cands)) + for i := range cands { + if !cands[i].hasLoc || !meta.Reaches(fnKey, cands[i].loc, callLoc) { + continue // cannot reach the call + } + if !killedByDominator(meta, fnKey, callLoc, cands, i) { + out = append(out, cands[i].status) + } + } + return dedupInts(out) +} + +// killedByDominator reports whether cands[i] is overwritten on every path to the +// call: some cands[j] between cands[i] and the call whose block dominates the call +// block. cands[j] must lie ON a path from cands[i] to the call — it must both be +// reachable from cands[i] AND itself reach the call. The j-reaches-call clause is +// essential: a later reassignment that sits AFTER the call in the SAME straight-line +// block satisfies Reaches(i,j) and the reflexive block self-domination, but executes +// after the response write and must NOT shadow the value the call actually read. +func killedByDominator(meta *metadata.Metadata, fnKey string, callLoc metadata.BlockLoc, cands []statusCand, i int) bool { + for j := range cands { + if i == j || !cands[j].hasLoc || cands[i].loc == cands[j].loc { continue } - statuses = append(statuses, status) - if assigns[i].Branch == nil { - lastUncond = len(statuses) - 1 + if meta.Reaches(fnKey, cands[i].loc, cands[j].loc) && + meta.Reaches(fnKey, cands[j].loc, callLoc) && + meta.Dominates(fnKey, cands[j].loc.Block, callLoc.Block) { + return true } } + return false +} - start := 0 - if lastUncond >= 0 { - start = lastUncond - } - seen := make(map[int]struct{}, len(statuses)) - out := make([]int, 0, len(statuses)) - for _, status := range statuses[start:] { - if _, dup := seen[status]; !dup { - seen[status] = struct{}{} - out = append(out, status) +// dedupInts returns the input with duplicate values removed, preserving first-seen order. +func dedupInts(in []int) []int { + seen := make(map[int]struct{}, len(in)) + out := make([]int, 0, len(in)) + for _, v := range in { + if _, dup := seen[v]; !dup { + seen[v] = struct{}{} + out = append(out, v) } } return out @@ -2122,37 +2825,6 @@ func statusFromCallArgs(r *ResponsePatternMatcherImpl, call *metadata.CallArgume return 0, false } -// positionAfter reports whether source position a occurs strictly after b. -// Positions are "file:line:col"; comparison is by (line, col), using the last -// two colon-separated fields. Missing/unparseable positions return false so the -// caller does not over-filter on incomplete data. -func positionAfter(a, b string) bool { - la, ca, oka := positionLineCol(a) - lb, cb, okb := positionLineCol(b) - if !oka || !okb { - return false - } - if la != lb { - return la > lb - } - return ca > cb -} - -// positionLineCol parses the trailing line and column from a "file:line:col" -// position string. -func positionLineCol(pos string) (line, col int, ok bool) { - parts := strings.Split(pos, ":") - if len(parts) < 2 { - return 0, 0, false - } - col, errC := strconv.Atoi(parts[len(parts)-1]) - line, errL := strconv.Atoi(parts[len(parts)-2]) - if errC != nil || errL != nil { - return 0, 0, false - } - return line, col, true -} - // resolveTypeOrigin traces the origin of a type through assignments and type parameters func (r *ResponsePatternMatcherImpl) resolveTypeOrigin(arg *metadata.CallArgument, node TrackerNodeInterface, originalType string) (string, *metadata.TypeRef) { // Honour explicit resolved-type info on the argument first — set when an diff --git a/internal/spec/extractor_additional_test.go b/internal/spec/extractor_additional_test.go index 78d83f3..c6337d8 100644 --- a/internal/spec/extractor_additional_test.go +++ b/internal/spec/extractor_additional_test.go @@ -297,32 +297,48 @@ func TestSplitByConditionalMethods_InvalidHTTPMethod_Skipped(t *testing.T) { assert.Nil(t, result) } -func TestSplitByConditionalMethods_NonSwitchCase_Skipped(t *testing.T) { +// An `if r.Method == …` dispatch (recorded as if-then blocks carrying method +// CaseValues, spec 009 US2) splits into one operation per method, the same as a +// switch. +func TestSplitByConditionalMethods_IfDispatch(t *testing.T) { meta := newTestMeta() - tree := NewMockTrackerTree(meta, metadata.TrackerLimits{ - MaxNodesPerTree: 100, MaxChildrenPerNode: 10, MaxArgsPerFunction: 5, MaxNestedArgsDepth: 3, - }) - cfg := &APISpecConfig{ - Defaults: Defaults{ResponseContentType: "application/json"}, + ext, _ := newTestExtractor(meta) + + route := &RouteInfo{ + Path: "/items", + Handler: "items", + Response: map[string]*ResponseInfo{ + "200": {StatusCode: 200, Branch: &metadata.BranchContext{BlockKind: "if-then", CaseValues: []string{"GET"}}}, + "201": {StatusCode: 201, Branch: &metadata.BranchContext{BlockKind: "if-then", CaseValues: []string{"POST"}}}, + }, } - ext := NewExtractor(tree, cfg) + + result := ext.splitByConditionalMethods(route) + require.Len(t, result, 2) + methods := map[string]bool{} + for _, r := range result { + methods[r.Method] = true + } + assert.True(t, methods["GET"]) + assert.True(t, methods["POST"]) +} + +// A non-method `if` (no method case values) must NOT split, even though if-then +// branches are now eligible. +func TestSplitByConditionalMethods_NonMethodIf_NotSplit(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) route := &RouteInfo{ Path: "/resource", Handler: "handler", Response: map[string]*ResponseInfo{ - "200": { - StatusCode: 200, - Branch: &metadata.BranchContext{ - BlockKind: "if-then", - CaseValues: []string{"GET"}, - }, - }, + "200": {StatusCode: 200, Branch: &metadata.BranchContext{BlockKind: "if-then"}}, + "500": {StatusCode: 500, Branch: &metadata.BranchContext{BlockKind: "if-then", CaseValues: []string{"notamethod"}}}, }, } - result := ext.splitByConditionalMethods(route) - assert.Nil(t, result) + assert.Nil(t, ext.splitByConditionalMethods(route)) } // =========================================================================== diff --git a/internal/spec/extractor_helper_test.go b/internal/spec/extractor_helper_test.go index d89252a..41f007e 100644 --- a/internal/spec/extractor_helper_test.go +++ b/internal/spec/extractor_helper_test.go @@ -1072,7 +1072,11 @@ func TestInferStatusParamFromCalls_NoStatusArg(t *testing.T) { // WriteHeader(500) inside an `if err != nil { ... }` branch. Returns the // route node and the conditional edge for assertions. func fallbackTestRig(meta *metadata.Metadata) (*TrackerNode, *metadata.CallGraphEdge) { + // The write edges live in WriteJSON's body, so their Caller is WriteJSON + // (classifyHelperWrites scopes collection to the host callee via sameFunc). + inHelper := metadata.Call{Meta: meta, Name: meta.StringPool.Get("WriteJSON"), Pkg: meta.StringPool.Get("helpers")} condWHEdge := &metadata.CallGraphEdge{ + Caller: inHelper, Callee: metadata.Call{ Meta: meta, Name: meta.StringPool.Get("WriteHeader"), @@ -1083,6 +1087,7 @@ func fallbackTestRig(meta *metadata.Metadata) (*TrackerNode, *metadata.CallGraph Branch: &metadata.BranchContext{BlockKind: "if-then"}, } succWHEdge := &metadata.CallGraphEdge{ + Caller: inHelper, Callee: metadata.Call{ Meta: meta, Name: meta.StringPool.Get("WriteHeader"), @@ -1124,7 +1129,9 @@ func TestHelperFallbackEdges_NoUnconditional_KeepsAll(t *testing.T) { meta := newTestMeta() ext, _ := newTestExtractor(meta) + inHelper := metadata.Call{Meta: meta, Name: meta.StringPool.Get("WriteEither"), Pkg: meta.StringPool.Get("helpers")} condWH1 := &metadata.CallGraphEdge{ + Caller: inHelper, Callee: metadata.Call{ Meta: meta, Name: meta.StringPool.Get("WriteHeader"), Pkg: meta.StringPool.Get("net/http"), RecvType: meta.StringPool.Get("ResponseWriter"), @@ -1133,6 +1140,7 @@ func TestHelperFallbackEdges_NoUnconditional_KeepsAll(t *testing.T) { Branch: &metadata.BranchContext{BlockKind: "if-then"}, } condWH2 := &metadata.CallGraphEdge{ + Caller: inHelper, Callee: metadata.Call{ Meta: meta, Name: meta.StringPool.Get("WriteHeader"), Pkg: meta.StringPool.Get("net/http"), RecvType: meta.StringPool.Get("ResponseWriter"), diff --git a/internal/spec/pattern_matchers.go b/internal/spec/pattern_matchers.go index 8be8dd7..648462b 100644 --- a/internal/spec/pattern_matchers.go +++ b/internal/spec/pattern_matchers.go @@ -127,6 +127,19 @@ type BasePatternMatcher struct { contextProvider ContextProvider cfg *APISpecConfig schemaMapper SchemaMapper + warnings *WarningSink // non-fatal analysis warnings → stderr (spec 009, FR-008) +} + +// warn records a non-fatal analysis warning (lazily creating a stderr sink). Used by +// the conditional-analysis degrade paths (FR-008/FR-012). +func (b *BasePatternMatcher) warn(pos, msg string) { + if b == nil { + return + } + if b.warnings == nil { + b.warnings = NewWarningSink() + } + b.warnings.Warn(pos, msg) } // NewBasePatternMatcher creates a new base pattern matcher diff --git a/internal/spec/split_classifier_test.go b/internal/spec/split_classifier_test.go new file mode 100644 index 0000000..ba48349 --- /dev/null +++ b/internal/spec/split_classifier_test.go @@ -0,0 +1,585 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 spec + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/antst/go-apispec/internal/metadata" +) + +// scExtractor builds an Extractor whose mock tree exposes meta. +func scExtractor(meta *metadata.Metadata) *Extractor { + tree := NewMockTrackerTree(meta, metadata.TrackerLimits{ + MaxNodesPerTree: 100, MaxChildrenPerNode: 10, MaxArgsPerFunction: 5, MaxNestedArgsDepth: 3, + }) + return NewExtractor(tree, &APISpecConfig{Defaults: Defaults{ResponseContentType: "application/json"}}) +} + +// scBranch builds a method-arm BranchContext at the given block, in dispatch group 1, +// whose ParentStmtPos (pos) resolves to a function's CFG (so primaryDispatchFn finds it). +func scBranch(meta *metadata.Metadata, block int32, pos string, methods ...string) *metadata.BranchContext { + return &metadata.BranchContext{ + BlockKind: "switch-case", + BlockIndex: block, + CaseValues: methods, + ParentStmtPos: meta.StringPool.Get(pos), + DispatchGroup: 1, + } +} + +// scArms records dispatch group 1's arm blocks (every method arm INCLUDING the +// default/dropped) on fn's CFG, so the dispatch root is their common dominator (the tag). +func scArms(meta *metadata.Metadata, fn string, blocks ...int32) { + meta.FunctionCFGs[fn].DispatchArms = map[int][]int32{1: blocks} +} + +// scResp is a tiny ResponseInfo constructor for these tests. +func scResp(code int, body string, br *metadata.BranchContext) *ResponseInfo { + return &ResponseInfo{StatusCode: code, ContentType: "application/json", BodyType: body, Branch: br} +} + +// scCollect returns method → sorted status codes present on each split route. +func scCollect(routes []*RouteInfo) map[string][]int { + out := map[string][]int{} + for _, r := range routes { + codes := make([]int, 0, len(r.Response)) + for _, resp := range r.Response { + codes = append(codes, resp.StatusCode) + } + // insertion-sort keeps the test assertion order-stable + for i := 1; i < len(codes); i++ { + for j := i; j > 0 && codes[j-1] > codes[j]; j-- { + codes[j-1], codes[j] = codes[j], codes[j-1] + } + } + out[r.Method] = codes + } + return out +} + +// switchCFG models go/cfg's lowering of `switch r.Method { case GET; case POST; +// default }`: a test chain (B0→B4→B6) with case bodies hanging off it. B2=GET body, +// B3=POST body, B5=default body, B1=SwitchDone merge. Verified against go/cfg. +func switchCFG() [][]int32 { + return [][]int32{ + {2, 4}, // B0 tag + {}, // B1 SwitchDone (merge) + {1}, // B2 GET body + {1}, // B3 POST body + {3, 6}, // B4 SwitchNextCase (tests POST) + {1}, // B5 default body + {5}, // B6 SwitchNextCase (→ default) + } +} + +// TestSplit_SwitchDefaultExcluded: the `default:` 405 descends from the dispatch +// root (B0) and is mutually exclusive with both arms → excluded from GET and POST. +func TestSplit_SwitchDefaultExcluded(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 2}, "post:1": {Block: 3}, "def:1": {Block: 5}, + }) + scArms(meta, fn, 2, 3, 5) // GET, POST, and the default arm — root is the switch tag + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 2, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + "405": scResp(405, "", scBranch(meta, 5, "def:1")), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200}, got["GET"], "GET must not carry the 405 default") + assert.Equal(t, []int{201}, got["POST"], "POST must not carry the 405 default") +} + +// TestSplit_ResponsesCopiedPerRoute: each split route owns its OWN *ResponseInfo for a +// shared response, and routes are emitted in deterministic method order — so a per-route +// ApplyOverrides mutation cannot leak across methods, and the route slice order is stable. +func TestSplit_ResponsesCopiedPerRoute(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 2}, "post:1": {Block: 3}, + }) + scArms(meta, fn, 2, 3) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 2, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + // a COMBINED case response shared across GET+POST via the method-specific + // (responses) map — exercises that copy site too, not just `shared`. + "204": scResp(204, "Both", scBranch(meta, 2, "get:1", "GET", "POST")), + "500": scResp(500, "Err", nil), // unconditional → shared onto every method + }, + } + routes := ext.splitByConditionalMethods(route) + require.Len(t, routes, 2) + + // Deterministic method order (sorted): GET before POST. + assert.Equal(t, "GET", routes[0].Method) + assert.Equal(t, "POST", routes[1].Method) + + getR, postR := routes[0], routes[1] + // Both the shared (500) and the combined-case method-specific (204) responses are + // COPIED per route — distinct pointers, and a per-route mutation does not leak. + for _, code := range []string{"500", "204"} { + require.NotNil(t, getR.Response[code], "GET missing %s", code) + require.NotNil(t, postR.Response[code], "POST missing %s", code) + assert.NotSame(t, getR.Response[code], postR.Response[code], + "%s must be COPIED per route, not aliased", code) + getR.Response[code].BodyType = "MUTATED" + assert.NotEqual(t, "MUTATED", postR.Response[code].BodyType, + "a per-route mutation of %s must not leak across methods", code) + } +} + +// TestSplit_AfterMergeIndependentShared: a conditional after the switch merge (B7, +// reachable from the arms) is reachable together with every method → shared. +func TestSplit_AfterMergeIndependentShared(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + succs := switchCFG() + succs[1] = []int32{7} // SwitchDone → after-block + succs = append(succs, nil) // B7 (after-merge independent), terminal + meta.InstallFunctionCFGForTest(fn, succs, map[string]metadata.BlockLoc{ + "get:1": {Block: 2}, "post:1": {Block: 3}, "ind:1": {Block: 7}, + }) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 2, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + // An if-form independent after the merge (a switch `default:` here would be + // the dispatch fallback; this is an orthogonal `if logErr { 500 }`). + "500": scResp(500, "Err", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 7, ParentStmtPos: meta.StringPool.Get("ind:1")}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200, 500}, got["GET"], "GET must carry the shared 500") + assert.Equal(t, []int{201, 500}, got["POST"], "POST must carry the shared 500") +} + +// ifFormCFG models `if bad {…return}; if GET {…} else if POST {…}`: the dispatch +// root is B2 (the first `if r.Method`), NOT the function entry, so the pre-dispatch +// 500 (B1) is not dominated by the root. +func ifFormCFG() [][]int32 { + return [][]int32{ + {1, 2}, // B0 `if bad` cond + {}, // B1 bad-then (500, returns) + {3, 4}, // B2 `if GET` cond (dispatch root) + {6}, // B3 GET body + {5, 6}, // B4 `else if POST` cond + {6}, // B5 POST body + {}, // B6 merge + } +} + +// TestSplit_PreDispatchIndependentShared: a pre-dispatch 500 sits outside the +// dispatch (root does not dominate it) → shared onto every method (issue F1). +func TestSplit_PreDispatchIndependentShared(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, ifFormCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 3}, "post:1": {Block: 5}, "bad:1": {Block: 1}, + }) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 3, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 5, "post:1", "POST")), + "500": scResp(500, "Err", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 1, ParentStmtPos: meta.StringPool.Get("bad:1")}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200, 500}, got["GET"]) + assert.Equal(t, []int{201, 500}, got["POST"]) +} + +// nestedCFG: GET arm (B1) contains a nested conditional (B3, idom=B1); POST is B2. +func nestedCFG() [][]int32 { + return [][]int32{ + {1, 2}, // B0 `if GET` cond + {3, 4}, // B1 GET arm, nested if + {5}, // B2 POST arm + {5}, // B3 nested 404 (idom B1) + {5}, // B4 GET 200 continuation (idom B1) + {}, // B5 merge + } +} + +// TestSplit_NestedConditionalAttributedToMethod: a conditional dominated by the GET +// arm belongs to GET only, never POST. +func TestSplit_NestedConditionalAttributedToMethod(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, nestedCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 1}, "post:1": {Block: 2}, "nf:1": {Block: 3}, + }) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 1, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 2, "post:1", "POST")), + "404": scResp(404, "NF", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 3, ParentStmtPos: meta.StringPool.Get("nf:1")}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200, 404}, got["GET"], "nested 404 belongs to GET") + assert.Equal(t, []int{201}, got["POST"], "POST must not carry GET's nested 404") +} + +// TestSplit_NoCFGModelDegrades: with no CFG model, a non-method conditional is +// conservatively excluded (never leaked onto a method as a phantom). +func TestSplit_NoCFGModelDegrades(t *testing.T) { + meta := newTestMeta() + ext := scExtractor(meta) // no InstallFunctionCFGForTest → fnKey == "" + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", &metadata.BranchContext{BlockKind: "switch-case", BlockIndex: 2, CaseValues: []string{"GET"}}), + "201": scResp(201, "Made", &metadata.BranchContext{BlockKind: "switch-case", BlockIndex: 3, CaseValues: []string{"POST"}}), + "500": scResp(500, "Err", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 5}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200}, got["GET"]) + assert.Equal(t, []int{201}, got["POST"]) +} + +// TestSplit_UnconditionalShared: a Branch==nil response is shared onto all methods. +func TestSplit_UnconditionalShared(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 2}, "post:1": {Block: 3}, + }) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 2, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + "400": scResp(400, "Err", nil), // unconditional + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200, 400}, got["GET"]) + assert.Equal(t, []int{201, 400}, got["POST"]) +} + +// combinedCFG: a combined `case "GET","HEAD"` arm (B1) holds a nested conditional +// (B3, idom=B1); POST is B2. The arm block B1 serves both GET and HEAD. +func combinedCFG() [][]int32 { + return [][]int32{ + {1, 2}, // B0 dispatch + {3, 4}, // B1 GET/HEAD arm, nested if + {5}, // B2 POST arm + {5}, // B3 nested 404 (idom B1) + {5}, // B4 GET/HEAD 200 continuation + {}, // B5 merge + } +} + +// TestSplit_CombinedMethodCase: a conditional nested in a combined `case "GET", +// "HEAD"` arm is attributed to BOTH methods, deterministically (not whichever the +// map happened to map the shared arm block to). +func TestSplit_CombinedMethodCase(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, combinedCFG(), map[string]metadata.BlockLoc{ + "gh:1": {Block: 1}, "post:1": {Block: 2}, "nf:1": {Block: 3}, + }) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 1, "gh:1", "GET", "HEAD")), + "201": scResp(201, "Made", scBranch(meta, 2, "post:1", "POST")), + "404": scResp(404, "NF", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 3, ParentStmtPos: meta.StringPool.Get("nf:1")}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200, 404}, got["GET"], "nested 404 attributed to GET") + assert.Equal(t, []int{200, 404}, got["HEAD"], "AND to HEAD (combined case)") + assert.Equal(t, []int{201}, got["POST"], "POST unaffected") +} + +// fallthroughCFG: the POST arm (B3) FALLS THROUGH into the default body (B5), so +// the default is reachable from an arm (mutual exclusivity fails) — it must still +// be excluded, recognised structurally as the switch `default:`. +func fallthroughCFG() [][]int32 { + return [][]int32{ + {2, 4}, // B0 tag + {}, // B1 SwitchDone + {1}, // B2 GET body → merge + {5}, // B3 POST body → default (fallthrough!) + {3, 6}, // B4 SwitchNextCase + {1}, // B5 default body (405) → merge + {5}, // B6 SwitchNextCase → default + } +} + +// TestSplit_FallthroughDefaultExcluded: a `fallthrough` into the default must not +// leak the 405 onto the handled methods (regression guard for the inner-loop find). +func TestSplit_FallthroughDefaultExcluded(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, fallthroughCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 2}, "post:1": {Block: 3}, "def:1": {Block: 5}, + }) + scArms(meta, fn, 2, 3, 5) // GET, POST, and the default arm — root is the switch tag + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 2, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + "405": scResp(405, "", scBranch(meta, 5, "def:1")), // switch-case, empty case values + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200}, got["GET"], "405 must not leak onto GET via fallthrough") + assert.Equal(t, []int{201}, got["POST"], "405 must not leak onto POST via fallthrough") +} + +// TestSplit_ForeignBranchExcluded: a non-method response whose branch belongs to a +// DIFFERENT function (an interprocedural helper) must be excluded, never reasoned +// about against the handler's CFG — else its helper-local block index aliases a +// handler block and leaks the response onto a method it does not belong to. +func TestSplit_ForeignBranchExcluded(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{ + "get:1": {Block: 2}, "post:1": {Block: 3}, + }) + // A separate helper function whose branch position resolves to "helper", not fn. + meta.InstallFunctionCFGForTest("helper", [][]int32{{1}, {}}, map[string]metadata.BlockLoc{ + "helper:1": {Block: 1}, + }) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "200": scResp(200, "List", scBranch(meta, 2, "get:1", "GET")), + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + "599": scResp(599, "Trace", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 1, ParentStmtPos: meta.StringPool.Get("helper:1")}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{200}, got["GET"], "foreign 599 must not appear on GET") + assert.Equal(t, []int{201}, got["POST"], "foreign 599 must not leak onto POST") +} + +// droppedArmCFG: a switch with GET (B2, whose success at B7 is branchless so GET +// drops out of methodResponses), POST (B3), DELETE (B4); the GET arm holds a nested +// 404 (B5, returns). With the recorded dispatch group covering ALL three arms, the +// dispatch root is the tag B0, so the orphaned 404 is excluded — not leaked onto +// POST/DELETE. +func droppedArmCFG() [][]int32 { + return [][]int32{ + {2, 6}, // B0 tag (GET test) + {}, // B1 SwitchDone + {5, 7}, // B2 GET body: nested if → 404 or 200 + {1}, // B3 POST body + {1}, // B4 DELETE body + {}, // B5 nested 404 (returns) + {3, 8}, // B6 SwitchNextCase (POST test) + {1}, // B7 GET 200 (branchless merge) + {4}, // B8 SwitchNextCase (→ DELETE) + } +} + +// TestSplit_DroppedArmConditionalExcluded: when a method arm drops out (its success +// lost to an early-return), a conditional orphaned in that arm must NOT leak onto the +// surviving methods — the dispatch root (from the recorded group's arms) still covers it. +func TestSplit_DroppedArmConditionalExcluded(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, droppedArmCFG(), map[string]metadata.BlockLoc{ + "post:1": {Block: 3}, "del:1": {Block: 4}, "nf:1": {Block: 5}, + }) + scArms(meta, fn, 2, 3, 4) // the group's arms incl. the dropped GET arm (block 2) + ext := scExtractor(meta) + route := &RouteInfo{ + Path: "/r", Function: "h", UsedTypes: map[string]*Schema{}, + Response: map[string]*ResponseInfo{ + "201": scResp(201, "Made", scBranch(meta, 3, "post:1", "POST")), + "204": scResp(204, "", scBranch(meta, 4, "del:1", "DELETE")), + "404": scResp(404, "NF", &metadata.BranchContext{BlockKind: "if-then", BlockIndex: 5, ParentStmtPos: meta.StringPool.Get("nf:1")}), + }, + } + got := scCollect(ext.splitByConditionalMethods(route)) + assert.Equal(t, []int{201}, got["POST"], "404 from the dropped GET arm must not leak onto POST") + assert.Equal(t, []int{204}, got["DELETE"], "nor onto DELETE") +} + +// --- direct helper unit tests --- + +func TestCommonDominator(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{"x": {Block: 2}}) + + root, ok := commonDominator(meta, fn, []int32{2, 3}) + require.True(t, ok) + assert.Equal(t, int32(0), root, "LCA of the GET/POST arms is the switch tag B0") + + single, ok := commonDominator(meta, fn, []int32{3}) + require.True(t, ok) + assert.Equal(t, int32(3), single) + + _, ok = commonDominator(meta, "", []int32{2, 3}) + assert.False(t, ok, "no fnKey → no root") + _, ok = commonDominator(meta, fn, nil) + assert.False(t, ok, "no blocks → no root") + + // An unreachable arm block (idom = -1) forces the ancestor walk to break + // without meeting a common ancestor; the call still returns, never panics. + um := newTestMeta() + um.InstallFunctionCFGForTest("u", [][]int32{{1}, {}, {}}, map[string]metadata.BlockLoc{"x": {Block: 0}}) + r2, ok := commonDominator(um, "u", []int32{1, 2}) + assert.True(t, ok) + assert.Equal(t, int32(2), r2) +} + +func TestMutuallyExclusiveWithArms(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{"x": {Block: 2}}) + + assert.True(t, mutuallyExclusiveWithArms(meta, fn, 5, []int32{2, 3}), "default arm shares no path with the arms") + assert.False(t, mutuallyExclusiveWithArms(meta, fn, 0, []int32{2, 3}), "the tag reaches both arms") + assert.False(t, mutuallyExclusiveWithArms(meta, fn, 2, []int32{2, 3}), "an arm block is not exclusive with itself") +} + +func TestDominatingMethods(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, nestedCFG(), map[string]metadata.BlockLoc{"x": {Block: 1}}) + + armBlocks := []int32{1, 2} + byBlock := map[int32][]string{1: {"GET"}, 2: {"POST"}} + m, ok := dominatingMethods(meta, fn, 3, armBlocks, byBlock) + require.True(t, ok) + assert.Equal(t, []string{"GET"}, m, "B3 is dominated by the GET arm B1") + + _, ok = dominatingMethods(meta, fn, 2, armBlocks, byBlock) + assert.False(t, ok, "POST arm dominates no other method's region") +} + +func TestBranchNamesMethod(t *testing.T) { + assert.True(t, branchNamesMethod([]string{"GET"})) + assert.True(t, branchNamesMethod([]string{"post"}), "case-insensitive") + assert.True(t, branchNamesMethod([]string{"active", "DELETE"})) + assert.False(t, branchNamesMethod([]string{"active"})) + assert.False(t, branchNamesMethod(nil)) +} + +func TestDispatchArms(t *testing.T) { + mr := map[string]map[string]*ResponseInfo{ + // GET has two statuses in the SAME arm block (exercises the per-method dedup). + "GET": {"200": scResp(200, "L", &metadata.BranchContext{BlockIndex: 2}), "204": scResp(204, "", &metadata.BranchContext{BlockIndex: 2})}, + "HEAD": {"200": scResp(200, "L", &metadata.BranchContext{BlockIndex: 2})}, // combined arm: same block + "POST": {"201": scResp(201, "M", &metadata.BranchContext{BlockIndex: 3})}, + "NIL": {"500": scResp(500, "", nil)}, // a branchless entry is skipped (defensive) + } + blocks, byBlock := dispatchArms(mr) + assert.Equal(t, []int32{2, 3}, blocks, "blocks sorted") + assert.Equal(t, []string{"GET", "HEAD"}, byBlock[2], "a combined case serves both methods, sorted (GET deduped)") + assert.Equal(t, []string{"POST"}, byBlock[3]) +} + +func TestPrimaryDispatchFn(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{"get:1": {Block: 2}}) + + resolved := map[string]map[string]*ResponseInfo{ + "GET": {"200": scResp(200, "L", scBranch(meta, 2, "get:1", "GET"))}, + } + assert.Equal(t, fn, primaryDispatchFn(meta, resolved)) + + unresolved := map[string]map[string]*ResponseInfo{ + "GET": {"200": scResp(200, "L", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("nowhere:9")})}, + } + assert.Equal(t, "", primaryDispatchFn(meta, unresolved)) + + nilBranch := map[string]map[string]*ResponseInfo{"GET": {"200": scResp(200, "L", nil)}} + assert.Equal(t, "", primaryDispatchFn(meta, nilBranch)) +} + +// TestPrimaryDispatchFn_MultiFunction: when responses resolve to more than one function +// (a sub-dispatch in a helper), the MOST COMMON fn is returned deterministically, never +// one that depends on map-iteration order. +func TestPrimaryDispatchFn_MultiFunction(t *testing.T) { + meta := newTestMeta() + meta.InstallFunctionCFGForTest("fnA", [][]int32{{}}, map[string]metadata.BlockLoc{"a:1": {Block: 0}}) + meta.InstallFunctionCFGForTest("fnB", [][]int32{{}}, map[string]metadata.BlockLoc{"b:1": {Block: 0}, "b:2": {Block: 0}}) + mr := map[string]map[string]*ResponseInfo{ + "GET": {"200": scResp(200, "L", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("a:1")})}, + "POST": {"201": scResp(201, "M", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("b:1")})}, + "PUT": {"204": scResp(204, "", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("b:2")})}, + } + for i := 0; i < 50; i++ { // fnB has 2 responses, fnA has 1 → fnB, stable across map orders + assert.Equal(t, "fnB", primaryDispatchFn(meta, mr)) + } +} + +// TestContributingDispatchArms: the dispatch root is scoped to the UNION of arms over +// every group that contributed a response — each distinct group counted once, with +// group-0 (post-merge) and foreign-function branches skipped. This is what lets a +// handler split across an `if r.Method` + a `switch r.Method` exclude the second +// dispatch's default (both groups in the union), while a non-responding dispatch's arms +// stay out. +func TestContributingDispatchArms(t *testing.T) { + meta := newTestMeta() + const fn = "fn" + meta.InstallFunctionCFGForTest(fn, switchCFG(), map[string]metadata.BlockLoc{"x": {Block: 2}}) + // fn records arms for groups 1, 2 (contributing), 0 and 7 (which MUST be skipped — the + // arms below let the test catch a removed skip: a leak would add 98 or 88/99). + meta.FunctionCFGs[fn].DispatchArms = map[int][]int32{1: {2, 3}, 2: {5, 6}, 0: {98}, 7: {88, 99}} + // A separate function owns the foreign branch's position. + meta.InstallFunctionCFGForTest("other", [][]int32{{}}, map[string]metadata.BlockLoc{"foreign:9": {Block: 0}}) + + mr := map[string]map[string]*ResponseInfo{ + // two responses in group 1 (same group → arms counted once) + one in group 2. + "GET": {"200": scResp(200, "L", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("x"), DispatchGroup: 1})}, + "HEAD": {"200": scResp(200, "L", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("x"), DispatchGroup: 1})}, + "POST": {"201": scResp(201, "M", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("x"), DispatchGroup: 2})}, + // group 0 (a post-dispatch merge response): the group-0 guard must skip it, so fn's + // DispatchArms[0]={98} is NOT unioned in. + "PUT": {"204": scResp(204, "", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("x"), DispatchGroup: 0})}, + // a branch in a FOREIGN function naming group 7 (which also exists in fn's arms): the + // FnKeyForPos guard must skip it, so fn's DispatchArms[7]={88,99} is NOT unioned in. + "DELETE": {"204": scResp(204, "", &metadata.BranchContext{ParentStmtPos: meta.StringPool.Get("foreign:9"), DispatchGroup: 7})}, + } + got := contributingDispatchArms(meta, fn, mr) + assert.ElementsMatch(t, []int32{2, 3, 5, 6}, got, + "union of contributing groups 1 and 2; group-0 arms {98} and the foreign-fn group-7 arms {88,99} excluded") + + assert.Empty(t, contributingDispatchArms(meta, "absent", mr), "unknown fn → no arms") +} diff --git a/internal/spec/typeswitch_binding_test.go b/internal/spec/typeswitch_binding_test.go new file mode 100644 index 0000000..cd2a9b8 --- /dev/null +++ b/internal/spec/typeswitch_binding_test.go @@ -0,0 +1,395 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 spec + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/antst/go-apispec/internal/metadata" +) + +func tsNamed(name string) *metadata.TypeRef { + return &metadata.TypeRef{Kind: metadata.RefNamed, Name: name, Pkg: "app"} +} + +func tsPtr(elem *metadata.TypeRef) *metadata.TypeRef { + return &metadata.TypeRef{Kind: metadata.RefPointer, Elem: elem} +} + +// tsArmWrite builds a WriteHeader response edge in a type-switch arm: caseRefs are +// the arm's case types (nil ⇒ the default arm), operand is the switched variable. +func tsArmWrite(meta *metadata.Metadata, pos string, caseRefs []*metadata.TypeRef, operand string) metadata.CallGraphEdge { + edge := metadata.CallGraphEdge{ + // Caller is the host helper "Respond" (app): the arm write lives in the + // helper body whose type-switch declares it, which collectSwitchArms scopes by. + Caller: metadata.Call{Meta: meta, Name: meta.StringPool.Get("Respond"), Pkg: meta.StringPool.Get("app")}, + Callee: metadata.Call{ + Meta: meta, + Name: meta.StringPool.Get("WriteHeader"), + Pkg: meta.StringPool.Get("net/http"), + RecvType: meta.StringPool.Get("ResponseWriter"), + Position: meta.StringPool.Get(pos), + }, + Position: meta.StringPool.Get(pos), + Args: []*metadata.CallArgument{makeIdentArg(meta, "code", "int")}, + Branch: &metadata.BranchContext{BlockKind: "switch-case", CaseTypeRefs: caseRefs, SwitchOperand: operand}, + } + edge.Callee.Edge = &edge + return edge +} + +// tsHostTree builds a route → Respond(w, x) host → {404 arm, 200 arm, default arm} +// tree, where x is bound to the given argument TypeRef. Returns the extractor, root, +// and the three arm edges. +func tsHostTree(t *testing.T, argRef *metadata.TypeRef) (*Extractor, *TrackerNode, metadata.CallGraphEdge, metadata.CallGraphEdge, metadata.CallGraphEdge) { + t.Helper() + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + + xarg := metadata.NewCallArgument(meta) + xarg.SetKind(metadata.KindIdent) + xarg.SetName("x") + xarg.TypeRef = argRef + host := makeHelperEdge(meta, "Respond", "app", map[string]metadata.CallArgument{"x": *xarg}) + + wh404 := tsArmWrite(meta, "h.go:10:3", []*metadata.TypeRef{tsPtr(tsNamed("NotFoundError"))}, "x") + wh200 := tsArmWrite(meta, "h.go:13:3", []*metadata.TypeRef{tsPtr(tsNamed("SuccessBody"))}, "x") + whDef := tsArmWrite(meta, "h.go:16:3", nil, "x") + + root := &TrackerNode{key: "route", Children: []*TrackerNode{ + {key: "respond", CallGraphEdge: &host, Children: []*TrackerNode{ + {key: "wh404", CallGraphEdge: &wh404}, + // Nest the 200 + default writes under an inner encoder node to prove the + // recursive arm collection reaches non-direct children. + {key: "enc", CallGraphEdge: &wh200, Children: []*TrackerNode{ + {key: "whdef", CallGraphEdge: &whDef}, + }}, + }}, + }} + return ext, root, wh404, wh200, whDef +} + +func TestHelperTypeSwitchEdges_PreciseMatch(t *testing.T) { + // x is *NotFoundError → only the 404 arm survives; the 200 arm and default arm + // are filtered, no warning. + ext, root, wh404, wh200, whDef := tsHostTree(t, tsPtr(tsNamed("NotFoundError"))) + var buf bytes.Buffer + ext.warnings = &WarningSink{out: &buf} + + filtered := ext.helperTypeSwitchEdges(root) + assert.False(t, filtered[wh404.Callee.ID()], "matched 404 arm kept") + assert.True(t, filtered[wh200.Callee.ID()], "unmatched 200 arm filtered") + assert.True(t, filtered[whDef.Callee.ID()], "default arm filtered on a precise match") + assert.Empty(t, buf.String(), "no warning on a precise match") +} + +func TestHelperTypeSwitchEdges_ImpreciseDegrades(t *testing.T) { + // x is an interface → degrade: both typed arms filtered, default kept, warn. + ext, root, wh404, wh200, whDef := tsHostTree(t, &metadata.TypeRef{Kind: metadata.RefInterface}) + var buf bytes.Buffer + ext.warnings = &WarningSink{out: &buf} + + filtered := ext.helperTypeSwitchEdges(root) + assert.True(t, filtered[wh404.Callee.ID()], "typed 404 arm filtered") + assert.True(t, filtered[wh200.Callee.ID()], "typed 200 arm filtered") + assert.False(t, filtered[whDef.Callee.ID()], "default arm kept") + assert.Contains(t, buf.String(), "type-switch", "imprecise binding warns") +} + +func TestHelperTypeSwitchEdges_ConcreteNoMatchHitsDefault(t *testing.T) { + // x is a concrete type matching no arm → it hits the default: typed arms + // filtered, default kept, but NO warning (the binding is precise). + ext, root, wh404, wh200, whDef := tsHostTree(t, tsPtr(tsNamed("OtherType"))) + var buf bytes.Buffer + ext.warnings = &WarningSink{out: &buf} + + filtered := ext.helperTypeSwitchEdges(root) + assert.True(t, filtered[wh404.Callee.ID()]) + assert.True(t, filtered[wh200.Callee.ID()]) + assert.False(t, filtered[whDef.Callee.ID()], "default arm kept") + assert.Empty(t, buf.String(), "a concrete type that hits the default does not warn") +} + +func TestHelperTypeSwitchEdges_NonHostInnerNodeSkipped(t *testing.T) { + // The arm writes sit under an inner node whose callee does NOT have the switched + // operand as a parameter — it must not filter anything. + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + + inner := makeHelperEdge(meta, "NewEncoder", "json", map[string]metadata.CallArgument{ + "w": *makeIdentArg(meta, "w", "http.ResponseWriter"), + }) + wh404 := tsArmWrite(meta, "h.go:10:3", []*metadata.TypeRef{tsPtr(tsNamed("NotFoundError"))}, "x") + root := &TrackerNode{key: "route", Children: []*TrackerNode{ + {key: "enc", CallGraphEdge: &inner, Children: []*TrackerNode{ + {key: "wh404", CallGraphEdge: &wh404}, + }}, + }} + assert.Empty(t, ext.helperTypeSwitchEdges(root), "inner non-host node filters nothing") +} + +func TestHelperTypeSwitchEdges_ArmScopedToHostFunction(t *testing.T) { + // An arm write whose Caller is a DIFFERENT function (a nested helper's switch + // that happens to share the operand name "x") must NOT be bound against this + // host's call-site argument — it is left to its own binding pass. + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + host := makeHelperEdge(meta, "Respond", "app", map[string]metadata.CallArgument{ + "x": *makeIdentArg(meta, "x", "any"), + }) + foreign := tsArmWrite(meta, "h.go:9:3", []*metadata.TypeRef{tsPtr(tsNamed("NotFoundError"))}, "x") + foreign.Caller = metadata.Call{Meta: meta, Name: meta.StringPool.Get("Nested"), Pkg: meta.StringPool.Get("app")} + root := &TrackerNode{key: "route", Children: []*TrackerNode{ + {key: "respond", CallGraphEdge: &host, Children: []*TrackerNode{ + {key: "foreign", CallGraphEdge: &foreign}, + }}, + }} + assert.Empty(t, ext.helperTypeSwitchEdges(root), "a nested helper's arm is not bound by the outer host") +} + +func TestHelperTypeSwitchEdges_NestedArmBailsKeepsAll(t *testing.T) { + // The host helper has a response write under an if-then branch — a type-switch + // arm with nested control flow whose write go/cfg attributes to the inner branch, + // not the arm. The binder cannot attribute every write to its arm, so it must + // keep ALL arms (filter nothing) — a safe over-approximation, never a mis-bind. + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + xarg := metadata.NewCallArgument(meta) + xarg.SetKind(metadata.KindIdent) + xarg.SetName("x") + xarg.TypeRef = tsPtr(tsNamed("NotFoundError")) // concrete: would normally bind the 404 arm + host := makeHelperEdge(meta, "Respond", "app", map[string]metadata.CallArgument{"x": *xarg}) + + wh404 := tsArmWrite(meta, "h.go:10:3", []*metadata.TypeRef{tsPtr(tsNamed("NotFoundError"))}, "x") + wh200 := tsArmWrite(meta, "h.go:13:3", []*metadata.TypeRef{tsPtr(tsNamed("SuccessBody"))}, "x") + whNested := tsArmWrite(meta, "h.go:16:3", nil, "") + whNested.Branch = &metadata.BranchContext{BlockKind: "if-then"} // escaped: nested control flow + root := &TrackerNode{key: "route", Children: []*TrackerNode{ + {key: "respond", CallGraphEdge: &host, Children: []*TrackerNode{ + {key: "wh404", CallGraphEdge: &wh404}, + {key: "wh200", CallGraphEdge: &wh200}, + {key: "whnested", CallGraphEdge: &whNested}, + }}, + }} + // Without the bail, the precise *NotFoundError match would filter the 200 arm; + // with it, nothing is filtered. + assert.Empty(t, ext.helperTypeSwitchEdges(root), "nested-arm control flow → keep all arms, filter nothing") +} + +func TestHelperTypeSwitchEdges_MultiCallSiteKeepsBoth(t *testing.T) { + // One route invokes the same type-switch helper from two sites with different + // concrete args. The arm writes share Callee IDs (same helper-internal call + // position), so the per-site filter decisions must INTERSECT: each site's matched + // arm survives; only the arm no site keeps (the default) is filtered. Without the + // kept-set, the union would erase every arm and the operation would lose all + // responses. + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + + site := func(argRef *metadata.TypeRef) *TrackerNode { + xarg := metadata.NewCallArgument(meta) + xarg.SetKind(metadata.KindIdent) + xarg.SetName("x") + xarg.TypeRef = argRef + host := makeHelperEdge(meta, "Respond", "app", map[string]metadata.CallArgument{"x": *xarg}) + // SHARED positions across both sites → colliding Callee IDs (one helper body). + wh404 := tsArmWrite(meta, "h.go:10:3", []*metadata.TypeRef{tsPtr(tsNamed("NotFoundError"))}, "x") + wh200 := tsArmWrite(meta, "h.go:13:3", []*metadata.TypeRef{tsPtr(tsNamed("SuccessBody"))}, "x") + whDef := tsArmWrite(meta, "h.go:16:3", nil, "x") + return &TrackerNode{key: "respond", CallGraphEdge: &host, Children: []*TrackerNode{ + {key: "wh404", CallGraphEdge: &wh404}, + {key: "wh200", CallGraphEdge: &wh200}, + {key: "whdef", CallGraphEdge: &whDef}, + }} + } + siteA := site(tsPtr(tsNamed("NotFoundError"))) + siteB := site(tsPtr(tsNamed("SuccessBody"))) + root := &TrackerNode{key: "route", Children: []*TrackerNode{siteA, siteB}} + + filtered := ext.helperTypeSwitchEdges(root) + wh404ID := siteA.Children[0].GetEdge().Callee.ID() + wh200ID := siteA.Children[1].GetEdge().Callee.ID() + whDefID := siteA.Children[2].GetEdge().Callee.ID() + assert.False(t, filtered[wh404ID], "404 kept by site A → must survive") + assert.False(t, filtered[wh200ID], "200 kept by site B → must survive") + assert.True(t, filtered[whDefID], "default kept by no site → filtered") +} + +func TestHelperTypeSwitchEdges_NilRoot(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + assert.Empty(t, ext.helperTypeSwitchEdges(nil)) +} + +func TestRefMatchesAnyCase(t *testing.T) { + nf := tsNamed("NotFoundError") + ptrNF := tsPtr(nf) + // Same shape binds: *T arg ↔ case *T, T arg ↔ case T. + assert.True(t, refMatchesAnyCase([]*metadata.TypeRef{ptrNF}, ptrNF)) + assert.True(t, refMatchesAnyCase([]*metadata.TypeRef{nf}, nf)) + // Pointer-ness MUST match (Go type-switch semantics): *T arg does not bind + // case T, and T arg does not bind case *T — no over-approximation (FR-012). + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{nf}, ptrNF), "*T arg must not bind case T") + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{ptrNF}, nf), "T arg must not bind case *T") + // Among several cases, the shape-matching one binds. + assert.True(t, refMatchesAnyCase([]*metadata.TypeRef{tsNamed("Other"), ptrNF}, ptrNF)) + // Negatives: wrong name, nil list, same name different package, non-named arg. + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{tsNamed("Other")}, nf)) + assert.False(t, refMatchesAnyCase(nil, nf)) + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{{Kind: metadata.RefNamed, Name: "NotFoundError", Pkg: "other"}}, nf)) + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{nf}, &metadata.TypeRef{Kind: metadata.RefInterface})) + + // Slice/array/map case types bind exactly (regression for the pointer-only gap): + // a []T arg matches `case []T` but not `case T` or `case []U`. + sliceItem := &metadata.TypeRef{Kind: metadata.RefSlice, Elem: tsNamed("Item")} + assert.True(t, refMatchesAnyCase([]*metadata.TypeRef{sliceItem}, sliceItem)) + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{tsNamed("Item")}, sliceItem), "[]T arg must not bind case T") + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{{Kind: metadata.RefSlice, Elem: tsNamed("Other")}}, sliceItem)) + + // Generic instantiations are distinguished by their type Args: Box[User] binds + // case Box[User] but NOT case Box[Order]. + boxUser := &metadata.TypeRef{Kind: metadata.RefNamed, Name: "Box", Pkg: "app", Args: []*metadata.TypeRef{tsNamed("User")}} + boxOrder := &metadata.TypeRef{Kind: metadata.RefNamed, Name: "Box", Pkg: "app", Args: []*metadata.TypeRef{tsNamed("Order")}} + assert.True(t, refMatchesAnyCase([]*metadata.TypeRef{boxUser}, boxUser)) + assert.False(t, refMatchesAnyCase([]*metadata.TypeRef{boxOrder}, boxUser), "Box[User] must not bind case Box[Order]") +} + +func TestTypeRefShapeEqual(t *testing.T) { + nf := tsNamed("NotFoundError") + assert.True(t, typeRefShapeEqual(nil, nil)) + assert.False(t, typeRefShapeEqual(nf, nil)) + assert.False(t, typeRefShapeEqual(nil, nf)) + assert.True(t, typeRefShapeEqual(tsPtr(nf), tsPtr(tsNamed("NotFoundError")))) + assert.False(t, typeRefShapeEqual(tsPtr(nf), nf)) // *T != T + // map[string]Item vs map[string]Other differ by value; vs map[int]Item by key. + mSI := &metadata.TypeRef{Kind: metadata.RefMap, Key: tsNamed("string"), Elem: tsNamed("Item")} + assert.True(t, typeRefShapeEqual(mSI, &metadata.TypeRef{Kind: metadata.RefMap, Key: tsNamed("string"), Elem: tsNamed("Item")})) + assert.False(t, typeRefShapeEqual(mSI, &metadata.TypeRef{Kind: metadata.RefMap, Key: tsNamed("string"), Elem: tsNamed("Other")})) + assert.False(t, typeRefShapeEqual(mSI, &metadata.TypeRef{Kind: metadata.RefMap, Key: tsNamed("int"), Elem: tsNamed("Item")})) + // arrays differ by length. + a3 := &metadata.TypeRef{Kind: metadata.RefArray, Len: 3, Elem: tsNamed("Item")} + assert.False(t, typeRefShapeEqual(a3, &metadata.TypeRef{Kind: metadata.RefArray, Len: 4, Elem: tsNamed("Item")})) +} + +func TestIsImpreciseLeaf(t *testing.T) { + assert.True(t, isImpreciseLeaf(nil)) + assert.True(t, isImpreciseLeaf(&metadata.TypeRef{Kind: metadata.RefInterface})) + for _, n := range []string{"", "error", "any", "interface{}"} { + assert.True(t, isImpreciseLeaf(&metadata.TypeRef{Kind: metadata.RefNamed, Name: n}), n) + } + assert.False(t, isImpreciseLeaf(tsNamed("NotFoundError"))) +} + +func TestBindHelperTypeSwitch_Guards(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + ae := armEdges{filtered: map[string]bool{}, kept: map[string]bool{}} + + // No edge → no-op. + ext.bindHelperTypeSwitch(&TrackerNode{key: "x"}, ae) + assert.Empty(t, ae.filtered) + assert.Empty(t, ae.kept) + + // Edge with no ParamArgMap → no-op. + noParams := makeHelperEdge(meta, "Respond", "app", nil) + ext.bindHelperTypeSwitch(&TrackerNode{key: "r", CallGraphEdge: &noParams}, ae) + assert.Empty(t, ae.filtered) +} + +func TestBindHelperTypeSwitch_DefaultOnlyArmIgnored(t *testing.T) { + // A switch with only a default arm (no typed cases) has nothing to bind. + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + host := makeHelperEdge(meta, "Respond", "app", map[string]metadata.CallArgument{ + "x": *makeIdentArg(meta, "x", "any"), + }) + whDef := tsArmWrite(meta, "h.go:9:3", nil, "x") + root := &TrackerNode{key: "route", Children: []*TrackerNode{ + {key: "respond", CallGraphEdge: &host, Children: []*TrackerNode{ + {key: "whdef", CallGraphEdge: &whDef}, + }}, + }} + assert.Empty(t, ext.helperTypeSwitchEdges(root)) +} + +func TestClassifyHelperWrites_Partition(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + + uncond := tsArmWrite(meta, "h.go:1:3", nil, "") + uncond.Branch = nil // unconditional response write + // An if-then write is a #27 defensive fallback (conditional). A switch-case arm + // is NOT a #27 fallback — it is owned by the type-switch binding — so it is + // excluded from both buckets here. + ifThen := tsArmWrite(meta, "h.go:2:3", nil, "") + ifThen.Branch = &metadata.BranchContext{BlockKind: "if-then"} + switchArm := tsArmWrite(meta, "h.go:3:3", []*metadata.TypeRef{tsPtr(tsNamed("T"))}, "x") + nonResp := makeHelperEdge(meta, "doWork", "app", nil) // not a response write + + // The host is the Respond helper; tsArmWrite tags each write's Caller as + // Respond/app, which classifyHelperWrites scopes to via sameFunc. + hostEdge := makeHelperEdge(meta, "Respond", "app", map[string]metadata.CallArgument{"x": {Meta: meta}}) + host := &TrackerNode{key: "h", CallGraphEdge: &hostEdge, Children: []*TrackerNode{ + {key: "nil"}, // nil edge → skipped + {key: "uncond", CallGraphEdge: &uncond}, + {key: "ifthen", CallGraphEdge: &ifThen}, + {key: "switcharm", CallGraphEdge: &switchArm}, + {key: "nonresp", CallGraphEdge: &nonResp}, + }} + hw := ext.classifyHelperWrites(host) + assert.Len(t, hw.unconditional, 1) + assert.Len(t, hw.conditional, 1, "only the if-then write is a #27 fallback; the switch-case arm is excluded") + assert.True(t, hw.hasUnconditional()) +} + +func TestBoundArgRef(t *testing.T) { + meta := newTestMeta() + ext, _ := newTestExtractor(meta) + node := buildTrackerNode(buildCallGraphEdge(meta, "h", "app", "Respond", "app", nil)) + + // Structured TypeRef present → precise, FULL ref returned (pointer preserved). + arg := metadata.NewCallArgument(meta) + arg.SetKind(metadata.KindIdent) + arg.TypeRef = tsPtr(tsNamed("NotFoundError")) + ref, precise := ext.boundArgRef(arg, node) + assert.True(t, precise) + if assert.NotNil(t, ref) { + assert.Equal(t, metadata.RefPointer, ref.Kind) + assert.Equal(t, "NotFoundError", ref.NamedLeaf().Name) + } + + // Interface TypeRef → imprecise (nil ref). + argI := metadata.NewCallArgument(meta) + argI.SetKind(metadata.KindIdent) + argI.TypeRef = &metadata.TypeRef{Kind: metadata.RefInterface} + ref, precise = ext.boundArgRef(argI, node) + assert.False(t, precise) + assert.Nil(t, ref) + + // No structured TypeRef → fall back to the string-based origin (ResolvedType). + argS := metadata.NewCallArgument(meta) + argS.SetKind(metadata.KindIdent) + argS.SetResolvedType("*app.NotFoundError") + argS.TypeRef = nil + argS.ResolvedTypeRef = nil + ref, precise = ext.boundArgRef(argS, node) + assert.True(t, precise) + if assert.NotNil(t, ref) { + assert.Equal(t, "NotFoundError", ref.NamedLeaf().Name) + } +} diff --git a/internal/spec/warnings.go b/internal/spec/warnings.go new file mode 100644 index 0000000..68b83a8 --- /dev/null +++ b/internal/spec/warnings.go @@ -0,0 +1,64 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 spec + +import ( + "fmt" + "io" + "os" + "sync" +) + +// WarningSink collects non-fatal analysis warnings — e.g. an unresolved +// conditional position or an imprecise helper binding (FR-008/FR-012, spec 009) — +// and flushes them to stderr as `warning: : `. Per Constitution +// Principle IV diagnostics go to stderr and never affect the exit code or the +// machine-parseable stdout spec. A nil *WarningSink is a no-op, so callers need +// not nil-check. +type WarningSink struct { + mu sync.Mutex + out io.Writer + warnings []string +} + +// NewWarningSink returns a sink that writes warnings to stderr. +func NewWarningSink() *WarningSink { + return &WarningSink{out: os.Stderr} +} + +// Warn records a warning for the given source position and writes it to the sink's +// output. Safe to call on a nil sink (no-op). +func (s *WarningSink) Warn(pos, message string) { + if s == nil { + return + } + line := fmt.Sprintf("warning: %s: %s", pos, message) + s.mu.Lock() + defer s.mu.Unlock() + s.warnings = append(s.warnings, line) + if s.out != nil { + _, _ = fmt.Fprintln(s.out, line) + } +} + +// Warnings returns a copy of the collected warning lines. Safe on a nil sink. +func (s *WarningSink) Warnings() []string { + if s == nil { + return nil + } + s.mu.Lock() + defer s.mu.Unlock() + return append([]string(nil), s.warnings...) +} diff --git a/internal/spec/warnings_test.go b/internal/spec/warnings_test.go new file mode 100644 index 0000000..b72c908 --- /dev/null +++ b/internal/spec/warnings_test.go @@ -0,0 +1,46 @@ +// Copyright 2025 Ehab Terra, 2025-2026 Anton Starikov +// +// 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 spec + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestWarningSink(t *testing.T) { + var buf bytes.Buffer + s := &WarningSink{out: &buf} + s.Warn("f.go:10:2", "imprecise binding") + s.Warn("f.go:20:4", "unresolved status") + + assert.Equal(t, []string{ + "warning: f.go:10:2: imprecise binding", + "warning: f.go:20:4: unresolved status", + }, s.Warnings()) + assert.Contains(t, buf.String(), "warning: f.go:10:2: imprecise binding") + assert.Contains(t, buf.String(), "warning: f.go:20:4: unresolved status") +} + +func TestWarningSink_NilSafe(t *testing.T) { + var nilSink *WarningSink + nilSink.Warn("x", "y") // must not panic + assert.Nil(t, nilSink.Warnings()) +} + +func TestNewWarningSink(t *testing.T) { + assert.NotNil(t, NewWarningSink()) +} diff --git a/testdata/cfg_branch_bodies/expected_openapi.json b/testdata/cfg_branch_bodies/expected_openapi.json new file mode 100644 index 0000000..b96964b --- /dev/null +++ b/testdata/cfg_branch_bodies/expected_openapi.json @@ -0,0 +1,80 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/user": { + "get": { + "operationId": "cfg_branch_bodies.getUser", + "parameters": [ + { + "name": "id", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_branch_bodies.FullUser" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_branch_bodies.ErrorBody" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_branch_bodies.ErrorBody": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "cfg_branch_bodies.FullUser": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "name": { + "type": "string" + } + }, + "required": [ + "id", + "name" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_branch_bodies/expected_openapi_legacy.json b/testdata/cfg_branch_bodies/expected_openapi_legacy.json new file mode 100644 index 0000000..cdda941 --- /dev/null +++ b/testdata/cfg_branch_bodies/expected_openapi_legacy.json @@ -0,0 +1,80 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/user": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_branch_bodies.getUser", + "parameters": [ + { + "name": "id", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_branch_bodies.FullUser" + } + } + } + }, + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_branch_bodies.ErrorBody" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_branch_bodies.ErrorBody": { + "type": "object", + "properties": { + "message": { + "type": "string" + } + }, + "required": [ + "message" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_branch_bodies.FullUser": { + "type": "object", + "properties": { + "id": { + "type": "integer" + }, + "name": { + "type": "string" + } + }, + "required": [ + "id", + "name" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_branch_bodies/go.mod b/testdata/cfg_branch_bodies/go.mod new file mode 100644 index 0000000..1ddaeda --- /dev/null +++ b/testdata/cfg_branch_bodies/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_branch_bodies + +go 1.24.3 diff --git a/testdata/cfg_branch_bodies/main.go b/testdata/cfg_branch_bodies/main.go new file mode 100644 index 0000000..13c7c87 --- /dev/null +++ b/testdata/cfg_branch_bodies/main.go @@ -0,0 +1,37 @@ +// Package main is the spec-009 fixture for branch-dependent response bodies +// (US3, FR-005): a handler that writes FullUser with 200 on one branch and +// ErrorBody with 404 on another. Each status must carry the body actually +// written on its path, not a single shape merged across branches. +package main + +import ( + "encoding/json" + "net/http" +) + +// FullUser is the 200 body. +type FullUser struct { + ID int `json:"id"` + Name string `json:"name"` +} + +// ErrorBody is the 404 body. +type ErrorBody struct { + Message string `json:"message"` +} + +func getUser(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("id") == "" { + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(ErrorBody{Message: "missing id"}) + } else { + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(FullUser{ID: 1, Name: "ada"}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("GET /user", getUser) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_helper_typeswitch/expected_openapi.json b/testdata/cfg_helper_typeswitch/expected_openapi.json new file mode 100644 index 0000000..13dd444 --- /dev/null +++ b/testdata/cfg_helper_typeswitch/expected_openapi.json @@ -0,0 +1,76 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/concrete": { + "get": { + "summary": "handleConcrete passes a statically-known *NotFoundError → only 404.", + "operationId": "cfg_helper_typeswitch.handleConcrete", + "responses": { + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_helper_typeswitch.NotFoundError" + } + } + } + } + } + } + }, + "/imprecise": { + "get": { + "summary": "handleImprecise passes a bare error (as any) whose concrete type is unknown\nhere → degrade to the default arm (500) + warn.", + "operationId": "cfg_helper_typeswitch.handleImprecise", + "responses": { + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_helper_typeswitch.DefaultError" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_helper_typeswitch.DefaultError": { + "type": "object", + "properties": { + "code": { + "type": "integer" + } + }, + "required": [ + "code" + ] + }, + "cfg_helper_typeswitch.NotFoundError": { + "type": "object", + "properties": { + "resource": { + "type": "string" + } + }, + "required": [ + "resource" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_helper_typeswitch/expected_openapi_legacy.json b/testdata/cfg_helper_typeswitch/expected_openapi_legacy.json new file mode 100644 index 0000000..a96b2e9 --- /dev/null +++ b/testdata/cfg_helper_typeswitch/expected_openapi_legacy.json @@ -0,0 +1,76 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/concrete": { + "get": { + "summary": "handleConcrete passes a statically-known *NotFoundError → only 404.", + "operationId": "github.com/antst/go-apispec/testdata/cfg_helper_typeswitch.handleConcrete", + "responses": { + "404": { + "description": "Not Found", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_helper_typeswitch.NotFoundError" + } + } + } + } + } + } + }, + "/imprecise": { + "get": { + "summary": "handleImprecise passes a bare error (as any) whose concrete type is unknown\nhere → degrade to the default arm (500) + warn.", + "operationId": "github.com/antst/go-apispec/testdata/cfg_helper_typeswitch.handleImprecise", + "responses": { + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_helper_typeswitch.DefaultError" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_helper_typeswitch.DefaultError": { + "type": "object", + "properties": { + "code": { + "type": "integer" + } + }, + "required": [ + "code" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_helper_typeswitch.NotFoundError": { + "type": "object", + "properties": { + "resource": { + "type": "string" + } + }, + "required": [ + "resource" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_helper_typeswitch/go.mod b/testdata/cfg_helper_typeswitch/go.mod new file mode 100644 index 0000000..66f0eea --- /dev/null +++ b/testdata/cfg_helper_typeswitch/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_helper_typeswitch + +go 1.24.3 diff --git a/testdata/cfg_helper_typeswitch/main.go b/testdata/cfg_helper_typeswitch/main.go new file mode 100644 index 0000000..5d951c7 --- /dev/null +++ b/testdata/cfg_helper_typeswitch/main.go @@ -0,0 +1,71 @@ +// Package main is the spec-009 fixture for helper-internal type-switch binding +// (FR-011/FR-012). A shared Respond(w, x) helper type-switches on the concrete +// type of x to choose status + body. The analyzer must bind the call-site +// argument to the matching arm: +// +// - GET /concrete passes a statically-known *NotFoundError, so ONLY the +// `case *NotFoundError` arm (404) fans out — not the *SuccessBody (200) or +// default (500) arms. +// - GET /imprecise passes a value whose concrete type is not statically known +// (an `any` holding an error), so the binding is imprecise: the analyzer +// degrades to the unconditionally-reachable default arm (500) and warns, +// rather than over-approximating with every arm. +package main + +import ( + "encoding/json" + "net/http" +) + +// NotFoundError is the 404 payload. +type NotFoundError struct { + Resource string `json:"resource"` +} + +// SuccessBody is the 200 payload. +type SuccessBody struct { + Message string `json:"message"` +} + +// DefaultError is the fallback (default arm) payload. +type DefaultError struct { + Code int `json:"code"` +} + +// Respond chooses the response status and body from the concrete type of x. +func Respond(w http.ResponseWriter, x any) { + switch v := x.(type) { + case *NotFoundError: + w.WriteHeader(http.StatusNotFound) + _ = json.NewEncoder(w).Encode(v) + case *SuccessBody: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(v) + default: + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(&DefaultError{Code: 500}) + } +} + +// process stands in for work that yields an error of statically-unknown concrete +// type at the call site. +func process(r *http.Request) error { return nil } + +// handleConcrete passes a statically-known *NotFoundError → only 404. +func handleConcrete(w http.ResponseWriter, r *http.Request) { + Respond(w, &NotFoundError{Resource: "user"}) +} + +// handleImprecise passes a bare error (as any) whose concrete type is unknown +// here → degrade to the default arm (500) + warn. +func handleImprecise(w http.ResponseWriter, r *http.Request) { + var x any = process(r) + Respond(w, x) +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("GET /concrete", handleConcrete) + mux.HandleFunc("GET /imprecise", handleImprecise) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_loop_status/expected_openapi.json b/testdata/cfg_loop_status/expected_openapi.json new file mode 100644 index 0000000..a5f09c2 --- /dev/null +++ b/testdata/cfg_loop_status/expected_openapi.json @@ -0,0 +1,36 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/loop": { + "get": { + "summary": "loop sets code = 200 before the loop and code = 202 inside the loop body.", + "description": "loop sets code = 200 before the loop and code = 202 inside the loop body. The\nloop-body assignment does not dominate the WriteHeader (the loop may run zero\ntimes), so it does not shadow the default — the response lists BOTH 200 and 202.", + "operationId": "cfg_loop_status.loop", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": {} + } + }, + "202": { + "description": "Accepted", + "content": { + "application/json": {} + } + } + } + } + } + }, + "components": {} +} \ No newline at end of file diff --git a/testdata/cfg_loop_status/expected_openapi_legacy.json b/testdata/cfg_loop_status/expected_openapi_legacy.json new file mode 100644 index 0000000..9f30275 --- /dev/null +++ b/testdata/cfg_loop_status/expected_openapi_legacy.json @@ -0,0 +1,36 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/loop": { + "get": { + "summary": "loop sets code = 200 before the loop and code = 202 inside the loop body.", + "description": "loop sets code = 200 before the loop and code = 202 inside the loop body. The\nloop-body assignment does not dominate the WriteHeader (the loop may run zero\ntimes), so it does not shadow the default — the response lists BOTH 200 and 202.", + "operationId": "github.com/antst/go-apispec/testdata/cfg_loop_status.loop", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": {} + } + }, + "202": { + "description": "Accepted", + "content": { + "application/json": {} + } + } + } + } + } + }, + "components": {} +} \ No newline at end of file diff --git a/testdata/cfg_loop_status/go.mod b/testdata/cfg_loop_status/go.mod new file mode 100644 index 0000000..8ba3cb1 --- /dev/null +++ b/testdata/cfg_loop_status/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_loop_status + +go 1.24.3 diff --git a/testdata/cfg_loop_status/main.go b/testdata/cfg_loop_status/main.go new file mode 100644 index 0000000..81358e1 --- /dev/null +++ b/testdata/cfg_loop_status/main.go @@ -0,0 +1,32 @@ +// Package main is the spec-009 fixture for loop-body status assignments +// (Edge Cases "Loops", FR-010). A status variable is given an unconditional +// default before a loop and conditionally reassigned inside the loop body; both +// values reach the single WriteHeader after the loop, so the CFG reachability +// model must (a) terminate across the loop back-edge and (b) report the +// loop-body assignment as reaching the response write. +package main + +import "net/http" + +// mapStatus wraps a status code in a call so each assignment's RHS is a call the +// status-expansion path inspects (mirrors conditional_status_reachability). +func mapStatus(s int) int { return s } + +// loop sets code = 200 before the loop and code = 202 inside the loop body. The +// loop-body assignment does not dominate the WriteHeader (the loop may run zero +// times), so it does not shadow the default — the response lists BOTH 200 and 202. +func loop(w http.ResponseWriter, r *http.Request) { + code := mapStatus(http.StatusOK) + for _, v := range r.Header["X-Items"] { + if v == "flag" { + code = mapStatus(http.StatusAccepted) + } + } + w.WriteHeader(code) +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("GET /loop", loop) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_combined_default/expected_openapi.json b/testdata/cfg_method_combined_default/expected_openapi.json new file mode 100644 index 0000000..942ffde --- /dev/null +++ b/testdata/cfg_method_combined_default/expected_openapi.json @@ -0,0 +1,61 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_combined_default.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_combined_default.Item" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_combined_default.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_combined_default.Item" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_combined_default.Item": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_combined_default/expected_openapi_legacy.json b/testdata/cfg_method_combined_default/expected_openapi_legacy.json new file mode 100644 index 0000000..ef99a98 --- /dev/null +++ b/testdata/cfg_method_combined_default/expected_openapi_legacy.json @@ -0,0 +1,61 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_combined_default.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_combined_default.Item" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_combined_default.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_combined_default.Item" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_combined_default.Item": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_combined_default/go.mod b/testdata/cfg_method_combined_default/go.mod new file mode 100644 index 0000000..5d19355 --- /dev/null +++ b/testdata/cfg_method_combined_default/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_combined_default + +go 1.24.3 diff --git a/testdata/cfg_method_combined_default/main.go b/testdata/cfg_method_combined_default/main.go new file mode 100644 index 0000000..53c0d05 --- /dev/null +++ b/testdata/cfg_method_combined_default/main.go @@ -0,0 +1,34 @@ +// Package main is the spec-009 guard for a `switch r.Method` whose responding arm is a +// COMBINED case (`case http.MethodGet, http.MethodPost:`) followed by a `default`. +// go/cfg lowers a combined case to ONE body block, so the dominator of that single block +// is the arm itself — not the switch tag. The dispatch root must therefore come from the +// recorded dispatch GROUP (every arm of the switch INCLUDING the default), not from the +// lone combined arm: only then is the default's 405 recognised as the dispatch fallback +// and NOT leaked onto the GET and POST operations the combined case fans out into. +package main + +import ( + "encoding/json" + "net/http" +) + +// Item is the body returned for both GET and POST by the shared combined case. +type Item struct { + ID int `json:"id"` +} + +func items(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodGet, http.MethodPost: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(Item{ID: 1}) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_helper_response/expected_openapi.json b/testdata/cfg_method_helper_response/expected_openapi.json new file mode 100644 index 0000000..cd44bd2 --- /dev/null +++ b/testdata/cfg_method_helper_response/expected_openapi.json @@ -0,0 +1,104 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/things": { + "get": { + "operationId": "testdata.cfg_method_helper_response.handler", + "parameters": [ + { + "name": "X-Trace", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_helper_response.List" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_helper_response.handler", + "parameters": [ + { + "name": "X-Trace", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_helper_response.Created" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_helper_response.Created": { + "type": "object", + "properties": { + "ok": { + "type": "boolean" + } + }, + "required": [ + "ok" + ] + }, + "cfg_method_helper_response.List": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + }, + "cfg_method_helper_response.Trace": { + "type": "object", + "properties": { + "t": { + "type": "string" + } + }, + "required": [ + "t" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_helper_response/expected_openapi_legacy.json b/testdata/cfg_method_helper_response/expected_openapi_legacy.json new file mode 100644 index 0000000..68dc2a9 --- /dev/null +++ b/testdata/cfg_method_helper_response/expected_openapi_legacy.json @@ -0,0 +1,104 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/things": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_helper_response.handler", + "parameters": [ + { + "name": "X-Trace", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_helper_response.List" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_helper_response.handler", + "parameters": [ + { + "name": "X-Trace", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_helper_response.Created" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_helper_response.Created": { + "type": "object", + "properties": { + "ok": { + "type": "boolean" + } + }, + "required": [ + "ok" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_helper_response.List": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_helper_response.Trace": { + "type": "object", + "properties": { + "t": { + "type": "string" + } + }, + "required": [ + "t" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_helper_response/go.mod b/testdata/cfg_method_helper_response/go.mod new file mode 100644 index 0000000..c5df728 --- /dev/null +++ b/testdata/cfg_method_helper_response/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_helper_response + +go 1.24.3 diff --git a/testdata/cfg_method_helper_response/main.go b/testdata/cfg_method_helper_response/main.go new file mode 100644 index 0000000..efe18f0 --- /dev/null +++ b/testdata/cfg_method_helper_response/main.go @@ -0,0 +1,55 @@ +// Package main is the spec-009 guard for cross-function response attribution. The +// GET arm writes a direct 200 (so the handler splits per method) AND calls a helper +// that writes a CONDITIONAL response. That helper response's CFG branch belongs to +// the HELPER's function, not the handler's, so its block index is meaningless in the +// handler's CFG. The classifier must NOT reason about it against the handler's CFG +// (which would alias an unrelated block and leak the response onto POST); it is +// conservatively excluded. Correct per-method attribution of an interprocedural +// conditional needs call-site context — a separate, future concern. +package main + +import ( + "encoding/json" + "net/http" +) + +// List is the GET response body. +type List struct { + Items []string `json:"items"` +} + +// Trace is the helper's conditional response body (GET-only in source). +type Trace struct { + T string `json:"t"` +} + +// Created is the POST response body. +type Created struct { + OK bool `json:"ok"` +} + +// auditGet writes a CONDITIONAL response; its branch block index is local to auditGet. +func auditGet(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-Trace") != "" { + w.WriteHeader(599) + _ = json.NewEncoder(w).Encode(Trace{T: "x"}) + } +} + +func handler(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "GET": + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(List{Items: []string{"a"}}) + auditGet(w, r) // helper's conditional 599 must NOT leak onto POST + case "POST": + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(Created{OK: true}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/things", handler) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_if_dispatch/expected_openapi.json b/testdata/cfg_method_if_dispatch/expected_openapi.json new file mode 100644 index 0000000..00f3aec --- /dev/null +++ b/testdata/cfg_method_if_dispatch/expected_openapi.json @@ -0,0 +1,75 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_if_dispatch.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_dispatch.ItemList" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_if_dispatch.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_dispatch.CreatedItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_if_dispatch.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "cfg_method_if_dispatch.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_if_dispatch/expected_openapi_legacy.json b/testdata/cfg_method_if_dispatch/expected_openapi_legacy.json new file mode 100644 index 0000000..f42f778 --- /dev/null +++ b/testdata/cfg_method_if_dispatch/expected_openapi_legacy.json @@ -0,0 +1,75 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_if_dispatch.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_dispatch.ItemList" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_if_dispatch.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_dispatch.CreatedItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_if_dispatch.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_if_dispatch.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_if_dispatch/go.mod b/testdata/cfg_method_if_dispatch/go.mod new file mode 100644 index 0000000..c065654 --- /dev/null +++ b/testdata/cfg_method_if_dispatch/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_if_dispatch + +go 1.24.3 diff --git a/testdata/cfg_method_if_dispatch/main.go b/testdata/cfg_method_if_dispatch/main.go new file mode 100644 index 0000000..3d2cb9a --- /dev/null +++ b/testdata/cfg_method_if_dispatch/main.go @@ -0,0 +1,37 @@ +// Package main is the spec-009 fixture for method-conditional dispatch via +// `if r.Method == …` guards (US2, FR-003). The handler is registered without a +// method, so each guarded arm is reachable: the POST arm creates an item (201) +// and the GET arm lists items (200). The analyzer must split this into one +// operation per reachable method, exactly as it does for a `switch r.Method`. +package main + +import ( + "encoding/json" + "net/http" +) + +// CreatedItem is the POST response body. +type CreatedItem struct { + ID int `json:"id"` +} + +// ItemList is the GET response body. +type ItemList struct { + Items []string `json:"items"` +} + +func items(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost { + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(CreatedItem{ID: 1}) + } else if r.Method == http.MethodGet { + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(ItemList{Items: []string{"a"}}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_if_independent/expected_openapi.json b/testdata/cfg_method_if_independent/expected_openapi.json new file mode 100644 index 0000000..329a987 --- /dev/null +++ b/testdata/cfg_method_if_independent/expected_openapi.json @@ -0,0 +1,124 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_if_independent.items", + "parameters": [ + { + "name": "bad", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_independent.ItemList" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_independent.ErrBody" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_if_independent.items", + "parameters": [ + { + "name": "bad", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_independent.CreatedItem" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_independent.ErrBody" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_if_independent.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "cfg_method_if_independent.ErrBody": { + "type": "object", + "properties": { + "msg": { + "type": "string" + } + }, + "required": [ + "msg" + ] + }, + "cfg_method_if_independent.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_if_independent/expected_openapi_legacy.json b/testdata/cfg_method_if_independent/expected_openapi_legacy.json new file mode 100644 index 0000000..174eb3c --- /dev/null +++ b/testdata/cfg_method_if_independent/expected_openapi_legacy.json @@ -0,0 +1,124 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_if_independent.items", + "parameters": [ + { + "name": "bad", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_independent.ItemList" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_independent.ErrBody" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_if_independent.items", + "parameters": [ + { + "name": "bad", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_independent.CreatedItem" + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_independent.ErrBody" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_if_independent.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_if_independent.ErrBody": { + "type": "object", + "properties": { + "msg": { + "type": "string" + } + }, + "required": [ + "msg" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_if_independent.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_if_independent/go.mod b/testdata/cfg_method_if_independent/go.mod new file mode 100644 index 0000000..6405bd6 --- /dev/null +++ b/testdata/cfg_method_if_independent/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_if_independent + +go 1.24.3 diff --git a/testdata/cfg_method_if_independent/main.go b/testdata/cfg_method_if_independent/main.go new file mode 100644 index 0000000..362a265 --- /dev/null +++ b/testdata/cfg_method_if_independent/main.go @@ -0,0 +1,49 @@ +// Package main is the spec-009 fixture for response attribution ACROSS a method +// dispatch (US2). The handler combines an `if r.Method ==` dispatch (GET lists, +// POST creates) with an INDEPENDENT pre-dispatch guard (`if bad { 500 }`) that is +// reachable whatever the request method is. The analyzer must split into one +// operation per method AND carry the independent 500 onto BOTH operations — the +// CFG tells it the 500 is orthogonal to the dispatch, not a method-specific or +// "other-methods" response. (Before the CFG model, splitting dropped it entirely.) +package main + +import ( + "encoding/json" + "net/http" +) + +// ErrBody is the independent (method-independent) error response body. +type ErrBody struct { + Msg string `json:"msg"` +} + +// ItemList is the GET response body. +type ItemList struct { + Items []string `json:"items"` +} + +// CreatedItem is the POST response body. +type CreatedItem struct { + ID int `json:"id"` +} + +func items(w http.ResponseWriter, r *http.Request) { + // Independent of the method dispatch: a bad request fails on ANY method. + if r.URL.Query().Get("bad") != "" { + w.WriteHeader(http.StatusInternalServerError) + _ = json.NewEncoder(w).Encode(ErrBody{Msg: "invalid"}) + return + } + if r.Method == http.MethodGet { + _ = json.NewEncoder(w).Encode(ItemList{Items: []string{"a"}}) + } else if r.Method == http.MethodPost { + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(CreatedItem{ID: 1}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_if_switch_default/expected_openapi.json b/testdata/cfg_method_if_switch_default/expected_openapi.json new file mode 100644 index 0000000..3f98025 --- /dev/null +++ b/testdata/cfg_method_if_switch_default/expected_openapi.json @@ -0,0 +1,72 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_if_switch_default.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_switch_default.GetItem" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_if_switch_default.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_if_switch_default.PostItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_if_switch_default.GetItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "cfg_method_if_switch_default.PostItem": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_if_switch_default/expected_openapi_legacy.json b/testdata/cfg_method_if_switch_default/expected_openapi_legacy.json new file mode 100644 index 0000000..b771da9 --- /dev/null +++ b/testdata/cfg_method_if_switch_default/expected_openapi_legacy.json @@ -0,0 +1,72 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_if_switch_default.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_switch_default.GetItem" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_if_switch_default.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_if_switch_default.PostItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_if_switch_default.GetItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_if_switch_default.PostItem": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_if_switch_default/go.mod b/testdata/cfg_method_if_switch_default/go.mod new file mode 100644 index 0000000..bccebfb --- /dev/null +++ b/testdata/cfg_method_if_switch_default/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_if_switch_default + +go 1.24.3 diff --git a/testdata/cfg_method_if_switch_default/main.go b/testdata/cfg_method_if_switch_default/main.go new file mode 100644 index 0000000..792f604 --- /dev/null +++ b/testdata/cfg_method_if_switch_default/main.go @@ -0,0 +1,45 @@ +// Package main is the spec-009 guard for a method dispatch SPLIT ACROSS an +// `if r.Method ==` arm AND a `switch r.Method` with a `default`. The two dispatches are +// DISTINCT groups (the GET response is attributed to the if group, POST to the switch +// group). The dispatch root must be the common dominator of the arms of BOTH contributing +// groups — so the switch's `default` 405 is still dominated and excluded, not leaked onto +// the GET and POST operations. Scoping the root to a single "primary" group regressed this +// (the if group's lone arm does not dominate the switch default) — the union of contributing +// groups fixes it. +package main + +import ( + "encoding/json" + "net/http" +) + +// GetItem is the GET response body. +type GetItem struct { + ID int `json:"id"` +} + +// PostItem is the POST response body. +type PostItem struct { + Name string `json:"name"` +} + +func items(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodGet { + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(GetItem{ID: 1}) + return + } + switch r.Method { + case http.MethodPost: + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(PostItem{Name: "x"}) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_switch_copy/expected_openapi.json b/testdata/cfg_method_switch_copy/expected_openapi.json new file mode 100644 index 0000000..1c13898 --- /dev/null +++ b/testdata/cfg_method_switch_copy/expected_openapi.json @@ -0,0 +1,72 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_switch_copy.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_switch_copy.GetItem" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_switch_copy.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_switch_copy.PostItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_switch_copy.GetItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "cfg_method_switch_copy.PostItem": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_switch_copy/expected_openapi_legacy.json b/testdata/cfg_method_switch_copy/expected_openapi_legacy.json new file mode 100644 index 0000000..34ab2de --- /dev/null +++ b/testdata/cfg_method_switch_copy/expected_openapi_legacy.json @@ -0,0 +1,72 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_switch_copy.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_switch_copy.GetItem" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_switch_copy.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_switch_copy.PostItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_switch_copy.GetItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_switch_copy.PostItem": { + "type": "object", + "properties": { + "name": { + "type": "string" + } + }, + "required": [ + "name" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_switch_copy/go.mod b/testdata/cfg_method_switch_copy/go.mod new file mode 100644 index 0000000..c45a869 --- /dev/null +++ b/testdata/cfg_method_switch_copy/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_switch_copy + +go 1.24.3 diff --git a/testdata/cfg_method_switch_copy/main.go b/testdata/cfg_method_switch_copy/main.go new file mode 100644 index 0000000..8fca935 --- /dev/null +++ b/testdata/cfg_method_switch_copy/main.go @@ -0,0 +1,42 @@ +// Package main is the spec-009 guard for a `switch` over a COPY of r.Method +// (`m := r.Method; switch m { … default }`). The switch TAG is `m`, not the bare +// `r.Method` selector, so the dispatch is recognised by its method-named CASE VALUES — the +// same test the route splitter uses to attribute a case to a method — rather than by the +// tag. The `default` 405 is then a recognised dispatch fallback and excluded, not leaked +// onto the GET and POST operations. Tag-gated detection regressed this common idiom. +package main + +import ( + "encoding/json" + "net/http" +) + +// GetItem is the GET response body. +type GetItem struct { + ID int `json:"id"` +} + +// PostItem is the POST response body. +type PostItem struct { + Name string `json:"name"` +} + +func items(w http.ResponseWriter, r *http.Request) { + m := r.Method + switch m { + case http.MethodGet: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(GetItem{ID: 1}) + case http.MethodPost: + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(PostItem{Name: "x"}) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_switch_dispatch/expected_openapi.json b/testdata/cfg_method_switch_dispatch/expected_openapi.json new file mode 100644 index 0000000..3f8d9ee --- /dev/null +++ b/testdata/cfg_method_switch_dispatch/expected_openapi.json @@ -0,0 +1,75 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_switch_dispatch.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_switch_dispatch.ItemList" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_switch_dispatch.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_switch_dispatch.CreatedItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_switch_dispatch.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "cfg_method_switch_dispatch.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_switch_dispatch/expected_openapi_legacy.json b/testdata/cfg_method_switch_dispatch/expected_openapi_legacy.json new file mode 100644 index 0000000..002e183 --- /dev/null +++ b/testdata/cfg_method_switch_dispatch/expected_openapi_legacy.json @@ -0,0 +1,75 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_switch_dispatch.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_switch_dispatch.ItemList" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_switch_dispatch.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_switch_dispatch.CreatedItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_switch_dispatch.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_switch_dispatch.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_switch_dispatch/go.mod b/testdata/cfg_method_switch_dispatch/go.mod new file mode 100644 index 0000000..41a21cd --- /dev/null +++ b/testdata/cfg_method_switch_dispatch/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_switch_dispatch + +go 1.24.3 diff --git a/testdata/cfg_method_switch_dispatch/main.go b/testdata/cfg_method_switch_dispatch/main.go new file mode 100644 index 0000000..97b952a --- /dev/null +++ b/testdata/cfg_method_switch_dispatch/main.go @@ -0,0 +1,38 @@ +// Package main is the spec-009 fixture for method-conditional dispatch via a +// `switch r.Method` whose cases are net/http METHOD CONSTANTS (`case http.MethodGet:`). +// It mirrors cfg_method_if_dispatch (the `if r.Method ==` form): the analyzer must +// split into one operation per method identically — whether the dispatch is a switch +// or an if, and whether the cases are string literals or net/http constants (FR-003). +package main + +import ( + "encoding/json" + "net/http" +) + +// CreatedItem is the POST response body. +type CreatedItem struct { + ID int `json:"id"` +} + +// ItemList is the GET response body. +type ItemList struct { + Items []string `json:"items"` +} + +func items(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodPost: + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(CreatedItem{ID: 1}) + case http.MethodGet: + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(ItemList{Items: []string{"a"}}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_switch_fallthrough/expected_openapi.json b/testdata/cfg_method_switch_fallthrough/expected_openapi.json new file mode 100644 index 0000000..fdf3bdc --- /dev/null +++ b/testdata/cfg_method_switch_fallthrough/expected_openapi.json @@ -0,0 +1,86 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "testdata.cfg_method_switch_fallthrough.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_switch_fallthrough.ItemList" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_switch_fallthrough.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_switch_fallthrough.CreatedItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_switch_fallthrough.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "cfg_method_switch_fallthrough.ErrBody": { + "type": "object", + "properties": { + "msg": { + "type": "string" + } + }, + "required": [ + "msg" + ] + }, + "cfg_method_switch_fallthrough.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_switch_fallthrough/expected_openapi_legacy.json b/testdata/cfg_method_switch_fallthrough/expected_openapi_legacy.json new file mode 100644 index 0000000..01826ea --- /dev/null +++ b/testdata/cfg_method_switch_fallthrough/expected_openapi_legacy.json @@ -0,0 +1,86 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/items": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_switch_fallthrough.items", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_switch_fallthrough.ItemList" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_switch_fallthrough.items", + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_switch_fallthrough.CreatedItem" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_switch_fallthrough.CreatedItem": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } + }, + "required": [ + "id" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_switch_fallthrough.ErrBody": { + "type": "object", + "properties": { + "msg": { + "type": "string" + } + }, + "required": [ + "msg" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_switch_fallthrough.ItemList": { + "type": "object", + "properties": { + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "required": [ + "items" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_switch_fallthrough/go.mod b/testdata/cfg_method_switch_fallthrough/go.mod new file mode 100644 index 0000000..2ac352d --- /dev/null +++ b/testdata/cfg_method_switch_fallthrough/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_switch_fallthrough + +go 1.24.3 diff --git a/testdata/cfg_method_switch_fallthrough/main.go b/testdata/cfg_method_switch_fallthrough/main.go new file mode 100644 index 0000000..25c54db --- /dev/null +++ b/testdata/cfg_method_switch_fallthrough/main.go @@ -0,0 +1,47 @@ +// Package main is the spec-009 guard for a `fallthrough` into a `switch r.Method` +// `default:` arm. The POST arm falls through into the default (405) — so go/cfg gives +// the default body a successor edge FROM the POST arm, defeating a pure +// mutual-exclusivity test. The default is still recognised structurally (a switch-case +// arm with empty case values) and excluded, so the 405 does NOT leak onto GET or POST. +// (fallthrough between HTTP-method cases is pathological, but must not corrupt output.) +package main + +import ( + "encoding/json" + "net/http" +) + +// ItemList is the GET response body. +type ItemList struct { + Items []string `json:"items"` +} + +// CreatedItem is the POST response body. +type CreatedItem struct { + ID int `json:"id"` +} + +// ErrBody is the default-arm 405 body. +type ErrBody struct { + Msg string `json:"msg"` +} + +func items(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case "GET": + _ = json.NewEncoder(w).Encode(ItemList{Items: []string{"a"}}) + case "POST": + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(CreatedItem{ID: 1}) + fallthrough + default: + w.WriteHeader(http.StatusMethodNotAllowed) + _ = json.NewEncoder(w).Encode(ErrBody{Msg: "nope"}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/items", items) + _ = http.ListenAndServe(":8080", mux) +} diff --git a/testdata/cfg_method_two_dispatch/expected_openapi.json b/testdata/cfg_method_two_dispatch/expected_openapi.json new file mode 100644 index 0000000..9036ace --- /dev/null +++ b/testdata/cfg_method_two_dispatch/expected_openapi.json @@ -0,0 +1,121 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/x": { + "get": { + "operationId": "testdata.cfg_method_two_dispatch.handler", + "parameters": [ + { + "name": "X-Auth", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_two_dispatch.GetR" + } + } + } + }, + "401": { + "description": "Unauthorized", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_two_dispatch.AuthErr" + } + } + } + } + } + }, + "post": { + "operationId": "go-apispec.testdata.cfg_method_two_dispatch.handler", + "parameters": [ + { + "name": "X-Auth", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_two_dispatch.PostR" + } + } + } + }, + "401": { + "description": "Unauthorized", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/cfg_method_two_dispatch.AuthErr" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "cfg_method_two_dispatch.AuthErr": { + "type": "object", + "properties": { + "m": { + "type": "string" + } + }, + "required": [ + "m" + ] + }, + "cfg_method_two_dispatch.GetR": { + "type": "object", + "properties": { + "a": { + "type": "integer" + } + }, + "required": [ + "a" + ] + }, + "cfg_method_two_dispatch.PostR": { + "type": "object", + "properties": { + "b": { + "type": "integer" + } + }, + "required": [ + "b" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_two_dispatch/expected_openapi_legacy.json b/testdata/cfg_method_two_dispatch/expected_openapi_legacy.json new file mode 100644 index 0000000..a190758 --- /dev/null +++ b/testdata/cfg_method_two_dispatch/expected_openapi_legacy.json @@ -0,0 +1,121 @@ +{ + "openapi": "3.1.1", + "info": { + "title": "Generated API", + "version": "1.0.0", + "contact": { + "name": "Anton Starikov", + "url": "https://github.com/antst/go-apispec", + "email": "antst@gmail.com" + } + }, + "paths": { + "/x": { + "get": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_two_dispatch.handler", + "parameters": [ + { + "name": "X-Auth", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.GetR" + } + } + } + }, + "401": { + "description": "Unauthorized", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.AuthErr" + } + } + } + } + } + }, + "post": { + "operationId": "github.com/antst/go-apispec/testdata/cfg_method_two_dispatch.handler", + "parameters": [ + { + "name": "X-Auth", + "in": "header", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.PostR" + } + } + } + }, + "401": { + "description": "Unauthorized", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.AuthErr" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.AuthErr": { + "type": "object", + "properties": { + "m": { + "type": "string" + } + }, + "required": [ + "m" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.GetR": { + "type": "object", + "properties": { + "a": { + "type": "integer" + } + }, + "required": [ + "a" + ] + }, + "github.com_antst_go-apispec_testdata_cfg_method_two_dispatch.PostR": { + "type": "object", + "properties": { + "b": { + "type": "integer" + } + }, + "required": [ + "b" + ] + } + } + } +} \ No newline at end of file diff --git a/testdata/cfg_method_two_dispatch/go.mod b/testdata/cfg_method_two_dispatch/go.mod new file mode 100644 index 0000000..db93f6d --- /dev/null +++ b/testdata/cfg_method_two_dispatch/go.mod @@ -0,0 +1,3 @@ +module github.com/antst/go-apispec/testdata/cfg_method_two_dispatch + +go 1.24.3 diff --git a/testdata/cfg_method_two_dispatch/main.go b/testdata/cfg_method_two_dispatch/main.go new file mode 100644 index 0000000..6e6331f --- /dev/null +++ b/testdata/cfg_method_two_dispatch/main.go @@ -0,0 +1,53 @@ +// Package main is the spec-009 guard for a handler with TWO separate `switch r.Method` +// dispatches and an INDEPENDENT conditional between them. The recorded method arms span +// both dispatches; the classifier must scope the dispatch root to the arms of ONE +// dispatch (the responding one) so the independent 401 between the dispatches is shared +// onto every method — not over-excluded by a root inflated to cover both dispatches. +package main + +import ( + "encoding/json" + "net/http" +) + +// GetR is the GET response body. +type GetR struct { + A int `json:"a"` +} + +// PostR is the POST response body. +type PostR struct { + B int `json:"b"` +} + +// AuthErr is the independent 401 body (reachable on every method). +type AuthErr struct { + M string `json:"m"` +} + +func handler(w http.ResponseWriter, r *http.Request) { + switch r.Method { // dispatch 1: per-method headers + case http.MethodGet: + w.Header().Set("X-A", "1") + case http.MethodPost: + w.Header().Set("X-B", "1") + } + if r.Header.Get("X-Auth") == "" { // independent of method, between the dispatches + w.WriteHeader(http.StatusUnauthorized) + _ = json.NewEncoder(w).Encode(AuthErr{M: "no auth"}) + return + } + switch r.Method { // dispatch 2: per-method bodies + case http.MethodGet: + _ = json.NewEncoder(w).Encode(GetR{A: 1}) + case http.MethodPost: + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(PostR{B: 2}) + } +} + +func main() { + mux := http.NewServeMux() + mux.HandleFunc("/x", handler) + _ = http.ListenAndServe(":8080", mux) +}