URISyntaxException in case of some undefined properties in POM #3700
Comments
|
Thanks for the detailed report and offer to help! I don't think we have encountered such cases before, but I could just not have seen them. To be clear: was this only an issue when the property is missing (or in a parent pom.xml)? Or also when the property is defined in the same file? In any case the way we usually start to resolve an issue is by replicating it with a test, which you can push up to a draft PR, and then from there see what we can improve. Any help there would be appreciated; with #3701 you already made a great start to contributing. :) |
|
First, to your questions. I extended the description of the POM above, so we can see more from the parent POM. The Maven logic behind this is probably: we define the property, and it is in undefined state. So in such case the usual JUnit 4 artifacts will be used. Because for Maven XML parser, undefined by property resolution is equals to empty. But in the child project, one could define some value for the property and use the artifacts as junit${junit.dep} artifact with expected path. The main issue here is that you use a different parser, and it pareses this differently. In the next line, we use a property for version, and it is working. Because the property has a value. But in case the property value is undefined, it doesn't work in your parser. But I have an appropriate error message and can correct the error. So, my general question here , before we write tests and fixes: Can we / Should we fix this? Is it possible to change the XML parser behavior in such case to be more Maven-like? Or is this not possible and not desired, and we should just see an exception and correct an error in POM (we just put <junit.dep></junit.dep>)? |
|
I continue here, because other ticket #3699 and PR are already closed. First, thank you for additional changes and release. I've tested the troublesome constellation and this time a have an exception with the message, that is direct diff of the place which causes the troubles. So now I don't need to debug and can simply take a look at the file and find the line with this. A big improvement. Back to the discussion. So what caused the troubles in this CCE, was an HTML greater sign ( What is already done: we have a proper error messaging here, so that end user can quickly find the source of the troubles and correct it. What can be done? We can change the Maven parser (not the general XML one) to behave the Maven way and ignore such things. Or we can go further, because we're already changing some parts of the POM and change these places also. So we would have more strict POM at the end. The question remains - should one of this thing be done? |
|
An XML entity (such as |
|
I thought once again about this, and you are right. Take a look at the example in the linked ticket. I think that it's unusual for POM to have the values at this place (configuration) directly and not in some sub-tag. But for XML it should be valid. |
|
Thinking about it once again: maybe original error is also an error in the parser; so maybe this should be |
|
I verified this happens if a dependency has a version property in its dependency management. For example, projectA -> dependencyA -> spring-framework-bom where dependencyA declares version property spring-framework.version The workaround is to pass the version property at the command line, that is, -Dspring-framework.version=value and in the OP example -Djunit.dep=4.13.2 |
What version of OpenRewrite are you using?
I am using
How are you running OpenRewrite?
I am using the Maven plugin, calling from the shell.
What is the smallest, simplest way to reproduce the problem?
We have parent POM with something like this:
if the first property isn't defined, there is an error here, see stack trace below.
What did you expect to see?
I expect Rewrite to work and not to break with URISyntaxException.
What did you see instead?
Instead, I see an exception; see the stack trace below.
What is the full stack trace of any errors you encountered?
Are you interested in contributing a fix to OpenRewrite?
The POM is malformed at the end, but maybe not. The issue here is that in such case the Maven XML parser decides that the property is effectively empty and goes this way in case that property is undefined. But your parser can't resolve this and tries to find a file with unresolved property in the path. And fails, of course.
The question here is: could it be handled the Maven way? Or at least handle gracefully and without error? We corrected our POM of course, but some improvement here would be nice.
I would like to contribute here, but I don't know how.
The text was updated successfully, but these errors were encountered: