diff --git a/src/core/extractors.ts b/src/core/extractors.ts index 8bcfd8e..0120eb4 100644 --- a/src/core/extractors.ts +++ b/src/core/extractors.ts @@ -59,7 +59,7 @@ export function extractStringValue(node: Node): string | null { * Extracts a path string from various AST node types. * Handles: plain strings, f-strings, concatenation, identifiers. */ -function extractPathFromNode(node: Node): string { +export function extractPathFromNode(node: Node): string { switch (node.type) { case "string": return extractStringValue(node) ?? "" diff --git a/src/core/importResolver.ts b/src/core/importResolver.ts index d345718..d8135f0 100644 --- a/src/core/importResolver.ts +++ b/src/core/importResolver.ts @@ -157,11 +157,18 @@ export async function resolveNamedImport( : modulePathToDir(importInfo, currentFileUri, projectRootUri, fs) for (const name of importInfo.names) { - // Try direct file: from .routes import users -> routes/users.py - const namedUri = fs.joinPath(baseDirUri, ...name.split(".")) - const resolved = await resolvePythonModule(namedUri, fs) - if (resolved) { - return resolved + // Only try submodule resolution if baseUri is a package (__init__.py) or namespace package (null). + // For regular .py files, the name is a variable inside the file, not a submodule. + // Example: "from .neon import router" where neon.py defines router + // should resolve to neon.py, not look for router.py + const isPackage = baseUri === null || baseUri.endsWith("__init__.py") + if (isPackage) { + // Try direct file: from .routes import users -> routes/users.py + const namedUri = fs.joinPath(baseDirUri, ...name.split(".")) + const resolved = await resolvePythonModule(namedUri, fs) + if (resolved) { + return resolved + } } // Try re-exports: from .routes import users where routes/__init__.py re-exports users diff --git a/src/core/pathUtils.ts b/src/core/pathUtils.ts index c2adaea..ba1f4fb 100644 --- a/src/core/pathUtils.ts +++ b/src/core/pathUtils.ts @@ -90,33 +90,52 @@ export function countSegments(path: string): number { /** * Checks if a test path matches an endpoint path pattern. - * Endpoint paths may contain path parameters like {item_id} which match any segment. + * Both paths may contain dynamic segments like {item_id} or {settings.API_V1_STR} + * which match any segment. + * + * Leading dynamic prefixes (like {settings.API_V1_STR}) and query strings are stripped + * before comparison. * * Examples: * pathMatchesEndpoint("/items/123", "/items/{item_id}") -> true * pathMatchesEndpoint("/items/123/details", "/items/{item_id}") -> false * pathMatchesEndpoint("/users/abc/posts/456", "/users/{user_id}/posts/{post_id}") -> true * pathMatchesEndpoint("/items/", "/items/{item_id}") -> false + * pathMatchesEndpoint("{settings.API}/apps/{id}", "/apps/{app_id}") -> true + * pathMatchesEndpoint("{BASE}/users/{id}", "/users/{user_id}") -> true + * pathMatchesEndpoint("/teams/?owner=true", "/teams") -> true (query string stripped) */ export function pathMatchesEndpoint( testPath: string, endpointPath: string, ): boolean { - const testSegments = testPath.split("/").filter(Boolean) - const endpointSegments = endpointPath.split("/").filter(Boolean) + // Strip query string from test path (e.g., "/teams/?owner=true" -> "/teams/") + const testPathWithoutQuery = testPath.split("?")[0] + + // Strip leading dynamic segments (e.g., {settings.API_V1_STR}) for comparison + const testSegments = stripLeadingDynamicSegments(testPathWithoutQuery) + .split("/") + .filter(Boolean) + const endpointSegments = stripLeadingDynamicSegments(endpointPath) + .split("/") + .filter(Boolean) // Segment counts must match if (testSegments.length !== endpointSegments.length) { return false } - return endpointSegments.every((seg, index) => { - // Path parameter (e.g., {item_id}) matches any segment - if (seg.startsWith("{") && seg.endsWith("}")) { + // Compare each segment positionally + return testSegments.every((testSeg, i) => { + const endpointSeg = endpointSegments[i] + // Dynamic segments (e.g., {id}, {app.id}) match any value + const testIsDynamic = testSeg.startsWith("{") && testSeg.endsWith("}") + const endpointIsDynamic = + endpointSeg.startsWith("{") && endpointSeg.endsWith("}") + if (testIsDynamic || endpointIsDynamic) { return true } - // Literal segments must match exactly - return seg === testSegments[index] + return testSeg === endpointSeg }) } diff --git a/src/core/routerResolver.ts b/src/core/routerResolver.ts index 3c995f0..f55a7eb 100644 --- a/src/core/routerResolver.ts +++ b/src/core/routerResolver.ts @@ -218,7 +218,7 @@ async function resolveRouterReference( if (localRouter) { // Filter routes that belong to this router (decorated with @router.method) const routerRoutes = analysis.routes.filter((r) => r.owner === moduleName) - return { + const routerNode: RouterNode = { filePath: currentFileUri, variableName: localRouter.variableName, type: localRouter.type, @@ -235,6 +235,39 @@ async function resolveRouterReference( })), children: [], } + + // Process include_router calls owned by this router (nested routers) + const routerIncludes = analysis.includeRouters.filter( + (inc) => inc.owner === moduleName, + ) + for (const include of routerIncludes) { + log( + `Resolving nested include_router: ${include.router} (owner: ${moduleName}, prefix: ${include.prefix || "none"})`, + ) + const childRouter = await resolveRouterReference( + include.router, + analysis, + currentFileUri, + projectRootUri, + parser, + fs, + visited, + ) + if (childRouter) { + if (include.tags.length > 0) { + childRouter.tags = [ + ...new Set([...childRouter.tags, ...include.tags]), + ] + } + routerNode.children.push({ + router: childRouter, + prefix: include.prefix, + tags: include.tags, + }) + } + } + + return routerNode } // Otherwise, look for an imported router @@ -250,11 +283,19 @@ async function resolveRouterReference( // Helper to analyze a file with the filesystem const analyzeFileFn = (uri: string) => analyzeFile(uri, parser, fs) + // Find the original import name (in case moduleName is an alias) + // e.g., "from .api_tokens import router as api_tokens_router" + // moduleName = "api_tokens_router", originalName = "router" + const namedImport = matchingImport.namedImports.find( + (ni) => (ni.alias ?? ni.name) === moduleName, + ) + const originalName = namedImport?.name ?? moduleName + // Resolve the imported module to a file URI const importedFileUri = await resolveNamedImport( { modulePath: matchingImport.modulePath, - names: [moduleName], + names: [originalName], isRelative: matchingImport.isRelative, relativeDots: matchingImport.relativeDots, }, @@ -293,7 +334,7 @@ async function resolveRouterReference( const routerRoutes = importedAnalysis.routes.filter( (r) => r.owner === attributeName, ) - return { + const routerNode: RouterNode = { filePath: importedFileUri, variableName: targetRouter.variableName, type: targetRouter.type, @@ -310,6 +351,39 @@ async function resolveRouterReference( })), children: [], } + + // Process include_router calls owned by this router (nested routers) + const routerIncludes = importedAnalysis.includeRouters.filter( + (inc) => inc.owner === attributeName, + ) + for (const include of routerIncludes) { + log( + `Resolving nested include_router: ${include.router} (owner: ${attributeName}, prefix: ${include.prefix || "none"})`, + ) + const childRouter = await resolveRouterReference( + include.router, + importedAnalysis, + importedFileUri, + projectRootUri, + parser, + fs, + visited, + ) + if (childRouter) { + if (include.tags.length > 0) { + childRouter.tags = [ + ...new Set([...childRouter.tags, ...include.tags]), + ] + } + routerNode.children.push({ + router: childRouter, + prefix: include.prefix, + tags: include.tags, + }) + } + } + + return routerNode } // If not found as a router, fall through to try building from file } diff --git a/src/providers/testCodeLensProvider.ts b/src/providers/testCodeLensProvider.ts index 10f82c5..a45a570 100644 --- a/src/providers/testCodeLensProvider.ts +++ b/src/providers/testCodeLensProvider.ts @@ -14,7 +14,7 @@ import { Uri, } from "vscode" import type { Node } from "web-tree-sitter" -import { extractStringValue, findNodesByType } from "../core/extractors" +import { extractPathFromNode, findNodesByType } from "../core/extractors" import { ROUTE_METHODS } from "../core/internal" import type { Parser } from "../core/parser" import { @@ -132,9 +132,8 @@ export class TestCodeLensProvider implements CodeLensProvider { } const pathArg = args[0] - // Only handle string literals for now - const path = extractStringValue(pathArg) - if (path === null) { + const path = extractPathFromNode(pathArg) + if (!path) { continue } diff --git a/src/test/core/importResolver.test.ts b/src/test/core/importResolver.test.ts index 5ae6b97..1786709 100644 --- a/src/test/core/importResolver.test.ts +++ b/src/test/core/importResolver.test.ts @@ -241,5 +241,32 @@ suite("importResolver", () => { assert.ok(result) assert.ok(result.endsWith("api_routes.py")) }) + + test("resolves variable import from .py file (not submodule)", async () => { + // This tests "from .neon import router" where router is a variable in neon.py, + // NOT a submodule. Should return neon.py, not look for router.py + const reexportRoot = fixtures.reexport.root + const currentFile = nodeFileSystem.joinPath( + reexportRoot, + "app", + "integrations", + "router.py", + ) + + const result = await resolveNamedImport( + { + modulePath: "neon", + names: ["router"], + isRelative: true, + relativeDots: 1, + }, + currentFile, + reexportRoot, + nodeFileSystem, + ) + + assert.ok(result, "Should resolve import") + assert.ok(result.endsWith("neon.py"), `Expected neon.py, got ${result}`) + }) }) }) diff --git a/src/test/core/pathUtils.test.ts b/src/test/core/pathUtils.test.ts index cac37ea..e8d1376 100644 --- a/src/test/core/pathUtils.test.ts +++ b/src/test/core/pathUtils.test.ts @@ -271,19 +271,49 @@ suite("pathUtils", () => { ) }) - test("matches paths with dynamic prefix", () => { - // Dynamic prefixes like {settings.API_V1_STR} match any segment (same as path params) + test("matches paths with dynamic prefix in endpoint", () => { assert.strictEqual( pathMatchesEndpoint( - "/v1/items/123", + "/items/123", "{settings.API_V1_STR}/items/{item_id}", ), true, ) assert.strictEqual( pathMatchesEndpoint("/api/v2/users", "{BASE}/users"), - false, // segment count differs + false, + ) + assert.strictEqual(pathMatchesEndpoint("/users", "{BASE}/users"), true) + assert.strictEqual(pathMatchesEndpoint("/items", "{BASE}/users"), false) + }) + + test("matches paths with dynamic prefix in test path (f-strings)", () => { + assert.strictEqual( + pathMatchesEndpoint( + "{settings.API_V1_STR}/apps/{app.id}/environment-variables", + "/apps/{app_id}/environment-variables", + ), + true, + ) + assert.strictEqual( + pathMatchesEndpoint( + "{settings.API}/items/{item_id}", + "{BASE}/items/{id}", + ), + true, + ) + assert.strictEqual( + pathMatchesEndpoint("{BASE}/users/{id}", "/items/{item_id}"), + false, + ) + }) + + test("strips query strings from test path", () => { + assert.strictEqual( + pathMatchesEndpoint("/teams/?owner=true&order_by=created_at", "/teams"), + true, ) + assert.strictEqual(pathMatchesEndpoint("/?page=1", "/"), true) }) }) }) diff --git a/src/test/core/routerResolver.test.ts b/src/test/core/routerResolver.test.ts index 604bf24..e0949fc 100644 --- a/src/test/core/routerResolver.test.ts +++ b/src/test/core/routerResolver.test.ts @@ -151,7 +151,6 @@ suite("routerResolver", () => { }) test("follows __init__.py re-exports to actual router file", async () => { - // Use reexport fixture which has integrations/__init__.py re-exporting from router.py const result = await buildRouterGraph( fixtures.reexport.initPy, parser, @@ -163,16 +162,26 @@ suite("routerResolver", () => { assert.strictEqual(result.type, "APIRouter") assert.strictEqual(result.variableName, "router") - // Should point to router.py, not __init__.py assert.ok( result.filePath.endsWith("router.py"), `Expected filePath to end with router.py, got ${result.filePath}`, ) - // Should have the routes defined in router.py (3 routes: github, slack, webhook) assert.ok(result.routes.length >= 3, "Should have routes from router.py") const githubRoute = result.routes.find((r) => r.path === "/github") assert.ok(githubRoute, "Should find github route") + + assert.strictEqual( + result.children.length, + 1, + "Should have one nested router (neon)", + ) + const neonChild = result.children[0] + assert.strictEqual(neonChild.router.prefix, "/neon") + assert.ok( + neonChild.router.routes.length >= 2, + "neon router should have routes", + ) }) test("includes router when following include_router chain", async () => { @@ -303,5 +312,146 @@ suite("routerResolver", () => { assert.strictEqual(result, null) }) + + test("resolves aliased import (from .tokens import router as tokens_router)", async () => { + const result = await buildRouterGraph( + fixtures.aliasedImport.mainPy, + parser, + fixtures.aliasedImport.root, + nodeFileSystem, + ) + + assert.ok(result) + assert.strictEqual(result.type, "FastAPI") + assert.strictEqual(result.variableName, "app") + + assert.strictEqual( + result.children.length, + 1, + "Should have one child router", + ) + + const tokensRouter = result.children[0] + assert.strictEqual(tokensRouter.router.type, "APIRouter") + assert.strictEqual(tokensRouter.router.prefix, "/tokens") + + assert.ok( + tokensRouter.router.routes.length >= 3, + `Expected at least 3 routes, got ${tokensRouter.router.routes.length}`, + ) + + assert.ok( + tokensRouter.router.filePath.endsWith("tokens.py"), + `Expected filePath to end with tokens.py, got ${tokensRouter.router.filePath}`, + ) + }) + + test("discovers nested routers (router.include_router)", async () => { + const result = await buildRouterGraph( + fixtures.nestedRouter.mainPy, + parser, + fixtures.nestedRouter.root, + nodeFileSystem, + ) + + assert.ok(result) + assert.strictEqual(result.type, "FastAPI") + assert.strictEqual(result.variableName, "app") + + // App includes apps_router with /api prefix + assert.strictEqual( + result.children.length, + 1, + "Should have one child router (apps)", + ) + + const appsChild = result.children[0] + assert.strictEqual(appsChild.prefix, "/api") + assert.strictEqual(appsChild.router.prefix, "/apps") + assert.strictEqual(appsChild.router.variableName, "router") + + // Apps router should have its direct routes + const appsRoutes = appsChild.router.routes.map((r) => r.path) + assert.ok(appsRoutes.includes("/"), "apps router should have / route") + assert.ok( + appsRoutes.includes("/{app_id}"), + "apps router should have /{app_id} route", + ) + + // Apps router includes tokens_router and settings_router (nested) + assert.strictEqual( + appsChild.router.children.length, + 2, + "apps router should have 2 nested routers", + ) + + const childPrefixes = appsChild.router.children.map( + (c) => c.router.prefix, + ) + assert.ok( + childPrefixes.includes("/{app_id}/tokens"), + "Should have tokens router", + ) + assert.ok( + childPrefixes.includes("/{app_id}/settings"), + "Should have settings router", + ) + + // Verify nested routers have their routes + const tokensChild = appsChild.router.children.find( + (c) => c.router.prefix === "/{app_id}/tokens", + ) + assert.ok(tokensChild) + assert.ok( + tokensChild.router.routes.length >= 2, + "tokens router should have routes", + ) + + const settingsChild = appsChild.router.children.find( + (c) => c.router.prefix === "/{app_id}/settings", + ) + assert.ok(settingsChild) + assert.ok( + settingsChild.router.routes.length >= 2, + "settings router should have routes", + ) + }) + + test("discovers nested routers via __init__.py re-export", async () => { + // This tests the pattern: main.py imports from integrations (package), + // integrations/__init__.py re-exports router from router.py, + // router.py has include_router calls for nested routers + const result = await buildRouterGraph( + fixtures.reexport.mainPy, + parser, + fixtures.reexport.root, + nodeFileSystem, + ) + + assert.ok(result) + assert.strictEqual(result.type, "FastAPI") + + assert.strictEqual( + result.children.length, + 1, + "Should have one child router (integrations)", + ) + + const integrationsChild = result.children[0] + assert.strictEqual(integrationsChild.router.prefix, "/integrations") + + assert.strictEqual( + integrationsChild.router.children.length, + 1, + "integrations router should have nested neon router", + ) + + const neonChild = integrationsChild.router.children[0] + assert.strictEqual(neonChild.router.prefix, "/neon") + assert.ok( + neonChild.router.routes.length >= 2, + "neon router should have routes", + ) + }) }) }) diff --git a/src/test/fixtures/aliased-import/app/__init__.py b/src/test/fixtures/aliased-import/app/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/test/fixtures/aliased-import/app/main.py b/src/test/fixtures/aliased-import/app/main.py new file mode 100644 index 0000000..4454be7 --- /dev/null +++ b/src/test/fixtures/aliased-import/app/main.py @@ -0,0 +1,12 @@ +from fastapi import FastAPI + +from .routes.tokens import router as tokens_router + +app = FastAPI(title="Aliased Import Test") + +app.include_router(tokens_router) + + +@app.get("/") +def root(): + return {"message": "Hello"} diff --git a/src/test/fixtures/aliased-import/app/routes/__init__.py b/src/test/fixtures/aliased-import/app/routes/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/test/fixtures/aliased-import/app/routes/tokens.py b/src/test/fixtures/aliased-import/app/routes/tokens.py new file mode 100644 index 0000000..a5e1fa2 --- /dev/null +++ b/src/test/fixtures/aliased-import/app/routes/tokens.py @@ -0,0 +1,18 @@ +from fastapi import APIRouter + +router = APIRouter(prefix="/tokens", tags=["tokens"]) + + +@router.get("/") +def list_tokens(): + return [] + + +@router.post("/") +def create_token(): + return {"id": 1} + + +@router.delete("/{token_id}") +def delete_token(token_id: int): + return {"deleted": token_id} diff --git a/src/test/fixtures/nested-router/app/__init__.py b/src/test/fixtures/nested-router/app/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/test/fixtures/nested-router/app/main.py b/src/test/fixtures/nested-router/app/main.py new file mode 100644 index 0000000..0e54274 --- /dev/null +++ b/src/test/fixtures/nested-router/app/main.py @@ -0,0 +1,13 @@ +from fastapi import FastAPI + +from .routes import apps + +app = FastAPI(title="Nested Router Test") + +# Use dotted reference like real codebase: apps.router +app.include_router(apps.router, prefix="/api") + + +@app.get("/") +def root(): + return {"message": "Hello"} diff --git a/src/test/fixtures/nested-router/app/routes/__init__.py b/src/test/fixtures/nested-router/app/routes/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/src/test/fixtures/nested-router/app/routes/apps.py b/src/test/fixtures/nested-router/app/routes/apps.py new file mode 100644 index 0000000..194c6e3 --- /dev/null +++ b/src/test/fixtures/nested-router/app/routes/apps.py @@ -0,0 +1,21 @@ +from fastapi import APIRouter + +from .tokens import router as tokens_router +from .settings import router as settings_router + +router = APIRouter(prefix="/apps", tags=["apps"]) + + +@router.get("/") +def list_apps(): + return [] + + +@router.get("/{app_id}") +def get_app(app_id: int): + return {"id": app_id} + + +# Nested routers - apps router includes tokens and settings routers +router.include_router(tokens_router) +router.include_router(settings_router) diff --git a/src/test/fixtures/nested-router/app/routes/settings.py b/src/test/fixtures/nested-router/app/routes/settings.py new file mode 100644 index 0000000..9224911 --- /dev/null +++ b/src/test/fixtures/nested-router/app/routes/settings.py @@ -0,0 +1,13 @@ +from fastapi import APIRouter + +router = APIRouter(prefix="/{app_id}/settings", tags=["settings"]) + + +@router.get("/") +def get_settings(app_id: int): + return {} + + +@router.put("/") +def update_settings(app_id: int): + return {} diff --git a/src/test/fixtures/nested-router/app/routes/tokens.py b/src/test/fixtures/nested-router/app/routes/tokens.py new file mode 100644 index 0000000..6f6b660 --- /dev/null +++ b/src/test/fixtures/nested-router/app/routes/tokens.py @@ -0,0 +1,13 @@ +from fastapi import APIRouter + +router = APIRouter(prefix="/{app_id}/tokens", tags=["tokens"]) + + +@router.get("/") +def list_tokens(app_id: int): + return [] + + +@router.post("/") +def create_token(app_id: int): + return {"id": 1} diff --git a/src/test/fixtures/reexport/app/integrations/neon.py b/src/test/fixtures/reexport/app/integrations/neon.py new file mode 100644 index 0000000..052048c --- /dev/null +++ b/src/test/fixtures/reexport/app/integrations/neon.py @@ -0,0 +1,13 @@ +from fastapi import APIRouter + +router = APIRouter(prefix="/neon", tags=["neon"]) + + +@router.get("/") +def get_neon(): + return {"provider": "neon"} + + +@router.post("/connect") +def connect_neon(): + return {"connected": True} diff --git a/src/test/fixtures/reexport/app/integrations/router.py b/src/test/fixtures/reexport/app/integrations/router.py index 5af9d05..f476178 100644 --- a/src/test/fixtures/reexport/app/integrations/router.py +++ b/src/test/fixtures/reexport/app/integrations/router.py @@ -1,7 +1,12 @@ from fastapi import APIRouter +from .neon import router as neon_router + router = APIRouter(prefix="/integrations", tags=["integrations"]) +# Nested router +router.include_router(neon_router) + @router.get("/github") def github_integration(): diff --git a/src/test/layouts.test.ts b/src/test/layouts.test.ts index a2e9f04..fbdc1e1 100644 --- a/src/test/layouts.test.ts +++ b/src/test/layouts.test.ts @@ -174,11 +174,12 @@ suite("Project Layouts", () => { const appDef = routerNodeToAppDefinition(graph, fixtures.reexport.root) const allRoutes = collectAllRoutes(appDef) - // Should have: GET /, GET /integrations/github, GET /integrations/slack, POST /integrations/webhook + // Should have: GET /, GET /integrations/github, GET /integrations/slack, POST /integrations/webhook, + // and nested neon routes: GET /integrations/neon/, POST /integrations/neon/connect assert.strictEqual( allRoutes.length, - 4, - `Expected 4 routes, got ${allRoutes.length}`, + 6, + `Expected 6 routes, got ${allRoutes.length}`, ) const paths = allRoutes.map((r) => `${r.method} ${r.path}`) @@ -198,5 +199,13 @@ suite("Project Layouts", () => { paths.some((p) => p === "POST /integrations/webhook"), "Should have POST /integrations/webhook", ) + assert.ok( + paths.some((p) => p === "GET /integrations/neon/"), + "Should have GET /integrations/neon/", + ) + assert.ok( + paths.some((p) => p === "POST /integrations/neon/connect"), + "Should have POST /integrations/neon/connect", + ) }) }) diff --git a/src/test/testUtils.ts b/src/test/testUtils.ts index 2af426b..7ac2b71 100644 --- a/src/test/testUtils.ts +++ b/src/test/testUtils.ts @@ -49,6 +49,26 @@ export const fixtures = { root: uri(join(fixturesPath, "multi-app")), mainPy: uri(join(fixturesPath, "multi-app", "main.py")), }, + aliasedImport: { + root: uri(join(fixturesPath, "aliased-import")), + mainPy: uri(join(fixturesPath, "aliased-import", "app", "main.py")), + tokensPy: uri( + join(fixturesPath, "aliased-import", "app", "routes", "tokens.py"), + ), + }, + nestedRouter: { + root: uri(join(fixturesPath, "nested-router")), + mainPy: uri(join(fixturesPath, "nested-router", "app", "main.py")), + appsPy: uri( + join(fixturesPath, "nested-router", "app", "routes", "apps.py"), + ), + tokensPy: uri( + join(fixturesPath, "nested-router", "app", "routes", "tokens.py"), + ), + settingsPy: uri( + join(fixturesPath, "nested-router", "app", "routes", "settings.py"), + ), + }, } /**