From fc6de8a51555112fca0f3999c66ea63e9cbba26d Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 27 Feb 2025 12:00:42 +0100 Subject: [PATCH] [java-highlighting] Migrate checkRedundantDefaultBranch Part of IDEA-365344 Create a new Java error highlighter with minimal dependencies (PSI only) GitOrigin-RevId: 8bf45e8e4e26a2766c9755a90dfba6d6cc94e198 --- .../JavaCompilationErrorBundle.properties | 3 + .../highlighting/SwitchChecker.java | 24 +++++++ .../highlighting/errors/JavaErrorKinds.java | 6 ++ .../impl/analysis/JavaErrorFixProvider.java | 5 ++ ...atternsInSwitchBlockHighlightingModel.java | 67 +++---------------- .../SwitchBlockHighlightingModel.java | 4 -- .../messages/JavaErrorBundle.properties | 3 - 7 files changed, 46 insertions(+), 66 deletions(-) diff --git a/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties b/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties index ab628a5386c9..c947cda4ca57 100644 --- a/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties +++ b/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties @@ -343,6 +343,9 @@ switch.label.combination.constants.and.patterns.unnamed=Invalid case label combi switch.label.multiple.patterns=Invalid case label combination: a case label must not consist of more than one case pattern switch.label.multiple.patterns.unnamed=Invalid case label combination: multiple patterns are allowed only if none of them declare any pattern variables switch.dominance.violation=Label is dominated by a preceding case label ''{0}'' +switch.unconditional.pattern.and.default='switch' has both an unconditional pattern and a default label +switch.default.and.boolean='switch' has all boolean values and a default label +switch.unconditional.pattern.and.boolean='switch' has all boolean values and an unconditional pattern guard.misplaced=Guard is allowed after patterns only guard.evaluated.to.false=Case label has a guard that is a constant expression with value 'false' diff --git a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/SwitchChecker.java b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/SwitchChecker.java index cb732cfb434f..8f2e4f4011fc 100644 --- a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/SwitchChecker.java +++ b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/SwitchChecker.java @@ -33,6 +33,7 @@ final class SwitchChecker { if (!myVisitor.hasErrorResults()) checkDuplicates(block); if (!myVisitor.hasErrorResults()) checkFallthroughLegality(block); if (!myVisitor.hasErrorResults()) checkDominance(block); + if (!myVisitor.hasErrorResults()) checkNoDefaultBranchAllowed(block); } void checkSwitchExpressionReturnTypeCompatible(@NotNull PsiSwitchExpression switchExpression) { @@ -618,4 +619,27 @@ final class SwitchChecker { myVisitor.report(JavaErrorKinds.SWITCH_DOMINANCE_VIOLATION.create(overWhom, who)); } } + + private void checkNoDefaultBranchAllowed(@NotNull PsiSwitchBlock block) { + if (!myVisitor.isApplicable(JavaFeature.PATTERNS_IN_SWITCH)) return; + //T is an intersection type T1& ... &Tn, and P covers Ti, for one of the type Ti (1≤i≤n) + PsiCaseLabelElement elementCoversType = JavaPsiSwitchUtil.getUnconditionalPatternLabel(block); + PsiElement defaultElement = JavaPsiSwitchUtil.findDefaultElement(block); + if (defaultElement != null && elementCoversType != null) { + myVisitor.report(JavaErrorKinds.SWITCH_UNCONDITIONAL_PATTERN_AND_DEFAULT.create(defaultElement.getFirstChild())); + myVisitor.report(JavaErrorKinds.SWITCH_UNCONDITIONAL_PATTERN_AND_DEFAULT.create(elementCoversType)); + return; + } + //default (or unconditional), TRUE and FALSE cannot be together + if ((defaultElement != null || elementCoversType != null) && + JavaPsiSwitchUtil.isBooleanSwitchWithTrueAndFalse(block)) { + if (defaultElement != null) { + myVisitor.report(JavaErrorKinds.SWITCH_DEFAULT_AND_BOOLEAN.create(defaultElement.getFirstChild())); + } + if (elementCoversType != null) { + myVisitor.report(JavaErrorKinds.SWITCH_UNCONDITIONAL_PATTERN_AND_BOOLEAN.create(elementCoversType)); + } + } + } + } diff --git a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java index aa5fd4690098..66ce2e74cc9f 100644 --- a/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java +++ b/java/codeserver/highlighting/src/com/intellij/java/codeserver/highlighting/errors/JavaErrorKinds.java @@ -1015,6 +1015,12 @@ public final class JavaErrorKinds { public static final Parameterized SWITCH_DOMINANCE_VIOLATION = parameterized(PsiCaseLabelElement.class, PsiElement.class, "switch.dominance.violation") .withDescription((overWhom, who) -> message("switch.dominance.violation", who.getText())); + public static final Simple SWITCH_UNCONDITIONAL_PATTERN_AND_DEFAULT = + error("switch.unconditional.pattern.and.default"); + public static final Simple SWITCH_DEFAULT_AND_BOOLEAN = + error("switch.default.and.boolean"); + public static final Simple SWITCH_UNCONDITIONAL_PATTERN_AND_BOOLEAN = + error("switch.unconditional.pattern.and.boolean"); public static final Simple EXPRESSION_EXPECTED = error("expression.expected"); public static final Parameterized EXPRESSION_SUPER_UNQUALIFIED_DEFAULT_METHOD = diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java index b6ce097949a8..99c2d924cdc1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/JavaErrorFixProvider.java @@ -760,6 +760,11 @@ final class JavaErrorFixProvider { sink.accept(myFactory.createDeleteSwitchLabelFix(overWhom)); } }); + fix(SWITCH_UNCONDITIONAL_PATTERN_AND_DEFAULT, error -> error.psi() instanceof PsiCaseLabelElement elementCoversType + ? myFactory.createDeleteSwitchLabelFix(elementCoversType) + : myFactory.createDeleteDefaultFix(null, error.psi())); + fix(SWITCH_UNCONDITIONAL_PATTERN_AND_BOOLEAN, error -> myFactory.createDeleteSwitchLabelFix(error.psi())); + fix(SWITCH_DEFAULT_AND_BOOLEAN, error -> myFactory.createDeleteDefaultFix(null, error.psi())); } private void createAccessFixes() { diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/PatternsInSwitchBlockHighlightingModel.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/PatternsInSwitchBlockHighlightingModel.java index 99e09a93cd83..1e39bb161a10 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/PatternsInSwitchBlockHighlightingModel.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/PatternsInSwitchBlockHighlightingModel.java @@ -2,7 +2,6 @@ package com.intellij.codeInsight.daemon.impl.analysis; import com.intellij.codeInsight.ExpressionUtil; -import com.intellij.codeInsight.daemon.JavaErrorBundle; import com.intellij.codeInsight.daemon.impl.HighlightInfo; import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInsight.intention.QuickFixFactory; @@ -53,59 +52,21 @@ public class PatternsInSwitchBlockHighlightingModel extends SwitchBlockHighlight PsiCodeBlock body = myBlock.getBody(); if (body == null) return; - List elementsToCheckCompleteness = new ArrayList<>(); - for (PsiStatement st : body.getStatements()) { - if (!(st instanceof PsiSwitchLabelStatementBase labelStatement) || labelStatement.isDefaultCase()) continue; - PsiCaseLabelElementList labelElementList = labelStatement.getCaseLabelElementList(); - if (labelElementList != null) { - Collections.addAll(elementsToCheckCompleteness, labelElementList.getElements()); - } - } if (ExpressionUtil.isEnhancedSwitch(myBlock)) { - if (checkRedundantDefaultBranch(errorSink)) return; if (JavaPsiSwitchUtil.getUnconditionalPatternLabel(myBlock) != null) return; if (JavaPsiSwitchUtil.findDefaultElement(myBlock) != null) return; + List elementsToCheckCompleteness = new ArrayList<>(); + for (PsiStatement st : body.getStatements()) { + if (!(st instanceof PsiSwitchLabelStatementBase labelStatement) || labelStatement.isDefaultCase()) continue; + PsiCaseLabelElementList labelElementList = labelStatement.getCaseLabelElementList(); + if (labelElementList != null) { + Collections.addAll(elementsToCheckCompleteness, labelElementList.getElements()); + } + } checkCompleteness(elementsToCheckCompleteness, errorSink); } } - private boolean checkRedundantDefaultBranch(@NotNull Consumer errorSink) { - //T is an intersection type T1& ... &Tn, and P covers Ti, for one of the type Ti (1≤i≤n) - PsiCaseLabelElement elementCoversType = JavaPsiSwitchUtil.getUnconditionalPatternLabel(myBlock); - PsiElement defaultElement = JavaPsiSwitchUtil.findDefaultElement(myBlock); - if (defaultElement != null && elementCoversType != null) { - HighlightInfo.Builder defaultInfo = - createError(defaultElement.getFirstChild(), JavaErrorBundle.message("switch.unconditional.pattern.and.default.exist")); - registerDeleteFixForDefaultElement(defaultInfo, defaultElement, defaultElement.getFirstChild()); - errorSink.accept(defaultInfo); - HighlightInfo.Builder patternInfo = createError(elementCoversType, JavaErrorBundle.message( - "switch.unconditional.pattern.and.default.exist")); - IntentionAction action = getFixFactory().createDeleteSwitchLabelFix(elementCoversType); - patternInfo.registerFix(action, null, null, null, null); - errorSink.accept(patternInfo); - return true; - } - //default (or unconditional), TRUE and FALSE cannot be together - if ((defaultElement != null || elementCoversType != null) && - JavaPsiSwitchUtil.isBooleanSwitchWithTrueAndFalse(myBlock)) { - if (defaultElement != null) { - HighlightInfo.Builder defaultInfo = - createError(defaultElement.getFirstChild(), JavaErrorBundle.message("switch.unconditional.boolean.and.default.exist")); - registerDeleteFixForDefaultElement(defaultInfo, defaultElement, defaultElement.getFirstChild()); - errorSink.accept(defaultInfo); - } - if (elementCoversType != null) { - HighlightInfo.Builder patternInfo = createError(elementCoversType, - JavaErrorBundle.message( - "switch.unconditional.boolean.and.unconditional.exist")); - IntentionAction action = getFixFactory().createDeleteSwitchLabelFix(elementCoversType); - patternInfo.registerFix(action, null, null, null, null); - errorSink.accept(patternInfo); - } - } - return defaultElement != null || elementCoversType != null; - } - private void checkCompleteness(@NotNull List elements, @NotNull Consumer errorSink) { if (isExhaustiveForSwitchSelectorPrimitiveWrapper(elements)) return; @@ -197,18 +158,6 @@ public class PatternsInSwitchBlockHighlightingModel extends SwitchBlockHighlight return ContainerUtil.exists(elements, element -> extractPattern(element) != null); } - private static void registerDeleteFixForDefaultElement(@NotNull HighlightInfo.Builder info, - PsiElement defaultElement, - @NotNull PsiElement duplicateElement) { - if (defaultElement instanceof PsiCaseLabelElement caseElement) { - IntentionAction action = getFixFactory().createDeleteSwitchLabelFix(caseElement); - info.registerFix(action, null, null, null, null); - return; - } - IntentionAction action = getFixFactory().createDeleteDefaultFix(null, duplicateElement); - info.registerFix(action, null, null, null, null); - } - private @Nullable HighlightInfo.Builder checkSealedClassCompleteness(@NotNull PsiType selectorType, @NotNull List elements) { Set missedClasses; diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java index de3a25ab3106..38420427f8d7 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java @@ -117,10 +117,6 @@ public class SwitchBlockHighlightingModel { return QuickFixFactory.getInstance(); } - static @Nullable Object evaluateConstant(@NotNull PsiCaseLabelElement constant) { - return JavaPsiFacade.getInstance(constant.getProject()).getConstantEvaluationHelper().computeConstantExpression(constant, false); - } - void checkEnumCompleteness(@NotNull PsiClass selectorClass, @NotNull List enumElements, @NotNull Consumer errorSink) { diff --git a/java/java-psi-impl/resources/messages/JavaErrorBundle.properties b/java/java-psi-impl/resources/messages/JavaErrorBundle.properties index b13ba1046c0b..1378c295eb16 100644 --- a/java/java-psi-impl/resources/messages/JavaErrorBundle.properties +++ b/java/java-psi-impl/resources/messages/JavaErrorBundle.properties @@ -37,9 +37,6 @@ variable.already.defined=Variable ''{0}'' is already defined in the scope # suppress inspection "UnusedProperty" incompatible.types=Incompatible types. Found: ''{1}'', required: ''{0}'' switch.dominance.of.preceding.label=Label is dominated by a preceding case label ''{0}'' -switch.unconditional.pattern.and.default.exist='switch' has both an unconditional pattern and a default label -switch.unconditional.boolean.and.default.exist='switch' has all boolean values and a default label -switch.unconditional.boolean.and.unconditional.exist='switch' has all boolean values and an unconditional pattern expression.expected=Expression expected qualified.enum.constant.in.switch.remove.fix=Remove qualifier deconstruction.pattern.requires.record=Deconstruction pattern can only be applied to a record, ''{0}'' is not a record