Add "Change Array by Copy" tests (stage 3)

[nicolo-ribaudo]

This PR adds tests for https://github.com/tc39/proposal-change-array-by-copy, which reached consensus for Stage 3 at the March 2022 meeting.

I'm marking it as draft because I wrote these tests a few months ago, so I need to check that they are up-to-date with the current specification.

cc @acutmore @catamorphism

[ljharb]

Is it intended that [].with always throws, no matter what arguments are passed?

[rwaldron]

@nicolo-ribaudo I'm not going to ask you to move the files out of the metadata/ dirs, but this is a heads up that I will move them later. 👍

[acutmore]

Is it intended that [].with always throws, no matter what arguments are passed?

Yep that is intentional. There are no valid indexes to change for an empty array. And passing no arguments or undefined is interpreted as passing index 0 the same as Array.prototype.at.

[acutmore]

Does it make sense to add tests to ensure that toSorted and toReversed are not susceptible to Array.prototype.{sort,reverse} being patched? (e.g. An implementation similar to: Array.prototype.toSorted = function(f) { return [...this].sort(f) }.

I don't know if this is the type of thing that test262 would test or not?

Thoughts @nicolo-ribaudo, @rwaldron, @ljharb ?

[ljharb]

I think it falls into the kind of category where it's one of the infinite things the spec doesn't permit and thus implicitly forbids - but unless there's an implementation that has that bug, we wouldn't necessarily need a test for it.

[acutmore]

thanks @ljharb! Makes sense 😀

[rwaldron]

@acutmore & @ljharb as long as every step in each method's algorithm has coverage, that will be assured 👍

[ljharb]

@rwaldron i mean, not necessarily - an implementation could potentially replace some of the steps with an observable call to another builtin method. in this case, some of my polyfills did that, and i had to revert that optimization because it broke some tests, so we're probably fine :-)

[nicolo-ribaudo]

Updated, I'm sorry that it took a while!

Fyi, we are running these tests at tc39/proposal-change-array-by-copy#83 to make sure that they respect the behavior of the "reference implementation" (which matches 1:1 the spec).

@rwaldron If you have any suggestion about how I should rename those files let me know (I think I copied the metadata/ folder from somewhere else?), but if it's faster to just do it by yourself please go ahead 🙂

[ljharb]

@nicolo-ribaudo i think this needs to be updated and rebased

[catamorphism]

@rwaldron Could you let me know if I've addressed your comments adequately? Thanks!

[acutmore]

EDIT: DONE

TODO: we should add a new test to cover yesterdays spec change tc39/proposal-change-array-by-copy#85

maybe something like:

let userLandCodeCalled = false;
const arr = new Uint8Array(2);
const nonPureValue = {
  valueOf() {
    userLandCodeCalled = true;
    arr[0] = 1;
    return 2;
  }
};
const indexToChange = 1;
const changed = arr.with(indexToChange, nonPureValue);
assert(userLandCodeCalled);
assertEqual(changed[0], 1); // the copy should include the mutation (previously would have returned 0)
assertEqual(changed[1], 2);

[Ms2ger]

Partial review.

[catamorphism]

@Ms2ger I think I've addressed all your comments. Github still says "1 change requested" by you, but I can't find it anywhere even after expanding all the resolved conversations :/

[Ms2ger]

Let's move any further work to new PRs.