Random false positive "Missing explicit access check on entity query"

[driskell]

I've been hitting some issues for a month or so where randomly after switching a branch I would see several "Missing explicit access check on entity query" reported. However, they're all fine and fixed. I would move the accessCheck() call around and then run again and the error would vanish. I would put the line back to the original and it wouldn't fail anymore it would be like a ghost that's now gone.

Push forward to now, and we upgraded PHPStan and went from 1.1.20 phpstan-drupal to 1.1.25 and now we have 6 false positives in our CI which won't go away. No matter what we do with the accessCheck call they report. And if we checkout the exact same code version and run PHPStan we get zero errors. So we seem to be seeing different results depending on where it runs - somewhat similar to before where random people would get random failures and it would just disappear - but instead it's now on the CI, no longer random, and completely reproducing every time. CI is GitHub Actions hosted so ephemeral. We just can't get the PHPStan to pass anymore. Locally, we get 0 errors and cannot get them to appear. I managed to get them once but they vanished on the second run, but no luck since.

Can't rule out something really weird in our configuration but I've yet to find it. The only difference I haven't explored in detail is GitHub Actions is using Ubuntu and I am using macOS.

It's only ever been a problem with this specific error though.

[driskell]

OK so I have managed to reproduce on my local but only with --debug:

XXX 9:05:17 XXX$ ./vendor/bin/phpstan analyze
Note: Using configuration file XXX/phpstan.neon.
 912/912 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors


XXX 9:05:17 XXX$ ./vendor/bin/phpstan analyze
Note: Using configuration file XXX/phpstan.neon.
 912/912 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%



 [OK] No errors


XXX 9:05:17 XXX$ ./vendor/bin/phpstan --debug analyze
Note: Using configuration file XXX/phpstan.neon.
---file-list---
 ------ ---------------------------------------------------------
  Line   XXX
 ------ ---------------------------------------------------------
  122    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
 ------ ---------------------------------------------------------

 ------ ---------------------------------------------------
  Line   XXX
 ------ ---------------------------------------------------
  492    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
  506    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
  1426   Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
 ------ ---------------------------------------------------

 ------ ---------------------------------------------------------------
  Line   XXX
 ------ ---------------------------------------------------------------
  76     Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
  119    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
 ------ ---------------------------------------------------------------

 ------ ------------------------------------------------------
  Line   XXX
 ------ ------------------------------------------------------
  534    Missing explicit access check on entity query.
         💡 See https://www.drupal.org/node/3201242
 ------ ------------------------------------------------------


 [ERROR] Found 7 errors

Subsequent analysis without --debug then reproduces, as expected due to caching. But if I remove the PHPStan cache folder and run without --debug there is then 0 errors, and continues to be on each run. Running again after with --debug the errors then appear again, presumedly as --debug is doing something differently (non-parallel?) and ignoring cache.

[driskell]

All instances are of the below pattern where condition appears after accessCheck. When accessCheck appears at the end just before execute the errors disappear even when running with --debug.

  $storage = \Drupal::entityTypeManager()->getStorage('entity');
  $ids = $storage->getQuery()
    ->accessCheck(false)
    ->condition('field', '', '<>')
    ->execute();

[driskell]

Found the issue.

https://github.com/mglaman/phpstan-drupal/blob/1.1.25/src/Type/EntityQuery/EntityQueryDynamicReturnTypeExtension.php#L59-L78

The return of $defaultReturnType is problematic - when tracing PHPStan this default return type comes out of a cache of the return type for the method being used. In my case, randomly, depending on file processing order, the cached result of this for a condition() call can sometimes return a cached EntityQueryType ancestor with hasAccessCheck false and because the Dynamic Return Extension doesn't run for this method it leaves it as the cached value.

The cache key for a return type appears to be purely based on method itself and not scope of the $this.

Will look at what needs to happen to involve the call chain in the cache key.

[driskell]

OK fully reproduced. Had an issue where I couldn't see why it wasn't failing for class methods and noticed there's different cache key when the code is inside a class method. But that was a red herring and I was just not writing the test right clearly.

PR will contain 2 test cases, function and class method, with the cache key fix that works for me.