diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ab47c8..9ef7696 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Added `clawpatch review --export-tribunal-ledger` to emit review findings as JSONL for downstream ledger ingestion, thanks @dpdanpittman. - Added `clawpatch review --prompt-file` to append extra reviewer guidance from a file or stdin, thanks @dpdanpittman. - Added deterministic Express, Fastify, and Hono route mapping for Node projects, thanks @rohitjavvadi. +- Fixed Express route mapping to recognize aliased Router factories from imports, CommonJS destructuring, and direct assignments, thanks @rohitjavvadi. - Added conservative Django `urls.py` route mapping for `path`, `re_path`, and legacy `url` declarations, thanks @rohitjavvadi. - Fixed provider commands with relative `--root` paths by canonicalizing explicit roots before invoking Codex or other providers. - Added first-pass Elixir Mix/Phoenix mapping for project metadata, contexts, Phoenix web slices, runtime config, Ecto migrations, project scripts, ExUnit tests, and Mix validation defaults, thanks @tears-mysthrala. diff --git a/src/mapper.test.ts b/src/mapper.test.ts index 39c0141..e6ef86f 100644 --- a/src/mapper.test.ts +++ b/src/mapper.test.ts @@ -1754,10 +1754,12 @@ describe("mapFeatures", () => { root, "src/server.ts", [ - "import express, { Router } from 'express';", + "// route imports", + "import express, { Router, Router as ExpressRouter } from 'express';", "", "const app = express();", "const router = Router();", + "const aliasRouter = ExpressRouter();", "const typedRouter: Router = Router();", "const projectRouter = Router({ mergeParams: true });", "let hitCount = 0;", @@ -1769,6 +1771,7 @@ describe("mapFeatures", () => { "app.get('/dynamic/' + version, dynamicRoute);", "app.all('/proxy', proxy);", "router.post('/admin/jobs', createJob);", + "aliasRouter.get('/aliased-router', listAliasedRouter);", "router.post<{ Body: CreateJob }>('/typed-jobs', createTypedJob);", "typedRouter.patch('/typed/:id', updateTyped);", "router.route('/users').get(listUsers).delete(deleteUsers);", @@ -1787,6 +1790,7 @@ describe("mapFeatures", () => { "function dynamicRoute() {}", "function proxy() {}", "function createJob() {}", + "function listAliasedRouter() {}", "function createTypedJob() {}", "function updateTyped() {}", "function listUsers() {}", @@ -1830,6 +1834,46 @@ describe("mapFeatures", () => { "", ].join("\n"), ); + await writeFixture( + root, + "src/cjs-router.cjs", + [ + "const { Router: CjsRouter, json: JsonFactory } = require('express');", + "", + "const cjsRouter = CjsRouter();", + "cjsRouter.get('/cjs-aliased-router', listCjsAliasedRouter);", + "const jsonFactory = JsonFactory();", + "jsonFactory.get('/cjs-not-router', ignored);", + "function listCjsAliasedRouter() {}", + "function ignored() {}", + "", + ].join("\n"), + ); + await writeFixture( + root, + "src/router-assignment.ts", + [ + "import express from 'express';", + "", + "const AssignedRouter = express.Router;", + "const TypedAssignedRouter: typeof express.Router = express.Router;", + "const RequiredRouter = require('express').Router;", + "const NotRouter = express.json;", + "const assignedRouter = AssignedRouter();", + "const typedAssignedRouter = TypedAssignedRouter();", + "const requiredRouter = RequiredRouter();", + "const notRouter = NotRouter();", + "assignedRouter.get('/assigned-router', listAssignedRouter);", + "typedAssignedRouter.get('/typed-assigned-router', listTypedAssignedRouter);", + "requiredRouter.get('/required-router', listRequiredRouter);", + "notRouter.get('/assigned-not-router', ignored);", + "function listAssignedRouter() {}", + "function listTypedAssignedRouter() {}", + "function listRequiredRouter() {}", + "function ignored() {}", + "", + ].join("\n"), + ); await writeFixture( root, "src/hono.ts", @@ -1866,12 +1910,21 @@ describe("mapFeatures", () => { "src/custom-router.ts", [ "// import { Router } from 'express';", - "import { type Router } from 'express';", + "import { Router as CustomRouter } from './custom-router-factory';", + "import express from 'express';", + "import { type Router, type Router as ExpressRouter } from 'express';", "", "declare function Router(): { get(path: string, handler: unknown): void };", + "declare function ExpressRouter(): { get(path: string, handler: unknown): void };", "", + "const app = express();", + "const customRouter = CustomRouter();", "const router = Router();", + "const aliasRouter = ExpressRouter();", + "app.get('/custom-file-real', handler);", + "customRouter.get('/custom-import-router', handler);", "router.get('/custom-router', handler);", + "aliasRouter.get('/custom-alias-router', handler);", "function handler() {}", "", ].join("\n"), @@ -1910,6 +1963,11 @@ describe("mapFeatures", () => { "Express route GET /anonymous", "Express route ALL /proxy", "Express route POST /admin/jobs", + "Express route GET /aliased-router", + "Express route GET /cjs-aliased-router", + "Express route GET /assigned-router", + "Express route GET /typed-assigned-router", + "Express route GET /required-router", "Express route POST /typed-jobs", "Express route PATCH /typed/:id", "Express route GET /users", @@ -1917,6 +1975,7 @@ describe("mapFeatures", () => { "Express route GET /reports", "Express route GET /projects/:projectId/items", "Express route GET /after-jsx-close", + "Express route GET /custom-file-real", "Fastify route GET /status", "Fastify route GET /typed-users/:id", "Fastify route GET /route-status", @@ -1931,7 +1990,11 @@ describe("mapFeatures", () => { expect(titles).not.toContain("Express route GET /regex-health"); expect(titles).not.toContain("Express route GET /arrow-regex"); expect(titles).not.toContain("Express route GET /returned-regex"); + expect(titles).not.toContain("Express route GET /custom-import-router"); expect(titles).not.toContain("Express route GET /custom-router"); + expect(titles).not.toContain("Express route GET /custom-alias-router"); + expect(titles).not.toContain("Express route GET /cjs-not-router"); + expect(titles).not.toContain("Express route GET /assigned-not-router"); expect(titles).not.toContain("Express route GET /dynamic/"); expect(titles).not.toContain("Fastify route GET /dynamic/"); expect(titles).not.toContain("Fastify route GET /concat-"); diff --git a/src/mappers/node-routes.ts b/src/mappers/node-routes.ts index 00b828d..10d42e3 100644 --- a/src/mappers/node-routes.ts +++ b/src/mappers/node-routes.ts @@ -302,8 +302,13 @@ function routeTargetNames(source: string, framework: ServerFramework): Set { + return new Set([ + ...expressRouterImportBindingNames(source), + ...expressRouterRequireBindingNames(source), + ...expressRouterAssignmentNames(source), + ]); } -function hasExpressRouterImportBinding(source: string): boolean { - const pattern = /\bimport\s+(?!type\b)([\s\S]{0,400}?)\bfrom\s*["']express["']/gu; +function expressRouterImportBindingNames(source: string): Set { + const names = new Set(); + const pattern = + /(?:^|[;\n])\s*import\s+(?!type\b)((?:(?!\n\s*import\b)[\s\S]){0,400}?)\bfrom\s*["']express["']/gu; pattern.lastIndex = 0; for (const match of source.matchAll(pattern)) { - if (isInsideCommentOrString(source, match.index ?? 0)) { + const importIndex = source.indexOf("import", match.index ?? 0); + if (importIndex < 0 || isInsideCommentOrString(source, importIndex)) { continue; } - if (importClauseHasBareValueRouter(match[1] ?? "")) { - return true; - } + addExpressRouterImportNames(names, match[1] ?? ""); } - return false; + return names; } -function importClauseHasBareValueRouter(clause: string): boolean { +function addExpressRouterImportNames(names: Set, clause: string): void { const named = /\{([^}]*)\}/u.exec(clause)?.[1]; if (named === undefined) { - return false; + return; } - return named.split(",").some((part) => { + for (const part of named.split(",")) { const binding = part.trim(); - return !binding.startsWith("type ") && /^Router(?:\s+as\s+Router)?$/u.test(binding); - }); + if (binding.startsWith("type ")) { + continue; + } + const match = /^Router(?:\s+as\s+([A-Za-z_$][A-Za-z0-9_$]*))?$/u.exec(binding); + if (match !== null) { + names.add(match[1] ?? "Router"); + } + } } -function codePatternMatches(source: string, pattern: RegExp): boolean { +function expressRouterRequireBindingNames(source: string): Set { + const names = new Set(); + const pattern = + /\b(?:const|let|var)\s*\{\s*([^}]*)\}\s*=\s*require\s*\(\s*["']express["']\s*\)/gu; pattern.lastIndex = 0; for (const match of source.matchAll(pattern)) { - if (!isInsideCommentOrString(source, match.index ?? 0)) { - return true; + if (isInsideCommentOrString(source, match.index ?? 0)) { + continue; } + addExpressRouterRequireNames(names, match[1] ?? ""); } - return false; + return names; +} + +function addExpressRouterRequireNames(names: Set, clause: string): void { + for (const part of clause.split(",")) { + const binding = part.trim(); + const match = /^Router(?:\s*:\s*([A-Za-z_$][A-Za-z0-9_$]*))?$/u.exec(binding); + if (match !== null) { + names.add(match[1] ?? "Router"); + } + } +} + +function expressRouterAssignmentNames(source: string): Set { + const names = new Set(); + const pattern = + /\b(?:const|let|var)\s+([A-Za-z_$][A-Za-z0-9_$]*)(?:\s*:\s*[^=;]+)?\s*=\s*(?:express\s*\.\s*Router|require\s*\(\s*["']express["']\s*\)\s*\.\s*Router)\b/gu; + pattern.lastIndex = 0; + for (const match of source.matchAll(pattern)) { + if (isInsideCommentOrString(source, match.index ?? 0)) { + continue; + } + const name = match[1]; + if (name !== undefined) { + names.add(name); + } + } + return names; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/gu, "\\$&"); } function declaredTargetNames(source: string, patterns: RegExp[]): Set {