Merge pull request #378 from brambaud/issue/315

Report missing explicit access check on entity queries

extension.neon

@@ -259,6 +259,7 @@ rules:
 	- mglaman\PHPStanDrupal\Rules\Deprecations\PluginAnnotationContextDefinitionsRule
 	- mglaman\PHPStanDrupal\Rules\Drupal\ModuleLoadInclude
 	- mglaman\PHPStanDrupal\Rules\Drupal\LoadIncludes
+	- mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery\EntityQueryHasAccessCheckRule
 services:
 	-
 		class: mglaman\PHPStanDrupal\Drupal\ServiceMap
@@ -286,6 +287,9 @@ services:
 	-
 		class: mglaman\PHPStanDrupal\Type\EntityQuery\EntityQueryDynamicReturnTypeExtension
 		tags: [phpstan.broker.dynamicMethodReturnTypeExtension]
+	-
+		class: mglaman\PHPStanDrupal\Type\EntityQuery\EntityQueryAccessCheckDynamicReturnTypeExtension
+		tags: [phpstan.broker.dynamicMethodReturnTypeExtension]
 	-
 		class: mglaman\PHPStanDrupal\Type\UrlToStringDynamicReturnTypeExtension
 		tags: [phpstan.broker.dynamicMethodReturnTypeExtension]

src/Rules/Drupal/EntityQuery/EntityQueryHasAccessCheckRule.php

@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery;
+
+use mglaman\PHPStanDrupal\Type\EntityQuery\EntityQueryExecuteWithoutAccessCheckCountType;
+use mglaman\PHPStanDrupal\Type\EntityQuery\EntityQueryExecuteWithoutAccessCheckType;
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+
+final class EntityQueryHasAccessCheckRule implements Rule
+{
+    public function getNodeType(): string
+    {
+        return Node\Expr\MethodCall::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (!$node instanceof Node\Expr\MethodCall) {
+            return [];
+        }
+
+        $name = $node->name;
+        if (!$name instanceof Node\Identifier) {
+            return [];
+        }
+        if ($name->toString() !== 'execute') {
+            return [];
+        }
+
+        $type = $scope->getType($node);
+
+        if (!$type instanceof EntityQueryExecuteWithoutAccessCheckCountType && !$type instanceof EntityQueryExecuteWithoutAccessCheckType) {
+            return [];
+        }
+
+        return [
+            RuleErrorBuilder::message(
+                'Missing explicit access check on entity query.'
+            )->tip('See https://www.drupal.org/node/3201242')->build(),
+        ];
+    }
+}

src/Type/EntityQuery/ConfigEntityQueryType.php

@@ -7,7 +7,7 @@
 /**
  * Type used to represent an entity query instance for config entity query.
  */
-final class ConfigEntityQueryType extends ObjectType
+final class ConfigEntityQueryType extends EntityQueryType
 {
 
 }

src/Type/EntityQuery/ContentEntityQueryType.php

@@ -7,7 +7,7 @@
 /**
  * Type used to represent an entity query instance for content entity query.
  */
-final class ContentEntityQueryType extends ObjectType
+final class ContentEntityQueryType extends EntityQueryType
 {
 
 }

src/Type/EntityQuery/EntityQueryAccessCheckDynamicReturnTypeExtension.php

@@ -0,0 +1,40 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Type\EntityQuery;
+
+use Drupal\Core\Entity\Query\QueryInterface;
+use PhpParser\Node\Expr\MethodCall;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\MethodReflection;
+use PHPStan\Reflection\ParametersAcceptorSelector;
+use PHPStan\Type\DynamicMethodReturnTypeExtension;
+use PHPStan\Type\Type;
+
+class EntityQueryAccessCheckDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
+{
+    public function getClass(): string
+    {
+        return QueryInterface::class;
+    }
+
+    public function isMethodSupported(MethodReflection $methodReflection): bool
+    {
+        return 'accessCheck' === $methodReflection->getName();
+    }
+
+    public function getTypeFromMethodCall(
+        MethodReflection $methodReflection,
+        MethodCall $methodCall,
+        Scope $scope
+    ): Type {
+        $varType = $scope->getType($methodCall->var);
+
+        if (!$varType instanceof EntityQueryType) {
+            return ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType();
+        }
+
+        return $varType->withAccessCheck();
+    }
+}

src/Type/EntityQuery/EntityQueryCountType.php

@@ -7,7 +7,7 @@
 /**
  * Type used to represent an entity query instance as count query.
  */
-final class EntityQueryCountType extends ObjectType
+final class EntityQueryCountType extends EntityQueryType
 {
 
 }

src/Type/EntityQuery/EntityQueryDynamicReturnTypeExtension.php

