Preserve coordinates when merging InputLocation

[ulofiai]

Fixes #12610.

Update the InputLocation MDO template so generated Maven API merge results retain line and column information. The boolean overload uses the dominant input location and falls back to the other input when its coordinates are unknown; indexed list merges prefer the target and use the source as fallback.

Add regression coverage for both merge overloads.

[gnodet]

Verdict: APPROVE ✅

Clean, focused fix for #12610. The merge logic correctly preserves line/column coordinates from the dominant location with a sensible fallback, replacing the old hardcoded -1,-1.

Key observations:

• The change is well-scoped: it only affects the $isMavenModel == true template branch, leaving the compat model (which already preserves target coordinates) untouched.
• The fallback condition (lineNumber < 0 && columnNumber < 0) sensibly requires both coordinates to be unknown before falling back — partial coordinates are still useful.
• Test coverage is adequate for the core scenarios (dominant selection, fallback on unknown coordinates) across both the boolean and list merge overloads.
• Correctly modifies only the Velocity template (src/mdo/java/InputLocation.java), not generated output files.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[gnodet]

APPROVE — Clean, focused fix that correctly preserves line/column coordinates during InputLocation merge operations, replacing the prior behavior of always discarding them to -1/-1. The logic, scoping, and test coverage are all sound.

Minor observations (non-blocking):

1. Coordinate/source pairing (low): The merged InputLocation uses coordinates from the dominant (or fallback) input, while InputSource is the merged result of both sources. This means coordinates could reference file A while InputSource represents {A, B}. This is a pre-existing design aspect, and preserving approximate coordinates is strictly better than the old -1/-1.

2. Variable name reuse (low): In the list merge method, the new variable InputLocation location = target at method scope shadows the for-loop's block-scoped InputLocation location. Valid Java 17, but briefly confusing when reading the generated output.

3. Edge case coverage (low): Tests cover the core scenarios well (dominant selection, fallback on unknown coordinates for both merge overloads), but don't cover the case where both target and source have -1/-1 coordinates. That's a trivial passthrough producing -1/-1 (same as old behavior), so acceptable.

The fallback condition getLineNumber() < 0 && getColumnNumber() < 0 (using AND, not OR) is a reasonable design choice: partial coordinates are still useful for error reporting, so fallback only triggers when we have no useful coordinate info at all.

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet