From 39e0598cb7e9b780ef39afdc04da8f691898ae03 Mon Sep 17 00:00:00 2001 From: "Andrey.Cherkasov" Date: Thu, 15 Sep 2022 12:51:18 +0400 Subject: [PATCH] [java-inspections] PatternVariableCanBeUsed: false positives IDEA-301287 GitOrigin-RevId: 84f0eaf8d62b5ab76be7f25c078bfb5fc24bb889 --- .../PatternVariableCanBeUsedInspection.java | 28 +++++++++++++++---- .../afterRecordPattern3.java | 9 ++++-- .../beforeNotEffectivelyFinal1.java | 9 ++++++ .../beforeNotEffectivelyFinal2.java | 9 ++++++ .../beforeRecordPattern3.java | 9 ++++-- 5 files changed, 55 insertions(+), 9 deletions(-) create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal1.java create mode 100644 java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal2.java diff --git a/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java b/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java index cf3e862fb611..53e758917ff9 100644 --- a/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java +++ b/java/java-impl-inspections/src/com/intellij/codeInspection/PatternVariableCanBeUsedInspection.java @@ -36,10 +36,13 @@ public class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLocalIns if (qualifier == null) return; if (qualifier.resolve() instanceof PsiPatternVariable variable && variable.getPattern() instanceof PsiDeconstructionPattern deconstruction) { + if (!isFinalOrEffectivelyFinal(variable)) return; PsiPatternVariable existingPatternVariable = findExistingPatternVariable(qualifier, deconstruction, call); if (existingPatternVariable == null) return; + if (!isFinalOrEffectivelyFinal(existingPatternVariable)) return; String patternName = existingPatternVariable.getName(); - if (PsiUtil.skipParenthesizedExprUp(call.getParent()) instanceof PsiLocalVariable localVariable) { + if (PsiUtil.skipParenthesizedExprUp(call.getParent()) instanceof PsiLocalVariable localVariable && + canReplaceLocalVariableWithPatternVariable(localVariable, existingPatternVariable)) { String name = localVariable.getName(); LocalQuickFix fix = new ExistingPatternVariableCanBeUsedFix(name, existingPatternVariable); holder.registerProblem(call, InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.message", @@ -54,6 +57,20 @@ public class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLocalIns } } + private static boolean isFinalOrEffectivelyFinal(@NotNull PsiPatternVariable variable) { + return (!(variable.getPattern() instanceof PsiDeconstructionPattern) && variable.hasModifierProperty(PsiModifier.FINAL)) || + !VariableAccessUtils.variableIsAssigned(variable, variable.getDeclarationScope()); + } + + private static boolean canReplaceLocalVariableWithPatternVariable(@NotNull PsiLocalVariable localVariable, + @NotNull PsiPatternVariable patternVariable) { + PsiElement scope = PsiUtil.getVariableCodeBlock(localVariable, null); + if (scope == null) return false; + return localVariable.hasModifierProperty(PsiModifier.FINAL) || + !patternVariable.hasModifierProperty(PsiModifier.FINAL) || + HighlightControlFlowUtil.isEffectivelyFinal(localVariable, scope, null); + } + @Nullable private static PsiReferenceExpression getQualifierReferenceExpression(@NotNull PsiMethodCallExpression call) { while (true) { @@ -110,17 +127,18 @@ public class PatternVariableCanBeUsedInspection extends AbstractBaseJavaLocalIns if (scope == null) return; PsiDeclarationStatement declaration = ObjectUtils.tryCast(variable.getParent(), PsiDeclarationStatement.class); if (declaration == null) return; - if (!PsiUtil.isLanguageLevel16OrHigher(holder.getFile()) && - !variable.hasModifierProperty(PsiModifier.FINAL) && - !HighlightControlFlowUtil.isEffectivelyFinal(variable, scope, null)) return; PsiInstanceOfExpression instanceOf = InstanceOfUtils.findPatternCandidate(cast); if (instanceOf != null) { PsiPattern pattern = instanceOf.getPattern(); PsiPatternVariable existingPatternVariable = JavaPsiPatternUtil.getPatternVariable(pattern); String name = identifier.getText(); if (existingPatternVariable != null) { + if (!canReplaceLocalVariableWithPatternVariable(variable, existingPatternVariable) || + !isFinalOrEffectivelyFinal(existingPatternVariable)) { + return; + } holder.registerProblem(identifier, - InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.message", + InspectionGadgetsBundle.message("inspection.pattern.variable.can.be.used.existing.message", existingPatternVariable.getName(), name), new ExistingPatternVariableCanBeUsedFix(name, existingPatternVariable)); } else { diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterRecordPattern3.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterRecordPattern3.java index 0eaafb7dc18e..64901d4fb0ba 100644 --- a/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterRecordPattern3.java +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/afterRecordPattern3.java @@ -1,14 +1,19 @@ // "Fix all 'Pattern variable can be used' problems in file" "true" class Main { void foo(Object obj) { - if (obj instanceof Rect(Point(double x1, double y1), Point(double x2, double y2)) rect) { + if (obj instanceof Rect(Point(double x1, double y1) point1, Point(final double x2, double y2)) rect) { System.out.println(x2); double x = x1; Point p1 = rect.point2(); System.out.println(rect.point2().z()); System.out.println(rect.point1().y()); System.out.println(rect.point1().z()); - + y2 = 0.0; + System.out.println(rect.point2().y()); + point1 = new Point(100.0, 200.0); + System.out.println(point1.y()); + double d = x2; + d = 10.0; } } } diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal1.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal1.java new file mode 100644 index 000000000000..43b4d8045c78 --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal1.java @@ -0,0 +1,9 @@ +// "Replace 'integer' with existing pattern variable 'i'" "false" +class X { + void test(Object obj) { + if (obj instanceof Integer i) { + i = 42; + Integer integer = (Integer)obj; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal2.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal2.java new file mode 100644 index 000000000000..3a3dfd2fd03f --- /dev/null +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeNotEffectivelyFinal2.java @@ -0,0 +1,9 @@ +// "Replace 'integer' with existing pattern variable 'i'" "false" +class X { + void test(Object obj) { + if (obj instanceof final Integer i) { + Integer integer = (Integer)obj; + integer = 42; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeRecordPattern3.java b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeRecordPattern3.java index e8d1ca1a48d8..4f564de81c3b 100644 --- a/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeRecordPattern3.java +++ b/java/java-tests/testData/inspection/patternVariableCanBeUsed/beforeRecordPattern3.java @@ -1,14 +1,19 @@ // "Fix all 'Pattern variable can be used' problems in file" "true" class Main { void foo(Object obj) { - if (obj instanceof Rect(Point(double x1, double y1), Point(double x2, double y2)) rect) { + if (obj instanceof Rect(Point(double x1, double y1) point1, Point(final double x2, double y2)) rect) { System.out.println(rect.point2().x()); double x = x1; Point p1 = rect.point2(); System.out.println(rect.point2().z()); System.out.println(rect.point1().y()); System.out.println(rect.point1().z()); - + y2 = 0.0; + System.out.println(rect.point2().y()); + point1 = new Point(100.0, 200.0); + System.out.println(point1.y()); + double d = rect.point2().x(); + d = 10.0; } } }