rewrite-migrate-java

rewrite-migrate-java copied to clipboard

What version of OpenRewrite are you using?

• OpenRewrite v8.24.0
• rewrite-migrate-java v2.12.0

How are you running OpenRewrite?

I'm running org.openrewrite.java.migrate.net.URLConstructorsToURI.URLThreeArgumentConstructorRecipe, via: https://github.com/openrewrite/rewrite-migrate-java/blob/f399d72046b68fd0820a6b326badfd5ff9482105/src/main/java/org/openrewrite/java/migrate/net/URLConstructorsToURI.java#L44-L58

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

class A {
    URL urlConstructor(String protocol, String host, String file) throws IOException {
        return new URL(protocol, host, file);
    }
}

What did you expect to see?

class A {
    URL urlConstructor(String protocol, String host, String file) throws URISyntaxException, IOException {
            return new URI(protocol, null, host, -1, file, null, null).toURL();
        }
}

What did you see instead?

class A {
    URL urlConstructor(String protocol, String host, String file) throws IOException {
            return new URI(protocol, null, host, -1, file, null, null).toURL();
        }
}

Note the lacking throws URISyntaxException.

What is the full stack trace of any errors you encountered?

Compiler error, as URISyntaxException does not extend IOException, in contrast to MalformedURLException thrown before. As reported by @IanDarwin

This recipe is implemented as a Refaster recipe, and as such it's well possible the fix should land in https://github.com/openrewrite/rewrite-templating to introduce the new exception there from what's thrown in the after template method. /cc @knutwannheden

Hey @timtebeek I have a suggestion on this issue It seems like in your URLThreeArgumentConstructor class, you are converting new URL(String, String, String) constructors to new URI(...).toURL() without adding the throws URISyntaxException declaration in the AfterTemplate method. To fix this issue, you should include throws URISyntaxException in your AfterTemplate method like this:

public static class URLThreeArgumentConstructor {
        @BeforeTemplate
        URL urlConstructor(String protocol, String host, String file) throws Exception {
            return new URL(protocol, host, file);
        }

        @AfterTemplate
        URL newUriToUrl(String protocol, String host, String file) throws URISyntaxException {
            return new URI(protocol, null, host, -1, file, null, null).toURL();
        }
    }

This ensures that the throws URISyntaxException is correctly propagated in the converted code, addressing the issue you encountered.

What are your thoughts @timtebeek over this ?

Hi! Thanks for the suggestion; the way that our rewrite-templating converts Refaster templates to OpenRewrite recipes does not yet include adding a throws declaration where necessary. It might be more complicated to add that there, but would indeed be helpful here.

Note that our implementation of Refaster is different from ErrorProne itself, if you're familiar with how ErrorProne works.

Similar issue here

 package org.jabref.logic.importer.fetcher;
 
 import java.io.IOException;
+import java.net.URI;
 import java.net.URL;
 import java.util.Objects;
 import java.util.Optional;
 
 import org.jabref.logic.importer.FulltextFetcher;
@@ -60,11 +61,11 @@ public class SpringerLink implements FulltextFetcher, CustomizableKeyFetcher {
                 JSONObject json = jsonResponse.getBody().getObject();
                 int results = json.getJSONArray("result").getJSONObject(0).getInt("total");
 
                 if (results > 0) {
                     LOGGER.info("Fulltext PDF found @ Springer.");
-                    return Optional.of(new URL("http", CONTENT_HOST, "/content/pdf/%s.pdf".formatted(doi.get().getDOI())));
+                    return Optional.of(new URI("http", null, CONTENT_HOST, -1, "/content/pdf/%s.pdf".formatted(doi.get().getDOI()), null, null).toURL());
                 }
             }
         } catch (UnirestException e) {
             LOGGER.warn("SpringerLink API request failed", e);
         }