mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
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 <ewidusoc498@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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<string, string> = {
|
||||
'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']);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user