Fix values, value? and key returning a superclass' overridden value - #63
Merged
Conversation
Danger ReportNo issues found. |
) values, value? and key each had their own logic for merging a subclass' enums with its superclass', instead of using the memoized, deduping _enum_hash used by to_h, keys, key?, value, parse and each. values did a recursive list concatenation (superclass.values + result), so a subclass redefining a parent's key showed both the parent's stale value and its own override. value? and key relied on _enums_by_value, a hash merged by value rather than by key, so a superclass' stale value=>enum mapping for an overridden key was never overwritten (it's a different hash key), leaving value?/key returning true/the key for values that no longer belong to that key. Fixed by: * values now reads from _enum_hash like every other method. * _enums_by_value is now derived from _enum_hash (keyed by key) instead of merged directly by value, so overridden keys can't leave stale by-value entries behind. Co-authored-by: Copilot <[email protected]>
dblock
force-pushed
the
fix-values-override-issue-62
branch
from
August 15, 2026 17:10
2cbc30f to
c3268a5
Compare
Owner
Author
|
@flvrone Take a look, this was YOLOed with Copilot but LGTM. |
Contributor
|
@dblock Looks good! 👍 |
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.
Fixes #62.
Bug
Several inheritance-aware methods had their own bespoke merge logic instead of using the memoized, deduping
_enum_hashused byto_h,keys,key?,value,parseandeach:valuesdid a recursive list concatenation (superclass.values + result), so a subclass redefining a parent's key showed both the parent's stale value and its own override.value?andkeyrelied on_enums_by_value, a hash merged by value rather than by key, so a superclass' stalevalue => enummapping for an overridden key was never overwritten (it's a different hash key), leavingvalue?/keyreturningtrue/the key for values that no longer belong to that key.Fix
valuesnow reads from_enum_hashlike every other method._enums_by_valueis now derived from_enum_hash(keyed by key) instead of merged directly by value, so overridden keys can't leave stale by-value entries behind.Updated/added specs covering both fixes.
Testing
bundle exec rake(rubocop + rspec): 70 examples, 0 failures, rubocop clean (25 files)spec_i18nsuite: 3 examples, 0 failures