Skip to content

Commit 48ef72d

Browse files
Suggest only calls allowed in any state when the receiver's state is unknown (#458)
Part of #454 (part (a): what the hint suggests when any state is possible, and how hints name states). Part (b), suggesting a check such as `canRedo()`, needs return-value queries modelled as guards. That is a separate change. ## Problem When the receiver's state is unknown, the hint has two problems: 1. It suggests transitions whose own precondition is just as unknown. In the issue, `redo()` needs `redoable`, and the hint suggests `undo()`, which needs `undoable`. Following it only moves the error to `undo()`. 2. It names states in every combination with state sets the call does not check: "undoable and redoable or notUndoable and redoable", and six combinations with the study's three-set spec. Hints for partly known states do the same ("No sequence of calls moves this from <6 combinations> to <6 combinations>"). Before (the issue's JabRef exercise in VS Code): ![before](https://raw.githubusercontent.com/liquid-java/liquidjava/pr-screenshots/issues/454/verification.png) ## Cause `StateHints.reachingCalls` listed every transition from a state that does not allow the call into one that does (`StateMachine.transitionsEnabling`), whatever its `from`. All hints printed a set of states by joining each state's names in every state set. ## Fix - **States are named briefly, everywhere.** `StateMachine.describe` (new `StateCover`) names a set of states as the fewest groups that each fix the state in some state sets and allow any state in the others: a smallest cover by maximal sub-products of the state sets. It works on the machine's solver-built states, with no string matching. Every state hint uses it, for the current, needed and reached states. For example: - `redoable(this)` with three state sets reads `redoable`; - `closed(this) && unlocked(this)` reads `closed and unlocked`; - `open(this) || loud(this)` across two sets reads `open or loud`; - two refinements `from = "undoable(this)"` and `from = "redoable(this)"` read `undoable or redoable`; - a receiver known only to be `notRedoable` reads `notRedoable`. The receiver's current state is named in every state set the call checks, plus any set its states agree on. So a join that differs in a checked set still lists each state (`closed and unlocked or opened and unlocked`), and one that differs only in an unchecked set does not (`closed`). A single fully known state reads as before (`closed and locked`). When every state allows the call, each by its own refinement, the needed state is named per refinement (`a or b`, not every state). - **Only calls that work from any state.** When any state is possible, the hint reuses the call sequence search of the known-state case, starting from every state. A call is suggested only if it is allowed in every state. That covers transitions with no `from` and sequences that start with one (`reset(), then open()`). Same limits as before: at most 3 sequences of at most 3 calls. - **The needed state is always named.** The `NEEDED_STATE` hint (renamed from `REACHING_CALLS`, which nothing in vscode-liquidjava or liquidjava-mcp uses) always names the state the call needs, plus the constructors that create an object in it. That includes a call that every state allows, each by a different refinement (a toggle). Constructors are not suggested when the receiver is `this`. The same applies to the existing "Create a new object instead" hint. - **Only constructors that can be called.** `StateMachineBuilder` leaves out the constructors of abstract classes and interfaces (`abstractType`, as in #464), enums, and private constructors, so no hint suggests `new` for them. - **A failed check is not "unknown".** In `if (!it.hasNext()) it.next();`, the receiver's own refinement says nothing, but the error's found (`(#fresh == true --> hasMore(it)) && !#fresh`) mentions its state through the check's result. `StateErrorCategories.checkedElsewhere` looks at what the error shows as found (`TypeChecker.shownFound`, the same premise chain the error prints). It only computes that when every state is possible and the receiver's own refinement mentions none. It counts a state only if it is applied to the receiver's instance, or to an object the solver proves equal to it (`Scanner same = it; if (!it.hasNext()) same.next();`). A check on another object (`if (other.hasNext()) it.next();`, or the "checked the wrong object" `if (it.hasNext()) other.next();`) leaves the state unknown. When it does count, the category is none instead of UNKNOWN, and the hint reads "What is known here does not decide the state of it, and this call needs it to be hasMore". Joins of every state across several state sets (`if (p) d.open(); if (q) d.lock(); d.enter();`) are not checks. They keep main's category (none) and main's "No sequence of calls moves d from … Create a new object instead" hint. - **What changes outside the unknown case:** - The only category change is UNKNOWN to none for a failed check on the receiver itself (`iterator_observer_error`, `state_test_method_error`). - Known-state hints change in two ways. States are named more briefly, as above. The constructor suggestions also go away for `this` receivers and for abstract classes, interfaces, enums and private constructors (in "Create a new object instead with …"). - `StateMachineBuilder` now leaves those constructors out of the machine. `canModel` and the methods it admits are unchanged. **Extension point for part (b).** `StateHints.anyState` builds the hints for "any state is possible": the needed state, then the call sequences from any state. It already tells the two causes apart: nothing known, or a check that did not decide the state. A guard hint goes between the two hints, once the machine records queries whose return value decides a state set. For example: "Check `undoManager.canRedo()` before this call", or for a failed check, "Call `it.next()` only when `it.hasNext()` returns true". **Clean-code pass** (878738a, no change in any hint: all 84 state fixtures and 30 reviewer reproducers print the same hints before and after): - `StateCover` searches for the smallest cover exactly, with no greedy fallback, and keeps each group's states with it. - `StateHints` takes the needed states from one source, and names the checked state sets (`checkedSets`) and the check flag (`checkedElsewhere`) apart. - `StateErrorCategories` shares its "every state possible, none mentioned" test. - `StateMachine.statesAllowing` is derived from `statesAllowingByRefinement`. - `AuxStateHandler` only looks for a check when no argument decided the state. ## Hint output, before (main) and after **Issue reproducer** (external `UndoManager` spec, two state sets, receiver from a method return): ``` before: The state of undoManager is unknown here. To get it to undoable and redoable or notUndoable and redoable, use: undoManager.undo() (from undoable and notRedoable) after: The state of undoManager is unknown here, and this call needs it to be redoable ``` **Three state sets** (parameter receiver): ``` before: The state of history is unknown here. To get it to start and undoable and redoable or start and notUndoable and redoable or canEdit and undoable and redoable or canEdit and notUndoable and redoable or end and undoable and redoable or end and notUndoable and redoable, use: history.undo() (from start and undoable and notRedoable or canEdit and undoable and notRedoable or end and undoable and notRedoable) after: The state of history is unknown here, and this call needs it to be redoable ``` **Disjunction across state sets** (`from = "open(this) || loud(this)"`) and **one refinement per set** (`undoOrRedo()`): ``` before: No state information is known for this object. Make sure its state is initialized and available at this point. after: The state of s is unknown here, and this call needs it to be open or loud after: The state of history is unknown here, and this call needs it to be undoable or redoable ``` **A failed check** (`if (!it.hasNext()) it.next();`): ``` before: (no hint) after: What is known here does not decide the state of it, and this call needs it to be hasMore ``` **A call allowed in any state** (`reset()` has no `from`; `open()` needs `created`): ``` before: The state of f is unknown here. To get it to open, use: f.open() (from created) after: The state of f is unknown here, and this call needs it to be open Call f.reset(), then f.open() before this call to move f from any state to open ``` **Partly known state** (`this` known to be `notRedoable`, three state sets): ``` before: No sequence of calls moves this from start and undoable and notRedoable or start and notUndoable and notRedoable or canEdit and undoable and notRedoable or canEdit and notUndoable and notRedoable or end and undoable and notRedoable or end and notUndoable and notRedoable to start and undoable and redoable or ... (6 combinations) In state start and undoable and notRedoable or ... (6 combinations), this can call: check() after: No sequence of calls moves this from notRedoable to redoable In state notRedoable, this can call: check() ``` **Toggle** (a class whose `flip()` goes from `up` to `down` and back): ``` before: The state of t is unknown here. To get it to up or down, use: new ErrorStateHintUnknownToggle() after: The state of t is unknown here, and this call needs it to be up or down. Objects created with new ErrorStateHintUnknownToggle() start in such a state ``` **Known state, unchanged** (after `open(); close();`): ``` Call f.reset(), then f.open() before this call to move f from closed to open In state closed, f can call: reset() ``` **Every other state fixture.** I ran each `Error*` state fixture and test-suite directory on main and on this branch and compared the hints. Apart from the cases above, the only changes are: - every "unknown" hint now has the new wording; - `iterator_observer_error` and `resultset_forward_error` (line 67) now get the needed-state hint. On main they got the generic "No state information is known…" hint, or no hint after a failed `hasNext()`; - `state_test_method_error` (`if (edit.canUndo()) {} else edit.undo();`) now reads "What is known here does not decide the state of edit…" instead of "unknown", and no longer suggests `edit.redo()`; - `ErrorLoopPostState` no longer suggests `new ErrorLoopPostState()` for `this`; - `ErrorPathDependentOtherStateSet` now reads "from closed to opened" instead of two combinations each. **Merging with #464.** Both PRs touch `StateMachineBuilder` and `StateHints`, and the conflicts are mechanical: - keep one `abstractType`: both PRs add an identical one; - move #464's `.distinct()` on the constructor lists into `StateHints.newObjects`, which replaces both lists here; - merge this PR's `typeName` (in `privateConstructor`) with #464's `parameterTypes`, which do the same thing. ## Tests Each test pins one behaviour. To keep CI fast, the fixtures are grouped so that one verifier run checks several errors. Every new test fails without its fix: on the PR's base, or on the commit before the fix it pins. | Test (`TestStateHints`) | Fixture | Pins | |---|---|---| | `unknownStateOfAnExternalClassNamesOnlyTheCheckedStateSet` | `classes/state_hint_undo_manager_error` | the issue reproducer: needs `redoable`, `undo()` not suggested | | `unknownStateDoesNotSuggestACallThatNeedsAnotherUnknownState` | `ErrorStateHintUnknownState` (existing, expectation changed) | a call that needs another unknown state is not suggested | | `unknownStateSuggestsCallsAllowedInAnyState` | `ErrorStateHintUnknownAnyState` | `reset(), then open()` is suggested; `open()` alone is not | | `statesAreNamedOnlyInTheStateSetsThatMatter` | `ErrorStateHintNaming` (3 errors) | `open or loud` across sets; a toggle on `{a, b} × {x, y, z}` reads `a or b`; a partly known state reads `notRedoable` | | `failedCheckOfTheReceiverIsNotAnUnknownState` | `classes/state_hint_checked_error` (3 errors) | a failed `hasNext()` on the receiver, or on a variable holding it, "does not decide", with no category; a check of another scanner leaves it unknown (UNKNOWN) | | `joinsAreNotTakenForAnUnknownState` | `ErrorStateHintJoins` (2 errors) | a join of every state of two sets keeps main's "No sequence of calls" hint, with no category; a join in one checked set lists both states | | `onlyConstructorsThatCanBeCalledAreSuggested` | `ErrorStateHintConstructors` (4 errors) | a concrete class's constructor is suggested; none for `this`, an abstract class or a private constructor | `StateMachineTest` has 10 unit tests for `describe` and `stateSetsNamed`. They run in milliseconds and cover the state naming: one set, cross-set and three-set disjunctions, every state, forced sets, and declaration order. Removed as overlapping: separate fixtures for three state sets, a one-set disjunction, one refinement per set, a two-set goal, a field receiver, several and long sequences, the one-set toggle, a known state, and the external abstract and interface specs. Each is covered by one of the tests above, by an existing test or by the unit tests. The interface spec never reached the guard, since an interface cannot declare a constructor refinement. **Test time** (`mvn test` for the verifier, two interleaved runs of each on the same machine, surefire time summed over test cases): | | tests | surefire | wall | `TestExamples` | `TestStateHints` | |---|---|---|---|---|---| | base (0053074) | 547 | 81.9 s | 89.1 s | 64.8 s | 2.2 s | | PR before trimming (b9683e9) | 613 | 98.6 s (+16.7) | 106.0 s (+16.9) | 72.2 s | 9.7 s | | PR now | 569 | 88.2 s (+6.3) | 95.2 s (+6.1) | 67.3 s | 5.3 s | `mvn test` for the verifier: **569 tests, 0 failures, 0 errors**. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 00cdb11 commit 48ef72d

