Bump PHPStan rule level to 3

The title is at odds with #3322735: [meta] Fix as many PHPStan level 2 issues as possible before bumping the rule level

I generated a baseline for each level from 1-6 so see where is the biggest jump.

level 1: 630 errors
level 2: 8415 errors
level 3: 10657 errors
level 4: 14012 errors
level 5: 16015 errors
level 6: 64641 errors

I think if we're happy with a baseline of ~8000 errors at level 2 then we should aim a little higher, perhaps level 3, or, if there is appetite, then 4 or 5.

Was hoping for some low hanging fruit we can knock off to quickly reduce the number but it's not looking that promising:

$ grep message core/.phpstan-baseline.php|sort|uniq -c|sort -rn|head
    203 	'message' => '#^Access to an undefined property Drupal\\\\Core\\\\Session\\\\AccountInterface\\:\\:\\$passRaw\\.$#',
    201 	'message' => '#^Access to an undefined property Drupal\\\\Core\\\\Session\\\\AccountInterface\\:\\:\\$name\\.$#',
     85 	'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\WebAssert\\:\\:waitForElementVisible\\(\\)\\.$#',
     80 	'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\WebAssert\\:\\:waitForElement\\(\\)\\.$#',
     79 	'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\WebAssert\\:\\:assertWaitOnAjaxRequest\\(\\)\\.$#',
     68 	'message' => '#^Call to an undefined method Drupal\\\\Core\\\\TypedData\\\\TraversableTypedDataInterface\\:\\:get\\(\\)\\.$#',
     48 	'message' => '#^Call to an undefined method Drupal\\\\Core\\\\Entity\\\\EntityInterface\\:\\:get\\(\\)\\.$#',
     41 	'message' => '#^Call to an undefined method Drupal\\\\workflows\\\\WorkflowTypeInterface\\:\\:addEntityTypeAndBundle\\(\\)\\.$#',
     32 	'message' => '#^Call to an undefined method Drupal\\\\Core\\\\Entity\\\\EntityInterface\\:\\:getTranslation\\(\\)\\.$#',
     30 	'message' => '#^Cannot access property \\$uri on object\\|false\\.$#',

Even solving all those will only take 867 off the 8,000+ errors. The WebAssert ones look like they might just need a return type declaring correctly but e.g. the raw password one is test specific and might need us to rethink how we handle passwords during functional tests.

MR appears to be unmergable, surprised the bot never picked it up.

I'm not super into the phpstan stuff like others here in this ticket but in regards to bumping levels. When a new rule or anything is added usually involves having to go back and update several MRs. Can be argued that's just the name of the game with Drupal but can understand community frustration when they have an MR all ready and it keeps getting sent back because of changing rules. So think doing small increments is good.

Just pointing out that phpstan-prophecy 1.0.1 has been released and the MR has been updated to that version, so the concerns in #8 about the dev release are no longer applicable. Let's get this in.

Looks like when we bump to PHPStan L2 we get an initial addition of ~7800 suppressions.
Seeing the amount of active issues to tackle the L1 suppressions, I think it's safe to assume these won't go away quickly.

So the benefit would be preventing new L2 issues creeping in new/refactored code.

It's probably my lack of Google skills, but what are the extra checks in L2 compared to L1, what will be prevented from creeping in by us doing PHPStan L++?

Level 2 gives you:

unknown methods checked on all expressions (not just $this), validating PHPDocs

See https://phpstan.org/user-guide/rule-levels

Or more specifically see https://github.com/phpstan/phpstan-src/blob/1.11.x/conf/config.level2.neon

Thanks @mstrelan, should have found that myself, but somehow didn't. (Makes mental note to check caffeine-percentage of new bought coffee-brand).

Seeing those checks and also the fact that if we want to do it with the least amount of disruptions to other MRs, it's probably now or when D12 arrives, I'm pulling the trigger and put the ball in core commiters court: RTBC for me.

I'm +1 here for the obvious reason that it will stop us introducing new errors, so even if it takes years to reduce the current baseline, at least it will be visibly getting smaller instead of silently getting larger.

On going to L3 or L5 etc., staggering the MR breakage for new rules seems like a good reason to do one at a time. We could still try to go up a level every 6 months or 3 months if we want to (I'm not saying we should definitely do that, but it's worth thinking about). It does look like level 5 would be a couple of small jumps.

I don't want to change the scope, will almost instantly open a follow up if this goes in, but level 3 gives us:

return types, types assigned to properties

On all new code, which most code reviews are picking up anyway, but would be much nicer if it was for free.

One argument against staggering would be that raising the PHPStan level would be disruptive to (almost) all open MRs for each bump, so bumping more then one level would make it less disruptive.

That might be the case when the baseline was in the NEON format, and IIRC was one (of many) reasons the Bump-to-L9 issue is stalling.
But as part of that issue, we switched the baseline over to PHP with the reasoning that Git would be much smarter about merging it.

Is bumping a PHPStan level (with each one bringing in loads of changes on the baseline file) still as disruptive to other MRs as we (or at least I) think, or is this a non-issue by now?

If we're in non-issue land on that, I'm all about staggering, if not, we might want to prevent all/a lot of open MRs grinding to a rebase-halt each time we stagger-bump and do it in one go?

