[java-intentions] Suggest 'replace with null-check' fix if pattern variable is present but unused

Fixes IDEA-265543 Pattern type is the same as expression type error missing fix

GitOrigin-RevId: a971caf5480cd29e50cfc271ae76891fbc750d6b

java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java

@@ -17,6 +17,7 @@ import com.intellij.codeInsight.intention.QuickFixFactory;
 import com.intellij.codeInsight.intention.impl.PriorityIntentionActionWrapper;
 import com.intellij.codeInsight.quickfix.UnresolvedReferenceQuickFixProvider;
 import com.intellij.codeInspection.LocalQuickFixOnPsiElementAsIntentionAdapter;
+import com.intellij.codeInspection.dataFlow.fix.RedundantInstanceofFix;
 import com.intellij.core.JavaPsiBundle;
 import com.intellij.ide.IdeBundle;
 import com.intellij.java.analysis.JavaAnalysisBundle;
@@ -63,6 +64,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.MultiMap;
 import com.intellij.util.ui.UIUtil;
 import com.siyeh.ig.psiutils.ControlFlowUtils;
+import com.siyeh.ig.psiutils.VariableAccessUtils;
 import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -1689,7 +1691,9 @@ public final class HighlightUtil {
 
   public static HighlightInfo checkInstanceOfPatternSupertype(PsiInstanceOfExpression expression) {
     PsiTypeTestPattern pattern = ObjectUtils.tryCast(expression.getPattern(), PsiTypeTestPattern.class);
-    if (pattern == null || pattern.getPatternVariable() == null) return null;
+    if (pattern == null) return null;
+    PsiPatternVariable variable = pattern.getPatternVariable();
+    if (variable == null) return null;
     PsiTypeElement typeElement = pattern.getCheckType();
     PsiType checkType = typeElement.getType();
     PsiType expressionType = expression.getOperand().getType();
@@ -1698,7 +1702,12 @@ public final class HighlightUtil {
         checkType.equals(expressionType) ?
         JavaErrorBundle.message("instanceof.pattern.equals", checkType.getPresentableText()) :
         JavaErrorBundle.message("instanceof.pattern.supertype", checkType.getPresentableText(), expressionType.getPresentableText());
-      return HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(typeElement).descriptionAndTooltip(description).create();
+      HighlightInfo info =
+        HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(typeElement).descriptionAndTooltip(description).create();
+      if (!VariableAccessUtils.variableIsUsed(variable, variable.getDeclarationScope())) {
+        QuickFixAction.registerQuickFixAction(info, new RedundantInstanceofFix(expression));
+      }
+      return info;
     }
     return null;
   }

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java

@@ -311,11 +311,12 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
         InstanceofInstruction instanceOf = (InstanceofInstruction)instruction;
         if (visitor.isInstanceofRedundant(instanceOf)) {
           PsiExpression expression = instanceOf.getExpression();
-          if (expression != null && 
-              (!JavaPsiPatternUtil.getExposedPatternVariables(expression).isEmpty() || shouldBeSuppressed(expression))) continue;
+          if (expression == null || shouldBeSuppressed(expression)) continue;
+          if (JavaPsiPatternUtil.getExposedPatternVariables(expression).stream()
+            .anyMatch(var -> VariableAccessUtils.variableIsUsed(var, var.getDeclarationScope()))) continue;
           reporter.registerProblem(expression,
                                    JavaAnalysisBundle.message("dataflow.message.redundant.instanceof"),
-                                   new RedundantInstanceofFix());
+                                   new RedundantInstanceofFix(expression));
         }
       }
     }
@@ -881,6 +882,13 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
     if (expression instanceof PsiInstanceOfExpression) {
       PsiType type = ((PsiInstanceOfExpression)expression).getOperand().getType();
       if (type == null || !TypeConstraints.instanceOf(type).isResolved()) return true;
+      PsiPattern pattern = ((PsiInstanceOfExpression)expression).getPattern();
+      if (pattern instanceof PsiTypeTestPattern && ((PsiTypeTestPattern)pattern).getPatternVariable() != null) {
+        if (((PsiTypeTestPattern)pattern).getCheckType().getType().isAssignableFrom(type)) {
+          // Reported as compilation error
+          return true;
+        }
+      }
     }
     PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent());
     // Don't report "x" in "x == null" as will be anyways reported as "always true"

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java

