PHPStan 2.0.0 was released on Nov 11, 2024. Bump to it.
------ ----------------------------------------------------------------------------------
Line core/tests/Drupal/Tests/Core/Render/RendererPlaceholdersTest.php
------ ----------------------------------------------------------------------------------
1154 Method
Drupal\Tests\Core\Render\RendererPlaceholdersTest::setupThemeManagerForDetails()
return type with generic class
PHPUnit\Framework\MockObject\Builder\InvocationMocker does not
specify its types: TMockedClass
🪪 missingType.generics
------ ----------------------------------------------------------------------------------
There is one MR:
phpstan2_with1 Updates to PHPStan 2 and allows use of PHPStan 1.
PHPStan has been upgraded to 2.0.4. Backward compatibility is provided so you can still install PHPStan 1.x until you are ready to upgrade.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !10541changes, plain diff MR !10535
Comments
Comment #2
spokjeThis needs a
mglaman/phpstan-drupalthat works withphpstan/phpstan:^2.0AFAICTComment #3
gorkagr commentedIn the core-dev (https://github.com/drupal/core-dev/blob/11.0.6/composer.json) is required both phpstan/phpstan and mglaman/phpstan-drupal, so indeed it is needed an update of the latest too
Comment #4
mondrakePostponed on that.
Comment #5
spokjeHere's the upstream issue: https://github.com/mglaman/phpstan-drupal/issues/806
Comment #6
mstrelan commentedI believe this means we could remove bleeding edge from phpstan config. To be discussed if that's something we want to do though.
Comment #7
mondrakehttps://github.com/mglaman/phpstan-drupal/releases/tag/2.0.0 is out, supporting PHPStan 2. This is unblocked now.
Comment #10
ptmkenny commentedAfter updating to PHPStan 2, there are two coversMethod errors:
I am confused about the above errors because in PHPStorm, I can jump from the @covers getLayoutOptions and @covers delete to the implementations, so I don't understand why PHPStan is flagging this (these were previously fixed in core in #3351095: Fix PHPStan L1 errors "@covers value foo references an invalid class or function.").
There are also 90 missingType.generics errors:
Should these errors be fixed in this issue, ignored in phpstan.neon.dist, or added to a baseline?
Comment #11
spokjeThanks for the hard work @ptmkenny
The first two errors you mentioned are caused by there being text after the
@coversSo for example
core/tests/Drupal/Tests/Core/State/StateTest.phpSo if we put that last line before the
@covers ::getPHPStan should be much happier.The
missingType.genericsare much harder to fix and we _could_ suppress them, so the behaviour on those would be the same as in PHPStan 1.x, with:https://github.com/phpstan/phpstan/blob/2.0.x/UPGRADING.md#removed-optio...
Comment #12
ptmkenny commented@spokje Ok, thanks! I fixed the @covers and updated one ignore in the baseline that is now processed differently, and I set missingType.generics to ignore.
Comment #13
ptmkenny commentedThe phpunit phpstan test is failing:
That test is:
I don't understand why this test is failing, though; on my local machine, both composer.json and the metapackage PinnedDevDependencies composer.json have phpstan 2.
Comment #14
spokjeNice,
The only failing test now is an annoying one, which always gets me.
In
core/tests/PHPStan/composer.jsonthe version ofphpstan/phpstanshould match the version that is incomposer.json.Comment #15
longwaveIs it possible to allow both PHPStan 1 and 2 in composer.json? While core can move to v2 all at once, contrib (including the CI templates) might not want to do so just yet, so allowing a downgrade might be useful.
Comment #17
ptmkenny commentedComment #18
ptmkenny commented@spokje Ok, it looks like the version in
core/tests/PHPStan/composer.jsonis hard-coded to the latest version, so I updated that.@longwave I created a new branch, phpstan2_with1, which allows for both PHPStan 2 and 1.
All tests are passing now in both branches, so I updated the issue summary and am setting this to "Needs review".
Comment #19
mondrakeComment #22
ptmkenny commented@mondrake: The list of missingType.generics violations is below.
The question is whether these should be added to the baseline or ignored as a group.
The PHPStan docs on missingType.generics list over a dozen rules that could report this error. So it may be worthwhile to find a more specific rule, or to ignore a string like "return type with generic class... does not specify its types".
@spokje mentioned that it might be better to ignore them as a group in the phpstan.neon.dist.
I am happy to do it either way; what does everyone think?
Comment #23
ptmkenny commentedComment #24
spokjeI agree, basically by default, with @mondrake.
Adding the current errors to the baseline would, as already stated by @mondrake, flag any new ones that pop up.
If we added them to be ignored by default with the
parameters: ignoreErrors:"magic" new ones would also be ignored without warning us and thus not give us the chance to fix them, or at the very least give it an attempt.So I'm on #TeamMondrake for adding them to the baseline.
Comment #25
mondrake😀
Comment #26
ptmkenny commentedOk, I went to add them to the baseline, but I couldn't figure out how to do that.
First, I tried using the baseline generated by GitLab CI, but the paths are different.
Then, based on this change record, I ran
vendor/bin/phpstan analyze --configuration=./core/phpstan.neon.dist --memory-limit=-1 --generate-baseline=core/.phpstan-baseline.phpwhich is the same code listed in the docs.
However, when I did this, the generated baseline has removed the comment for the identifier from all entries:
I don't understand why it is doing this and how I can generate a new baseline in the same format as has already been committed.
Comment #27
nicxvan commentedThe comment has been removed but now it's part of the array.
That's actually better in my opinion cause now it's data.
Comment #28
ptmkenny commentedComment #29
ptmkenny commentedComment #30
ptmkenny commentedOk, tests are all passing and I believe I have addressed all feedback thus far, so I am setting back to "Needs review."
Comment #31
mondrakeUpdated to PHPStan 2.0.4, removed ignore of
missingType.genericsand regenerated baselineComment #32
mondrakeComment #33
spokjeThanks @ptmkenny and @mondrake: All changes make sense => RTBC,
Comment #35
longwaveI was a bit concerned that this is going to make backports tricky because of the format change in the baseline, but I looked at git history and we don't tend to change the baseline that often in backported issues anyway - mostly only in 11.x, so going forwards any patch that currently touches the baseline will need reroll now but then we should be good for the future.
Committed 2209889 and pushed to 11.x. Thanks!
Comment #37
longwave