Update phpstan/phpstan and mglaman/phpstan-drupal to latest versions

+++ b/core/phpstan-bootstrap.php
@@ -0,0 +1,16 @@
+if (!trait_exists(PhpUnitVersionDependentTestCompatibilityTrait::class, FALSE)) {

What I am missing is when should exist and when not? I cannot find it in D10 and this patch is for D10. Can we remove the file phpstan-bootstrap.php for D10? If it is needed, can we than add a comment with when the trait exists and when not.

All the other changes look good to me.

1. +++ b/core/phpstan-baseline.neon
   @@ -805,11 +805,6 @@ parameters:
    			count: 1
    			path: modules/color/color.module
    
   -		-
   -			message: "#^Call to deprecated constant REQUEST_TIME\\: Deprecated in drupal\\:8\\.3\\.0 and is removed from drupal\\:10\\.0\\.0\\. Use \\\\Drupal\\:\\:time\\(\\)\\-\\>getRequestTime\\(\\); $#"
   -			count: 1
   -			path: modules/comment/comment.module
   -
    		-
    			message: "#^Call to deprecated constant REQUEST_TIME\\: Deprecated in drupal\\:8\\.3\\.0 and is removed from drupal\\:10\\.0\\.0\\. Use \\\\Drupal\\:\\:time\\(\\)\\-\\>getRequestTime\\(\\); $#"
    			count: 1
   @@ -1317,7 +1312,7 @@ parameters:
   
   @@ -1317,7 +1312,7 @@ parameters:
    
    		-
    			message: "#^Call to deprecated constant REQUEST_TIME\\: Deprecated in drupal\\:8\\.3\\.0 and is removed from drupal\\:10\\.0\\.0\\. Use \\\\Drupal\\:\\:time\\(\\)\\-\\>getRequestTime\\(\\); $#"
   -			count: 3
   +			count: 2
    			path: modules/history/history.module
    
   

   If I run this patch locally I still get

   ../vendor/bin/phpstan                                                                                                                        Thu 26 May 09:35:29 2022
   Note: Using configuration file /Volumes/dev/sites/drupal8alt.dev/core/phpstan.neon.dist.
    9189/9189 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
   
    ------ -------------------------------------------------------------------------------------------------------------------------------------------------
     Line   core/modules/comment/comment.module
    ------ -------------------------------------------------------------------------------------------------------------------------------------------------
     40     Call to deprecated constant REQUEST_TIME: Deprecated in drupal:8.3.0 and is removed from drupal:10.0.0. Use \Drupal::time()->getRequestTime();
    ------ -------------------------------------------------------------------------------------------------------------------------------------------------
   
    ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
     Line   core/modules/history/history.module
    ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
     24     Ignored error pattern #^Call to deprecated constant REQUEST_TIME\: Deprecated in drupal\:8\.3\.0 and is removed from drupal\:10\.0\.0\. Use \\Drupal\:\:time\(\)\->getRequestTime\(\); $# in path
            /Volumes/dev/sites/drupal8alt.dev/core/modules/history/history.module is expected to occur 2 times, but occurred 3 times.
     121    Call to deprecated constant REQUEST_TIME: Deprecated in drupal:8.3.0 and is removed from drupal:10.0.0. Use \Drupal::time()->getRequestTime();
    ------ ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
   
   
    [ERROR] Found 3 errors
   

   One of the differences between my environment and DrupalCI is PHP version. DrupalCI is on 8.1.0 and I'm on 8.1.3. Not sure if that is to blame.

2. +++ b/core/phpstan-bootstrap.php
   @@ -0,0 +1,16 @@
   +<?php
   +
   +/**
   + * @file
   + * Performs PHPStan boostrap for Drupal static analysis.
   + */
   +
   +use Drupal\Tests\PhpUnitVersionDependentTestCompatibilityTrait;
   +use Drupal\TestTools\PhpUnitCompatibility\RunnerVersion;
   +
   +// In order to manage different method signatures between PHPUnit versions, we
   +// dynamically load a compatibility trait dependent on the PHPUnit runner
   +// version.
   +if (!trait_exists(PhpUnitVersionDependentTestCompatibilityTrait::class, FALSE)) {
   +  class_alias("Drupal\TestTools\PhpUnitCompatibility\PhpUnit" . RunnerVersion::getMajor() . "\TestCompatibilityTrait", PhpUnitVersionDependentTestCompatibilityTrait::class);
   +}
   diff --git a/core/phpstan.neon.dist b/core/phpstan.neon.dist
   
   diff --git a/core/phpstan.neon.dist b/core/phpstan.neon.dist
   index 41bedcd7d3..42bfa78174 100644
   
   index 41bedcd7d3..42bfa78174 100644
   --- a/core/phpstan.neon.dist
   
   --- a/core/phpstan.neon.dist
   +++ b/core/phpstan.neon.dist
   
   +++ b/core/phpstan.neon.dist
   +++ b/core/phpstan.neon.dist
   @@ -7,6 +7,9 @@ parameters:
   
   @@ -7,6 +7,9 @@ parameters:
    
      level: 0
    
   +  bootstrapFiles:
   +    - phpstan-bootstrap.php
   +
   

   I think this is the wrong fix for this problem. The correct fix is to do what Symfony does - see #3282395: Latest versions of PHPStan and mglaman/phpstan-drupal cannot find PhpUnitVersionDependentTestCompatibilityTrait

Looks good to me.

     },