@@ -15,9 +15,9 @@
  */
 package com.intellij.codeInspection.dataFlow.fix;
 
-import com.intellij.codeInspection.LocalQuickFix;
-import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement;
 import com.intellij.java.analysis.JavaAnalysisBundle;
+import com.intellij.openapi.editor.Editor;
 import com.intellij.openapi.project.Project;
 import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
@@ -26,11 +26,13 @@ import com.intellij.util.ArrayUtil;
 import com.siyeh.ig.psiutils.BoolUtils;
 import com.siyeh.ig.psiutils.CommentTracker;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+public class RedundantInstanceofFix extends LocalQuickFixAndIntentionActionOnPsiElement {
+  public RedundantInstanceofFix(@Nullable PsiElement element) {
+    super(element);
+  }
 
-/**
- * @author peter
- */
-public class RedundantInstanceofFix implements LocalQuickFix {
   @Override
   @NotNull
   public String getFamilyName() {
@@ -38,8 +40,16 @@ public class RedundantInstanceofFix implements LocalQuickFix {
   }
 
   @Override
-  public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
-    PsiElement psiElement = descriptor.getPsiElement();
+  public @NotNull String getText() {
+    return getFamilyName();
+  }
+
+  @Override
+  public void invoke(@NotNull Project project,
+                     @NotNull PsiFile file,
+                     @Nullable Editor editor,
+                     @NotNull PsiElement startElement, @NotNull PsiElement endElement) {
+    PsiElement psiElement = startElement;
     CommentTracker ct = new CommentTracker();
     if (psiElement instanceof PsiMethodReferenceExpression) {
       String replacement = CommonClassNames.JAVA_UTIL_OBJECTS + "::nonNull";

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPattern.java

@@ -0,0 +1,9 @@
+// "Replace with a null check" "true"
+class Test {
+  void test(String s) {
+    Object object = s;
+    if(object != null) {
+      System.out.println("always");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPatternCompileError.java

@@ -0,0 +1,8 @@
+// "Replace with a null check" "true"
+class Test {
+  void test(String s) {
+    if(s != null) {
+      System.out.println("always");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPattern.java

@@ -0,0 +1,9 @@
+// "Replace with a null check" "true"
+class Test {
+  void test(String s) {
+    Object object = s;
+    if(object instanceof <caret>String s1) {
+      System.out.println("always");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileError.java

@@ -0,0 +1,8 @@
+// "Replace with a null check" "true"
+class Test {
+  void test(String s) {
+    if(s instanceof <caret>String s1) {
+      System.out.println("always");
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileErrorUsed.java

@@ -0,0 +1,8 @@
+// "Replace with a null check" "false"
+class Test {
+  void test(String s) {
+    if(s instanceof <caret>String s1) {
+      System.out.println("always: " + s1);
+    }
+  }
+}

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternUsed.java

@@ -0,0 +1,9 @@
+// "Replace with a null check" "false"
+class Test {
+  void test(String s) {
+    Object object = s;
+    if(object instanceof <caret>String s1) {
+      System.out.println("always: " + s1);
+    }
+  }
+}

java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java

@@ -7,19 +7,25 @@ public class InstanceOfPattern {
       System.out.println(s.length());
     }
   }
-  
+
   void test(Object obj) {
     if (obj instanceof Number n) {
       if (<warning descr="Condition 'n == obj' is always 'true'">n == obj</warning>) {}
     }
   }
-  
+
   void testNullCheck(String s) {
     if (s instanceof <error descr="Pattern type 'String' is the same as expression type">String</error> s1) {
       System.out.println(s1);
     }
   }
-  
+
+  void testNullCheckUnusedPatternVariable(String s) {
+    if (s instanceof <error descr="Pattern type 'String' is the same as expression type">String</error> s1) {
+      System.out.println("foo");
+    }
+  }
+
   interface Foo {
     @Nullable Object bar();
   }

java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/RedundantInstanceofFixTest.java

@@ -17,7 +17,7 @@ public class RedundantInstanceofFixTest extends LightQuickFixParameterizedTestCa
   @NotNull
   @Override
   protected LightProjectDescriptor getProjectDescriptor() {
-    return LightJavaCodeInsightFixtureTestCase.JAVA_8;
+    return LightJavaCodeInsightFixtureTestCase.JAVA_16;
   }
 
   @Override