bugfix: false positive in AnnotateNullableParameters when parameter … (#865)

* bugfix:  false positive in AnnotateNullableParameters when parameter is dereferenced before null check

* Check all arguments for null-checked parameters

Addresses review feedback: custom null-checkers may accept the parameter in a position other than first (e.g., `assertNotNull(message, param)`).

---------

Co-authored-by: stefanod <stefano.dallapalma@adyen.com>
Co-authored-by: Tim te Beek <tim@moderne.io>

Author: 3 people

src/main/java/org/openrewrite/staticanalysis/AnnotateNullableParameters.java

@@ -319,15 +319,16 @@ private void handleBinary(J.Binary binary, Set<J.Identifier> nullCheckedParams)
 
         private void handleMethodInvocation(J.MethodInvocation mi, Set<J.Identifier> nullCheckedParams) {
             if (isKnownNullMethodChecker(mi)) {
-                new JavaIsoVisitor<Set<J.Identifier>>() {
-                    @Override
-                    public J.Identifier visitIdentifier(J.Identifier identifier, Set<J.Identifier> set) {
+                // Only consider the parameter as null-checked if it's passed directly,
+                // not if it's dereferenced first (e.g., map.get("key"))
+                for (Expression arg : mi.getArguments()) {
+                    if (arg instanceof J.Identifier) {
+                        J.Identifier identifier = (J.Identifier) arg;
                         if (containsIdentifierByName(identifiers, identifier)) {
-                            set.add(identifier);
+                            nullCheckedParams.add(identifier);
                         }
-                        return identifier;
                     }
-                }.visit(mi.getArguments(), nullCheckedParams);
+                }
             }
         }
 

src/test/java/org/openrewrite/staticanalysis/AnnotateNullableParametersTest.java

@@ -909,4 +909,30 @@ public PersonBuilder setInfo(@Nullable String name, @Nullable String lastName) {
           )
         );
     }
+
+    @Test
+    void unchangedWhenParameterDereferencedBeforeNullCheckingMethod() {
+        List<String> additionalNullCheckingMethods = List.of("org.my.util.Text isEmptyOrNull(java.lang.String)");
+        rewriteRun(
+          spec -> spec.recipe(new AnnotateNullableParameters(null, additionalNullCheckingMethods)),
+          //language=java
+          java(
+            """
+              import java.util.Map;
+              import org.my.util.Text;
+
+              public class PersonBuilder {
+                  private String name = "Unknown";
+
+                  public PersonBuilder setInfo(Map<String, String> infoMap) {
+                      if (!Text.isEmptyOrNull(infoMap.get("name"))) {
+                          this.name = infoMap.get("name");
+                      }
+                      return this;
+                  }
+              }
+              """
+          )
+        );
+    }
 }