Skip to content

Commit 39c8e64

Browse files
authored
Improve gorilla mux route extraction (#1779)
1 parent 902b8c0 commit 39c8e64

8 files changed

Lines changed: 335 additions & 75 deletions

File tree

spec/functional_test/fixtures/go/mux/server.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func main() {
5656

5757
// Static file serving
5858
r.PathPrefix("/static/").Handler(http.StripPrefix("/static/", http.FileServer(http.Dir("./static/"))))
59-
59+
6060
// Test multi-line route definition
6161
r.HandleFunc(
6262
"/multiline",
@@ -65,5 +65,18 @@ func main() {
6565
},
6666
).Methods("GET")
6767

68+
// Handler route and route-builder chains common in larger mux apps
69+
r.Handle("/handler-route", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
70+
fmt.Fprintf(w, "handler route")
71+
})).Methods("POST").Name("handler.create")
72+
73+
r.Methods("PATCH").Path("/builder-route").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
74+
fmt.Fprintf(w, "builder route")
75+
})
76+
77+
r.Path("/builder-query").Methods("GET").Queries("type", "{type}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
78+
fmt.Fprintf(w, "builder query")
79+
})
80+
6881
http.ListenAndServe(":8080", r)
69-
}
82+
}

spec/functional_test/fixtures/go/mux_callees/main.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,13 @@ func main() {
1313
w.Write([]byte("ok"))
1414
}).Methods("GET")
1515
r.HandleFunc("/profile", listProfile).Methods("GET")
16+
r.Methods("POST").
17+
Path("/builder-users").
18+
HandlerFunc(createUser)
19+
r.Path("/builder-healthz").
20+
Methods("GET").
21+
Handler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
22+
w.Write([]byte("builder-ok"))
23+
}))
1624
http.ListenAndServe(":8080", r)
1725
}

spec/functional_test/testers/go/mux_callees_spec.cr

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ require "../../func_spec.cr"
1313
# - GET /healthz — same chain shape with an inline closure handler
1414
# in main.go.
1515
# - GET /profile — second named handler in handlers.go.
16+
# - POST /builder-users
17+
# — builder chain with named handler in handlers.go.
18+
# - GET /builder-healthz
19+
# — builder chain with http.HandlerFunc inline handler.
1620
helpers_path = "./spec/functional_test/fixtures/go/mux_callees/helpers.go"
1721