-    "plugin-api-version": "2.3.0"
+    "plugin-api-version": "2.1.0"

Could this change also be removed?

@mondrake and others - does phpstan pass the full analysis locally with #14 or do you get the REQUEST_TIME deprecation baseline changes like DrupalCI?

Trying to work out if it is me or DrupalCI that's being odd.

FWIW, just ran both patches, and ran phpstan without any issue (locally).

drupal(10.0.x**) $ php -v
PHP 8.1.2 (cli) (built: Apr  7 2022 17:46:26) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.1.2, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.2, Copyright (c), by Zend Technologies

So the main question is, why does this work with --debug on drupalci, and not with the parallellization. And why on GHA, does this only resolve 1 error, and it does resolve both on drupalCI.

It is still running on drupalCI.

The deprecated constant rule comes from phpstan-drupal. So possible there would be something wrong in that rule.
https://github.com/mglaman/phpstan-drupal/blob/main/src/Rules/Deprecatio...

However since we always see the deprecated constant coming up in the same 2 files (comment.module / history.module) we might workaround it, by fixing the deprecations here as well?

Unfortunately fixing these constants is not that simple. We really need to refactor them both away. See #2006632: Make HISTORY_READ_LIMIT configurable

https://www.drupal.org/pift-ci-job/2392768 is also failing because of #10. This is kinda bad because that means it's not testing any other dependency updates since CS checks fail first.

Is #2006632: Make HISTORY_READ_LIMIT configurable supposed to address this? It doesn't seem to. And it'd be 9.5.x only, but we need to fix HEAD in 9.4.x somehow. Sorry, forgot for a moment that this is a D10-only issue for core.

The two errors blocking us are using the constant in a define.

define('HISTORY_READ_LIMIT', REQUEST_TIME - 30 * 24 * 60 * 60);

define('COMMENT_NEW_LIMIT', REQUEST_TIME - 30 * 24 * 60 * 60);

Why those are found locally and not on CI is still puzzling me. Guess would be that the reflection provides behaves differently in the minor php-version differences. https://github.com/phpstan/phpstan-src/tree/1.7.x/src/Reflection (But that code is way above my head)

I don't think those are blocked on #2006632: Make HISTORY_READ_LIMIT configurable which is about removing the entire constant.

Let's see if a workaround by just adjusting the defined constant with a non-deprecated constant could work?

Forgot to update baseline.

Adjusted to $_SERVER[‘REQUEST_TIME’] instead of the container call.

The patch from comment #35 looks good to me.
Changing REQUEST_TIME in the defined variables COMMENT_NEW_LIMIT and HISTORY_READ_LIMIT to \Drupal::time()->getRequestTime() is good to me.
The PHPStan baseline file has 1 suppression message removed and another happens now only 2 instead of 3 times.
The packages mglaman/phpstan-drupal and phpstan/phpstan are updated.
For me it is RTBC.

For the patch from comment #36: I really do not like adding $_SERVER[‘REQUEST_TIME’] to Drupal core.

Seems the $_SERVER[‘REQUEST_TIME’] breaks all kinds of tests.

This was suggested by alexpott to give it a try. He also proposed (and pinged mixologic for it) to see if we could first try updating php 8.1 to the latest patch release on DrupalCI to see if that fixes it.
So let's wait a bit on that first.

I really don't think that accessing \Drupal from the main of a module file is a not great idea - even if our test coverage is green. I'd rather try and get DrupalCI's PHP updated and see if it is that before changing this code but if we do have to change the code using $_SERVER['REQUEST_TIME'] would, for me, be the way safer way to do this. Hopefully we don't have to touch it.

Opened #3283430: Update PHP 8.1 to latest bugfix release to track updating the PHP 8.1 version

Updated phpstan again, and disabled the RuntimeReflectionProvider to see if that's the one to blame.

So seems disabling the ReflectionProvider does not solve.

Updated 41 with latest version of phpstan (there was another updated version just now).

Here's a silly wild guess...

Here's the update to PHPStan and PHPStan Drupal and the fix :)

