[java][switch] IDEA-273874 "Can't resolve symbol" false-negative in switch with fall-through semantics

Resolve as many references as possible in order to keep the GoToSymbol action available but move invalid references in pattern matching for switch to the highlighter pass as the code review suggests

GitOrigin-RevId: 2339b7c9cd02b0d1e3c793c30a8a3338c28c9b73
This commit is contained in:
Nikita Eshkeev
2021-07-26 03:13:56 +03:00
committed by intellij-monorepo-bot
parent 2d67e008e1
commit e807464037
5 changed files with 114 additions and 55 deletions

View File

@@ -24,6 +24,7 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
import java.util.stream.Collectors;
import static com.intellij.codeInsight.daemon.impl.analysis.SwitchBlockHighlightingModel.PatternsInSwitchBlockHighlightingModel.CompletenessResult.*;
import static com.intellij.psi.PsiModifier.ABSTRACT;
@@ -325,7 +326,6 @@ public class SwitchBlockHighlightingModel {
private enum SelectorKind {INT, ENUM, STRING, CLASS_OR_ARRAY}
public static class PatternsInSwitchBlockHighlightingModel extends SwitchBlockHighlightingModel {
PatternsInSwitchBlockHighlightingModel(@NotNull LanguageLevel languageLevel,
@NotNull PsiSwitchBlock switchBlock,
@NotNull PsiFile psiFile) {
@@ -569,6 +569,7 @@ public class SwitchBlockHighlightingModel {
}
}
checkFallThroughInSwitchLabels(switchBlockGroup, results, alreadyFallThroughElements);
checkFallthroughReferences(switchBlockGroup, results);
}
private static void checkFallThroughInSwitchLabels(@NotNull List<List<PsiSwitchLabelStatementBase>> switchBlockGroup,
@@ -576,13 +577,13 @@ public class SwitchBlockHighlightingModel {
@NotNull Set<PsiElement> alreadyFallThroughElements) {
for (int i = 1; i < switchBlockGroup.size(); i++) {
List<PsiSwitchLabelStatementBase> switchLabels = switchBlockGroup.get(i);
PsiSwitchLabelStatementBase firstSwitchLabelInGroup = switchLabels.get(0);
for (PsiSwitchLabelStatementBase switchLabel : switchLabels) {
if (!(switchLabel instanceof PsiSwitchLabelStatement)) return;
PsiCaseLabelElementList labelElementList = switchLabel.getCaseLabelElementList();
if (labelElementList == null) continue;
var patternElements = ContainerUtil.filter(labelElementList.getElements(), labelElement -> labelElement instanceof PsiPattern);
if (patternElements.isEmpty()) continue;
PsiSwitchLabelStatementBase firstSwitchLabelInGroup = switchLabels.get(0);
PsiStatement prevStatement = PsiTreeUtil.getPrevSiblingOfType(firstSwitchLabelInGroup, PsiStatement.class);
if (prevStatement == null) continue;
if (ControlFlowUtils.statementMayCompleteNormally(prevStatement)) {
@@ -731,6 +732,70 @@ public class SwitchBlockHighlightingModel {
results.add(info);
}
private static void checkFallthroughReferences(@NotNull List<List<PsiSwitchLabelStatementBase>> switchBlockGroup,
@NotNull List<HighlightInfo> results) {
final List<PsiSwitchLabelStatementBase> switches = switchBlockGroup.stream()
.flatMap(List::stream)
.limit(2)
.collect(Collectors.toList());
if (switches.size() < 2) return;
final Set<PsiPatternVariable> patternVariables = new HashSet<>();
int caseLabelsProcessed = 0;
for (PsiElement element = switches.get(0); element != null; element = element.getNextSibling()) {
if (element instanceof PsiSwitchLabelStatementBase) {
final PsiSwitchLabelStatementBase caseLabel = (PsiSwitchLabelStatementBase)element;
final PsiStatement prevStmt = PsiTreeUtil.getPrevSiblingOfType(caseLabel, PsiStatement.class);
final boolean currentLabelIsFallthrough = ControlFlowUtils.statementMayCompleteNormally(prevStmt);
if (!currentLabelIsFallthrough) {
caseLabelsProcessed = 0;
patternVariables.clear();
}
if (!isCaseNull(element)) caseLabelsProcessed++;
patternVariables.addAll(getPatternVariables(caseLabel));
continue;
}
if (caseLabelsProcessed < 2) continue;
final PatternVariableReferencesResolveVisitor visitor = new PatternVariableReferencesResolveVisitor(caseLabelsProcessed, patternVariables);
element.accept(visitor);
results.addAll(visitor.myResults);
}
}
@NotNull
private static Set<PsiPatternVariable> getPatternVariables(@Nullable PsiSwitchLabelStatementBase label) {
if (label == null) return Collections.emptySet();
final PsiCaseLabelElementList list = ((PsiSwitchLabelStatementBase)label).getCaseLabelElementList();
if (list == null) return Collections.emptySet();
return Arrays.stream(list.getElements())
.filter(PsiTypeTestPattern.class::isInstance)
.map(PsiTypeTestPattern.class::cast)
.map(PsiTypeTestPattern::getPatternVariable)
.collect(Collectors.toSet());
}
private static boolean isCaseNull(@Nullable PsiElement item) {
if (item == null) return false;
if (!(item instanceof PsiSwitchLabelStatementBase)) return false;
final PsiSwitchLabelStatementBase switchLabel = (PsiSwitchLabelStatementBase)item;
final PsiCaseLabelElementList caseElementsList = switchLabel.getCaseLabelElementList();
if (caseElementsList == null) return false;
final PsiCaseLabelElement[] elements = caseElementsList.getElements();
if (elements.length != 1) return false;
return elements[0].getNode().getFirstChildNode().getElementType() == JavaTokenType.NULL_KEYWORD;
}
@Nullable
private PsiElement findDefaultElement() {
PsiCodeBlock body = myBlock.getBody();
@@ -832,6 +897,32 @@ public class SwitchBlockHighlightingModel {
COMPLETE_WITH_TOTAL,
COMPLETE_WITHOUT_TOTAL
}
private static class PatternVariableReferencesResolveVisitor extends JavaRecursiveElementWalkingVisitor {
private final int myLabelsProcessed;
private final Set<PsiPatternVariable> myPatternVariables;
private final List<HighlightInfo> myResults;
public PatternVariableReferencesResolveVisitor(int totalCaseLabelsProcessed, Set<PsiPatternVariable> patternVariables) {
myLabelsProcessed = totalCaseLabelsProcessed;
myPatternVariables = patternVariables;
myResults = new SmartList<>();
}
@Override
public void visitReferenceElement(PsiJavaCodeReferenceElement reference) {
if (myLabelsProcessed < 2) return;
super.visitReferenceElement(reference);
final PsiElement anchor = reference.resolve();
if (!(anchor instanceof PsiPatternVariable) || !myPatternVariables.contains(anchor)) return;
final String referenceName = reference.getElement().getText();
final HighlightInfo error = createError(reference, JavaErrorBundle.message("cannot.resolve.symbol", referenceName));
myResults.add(error);
}
}
}
}

View File

@@ -104,53 +104,6 @@ public class PsiSwitchLabelStatementImpl extends PsiSwitchLabelStatementBaseImpl
return true;
});
if (!thisSwitchLabelIsImmediate.get()) return false;
final PsiElement prevSibling = PsiTreeUtil.skipWhitespacesBackward(getPrevSibling());
if (prevSibling instanceof PsiBreakStatement) return true;
if (prevSibling instanceof PsiBlockStatement && hasBreakStatement((PsiBlockStatement)prevSibling)) return true;
final PsiSwitchLabelStatement prevCaseLabel = PsiTreeUtil.getPrevSiblingOfType(this, PsiSwitchLabelStatement.class);
if (prevCaseLabel != null && isNullCaseLabel(prevCaseLabel)) return true;
return isFirstCaseLabel(this);
}
/**
* Checks if the passed switch case label is {@code "case null"} only
* @param label a switch case label to check
* @return true if the passed label is for {@code "case null"}
*/
private static boolean isNullCaseLabel(PsiSwitchLabelStatement label) {
final PsiCaseLabelElementList list = label.getCaseLabelElementList();
if (list == null) return true;
final PsiCaseLabelElement[] elements = list.getElements();
return elements.length == 1 && elements[0].textMatches(PsiKeyword.NULL);
}
/**
* Checks if the block statement contains the {@code break} keyword
* @param block a code block to analyze
* @return true if the passed code block contains {@code break}
*/
private static boolean hasBreakStatement(PsiBlockStatement block) {
final PsiCodeBlock prevCaseLabelCodeBlock = block.getCodeBlock();
final PsiStatement[] statements = prevCaseLabelCodeBlock.getStatements();
return statements.length != 0 && statements[statements.length - 1] instanceof PsiBreakStatement;
}
/**
* Checks if the passed {@link PsiSwitchLabelStatement} instance is the first switch case label of its switch statement
* @param switchLabel a switch case label to check
* @return true if the passed switch case label case is the first in its switch statement
*/
private static boolean isFirstCaseLabel(PsiSwitchLabelStatement switchLabel) {
final PsiElement switchBody = switchLabel.getParent();
if (!(switchBody instanceof PsiCodeBlock)) return false;
final PsiStatement[] statements = ((PsiCodeBlock)switchBody).getStatements();
return statements.length != 0 && statements[0] == switchLabel;
return thisSwitchLabelIsImmediate.get();
}
}

View File

@@ -1,5 +1,20 @@
class Main {
void ff(Object o) {
switch (o) {
case String s:
case null:
case <error descr="Illegal fall-through to a pattern">Integer i</error>:
System.out.println(<error descr="Cannot resolve symbol 'i'">i</error> + 1);
break;
case Long l:
System.out.println(l);
case <error descr="Illegal fall-through to a pattern">Character c</error>:
System.out.println(<error descr="Cannot resolve symbol 'c'">c</error>);
default:
throw new IllegalStateException("Unexpected value: " + o);
}
}
void f(Object o) {
switch (o) {
case null: {
@@ -74,7 +89,7 @@ class Main {
void m(Object o) {
switch (o) {
case String s:
case Integer i:
case <error descr="Illegal fall-through to a pattern">Integer i</error>:
System.out.println(<error descr="Cannot resolve symbol 'i'">i</error> + 1);
default:
throw new IllegalStateException("Unexpected value: " + o);

View File

@@ -15,7 +15,7 @@ public class Main {
case String s1:
System.out.println(<error descr="Cannot resolve symbol 's'">s</error>.length());
case Integer i:
System.out.println(<error descr="Cannot resolve symbol 'i'">i</error>);
System.out.println(i);
default:
System.out.println(1);
};

View File

@@ -40,12 +40,12 @@ class Main {
case Integer n && n > 1:
switch(o2) {
case Integer <error descr="Variable 'm' is already defined in the scope">m</error> && m > 0:
m += <error descr="Cannot resolve symbol 'n'">n</error>;
m += n;
case Integer <error descr="Variable 'p' is already defined in the scope">p</error> && p > 0:
p += <error descr="Cannot resolve symbol 'n'">n</error> + m;
p += n + m;
break;
case Integer p1:
p += <error descr="Cannot resolve symbol 'n'">n</error> + m + p1;
p += n + m + p1;
};
break;
};