1822
expected_endpoints = [
@@ -32,6 +36,17 @@ expected_endpoints = [
3236
ep.push_callee(Callee.new("auditLog", helpers_path, 7))
3337
ep.push_callee(Callee.new("w.Write", line: 17))
3438
end,
39+
40+
Endpoint.new("/builder-users", "POST").tap do |ep|
41+
ep.push_callee(Callee.new("r.FormValue"))
42+
ep.push_callee(Callee.new("saveUser"))
43+
ep.push_callee(Callee.new("auditLog"))
44+
ep.push_callee(Callee.new("w.Write"))
45+
end,
46+
47+
Endpoint.new("/builder-healthz", "GET").tap do |ep|
48+
ep.push_callee(Callee.new("w.Write"))
49+
end,
3550
]
3651

3752
FunctionalTester.new("fixtures/go/mux_callees/", {

spec/functional_test/testers/go/mux_spec.cr

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ expected_endpoints = [
2424
Endpoint.new("/api/v1/health", "GET"),
2525
Endpoint.new("/static/test.txt", "GET"),
2626
Endpoint.new("/multiline", "GET"),
27+
Endpoint.new("/handler-route", "POST"),
28+
Endpoint.new("/builder-route", "PATCH"),
29+
Endpoint.new("/builder-query", "GET", [
30+
Param.new("type", "", "query"),
31+
]),
2732
# handlers.go: PUT method with path variable
2833
Endpoint.new("/items/{id}", "PUT", [
2934
Param.new("id", "", "path"),

spec/unit_test/miniparser/go_callee_extractor_spec.cr

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,5 +208,59 @@ describe Noir::GoCalleeExtractor do
208208
names.should contain("lookupUser")
209209
names.should contain("c.JSON")
210210
end
211+
212+
it "resolves mux builder-chain HandlerFunc handlers" do
213+
source = <<-GO
214+
package main
215+
216+
func createUser(w http.ResponseWriter, r *http.Request) {
217+
user := saveUser(r)
218+
w.Write([]byte(user))
219+
}
220+
221+
func register(r *mux.Router) {
222+
r.Methods("POST").
223+
Path("/users").
224+
HandlerFunc(createUser)
225+
}
226+
GO
227+
228+
target_row = source.lines.index!(&.includes?("r.Methods(\"POST\")"))
229+
callees = Noir::GoCalleeExtractor.callees_for_routes(
230+
source, "main.go", Set{target_row},
231+
Hash(String, Noir::GoCalleeExtractor::FunctionBody).new
232+
)
233+
names = callees[target_row].map(&.[0])
234+
235+
names.should contain("saveUser")
236+
names.should contain("w.Write")
237+
end
238+
239+
it "unwraps http.HandlerFunc wrappers in mux Handler chains" do
240+
source = <<-GO
241+
package main
242+
243+
func createUser(w http.ResponseWriter, r *http.Request) {
244+
user := saveUser(r)
245+
w.Write([]byte(user))
246+
}
247+
248+
func register(r *mux.Router) {
249+
r.Path("/users").
250+
Methods("POST").
251+
Handler(http.HandlerFunc(createUser))
252+
}
253+
GO
254+
255+
target_row = source.lines.index!(&.includes?("r.Path(\"/users\")"))
256+
callees = Noir::GoCalleeExtractor.callees_for_routes(
257+
source, "main.go", Set{target_row},
258+
Hash(String, Noir::GoCalleeExtractor::FunctionBody).new
259+
)
260+
names = callees[target_row].map(&.[0])
261+
262+
names.should contain("saveUser")
263+
names.should contain("w.Write")
264+
end
211265
end
212266
end

spec/unit_test/miniparser/go_route_extractor_ts_spec.cr

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,4 +194,58 @@ describe Noir::TreeSitterGoRouteExtractor do
194194
routes = Noir::TreeSitterGoRouteExtractor.extract_routes(source)
195195
routes.map { |r| {r.verb, r.path} }.should eq([{"GET", "/legit"}])
196196
end
197+
198+
it "extracts gorilla mux Handle routes and named tail chains" do
199+
source = <<-GO
200+
package main
201+
func main() {
202+
r := mux.NewRouter()
203+
r.Handle("/handler", http.HandlerFunc(handler)).Methods("POST").Name("handler.create")
204+
r.HandleFunc("/multi", handler).Methods("GET", "POST").Name("multi")
205+
}
206+
GO
207+
208+
routes = Noir::TreeSitterGoRouteExtractor.extract_routes(source, handlefunc_methods: true)
209+
routes.map { |r| {r.verb, r.path} }.sort!.should eq([
210+
{"GET", "/multi"},
211+
{"POST", "/handler"},
212+
{"POST", "/multi"},
213+
].sort)
214+
end
215+
216+
it "extracts gorilla mux route builder chains" do
217+
source = <<-GO
218+
package main
219+
func main() {
220+
r := mux.NewRouter()
221+
r.Methods("GET").Path("/builder").HandlerFunc(handler)
222+
r.Path("/alternate").Methods("PATCH").Handler(http.HandlerFunc(handler))
223+
r.Methods("GET").Path("/filtered").Queries("type", "{type}", "page", "{page}").HandlerFunc(handler)
224+
}
225+
GO
226+
227+
routes = Noir::TreeSitterGoRouteExtractor.extract_routes(source, handlefunc_methods: true)
228+
routes.map { |r| {r.verb, r.path, r.query_params} }.sort_by! { |r| r[1] }.should eq([
229+
{"PATCH", "/alternate", [] of String},
230+
{"GET", "/builder", [] of String},
231+
{"GET", "/filtered", ["type", "page"]},
232+
])
233+
end
234+
235+
it "preserves wildcard methods for unconstrained gorilla mux routes" do
236+
source = <<-GO
237+
package main
238+
func main() {
239+
r := mux.NewRouter()
240+
r.HandleFunc("/all", handler)
241+
r.Path("/all-builder").HandlerFunc(handler)
242+
}
243+
GO
244+
245+
routes = Noir::TreeSitterGoRouteExtractor.extract_routes(source, handlefunc_methods: true)
246+
routes.map { |r| {r.verb, r.path} }.sort!.should eq([
247+
{"ANY", "/all"},
248+
{"ANY", "/all-builder"},
249+
].sort)
250+
end
197251
end

src/miniparsers/go_callee_extractor.cr

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ module Noir::GoCalleeExtractor
150150
row = Noir::TreeSitter.node_start_row(node)
151151
next unless route_rows.includes?(row)
152152

153-
handler_arg = find_handler_arg(node, source)
153+
handler_arg = find_handler_arg(node, source).try { |arg| unwrap_handler_arg(arg, source) }
154154
next unless handler_arg
155155

156156
callees = [] of Tuple(String, String, Int32)
@@ -182,21 +182,62 @@ module Noir::GoCalleeExtractor
182182
# First non-string positional argument after a string-literal arg in a
183183
# verb-route call. Mirrors the convention used by
184184
# `TreeSitterGoRouteExtractor#decode_verb_call`.
185+
#
186+
# Mux builder chains are the exception: `.Path("/x").HandlerFunc(h)`
187+
# carries the path and handler in different calls. When the routed call
188+
# itself is `Handler` / `HandlerFunc`, the first non-string arg is the
189+
# handler.
185190
private def find_handler_arg(call_node : LibTreeSitter::TSNode, source : String) : LibTreeSitter::TSNode?
186191
args = Noir::TreeSitter.field(call_node, "arguments")
187192
return unless args
193+
handler_only = handler_only_call?(call_node, source)
188194
seen_path = false
189195
Noir::TreeSitter.each_named_child(args) do |arg|
190196
ty = Noir::TreeSitter.node_type(arg)
191197
if ty == "interpreted_string_literal" || ty == "raw_string_literal"
192198
seen_path = true
193199
next
194200
end
195-
return arg if seen_path
201+
return arg if seen_path || handler_only
196202
end
197203
nil
198204
end
199205

206+
private def handler_only_call?(call_node : LibTreeSitter::TSNode, source : String) : Bool
207+
function = Noir::TreeSitter.field(call_node, "function")
208+
return false unless function
209+
return false unless Noir::TreeSitter.node_type(function) == "selector_expression"
210+
field = Noir::TreeSitter.field(function, "field")
211+
return false unless field
212+
213+
case Noir::TreeSitter.node_text(field, source)
214+
when "Handler", "HandlerFunc"
215+
true
216+
else
217+
false
218+
end
219+
end
220+
221+
private def unwrap_handler_arg(arg : LibTreeSitter::TSNode, source : String) : LibTreeSitter::TSNode
222+
return arg unless Noir::TreeSitter.node_type(arg) == "call_expression"
223+
224+
function = Noir::TreeSitter.field(arg, "function")
225+
return arg unless function
226+
return arg unless Noir::TreeSitter.node_type(function) == "selector_expression"
227+
228+
field = Noir::TreeSitter.field(function, "field")
229+
return arg unless field
230+
return arg unless Noir::TreeSitter.node_text(field, source) == "HandlerFunc"
231+
232+
args = Noir::TreeSitter.field(arg, "arguments")
233+
return arg unless args
234+
Noir::TreeSitter.each_named_child(args) do |child|
235+
return child unless Noir::TreeSitter.node_type(child) == "interpreted_string_literal" ||
236+
Noir::TreeSitter.node_type(child) == "raw_string_literal"
237+
end
238+
arg
239+
end
240+
200241
# Walk `body_node` for call expressions and append `(name, file_path,
201242
# file_line)` tuples. `line_offset` lets callers translate
202243
# tree-sitter rows (already absolute when the parse covers the whole

0 commit comments

Comments
 (0)