diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 357d337ae..a03b59e6a 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -174,7 +174,7 @@ converging on the routes phase's `(method, url)` registry: | Filesystem convention | path → URL, no parsing | Next.js `app/`, Expo, PHP | | Single-file framework route | `isRouteFile` + worker extraction | Laravel `routes/*.php` | | Cross-file framework route | `discoverRootRouteFiles` + `extractRoutes` | Django `urlpatterns` | -| AST-level route in a normal file | `extractDecoratorRoutes` | Spring, FastAPI, NestJS, **JS/TS dispatch guards and static data route tables** | +| AST-level route in a normal file | `extractDecoratorRoutes` | Spring, FastAPI, NestJS (`@Controller` + `@Get`/`@Post`/…), **JS/TS dispatch guards and static data route tables** | The last row is the one whose name undersells it. A route is DECLARED by a decorator, but it can also be **inferred** from a raw `node:http` server's own diff --git a/gitnexus/src/core/ingestion/languages/typescript.ts b/gitnexus/src/core/ingestion/languages/typescript.ts index eb757a7b5..6dce7db2c 100644 --- a/gitnexus/src/core/ingestion/languages/typescript.ts +++ b/gitnexus/src/core/ingestion/languages/typescript.ts @@ -126,10 +126,12 @@ import { } from './javascript/index.js'; import { extractDispatchGuardRoutes } from '../route-extractors/dispatch-guard.js'; import { extractDataRouteTableRoutes } from '../route-extractors/data-route-table.js'; +import { extractNestRoutes } from '../route-extractors/nest.js'; const extractJsTsRoutes = (...args: Parameters) => [ ...extractDispatchGuardRoutes(...args), ...extractDataRouteTableRoutes(...args), + ...extractNestRoutes(...args), ]; /** diff --git a/gitnexus/src/core/ingestion/route-extractors/nest.ts b/gitnexus/src/core/ingestion/route-extractors/nest.ts new file mode 100644 index 000000000..a8ec4c60a --- /dev/null +++ b/gitnexus/src/core/ingestion/route-extractors/nest.ts @@ -0,0 +1,245 @@ +/** + * NestJS decorator routes for the indexer. + * + * A NestJS endpoint is declared across two decorators: `@Controller('venues')` + * on the class supplies the prefix, and `@Get('search')` on a method supplies + * the verb and the remainder. Neither half is a route on its own, which is why + * a pattern that only looks at one of them finds nothing. + * + * Until this existed, TypeScript's `extractDecoratorRoutes` hook was dispatch + * guards plus static data route tables only, so a NestJS repo produced + * essentially no `Route` nodes. That is not a quiet gap: `route_map`, + * `api_impact` and `shape_check` all read `Route` nodes and answer "no routes + * matching …" when there are none — so `api_impact`, whose documented job is to + * be run BEFORE modifying a route handler, reported every live endpoint as + * non-existent, and a not-found reads as a safe change (#3009). + * + * The extraction mirrors `spring.ts`, which solves the identical shape for + * `@RequestMapping` + `@GetMapping`: collect class-level prefixes keyed by class + * node id, then walk method decorators and attach the prefix of their enclosing + * class. As there, the prefix travels on `ExtractedDecoratorRoute.prefix` and + * the routes phase performs the join via `normalizeExtractedRoutePath`, so + * NestJS routes are keyed identically to every other framework's. + */ + +import type Parser from 'tree-sitter'; +import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js'; + +/** NestJS method decorators → HTTP verb. */ +const NEST_METHOD_DECORATORS: Record = { + Get: 'GET', + Post: 'POST', + Put: 'PUT', + Patch: 'PATCH', + Delete: 'DELETE', + Head: 'HEAD', + Options: 'OPTIONS', + All: '*', +}; + +/** + * Cheap parse-free gate. Every JS/TS file in every repo reaches this hook, so + * skip the walk unless the file could plausibly declare a controller. A file + * without the substring cannot produce a route here, because a `@Controller` + * decorator is REQUIRED before any method decorator is believed (see below). + */ +const CONTROLLER_HINT = '@Controller'; + +/** The decorator's call expression, whether or not it has an argument list. */ +function decoratorCall(decorator: Parser.SyntaxNode): Parser.SyntaxNode | null { + const inner = decorator.namedChild(0); + if (!inner) return null; + if (inner.type === 'call_expression') return inner; + return null; +} + +/** The decorator's name — `Controller` for `@Controller('x')`, `Get` for `@Get()`. */ +function decoratorName(decorator: Parser.SyntaxNode): string | null { + const inner = decorator.namedChild(0); + if (!inner) return null; + // `@Get()` is a call_expression; a bare `@Injectable` is a plain identifier. + if (inner.type === 'identifier') return inner.text; + if (inner.type === 'call_expression') { + const fn = inner.childForFieldName('function'); + return fn?.type === 'identifier' ? fn.text : null; + } + return null; +} + +/** + * The literal string first argument of a decorator call, or `''` when the + * decorator takes no argument (`@Controller()` / `@Get()` — both legal and both + * meaning "no path segment of my own"). + * + * Returns `null` when an argument IS present but is not a plain literal. That + * is deliberately distinct from `''`: a computed prefix + * (`@Controller(ROUTES.VENUES)`) whose value we cannot read must drop the route + * rather than silently mount it at the wrong URL. `route_map` presents its + * output as fact, and a wrong path is worse than a missing one. + */ +function decoratorLiteralArg(decorator: Parser.SyntaxNode): string | null | undefined { + const call = decoratorCall(decorator); + if (!call) return ''; // bare `@Get` with no call — no path of its own + const args = call.childForFieldName('arguments'); + const first = args?.namedChild(0); + if (!first) return ''; // `@Get()` — no path of its own + + if (first.type === 'string' || first.type === 'template_string') { + if (first.namedChildren.some((c) => c.type === 'template_substitution')) return null; + const fragments = first.namedChildren.filter((c) => c.type === 'string_fragment'); + // An empty literal (`@Controller('')`) has no fragments and is a real ''. + return fragments.map((f) => f.text).join(''); + } + + // An array form (`@Get(['a', 'b'])`), an identifier, a member expression, a + // call — all unreadable here. Skip rather than guess. + return null; +} + +/** + * Decorators that immediately precede `node` among its parent's named children. + * In tree-sitter-typescript a decorator is a SIBLING placed before the thing it + * decorates — both at `export_statement`/`program` level for a class and inside + * `class_body` for a method — and decorators stack. + */ +function precedingDecorators(node: Parser.SyntaxNode): Parser.SyntaxNode[] { + const parent = node.parent; + if (!parent) return []; + const siblings = parent.namedChildren; + const index = siblings.findIndex((c) => c.id === node.id); + if (index < 0) return []; + + const out: Parser.SyntaxNode[] = []; + for (let i = index - 1; i >= 0; i--) { + const sibling = siblings[i]; + // A comment between the decorators and the thing they decorate is ordinary + // (`@Post('x')` then a JSDoc block then the method) and must not terminate + // the stack — doing so makes the whole decorated route invisible. + if (sibling.type === 'comment') continue; + if (sibling.type !== 'decorator') break; + out.push(sibling); + } + return out; +} + +/** + * Leading `decorator` children of a node, stopping at the first child that is + * neither a decorator nor a comment. Comments are skipped for the same reason + * as in {@link precedingDecorators}: a doc block sitting between `@Controller` + * and the class must not hide the decorator. + */ +function leadingDecorators(node: Parser.SyntaxNode): Parser.SyntaxNode[] { + const out: Parser.SyntaxNode[] = []; + for (const child of node.namedChildren) { + if (child.type === 'comment') continue; + if (child.type !== 'decorator') break; + out.push(child); + } + return out; +} + +/** + * Every decorator attached to a class, across the two shapes the grammar + * produces — which differ by whether the class is exported: + * + * `@Controller('a') class A {}` → decorator is a CHILD of class_declaration + * `@Controller('a') export class A {}` → decorator is a child of export_statement, + * i.e. a SIBLING of the class_declaration + * + * Checking only one of them silently drops half of all controllers, so collect + * from both plus the sibling position for either node. + */ +function classDecorators(classNode: Parser.SyntaxNode): Parser.SyntaxNode[] { + const out = [...leadingDecorators(classNode), ...precedingDecorators(classNode)]; + const wrapper = classNode.parent; + if (wrapper?.type === 'export_statement') { + out.push(...leadingDecorators(wrapper), ...precedingDecorators(wrapper)); + } + return out; +} + +/** The `@Controller(...)` prefix for a class, or undefined when it has none. */ +function controllerPrefix(classNode: Parser.SyntaxNode): string | null | undefined { + for (const decorator of classDecorators(classNode)) { + if (decoratorName(decorator) !== 'Controller') continue; + return decoratorLiteralArg(decorator); + } + return undefined; +} + +/** + * Extract NestJS routes from one parsed TypeScript/JavaScript file. + * + * A method decorator is only believed when its enclosing class carries a + * `@Controller`. `@Get`/`@Post`/`@Delete` are common identifiers, and without + * that requirement any unrelated library using the same decorator names would + * mint phantom endpoints. + */ +export function extractNestRoutes( + tree: Parser.Tree, + filePath: string, + lineOffset = 0, +): ExtractedDecoratorRoute[] { + if (!tree.rootNode.text.includes(CONTROLLER_HINT)) return []; + + const out: ExtractedDecoratorRoute[] = []; + + const visit = (node: Parser.SyntaxNode): void => { + if (node.type === 'class_declaration') { + const prefix = controllerPrefix(node); + // `undefined` — not a controller at all. `null` — a controller whose + // prefix could not be read, so its routes' URLs are unknowable. + if (prefix !== undefined) { + if (prefix !== null) collectClassRoutes(node, prefix, filePath, lineOffset, out); + return; // a controller's methods are handled here; don't re-walk them + } + } + for (const child of node.namedChildren) visit(child); + }; + + visit(tree.rootNode); + return out; +} + +function collectClassRoutes( + classNode: Parser.SyntaxNode, + prefix: string, + filePath: string, + lineOffset: number, + out: ExtractedDecoratorRoute[], +): void { + const body = classNode.childForFieldName('body'); + if (!body) return; + + for (const member of body.namedChildren) { + if (member.type !== 'method_definition') continue; + + for (const decorator of precedingDecorators(member)) { + const name = decoratorName(decorator); + if (name === null) continue; + const httpMethod = NEST_METHOD_DECORATORS[name]; + if (!httpMethod) continue; + + const routePath = decoratorLiteralArg(decorator); + if (routePath === null || routePath === undefined) continue; // unreadable → skip + + const handlerName = member.childForFieldName('name')?.text; + + out.push({ + filePath, + // A pathless `@Get()` is the controller's index route and carries no + // segment of its own. Emit '/' rather than '': `claim()` in + // call-processor short-circuits on a falsy routePath, so an empty + // string would still produce the Route node but silently lose its + // handler symbol — the route would exist with nothing attached to it. + // Both spellings normalize to the same URL against the prefix. + routePath: routePath === '' ? '/' : routePath, + httpMethod, + decoratorName: name, + lineNumber: member.startPosition.row + 1 + lineOffset, + prefix: prefix === '' ? null : prefix, + ...(handlerName === undefined ? {} : { handlerName }), + }); + } + } +} diff --git a/gitnexus/src/storage/parse-cache.ts b/gitnexus/src/storage/parse-cache.ts index f5c8be27f..3a5fcab0f 100644 --- a/gitnexus/src/storage/parse-cache.ts +++ b/gitnexus/src/storage/parse-cache.ts @@ -538,7 +538,15 @@ import type { ParseWorkerResult } from '../core/ingestion/workers/parse-worker.j // cache would replay unchanged worker results without those routes. Version 70 // then adds Spring non-HTTP handler side-channel facts (#2417 / #2891), so Java // and Kotlin caches persist scheduled, event, messaging, and managed-job facts. -const SCHEMA_BUMP = 70; +// +// 70 -> 71 adds #3009's NestJS decorator routes to the JS/TS decoratorRoutes +// channel. Same reasoning as 69: a warm v70 cache replays unchanged worker +// results, which for every already-indexed NestJS repo means replaying the +// empty route set this change exists to fix — the fix would appear to do +// nothing until something else invalidated the cache. +// RE-CHECK AGAINST origin/main IMMEDIATELY BEFORE MERGING (origin/main was 70 +// at the time of writing). +const SCHEMA_BUMP = 71; const GITNEXUS_PKG_VERSION = (() => { try { // package.json sits at gitnexus/package.json — two levels up from diff --git a/gitnexus/test/unit/incremental-parse-cache.test.ts b/gitnexus/test/unit/incremental-parse-cache.test.ts index 17595f15b..a0c72b311 100644 --- a/gitnexus/test/unit/incremental-parse-cache.test.ts +++ b/gitnexus/test/unit/incremental-parse-cache.test.ts @@ -218,11 +218,14 @@ describe('PARSE_CACHE_VERSION', () => { // the pre-fix fan-out. This branch staged 64 above the claims live at the // time (61, 62, 63); all three landed and cascaded main to 67, so 68 is the // next free value above every claim at merge — the rule, re-applied. + // Version 71 adds #3009's NestJS decorator routes to the same JS/TS + // decoratorRoutes channel, so a warm v70 cache cannot replay the empty + // route set that change fixes. // Version 69 added #2969's JS/TS data-route-table decoratorRoutes. Version 70 // adds Spring non-HTTP handler side-channel facts (#2417 / #2891), so it is // the next free value after both cache payload changes. - it('pins SCHEMA_BUMP to 70 so concurrent bumps cannot silently collide (#2766)', () => { - expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(70); + it('pins SCHEMA_BUMP to 71 so concurrent bumps cannot silently collide (#2766)', () => { + expect(Number(PARSE_CACHE_VERSION.split('+', 1)[0])).toBe(71); // The PREVIOUS version must fail the reuse gate, not merely differ from the // current one — a hardcoded number outside the conflict hunk rebases cleanly // while being wrong, which is exactly how the 37/38 exact clashes landed. diff --git a/gitnexus/test/unit/nest-decorator-routes.test.ts b/gitnexus/test/unit/nest-decorator-routes.test.ts new file mode 100644 index 000000000..82698ff19 --- /dev/null +++ b/gitnexus/test/unit/nest-decorator-routes.test.ts @@ -0,0 +1,232 @@ +import { describe, expect, it } from 'vitest'; +import Parser from 'tree-sitter'; +import TypeScript from 'tree-sitter-typescript'; +import { extractNestRoutes } from '../../src/core/ingestion/route-extractors/nest.js'; +import { normalizeExtractedRoutePath } from '../../src/core/ingestion/route-extractors/route-path.js'; + +const tsParser = new Parser(); +tsParser.setLanguage(TypeScript.typescript); + +const extract = (source: string) => + extractNestRoutes(tsParser.parse(source), 'src/x.controller.ts'); + +/** What the routes phase will key the Route node by: verb + joined path. */ +const urls = (source: string) => + extract(source).map( + (r) => `${r.httpMethod} ${normalizeExtractedRoutePath(r.routePath, r.prefix ?? null)}`, + ); + +describe('NestJS decorator routes', () => { + it('joins the controller prefix with each method path', () => { + expect( + urls(` + @Controller('venues') + export class VenueController { + @Get() + findAll() {} + + @Get('search') + search() {} + + @Post(':id/follow') + follow(@Param('id') id: string) {} + + @Delete(':id') + remove() {} + } + `), + ).toEqual([ + 'GET /venues', + 'GET /venues/search', + 'POST /venues/:id/follow', + 'DELETE /venues/:id', + ]); + }); + + it("emits '/' rather than '' for a pathless @Get, so the handler still resolves", () => { + // call-processor's claim() short-circuits on a falsy routePath, so '' + // would create the Route node but silently lose its handler symbol. + // Both spellings normalize to the same URL. + const [route] = extract(` + @Controller('venues') + export class VenueController { + @Get() + findAll() {} + } + `); + expect(route.routePath).toBe('/'); + expect(normalizeExtractedRoutePath(route.routePath, route.prefix ?? null)).toBe('/venues'); + }); + + it('handles a prefixless @Controller()', () => { + expect( + urls(` + @Controller() + export class AppController { + @Get('health') + health() {} + } + `), + ).toEqual(['GET /health']); + }); + + it('captures the handler method name for symbol resolution', () => { + const routes = extract(` + @Controller('users') + export class UserController { + @Patch(':id') + updateOne() {} + } + `); + + expect(routes).toHaveLength(1); + expect(routes[0]).toMatchObject({ + httpMethod: 'PATCH', + routePath: ':id', + prefix: 'users', + decoratorName: 'Patch', + handlerName: 'updateOne', + filePath: 'src/x.controller.ts', + }); + }); + + it('supports a non-exported controller and all verbs', () => { + expect( + urls(` + @Controller('a') + class A { + @Put('p') p() {} + @Head('h') h() {} + @Options('o') o() {} + @All('any') any() {} + } + `), + ).toEqual(['PUT /a/p', 'HEAD /a/h', 'OPTIONS /a/o', '* /a/any']); + }); + + it('applies each controller its own prefix when a file declares several', () => { + expect( + urls(` + @Controller('one') + export class One { @Get('x') x() {} } + + @Controller('two') + export class Two { @Get('y') y() {} } + `), + ).toEqual(['GET /one/x', 'GET /two/y']); + }); + + it('carries stacked decorators through to the route', () => { + expect( + urls(` + @Controller('secure') + export class SecureController { + @UseGuards(AuthGuard) + @Get('me') + me() {} + } + `), + ).toEqual(['GET /secure/me']); + }); + + it('sees through a comment between the decorators and the method', () => { + // Found on a real controller: four stacked decorators, then a JSDoc block, + // then the method. Breaking the backward walk at the comment made the + // entire route invisible. + expect( + urls(` + @Controller('dev') + export class DevController { + @Post('simulate-expiry') + @HttpCode(HttpStatus.OK) + @ApiOperation({ summary: 'x' }) + /** + * Simulate an expiry. + */ + simulateExpiry() {} + } + `), + ).toEqual(['POST /dev/simulate-expiry']); + }); + + it('sees through a comment between @Controller and the class', () => { + expect( + urls(` + @Controller('docs') + /** The controller. */ + export class DocsController { + @Get('x') x() {} + } + `), + ).toEqual(['GET /docs/x']); + }); + + // ─── Precision guards ────────────────────────────────────────────── + + it('ignores verb-named decorators on a class that is not a @Controller', () => { + // `Get`/`Post` are ordinary identifiers; without the @Controller + // requirement any library reusing those names mints phantom endpoints. + expect( + extract(` + @Injectable() + export class NotAController { + @Get('looks-like-a-route') + nope() {} + } + `), + ).toEqual([]); + }); + + it('drops a route whose path is a constant it cannot read', () => { + // A wrong URL is worse than a missing one — route_map presents this as fact. + expect( + extract(` + @Controller('x') + export class C { + @Get(ROUTES.SEARCH) + search() {} + } + `), + ).toEqual([]); + }); + + it('drops every route of a controller whose prefix cannot be read', () => { + expect( + extract(` + @Controller(BASE_PATH) + export class C { + @Get('search') + search() {} + } + `), + ).toEqual([]); + }); + + it('drops an interpolated template path rather than emitting its source text', () => { + expect( + extract(` + @Controller('x') + export class C { + @Get(\`\${prefix}/search\`) + search() {} + } + `), + ).toEqual([]); + }); + + it('ignores an array-form path it cannot reduce to one URL', () => { + expect( + extract(` + @Controller('x') + export class C { + @Get(['a', 'b']) + multi() {} + } + `), + ).toEqual([]); + }); + + it('returns nothing for a file with no @Controller at all', () => { + expect(extract(`export function get() { return 1; }`)).toEqual([]); + }); +});