Skip to content

Read named state sets and states written as values of their sets - #17

Merged
CatarinaGamboa merged 2 commits into
mainfrom
feat/state-values
Nov 9, 2026
Merged

CatarinaGamboa merged 2 commits into
mainfrom
feat/state-values

Conversation

@CatarinaGamboa

@CatarinaGamboa CatarinaGamboa commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Follows the verifier's named state sets (liquid-java/liquidjava#498) and states written as values of their sets (liquid-java/liquidjava#510, design in liquid-java/liquidjava#502).

Problem

With the verifier on main, a spec can write

@StateSet(name = "phase", value = {"open", "closed", "broken"})
...
@StateRefinement(from = "open == this.phase()", to = "closed(this)")

The parser took open == phase(this) for a condition that holds in any state, so close was drawn from open, closed and broken, each with the condition open == phase(this). phase(this) != open went the same way, and a value of another set in a to was kept instead of being left out.

Change

  • getStateName also reads set(x) == state and state == set(x) (a one-argument function compared with a state) as the state, and getNegated reads a != b as the negation of a == b, like !. This is the verifier's reading (StateValues.toShorthand), and it applies wherever the shorthand is read: from, to, constructors and factories, this.phase(), phase(), old(this), and generated names such as state2(this) == unlocked.
  • Conditions keep the form the spec wrote, e.g. phase(this) != broken.
  • StateMachine gets stateSetName: the name of its @StateSet, or null when it has none. Generated names (state1, ...) are not exposed, since the verifier does not show them in messages either. The old six-argument constructor is kept, as vscode-liquidjava uses it.
  • README: the new field and the value form.

Unchanged: the parser stays as lax as for the shorthand (it does not check that the function is a state set of the class, nor that a parameter is named like a state); a spec where that matters is rejected by the verifier.

Tests

One fixture, StateValues.java (a named and an unnamed set, == in both orders, !=, this.phase(), phase(), a generated name, mixed with the shorthand), and one test that fails without the change. mvn test passes locally (33 tests).

Screenshots of the diagrams before and after, in VS Code: liquid-java/vscode-liquidjava#178.

A release is needed before vscode-liquidjava can use stateSetName (PR to follow there).

🤖 Generated with Claude Code

The verifier now accepts @stateset(name = "phase", value = {...}) and a
state written as a value of its set, phase(this) == open, which it reads
as open(this), and phase(this) != open as !open(this). The parser took
such a from for a condition on every state, so the diagram lost its
transitions.

A comparison of a one-argument function with a state, in either order,
is now that state, and != negates it, wherever the shorthand is read:
from, to, constructors and factories. Conditions keep the form the spec
wrote. Each state machine carries stateSetName, the name given to its
state set, or null when it has none, so a client can label the diagram
with the name the verifier's messages use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CatarinaGamboa

Copy link
Copy Markdown
Contributor Author

Codex review, round 1

codex exec review --base origin/main:

  • [P2] Recognize zero-argument state-set comparisons (StateMachineParser.java:796-799): when a refinement uses phase() == open, getStateValue rejects the zero-argument call, even though the new test fixture uses phase() != broken. An equality in to then produces no destination state, while one in from becomes a guard on every state rather than selecting open.
  • [P2] Check which state set the comparison calls (StateMachineParser.java:796-799): with several state sets, any one-argument function compared with a state name is taken as a value of the current set. phase(this) == unlocked is parsed as an unlocked transition for the unlocked/locked set, although phase names the open/closed/broken set.

Response

Both declined:

  1. Zero arguments: the refinements parser already reads phase() as phase(this), and this.phase() too (checked on verifier 0.0.36: phase() == open gives a FunctionInvocation phase with the argument this). The fixture's phase() != broken is covered by the test and gives open and closed, not a guard.
  2. Wrong set: phase(this) == unlocked is rejected by the verifier (State 'unlocked' is not a state of 'phase' (its states are ...)), so the diagram never has to draw it. The parser is equally lax for the shorthand: it does not check that unlocked(other) is about this either. Checking the set name would mean passing the state set names through every helper for a spec the verifier rejects.

No changes in this round.

open(this) != false is not phase(this) != open: it means open(this), so
its complement drew the transition from the other states. Only a
comparison that is itself a state value is read as its negation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CatarinaGamboa

CatarinaGamboa commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor Author

Codex review, round 2

codex exec on git diff origin/main...HEAD, told the reasons for the two findings declined in round 1:

StateMachineParser.java:810: getNegated treats every != involving a state as the complement of that state. The verifier accepts from = "open(this) != false", but the parser draws close from closed instead of open.

Response

Fixed in b5a1061: getNegatedStates now reads a != b as a negation only when a == b is itself a state written as a value of its set. open(this) != false is parsed as on main again (a condition), so no new test: it behaves as before this PR.

@CatarinaGamboa

Copy link
Copy Markdown
Contributor Author

Codex review, round 3

codex exec on git diff origin/main...HEAD, with the round 1 and 2 outcomes: No findings. The loop ends here.

@CatarinaGamboa

Copy link
Copy Markdown
Contributor Author

Clean-code review

