Skip to content

Conversation

@gnodet
Copy link
Contributor

@gnodet gnodet commented Nov 6, 2025

Problem

The location for properties (Map elements) in the MavenStaxReader was being captured after calling nextText(), which moves the parser position past the element. This resulted in incorrect location information being reported.

For example, when parsing:

<properties>
  <maven.compiler.source>17</maven.compiler.source>
</properties>

The location for maven.compiler.source would point to the position after the closing tag instead of at the opening tag.

Solution

This PR fixes the timing of location capture for properties by saving the line and column numbers before calling nextText() that moves the parser position.

Changes

Modified Files

  1. src/mdo/reader-stax.vm

    • Fixed location capture timing for properties (Map elements)
    • Location is now captured before nextText() is called
  2. impl/maven-support/src/test/java/org/apache/maven/model/v4/MavenStaxReaderTest.java

    • Added testLocationReportingForElements() - tests regular elements with specific line and column number assertions
    • Added testLocationReportingForAttributes() - tests XML attributes (root on project, child.scm.connection.inherit.append.path on scm)
    • Added testLocationReportingForListElements() - tests list elements (modules) with specific line and column number assertions

Testing

All tests pass successfully:

  • ✅ 10 tests in MavenStaxReaderTest (including 3 new comprehensive location reporting tests)
  • ✅ Location reporting now works correctly for all element types
  • ✅ Tests verify exact line and column numbers for accurate location tracking
  • ✅ Existing ProjectBuilderTest continues to pass

Benefits

  • More accurate location information for error reporting
  • Better debugging experience when working with Maven POMs
  • Consistent location reporting across all element types

@gnodet gnodet force-pushed the fix-stax-reader-location-reporting branch from f9e647b to 115b7c4 Compare November 6, 2025 10:51
@gnodet gnodet changed the title Fix MavenStaxReader location reporting to point to start of XML tags Fix MavenStaxReader location reporting for properties Nov 6, 2025
@gnodet gnodet force-pushed the fix-stax-reader-location-reporting branch from 115b7c4 to e6f99ee Compare November 6, 2025 12:52
The location for properties (Map elements) was being captured AFTER calling
nextText(), which moves the parser position past the element. This resulted
in incorrect location information.

This commit fixes the timing of location capture for properties by saving
the line and column numbers BEFORE calling nextText().

Changes:
- Modified src/mdo/reader-stax.vm to capture location before nextText() for properties
- Added comprehensive unit tests for location reporting:
  * testLocationReportingForElements() - tests regular elements with exact line/column numbers
  * testLocationReportingForAttributes() - tests XML attributes (root, child.scm.connection.inherit.append.path)
    Note: Attributes get the location of their containing element since XMLStreamReader doesn't
    provide individual attribute positions
  * testLocationReportingForListElements() - tests list elements (modules) with exact line/column numbers

All tests pass successfully.
@gnodet gnodet force-pushed the fix-stax-reader-location-reporting branch from e6f99ee to 53575fa Compare November 6, 2025 13:18
@gnodet gnodet added bug Something isn't working backport-to-4.0.x labels Nov 6, 2025
@gnodet gnodet requested a review from cstamas November 6, 2025 14:35
@gnodet gnodet merged commit 6f5c837 into apache:master Nov 6, 2025
41 of 42 checks passed
@github-actions github-actions bot added this to the 4.1.0 milestone Nov 6, 2025
gnodet added a commit to gnodet/maven that referenced this pull request Nov 6, 2025
The location for properties (Map elements) was being captured AFTER calling
nextText(), which moves the parser position past the element. This resulted
in incorrect location information.

This commit fixes the timing of location capture for properties by saving
the line and column numbers BEFORE calling nextText().

Changes:
- Modified src/mdo/reader-stax.vm to capture location before nextText() for properties
- Added comprehensive unit tests for location reporting:
  * testLocationReportingForElements() - tests regular elements with exact line/column numbers
  * testLocationReportingForAttributes() - tests XML attributes (root, child.scm.connection.inherit.append.path)
    Note: Attributes get the location of their containing element since XMLStreamReader doesn't
    provide individual attribute positions
  * testLocationReportingForListElements() - tests list elements (modules) with exact line/column numbers

(cherry picked from commit 6f5c837)

# Conflicts:
#	src/mdo/reader-stax.vm
@gnodet
Copy link
Contributor Author

gnodet commented Nov 6, 2025

💚 All backports created successfully

Status Branch Result
maven-4.0.x

Questions ?

Please refer to the Backport tool documentation

gnodet added a commit that referenced this pull request Nov 6, 2025
The location for properties (Map elements) was being captured AFTER calling
nextText(), which moves the parser position past the element. This resulted
in incorrect location information.

This commit fixes the timing of location capture for properties by saving
the line and column numbers BEFORE calling nextText().

Changes:
- Modified src/mdo/reader-stax.vm to capture location before nextText() for properties
- Added comprehensive unit tests for location reporting:
  * testLocationReportingForElements() - tests regular elements with exact line/column numbers
  * testLocationReportingForAttributes() - tests XML attributes (root, child.scm.connection.inherit.append.path)
    Note: Attributes get the location of their containing element since XMLStreamReader doesn't
    provide individual attribute positions
  * testLocationReportingForListElements() - tests list elements (modules) with exact line/column numbers

(cherry picked from commit 6f5c837)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-4.0.x bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants