refactor(ingestion): simplify and speed up the NestJS route extractor

Quality pass over the extractor and its tests. Behaviour is unchanged: verified
by a differential harness running the pre-change and post-change extractors over
7464 cases — every backtick-quoted source in the unit suite plus every .ts/.tsx/
.js/.mjs/.cjs file under gitnexus/src and test/fixtures, each also probed with
the file gate forced open, across the TypeScript, TSX and JavaScript grammars.
Zero output differences.

Efficiency:

- `precedingDecorators` walks the sibling chain instead of indexing into
  `parent.namedChildren`. This was the same uncached-getter trap the earlier fix
  removed from `collectClassRoutes`, left in place one level up: a class's parent
  is usually `program`, so reading that list marshalled every top-level statement
  in the file into fresh JS objects, once per class — quadratic in top-level
  statements rather than in methods. A file of 800 classes went 177.7ms -> 10.2ms,
  and the curve is linear instead of quadratic (50/200/800 classes:
  1.87/13.96/177.72ms before, 0.82/2.66/10.24ms after). The new form is also
  shorter than the one it replaces.

  Deliberately NOT applied to `collectClassRoutes` or `leadingDecorators`: one
  bulk `namedChildren` marshal beats N individual sibling steps there, and the
  sibling form measured consistently slower on the hit path. The trap is
  specifically marshalling a whole list to use one item.

Simplification:

- `classDecorators` dropped its `precedingDecorators(wrapper)` call. Both
  grammars fold a class's decorators into the `export_statement` production, so
  an `export_statement` never has one as a preceding sibling; probed across 15
  export shapes in both grammars and it was empty in every one. The comment now
  says there is no fourth source, so the next reader does not have to re-derive
  it. `leadingDecorators(wrapper)` stays — it is the one that finds decorators
  on an exported class, and it is also the only one that survives the JavaScript
  grammar's ERROR-recovery tree for `export abstract class`.

- `collectClassRoutes` no longer states "a non-decorator member ends the run"
  twice. The `!== 'method_definition'` arm existed only to reset the accumulator
  and continue, which the method arm also did at the bottom; hoisting the reset
  removes the branch and the second reset site.

- `decoratorCall` was a one-caller wrapper whose null-vs-not-a-call distinction
  its only caller discarded — both mapped to `''`. Inlined to one `?.type !==`
  check, which also removes a derivable intermediate binding.

- `objectKeyName` was a fourth-of-a-kind: `data-route-table.ts` already had
  `propertyName`, with the same contract and one more accepted key spelling.
  Exported it rather than keeping two readers that would drift the moment either
  side learned a new key shape. The `pathPair`/`value` pair collapsed into one
  lookup at the same time.

Tests:

- The unit suite's `urls` and `jsUrls` duplicated the whole verb-plus-joined-path
  formatting to vary one parser; that expression is the thing a reader must check
  is identical on both sides, so it is now written once.
- Three tests differing in a single token — an unreadable constant, an
  interpolated template, an array form — became one `it.each`, matching the two
  `it.each` blocks the file already had.
- The pipeline suite dropped its identities-only assertion. It was strictly
  subsumed by the full-table assertion below it, with a worse failure message;
  the invariant it named moved into that test's name and comment. The narrower
  projections and the two non-vacuity gates stay — those fail more usefully than
  the table does.
This commit is contained in:
Gergo Magyar 2026-08-26 13:05:14 +00:00
parent ce4f85ac34
commit 06ca4f73e3
4 changed files with 65 additions and 103 deletions

View file

