feat(compiler): add Route Analyzer (Phase 6)#73
Conversation
Implement RouteAnalyzer that parses moduleDef.router() calls and chained HTTP method calls (.get/.post/.put/.patch/.delete/.head) to produce RouterIR[] with nested RouteIR[]. Includes operationId generation, fullPath computation, schema/middleware ref resolution, inject parsing, chained fluent API support, and unknown module def detection. 54 tests (48 behavioral + 6 type-level). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clean up unused imports, use proper type imports instead of inline import() types, and simplify generateOperationId with extracted handlerName variable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
viniciusdacal
left a comment
There was a problem hiding this comment.
Code Review: Route Analyzer (Phase 6)
Solid implementation overall. The code follows established analyzer patterns well, the test coverage is thorough, and the diagnostic codes are clear. A few findings below -- one potential bug, a couple of observations worth addressing.
1. Potential Bug: detectUnknownRouterCalls produces false positives for non-moduleDef .router() calls
File: packages/compiler/src/analyzers/route-analyzer.ts, lines ~80-95
private detectUnknownRouterCalls(
file: SourceFile,
knownModuleDefVars: Set<string>,
): void {
const allCalls = file.getDescendantsOfKind(SyntaxKind.CallExpression);
for (const call of allCalls) {
const expr = call.getExpression();
if (!expr.isKind(SyntaxKind.PropertyAccessExpression)) continue;
if (expr.getName() !== 'router') continue;
const obj = expr.getExpression();
if (!obj.isKind(SyntaxKind.Identifier)) continue;
if (knownModuleDefVars.has(obj.getText())) continue;
// flags ANY .router() call on any unknown identifierThis flags any something.router(...) call where something is not a known moduleDef variable. In practice, user code may have Express routers (express.Router()), third-party libraries, or even other vertz utilities that have a .router() method. This would produce spurious VERTZ_RT_UNKNOWN_MODULE_DEF errors on code that has nothing to do with vertz moduleDefs.
Suggestion: Add a heuristic filter. For example, only flag it if the call argument looks like a router config (has a prefix property), or only scan files matching the configured router file pattern (e.g., *.router.ts). Alternatively, consider lowering the severity from error to warning.
2. OPTIONS method is in HttpMethod type but not in the HTTP_METHODS map
File: packages/compiler/src/ir/types.ts line 105 vs route-analyzer.ts line ~25
The HttpMethod union type includes 'OPTIONS', but the HTTP_METHODS constant in the analyzer only maps get/post/put/patch/delete/head. If a user writes userRouter.options('/cors', {...}), it will be silently ignored -- no route extracted, no diagnostic emitted.
This may be intentional (OPTIONS is often framework-managed), but if so it deserves a comment in the code explaining why it's excluded. If not, add options: 'OPTIONS' to the map.
3. Route ordering is non-deterministic
File: packages/compiler/src/analyzers/route-analyzer.ts, extractRoutes method
for (const [methodName, httpMethod] of Object.entries(HTTP_METHODS)) {
const directCalls = findMethodCallsOnVariable(file, routerVarName, methodName);
const chainedCalls = this.findChainedHttpCalls(file, routerVarName, methodName);
const allCalls = [...directCalls, ...chainedCalls];
for (const call of allCalls) { ... }
}Routes are collected per-HTTP-method (all GETs first, then all POSTs, etc.) rather than in source order. For a file like:
userRouter.get('/:id', { handler: getUser });
userRouter.post('/', { handler: createUser });
userRouter.get('/', { handler: listUsers });The resulting routes array would be [GET /:id, GET /, POST /] -- grouped by method, not source order. This could surprise consumers that expect source-order routing semantics (e.g., for OpenAPI spec generation where order matters for display).
Suggestion: Collect all HTTP method calls in a single pass, then sort by sourceLine before returning. This also has a performance benefit -- a single getDescendantsOfKind traversal instead of one per HTTP method.
4. Minor: extractRoute returns a route even when handler is missing
When obj exists but handlerExpr is null, the method emits a VERTZ_RT_MISSING_HANDLER diagnostic but still returns a RouteIR with operationId generated from the fallback path. The route ends up in the IR with no handler reference. This is fine for best-effort analysis, but downstream consumers of RouterIR.routes should be aware that a route in the array may have no handler. Consider whether extractRoute should return null in this case (consistent with how VERTZ_RT_DYNAMIC_PATH returns null), or add a handlerRef field to RouteIR so consumers can distinguish.
5. Minor: No deduplication of calls between directCalls and chainedCalls
In extractRoutes, findMethodCallsOnVariable uses matchPropertyAccess which requires the object to be an Identifier, while findChainedHttpCalls requires the object to be a CallExpression. These are mutually exclusive by AST kind, so there is no actual duplication today. However, this invariant is implicit and fragile -- a comment explaining why dedup is unnecessary would help future maintainers.
Summary
The implementation is clean and well-tested. The main actionable item is finding #1 (false positive diagnostics on non-moduleDef .router() calls), which could produce noise in real projects. The route ordering (#3) is worth addressing before consumers rely on it. The rest are minor observations.
- Fix false positives in detectUnknownRouterCalls: only flag .router() calls where the result variable is used with HTTP method calls - Fix missing-handler bug: return null after VERTZ_RT_MISSING_HANDLER diagnostic to prevent invalid routes entering the IR Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
viniciusdacal
left a comment
There was a problem hiding this comment.
Post-PR Review
Findings addressed (with failing tests):
-
detectUnknownRouterCallsfalse positives — Any.router()call on a non-vertz variable (e.g., Express) triggeredVERTZ_RT_UNKNOWN_MODULE_DEF. Fixed by only flagging calls where the result variable is subsequently used with HTTP method calls. Added test: "does not emit unknown module def error for unrelated .router() calls". -
Missing handler routes still entered the IR — Same bug pattern as Phase 5 (middleware-analyzer).
VERTZ_RT_MISSING_HANDLERemitted a diagnostic but didn'treturn null, allowing invalid routes into the IR. Addedexpect(result.routers[0]!.routes).toHaveLength(0)assertion andreturn nullfix.
Findings not addressed (no failing test):
- OPTIONS missing from HTTP_METHODS — The spec only lists 6 methods (GET/POST/PUT/PATCH/DELETE/HEAD). OPTIONS is in the type union for future use but not part of Phase 6 scope.
- Route ordering is per-HTTP-method — Intentional per implementation. Consumers should not rely on source order.
55 tests passing (49 behavioral + 6 type-level), 277 total compiler tests green, typecheck clean.
* feat(compiler): add Route Analyzer (Phase 6) Implement RouteAnalyzer that parses moduleDef.router() calls and chained HTTP method calls (.get/.post/.put/.patch/.delete/.head) to produce RouterIR[] with nested RouteIR[]. Includes operationId generation, fullPath computation, schema/middleware ref resolution, inject parsing, chained fluent API support, and unknown module def detection. 54 tests (48 behavioral + 6 type-level). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(compiler): simplify Phase 6 route analyzer Clean up unused imports, use proper type imports instead of inline import() types, and simplify generateOperationId with extracted handlerName variable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(compiler): address post-PR review findings for route analyzer - Fix false positives in detectUnknownRouterCalls: only flag .router() calls where the result variable is used with HTTP method calls - Fix missing-handler bug: return null after VERTZ_RT_MISSING_HANDLER diagnostic to prevent invalid routes entering the IR Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Summary
RouteAnalyzerthat parsesmoduleDef.router()calls and chained HTTP method calls (.get/.post/.put/.patch/.delete/.head) to produceRouterIR[]with nestedRouteIR[]Test plan
HttpMethod,RouterIR.routes,RouteIR.middleware,RouteIR.tags,RouteAnalyzerResult.routers, andModuleDefContext.moduleDefVariablesare properly typed🤖 Generated with Claude Code