Skip to content

Commit 8a03a74

Browse files
CatarinaGamboaclaude
CatarinaGamboa
and
claude
authored
Check each case of a switch from the state before it (#497)
Fixes #494 The verifier had no `switch` handling, so the cases ran one after another as straight-line code: `break` was ignored and the `case` label never entered the path condition. A `switch` is now checked like the `if` / `else if` chain over its labels: - each case starts from the state before the switch and knows `selector == label` (grouped labels: any of them; `default`: none of the other labels) - a case without `break` falls through into the next one - the states of the cases that reach the end are joined after the switch, as after an `if` - `return` / `throw` / `continue` in a case drop that path, as in an `if` - switch expressions (`->`, `yield`) are checked the same way; selectors may be enums, ints or strings (a string label has no known value, so it adds no path condition) Two forms are handled conservatively: - **pattern or guarded cases** (`case String s ->`, `when`): checked as before (#345 is unchanged) - **a `break` / `yield` nested inside a case** (e.g. `if (c) { ...; break; }`): the state at that jump is not followed, so what those cases change is forgotten on that path after the switch (shown as `true` in Found) **Study exercise A1_09 (litiengine), the partial fix `else if (newMode == BORDERLESS) host.dispose();`** Before, the same error at the same line, though BORDERLESS is now disposed: ``` State Refinement Error: found previousMode == BORDERLESS && newMode != BORDERLESS ? notDisplayable(host) : (newMode == BORDERLESS ? notDisplayable(host) : displayable(host)) but expected notDisplayable(host) 49 | host.setUndecorated(true); ``` After, the error moves to the remaining bug, the full-screen fallback: ``` State Refinement Error: found newMode == FULLSCREEN && (previousMode == BORDERLESS && newMode != BORDERLESS ? notDisplayable(host) : (newMode == BORDERLESS ? notDisplayable(host) : displayable(host))) but expected notDisplayable(host) 62 | host.setUndecorated(true); ``` **Study exercise A1_56 (JabRef), only `redo()` guarded** Before, the UNDO case saw what the REDO case did: ``` State Refinement Error: found (#canRedo_result ? redoable(undoManager@70) : notRedoable(undoManager@70)) && state2(undoManager@70) == state2(#getUndoManager_result) && limit(undoManager@70) == limit(#getUndoManager_result) && (#canRedo_result ? undoable(undoManager) && notRedoable(undoManager) && ... : ...) but expected undoable(undoManager) ``` After, only what holds on the UNDO path: ``` State Refinement Error: expected undoable(undoManager) but could not prove it 72 | case UNDO -> undoManager.undo(); Hints: - The state of undoManager is unknown here, and this call needs it to be undoable - Guard this call with if (undoManager.canUndo()): when it returns true, undoManager is undoable ``` The six study exercises still report their error at the same line. A1_09's Found now starts with the case's label: `found newMode == BORDERLESS && (...)`. The other study variants report the same as before, apart from A1_09's first-only fix: same line (63), but its Found was `displayable(host)`, the state the BORDERLESS case left, and is now the full-screen path's `newMode == FULLSCREEN && (...)`, longer than before. Tests: `CorrectEnumUsage` (state and label of each case, switch statement and expression), `ErrorEnumUsage` (an error that was missed in a case reached directly and by fall-through, and the nested `break`). `TestStateHints` now expects A1_56's UNDO hints above. Each fails on `main`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8958c70 commit 8a03a74

5 files changed

Lines changed: 292 additions & 11 deletions

File tree

‎liquidjava-example/src/main/java/testSuite/CorrectEnumUsage.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,54 @@ public static void main(String[] args) {
4242
st.setMode(Mode.Video); // noMode -> videoMode
4343
st.takeVideo();
4444
}
45+
46+
// Each case starts from the state before the switch and knows the value of the selector
47+
public static void capture(Mode mode) {
48+
CorrectEnumUsage st = new CorrectEnumUsage();
49+
switch (mode) {
50+
case Photo:
51+
st.setMode(Mode.Photo);
52+
st.takePhoto();
53+
break;
54+
case Video:
55+
st.setMode(mode);
56+
st.takeVideo();
57+
break;
58+
default:
59+
break;
60+
}
61+
}
62+
63+
public static boolean captureExpression(Mode mode) {
64+
CorrectEnumUsage st = new CorrectEnumUsage();
65+
return switch (mode) {
66+
case Photo -> {
67+
st.setMode(Mode.Photo);
68+
st.takePhoto();
69+
yield true;
70+
}
71+
case Video -> {
72+
st.setMode(Mode.Video);
73+
st.takeVideo();
74+
yield true;
75+
}
76+
default -> false;
77+
};
78+
}
79+
80+
// A switch expression that matches no label throws: it does not reach the code after it
81+
public static void captureOnThread(Thread.State state) {
82+
CorrectEnumUsage st = new CorrectEnumUsage();
83+
boolean running = switch (state) {
84+
case NEW, RUNNABLE, BLOCKED -> {
85+
st.setMode(Mode.Photo);
86+
yield true;
87+
}
88+
case WAITING, TIMED_WAITING, TERMINATED -> {
89+
st.setMode(Mode.Photo);
90+
yield false;
91+
}
92+
};
93+
st.takePhoto();
94+
}
4595
}

