Skip to content

Preserve @Nullable parameters in AnnotateRequiredParameters #897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
timtebeek wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Preserve @Nullable parameters in AnnotateRequiredParameters #897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eb62b1e
Preserve `@Nullable` parameters in `AnnotateRequiredParameters`
timtebeek May 20, 2026
77c68f5
Preserve null-check + throw when the throw carries information
timtebeek May 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 50 additions & 15 deletions src/main/java/org/openrewrite/staticanalysis/AnnotateRequiredParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -145,16 +146,32 @@ private static boolean containsIdentifierByName(Collection<J.Identifier> 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<J.Identifier> getAllParameters(J.MethodDeclaration md) {
Set<J.Identifier> 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<AtomicBoolean>() {
@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;
Expand Down Expand Up @@ -188,9 +205,10 @@ public J.If visitIf(J.If iff, RequiredParameterAnalysis analysis) {
List<J.Identifier> 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);
Expand Down Expand Up @@ -257,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<Expression> 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;
}
}

Expand Down
69 changes: 53 additions & 16 deletions src/test/java/org/openrewrite/staticanalysis/AnnotateRequiredParametersTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -454,8 +454,9 @@ boolean otherCondition() {
);
}

@Issue("https://github.com/openrewrite/rewrite/issues/7741")
@Test
void removeNullableAnnotationWhenAddingNonNull() {
void preserveNullableAnnotationAndNullCheck() {
rewriteRun(
//language=java
java(
Expand All @@ -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);
}
}
"""
)
);
Expand Down Expand Up @@ -645,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);
}
}
"""
)
);
}
}
}
Loading