Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

composer: Update PHPStan to 2.0 #1505

Merged
merged 9 commits into from
Nov 23, 2024
Merged

composer: Update PHPStan to 2.0 #1505

merged 9 commits into from
Nov 23, 2024

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Nov 23, 2024

Copy link

netlify bot commented Nov 23, 2024

Deploy Preview for selfoss canceled.

Name Link
🔨 Latest commit 1b2eeda
🔍 Latest deploy log https://app.netlify.com/sites/selfoss/deploys/6741dc4a7369db00084f5cf2

So that PHPStan 2.0 does not complain about `id` key being always present as declared by the PHPDoc `@param` type.
This also makes the wrong input result in an error rather than silently ignoring the list items without an `id`.
PHP does not allow specifying an explicit default value on readonly properties and PHPStan 2.0 enforces the same for `@readonly` PHPDoc marker.

Let’s move the initial values to else branches. This required turning the method into a constructor.

Also fix `@readonly` for `$extraIds` property – it needs to be on a separate line for PHPStan to recognize it.
`Item::withExtraData()` is an instance of `Functor::map()` so it can change the `Item`’s generic argument type.

But since PHPStan has no concept of transmuting types, it would start to complain after upgrading to version 2.0:

    PHPDoc tag @var with type spouts\Item<NewExtra> is not subtype of native type $this(spouts\Item<Extra>).
             🪪  varTag.nativeType

Let’s create a fresh instance instead.
We want to make sure we are checking all variants exhaustively but
PHPStan 2.0 started to complain about using conjunction:

    258    Strict comparison using === between 'media' and 'media' will always evaluate to true.
           🪪  identical.alwaysTrue

Let’s switch to `switch` statement for now, which supports exhaustiveness checking.
Once we drop support PHP 7.4, we can port it to `match`.
`preg_match` without `PREG_UNMATCHED_AS_NULL` behaves inconsistently – trailing unmatched capture groups are omitted but non-trailing ones resolve to an empty string. This would have been confusing if we ever decided to modify the expression, so we were defensively checking for both options. But PHPStan 2.0 got smarter and knows the empty string cannot currently occur, so it started to complain:

    71     Strict comparison using !== between non-empty-string and '' will always evaluate to true.
           🪪  notIdentical.alwaysTrue

Let’s just add `PREG_UNMATCHED_AS_NULL`, which will result in `null` in both cases since PHP 7.4.

Though we still need to cast away the null since PHPStan is not smart enough to know one of the alternatives must be present.
@jtojnar jtojnar merged commit 1b2eeda into master Nov 23, 2024
15 checks passed
@jtojnar jtojnar deleted the phpstan-2 branch November 23, 2024 13:49
@jtojnar jtojnar added the dependencies Pull requests that update a dependency file label Nov 26, 2024
@jtojnar jtojnar added this to the 2.20 milestone Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant