openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.1k stars 313 forks source link

URISyntaxException in case of some undefined properties in POM #3700

Open api-from-the-ion opened 10 months ago

api-from-the-ion commented 10 months ago

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.

mvn -U org.openrewrite.maven:rewrite-maven-plugin:5.11.0:run \
  -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:RELEASE \
  -Drewrite.activeRecipes=org.openrewrite.java.testing.junit5.JUnit4to5Migration    ... 

What is the smallest, simplest way to reproduce the problem?

We have parent POM with something like this:

 <dependency>
  <groupId>junit</groupId>
  <artifactId>junit${junit.dep}</artifactId>
  <version>${junit.version}</version>
 </dependency>

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?

[ERROR] Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:5.11.0:run (default-cli) on project orderqueryservice-integrationtest: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:5.11.0:run failed: Illegal character in path at index 46: file:///home/anton/.m2/repository/junit/junit${junit.dep}/4.12/junit${junit.dep}-4.12.pom -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:5.11.0:run (default-cli) on project orderqueryservice-integrationtest: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:5.11.0:run failed: Illegal character in path at index 46: file:///home/anton/.m2/repository/junit/junit${junit.dep}/4.12/junit${junit.dep}-4.12.pom
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:333)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
    at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:498)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:283)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:226)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:407)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:348)
Caused by: org.apache.maven.plugin.PluginExecutionException: Execution default-cli of goal org.openrewrite.maven:rewrite-maven-plugin:5.11.0:run failed: Illegal character in path at index 46: file:///home/anton/.m2/repository/junit/junit${junit.dep}/4.12/junit${junit.dep}-4.12.pom
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:133)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
    at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:498)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:283)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:226)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:407)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:348)
Caused by: java.lang.IllegalArgumentException: Illegal character in path at index 46: file:///home/anton/.m2/repository/junit/junit${junit.dep}/4.12/junit${junit.dep}-4.12.pom
    at java.net.URI.create (URI.java:852)
    at org.openrewrite.maven.internal.MavenPomDownloader.download (MavenPomDownloader.java:513)
    at org.openrewrite.maven.tree.ResolvedPom.resolveDependencies (ResolvedPom.java:651)
    at org.openrewrite.maven.tree.ResolvedPom.resolveDependencies (ResolvedPom.java:574)
    at org.openrewrite.maven.tree.MavenResolutionResult.resolveDependencies (MavenResolutionResult.java:174)
    at org.openrewrite.maven.MavenParser.parseInputs (MavenParser.java:107)
    at org.openrewrite.Parser.parse (Parser.java:59)
    at org.openrewrite.maven.MavenMojoProjectParser.parseMaven (MavenMojoProjectParser.java:478)
    at org.openrewrite.maven.MavenMojoProjectParser.listSourceFiles (MavenMojoProjectParser.java:150)
    at org.openrewrite.maven.AbstractRewriteMojo.loadSourceSet (AbstractRewriteMojo.java:258)
    at org.openrewrite.maven.AbstractRewriteMojo.listResults (AbstractRewriteMojo.java:240)
    at org.openrewrite.maven.AbstractRewriteRunMojo.execute (AbstractRewriteRunMojo.java:53)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
    at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:498)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:283)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:226)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:407)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:348)
Caused by: java.net.URISyntaxException: Illegal character in path at index 46: file:///home/anton/.m2/repository/junit/junit${junit.dep}/4.12/junit${junit.dep}-4.12.pom
    at java.net.URI$Parser.fail (URI.java:2847)
    at java.net.URI$Parser.checkChars (URI.java:3020)
    at java.net.URI$Parser.parseHierarchical (URI.java:3104)
    at java.net.URI$Parser.parse (URI.java:3052)
    at java.net.URI.<init> (URI.java:588)
    at java.net.URI.create (URI.java:850)
    at org.openrewrite.maven.internal.MavenPomDownloader.download (MavenPomDownloader.java:513)
    at org.openrewrite.maven.tree.ResolvedPom.resolveDependencies (ResolvedPom.java:651)
    at org.openrewrite.maven.tree.ResolvedPom.resolveDependencies (ResolvedPom.java:574)
    at org.openrewrite.maven.tree.MavenResolutionResult.resolveDependencies (MavenResolutionResult.java:174)
    at org.openrewrite.maven.MavenParser.parseInputs (MavenParser.java:107)
    at org.openrewrite.Parser.parse (Parser.java:59)
    at org.openrewrite.maven.MavenMojoProjectParser.parseMaven (MavenMojoProjectParser.java:478)
    at org.openrewrite.maven.MavenMojoProjectParser.listSourceFiles (MavenMojoProjectParser.java:150)
    at org.openrewrite.maven.AbstractRewriteMojo.loadSourceSet (AbstractRewriteMojo.java:258)
    at org.openrewrite.maven.AbstractRewriteMojo.listResults (AbstractRewriteMojo.java:240)
    at org.openrewrite.maven.AbstractRewriteRunMojo.execute (AbstractRewriteRunMojo.java:53)
    at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:126)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:328)
    at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:316)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:212)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:174)
    at org.apache.maven.lifecycle.internal.MojoExecutor.access$000 (MojoExecutor.java:75)
    at org.apache.maven.lifecycle.internal.MojoExecutor$1.run (MojoExecutor.java:162)
    at org.apache.maven.plugin.DefaultMojosExecutionStrategy.execute (DefaultMojosExecutionStrategy.java:39)
    at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:159)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:105)
    at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:73)
    at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:53)
    at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:118)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:261)
    at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:173)
    at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:101)
    at org.apache.maven.cli.MavenCli.execute (MavenCli.java:906)
    at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:283)
    at org.apache.maven.cli.MavenCli.main (MavenCli.java:206)
    at sun.reflect.NativeMethodAccessorImpl.invoke0 (Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke (NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke (DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke (Method.java:498)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:283)
    at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:226)
    at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:407)
    at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:348)

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.

timtebeek commented 10 months ago

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. :)

api-from-the-ion commented 10 months ago

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>)? I would like to run the Rewrite on the projects as it is, without changing even obvious errors if Maven has no troubles with such POM. But only if this is possible.

api-from-the-ion commented 9 months ago

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 (&gt;) in the POM file in between the tags. This is of course not a valid XML, but Maven has no problems with things like this. Or as I described already in this ticket - undefined property is empty for Maven but not for the standard XML parser.

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?

knutwannheden commented 9 months ago

An XML entity (such as &gt;) in the content of an XML tag is allowed. But our parser indeed has an issue with that, which we need to fix.

api-from-the-ion commented 9 months ago

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.

api-from-the-ion commented 9 months ago

Thinking about it once again: maybe original error is also an error in the parser; so maybe this should be == </junit.dep>

ocaro commented 2 weeks ago

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