test(konsist): enforce that domain models are immutable - #136
Merged
Conversation
The last invariant in AGENTS.md §3 with no rule behind it. All six models already comply, so this is a ratchet rather than a fix - it stops the seventh from being written differently. Two rules, because there are two ways to break it. A var property is the obvious one. The other is a val holding a mutable collection: 'val items: MutableList' reads as immutable and is not, since the reference is fixed but the contents are not, and whoever received the model can change what the sender is still holding. That is the half that slips through review. Array is banned for a second reason - identity equality gives a data class an equals that reports two identical models as different, which breaks the same Compose recomposition comparison from the opposite direction. Both halves were checked against a real violation rather than assumed: a var and a MutableList were each introduced into a real model in the primary constructor, and each failed the build. That specifically confirms Konsist's properties() sees primary-constructor properties - had it not, the rule would have passed vacuously on every data class here, since that is where they all declare state. AGENTS.md and the README rule table go from eight to nine in the same commit.
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.
The last invariant in AGENTS.md §3 that had no rule behind it. All six domain models already comply, so this is a ratchet, not a fix — it stops the seventh from being written differently.
Two rules, because there are two ways to break it
A
varproperty is the obvious one. The other is avalholding a mutable collection:The reference is fixed, the contents are not. Whoever receives the model can call
items.add(...)and change what the sender is still holding. That's the half that slips through review, which is why it's worth enforcing rather than trusting.Arrayis banned for a second reason: identity equality gives a data class anequalsthat reports two identical models as different — breaking the same Compose recomposition comparison from the opposite direction.Why this matters beyond tidiness
PagedListReducerfolds pages into a list and hands it to Compose, which decides what to recompose by comparing old state to new. A model mutated in place is the same reference, so the comparison says "unchanged" and the UI silently keeps rendering stale data. The mappers and the pager pass the same instances around on the same assumption.Verified against real violations, not assumed
Both halves were checked by introducing a violation into a real model in the primary constructor and confirming the build fails:
data class BeerStyle(val id: String, var name: String)→ failsdata class BeerPage(val items: MutableList<Beer>, ...)→ failsThat specifically confirms Konsist's
properties()sees primary-constructor properties. Had it not, the rule would have passed vacuously on every data class here — that's where all of them declare their state, so the check would have been decorative.Docs kept in step
AGENTS.md and the README rule table go from eight to nine in the same commit, and AGENTS.md §3 now states that every invariant it lists is mechanically enforced, with a note to add the rule alongside any new one.
make konsist·spotlessCheck·make lint— green.