mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-25 05:53:44 +00:00
fix(routes): resolve Express registrations made through helper parameters (#3629)
* fix(routes): resolve Express registrations made through helper parameters (#3487) #3603 made Express receivers provenance-proven, which removed the receiver- name false positives but also every registration on a function parameter: a parameter has no binding value, so `app.get(...)` inside a helper never resolved. On GitNexus's own server that dropped `* /api/mcp` (`mountMCPEndpoints(app, ...)`) and `GET /` (`registerWebUI(app, ...)`), and the two SSE progress routes registered by `mountSSEProgress(app, '/api/.../progress', jobs)` were never found at all (#3487, item a). Named function values in the route facts now carry an identity and the binding id of each parameter (a TypeScript `this` parameter takes no position, and a parameter written anywhere in the body, nested closures included, takes none either). Calls to plain functions and to namespace or default import members are recorded as invocations, keeping only the arguments that can carry an allocation or a literal path; arguments after a spread have no position. The resolver resolves a registration whose receiver, literal path or mounted child is a parameter through each call site of its function, once per site and in the helper's file. This is one hop only: a site argument that is itself a parameter does not resolve. When no call site is recorded, the call is evaluated as written, so an unknown mount path still suppresses its child router instead of exposing its paths at the root. Only calls whose callee is named like a function that registers on its parameters are evaluated, which keeps resolution at its previous cost on repositories with many helper calls. A function value's invalidation key now includes its identity, so a property write on one function (`Button.displayName = ...`) no longer disables every other. The group layer shares the resolver, so it gets the same routes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(routes): forward mounted-child parameters and aliased helper imports Review follow-ups for the helper-parameter forwarding: - A helper that forwards only its mounted router (`function mountChild(child) { app.use('/api', child) }`) was never resolved, because the forwarded function was chosen from the receiver and path alone. Any parameter among the receiver and arguments now selects it, as long as all come from one function. - A helper called through an aliased import (`{ mountSSEProgress as mountProgress }`) was filtered out before its call sites were recorded, since the filter compared only the local name. It now also accepts the imported name. - A write to a same-named local declared in a nested block (`{ let app; app = other; }`) discarded the outer parameter. A write to a binding that a nested block, function or catch clause declares no longer counts. - The fixture's `registerWebUI` uses its static directory. The release tool-accuracy gate now passes `routes.helper-registration`, so its known-gap allowance is removed. Its expected file moves from the caller (`api.ts`) to the helper (`progress.ts`): an Express route belongs to the file of its `app.get` call, which is also where its handler and response keys are, as for every directly registered route. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(routes): treat destructuring reassignment as a parameter write `[app] = [other]` reassigns `app`, but the write check compared the whole target text (`[app]`) with the parameter name, so the parameter stayed forwardable and later registrations used the call-site argument. A destructuring target now writes each name it binds; a member target still writes only a property. Also drops an unused local flagged by CodeQL. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(routes): follow helpers exported under another name `export { mountRoutes as setup }` records the export as a binding, so the call-site filter knew the helper only as `mountRoutes` and skipped callers importing `setup`. An export of a binding to a named function now adds its exported name too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
This commit is contained in:
parent
aa33a6e94c
commit
565e50276d
12 changed files with 608 additions and 99 deletions
|
|
@ -212,6 +212,13 @@ They follow explicit ESM/CommonJS imports and exports through the configured
|
|||
`router` is not evidence; type-only imports, shadowed bindings, conflicting writes,
|
||||
and unproven factories do not establish a receiver. Directly proven Hono verb
|
||||
registrations remain supported, without applying Express mounting semantics.
|
||||
A registration whose receiver, literal path or mounted router is a parameter of a
|
||||
named function (`mountSSEProgress(app, '/p', jobs)` → `app.get(routePath, …)`)
|
||||
resolves through each call site of that function, once per site and in the
|
||||
helper's file. This is one hop only: a site argument that is itself a parameter,
|
||||
an unproven argument, or a reassigned parameter establishes nothing, and a helper
|
||||
with no recorded call site is read as written, so an unknown mount path still
|
||||
suppresses its router's standalone paths.
|
||||
|
||||
A standalone router retains its local paths. A known dynamic mount suppresses
|
||||
that fallback, while independently proven static mounts remain available. Cycles
|
||||
|
|
|
|||
|
|
@ -117,7 +117,9 @@ export const EXPECTATIONS = [
|
|||
id: 'routes.helper-registration',
|
||||
issue: 3487,
|
||||
title: 'Literal route forwarded through a local helper',
|
||||
expected: ['GET /api/progress src/server/api.ts'],
|
||||
// Like every Express route, it belongs to the file of its `app.get` call,
|
||||
// which is also where its handler and response keys are.
|
||||
expected: ['GET /api/progress src/server/progress.ts'],
|
||||
},
|
||||
{
|
||||
id: 'api.template-wrapper',
|
||||
|
|
|
|||
|
|
@ -2,12 +2,6 @@
|
|||
"schemaVersion": 1,
|
||||
"baselineSourceSha": "dd096e690c06e5d54ab3595c63b83f15e04b6191",
|
||||
"gaps": [
|
||||
{
|
||||
"id": "routes.helper-registration",
|
||||
"issue": 3487,
|
||||
"reason": "A literal route forwarded through a local helper is not extracted (#3487).",
|
||||
"baselineActual": []
|
||||
},
|
||||
{
|
||||
"id": "explain.guard",
|
||||
"issue": 3490,
|
||||
|
|
|
|||
|
|
@ -7,6 +7,20 @@ import type { FrameworkExpression, FrameworkRouteFacts } from './framework-route
|
|||
type Node = Parser.SyntaxNode;
|
||||
const UNKNOWN: FrameworkExpression = { kind: 'unknown' };
|
||||
const HANDLER: FrameworkExpression = { kind: 'handler' };
|
||||
/** Expressions that can evaluate to a framework allocation passed into a helper. */
|
||||
const CARRIER_KINDS = new Set<FrameworkExpression['kind']>([
|
||||
'binding',
|
||||
'call',
|
||||
'member',
|
||||
'import',
|
||||
'array',
|
||||
]);
|
||||
const FUNCTION_TYPES = [
|
||||
'arrow_function',
|
||||
'function_expression',
|
||||
'function_declaration',
|
||||
'generator_function_declaration',
|
||||
];
|
||||
const METHODS = new Set(['get', 'post', 'put', 'delete', 'patch', 'all', 'route', 'use']);
|
||||
const children = (node: Node | null | undefined): Node[] =>
|
||||
node?.namedChildren.filter((child) => child.type !== 'comment') ?? [];
|
||||
|
|
@ -41,6 +55,7 @@ export function extractFrameworkRouteFacts(
|
|||
invalidBindings: [],
|
||||
mutations: [],
|
||||
calls: [],
|
||||
invocations: [],
|
||||
};
|
||||
const scopes = buildScopeTree(parsedFile.scopes);
|
||||
const positions = buildPositionIndex(parsedFile.scopes);
|
||||
|
|
@ -53,6 +68,11 @@ export function extractFrameworkRouteFacts(
|
|||
importsAtPosition.set(key, imports);
|
||||
}
|
||||
const expressionCache = new Map<number, FrameworkExpression>();
|
||||
const importedNames = new Set(
|
||||
parsedFile.parsedImports.flatMap((imported) =>
|
||||
'localName' in imported ? [imported.localName] : [],
|
||||
),
|
||||
);
|
||||
const bindingKey = (scope: string, name: string): string => `${scope}\0${name}`;
|
||||
const ownership = (node: Node, name = node.text, atUse = false) => {
|
||||
const scope = positions.atPosition(
|
||||
|
|
@ -100,6 +120,74 @@ export function extractFrameworkRouteFacts(
|
|||
return owner?.scope ? bindingKey(owner.scope.id, name) : undefined;
|
||||
};
|
||||
const isGlobal = (node: Node, name: string) => ownership(node, name)?.status === 'absent';
|
||||
/** Whether a scope between `target` and `fn` declares the name `target` writes. */
|
||||
const shadowedBetween = (target: Node, fn: Node): boolean => {
|
||||
const declares = (node: Node | null | undefined) =>
|
||||
!!node &&
|
||||
(node.type === 'identifier'
|
||||
? node.text === target.text
|
||||
: node.descendantsOfType('identifier').some((id) => id.text === target.text));
|
||||
for (let scope = target.parent; scope && scope.id !== fn.id; scope = scope.parent) {
|
||||
const ownsVar = FUNCTION_TYPES.includes(scope.parent?.type ?? '');
|
||||
if (
|
||||
children(scope).some(
|
||||
(statement) =>
|
||||
(statement.type === 'lexical_declaration' ||
|
||||
(ownsVar && statement.type === 'variable_declaration')) &&
|
||||
children(statement).some((d) => declares(d.childForFieldName('name'))),
|
||||
) ||
|
||||
(FUNCTION_TYPES.includes(scope.type) &&
|
||||
(declares(scope.childForFieldName('parameter')) ||
|
||||
declares(scope.childForFieldName('parameters')))) ||
|
||||
(scope.type === 'catch_clause' && declares(scope.childForFieldName('parameter')))
|
||||
)
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
const functionValue = (fn: Node): FrameworkExpression => {
|
||||
const single = fn.childForFieldName('parameter');
|
||||
const params = single ? [single] : children(fn.childForFieldName('parameters'));
|
||||
// A parameter written anywhere in the body, nested closures included, is
|
||||
// not the argument any more; fail closed rather than track which write runs.
|
||||
// A write to a same-named binding a nested scope declares is not to it.
|
||||
const written = new Set(
|
||||
fn
|
||||
.descendantsOfType([
|
||||
'assignment_expression',
|
||||
'augmented_assignment_expression',
|
||||
'update_expression',
|
||||
])
|
||||
.map((write) => write.childForFieldName('left') ?? write.childForFieldName('argument'))
|
||||
// A destructuring target (`[app] = [other]`) writes each name it binds;
|
||||
// a member target (`app.locals = …`) writes a property, not the binding.
|
||||
.flatMap((target) =>
|
||||
!target || ['member_expression', 'subscript_expression'].includes(target.type)
|
||||
? []
|
||||
: target.type === 'identifier'
|
||||
? [target]
|
||||
: target.descendantsOfType(['identifier', 'shorthand_property_identifier_pattern']),
|
||||
)
|
||||
.filter((target) => !shadowedBetween(target, fn))
|
||||
.map((target) => target.text),
|
||||
);
|
||||
return {
|
||||
kind: 'handler',
|
||||
fn: `${filePath}:${fn.startIndex}`,
|
||||
params: params
|
||||
// TypeScript's `this: T` declares the receiver, not an argument position.
|
||||
.filter((param) => param.childForFieldName('pattern')?.type !== 'this')
|
||||
.map((param) => {
|
||||
let name: Node | null = param;
|
||||
if (param.type === 'required_parameter' || param.type === 'optional_parameter')
|
||||
name = param.childForFieldName('pattern');
|
||||
if (name?.type === 'assignment_pattern') name = name.childForFieldName('left');
|
||||
return (
|
||||
(name?.type === 'identifier' && !written.has(name.text) && bindingId(name)) || null
|
||||
);
|
||||
}),
|
||||
};
|
||||
};
|
||||
|
||||
const expression = (node: Node | null | undefined, depth = 0): FrameworkExpression => {
|
||||
if (!node || depth > 64 || node.hasError) return UNKNOWN;
|
||||
|
|
@ -123,14 +211,7 @@ export function extractFrameworkRouteFacts(
|
|||
].includes(node.type)
|
||||
) {
|
||||
value = expression(children(node)[0], depth + 1);
|
||||
} else if (
|
||||
[
|
||||
'arrow_function',
|
||||
'function_expression',
|
||||
'function_declaration',
|
||||
'generator_function_declaration',
|
||||
].includes(node.type)
|
||||
) {
|
||||
} else if (FUNCTION_TYPES.includes(node.type)) {
|
||||
value = HANDLER;
|
||||
} else if (node.type === 'member_expression') {
|
||||
const name = node.childForFieldName('property');
|
||||
|
|
@ -225,13 +306,20 @@ export function extractFrameworkRouteFacts(
|
|||
}
|
||||
} else if (node.type === 'variable_declarator') {
|
||||
const name = node.childForFieldName('name');
|
||||
if (name) bind(name, expression(node.childForFieldName('value')));
|
||||
const value = node.childForFieldName('value');
|
||||
// Only a named function can be called by name, so only it carries the
|
||||
// identity and parameters call-site forwarding needs.
|
||||
if (name)
|
||||
bind(
|
||||
name,
|
||||
value && FUNCTION_TYPES.includes(value.type) ? functionValue(value) : expression(value),
|
||||
);
|
||||
} else if (
|
||||
node.type === 'function_declaration' ||
|
||||
node.type === 'generator_function_declaration'
|
||||
) {
|
||||
const name = node.childForFieldName('name');
|
||||
if (name) bind(name, HANDLER);
|
||||
if (name) bind(name, functionValue(node));
|
||||
} else if (node.type === 'export_statement') {
|
||||
const sourceNode = node.childForFieldName('source');
|
||||
const source = sourceNode && plainString(sourceNode);
|
||||
|
|
@ -249,13 +337,15 @@ export function extractFrameworkRouteFacts(
|
|||
);
|
||||
}
|
||||
} else if (node.children.some((child) => child.type === 'default')) {
|
||||
const declaration =
|
||||
node.childForFieldName('value') ??
|
||||
node.childForFieldName('declaration') ??
|
||||
children(node)[0];
|
||||
exported(
|
||||
'default',
|
||||
expression(
|
||||
node.childForFieldName('value') ??
|
||||
node.childForFieldName('declaration') ??
|
||||
children(node)[0],
|
||||
),
|
||||
declaration && FUNCTION_TYPES.includes(declaration.type)
|
||||
? functionValue(declaration)
|
||||
: expression(declaration),
|
||||
);
|
||||
} else if (source) {
|
||||
const namespace = children(node).find((child) => child.type === 'namespace_export');
|
||||
|
|
@ -382,6 +472,26 @@ export function extractFrameworkRouteFacts(
|
|||
lineNumber: node.startPosition.row + lineOffset,
|
||||
...(handler?.type === 'identifier' ? { handlerName: handler.text } : {}),
|
||||
});
|
||||
} else if (
|
||||
callee?.type === 'identifier' ||
|
||||
(callee?.type === 'member_expression' &&
|
||||
importedNames.has(callee.childForFieldName('object')?.text ?? ''))
|
||||
) {
|
||||
// A helper such as `mountRoutes(app, '/p')` may register on its
|
||||
// parameters. Only a plain call or a namespace/default import member can
|
||||
// reach a named function; arguments after a spread have no position.
|
||||
const arguments_ = args(node);
|
||||
const spread = arguments_.findIndex((argument) => argument.type === 'spread_element');
|
||||
const positional = spread < 0 ? arguments_ : arguments_.slice(0, spread);
|
||||
// Keep only arguments that can carry an allocation or a literal path;
|
||||
// a call passing neither cannot forward anything a resolver could use.
|
||||
const carried = positional.map((argument) => {
|
||||
const value = expression(argument);
|
||||
return CARRIER_KINDS.has(value.kind) ? value : UNKNOWN;
|
||||
});
|
||||
const paths = positional.map((argument) => literalPaths(argument));
|
||||
if (carried.some((value) => value !== UNKNOWN) || paths.some((path) => path !== null))
|
||||
facts.invocations.push({ callee: expression(callee), arguments: carried, paths });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -392,5 +502,41 @@ export function extractFrameworkRouteFacts(
|
|||
if (cjsDefaultAssignments <= 1) facts.exports.push(...cjsExports);
|
||||
}
|
||||
facts.invalidBindings = [...invalid];
|
||||
// A callee bound in this file to anything but a named function, an import or
|
||||
// an alias (a callback parameter such as `next(err)`, a local object) can
|
||||
// never reach a function registering on its parameters.
|
||||
// An argument bound to a parameter or a function cannot be an allocation (a
|
||||
// site argument that is itself a parameter never resolves), so it is dropped,
|
||||
// and so is a call left with nothing to forward.
|
||||
const valueKinds = new Map<string, Set<FrameworkExpression['kind']>>();
|
||||
for (const { id, value } of facts.bindings)
|
||||
valueKinds.set(id, (valueKinds.get(id) ?? new Set()).add(value.kind));
|
||||
const callable = (id: string) =>
|
||||
['handler', 'import', 'binding', 'member'].some((kind) =>
|
||||
valueKinds.get(id)?.has(kind as FrameworkExpression['kind']),
|
||||
);
|
||||
const allocatable = (value: FrameworkExpression): boolean => {
|
||||
switch (value.kind) {
|
||||
case 'binding':
|
||||
return [...(valueKinds.get(value.id) ?? [])].some(
|
||||
(kind) => kind !== 'handler' && kind !== 'unknown',
|
||||
);
|
||||
case 'member':
|
||||
return allocatable(value.object);
|
||||
case 'call':
|
||||
return allocatable(value.callee);
|
||||
case 'array':
|
||||
return value.elements.some(allocatable);
|
||||
default:
|
||||
return value.kind === 'import';
|
||||
}
|
||||
};
|
||||
facts.invocations = facts.invocations.flatMap((invocation) => {
|
||||
if (invocation.callee.kind === 'binding' && !callable(invocation.callee.id)) return [];
|
||||
const carried = invocation.arguments.map((value) => (allocatable(value) ? value : UNKNOWN));
|
||||
return carried.some((value) => value !== UNKNOWN) || invocation.paths.some(Boolean)
|
||||
? [{ ...invocation, arguments: carried }]
|
||||
: [];
|
||||
});
|
||||
return facts;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ type Value =
|
|||
| { kind: 'instance'; allocation: FrameworkAllocation }
|
||||
| { kind: 'builder'; receiverId: string; paths: string[] }
|
||||
| { kind: 'array'; elements: (Value | undefined)[] }
|
||||
| { kind: 'handler' };
|
||||
| { kind: 'handler'; fn?: string };
|
||||
type ExportResult = { found: boolean; value?: Value };
|
||||
const keyOf = (value: Value): string => {
|
||||
switch (value.kind) {
|
||||
|
|
@ -32,7 +32,7 @@ const keyOf = (value: Value): string => {
|
|||
case 'array':
|
||||
return `array:${value.elements.map((element) => element && keyOf(element)).join(',')}`;
|
||||
case 'handler':
|
||||
return 'handler';
|
||||
return `handler:${value.fn ?? ''}`;
|
||||
}
|
||||
};
|
||||
|
||||
|
|
@ -175,7 +175,7 @@ export function resolveFrameworkRouteFacts(
|
|||
case 'unknown':
|
||||
return undefined;
|
||||
case 'handler':
|
||||
return { kind: 'handler' };
|
||||
return { kind: 'handler', ...(expression.fn ? { fn: expression.fn } : {}) };
|
||||
case 'array':
|
||||
return {
|
||||
kind: 'array',
|
||||
|
|
@ -301,84 +301,195 @@ export function resolveFrameworkRouteFacts(
|
|||
}
|
||||
cache.clear();
|
||||
allocations.clear();
|
||||
|
||||
// One call-site hop (#3487): a registration whose receiver, path or mounted
|
||||
// child is a parameter of a named function takes that argument from each
|
||||
// call of the function. A site argument that is itself a parameter does not
|
||||
// resolve, so forwarding never chains, and a reassigned parameter is null.
|
||||
const parameterOf = new Map<string, { fn: string; index: number }>();
|
||||
/** The names each function is bound or exported under, for the call-site filter below. */
|
||||
const fnNames = new Map<string, Set<string>>();
|
||||
for (const file of files)
|
||||
for (const { name, value } of [
|
||||
...file.bindings.map(({ id, value }) => ({
|
||||
name: id.slice(id.lastIndexOf('\0') + 1),
|
||||
value,
|
||||
})),
|
||||
...file.exports,
|
||||
])
|
||||
if (value.kind === 'handler' && value.fn) {
|
||||
value.params?.forEach((param, index) => {
|
||||
if (param && !invalidBindings.has(param))
|
||||
parameterOf.set(param, { fn: value.fn!, index });
|
||||
});
|
||||
fnNames.set(value.fn, (fnNames.get(value.fn) ?? new Set()).add(name));
|
||||
}
|
||||
// `export { mountRoutes as setup }` exports the binding: callers know `setup`.
|
||||
for (const file of files)
|
||||
for (const { name, value } of file.exports) {
|
||||
const bound = value.kind === 'binding' ? bindings.get(value.id)?.values : undefined;
|
||||
const fn = bound?.length === 1 && bound[0].kind === 'handler' ? bound[0].fn : undefined;
|
||||
if (fn) fnNames.set(fn, (fnNames.get(fn) ?? new Set()).add(name));
|
||||
}
|
||||
const parameter = (expression: FrameworkExpression | undefined) =>
|
||||
expression?.kind === 'binding' ? parameterOf.get(expression.id) : undefined;
|
||||
// Only calls that can reach a function registering on its parameters are
|
||||
// evaluated: resolving every helper call in a large repo is the cost cliff.
|
||||
const forwardingNames = new Set<string>();
|
||||
for (const file of files)
|
||||
for (const call of file.calls)
|
||||
for (const expression of [call.receiver, ...call.arguments]) {
|
||||
const fn = parameter(expression)?.fn;
|
||||
for (const name of (fn && fnNames.get(fn)) ?? []) forwardingNames.add(name);
|
||||
}
|
||||
/** Local and imported names a callee may be known by (`import { a as b }` gives both). */
|
||||
const calleeNames = (callee: FrameworkExpression): string[] =>
|
||||
callee.kind === 'binding'
|
||||
? [
|
||||
callee.id.slice(callee.id.lastIndexOf('\0') + 1),
|
||||
...(bindings.get(callee.id)?.values ?? []).flatMap((value) =>
|
||||
value.kind === 'import' ? [value.name] : [],
|
||||
),
|
||||
]
|
||||
: callee.kind === 'import' || callee.kind === 'member'
|
||||
? [callee.name]
|
||||
: [];
|
||||
const sitesOf = new Map<
|
||||
string,
|
||||
{ filePath: string; arguments: FrameworkExpression[]; paths: (string[] | null)[] }[]
|
||||
>();
|
||||
for (const file of files)
|
||||
for (const invocation of forwardingNames.size ? file.invocations : []) {
|
||||
if (!calleeNames(invocation.callee).some((name) => forwardingNames.has(name))) continue;
|
||||
const callee = evaluate(invocation.callee, file.filePath);
|
||||
if (callee?.kind !== 'handler' || !callee.fn) continue;
|
||||
const sites = sitesOf.get(callee.fn) ?? [];
|
||||
sites.push({ filePath: file.filePath, ...invocation });
|
||||
sitesOf.set(callee.fn, sites);
|
||||
}
|
||||
interface Target {
|
||||
receiver: Value | undefined;
|
||||
paths: string[] | null;
|
||||
/** Evaluates an argument of the call, through the site for a forwarded parameter. */
|
||||
argument: (expression: FrameworkExpression) => Value | undefined;
|
||||
}
|
||||
/** Receiver, paths and arguments per call site, or undefined when nothing is forwarded. */
|
||||
const forwardedTargets = (call: FrameworkCallFact, filePath: string): Target[] | undefined => {
|
||||
const pathParam = call.paths ? undefined : parameter(call.arguments[0]);
|
||||
// The receiver, the path or any argument (a mounted child) may be forwarded;
|
||||
// all of them must come from the same function.
|
||||
const fns = new Set(
|
||||
[call.receiver, ...call.arguments].flatMap((expression) => parameter(expression)?.fn ?? []),
|
||||
);
|
||||
const [fn] = fns;
|
||||
const sites = fn ? sitesOf.get(fn) : undefined;
|
||||
// No call site proves an argument: evaluate the call as written, which keeps
|
||||
// an unknown mount path suppressing its child router (no invented root).
|
||||
if (!fn || !sites || fns.size > 1) return undefined;
|
||||
return sites.map((site) => {
|
||||
const fromSite = (expression: FrameworkExpression | undefined) => {
|
||||
const forwarded = parameter(expression);
|
||||
return forwarded?.fn === fn
|
||||
? evaluate(site.arguments[forwarded.index] ?? { kind: 'unknown' }, site.filePath)
|
||||
: expression && evaluate(expression, filePath);
|
||||
};
|
||||
return {
|
||||
receiver: fromSite(call.receiver),
|
||||
paths: call.paths ?? (pathParam ? (site.paths[pathParam.index] ?? null) : null),
|
||||
argument: fromSite,
|
||||
};
|
||||
});
|
||||
};
|
||||
|
||||
const result: ResolvedFrameworkRoutes = { allocations: [], registrations: [], mounts: [] };
|
||||
for (const file of files) {
|
||||
for (const binding of file.bindings)
|
||||
evaluate({ kind: 'binding', id: binding.id }, file.filePath);
|
||||
for (const call of file.calls) {
|
||||
const receiver = evaluate(call.receiver, file.filePath);
|
||||
if (!receiver) {
|
||||
result.mounts.push(...(uncertainMounts.get(call) ?? []));
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
call.method === 'use' &&
|
||||
receiver.kind === 'instance' &&
|
||||
receiver.allocation.kind !== 'hono'
|
||||
) {
|
||||
const values = call.arguments.flatMap((argument) =>
|
||||
flatten(evaluate(argument, file.filePath)),
|
||||
);
|
||||
// An invalidated argument can hide a known child even when its parent
|
||||
// still resolves. Keep that incoming edge without asserting a path.
|
||||
if (values.some((value) => value === undefined))
|
||||
result.mounts.push(...(uncertainMounts.get(call) ?? []));
|
||||
const first = values[0];
|
||||
const prefixes =
|
||||
call.paths ??
|
||||
(first?.kind === 'handler' ||
|
||||
(first?.kind === 'instance' && first.allocation.kind !== 'hono')
|
||||
? ['']
|
||||
: null);
|
||||
for (const child of values)
|
||||
if (child?.kind === 'instance' && child.allocation.kind !== 'hono') {
|
||||
result.mounts.push({
|
||||
parentId: receiver.allocation.id,
|
||||
childId: child.allocation.id,
|
||||
prefixes,
|
||||
});
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (!VERBS.has(call.method)) continue;
|
||||
const builder = receiver.kind === 'builder';
|
||||
if (receiver.kind !== 'instance' && !builder) continue;
|
||||
if (call.arguments.length < (builder ? 1 : 2)) continue;
|
||||
const paths = builder ? receiver.paths : call.paths;
|
||||
if (!paths) continue;
|
||||
const framework =
|
||||
receiver.kind === 'instance' && receiver.allocation.kind === 'hono' ? 'hono' : 'express';
|
||||
// The shared symbol model resolves a bare name only within this file.
|
||||
// Retain that evidence only for the exact lexical callable declaration;
|
||||
// parameters, imported aliases and unknown values must not borrow a
|
||||
// same-named function elsewhere in the file.
|
||||
const handler = call.arguments.at(-1);
|
||||
const handlerBinding = handler?.kind === 'binding' ? bindings.get(handler.id) : undefined;
|
||||
const handlerName =
|
||||
call.handlerName &&
|
||||
handlerBinding?.filePath === file.filePath &&
|
||||
handlerBinding.values.length === 1 &&
|
||||
handlerBinding.values[0].kind === 'handler' &&
|
||||
handler?.kind === 'binding' &&
|
||||
evaluate(handler, file.filePath)?.kind === 'handler'
|
||||
? call.handlerName
|
||||
: undefined;
|
||||
for (const routePath of paths)
|
||||
result.registrations.push({
|
||||
receiverId: builder ? receiver.receiverId : receiver.allocation.id,
|
||||
framework,
|
||||
route: {
|
||||
frameworkCall: call,
|
||||
filePath: file.filePath,
|
||||
routePath,
|
||||
httpMethod: call.method === 'all' ? '*' : call.method.toUpperCase(),
|
||||
decoratorName: `${framework}.${call.method}`,
|
||||
lineNumber: call.lineNumber,
|
||||
...(handlerName ? { handlerName } : {}),
|
||||
},
|
||||
});
|
||||
}
|
||||
for (const call of file.calls)
|
||||
for (const target of forwardedTargets(call, file.filePath) ?? [
|
||||
{
|
||||
receiver: evaluate(call.receiver, file.filePath),
|
||||
paths: call.paths,
|
||||
argument: (expression: FrameworkExpression) => evaluate(expression, file.filePath),
|
||||
},
|
||||
])
|
||||
register(file.filePath, call, target);
|
||||
}
|
||||
result.allocations = [...allocations.values()];
|
||||
return result;
|
||||
|
||||
function register(
|
||||
filePath: string,
|
||||
call: FrameworkCallFact,
|
||||
{ receiver, paths: callPaths, argument }: Target,
|
||||
): void {
|
||||
if (!receiver) {
|
||||
result.mounts.push(...(uncertainMounts.get(call) ?? []));
|
||||
return;
|
||||
}
|
||||
if (
|
||||
call.method === 'use' &&
|
||||
receiver.kind === 'instance' &&
|
||||
receiver.allocation.kind !== 'hono'
|
||||
) {
|
||||
const values = call.arguments.flatMap((expression) => flatten(argument(expression)));
|
||||
// An invalidated argument can hide a known child even when its parent
|
||||
// still resolves. Keep that incoming edge without asserting a path.
|
||||
if (values.some((value) => value === undefined))
|
||||
result.mounts.push(...(uncertainMounts.get(call) ?? []));
|
||||
const first = values[0];
|
||||
const prefixes =
|
||||
callPaths ??
|
||||
(first?.kind === 'handler' ||
|
||||
(first?.kind === 'instance' && first.allocation.kind !== 'hono')
|
||||
? ['']
|
||||
: null);
|
||||
for (const child of values)
|
||||
if (child?.kind === 'instance' && child.allocation.kind !== 'hono') {
|
||||
result.mounts.push({
|
||||
parentId: receiver.allocation.id,
|
||||
childId: child.allocation.id,
|
||||
prefixes,
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (!VERBS.has(call.method)) return;
|
||||
const builder = receiver.kind === 'builder';
|
||||
if (receiver.kind !== 'instance' && !builder) return;
|
||||
if (call.arguments.length < (builder ? 1 : 2)) return;
|
||||
const paths = builder ? receiver.paths : callPaths;
|
||||
if (!paths) return;
|
||||
const framework =
|
||||
receiver.kind === 'instance' && receiver.allocation.kind === 'hono' ? 'hono' : 'express';
|
||||
// The shared symbol model resolves a bare name only within this file.
|
||||
// Retain that evidence only for the exact lexical callable declaration;
|
||||
// parameters, imported aliases and unknown values must not borrow a
|
||||
// same-named function elsewhere in the file.
|
||||
const handler = call.arguments.at(-1);
|
||||
const handlerBinding = handler?.kind === 'binding' ? bindings.get(handler.id) : undefined;
|
||||
const handlerName =
|
||||
call.handlerName &&
|
||||
handlerBinding?.filePath === filePath &&
|
||||
handlerBinding.values.length === 1 &&
|
||||
handlerBinding.values[0].kind === 'handler' &&
|
||||
handler?.kind === 'binding' &&
|
||||
evaluate(handler, filePath)?.kind === 'handler'
|
||||
? call.handlerName
|
||||
: undefined;
|
||||
for (const routePath of paths)
|
||||
result.registrations.push({
|
||||
receiverId: builder ? receiver.receiverId : receiver.allocation.id,
|
||||
framework,
|
||||
route: {
|
||||
frameworkCall: call,
|
||||
filePath,
|
||||
routePath,
|
||||
httpMethod: call.method === 'all' ? '*' : call.method.toUpperCase(),
|
||||
decoratorName: `${framework}.${call.method}`,
|
||||
lineNumber: call.lineNumber,
|
||||
...(handlerName ? { handlerName } : {}),
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,7 +3,12 @@ import type { ExtractedDecoratorRoute } from '../workers/parse-worker.js';
|
|||
/** Provider-private facts. Only JSON values cross the worker/cache boundary. */
|
||||
export type FrameworkExpression =
|
||||
| { kind: 'unknown' }
|
||||
| { kind: 'handler' }
|
||||
/**
|
||||
* A function value. `fn` identifies it and `params` holds each parameter's
|
||||
* binding id (null for a destructured or rest parameter), so a registration
|
||||
* on a parameter can be resolved from the function's call sites.
|
||||
*/
|
||||
| { kind: 'handler'; fn?: string; params?: (string | null)[] }
|
||||
| { kind: 'array'; elements: FrameworkExpression[] }
|
||||
| { kind: 'binding'; id: string }
|
||||
| { kind: 'import'; source: string; name: string }
|
||||
|
|
@ -26,6 +31,14 @@ export interface FrameworkCallFact {
|
|||
handlerName?: string;
|
||||
}
|
||||
|
||||
/** A call to a plain or imported function: the sites a parameter is forwarded from. */
|
||||
export interface FrameworkInvocationFact {
|
||||
callee: FrameworkExpression;
|
||||
arguments: FrameworkExpression[];
|
||||
/** Literal paths of each argument, for a path forwarded as a parameter. */
|
||||
paths: (string[] | null)[];
|
||||
}
|
||||
|
||||
export interface FrameworkRouteFacts {
|
||||
filePath: string;
|
||||
bindings: { id: string; value: FrameworkExpression }[];
|
||||
|
|
@ -35,6 +48,7 @@ export interface FrameworkRouteFacts {
|
|||
/** Property writes invalidate the value behind aliases, including imports. */
|
||||
mutations: FrameworkExpression[];
|
||||
calls: FrameworkCallFact[];
|
||||
invocations: FrameworkInvocationFact[];
|
||||
}
|
||||
|
||||
export interface FrameworkAllocation {
|
||||
|
|
|
|||
18
gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/api.ts
vendored
Normal file
18
gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/api.ts
vendored
Normal file
|
|
@ -0,0 +1,18 @@
|
|||
import express from 'express';
|
||||
import { mountMCPEndpoints } from './mcp-http';
|
||||
import { mountSSEProgress } from './sse-progress';
|
||||
|
||||
export const registerWebUI = (app: express.Express, staticDir: string | null): void => {
|
||||
if (!staticDir) return;
|
||||
app.get('/', (_req, res) => res.sendFile('index.html', { root: staticDir }));
|
||||
};
|
||||
|
||||
export async function createServer(jobs: unknown) {
|
||||
const app = express();
|
||||
app.get('/api/info', (_req, res) => res.json({ ok: true }));
|
||||
const cleanupMcp = await mountMCPEndpoints(app, {});
|
||||
mountSSEProgress(app, '/api/analyze/:jobId/progress', jobs);
|
||||
mountSSEProgress(app, '/api/embed/:jobId/progress', jobs);
|
||||
registerWebUI(app, 'web');
|
||||
return { app, cleanupMcp };
|
||||
}
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
import type { Express } from 'express';
|
||||
|
||||
export async function mountMCPEndpoints(app: Express, backend: unknown): Promise<() => void> {
|
||||
app.all('/api/mcp', (_req, res) => res.end(String(backend)));
|
||||
return () => {};
|
||||
}
|
||||
|
|
@ -0,0 +1,7 @@
|
|||
import type express from 'express';
|
||||
|
||||
export const mountSSEProgress = (app: express.Express, routePath: string, jm: unknown) => {
|
||||
app.get(routePath, (_req, res) => {
|
||||
res.write(`data: ${JSON.stringify(jm)}\n\n`);
|
||||
});
|
||||
};
|
||||
6
gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/unused.ts
vendored
Normal file
6
gitnexus/test/fixtures/lang-resolution/express-forwarded-registrations/src/server/unused.ts
vendored
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
import type { Express } from 'express';
|
||||
|
||||
// Never called: a parameter receiver proves nothing without a call site.
|
||||
export function registerUnused(app: Express) {
|
||||
app.get('/api/never-mounted', (_req, res) => res.end());
|
||||
}
|
||||
|
|
@ -26,6 +26,34 @@ import {
|
|||
|
||||
const parseIdentity = await runtimeParseIdentity();
|
||||
|
||||
// GitNexus's own server registers through helpers that receive the app and,
|
||||
// for SSE progress, the path (#3487); the app comes from `express()` in the
|
||||
// caller, so each registration resolves through exactly one call site.
|
||||
describe('Express registrations forwarded through helper parameters', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(
|
||||
path.join(FIXTURES, 'express-forwarded-registrations'),
|
||||
() => {},
|
||||
);
|
||||
}, 60000);
|
||||
|
||||
it('emits each helper registration once per call site, in the helper file', () => {
|
||||
expect(
|
||||
getNodesByLabelFull(result, 'Route')
|
||||
.map((node) => `${node.properties.method} ${node.name} ${node.properties.filePath}`)
|
||||
.sort(),
|
||||
).toEqual([
|
||||
'* /api/mcp src/server/mcp-http.ts',
|
||||
'GET / src/server/api.ts',
|
||||
'GET /api/analyze/:jobId/progress src/server/sse-progress.ts',
|
||||
'GET /api/embed/:jobId/progress src/server/sse-progress.ts',
|
||||
'GET /api/info src/server/api.ts',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Express/Hono route detection', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
|
|
|
|||
|
|
@ -155,6 +155,176 @@ function handler() {} function other() {}`);
|
|||
expect(urls(`${setup} r.get('/unsafe', handler);`)).toEqual([]);
|
||||
});
|
||||
|
||||
// GitNexus's own server registers through helpers that receive the app and,
|
||||
// for SSE progress, the path (#3487): one call-site hop, never two.
|
||||
it('forwards a proven receiver and literal path through one helper call', () => {
|
||||
const result = resolve({
|
||||
'sse.ts': `
|
||||
import type express from 'express';
|
||||
export const mountSSEProgress = (app: express.Express, routePath: string, jm: unknown) => {
|
||||
app.get(routePath, (req, res) => {});
|
||||
};`,
|
||||
'mcp.ts': `
|
||||
export async function mountMCPEndpoints(app, backend) {
|
||||
app.all('/api/mcp', handler);
|
||||
app.use('/api/mcp', auth);
|
||||
return () => {};
|
||||
}`,
|
||||
'server.ts': `
|
||||
import express from 'express';
|
||||
import { mountSSEProgress } from './sse';
|
||||
import { mountMCPEndpoints } from './mcp';
|
||||
export const registerWebUI = (app: express.Express, dir: string | null): void => {
|
||||
app.get('/', handler);
|
||||
};
|
||||
const app = express();
|
||||
const cleanup = await mountMCPEndpoints(app, backend);
|
||||
mountSSEProgress(app, '/api/analyze/:jobId/progress', jobs);
|
||||
mountSSEProgress(app, '/api/embed/:jobId/progress', jobs);
|
||||
registerWebUI(app, null);`,
|
||||
});
|
||||
expect(
|
||||
result.registrations
|
||||
.map(({ route }) => `${route.httpMethod} ${route.routePath} ${route.filePath}`)
|
||||
.sort(),
|
||||
).toEqual([
|
||||
'* /api/mcp mcp.ts',
|
||||
'GET / server.ts',
|
||||
'GET /api/analyze/:jobId/progress sse.ts',
|
||||
'GET /api/embed/:jobId/progress sse.ts',
|
||||
]);
|
||||
});
|
||||
|
||||
it('follows a helper exported under another name', () => {
|
||||
const result = resolve({
|
||||
'routes.ts': `function mountRoutes(app) { app.get('/api/renamed', handler); }
|
||||
export { mountRoutes as setup };`,
|
||||
'server.ts': `import express from 'express';
|
||||
import { setup } from './routes';
|
||||
setup(express());`,
|
||||
});
|
||||
expect(
|
||||
result.registrations.map(
|
||||
({ route }) => `${route.httpMethod} ${route.routePath} ${route.filePath}`,
|
||||
),
|
||||
).toEqual(['GET /api/renamed routes.ts']);
|
||||
});
|
||||
|
||||
it('follows a helper called through an aliased import', () => {
|
||||
const result = resolve({
|
||||
'sse.ts': `export const mountSSEProgress = (app, routePath) => { app.get(routePath, handler); };`,
|
||||
'server.ts': `import express from 'express';
|
||||
import { mountSSEProgress as mountProgress } from './sse';
|
||||
const app = express();
|
||||
mountProgress(app, '/api/progress');`,
|
||||
});
|
||||
expect(
|
||||
result.registrations.map(
|
||||
({ route }) => `${route.httpMethod} ${route.routePath} ${route.filePath}`,
|
||||
),
|
||||
).toEqual(['GET /api/progress sse.ts']);
|
||||
});
|
||||
|
||||
it('declines forwarding through two hops, unproven arguments, and reassigned parameters', () => {
|
||||
expect(
|
||||
urls(`
|
||||
import express from 'express';
|
||||
function inner(app) { app.get('/two-hop', handler); }
|
||||
function outer(app) { inner(app); }
|
||||
outer(express());
|
||||
function unproven(app) { app.get('/unproven', handler); }
|
||||
unproven(storage);
|
||||
function reassigned(app) { app = storage; app.get('/reassigned', handler); }
|
||||
reassigned(express());
|
||||
function runtime(app, path) { app.get(path, handler); }
|
||||
runtime(express(), runtimePath);
|
||||
function direct(app, path) { app.post(path, handler); }
|
||||
direct(express(), '/direct');
|
||||
`),
|
||||
).toEqual(['POST /direct']);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{
|
||||
shape: 'a literal mount path forwarded to a helper',
|
||||
calls: `function mountApi(prefix) { app.use(prefix, router); }
|
||||
mountApi('/api');`,
|
||||
routes: ['GET /api/users'],
|
||||
},
|
||||
{
|
||||
shape: 'a runtime mount path forwarded to a helper',
|
||||
calls: `function mountApi(prefix) { app.use(prefix, router); }
|
||||
mountApi(process.env.API_PREFIX);`,
|
||||
routes: [],
|
||||
},
|
||||
{
|
||||
shape: 'a helper whose mount path no call proves',
|
||||
calls: 'function mountApi(prefix) { app.use(prefix, router); }',
|
||||
routes: [],
|
||||
},
|
||||
{
|
||||
shape: 'a router forwarded as the mounted child',
|
||||
calls: `function mountApi(target, child) { target.use('/api', child); }
|
||||
mountApi(app, router);`,
|
||||
routes: ['GET /api/users'],
|
||||
},
|
||||
{
|
||||
shape: 'a TypeScript this parameter',
|
||||
calls: `function wire(this: void, target, prefix) { target.use(prefix, router); }
|
||||
wire(app, '/v1');`,
|
||||
routes: ['GET /v1/users'],
|
||||
},
|
||||
{
|
||||
shape: 'an argument after a spread',
|
||||
calls: `function wire(target, prefix) { target.use(prefix, router); }
|
||||
wire(...targets, '/spread');`,
|
||||
// Unproven receiver: the router keeps its standalone paths, as without
|
||||
// any helper, rather than borrowing '/spread' from the wrong position.
|
||||
routes: ['GET /users'],
|
||||
},
|
||||
{
|
||||
shape: 'a parameter reassigned in a nested closure',
|
||||
calls: `function wire(target) { const swap = () => { target = other; }; swap(); target.use('/api', router); }
|
||||
wire(app);`,
|
||||
routes: ['GET /users'],
|
||||
},
|
||||
{
|
||||
shape: 'a helper forwarding only its mounted child',
|
||||
calls: `function mountChild(child) { app.use('/api', child); }
|
||||
mountChild(router);`,
|
||||
routes: ['GET /api/users'],
|
||||
},
|
||||
{
|
||||
shape: 'a parameter reassigned by destructuring',
|
||||
calls: `function wire(target) { [target] = [other]; target.use('/api', router); }
|
||||
wire(app);`,
|
||||
routes: ['GET /users'],
|
||||
},
|
||||
{
|
||||
shape: 'a write to a same-named local in a nested block',
|
||||
calls: `function wire(target) { { let target; target = other; } target.use('/api', router); }
|
||||
wire(app);`,
|
||||
routes: ['GET /api/users'],
|
||||
},
|
||||
{
|
||||
shape: 'a property write on another function',
|
||||
calls: `function Button() {} Button.displayName = 'Button';
|
||||
function mountApi(prefix) { app.use(prefix, router); }
|
||||
mountApi('/api');`,
|
||||
routes: ['GET /api/users'],
|
||||
},
|
||||
])('composes mounts through helper parameters: $shape', ({ calls, routes }) => {
|
||||
const result = resolve({
|
||||
'server.ts': `import express from 'express';
|
||||
const app = express(); const router = express.Router();
|
||||
router.get('/users', handler);
|
||||
${calls}`,
|
||||
});
|
||||
expect(
|
||||
composeFrameworkRoutePaths(result).map((route) => `${route.httpMethod} ${route.routePath}`),
|
||||
).toEqual(routes);
|
||||
});
|
||||
|
||||
it('does not lend later allocations to calls in a temporal dead zone', () => {
|
||||
expect(
|
||||
urls(`
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue