openrewrite / rewrite-migrate-java

OpenRewrite recipes for migrating to newer versions of Java.
Apache License 2.0
103 stars 72 forks source link

Add explicit imports to avoid conflicts with classes added to `java.lang`, like `Record` #540

Open timtebeek opened 1 month ago

timtebeek commented 1 month ago

What problem are you trying to solve?

Folks using com.something.Record through import com.something.*; might find that the java.lang.Record is picked up when they migrate to Java 16+. Same for other classes added to the JDK:

What precondition(s) should be checked before applying this recipe?

Using any conflicting classes on older Java versions

Describe the situation before applying the recipe

import com.something.*;
class A {
    void foo(Record r) {
    }
}

Describe the situation after applying the recipe

import com.something.Record;
class A {
    void foo(Record r) {
    }
}

Any additional context

Reported via email, with an example use case in https://github.com/fmcdevops/medrec/blob/master/src/common/value/com/bea/medrec/value/Record.java

junior commented 1 month ago

This is something to be included on the "Migrate to Java 17" recipe or will be standalone recipe?

timtebeek commented 1 month ago

I'd implement this as a recipe that takes in an argument, which we then add to the Java 17 and 21 migrations.

timtebeek commented 1 month ago

As a quick starter; here's how we'd test this type of recipe

import org.junit.jupiter.api.Test;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;

class AvoidConflictingImportsTest implements RewriteTest {

    @Test
    void importRecord() {
        rewriteRun(
          spec -> spec.recipe(new AvoidConflictingImports("java.lang.Record")),
          //language=java
          java(
            """
              package com.acme.bank;
              public class Record {}
              """
          ),
          //language=java
          java(
            """
              import com.acme.bank.*;

              class Foo {
                  Record r;
              }
              """,
            """
              import com.acme.bank.Record;

              class Foo {
                  Record r;
              }
              """
          )
        );
    }
}

And then beyond that we'd need to implement the visitor to actually add the unambiguous import that avoids java.lang.Record.

import lombok.EqualsAndHashCode;
import lombok.Value;
import org.openrewrite.ExecutionContext;
import org.openrewrite.Option;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.tree.J;

@Value
@EqualsAndHashCode(callSuper = false)
public class AvoidConflictingImports extends Recipe {

    @Option(displayName = "Import",
            description = "The fully qualified name of the class introduced.",
            example = "java.lang.Record")
    String fullyQualifiedClassName;

    @Override
    public String getDisplayName() {
        return "Avoid conflicting imports";
    }

    @Override
    public String getDescription() {
        return "Add explicit imports for classes that might otherwise conflict with the argument FQCN.";
    }

    @Override
    public TreeVisitor<J, ExecutionContext> getVisitor() {
        return TreeVisitor.noop(); // TODO find any use of `*..Record` and replace wildcard import with explicit import
    }
}
junior commented 3 weeks ago

That's the best TODO for this feature. How do we vote? ;)

TODO find any use of `*..Record` and replace wildcard import with explicit import