From 26fa60a39574df14d2b1cdfb73f123326e7ec2c9 Mon Sep 17 00:00:00 2001 From: abhinav-phi Date: Thu, 16 Jul 2026 00:38:15 +0530 Subject: [PATCH 1/3] feat(graph): add NestJS controller route resolver --- src/graph/__tests__/fixtures/nestjs-app.ts | 45 ++++++ src/graph/__tests__/resolver-nestjs.test.ts | 109 ++++++++++++++ src/graph/resolution/frameworks/index.ts | 8 +- src/graph/resolution/frameworks/nestjs.ts | 159 ++++++++++++++++++++ 4 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 src/graph/__tests__/fixtures/nestjs-app.ts create mode 100644 src/graph/__tests__/resolver-nestjs.test.ts create mode 100644 src/graph/resolution/frameworks/nestjs.ts diff --git a/src/graph/__tests__/fixtures/nestjs-app.ts b/src/graph/__tests__/fixtures/nestjs-app.ts new file mode 100644 index 00000000..96c4b602 --- /dev/null +++ b/src/graph/__tests__/fixtures/nestjs-app.ts @@ -0,0 +1,45 @@ +import { Controller, Get, Post, Param, Delete, Put, Patch, Options, Head, All } from '@nestjs/common'; + +@Controller('users') +export class UsersController { + + // Empty method path -> GET /users + @Get() + async findAll() { + return []; + } + + // Parameterized path -> GET /users/:id + @Get(':id') + findOne(@Param('id') id: string) { + return { id }; + } + + // Nested parameterized path -> POST /users/:id/posts + @Post(':id/posts') + @HttpCode(201) // Simulate other decorators + createPost(@Param('id') id: string) { + return { id, post: true }; + } + + // Missing handler name (anonymous function) - Should not happen typically, but simulating edge case + @Delete('anonymous') + // We don't have a handler name here for extraction, let's just make it a normal one for positive testing + deleteUser() { + return false; + } +} + +// Controller with no prefix +@Controller() +export class RootController { + @Get('health') + healthCheck() { + return 'ok'; + } + + @All() // Test All decorator + fallback() { + return 'fallback'; + } +} diff --git a/src/graph/__tests__/resolver-nestjs.test.ts b/src/graph/__tests__/resolver-nestjs.test.ts new file mode 100644 index 00000000..b6a85058 --- /dev/null +++ b/src/graph/__tests__/resolver-nestjs.test.ts @@ -0,0 +1,109 @@ +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; +import { nestjsResolver } from "../resolution/frameworks/nestjs.js"; +import type { GraphNode } from "../types.js"; +import type { ResolutionContext } from "../resolution/types.js"; + +const fixturePath = join(dirname(fileURLToPath(import.meta.url)), "fixtures", "nestjs-app.ts"); +const source = readFileSync(fixturePath, "utf-8"); + +describe("NestJS framework resolver", () => { + it("detects NestJS via @nestjs/core or @nestjs/common dependencies", () => { + let context = fakeContext([], { "package.json": JSON.stringify({ dependencies: { "@nestjs/core": "^9.0.0" } }) }); + expect(nestjsResolver.detect(context)).toBe(true); + + context = fakeContext([], { "package.json": JSON.stringify({ devDependencies: { "@nestjs/common": "^9.0.0" } }) }); + expect(nestjsResolver.detect(context)).toBe(true); + + context = fakeContext([], { "package.json": JSON.stringify({ dependencies: { "express": "^4.17.1" } }) }); + expect(nestjsResolver.detect(context)).toBe(false); + }); + + it("extracts route nodes and function references for controllers", () => { + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); + + // Check nodes (route paths normalized) + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "GET /users" })); + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "GET /users/:id" })); + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "POST /users/:id/posts" })); + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "DELETE /users/anonymous" })); + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "GET /health" })); + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "ALL /" })); + + // Check references (handler methods) + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "findAll", referenceKind: "function_ref" })); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "findOne", referenceKind: "function_ref" })); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "createPost", referenceKind: "function_ref" })); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "deleteUser", referenceKind: "function_ref" })); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "healthCheck", referenceKind: "function_ref" })); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "fallback", referenceKind: "function_ref" })); + + // Ensure no false positives + expect(result.nodes.length).toBe(6); + expect(result.references.length).toBe(6); + }); + + it("binds the extracted route reference to its same-file handler", () => { + const findAllHandler = node("method:findAll", "findAll"); + const healthCheckHandler = node("method:healthCheck", "healthCheck"); + const context = fakeContext([findAllHandler, healthCheckHandler]); + + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); + const findAllRef = result.references.find(r => r.referenceName === "findAll")!; + const healthCheckRef = result.references.find(r => r.referenceName === "healthCheck")!; + + expect(nestjsResolver.resolve(findAllRef, context)).toMatchObject({ + targetNodeId: findAllHandler.id, + confidence: 1, + resolvedBy: "framework", + }); + + expect(nestjsResolver.resolve(healthCheckRef, context)).toMatchObject({ + targetNodeId: healthCheckHandler.id, + confidence: 1, + resolvedBy: "framework", + }); + }); + + it("leaves ambiguous references unresolved when multiple identical method names exist", () => { + // If a method is found in another file, it should NOT resolve because NestJS dictates same-file. + // If multiple in the same file, it also shouldn't guess. + const handler1 = node("method:1", "duplicateMethod"); + const handler2 = { ...node("method:2", "duplicateMethod"), startLine: 10 }; + const context = fakeContext([handler1, handler2]); + + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); + // Let's pretend one of the extracted references was named 'duplicateMethod' + const fakeRef = { ...result.references[0]!, referenceName: "duplicateMethod" }; + + expect(nestjsResolver.resolve(fakeRef, context)).toBeNull(); + }); + + it("leaves missing handlers unresolved", () => { + const context = fakeContext([]); + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); + const fakeRef = { ...result.references[0]!, referenceName: "missingMethod" }; + expect(nestjsResolver.resolve(fakeRef, context)).toBeNull(); + }); +}); + +function node(id: string, name: string): GraphNode { + return { id, kind: "method", name, qualifiedName: name, filePath: "src/nestjs-app.ts", + language: "typescript", startLine: 1, endLine: 2, startColumn: 0, endColumn: 0, updatedAt: 0 }; +} + +function fakeContext(nodes: GraphNode[], files: Record = {}): ResolutionContext { + return { + getNodesInFile: (path) => nodes.filter((entry) => entry.filePath === path), + getNodesByName: (name) => nodes.filter((entry) => entry.name === name), + getNodesByQualifiedName: (name) => nodes.filter((entry) => entry.qualifiedName === name), + getNodesByKind: (kind) => nodes.filter((entry) => entry.kind === kind), + getNodeById: (id) => nodes.find((entry) => entry.id === id) ?? null, + fileExists: (path) => path in files, + readFile: (path) => files[path] ?? null, + getProjectRoot: () => "/repo", + getAllFiles: () => Object.keys(files), + }; +} diff --git a/src/graph/resolution/frameworks/index.ts b/src/graph/resolution/frameworks/index.ts index 3df38a74..d0285e93 100644 --- a/src/graph/resolution/frameworks/index.ts +++ b/src/graph/resolution/frameworks/index.ts @@ -1,8 +1,14 @@ import { expressResolver } from "./express.js"; import { nextjsResolver } from "./nextjs.js"; +import { nestjsResolver } from "./nestjs.js"; import type { FrameworkResolver } from "../types.js"; /** Reference registry. Community resolvers add one entry here. */ -export const FRAMEWORK_RESOLVERS: readonly FrameworkResolver[] = [expressResolver, nextjsResolver]; +export const FRAMEWORK_RESOLVERS: readonly FrameworkResolver[] = [ + expressResolver, + nextjsResolver, + nestjsResolver, +]; export { expressResolver } from "./express.js"; export { nextjsResolver } from "./nextjs.js"; +export { nestjsResolver } from "./nestjs.js"; diff --git a/src/graph/resolution/frameworks/nestjs.ts b/src/graph/resolution/frameworks/nestjs.ts new file mode 100644 index 00000000..03b7ab42 --- /dev/null +++ b/src/graph/resolution/frameworks/nestjs.ts @@ -0,0 +1,159 @@ +import { generateNodeId } from "../../extraction/node-id.js"; +import type { GraphNode, Language } from "../../types.js"; +import type { FrameworkExtractionResult, FrameworkResolver, ResolvedRef, UnresolvedRef } from "../types.js"; + +// Basic parsing for NestJS decorators without a full AST walk. +// It assumes standard formatting and single controller per file for simplicity, +// or sequentially processes them if multiple exist. + +const DECORATOR_REGEX = /@(Controller|Get|Post|Put|Patch|Delete|Options|Head|All)\s*\(\s*(?:(["'`])(.*?)\2)?\s*\)/g; + +export const nestjsResolver: FrameworkResolver = { + name: "nestjs", + languages: ["typescript", "javascript"], + detect(context) { + const pkg = context.readFile("package.json"); + if (!pkg) return false; + try { + const parsed = JSON.parse(pkg) as { dependencies?: Record; devDependencies?: Record }; + return Boolean( + parsed.dependencies?.["@nestjs/core"] ?? + parsed.dependencies?.["@nestjs/common"] ?? + parsed.devDependencies?.["@nestjs/core"] ?? + parsed.devDependencies?.["@nestjs/common"] + ); + } catch { return false; } + }, + claimsReference: (name) => /^[A-Za-z_$][\w$]*$/.test(name), + extract(filePath, content): FrameworkExtractionResult { + const nodes: GraphNode[] = []; + const references: UnresolvedRef[] = []; + const language = languageFor(filePath); + if (!language) return { nodes, references }; + + let currentControllerPath = ""; + + for (const match of content.matchAll(DECORATOR_REGEX)) { + const type = match[1]!; + const pathArg = match[3] ?? ""; + + if (type === "Controller") { + currentControllerPath = pathArg; + continue; + } + + // It's an HTTP method + const httpMethod = type.toUpperCase(); + + // Normalize route path: GET /controllerPath/methodPath + let fullPath = currentControllerPath; + if (fullPath && !fullPath.startsWith("/")) fullPath = "/" + fullPath; + + let subPath = pathArg; + if (subPath && !subPath.startsWith("/")) subPath = "/" + subPath; + if (subPath === "/") subPath = ""; + + fullPath += subPath; + if (!fullPath) fullPath = "/"; + else if (fullPath.length > 1 && fullPath.endsWith("/")) fullPath = fullPath.slice(0, -1); + + const routeName = `${httpMethod} ${fullPath}`; + + // Forward scan to find the handler method name + // Skip whitespace, comments, and other decorators until we find an identifier followed by '(' or '<' + let handlerName = ""; + let idx = match.index + match[0].length; + + while (idx < content.length) { + // Skip whitespace + if (/\s/.test(content[idx]!)) { + idx++; + continue; + } + + // Skip line comments + if (content[idx] === "/" && content[idx+1] === "/") { + while (idx < content.length && content[idx] !== "\n") idx++; + continue; + } + + // Skip block comments + if (content[idx] === "/" && content[idx+1] === "*") { + idx += 2; + while (idx < content.length && !(content[idx] === "*" && content[idx+1] === "/")) idx++; + idx += 2; + continue; + } + + // Skip other decorators + if (content[idx] === "@") { + idx++; + // Skip identifier + while (idx < content.length && /[A-Za-z0-9_$]/.test(content[idx]!)) idx++; + // If it has parens, skip them + if (content[idx] === "(") { + let depth = 1; + idx++; + while (idx < content.length && depth > 0) { + if (content[idx] === "(") depth++; + else if (content[idx] === ")") depth--; + idx++; + } + } + continue; + } + + // Skip keywords like 'async', 'public', 'private', 'protected' + const substr = content.slice(idx); + const keywordMatch = /^(?:async|public|private|protected)\s+/.exec(substr); + if (keywordMatch) { + idx += keywordMatch[0].length; + continue; + } + + // We should be at the method name now + const methodMatch = /^([A-Za-z_$][\w$]*)\s*[<([]/.exec(substr); + if (methodMatch) { + handlerName = methodMatch[1]!; + break; + } + + // If we hit something unexpected, stop + break; + } + + if (!handlerName) continue; + + const line = content.slice(0, match.index).split("\n").length; + const id = generateNodeId(filePath, "route", routeName); + nodes.push({ id, kind: "route", name: routeName, qualifiedName: routeName, filePath, language, + startLine: line, endLine: line, startColumn: 0, endColumn: match[0].length, + isExported: false, updatedAt: 0 }); + references.push({ fromNodeId: id, referenceName: handlerName, referenceKind: "function_ref", + filePath, language, line: line, column: 0 }); // Note: line is the decorator line + } + + return { nodes, references }; + }, + resolve(ref, context): ResolvedRef | null { + if (ref.referenceKind !== "function_ref") return null; + const candidates = context.getNodesByName(ref.referenceName) + .filter((node) => node.kind === "method" || node.kind === "function"); + const sameFile = candidates.filter((node) => node.filePath === ref.filePath); + + // NestJS methods are always in the same file as the controller route decorators. + if (sameFile.length === 1) { + return { original: ref, targetNodeId: sameFile[0]!.id, confidence: 1, resolvedBy: "framework" }; + } + + return null; + }, +}; + +function languageFor(filePath: string): Language | null { + if (/\.(ts|mts|cts)$/.test(filePath)) return "typescript"; + if (/\.tsx$/.test(filePath)) return "tsx"; + if (/\.(js|mjs|cjs)$/.test(filePath)) return "javascript"; + if (/\.jsx$/.test(filePath)) return "jsx"; + return null; +} From be600123482c362b6a53077049fea0fecbda2ac0 Mon Sep 17 00:00:00 2001 From: abhinav-phi Date: Sat, 12 Sep 2026 01:59:41 +0530 Subject: [PATCH 2/3] fix(graph): harden NestJS resolver against real-world controller forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-driven rework of the NestJS route resolver, addressing three must-fix findings from real-build probing: - Duplicate node ids killed whole builds: NestJS versioning (@Version('1') @Get() findAllV1 / @Version('2') @Get() findAllV2) emits two routes with the same method+path. The handler now rides in the signature (GET /users -> findAllV1) and an ordinal joins the role, mirroring Express, so same-named routes keep distinct ids. - @Controller arguments beyond string literals were skipped entirely, so the prefix was lost or inherited from the previous controller. Arguments are now read string-aware (a ')' inside 'List users :)' no longer breaks anything): string form, object form ({ path: ... }), and empty are handled; constants and arrays skip that controller's routes rather than guessing. Every @Controller resets the prefix and binds to the next class, so two controllers in one file each keep their own prefix. - Commented-out decorators invented phantom routes. Comments are blanked with spaces before scanning — offsets and line numbers survive, comments stop being code. Resolution now records the owning controller class in the reference candidates and prefers a same-file qualified-name match, so two controllers that both declare findAll no longer leave both routes unresolved. Edge label moves to nestjs-route-handler at confidence 0.8, matching Express's evidence class. Fixture notes tidied; a rebuildGraph integration test covers the versioned-route and comment-decorator cases end to end. Addresses review on #102 --- src/graph/__tests__/fixtures/nestjs-app.ts | 50 ++- src/graph/__tests__/resolver-nestjs.test.ts | 153 ++++++-- src/graph/resolution/frameworks/nestjs.ts | 373 ++++++++++++++------ test/graph-integration.test.ts | 50 +++ 4 files changed, 471 insertions(+), 155 deletions(-) diff --git a/src/graph/__tests__/fixtures/nestjs-app.ts b/src/graph/__tests__/fixtures/nestjs-app.ts index 96c4b602..fd2174b6 100644 --- a/src/graph/__tests__/fixtures/nestjs-app.ts +++ b/src/graph/__tests__/fixtures/nestjs-app.ts @@ -1,45 +1,65 @@ -import { Controller, Get, Post, Param, Delete, Put, Patch, Options, Head, All } from '@nestjs/common'; +import { Controller, Get, Post, Param, Delete, Version } from '@nestjs/common'; @Controller('users') export class UsersController { - - // Empty method path -> GET /users @Get() async findAll() { return []; } - // Parameterized path -> GET /users/:id @Get(':id') findOne(@Param('id') id: string) { return { id }; } - // Nested parameterized path -> POST /users/:id/posts @Post(':id/posts') - @HttpCode(201) // Simulate other decorators + @HttpCode(201) createPost(@Param('id') id: string) { return { id, post: true }; } - - // Missing handler name (anonymous function) - Should not happen typically, but simulating edge case - @Delete('anonymous') - // We don't have a handler name here for extraction, let's just make it a normal one for positive testing - deleteUser() { - return false; + + @Version('1') + @Get() + listV1() { + return ['v1']; + } + + @Version('2') + @Get() + listV2() { + return ['v2']; } } -// Controller with no prefix @Controller() export class RootController { @Get('health') healthCheck() { return 'ok'; } - - @All() // Test All decorator + + @All() fallback() { return 'fallback'; } } + +// A commented-out route is not a route. +// @Get('legacy') +// legacyHandler() {} + +@Controller({ path: 'admin', version: '2' }) +export class AdminController { + @Delete(':id') + remove(@Param('id') id: string) { + return { id }; + } +} + +@Controller(ADMIN_PATH_CONSTANT) +export class UnreadableController { + @Get('probe') + probe() { + return null; + } +} diff --git a/src/graph/__tests__/resolver-nestjs.test.ts b/src/graph/__tests__/resolver-nestjs.test.ts index b6a85058..1719cf25 100644 --- a/src/graph/__tests__/resolver-nestjs.test.ts +++ b/src/graph/__tests__/resolver-nestjs.test.ts @@ -13,74 +13,173 @@ describe("NestJS framework resolver", () => { it("detects NestJS via @nestjs/core or @nestjs/common dependencies", () => { let context = fakeContext([], { "package.json": JSON.stringify({ dependencies: { "@nestjs/core": "^9.0.0" } }) }); expect(nestjsResolver.detect(context)).toBe(true); - + context = fakeContext([], { "package.json": JSON.stringify({ devDependencies: { "@nestjs/common": "^9.0.0" } }) }); expect(nestjsResolver.detect(context)).toBe(true); - + context = fakeContext([], { "package.json": JSON.stringify({ dependencies: { "express": "^4.17.1" } }) }); expect(nestjsResolver.detect(context)).toBe(false); }); it("extracts route nodes and function references for controllers", () => { const result = nestjsResolver.extract!("src/nestjs-app.ts", source); - - // Check nodes (route paths normalized) + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "GET /users" })); expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "GET /users/:id" })); expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "POST /users/:id/posts" })); - expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "DELETE /users/anonymous" })); expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "GET /health" })); expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "ALL /" })); + expect(result.nodes).toContainEqual(expect.objectContaining({ kind: "route", name: "DELETE /admin/:id" })); - // Check references (handler methods) expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "findAll", referenceKind: "function_ref" })); expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "findOne", referenceKind: "function_ref" })); expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "createPost", referenceKind: "function_ref" })); - expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "deleteUser", referenceKind: "function_ref" })); expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "healthCheck", referenceKind: "function_ref" })); expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "fallback", referenceKind: "function_ref" })); - - // Ensure no false positives - expect(result.nodes.length).toBe(6); - expect(result.references.length).toBe(6); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "remove", referenceKind: "function_ref" })); + + // NestJS versioning declares three @Get() routes with the same name; the + // handler in the signature keeps their node ids distinct (#102 review). + const usersRoutes = result.nodes.filter((n) => n.name === "GET /users"); + expect(usersRoutes).toHaveLength(3); + expect(new Set(usersRoutes.map((n) => n.id)).size).toBe(3); + expect(usersRoutes.map((n) => n.signature).sort()).toEqual([ + "GET /users -> findAll", + "GET /users -> listV1", + "GET /users -> listV2", + ]); + }); + + it("keeps commented-out decorators out of the route table", () => { + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); + expect(result.nodes.some((n) => n.name.includes("legacy"))).toBe(false); + expect(result.references.some((r) => r.referenceName === "legacyHandler")).toBe(false); + // The unreadable controller's routes are skipped rather than guessed. + expect(result.nodes.some((n) => n.name.endsWith("/probe"))).toBe(false); }); it("binds the extracted route reference to its same-file handler", () => { const findAllHandler = node("method:findAll", "findAll"); const healthCheckHandler = node("method:healthCheck", "healthCheck"); const context = fakeContext([findAllHandler, healthCheckHandler]); - + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); const findAllRef = result.references.find(r => r.referenceName === "findAll")!; const healthCheckRef = result.references.find(r => r.referenceName === "healthCheck")!; - + expect(nestjsResolver.resolve(findAllRef, context)).toMatchObject({ targetNodeId: findAllHandler.id, - confidence: 1, - resolvedBy: "framework", + confidence: 0.8, + resolvedBy: "nestjs-route-handler", }); - + expect(nestjsResolver.resolve(healthCheckRef, context)).toMatchObject({ targetNodeId: healthCheckHandler.id, - confidence: 1, - resolvedBy: "framework", + confidence: 0.8, + resolvedBy: "nestjs-route-handler", }); }); - it("leaves ambiguous references unresolved when multiple identical method names exist", () => { - // If a method is found in another file, it should NOT resolve because NestJS dictates same-file. - // If multiple in the same file, it also shouldn't guess. + it("resolves two same-named handlers across controllers via the owning class", () => { + const usersFindAll = node("method:users-findAll", "findAll", "UsersController::findAll", "src/two-controllers.ts"); + const adminFindAll = node("method:admin-findAll", "findAll", "AdminController::findAll", "src/two-controllers.ts"); + const context = fakeContext([usersFindAll, adminFindAll]); + + const custom = [ + "@Controller('users')", + "export class UsersController {", + " @Get()", + " findAll() {}", + "}", + "", + "@Controller('admin')", + "export class AdminController {", + " @Get()", + " findAll() {}", + "}", + "", + ].join("\n"); + const result = nestjsResolver.extract!("src/two-controllers.ts", custom); + expect(result.references).toHaveLength(2); + + const usersRef = result.references[0]!; + expect(usersRef.candidates).toEqual(["UsersController::findAll"]); + expect(nestjsResolver.resolve(usersRef, context)).toMatchObject({ + targetNodeId: usersFindAll.id, + resolvedBy: "nestjs-route-handler", + }); + + const adminRef = result.references[1]!; + expect(nestjsResolver.resolve(adminRef, context)).toMatchObject({ + targetNodeId: adminFindAll.id, + }); + }); + + it("reads the object-form controller path and skips unreadable ones", () => { + const custom = [ + "@Controller({ path: 'users', version: '1' })", + "export class UsersController {", + " @Get(':id')", + " findOne() {}", + "}", + "", + "@Controller(ADMIN_PATH)", + "export class UnreadableController {", + " @Get('probe')", + " probe() {}", + "}", + "", + "@Controller('users')", + "export class SecondUsersController {", + " @Get('again')", + " again() {}", + "}", + "", + ].join("\n"); + const result = nestjsResolver.extract!("src/forms.ts", custom); + + // Object form with a `path` property contributes its prefix. + expect(result.nodes).toContainEqual(expect.objectContaining({ name: "GET /users/:id" })); + // A constant argument is not statically readable — its routes are skipped… + expect(result.nodes.some((n) => n.name.endsWith("/probe"))).toBe(false); + // …and a later controller resets the prefix instead of inheriting it. + expect(result.nodes).toContainEqual(expect.objectContaining({ name: "GET /users/again" })); + }); + + it("does not let a string with a parenthesis break decorator scanning", () => { + const custom = [ + "@Controller('users')", + "export class UsersController {", + " @ApiOperation({ summary: 'List users :)' })", + " @Get()", + " findAll() {}", + "}", + "", + ].join("\n"); + const result = nestjsResolver.extract!("src/smiley.ts", custom); + expect(result.nodes).toContainEqual(expect.objectContaining({ name: "GET /users" })); + expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "findAll" })); + }); + + it("leaves ambiguous references unresolved unless the owning class disambiguates", () => { const handler1 = node("method:1", "duplicateMethod"); const handler2 = { ...node("method:2", "duplicateMethod"), startLine: 10 }; const context = fakeContext([handler1, handler2]); - + const result = nestjsResolver.extract!("src/nestjs-app.ts", source); - // Let's pretend one of the extracted references was named 'duplicateMethod' const fakeRef = { ...result.references[0]!, referenceName: "duplicateMethod" }; - expect(nestjsResolver.resolve(fakeRef, context)).toBeNull(); + + // With the owning class recorded, the same ambiguity resolves. + const owners = [ + { ...node("method:1", "duplicateMethod"), qualifiedName: "UsersController::duplicateMethod" }, + { ...node("method:2", "duplicateMethod"), qualifiedName: "AdminController::duplicateMethod" }, + ]; + const owningContext = fakeContext(owners); + const usersScoped = { ...fakeRef, candidates: ["UsersController::duplicateMethod"] }; + expect(nestjsResolver.resolve(usersScoped, owningContext)).toMatchObject({ targetNodeId: owners[0]!.id }); }); - + it("leaves missing handlers unresolved", () => { const context = fakeContext([]); const result = nestjsResolver.extract!("src/nestjs-app.ts", source); @@ -89,8 +188,8 @@ describe("NestJS framework resolver", () => { }); }); -function node(id: string, name: string): GraphNode { - return { id, kind: "method", name, qualifiedName: name, filePath: "src/nestjs-app.ts", +function node(id: string, name: string, qualifiedName = name, filePath = "src/nestjs-app.ts"): GraphNode { + return { id, kind: "method", name, qualifiedName, filePath, language: "typescript", startLine: 1, endLine: 2, startColumn: 0, endColumn: 0, updatedAt: 0 }; } diff --git a/src/graph/resolution/frameworks/nestjs.ts b/src/graph/resolution/frameworks/nestjs.ts index 03b7ab42..bda52543 100644 --- a/src/graph/resolution/frameworks/nestjs.ts +++ b/src/graph/resolution/frameworks/nestjs.ts @@ -1,12 +1,12 @@ -import { generateNodeId } from "../../extraction/node-id.js"; +import { canonicalNodeIdentity, generateNodeId } from "../../extraction/node-id.js"; import type { GraphNode, Language } from "../../types.js"; import type { FrameworkExtractionResult, FrameworkResolver, ResolvedRef, UnresolvedRef } from "../types.js"; -// Basic parsing for NestJS decorators without a full AST walk. -// It assumes standard formatting and single controller per file for simplicity, -// or sequentially processes them if multiple exist. - -const DECORATOR_REGEX = /@(Controller|Get|Post|Put|Patch|Delete|Options|Head|All)\s*\(\s*(?:(["'`])(.*?)\2)?\s*\)/g; +const CONTROLLER_DECORATOR = /^@Controller\b/; +const HTTP_DECORATOR = /^@(Get|Post|Put|Patch|Delete|Options|Head|All)\b/; +const CLASS_DECLARATION = /^(?:export\s+)?(?:abstract\s+)?(?:declare\s+)?class\s+([A-Za-z_$][\w$]*)/; +const STRING_LITERAL_ARG = /^(["'`])([^"'\\]*)\1/; +const OBJECT_PATH_ARG = /(?:^|[,{]\s*)path\s*:\s*(["'])([^"'\\]*)\1/; export const nestjsResolver: FrameworkResolver = { name: "nestjs", @@ -26,127 +26,124 @@ export const nestjsResolver: FrameworkResolver = { }, claimsReference: (name) => /^[A-Za-z_$][\w$]*$/.test(name), extract(filePath, content): FrameworkExtractionResult { + const language = languageFor(filePath); + if (!language) return { nodes: [], references: [] }; + const nodes: GraphNode[] = []; const references: UnresolvedRef[] = []; - const language = languageFor(filePath); - if (!language) return { nodes, references }; - - let currentControllerPath = ""; - - for (const match of content.matchAll(DECORATOR_REGEX)) { - const type = match[1]!; - const pathArg = match[3] ?? ""; - - if (type === "Controller") { - currentControllerPath = pathArg; + + // Comments are blanked (with spaces, so offsets and line numbers survive) + // before scanning: a commented-out `// @Get('legacy')` or a handler left + // inside a block comment is not a route, and reading raw text invented + // phantom routes pointing at real handlers (#102 review). + const blanked = blankComments(content); + const lines = blanked.split("\n"); + + // Controller state binds to the NEXT class declaration, so two controllers + // in one file each keep their own prefix and neither leaks into the other. + let pendingController: { prefix: string; skip: boolean } | null = null; + let activeController: { prefix: string; skip: boolean } | null = null; + let currentClass: string | null = null; + const occurrences = new Map(); + + let lineStart = 0; + for (const line of lines) { + const trimmed = line.trim(); + + if (CONTROLLER_DECORATOR.test(trimmed)) { + pendingController = parseControllerArgs(trimmed); + lineStart += line.length + 1; continue; } - // It's an HTTP method - const httpMethod = type.toUpperCase(); - - // Normalize route path: GET /controllerPath/methodPath - let fullPath = currentControllerPath; - if (fullPath && !fullPath.startsWith("/")) fullPath = "/" + fullPath; - - let subPath = pathArg; - if (subPath && !subPath.startsWith("/")) subPath = "/" + subPath; - if (subPath === "/") subPath = ""; - - fullPath += subPath; - if (!fullPath) fullPath = "/"; - else if (fullPath.length > 1 && fullPath.endsWith("/")) fullPath = fullPath.slice(0, -1); - - const routeName = `${httpMethod} ${fullPath}`; - - // Forward scan to find the handler method name - // Skip whitespace, comments, and other decorators until we find an identifier followed by '(' or '<' - let handlerName = ""; - let idx = match.index + match[0].length; - - while (idx < content.length) { - // Skip whitespace - if (/\s/.test(content[idx]!)) { - idx++; - continue; - } - - // Skip line comments - if (content[idx] === "/" && content[idx+1] === "/") { - while (idx < content.length && content[idx] !== "\n") idx++; - continue; - } - - // Skip block comments - if (content[idx] === "/" && content[idx+1] === "*") { - idx += 2; - while (idx < content.length && !(content[idx] === "*" && content[idx+1] === "/")) idx++; - idx += 2; - continue; - } - - // Skip other decorators - if (content[idx] === "@") { - idx++; - // Skip identifier - while (idx < content.length && /[A-Za-z0-9_$]/.test(content[idx]!)) idx++; - // If it has parens, skip them - if (content[idx] === "(") { - let depth = 1; - idx++; - while (idx < content.length && depth > 0) { - if (content[idx] === "(") depth++; - else if (content[idx] === ")") depth--; - idx++; - } - } - continue; - } - - // Skip keywords like 'async', 'public', 'private', 'protected' - const substr = content.slice(idx); - const keywordMatch = /^(?:async|public|private|protected)\s+/.exec(substr); - if (keywordMatch) { - idx += keywordMatch[0].length; - continue; - } + const classMatch = CLASS_DECLARATION.exec(trimmed); + if (classMatch) { + currentClass = classMatch[1]!; + activeController = pendingController; + pendingController = null; + lineStart += line.length + 1; + continue; + } - // We should be at the method name now - const methodMatch = /^([A-Za-z_$][\w$]*)\s*[<([]/.exec(substr); - if (methodMatch) { - handlerName = methodMatch[1]!; - break; + const decoratorMatch = HTTP_DECORATOR.exec(trimmed); + if (decoratorMatch && activeController && !activeController.skip) { + const method = decoratorMatch[1]!.toUpperCase(); + const handlerName = findHandlerName(blanked, lineStart + line.length); + if (handlerName) { + // An empty argument list is a route with no path; an argument that + // is not a string literal (an array, a constant) cannot be read + // statically — no route beats a wrong route. + const args = extractBalancedArgs(trimmed, trimmed.indexOf("(")); + const trimmedArgs = args === null ? null : args.trim(); + const rawPath: string | null = trimmedArgs === null + ? null + : trimmedArgs === "" ? "" : firstStringArgument(args!); + if (rawPath !== null) { + const routeName = `${method} ${normalizeRoutePath(activeController.prefix, rawPath)}`; + const handler = handlerName; + const signature = `${routeName} -> ${handler}`; + // Two routes can share a name in one file (NestJS versioning: + // @Version('1') @Get() findAllV1 / @Version('2') @Get() + // findAllV2). The handler in the signature distinguishes most; + // the ordinal in the role covers the rest, mirroring Express. + const ordinal = occurrences.get(routeName) ?? 0; + occurrences.set(routeName, ordinal + 1); + const role = `nestjs-route:${ordinal}`; + const id = generateNodeId(filePath, "route", routeName, routeName, role, signature); + nodes.push({ + id, + identityKey: canonicalNodeIdentity(filePath, "route", routeName, role, signature), + kind: "route", + name: routeName, + qualifiedName: routeName, + filePath, + language, + startLine: blanked.slice(0, lineStart).split("\n").length, + endLine: blanked.slice(0, lineStart).split("\n").length, + startColumn: 0, + endColumn: line.length, + signature, + isExported: false, + updatedAt: 0, + }); + references.push({ + fromNodeId: id, + referenceName: handler, + referenceKind: "function_ref", + filePath, + language, + line: blanked.slice(0, lineStart).split("\n").length - 1, + column: 0, + // The TypeScript extractor names methods `Class::method`; + // carrying the owning controller lets resolution distinguish + // two same-named handlers across controllers in one file. + ...(currentClass ? { candidates: [`${currentClass}::${handler}`] } : {}), + }); + } } - - // If we hit something unexpected, stop - break; } - - if (!handlerName) continue; - - const line = content.slice(0, match.index).split("\n").length; - const id = generateNodeId(filePath, "route", routeName); - nodes.push({ id, kind: "route", name: routeName, qualifiedName: routeName, filePath, language, - startLine: line, endLine: line, startColumn: 0, endColumn: match[0].length, - isExported: false, updatedAt: 0 }); - references.push({ fromNodeId: id, referenceName: handlerName, referenceKind: "function_ref", - filePath, language, line: line, column: 0 }); // Note: line is the decorator line + + lineStart += line.length + 1; } - + return { nodes, references }; }, resolve(ref, context): ResolvedRef | null { if (ref.referenceKind !== "function_ref") return null; - const candidates = context.getNodesByName(ref.referenceName) - .filter((node) => node.kind === "method" || node.kind === "function"); - const sameFile = candidates.filter((node) => node.filePath === ref.filePath); - - // NestJS methods are always in the same file as the controller route decorators. - if (sameFile.length === 1) { - return { original: ref, targetNodeId: sameFile[0]!.id, confidence: 1, resolvedBy: "framework" }; + const sameFile = context.getNodesInFile(ref.filePath) + .filter((node) => (node.kind === "method" || node.kind === "function") && node.name === ref.referenceName); + // NestJS handlers live in the same file as the controller. A unique + // same-file declaration binds; when several share the name (two + // controllers in one file), the owning class recorded at extraction + // picks the one the route was declared on. + let target = sameFile.length === 1 ? sameFile[0] : null; + if (!target && sameFile.length > 1 && ref.candidates?.length) { + const qualified = sameFile.filter((node) => ref.candidates!.includes(node.qualifiedName)); + if (qualified.length === 1) target = qualified[0]!; } - - return null; + return target + ? { original: ref, targetNodeId: target.id, confidence: 0.8, resolvedBy: "nestjs-route-handler" } + : null; }, }; @@ -157,3 +154,153 @@ function languageFor(filePath: string): Language | null { if (/\.jsx$/.test(filePath)) return "jsx"; return null; } + +/** + * Blank comments out of the source, replacing every comment character with a + * space and preserving all newlines, so scanning sees comment-free code while + * every offset, line number, and column stays valid against the original. + * String literals are preserved verbatim — decorator arguments live in them. + */ +function blankComments(content: string): string { + const out: string[] = []; + let state: "code" | "line" | "block" | "string" = "code"; + let quote = ""; + for (let i = 0; i < content.length; i++) { + const ch = content[i]!; + const next = content[i + 1]; + if (state === "code") { + if (ch === "/" && next === "/") { state = "line"; out.push(" "); i++; continue; } + if (ch === "/" && next === "*") { state = "block"; out.push(" "); i++; continue; } + if (ch === "\"" || ch === "'" || ch === "`") { state = "string"; quote = ch; out.push(ch); continue; } + out.push(ch); + continue; + } + if (state === "line") { + if (ch === "\n") { state = "code"; out.push("\n"); } else out.push(" "); + continue; + } + if (state === "block") { + if (ch === "*" && next === "/") { state = "code"; out.push(" "); i++; continue; } + out.push(ch === "\n" ? "\n" : " "); + continue; + } + // Inside a string: escape sequences cannot close it. + if (ch === quote && content[i - 1] !== "\\") state = "code"; + out.push(ch); + } + return out.join(""); +} + +/** + * Read `@Controller(...)` arguments into a prefix, or a skip when the argument + * cannot be read statically. Every form resets whatever the previous + * controller left behind — a `@Controller` without a readable path must not + * inherit one (#102 review). + */ +function parseControllerArgs(line: string): { prefix: string; skip: boolean } { + const open = line.indexOf("("); + if (open < 0) return { prefix: "", skip: false }; + const args = extractBalancedArgs(line, open); + if (args === null) return { prefix: "", skip: true }; + const trimmedArgs = args.trim(); + if (trimmedArgs === "") return { prefix: "", skip: false }; + + const literal = STRING_LITERAL_ARG.exec(trimmedArgs); + if (literal) return { prefix: literal[2]!, skip: false }; + + if (trimmedArgs.startsWith("{")) { + const objectPath = OBJECT_PATH_ARG.exec(trimmedArgs); + return { prefix: objectPath ? objectPath[2]! : "", skip: false }; + } + + // A constant identifier or an array of paths is not statically readable + // here; skip this controller's routes rather than emit a wrong prefix. + return { prefix: "", skip: true }; +} + +/** The first positional string-literal argument, or null when absent. */ +function firstStringArgument(argsText: string): string | null { + const match = STRING_LITERAL_ARG.exec(argsText.trim()); + return match ? match[2]! : null; +} + +/** `GET /users/:id` from the controller prefix and the method's path. */ +function normalizeRoutePath(prefix: string, methodPath: string): string { + let fullPath = prefix; + if (fullPath && !fullPath.startsWith("/")) fullPath = "/" + fullPath; + let subPath = methodPath; + if (subPath && !subPath.startsWith("/")) subPath = "/" + subPath; + if (subPath === "/") subPath = ""; + fullPath += subPath; + if (!fullPath) fullPath = "/"; + else if (fullPath.length > 1 && fullPath.endsWith("/")) fullPath = fullPath.slice(0, -1); + return fullPath; +} + +/** + * Extract the argument text of the call whose `(` sits at `open`, skipping + * over string literals so a `)` inside `'List users :)'` does not close it. + * Returns null when the call does not close on this line. + */ +function extractBalancedArgs(line: string, open: number): string | null { + let depth = 0; + let quote: string | null = null; + for (let i = open; i < line.length; i++) { + const ch = line[i]!; + if (quote) { + if (ch === quote && line[i - 1] !== "\\") quote = null; + continue; + } + if (ch === "\"" || ch === "'" || ch === "`") { quote = ch; continue; } + if (ch === "(") depth++; + else if (ch === ")") { + depth--; + if (depth === 0) return line.slice(open + 1, i); + } + } + return null; +} + +/** + * Forward-scan from just past a decorator line for the handler name: skip + * whitespace, further decorators (string-aware, so `@ApiOperation({ + * summary: 'List users :)' })` no longer swallows the route), and modifier + * keywords, then read the identifier that opens a parameter list. + */ +function findHandlerName(content: string, from: number): string | null { + let idx = from; + while (idx < content.length) { + const ch = content[idx]!; + if (/\s/.test(ch)) { idx++; continue; } + if (ch === "@") { + idx++; + while (idx < content.length && /[A-Za-z0-9_$]/.test(content[idx]!)) idx++; + if (content[idx] === "(") { + let depth = 0; + let quote: string | null = null; + while (idx < content.length) { + const c = content[idx]!; + if (quote) { + if (c === quote && content[idx - 1] !== "\\") quote = null; + } else if (c === "\"" || c === "'" || c === "`") { + quote = c; + } else if (c === "(") { + depth++; + } else if (c === ")") { + depth--; + if (depth === 0) { idx++; break; } + } + idx++; + } + } + continue; + } + const rest = content.slice(idx); + const keyword = /^(?:public|private|protected|static|readonly|override|async)\s+/.exec(rest); + if (keyword) { idx += keyword[0].length; continue; } + const method = /^([A-Za-z_$][\w$]*)\s*[<(]/.exec(rest); + if (method) return method[1]!; + return null; + } + return null; +} diff --git a/test/graph-integration.test.ts b/test/graph-integration.test.ts index b8ae3853..c576a42d 100644 --- a/test/graph-integration.test.ts +++ b/test/graph-integration.test.ts @@ -16,6 +16,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import type { MexConfig } from "../src/types.js"; import { runDriftCheckWithGraphStatus } from "../src/drift/index.js"; import { createGraphEngine } from "../src/graph/engine-impl.js"; +import { rebuildGraph } from "../src/graph/maintenance.js"; import { loadGroundingRuntime, loadReadOnlyGroundingRuntime, @@ -865,4 +866,53 @@ describe("code-graph grounding integration", () => { process.exitCode = previousExitCode; } }, 15_000); + + it("persists NestJS versioned routes through a real build without duplicate-id failures (#102)", async () => { + const root = mkdtempSync(join(tmpdir(), "mex-nestjs-integration-")); + roots.push(root); + const controller = join(root, "src", "users.controller.ts"); + mkdirSync(join(root, "src"), { recursive: true }); + mkdirSync(join(root, ".mex"), { recursive: true }); + writeFileSync(join(root, ".mex", "ROUTER.md"), "# Router\n"); + writeFileSync(join(root, "package.json"), JSON.stringify({ + name: "fixture", dependencies: { "@nestjs/common": "^10.0.0" }, + })); + writeFileSync(controller, [ + "import { Controller, Get, Version } from '@nestjs/common';", + "", + "@Controller('users')", + "export class UsersController {", + " @Version('1')", + " @Get()", + " findAllV1() { return ['v1']; }", + "", + " @Version('2')", + " @Get()", + " findAllV2() { return ['v2']; }", + "}", + "", + "// @Get('legacy')", + "// legacyRoute() {}", + "", + ].join("\n")); + + const result = await rebuildGraph(root); + expect(result.status.status).toBe("fresh"); + + const db = openSqlite(join(root, ".mex", "graph.db")); + try { + const routes = db.prepare( + "SELECT name, signature FROM nodes WHERE kind = 'route' ORDER BY name", + ).all() as Array<{ name: string; signature: string }>; + // Both versioned routes persist with distinct ids; the commented-out + // decorator produces no phantom route. + expect(routes.map((route) => route.name)).toEqual(["GET /users", "GET /users"]); + expect(new Set(routes.map((route) => route.signature))).toEqual( + new Set(["GET /users -> findAllV1", "GET /users -> findAllV2"]), + ); + expect(routes.some((route) => route.signature.includes("legacy"))).toBe(false); + } finally { + db.close(); + } + }, 20_000); }); From 57ebdcba809e54fffd1dd6c1c70aa44e2d8b38f0 Mon Sep 17 00:00:00 2001 From: abhinav-phi Date: Sat, 12 Sep 2026 22:20:40 +0530 Subject: [PATCH 3/3] fix(graph): harden NestJS resolver per round-2 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Inline decorators bound the wrong handler: scanning started at the decorator line's end, so `@Get() list() {}` picked the NEXT method. Extraction now starts at the decorator's closing paren (the span returned by the balanced-args reader), which also keeps the next-line form working. Repro snippet is now a test. - `{ path: CONSTANT }` skipped like the bare-constant form (was emitting an unprefixed route); an object without a `path` key stays unprefixed. - Prefix trailing slashes are trimmed before joining, so `@Controller('/users/')` + `@Get('/:id/')` gives `GET /users/:id` instead of `GET /users//:id`. - Backtick paths with `${` interpolation are unreadable → the route is skipped instead of emitting `GET /users/${BASE}/x`. - Registry rebased with main's nextjsResolver ([express, nextjs, nestjs]); CHANGELOG entry added under [Unreleased] → Added. Resolves review items on #102; multi-line controller objects and array method paths remain documented follow-ups. --- CHANGELOG.md | 1 + src/graph/__tests__/resolver-nestjs.test.ts | 39 +++++++++++++ src/graph/resolution/frameworks/nestjs.ts | 64 ++++++++++++++------- 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab1fda31..63048048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Added - A bounded Next.js App Router resolver turning `app/**/route.ts|js` modules (including `src/app` roots) into route nodes: one per exported HTTP handler (`GET` through `HEAD`), with the URL path derived from the route file's directory, dynamic segments such as `[id]` and catch-alls preserved verbatim, and route groups `(marketing)` excluded the way Next resolves them. Same-file handlers resolve only when unambiguous; Pages Router, layouts, and pages stay out of scope (#95). +- A bounded NestJS controller route resolver: `@Controller()` prefixes combine with `@Get`/`@Post`/`@Put`/`@Patch`/`@Delete`/`@Options`/`@Head`/`@All` method paths into `METHOD /path` route nodes, with the handler in the signature and an ordinal in the role so versioned duplicates (`@Version('1')`/`@Version('2')`) keep distinct ids. Controller arguments are read from string literals and `{ path: '…' }` objects; unreadable forms (constants, arrays) skip rather than guess. Comments are blanked before scanning, routes resolve to same-file handlers — disambiguating between controllers in one file via the owning class — and edge resolution is labeled `nestjs-route-handler` (#98). ### Fixed diff --git a/src/graph/__tests__/resolver-nestjs.test.ts b/src/graph/__tests__/resolver-nestjs.test.ts index 1719cf25..0331ec3b 100644 --- a/src/graph/__tests__/resolver-nestjs.test.ts +++ b/src/graph/__tests__/resolver-nestjs.test.ts @@ -161,6 +161,45 @@ describe("NestJS framework resolver", () => { expect(result.references).toContainEqual(expect.objectContaining({ referenceName: "findAll" })); }); + it("binds a decorator written on the same line as its method (#102 review r2)", () => { + const custom = [ + "@Controller('users')", + "export class UsersController {", + " @Get() list() { return []; }", + " @Get(':id') findOne(@Param('id') id: string) { return { id }; }", + "}", + "", + ].join("\n"); + const result = nestjsResolver.extract!("src/inline.ts", custom); + expect(result.nodes.map((node) => node.signature)).toEqual([ + "GET /users -> list", + "GET /users/:id -> findOne", + ]); + }); + + it("skips an object controller whose path is not a literal, trims slashes, and refuses interpolated templates", () => { + const custom = [ + "@Controller({ path: USERS_PATH, version: '1' })", + "export class UnreadableController {", + " @Get(':id')", + " findOne() {}", + "}", + "", + "@Controller('/users/')", + "export class SlashedController {", + " @Get('/:id/')", + " findOne() {}", + "", + " @Get(`${BASE}/x`)", + " templated() {}", + "}", + "", + ].join("\n"); + const result = nestjsResolver.extract!("src/edge.ts", custom); + // { path: CONSTANT } behaves like the bare-constant form: routes skipped. + expect(result.nodes.map((node) => node.name)).toEqual(["GET /users/:id"]); + }); + it("leaves ambiguous references unresolved unless the owning class disambiguates", () => { const handler1 = node("method:1", "duplicateMethod"); const handler2 = { ...node("method:2", "duplicateMethod"), startLine: 10 }; diff --git a/src/graph/resolution/frameworks/nestjs.ts b/src/graph/resolution/frameworks/nestjs.ts index bda52543..eb5f8014 100644 --- a/src/graph/resolution/frameworks/nestjs.ts +++ b/src/graph/resolution/frameworks/nestjs.ts @@ -68,16 +68,19 @@ export const nestjsResolver: FrameworkResolver = { const decoratorMatch = HTTP_DECORATOR.exec(trimmed); if (decoratorMatch && activeController && !activeController.skip) { const method = decoratorMatch[1]!.toUpperCase(); - const handlerName = findHandlerName(blanked, lineStart + line.length); - if (handlerName) { + // Scan from just past the decorator's closing paren, not the line + // end: `@Get() list() {}` on one line must bind `list`, not the next + // method. (#102 review round 2.) + const openInLine = line.length - line.trimStart().length + trimmed.indexOf("("); + const span = openInLine >= 0 ? extractBalancedArgs(line, openInLine) : null; + const handlerName = findHandlerName(blanked, lineStart + (span?.end ?? line.length)); + if (handlerName && span) { // An empty argument list is a route with no path; an argument that - // is not a string literal (an array, a constant) cannot be read - // statically — no route beats a wrong route. - const args = extractBalancedArgs(trimmed, trimmed.indexOf("(")); - const trimmedArgs = args === null ? null : args.trim(); - const rawPath: string | null = trimmedArgs === null - ? null - : trimmedArgs === "" ? "" : firstStringArgument(args!); + // is not a string literal (an array, a constant, a template with + // an interpolation) cannot be read statically — no route beats a + // wrong route. + const trimmedArgs = span.args.trim(); + const rawPath: string | null = trimmedArgs === "" ? "" : firstStringArgument(span.args); if (rawPath !== null) { const routeName = `${method} ${normalizeRoutePath(activeController.prefix, rawPath)}`; const handler = handlerName; @@ -200,17 +203,26 @@ function blankComments(content: string): string { function parseControllerArgs(line: string): { prefix: string; skip: boolean } { const open = line.indexOf("("); if (open < 0) return { prefix: "", skip: false }; - const args = extractBalancedArgs(line, open); - if (args === null) return { prefix: "", skip: true }; - const trimmedArgs = args.trim(); + const span = extractBalancedArgs(line, open); + if (span === null) return { prefix: "", skip: true }; + const trimmedArgs = span.args.trim(); if (trimmedArgs === "") return { prefix: "", skip: false }; const literal = STRING_LITERAL_ARG.exec(trimmedArgs); - if (literal) return { prefix: literal[2]!, skip: false }; + if (literal) { + const path = firstStringArgument(trimmedArgs); + return path === null ? { prefix: "", skip: true } : { prefix: path, skip: false }; + } if (trimmedArgs.startsWith("{")) { + // An object with a `path` key must have a statically readable value for + // it; `{ path: CONSTANT }` is as unreadable as a bare constant argument. + // An object with no `path` at all (`{ host: '…' }`) is unprefixed. + const hasPath = /(?:^|[,{]\s*)path\s*:/.test(trimmedArgs); + if (!hasPath) return { prefix: "", skip: false }; const objectPath = OBJECT_PATH_ARG.exec(trimmedArgs); - return { prefix: objectPath ? objectPath[2]! : "", skip: false }; + if (!objectPath) return { prefix: "", skip: true }; + return { prefix: objectPath[2]!, skip: false }; } // A constant identifier or an array of paths is not statically readable @@ -218,15 +230,21 @@ function parseControllerArgs(line: string): { prefix: string; skip: boolean } { return { prefix: "", skip: true }; } -/** The first positional string-literal argument, or null when absent. */ +/** + * The first positional string-literal argument, or null when absent, or when + * it is a template with an interpolation — `` `${BASE}/x` `` has no static + * path and must not be emitted verbatim. + */ function firstStringArgument(argsText: string): string | null { const match = STRING_LITERAL_ARG.exec(argsText.trim()); - return match ? match[2]! : null; + if (!match) return null; + if (match[1] === "`" && match[2]!.includes("${")) return null; + return match[2]!; } /** `GET /users/:id` from the controller prefix and the method's path. */ function normalizeRoutePath(prefix: string, methodPath: string): string { - let fullPath = prefix; + let fullPath = prefix.replace(/\/+$/, ""); if (fullPath && !fullPath.startsWith("/")) fullPath = "/" + fullPath; let subPath = methodPath; if (subPath && !subPath.startsWith("/")) subPath = "/" + subPath; @@ -238,11 +256,13 @@ function normalizeRoutePath(prefix: string, methodPath: string): string { } /** - * Extract the argument text of the call whose `(` sits at `open`, skipping - * over string literals so a `)` inside `'List users :)'` does not close it. - * Returns null when the call does not close on this line. + * Extract the argument text of the call whose `(` sits at `open`, and the + * offset just past its closing `)`. Skipping over string literals keeps a `)` + * inside `'List users :)'` from closing it; the end offset lets the caller + * start the handler scan at the right place when the decorator and its method + * share a line. Returns null when the call does not close on this line. */ -function extractBalancedArgs(line: string, open: number): string | null { +function extractBalancedArgs(line: string, open: number): { args: string; end: number } | null { let depth = 0; let quote: string | null = null; for (let i = open; i < line.length; i++) { @@ -255,7 +275,7 @@ function extractBalancedArgs(line: string, open: number): string | null { if (ch === "(") depth++; else if (ch === ")") { depth--; - if (depth === 0) return line.slice(open + 1, i); + if (depth === 0) return { args: line.slice(open + 1, i), end: i + 1 }; } } return null;