From 4221b40fb3a348fdf1778c6c82ccc2a697e1b911 Mon Sep 17 00:00:00 2001 From: rgb-vgx Date: Sun, 4 Oct 2026 15:23:36 +0700 Subject: [PATCH] fix(group): decline over-deep Go group chains and shadowed echo names - groupPrefix / receiverBindsToEchoConstructor return null past MAX_GROUP_DEPTH; scan declines the route instead of emitting a truncated prefix or falling back to gin's handler order for an echo chain. - The echo constructor check rejects an `echo` qualifier shadowed by a local binding or an enclosing function's parameter/receiver. - The gin integration fixture now imports gin like a real file. Co-Authored-By: Claude Opus 5.5 --- .../core/group/extractors/http-patterns/go.ts | 58 +++++++++++++++---- .../unit/group/go-gin-route-groups.test.ts | 50 ++++++++++++++++ 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/go.ts b/gitnexus/src/core/group/extractors/http-patterns/go.ts index bda4e30f8..a36a236b0 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/go.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/go.ts @@ -288,15 +288,17 @@ function findBinding(ident: Parser.SyntaxNode): Parser.SyntaxNode | null { * enclosing `Group(...)` calls (`users := api.Group(…)` ← `api := e.Group(…)` * ← `echo.New()`), the normal shape of grouped routes (review #7, #10). * Provenance must still END at a constructor: parameters, unrelated packages' - * `New()`, and anything else return false so the caller keeps the - * conservative last-argument fallback instead of guessing. + * `New()`, a local that shadows the echo import name, and anything else return + * false so the caller keeps the conservative last-argument fallback instead of + * guessing. Returns null when the Group chain exceeds MAX_GROUP_DEPTH: the + * framework is then unprovable either way, so the caller declines the route. */ function receiverBindsToEchoConstructor( receiver: Parser.SyntaxNode, echoAliases: ReadonlySet, depth = 0, -): boolean { - if (depth > MAX_GROUP_DEPTH) return false; +): boolean | null { + if (depth > MAX_GROUP_DEPTH) return null; // An identifier resolves through its binding; a chained `X.Group(…).Group(…)` // operand is already a call and is inspected as-is. const value = receiver.type === 'identifier' ? findBinding(receiver) : receiver; @@ -307,7 +309,7 @@ function receiverBindsToEchoConstructor( const operand = fn.childForFieldName('operand'); if (!operand) return false; if (field === 'New' || field === 'Default') { - return operand.type === 'identifier' && echoAliases.has(operand.text); + return operand.type === 'identifier' && echoAliases.has(operand.text) && !isLocalName(operand); } if (field === 'Group') { return receiverBindsToEchoConstructor(operand, echoAliases, depth + 1); @@ -315,13 +317,42 @@ function receiverBindsToEchoConstructor( return false; } -/** Joined `Group(...)` prefix of a route receiver; '' when it cannot be traced. */ -function groupPrefix(receiver: Parser.SyntaxNode, depth = 0): string { - if (depth > MAX_GROUP_DEPTH) return ''; +/** + * Whether `ident` names a local value rather than an imported package: a + * binding in scope or a parameter/receiver of an enclosing function. Go lets + * either shadow a package qualifier (`func f(echo *Factory) { echo.New() }`). + */ +function isLocalName(ident: Parser.SyntaxNode): boolean { + if (findBinding(ident) !== null) return true; + for (let node = ident.parent; node; node = node.parent) { + if ( + node.type !== 'func_literal' && + node.type !== 'function_declaration' && + node.type !== 'method_declaration' + ) { + continue; + } + const lists = [node.childForFieldName('parameters'), node.childForFieldName('receiver')]; + for (const list of lists) { + if (list?.descendantsOfType('identifier').some((p) => p.text === ident.text)) return true; + } + if (node.type !== 'func_literal') return false; + } + return false; +} + +/** + * Joined `Group(...)` prefix of a route receiver; '' when it cannot be traced. + * Returns null when the chain exceeds MAX_GROUP_DEPTH: a partial prefix would + * silently drop the inner groups, so the caller declines the route instead. + */ +function groupPrefix(receiver: Parser.SyntaxNode, depth = 0): string | null { + if (depth > MAX_GROUP_DEPTH) return null; const value = receiver.type === 'identifier' ? findBinding(receiver) : receiver; const group = value ? asGroupCall(value) : null; if (!group) return ''; - return joinRoutePath(groupPrefix(group.parent, depth + 1), group.prefix); + const outer = groupPrefix(group.parent, depth + 1); + return outer === null ? null : joinRoutePath(outer, group.prefix); } // ─── Provider: net/http `http.HandleFunc("/p", handler)` ───────────── @@ -446,11 +477,14 @@ export const GO_HTTP_PLUGIN: HttpLanguagePlugin = { ? receiverBindsToEchoConstructor(receiverNode, imports.echo) : false : echoOnly; + // A Group chain deeper than MAX_GROUP_DEPTH proves neither the full + // prefix nor the framework order: decline rather than emit a guess. + if (echoOrder === null) continue; + const prefix = receiverNode ? groupPrefix(receiverNode) : ''; + if (prefix === null) continue; const handlerNode = echoOrder ? rest[0] : rest[rest.length - 1]; if (!HANDLER_ARG_TYPES.has(handlerNode.type)) continue; - const path = receiverNode - ? joinRoutePath(groupPrefix(receiverNode), literalPath) - : literalPath; + const path = receiverNode ? joinRoutePath(prefix, literalPath) : literalPath; // An inline `func(){…}` handler has no name → emit `name: null` and a // `line` so it resolves to its containing/closure symbol by line-span // containment (like a consumer). A named handler keeps its name and diff --git a/gitnexus/test/unit/group/go-gin-route-groups.test.ts b/gitnexus/test/unit/group/go-gin-route-groups.test.ts index 2a7bfaf30..0285dec42 100644 --- a/gitnexus/test/unit/group/go-gin-route-groups.test.ts +++ b/gitnexus/test/unit/group/go-gin-route-groups.test.ts @@ -39,6 +39,8 @@ function providers(src: string): Provider[] { // handlers, and variadic middleware before the handler. const GIN_ROUTES = `package handlers +import "github.com/gin-gonic/gin" + func RegisterRoutes(r *gin.Engine, svc *service.Service) { playerHandler := NewPlayerHandler(svc.Player) matchHandler := NewMatchHandler(svc.Match) @@ -575,6 +577,54 @@ func routes() { ).toEqual([{ method: 'GET', path: '/x', name: 'Middleware' }]); }); + it('does not mistake a local shadowing the echo import for its constructor', () => { + // A parameter named `echo` shadows the package qualifier: `echo.New()` + // is then a method on that value, not echo's constructor, so the mixed + // file keeps the conservative last-argument fallback. + expect( + providers(`package main +import ( + "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" +) + +func routes(echo *Factory) { + e := echo.New() + e.GET("/x", h.Handler, auth.Middleware) +} +`), + ).toEqual([{ method: 'GET', path: '/x', name: 'Middleware' }]); + }); + + it('declines a route whose Group chain exceeds the depth cap', () => { + // Past MAX_GROUP_DEPTH (32) the full prefix — and, in a mixed file, the + // framework order — is unprovable: emitting the outer prefixes alone (or + // gin's handler order for an echo chain) would be a silent guess. + const chain = (ctor: string, imports: string) => { + const lines = [`g0 := ${ctor}`]; + for (let i = 1; i <= 34; i++) lines.push(`g${i} := g${i - 1}.Group("/p${i}")`); + return `package main +${imports} + +func routes() { + ${lines.join('\n\t')} + g34.GET("/x", h.Handler, auth.Middleware) + g2.GET("/y", h.Handler, auth.Middleware) +} +`; + }; + expect(providers(chain('gin.Default()', 'import "github.com/gin-gonic/gin"'))).toEqual([ + { method: 'GET', path: '/p1/p2/y', name: 'Middleware' }, + ]); + const mixed = `import ( + "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" +)`; + expect(providers(chain('echo.New()', mixed))).toEqual([ + { method: 'GET', path: '/p1/p2/y', name: 'Handler' }, + ]); + }); + it('traces a grouped receiver back to its framework constructor in a mixed file', () => { // The normal grouped shape: `users := api.Group(…)` ← `api := e.Group(…)` // ← `echo.New()` proves echo order through the Group chain, while the