@@ -52,13 +52,19 @@ public function getTypeFromMethodCall(
 
         if ($methodName === 'execute') {
             if ($varType instanceof EntityQueryCountType) {
-                return new IntegerType();
+                return $varType->hasAccessCheck()
+                    ? new IntegerType()
+                    : new EntityQueryExecuteWithoutAccessCheckCountType();
             }
             if ($varType instanceof ConfigEntityQueryType) {
-                return new ArrayType(new StringType(), new StringType());
+                return $varType->hasAccessCheck()
+                    ? new ArrayType(new StringType(), new StringType())
+                    : new EntityQueryExecuteWithoutAccessCheckType(new StringType(), new StringType());
             }
             if ($varType instanceof ContentEntityQueryType) {
-                return new ArrayType(new IntegerType(), new StringType());
+                return $varType->hasAccessCheck()
+                    ? new ArrayType(new IntegerType(), new StringType())
+                    : new EntityQueryExecuteWithoutAccessCheckType(new IntegerType(), new StringType());
             }
             return $defaultReturnType;
         }

src/Type/EntityQuery/EntityQueryExecuteWithoutAccessCheckCountType.php

@@ -0,0 +1,12 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Type\EntityQuery;
+
+use PHPStan\Type\IntegerType;
+
+final class EntityQueryExecuteWithoutAccessCheckCountType extends IntegerType
+{
+
+}

src/Type/EntityQuery/EntityQueryExecuteWithoutAccessCheckType.php

@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Type\EntityQuery;
+
+use PHPStan\Type\ArrayType;
+
+use PHPStan\Type\StringType;
+
+final class EntityQueryExecuteWithoutAccessCheckType extends ArrayType
+{
+
+}

src/Type/EntityQuery/EntityQueryType.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Type\EntityQuery;
+
+use PHPStan\Type\ObjectType;
+
+class EntityQueryType extends ObjectType
+{
+    private bool $hasAccessCheck = false;
+
+    public function hasAccessCheck(): bool
+    {
+        return $this->hasAccessCheck;
+    }
+
+    public function withAccessCheck(): self
+    {
+        $type = clone $this;
+        $type->hasAccessCheck = true;
+
+        return $type;
+    }
+}

tests/fixtures/drupal/modules/phpstan_fixtures/src/EntityQueryHasAccessRule.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace fixtures\drupal\modules\phpstan_fixtures\src;
+
+use function PHPStan\dumpType;
+
+final class EntityQueryHasAccessRule
+{
+    public function foo(): void
+    {
+        \Drupal::entityTypeManager()->getStorage('node')
+            ->getQuery()
+            ->accessCheck(FALSE)
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+    }
+}

tests/fixtures/drupal/modules/phpstan_fixtures/src/EntityQueryWithAccessRule.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace fixtures\drupal\modules\phpstan_fixtures\src;
+
+final class EntityQueryWithAccessRule
+{
+    public function foo(): void
+    {
+        \Drupal::entityTypeManager()->getStorage('node')
+            ->getQuery()
+            ->accessCheck(FALSE)
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+    }
+
+    public function bar(): void
+    {
+        \Drupal::entityQuery('node')
+            ->accessCheck(FALSE)
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+    }
+}

tests/fixtures/drupal/modules/phpstan_fixtures/src/EntityQueryWithoutAccessRule.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace fixtures\drupal\modules\phpstan_fixtures\src;
+
+final class EntityQueryWithoutAccessRule
+{
+    public function foo(): void
+    {
+        \Drupal::entityTypeManager()->getStorage('node')
+            ->getQuery()
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+    }
+
+    public function bar(): void
+    {
+        \Drupal::entityQuery('node')
+            ->condition('field_test', 'foo', '=')
+            ->execute();
+    }
+
+}

tests/src/Rules/EntityQueryHasAccessCheckRuleTest.php

@@ -0,0 +1,48 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Tests\Rules;
+
+use mglaman\PHPStanDrupal\Rules\Drupal\EntityQuery\EntityQueryHasAccessCheckRule;
+use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
+
+final class EntityQueryHasAccessCheckRuleTest extends DrupalRuleTestCase
+{
+    protected function getRule(): \PHPStan\Rules\Rule
+    {
+        return new EntityQueryHasAccessCheckRule();
+    }
+
+    /**
+     * @dataProvider cases
+     */
+    public function test(array $files, array $errors): void
+    {
+        $this->analyse($files, $errors);
+    }
+
+    public function cases(): \Generator
+    {
+        yield [
+            [__DIR__.'/../../fixtures/drupal/modules/phpstan_fixtures/src/EntityQueryWithAccessRule.php'],
+            [],
+        ];
+
+        yield [
+            [__DIR__.'/../../fixtures/drupal/modules/phpstan_fixtures/src/EntityQueryWithoutAccessRule.php'],
+            [
+                [
+                    'Missing explicit access check on entity query.',
+                    11,
+                    'See https://www.drupal.org/node/3201242',
+                ],
+                [
+                    'Missing explicit access check on entity query.',
+                    19,
+                    'See https://www.drupal.org/node/3201242',
+                ],
+            ],
+        ];
+    }
+}