Count entity queries on config entities incorrectly require accesCheck() (#531)

Author: Boegie

src/Type/EntityQuery/EntityQueryDynamicReturnTypeExtension.php

@@ -44,20 +44,21 @@ public function getTypeFromMethodCall(
         }
 
         if ($methodName === 'count') {
-            $returnType = new EntityQueryCountType(
+            if ($varType instanceof EntityQueryType) {
+                return $varType->asCount();
+            }
+            return new EntityQueryCountType(
                 $varType->getClassName(),
                 $varType->getSubtractedType(),
                 $varType->getClassReflection()
             );
-            if ($varType instanceof EntityQueryType && $varType->hasAccessCheck()) {
-                return $returnType->withAccessCheck();
-            }
-
-            return $returnType;
         }
 
         if ($methodName === 'execute') {
-            if ($varType instanceof EntityQueryCountType) {
+            if (!$varType instanceof EntityQueryType) {
+                return $defaultReturnType;
+            }
+            if ($varType->isCount()) {
                 return $varType->hasAccessCheck()
                     ? new IntegerType()
                     : new EntityQueryExecuteWithoutAccessCheckCountType();
@@ -72,12 +73,9 @@ public function getTypeFromMethodCall(
                     ? new ArrayType(new IntegerType(), new StringType())
                     : new EntityQueryExecuteWithoutAccessCheckType(new IntegerType(), new StringType());
             }
-            if ($varType instanceof EntityQueryType) {
-                return $varType->hasAccessCheck()
-                    ? new ArrayType(new IntegerType(), new StringType())
-                    : new EntityQueryExecuteWithoutAccessCheckType(new IntegerType(), new StringType());
-            }
-            return $defaultReturnType;
+            return $varType->hasAccessCheck()
+                ? new ArrayType(new IntegerType(), new StringType())
+                : new EntityQueryExecuteWithoutAccessCheckType(new IntegerType(), new StringType());
         }
 
         return $defaultReturnType;

src/Type/EntityQuery/EntityQueryType.php

@@ -10,11 +10,18 @@ class EntityQueryType extends ObjectType
 {
     private bool $hasAccessCheck = false;
 
+    private bool $isCount = false;
+
     public function hasAccessCheck(): bool
     {
         return $this->hasAccessCheck;
     }
 
+    public function isCount(): bool
+    {
+        return $this->isCount;
+    }
+
     public function withAccessCheck(): self
     {
         // The constructor of ObjectType is under backward compatibility promise.
@@ -26,11 +33,29 @@ public function withAccessCheck(): self
             $this->getClassReflection()
         );
         $type->hasAccessCheck = true;
+        $type->isCount = $this->isCount;
+        return $type;
+    }
+
+    public function asCount(): self
+    {
+        // @phpstan-ignore-next-line
+        $type = new static(
+            $this->getClassName(),
+            $this->getSubtractedType(),
+            $this->getClassReflection()
+        );
+        $type->hasAccessCheck = $this->hasAccessCheck;
+        $type->isCount = true;
         return $type;
     }
 
     protected function describeAdditionalCacheKey(): string
     {
-        return $this->hasAccessCheck ? 'with-access-check' : 'without-access-check';
+        $parts = [
+            $this->hasAccessCheck ? 'with-access-check' : 'without-access-check',
+            $this->isCount ? '' : 'count'
+        ];
+        return implode('-', $parts);
     }
 }

src/Type/EntityStorage/GetQueryReturnTypeExtension.php

@@ -42,7 +42,7 @@ public function getTypeFromMethodCall(
         }
 
         $callerType = $scope->getType($methodCall->var);
-        if (!$callerType instanceof ObjectType) {
+        if (!$callerType->isObject()->yes()) {
             return $returnType;
         }
 

tests/src/Rules/EntityQueryHasAccessCheckRuleTest.php

@@ -137,5 +137,11 @@ public function cases(): \Generator
             [__DIR__.'/data/bug-479.php'],
             []
         ];
+
+        yield 'bug-530.php' => [
+            [__DIR__.'/data/bug-530.php'],
+            []
+        ];
+
     }
 }

tests/src/Rules/data/bug-530.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Bug530;
+
+use Drupal\Core\Config\Entity\ConfigEntityStorageInterface;
+
+/**
+ * Tests entity count queries with access check.
+ */
+class TestClass {
+
+    public ConfigEntityStorageInterface $storage;
+
+    public function setUp(): void
+    {
+        $this->storage = \Drupal::entityTypeManager()->getStorage('menu');
+    }
+
+    public function bug530(string $entity_type): void
+    {
+        // Test "normal" entity query with class property.
+        $this->storage->getQuery()
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+
+        // Test "normal" entity query with inline type hint.
+        /** @var \Drupal\Core\Config\Entity\ConfigEntityStorageInterface $storage */
+        $storage = \Drupal::entityTypeManager()->getStorage('menu');
+        $count = $storage->getQuery()
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+
+        // Test count entity query with class property.
+        $this->storage->getQuery()
+            ->condition('field_test', 'foo', '=')
+            ->count()
+            ->execute();
+
+        // Test count entity query with inline type hint.
+        /** @var \Drupal\Core\Config\Entity\ConfigEntityStorageInterface $storage */
+        $storage = \Drupal::entityTypeManager()->getStorage('menu');
+        $count = $storage->getQuery()
+            ->condition('field_test', 'foo', '=')
+            ->count()
+            ->execute();
+    }
+
+}