@ -136,7 +136,12 @@ export function plainString(node: SyntaxNode): string | null {
return null;
}
function propertyName(node: SyntaxNode): string | null {
/**
* A property key's name, for the spellings that carry one — `{ path: … }` and
* `{ 'path': … }`. A computed key (`{ [KEY]: … }`) has none. Shared with the
* NestJS extractor, which reads `@Controller({ path: … })` the same way.
*/
export function propertyName(node: SyntaxNode): string | null {
if (node.type === 'identifier' || node.type === 'property_identifier') return node.text;
if (node.type === 'string') return plainString(node);
return null;

View file

@ -34,7 +34,7 @@
import type Parser from 'tree-sitter';
import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js';
import { plainString } from './data-route-table.js';
import { plainString, propertyName } from './data-route-table.js';
/**
* NestJS method decorators → HTTP verb. A Map rather than an object literal
@ -78,14 +78,6 @@ const CLASS_DECLARATION_TYPES: ReadonlySet<string> = new Set([
*/
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);
@ -119,25 +111,13 @@ function decoratorName(decorator: Parser.SyntaxNode): string | null {
* `: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.
*/
/**
* 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
const call = decorator.namedChild(0);
// A bare `@Injectable` with no call, or `@Get()` with no argument — legal,
// and both mean "no path segment of my own".
if (call?.type !== 'call_expression') return '';
const first = call.childForFieldName('arguments')?.namedChild(0);
if (!first) return '';
// `@Controller({ path: 'cats', version: '1' })` is the documented form for
// URI/header versioning, and its path is a plain literal sitting right there.
@ -145,12 +125,16 @@ function decoratorLiteralArg(decorator: Parser.SyntaxNode): string | null {
// 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);
// `propertyName` reads both spellings that carry a name — `{ path: … }` and
// `{ 'path': … }` — so the class is not dropped over a pair of quotes. A
// computed key (`{ [KEY]: … }`) has none, and keeps the drop, as does a
// computed value.
const path = first.namedChildren.find((child) => {
const key = child.type === 'pair' ? child.childForFieldName('key') : null;
return key !== null && propertyName(key) === 'path';
});
const value = path?.childForFieldName('value');
return value ? plainString(value) : null;
}
// An array form (`@Get(['a', 'b'])`), an identifier, a member expression, a
@ -162,19 +146,19 @@ function decoratorLiteralArg(decorator: Parser.SyntaxNode): string | 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.
* decorates — at `export_statement`/`program` level for a class — and
* decorators stack.
*
* Walks the sibling chain rather than indexing into `parent.namedChildren`,
* which is the same uncached-getter trap {@link collectClassRoutes} documents:
* a class's parent is usually `program`, so reading the list marshals every
* top-level statement in the file, once per class. That is quadratic in
* top-level statements — measured 200ms for a file of 800 classes, against
* 0.9ms for this form.
*/
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];
for (let sibling = node.previousNamedSibling; sibling; sibling = sibling.previousNamedSibling) {
// 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.
@ -210,14 +194,14 @@ function leadingDecorators(node: Parser.SyntaxNode): Parser.SyntaxNode[] {
* 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.
* from both, plus the sibling position for the class itself. There is no fourth
* source: both grammars fold a class's decorators INTO the `export_statement`
* production, so an `export_statement` never has one as a preceding sibling.
*/
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));
}
if (wrapper?.type === 'export_statement') out.push(...leadingDecorators(wrapper));
return out;
}
@ -298,10 +282,6 @@ function collectClassRoutes(
// 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
@ -310,7 +290,9 @@ function collectClassRoutes(
// 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 decorators =
member.type === 'method_definition' ? [...pending, ...leadingDecorators(member)] : [];
for (const decorator of decorators) {
const name = decoratorName(decorator);
if (name === null) continue;
const httpMethod = NEST_METHOD_DECORATORS.get(name);
@ -337,6 +319,10 @@ function collectClassRoutes(
...(handlerName === undefined ? {} : { handlerName }),
});
}
// Anything that is not a decorator or a comment ends the run — including
// the method that just consumed it, so a decorated FIELD's stack is never
// absorbed onto the method after it.
pending.length = 0;
}
}

View file

@ -88,19 +88,6 @@ describe('NestJS decorator route ingestion pipeline', () => {
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
@ -115,10 +102,14 @@ describe('NestJS decorator route ingestion pipeline', () => {
});
});
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.
it('joins each @Controller prefix with its method paths and resolves every handler', () => {
// Two invariants, one table, because a projection of this assertion can
// only fail where this one already does. The join is what the extractor
// cannot do alone — it ships `prefix` and the routes phase folds it in via
// `normalizeExtractedRoutePath`, so these read `/venues/search`, not
// `search`. And `claim()` is first-writer-wins per `(method, url)`, so a
// mis-keyed route surfaces as a handler donated to the wrong URL rather
// than as an absence.
expect(routes()).toEqual([
{
identity: 'DELETE /venues/:id',

View file

@ -16,17 +16,17 @@ jsParser.setLanguage(JavaScript);
const extract = (source: string) =>
extractNestRoutes(tsParser.parse(source), 'src/x.controller.ts');
const extractJs = (source: string) =>
extractNestRoutes(jsParser.parse(source), 'src/x.controller.js');
/** What the routes phase will key the Route node by: verb + joined path. */
const urls = (source: string) =>
extract(source).map(
const format = (routes: ReturnType<typeof extract>) =>
routes.map(
(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)}`,
);
const urls = (source: string) => format(extract(source));
const jsUrls = (source: string) => format(extractJs(source));
describe('NestJS decorator routes', () => {
it('joins the controller prefix with each method path', () => {
@ -198,13 +198,17 @@ describe('NestJS decorator routes', () => {
).toEqual(['GET /y/real']);
});
it('drops a route whose path is a constant it cannot read', () => {
it.each([
{ label: 'a constant it cannot read', argument: 'ROUTES.SEARCH' },
{ label: 'an interpolated template', argument: '`${prefix}/search`' },
{ label: 'an array form it cannot reduce to one URL', argument: "['a', 'b']" },
])('drops a route whose path is $label', ({ argument }) => {
// 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)
@Get(${argument})
search() {}
}
`),
@ -223,30 +227,6 @@ describe('NestJS decorator routes', () => {
).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([]);
});