‎liquidjava-example/src/main/java/testSuite/ErrorEnumUsage.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,33 @@ public static void main(String[] args) {
3030
st.setMode(Mode.Video);
3131
st.takePhoto(); // Expect: State Refinement Error
3232
}
33+
34+
// Video is not in photoMode: only Photo falls through to it
35+
public static void capture(Mode mode) {
36+
ErrorEnumUsage st = new ErrorEnumUsage();
37+
switch (mode) {
38+
case Photo:
39+
st.setMode(Mode.Photo);
40+
case Video:
41+
st.takePhoto(); // Expect: State Refinement Error
42+
break;
43+
default:
44+
break;
45+
}
46+
}
47+
48+
// The break inside the if leaves the switch before setMode
49+
public static void captureUnlessSkipped(Mode mode, boolean skip) {
50+
ErrorEnumUsage st = new ErrorEnumUsage();
51+
switch (mode) {
52+
case Video:
53+
if (skip)
54+
break;
55+
st.setMode(Mode.Photo);
56+
break;
57+
default:
58+
st.setMode(Mode.Photo);
59+
}
60+
st.takePhoto(); // Expect: State Refinement Error
61+
}
3362
}

‎liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/RefinementTypeChecker.java‎

Lines changed: 206 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.util.ArrayList;
66
import java.util.Arrays;
77
import java.util.Deque;
8+
import java.util.HashSet;
89
import java.util.LinkedHashSet;
910
import java.util.List;
1011
import java.util.Optional;
@@ -29,13 +30,17 @@
2930
import liquidjava.utils.constants.Types;
3031

3132
import org.apache.commons.lang3.NotImplementedException;
33+
import spoon.reflect.code.CaseKind;
3234
import spoon.reflect.code.CtArrayRead;
3335
import spoon.reflect.code.CtAbstractInvocation;
36+
import spoon.reflect.code.CtAbstractSwitch;
3437
import spoon.reflect.code.CtArrayWrite;
3538
import spoon.reflect.code.CtAssignment;
3639
import spoon.reflect.code.CtBinaryOperator;
3740
import spoon.reflect.code.CtBlock;
3841
import spoon.reflect.code.CtBreak;
42+
import spoon.reflect.code.CtCFlowBreak;
43+
import spoon.reflect.code.CtCase;
3944
import spoon.reflect.code.CtCatchVariable;
4045
import spoon.reflect.code.CtConditional;
4146
import spoon.reflect.code.CtContinue;
@@ -57,6 +62,7 @@
5762
import spoon.reflect.code.CtNewArray;
5863
import spoon.reflect.code.CtNewClass;
5964
import spoon.reflect.code.CtOperatorAssignment;
65+
import spoon.reflect.code.CtPattern;
6066
import spoon.reflect.code.CtReturn;
6167
import spoon.reflect.code.CtStatement;
6268
import spoon.reflect.code.CtSuperAccess;
@@ -71,6 +77,7 @@
7177
import spoon.reflect.code.CtVariableRead;
7278
import spoon.reflect.code.CtVariableWrite;
7379
import spoon.reflect.code.CtWhile;
80+
import spoon.reflect.code.CtYieldStatement;
7481
import spoon.reflect.declaration.*;
7582
import spoon.reflect.factory.Factory;
7683
import spoon.reflect.reference.CtFieldReference;
@@ -575,6 +582,190 @@ public void visitCtIf(CtIf ifElement) {
575582
context.variablesFinishIfCombination();
576583
}
577584

