Problem/Motivation
mglaman/phpstan-drupal has a new release
Proposed resolution
composer update mglaman/phpstan-drupal phpstan/extension-installer phpstan/phpstan phpstan/phpstan-deprecation-rules phpstan/phpstan-phpunit spaze/phpstan-disallowed-calls- update manually the constraints to latest versions
composer update --lock
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|
Issue fork drupal-3585555
Show commands
Start 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:
- 3585555-11.x
changes, plain diff MR !15713
- 3585555-bump-phpstan-
changes, plain diff MR !15498
Comments
Comment #3
mondrakeComment #4
smustgrave commentedSeems straight forward enough
Comment #5
xjmEverything in this looks patch-safe except for the assertions about link items, which I assume were added to resolve a test failure or something?
Theoretically it's prod-safe, but it could disrupt CIs and etc. if someone has non-link-item things there (if that was possible before).
Clarification about that change with Re-TBC would be super. :)
Comment #6
mondrakeHonestly I did not mean this for a patch release, just for main for now. Then all dependencies will be bumped on a minor anyway, no?
I started this because the mglaman/phpstan-drupal release was including https://github.com/mglaman/phpstan-drupal/pull/926 and I was testing #[IgnoreDeprecations] with an argument - ending up in a branch that would fit with a MR.
Yes, the asserts are there to validate a type and avoid PHPStan reporting unknown var type.
In any case, both PHPStan and PHPStan-Drupal have released again since this issue was started, so it's probably worth rebase to latest.
Comment #7
mondrakeThat's unlikely though because just below the ::buildUrl() method is called and that method has a native type describing the $item parameter,
https://git.drupalcode.org/project/drupal/-/blob/2f5f71dde7192dc50778cce..., so if anything else than a link item is passed we would have a type error thrown.
Comment #8
mondrakeLet's bump again, and see if instead of assert we can use generics here to indicate the type of the values in the $items array.
Comment #9
mondrakeYes #8 worked.
Comment #10
mondrakeComment #11
longwave2.1.52 is out which contains a larger than normal number of fixes: https://github.com/phpstan/phpstan/releases/tag/2.1.52
Comment #12
mondrake#11 2.1.53 actually.
Bumped:
- Upgrading mglaman/phpstan-drupal (2.0.14 => 2.0.15)
- Upgrading phpstan/phpstan (2.1.51 => 2.1.53)
- Upgrading spaze/phpstan-disallowed-calls (v4.11.0 => v4.12.0)
I am afraid if we do not draw a line for the issue here, we are getting in a chase game as all these tools are releasing very frequently.
Comment #13
mondrakeComment #14
longwaveI think we should fix
Call to deprecated method allowsNull\\(\\) of class ReflectionParameteror this is going to spread to everything that uses AutowireTrait.The fix is likely just
Comment #15
mondrakedone #14
Comment #16
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #17
mondrakeBumped:
- Upgrading phpstan/phpstan (2.1.53 => 2.1.54)
Comment #18
smustgrave commentedFeedback for this one appears to be addressed
Comment #21
catchNormally I'd commit this to main and mark it fixed, on the basis that 11.x/11.4 will eventually have an 'update dev dependencies' issue and it'll be caught there. But I think we will want to backport the AutowiredInstanceTrait and link changes to 11.x from here so we don't have to find and fix them again later.
Comment #23
mondrakebackport
Comment #24
mondrakeComment #25
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #26
smustgrave commentedSeems like a good backport
Comment #27
quietone commentedComment #29
xjmI gave the 11.x backport a quick review, comparing it to the changes that have already been committed to
main, and it all matches up aside from the newspaze/phpstan-disallowed-callsdependency in main.Committed to 11.x. Thanks!