From 023fc3111b7ffd6f2fa993020c67fd54b335fc7c Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Mon, 5 Oct 2026 22:00:29 +0000 Subject: [PATCH] fix(axum,actix): link a route to a handler written below its call (#2326) (#2359) With rustfmt's wrapped form .route( "/multi", get(handlers::multi_line_handler), ) the route was never linked to `multi_line_handler`, even when it was the only function of that name; the single-line form worked. Rustfmt wraps every `.route(` past the line width, so most routes of a real router were affected (31 of 364 handler references resolved in the reporter's router). Cause: the Axum block of the Rust framework extractor recorded the handler reference at the line of `.route(`, column 0. Every resolved reference then passes the Rust scope gate (isRustNameInScope), which reads that one line to see how the name is written (`handlers::f` names the `handlers` module). On a line holding only `.route(` it found nothing and dropped the edge. The same position also broke single-line routes whose handler name appears elsewhere on the line: in `.route("/login", post(handlers::user::login))` or `delete(handlers::user::delete)` the gate read the path string or the method router as a bare use of the name. Actix's `web::resource(..)` chains and App-level `.route("/p", web::get().to(h))` had the same weakness. Fix (src/resolution/frameworks/rust.ts): - The handler reference carries the line and column of the handler's own name (its last path segment), for Axum and the three Actix forms; the route node stays on the `.route(` / `web::resource(` line. The gate is unchanged and still reads one line, the one the handler is written on. - Axum's method routers are the top-level chain of `.route(`'s second argument: a `get(` nested inside it is a call in a closure handler's body (`post(|h| async move { h.get(CONTENT_LENGTH) })`), not a route. With the reference moved, such a call would otherwise gain an edge. - An Actix resource's method chain also ends at the `)` of the call it is an argument of, so in `.service(web::resource("/").to(index)) .route("/count", web::get().to(get_count))` the resource keeps `.to(index)` instead of taking `/count`'s handler. Adopted from #2329 by @danusha2345 (the fix and the tests are theirs), without its EXTRACTION_VERSION bump (already 28 on main); added a case for a single-line route whose handler name the line also writes elsewhere. Verification: __tests__/rust-route-handler-line.test.ts: 10 of 13 fail on main in both wasm and kernel modes; 13/13 pass with the fix in both modes. The issue's repro now prints `GET /multi [references] src/main.rs:7`. Validation (main -> branch, route nodes and route->handler edges keyed by route name/file/line and target QN/file): - rust-lang/crates.io: 1 -> 5 linked (+4 wrapped routes). - atuinsh/atuin server: 7 -> 12; +5 newly linked and 2 wrong targets replaced (`GET /api/v0/me` user::get -> v0::me::get, `GET /api/v0/record/next` a TUI `Events::next` method -> v0::record::next). - launchbadge/realworld-axum-sqlx: 18 -> 19 (+1 wrapped chain link); 11 same-file edges now carry the handler's line. - tokio-rs/axum (whole repo): 204 -> 204, 19 edges carry the handler's line; 2 bogus `GET /` route nodes from `headers.get(..)` in closure handlers in routing tests are gone. - actix/examples: 116 -> 116; `ANY / -> index` x2 replaces two wrong `GET /` edges (websockets/chat took the next routes' handlers; basics/hello-world now records its real `.service(web::resource("/").to(index))`). No other edge changed in any of the five repos. Co-authored-by: danusha2345 Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/rust-route-handler-line.test.ts | 208 ++++++++++++++++++++++ src/resolution/frameworks/rust.ts | 47 ++++- 3 files changed, 247 insertions(+), 9 deletions(-) create mode 100644 __tests__/rust-route-handler-line.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d46e1fef9..16c79d3d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - Indexing a project that includes large bundled JavaScript files, such as a copy of pdf.js or d3, is fast again: since 1.6.2, resolving the calls in a JavaScript or TypeScript file re-read the file's text above each call, so a single bundled library could add many seconds to an index. The graph it builds is unchanged. Thanks @bompus for the report. (#2334) - The time `codegraph init` and `codegraph index` print beside the node and edge counts now covers the whole run, resolving references and linking included, and `codegraph sync` reports its whole run the same way. Before, it counted only reading and parsing the files, which can be a small part of an index, so a slow index looked fast. Thanks @bompus for the report. (#2334) - In Rust, code that uses an enum only through its variants, like `mode::Mode::A` in an expression, a `Mode::B =>` match arm or `Self::A` inside the enum's own `impl`, now shows up in that enum's callers and impact, linked to the enum that actually declares the variant rather than to a same-named type elsewhere. Standard-library variants like `Ordering::Less` or `Option::Some` and associated items like `Foo::new()` or `Foo::MAX` don't count as using a project enum. Re-index Rust projects after upgrading. Thanks @mg-mg-mg for the report and @danusha2345. (#2328) +- Axum routes now link to their handler when rustfmt wraps the `.route(` call so the handler sits on a later line, and when the handler's name also appears elsewhere on the line, as in `delete(handlers::user::delete)` or `.route("/login", post(handlers::user::login))`; Actix's `web::resource(…)` and `.route(…)` registrations get the same fix. A call inside a closure handler is no longer mistaken for a route, and an Actix resource no longer takes the next route's handler as its own. Thanks @mg-mg-mg for the report and @danusha2345. (#2326) ## [1.6.2] - 2026-10-03 diff --git a/__tests__/rust-route-handler-line.test.ts b/__tests__/rust-route-handler-line.test.ts new file mode 100644 index 000000000..41e4749fd --- /dev/null +++ b/__tests__/rust-route-handler-line.test.ts @@ -0,0 +1,208 @@ +/** + * A Rust route's handler reference sits on the line its handler is written + * on (#2326). rustfmt wraps a long `.route(` call, putting the handler on a + * line below `.route(`; the Rust scope gate reads the reference's line to see + * how the name is written (`handlers::f` names the `handlers` module), so a + * reference left on the `.route(` line never found the name there and the + * edge was dropped — as it was when the line wrote the handler's name + * elsewhere too (`/login` in the path, `delete(` as the method router), which + * the gate read as a bare use. A call inside a closure handler's body is not a method + * router: `cache.get(keys::session_key)` is not a `GET` route handled by + * `session_key`. Nor is an Actix resource's chain longer than the call it is + * an argument of: the App-level `.route(..)` after it is not its method. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { rustResolver } from '../src/resolution/frameworks/rust'; + +const MAIN_RS = `mod actix_app; +mod handlers; +mod keys; +use axum::{routing::{delete, get, post}, Router}; +use crate::handlers::used_handler; + +pub fn app() -> Router { + Router::new() + .route("/single", get(handlers::single_line_handler)) + .route( + "/multi", + get(handlers::multi_line_handler), + ) + .route( + "/deep", + get( + handlers::deep_line_handler, + ), + ) + .route( + "/items", + get(handlers::list_items) + .post(handlers::create_item) + .delete(handlers::delete_item), + ) + .route( + "/used", + post(used_handler), + ) + .route( + "/other", + get(metrics::render_handler), + ) + .route("/login", post(handlers::login)) + .route("/account", delete(handlers::delete)) + .route("/closure-inline", get(|| async { cache().get(keys::session_key) })) + .route( + "/closure", + get(|| async move { + cache().get(keys::session_key) + }), + ) +} +`; + +const ACTIX_RS = `use actix_web::{web, App}; +use crate::handlers; + +pub fn actix() { + App::new() + .service( + web::resource("/index/with/a/rather/long/path/{id}") + .to(handlers::actix_index), + ) + .service( + web::resource("/user/{id}") + .route( + web::get() + .to(handlers::actix_get_user), + ), + ) + .route( + "/hey", + web::get().to(handlers::actix_hello), + ); +} +`; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-rust-route-line-')); + const files: Record = { + 'Cargo.toml': '[package]\nname = "repro"\nversion = "0.1.0"\nedition = "2021"\n\n[dependencies]\naxum = "0.7"\nactix-web = "4"\nmetrics = "0.23"\n', + 'src/main.rs': MAIN_RS, + 'src/actix_app.rs': ACTIX_RS, + 'src/handlers/mod.rs': `pub async fn single_line_handler() {} +pub async fn multi_line_handler() {} +pub async fn deep_line_handler() {} +pub async fn list_items() {} +pub async fn create_item() {} +pub async fn delete_item() {} +pub async fn used_handler() {} +pub async fn render_handler() {} +pub async fn login() {} +pub async fn delete() {} +pub async fn actix_index() {} +pub async fn actix_get_user() {} +pub async fn actix_hello() {} +`, + 'src/keys.rs': 'pub fn session_key() -> &\'static str { "s" }\n', + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +/** `METHOD /path -> file:handler` for every route -> function `references` edge out of a file's routes. */ +const routeEdges = (file: string): string[] => { + const routes = cg.getNodesInFile(file).filter((n) => n.kind === 'route'); + return cg.getOutgoingEdgesFrom(routes.map((r) => r.id), ['references']) + .map((e) => `${cg.getNode(e.source)!.name} -> ${cg.getNode(e.target)!.filePath}:${cg.getNode(e.target)!.name}`) + .sort(); +}; + +describe('Axum: a handler written below `.route(` is linked to its route (#2326)', () => { + it('links a single-line route, as before', () => { + expect(routeEdges('src/main.rs')).toContain('GET /single -> src/handlers/mod.rs:single_line_handler'); + }); + + it('links a rustfmt-wrapped route', () => { + expect(routeEdges('src/main.rs')).toContain('GET /multi -> src/handlers/mod.rs:multi_line_handler'); + }); + + it('links a handler wrapped below its method router too', () => { + expect(routeEdges('src/main.rs')).toContain('GET /deep -> src/handlers/mod.rs:deep_line_handler'); + }); + + it('links every method of a chain wrapped over several lines', () => { + expect(routeEdges('src/main.rs')).toEqual(expect.arrayContaining([ + 'GET /items -> src/handlers/mod.rs:list_items', + 'POST /items -> src/handlers/mod.rs:create_item', + 'DELETE /items -> src/handlers/mod.rs:delete_item', + ])); + }); + + it('links a wrapped handler named through a `use`', () => { + expect(routeEdges('src/main.rs')).toContain('POST /used -> src/handlers/mod.rs:used_handler'); + }); + + it('does not link a wrapped handler another module names to a same-named one', () => { + expect(routeEdges('src/main.rs').filter((e) => e.includes('render_handler'))).toEqual([]); + }); + + it('links a single-line route whose line also writes the handler name elsewhere', () => { + // `/login` in the path, `delete(` as the method router: neither is how the handler is named. + expect(routeEdges('src/main.rs')).toEqual(expect.arrayContaining([ + 'POST /login -> src/handlers/mod.rs:login', + 'DELETE /account -> src/handlers/mod.rs:delete', + ])); + }); + + it('never reads a call inside a closure handler as a route', () => { + expect(routeEdges('src/main.rs').filter((e) => e.includes('closure'))).toEqual([]); + const routes = cg.getNodesInFile('src/main.rs').filter((n) => n.kind === 'route').map((n) => n.name); + expect(routes.filter((n) => n.includes('closure'))).toEqual([]); + }); + + it('keeps the route on the `.route(` line and puts the reference on the handler', () => { + const { nodes, references } = rustResolver.extract!('src/main.rs', MAIN_RS); + const lines = MAIN_RS.split('\n'); + const lineOf = (text: string) => lines.findIndex((l) => l.includes(text)) + 1; + const multi = nodes.find((n) => n.name === 'GET /multi')!; + expect(multi.startLine).toBe(lineOf('"/multi"') - 1); + const ref = references.find((r) => r.referenceName === 'multi_line_handler')!; + expect(ref.line).toBe(lineOf('handlers::multi_line_handler')); + expect(lines[ref.line - 1]!.startsWith('multi_line_handler', ref.column)).toBe(true); + const single = references.find((r) => r.referenceName === 'single_line_handler')!; + expect(single.line).toBe(nodes.find((n) => n.name === 'GET /single')!.startLine); + }); +}); + +describe('Actix: a handler written below its route is linked to it (#2326)', () => { + it('links a wrapped `web::resource(..).to(handler)`', () => { + expect(routeEdges('src/actix_app.rs')).toContain('ANY /index/with/a/rather/long/path/{id} -> src/handlers/mod.rs:actix_index'); + }); + + it('links a wrapped `web::get().to(handler)` inside `.route(`', () => { + expect(routeEdges('src/actix_app.rs')).toContain('GET /user/{id} -> src/handlers/mod.rs:actix_get_user'); + }); + + it('links a wrapped App-level `.route("/path", web::get().to(handler))`', () => { + expect(routeEdges('src/actix_app.rs')).toContain('GET /hey -> src/handlers/mod.rs:actix_hello'); + }); + + it('does not read the App-level route after a `.service(..)` into its resource', () => { + expect(routeEdges('src/actix_app.rs').filter((e) => e.startsWith('GET /user/{id}'))) + .toEqual(['GET /user/{id} -> src/handlers/mod.rs:actix_get_user']); + }); +}); diff --git a/src/resolution/frameworks/rust.ts b/src/resolution/frameworks/rust.ts index 1fa4f79d1..ffe15926a 100644 --- a/src/resolution/frameworks/rust.ts +++ b/src/resolution/frameworks/rust.ts @@ -107,6 +107,13 @@ export const rustResolver: FrameworkResolver = { const references: UnresolvedRef[] = []; const now = Date.now(); const safe = stripCommentsForRegex(content, 'rust'); + // Where a handler's own name is written, for the handler expression `expr` + // ending at `exprEnd`: the Rust scope gate reads the reference's line, and + // rustfmt wraps a long `.route(` call so its handler sits below it (#2326). + const handlerSite = (exprEnd: number, expr: string, handler: string) => { + const offset = exprEnd - expr.length + expr.lastIndexOf(handler); + return { line: safe.slice(0, offset).split('\n').length, column: offset - safe.lastIndexOf('\n', offset - 1) - 1 }; + }; // Actix-web / Rocket attribute: #[get("/path")] fn handler(..) // Capture the method, path, and the fn identifier that follows. @@ -165,12 +172,24 @@ export const rustResolver: FrameworkResolver = { const line = safe.slice(0, match.index).split('\n').length; const methodBody = args.slice(pathMatch[0].length); + const bodyAt = openIdx + 1 + pathMatch[0].length; const methodHandlerRegex = /\b(get|post|put|patch|delete|head|options|trace)\s*\(\s*([A-Za-z_][\w:]*)/g; let mh: RegExpExecArray | null; + // The method routers are the argument's top-level chain (`get(a).post(b)`); + // a `get(` nested in it — `get(|| async { cache.get(key) })` — is a call + // inside a closure handler, not a route. + let depth = 0; + let scanned = 0; while ((mh = methodHandlerRegex.exec(methodBody)) !== null) { + for (; scanned < mh.index; scanned++) { + if (methodBody[scanned] === '(') depth++; + else if (methodBody[scanned] === ')') depth--; + } + if (depth > 0) continue; const upper = mh[1]!.toUpperCase(); const handler = mh[2]!.split('::').filter(Boolean).pop(); if (!handler) continue; + const site = handlerSite(bodyAt + mh.index + mh[0].length, mh[2]!, handler); const routeNode: Node = { id: `route:${filePath}:${line}:${upper}:${routePath}`, @@ -191,8 +210,8 @@ export const rustResolver: FrameworkResolver = { fromNodeId: routeNode.id, referenceName: handler, referenceKind: 'references', - line, - column: 0, + line: site.line, + column: site.column, filePath, language: 'rust', }); @@ -201,9 +220,10 @@ export const rustResolver: FrameworkResolver = { // Actix-web builder API (the dominant actix routing style; attribute macros // are handled above). The handler lives in `.to(handler)`, not `get(handler)`. - const pushActixRoute = (routePath: string, method: string, handlerExpr: string, line: number) => { + const pushActixRoute = (routePath: string, method: string, handlerExpr: string, line: number, exprEnd: number) => { const handler = handlerExpr.split('::').filter(Boolean).pop(); if (!handler) return; + const site = handlerSite(exprEnd, handlerExpr, handler); const upper = method.toUpperCase(); const routeNode: Node = { id: `route:${filePath}:${line}:${upper}:${routePath}`, @@ -223,8 +243,8 @@ export const rustResolver: FrameworkResolver = { fromNodeId: routeNode.id, referenceName: handler, referenceKind: 'references', - line, - column: 0, + line: site.line, + column: site.column, filePath, language: 'rust', }); @@ -238,7 +258,16 @@ export const rustResolver: FrameworkResolver = { const after = match.index + match[0].length; // Bound the resource's method chain at the next resource() to avoid bleed. const nextRes = safe.indexOf('web::resource', after); - const end = Math.min(after + 500, nextRes === -1 ? safe.length : nextRes); + let end = Math.min(after + 500, nextRes === -1 ? safe.length : nextRes); + // ...and at the `)` of the call it is an argument of: in + // `.service(web::resource("/a").to(a)).route("/b", web::get().to(b))`, `b` is not `/a`'s. + for (let i = after, depth = 0; i < end; i++) { + if (safe[i] === '(') depth++; + else if (safe[i] === ')' && --depth < 0) { + end = i; + break; + } + } const chain = safe.slice(after, end); const methodTo = /web::(get|post|put|patch|delete|head)\s*\(\s*\)\s*\.to\s*\(\s*([A-Za-z_][\w:]*)/g; @@ -246,13 +275,13 @@ export const rustResolver: FrameworkResolver = { let found = false; while ((m2 = methodTo.exec(chain)) !== null) { const mLine = startLine + chain.slice(0, m2.index).split('\n').length - 1; - pushActixRoute(routePath, m2[1]!, m2[2]!, mLine); + pushActixRoute(routePath, m2[1]!, m2[2]!, mLine, after + m2.index + m2[0].length); found = true; } // Direct `.resource("/x").to(handler)` (all methods) when no explicit verb route. if (!found) { const direct = chain.match(/^\s*\.to\s*\(\s*([A-Za-z_][\w:]*)/); - if (direct) pushActixRoute(routePath, 'ANY', direct[1]!, startLine); + if (direct) pushActixRoute(routePath, 'ANY', direct[1]!, startLine, after + direct[0].length); } } @@ -260,7 +289,7 @@ export const rustResolver: FrameworkResolver = { const appRouteRegex = /\.route\s*\(\s*"([^"]+)"\s*,\s*web::(get|post|put|patch|delete|head)\s*\(\s*\)\s*\.to\s*\(\s*([A-Za-z_][\w:]*)/g; while ((match = appRouteRegex.exec(safe)) !== null) { const line = safe.slice(0, match.index).split('\n').length; - pushActixRoute(match[1]!, match[2]!, match[3]!, line); + pushActixRoute(match[1]!, match[2]!, match[3]!, line, match.index + match[0].length); } return { nodes, references };