[java-inspection] IDEA-310469 Java inspection for "Pattern variable can be used" does not find all cases

- support more cases

GitOrigin-RevId: 28301b79d4c4acd343a35474d1ab44096dcea8cd

java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties

@@ -2334,11 +2334,16 @@ inspection.enhanced.for.with.record.pattern.can.be.used.maximum.depth.disabled=D
 
 inspection.pattern.variable.can.be.used.display.name=Pattern variable can be used
 inspection.pattern.variable.can.be.used.message=Variable ''{0}'' can be replaced with pattern variable
+inspection.pattern.variable.can.be.used.instead.of.cast.message=Cast expression can be replaced with pattern variable
 inspection.pattern.variable.can.be.used.fix.family.name=Replace with pattern variable
+inspection.pattern.variable.instead.of.cast.can.be.used.fix.family.name=Replace cast expressions with pattern variable
 inspection.pattern.variable.can.be.used.fix.name=Replace ''{0}'' with pattern variable
 inspection.pattern.variable.can.be.used.existing.message=Existing pattern variable ''{0}'' can be used instead of ''{1}''
+inspection.pattern.variable.can.be.used.existing.cast.message=Existing pattern variable ''{0}'' can be used instead of cast expression
 inspection.pattern.variable.can.be.used.existing.fix.family.name=Replace with existing pattern variable
 inspection.pattern.variable.can.be.used.existing.fix.name=Replace ''{0}'' with existing pattern variable ''{1}''
+inspection.pattern.variable.instead.of.cast.can.be.used.existing.fix.name=Replace cast expression with existing pattern variable ''{0}''
+inspection.pattern.variable.can.be.used.report.cast.only=Report cast expressions which can be replaced with new pattern variables
 
 inspection.cast.can.be.replaced.with.variable.display.name=Cast can be replaced with variable
 inspection.cast.can.be.replaced.with.variable.message=Variable ''{0}'' can be used instead of ''{1}''

java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java

@@ -1,7 +1,8 @@
-// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
+// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
 package com.intellij.codeInspection;
 
 import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
+import com.intellij.codeInspection.options.OptPane;
 import com.intellij.modcommand.ModPsiUpdater;
 import com.intellij.modcommand.PsiUpdateModCommandQuickFix;
 import com.intellij.openapi.project.Project;
@@ -23,11 +24,23 @@ import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
+import static com.intellij.codeInspection.options.OptPane.checkbox;
+
 public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLocalInspectionTool implements CleanupLocalInspectionTool {
 
+  @SuppressWarnings("PublicField")
+  public boolean reportOnCastOnly = false;
+
+  @Override
+  public @NotNull OptPane getOptionsPane() {
+    return OptPane.pane(checkbox("reportOnCastOnly",
+                                 InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.report.cast.only")));
+  }
+
   @Override
   public @NotNull Set<@NotNull JavaFeature> requiredFeatures() {
     return Set.of(JavaFeature.PATTERNS);
@@ -118,6 +131,42 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
         return null;
       }
 
+      @Override
+      public void visitTypeCastExpression(@NotNull PsiTypeCastExpression expression) {
+        InstanceOfCandidateResult result = findInstanceOfCandidateResult(expression);
+        if (result == null) return;
+        if (result.instanceOf() != null) {
+          PsiPattern pattern = result.instanceOf().getPattern();
+          PsiPatternVariable existingPatternVariable = JavaPsiPatternUtil.getPatternVariable(pattern);
+          if (pattern != null && existingPatternVariable == null) {
+            return;
+          }
+          if (existingPatternVariable != null) {
+            if (!isFinalOrEffectivelyFinal(existingPatternVariable)) {
+              return;
+            }
+            holder.registerProblem(result.castTypeElement(),
+                                   InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.cast.message",
+                                                                   existingPatternVariable.getName()),
+                                   new ExistingPatternVariableCanBeUsedFix(null, existingPatternVariable));
+          }
+          else {
+            if (!reportOnCastOnly) {
+              if (isOnTheFly) {
+                holder.registerProblem(expression,
+                                       InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.instead.of.cast.message"),
+                                       ProblemHighlightType.INFORMATION,
+                                       new CastExpressionsCanBeReplacedWithPatternVariableFix(result.instanceOf()));
+              }
+              return;
+            }
+            holder.registerProblem(result.castTypeElement(),
+                                   InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.instead.of.cast.message"),
+                                   new CastExpressionsCanBeReplacedWithPatternVariableFix(result.instanceOf()));
+          }
+        }
+      }
+
       @Override
       public void visitLocalVariable(@NotNull PsiLocalVariable variable) {
         PsiIdentifier identifier = variable.getNameIdentifier();
@@ -164,15 +213,20 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
           }
         }
       }
