openrewrite / rewrite-templating

Automated templating using code snippets.
Apache License 2.0
16 stars 7 forks source link

refaster recipe does not create import statement but inlines package info #67

Closed timo-abele closed 9 months ago

timo-abele commented 9 months ago

What version of OpenRewrite are you using?

I am editing https://github.com/moderneinc/rewrite-recipe-starter/commit/3fd5d7a38d7fccb8cb55cbb62277e7bd5af3004a directly

How are you running OpenRewrite?

I'm running SimplifyTernaryTest#simplified in the IDE.

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

see https://github.com/timo-abele/rewrite-recipe-starter/tree/debug/refaster

The modified SimplifyTernaryTest should create an import statement, but inlines the package info instead:

[Unexpected result in "Test.java":
diff --git a/Test.java b/Test.java
index febde9c..a9e5838 100644
--- a/Test.java
+++ b/Test.java
@@ -1,5 +1,3 @@ 
-import com.google.common.primitives.Booleans;
-
 class Test {
-    boolean trueCondition1 = Booleans.contains(new boolean[]{true}, true);
+    boolean trueCondition1 = com.google.common.primitives.Booleans.contains(new boolean[]{true}, true);
 }
\ No newline at end of file
] 
expected: 
  "import com.google.common.primitives.Booleans;

  class Test {
      boolean trueCondition1 = Booleans.contains(new boolean[]{true}, true);
  }"
 but was: 
  "class Test {
      boolean trueCondition1 = com.google.common.primitives.Booleans.contains(new boolean[]{true}, true);
  }"

Are you interested in contributing a fix to OpenRewrite?

no

timtebeek commented 9 months ago

That's odd; normally we shorten qualified type references in the generated recipe. 🤔 https://github.com/openrewrite/rewrite-templating/blob/0e4a1440df56537041bbc86294dc77f22758c288/src/main/java/org/openrewrite/java/template/internal/AbstractRefasterJavaVisitor.java#L44-L46

Now I'm wondering if that EmbeddingOption.SHORTEN_NAMES is correctly set on your generated recipe, or if it's not what factors in there. To be investigated and fixed!

Bananeweizen commented 9 months ago

duplicate of #66, as far as I understand.

timtebeek commented 9 months ago

Awesome, thanks for the fix @Bananeweizen ! Proactively closing this one; hope to hear from @timo-abele that his issue has been resolved too.

timo-abele commented 9 months ago

I confirm, after switching to template snapshots, the tests work!