rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

Do not change package when non-recursive

Open timtebeek opened this issue 1 year ago • 5 comments

Add unit test as reported via Slack by @BhavanaPidapa; update precondition to evaluate recursive boolean for package declaration too.

timtebeek avatar Aug 26 '24 21:08 timtebeek

Hey Tim,

Below is the test case where changes to com.sun.net.ssl.internal.* are not expected. Please refer to the attached screenshot.

@Test
void excludePackageTest() {
    //language=java
    rewriteRun(
      spec -> spec.recipe(new ChangePackage(
        "com.sun.net.ssl",
        "javax.net.ssl",
        false
      )),
      java(
        """
          package com.test;
          import com.sun.net.ssl.internal.*;
          import com.sun.net.ssl.HostnameVerifier;
          class Test {
           public com.sun.net.ssl.HostnameVerifier hv;
              void exampleMethod() {
              }
          }
          """,
        """
          package com.test;
          import com.sun.net.ssl.internal.*;
          import javax.net.ssl.HostnameVerifier;
          class Test {
           public javax.net.ssl.HostnameVerifier hv;
              void exampleMethod() {
              }
          }
          """
      )
    );
}

image

BhavanaPidapa avatar Aug 27 '24 18:08 BhavanaPidapa

Capturing the nonRecursiveShouldIgnoreNestedWildcardImport test failure output

diff --git a/Foo.java b/Foo.java	
index 562edaa..d960c46 100644	
--- a/Foo.java	
+++ b/Foo.java	
@@ -1,4 +1,4 @@ 	
-import com.sun.net.ssl.internal.*;	
+import javax.net.ssl.internal.*;	
 import javax.net.ssl.HostnameVerifier;	
 class Foo {	
     javax.net.ssl.HostnameVerifier hv;	

timtebeek avatar Jan 22 '25 14:01 timtebeek

This is an interesting case: we first and separate evaluate a precondition to determine if we need further changes; that precondition resolves to true due to the presence of com.sun.net.ssl.HostnameVerifier; then we unconditionally change any package references in the actual visitor, whereas we should probably still take into account there whether we should do so recursively.

timtebeek avatar Jan 22 '25 14:01 timtebeek

Maybe fixed by https://github.com/openrewrite/rewrite/pull/4942? Not sure, as the work I did was for YAML.

knutwannheden avatar Jan 24 '25 10:01 knutwannheden

This PR is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jun 02 '25 04:06 github-actions[bot]