From eb62b1e13af08b4ddec762e3a1d2dc72b7551cd5 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 May 2026 15:36:25 +0200 Subject: [PATCH 1/2] Preserve `@Nullable` parameters in `AnnotateRequiredParameters` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip parameters already annotated `@Nullable` so the developer's explicit intent to allow nulls — along with any accompanying custom null check and business exception — is preserved. --- .../AnnotateRequiredParameters.java | 23 ++++++++++++++++--- .../AnnotateRequiredParametersTest.java | 14 +++-------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java b/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java index fc75844f9..ef1bb37e6 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java +++ b/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java @@ -28,6 +28,7 @@ import org.openrewrite.staticanalysis.java.MoveFieldAnnotationToType; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import static java.util.Comparator.comparing; @@ -145,16 +146,32 @@ private static boolean containsIdentifierByName(Collection identif } /** - * Gets all method parameters. + * Gets all method parameters, excluding those already annotated with `@Nullable`. + * A parameter explicitly marked `@Nullable` signals that the developer intends + * to allow nulls; any accompanying null check + throw is therefore intentional + * business logic and must not be removed. * * @param md the method declaration to analyze - * @return set of all parameter declarations + * @return set of candidate parameter identifiers */ private Set getAllParameters(J.MethodDeclaration md) { Set allParams = new LinkedHashSet<>(); for (Statement parameter : md.getParameters()) { if (parameter instanceof J.VariableDeclarations) { - allParams.add(((J.VariableDeclarations) parameter).getVariables().get(0).getName()); + J.VariableDeclarations vd = (J.VariableDeclarations) parameter; + boolean nullable = new JavaIsoVisitor() { + @Override + public J.Annotation visitAnnotation(J.Annotation annotation, AtomicBoolean f) { + if ("Nullable".equals(annotation.getSimpleName())) { + f.set(true); + } + return annotation; + } + }.reduce(vd, new AtomicBoolean()).get(); + if (nullable) { + continue; + } + allParams.add(vd.getVariables().get(0).getName()); } } return allParams; diff --git a/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java b/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java index a9f7775a1..905f2fe9e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java @@ -454,8 +454,9 @@ boolean otherCondition() { ); } + @Issue("https://github.com/openrewrite/rewrite/issues/7741") @Test - void removeNullableAnnotationWhenAddingNonNull() { + void preserveNullableAnnotationAndNullCheck() { rewriteRun( //language=java java( @@ -465,20 +466,11 @@ void removeNullableAnnotationWhenAddingNonNull() { class Test { public void process(@Nullable String value) { if (value == null) { - throw new IllegalArgumentException(); + throw new IllegalArgumentException("my concrete business exception"); } System.out.println(value); } } - """, - """ - import org.jspecify.annotations.NonNull; - - class Test { - public void process(@NonNull String value) { - System.out.println(value); - } - } """ ) ); From 77c68f53a9ec9b685b6b0fc3e2ad59c5867dbf6e Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Wed, 20 May 2026 15:51:50 +0200 Subject: [PATCH 2/2] Preserve null-check + throw when the throw carries information MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only treat a null-check as redundant when the thrown exception is a JDK type constructed with no arguments. Custom exception types and non-empty message arguments signal intentional business logic and are left intact, along with the surrounding `if` and the parameter (no `@NonNull` added). `Objects.requireNonNull(value, "msg")` continues to be rewritten — that form is idiomatic null-guard boilerplate where message loss is acceptable. --- .../AnnotateRequiredParameters.java | 42 ++++++++++---- .../AnnotateRequiredParametersTest.java | 55 +++++++++++++++++-- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java b/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java index ef1bb37e6..13bb2f7b2 100644 --- a/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java +++ b/src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java @@ -205,9 +205,10 @@ public J.If visitIf(J.If iff, RequiredParameterAnalysis analysis) { List nullCheckedParams = extractNullCheckedParameters(condition); if (!nullCheckedParams.isEmpty()) { - // Check if the then-body throws an exception - if (bodyThrowsException(iff.getThenPart())) { - // Add all null-checked parameters as required + // Only treat the null check as redundant when the throw carries no information: + // a JDK exception type with no constructor arguments. A custom exception type or + // a non-empty message indicates intentional business logic that must be preserved. + if (throwsInformationlessException(iff.getThenPart())) { for (J.Identifier param : nullCheckedParams) { if (containsIdentifierByName(parameterIdentifiers, param)) { analysis.requiredIdentifiers.add(param); @@ -274,22 +275,39 @@ else if (operator == J.Binary.Type.Equal) { } /** - * Checks if a statement block throws an exception. + * Returns true when the body throws a JDK exception constructed with no arguments — + * a purely defensive check that can be safely replaced by a `@NonNull` annotation. + * Custom exception types or non-empty messages are treated as intentional and preserved. */ - private boolean bodyThrowsException(Statement body) { - if (body instanceof J.Throw) { + private boolean throwsInformationlessException(Statement body) { + J.Throw thr = findThrow(body); + if (thr == null || !(thr.getException() instanceof J.NewClass)) { + return false; + } + J.NewClass nc = (J.NewClass) thr.getException(); + JavaType.FullyQualified fq = TypeUtils.asFullyQualified(nc.getType()); + if (fq == null || !fq.getFullyQualifiedName().startsWith("java.")) { + return false; + } + List args = nc.getArguments(); + if (args == null || args.isEmpty()) { return true; } + return args.size() == 1 && args.get(0) instanceof J.Empty; + } + + private J.@Nullable Throw findThrow(Statement body) { + if (body instanceof J.Throw) { + return (J.Throw) body; + } if (body instanceof J.Block) { - J.Block block = (J.Block) body; - // Check if any statement in the block is a throw - for (Statement statement : block.getStatements()) { - if (statement instanceof J.Throw) { - return true; + for (Statement s : ((J.Block) body).getStatements()) { + if (s instanceof J.Throw) { + return (J.Throw) s; } } } - return false; + return null; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java b/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java index 905f2fe9e..bddc32ee5 100644 --- a/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java @@ -42,7 +42,7 @@ void annotateParameterWithNullCheckThrowingException() { class Test { public void process(String value) { if (value == null) { - throw new IllegalArgumentException("value cannot be null"); + throw new IllegalArgumentException(); } System.out.println(value); } @@ -251,7 +251,7 @@ void annotateWithDifferentExceptionTypes() { class Test { public void process(String value) { if (value == null) { - throw new RuntimeException("Required parameter"); + throw new RuntimeException(); } System.out.println(value); } @@ -365,7 +365,7 @@ void annotateThreeParametersInSameCondition() { class Test { public void process(String first, String second, String third) { if (first == null || second == null || third == null) { - throw new IllegalArgumentException("All parameters are required"); + throw new IllegalArgumentException(); } System.out.println(first + second + third); } @@ -424,7 +424,7 @@ void doNotRemoveOtherCondition() { class Test { public void process(String value) { if (value == null || otherCondition()) { - throw new IllegalArgumentException("value cannot be null"); + throw new IllegalArgumentException(); } System.out.println(value); } @@ -440,7 +440,7 @@ boolean otherCondition() { class Test { public void process(@NonNull String value) { if (otherCondition()) { - throw new IllegalArgumentException("value cannot be null"); + throw new IllegalArgumentException(); } System.out.println(value); } @@ -637,5 +637,50 @@ public void process(boolean secondRequired, String second) { ) ); } + + @Issue("https://github.com/openrewrite/rewrite/issues/7741") + @Test + void preserveThrowWithCustomMessage() { + rewriteRun( + //language=java + java( + """ + class Test { + public byte[] generate(final String printMode, final String uiStateDto) { + if (printMode == null) { + throw new IllegalArgumentException("my concrete business exception"); + } + if (uiStateDto == null) { + throw new IllegalArgumentException("my concrete business exception"); + } + return new byte[0]; + } + } + """ + ) + ); + } + + @Issue("https://github.com/openrewrite/rewrite/issues/7741") + @Test + void preserveThrowWithCustomExceptionType() { + rewriteRun( + //language=java + java( + """ + class Test { + static class MyBusinessException extends RuntimeException {} + + public void process(String value) { + if (value == null) { + throw new MyBusinessException(); + } + System.out.println(value); + } + } + """ + ) + ); + } } }