585+
@Override
586+
public <S> void visitCtSwitch(CtSwitch<S> switchStatement) {
587+
if (!visitSwitch(switchStatement))
588+
super.visitCtSwitch(switchStatement);
589+
}
590+
591+
@Override
592+
public <T, S> void visitCtSwitchExpression(CtSwitchExpression<T, S> switchExpression) {
593+
if (!visitSwitch(switchExpression))
594+
super.visitCtSwitchExpression(switchExpression);
595+
}
596+
597+
/**
598+
* Checks a switch as the chain of ifs that tests its labels in turn (see visitCtIf): each case is checked from the
599+
* state before the switch, knowing that the selector equals one of its labels, and the default case knowing that it
600+
* equals none of the others. A case that falls through goes on into the next one. The states of the cases that
601+
* reach the end of the switch are joined after it. Pattern and guarded cases are not modelled.
602+
*
603+
* @return whether the switch was checked, false when it has pattern or guarded cases
604+
*/
605+
private boolean visitSwitch(CtAbstractSwitch<?> switchElement) {
606+
List<CtCase<?>> cases = new ArrayList<>(switchElement.getCases());
607+
if (cases.stream().anyMatch(c -> c.getGuard() != null
608+
|| c.getCaseExpressions().stream().anyMatch(label -> label instanceof CtPattern)))
609+
return false;
610+
CtExpression<?> selector = switchElement.getSelector();
611+
scan(selector);
612+
// the selector is evaluated once, so the cases compare its value then, not what its variables hold later
613+
String selectorName = GeneratedNames.of(GeneratedNames.SWITCH_SELECTOR, context.getCounter());
614+
Predicate selectorRefs = getRefinement(selector).substituteVariable(Keys.WILDCARD, selectorName);
615+
selectorRefs = snapshotMutableParameters(substituteAllVariablesForLastInstance(selectorRefs), selector);
616+
context.addInstanceToContext(selectorName, selector.getType(), selectorRefs, selector);
617+
618+
// the cases with no statements share the statements of the next case, the default one is tested last
619+
List<CaseEntry> entries = new ArrayList<>();
620+
CaseEntry defaultEntry = null;
621+
List<Predicate> labels = new ArrayList<>();
622+
boolean isDefault = false;
623+
int start = 0;
624+
for (int i = 0; i < cases.size(); i++) {
625+
CtCase<?> c = cases.get(i);
626+
for (CtExpression<?> label : c.getCaseExpressions()) {
627+
scan(label);
628+
labels.add(getRefinement(label));
629+
}
630+
isDefault |= c.getCaseExpressions().isEmpty() || c.getIncludesDefault();
631+
if (c.getStatements().isEmpty() && i < cases.size() - 1)
632+
continue;
633+
CaseEntry entry = new CaseEntry(start, labels);
634+
if (isDefault)
635+
defaultEntry = entry;
636+
else
637+
entries.add(entry);
638+
labels = new ArrayList<>();
639+
isDefault = false;
640+
start = i + 1;
641+
}
642+
if (defaultEntry != null)
643+
entries.add(defaultEntry);
644+
visitCases(switchElement, cases, entries, 0, selectorName, defaultEntry != null, new HashSet<>());
645+
return true;
646+
}
647+
648+
/** A case to which the selector jumps when it equals one of {@code labels}, by their refinements */
649+
private record CaseEntry(int start, List<Predicate> labels) {
650+
/** That the selector equals one of the labels, or null when a label has no known value */
651+
Predicate matches(String selectorName) {
652+
Predicate matches = null;
653+
for (Predicate label : labels) {
654+
if (!label.getVariableNames().contains(Keys.WILDCARD))
655+
return null;
656+
Predicate equals = label.substituteVariable(Keys.WILDCARD, selectorName);
657+
matches = matches == null ? equals : Predicate.createDisjunction(matches, equals);
658+
}
659+
return matches;
660+
}
661+
}
662+
663+
/**
664+
* Checks the entries of a switch from {@code index} on, as an if that tests the one at {@code index} and checks the
665+
* others in its else, the last one being the default case if there is one
666+
*
667+
* @return whether the end of the switch can be reached from here
668+
*/
669+
private boolean visitCases(CtAbstractSwitch<?> switchElement, List<CtCase<?>> cases, List<CaseEntry> entries,
670+
int index, String selectorName, boolean hasDefault, Set<CtCase<?>> checked) {
671+
// no label matches and there is no default case: a switch expression then throws
672+
if (index == entries.size())
673+
return !(switchElement instanceof CtSwitchExpression<?, ?>);
674+
CaseEntry entry = entries.get(index);
675+
if (hasDefault && index == entries.size() - 1)
676+
return visitCasesFrom(switchElement, cases, entry.start(), checked);
677+
678+
String pathVarName = GeneratedNames.of(GeneratedNames.CASE_CONDITION, context.getCounter());
679+
// a label with no known value gives no information about which way the test went
680+
Predicate matches = entry.matches(selectorName);
681+
Predicate branchGuard = matches != null ? matches : Predicate.createVar(pathVarName);
682+
RefinedVariable pathVar = context.addInstanceToContext(pathVarName, factory.Type().BOOLEAN_PRIMITIVE,
683+
branchGuard, cases.get(entry.start()));
684+
keepFreshValues(branchGuard);
685+
vcChecker.addPathVariable(pathVar);
686+
687+
context.variablesNewIfCombination();
688+
context.variablesSetBeforeIf();
689+
context.enterContext();
690+
691+
context.enterContext();
692+
boolean thenReaches = visitCasesFrom(switchElement, cases, entry.start(), checked);
693+
if (thenReaches)
694+
context.variablesSetThenIf();
695+
context.exitContext();
696+
697+
context.newRefinementToVariableInContext(pathVarName, branchGuard.negate());
698+
context.enterContext();
699+
boolean elseReaches = visitCases(switchElement, cases, entries, index + 1, selectorName, hasDefault, checked);
700+
if (elseReaches)
701+
context.variablesSetElseIf();
702+
context.exitContext();
703+
704+
if (thenReaches == elseReaches) {
705+
context.newRefinementToVariableInContext(pathVarName, matches != null ? matches : new Predicate());
706+
vcChecker.removePathVariable(pathVar);
707+
} else {
708+
// keep the guard of the only branch that reaches the end of the switch
709+
context.newRefinementToVariableInContext(pathVarName, thenReaches ? branchGuard : branchGuard.negate());
710+
}
711+
context.exitContext();
712+
context.variablesCombineFromIf(branchGuard);
713+
context.variablesFinishIfCombination();
714+
return thenReaches || elseReaches;
715+
}
716+
717+
/**
718+
* Checks the statements of the case at {@code start}, and of the next cases while they fall through. The path
719+
* conditions they add are dropped after them.
720+
*
721+
* @return whether they reach the end of the switch
722+
*/
723+
private boolean visitCasesFrom(CtAbstractSwitch<?> switchElement, List<CtCase<?>> cases, int start,
724+
Set<CtCase<?>> checked) {
725+
List<RefinedVariable> pathVariables = vcChecker.getPathVariables();
726+
List<CtCFlowBreak> exits = new ArrayList<>();
727+
int end = start;
728+
boolean completes;
729+
while (true) {
730+
CtCase<?> c = cases.get(end);
731+
if (!checked.add(c))
732+
removeCheckMetadata(c, List.of()); // checked again after falling through from another case
733+
for (CtStatement statement : c.getStatements())
734+
scan(statement);
735+
exits.addAll(c.getElements(new TypeFilter<>(CtCFlowBreak.class)).stream()
736+
.filter(jump -> leaves(jump, switchElement)).toList());
737+
List<CtStatement> statements = c.getStatements();
738+
completes = statements.isEmpty() || canCompleteNormally(statements.get(statements.size() - 1));
739+
if (!completes || c.getCaseKind() == CaseKind.ARROW || end == cases.size() - 1)
740+
break;
741+
end++;
742+
}
743+
List<CtStatement> last = cases.get(end).getStatements();
744+
CtStatement lastExit = last.isEmpty() ? null : endingJump(last.get(last.size() - 1));
745+
// the states where a break or yield inside a statement leaves the switch are not followed: forget what these
746+
// cases change
747+
if (exits.stream().anyMatch(jump -> jump != lastExit))
748+
for (CtCase<?> c : cases.subList(start, end + 1))
749+
for (String name : changedIn(c, switchElement))
750+
forgetChanges(name, new Predicate(), c);
751+
vcChecker.restorePathVariables(pathVariables);
752+
return completes || !exits.isEmpty();
753+
}
754+
755+
/** Whether {@code jump} is a break or yield that leaves {@code switchElement} */
756+
private static boolean leaves(CtCFlowBreak jump, CtAbstractSwitch<?> switchElement) {
757+
if (jump instanceof CtBreak breakStatement)
758+
return jumpTarget(breakStatement) == switchElement;
759+
return jump instanceof CtYieldStatement && jump.getParent(CtSwitchExpression.class) == switchElement;
760+
}
761+
762+
/** The jump that a statement ends with, as the last statement of its blocks */
763+
private static CtStatement endingJump(CtStatement statement) {
764+
if (statement instanceof CtBlock<?> block)
765+
return block.getStatements().isEmpty() ? null : endingJump(block.getLastStatement());
766+
return statement instanceof CtCFlowBreak ? statement : null;
767+
}
768+
578769
private Predicate snapshotMutableParameters(Predicate predicate, CtExpression<?> condition) {
579770
// An unchanged parameter can keep its name; a later write must not change this guard's meaning.
580771
CtExecutable<?> executable = condition.getParent(CtExecutable.class);
@@ -651,7 +842,7 @@ boolean canCompleteNormally(CtStatement statement) {
651842
if (statement == null || isLeftByLabelledBreak(statement))
652843
return true;
653844
if (statement instanceof CtReturn<?> || statement instanceof CtThrow || statement instanceof CtBreak
654-
|| statement instanceof CtContinue)
845+
|| statement instanceof CtContinue || statement instanceof CtYieldStatement)
655846
return false;
656847
if (statement instanceof CtBlock<?> block) {
657848
List<CtStatement> statements = block.getStatements();
@@ -804,7 +995,11 @@ private static void removeCheckMetadata(CtLoop loop) {
804995
checkedBefore.addAll(forLoop.getForInit());
805996
if (loop instanceof CtForEach forEach)
806997
checkedBefore.add(forEach.getExpression());
807-
for (CtElement element : loop.getElements((CtElement e) -> true)) {
998+
removeCheckMetadata(loop, checkedBefore);
999+
}
1000+
1001+
private static void removeCheckMetadata(CtElement checked, List<CtElement> checkedBefore) {
1002+
for (CtElement element : checked.getElements((CtElement e) -> true)) {
8081003
if (element instanceof CtTypeMember
8091004
|| (element instanceof CtParameter<?> && element.getParent() instanceof CtTypeMember)
8101005
|| checkedBefore.stream().anyMatch(before -> element == before || element.hasParent(before)))
@@ -906,9 +1101,14 @@ void forgetChanges(String name, Predicate known, CtElement element) {
9061101
* if it calls any method or constructor (which may write them)
9071102
*/
9081103
private Set<String> changedIn(CtLoop loop) {
1104+
return changedIn(loop, loop.getBody());
1105+
}
1106+
1107+
/** What {@code element} may change, but the variables declared in {@code scope}, which do not outlive it */
1108+
private Set<String> changedIn(CtElement element, CtElement scope) {
9091109
List<CtVariableAccess<?>> changed = new ArrayList<>(
910-
loop.getElements(new TypeFilter<CtVariableWrite<?>>(CtVariableWrite.class)));
911-
List<CtAbstractInvocation<?>> calls = loop
1110+
element.getElements(new TypeFilter<CtVariableWrite<?>>(CtVariableWrite.class)));
1111+
List<CtAbstractInvocation<?>> calls = element
9121112
.getElements(new TypeFilter<CtAbstractInvocation<?>>(CtAbstractInvocation.class));
9131113
Set<String> names = new LinkedHashSet<>();
9141114
for (CtAbstractInvocation<?> call : calls) {
@@ -921,8 +1121,8 @@ else if (inv.getTarget() != null && AuxStateHandler.isCurrentReceiver(inv.getTar
9211121
}
9221122
for (CtVariableAccess<?> access : changed) {
9231123
CtVariable<?> declaration = access.getVariable().getDeclaration();
924-
if (declaration != null && declaration.hasParent(loop.getBody()))
925-
continue; // declared in the body: a new variable on each iteration
1124+
if (declaration != null && declaration.hasParent(scope))
1125+
continue; // e.g. declared in a loop body: a new variable on each iteration
9261126
String name = access.getVariable().getSimpleName();
9271127
names.add(access instanceof CtFieldAccess<?> ? String.format(Formats.THIS, name) : name);
9281128
}

‎liquidjava-verifier/src/main/java/liquidjava/utils/GeneratedNames.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ public final class GeneratedNames {
1919
public static final String IF_CONDITION = "if_condition";
2020
public static final String LOOP_CONDITION = "loop_condition";
2121
public static final String TERNARY_CONDITION = "ternary_condition";
22+
public static final String SWITCH_SELECTOR = "switch_selector";
23+
public static final String CASE_CONDITION = "case_condition";
2224
public static final String TRY_BRANCH = "try_branch";
2325
public static final String CATCH_BRANCH = "catch_branch";
2426
public static final String ARRAY_LENGTH = "array_length";

‎liquidjava-verifier/src/test/java/liquidjava/api/tests/TestStateHints.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,11 @@ void checksOfTheStudysUndoManagerAreSuggestedOneAfterTheOther() {
325325
"The state of undoManager is unknown here, and this call needs it to be redoable"),
326326
new Hint(Kind.GUARD,
327327
"Guard this call with if (undoManager.canRedo()): when it returns true, undoManager is redoable"));
328-
assertHints(errors.get(1), new Hint(Kind.GUARD,
329-
"Guard this call with if (undoManager.canUndo()): when it returns true, undoManager is undoable"),
330-
new Hint(Kind.AVAILABLE_CALLS,
331-
"In state undoable and notRedoable or notUndoable and notRedoable, undoManager can call: "
332-
+ "canUndo(), canRedo() (and possibly others)"));
328+
assertHints(errors.get(1),
329+
new Hint(Kind.NEEDED_STATE,
330+
"The state of undoManager is unknown here, and this call needs it to be undoable"),
331+
new Hint(Kind.GUARD,
332+
"Guard this call with if (undoManager.canUndo()): when it returns true, undoManager is undoable"));
333333
}
334334

335335
@Test

0 commit comments

Comments
 (0)