-
     };
   }
 
   private static class ExistingPatternVariableCanBeUsedFix extends PsiUpdateModCommandQuickFix {
-    private final @NotNull String myName;
+    private final @Nullable String myName;
     private final @NotNull String myPatternName;
 
-    private ExistingPatternVariableCanBeUsedFix(@NotNull String name, @NotNull PsiPatternVariable existingVariable) {
+    /**
+     * Creates a fix for using an existing pattern variable.
+     *
+     * @param name             The name of the variable being replaced (null if there is no such a variable, only cast expressions)
+     * @param existingVariable The existing pattern variable to use
+     */
+    private ExistingPatternVariableCanBeUsedFix(@Nullable String name, @NotNull PsiPatternVariable existingVariable) {
       myName = name;
       myPatternName = existingVariable.getName();
     }
@@ -181,9 +235,12 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
     @NotNull
     @Override
     public String getName() {
-      return InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.fix.name", myName, myPatternName);
+      if (myName != null) {
+        return InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.fix.name", myName, myPatternName);
+      }
+      return InspectionGadgetsBundle.message("inspection.pattern.variable.instead.of.cast.can.be.used.existing.fix.name", myPatternName);
     }
-    
+
     @Nls(capitalization = Nls.Capitalization.Sentence)
     @NotNull
     @Override
@@ -193,7 +250,15 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
 
     @Override
     protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) {
-      if (!myName.endsWith("()")) {
+      if (myName == null) {
+        PsiElement castExpression = PsiTreeUtil.getParentOfType(element, PsiTypeCastExpression.class);
+        if (castExpression == null) return;
+        while (castExpression.getParent() instanceof PsiParenthesizedExpression parenthesizedExpression) {
+          castExpression = parenthesizedExpression;
+        }
+        new CommentTracker().replace(castExpression, myPatternName);
+      }
+      else if (!myName.endsWith("()")) {
         PsiLocalVariable variable = PsiTreeUtil.getParentOfType(element, PsiLocalVariable.class);
         if (variable == null) return;
         if (VariableAccessUtils.variableIsAssigned(variable)) {
@@ -212,8 +277,93 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
     }
   }
 
-  private static class PatternVariableCanBeUsedFix extends PsiUpdateModCommandQuickFix {
+  private static class CastExpressionsCanBeReplacedWithPatternVariableFix extends PsiUpdateModCommandQuickFix {
+
+    @NotNull
     private final SmartPsiElementPointer<PsiInstanceOfExpression> myInstanceOfPointer;
+
+    private CastExpressionsCanBeReplacedWithPatternVariableFix(@NotNull PsiInstanceOfExpression instanceOf) {
+      myInstanceOfPointer = SmartPointerManager.createPointer(instanceOf);
+    }
+
+    @Override
+    public @NotNull String getFamilyName() {
+      return InspectionGadgetsBundle.message("inspection.pattern.variable.instead.of.cast.can.be.used.fix.family.name");
+    }
+
+    @Override
+    protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) {
+      PsiTypeCastExpression castExpression = PsiTreeUtil.getParentOfType(element, false, PsiTypeCastExpression.class);
+      if (castExpression == null) return;
+      PsiTypeElement originalTypeElement = castExpression.getCastType();
+      if (originalTypeElement == null) return;
+      PsiInstanceOfExpression originalInstanceOf = myInstanceOfPointer.getElement();
+      if (originalInstanceOf == null) return;
+      PsiInstanceOfExpression instanceOf = PsiTreeUtil.findSameElementInCopy(originalInstanceOf, element.getContainingFile());
+      if (instanceOf.getPattern() instanceof PsiDeconstructionPattern) return;
+      PsiTypeElement instanceOfType = instanceOf.getCheckType();
+      PsiTypeElement typeElement = originalTypeElement;
+      if (instanceOfType != null && instanceOfType.getType() instanceof PsiClassType classType && !classType.isRaw()) {
+        typeElement = instanceOfType;
+      }
+      PsiIfStatement psiIfStatement = PsiTreeUtil.getParentOfType(instanceOf, PsiIfStatement.class);
+      if (psiIfStatement == null) return;
+      var visitor = new JavaRecursiveElementVisitor() {
+        final List<PsiTypeCastExpression> myCasts = new ArrayList<>();
+
+        @Override
+        public void visitTypeCastExpression(@NotNull PsiTypeCastExpression expression) {
+          PsiTypeElement castType = expression.getCastType();
+          if (castType == null) return;
+          if (!castType.textMatches(originalTypeElement)) {
+            return;
+          }
+          InstanceOfCandidateResult result = findInstanceOfCandidateResult(expression);
+          if (result != null && result.instanceOf == instanceOf) {
+            myCasts.add(expression);
+          }
+        }
+      };
+
+      psiIfStatement.accept(visitor);
+      List<PsiTypeCastExpression> casts = visitor.myCasts;
+      if (casts.isEmpty()) return;
+
+      CommentTracker ct = new CommentTracker();
+      StringBuilder text = generateTextForInstanceOf(null, ct, instanceOf, typeElement);
+      if (text == null) return;
+      PsiElement replaced = ct.replace(instanceOf, text.toString());
+      if (!(replaced instanceof PsiInstanceOfExpression instanceOfExpression)) {
+        return;
+      }
+      PsiPrimaryPattern pattern = instanceOfExpression.getPattern();
+      if (!(pattern instanceof PsiTypeTestPattern typeTestPattern)) {
+        return;
+      }
+      PsiPatternVariable patternVariable = typeTestPattern.getPatternVariable();
+      if (patternVariable == null) return;
+      String variableName = patternVariable.getName();
+      for (PsiTypeCastExpression cast : casts) {
+        PsiElement currentCast = cast;
+        CommentTracker castCt = new CommentTracker();
+        while (currentCast.getParent() instanceof PsiParenthesizedExpression parenthesizedExpression) {
+          currentCast = parenthesizedExpression;
+        }
+        castCt.replace(currentCast, variableName);
+      }
+      final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(project);
+      List<String> names = new VariableNameGenerator(instanceOfExpression, VariableKind.LOCAL_VARIABLE)
+        .byType(patternVariable.getType()).byName(
+          codeStyleManager.suggestUniqueVariableName(variableName, instanceOfExpression, true)).generateAll(true);
+      names.remove(patternVariable.getName());
+      updater.rename(patternVariable, names);
+    }
+  }
+
+  private static class PatternVariableCanBeUsedFix extends PsiUpdateModCommandQuickFix {
+    @NotNull
+    private final SmartPsiElementPointer<PsiInstanceOfExpression> myInstanceOfPointer;
+    @NotNull
     private final String myName;
 
     private PatternVariableCanBeUsedFix(@NotNull String name, @NotNull PsiInstanceOfExpression instanceOf) {
@@ -237,7 +387,7 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
 
     @Override
     protected void applyFix(@NotNull Project project, @NotNull PsiElement element, @NotNull ModPsiUpdater updater) {
-      if (!(element.getParent()  instanceof PsiLocalVariable variable)) return;
+      if (!(element.getParent() instanceof PsiLocalVariable variable)) return;
       PsiTypeCastExpression cast = ObjectUtils.tryCast(PsiUtil.skipParenthesizedExprDown(variable.getInitializer()),
                                                        PsiTypeCastExpression.class);
       if (cast == null) return;
@@ -250,22 +400,8 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
         typeElement = instanceOfType;
       }
       CommentTracker ct = new CommentTracker();
-      StringBuilder text = new StringBuilder(ct.text(instanceOf.getOperand()));
-      text.append(" instanceof ");
-      PsiModifierList modifierList = variable.getModifierList();
-      JavaCodeStyleSettings codeStyleSettings = JavaCodeStyleSettings.getInstance(variable.getContainingFile());
-      if (modifierList != null && modifierList.getTextLength() > 0) {
-        modifierList.setModifierProperty(PsiModifier.FINAL, codeStyleSettings.GENERATE_FINAL_LOCALS);
-        text.append(ct.text(modifierList)).append(' ');
-      }
-      else if (codeStyleSettings.GENERATE_FINAL_LOCALS) {
-        text.append("final ");
-      }
-      text.append(typeElement.getText()).append(' ');
-      if (instanceOf.getPattern() instanceof PsiDeconstructionPattern) {
-        return;
-      }
-      text.append(variable.getName());
+      StringBuilder text = generateTextForInstanceOf(variable, ct, instanceOf, typeElement);
+      if (text == null) return;
       PsiElement replaced = ct.replace(instanceOf, text.toString());
       ct.deleteAndRestoreComments(variable);
       if (!(replaced instanceof PsiInstanceOfExpression instanceOfExpression)) {
@@ -288,4 +424,51 @@ public final class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLo
       updater.rename(patternVariable, names);
     }
   }
+
+  private static @Nullable InstanceOfCandidateResult findInstanceOfCandidateResult(@NotNull PsiTypeCastExpression expression) {
+    PsiTypeElement castTypeElement = expression.getCastType();
+    if (castTypeElement == null) return null;
+    PsiExpression operand = expression.getOperand();
+    if (operand == null) return null;
+    PsiType castType = castTypeElement.getType();
+    if (castType instanceof PsiPrimitiveType) return null;
+    PsiType operandType = operand.getType();
+    if (operandType == null || castType.isAssignableFrom(operandType)) return null;
+    PsiInstanceOfExpression instanceOf = InstanceOfUtils.findPatternCandidate(expression, null);
+    return new InstanceOfCandidateResult(castTypeElement, instanceOf);
+  }
+
+  private record InstanceOfCandidateResult(@NotNull PsiTypeElement castTypeElement,
+                                           @Nullable PsiInstanceOfExpression instanceOf) {
+  }
+
+  private static @Nullable StringBuilder generateTextForInstanceOf(@Nullable PsiLocalVariable variable,
+                                                                   @NotNull CommentTracker ct,
+                                                                   @NotNull PsiInstanceOfExpression instanceOf,
+                                                                   @NotNull PsiTypeElement typeElement) {
+    StringBuilder text = new StringBuilder(ct.text(instanceOf.getOperand()));
+    text.append(" instanceof ");
+    PsiModifierList modifierList = variable != null ? variable.getModifierList() : null;
+    JavaCodeStyleSettings codeStyleSettings = JavaCodeStyleSettings.getInstance(typeElement.getContainingFile());
+    if (modifierList != null && modifierList.getTextLength() > 0) {
+      modifierList.setModifierProperty(PsiModifier.FINAL, codeStyleSettings.GENERATE_FINAL_LOCALS);
+      text.append(ct.text(modifierList)).append(' ');
+    }
+    else if (codeStyleSettings.GENERATE_FINAL_LOCALS) {
+      text.append("final ");
+    }
+    text.append(typeElement.getText()).append(' ');
+    if (instanceOf.getPattern() instanceof PsiDeconstructionPattern) {
+      return null;
+    }
+    if (variable == null) {
+      String name = new VariableNameGenerator(instanceOf, VariableKind.LOCAL_VARIABLE)
+        .byType(typeElement.getType()).generate(true);
+      text.append(name);
+    }
+    else {
+      text.append(variable.getName());
+    }
+    return text;
+  }
 }

java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCast.java

@@ -0,0 +1,12 @@
+// "Replace cast expression with existing pattern variable 'i'" "true-preview"
+import java.util.*;
+
+class X {
+  void test(Object obj) {
+    if (obj instanceof Integer i) {
+      doSomething(i);
+    }
+  }
+
+  void doSomething(Integer i) {}
+}

java/java-tests/testData/inspection/patternVariableCanBeUsed/afterExistingCastInsideCondition.java

@@ -0,0 +1,11 @@
+// "Replace cast expression with existing pattern variable 'i'" "true-preview"
+
+class X {
+  void test(Object obj) {
+    if (obj instanceof Integer i && i.intValue() == 1) {
+      doSomething(1);
+    }
+  }
+
+  void doSomething(Integer i) {}
+}

java/java-tests/testData/inspection/patternVariableCanBeUsed/afterReplaceCastsWithNewName.java

@@ -0,0 +1,12 @@
+// "Replace cast expressions with pattern variable" "true"
+import java.util.*;
+
+class X {
+  void test(Object obj) {
+    if (obj instanceof Integer integer && integer.intValue() == 1) {
+      doSomething(integer);
+    }
+  }
+
+  void doSomething(Integer i) {}
+}

java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCast.java

@@ -0,0 +1,12 @@
+// "Replace cast expression with existing pattern variable 'i'" "true-preview"
+import java.util.*;
+
+class X {
+  void test(Object obj) {
+    if (obj instanceof Integer i) {
+      doSomething((In<caret>teger)obj);
+    }
+  }
+
+  void doSomething(Integer i) {}
+}

java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeExistingCastInsideCondition.java

@@ -0,0 +1,11 @@
+// "Replace cast expression with existing pattern variable 'i'" "true-preview"
+
+class X {
+  void test(Object obj) {
+    if (obj instanceof Integer i && ((Intege<caret>r)obj).intValue() == 1) {
+      doSomething(1);
+    }
+  }
+
+  void doSomething(Integer i) {}
+}

java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeReplaceCastsWithNewName.java

@@ -0,0 +1,12 @@
+// "Replace cast expressions with pattern variable" "true"
+import java.util.*;
+
+class X {
+  void test(Object obj) {
+    if (obj instanceof Integer && ((Integer)obj).intValue() == 1) {
+      doSomething((In<caret>t<caret>eger)obj);
+    }
+  }
+
+  void doSomething(Integer i) {}
+}

java/java-tests/testSrc/com/intellij/java/codeInspection/PatternVariableCanBeUsedInspectionTest.java

@@ -12,7 +12,9 @@ import org.jetbrains.annotations.NotNull;
 public class PatternVariableCanBeUsedInspectionTest extends LightQuickFixParameterizedTestCase {
   @Override
   protected LocalInspectionTool @NotNull [] configureLocalInspectionTools() {
-    return new LocalInspectionTool[]{new PatternVariableCanBeUsedInspection()};
+    PatternVariableCanBeUsedInspection inspection = new PatternVariableCanBeUsedInspection();
+    inspection.reportOnCastOnly = true;
+    return new LocalInspectionTool[]{inspection};
   }
 
   @Override