Add recipe ReplaceClassIsInstanceWithInstanceof for SonarQube RSPEC…

…-6202 (openrewrite#381) * add: recipe for RSPEC-S1170, adding "static" to "public final" constants and fields * add: recipe and unit test for RSPEC-S6202 * update: add recipe to list * fix: correct the name of the recipe * update: for more cases in unit test * update: correct copyright messages * refactor: format code * Update src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/main/java/org/openrewrite/staticanalysis/AddStaticModifierToPublicFinalConstantsAndFields.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Test additional cases pointed out in review * Format tests * Slight polish to ReplaceClassIsInstanceWithInstanceof * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * update: Recipe for RSPEC-S1170 is removed because it needs further consideration, so we will do it in another PR * Minor polish --------- Co-authored-by: Sheng Yu <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek <[email protected]> Co-authored-by: Tim te Beek <[email protected]>
adambir · Oct 28, 2024 · 286d4ee · 286d4ee
1 parent be381dc
commit 286d4ee
Show file tree

Hide file tree

Showing 3 changed files with 203 additions and 0 deletions.
diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceof.java
@@ -0,0 +1,86 @@
+/*
+ * Copyright 2024 the original author or authors.
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (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://www.apache.org/licenses/LICENSE-2.0
+ * <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 org.jspecify.annotations.Nullable;
+import org.openrewrite.ExecutionContext;
+import org.openrewrite.Recipe;
+import org.openrewrite.java.JavaTemplate;
+import org.openrewrite.java.JavaVisitor;
+import org.openrewrite.java.MethodMatcher;
+import org.openrewrite.java.tree.Expression;
+import org.openrewrite.java.tree.J;
+import org.openrewrite.java.tree.J.FieldAccess;
+import org.openrewrite.java.tree.J.Identifier;
+import org.openrewrite.java.tree.J.MethodInvocation;
+import org.openrewrite.java.tree.JavaType;
+
+import java.util.Collections;
+import java.util.Set;
+
+public class ReplaceClassIsInstanceWithInstanceof extends Recipe {
+
+    @Override
+    public String getDisplayName() {
+        return "Replace `A.class.isInstance(a)` with `a instanceof A`";
+    }
+
+    @Override
+    public String getDescription() {
+        return "There should be no `A.class.isInstance(a)`, it should be replaced by `a instanceof A`.";
+    }
+
+    @Override
+    public Set<String> getTags() {
+        return Collections.singleton("RSPEC-S6202");
+    }
+
+    @Override
+    public JavaVisitor<ExecutionContext> getVisitor() {
+        // use JavaVisitor instead of JavaIsoVisitor because we changed the type of LST
+        return new JavaVisitor<ExecutionContext>() {
+
+            private final MethodMatcher matcher = new MethodMatcher("java.lang.Class isInstance(java.lang.Object)");
+
+            @Override
+            public J visitMethodInvocation(MethodInvocation method, ExecutionContext ctx) {
+                // make sure we find the right method and the left part is something like "SomeClass.class"
+                if (matcher.matches(method) && isObjectClass(method.getSelect())) {
+                    // for code like "A.class.isInstance(a)", select is "String.class", name is "isInstance", argument is "a"
+                    Identifier objectExpression = (Identifier) method.getArguments().get(0);
+                    FieldAccess fieldAccessPart = (FieldAccess) method.getSelect();
+                    String className = ((JavaType.Class) fieldAccessPart.getTarget().getType()).getClassName();
+                    // upcast to type J, so J.MethodInvocation can be replaced by J.InstanceOf
+                    J.InstanceOf instanceOf = JavaTemplate.apply("#{any(org.openrewrite.java.tree.Expression)} instanceof #{}",
+                            getCursor(), method.getCoordinates().replace(), objectExpression, className);
+                    return maybeAutoFormat(method, instanceOf, ctx);
+                }
+                return super.visitMethodInvocation(method, ctx);
+            }
+
+            private boolean isObjectClass(@Nullable Expression expression) {
+                if (expression instanceof J.FieldAccess) {
+                    J.FieldAccess fieldAccess = (J.FieldAccess) expression;
+                    if (fieldAccess.getTarget() instanceof Identifier) {
+                        Identifier identifier = (Identifier) fieldAccess.getTarget();
+                        return identifier.getType() instanceof JavaType.Class;
+                    }
+                }
+                return false;
+            }
+        };
+    }
+}
diff --git a/src/main/resources/META-INF/rewrite/common-static-analysis.yml b/src/main/resources/META-INF/rewrite/common-static-analysis.yml
@@ -72,6 +72,7 @@ recipeList:
   - org.openrewrite.staticanalysis.RenameLocalVariablesToCamelCase
   - org.openrewrite.staticanalysis.RenameMethodsNamedHashcodeEqualOrToString
   - org.openrewrite.staticanalysis.RenamePrivateFieldsToCamelCase
+  - org.openrewrite.staticanalysis.ReplaceClassIsInstanceWithInstanceof
   - org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference
   - org.openrewrite.staticanalysis.ReplaceStringBuilderWithString
   - org.openrewrite.staticanalysis.SimplifyBooleanExpression

diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceClassIsInstanceWithInstanceofTest.java
@@ -0,0 +1,116 @@
+/*
+ * Copyright 2024 the original author or authors.
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (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://www.apache.org/licenses/LICENSE-2.0
+ * <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 org.junit.jupiter.api.Test;
+import org.openrewrite.DocumentExample;
+import org.openrewrite.test.RecipeSpec;
+import org.openrewrite.test.RewriteTest;
+
+import static org.openrewrite.java.Assertions.java;
+import static org.openrewrite.java.Assertions.javaVersion;
+
+@SuppressWarnings({"RedundantClassCall", "ConstantValue", "UnusedAssignment"})
+class ReplaceClassIsInstanceWithInstanceofTest implements RewriteTest {
+
+    @Override
+    public void defaults(RecipeSpec spec) {
+        spec.recipe(new ReplaceClassIsInstanceWithInstanceof());
+    }
+
+    @Test
+    @DocumentExample
+    void changeInstanceOf() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class A {
+                  void foo() {
+                      String s = "";
+                      boolean result = String.class.isInstance(s);
+                      result = Integer.class.isInstance(s);
+                  }
+              }
+              """,
+            """
+              class A {
+                  void foo() {
+                      String s = "";
+                      boolean result = s instanceof String;
+                      result = s instanceof Integer;
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void doNotChangeWhenAlreadyInstanceOf() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class A {
+                  boolean foo() {
+                    String s = "";
+                    return s instanceof String;
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void doNotChangeWhenVariable() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class A {
+                  void foo(Class<?> clazz) {
+                      String s = "";
+                      boolean result = clazz.isInstance(s);
+                  }
+              }
+              """
+          )
+        );
+    }
+
+    @Test
+    void doNotChangeInstanceOfWithVariable() {
+        rewriteRun(
+          //language=java
+          java(
+            """
+              class A {
+                  String foo(Object obj) {
+                      if (obj instanceof String s) {
+                          return s;
+                      }
+                      return null;
+                  }
+              }
+              """,
+            spec -> spec.markers(javaVersion(17))
+          )
+        );
+    }
+
+}