phpstan-drupal
phpstan-drupal copied to clipboard
Random false positive "Missing explicit access check on entity query"
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.
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.
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();
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.
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.