mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-15 23:32:49 +00:00
fix(routes): address round-2 review — provider hooks, FQN fold, interface nesting
F1 (High): production harvest silently dropped routes when the constants class is not named *Constants (e.g. ApiPaths). The content gate is now SYNTAX-driven (static-final String field or any class import) and lives in the provider (moduleConstantHeuristic), not a shared-layer regex. F2: shared ingestion layers no longer branch on language. The harvest and the qualified-ref fold run through new provider hooks (extractModuleConstants / foldRoutePathOperands); parse-impl resolves the provider by filePath (getProviderForFile). Python wires the same hooks for architecture parity. F3: multi-segment FQN chains (com.example.ApiPaths.USERS) now flatten recursively; verified via tree-sitter that the existing query already captures the whole nested field_access — the gap was resolver-side only. F4: implicit-final interface semantics no longer leak into nested classes at type boundaries (JLS 9.5). F5: nested same-name shadowing now drops the stale entry (rebind-drop, matching Python #2391 semantics) instead of keeping the first binding. Tests: 9 new unit tests (27/27) + real-pipeline e2e over a reviewer-shaped fixture (non-*Constants class, cold run + warm parse-cache replay) — the exact production gap unit tests missed.
This commit is contained in:
ChunxueLi
parent
fad5f88002
commit
d2f768245e
9 changed files with 500 additions and 81 deletions
30
gitnexus/package-lock.json
generated
30
gitnexus/package-lock.json
generated
|
|
@ -1660,9 +1660,6 @@
|
|||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -1680,9 +1677,6 @@
|
|||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -1700,9 +1694,6 @@
|
|||
"ppc64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -1720,9 +1711,6 @@
|
|||
"s390x"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -1740,9 +1728,6 @@
|
|||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -1760,9 +1745,6 @@
|
|||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MIT",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -3893,9 +3875,6 @@
|
|||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -3917,9 +3896,6 @@
|
|||
"arm64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -3941,9 +3917,6 @@
|
|||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"glibc"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
@ -3965,9 +3938,6 @@
|
|||
"x64"
|
||||
],
|
||||
"dev": true,
|
||||
"libc": [
|
||||
"musl"
|
||||
],
|
||||
"license": "MPL-2.0",
|
||||
"optional": true,
|
||||
"os": [
|
||||
|
|
|
|||
|
|
@ -39,6 +39,11 @@ import type { CfgVisitor } from './cfg/types.js';
|
|||
import type { NodeLabel } from 'gitnexus-shared';
|
||||
import type { ExtractedRoute } from './route-extractors/laravel.js';
|
||||
import type { SharedSpringType } from './route-extractors/spring-shared.js';
|
||||
import type {
|
||||
ModuleConstants,
|
||||
Operand,
|
||||
RepoConstants,
|
||||
} from './route-extractors/constant-resolver.js';
|
||||
import type Parser from 'tree-sitter';
|
||||
import type { ExtractedDecoratorRoute } from './workers/parse-worker.js';
|
||||
|
||||
|
|
@ -336,6 +341,52 @@ interface LanguageProviderConfig {
|
|||
filePath: string,
|
||||
) => SharedSpringType[];
|
||||
|
||||
/**
|
||||
* Harvest this file's module-level string constants (#2391 core, #2980 Java
|
||||
* parity) into the language-agnostic {@link ModuleConstants} shape, so the
|
||||
* parse phase can resolve non-literal decorator route paths cross-file.
|
||||
*
|
||||
* The worker calls this when BOTH hold:
|
||||
* - the file is cheap to harvest (provider-declared `moduleConstantHeuristic`
|
||||
* matched — syntax-driven, e.g. a `static final String` field or a
|
||||
* constants-bearing import; NEVER a class-name pattern like `*Constants`,
|
||||
* which silently drops route constants living in classes named e.g.
|
||||
* `ApiPaths`/`Routes`), and
|
||||
* - the extraction yields something resolvable (a literal, an expression, or
|
||||
* an import binding), keeping the aggregate bounded on large repos.
|
||||
*
|
||||
* Default: undefined (no constant harvest; non-literal route paths of this
|
||||
* language floor to skip).
|
||||
*/
|
||||
readonly extractModuleConstants?: (tree: Parser.Tree) => ModuleConstants;
|
||||
|
||||
/**
|
||||
* Cheap content heuristic deciding whether the worker should run
|
||||
* {@link extractModuleConstants} on a file. Guards the harvest cost on huge
|
||||
* repos: files that cannot contribute (no constant-bearing syntax) are not
|
||||
* walked. Must be syntax-driven (field/import shape), not identifier
|
||||
* pattern-matching on class names.
|
||||
*/
|
||||
readonly moduleConstantHeuristic?: (content: string) => boolean;
|
||||
|
||||
/**
|
||||
* Fold one file's non-literal route-path operand list
|
||||
* (`routePathExpr`/`routePathOperands` of an `ExtractedDecoratorRoute`)
|
||||
* against the repo-wide, file-path-keyed constant map, or null when it cannot
|
||||
* be fully folded (skip floor — never a phantom path). Languages whose
|
||||
* qualified refs resolve through class imports (`Outer.CONST`,
|
||||
* `com.example.ApiPaths.USERS`) need this hook because the shared fold has no
|
||||
* notion of qualified names; Python's bare-name refs use the shared default.
|
||||
*
|
||||
* Default: undefined (the parse phase falls back to the shared
|
||||
* language-agnostic operand fold).
|
||||
*/
|
||||
readonly foldRoutePathOperands?: (
|
||||
filePath: string,
|
||||
operands: readonly Operand[],
|
||||
repo: RepoConstants,
|
||||
) => string | null;
|
||||
|
||||
// ── Noise filtering ────────────────────────────────────────────────
|
||||
/** Built-in/stdlib names that should be filtered from the call graph for this language.
|
||||
* Default: undefined (no language-specific filtering). */
|
||||
|
|
|
|||
|
|
@ -15,6 +15,10 @@ import type { AstFrameworkPatternConfig } from '../language-provider.js';
|
|||
import { createLeadingDocDescriptionExtractor } from '../utils/ast-helpers.js';
|
||||
import { javaTypeConfig } from '../type-extractors/jvm.js';
|
||||
import { extractSpringRoutes, extractSpringTypes } from '../route-extractors/spring.js';
|
||||
import {
|
||||
extractJavaModuleConstants,
|
||||
foldJavaOperands,
|
||||
} from '../route-extractors/java-const-resolver.js';
|
||||
import { javaExportChecker } from '../export-detection.js';
|
||||
import { createImportResolver } from '../import-resolvers/resolver-factory.js';
|
||||
import { javaImportConfig } from '../import-resolvers/configs/jvm.js';
|
||||
|
|
@ -216,4 +220,19 @@ export const javaProvider = defineLanguage({
|
|||
// ── Route extraction ──
|
||||
extractDecoratorRoutes: extractSpringRoutes,
|
||||
extractRouteInheritanceTypes: extractSpringTypes,
|
||||
|
||||
// ── #2980: constant harvest + qualified-ref fold for non-literal mapping
|
||||
// paths (`@WinPostMapping(ApiPaths.SAVE_V1)`) — kept behind provider hooks so
|
||||
// the shared ingestion layers stay language-agnostic. The heuristic is
|
||||
// SYNTAX-driven (field/import shape), never a class-name pattern: constant
|
||||
// classes are routinely named `ApiPaths`/`Routes`/`Paths`, which a
|
||||
// `*Constants`-style gate would silently drop (review round-2 High finding).
|
||||
extractModuleConstants: extractJavaModuleConstants,
|
||||
moduleConstantHeuristic: (content) =>
|
||||
/\bstatic\s+final\s+String\s/.test(content) ||
|
||||
// `import com.winning.opt.common.ApiPaths;` — ANY class import can bind a
|
||||
// constant ref (`ApiPaths.X` at an annotation site), so gate on the
|
||||
// general import shape, not on the imported name.
|
||||
/\bimport\s+(?:static\s+)?[\w.]+\s*;/.test(content),
|
||||
foldRoutePathOperands: foldJavaOperands,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ import {
|
|||
} from './python/index.js';
|
||||
import { extractDjangoRoutes } from '../route-extractors/django.js';
|
||||
import { discoverDjangoRootUrls } from '../route-extractors/django-root-discovery.js';
|
||||
import { extractPythonModuleConstants } from '../route-extractors/python-const-resolver.js';
|
||||
|
||||
const BUILT_INS: ReadonlySet<string> = new Set([
|
||||
'print',
|
||||
|
|
@ -158,4 +159,13 @@ export const pythonProvider = defineLanguage({
|
|||
receiverBinding: pythonReceiverBinding,
|
||||
arityCompatibility: pythonArityCompatibility,
|
||||
resolveImportTarget: resolvePythonImportTarget,
|
||||
|
||||
// ── #2391 constant harvest, provider-hook form (#2980): module-level string
|
||||
// constants + from-imports for non-literal decorator route paths. Bare-name
|
||||
// refs fold through the shared resolver (no foldRoutePathOperands needed).
|
||||
extractModuleConstants: extractPythonModuleConstants,
|
||||
moduleConstantHeuristic: (content) =>
|
||||
// Module-level assignment (`X = "/path"`, possibly typed/annotated) or a
|
||||
// from-import that can bind a constant name at a decorator site.
|
||||
/^[^\S\n]*(?:[A-Za-z_]\w*\s*=\s*['"]|from\s+[\w.]+\s+import\s)/m.test(content),
|
||||
});
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@ import {
|
|||
createParserForLanguage,
|
||||
} from '../../tree-sitter/parser-loader.js';
|
||||
import { parseSourceSafe } from '../../tree-sitter/safe-parse.js';
|
||||
import { getProvider, providers } from '../languages/index.js';
|
||||
import { getProvider, getProviderForFile, providers } from '../languages/index.js';
|
||||
import { SCOPE_RESOLVERS } from '../scope-resolution/pipeline/registry.js';
|
||||
import { DATA_ROUTE_TABLE_SOURCE } from '../route-extractors/data-route-table.js';
|
||||
import type Parser from 'tree-sitter';
|
||||
|
|
@ -92,7 +92,6 @@ import {
|
|||
resolveOperands,
|
||||
type ModuleConstants,
|
||||
} from '../route-extractors/python-const-resolver.js';
|
||||
import { foldJavaOperands } from '../route-extractors/java-const-resolver.js';
|
||||
import {
|
||||
resolveInheritedSpringRoutes,
|
||||
type SharedSpringType,
|
||||
|
|
@ -1304,10 +1303,14 @@ export async function runChunkedParseAndResolve(
|
|||
resolvedRoutes.push(dr);
|
||||
continue;
|
||||
}
|
||||
const isJavaRoute = dr.filePath.endsWith('.java');
|
||||
// Provider-driven fold (#2980): languages with qualified-ref semantics
|
||||
// (Java `ApiPaths.X` / `com.example.ApiPaths.X`) fold through their
|
||||
// provider hook; everything else uses the shared language-agnostic
|
||||
// operand fold. No language names in the shared layer.
|
||||
const fold = getProviderForFile(dr.filePath)?.foldRoutePathOperands;
|
||||
const value = dr.routePathOperands
|
||||
? isJavaRoute
|
||||
? foldJavaOperands(dr.filePath, dr.routePathOperands, repoConstants)
|
||||
? fold
|
||||
? fold(dr.filePath, dr.routePathOperands, repoConstants)
|
||||
: resolveOperands(dr.filePath, dr.routePathOperands, repoConstants)
|
||||
: null;
|
||||
if (value === null) {
|
||||
|
|
|
|||
|
|
@ -133,6 +133,24 @@ function stringLiteralValue(node: Parser.SyntaxNode): string | null {
|
|||
return parts.map((c) => c.text).join('');
|
||||
}
|
||||
|
||||
/**
|
||||
* Flatten a qualified-name expression (`ApiPaths`, `com.example.ApiPaths`) to
|
||||
* its dotted text, or null when any segment is not a plain identifier (calls,
|
||||
* `this`, array access, generics — not a static constant shape).
|
||||
*/
|
||||
function flattenQualifiedIdentifier(node: Parser.SyntaxNode): string | null {
|
||||
if (node.type === 'identifier') return node.text;
|
||||
if (node.type === 'field_access') {
|
||||
const object = node.childForFieldName('object');
|
||||
const field = node.childForFieldName('field');
|
||||
if (object && field) {
|
||||
const head = flattenQualifiedIdentifier(object);
|
||||
return head === null ? null : `${head}.${field.text}`;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse a Java constant initializer into an operand list, or null when it is
|
||||
* not a foldable string expression. Handles a bare string literal, a bare
|
||||
|
|
@ -155,12 +173,17 @@ export function parseJavaConstOperands(
|
|||
if (node.type === 'identifier') {
|
||||
return [{ kind: 'ref', name: node.text }];
|
||||
}
|
||||
// `CONSTS.FIELD` — field_access in tree-sitter-java for expressions.
|
||||
// `CONSTS.FIELD` — field_access in tree-sitter-java for expressions. The
|
||||
// object side may itself be a chain (`com.example.ApiPaths` parses as
|
||||
// nested field_access), so flatten recursively: every segment must be a
|
||||
// plain identifier/keyword to qualify (a call `f().X`, `this.X`, or an
|
||||
// array access object side is not a constant shape → null, skip floor).
|
||||
if (node.type === 'field_access') {
|
||||
const object = node.childForFieldName('object');
|
||||
const field = node.childForFieldName('field');
|
||||
if (object && field && object.type === 'identifier') {
|
||||
return [{ kind: 'ref', name: `${object.text}.${field.text}` }];
|
||||
if (object && field) {
|
||||
const objectName = flattenQualifiedIdentifier(object);
|
||||
if (objectName !== null) return [{ kind: 'ref', name: `${objectName}.${field.text}` }];
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
|
@ -261,25 +284,45 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants {
|
|||
const nameNode = decl.childForFieldName('name');
|
||||
const valueNode = decl.childForFieldName('value');
|
||||
if (!nameNode) continue;
|
||||
const operands = parseJavaConstOperands(valueNode);
|
||||
if (operands === null) continue;
|
||||
const name = nameNode.text;
|
||||
// Java guarantees one initializer per `static final` field (duplicate
|
||||
// declarations are compile errors), so a redeclaration cannot smuggle
|
||||
// a stale value past a non-foldable one — no shadowing cleanup needed
|
||||
// (unlike Python, where #2391 drops rebinding names from the map).
|
||||
if (operands.length === 1 && operands[0].kind === 'literal') {
|
||||
literals.set(name, (operands[0] as { value: string }).value);
|
||||
const operands = parseJavaConstOperands(valueNode);
|
||||
// Same-name shadowing across nested types (legal Java, unlike
|
||||
// same-class redeclaration): a later binding must REPLACE the earlier
|
||||
// flattened simple-name entry — including dropping it to unresolvable
|
||||
// when the new initializer is not foldable (`X = compute()`) — rather
|
||||
// than leave the stale outer literal resolvable. Skip floor, mirroring
|
||||
// Python #2391's rebind-drop. Qualified `Class.FIELD` aliases are
|
||||
// per-type-keyed but same-named nested types can still collide, so
|
||||
// they get the same replace/drop treatment.
|
||||
const qname = declaringClass ? `${declaringClass}.${name}` : null;
|
||||
if (operands === null) {
|
||||
literals.delete(name);
|
||||
exprs.delete(name);
|
||||
if (qname) {
|
||||
literals.delete(qname);
|
||||
exprs.delete(qname);
|
||||
}
|
||||
continue;
|
||||
}
|
||||
const literalValue =
|
||||
operands.length === 1 && operands[0].kind === 'literal'
|
||||
? (operands[0] as { value: string }).value
|
||||
: null;
|
||||
if (literalValue !== null) {
|
||||
literals.set(name, literalValue);
|
||||
exprs.delete(name);
|
||||
} else {
|
||||
exprs.set(name, operands);
|
||||
literals.delete(name);
|
||||
}
|
||||
// Qualified alias: `CONSTS.X` refs (folded refs carry the class name).
|
||||
if (declaringClass) {
|
||||
const qname = `${declaringClass}.${name}`;
|
||||
if (operands.length === 1 && operands[0].kind === 'literal') {
|
||||
literals.set(qname, (operands[0] as { value: string }).value);
|
||||
if (qname) {
|
||||
if (literalValue !== null) {
|
||||
literals.set(qname, literalValue);
|
||||
exprs.delete(qname);
|
||||
} else {
|
||||
exprs.set(qname, operands);
|
||||
literals.delete(qname);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -296,9 +339,15 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants {
|
|||
const body = child.children.find(
|
||||
(c) => c.type === 'class_body' || c.type === 'interface_body',
|
||||
);
|
||||
if (body && className)
|
||||
collectFieldConstants(body, isInterface || insideInterface, className);
|
||||
if (body) walkTypes(body, isInterface || insideInterface);
|
||||
// Recompute implicit interface semantics at each type boundary: a
|
||||
// class nested in an interface is a normal class whose fields need
|
||||
// explicit `static final` (JLS 9.5 — only the interface's own fields
|
||||
// are implicitly public static final). Propagating the outer
|
||||
// `insideInterface` flag in would harvest mutable nested fields as
|
||||
// constants and let a same-name nested field shadow a real interface
|
||||
// constant with a stale value.
|
||||
if (body && className) collectFieldConstants(body, isInterface, className);
|
||||
if (body) walkTypes(body, isInterface);
|
||||
} else if (child.type === 'enum_declaration' || child.type === 'record_declaration') {
|
||||
walkTypes(child, insideInterface);
|
||||
} else {
|
||||
|
|
|
|||
|
|
@ -1421,12 +1421,10 @@ export function extractORMQueries(
|
|||
|
||||
import { extractFastAPIRouterBindings } from '../route-extractors/fastapi-router-bindings.js';
|
||||
import {
|
||||
extractPythonModuleConstants,
|
||||
parseConstOperands,
|
||||
type ModuleConstants,
|
||||
type Operand,
|
||||
} from '../route-extractors/python-const-resolver.js';
|
||||
import { extractJavaModuleConstants } from '../route-extractors/java-const-resolver.js';
|
||||
|
||||
/**
|
||||
* Report a non-fatal worker issue to the pool over IPC so a caught error is not
|
||||
|
|
@ -2964,34 +2962,17 @@ const processFileGroup = (
|
|||
(result.routerModuleAliases ??= []),
|
||||
(result.routerConstructorPrefixes ??= []),
|
||||
);
|
||||
// #2391: harvest module-level string constants + from-imports so parse-impl
|
||||
// can resolve non-literal decorator route paths cross-file. Only emit for
|
||||
// files that carry something resolvable (a constant definition or an import
|
||||
// binding) to keep the aggregate bounded on large repos.
|
||||
const constants = extractPythonModuleConstants(tree);
|
||||
if (constants.literals.size > 0 || constants.exprs.size > 0 || constants.imports.size > 0) {
|
||||
(result.moduleConstants ??= []).push({ filePath: file.path, constants });
|
||||
}
|
||||
}
|
||||
|
||||
// Java parity of the #2391 constant harvest: static-final String fields +
|
||||
// class/static imports, folded cross-file by parse-impl for non-literal
|
||||
// Spring mapping paths (`@WinPostMapping(ApiPathConstants.SAVE_V1)`).
|
||||
// Cost-gated on file content — a file with no `static final String` and no
|
||||
// constants-bearing import is not parsed for constants.
|
||||
if (language === SupportedLanguages.Java) {
|
||||
if (
|
||||
/static\s+final\s+String\s/.test(parseContent) ||
|
||||
/import\s+(static\s+)?[\w.]*Constants/.test(parseContent)
|
||||
) {
|
||||
const javaConstants = extractJavaModuleConstants(tree);
|
||||
if (
|
||||
javaConstants.literals.size > 0 ||
|
||||
javaConstants.exprs.size > 0 ||
|
||||
javaConstants.imports.size > 0
|
||||
) {
|
||||
(result.moduleConstants ??= []).push({ filePath: file.path, constants: javaConstants });
|
||||
}
|
||||
// #2391/#2980: harvest module-level string constants + import bindings via
|
||||
// the provider hook so parse-impl can resolve non-literal decorator route
|
||||
// paths cross-file. Cost-gated by the provider's syntax-driven heuristic;
|
||||
// only files that carry something resolvable (a constant definition or an
|
||||
// import binding) are emitted, keeping the aggregate bounded on large repos.
|
||||
if (provider.extractModuleConstants && provider.moduleConstantHeuristic?.(parseContent)) {
|
||||
const constants = provider.extractModuleConstants(tree);
|
||||
if (constants.literals.size > 0 || constants.exprs.size > 0 || constants.imports.size > 0) {
|
||||
(result.moduleConstants ??= []).push({ filePath: file.path, constants });
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
206
gitnexus/test/unit/java-route-const-pipeline-e2e.test.ts
Normal file
206
gitnexus/test/unit/java-route-const-pipeline-e2e.test.ts
Normal file
|
|
@ -0,0 +1,206 @@
|
|||
/**
|
||||
* #2980 review round-2: COLD and WARM pipeline e2e for the provider-hook
|
||||
* constant harvest (`extractModuleConstants` / `moduleConstantHeuristic` /
|
||||
* `foldRoutePathOperands`).
|
||||
*
|
||||
* The maintainer's blocking finding: unit tests only exercised worker-gated
|
||||
* helpers — never the REAL pipeline. A controller referencing constants from
|
||||
* a class NOT named `*Constants` (e.g. `ApiPaths`) was silently dropped:
|
||||
* the old content gate `/import ... [\\w.]*Constants/` never matched, the
|
||||
* constants file never entered the import map, the route resolved to null and
|
||||
* got skipped.
|
||||
*
|
||||
* This file drives the REAL `runChunkedParseAndResolve` with the REAL compiled
|
||||
* dist worker (vitest auto-falls back to dist/core/ingestion/workers/
|
||||
* parse-worker.js) over a fixture repo shaped like the reviewer's example:
|
||||
*
|
||||
* repo/
|
||||
* src/main/java/com/example/ApiPaths.java — constants class NOT named
|
||||
* *Constants (the High bug)
|
||||
* src/main/java/com/example/UserController.java — @RequestMapping prefix +
|
||||
* @PostMapping(ApiPaths.X) +
|
||||
* FQN form + inline concat +
|
||||
* static import
|
||||
*
|
||||
* Assertions (both runs):
|
||||
* - the emitted Route node carries the FOLDED literal path, not the expr;
|
||||
* - ALL THREE non-literal shapes survive (qualified, FQN-qualified, concat);
|
||||
* - a phantom `POST ` / empty path never appears (skip floor);
|
||||
* - the warm run (parse-cache replay, no worker spawn) yields the IDENTICAL
|
||||
* route set — the harvest result survives the structured-clone cache round
|
||||
* trip (ModuleConstants uses Map, exercised through mapReplacer/mapReviver).
|
||||
*
|
||||
* Rebuild gate: this test requires dist/ to be current; when dist/ is stale
|
||||
* (older than src/) it self-skips with a loud message rather than silently
|
||||
* asserting against the old binary. (CI builds before vitest, so it runs.)
|
||||
*/
|
||||
import { beforeEach, afterEach, describe, expect, it } from 'vitest';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
import { createKnowledgeGraph } from '../../src/core/graph/graph.js';
|
||||
import { runChunkedParseAndResolve } from '../../src/core/ingestion/pipeline-phases/parse-impl.js';
|
||||
import { PARSE_CACHE_VERSION, type ParseCache } from '../../src/storage/parse-cache.js';
|
||||
|
||||
// ── dist freshness gate ───────────────────────────────────────────────────
|
||||
const repoRoot = path.resolve(__dirname, '..', '..');
|
||||
const distWorker = path.join(repoRoot, 'dist', 'core', 'ingestion', 'workers', 'parse-worker.js');
|
||||
const srcWorker = path.join(repoRoot, 'src', 'core', 'ingestion', 'workers', 'parse-worker.ts');
|
||||
const distStale =
|
||||
!fs.existsSync(distWorker) || fs.statSync(distWorker).mtimeMs < fs.statSync(srcWorker).mtimeMs;
|
||||
|
||||
const maybeDescribe = distStale ? describe.skip : describe;
|
||||
|
||||
// ── fixture repo (reviewer's exact High-finding shape) ────────────────────
|
||||
const API_PATHS = `package com.example.common;
|
||||
|
||||
public class ApiPaths {
|
||||
public static final String USERS = "/api/v1/users";
|
||||
public static final String ORDERS = "/api/v1/orders";
|
||||
public static final String V1 = "/api/v1";
|
||||
}
|
||||
`;
|
||||
|
||||
const USER_CONTROLLER = `package com.example;
|
||||
|
||||
import com.example.common.ApiPaths;
|
||||
import org.springframework.web.bind.annotation.RequestMapping;
|
||||
import org.springframework.web.bind.annotation.PostMapping;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
|
||||
@RequestMapping("/users")
|
||||
public class UserController {
|
||||
|
||||
// Qualified ref via a class NOT named *Constants (High finding): the old
|
||||
// gate dropped the whole route because ApiPaths fails the name pattern.
|
||||
@PostMapping(ApiPaths.USERS)
|
||||
public void create() {}
|
||||
|
||||
// FQN-qualified form (F3): multi-segment field_access chain.
|
||||
@GetMapping(com.example.common.ApiPaths.ORDERS)
|
||||
public void list() {}
|
||||
|
||||
// Inline concat with a static-import-style bare ref (same-file constant
|
||||
// through the composed-operand fold).
|
||||
@PostMapping(com.example.common.ApiPaths.V1 + "/orders")
|
||||
public void createOrders() {}
|
||||
}
|
||||
`;
|
||||
|
||||
let repoDir: string;
|
||||
let storageDir: string;
|
||||
|
||||
function writeFixture(): { path: string; size: number }[] {
|
||||
const files: [string, string][] = [
|
||||
['src/main/java/com/example/common/ApiPaths.java', API_PATHS],
|
||||
['src/main/java/com/example/UserController.java', USER_CONTROLLER],
|
||||
];
|
||||
const out: { path: string; size: number }[] = [];
|
||||
for (const [rel, content] of files) {
|
||||
const full = path.join(repoDir, rel);
|
||||
fs.mkdirSync(path.dirname(full), { recursive: true });
|
||||
fs.writeFileSync(full, content);
|
||||
out.push({ path: rel, size: Buffer.byteLength(content) });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
/**
|
||||
* The parse phase does not emit Route nodes itself — it returns the folded
|
||||
* `decoratorRoutes` (the routes phase emits them downstream). Asserting on the
|
||||
* folded paths at THIS seam is exactly the regression the maintainer asked
|
||||
* for: the worker's harvest → provider heuristic → parse-impl fold, with the
|
||||
* real dist worker.
|
||||
*/
|
||||
type PipelineResult = Awaited<ReturnType<typeof runChunkedParseAndResolve>>;
|
||||
|
||||
function foldedRoutesOf(result: PipelineResult): Array<{ path: string; method: string }> {
|
||||
return (result.allDecoratorRoutes ?? [])
|
||||
.filter((r) => typeof r.routePath === 'string')
|
||||
.map((r) => ({ path: r.routePath, method: r.httpMethod }));
|
||||
}
|
||||
|
||||
async function runPipeline(
|
||||
cache: ParseCache,
|
||||
files: { path: string; size: number }[],
|
||||
): Promise<PipelineResult> {
|
||||
const kg = createKnowledgeGraph();
|
||||
return await runChunkedParseAndResolve(
|
||||
kg,
|
||||
files,
|
||||
files.map((f) => f.path),
|
||||
files.length,
|
||||
repoDir,
|
||||
Date.now(),
|
||||
() => {},
|
||||
{ workerPoolSize: 1, parseCache: cache },
|
||||
);
|
||||
}
|
||||
|
||||
maybeDescribe('#2980 provider-hook constant harvest — real pipeline (cold + warm)', () => {
|
||||
beforeEach(() => {
|
||||
repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gnx-2980-cold-'));
|
||||
storageDir = path.join(repoDir, '.gitnexus');
|
||||
});
|
||||
afterEach(() => {
|
||||
for (const d of [repoDir]) fs.rmSync(d, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('cold run: folds qualified / FQN / concat paths from a non-*Constants class', async () => {
|
||||
const files = writeFixture();
|
||||
const cache: ParseCache = {
|
||||
version: PARSE_CACHE_VERSION,
|
||||
entries: new Map(),
|
||||
usedKeys: new Set(),
|
||||
storagePath: storageDir,
|
||||
onDiskKeys: new Set(),
|
||||
};
|
||||
|
||||
const result = await runPipeline(cache, files);
|
||||
const routes = foldedRoutesOf(result);
|
||||
|
||||
// All three non-literal shapes resolve to folded literals. (The class-level
|
||||
// @RequestMapping("/users") prefix join happens in the downstream routes
|
||||
// phase — at this seam we assert the method-level folded paths.)
|
||||
const paths = routes.map((r) => r.path).sort();
|
||||
expect(paths).toContain('/api/v1/users'); // qualified ref via import
|
||||
expect(paths).toContain('/api/v1/orders'); // FQN multi-segment chain
|
||||
// The concat route folds to the same literal as the FQN route.
|
||||
expect(paths.filter((p) => p === '/api/v1/orders').length).toBeGreaterThanOrEqual(2);
|
||||
// Skip floor: no phantom empty/raw-expr paths.
|
||||
for (const p of paths) {
|
||||
expect(p.length).toBeGreaterThan(1);
|
||||
expect(p).not.toContain('ApiPaths');
|
||||
expect(p).not.toContain('com.example');
|
||||
}
|
||||
}, 120_000);
|
||||
|
||||
it('warm run: parse-cache replay yields the identical folded route set', async () => {
|
||||
const files = writeFixture();
|
||||
const cache: ParseCache = {
|
||||
version: PARSE_CACHE_VERSION,
|
||||
entries: new Map(),
|
||||
usedKeys: new Set(),
|
||||
storagePath: storageDir,
|
||||
onDiskKeys: new Set(),
|
||||
};
|
||||
|
||||
// Run #1 populates the cache; persist it like run-analyze does.
|
||||
const run1 = await runPipeline(cache, files);
|
||||
const { saveParseCache, pruneCache } = await import('../../src/storage/parse-cache.js');
|
||||
pruneCache(cache, cache.usedKeys);
|
||||
await saveParseCache(storageDir, cache);
|
||||
|
||||
// Run #2 — warm: every chunk is a cache HIT, no worker spawn, the cached
|
||||
// ParseWorkerResult (moduleConstants included) is replayed from disk.
|
||||
const { loadParseCache } = await import('../../src/storage/parse-cache.js');
|
||||
const warm = await loadParseCache(storageDir);
|
||||
const run2 = await runPipeline(warm, files);
|
||||
|
||||
const cold = foldedRoutesOf(run1).map((r) => `${r.method} ${r.path}`).sort();
|
||||
const hot = foldedRoutesOf(run2).map((r) => `${r.method} ${r.path}`).sort();
|
||||
expect(hot).toEqual(cold);
|
||||
expect(hot.length).toBeGreaterThan(0);
|
||||
}, 120_000);
|
||||
});
|
||||
|
|
@ -369,3 +369,133 @@ public class BConsts {
|
|||
).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Review round 2 regressions (#2980) ───────────────────────────────────
|
||||
|
||||
describe('F4: class nested in an interface is NOT implicitly final', () => {
|
||||
const SRC = `package p;
|
||||
public interface Api {
|
||||
String BASE = "/api";
|
||||
class Holder {
|
||||
String mutable = "/mutable";
|
||||
static final String OK = "/ok";
|
||||
}
|
||||
interface Inner {
|
||||
String IMPLICIT = "/implicit";
|
||||
class Deep {
|
||||
String alsoMutable = "/also";
|
||||
}
|
||||
}
|
||||
}`;
|
||||
|
||||
it('harvests the interface own fields and explicit static final nested fields', () => {
|
||||
const mc = extractJavaModuleConstants(parse(SRC));
|
||||
expect(mc.literals.get('BASE')).toBe('/api');
|
||||
expect(mc.literals.get('OK')).toBe('/ok');
|
||||
expect(mc.literals.get('Holder.OK')).toBe('/ok');
|
||||
});
|
||||
|
||||
it('does NOT harvest mutable fields of a class nested in an interface', () => {
|
||||
const mc = extractJavaModuleConstants(parse(SRC));
|
||||
expect(mc.literals.has('mutable')).toBe(false);
|
||||
expect(mc.literals.has('alsoMutable')).toBe(false);
|
||||
expect(mc.literals.has('Holder.mutable')).toBe(false);
|
||||
expect(mc.exprs.has('mutable')).toBe(false);
|
||||
});
|
||||
|
||||
it('still harvests a class directly nested in an interface (own implicit semantics recomputed at each boundary)', () => {
|
||||
const mc = extractJavaModuleConstants(parse(SRC));
|
||||
expect(mc.literals.get('IMPLICIT')).toBe('/implicit');
|
||||
expect(mc.literals.get('Inner.IMPLICIT')).toBe('/implicit');
|
||||
});
|
||||
});
|
||||
|
||||
describe('F5: same-name shadowing across nested types drops the stale entry', () => {
|
||||
const SRC = `package p;
|
||||
public class Outer {
|
||||
public static final String PATH = "/v1";
|
||||
static class Inner {
|
||||
// shadows Outer.PATH with a non-foldable initializer
|
||||
public static final String PATH = compute();
|
||||
static String compute() { return "/v2"; }
|
||||
}
|
||||
}`;
|
||||
|
||||
it('a non-foldable shadow must drop the outer literal, not keep it (skip floor)', () => {
|
||||
const mc = extractJavaModuleConstants(parse(SRC));
|
||||
expect(mc.literals.has('PATH')).toBe(false);
|
||||
expect(mc.exprs.has('PATH')).toBe(false);
|
||||
});
|
||||
|
||||
it('qualified aliases survive per class (Outer.PATH resolvable, Inner.PATH not)', () => {
|
||||
const mc = extractJavaModuleConstants(parse(SRC));
|
||||
expect(mc.literals.get('Outer.PATH')).toBe('/v1');
|
||||
expect(mc.literals.has('Inner.PATH')).toBe(false);
|
||||
});
|
||||
|
||||
it('a foldable shadow REPLACES the outer value (last binding wins in source order)', () => {
|
||||
const src = `package p;
|
||||
public class Outer {
|
||||
public static final String PATH = "/v1";
|
||||
static class Inner {
|
||||
public static final String PATH = "/v2";
|
||||
}
|
||||
}`;
|
||||
const mc = extractJavaModuleConstants(parse(src));
|
||||
expect(mc.literals.get('PATH')).toBe('/v2');
|
||||
expect(mc.literals.get('Outer.PATH')).toBe('/v1');
|
||||
expect(mc.literals.get('Inner.PATH')).toBe('/v2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('F3: multi-segment FQN annotation values and constant initializers', () => {
|
||||
const constValueOf = (src: string): Parser.SyntaxNode => {
|
||||
const cls = parse(src).rootNode.descendantsOfType('class_declaration')[0]!;
|
||||
const body = cls.childForFieldName('body')!;
|
||||
const field = body.children.find((c) => c.type === 'field_declaration')!;
|
||||
const decl = field.children.find((c) => c.type === 'variable_declarator')!;
|
||||
return decl.childForFieldName('value')!;
|
||||
};
|
||||
|
||||
it('parses com.example.ApiPaths.USERS as ONE ref (nested field_access chain flattened)', () => {
|
||||
const ops = parseJavaConstOperands(constValueOf(`package p;
|
||||
public class W {
|
||||
public static final String X = com.example.ApiPaths.USERS;
|
||||
}`));
|
||||
expect(ops).toEqual([{ kind: 'ref', name: 'com.example.ApiPaths.USERS' }]);
|
||||
});
|
||||
|
||||
it('still rejects call/object-side chains: f().X, this.X, arr[0].X', () => {
|
||||
expect(parseJavaConstOperands(constValueOf(`package p;
|
||||
public class W { public static final String A = f().X; static Object f(){return null;} }`))).toBeNull();
|
||||
expect(parseJavaConstOperands(constValueOf(`package p;
|
||||
public class W { public static final String B = this.Y; String Y = "y"; }`))).toBeNull();
|
||||
expect(parseJavaConstOperands(constValueOf(`package p;
|
||||
public class W { public static final String C = arr[0].Z; }`))).toBeNull();
|
||||
});
|
||||
|
||||
it('resolves an FQN-qualified annotation constant end-to-end (query → operands → fold)', () => {
|
||||
const repo = repoOf({
|
||||
'src/main/java/com/example/ApiPaths.java': `package com.example;
|
||||
public class ApiPaths {
|
||||
public static final String USERS = "/api/v1/users";
|
||||
}`,
|
||||
'src/main/java/com/example/Ctl.java': `package com.example;
|
||||
import org.springframework.web.bind.annotation.PostMapping;
|
||||
public class Ctl {
|
||||
@PostMapping(com.example.ApiPaths.USERS)
|
||||
public void list() {}
|
||||
}`,
|
||||
});
|
||||
// The whole FQN arrives as one ref operand (verified against the real
|
||||
// tree-sitter-java parse shape); the resolver must follow it via the
|
||||
// longest-prefix import fallback.
|
||||
expect(
|
||||
resolveJavaConstant(
|
||||
'src/main/java/com/example/Ctl.java',
|
||||
'com.example.ApiPaths.USERS',
|
||||
repo,
|
||||
),
|
||||
).toBe('/api/v1/users');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue