Skip to content

Fix mvnup comment formatting for override comments#12443

Open
gnodet wants to merge 2 commits into
apache:masterfrom
gnodet:fix/23134-mvnup-comment-formatting
Open

Fix mvnup comment formatting for override comments#12443
gnodet wants to merge 2 commits into
apache:masterfrom
gnodet:fix/23134-mvnup-comment-formatting

Conversation

@gnodet

@gnodet gnodet commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix PluginUpgradeStrategy to set precedingWhitespace on Comment nodes inserted before plugin elements, so the "Override version inherited from parent" comment appears on its own line instead of being appended to the preceding </plugin> closing tag
  • Add test assertions to verify comments are not on the same line as closing tags

Problem

When mvnup adds plugin management entries for plugins inherited from a remote parent, it inserts a <!-- Override version inherited from parent --> comment before the <plugin> element. The Comment.of(...) factory creates a comment with no precedingWhitespace, causing it to render on the same line as the previous </plugin> closing tag:

            </plugin><!-- Override version inherited from parent -->
            <plugin>

This breaks projects that enforce POM formatting (e.g. via spotless-maven-plugin), which expect:

            </plugin>
            <!-- Override version inherited from parent -->
            <plugin>

Fix

Set precedingWhitespace on the Comment node to match the plugin element's indentation at both insertion sites in PluginUpgradeStrategy:

  • addPluginManagementEntryFromUpgrade() — for build/pluginManagement/plugins
  • addDirectPluginOverrides() — for build/plugins

Fixes gnodet/maven4-testing#23134

Test plan

  • Existing PluginUpgradeStrategyTest passes (31 tests)
  • New assertions verify comments are on their own line (assertFalse(xml.contains("</plugin><!--"), ...))
  • Full maven-cli module verify passes (468 tests)

🤖 Generated with Claude Code

When mvnup inserts "Override version inherited from parent" comments
before plugin elements, the comment was placed on the same line as
the preceding </plugin> closing tag, causing POM formatting violations
in projects that enforce XML formatting (e.g. via spotless).

The fix sets precedingWhitespace on the Comment node to match the
plugin element's indentation, ensuring the comment appears on its
own line.

Fixes gnodet/maven4-testing#23134

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet

gnodet commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Note: This PR is a workaround for a missing API in DomTrip. The Editor class handles whitespace automatically for element insertion (insertElementBefore/After) and comment appending (addComment), but has no positional comment insertion method (insertCommentBefore/After). This forced mvnup to use the raw ContainerNode.insertChildBefore API which doesn't handle precedingWhitespace.

Filed upstream: maveniverse/domtrip#254

Once DomTrip adds Editor.insertCommentBefore(), the manual precedingWhitespace() calls here should be replaced with the Editor API.

Replace the manual precedingWhitespace() workaround with the new
Editor.insertCommentBefore() method added in DomTrip 1.6.0
(maveniverse/domtrip#254), which handles whitespace automatically.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gnodet

gnodet commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Updated: upgraded DomTrip to 1.6.0 which includes the new Editor.insertCommentBefore() / insertCommentAfter() API (maveniverse/domtrip#254). The manual precedingWhitespace() workaround has been replaced with the proper Editor API call.

@gnodet gnodet requested a review from cstamas July 8, 2026 17:43
@gnodet gnodet added this to the 4.0.0-rc-6 milestone Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maven 4 Test Results: sling-org-apache-sling-i18n (pull/12430/head (built with 4.0.0-rc-5))

1 participant