diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 37bfa0875..6fe547b70 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 (`@Controller` + `@Get`/`@Post`/…), **JS/TS dispatch guards and static data route tables** | +| AST-level route in a normal file | `extractDecoratorRoutes` | Spring, FastAPI, NestJS (`@Controller` + `@Get`/`@Post`/…; URLs are controller-relative — `setGlobalPrefix` and URI versioning live in the bootstrap file and are not applied), **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/route-extractors/data-route-table.ts b/gitnexus/src/core/ingestion/route-extractors/data-route-table.ts index 053d207fe..8fb369fea 100644 --- a/gitnexus/src/core/ingestion/route-extractors/data-route-table.ts +++ b/gitnexus/src/core/ingestion/route-extractors/data-route-table.ts @@ -116,7 +116,14 @@ function decodeJavaScriptStringLiteral(raw: string): string | null { return decoded; } -function plainString(node: SyntaxNode): string | null { +/** + * A `string`/`template_string` node's decoded value, or `null` when it is not a + * readable literal (an interpolated template, an unterminated escape). Shared + * with the NestJS extractor so both agree on what a readable literal is — + * notably that escapes must be DECODED, not dropped, because tree-sitter splits + * a literal around every `escape_sequence`. + */ +export function plainString(node: SyntaxNode): string | null { if (node.type === 'string') return decodeJavaScriptStringLiteral(node.text); if ( node.type === 'template_string' && diff --git a/gitnexus/src/core/ingestion/route-extractors/nest.ts b/gitnexus/src/core/ingestion/route-extractors/nest.ts index a8ec4c60a..e27b7c4c2 100644 --- a/gitnexus/src/core/ingestion/route-extractors/nest.ts +++ b/gitnexus/src/core/ingestion/route-extractors/nest.ts @@ -20,22 +20,55 @@ * 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. + * + * Known limitation: the URLs produced here are CONTROLLER-RELATIVE. A global + * prefix (`app.setGlobalPrefix('api')`) and URI versioning are applied by the + * bootstrap file, not by any decorator this file can see, so neither is + * reflected — a route served at `/api/v1/venues/search` is stored as + * `/venues/search`. The module's "drop rather than guess" floor is unavailable + * for it: the evidence lives in a different file, so honouring it would mean + * dropping every Nest route in every repo. `spring.ts` has the same hole for + * `server.servlet.context-path`; `ExtractedDecoratorRoute.prefix` is the + * channel a cross-file follow-up would use, the way FastAPI resolves its mount. */ import type Parser from 'tree-sitter'; import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js'; +import { plainString } from './data-route-table.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: '*', -}; +/** + * NestJS method decorators → HTTP verb. A Map rather than an object literal + * because the lookup key is an arbitrary decorator name read out of source: a + * plain object answers `@toString()` with `Object.prototype.toString`, which is + * truthy and would be emitted verbatim as the route's httpMethod. + */ +const NEST_METHOD_DECORATORS: ReadonlyMap = new Map([ + ['Get', 'GET'], + ['Post', 'POST'], + ['Put', 'PUT'], + ['Patch', 'PATCH'], + ['Delete', 'DELETE'], + ['Head', 'HEAD'], + ['Options', 'OPTIONS'], + ['All', '*'], + // `@Sse` mounts a real GET endpoint that streams; it is as much a route as + // `@Get`. `@Search` is deliberately absent — `normalizeRouteMethod` rejects + // SEARCH as non-standard and would key the route by URL alone, colliding + // with every other verb on that path. + ['Sse', 'GET'], +]); + +/** + * Class node types that can carry a `@Controller`. `export abstract class C` + * parses as `abstract_class_declaration`, a DIFFERENT node type — and a + * decorated abstract base sharing CRUD routes with its subclasses is ordinary + * Nest, so matching `class_declaration` alone silently drops the whole + * controller rather than one route. + */ +const CLASS_DECLARATION_TYPES: ReadonlySet = new Set([ + 'class_declaration', + 'abstract_class_declaration', +]); /** * Cheap parse-free gate. Every JS/TS file in every repo reaches this hook, so @@ -76,24 +109,54 @@ function decoratorName(decorator: Parser.SyntaxNode): string | null { * (`@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. + * + * Reading the literal is delegated to `plainString`, the same judge the + * data-route-table extractor uses, so both agree on what is readable. Filtering + * `string_fragment` children and joining them looks equivalent and is not: + * tree-sitter SPLITS a literal around each `escape_sequence`, and the join then + * DELETES the escape rather than decoding it. `@Get(':id(\\d+)')` — the ordinary + * spelling of a Nest regex param, whose value is `:id(\d+)` — came out as + * `:id(d+)`, and `@Get('/v\u0069ews')` came out as `/vews`. Both are paths the + * app never serves, i.e. the wrong-URL outcome the paragraph above forbids. */ -function decoratorLiteralArg(decorator: Parser.SyntaxNode): string | null | undefined { +/** + * A property key's name, for the two spellings that carry one: `{ path: … }` + * (a `property_identifier`) and `{ 'path': … }` (a `string`). Reading only the + * first would drop the ENTIRE controller over a pair of quotes — the same + * whole-class cost the object form is handled to avoid. A computed key + * (`{ [KEY]: … }`) has no readable name and keeps the drop. + */ +function objectKeyName(pair: Parser.SyntaxNode): string | null { + const key = pair.childForFieldName('key'); + if (!key) return null; + return key.type === 'property_identifier' ? key.text : plainString(key); +} + +function decoratorLiteralArg(decorator: Parser.SyntaxNode): string | null { 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(''); + // `@Controller({ path: 'cats', version: '1' })` is the documented form for + // URI/header versioning, and its path is a plain literal sitting right there. + // Worth reading rather than dropping, because the asymmetry is severe: an + // unreadable METHOD path costs one route, an unreadable PREFIX costs every + // route on the class. + if (first.type === 'object') { + const pathPair = first.namedChildren.find( + (child) => child.type === 'pair' && objectKeyName(child) === 'path', + ); + const value = pathPair?.childForFieldName('value') ?? null; + // No `path` key, or a computed one, leaves the prefix unknowable. + return value === null ? null : plainString(value); } // An array form (`@Get(['a', 'b'])`), an identifier, a member expression, a - // call — all unreadable here. Skip rather than guess. - return null; + // call — none of them is a readable literal, and `plainString` answers `null` + // for every one of them. Skip rather than guess. + return plainString(first); } /** @@ -185,7 +248,7 @@ export function extractNestRoutes( const out: ExtractedDecoratorRoute[] = []; const visit = (node: Parser.SyntaxNode): void => { - if (node.type === 'class_declaration') { + if (CLASS_DECLARATION_TYPES.has(node.type)) { 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. @@ -211,17 +274,50 @@ function collectClassRoutes( const body = classNode.childForFieldName('body'); if (!body) return; - for (const member of body.namedChildren) { - if (member.type !== 'method_definition') continue; + // ONE forward pass over the body, accumulating the decorator run and flushing + // it at each method. Calling `precedingDecorators` per method instead is + // quadratic in methods-per-controller for a reason that is invisible in the + // source: `namedChildren` is an UNCACHED getter in node-tree-sitter, so every + // call re-marshals the entire class body into fresh JS objects before the + // `findIndex`. Measured here, 800 methods cost 362ms (450us/method, up from + // 42us/method at 50); a single pass is flat. `spring.ts` never had this + // because a Java annotation is a child of the declaration it annotates. + const pending: Parser.SyntaxNode[] = []; - for (const decorator of precedingDecorators(member)) { + for (const member of body.namedChildren) { + if (member.type === 'decorator') { + pending.push(member); + continue; + } + // Same reason as in `precedingDecorators`: a JSDoc block between a + // decorator stack and its method must not hide the route (a real + // controller shape, pinned by the suite). Known limitation of that skip: a + // decorator ORPHANED by a commented-out handler is then absorbed onto the + // NEXT method, minting a phantom route with the wrong handler. There is no + // AST fix — an orphan followed by a comment is indistinguishable from a + // stack whose method happens to be documented — and losing every + // documented route is the worse trade, so it is made deliberately. + if (member.type === 'comment') continue; + if (member.type !== 'method_definition') { + pending.length = 0; + continue; + } + + // tree-sitter-javascript makes a method decorator a CHILD of the + // `method_definition`, not a preceding sibling as in tree-sitter-typescript + // — and this extractor is registered on the JavaScript provider too, which + // already advertises `framework: 'nestjs'`. Reading only siblings meant + // every `.js` Nest controller emitted nothing. On TypeScript the first + // named child is the method name, so `leadingDecorators` contributes + // nothing there and no route is collected twice. + for (const decorator of [...pending, ...leadingDecorators(member)]) { const name = decoratorName(decorator); if (name === null) continue; - const httpMethod = NEST_METHOD_DECORATORS[name]; - if (!httpMethod) continue; + const httpMethod = NEST_METHOD_DECORATORS.get(name); + if (httpMethod === undefined) continue; const routePath = decoratorLiteralArg(decorator); - if (routePath === null || routePath === undefined) continue; // unreadable → skip + if (routePath === null) continue; // unreadable → skip const handlerName = member.childForFieldName('name')?.text; @@ -241,5 +337,6 @@ function collectClassRoutes( ...(handlerName === undefined ? {} : { handlerName }), }); } + pending.length = 0; } } diff --git a/gitnexus/test/fixtures/nest-route-app/src/health/health.controller.ts b/gitnexus/test/fixtures/nest-route-app/src/health/health.controller.ts new file mode 100644 index 000000000..462ba7717 --- /dev/null +++ b/gitnexus/test/fixtures/nest-route-app/src/health/health.controller.ts @@ -0,0 +1,14 @@ +import { Controller, Get } from '@nestjs/common'; + +/** + * A controller with no prefix of its own. `@Controller()` is legal NestJS and + * means "mount at the root", so the method path must reach the graph bare — + * this is the fixture half of the extractor's `prefix: '' -> null` mapping. + */ +@Controller() +export class HealthController { + @Get('health') + health(): string { + return 'ok'; + } +} diff --git a/gitnexus/test/fixtures/nest-route-app/src/legacy/legacy.controller.ts b/gitnexus/test/fixtures/nest-route-app/src/legacy/legacy.controller.ts new file mode 100644 index 000000000..d91a8ee82 --- /dev/null +++ b/gitnexus/test/fixtures/nest-route-app/src/legacy/legacy.controller.ts @@ -0,0 +1,16 @@ +import { Controller, Get } from '@nestjs/common'; + +const ROUTE_PREFIXES = { legacy: 'legacy' }; + +/** + * The prefix is not a literal, so the URLs of this controller's methods are + * unknowable here. `route_map` presents its output as fact: a missing route is + * recoverable, a route mounted at the wrong URL is not. + */ +@Controller(ROUTE_PREFIXES.legacy) +export class LegacyController { + @Get('reports') + legacyReports(): string { + return 'legacy'; + } +} diff --git a/gitnexus/test/fixtures/nest-route-app/src/venues/venues.controller.ts b/gitnexus/test/fixtures/nest-route-app/src/venues/venues.controller.ts new file mode 100644 index 000000000..4a519f31c --- /dev/null +++ b/gitnexus/test/fixtures/nest-route-app/src/venues/venues.controller.ts @@ -0,0 +1,35 @@ +import { Body, Controller, Delete, Get, Param, Post, Query } from '@nestjs/common'; +import { VenuesService } from './venues.service'; +import type { Venue } from './venues.service'; + +/** + * The shape #3009 is about: the URL is split across two decorators. The class + * decorator carries the prefix and the method decorator carries the verb plus + * the remainder, so neither half is a route on its own. + */ +@Controller('venues') +export class VenuesController { + constructor(private readonly venues: VenuesService) {} + + // Pathless index route: its URL is the controller prefix and nothing else. + @Get() + findAll(): Venue[] { + return this.venues.listVenues(); + } + + @Get('search') + search(@Query('q') term: string): Venue[] { + return this.venues.searchVenues(term); + } + + // Same URL as findAll(), different verb — two Route nodes, not one. + @Post() + create(@Body() input: Venue): Venue { + return this.venues.insertVenue(input); + } + + @Delete(':id') + remove(@Param('id') id: string): void { + this.venues.deleteVenue(id); + } +} diff --git a/gitnexus/test/fixtures/nest-route-app/src/venues/venues.service.ts b/gitnexus/test/fixtures/nest-route-app/src/venues/venues.service.ts new file mode 100644 index 000000000..5d6af8c20 --- /dev/null +++ b/gitnexus/test/fixtures/nest-route-app/src/venues/venues.service.ts @@ -0,0 +1,36 @@ +import { Injectable } from '@nestjs/common'; + +export interface Venue { + id: string; + name: string; +} + +/** + * Not a controller. It exists so each handler body calls a real symbol, and so + * the `@Controller` file gate is shown skipping a decorated class that declares + * no routes. + */ +@Injectable() +export class VenuesService { + private readonly rows: Venue[] = []; + + listVenues(): Venue[] { + return this.rows; + } + + searchVenues(term: string): Venue[] { + return this.rows.filter((row) => row.name.includes(term)); + } + + insertVenue(input: Venue): Venue { + this.rows.push(input); + return input; + } + + deleteVenue(id: string): void { + this.rows.splice( + this.rows.findIndex((row) => row.id === id), + 1, + ); + } +} diff --git a/gitnexus/test/integration/nest-route-pipeline.test.ts b/gitnexus/test/integration/nest-route-pipeline.test.ts new file mode 100644 index 000000000..7f9ee65da --- /dev/null +++ b/gitnexus/test/integration/nest-route-pipeline.test.ts @@ -0,0 +1,225 @@ +/** + * End-to-end coverage of NestJS `@Controller` + `@Get`/`@Post`/`@Delete` route + * ingestion (#3009). + * + * The unit suite (`test/unit/nest-decorator-routes.test.ts`) pins what the + * extractor RETURNS. Nothing there proves the return value becomes anything: + * the reported symptom was not a wrong `ExtractedDecoratorRoute`, it was + * `api_impact` — whose documented job is to be run BEFORE modifying a route + * handler — reporting every live endpoint as non-existent, because the graph + * held no `Route` nodes at all. That is the tier this file covers: the routes + * phase performing the prefix/path join, `claim()` in call-processor resolving + * `handlerName` to a real symbol UID, and `prefix: null` surviving both. + * + * The fixture lives at `test/fixtures/nest-route-app/`, mirroring + * `spring-route-app/` — Spring solves the identical two-decorator shape and + * `nest.ts` is modelled on it. + */ + +import { describe, it, expect, beforeAll } from 'vitest'; +import path from 'node:path'; +import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; +import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; +import type { PipelineResult } from '../../src/types/pipeline.js'; + +const FIXTURE = path.resolve(__dirname, '..', 'fixtures', 'nest-route-app'); + +// Compared against POSIX-normalized graph paths (see `routes()`), so these stay +// forward-slashed rather than going through `path.join` — the Windows shard +// would otherwise compare backslashes against the graph's forward slashes. +const CONTROLLER_FILE = 'src/venues/venues.controller.ts'; +const HEALTH_FILE = 'src/health/health.controller.ts'; + +interface RouteView { + /** `${method} ${url}` — the `routeNodeKey` identity, as a sortable string. */ + readonly identity: string; + /** The handler the route resolved to, or the literal `'undefined'`. */ + readonly handler: string; + readonly handlerLabel: string; + readonly handlerFile: string; +} + +describe('NestJS decorator route ingestion pipeline', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(FIXTURE, () => {}, {}); + }, 120_000); + + const nodes = (): readonly GraphNode[] => { + const out: GraphNode[] = []; + result.graph.forEachNode((node) => void out.push(node)); + return out; + }; + + const relationships = (): readonly GraphRelationship[] => { + const out: GraphRelationship[] = []; + result.graph.forEachRelationship((rel) => void out.push(rel)); + return out; + }; + + const routeNodes = (): readonly GraphNode[] => nodes().filter((node) => node.label === 'Route'); + + /** + * Unresolved fields are stringified rather than branched on, so a route that + * lost its handler reads as the literal `'undefined'` in the diff instead of + * quietly skipping an assertion. + */ + const routes = (): readonly RouteView[] => + routeNodes() + .map((node) => { + const handler = result.graph.getNode(String(node.properties.handlerSymbolId)); + return { + identity: `${String(node.properties.method)} ${String(node.properties.name)}`, + handler: String(handler?.properties.name), + handlerLabel: String(handler?.label), + handlerFile: String(handler?.properties.filePath).replaceAll('\\', '/'), + }; + }) + .sort((a, b) => a.identity.localeCompare(b.identity)); + + const identities = (): readonly string[] => routes().map((route) => route.identity); + + it('emits Route nodes at all — the reported symptom was zero', () => { + // Its own case because every expectation below is satisfiable by an empty + // graph in the ways that matter least: `not.toContain` passes vacuously on + // an empty array, and a full-table `toEqual` against `[]` reports a missing + // element rather than "the extractor never ran". + expect(routeNodes().length).toBeGreaterThan(0); + }); + + it('joins the @Controller prefix with each method path into the Route node URL', () => { + // `/venues/search`, not `search` and not `/search`: the join is what the + // extractor cannot do alone — it ships `prefix` and the routes phase folds + // it in via `normalizeExtractedRoutePath`. + expect(identities()).toEqual([ + 'DELETE /venues/:id', + 'GET /health', + 'GET /venues', + 'GET /venues/search', + 'POST /venues', + ]); + }); + + it('mounts a pathless @Get() at the controller prefix WITH its handler attached', () => { + // The reason `nest.ts` emits '/' instead of '' for a pathless decorator: + // `claim()` short-circuits on a falsy `routePath`, so '' would still create + // this Route node and silently drop `handlerSymbolId`. The unit test can + // only see `routePath === '/'` at the extractor boundary; the guard being + // load-bearing is observable here and nowhere else. + expect(routes().find((route) => route.identity === 'GET /venues')).toEqual({ + identity: 'GET /venues', + handler: 'findAll', + handlerLabel: 'Method', + handlerFile: CONTROLLER_FILE, + }); + }); + + it('resolves every route to the controller method that declared it', () => { + // `claim()` is first-writer-wins per `(method, url)`; a mis-keyed route + // would show up here as a handler donated to the wrong URL, not as an + // absence. + expect(routes()).toEqual([ + { + identity: 'DELETE /venues/:id', + handler: 'remove', + handlerLabel: 'Method', + handlerFile: CONTROLLER_FILE, + }, + { + identity: 'GET /health', + handler: 'health', + handlerLabel: 'Method', + handlerFile: HEALTH_FILE, + }, + { + identity: 'GET /venues', + handler: 'findAll', + handlerLabel: 'Method', + handlerFile: CONTROLLER_FILE, + }, + { + identity: 'GET /venues/search', + handler: 'search', + handlerLabel: 'Method', + handlerFile: CONTROLLER_FILE, + }, + { + identity: 'POST /venues', + handler: 'create', + handlerLabel: 'Method', + handlerFile: CONTROLLER_FILE, + }, + ]); + }); + + it('splits one URL into one Route node per verb', () => { + // `@Get()` and `@Post()` on the same controller share a URL. `routeNodeKey` + // makes them distinct identities; collapsing them would drop a live + // endpoint and hand its handler to the survivor. + expect( + routes() + .filter((route) => route.identity.endsWith(' /venues')) + .map((route) => `${route.identity} -> ${route.handler}`), + ).toEqual(['GET /venues -> findAll', 'POST /venues -> create']); + }); + + it('carries a prefix-less @Controller() through to a bare URL', () => { + // `@Controller()` maps to `prefix: null`, and null must reach the join as + // "no prefix" rather than as the string 'null' or a leading empty segment. + expect(identities()).toContain('GET /health'); + expect(identities().filter((id) => id.includes('//'))).toEqual([]); + }); + + it('records per-decorator provenance on the HANDLES_ROUTE edge', () => { + // Nest routes are DECLARED by an annotation, so they take the generic + // `decorator-` source rather than a bespoke one — the same channel + // Spring and FastAPI use, which is the point of modelling on `spring.ts`. + const routeIds = new Set(routeNodes().map((node) => node.id)); + expect( + [ + ...new Set( + relationships() + .filter((rel) => rel.type === 'HANDLES_ROUTE' && routeIds.has(rel.targetId)) + .map((rel) => String(rel.reason)), + ), + ].sort(), + ).toEqual(['decorator-Delete', 'decorator-Get', 'decorator-Post']); + }); + + describe('precision — what must NOT become a route', () => { + // Absence assertions are satisfied just as well by a file that was never + // read, so prove ingestion first; otherwise this whole block is decoration. + it('ingested the unreadable-prefix controller', () => { + expect( + nodes() + .filter((node) => String(node.properties.filePath ?? '').endsWith('legacy.controller.ts')) + .map((node) => String(node.properties.name)), + ).toEqual(expect.arrayContaining(['LegacyController', 'legacyReports'])); + }); + + it('drops a controller whose prefix is not a literal', () => { + // `@Controller(ROUTE_PREFIXES.legacy)` — the URL is unknowable, and + // `route_map` presents its output as fact. Emitting `/reports` here would + // be a wrong route, which is worse than a missing one. + expect(identities().filter((id) => id.includes('reports'))).toEqual([]); + expect(identities().filter((id) => id.includes('legacy'))).toEqual([]); + }); + + it('ingested the decorated non-controller service', () => { + expect( + nodes() + .filter((node) => String(node.properties.filePath ?? '').endsWith('venues.service.ts')) + .map((node) => String(node.properties.name)), + ).toEqual(expect.arrayContaining(['VenuesService', 'listVenues', 'searchVenues'])); + }); + + it('does not mint routes from a class with no @Controller', () => { + // Verb decorators are believed only inside a `@Controller` class; without + // that gate any library sharing the names would mint phantom endpoints. + expect(routes().filter((route) => route.handlerFile.endsWith('venues.service.ts'))).toEqual( + [], + ); + }); + }); +}); diff --git a/gitnexus/test/unit/nest-decorator-routes.test.ts b/gitnexus/test/unit/nest-decorator-routes.test.ts index 82698ff19..f6bfe2f55 100644 --- a/gitnexus/test/unit/nest-decorator-routes.test.ts +++ b/gitnexus/test/unit/nest-decorator-routes.test.ts @@ -1,12 +1,18 @@ import { describe, expect, it } from 'vitest'; import Parser from 'tree-sitter'; import TypeScript from 'tree-sitter-typescript'; +import JavaScript from 'tree-sitter-javascript'; 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); +// The extractor is registered on the JavaScript provider too, and the two +// grammars place a method decorator differently, so both must be exercised. +const jsParser = new Parser(); +jsParser.setLanguage(JavaScript); + const extract = (source: string) => extractNestRoutes(tsParser.parse(source), 'src/x.controller.ts'); @@ -16,6 +22,12 @@ const urls = (source: string) => (r) => `${r.httpMethod} ${normalizeExtractedRoutePath(r.routePath, r.prefix ?? null)}`, ); +/** The same, parsed with tree-sitter-javascript instead. */ +const jsUrls = (source: string) => + extractNestRoutes(jsParser.parse(source), 'src/x.controller.js').map( + (r) => `${r.httpMethod} ${normalizeExtractedRoutePath(r.routePath, r.prefix ?? null)}`, + ); + describe('NestJS decorator routes', () => { it('joins the controller prefix with each method path', () => { expect( @@ -166,15 +178,24 @@ describe('NestJS decorator routes', () => { 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. + // The unrelated controller is what makes this test reach the per-class + // check: without a `@Controller` anywhere the file short-circuits at the + // parse-free substring gate and the assertion proves nothing. expect( - extract(` + urls(` + @Controller('y') + export class RealController { + @Get('real') + real() {} + } + @Injectable() export class NotAController { @Get('looks-like-a-route') nope() {} } `), - ).toEqual([]); + ).toEqual(['GET /y/real']); }); it('drops a route whose path is a constant it cannot read', () => { @@ -229,4 +250,175 @@ describe('NestJS decorator routes', () => { it('returns nothing for a file with no @Controller at all', () => { expect(extract(`export function get() { return 1; }`)).toEqual([]); }); + + // ─── Literal decoding ────────────────────────────────────────────── + + it('decodes an escape in a path instead of deleting it', () => { + // The source below spells the Nest regex param the way a controller does, + // `@Get(':id(\\d+)')`, whose runtime value is `:id(\d+)`. tree-sitter SPLITS + // that literal around the escape_sequence, so keeping only the + // string_fragment children and joining them yielded `:id(d+)` — a URL the + // app never serves, i.e. the wrong-path outcome this module calls worse + // than a missing one. + const [route] = extract(` + @Controller('users') + export class UserController { + @Get(':id(\\\\d+)') + one() {} + } + `); + expect(route.routePath).toBe(':id(\\d+)'); + }); + + it('decodes a unicode escape rather than dropping its payload', () => { + expect( + urls(` + @Controller('v') + export class C { + @Get('/v\\u0069ews') + views() {} + } + `), + ).toEqual(['GET /v/views']); + }); + + it("treats an empty @Controller('') as carrying no prefix", () => { + const [route] = extract(` + @Controller('') + export class C { + @Get('a') a() {} + } + `); + expect(route.prefix).toBeNull(); + expect(normalizeExtractedRoutePath(route.routePath, route.prefix ?? null)).toBe('/a'); + }); + + // ─── Controller shapes ───────────────────────────────────────────── + + it('extracts routes from an abstract controller base class', () => { + // `export abstract class` parses as abstract_class_declaration, a separate + // node type — and a decorated abstract base sharing CRUD routes with its + // subclasses is ordinary Nest, so missing it drops the whole controller. + expect( + urls(` + @Controller('base') + export abstract class BaseController { + @Get('a') a() {} + } + `), + ).toEqual(['GET /base/a']); + }); + + it('reads the path out of the object form used for URI versioning', () => { + expect( + urls(` + @Controller({ path: 'cats', version: '1' }) + export class CatsController { + @Get('breeds') breeds() {} + } + `), + ).toEqual(['GET /cats/breeds']); + }); + + it('reads a quoted path key, rather than dropping the class over the quotes', () => { + expect( + urls(` + @Controller({ 'path': 'cats' }) + export class CatsController { + @Get('breeds') breeds() {} + } + `), + ).toEqual(['GET /cats/breeds']); + }); + + it.each([ + { label: 'no path key', argument: "{ version: '1' }" }, + { label: 'a computed path', argument: '{ path: BASE_PATH }' }, + { label: 'a computed path key', argument: "{ [PATH_KEY]: 'cats' }" }, + ])('still drops a controller whose object form has $label', ({ argument }) => { + expect( + extract(` + @Controller(${argument}) + export class C { + @Get('a') a() {} + } + `), + ).toEqual([]); + }); + + it('extracts from a .js controller, where a decorator is a CHILD of the method', () => { + // tree-sitter-javascript nests a method decorator inside method_definition + // rather than placing it before as a sibling. The same extractor serves the + // JavaScript provider, so reading siblings only meant every .js Nest + // controller emitted nothing while the wiring claimed nestjs coverage. + expect( + jsUrls(` + @Controller('venues') + export class VenueController { + @UseGuards(AuthGuard) + @Get('search') + search() {} + } + `), + ).toEqual(['GET /venues/search']); + }); + + // ─── Verb coverage ───────────────────────────────────────────────── + + it('treats @Sse as the GET endpoint it mounts', () => { + expect( + urls(` + @Controller('events') + export class EventsController { + @Sse('stream') stream() {} + } + `), + ).toEqual(['GET /events/stream']); + }); + + it('emits nothing for a decorator that mounts no endpoint', () => { + // Paired with the @Sse case above: without it, an unsupported route + // decorator and a non-route decorator are the same silent []. + expect( + extract(` + @Controller('events') + export class EventsController { + @UseGuards(AuthGuard) guarded() {} + } + `), + ).toEqual([]); + }); + + it.each(['toString', 'constructor'])( + 'does not mint a route for a decorator named @%s', + (name) => { + // The verb table is looked up by decorator name, so a plain object would + // answer `Object.prototype.toString` here — truthy, and emitted verbatim + // as the route's httpMethod. + expect( + extract(` + @Controller('x') + export class C { + @${name}() f() {} + } + `), + ).toEqual([]); + }, + ); + + it("does not carry a decorated property's decorators onto the next method", () => { + // The decorator run is accumulated in one forward pass over the class body; + // a non-method member must reset it, the way the backward walk used to stop. + expect( + urls(` + @Controller('di') + export class C { + @Inject(SERVICE) + private readonly svc: Service; + + @Get('a') a() {} + } + `), + ).toEqual(['GET /di/a']); + }); });