19 files changed

Lines changed: 856 additions & 67 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package testSuite;
2+
3+
import liquidjava.specification.StateRefinement;
4+
import liquidjava.specification.StateSet;
5+
6+
// Each constructor creates the needed state, but only one that can be called is suggested
7+
@StateSet({"fresh", "ready"})
8+
public class ErrorStateHintConstructors {
9+
10+
@StateRefinement(to = "ready(this)")
11+
public ErrorStateHintConstructors() {}
12+
13+
@StateRefinement(from = "ready(this)", to = "fresh(this)")
14+
public void use() {}
15+
16+
// a concrete class: the constructor is suggested
17+
static void concrete(ErrorStateHintConstructors value) {
18+
value.use(); // Expect: State Refinement Error
19+
}
20+
21+
// the receiver is this object, which a new object would not change
22+
public void self() {
23+
this.use(); // Expect: State Refinement Error
24+
}
25+
}
26+
27+
@StateSet({"fresh", "ready"})
28+
abstract class ErrorStateHintConstructorsAbstract {
29+
30+
@StateRefinement(to = "ready(this)")
31+
public ErrorStateHintConstructorsAbstract() {}
32+
33+
@StateRefinement(from = "ready(this)", to = "fresh(this)")
34+
public void use() {}
35+
36+
static void fail(ErrorStateHintConstructorsAbstract value) {
37+
value.use(); // Expect: State Refinement Error
38+
}
39+
}
40+
41+
@StateSet({"fresh", "ready"})
42+
class ErrorStateHintConstructorsPrivate {
43+
44+
@StateRefinement(to = "ready(this)")
45+
private ErrorStateHintConstructorsPrivate() {}
46+
47+
@StateRefinement(from = "ready(this)", to = "fresh(this)")
48+
public void use() {}
49+
}
50+
51+
class ErrorStateHintConstructorsPrivateUser {
52+
static void fail(ErrorStateHintConstructorsPrivate value) {
53+
value.use(); // Expect: State Refinement Error
54+
}
55+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
package testSuite;
2+
3+
import liquidjava.specification.StateRefinement;
4+
import liquidjava.specification.StateSet;
5+
6+
// Joins of the states reached on different paths are not a lack of knowledge
7+
@StateSet({"closed", "opened"})
8+
@StateSet({"unlocked", "locked"})
9+
public class ErrorStateHintJoins {
10+
11+
@StateRefinement(to = "closed(this) && unlocked(this)")
12+
public ErrorStateHintJoins() {}
13+
14+
@StateRefinement(from = "closed(this)", to = "opened(this)")
15+
public void open() {}
16+
17+
@StateRefinement(from = "unlocked(this)", to = "locked(this)")
18+
public void lock() {}
19+
20+
@StateRefinement(from = "closed(this) && unlocked(this)")
21+
public void enter() {}
22+
23+
// every combination of the two state sets is reached on some path
24+
public static void everyState(boolean p, boolean q) {
25+
ErrorStateHintJoins d = new ErrorStateHintJoins();
26+
if (p)
27+
d.open();
28+
if (q)
29+
d.lock();
30+
d.enter(); // Expect: State Refinement Error
31+
}
32+
33+
// the paths differ in one of the two state sets the call checks, so both states are named in full
34+
public static void oneSet(boolean p) {
35+
ErrorStateHintJoins d = new ErrorStateHintJoins();
36+
if (p)
37+
d.open();
38+
d.enter(); // Expect: State Refinement Error
39+
}
40+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
package testSuite;
2+
3+
import liquidjava.specification.StateRefinement;
4+
import liquidjava.specification.StateSet;
5+
6+
// The needed and current states are named only in the state sets that matter
7+
@StateSet({"created", "open", "ready"})
8+
@StateSet({"quiet", "loud"})
9+
public class ErrorStateHintNaming {
10+
11+
@StateRefinement(to = "created(this) && quiet(this)")
12+
public ErrorStateHintNaming() {}
13+
14+
// a state of either set: each is named alone, not combined with every state of the other set
15+
@StateRefinement(from = "open(this) || loud(this)")
16+
public void read() {}
17+
18+
public static void use(ErrorStateHintNaming s) {
19+
s.read(); // Expect: State Refinement Error
20+
}
21+
}
22+
23+
// every state allows flip(), each by its own refinement: a or b, not all six states
24+
@StateSet({"a", "b"})
25+
@StateSet({"x", "y", "z"})
26+
class ErrorStateHintNamingToggle {
27+
28+
@StateRefinement(to = "a(this) && x(this)")
29+
public ErrorStateHintNamingToggle() {}
30+
31+
@StateRefinement(from = "a(this)", to = "b(this)")
32+
@StateRefinement(from = "b(this)", to = "a(this)")
33+
public void flip() {}
34+
35+
public static void use(ErrorStateHintNamingToggle t) {
36+
t.flip(); // Expect: State Refinement Error
37+
}
38+
}
39+
40+
// known in one of three state sets: only that set is named
41+
@StateSet({"start", "canEdit", "end"})
42+
@StateSet({"undoable", "notUndoable"})
43+
@StateSet({"redoable", "notRedoable"})
44+
class ErrorStateHintNamingPartlyKnown {
45+
46+
@StateRefinement(to = "start(this) && notUndoable(this) && notRedoable(this)")
47+
public ErrorStateHintNamingPartlyKnown() {}
48+
49+
@StateRefinement(from = "redoable(this)", to = "undoable(this) && notRedoable(this)")
50+
public void redo() {}
51+
52+
@StateRefinement(from = "undoable(this)", to = "notUndoable(this) && redoable(this)")
53+
public void undo() {}
54+
55+
@StateRefinement(from = "notRedoable(this)")
56+
public void check() {
57+
this.redo(); // Expect: State Refinement Error
58+
}
59+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package testSuite;
2+
3+
import liquidjava.specification.StateRefinement;
4+
import liquidjava.specification.StateSet;
5+
6+
// reset() works in any state, so it is suggested even though the state is unknown; open() alone is not
7+
@StateSet({"created", "open", "closed"})
8+
public class ErrorStateHintUnknownAnyState {
9+
10+
@StateRefinement(to = "created(this)")
11+
public ErrorStateHintUnknownAnyState() {}
12+
13+
@StateRefinement(to = "created(this)")
14+
public void reset() {}
15+
16+
@StateRefinement(from = "created(this)", to = "open(this)")
17+
public void open() {}
18+
19+
@StateRefinement(from = "open(this)")
20+
public void read() {}
21+
22+
@StateRefinement(from = "open(this)", to = "closed(this)")
23+
public void close() {}
24+
25+
public static void use(ErrorStateHintUnknownAnyState f) {
26+
f.read(); // Expect: State Refinement Error
27+
}
28+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package testSuite.classes.state_hint_checked_error;
2+
3+
import java.util.Scanner;
4+
5+
// A failed check of the receiver does not decide its state; a check of another object says nothing about it
6+
public class Reader {
7+
8+
public static void failedCheck(Scanner it) {
9+
if (!it.hasNext()) {
10+
it.next(); // Expect: State Refinement Error
11+
}
12+
}
13+
14+
public static void failedCheckOfTheSameScanner(Scanner scanner) {
15+
Scanner same = scanner;
16+
if (!scanner.hasNext()) {
17+
same.next(); // Expect: State Refinement Error
18+
}
19+
}
20+
21+
public static void checkOfAnotherScanner(Scanner other, Scanner input) {
22+
if (other.hasNext()) {
23+
input.next(); // Expect: State Refinement Error
24+
}
25+
}
26+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package testSuite.classes.state_hint_checked_error;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.Refinement;
5+
import liquidjava.specification.StateRefinement;
6+
import liquidjava.specification.StateSet;
7+
8+
@StateSet({"hasMore", "noMore"})
9+
@ExternalRefinementsFor("java.util.Scanner")
10+
public interface ScannerRefinements {
11+
12+
@Refinement("_ == true --> hasMore(this)")
13+
boolean hasNext();
14+
15+
@StateRefinement(from = "hasMore(this)", to = "noMore(this)")
16+
String next();
17+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package testSuite.classes.state_hint_undo_manager_error;
2+
3+
import javax.swing.undo.UndoManager;
4+
5+
public class Editor {
6+
7+
static UndoManager history() {
8+
return new UndoManager();
9+
}
10+
11+
static void redoLast() {
12+
UndoManager undoManager = history();
13+
undoManager.redo(); // Expect: State Refinement Error
14+
}
15+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package testSuite.classes.state_hint_undo_manager_error;
2+
3+
import liquidjava.specification.ExternalRefinementsFor;
4+
import liquidjava.specification.StateRefinement;
5+
import liquidjava.specification.StateSet;
6+
7+
// Issue #454: with an unknown state, redo() needs redoable, and the only call that leads there, undo(), needs
8+
// undoable, which is just as unknown
9+
@ExternalRefinementsFor("javax.swing.undo.UndoManager")
10+
@StateSet({"undoable", "notUndoable"})
11+
@StateSet({"redoable", "notRedoable"})
12+
public interface UndoManagerRefinements {
13+
14+
@StateRefinement(to = "notUndoable(this) && notRedoable(this)")
15+
void UndoManager();
16+
17+
@StateRefinement(from = "redoable(this)", to = "undoable(this) && notRedoable(this)")
18+
void redo();
19+
20+
@StateRefinement(from = "undoable(this)", to = "notUndoable(this) && redoable(this)")
21+
void undo();
22+
23+
@StateRefinement(from = "redoable(this)", to = "return && redoable(this)")
24+
@StateRefinement(to = "return ? redoable(this) : notRedoable(this)")
25+
boolean canRedo();
26+
}

‎liquidjava-verifier/src/main/java/liquidjava/diagnostics/hints/Hint.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ public enum Kind {
2525
PATH_DEPENDENT_STATE,
2626
/** The object is in a state where no call with a state refinement is allowed */
2727
FINAL_STATE,
28-
/** The object's state is unknown, so these are the calls that lead to a state that allows the failing call */
29-
REACHING_CALLS
28+
/** The object can be in any state: the state the failing call needs, and the constructors that create it */
29+
NEEDED_STATE
3030
}
3131

3232
public static Hint general(String message) {

0 commit comments

Comments
 (0)