diff --git a/CHANGELOG.md b/CHANGELOG.md index f827495..ebbc033 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - Improved OpenCode malformed JSON diagnostics with output length, event kinds, and a bounded preview, thanks @rohitjavvadi. - Fixed finding signatures so equivalent evidence remains stable across re-reviews, thanks @rohitjavvadi. - Fixed provider exit-code classification for stdout-only authentication and quota failures, thanks @rohitjavvadi. +- Improved Node route mapping to preserve literal Express and Hono mount prefixes, thanks @rohitjavvadi. - Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi. - Fixed Laravel route mapping to include array-style `Route::group` prefixes, thanks @rohitjavvadi. - Fixed Fastify route-object mapping to emit static method arrays while ignoring dynamic entries, thanks @rohitjavvadi. diff --git a/src/mapper.test.ts b/src/mapper.test.ts index 9b324b4..1367ed3 100644 --- a/src/mapper.test.ts +++ b/src/mapper.test.ts @@ -2852,6 +2852,219 @@ describe("mapFeatures", () => { expect(routes.some((route) => route.endsWith(" /template-dynamic"))).toBe(false); }); + it("preserves literal Express and Hono mount prefixes for child routes", async () => { + const root = await fixtureRoot("clawpatch-node-mounted-route-prefixes-"); + await writeFixture( + root, + "package.json", + JSON.stringify( + { + name: "mounted-route-server", + dependencies: { express: "1.0.0", hono: "1.0.0" }, + }, + null, + 2, + ), + ); + await writeFixture( + root, + "src/express-mounted.ts", + [ + "import express, { Router } from 'express';", + "import ensureLoggedIn from './auth';", + "", + "const app = express();", + "const apiApp = express();", + "const router = Router();", + "const nestedRouter = Router();", + "const middlewareRouter = Router();", + "const genericMiddlewareRouter = Router();", + "const asyncMiddlewareRouter = Router();", + "const expressionMiddlewareRouter = Router();", + "const importedMiddlewareRouter = Router();", + "const pathlessRouter = Router();", + "const directPathlessRouter = Router();", + "const firstPathlessRouter = Router();", + "const secondPathlessRouter = Router();", + "const arrayRouter = Router();", + "const wildcardRouter = Router();", + "const dynamicRouter = Router();", + "const dynamicParent = Router();", + "const dynamicChild = Router();", + "const authPathRouter = Router();", + "const tenantRouter = Router();", + "const memberRouter = Router();", + "const falseRouter = Router();", + "const notApp = createClient();", + "app.use('/api', router);", + "app.use(dynamicTenant, router);", + "app.use('/service', apiApp);", + "router.use('/v1', nestedRouter);", + "app.use('/middleware', requireAuth, middlewareRouter);", + "apiApp.use(mw, genericMiddlewareRouter);", + "apiApp.use(amw, asyncMiddlewareRouter);", + "apiApp.use(express.json(), expressionMiddlewareRouter);", + "apiApp.use(ensureLoggedIn, importedMiddlewareRouter);", + "apiApp.use(requireAuth, pathlessRouter);", + "apiApp.use(directPathlessRouter);", + "apiApp.use(firstPathlessRouter, secondPathlessRouter);", + "app.use(['/array', '/alt-array'], arrayRouter);", + "app.use('*', wildcardRouter);", + "app.use(dynamicPrefix, dynamicRouter);", + "app.use(dynamicBase, dynamicParent);", + "dynamicParent.use('/v1', dynamicChild);", + "app.use(authPath, authPathRouter);", + "app.use(tenant, tenantRouter);", + "server.app.use('/member', memberRouter);", + "notApp.use('/false', falseRouter);", + "router.get('/users', listUsers);", + "router.route('/reports').get(listReports);", + "nestedRouter.post('/teams', createTeam);", + "apiApp.delete('/sessions/:id', deleteSession);", + "middlewareRouter.get('/users', listMiddlewareUsers);", + "genericMiddlewareRouter.get('/generic-middleware-users', listGenericMiddlewareUsers);", + "asyncMiddlewareRouter.get('/async-middleware-users', listAsyncMiddlewareUsers);", + "expressionMiddlewareRouter.get('/json-users', listJsonUsers);", + "importedMiddlewareRouter.get('/imported-users', listImportedUsers);", + "pathlessRouter.get('/pathless-users', listPathlessUsers);", + "directPathlessRouter.get('/direct-pathless-users', listDirectPathlessUsers);", + "firstPathlessRouter.get('/first-pathless-users', listFirstPathlessUsers);", + "secondPathlessRouter.get('/second-pathless-users', listSecondPathlessUsers);", + "arrayRouter.get('/array-users', listArrayUsers);", + "wildcardRouter.get('/wildcard-users', listWildcardUsers);", + "dynamicRouter.get('/dynamic-users', dynamicUsers);", + 'dynamicChild.get("/dynamic-child-users", dynamicChildUsers);', + 'authPathRouter.get("/auth-path-users", authPathUsers);', + 'tenantRouter.get("/tenant-users", tenantUsers);', + 'memberRouter.get("/member-users", memberUsers);', + "falseRouter.get('/false-users', falseUsers);", + "function createClient() { return { use() {} }; }", + "function listUsers() {}", + "function listReports() {}", + "function createTeam() {}", + "function deleteSession() {}", + "function requireAuth() {}", + "function mw() {}", + "async function amw() {}", + "function listMiddlewareUsers() {}", + "function listGenericMiddlewareUsers() {}", + "function listAsyncMiddlewareUsers() {}", + "function listJsonUsers() {}", + "function listImportedUsers() {}", + "function listPathlessUsers() {}", + "function listDirectPathlessUsers() {}", + "function listFirstPathlessUsers() {}", + "function listSecondPathlessUsers() {}", + "function listArrayUsers() {}", + "function listWildcardUsers() {}", + "function dynamicUsers() {}", + "function dynamicChildUsers() {}", + "function authPathUsers() {}", + "function tenantUsers() {}", + "function memberUsers() {}", + "function falseUsers() {}", + "", + ].join("\n"), + ); + await writeFixture( + root, + "src/hono-mounted.ts", + [ + "import { Hono } from 'hono';", + "", + "const app = new Hono();", + "const subApp = new Hono();", + "const nestedSubApp = new Hono();", + "const dynamicSubApp = new Hono();", + "const falseSubApp = new Hono();", + "const client = createClient();", + "app.route('/api', subApp);", + "subApp.route('/v1', nestedSubApp);", + "app.route(dynamicPrefix, dynamicSubApp);", + "client.route('/false', falseSubApp);", + "subApp.get('/users', listUsers);", + "nestedSubApp.delete('/sessions/:id', deleteSession);", + "dynamicSubApp.get('/dynamic-users', dynamicUsers);", + "falseSubApp.get('/false-users', falseUsers);", + "function createClient() { return { route() {} }; }", + "function listUsers() {}", + "function deleteSession() {}", + "function dynamicUsers() {}", + "function falseUsers() {}", + "", + ].join("\n"), + ); + await writeFixture( + root, + "src/express-dynamic-mw.ts", + [ + "import express, { Router } from 'express';", + "", + "const app = express();", + "const router = Router();", + "const mw = dynamicPrefix;", + "app.use(mw, router);", + "router.get('/dynamic-mw-users', listDynamicMwUsers);", + "function listDynamicMwUsers() {}", + "", + ].join("\n"), + ); + + const project = await detectProject(root); + const result = await mapFeatures(root, project, []); + const titles = result.features.map((feature) => feature.title); + + expect(titles).toEqual( + expect.arrayContaining([ + "Express route GET /api/users", + "Express route GET /api/reports", + "Express route POST /api/v1/teams", + "Express route DELETE /service/sessions/:id", + "Express route GET /middleware/users", + "Express route GET /service/generic-middleware-users", + "Express route GET /service/async-middleware-users", + "Express route GET /service/json-users", + "Express route GET /service/imported-users", + "Express route GET /service/pathless-users", + "Express route GET /service/direct-pathless-users", + "Express route GET /service/first-pathless-users", + "Express route GET /service/second-pathless-users", + "Express route GET /array/array-users", + "Express route GET /alt-array/array-users", + "Express route GET /*/wildcard-users", + "Express route GET /member-users", + "Hono route GET /api/users", + "Hono route DELETE /api/v1/sessions/:id", + ]), + ); + expect(titles).not.toContain("Express route GET /users"); + expect(titles).not.toContain("Express route GET /reports"); + expect(titles).not.toContain("Express route POST /v1/teams"); + expect(titles).not.toContain("Express route DELETE /sessions/:id"); + expect(titles).not.toContain("Express route GET /false/false-users"); + expect(titles).not.toContain("Express route GET /generic-middleware-users"); + expect(titles).not.toContain("Express route GET /async-middleware-users"); + expect(titles).not.toContain("Express route GET /json-users"); + expect(titles).not.toContain("Express route GET /imported-users"); + expect(titles).not.toContain("Express route GET /pathless-users"); + expect(titles).not.toContain("Express route GET /direct-pathless-users"); + expect(titles).not.toContain("Express route GET /first-pathless-users"); + expect(titles).not.toContain("Express route GET /second-pathless-users"); + expect(titles).not.toContain("Express route GET /array-users"); + expect(titles).not.toContain("Express route GET /wildcard-users"); + expect(titles).not.toContain("Express route GET /dynamic-users"); + expect(titles).not.toContain("Express route GET /dynamic-child-users"); + expect(titles).not.toContain("Express route GET /auth-path-users"); + expect(titles).not.toContain("Express route GET /dynamic-mw-users"); + expect(titles).not.toContain("Express route GET /tenant-users"); + expect(titles).not.toContain("Express route GET /member/member-users"); + expect(titles).not.toContain("Express route GET /v1/dynamic-child-users"); + expect(titles).not.toContain("Hono route GET /users"); + expect(titles).not.toContain("Hono route GET /dynamic-users"); + expect(titles).not.toContain("Hono route DELETE /v1/sessions/:id"); + expect(titles).not.toContain("Hono route GET /false/false-users"); + }); + it("keeps index route tests scoped to their route directory", async () => { const root = await fixtureRoot("clawpatch-node-server-index-route-tests-"); await writeFixture( diff --git a/src/mappers/node-routes.ts b/src/mappers/node-routes.ts index 2a6ab09..b857948 100644 --- a/src/mappers/node-routes.ts +++ b/src/mappers/node-routes.ts @@ -27,6 +27,22 @@ type ServerRoute = { symbol: string | null; }; +type MountedRouteTarget = { + parent: string; + child: string; + prefix: string; +}; + +type MountedRouteTargets = { + mounts: MountedRouteTarget[]; + unknownMountedTargets: Set; +}; + +type RouteTargetPrefixes = { + prefixes: Map; + unknownTargets: Set; +}; + const sourceRoots = ["src", "lib", "app", "server", "routes", "api"] as const; const sourceExtensions = ["ts", "tsx", "js", "jsx", "mts", "cts", "mjs", "cjs"] as const; const rootEntryNames = ["server", "app", "index", "main", "api"] as const; @@ -163,9 +179,10 @@ function parseServerRoutes( if (targets.size === 0 && scopedFastifyRoutes.length === 0) { continue; } - routes.push(...directMethodRoutes(source, filePath, framework, targets)); + const prefixes = mountedRouteTargetPrefixes(source, framework, targets); + routes.push(...directMethodRoutes(source, filePath, framework, targets, prefixes)); if (framework === "express") { - routes.push(...expressRouteChains(source, filePath, targets)); + routes.push(...expressRouteChains(source, filePath, targets, prefixes)); } else if (framework === "fastify") { routes.push(...fastifyRouteObjects(source, filePath, targets)); routes.push(...scopedFastifyRoutes); @@ -179,6 +196,7 @@ function directMethodRoutes( filePath: string, framework: ServerFramework, targets: ReadonlySet, + prefixes: RouteTargetPrefixes = emptyRouteTargetPrefixes(), ): ServerRoute[] { const routes: ServerRoute[] = []; routeMethodPattern.lastIndex = 0; @@ -203,13 +221,16 @@ function directMethodRoutes( continue; } const callEnd = endOfCall(source, openParenIndex + 1); - routes.push({ - framework, - filePath, - method: method.toUpperCase(), - routePath: routePath.value, - symbol: callEnd === null ? null : readHandlerSymbol(source, routePath.end, callEnd - 1), - }); + const symbol = callEnd === null ? null : readHandlerSymbol(source, routePath.end, callEnd - 1); + for (const resolvedPath of routePathsForTarget(prefixes, target, routePath.value)) { + routes.push({ + framework, + filePath, + method: method.toUpperCase(), + routePath: resolvedPath, + symbol, + }); + } } return routes; } @@ -265,6 +286,7 @@ function expressRouteChains( source: string, filePath: string, targets: ReadonlySet, + prefixes: RouteTargetPrefixes = emptyRouteTargetPrefixes(), ): ServerRoute[] { const routes: ServerRoute[] = []; routeChainPattern.lastIndex = 0; @@ -288,18 +310,346 @@ function expressRouteChains( continue; } for (const method of expressChainMethods(source, routePath.end)) { - routes.push({ - framework: "express", - filePath, - method, - routePath: routePath.value, - symbol: null, - }); + for (const resolvedPath of routePathsForTarget(prefixes, target, routePath.value)) { + routes.push({ + framework: "express", + filePath, + method, + routePath: resolvedPath, + symbol: null, + }); + } } } return routes; } +function mountedRouteTargetPrefixes( + source: string, + framework: ServerFramework, + targets: ReadonlySet, +): RouteTargetPrefixes { + if (framework !== "express" && framework !== "hono") { + return emptyRouteTargetPrefixes(); + } + const mounts = + framework === "express" + ? mountedTargets(source, targets, "use") + : mountedTargets(source, targets, "route"); + return resolveMountedRouteTargetPrefixes(mounts, targets); +} + +function mountedTargets( + source: string, + targets: ReadonlySet, + method: "route" | "use", +): MountedRouteTargets { + const mounts: MountedRouteTarget[] = []; + const unknownMountedTargets = new Set(); + const pattern = new RegExp( + `(^|[^A-Za-z0-9_$.])([A-Za-z_$][A-Za-z0-9_$]*)\\s*\\.\\s*${method}\\s*\\(`, + "gu", + ); + pattern.lastIndex = 0; + for (const match of source.matchAll(pattern)) { + const matchIndex = match.index ?? 0; + const targetIndex = matchIndex + (match[1]?.length ?? 0); + if (isInsideCommentOrString(source, targetIndex)) { + continue; + } + const parent = match[2]; + if (parent === undefined || !targets.has(parent)) { + continue; + } + const openParenIndex = matchIndex + match[0].lastIndexOf("("); + const callEnd = endOfCall(source, openParenIndex + 1); + if (callEnd === null) { + continue; + } + const args = splitTopLevelArguments(source.slice(openParenIndex + 1, callEnd - 1)); + const literalPrefixes = + args[0] === undefined + ? null + : method === "use" + ? standaloneMountPrefixes(args[0]) + : singletonMountPrefix(args[0]); + const pathlessExpressMount = + method === "use" && + (isPathlessExpressMount(source, args) || isPathlessExpressChildMount(args, parent, targets)); + const prefixes = literalPrefixes ?? (pathlessExpressMount ? ["/"] : null); + if (prefixes === null || !prefixes.every(isMountPrefix)) { + if (method === "route" || isLikelyDynamicMountPrefix(source, args[0]?.trim() ?? "")) { + const childArgs = method === "use" ? args.slice(1) : args.slice(1, 2); + for (const childArg of childArgs) { + const child = childArg.trim(); + if (isSimpleIdentifier(child) && child !== parent && targets.has(child)) { + unknownMountedTargets.add(child); + } + } + } + continue; + } + const childArgs = + method === "use" ? args.slice(literalPrefixes === null ? 0 : 1) : args.slice(1, 2); + for (const childArg of childArgs) { + const child = childArg.trim(); + if (!isSimpleIdentifier(child) || child === parent || !targets.has(child)) { + continue; + } + for (const prefix of prefixes) { + mounts.push({ parent, child, prefix }); + } + } + } + return { mounts, unknownMountedTargets }; +} + +function resolveMountedRouteTargetPrefixes( + routeMounts: MountedRouteTargets, + targets: ReadonlySet, +): RouteTargetPrefixes { + const mountsByChild = new Map(); + for (const mount of routeMounts.mounts) { + mountsByChild.set(mount.child, [...(mountsByChild.get(mount.child) ?? []), mount]); + } + const resolved = new Map(); + const unknown = new Set(); + const resolving = new Set(); + + const resolve = (target: string): string[] | null => { + if (unknown.has(target)) { + return null; + } + const cached = resolved.get(target); + if (cached !== undefined) { + return cached; + } + if (resolving.has(target)) { + return []; + } + resolving.add(target); + const prefixes: string[] = []; + for (const mount of mountsByChild.get(target) ?? []) { + const parentPrefixes = resolve(mount.parent); + if (parentPrefixes === null) { + unknown.add(target); + continue; + } else if (parentPrefixes.length === 0) { + prefixes.push(mount.prefix); + } else { + prefixes.push(...parentPrefixes.map((prefix) => joinRoutePaths(prefix, mount.prefix))); + } + } + resolving.delete(target); + const uniquePrefixes = [...new Set(prefixes)]; + resolved.set(target, uniquePrefixes); + if (uniquePrefixes.length === 0 && routeMounts.unknownMountedTargets.has(target)) { + unknown.add(target); + return null; + } + return uniquePrefixes; + }; + + for (const target of targets) { + resolve(target); + } + + return { + prefixes: new Map([...resolved].filter(([, prefixes]) => prefixes.length > 0)), + unknownTargets: unknown, + }; +} + +function isPathlessExpressChildMount( + args: string[], + parent: string, + targets: ReadonlySet, +): boolean { + if (args.length === 0) { + return false; + } + return args.every((arg) => { + const child = arg.trim(); + return isSimpleIdentifier(child) && child !== parent && targets.has(child); + }); +} + +function isPathlessExpressMount(source: string, args: string[]): boolean { + const first = args[0]?.trim(); + if (first === undefined || standaloneStringLiteral(first) !== null) { + return false; + } + if (isLikelyDynamicMountPrefix(source, first)) { + return false; + } + return ( + isLikelyExpressMiddleware(first) || + isDeclaredFunctionLike(source, first) || + (isSimpleIdentifier(first) && isImportedIdentifier(source, first)) + ); +} + +function isLikelyExpressMiddleware(value: string): boolean { + return ( + /^(?:async\s*)?(?:function\b|\([^)]*\)\s*=>|[A-Za-z_$][A-Za-z0-9_$]*\s*=>)/u.test(value) || + /(?:auth|authorize|guard|middleware|handler|validate|validator|schema|session|cookie|cors|helmet|parser|logger|compress|ratelimit|limiter|json|urlencoded|static)/iu.test( + value, + ) + ); +} + +function isLikelyDynamicMountPrefix(source: string, value: string): boolean { + if (/(?:prefix|base|path|tenant|mount|route)/iu.test(value)) { + return true; + } + if (!isSimpleIdentifier(value)) { + return false; + } + const escapedName = escapeRegExp(value); + const pattern = new RegExp( + `${declarationPrefix.replace("([A-Za-z_$][A-Za-z0-9_$]*)", escapedName)}`, + "gu", + ); + for (const match of source.matchAll(pattern)) { + if (isInsideCommentOrString(source, match.index ?? 0)) { + continue; + } + const valueStart = (match.index ?? 0) + match[0].length; + const assignedValue = source.slice(valueStart, valueStart + 200).split(/[;\n]/u)[0] ?? ""; + if (/(?:prefix|base|path|tenant|mount|route)/iu.test(assignedValue)) { + return true; + } + } + return false; +} + +function isImportedIdentifier(source: string, name: string): boolean { + const escapedName = escapeRegExp(name); + const patterns = [ + new RegExp(String.raw`\bimport\s+${escapedName}\b`, "gu"), + new RegExp(String.raw`\bimport\s+\*\s+as\s+${escapedName}\b`, "gu"), + new RegExp( + String.raw`\bimport\s*\{[^}]*\b(?:${escapedName}|[A-Za-z_$][A-Za-z0-9_$]*\s+as\s+${escapedName})\b[^}]*\}\s*from\b`, + "gu", + ), + new RegExp(String.raw`\b(?:const|let|var)\s+${escapedName}\s*=\s*require\s*\(`, "gu"), + new RegExp( + String.raw`\b(?:const|let|var)\s*\{[^}]*\b(?:${escapedName}|[A-Za-z_$][A-Za-z0-9_$]*\s*:\s*${escapedName})\b[^}]*\}\s*=\s*require\s*\(`, + "gu", + ), + ]; + for (const pattern of patterns) { + pattern.lastIndex = 0; + for (const match of source.matchAll(pattern)) { + if (!isInsideCommentOrString(source, match.index ?? 0)) { + return true; + } + } + } + return false; +} + +function isDeclaredFunctionLike(source: string, name: string): boolean { + if (!isSimpleIdentifier(name)) { + return false; + } + const escapedName = escapeRegExp(name); + const patterns = [ + new RegExp(`\\b(?:async\\s+)?function\\s+${escapedName}\\s*\\(`, "gu"), + new RegExp( + `${declarationPrefix.replace("([A-Za-z_$][A-Za-z0-9_$]*)", escapedName)}(?:async\\s*)?(?:function\\b|\\([^)]*\\)\\s*=>|[A-Za-z_$][A-Za-z0-9_$]*\\s*=>)`, + "gu", + ), + ]; + for (const pattern of patterns) { + pattern.lastIndex = 0; + for (const match of source.matchAll(pattern)) { + if (!isInsideCommentOrString(source, match.index ?? 0)) { + return true; + } + } + } + return false; +} + +function routePathsForTarget( + prefixes: RouteTargetPrefixes, + target: string, + routePath: string, +): string[] { + const targetPrefixes = prefixes.prefixes.get(target) ?? []; + if (targetPrefixes.length > 0) { + return targetPrefixes.map((prefix) => joinRoutePaths(prefix, routePath)); + } + if (prefixes.unknownTargets.has(target)) { + return []; + } + return [routePath]; +} + +function emptyRouteTargetPrefixes(): RouteTargetPrefixes { + return { prefixes: new Map(), unknownTargets: new Set() }; +} + +function singletonMountPrefix(source: string): string[] | null { + const literal = standaloneStringLiteral(source); + return literal === null ? null : [literal]; +} + +function standaloneMountPrefixes(source: string): string[] | null { + const literal = standaloneStringLiteral(source); + if (literal !== null) { + return [literal]; + } + const arrayStart = skipWhitespace(source, 0); + if (source[arrayStart] !== "[") { + return null; + } + const arrayEnd = endOfArray(source, arrayStart + 1); + if (arrayEnd === null || nextRouteValueDelimiter(source, arrayEnd) !== null) { + return null; + } + const prefixes: string[] = []; + for (const element of splitTopLevelArguments(source.slice(arrayStart + 1, arrayEnd - 1))) { + const elementLiteral = standaloneStringLiteral(element); + if (elementLiteral === null) { + return null; + } + prefixes.push(elementLiteral); + } + return prefixes.length === 0 ? null : prefixes; +} + +function standaloneStringLiteral(source: string): string | null { + const literal = readStringLiteralArgument(source, 0); + if (literal === null) { + return null; + } + return nextRouteValueDelimiter(source, literal.end) === null ? literal.value : null; +} + +function isMountPrefix(path: string): boolean { + return path === "*" || path.startsWith("/"); +} + +function isSimpleIdentifier(value: string): boolean { + return /^[A-Za-z_$][A-Za-z0-9_$]*$/u.test(value); +} + +function joinRoutePaths(prefix: string, routePath: string): string { + const normalizedPrefix = prefix === "*" ? "/*" : prefix.replace(/\/+$/u, "") || "/"; + if (normalizedPrefix === "/") { + return routePath; + } + if (routePath === "/") { + return normalizedPrefix; + } + if (routePath === "*") { + return `${normalizedPrefix}/*`; + } + return `${normalizedPrefix}/${routePath.replace(/^\/+/, "")}`; +} + function routeTargetNames(source: string, framework: ServerFramework): Set { if (framework === "express") { const patterns = [