Audit of this PR's diff against the clean-code-principles checklist (file:line - [principle] issue):

  • StateMachineParser.java:615-623 and :756-761 - [core-dry] the complement of the negated states is built twice, once as TransitionSources and once as names. Declined: the duplication was already on main (the two ! branches); this PR only changes where the negated states come from, and the PM prefers small PRs over refactors.
  • StateMachineParser.java:753 - [core-kiss] getStateExpressions computes getNegatedStates even when the expression is already a state. Declined: one extra walk of a small refinement; computing it inside the branch would need the else-chain split up.
  • StateMachineParser.java:807 - [core-dry] getNegatedStates builds a == b to reuse getStateName, rather than repeating the two operand orders. Kept on purpose.
  • StateMachine.java:20 - [core-yagni] the old six-argument constructor is kept. Kept: it is the public constructor vscode-liquidjava's tests and the shorter constructors use; removing it would break callers for no gain.
  • Naming (StateSet, getStateSetName, getStateValue, getNegatedStates) and function size follow the file's existing style. No findings on SOLID.

No changes.

@CatarinaGamboa
CatarinaGamboa merged commit 710ee40 into main Nov 9, 2026
2 checks passed
CatarinaGamboa added a commit to liquid-java/vscode-liquidjava that referenced this pull request Nov 10, 2026
Brings
[liquidjava-fsm#17](liquid-java/liquidjava-fsm#17)
to the extension, for the verifier's named state sets and states written
as values of their sets (liquid-java/liquidjava#498, #510, #511; design
in liquid-java/liquidjava#502).

**Depends on a liquidjava-fsm 0.2.3 release** of liquidjava-fsm#17:
until it is on Maven Central, `server` does not build in CI.

## What changes

- `server/pom.xml`: liquidjava-fsm 0.2.2 → 0.2.3, which reads
`phase(this) == open`, `open == phase(this)` and `phase(this) != open`
in `from`/`to` as the states they name, and reports each diagram's
`stateSetName`.
- `StateMachineDTO`: passes `stateSetName` to the client (the
`liquidjava/fsm` request returns the fsm records, so it gets it as is).
- State Machine view: a named state set's diagram is titled with its
name, also when it is the class's only state set, so it can be matched
with the error message (`doorState(d) == closed`). A state set without a
name keeps its states as the title, as now.

## Checked, no change needed

The verifier sends the same shapes to the IDE as before (`git diff
fbc5e9f6 origin/main`): `StateErrorContext` still names states by their
names, and hints are unchanged. Only the text of a state error's
message, `expected` and `found` changes, to `doorState(d) == closed` for
named sets. The client renders those strings as they come, with the
generic highlighter, and nothing in the client parses state names out of
message text: the diagram marks and the summary line use `errorContext`.
The verification view shows the new form as is (last screenshot).

## Screenshots

Headless VS Code with this branch, liquidjava-fsm#17 installed locally
as 0.2.3 and the verifier built from liquid-java/liquidjava main, on
this spec (`d.lock(); d.open();` as the error):

```java
@stateset(name = "doorState", value = {"closed", "opened"})
@stateset(name = "lockState", value = {"unlocked", "locked"})
public class Door {
    @StateRefinement(to = "doorState(this) == closed && lockState(this) == unlocked")
    public Door() {}
    @StateRefinement(from = "doorState(this) == closed && lockState(this) == unlocked", to = "doorState(this) == opened")
    public void open() {}
    @StateRefinement(from = "doorState(this) == opened", to = "doorState(this) == closed")
    public void close() {}
    @StateRefinement(from = "doorState(this) == closed && lockState(this) != locked", to = "lockState(this) == locked")
    public void lock() {}
    @StateRefinement(from = "lockState(this) == locked", to = "lockState(this) == unlocked")
    public void unlock() {}
}
```

Before (main, fsm 0.2.2), the diagrams are wrong: `open` and `lock` go
from every state with the whole `from` as a condition, `close` and
`unlock` are missing, the summary only names `lockState`, and the titles
are the states. After, the diagrams match the spec, the conditions keep
the form the spec wrote, and the titles are the set names:

| | Before | After |
|---|---|---|
| Show all | <img
src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/178/before-all.png"
width="380"> | <img
src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/178/after-all.png"
width="380"> |
| Focus on error | <img
src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/178/before-focus.png"
width="380"> | <img
src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/178/after-focus.png"
width="380"> |

The verifier's message for the same error (no change here; shown for
context):

<img
src="https://raw.githubusercontent.com/liquid-java/vscode-liquidjava/pr-screenshots/178/verification.png"
width="380">

## Not in this PR

- The verifier bump: main of liquid-java/liquidjava is not released yet.
When it is, `DiagnosticConverterTest` needs the new
`StateRefinementError` constructor argument (`display`, from #511) and
the `RefinementError` one (`aliases`); the server code itself compiles
against it.
- The walkthrough and README examples stay in the shorthand until the
extension ships a verifier that accepts named sets.

## Tests

- client: `vitest run` 124 passed (one new test, fails without the
change), typecheck and lint clean
- server: `mvn test` 35 passed, against liquidjava-fsm#17 installed as
0.2.3 in an isolated local Maven repository

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant