mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-11 03:38:07 +00:00
fix(group): match Go route extraction to framework argument order
Address the three nexus-check findings on PR #3458: - Echo handler position: `e.GET("/x", h.Handler, auth.Middleware)` puts the handler SECOND (echo: path, handler, middleware...), gin puts it last (path, middleware..., handler). An echo-only import (matched on the import path, alias-safe) now selects the first argument after the path; gin-only, mixed, and import-less files keep the last-argument anchor. - For-loop post binding: findBinding scanned every named child of a for_clause, including `update`, which runs after the body — a `g = r.Group("/new")` post statement shadowed the group the body sees. Only the `initializer` slot binds before the body. - Duplicate slashes: joinRoutePath collapsed "//" only at the join boundary, while ingestion's normalizeExtractedRoutePath collapses every run and the downstream contract-id normalizer does not — a path keeping "//" split into two contract ids across the strategies. Collapse the final result on every return branch instead. Tests: +6 (echo-only handler, both-imports fallback, for-post scoping, duplicate-slash collapse); go-gin suite 36, group suite 1297, ingestion Go 71. Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
parent
cb9edf4305
commit
d0bfc3ee8d
2 changed files with 156 additions and 16 deletions
|
|
@ -19,13 +19,15 @@ import type { HttpDetection, HttpLanguagePlugin } from './types.js';
|
|||
|
||||
// ─── Provider: framework routing ──────────────────────────────────────
|
||||
// Matches `\w+\.GET(...)` etc. (gin and echo share this shape).
|
||||
// Captures the receiver, the HTTP method (field name), path literal, and the
|
||||
// handler — anchored to the LAST argument (`@handler .`) so a variadic
|
||||
// middleware chain (`r.GET("/x", mw, handler)`, gin style) binds the real
|
||||
// handler, not a middleware identifier (which would otherwise over-match
|
||||
// and attach the route to the wrong symbol — see #2276 review). The handler
|
||||
// may be a function name, an inline func literal, or a method value /
|
||||
// package-qualified function (`h.ListUsers`, `handlers.ListUsers`).
|
||||
// Captures the receiver, the HTTP method (field name), and the path literal
|
||||
// — anchored as the FIRST argument so the code can pick the handler out of
|
||||
// the remaining arguments. Which argument that is depends on the framework:
|
||||
// gin is `GET(path, middleware..., handler)` (last), echo is
|
||||
// `GET(path, handler, middleware...)` (first) — see importsEchoOnly below.
|
||||
// The handler must be an identifier, an inline func literal, or a method
|
||||
// value / package-qualified function (`h.ListUsers`, `handlers.ListUsers`);
|
||||
// anything else there means the call cannot be attributed to a symbol, so it
|
||||
// is dropped rather than guessed (variadic-middleware over-match, #2276).
|
||||
const FRAMEWORK_ROUTE_PATTERNS = compilePatterns({
|
||||
name: 'go-framework-route',
|
||||
language: Go,
|
||||
|
|
@ -38,14 +40,48 @@ const FRAMEWORK_ROUTE_PATTERNS = compilePatterns({
|
|||
operand: (_) @receiver
|
||||
field: (field_identifier) @http_method (#match? @http_method "^(GET|POST|PUT|DELETE|PATCH)$"))
|
||||
arguments: (argument_list
|
||||
(interpreted_string_literal) @path
|
||||
[(identifier) (func_literal) (selector_expression)] @handler
|
||||
.))
|
||||
.
|
||||
(interpreted_string_literal) @path))
|
||||
`,
|
||||
},
|
||||
],
|
||||
} satisfies LanguagePatterns<Record<string, never>>);
|
||||
|
||||
/** Argument forms a route handler may take. */
|
||||
const HANDLER_ARG_TYPES: ReadonlySet<string> = new Set([
|
||||
'identifier',
|
||||
'func_literal',
|
||||
'selector_expression',
|
||||
]);
|
||||
|
||||
/**
|
||||
* Whether the file's imports say it routes with echo and not gin: echo
|
||||
* verb calls take the handler as the FIRST argument after the path
|
||||
* (`GET(path, handler, middleware...)`), gin's as the LAST
|
||||
* (`GET(path, middleware..., handler)`). Matched on the import path rather
|
||||
* than the local name, so an aliased import still counts. Both frameworks
|
||||
* or neither → not echo-only → callers keep the last-argument anchor, which
|
||||
* is gin's order and the safer default when the file proves nothing.
|
||||
*/
|
||||
function importsEchoOnly(root: Parser.SyntaxNode): boolean {
|
||||
// Imports sit only at file scope; skip bodies.
|
||||
const specs = root.namedChildren
|
||||
.filter((node) => node.type === 'import_declaration')
|
||||
.flatMap((decl) => decl.descendantsOfType('import_spec'));
|
||||
let echo = false;
|
||||
let gin = false;
|
||||
for (const spec of specs) {
|
||||
const importPath = stringLiteral(spec.childForFieldName('path'));
|
||||
if (importPath === null) continue;
|
||||
// `_` and `.` imports bind no qualifier this file can route through.
|
||||
const local = spec.childForFieldName('name')?.text;
|
||||
if (local === '_' || local === '.') continue;
|
||||
if (importPath.includes('labstack/echo')) echo = true;
|
||||
else if (importPath.includes('gin-gonic/gin')) gin = true;
|
||||
}
|
||||
return echo && !gin;
|
||||
}
|
||||
|
||||
// ─── Route groups: `v1 := r.Group("/api/v1")` ─────────────────────────
|
||||
// gin (`*gin.RouterGroup`) and echo (`*echo.Group`) routes registered on a
|
||||
// group inherit every enclosing `Group(prefix)`. The prefix is recovered by
|
||||
|
|
@ -62,9 +98,17 @@ const FRAMEWORK_ROUTE_PATTERNS = compilePatterns({
|
|||
const MAX_GROUP_DEPTH = 32;
|
||||
|
||||
function joinRoutePath(prefix: string, relative: string): string {
|
||||
if (!prefix) return relative;
|
||||
if (!relative) return prefix;
|
||||
return `${prefix.replace(/\/+$/, '')}/${relative.replace(/^\/+/, '')}`;
|
||||
let joined = relative;
|
||||
if (prefix && relative) {
|
||||
joined = `${prefix.replace(/\/+$/, '')}/${relative.replace(/^\/+/, '')}`;
|
||||
} else if (prefix) {
|
||||
joined = prefix;
|
||||
}
|
||||
// Collapse duplicate slashes on the FINAL result — every return branch, not
|
||||
// just the join — because ingestion's normalizeExtractedRoutePath collapses
|
||||
// all "//" while the downstream contract-id normalizer does not: a path
|
||||
// that keeps "//" would split into two contract ids across the strategies.
|
||||
return joined.replace(/\/+/g, '/');
|
||||
}
|
||||
|
||||
/** `parent.Group("/p", mw...)` → its receiver and literal prefix; null otherwise. */
|
||||
|
|
@ -167,8 +211,13 @@ function findBinding(ident: Parser.SyntaxNode): Parser.SyntaxNode | null {
|
|||
if (node.type === 'for_statement') {
|
||||
const clause = node.namedChildren[0];
|
||||
if (clause?.type === 'for_clause') {
|
||||
for (const stmt of clause.namedChildren) {
|
||||
const value = boundValue(stmt, name);
|
||||
// Only the initializer runs before the body: `condition` and
|
||||
// `update` (`g = r.Group("/post")` in the post slot) evaluate after
|
||||
// it, so they must not shadow what the body sees on entry. An absent
|
||||
// initializer (`for ; c; i++`) binds nothing.
|
||||
const init = clause.childForFieldName('initializer');
|
||||
if (init) {
|
||||
const value = boundValue(init, name);
|
||||
if (value !== undefined) return value;
|
||||
}
|
||||
} else if (clause?.type === 'range_clause') {
|
||||
|
|
@ -295,14 +344,23 @@ export const GO_HTTP_PLUGIN: HttpLanguagePlugin = {
|
|||
const out: HttpDetection[] = [];
|
||||
|
||||
// Framework providers: r.GET/POST/... on an engine or (nested) route group
|
||||
const echoOnly = importsEchoOnly(tree.rootNode);
|
||||
for (const match of runCompiledPatterns(FRAMEWORK_ROUTE_PATTERNS, tree)) {
|
||||
const methodNode = match.captures.http_method;
|
||||
const pathNode = match.captures.path;
|
||||
const handlerNode = match.captures.handler;
|
||||
const receiverNode = match.captures.receiver;
|
||||
if (!methodNode || !pathNode) continue;
|
||||
const literalPath = stringLiteral(pathNode);
|
||||
if (literalPath === null) continue;
|
||||
const argList = pathNode.parent;
|
||||
if (argList?.type !== 'argument_list') continue;
|
||||
// The path is anchored first, so everything after it is a handler or
|
||||
// middleware candidate: an echo-only file takes the first of those,
|
||||
// any other file the last (see FRAMEWORK_ROUTE_PATTERNS / importsEchoOnly).
|
||||
const rest = argList.namedChildren.slice(1);
|
||||
if (rest.length === 0) continue;
|
||||
const handlerNode = echoOnly ? rest[0] : rest[rest.length - 1];
|
||||
if (!HANDLER_ARG_TYPES.has(handlerNode.type)) continue;
|
||||
const path = receiverNode
|
||||
? joinRoutePath(groupPrefix(receiverNode), literalPath)
|
||||
: literalPath;
|
||||
|
|
|
|||
|
|
@ -360,6 +360,31 @@ func routes(r *gin.Engine, n int, subs []*gin.RouterGroup) {
|
|||
]);
|
||||
});
|
||||
|
||||
it('ignores a for-loop post assignment and only honors its initializer', () => {
|
||||
// The post statement runs AFTER each body, so a `g = …` there must not
|
||||
// shadow the group the body sees on entry; an initializer still does.
|
||||
expect(
|
||||
providers(`package main
|
||||
func routes(r *gin.Engine, cond bool, n int) {
|
||||
g := r.Group("/old")
|
||||
for ; cond; g = r.Group("/post") {
|
||||
g.GET("/a", h.A)
|
||||
}
|
||||
for i := 0; i < n; g = r.Group("/post2") {
|
||||
g.GET("/b", h.B)
|
||||
}
|
||||
for g := r.Group("/init"); ; g = r.Group("/post3") {
|
||||
g.GET("/c", h.C)
|
||||
}
|
||||
}
|
||||
`),
|
||||
).toEqual([
|
||||
{ method: 'GET', path: '/old/a', name: 'A' },
|
||||
{ method: 'GET', path: '/old/b', name: 'B' },
|
||||
{ method: 'GET', path: '/init/c', name: 'C' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('accepts a raw-string (backtick) group prefix', () => {
|
||||
expect(
|
||||
providers(`package main
|
||||
|
|
@ -396,6 +421,63 @@ func routes(r *gin.Engine) {
|
|||
).toEqual([{ method: 'GET', path: '/raw\\x2fy/z', name: 'Z' }]);
|
||||
});
|
||||
|
||||
it('collapses duplicate slashes so the path matches ingestion normalization', () => {
|
||||
// normalizeExtractedRoutePath collapses every "//" run; the downstream
|
||||
// contract-id normalizer does not — a path keeping "//" would get a
|
||||
// different id than the graph's route node for the same route.
|
||||
expect(
|
||||
providers(`package main
|
||||
func routes(r *gin.Engine) {
|
||||
g := r.Group("/api//v1")
|
||||
g.GET("/a//b", h.A)
|
||||
r.GET("//root", h.R)
|
||||
r.GET("/ok", h.Ok)
|
||||
}
|
||||
`),
|
||||
).toEqual([
|
||||
{ method: 'GET', path: '/api/v1/a/b', name: 'A' },
|
||||
{ method: 'GET', path: '/root', name: 'R' },
|
||||
{ method: 'GET', path: '/ok', name: 'Ok' },
|
||||
]);
|
||||
});
|
||||
|
||||
it("binds echo's handler (first argument) when the file imports echo only", () => {
|
||||
// echo is `GET(path, handler, middleware...)` — the handler is second,
|
||||
// not last, so a middleware selector must not become the route's name.
|
||||
expect(
|
||||
providers(`package main
|
||||
import "github.com/labstack/echo/v4"
|
||||
|
||||
func routes(e *echo.Echo) {
|
||||
e.GET("/x", h.Handler, auth.Middleware)
|
||||
e.POST("/y", handlerID)
|
||||
e.PUT("/z", func(c echo.Context) error { return nil })
|
||||
}
|
||||
`),
|
||||
).toEqual([
|
||||
{ method: 'GET', path: '/x', name: 'Handler' },
|
||||
{ method: 'POST', path: '/y', name: 'handlerID' },
|
||||
{ method: 'PUT', path: '/z', name: null },
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps the last-argument handler when a file imports both gin and echo', () => {
|
||||
// Ambiguous imports prove nothing about argument order → conservative
|
||||
// fallback to the last-argument anchor (gin's order).
|
||||
expect(
|
||||
providers(`package main
|
||||
import (
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/labstack/echo/v4"
|
||||
)
|
||||
|
||||
func routes(e *echo.Echo) {
|
||||
e.GET("/x", h.Handler, auth.Middleware)
|
||||
}
|
||||
`),
|
||||
).toEqual([{ method: 'GET', path: '/x', name: 'Middleware' }]);
|
||||
});
|
||||
|
||||
it('applies the same group logic to echo', () => {
|
||||
expect(
|
||||
providers(`package main
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue