diff --git a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/SwitchUtils.java b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/SwitchUtils.java index 1c75e33cfd12..7dc08fee3ed1 100644 --- a/java/java-analysis-impl/src/com/siyeh/ig/psiutils/SwitchUtils.java +++ b/java/java-analysis-impl/src/com/siyeh/ig/psiutils/SwitchUtils.java @@ -182,10 +182,10 @@ public final class SwitchUtils { } else if (operation.equals(JavaTokenType.EQEQ) && operands.length == 2) { return (canBeCaseLabel(operands[0], languageLevel, existingCaseValues) && - !isExtendedPrimitives(PsiPrimitiveType.getOptionallyUnboxedType(operands[1].getType())) && + !isExtendedSwitchSelectorType(operands[1].getType()) && EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(switchExpression, operands[1])) || (canBeCaseLabel(operands[1], languageLevel, existingCaseValues) && - !isExtendedPrimitives(PsiPrimitiveType.getOptionallyUnboxedType(operands[0].getType())) && + !isExtendedSwitchSelectorType(operands[0].getType()) && EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(switchExpression, operands[0])); } else { @@ -194,12 +194,13 @@ public final class SwitchUtils { } /** - * Checks whether the given primitive type is an extended switch with primitive types. + * Checks whether the given primitive or boxed type is an extended switch with primitive types. * - * @param primitiveType the primitive type to check + * @param primitiveOrBoxedType the primitive type to check * @return true if the primitive type is an extended primitive type, otherwise false */ - public static boolean isExtendedPrimitives(@Nullable PsiPrimitiveType primitiveType) { + public static boolean isExtendedSwitchSelectorType(@Nullable PsiType primitiveOrBoxedType) { + PsiPrimitiveType primitiveType = PsiPrimitiveType.getOptionallyUnboxedType(primitiveOrBoxedType); if (primitiveType == null) return false; return PsiTypes.booleanType().equals(primitiveType) || PsiTypes.longType().equals(primitiveType) || @@ -208,7 +209,7 @@ public final class SwitchUtils { } /** - * Checks if the given expression is a comparison with primitives for extended switch with primitive types + * Checks if the given expression is a comparison with primitives or wrappers for extended switch with primitive types * * @param psiExpression the expression to check * @param switchExpression the switch expression to compare with @@ -226,16 +227,17 @@ public final class SwitchUtils { if (psiBinaryExpression.getOperationTokenType() != JavaTokenType.EQEQ) { return false; } + boolean isEqual = psiBinaryExpression.getOperationTokenType() == JavaTokenType.EQEQ; final PsiExpression left = psiBinaryExpression.getLOperand(); final PsiExpression right = psiBinaryExpression.getROperand(); if (right == null) return false; if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(switchExpression, right) && - primitiveValueCanBeUsedForComparisonInCase(left, right, existingCaseValues)) { + primitiveValueCanBeUsedForComparisonInCase(left, right, isEqual ? existingCaseValues : null)) { return true; } else { if (EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(switchExpression, left) && - primitiveValueCanBeUsedForComparisonInCase(right, left, existingCaseValues)) { + primitiveValueCanBeUsedForComparisonInCase(right, left, isEqual ? existingCaseValues : null)) { return true; } } @@ -260,7 +262,7 @@ public final class SwitchUtils { PsiType valueType = value.getType(); if (selectorType == null || valueType == null) return false; PsiPrimitiveType unwrapped = PsiPrimitiveType.getOptionallyUnboxedType(selectorType); - if (unwrapped != null && (isExtendedPrimitives(unwrapped))) { + if (unwrapped != null && isExtendedSwitchSelectorType(unwrapped)) { return unwrapped.equals(valueType); } return TypeConversionUtil.isAssignable(selectorType, valueType); @@ -269,20 +271,47 @@ public final class SwitchUtils { private static boolean canBePatternSwitchCase(@Nullable PsiExpression expression, @NotNull PsiExpression switchExpression, @NotNull Set existingCaseValues) { - if (canBePatternSwitchCase(expression, switchExpression)) { - final PsiCaseLabelElement pattern = createPatternFromExpression(expression); - if (pattern == null) return true; - for (Object caseValue : existingCaseValues) { - if (caseValue instanceof PsiPattern && JavaPsiPatternUtil.dominates((PsiPattern)caseValue, pattern)) { - return false; - } - } - existingCaseValues.add(pattern); - return true; - } - else { + if (!canBePatternSwitchCase(expression, switchExpression)) { return false; } + if (isNullComparison(expression, switchExpression)) { + return existingCaseValues.add(null); + } + final PsiCaseLabelElement pattern = createPatternFromExpression(expression); + if (pattern == null) { + if (expression instanceof PsiPolyadicExpression polyadicExpression && + polyadicExpression.getOperationTokenType().equals(JavaTokenType.OROR)) { + for (@NotNull PsiElement child : polyadicExpression.getOperands()) { + if (!(child instanceof PsiExpression childExpression)) { + return false; + } + if (!canBePatternSwitchCase(childExpression, switchExpression, existingCaseValues)) { + return false; + } + return true; + } + } + return false; + } + if (VariableAccessUtils.isAnyVariableAssigned(VariableAccessUtils.collectUsedVariables(expression), expression)) { + return false; + } + if (!PsiTreeUtil.findChildrenOfType(expression, PsiDeclarationStatement.class).isEmpty()) { + return false; + } + for (Object caseValue : existingCaseValues) { + if (caseValue instanceof PsiPattern && JavaPsiPatternUtil.dominates((PsiPattern)caseValue, pattern)) { + return false; + } + } + existingCaseValues.add(pattern); + return true; + } + + private static boolean isNullComparison(@Nullable PsiExpression expression, @NotNull PsiExpression switchExpression) { + if (expression == null) return false; + PsiExpression operand = findNullCheckedOperand(expression); + return EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(switchExpression, operand); } public static @Nullable PsiCaseLabelElement createPatternFromExpression(@NotNull PsiExpression expression) { @@ -612,7 +641,7 @@ public final class SwitchUtils { for (PsiElement child : expression.getChildren()) { if (child instanceof PsiBinaryExpression childBinaryExpression) { for (PsiElement binaryChild : childBinaryExpression.getChildren()) { - if (binaryChild == switchSelector) { + if (isSwitchSelector(binaryChild, switchSelector)) { stringBuilder.append(name); } else { @@ -621,7 +650,7 @@ public final class SwitchUtils { } } else { - if (child == switchSelector) { + if (isSwitchSelector(child, switchSelector)) { stringBuilder.append(name); } else { @@ -632,6 +661,16 @@ public final class SwitchUtils { return stringBuilder.toString(); } + private static boolean isSwitchSelector(@NotNull PsiElement child, @NotNull PsiExpression selector) { + if (child == selector) return true; + if (!child.getText().equals(selector.getText())) return false; + return child instanceof PsiReferenceExpression childReference && + selector instanceof PsiReferenceExpression selectorReference && + childReference.resolve() instanceof PsiVariable childVariable && + selectorReference.resolve() instanceof PsiVariable selectorVariable && + child.getManager().areElementsEquivalent(childVariable, selectorVariable); + } + private static @Nullable PsiExpression findSelectorWithComparedPrimitives(@NotNull PsiPolyadicExpression expression) { if (!PsiUtil.isAvailable(JavaFeature.PRIMITIVE_TYPES_IN_PATTERNS, expression)) { return null; diff --git a/java/java-impl/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java b/java/java-impl/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java index bb6f4eca0b6a..9555b475bea1 100644 --- a/java/java-impl/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java +++ b/java/java-impl/src/com/siyeh/ig/migration/IfCanBeSwitchInspection.java @@ -318,7 +318,7 @@ public final class IfCanBeSwitchInspection extends BaseInspection { } if (ContainerUtil.or(branches, branch -> branch.hasPattern() && !(switchExpression.getType() instanceof PsiPrimitiveType)) || - SwitchUtils.isExtendedPrimitives(PsiPrimitiveType.getOptionallyUnboxedType(switchExpression.getType()))) { + SwitchUtils.isExtendedSwitchSelectorType(switchExpression.getType())) { final boolean hasDefaultElse = ContainerUtil.exists(branches, (branch) -> branch.isElse()); if (!hasDefaultElse && !hasUnconditionalPatternCheck(ifStatement, switchExpression)) { branches.add(new IfStatementBranch(new PsiEmptyStatementImpl(), true)); diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonNonPrimitive.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonNonPrimitive.java index b111f43250de..5b304493c04e 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonNonPrimitive.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonNonPrimitive.java @@ -1,7 +1,7 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(Number code) { + String test(Number code) { if (code == 100) { return "Continue"; } else if (code == 200) { diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.after.java index c6cef1020bb9..8a843e72dafb 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.after.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.after.java @@ -1,13 +1,13 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(@NotNull Integer code) { - return switch (code) { - case 100 -> "Continue"; - case 200 -> "OK"; - case 301 -> "Moved permanently"; - case Integer i when 502 > i -> "Server error"; - default -> "unknown code"; - }; + String test(@NotNull Integer code) { + return switch (code) { + case 100 -> "Continue"; + case 200 -> "OK"; + case 301 -> "Moved permanently"; + case Integer i when 502 > i -> "Server error"; + default -> "unknown code"; + }; } } \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.java index 403eedb0eefc..e7a9f299ee5d 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives1.java @@ -1,7 +1,7 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(@NotNull Integer code) { + String test(@NotNull Integer code) { if (code == 100) { return "Continue"; } else if (code == 200) { diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.after.java index 93cb1e500ca6..577f16304a8f 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.after.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.after.java @@ -1,13 +1,13 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(@NotNull Integer code) { - return switch (code) { - case 100 -> "Continue"; - case 200 -> "OK"; - case 301 -> "Moved permanently"; - case Integer i when code > 502 && i < 600 -> "Server error"; - default -> "unknown code"; - }; + String test(@NotNull Integer code) { + return switch (code) { + case 100 -> "Continue"; + case 200 -> "OK"; + case 301 -> "Moved permanently"; + case Integer i when i > 502 && i < 600 -> "Server error"; + default -> "unknown code"; + }; } } \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.java index 6f8ae3b91dc1..a146dfd70796 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives2.java @@ -1,7 +1,7 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(@NotNull Integer code) { + String test(@NotNull Integer code) { if (code == 100) { return "Continue"; } else if (code == 200) { diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.after.java index 3bf12850abb1..d56c7779f40f 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.after.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.after.java @@ -1,12 +1,12 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(int code) { + String test(int code) { return switch (code) { case 100 -> "Continue"; case 200 -> "OK"; case 301 -> "Moved permanently"; - case int i when code > 502 && i < 600 -> "Server error"; + case int i when i > 502 && i < 600 -> "Server error"; default -> "unknown code"; }; } diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.java index 554af3274b7f..2ce8de6316f4 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitives3.java @@ -1,7 +1,7 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(int code) { + String test(int code) { if (code == 100) { return "Continue"; } else if (code == 200) { diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitivesUsedTwice.after.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitivesUsedTwice.after.java new file mode 100644 index 000000000000..33fd55b76b4f --- /dev/null +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitivesUsedTwice.after.java @@ -0,0 +1,13 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + String test(@NotNull Integer code) { + return switch (code) { + case 100 -> "Continue"; + case 200 -> "OK"; + case 301 -> "Moved permanently"; + case Integer i when 301 > i -> "Server error"; + default -> "unknown code"; + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitivesUsedTwice.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitivesUsedTwice.java new file mode 100644 index 000000000000..0e7e100e3bbf --- /dev/null +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/ComparisonWithPrimitivesUsedTwice.java @@ -0,0 +1,17 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + String test(@NotNull Integer code) { + if (code == 100) { + return "Continue"; + } else if (code == 200) { + return "OK"; + } else if (301 == code) { + return "Moved permanently"; + } else if (301 > code) { + return "Server error"; + } else { + return "unknown code"; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/DoubleComparison.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/DoubleComparison.java index 8827f03ea8f1..fda3e74cfa9e 100644 --- a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/DoubleComparison.java +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/DoubleComparison.java @@ -1,7 +1,7 @@ import org.jetbrains.annotations.NotNull; class Test { - void test(@NotNull Integer code) { + String test(@NotNull Integer code) { if (code == 100) { return "Continue"; } else if (code == 100) { diff --git a/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/VariableAssigned.java b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/VariableAssigned.java new file mode 100644 index 000000000000..befb0dc022b7 --- /dev/null +++ b/java/java-tests/testData/ig/com/siyeh/igfixes/migration/if_can_be_switch/VariableAssigned.java @@ -0,0 +1,18 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + String test(@NotNull Integer code) { + int aa = 0; + if (code == 100) { + return "Continue"; + } else if (code == 200) { + return "OK"; + } else if (301 == code) { + return "Moved permanently"; + } else if (301 > code && aa++>0) { + return "Server error"; + } else { + return "unknown code"; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/siyeh/ig/fixes/migration/IfCanBePrimitivePatternSwitchFixTest.java b/java/java-tests/testSrc/com/siyeh/ig/fixes/migration/IfCanBePrimitivePatternSwitchFixTest.java index 1a64bd658b48..aa3a67ce1453 100644 --- a/java/java-tests/testSrc/com/siyeh/ig/fixes/migration/IfCanBePrimitivePatternSwitchFixTest.java +++ b/java/java-tests/testSrc/com/siyeh/ig/fixes/migration/IfCanBePrimitivePatternSwitchFixTest.java @@ -58,8 +58,10 @@ public class IfCanBePrimitivePatternSwitchFixTest extends IGQuickFixesTestCase { public void testComparisonWithPrimitives1() { doTest(); } public void testComparisonWithPrimitives2() { doTest(); } public void testComparisonWithPrimitives3() { doTest(); } + public void testComparisonWithPrimitivesUsedTwice() { doTest(); } public void testComparisonNonPrimitive() { assertQuickfixNotAvailable(); } public void testDoubleComparison() { assertQuickfixNotAvailable(); } public void testPrimitiveDominates() { assertQuickfixNotAvailable(); } + public void testVariableAssigned() { assertQuickfixNotAvailable(); } }