Skip to content

fix(item): normalize offsetSet to drop cross-version phpstan ignore#1

Open
louisbels wants to merge 1 commit intotkaratug:mainfrom
louisbels:fix/phpstan-unmatched-ignore
Open

fix(item): normalize offsetSet to drop cross-version phpstan ignore#1
louisbels wants to merge 1 commit intotkaratug:mainfrom
louisbels:fix/phpstan-unmatched-ignore

Conversation

@louisbels
Copy link
Copy Markdown

Fix the failing CI on your roach-php#292 PR against roach-php/core.

Problem

The inline @phpstan-ignore offsetAccess.invalidOffset added to Item::offsetSet() is only triggered on PHP 8.5. On PHP 8.2/8.3/8.4, PHPStan doesn't report that error, so the ignore becomes unmatched and CI fails with:

ItemPipeline/Item.php
  63     No error with identifier offsetAccess.invalidOffset is reported on line 63.

That explains the 7 red jobs on the upstream PR roach-php#292.

Fix

Narrow the offset handling properly so no ignore is needed at all:

public function offsetSet(mixed $offset, mixed $value): void
{
    if ($offset === null) {
        $this->data[] = $value;
        return;
    }

    $this->data[$offset] = $value;
}

This handles the $array[] = $value case (where $offset is null per ArrayAccess semantics) explicitly, which makes the narrow type of the remaining $offset (int|string) enough for PHPStan.

Verified locally

PHPStan + full test suite on PHP 8.2, 8.4 and 8.5:

PHPStan: [OK] No errors
PHPUnit: Tests: 502, Assertions: 808, Warnings: 1, Deprecations: 5, Skipped: 16

Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant