From a0a8ed752b24947ce765bf9a3f88ca769c1baae4 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 1 Apr 2021 12:17:05 +0700 Subject: [PATCH] [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 --- .../daemon/impl/analysis/HighlightUtil.java | 13 ++++++++-- .../dataFlow/DataFlowInspectionBase.java | 14 +++++++--- .../dataFlow/fix/RedundantInstanceofFix.java | 26 +++++++++++++------ .../afterInstanceOfPattern.java | 9 +++++++ .../afterInstanceOfPatternCompileError.java | 8 ++++++ .../beforeInstanceOfPattern.java | 9 +++++++ .../beforeInstanceOfPatternCompileError.java | 8 ++++++ ...foreInstanceOfPatternCompileErrorUsed.java | 8 ++++++ .../beforeInstanceOfPatternUsed.java | 9 +++++++ .../dataFlow/fixture/InstanceOfPattern.java | 12 ++++++--- .../quickFix/RedundantInstanceofFixTest.java | 2 +- 11 files changed, 101 insertions(+), 17 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPattern.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPatternCompileError.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPattern.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileError.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileErrorUsed.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternUsed.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java index ed6dc1398d7a..91a16b0f17d9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightUtil.java +++ b/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; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 43ab90e9ce24..a091eb4d49a5 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/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" diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java index 09acda0ae086..05ec827f8170 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java +++ b/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"; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPattern.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPattern.java new file mode 100644 index 000000000000..49660285d9fc --- /dev/null +++ b/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"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPatternCompileError.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfPatternCompileError.java new file mode 100644 index 000000000000..01a95551b790 --- /dev/null +++ b/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"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPattern.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPattern.java new file mode 100644 index 000000000000..d5b3d8ff7bf2 --- /dev/null +++ b/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 String s1) { + System.out.println("always"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileError.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileError.java new file mode 100644 index 000000000000..0099ed72afbd --- /dev/null +++ b/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 String s1) { + System.out.println("always"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileErrorUsed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternCompileErrorUsed.java new file mode 100644 index 000000000000..342865888b9e --- /dev/null +++ b/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 String s1) { + System.out.println("always: " + s1); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternUsed.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfPatternUsed.java new file mode 100644 index 000000000000..5046e7488e54 --- /dev/null +++ b/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 String s1) { + System.out.println("always: " + s1); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java b/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java index 5a7fa0c513b8..a1b71ec96912 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/InstanceOfPattern.java +++ b/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 (n == obj) {} } } - + void testNullCheck(String s) { if (s instanceof String s1) { System.out.println(s1); } } - + + void testNullCheckUnusedPatternVariable(String s) { + if (s instanceof String s1) { + System.out.println("foo"); + } + } + interface Foo { @Nullable Object bar(); } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/RedundantInstanceofFixTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/RedundantInstanceofFixTest.java index 5d7cb7084e92..d4ccba1a3280 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/quickFix/RedundantInstanceofFixTest.java +++ b/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