Updated.

There was one rename of a notice in the baseline nothing more.

Going through both release notes, i don't see much changed (on level 0 that is, the stubs can give people some help running on higher levels, if they follow the version of core, and since this is 10.x, this is probably too early).

Both phpstan and phpstan-drupal update a lot (weekly and more) so chasing is not always needed IMO (for patch releases) unless we see performance improvements or fixing of false posisitives.

Updated constraints in composer.json

Committed f50b372 and pushed to 10.0.x. Thanks!

I think the change to how the constants are defined is pragmatic - given that we'd like to completely remove them at some point.

Let's fix the update dependency build wrt to PHPStan...

+++ b/composer.json
@@ -21,12 +21,12 @@
-        "mglaman/phpstan-drupal": "^1.1.9",
+        "mglaman/phpstan-drupal": "1.1.19",
...
-        "phpstan/phpstan": "^1.4.0",
+        "phpstan/phpstan": "1.7.10",

Hmmm ... I disagree with the change to fixed constants. That makes it harder to update and test the future. This is what lock files are for. Reverted so we can go back to constants that allow this to be upgraded.

Updated again to phpstan 1.7.11 and phpstan-drupal to 1.1.20.
In this latest updates no baseline changes were introduced, so i kept the composer.json constraints on 1.7.10 and 1.1.19 but updated to not lock to those versions. (So used the caret ^).

The caret ^ characters have been added.
Back to RTBC.

Committed dea3a7f and pushed to 10.0.x. Thanks!

Okay this is very very odd... but it failed once the patch had been committed. See https://dispatcher.drupalci.org/job/drupal8_core_regression_tests/56659/... - this was testing:

15:04:02 Git commit info:
15:04:02 	dea3a7f (HEAD, origin/10.0.x, 10.0.x) Issue #3282315 by mondrake, mallezie, alexpott: Update phpstan/phpstan and mglaman/phpstan-drupal to latest 

But it failed due to:

15:05:06 Running PHPStan on *all* files.
15:07:21  ------ ---------------------------------------------------------------------- 
15:07:21   Line   core/modules/comment/comment.module                                   
15:07:21  ------ ---------------------------------------------------------------------- 
15:07:21          Ignored error pattern #^Call to deprecated constant REQUEST_TIME\:    
15:07:21          Deprecated in drupal\:8\.3\.0 and is removed from drupal\:10\.0\.0\.  
15:07:21          Use \\Drupal\:\:time\(\)\->getRequestTime\(\); $# in path             
15:07:21          /var/www/html/core/modules/comment/comment.module was not matched in  
15:07:21          reported errors.                                                      
15:07:21  ------ ---------------------------------------------------------------------- 
15:07:21 
15:07:21  ------ ---------------------------------------------------------------------- 
15:07:21   Line   core/modules/history/history.module                                   
15:07:21  ------ ---------------------------------------------------------------------- 
15:07:21   117    Ignored error pattern #^Call to deprecated constant REQUEST_TIME\:    
15:07:21          Deprecated in drupal\:8\.3\.0 and is removed from drupal\:10\.0\.0\.  
15:07:21          Use \\Drupal\:\:time\(\)\->getRequestTime\(\); $# in path             
15:07:21          /var/www/html/core/modules/history/history.module is expected to      
15:07:21          occur 3 times, but occurred only 2 times.                             
15:07:21  ------ ----------------------------------------------------------------------

No idea what's going to make that happen.

Could we just add Call to deprecated constant REQUEST_TIME to the ignored error list for now? It seems unlikely we are going to solve them all for 10.0.x anyway and they are kinda cluttering up the baseline. We already know it's deprecated and any new uses are unlikely to pass code review.

The updated code is entirely equivalent and while I'm not a fan of changing code just to work around a test or a tool, in this case it seems the most pragmatic thing to do.

I don't wanna derail RTBC but 1.8.0 is out and it's required to parse PHP 8.2 attribute at least.

I filled #3293933: Upgrade phpstan/phpstan to 1.8.1 which looks a duplicate now

Updated IS

It's a blocker for #3275851: [META] Fix PHP 8.2 dynamic property deprecations so raised to major

Needs a reroll now that the REQUEST_TIME issue has been resolved by #3112290: Replace REQUEST_TIME in procedural code

#73 with the module and baseline changes removed following #3112290: Replace REQUEST_TIME in procedural code - this is composer.json/lock updates only now.

+1 RTBC, blocker for PHP 8.2

NW for newer version https://github.com/mglaman/phpstan-drupal/releases/tag/1.1.21

RTBC assuming tests are green.

Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!

Follow-up filed #3293933: Upgrade phpstan/phpstan to 1.8.1

Should we bump it again? at least for PHP 8.2 support