mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-06 02:49:56 +00:00
* feat(group): resolve Java constant-based route paths via repo constant map - prepareRepo builds repo-wide Java constant map (constant-definition files only, cheap regex gate; per-file try/catch so one bad file degrades not forfeits) - bind parser language in prepareRepo (orchestrator hands over a bare Parser) - scan() lazily overlays the importing file's own import table (extracted from the tree already in hand, zero extra parses) before folding operands - foldJavaOperands resolves qualified refs (Class.CONST) + static imports + string concatenation against the merged view; unresolved refs are skipped, never guessed Real-repo validation (winning-winex-opt, 23k Java files): providers 2 -> 1701 (1700 source_scan_resolved), cross-links 0 -> 589 exact Unit: 14/14 (java-route-const-resolver.test.ts) * fix(review): address bot review findings on PR #2980 - P2-1 (real): spring.ts route loop dropped every @value_expr match — the '!valueNode' guard ran before the operand branch, so ingestion emitted zero constant-referencing routes. Guard now accepts @value_expr when @value is absent; two downstream valueNode dereferences made conditional. Added 2 extractor-level regression tests (16 total). - P2-2 (real): collectSpringTypes copied rawPath:'' for constant routes into the shared Spring inheritance view — now skipped there (fold happens in scan(); empty-path noise would leak into inheritance-based providers). - P1-1 (false positive): Java 'static final' allows exactly one initializer (duplicate declarations are compile errors), so the Python-style rebinding shadowing cleanup does not apply — documented at the site. - P1-2 (false positive): constant-resolver.ts and prepareDurableParsedFileChunk both exist on upstream main (#2391 / parsedfile-store.ts:562); the bot's 'repository lookup' appears to have compared against a stale index. - P3: removed dead FQN_CONTROLLER fixture. Real-repo regression: 589 cross-links / 2423 contracts (was 2424 — the dropped contract is the empty-path inheritance artifact fixed above). * docs(cache): note Java constant-route capture set in the SCHEMA_BUMP ledger The Java constant-route harvest (route-extractors/java-const-resolver.ts + the spring.ts operand branch + the parse-worker Java constant harvest) changes the worker capture set: a warm pre-feature cache replays moduleConstants=0 captures verbatim and silently drops every constant-based Spring route on unchanged files. After rebasing onto current main the ledger already sits at 70, whose capture set post-dates and includes this harvest, so v70 invalidates those caches — no additional bump is needed. * fix(feign): guard @RequestLine against the constant-valued shape A constant-valued `@RequestLine(SOME_CONST)` is captured as @value_expr, not @value, so `valueNode` is undefined in that shape and the literal dereference crashed the scan. Skip instead — folding verb+path literals through the constant map is out of scope for this PR. Found in maintainer review of #2980. * fix(resolver): bound qualified-ref recursion depth for self/mutual import cycles Maintainer review point: the qualified branch of resolveJavaConstant recurses through resolveJavaImport without a guard — a self-import (X = SelfConsts.X + ...) or a pair of mutually-importing constants would recurse without bound before reaching the shared fold's visited-stack, which only guards the bare-name path. Bound the Java-qualified walk with a depth cap (32) and thread it through every recursive call. Two regression tests use real repo shapes (repoOf fixtures): self-import and mutual-import cycles both terminate with null (skip floor), as before, but promptly. Also drops the stray machine-local .gitignore entry that rode along from the fork's dev branch. * 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. * style: prettier --write on the two touched test files (CI format gate) * fix(routes): address the open review findings on Java constant route folding Answers every reproduced finding still open on #2980, plus the defects an adversarial pass found in the first round of those fixes. The wrong-path group each turned a *missing* fact into a *wrong* one, which is what this module's skip-or-correct contract exists to prevent. Wrong-path fixes * Escapes were deleted from constant values. tree-sitter-java splits a `string_literal` around its `escape_sequence` children, so joining `string_fragment`s alone folded `"/user/{id:\\d+}"` — the standard Spring path-variable constraint — to `/user/{id:d+}`, and a pure-escape literal to the empty string. Worse, the LITERAL path keeps escapes verbatim, so one Java route had two irreconcilable spellings. `stringLiteralValue` now reuses `unquoteSpringLiteral`, the helper that literal path already uses. Java text blocks are excluded: that helper's `"""` arm would hand back the raw block, newline and incidental indentation included, so they keep the old skip. * A constant-valued class prefix produced a truncated route. The new `@value_expr` query branches were `method_declaration`-only, so `@RequestMapping(ApiPaths.BASE)` left the prefix empty and the method route was emitted unprefixed — a path the application does not serve, where the base emitted nothing at all. Both subsystems now detect such a class and suppress its method routes, the rule `classesWithArrayPrefix` already encodes for the array form. The suppression covers ingestion's separate no-argument-mapping loop too, without which a bare `@GetMapping` under a constant prefix still shipped an empty-path Route while the group emitted nothing. * A shadowed static import survived a non-foldable rebind. The rebind-drop deleted `literals`/`exprs` but not `imports`, so a name both static-imported and locally redeclared resolved through the stale import to the imported value instead of skipping (#2393's Python defect, reproduced for Java). * `resolveJavaImport` guessed where its own docstring promised null. The nearest-shared-directory tie-break is gone: javac resolves duplicate FQNs by classpath order, so proximity can return a src/test fixture copy. Parity and coverage fixes * One constant-file gate, exported as `isJavaConstantFile` and used by both the ingestion provider and the group `prepareRepo` pre-pass. The two spellings disagreed on a constant INTERFACE — implicitly `public static final`, so it carries neither keyword — which the group admitted and ingestion rejected, so the group published a contract while the graph got no Route node. It is also modifier-order agnostic now, and its interface arm requires a String assignment so a javadoc mentioning "interface" no longer costs a parse. * Import ambiguity is measured over constant-DEFINING files on both sides. Ingestion's harvest gate also admits import-only files, so handing `resolveJavaImport` every repo key let a duplicate FQN that defines nothing make ingestion alone floor to skip — reopening the same parity break in the same losing direction. * Python's constant harvest is unconditional again. The gate added here required NAME immediately followed by `=`, so it dropped `API: str = "/api"`, `API: Final[str] = "/api"` and every composed constant whose RHS starts with an identifier — routes that already resolve on main. The worker now treats a missing heuristic as "harvest" rather than "skip". * Enum and record declarations were traversed but never collected, so a `static final String` declared in one was absent from the map. The walk still descends the whole body, so a type nested in an enum-constant body is kept. * Constants composed across files through a qualified ref never resolved: operands found inside an initializer went to the agnostic core, which only knows bare names, so `X = BConsts.Y + "/tail"` floored to null even acyclically. The Java binding now folds its own expressions — and carries the core's guards with them: a `visited` stack popped on unwind, a memo of successes, and `MAX_FOLD_LENGTH`. Without the memo a shared-descendant DAG re-folds each child per reference; because a chain of empty strings never accumulates output, the length cap could not stop it, and one route over a 31-line constants file took 11 s at 28 levels on the main thread. * Dropped the dead `com.java.lang.` type normalization. Cache * `SCHEMA_BUMP` 70 -> 72. Leaving it at 70 was justified by "the ledger already sits at 70, whose capture set post-dates and includes this harvest" — it does not: 70 was cut byfe3d7e56bfor #2417/#2891, an ancestor of this base. With package.json untouched, `PARSE_CACHE_VERSION` was byte-identical across the merge, so every same-version warm cache replayed pre-feature captures and the feature was inert. 72 rather than 71 because open PR #3017 already claims 71 with an identical pin test — the ledger's rule is the next value above every in-flight claim, not above origin/main. Tests * Regression cover for each fix above, including a gate-level test (the gate itself had none), an import-ambiguity test, a text-block test, and a 30-level shared-descendant DAG that fails by timeout if the memo is ever removed. * New `group/java-const-route-parity.test.ts` drives `prepareRepo` + a three-argument `scan`. Every existing Spring parity guard calls `scan(tree)` with ONE argument, and the plugin drops constant-valued routes without a repo context — so those guards were structurally blind to this whole feature. * The pipeline e2e now proves the warm run is a REPLAY (`usedWorkerPool` false) instead of only comparing route sets. It was not one: the test never persisted the durable ParsedFile store, so the "warm" run reparsed through the workers and would have passed with the cache round-trip completely broken. * Its dist freshness gate covers every source the pipeline loads, not just parse-worker.ts, and prints the loud message the docblock promised. * The self-import cycle fixture now actually self-imports, so it reaches the qualified-ref recursion and its depth cap. * Removed the dead `WIN_POST_MAPPING` fixture and the claim behind it: Spring alias recognition is an exact-name map on this base, so `@WinPostMapping` extracts zero routes no matter how its value folds (#2883 is still open). Fixtures now use annotations this branch actually recognises. * fix(routes): widen the Java constant-file gate to match its extractor Answers the gitnexus-check round on43a0ff290. The gate was still narrower than the extractor it feeds, in two ways the extractor explicitly supports: * `static final String` was matched as an ADJACENT pair, but the extractor scans modifiers independently (`isStaticFinal`), so `static public final String PATH = "/x";` — legal Java — was extracted when parsed and never parsed, because the gate returned false. * the type had to be the bare token `String`, but the extractor also accepts `java.lang.String`, so `public static final java.lang.String PATH = "/x";` was skipped the same way. Both are the same defect class as the ingestion/group divergence this predicate was introduced to prevent, one layer down: a cost gate that is narrower than the thing it gates silently drops facts. The modifier run is now matched as a span excluding `;{}()`, so every legal order and the qualified type name are admitted while precision holds — a local `String s = "x"` inside `static void f() { … }` still does not match, because reaching it from `static` crosses `(`, `)` and `{`. `final` is deliberately not required: the gate may be wider than the extractor, never narrower. Also: the worker's harvest condition moves into `shouldHarvestModuleConstants` in `language-provider.ts`. The rule that is easy to get backwards — a provider declaring no `moduleConstantHeuristic` harvests unconditionally — was only reachable by booting a worker, so the Python tests could assert the extractor harvests and the provider declares no heuristic while a regression to `provider.moduleConstantHeuristic?.(content)` still turned the hook off. The tests now drive the predicate itself, plus the two branches around it. One finding in that round is not reproducible: the parity helper is not made unresolvable by its import-only fixture. Every `resolveJavaImport` call site passes the fold state's `constantKeys` — files with `literals`/`exprs` — not `repo.keys()`, so a same-FQN class defining nothing creates no ambiguity. That filtering is what the helper exists to exercise, and the test is green. --------- Co-authored-by: ChunxueLi <mecoloud@users.noreply.gitee.com> Co-authored-by: Gergő Magyar <gergomagyar@icloud.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
213 lines
8.3 KiB
TypeScript
213 lines
8.3 KiB
TypeScript
/**
|
|
* Group ↔ ingestion parity for Java constant-valued Spring route paths (#2980).
|
|
*
|
|
* Drives `JAVA_HTTP_PLUGIN.prepareRepo` + `scan(tree, ctx, rel)` with all three
|
|
* arguments and compares the result against what `extractSpringRoutes` + the
|
|
* Java operand fold produce on the ingestion side. The existing Spring parity
|
|
* guards call `scan(tree)` with ONE argument, which makes them structurally
|
|
* blind here: without a repo context the plugin drops every constant-valued
|
|
* route, so no fixture they carry can exercise this feature.
|
|
*
|
|
* Asserted:
|
|
* • a constant-valued mapping resolves to the SAME path on both sides;
|
|
* • a CONSTANT class prefix suppresses the method route on both sides — the
|
|
* prefix cannot be folded at extraction time, and emitting the route
|
|
* unprefixed would publish a path the application does not serve;
|
|
* • without a repo context the group side emits nothing (the documented skip
|
|
* floor, and the branch that makes the 1-arg guards blind);
|
|
* • literal routes are untouched.
|
|
*/
|
|
|
|
import { describe, it, expect } from 'vitest';
|
|
import Parser from 'tree-sitter';
|
|
import Java from 'tree-sitter-java';
|
|
import { JAVA_HTTP_PLUGIN } from '../../../src/core/group/extractors/http-patterns/java.js';
|
|
import type { HttpDetection } from '../../../src/core/group/extractors/http-patterns/types.js';
|
|
import { extractSpringRoutes } from '../../../src/core/ingestion/route-extractors/spring.js';
|
|
import { javaProvider } from '../../../src/core/ingestion/languages/java.js';
|
|
import {
|
|
extractJavaModuleConstants,
|
|
foldJavaOperands,
|
|
type RepoConstants,
|
|
} from '../../../src/core/ingestion/route-extractors/java-const-resolver.js';
|
|
|
|
const parser = new Parser();
|
|
const parseSource = (p: Parser, src: string): Parser.Tree => {
|
|
p.setLanguage(Java);
|
|
return p.parse(src);
|
|
};
|
|
const parse = (src: string): Parser.Tree => parseSource(parser, src);
|
|
|
|
/** Group side: prepareRepo + a 3-argument scan over every .java file. */
|
|
function groupProviders(files: Record<string, string>): string[] {
|
|
const ctx = JAVA_HTTP_PLUGIN.prepareRepo?.({
|
|
files: Object.keys(files),
|
|
parser: new Parser(),
|
|
readFile: (rel: string) => files[rel] ?? null,
|
|
parseSource,
|
|
});
|
|
const out: string[] = [];
|
|
for (const rel of Object.keys(files)) {
|
|
const detections: HttpDetection[] = JAVA_HTTP_PLUGIN.scan(parse(files[rel]), ctx, rel);
|
|
for (const d of detections) {
|
|
if (d.role === 'provider') out.push(`${d.method} ${d.path}`);
|
|
}
|
|
}
|
|
return out.sort();
|
|
}
|
|
|
|
/** Ingestion side: extract routes, then fold operands against the same map. */
|
|
function ingestionRoutes(files: Record<string, string>): string[] {
|
|
const repo: RepoConstants = new Map();
|
|
for (const [rel, src] of Object.entries(files)) {
|
|
repo.set(rel, extractJavaModuleConstants(parse(src)));
|
|
}
|
|
const out: string[] = [];
|
|
for (const [rel, src] of Object.entries(files)) {
|
|
for (const route of extractSpringRoutes(parse(src), rel, 0)) {
|
|
const path = route.routePathOperands
|
|
? foldJavaOperands(rel, route.routePathOperands, repo)
|
|
: route.routePath;
|
|
if (path === null) continue;
|
|
out.push(`${route.httpMethod} ${`${route.prefix ?? ''}${path}`.replace(/\/{2,}/g, '/')}`);
|
|
}
|
|
}
|
|
return out.sort();
|
|
}
|
|
|
|
const CONSTS = 'src/main/java/com/example/ApiPaths.java';
|
|
const CTL = 'src/main/java/com/example/OrderController.java';
|
|
|
|
const CONSTS_SRC = `package com.example;
|
|
public class ApiPaths {
|
|
public static final String BASE = "/api/v1";
|
|
public static final String ORDERS = "/api/v1/orders";
|
|
}`;
|
|
|
|
describe('Java constant-valued routes: group ↔ ingestion parity (#2980)', () => {
|
|
it('resolves a constant-valued mapping to the same path on both sides', () => {
|
|
const files = {
|
|
[CONSTS]: CONSTS_SRC,
|
|
[CTL]: `package com.example;
|
|
import com.example.ApiPaths;
|
|
public class OrderController {
|
|
@GetMapping(ApiPaths.ORDERS)
|
|
public void list() {}
|
|
}`,
|
|
};
|
|
expect(groupProviders(files)).toEqual(['GET /api/v1/orders']);
|
|
expect(ingestionRoutes(files)).toEqual(groupProviders(files));
|
|
});
|
|
|
|
it('suppresses the method route under a CONSTANT class prefix on both sides', () => {
|
|
// The class prefix needs the repo-wide constant map, which does not exist
|
|
// at extraction time on either side. Emitting the method route would drop
|
|
// the prefix and publish `GET /api/v1/orders`-without-its-base — a path the
|
|
// application never serves. On base such a route was not emitted at all, so
|
|
// shipping it unprefixed would turn a missing fact into a wrong one.
|
|
const files = {
|
|
[CONSTS]: CONSTS_SRC,
|
|
[CTL]: `package com.example;
|
|
import com.example.ApiPaths;
|
|
@RequestMapping(ApiPaths.BASE)
|
|
public class OrderController {
|
|
@GetMapping(ApiPaths.ORDERS)
|
|
public void list() {}
|
|
|
|
@GetMapping("/literal")
|
|
public void literal() {}
|
|
}`,
|
|
};
|
|
expect(groupProviders(files)).toEqual([]);
|
|
expect(ingestionRoutes(files)).toEqual([]);
|
|
});
|
|
|
|
it('still applies a LITERAL class prefix', () => {
|
|
const files = {
|
|
[CONSTS]: CONSTS_SRC,
|
|
[CTL]: `package com.example;
|
|
import com.example.ApiPaths;
|
|
@RequestMapping("/api/v1")
|
|
public class OrderController {
|
|
@GetMapping("/orders")
|
|
public void list() {}
|
|
}`,
|
|
};
|
|
expect(groupProviders(files)).toEqual(['GET /api/v1/orders']);
|
|
expect(ingestionRoutes(files)).toEqual(['GET /api/v1/orders']);
|
|
});
|
|
|
|
it('emits nothing for a constant route when scanned without a repo context', () => {
|
|
// This is the branch that makes the 1-argument parity guards blind to the
|
|
// whole feature; pin it so it is not silently dead in the suite.
|
|
const src = `package com.example;
|
|
import com.example.ApiPaths;
|
|
public class OrderController {
|
|
@GetMapping(ApiPaths.ORDERS)
|
|
public void list() {}
|
|
}`;
|
|
const detections = JAVA_HTTP_PLUGIN.scan(parse(src));
|
|
expect(detections.filter((d) => d.role === 'provider')).toEqual([]);
|
|
});
|
|
|
|
it('suppresses a NO-ARGUMENT mapping under a constant class prefix on both sides', () => {
|
|
// A bare `@GetMapping` IS the class prefix, so an unfoldable class prefix
|
|
// leaves nothing to emit. Ingestion routes these through a separate loop
|
|
// from the path-carrying ones, and that loop needs the same guard — without
|
|
// it ingestion emitted an empty-path Route where the group emitted nothing.
|
|
const files = {
|
|
[CONSTS]: CONSTS_SRC,
|
|
[CTL]: `package com.example;
|
|
import com.example.ApiPaths;
|
|
@RequestMapping(ApiPaths.BASE)
|
|
public class OrderController {
|
|
@GetMapping public void list() {}
|
|
@PostMapping public void create() {}
|
|
}`,
|
|
};
|
|
expect(ingestionRoutes(files)).toEqual([]);
|
|
expect(groupProviders(files)).toEqual([]);
|
|
});
|
|
|
|
it('measures import ambiguity over the same candidate set on both sides', () => {
|
|
// Ingestion's harvest gate also admits import-only files, so its repo map is
|
|
// a superset of the group's. When ambiguity was measured over every key, a
|
|
// duplicate FQN belonging to a class that defines NOTHING was invisible to
|
|
// the group and made ingestion alone floor to skip — reopening the very
|
|
// parity break this feature exists to close. Both sides now measure over
|
|
// constant-DEFINING files only.
|
|
const files = {
|
|
'svc-a/src/main/java/com/x/ApiPaths.java': `package com.x;
|
|
public class ApiPaths { public static final String ORDERS = "/api/v1/orders"; }`,
|
|
// Same FQN, different module, defines no constant — must not create ambiguity.
|
|
'svc-b/src/main/java/com/x/ApiPaths.java': `package com.x;
|
|
import java.util.List;
|
|
public class ApiPaths {}`,
|
|
'svc-a/src/main/java/com/x/web/OrderController.java': `package com.x.web;
|
|
import com.x.ApiPaths;
|
|
public class OrderController {
|
|
@GetMapping(ApiPaths.ORDERS)
|
|
public void list() {}
|
|
}`,
|
|
};
|
|
// Guard the premise: the two maps really are different sizes.
|
|
const ingestionKeys = Object.entries(files).filter(([, src]) =>
|
|
javaProvider.moduleConstantHeuristic?.(src),
|
|
).length;
|
|
expect(ingestionKeys).toBe(3);
|
|
expect(groupProviders(files)).toEqual(['GET /api/v1/orders']);
|
|
expect(ingestionRoutes(files)).toEqual(['GET /api/v1/orders']);
|
|
});
|
|
|
|
it('leaves literal routes unchanged with no constant map at all', () => {
|
|
const files = {
|
|
[CTL]: `package com.example;
|
|
public class OrderController {
|
|
@PostMapping("/api/v1/orders")
|
|
public void create() {}
|
|
}`,
|
|
};
|
|
expect(groupProviders(files)).toEqual(['POST /api/v1/orders']);
|
|
expect(ingestionRoutes(files)).toEqual(['POST /api/v1/orders']);
|
|
});
|
|
});
|