diff --git a/src/core/analyzer.ts b/src/core/analyzer.ts index 5822052..736834e 100644 --- a/src/core/analyzer.ts +++ b/src/core/analyzer.ts @@ -42,7 +42,7 @@ export function analyzeTree(tree: Tree, filePath: string): FileAnalysis { // Get all decorated definitions (functions and classes with decorators) const decoratedDefs = nodesByType.get("decorated_definition") ?? [] - const routes = decoratedDefs.map(decoratorExtractor).filter(notNull) + const routes = decoratedDefs.flatMap(decoratorExtractor) // Get all router assignments const assignments = nodesByType.get("assignment") ?? [] diff --git a/src/core/extractors.ts b/src/core/extractors.ts index a9cc2df..081c220 100644 --- a/src/core/extractors.ts +++ b/src/core/extractors.ts @@ -167,66 +167,14 @@ export function extractPathFromNode(node: Node): string { /** * Extracts from route decorators like @app.get("/path"), @router.post("/path"), etc. + * Handles stacked decorators — returns one RouteInfo per route decorator found. */ -export function decoratorExtractor(node: Node): RouteInfo | null { +export function decoratorExtractor(node: Node): RouteInfo[] { if (node.type !== "decorated_definition") { - return null - } - - // Grammar guarantees: decorated_definition always has a first child (the decorator) - const decoratorNode = node.firstNamedChild! - - const callNode = - decoratorNode.firstNamedChild?.type === "call" - ? decoratorNode.firstNamedChild - : null - - const functionNode = callNode?.childForFieldName("function") - const argumentsNode = callNode?.childForFieldName("arguments") - const objectNode = functionNode?.childForFieldName("object") - const methodNode = functionNode?.childForFieldName("attribute") - - if (!objectNode || !methodNode || !argumentsNode) { - return null - } - - // Filter out non-route decorators (exception_handler, middleware, on_event) - const method = methodNode.text.toLowerCase() - const isApiRoute = method === "api_route" - if (!ROUTE_METHODS.has(method) && !isApiRoute) { - return null + return [] } - // Find path: first positional arg, or "path" keyword argument - const nonCommentArgs = argumentsNode.namedChildren.filter( - (child) => child.type !== "comment", - ) - const pathArgNode = resolveArgNode(nonCommentArgs, 0, "path") - const path = pathArgNode ? extractPathFromNode(pathArgNode) : "" - - // For api_route, extract methods from keyword argument - let resolvedMethod = methodNode.text - if (isApiRoute) { - // Default to GET if no methods specified - resolvedMethod = "GET" - for (const argNode of argumentsNode.namedChildren) { - if (argNode.type === "keyword_argument") { - const nameNode = argNode.childForFieldName("name") - const valueNode = argNode.childForFieldName("value") - if (nameNode?.text === "methods" && valueNode) { - // Extract first method from list - const listItems = valueNode.namedChildren - const firstMethod = - listItems.length > 0 ? extractStringValue(listItems[0]) : null - if (firstMethod) { - resolvedMethod = firstMethod - } - } - } - } - } - - // Grammar guarantees: decorated_definition always has a definition field with a name + // Shared across all stacked decorators: function name and docstring const functionDefNode = node.childForFieldName("definition")! const functionName = functionDefNode.childForFieldName("name")?.text ?? "" const functionBody = functionDefNode.childForFieldName("body") @@ -238,15 +186,76 @@ export function decoratorExtractor(node: Node): RouteInfo | null { docstring = stripDocstring(expr.text) } } - return { - owner: objectNode.text, - method: resolvedMethod, - path, - function: functionName, - line: node.startPosition.row + 1, - column: node.startPosition.column, - docstring, + + const routes: RouteInfo[] = [] + + for (const decoratorNode of node.namedChildren) { + if (decoratorNode.type !== "decorator") { + continue + } + + const callNode = + decoratorNode.firstNamedChild?.type === "call" + ? decoratorNode.firstNamedChild + : null + + const functionNode = callNode?.childForFieldName("function") + const argumentsNode = callNode?.childForFieldName("arguments") + const objectNode = functionNode?.childForFieldName("object") + const methodNode = functionNode?.childForFieldName("attribute") + + if (!objectNode || !methodNode || !argumentsNode) { + continue + } + + // Filter out non-route decorators (exception_handler, middleware, on_event) + const method = methodNode.text.toLowerCase() + const isApiRoute = method === "api_route" + if (!ROUTE_METHODS.has(method) && !isApiRoute) { + continue + } + + // Find path: first positional arg, or "path" keyword argument + const nonCommentArgs = argumentsNode.namedChildren.filter( + (child) => child.type !== "comment", + ) + const pathArgNode = resolveArgNode(nonCommentArgs, 0, "path") + const path = pathArgNode ? extractPathFromNode(pathArgNode) : "" + + let deprecated: boolean | undefined + let resolvedMethod = methodNode.text + if (isApiRoute) resolvedMethod = "GET" + + for (const argNode of argumentsNode.namedChildren) { + if (argNode.type !== "keyword_argument") continue + const nameNode = argNode.childForFieldName("name") + const valueNode = argNode.childForFieldName("value") + if (nameNode?.text === "deprecated" && valueNode?.text === "True") { + deprecated = true + } + if (isApiRoute && nameNode?.text === "methods" && valueNode) { + // Extract first method from list + const firstMethod = + valueNode.namedChildren.length > 0 + ? extractStringValue(valueNode.namedChildren[0]) + : null + if (firstMethod) resolvedMethod = firstMethod + } + } + + routes.push({ + owner: objectNode.text, + method: resolvedMethod, + path, + function: functionName, + line: node.startPosition.row + 1, + column: node.startPosition.column, + docstring, + deprecated, + }) } + + return routes } /** Extracts tags from a list node like ["users", "admin"] */ diff --git a/src/core/internal.ts b/src/core/internal.ts index ede8b20..a282b62 100644 --- a/src/core/internal.ts +++ b/src/core/internal.ts @@ -36,6 +36,7 @@ export interface RouteInfo { line: number column: number docstring?: string + deprecated?: boolean } export type RouterType = "APIRouter" | "FastAPI" | "Unknown" diff --git a/src/core/routerResolver.ts b/src/core/routerResolver.ts index c7a11bd..91cd45e 100644 --- a/src/core/routerResolver.ts +++ b/src/core/routerResolver.ts @@ -50,14 +50,7 @@ function createRouterNode( tags: router.tags, line: router.line, column: router.column, - routes: routes.map((r) => ({ - method: r.method, - path: r.path, - function: r.function, - line: r.line, - column: r.column, - docstring: r.docstring, - })), + routes: routes.map(({ owner: _, ...rest }) => rest), children: [], } } diff --git a/src/core/transformer.ts b/src/core/transformer.ts index 0289fb6..f5902dc 100644 --- a/src/core/transformer.ts +++ b/src/core/transformer.ts @@ -21,6 +21,7 @@ function toRouteDefinition( path: prefix + route.path, functionName: route.function, docstring: route.docstring, + deprecated: route.deprecated, location: { filePath, line: route.line, diff --git a/src/core/types.ts b/src/core/types.ts index 1f95c47..5019f64 100644 --- a/src/core/types.ts +++ b/src/core/types.ts @@ -25,6 +25,7 @@ export interface RouteDefinition { functionName: string location: SourceLocation docstring?: string + deprecated?: boolean } export interface RouterDefinition { diff --git a/src/test/core/extractors.test.ts b/src/test/core/extractors.test.ts index e85bd48..5a46257 100644 --- a/src/test/core/extractors.test.ts +++ b/src/test/core/extractors.test.ts @@ -46,11 +46,12 @@ def list_users(): assert.strictEqual(decoratedDefs.length, 1) const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.owner, "router") - assert.strictEqual(result.method, "get") - assert.strictEqual(result.path, "/users") - assert.strictEqual(result.function, "list_users") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.owner, "router") + assert.strictEqual(route.method, "get") + assert.strictEqual(route.path, "/users") + assert.strictEqual(route.function, "list_users") }) test("extracts route with path parameter", () => { @@ -64,8 +65,9 @@ def get_user(user_id: int): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "/users/{user_id}") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.path, "/users/{user_id}") }) test("extracts POST route", () => { @@ -79,10 +81,11 @@ def create_item(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.owner, "app") - assert.strictEqual(result.method, "post") - assert.strictEqual(result.path, "/items") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.owner, "app") + assert.strictEqual(route.method, "post") + assert.strictEqual(route.path, "/items") }) test("extracts websocket route", () => { @@ -96,9 +99,10 @@ def websocket_handler(websocket: WebSocket): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.method, "websocket") - assert.strictEqual(result.path, "/ws") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.method, "websocket") + assert.strictEqual(route.path, "/ws") }) test("handles dynamic path with variable", () => { @@ -112,8 +116,9 @@ def handler(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "\uE000BASE_PATH\uE000") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.path, "\uE000BASE_PATH\uE000") }) test("handles dynamic path with attribute", () => { @@ -127,8 +132,9 @@ def handler(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "\uE000settings.API_PREFIX\uE000") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.path, "\uE000settings.API_PREFIX\uE000") }) test("handles path concatenation", () => { @@ -142,11 +148,12 @@ def handler(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "\uE000BASE\uE000/users") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.path, "\uE000BASE\uE000/users") }) - test("returns null for simple decorator without call", () => { + test("returns empty array for simple decorator without call", () => { const code = ` @staticmethod def handler(): @@ -156,10 +163,10 @@ def handler(): const nodesByType = getNodesByType(tree.rootNode) const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.strictEqual(result, null) + assert.strictEqual(result.length, 0) }) - test("returns null for non-route decorator", () => { + test("returns empty array for non-route decorator", () => { const code = ` @app.exception_handler(404) def not_found(request, exc): @@ -169,7 +176,7 @@ def not_found(request, exc): const nodesByType = getNodesByType(tree.rootNode) const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.strictEqual(result, null) + assert.strictEqual(result.length, 0) }) test("extracts api_route decorator", () => { @@ -183,9 +190,10 @@ def handle_items(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.method, "POST") - assert.strictEqual(result.path, "/items") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.method, "POST") + assert.strictEqual(route.path, "/items") }) test("extracts api_route with default GET when no methods specified", () => { @@ -199,8 +207,9 @@ def handle_items(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.method, "GET") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.method, "GET") }) test("extracts route with 'path' keyword argument", () => { @@ -214,11 +223,12 @@ def get_user(user_id: int): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "/users/{user_id}") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.path, "/users/{user_id}") }) - test("returns null for non-decorated definition", () => { + test("returns empty array for non-decorated definition", () => { const code = ` def regular_function(): pass @@ -228,7 +238,7 @@ def regular_function(): const funcDefs = nodesByType.get("function_definition") ?? [] const result = decoratorExtractor(funcDefs[0]) - assert.strictEqual(result, null) + assert.strictEqual(result.length, 0) }) test("includes line and column information", () => { @@ -242,9 +252,10 @@ def handler(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.line, 2) // 1-indexed - assert.strictEqual(result.column, 0) + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.line, 2) // 1-indexed + assert.strictEqual(route.column, 0) }) test("extracts single-line docstring", () => { @@ -259,8 +270,9 @@ def list_users(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.docstring, "List all users.") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.docstring, "List all users.") }) test("extracts multi-line docstring and dedents", () => { @@ -279,9 +291,10 @@ def list_users(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) + assert.strictEqual(result.length, 1) + const route = result[0] assert.strictEqual( - result.docstring, + route.docstring, "List all users.\n\nReturns a list of user objects.", ) }) @@ -298,8 +311,9 @@ def list_users(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.docstring, "List all users.") + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.docstring, "List all users.") }) test("returns undefined docstring when none present", () => { @@ -313,8 +327,9 @@ def list_users(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.docstring, undefined) + assert.strictEqual(result.length, 1) + const route = result[0] + assert.strictEqual(route.docstring, undefined) }) }) @@ -951,8 +966,8 @@ def handler(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "/api/v1/users") + assert.strictEqual(result.length, 1) + assert.strictEqual(result[0].path, "/api/v1/users") }) test("handles function call as path", () => { @@ -966,8 +981,61 @@ def handler(): const decoratedDefs = nodesByType.get("decorated_definition") ?? [] const result = decoratorExtractor(decoratedDefs[0]) - assert.ok(result) - assert.strictEqual(result.path, "\uE000get_path()\uE000") + assert.strictEqual(result.length, 1) + assert.strictEqual(result[0].path, "\uE000get_path()\uE000") + }) + + test("extracts both routes from stacked decorators", () => { + const code = ` +@router.post("/vendor-order", deprecated=True) +@router.post("/lab/orders") +async def post_order(): + pass +` + const tree = parse(code) + const nodesByType = getNodesByType(tree.rootNode) + const decoratedDefs = nodesByType.get("decorated_definition") ?? [] + const result = decoratorExtractor(decoratedDefs[0]) + + assert.strictEqual(result.length, 2) + assert.strictEqual(result[0].path, "/vendor-order") + assert.strictEqual(result[0].method, "post") + assert.strictEqual(result[0].deprecated, true) + assert.strictEqual(result[1].path, "/lab/orders") + assert.strictEqual(result[1].method, "post") + assert.strictEqual(result[1].deprecated, undefined) + assert.strictEqual(result[0].function, "post_order") + assert.strictEqual(result[1].function, "post_order") + }) + + test("extracts deprecated=True flag", () => { + const code = ` +@router.get("/old-path", deprecated=True) +def handler(): + pass +` + const tree = parse(code) + const nodesByType = getNodesByType(tree.rootNode) + const decoratedDefs = nodesByType.get("decorated_definition") ?? [] + const result = decoratorExtractor(decoratedDefs[0]) + + assert.strictEqual(result.length, 1) + assert.strictEqual(result[0].deprecated, true) + }) + + test("does not set deprecated when not present", () => { + const code = ` +@router.get("/path") +def handler(): + pass +` + const tree = parse(code) + const nodesByType = getNodesByType(tree.rootNode) + const decoratedDefs = nodesByType.get("decorated_definition") ?? [] + const result = decoratorExtractor(decoratedDefs[0]) + + assert.strictEqual(result.length, 1) + assert.strictEqual(result[0].deprecated, undefined) }) }) }) diff --git a/src/test/providers/pathOperationTreeProvider.test.ts b/src/test/providers/pathOperationTreeProvider.test.ts index 3ee8901..0c9c077 100644 --- a/src/test/providers/pathOperationTreeProvider.test.ts +++ b/src/test/providers/pathOperationTreeProvider.test.ts @@ -517,4 +517,75 @@ suite("PathOperationTreeProvider", () => { const parent = p.getParent({ type: "router", router: childRouter }) assert.strictEqual(parent?.type, "router") }) + + test("deprecated route shows '· deprecated' in description", () => { + const route: RouteDefinition = { + method: "POST", + path: "/vendor-order", + functionName: "post_order", + deprecated: true, + location: { filePath: "test.py", line: 1, column: 0 }, + } + const p = new PathOperationTreeProvider(testExtUri, []) + const treeItem = p.getTreeItem({ type: "route", route }) + assert.ok( + treeItem.description?.toString().includes("deprecated"), + "Description should include 'deprecated'", + ) + assert.ok( + treeItem.description?.toString().includes("post_order"), + "Description should include function name", + ) + }) + + test("non-deprecated route description shows only function name", () => { + const route: RouteDefinition = { + method: "GET", + path: "/path", + functionName: "get_handler", + location: { filePath: "test.py", line: 1, column: 0 }, + } + const p = new PathOperationTreeProvider(testExtUri, []) + const treeItem = p.getTreeItem({ type: "route", route }) + assert.strictEqual(treeItem.description, "get_handler") + }) + + test("deprecated route tooltip includes deprecated warning", () => { + const route: RouteDefinition = { + method: "POST", + path: "/vendor-order", + functionName: "post_order", + deprecated: true, + location: { filePath: "test.py", line: 1, column: 0 }, + } + const p = new PathOperationTreeProvider(testExtUri, []) + const treeItem = p.getTreeItem({ type: "route", route }) + const tooltipValue = + typeof treeItem.tooltip === "string" + ? treeItem.tooltip + : (treeItem.tooltip as { value: string }).value + assert.ok( + tooltipValue.includes("Deprecated"), + "Tooltip should include 'Deprecated'", + ) + }) + + test("non-deprecated route tooltip does not include deprecated warning", () => { + const route: RouteDefinition = { + method: "GET", + path: "/path", + functionName: "get_handler", + location: { filePath: "test.py", line: 1, column: 0 }, + } + const p = new PathOperationTreeProvider(testExtUri, []) + const treeItem = p.getTreeItem({ type: "route", route }) + const tooltipValue = + typeof treeItem.tooltip === "string" + ? treeItem.tooltip + : (treeItem.tooltip as { value: string }).value + assert.ok( + !tooltipValue.includes("Deprecated"), + "Tooltip should not include 'Deprecated'", + ) + }) }) diff --git a/src/vscode/pathOperationTreeProvider.ts b/src/vscode/pathOperationTreeProvider.ts index f3447da..35d7298 100644 --- a/src/vscode/pathOperationTreeProvider.ts +++ b/src/vscode/pathOperationTreeProvider.ts @@ -271,22 +271,30 @@ export class PathOperationTreeProvider case "route": { const routeItem = new TreeItem(getRouteLabel(element.route)) - routeItem.description = element.route.functionName + routeItem.description = element.route.deprecated + ? `${element.route.functionName} · deprecated` + : element.route.functionName routeItem.iconPath = getMethodSvgIcon( this.extensionUri, element.route.method, ) routeItem.contextValue = "route" const tooltipPath = stripLeadingDynamicSegments(element.route.path) + const deprecatedSection = element.route.deprecated + ? "\n\n$(warning) **Deprecated**" + : "" const docstringSection = element.route.docstring ? `\n\n---\n\n${element.route.docstring}` : "" - routeItem.tooltip = new MarkdownString( - `**${element.route.method}** \`${tooltipPath}\`\n\n` + - `**Function:** \`${element.route.functionName}\`\n\n` + + const tooltip = new MarkdownString( + `**${element.route.method}** \`${tooltipPath}\`` + + deprecatedSection + + `\n\n**Function:** \`${element.route.functionName}\`\n\n` + `**File:** ${Uri.parse(element.route.location.filePath).fsPath}:${element.route.location.line}` + docstringSection, ) + tooltip.supportThemeIcons = true + routeItem.tooltip = tooltip routeItem.command = { command: "fastapi-vscode.goToPathOperation", title: "Go to Definition",