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).
This commit is contained in:
ChunxueLi 2026-08-16 09:00:22 +08:00
parent 41d874dcb3
commit a4fc7a05f8
4 changed files with 55 additions and 12 deletions

View file

@ -759,6 +759,12 @@ function collectSpringTypes(filePath: string, tree: Parser.Tree): SharedSpringTy
const { prefixByTypeId, methodRoutes } = scanRouteAnnotations(tree);
const routesByMethodId = new Map<number, Array<{ method: string; path: string }>>();
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);

View file

@ -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 {

View file

@ -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;
}

View file

@ -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);
});
});