Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
67 changes: 65 additions & 2 deletions src/mapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;",
Expand All @@ -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);",
Expand All @@ -1787,6 +1790,7 @@ describe("mapFeatures", () => {
"function dynamicRoute() {}",
"function proxy() {}",
"function createJob() {}",
"function listAliasedRouter() {}",
"function createTypedJob() {}",
"function updateTyped() {}",
"function listUsers() {}",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -1910,13 +1963,19 @@ 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",
"Express route DELETE /users",
"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",
Expand All @@ -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-");
Expand Down
101 changes: 71 additions & 30 deletions src/mappers/node-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,13 @@ function routeTargetNames(source: string, framework: ServerFramework): Set<strin
"gu",
),
];
if (hasExpressRouterBinding(source)) {
patterns.push(new RegExp(`${declarationPrefix}Router${genericArguments}\\s*\\(`, "gu"));
for (const factoryName of expressRouterFactoryNames(source)) {
patterns.push(
new RegExp(
`${declarationPrefix}${escapeRegExp(factoryName)}${genericArguments}\\s*\\(`,
"gu",
),
);
}
return declaredTargetNames(source, patterns);
}
Expand All @@ -320,53 +325,89 @@ function routeTargetNames(source: string, framework: ServerFramework): Set<strin
]);
}

function hasExpressRouterBinding(source: string): boolean {
return (
hasExpressRouterImportBinding(source) ||
codePatternMatches(
source,
/\b(?:const|let|var)\s*\{\s*[^}]*\bRouter\b[^}]*\}\s*=\s*require\s*\(\s*["']express["']\s*\)/gu,
) ||
codePatternMatches(
source,
/\b(?:const|let|var)\s+Router\s*=\s*(?:express\s*\.\s*Router|require\s*\(\s*["']express["']\s*\)\s*\.\s*Router)\b/gu,
)
);
function expressRouterFactoryNames(source: string): Set<string> {
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<string> {
const names = new Set<string>();
const pattern =
/(?:^|[;\n])\s*import\s+(?!type\b)((?:(?!\n\s*import\b)[\s\S]){0,400}?)\bfrom\s*["']express["']/gu;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect Express imports after inline block comments

The new expressRouterImportBindingNames regex only matches import when it is preceded by start-of-file, ;, or \n, so valid module code like /* banner */ import { Router as R } from 'express' no longer registers R as a Router factory. In that case, const router = R(); router.get(...) routes are silently skipped, which regresses route mapping accuracy for files that keep a header comment on the same line as the first import.

Useful? React with 👍 / 👎.

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<string>, 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<string> {
const names = new Set<string>();
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<string>, 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<string> {
const names = new Set<string>();
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<string> {
Expand Down