Repository navigation
Named state sets: optional name on @StateSet - #498
Open
CatarinaGamboa wants to merge 6 commits into
Open
CatarinaGamboa wants to merge 6 commits into
CatarinaGamboa wants to merge 6 commits into
Conversation
A named state set's ghost function takes that name instead of the generated stateN, so messages, hints and the equalities that keep unmentioned sets print e.g. redo(um), and specs can use it like any ghost (redo(old(this))). Unnamed sets keep stateN. A name must be a lowercase identifier that is not already a state, ghost or other state set of the class. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Collaborator
Author
|
Codex review, round 1 (run locally before the first push)
|
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Collaborator
Author
|
Codex review, round 2
|
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Collaborator
Author
|
Codex review, round 3
|
Collaborator
Author
|
Codex review, round 4: no actionable findings (the full Maven suite passes). Review loop done. |
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Collaborator
Author
Clean-code review (clean-code-principles)Reviewed the diff only, weighed against the standing rules (smallest PR that tests well, match surrounding style, no tidy-only refactors, lean tests).
Full suite locally after the fix: 714 tests, 0 failures. |
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #435 (first part: named sets only; carrying a set per branch stays open).
What
@StateSettakes an optionalname:A named set's ghost function is called by that name instead of the generated
stateN. Messages, hints, simplification steps and the equalities that keep unmentioned sets (AuxStateHandler.getMissingStates) showredo(um)instead ofstate3(um). Specs can use the name anywhere a ghost works, for exampleto = "return ? undoable(this) : undo(this) == undo(old(this))". It works in classes, interfaces and@ExternalRefinementsForspecs. Unnamed sets keepstateNwith the same numbering, so existing specs behave the same.Checks, all reported as
Error(CustomError, the same kind as "State names must start with lowercase"), on the name:old/true/false@Ghost/@RefinementPredicateghost or another set of the class (generatedstateNincluded). An alias can't clash, because alias names start with an uppercase letter.Before / after
Study variant A1_56 (redo guarded, undo not), with the names
phase/undo/redoon the spec:Before:
After:
ErrorStateHintMultipleSetswith its sets nameddoorStateandlockState:New errors:
Tests (each fails on main)
state_hint_study_undo_manager_correct: an external spec with named sets.canUndo()keeps its undo set by name (undo(this) == undo(old(this))) when it returns false. On main the name is unknown, so this fails.ErrorStateSetNames: invalid name and clashes with a state, a ghost and another set.TestStateHints#foundKeepsANamedStateSetByItsName: exact message withdoorState(d).Full suite passes locally (714 tests). The 6 study exercises and the 19 study variants give byte-identical output to main without names. With names on every multi-set exercise spec, the verdict and line stay the same.
API release needed
name()is new inliquidjava-api. The in-repo api is still0.0.7, which is already published withoutname. Reactor builds from the root (CI and the shaded jar) use the in-repo module. A standalone build of the verifier, or any user project (VS Code study projects included), resolves the published0.0.7, where@StateSet(name = ...)does not compile. Before anyone uses names:./release.sh api(→0.0.8, which also bumps the verifier/example dependency), then a verifier release. Nothing is published in this PR.Notes
oldworks on the receiver, as for every ghost:undo(old(this)).old(undo(this))(the form in the issue) is not supported for any ghost today.tothat only mentions the set function (e.g.undo(this) == ...without any of its states) does not count as mentioning the set, so the keep equality is still added (same as forstateNtoday).server/.../GhostDTOmarks states with the regex^state\d+\(_\) == \d+$. States of a named set would show as "ghost" in autocomplete and the context view. UseghostState.getParent() != nullinstead. Set names are not offered in autocomplete, becauseContextHistoryonly carriesGhostStates. The state machine view (liquidjava-fsm) readsvalueof the first@StateSet, so it is unaffected (fsm#8 could use names as titles).🤖 Generated with Claude Code