From c70a78a83c7352cfbcedf03f3734df31dc90a0ba Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Mon, 10 Aug 2026 10:49:12 +0200 Subject: [PATCH 1/3] MigrateSecurityManagerMulticast: only migrate provably equivalent calls The recipe rewrote every `checkMulticast(InetAddress, byte)` call to the one argument overload by dropping the second argument, which broke two contracts. Java evaluates an argument before the call, so the side effects and exceptions of the discarded time to live expression disappeared, silently losing an increment, a method invocation, an array access, or the `NullPointerException` from unboxing a null `Byte`. The two overloads are also separate virtual methods that a `SecurityManager` subclass may override independently, so the rewrite changed which method runs whenever the receiver held a subclass. A call is now migrated only when both hazards are ruled out. The receiver must be a direct `new SecurityManager()` allocation without an anonymous class body, the only shape whose runtime class is known here; for `java.lang.SecurityManager` itself both overloads are specified to call `checkPermission` with the same `SocketPermission`, and the two argument form is documented not to use the time to live. The discarded argument must be a literal, a primitive to primitive cast of one, or a read of a primitive local or parameter. Every other receiver, including `super` calls and anonymous subclasses, and every other argument shape, is left unchanged. The migrated call also references the declared `checkMulticast(InetAddress)` overload resolved from the declaring type, rather than a method type trimmed down from the two argument overload, which carried over the latter's deprecation metadata. Two points for review. Calls through a variable of static type `SecurityManager` are no longer migrated at all, which is deliberate: their runtime class cannot be established, so no check on the discarded argument can make the switch safe. The existing test and the generated example now declare `throws Exception`, because their `InetAddress.getByName` call is checked and both sides of the example have to compile. The recipe description and recipes.csv state the new constraint. --- .../lang/MigrateSecurityManagerMulticast.java | 91 ++++- .../resources/META-INF/rewrite/examples.yml | 4 +- .../resources/META-INF/rewrite/recipes.csv | 2 +- .../MigrateSecurityManagerMulticastTest.java | 313 +++++++++++++++++- 4 files changed, 398 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java b/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java index df29d372d5..10162e03cd 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java @@ -16,6 +16,7 @@ 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; @@ -23,7 +24,10 @@ 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 tags = singleton( "deprecated" ); @@ -49,16 +58,84 @@ public TreeVisitor 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; } }); } + /** + * {@code checkMulticast(InetAddress, byte)} and {@code checkMulticast(InetAddress)} are separate virtual + * methods, so switching a call from one to the other changes which method runs whenever the receiver holds a + * {@code SecurityManager} subclass that overrides either overload. The receiver's runtime class is only known + * here when the receiver is a direct {@code new SecurityManager()} allocation without an anonymous class body. + * For {@code java.lang.SecurityManager} itself both overloads are specified to call {@code checkPermission} + * with the same {@code SocketPermission}, and the two argument form is documented not to use the time to + * live, so only then is the switch unobservable. Every other receiver, including any expression whose static + * type merely is {@code SecurityManager}, may hold a subclass at run time and 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 {@code checkMulticast(InetAddress)} overload rather than a + * synthetic method type trimmed down from the two argument overload, which would carry over the latter's + * deprecation metadata. When the overload cannot be resolved the call is left unchanged. + */ + 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; + } + + /** + * The one argument overload ignores the time to live, so dropping the second argument also stops evaluating it. + * That is only safe when evaluating the argument can neither be observed nor throw, which is limited to + * constants and reads of a local variable or parameter of primitive type. Anything else, such as a method + * invocation, an increment, an array access, a field read that might be volatile, or an unboxing conversion, + * 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 cast. + 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; + } } diff --git a/src/main/resources/META-INF/rewrite/examples.yml b/src/main/resources/META-INF/rewrite/examples.yml index 1dfc38ed48..2b85b9a89b 100644 --- a/src/main/resources/META-INF/rewrite/examples.yml +++ b/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); diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index e51cb061b2..ea51d263de 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/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.,, diff --git a/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java b/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java index 196775e95b..c4e54e8cce 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java +++ b/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,313 @@ 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 retainMethodInvocation() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + void method(InetAddress maddr) { + new SecurityManager().checkMulticast(maddr, nextTtl()); + new SecurityManager().checkMulticast(maddr, (byte) nextTtl()); + } + + byte nextTtl() { + throw new IllegalStateException("evaluated"); + } + } + """ + ) + ); + } + + @Test + void retainIncrement() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + byte ttl; + + void method(InetAddress maddr) { + new SecurityManager().checkMulticast(maddr, ttl++); + new SecurityManager().checkMulticast(maddr, ++ttl); + } + } + """ + ) + ); + } + + @Test + void retainArrayAccess() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + byte[] ttls = {1}; + int index; + + void method(InetAddress maddr) { + new SecurityManager().checkMulticast(maddr, ttls[index++]); + } + } + """ + ) + ); + } + + @Test + void retainUnboxing() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + void method(InetAddress maddr, Byte boxedTtl) { + new SecurityManager().checkMulticast(maddr, boxedTtl); + new SecurityManager().checkMulticast(maddr, (byte) boxedTtl); + } + } + """ + ) + ); + } + + @Test + void retainFieldRead() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + byte ttl; + volatile byte volatileTtl; + static final byte DEFAULT_TTL = 1; + + void method(InetAddress maddr) { + new SecurityManager().checkMulticast(maddr, ttl); + new SecurityManager().checkMulticast(maddr, this.ttl); + new SecurityManager().checkMulticast(maddr, volatileTtl); + new SecurityManager().checkMulticast(maddr, DEFAULT_TTL); + } + } + """ + ) + ); + } + + @Test + void retainAssignment() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + byte ttl; + + void method(InetAddress maddr) { + new SecurityManager().checkMulticast(maddr, ttl = 1); + new SecurityManager().checkMulticast(maddr, ttl += 1); + } + } + """ + ) + ); + } + + @Test + void retainConditionalAndSwitch() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + void method(InetAddress maddr, boolean flag, int mode) { + new SecurityManager().checkMulticast(maddr, flag ? nextTtl() : (byte) 1); + new SecurityManager().checkMulticast(maddr, switch (mode) { + case 1 -> (byte) 1; + default -> nextTtl(); + }); + } + + byte nextTtl() { + throw new IllegalStateException("evaluated"); + } + } + """ + ) + ); + } + + @Test + void retainDivisionAndAllocation() { + //language=java + rewriteRun( + java( + """ + import java.net.InetAddress; + + class Test { + byte ttl; + int divisor; + + void method(InetAddress maddr) { + new SecurityManager().checkMulticast(maddr, (byte) (ttl / divisor)); + new SecurityManager().checkMulticast(maddr, new Test().ttl); + } + } + """ + ) + ); + } } From 2a0ceff8ccb93fc2f2e5b89387b7a49a34255d76 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 12 Aug 2026 00:30:39 +0200 Subject: [PATCH 2/3] Trim commentary --- .../lang/MigrateSecurityManagerMulticast.java | 28 ++++++++----------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java b/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java index 10162e03cd..8f83b540fd 100644 --- a/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java +++ b/src/main/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticast.java @@ -77,14 +77,11 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu } /** - * {@code checkMulticast(InetAddress, byte)} and {@code checkMulticast(InetAddress)} are separate virtual - * methods, so switching a call from one to the other changes which method runs whenever the receiver holds a - * {@code SecurityManager} subclass that overrides either overload. The receiver's runtime class is only known - * here when the receiver is a direct {@code new SecurityManager()} allocation without an anonymous class body. - * For {@code java.lang.SecurityManager} itself both overloads are specified to call {@code checkPermission} - * with the same {@code SocketPermission}, and the two argument form is documented not to use the time to - * live, so only then is the switch unobservable. Every other receiver, including any expression whose static - * type merely is {@code SecurityManager}, may hold a subclass at run time and is left unchanged. + * 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; @@ -99,9 +96,8 @@ private static boolean cannotDispatchToAnOverride(@Nullable Expression select) { } /** - * The migrated call must reference the declared {@code checkMulticast(InetAddress)} overload rather than a - * synthetic method type trimmed down from the two argument overload, which would carry over the latter's - * deprecation metadata. When the overload cannot be resolved the call is left unchanged. + * 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) { @@ -116,18 +112,16 @@ private static boolean cannotDispatchToAnOverride(@Nullable Expression select) { } /** - * The one argument overload ignores the time to live, so dropping the second argument also stops evaluating it. - * That is only safe when evaluating the argument can neither be observed nor throw, which is limited to - * constants and reads of a local variable or parameter of primitive type. Anything else, such as a method - * invocation, an increment, an array access, a field read that might be volatile, or an unboxing conversion, - * is left unchanged. + * 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 cast. + // 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()); } From 10d2a2816b867116b71a03cd6172f6286e34d23e Mon Sep 17 00:00:00 2001 From: martinfrancois Date: Sun, 16 Aug 2026 15:16:19 +0200 Subject: [PATCH 3/3] Consolidate observable argument tests --- .../MigrateSecurityManagerMulticastTest.java | 152 ++---------------- 1 file changed, 11 insertions(+), 141 deletions(-) diff --git a/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java b/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java index c4e54e8cce..d34f20c342 100644 --- a/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java +++ b/src/test/java/org/openrewrite/java/migrate/lang/MigrateSecurityManagerMulticastTest.java @@ -199,30 +199,7 @@ void method(InetAddress maddr) { } @Test - void retainMethodInvocation() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { - void method(InetAddress maddr) { - new SecurityManager().checkMulticast(maddr, nextTtl()); - new SecurityManager().checkMulticast(maddr, (byte) nextTtl()); - } - - byte nextTtl() { - throw new IllegalStateException("evaluated"); - } - } - """ - ) - ); - } - - @Test - void retainIncrement() { + void retainPotentiallyObservableArguments() { //language=java rewriteRun( java( @@ -231,118 +208,33 @@ void retainIncrement() { class Test { byte ttl; - - void method(InetAddress maddr) { - new SecurityManager().checkMulticast(maddr, ttl++); - new SecurityManager().checkMulticast(maddr, ++ttl); - } - } - """ - ) - ); - } - - @Test - void retainArrayAccess() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { + volatile byte volatileTtl; + static final byte DEFAULT_TTL = 1; byte[] ttls = {1}; int index; + int divisor; - void method(InetAddress maddr) { + 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++]); - } - } - """ - ) - ); - } - - @Test - void retainUnboxing() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { - void method(InetAddress maddr, Byte boxedTtl) { new SecurityManager().checkMulticast(maddr, boxedTtl); new SecurityManager().checkMulticast(maddr, (byte) boxedTtl); - } - } - """ - ) - ); - } - - @Test - void retainFieldRead() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { - byte ttl; - volatile byte volatileTtl; - static final byte DEFAULT_TTL = 1; - - void method(InetAddress maddr) { new SecurityManager().checkMulticast(maddr, ttl); new SecurityManager().checkMulticast(maddr, this.ttl); new SecurityManager().checkMulticast(maddr, volatileTtl); new SecurityManager().checkMulticast(maddr, DEFAULT_TTL); - } - } - """ - ) - ); - } - - @Test - void retainAssignment() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { - byte ttl; - - void method(InetAddress maddr) { new SecurityManager().checkMulticast(maddr, ttl = 1); new SecurityManager().checkMulticast(maddr, ttl += 1); - } - } - """ - ) - ); - } - - @Test - void retainConditionalAndSwitch() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { - void method(InetAddress maddr, boolean flag, int mode) { 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() { @@ -353,26 +245,4 @@ byte nextTtl() { ) ); } - - @Test - void retainDivisionAndAllocation() { - //language=java - rewriteRun( - java( - """ - import java.net.InetAddress; - - class Test { - byte ttl; - int divisor; - - void method(InetAddress maddr) { - new SecurityManager().checkMulticast(maddr, (byte) (ttl / divisor)); - new SecurityManager().checkMulticast(maddr, new Test().ttl); - } - } - """ - ) - ); - } }