fix(patterns): Update containerHasSplit(copyArray, ...) to return copyArray results#3065
fix(patterns): Update containerHasSplit(copyArray, ...) to return copyArray results#3065
containerHasSplit(copyArray, ...) to return copyArray results#3065Conversation
…opyArray results (which must be hardened) Fixes #3064
CopyArray is not mutable
| const specimen = harden([1, 'foo', 2, 'bar']); | ||
|
|
||
| test('split first match from copyArray', testContainerHasSplit, { | ||
| specimen, | ||
| pattern: M.string(), | ||
| bound: 1n, | ||
| expectAccepted: ['bar'], | ||
| expectRejected: [2, 'foo', 1], | ||
| }); | ||
|
|
||
| test('split first two matches from copyArray', testContainerHasSplit, { | ||
| specimen, | ||
| pattern: M.string(), | ||
| bound: 2n, | ||
| expectAccepted: ['bar', 'foo'], | ||
| expectRejected: [2, 1], | ||
| }); |
There was a problem hiding this comment.
@erights Whether or not this was intentional, we should reconsider the direction for CopyArray scanning. It makes sense for CopySet and CopyBag to start at the end because of their ordering constraints, but I think CopyArray scanning should start at the beginning.
There was a problem hiding this comment.
That makes sense. But I won't get to it anytime soon, so putting this in Draft until then. Unless you wanna give it a try?
michaelfig
left a comment
michaelfig
left a comment
There was a problem hiding this comment.
Good stuff! Thanks for addressing this (presuming you're heading for CopyArray = ReadonlyArray<Passable>).
| test('split first match from copyArray', testContainerHasSplit, { | ||
| specimen, | ||
| pattern: M.string(), | ||
| bound: 1n, | ||
| expectAccepted: ['bar'], | ||
| expectRejected: [2, 'foo', 1], | ||
| }); |
There was a problem hiding this comment.
Is bound: 0n a relevant edge case to test?
There was a problem hiding this comment.
No, the function is documented to require a minimum bound of 1n:
But I have added 1d6b76e for hopefully propagating such documentation to https://docs.endojs.org/variables/_endo_patterns.containerHasSplit.html .
3 participants
Fixes #3064
Description
containerHasSplitcontainerHasSplitso they actually have pass style "copyArray"containerHasSplitSecurity Considerations
None known.
Scaling Considerations
n/a
Documentation Considerations
n/a
Testing Considerations
✔
Compatibility Considerations
CopyArrays are already claimed by documentation to be immutable, so this shouldn't affect anything (and doesn't AFAICT).
Upgrade Considerations
I think this fix falls below the level of inclusion in NEWS.md, but could be persuaded out of that position.