Sfoglia il codice sorgente

fix(go): require URL-shaped paths for route detection (#1308)

cache.Put("a", 1), store.Get("config", out), bus.Handle("user.created",
h) — any verb-named method with a string first arg — were indexed as
HTTP routes (38 of 82 route nodes were false positives on the
reporter's 200 KLOC Go codebase). A registration's first argument must
now start with "/" (every router style), or be a Go 1.22
"METHOD /path" mux pattern on Handle/HandleFunc — which now also
extracts the real method instead of ANY.

Validated on go-chi/chi (212 real routes retained, all path-shaped)
and golang/groupcache (0 route nodes).

Fixes #1259

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Colby Mchenry 1 mese fa
parent
commit
e1f339f732
3 ha cambiato i file con 60 aggiunte e 5 eliminazioni
  1. 1 0
      CHANGELOG.md
  2. 31 0
      __tests__/frameworks.test.ts
  3. 28 5
      src/resolution/frameworks/go.ts

+ 1 - 0
CHANGELOG.md

@@ -18,6 +18,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ### Fixes
 
+- Go route detection no longer misidentifies ordinary method calls that share HTTP verb names — `cache.Put("key", value)`, `store.Get("config", out)`, `bus.Handle("user.created", handler)` and the like were being indexed as HTTP routes, polluting route listings in cache-heavy codebases. A registration now has to look like one: its first argument must be a `/`-prefixed path (all routers) or a Go 1.22 `"METHOD /path"` pattern on `Handle`/`HandleFunc`, which now also extracts the method instead of listing the route as `ANY`. (#1259)
 - Progress output on Windows no longer mixes ASCII `|` rails with the Unicode `│ ◆ ●` frame around them. In terminals that render Unicode (Windows Terminal, VS Code, ConEmu/Cmder, JetBrains, Alacritty), the whole `codegraph init` / `index` / `sync` block now draws with matching box-drawing characters; unrecognized legacy consoles keep the safe all-ASCII output that avoids garbled characters. `CODEGRAPH_ASCII=1` / `CODEGRAPH_UNICODE=1` still override in either direction. (#398)
 - CLI output now honors the `NO_COLOR` convention and new `--color` / `--no-color` flags, and goes plain automatically when piped: commands like `codegraph status`, `query`, `callers`, and `files` no longer embed ANSI color codes when stdout isn't a terminal, and a piped `codegraph init` / `index` / `sync` prints simple per-phase lines instead of progress-animation control characters. `FORCE_COLOR` or `--color` forces color back on for pipes that render it. (#1281)
 - Callers and impact analysis no longer silently under-count a function that calls the same callee many times. When one caller contained several call sites to the same callee and an internal resolution batch boundary happened to split them, cleanup after the first batch removed the later sites' pending rows before they were ever attempted — their edges were never created, deterministically, and which edges went missing shifted with unrelated changes to the project's total reference count. Post-pass cleanup now targets the exact database row each processed reference came from. Found while validating the operator-call fix on nlohmann/json, where `write_cbor`'s 11 calls to `to_char_type` indexed as 10. (#1269)

+ 31 - 0
__tests__/frameworks.test.ts

@@ -942,6 +942,37 @@ describe('goResolver.extract', () => {
     const { references } = goResolver.extract!('routes.go', src);
     expect(references[0].referenceName).toBe('listUsers');
   });
+
+  it('does NOT treat verb-named method calls with non-path args as routes (#1259)', () => {
+    // The issue's repro: a generic cache type whose Put/Get share router verb
+    // names. First args are keys, not URL paths — no route nodes.
+    const src = [
+      `c.Put("a", 1)`,
+      `c.Put("user:123", value)`,
+      `store.Get("config", out)`,
+      `bus.Handle("user.created", onUserCreated)`,
+      `m.HandleFunc("shutdown", hook)`,
+    ].join('\n');
+    const { nodes } = goResolver.extract!('cache.go', src);
+    expect(nodes).toHaveLength(0);
+  });
+
+  it('keeps real registrations whose paths start with "/" for every router style', () => {
+    const src = [
+      `r.Put("/users/{id}", updateUser)`, // chi
+      `v1.GET("/ping", ping)`, // gin group
+      `mux.HandleFunc("/healthz", health)`, // net/http
+    ].join('\n');
+    const { nodes } = goResolver.extract!('routes.go', src);
+    expect(nodes.map((n) => n.name)).toEqual(['PUT /users/{id}', 'GET /ping', 'ANY /healthz']);
+  });
+
+  it('recognizes Go 1.22 "METHOD /path" patterns on HandleFunc and extracts the method', () => {
+    const src = `mux.HandleFunc("GET /api/users/{id}", getUser)\n`;
+    const { nodes, references } = goResolver.extract!('main.go', src);
+    expect(nodes[0].name).toBe('GET /api/users/{id}');
+    expect(references[0].referenceName).toBe('getUser');
+  });
 });
 
 import { goframeResolver } from '../src/resolution/frameworks/goframe';

+ 28 - 5
src/resolution/frameworks/go.ts

@@ -96,17 +96,29 @@ export const goResolver: FrameworkResolver = {
     let match: RegExpExecArray | null;
     while ((match = routeRegex.exec(safe)) !== null) {
       const [, rawMethod, routePath, handlerExpr] = match;
+
+      // The first argument must be URL-shaped, or this is just a method that
+      // happens to share a verb name — `cache.Put("key", val)`, `store.Get(...)`,
+      // `bus.Handle("user.created", h)` all polluted the route index (#1259).
+      // Real registrations use "/path" (every router), or net/http's Go 1.22
+      // "METHOD /path" patterns on Handle/HandleFunc.
+      const methodPrefix = matchGo122MethodPattern(routePath!, rawMethod!);
+      if (!routePath!.startsWith('/') && !methodPrefix) continue;
+
       const line = safe.slice(0, match.index).split('\n').length;
-      const method =
-        rawMethod === 'Handle' || rawMethod === 'HandleFunc'
+      // "GET /users/{id}" -> method GET, path /users/{id}
+      const path = methodPrefix ? routePath!.slice(methodPrefix.length).trimStart() : routePath!;
+      const method = methodPrefix
+        ? methodPrefix
+        : rawMethod === 'Handle' || rawMethod === 'HandleFunc'
           ? 'ANY'
           : rawMethod!.toUpperCase();
 
       const routeNode: Node = {
-        id: `route:${filePath}:${line}:${method}:${routePath}`,
+        id: `route:${filePath}:${line}:${method}:${path}`,
         kind: 'route',
-        name: `${method} ${routePath}`,
-        qualifiedName: `${filePath}::route:${routePath}`,
+        name: `${method} ${path}`,
+        qualifiedName: `${filePath}::route:${path}`,
         filePath,
         startLine: line,
         endLine: line,
@@ -135,6 +147,17 @@ export const goResolver: FrameworkResolver = {
   },
 };
 
+/**
+ * Go 1.22 net/http mux patterns: `mux.HandleFunc("GET /users/{id}", h)`.
+ * Returns the HTTP method when the pattern starts with one, null otherwise.
+ * Only Handle/HandleFunc take these — Gin/Chi verb methods take a bare path.
+ */
+function matchGo122MethodPattern(routePath: string, rawMethod: string): string | null {
+  if (rawMethod !== 'Handle' && rawMethod !== 'HandleFunc') return null;
+  const m = routePath.match(/^(GET|POST|PUT|PATCH|DELETE|OPTIONS|HEAD|CONNECT|TRACE)\s+\S/);
+  return m ? m[1]! : null;
+}
+
 /** Extract the last identifier from an expression like `pkg.Sub.handler` or `handler`. */
 function extractGoTailIdent(expr: string): string | null {
   const cleaned = expr.trim().replace(/\s+/g, '').replace(/\(\)$/, '');