Recipe RemoveRedundantNullCheckBeforeInstanceof

.claude/settings.local.json

@@ -27,8 +27,9 @@
       "mcp__idea__open_file_in_editor",
       "mcp__idea__replace_selected_text",
       "mcp__idea__replace_specific_text",
-      "mcp__idea__search_in_files_content"
+      "mcp__idea__search_in_files_content",
+      "Bash(./gradlew compileTestJava:*)"
     ],
     "deny": []
   }
-}
+}

CLAUDE.md

@@ -0,0 +1,55 @@
+# Recipe Development Instructions
+
+## Code Style Preferences
+
+### 1. Avoid Small Utility Methods
+- Prefer more concise code over many small utility methods
+- Inline logic when it doesn't significantly reduce readability
+- Consolidate related logic into fewer, more comprehensive methods
+
+### 2. Use SemanticallyEqual for Expression Comparison
+- When comparing expressions in recipes, use `SemanticallyEqual.areEqual()` from `org.openrewrite.java.search.SemanticallyEqual`
+- This provides more robust comparison than string matching
+- Handles formatting and whitespace differences automatically
+
+## Test Development Guidelines
+
+### 1. Remove Overlapping Test Cases
+- Avoid test cases that essentially test the same functionality
+- Each test should cover a distinct scenario or edge case
+- Consolidate similar tests when they don't add unique value
+
+### 2. Use Method Parameters Instead of Local Variables
+- In test text blocks, prefer method parameters over local variable declarations
+- This makes tests more concise and focuses on the transformation being tested
+- Example:
+  ```java
+  // Instead of:
+  void foo() {
+      String s = "test";
+      if (s != null && s instanceof String) {
+          // ...
+      }
+  }
+
+  // Use:
+  void foo(String s) {
+      if (s != null && s instanceof String) {
+          // ...
+      }
+  }
+  ```
+
+### 3. Suppress Warnings Appropriately
+- Add `@SuppressWarnings` annotations to test classes to suppress expected warnings
+- Common suppressions for this type of recipe:
+  - `"ConstantConditions"` - for redundant null checks that the recipe will remove
+  - Other suppressions as needed based on the specific warnings in test cases
+
+## Testing Commands
+After making changes to a recipe, always run the respective tests, such as for instance:
+```bash
+./gradlew test --tests RemoveRedundantNullCheckBeforeInstanceofTest
+```
+
+Ensure all tests pass before considering the changes complete.

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

@@ -0,0 +1,112 @@
+/*
+ * Copyright 2024 the original author or authors.
+ * <p>
+ * Licensed under the Moderne Source Available License (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.openrewrite.staticanalysis;
+
+import lombok.EqualsAndHashCode;
+import lombok.Value;
+import org.openrewrite.ExecutionContext;
+import org.openrewrite.Recipe;
+import org.openrewrite.TreeVisitor;
+import org.openrewrite.java.JavaVisitor;
+import org.openrewrite.java.search.SemanticallyEqual;
+import org.openrewrite.java.tree.Expression;
+import org.openrewrite.java.tree.J;
+
+import java.time.Duration;
+import java.util.Collections;
+import java.util.Set;
+
+@EqualsAndHashCode(callSuper = false)
+@Value
+public class RemoveRedundantNullCheckBeforeInstanceof extends Recipe {
+
+    @Override
+    public String getDisplayName() {
+        return "Remove redundant null checks before instanceof";
+    }
+
+    @Override
+    public String getDescription() {
+        return "Removes redundant null checks before instanceof operations since instanceof returns false for null.";
+    }
+
+    @Override
+    public Set<String> getTags() {
+        return Collections.singleton("RSPEC-S1697");
+    }
+
+    @Override
+    public Duration getEstimatedEffortPerOccurrence() {
+        return Duration.ofMinutes(1);
+    }
+
+    @Override
+    public TreeVisitor<?, ExecutionContext> getVisitor() {
+        return new JavaVisitor<ExecutionContext>() {
+            @Override
+            public J visitBinary(J.Binary binary, ExecutionContext ctx) {
+                J.Binary bi = (J.Binary) super.visitBinary(binary, ctx);
+                if (bi.getOperator() != J.Binary.Type.And) {
+                    return bi;
+                }
+
+                Expression left = bi.getLeft();
+                Expression right = bi.getRight();
+
+                // Check if we have a pattern: (expr != null) && (expr instanceof Type)
+                if (left instanceof J.Binary && right instanceof J.InstanceOf) {
+                    J.Binary nullCheck = (J.Binary) left;
+                    J.InstanceOf instanceOf = (J.InstanceOf) right;
+
+                    if (isRedundantNullCheck(nullCheck, instanceOf)) {
+                        // Return just the instanceof check
+                        return instanceOf.withPrefix(bi.getPrefix());
+                    }
+                }
+
+                // Check if we have chained patterns like:
+                // (... && expr != null) && (expr instanceof Type)
+                if (left instanceof J.Binary && ((J.Binary) left).getOperator() == J.Binary.Type.And &&
+                        right instanceof J.InstanceOf) {
+                    J.Binary leftBinary = (J.Binary) left;
+                    J.InstanceOf instanceOf = (J.InstanceOf) right;
+
+                    // Check if the rightmost part of left is a null check for the same expression
+                    Expression rightmostOfLeft = leftBinary.getRight();
+                    if (rightmostOfLeft instanceof J.Binary &&
+                            isRedundantNullCheck((J.Binary) rightmostOfLeft, instanceOf)) {
+                        // Remove the null check from the left side
+                        return bi.withLeft(leftBinary.getLeft()).withRight(instanceOf);
+                    }
+                }
+
+                return bi;
+            }
+
+            private boolean isRedundantNullCheck(J.Binary nullCheck, J.InstanceOf instanceOf) {
+                if (nullCheck.getOperator() == J.Binary.Type.NotEqual) {
+                    if (J.Literal.isLiteralValue(nullCheck.getLeft(), null)) {
+                        return SemanticallyEqual.areEqual(nullCheck.getRight(), instanceOf.getExpression());
+                    }
+                    if (J.Literal.isLiteralValue(nullCheck.getRight(), null)) {
+                        return SemanticallyEqual.areEqual(nullCheck.getLeft(), instanceOf.getExpression());
+                    }
+                }
+                return false;
+            }
+        };
+    }
+}