From 89dda01eebf334fbaea47503f0dc06f6551c0f8b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 28 Feb 2025 12:59:36 +0100 Subject: [PATCH] [java-highlighting] Exhaustiveness checks migrated (the last error from HighlightVisitorImpl!) Part of IDEA-365344 Create a new Java error highlighter with minimal dependencies (PSI only) GitOrigin-RevId: c45b258fc18e267122946be7d3aebfd9dc77387f --- .../JavaCompilationErrorBundle.properties | 8 +-- .../highlighting/SwitchChecker.java | 16 ++++++ .../highlighting/errors/JavaErrorKinds.java | 6 +++ .../impl/analysis/HighlightVisitorImpl.java | 42 +-------------- .../impl/analysis/JavaErrorFixProvider.java | 6 +++ .../SwitchBlockHighlightingModel.java | 51 ------------------- .../PatternMatchingInSwitchJava21.java | 4 +- ...tionalDestructuringAndDefaultIn21Java.java | 4 +- .../inspection/dataFlow/fixture/Patterns.java | 2 +- .../dataFlow/fixture/WhenPatterns.java | 2 +- 10 files changed, 41 insertions(+), 100 deletions(-) delete mode 100644 java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java diff --git a/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties b/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties index c947cda4ca57..9fb56c7b9f16 100644 --- a/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties +++ b/java/codeserver/highlighting/resources/messages/JavaCompilationErrorBundle.properties @@ -325,11 +325,11 @@ switch.different.case.kinds=Different 'case' kinds used in 'switch' switch.selector.type.invalid=Selector type of ''{0}'' is not supported switch.selector.type.invalid.level=Selector type of ''{0}'' is not supported at language level ''{1}'' switch.null.type.incompatible=''null'' cannot be converted to ''{0}'' +switch.null.label.not.allowed=Invalid case label combination: 'null' can only be used as a single case label or paired only with 'default' switch.label.qualified.enum=An enum switch case label must be the unqualified name of an enumeration constant switch.label.constant.expected=Constant expression required switch.label.pattern.expected=Pattern expected for switch selector type ''{0}'' switch.label.unexpected=Constant expression, pattern or null is required -switch.default.label.contains.case=The label for the default case must only use the 'default' keyword, without 'case' switch.label.duplicate.unconditional.pattern=Duplicate unconditional pattern switch.label.duplicate.default=Duplicate default label switch.label.duplicate=Duplicate label ''{0}'' @@ -337,15 +337,17 @@ switch.fallthrough.to.pattern=Illegal fall-through to a pattern switch.multiple.labels.with.pattern.variables=Multiple switch labels are permitted for a switch labeled statement group only if none of them declare any pattern variables switch.default.null.order=Invalid case label order: 'null' must be first and 'default' must be second switch.default.label.not.allowed=Default label not allowed here: 'default' can only be used as a single case label or paired only with 'null' -switch.null.label.not.allowed=Invalid case label combination: 'null' can only be used as a single case label or paired only with 'default' switch.label.combination.constants.and.patterns=Invalid case label combination: a case label must consist of either a list of case constants or a single case pattern switch.label.combination.constants.and.patterns.unnamed=Invalid case label combination: a case label must consist of either a list of case constants or a list of case patterns 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.default.label.contains.case=The label for the default case must only use the 'default' keyword, without 'case' +switch.unconditional.pattern.and.default='switch' has both an unconditional pattern and a default label switch.unconditional.pattern.and.boolean='switch' has all boolean values and an unconditional pattern +switch.empty=''switch'' {0} does not have any case clauses +switch.incomplete=''switch'' {0} does not cover all possible input values 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 008a1414e22c..f76bebd67999 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 @@ -21,6 +21,8 @@ import org.jetbrains.annotations.Nullable; import java.util.*; +import static com.intellij.java.codeserver.core.JavaPatternExhaustivenessUtil.hasExhaustivenessError; + final class SwitchChecker { private final @NotNull JavaErrorVisitor myVisitor; @@ -34,6 +36,7 @@ final class SwitchChecker { if (!myVisitor.hasErrorResults()) checkFallthroughLegality(block); if (!myVisitor.hasErrorResults()) checkDominance(block); if (!myVisitor.hasErrorResults()) checkNoDefaultBranchAllowed(block); + if (!myVisitor.hasErrorResults()) checkExhaustiveness(block); } void checkSwitchExpressionReturnTypeCompatible(@NotNull PsiSwitchExpression switchExpression) { @@ -632,4 +635,17 @@ final class SwitchChecker { } } + void checkExhaustiveness(@NotNull PsiSwitchBlock block) { + PsiCodeBlock body = block.getBody(); + if (body == null) return; + + if (!ExpressionUtil.isEnhancedSwitch(block)) return; + if (JavaPsiSwitchUtil.getUnconditionalPatternLabel(block) != null) return; + if (JavaPsiSwitchUtil.findDefaultElement(block) != null) return; + if (!hasExhaustivenessError(block)) return; + + boolean hasAnyCaseLabels = JavaPsiSwitchUtil.hasAnyCaseLabels(block); + var kind = hasAnyCaseLabels ? JavaErrorKinds.SWITCH_INCOMPLETE : JavaErrorKinds.SWITCH_EMPTY; + myVisitor.report(kind.create(block)); + } } 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 66ce2e74cc9f..f92c84d5e4fb 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 @@ -1021,6 +1021,12 @@ public final class JavaErrorKinds { error("switch.default.and.boolean"); public static final Simple SWITCH_UNCONDITIONAL_PATTERN_AND_BOOLEAN = error("switch.unconditional.pattern.and.boolean"); + public static final Simple SWITCH_EMPTY = error(PsiSwitchBlock.class, "switch.empty") + .withAnchor(block -> requireNonNullElse(block.getExpression(), block.getFirstChild())) + .withDescription(block -> message("switch.empty", JavaElementKind.fromElement(block).subject())); + public static final Simple SWITCH_INCOMPLETE = error(PsiSwitchBlock.class, "switch.incomplete") + .withAnchor(block -> requireNonNullElse(block.getExpression(), block.getFirstChild())) + .withDescription(psi -> message("switch.incomplete", JavaElementKind.fromElement(psi).subject())); 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/HighlightVisitorImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java index 235848cef64c..eb5137c2ce50 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/HighlightVisitorImpl.java @@ -10,7 +10,6 @@ import com.intellij.codeInspection.ex.GlobalInspectionContextBase; import com.intellij.java.codeserver.highlighting.JavaErrorCollector; import com.intellij.java.codeserver.highlighting.errors.JavaCompilationError; import com.intellij.java.codeserver.highlighting.errors.JavaErrorHighlightType; -import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.lang.injection.InjectedLanguageManager; import com.intellij.openapi.editor.DefaultLanguageHighlighterColors; import com.intellij.openapi.editor.Document; @@ -19,17 +18,13 @@ import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.HtmlChunk; -import com.intellij.pom.java.LanguageLevel; import com.intellij.psi.*; -import com.intellij.psi.util.PsiUtil; import com.intellij.ui.ColorUtil; import com.intellij.ui.NewUI; import com.intellij.util.ui.JBUI; import com.intellij.util.ui.NamedColorUtil; import com.intellij.util.ui.UIUtil; -import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.awt.*; import java.util.Map; @@ -43,13 +38,10 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh private final Map myTooltipStyles = initTooltipStyles(); private @NotNull HighlightInfoHolder myHolder; - private @NotNull LanguageLevel myLanguageLevel; private @NotNull PsiFile myFile; private JavaErrorCollector myCollector; - private boolean myHasError; // true if myHolder.add() was called with HighlightInfo of >=ERROR severity. On each .visit(PsiElement) call this flag is reset. Useful to determine whether the error was already reported while visiting this PsiElement. - protected HighlightVisitorImpl() { } @@ -68,11 +60,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh "color: " + ColorUtil.toHtmlColor(NamedColorUtil.getErrorForeground())); } - @Contract(pure = true) - private boolean hasErrorResults() { - return myHasError; - } - /** * @deprecated use {@link #HighlightVisitorImpl()} */ @@ -99,7 +86,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh @Override public void visit(@NotNull PsiElement element) { - myHasError = false; element.accept(this); } @@ -137,9 +123,8 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh private void prepare(@NotNull HighlightInfoHolder holder, @NotNull PsiFile file) { myHolder = holder; myFile = file; - myLanguageLevel = PsiUtil.getLanguageLevel(file); JavaErrorFixProvider errorFixProvider = JavaErrorFixProvider.getInstance(); - myCollector = new JavaErrorCollector(myFile, error -> reportError(error, errorFixProvider)); + myCollector = new JavaErrorCollector(file, error -> reportError(error, errorFixProvider)); } private void reportError(JavaCompilationError error, JavaErrorFixProvider errorFixProvider) { @@ -178,7 +163,7 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh .or(() -> error.psiForKind(TYPE_UNKNOWN_CLASS).map(PsiTypeElement::getInnermostComponentReferenceElement)) .or(() -> error.psiForKind(CALL_AMBIGUOUS_NO_MATCH, CALL_UNRESOLVED).map(PsiMethodCallExpression::getMethodExpression)) .ifPresent(ref -> UnresolvedReferenceQuickFixProvider.registerUnresolvedReferenceLazyQuickFixes(ref, info)); - add(info); + myHolder.add(info.create()); } @Override @@ -186,32 +171,9 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh myCollector.processElement(element); } - private boolean add(@Nullable HighlightInfo.Builder builder) { - if (builder != null) { - HighlightInfo info = builder.create(); - if (info != null && info.getSeverity().compareTo(HighlightSeverity.ERROR) >= 0) { - myHasError = true; - } - return myHolder.add(info); - } - return false; - } - @Override public void visitReferenceExpression(@NotNull PsiReferenceExpression expression) { // Necessary to call visitElement, as super-implementation is empty visitElement(expression); } - - @Override - public void visitSwitchStatement(@NotNull PsiSwitchStatement statement) { - super.visitSwitchStatement(statement); - if (!hasErrorResults()) SwitchBlockHighlightingModel.checkExhaustiveness(statement, builder -> add(builder)); - } - - @Override - public void visitSwitchExpression(@NotNull PsiSwitchExpression expression) { - super.visitSwitchExpression(expression); - if (!hasErrorResults()) SwitchBlockHighlightingModel.checkExhaustiveness(expression, builder -> add(builder)); - } } 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 99c2d924cdc1..120a897b2200 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 @@ -765,6 +765,12 @@ final class JavaErrorFixProvider { : 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())); + JavaFixesPusher switchFixes = (error, sink) -> { + sink.accept(myFactory.createAddSwitchDefaultFix(error.psi(), null)); + HighlightFixUtil.addCompletenessFixes(error.psi(), sink); + }; + fixes(SWITCH_EMPTY, switchFixes); + fixes(SWITCH_INCOMPLETE, switchFixes); } private void createAccessFixes() { 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 deleted file mode 100644 index ef99f3478e73..000000000000 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/SwitchBlockHighlightingModel.java +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -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.daemon.impl.HighlightInfoType; -import com.intellij.codeInsight.intention.IntentionAction; -import com.intellij.codeInsight.intention.QuickFixFactory; -import com.intellij.java.codeserver.core.JavaPsiSwitchUtil; -import com.intellij.psi.PsiCodeBlock; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiExpression; -import com.intellij.psi.PsiSwitchBlock; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.PropertyKey; - -import java.util.function.Consumer; - -import static com.intellij.java.codeserver.core.JavaPatternExhaustivenessUtil.hasExhaustivenessError; -import static java.util.Objects.requireNonNullElse; - -public final class SwitchBlockHighlightingModel { - - static void checkExhaustiveness(@NotNull PsiSwitchBlock block, @NotNull Consumer errorSink) { - PsiCodeBlock body = block.getBody(); - if (body == null) return; - - if (!ExpressionUtil.isEnhancedSwitch(block)) return; - if (JavaPsiSwitchUtil.getUnconditionalPatternLabel(block) != null) return; - if (JavaPsiSwitchUtil.findDefaultElement(block) != null) return; - if (!hasExhaustivenessError(block)) return; - - boolean hasAnyCaseLabels = JavaPsiSwitchUtil.hasAnyCaseLabels(block); - @PropertyKey(resourceBundle = JavaErrorBundle.BUNDLE) String messageKey; - boolean isSwitchExpr = block instanceof PsiExpression; - if (hasAnyCaseLabels) { - messageKey = isSwitchExpr ? "switch.expr.incomplete" : "switch.statement.incomplete"; - } - else { - messageKey = isSwitchExpr ? "switch.expr.empty" : "switch.statement.empty"; - } - PsiElement anchor = requireNonNullElse(block.getExpression(), block.getFirstChild()); - HighlightInfo.Builder info = - HighlightInfo.newHighlightInfo(HighlightInfoType.ERROR).range(anchor).descriptionAndTooltip(JavaErrorBundle.message(messageKey)); - IntentionAction action = QuickFixFactory.getInstance().createAddSwitchDefaultFix(block, null); - info.registerFix(action, null, null, null, null); - HighlightFixUtil.addCompletenessFixes(block, fix -> info.registerFix(fix.asIntention(), null, null, null, null)); - errorSink.accept(info); - } -} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitchJava21.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitchJava21.java index 38808b8e0896..90fd75da453a 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitchJava21.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/PatternMatchingInSwitchJava21.java @@ -241,8 +241,8 @@ class Main { System.out.println("ok"); }; - str = switch (d) { - case MONDAY, TUESDAY -> System.out.println("ok"); + str = switch (d) { + case MONDAY, TUESDAY -> System.out.println("ok"); }; str = switch (d) { case MONDAY, TUESDAY -> "ok"; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn21Java.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn21Java.java index 79edbde6d644..58bb3a221b8a 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn21Java.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlightingPatternsInSwitch/UnconditionalDestructuringAndDefaultIn21Java.java @@ -246,8 +246,8 @@ class Main { System.out.println("ok"); } - str = switch (d) { - case MONDAY, TUESDAY -> System.out.println("ok"); + str = switch (d) { + case MONDAY, TUESDAY -> System.out.println("ok"); }; str = switch (d) { case MONDAY, TUESDAY, WEDNESDAY -> "ok"; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/Patterns.java b/java/java-tests/testData/inspection/dataFlow/fixture/Patterns.java index 8fa57984fc28..486cd0799be3 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/Patterns.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/Patterns.java @@ -1,6 +1,6 @@ class Test { int testIncomplete(Object obj) { - return switch(obj) { + return switch(obj) { case String s when }; } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/WhenPatterns.java b/java/java-tests/testData/inspection/dataFlow/fixture/WhenPatterns.java index d63cd1864776..564cffb7ebaa 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/WhenPatterns.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/WhenPatterns.java @@ -1,6 +1,6 @@ class Test { int testIncomplete(Object obj) { - return switch(obj) { + return switch(obj) { case String s when }; }