Remove ArrayAccess from NodeList and enforce contiguous int keys

[spawnia]

Reintroduces #1315 and #1321.

I found that AST manipulation is common enough that we should not break it in a minor release.

[Copilot]

Pull request overview

This PR updates the NodeList AST container to behave as a true list (contiguous int keys) and removes array-style access, then adjusts core visitor logic and test suite usages accordingly.

Changes:

• Replace ArrayAccess semantics on NodeList with explicit list APIs (get/has/set/add/unset) and update internal iterator behavior.
• Update visitor/editing logic and multiple tests to use NodeList list APIs instead of []/offsetGet.
• Document the behavioral change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/Utils/BuildSchemaTest.php	Switches definitions[0] to definitions->get(0) for the new NodeList API.
tests/Utils/AstGetOperationAstTest.php	Replaces offsetGet() access with get() in operation AST assertions.
tests/Language/VisitorTest.php	Updates NodeList parent/key assertions and element access to has()/get().
tests/Language/SerializationTest.php	Iterates NodeLists using get() and normalizes path segments to strings.
tests/Language/ParserTest.php	Updates definitions[0] access to definitions->get(0).
tests/Language/NodeListTest.php	Refactors NodeList tests to reflect list-only behavior and new mutator APIs.
tests/Language/AST/NodeTest.php	Replaces array indexing with get() for nested NodeLists.
tests/Executor/ExecutorTest.php	Uses get() for definitions/selections NodeList indexing.
tests/Error/PrintErrorTest.php	Switches AST element access from [] to get().
tests/Error/ErrorTest.php	Switches AST element access from [] to get().
src/Validator/Rules/QueryComplexity.php	Collects variable definitions via NodeList->add() instead of []=.
src/Language/Visitor.php	Uses NodeList->set() and NodeList->get() in edit/traversal paths.
src/Language/AST/NodeList.php	Core change: remove ArrayAccess, add list APIs, and adjust iterator/splice/merge/clone behaviors.
CHANGELOG.md	Notes NodeList behavior change under “Unreleased”.

Comments suppressed due to low confidence (2)

src/Language/AST/NodeList.php:98

• NodeList::splice() always forwards the default $replacement = null to array_splice(). In PHP, passing null as the 4th argument inserts a null element instead of performing a pure removal. Call sites like SingleFieldSubscription ($selections->splice(1, count($selections))) and Visitor ($node->splice($editKey, 1)) expect removal with no replacement, so this will leave null entries in the list and break downstream iteration/type expectations.

    public function splice(int $offset, int $length, $replacement = null): NodeList
    {
        $spliced = \array_splice($this->nodes, $offset, $length, $replacement);

        // @phpstan-ignore-next-line generic type mismatch

src/Language/AST/NodeList.php:56

• NodeList::set() relies on assert() to enforce contiguous keys. Assertions are commonly disabled in production, so this does not reliably enforce the invariant advertised by the PR title. Also, without a real check, callers can create gaps (e.g. set(5, ...)) which will later cause surprises elsewhere. Consider validating the offset and throwing InvariantViolation when it would create a non-contiguous list.

    public function set(int $offset, Node $value): void
    {
        $this->nodes[$offset] = $value;

        assert($this->nodes === \array_values($this->nodes));

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.