openrewrite
fix(maven): POM file path in deserialization exception message
- Fixes #5558.
It is probably a very narrow corner case. Reproduced with a POM file with an incorrect XML syntax:
<!-- EMPTY LINE -->
<?xml version="1.0" encoding="UTF-8"?>
<project>
<modelVersion>4.0.0</modelVersion>
Notice the <!-- EMPTY LINE --> before <?xml opening tag. In the original reproducer furnished with the issue linked it is really an empty line not a XML comment.
Seems Maven parsers are very confortable with this but not Jackson Databind XML.
Here we add an argument to RawPom.parse with the Path to the pom file being parsed. In case of exception the path is added to the exception message.
Tests adapted and 1 additional test to ensure the POM path is in the exception message.
What's changed?
rewrite-maven and RawPom class
What's your motivation?
Enhance user experience by telling which POM could not be deserialized by Jackson.
Have you considered any alternatives or workarounds?
The failing run of the rewrite-maven-plugin, as communicated in the linked issue, does not tell which is the POM file that failed in its deserialization, even with --debug flag.
Checklist
- [x] I've added unit tests to cover both positive and negative cases
- [x] I've read and applied the recipe conventions and best practices
- [ ] I've used the IntelliJ IDEA auto-formatter on affected files --> Sorry I could not find where the configuration of the formatter to use.
Thanks for the help here @edeweerd1A ! Just for my own understanding: did the recently added diagnostics not already help pinpoint the cause? Just to know if paths are missed there for other unexpected failures.
Also downstream from this change we'd need to update the usage in rewrite-maven-plugin as well. Wondering if we should do that backwards compatible or not; checking potential impact here.
Thanks for the help here @edeweerd1A ! Just for my own understanding: did the recently added diagnostics not already help pinpoint the cause? Just to know if paths are missed there for other unexpected failures.
Also downstream from this change we'd need to update the usage in rewrite-maven-plugin as well. Wondering if we should do that backwards compatible or not; checking potential impact here.
Are you referring at running the following recipe DependencyResolutionDiagnostic ? because the failure is happening at POM resolution time, so no single recipe will be applied.
About doing the reporting in the maven plugin I let you decide on this.
Added some more details here now that I've found some time for a closer look:
- https://github.com/openrewrite/rewrite/issues/5558#issuecomment-2971150138
Would you mind chiming in there (for SEO) on whether that alternative approach is helpful?