One argument against staggering would be that raising the PHPStan level would be disruptive to (almost) all open MRs for each bump, so bumping more then one level would make it less disruptive.

This is true to an extent, but it will only be disruptive in the following cases:
1. The MR is changing the baseline already, and has a conflict.
2. The MR is introducing new PHPStan failures which are caught by the new level.

Hopefully this won't be almost all open MRs but just a small subset.

There have been 698 commits to the 11.x branch in the past 90 days, so even if we went up a level every three months, anything committed in the interim between bumps by definition won't get affected. That still leaves the thousands of open MRs, but not all of these are rebased that often too.

Does this need a dependency evaluation also?

Back to NR?

Does this still need framework manager review? Otherwise RTBC from me.

Let's bump it to RTBC and see if the core committers think we still need FM approval and if so, if they can ping them :)

We probably need a change record :D

Added two CRs.

Thanks for creating the CRs. This looks good. Baseline may need to be regenerated when committing.

@alexpott was concerned about the size of the baseline, a handful of quick fixes in MR!7547 cut it down by about 25%.

a handful of quick fixes in MR!7547 cut it down by about 25%.

Great, but not sure this is part of this issue? Or even what an acceptable amount of added errors to a baseline would be?

Personally, (n=1, yadayada) this seems like follow-up issue material to me and might be derailing the original intent?
But what do I know? _shrug_

Oh yeah, that's why I made a separate MR, only as a demo. But maybe we move that to a separate issue and try and pick off these and any other widespread ones caused by traits or base classes, before doing this, if there are concerns about baseline size.

Let's do what #42 suggests, it's not just the actual baseline size, but that'll also help to avoid some MR conflicts potentially later on.

https://github.com/kenaths/phpstan-fixer ?

Faced that NoDiscard is skipped because checkThisOnly: true in #3560672-12: [policy, no patch] Decide if and where to adopt the #[NoDiscard] attribute

When we added return types to the baseline we had 12,810 entries:

$ git show 4548e0708681707ce1eb99226dfedb5819301ae8:core/.phpstan-baseline.php|grep '^]'|wc -l
12810

We are currently at 6,632 entries in main. We've completed #42 at level 1 by fixing most test traits and trivial cases; we're left with largely concrete return types now that we can't necessarily add because of downstream users. #3486376: Extend Symfony DebugClassLoader to report missing cross-module @return types will notify those users in most cases, but there isn't a huge amount we can do until then.

The latest job shows the baseline would be bumped to 13,881 entries if we go to level 2 now. Perhaps it's time for Drupal 12?

Level 3 takes us up to 15,343 entries, which isn't significantly more. Maybe we can make that jump?

phpstan/phpstan-symfony is at 15,311 which is only a reduction of 32 entries. It looks like if we can feed it an XML container definition it might be more useful, but I don't think it's worth adding in this issue.

CR needs updating to mention level 3

Appears to need a rebase but there are some trait tickets and other random ticket that add/remove chunks for the baseline. Should those land and then bump?

During the beta window sounds like a good idea for commit timing. As well as lots of rebases this is also going to mean most backports to 11.x won't cherry pick, although that's increasingly the case already.

About 15k entries seems like it should hopefully be manageable to deal with.

PHPStan has got a lot faster at generating a baseline so it's not too bad doing that as an extra step before commit occasionally.

Time to revive this again if we are to land it in two weeks.

That issue has been marked Fixed. Changing status.

Updating IS to PHPStan-3

Checked jangregor/phpstan-prophecy and it seems to be well supported (know this was already checked just didn't know what to verify). They've had 1 release this year (2026) and 3 last year (2025) so think that's a good pace.

Results are in the pudding right. Upping to level 3 passed the pipeline.

I'm not framework manager but going to mark.

Summary seems fine to me.

Hope this is okay to mark..

This has a chance of conflicting with everything else that is RTBC and hoping to land before beta, so I think this should wait for commit until just after beta1 is released, I just wanted to resurrect it early enough that we don't miss it or forget.

Bumping the versions in the IS.

Discussed with @longwave - ideally #3067979: Exclude test files from release packages would have landed so the baseline would be excluded from most Drupal site. @longwave has a great idea of adding just the baseline to .gitattributes. Let's open a follow-up to do that.

I'm very +1 to making this change and setting a new line in the sand for Drupal 12.

Removing the date since it is September.

Updated according to the dependency policy by converting to the template in the policy and tagging for release manager review.

If this is approved, the issue is to be tagged, "approved dependency evaluation"

Discussed with longwave and alexpott
Both Alex and I are in support

The idea to use gitattributes is fine for a follow up

+1 from me.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. 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.

There was some discussion in slack https://drupal.slack.com/archives/C079NQPQUEN/p1788462517402589?thread_t...

I had some concerns about generating the baseline locally and resources required.

The thinking is since the job will generate the baseline that can be downloaded the local generation is not a concern.

One more question does the failure link to the new baseline file?

Also I had a question about the baseline size, this will about double the size, but that brings it to about the size the initial baseline was so that should be fine too.

Also I had a question about the baseline size, this will about double the size, but that brings it to about the size the initial baseline was so that should be fine too.

I agree that I don't think this will be a problem at the moment, because we've worked with a baseline of this size before, because of that I think it should also not be a very big issue regarding resources. It seems like there are a couple of traits that we can fix which will bring this back down to a more manageable state soon.