Problem/Motivation

Our dev dependency on phpstan is not up to date.

phpstan/phpstan 1.4.0 1.6.7 PHPStan - PHP Static Analysis Tool

Updating those provides us with the latest improvements.
For the phpstan updates this might give us performance improvements. (which might help with #3259355: Always do a full phpstan analysis on DrupalCI)
https://phpstan.org/blog/phpstan-1-6-0-with-conditional-return-types

Proposed resolution

Remaining tasks

Updating mglaman/phpstan-drupal was split of to #3279840: Update mglaman/phpstan-drupal

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3278916

Command icon 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:

About issue forks

Comments

mallezie created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Related issues: +#3278162: Update Composer dependencies to the latest minor and patch versions

#3278162: Update Composer dependencies to the latest minor and patch versions covers this and more.

mallezie opened merge request !2227

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Status: Active » Needs review

This also updates the baseline. Most are false positives being removed.

Most added new entry added is:

message: "#^Missing explicit access check on entity query\\.$#"
			count: 1
			path: modules/media_library/src/Plugin/CKEditorPlugin/DrupalMediaLibrary.php

Not sure if we could just add those, or fix them before updating the dependencies.

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented

This is strange, I don't see that one when I ran phpstan locally.

Also when checking the codebase, the 2 mentioned missing accescheck calls are there (although false)

mondrake’s picture

mondrake
Location 🇮🇹
commented

@mallezie @mglaman it could make sense to split the update in two, one for phpstan itself and one for phpstan-drupal...

Missing explicit access check on entity query sounds definitely like a phpstan-drupal error.

Maybe the baseline changes of phpstan itself are more manageable.

xjm’s picture

xjm
she/her
Primary language English
commented
Priority: Normal » Critical
Issue tags: +Drupal 10
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs reroll following #3278162: Update Composer dependencies to the latest minor and patch versions

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Title: Update phpstan and related dependencies to latest version » Update phpstan/phpstan to latest version
Issue summary: View changes

Rerolling, and only updating phpstan/phpstan in this issue.

phpstan-drupal can be updated in #3279840: Update mglaman/phpstan-drupal

This still includes changes from #3278782: PHPStan baseline is out of sync

mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented
Status: Needs work » Needs review
mallezie’s picture

mallezie
Primary language Dutch
Location Loenhout
commented

Going through the baseline diff. Only thing i'm seeing is removals of items from the baseline. This means phpstan is fixing false positives.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Look good to me, thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Credited @mondrake for issue scoping.

Committed 810334c and pushed to 10.0.x. Thanks!

  • alexpott committed 810334c on 10.0.x 
    Issue #3278916 by mallezie, mondrake: Update phpstan/phpstan  to latest...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +10.0.0 release notes