Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add recipes to migrate boxed primitive constructors deprecated in Java 9 #332

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Add recipes to migrate boxed primitive constructors deprecated in Java 9 #332

merged 1 commit into from
Oct 27, 2023

Conversation

dsibilio
Copy link
Contributor

@dsibilio dsibilio commented Oct 27, 2023

What's changed?

Added a set of Refaster recipes to deal with boxed primitive constructors that were deprecated in Java 9.

What's your motivation?

https://rewriteoss.slack.com/archives/C01A843MWG5/p1698430647117759?thread_ts=1698429230.444949&cid=C01A843MWG5

Anything in particular you'd like reviewers to focus on?

Unless I missed something, I think I covered all deprecated constructors except new Float(double) -> Float.valueOf((float) double). Due to how Refaster generates the code, I end up in a situation like the following:

Refaster Template

@RecipeDescriptor(
    name = "Replace deprecated `Float(double)` constructor invocations",
    description = "Replace deprecated `Float(double)` constructor invocations with `Float.valueOf((float) double)`."
)
public static class FloatDoubleConstructor {
    @BeforeTemplate
    public Float doubleConstructor(double value) {
        return new Float(value);
    }

    @AfterTemplate
    public Float valueOf(double value) {
        return Float.valueOf((float) value);
    }
}

Generated Code Error

error: incompatible types: Double cannot be converted to float
                final JavaTemplate valueOf = Semantics.expression(this, "valueOf", (@Primitive Double value) -> Float.valueOf((float)value)).build();

Therefore I was only able to cover new Float(Double) -> new Float(Double.floatValue()) instead.

Is there any way to force a real primitive in this line via Refaster (or use e.g.: a String-format template somehow)?
Do we need to make a separate imperative recipe just to cover the Float(double) edge case?

Anyone you would like to review specifically?

@timtebeek 🙏

Have you considered any alternatives or workarounds?

We could use imperative recipes but the change seems simple enough that the Refaster usage seems to be justified and seems to avoid a lot of repeated boilerplate code.

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've added the license header to any new files through ./gradlew licenseFormat
  • I've used the IntelliJ IDEA auto-formatter on affected files

Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the quick turn around on this & thanks for contributing.

I don't know of a way to force the case for new Float(double) using Refaster; I think we can leave that out for now; it should be uncommon, and perhaps not worth the effort of a separate recipe.

Really like how you cover every case with a single test, and how you took care to match our expected formatting.

@timtebeek timtebeek merged commit 59f1bb5 into openrewrite:main Oct 27, 2023
1 check passed
@dsibilio dsibilio deleted the feature/migrate-deprecated-boxed-primitive-constructors branch October 27, 2023 23:27
timtebeek pushed a commit that referenced this pull request Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants