MigrateSecurityManagerMulticast: only migrate provably equivalent calls

src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java

@@ -16,14 +16,18 @@
 package org.openrewrite.java.migrate.lang;
 
 import lombok.Getter;
+import org.jspecify.annotations.Nullable;
 import org.openrewrite.ExecutionContext;
 import org.openrewrite.Preconditions;
 import org.openrewrite.Recipe;
 import org.openrewrite.TreeVisitor;
 import org.openrewrite.java.JavaIsoVisitor;
 import org.openrewrite.java.MethodMatcher;
 import org.openrewrite.java.search.UsesMethod;
+import org.openrewrite.java.tree.Expression;
 import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.JavaType;
+import org.openrewrite.java.tree.TypeUtils;
 
 import java.util.Set;
 
@@ -37,7 +41,12 @@ public class MigrateSecurityManagerMulticast extends Recipe {
     final String displayName = "Use `SecurityManager#checkMulticast(InetAddress)`";
 
     @Getter
-    final String description = "Use `SecurityManager#checkMulticast(InetAddress)` instead of the deprecated `SecurityManager#checkMulticast(InetAddress, byte)` in Java 1.4 or higher.";
+    final String description = "Use `SecurityManager#checkMulticast(InetAddress)` instead of the deprecated " +
+            "`SecurityManager#checkMulticast(InetAddress, byte)` in Java 1.4 or higher. " +
+            "The two overloads are separate methods that a `SecurityManager` subclass can override independently, " +
+            "so a call is only migrated when the receiver is a plain `new SecurityManager()` instance, " +
+            "where no override can be reached, and when evaluating the discarded time to live argument " +
+            "can neither be observed nor throw.";
 
     @Getter
     final Set<String> tags = singleton( "deprecated" );
@@ -49,16 +58,78 @@ public TreeVisitor<?, ExecutionContext> getVisitor() {
             public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
                 J.MethodInvocation m = super.visitMethodInvocation(method, ctx);
 
-                if (MULTICAST_METHOD.matches(m) && m.getArguments().size() == 2) {
-                    return m.withArguments(singletonList(m.getArguments().get(0)))
-                            .withMethodType(m.getMethodType()
-                                    .withParameterNames(m.getMethodType().getParameterNames().subList(0, 1))
-                                    .withParameterTypes(m.getMethodType().getParameterTypes().subList(0, 1))
-                            );
+                if (MULTICAST_METHOD.matches(m) && m.getArguments().size() == 2 &&
+                        cannotDispatchToAnOverride(m.getSelect()) &&
+                        isPureAndNonThrowing(m.getArguments().get(1))) {
+                    JavaType.Method singleArgumentOverload = singleArgumentOverload(m.getMethodType());
+                    if (singleArgumentOverload != null) {
+                        J.MethodInvocation migrated = m.withArguments(singletonList(m.getArguments().get(0)))
+                                .withMethodType(singleArgumentOverload);
+                        if (migrated.getName().getType() != null) {
+                            migrated = migrated.withName(migrated.getName().withType(singleArgumentOverload));
+                        }
+                        return migrated;
+                    }
                 }
                 return m;
             }
         });
     }
 
