From a4fc7a05f86c77366eb8fa1c7e4fb030e1e09c69 Mon Sep 17 00:00:00 2001 From: ChunxueLi Date: Sun, 16 Aug 2026 09:00:22 +0800 Subject: [PATCH] fix(review): address bot review findings on PR #2980 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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). --- .../group/extractors/http-patterns/java.ts | 6 +++ .../route-extractors/java-const-resolver.ts | 4 ++ .../core/ingestion/route-extractors/spring.ts | 12 +++-- .../unit/java-route-const-resolver.test.ts | 45 +++++++++++++++---- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/gitnexus/src/core/group/extractors/http-patterns/java.ts b/gitnexus/src/core/group/extractors/http-patterns/java.ts index 2e62d77b8..a7030494e 100644 --- a/gitnexus/src/core/group/extractors/http-patterns/java.ts +++ b/gitnexus/src/core/group/extractors/http-patterns/java.ts @@ -759,6 +759,12 @@ function collectSpringTypes(filePath: string, tree: Parser.Tree): SharedSpringTy const { prefixByTypeId, methodRoutes } = scanRouteAnnotations(tree); const routesByMethodId = new Map>(); for (const route of methodRoutes) { + // A constant-referencing route still carries `rawPath: ''` here — folding + // happens in scan() against the repo constant map, which this + // inheritance-view collector has no access to. Emitting it as an empty + // path would publish `POST /`-shaped noise into the shared type view; + // skip instead (ingestion keeps the same skip floor — R4 parity). + if (route.pathOperands) continue; const routes = routesByMethodId.get(route.methodNode.id) ?? []; routes.push({ method: route.httpMethod, path: route.rawPath }); routesByMethodId.set(route.methodNode.id, routes); diff --git a/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts b/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts index 033326fb8..370ca93cf 100644 --- a/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts +++ b/gitnexus/src/core/ingestion/route-extractors/java-const-resolver.ts @@ -264,6 +264,10 @@ export function extractJavaModuleConstants(tree: Parser.Tree): ModuleConstants { 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); } else { diff --git a/gitnexus/src/core/ingestion/route-extractors/spring.ts b/gitnexus/src/core/ingestion/route-extractors/spring.ts index 5161acf79..48933ef3f 100644 --- a/gitnexus/src/core/ingestion/route-extractors/spring.ts +++ b/gitnexus/src/core/ingestion/route-extractors/spring.ts @@ -185,7 +185,11 @@ export function extractSpringRoutes( const node = caps['node']; const valueNode = caps['value']; const keyNode = caps['key']; - if (!annNode || !node || !valueNode) continue; + // A constant-referencing value arrives as @value_expr, not @value — the + // match carries exactly one of the two. Require @value only when no + // @value_expr is present; the operand branch below folds the expression. + const valueExprCapture = match.captures.find((c) => c.name === 'value_expr')?.node ?? null; + if (!annNode || !node || (!valueNode && !valueExprCapture)) continue; if (node.type !== 'method_declaration') continue; @@ -203,8 +207,8 @@ export function extractSpringRoutes( // #2391-style non-literal path (constant ref or `+`-concat): emit with // operands for cross-file folding in the parse phase. The match carries // either @value (literal) or @value_expr (non-literal) — never both. - const valueExprNode = match.captures.find((c) => c.name === 'value_expr')?.node ?? null; - const routePath = unquoteSpringLiteral(valueNode.text); + const valueExprNode = valueExprCapture; + const routePath = valueNode ? unquoteSpringLiteral(valueNode.text) : null; if (routePath === null && !valueExprNode) continue; const enclosingType = findEnclosingType(node); @@ -229,7 +233,7 @@ export function extractSpringRoutes( // scan — safe under routeCoverage:'partial'. Full class-array cross-product // support is tracked in #2280. (Scalar method paths under an array class // prefix are left unchanged: that pre-existing divergence is out of scope.) - const isArrayElement = valueNode.parent?.type === 'element_value_array_initializer'; + const isArrayElement = valueNode?.parent?.type === 'element_value_array_initializer'; if (isArrayElement && enclosingClass && classesWithArrayPrefix.has(enclosingClass.id)) { continue; } diff --git a/gitnexus/test/unit/java-route-const-resolver.test.ts b/gitnexus/test/unit/java-route-const-resolver.test.ts index e4bda047e..8b4ae0ce0 100644 --- a/gitnexus/test/unit/java-route-const-resolver.test.ts +++ b/gitnexus/test/unit/java-route-const-resolver.test.ts @@ -105,14 +105,6 @@ public interface LabApiPath { String LAB_QUERY_V1 = "/api/v1/labtest/query"; }`; -const FQN_CONTROLLER = `package com.winning.opt.other; - -public class FqnController { - - @WinPostMapping(com.winning.opt.diagnosis.api.constants.ApiPathConstants.DIAGNOSIS_SAVE_V1) - public String save() { return "{}"; } -}`; - const WIN_POST_MAPPING = `package com.winning.opt.annotations; public @interface WinPostMapping { @@ -305,3 +297,40 @@ describe('parseJavaConstOperands', () => { expect(parseJavaConstOperands(valueNode)).toBeNull(); }); }); + +// ── Ingestion extractor level: constant-referencing annotation values ── +// (regression for the review finding where the route loop's `!valueNode` +// guard dropped every @value_expr match before the operand branch ran) +describe('extractSpringRoutes constant value', () => { + it('emits routePathExpr + operands for @Mapping(CONSTS.X)', async () => { + const { extractSpringRoutes } = + await import('../../src/core/ingestion/route-extractors/spring.js'); + const tree = parser.parse(` +package com.winning.opt.demo; +public class DemoController { + @org.springframework.web.bind.annotation.PostMapping(ApiPathConstants.DIAGNOSIS_SAVE_V1) + public String save() { return "ok"; } +}`); + const routes = extractSpringRoutes(tree, 'DemoController.java', 0); + assert.strictEqual(routes.length, 1); + assert.strictEqual(routes[0].httpMethod, 'POST'); + assert.strictEqual(routes[0].routePathExpr, 'ApiPathConstants.DIAGNOSIS_SAVE_V1'); + assert.ok(routes[0].routePathOperands && routes[0].routePathOperands.length > 0); + assert.strictEqual(routes[0].routePath, ''); + }); + + it('keeps literal routes unchanged', async () => { + const { extractSpringRoutes } = + await import('../../src/core/ingestion/route-extractors/spring.js'); + const tree = parser.parse(` +package com.winning.opt.demo; +public class DemoController { + @org.springframework.web.bind.annotation.PostMapping("/literal/path") + public String save() { return "ok"; } +}`); + const routes = extractSpringRoutes(tree, 'DemoController.java', 0); + assert.strictEqual(routes.length, 1); + assert.strictEqual(routes[0].routePath, '/literal/path'); + assert.strictEqual(routes[0].routePathExpr, undefined); + }); +});