diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eeffed..4a875cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## 0.3.1 - Unreleased +- Fixed Express route mapping for aliased Router imports that follow block comment banners, thanks @rohitjavvadi. + ## 0.3.0 - 2026-05-18 - Added a `pi` provider for routing review, fix, revalidate, and agent map through the [pi coding agent](https://pi.dev) in non-interactive print mode, thanks @danielmarbach. diff --git a/src/mapper.test.ts b/src/mapper.test.ts index e6ef86f..8d5945b 100644 --- a/src/mapper.test.ts +++ b/src/mapper.test.ts @@ -1755,11 +1755,31 @@ describe("mapFeatures", () => { "src/server.ts", [ "// route imports", + "import { Router as OtherRouter } from 'other-router';", + "/* import { Router as CommentedOutRouter } from 'express'; */", + "/* import banner */ import { Router as BannerRouter } from 'express';", + "/*", + " * multiline import banner", + " */ import { Router as MultilineBannerRouter } from 'express';", + "import unused from 'unused'; /* stacked */ /* import banner */ import { Router as SemicolonBannerRouter } from 'express';", + "import from, { Router as FromBindingRouter } from 'express';", + "import 'reflect-metadata'", + "import/* type banner */type { Router as CommentedTypeRouter } from 'express';", "import express, { Router, Router as ExpressRouter } from 'express';", "", + "const config = { import: true }", + "export type { Router as ExportedTypeRouter } from 'express';", "const app = express();", + "const otherRouter = OtherRouter();", + "const commentedOutRouter = CommentedOutRouter();", "const router = Router();", "const aliasRouter = ExpressRouter();", + "const bannerRouter = BannerRouter();", + "const multilineBannerRouter = MultilineBannerRouter();", + "const semicolonBannerRouter = SemicolonBannerRouter();", + "const fromBindingRouter = FromBindingRouter();", + "const commentedTypeRouter = CommentedTypeRouter();", + "const exportedTypeRouter = ExportedTypeRouter();", "const typedRouter: Router = Router();", "const projectRouter = Router({ mergeParams: true });", "let hitCount = 0;", @@ -1770,8 +1790,16 @@ describe("mapFeatures", () => { "app.get('/anonymous', requireAuth, (_req, res) => res.send('ok'));", "app.get('/dynamic/' + version, dynamicRoute);", "app.all('/proxy', proxy);", + "otherRouter.get('/other-router', ignoredOtherRouter);", + "commentedOutRouter.get('/commented-out-router', ignoredCommentedOutRouter);", "router.post('/admin/jobs', createJob);", "aliasRouter.get('/aliased-router', listAliasedRouter);", + "bannerRouter.get('/banner-router', listBannerRouter);", + "multilineBannerRouter.get('/multiline-banner-router', listMultilineBannerRouter);", + "semicolonBannerRouter.get('/semicolon-banner-router', listSemicolonBannerRouter);", + "fromBindingRouter.get('/from-binding-router', listFromBindingRouter);", + "commentedTypeRouter.get('/commented-type-router', ignoredCommentedTypeRouter);", + "exportedTypeRouter.get('/exported-type-router', ignoredExportedTypeRouter);", "router.post<{ Body: CreateJob }>('/typed-jobs', createTypedJob);", "typedRouter.patch('/typed/:id', updateTyped);", "router.route('/users').get(listUsers).delete(deleteUsers);", @@ -1789,8 +1817,16 @@ describe("mapFeatures", () => { "function showAdmin() {}", "function dynamicRoute() {}", "function proxy() {}", + "function ignoredOtherRouter() {}", + "function ignoredCommentedOutRouter() {}", "function createJob() {}", "function listAliasedRouter() {}", + "function listBannerRouter() {}", + "function listMultilineBannerRouter() {}", + "function listSemicolonBannerRouter() {}", + "function listFromBindingRouter() {}", + "function ignoredCommentedTypeRouter() {}", + "function ignoredExportedTypeRouter() {}", "function createTypedJob() {}", "function updateTyped() {}", "function listUsers() {}", @@ -1964,6 +2000,10 @@ describe("mapFeatures", () => { "Express route ALL /proxy", "Express route POST /admin/jobs", "Express route GET /aliased-router", + "Express route GET /banner-router", + "Express route GET /multiline-banner-router", + "Express route GET /semicolon-banner-router", + "Express route GET /from-binding-router", "Express route GET /cjs-aliased-router", "Express route GET /assigned-router", "Express route GET /typed-assigned-router", @@ -1990,6 +2030,10 @@ 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 /other-router"); + expect(titles).not.toContain("Express route GET /commented-out-router"); + expect(titles).not.toContain("Express route GET /commented-type-router"); + expect(titles).not.toContain("Express route GET /exported-type-router"); 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"); diff --git a/src/mappers/node-routes.ts b/src/mappers/node-routes.ts index 10d42e3..5eaafea 100644 --- a/src/mappers/node-routes.ts +++ b/src/mappers/node-routes.ts @@ -335,19 +335,72 @@ function expressRouterFactoryNames(source: string): Set { 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; + const pattern = /\bimport\b/gu; pattern.lastIndex = 0; for (const match of source.matchAll(pattern)) { - const importIndex = source.indexOf("import", match.index ?? 0); - if (importIndex < 0 || isInsideCommentOrString(source, importIndex)) { + const importIndex = match.index ?? 0; + if (isInsideCommentOrString(source, importIndex)) { continue; } - addExpressRouterImportNames(names, match[1] ?? ""); + const clause = readExpressStaticImportClause(source, importIndex); + if (clause !== null) { + addExpressRouterImportNames(names, clause); + } } return names; } +function readExpressStaticImportClause(source: string, importIndex: number): string | null { + let cursor = importIndex + "import".length; + cursor = skipWhitespaceAndComments(source, cursor); + if ( + source[cursor] === "(" || + source[cursor] === "." || + source[cursor] === "'" || + source[cursor] === '"' || + (source.startsWith("type", cursor) && !isIdentifierChar(source[cursor + "type".length])) + ) { + return null; + } + if (!isImportClauseStart(source[cursor])) { + return null; + } + const clauseStart = cursor; + const limit = Math.min(source.length, importIndex + 500); + while (cursor < limit) { + const char = source[cursor]; + const next = source[cursor + 1]; + if (char === undefined) { + break; + } + if (char === ";") { + return null; + } + if (char === "/" && next === "/") { + cursor = skipLineComment(source, cursor + 2); + continue; + } + if (char === "/" && next === "*") { + cursor = skipBlockComment(source, cursor + 2); + continue; + } + if (char === "'" || char === '"' || char === "`") { + cursor = skipQuoted(source, cursor, char); + continue; + } + if (isKeywordAt(source, cursor, "from")) { + const specifier = readImportSpecifier(source, cursor + "from".length); + if (specifier === null) { + cursor += "from".length; + continue; + } + return specifier.value === "express" ? source.slice(clauseStart, cursor) : null; + } + cursor += 1; + } + return null; +} + function addExpressRouterImportNames(names: Set, clause: string): void { const named = /\{([^}]*)\}/u.exec(clause)?.[1]; if (named === undefined) { @@ -484,6 +537,22 @@ function expressChainMethods(source: string, start: number): string[] { return methods; } +function isKeywordAt(source: string, index: number, keyword: string): boolean { + return ( + source.startsWith(keyword, index) && + !isIdentifierChar(source[index - 1]) && + !isIdentifierChar(source[index + keyword.length]) + ); +} + +function isIdentifierChar(char: string | undefined): boolean { + return char !== undefined && /[A-Za-z0-9_$]/u.test(char); +} + +function isImportClauseStart(char: string | undefined): boolean { + return char !== undefined && (char === "{" || char === "*" || /[A-Za-z_$]/u.test(char)); +} + function skipWhitespace(source: string, start: number): number { let cursor = start; while (/\s/u.test(source[cursor] ?? "")) { @@ -492,6 +561,82 @@ function skipWhitespace(source: string, start: number): number { return cursor; } +function skipWhitespaceAndComments(source: string, start: number): number { + let cursor = start; + while (cursor < source.length) { + const next = skipWhitespace(source, cursor); + if (source[next] === "/" && source[next + 1] === "*") { + cursor = skipBlockComment(source, next + 2); + continue; + } + if (source[next] === "/" && source[next + 1] === "/") { + cursor = skipLineComment(source, next + 2); + continue; + } + return next; + } + return cursor; +} + +function skipLineComment(source: string, start: number): number { + const newline = source.indexOf("\n", start); + return newline < 0 ? source.length : newline + 1; +} + +function skipBlockComment(source: string, start: number): number { + const close = source.indexOf("*/", start); + return close < 0 ? source.length : close + 2; +} + +function skipQuoted(source: string, start: number, quote: string): number { + let cursor = start + 1; + let escaped = false; + while (cursor < source.length) { + const char = source[cursor]; + if (char === undefined) { + break; + } + if (escaped) { + escaped = false; + } else if (char === "\\") { + escaped = true; + } else if (char === quote) { + return cursor + 1; + } + cursor += 1; + } + return source.length; +} + +function readImportSpecifier(source: string, start: number): { value: string; end: number } | null { + let cursor = skipWhitespace(source, start); + const quote = source[cursor]; + if (quote !== "'" && quote !== '"') { + return null; + } + cursor += 1; + let value = ""; + let escaped = false; + while (cursor < source.length) { + const char = source[cursor]; + if (char === undefined) { + break; + } + if (escaped) { + value += char; + escaped = false; + } else if (char === "\\") { + escaped = true; + } else if (char === quote) { + return { value, end: cursor + 1 }; + } else { + value += char; + } + cursor += 1; + } + return null; +} + function nextRouteValueDelimiter(source: string, start: number): string | null { let cursor = start; while (cursor < source.length) {