+    /**
+     * The two overloads are separate virtual methods, so switching between them changes which one runs whenever
+     * the receiver holds a subclass overriding either. Only a direct {@code new SecurityManager()} without an
+     * anonymous body pins the runtime class, and only for {@code java.lang.SecurityManager} are both overloads
+     * specified to check the same {@code SocketPermission} and ignore the time to live. Anything else, including
+     * an expression merely typed as {@code SecurityManager}, is left unchanged.
+     */
+    private static boolean cannotDispatchToAnOverride(@Nullable Expression select) {
+        J receiver = select;
+        while (receiver instanceof J.Parentheses) {
+            receiver = ((J.Parentheses<?>) receiver).getTree();
+        }
+        if (!(receiver instanceof J.NewClass)) {
+            return false;
+        }
+        J.NewClass newClass = (J.NewClass) receiver;
+        return newClass.getBody() == null && TypeUtils.isOfClassType(newClass.getType(), "java.lang.SecurityManager");
+    }
+
+    /**
+     * The migrated call must reference the declared one argument overload rather than a synthetic type trimmed
+     * from the two argument one, which would carry over its deprecation metadata.
+     */
+    private static JavaType.@Nullable Method singleArgumentOverload(JavaType.@Nullable Method twoArgumentOverload) {
+        if (twoArgumentOverload == null) {
+            return null;
+        }
+        for (JavaType.Method candidate : twoArgumentOverload.getDeclaringType().getMethods()) {
+            if ("checkMulticast".equals(candidate.getName()) && candidate.getParameterTypes().size() == 1) {
+                return candidate;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Dropping the ignored second argument also stops evaluating it, which is only safe when that evaluation can
+     * neither be observed nor throw: constants and reads of a primitive local or parameter. Anything else — an
+     * invocation, increment, array access, possibly volatile field read or unboxing — is left unchanged.
+     */
+    private static boolean isPureAndNonThrowing(Expression argument) {
+        if (argument instanceof J.Literal) {
+            return true;
+        }
+        if (argument instanceof J.TypeCast) {
+            // A primitive to primitive cast neither throws nor has side effects, unlike a reference or unboxing one
+            J.TypeCast typeCast = (J.TypeCast) argument;
+            return typeCast.getType() instanceof JavaType.Primitive && isPureAndNonThrowing(typeCast.getExpression());
+        }
+        if (argument instanceof J.Identifier) {
+            JavaType.Variable variable = ((J.Identifier) argument).getFieldType();
+            return variable != null && variable.getType() instanceof JavaType.Primitive &&
+                    variable.getOwner() instanceof JavaType.Method;
+        }
+        return false;
+    }
 }

src/main/resources/META-INF/rewrite/examples.yml

@@ -7678,7 +7678,7 @@ examples:
       import java.lang.SecurityManager;
 
       class Test {
-          public void method() {
+          public void method() throws Exception {
               InetAddress maddr = InetAddress.getByName("127.0.0.1");
               byte b = 100;
               new SecurityManager().checkMulticast(maddr, b);
@@ -7691,7 +7691,7 @@ examples:
       import java.lang.SecurityManager;
 
       class Test {
-          public void method() {
+          public void method() throws Exception {
               InetAddress maddr = InetAddress.getByName("127.0.0.1");
               byte b = 100;
               new SecurityManager().checkMulticast(maddr);

src/main/resources/META-INF/rewrite/recipes.csv

@@ -377,7 +377,7 @@ maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.l
 maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.MigrateRuntimeVersionMajorToFeature,Use `Runtime.Version#feature()`,Use `Runtime.Version#feature()` instead of the deprecated `Runtime.Version#major()` in Java 10 or higher.,2,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.MigrateRuntimeVersionMinorToInterim,Use `Runtime.Version#interim()`,Use `Runtime.Version#interim()` instead of the deprecated `Runtime.Version#minor()` in Java 10 or higher.,2,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.MigrateRuntimeVersionSecurityToUpdate,Use `Runtime.Version#update()`,Use `Runtime.Version#update()` instead of the deprecated `Runtime.Version#security()` in Java 10 or higher.,2,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
-maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.MigrateSecurityManagerMulticast,Use `SecurityManager#checkMulticast(InetAddress)`,"Use `SecurityManager#checkMulticast(InetAddress)` instead of the deprecated `SecurityManager#checkMulticast(InetAddress, byte)` in Java 1.4 or higher.",1,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
+maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.MigrateSecurityManagerMulticast,Use `SecurityManager#checkMulticast(InetAddress)`,"Use `SecurityManager#checkMulticast(InetAddress)` instead of the deprecated `SecurityManager#checkMulticast(InetAddress, byte)` in Java 1.4 or higher. The two overloads are separate methods that a `SecurityManager` subclass can override independently, so a call is only migrated when the receiver is a plain `new SecurityManager()` instance, where no override can be reached, and when evaluating the discarded time to live argument can neither be observed nor throw.",1,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.NullCheckAsSwitchCase,Add null check to existing switch cases,"In later Java 21+, null checks are valid in switch cases. This recipe will only add null checks to existing switch cases if there are no other statements in between them or if the block in the if statement is not impacting the flow of the switch.",1,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.RefineSwitchCases,Use switch cases refinement when possible,Use guarded switch case labels and guards if all the statements in the switch block do if/else if/else on the guarded label.,1,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,
 maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.lang.RenameUnderscoreIdentifier,Rename `_` identifier to `__`,"Renames single-underscore identifiers to double-underscore in Java source files with source compatibility of Java 8 or below. In Java 9+, `_` is a reserved keyword and causes a compile error.",1,,`java.lang` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,

src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java

@@ -42,7 +42,7 @@ void migrateCheckMulticast() {
               import java.lang.SecurityManager;
 
               class Test {
-                  public void method() {
+                  public void method() throws Exception {
                       InetAddress maddr = InetAddress.getByName("127.0.0.1");
                       byte b = 100;
                       new SecurityManager().checkMulticast(maddr, b);
@@ -56,7 +56,7 @@ public void method() {
               import java.lang.SecurityManager;
 
               class Test {
-                  public void method() {
+                  public void method() throws Exception {
                       InetAddress maddr = InetAddress.getByName("127.0.0.1");
                       byte b = 100;
                       new SecurityManager().checkMulticast(maddr);
@@ -66,4 +66,183 @@ public void method() {
           )
         );
     }
+
+    @Test
+    void migrateWhenReceiverIsExactAndTimeToLiveIsConstantOrLocal() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.net.InetAddress;
+
+              class Test {
+                  void method(InetAddress maddr, byte parameterTtl) {
+                      byte localTtl = 100;
+                      new SecurityManager().checkMulticast(maddr, (byte) 100);
+                      new SecurityManager().checkMulticast(maddr, parameterTtl);
+                      new SecurityManager().checkMulticast(maddr, localTtl);
+                      (new SecurityManager()).checkMulticast(address(maddr), localTtl);
+                  }
+
+                  InetAddress address(InetAddress maddr) {
+                      return maddr;
+                  }
+              }
+              """,
+            """
+              import java.net.InetAddress;
+
+              class Test {
+                  void method(InetAddress maddr, byte parameterTtl) {
+                      byte localTtl = 100;
+                      new SecurityManager().checkMulticast(maddr);
+                      new SecurityManager().checkMulticast(maddr);
+                      new SecurityManager().checkMulticast(maddr);
+                      (new SecurityManager()).checkMulticast(address(maddr));
+                  }
+
+                  InetAddress address(InetAddress maddr) {
+                      return maddr;
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void retainCallWhenReceiverMayBeASubclass() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.net.InetAddress;
+
+              class Test {
+                  void method(SecurityManager sm, InetAddress maddr, byte ttl) {
+                      sm.checkMulticast(maddr, ttl);
+                      sm.checkMulticast(maddr, (byte) 0);
+                      manager(sm).checkMulticast(maddr, ttl);
+                  }
+
+                  SecurityManager manager(SecurityManager sm) {
+                      return sm;
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void retainCallWhenAnOverloadIsOverridden() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.net.InetAddress;
+
+              class Test {
+                  static class SendingSecurityManager extends SecurityManager {
+                      @Override
+                      public void checkMulticast(InetAddress maddr) {
+                          super.checkMulticast(maddr);
+                      }
+                  }
+
+                  void method(InetAddress maddr) {
+                      SendingSecurityManager sm = new SendingSecurityManager();
+                      sm.checkMulticast(maddr, (byte) 0);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void retainCallDelegatingToSuper() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.net.InetAddress;
+
+              class AuditingSecurityManager extends SecurityManager {
+                  @Override
+                  public void checkMulticast(InetAddress maddr, byte ttl) {
+                      super.checkMulticast(maddr, ttl);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void retainCallOnAnonymousSubclass() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.net.InetAddress;
+
+              class Test {
+                  void method(InetAddress maddr) {
+                      new SecurityManager() {
+                      }.checkMulticast(maddr, (byte) 0);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void retainPotentiallyObservableArguments() {
+        //language=java
+        rewriteRun(
+          java(
+            """
+              import java.net.InetAddress;
+
+              class Test {
+                  byte ttl;
+                  volatile byte volatileTtl;
+                  static final byte DEFAULT_TTL = 1;
+                  byte[] ttls = {1};
+                  int index;
+                  int divisor;
+
+                  void method(InetAddress maddr, Byte boxedTtl, boolean flag, int mode) {
+                      new SecurityManager().checkMulticast(maddr, nextTtl());
+                      new SecurityManager().checkMulticast(maddr, (byte) nextTtl());
+                      new SecurityManager().checkMulticast(maddr, ttl++);
+                      new SecurityManager().checkMulticast(maddr, ++ttl);
+                      new SecurityManager().checkMulticast(maddr, ttls[index++]);
+                      new SecurityManager().checkMulticast(maddr, boxedTtl);
+                      new SecurityManager().checkMulticast(maddr, (byte) boxedTtl);
+                      new SecurityManager().checkMulticast(maddr, ttl);
+                      new SecurityManager().checkMulticast(maddr, this.ttl);
+                      new SecurityManager().checkMulticast(maddr, volatileTtl);
+                      new SecurityManager().checkMulticast(maddr, DEFAULT_TTL);
+                      new SecurityManager().checkMulticast(maddr, ttl = 1);
+                      new SecurityManager().checkMulticast(maddr, ttl += 1);
+                      new SecurityManager().checkMulticast(maddr, flag ? nextTtl() : (byte) 1);
+                      new SecurityManager().checkMulticast(maddr, switch (mode) {
+                          case 1 -> (byte) 1;
+                          default -> nextTtl();
+                      });
+                      new SecurityManager().checkMulticast(maddr, (byte) (ttl / divisor));
+                      new SecurityManager().checkMulticast(maddr, new Test().ttl);
+                  }
+
+                  byte nextTtl() {
+                      throw new IllegalStateException("evaluated");
+                  }
+              }
+              """
+          )
+        );
+    }
 }