Add AccessResultConditionRule (#737)

* AccessResultConditionTypeSpecifyingExtension

* rework the rule

* treatPhpDocTypesAsCertain

* fix logic

Author: mglaman

extension.neon

@@ -22,6 +22,7 @@ parameters:
 		rules:
 			testClassSuffixNameRule: false
 			dependencySerializationTraitPropertyRule: false
+			accessResultConditionRule: false
 		entityMapping:
 			aggregator_feed:
 				class: Drupal\aggregator\Entity\Feed
@@ -244,6 +245,7 @@ parametersSchema:
 		rules: structure([
 			testClassSuffixNameRule: boolean()
 			dependencySerializationTraitPropertyRule: boolean()
+			accessResultConditionRule: boolean()
 		])
 		entityMapping: arrayOf(anyOf(
 			structure([
@@ -314,7 +316,6 @@ services:
 		class: mglaman\PHPStanDrupal\DeprecatedScope\GroupLegacyScope
 		tags:
 			- phpstan.deprecations.deprecatedScopeResolver
-
 	-
 		class: mglaman\PHPStanDrupal\DeprecatedScope\DeprecationHelperScope
 		tags:

rules.neon

@@ -25,9 +25,15 @@ conditionalTags:
 		phpstan.rules.rule: %drupal.rules.testClassSuffixNameRule%
 	mglaman\PHPStanDrupal\Rules\Drupal\DependencySerializationTraitPropertyRule:
 		phpstan.rules.rule: %drupal.rules.dependencySerializationTraitPropertyRule%
+	mglaman\PHPStanDrupal\Rules\Drupal\AccessResultConditionRule:
+		phpstan.rules.rule: %drupal.rules.accessResultConditionRule%
 
 services:
 	-
 		class: mglaman\PHPStanDrupal\Rules\Drupal\Tests\TestClassSuffixNameRule
 	-
 		class: mglaman\PHPStanDrupal\Rules\Drupal\DependencySerializationTraitPropertyRule
+	-
+		class: mglaman\PHPStanDrupal\Rules\Drupal\AccessResultConditionRule
+		arguments:
+			treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%

src/Rules/Drupal/AccessResultConditionRule.php

@@ -0,0 +1,79 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Rules\Drupal;
+
+use Drupal\Core\Access\AccessResult;
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\VerbosityLevel;
+
+/**
+ * @implements Rule<Node\Expr\StaticCall>
+ */
+final class AccessResultConditionRule implements Rule
+{
+
+    /** @var bool */
+    private $treatPhpDocTypesAsCertain;
+
+    /**
+     * @param bool $treatPhpDocTypesAsCertain
+     */
+    public function __construct($treatPhpDocTypesAsCertain)
+    {
+        $this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
+    }
+
+    public function getNodeType(): string
+    {
+        return Node\Expr\StaticCall::class;
+    }
+
+    public function processNode(Node $node, Scope $scope): array
+    {
+        if (!$node->name instanceof Node\Identifier) {
+            return [];
+        }
+        $methodName = $node->name->toString();
+        if (!in_array($methodName, ['allowedIf', 'forbiddenIf'], true)) {
+            return [];
+        }
+        if (!$node->class instanceof Node\Name) {
+            return [];
+        }
+        $className = $scope->resolveName($node->class);
+        if ($className !== AccessResult::class) {
+            return [];
+        }
+        $args = $node->getArgs();
+        if (count($args) === 0) {
+            return [];
+        }
+        $condition = $args[0]->value;
+        if (!$condition instanceof Node\Expr\BinaryOp\Identical && !$condition instanceof Node\Expr\BinaryOp\NotIdentical) {
+            return [];
+        }
+        $conditionType = $this->treatPhpDocTypesAsCertain ? $scope->getType($condition) : $scope->getNativeType($condition);
+        $bool = $conditionType->toBoolean();
+
+        if ($bool->isTrue()->or($bool->isFalse())->yes()) {
+            $leftType = $this->treatPhpDocTypesAsCertain ? $scope->getType($condition->left) : $scope->getNativeType($condition->left);
+            $rightType = $this->treatPhpDocTypesAsCertain ? $scope->getType($condition->right) : $scope->getNativeType($condition->right);
+
+            return [
+                RuleErrorBuilder::message(sprintf(
+                    'Strict comparison using %s between %s and %s will always evaluate to %s.',
+                    $condition->getOperatorSigil(),
+                    $leftType->describe(VerbosityLevel::value()),
+                    $rightType->describe(VerbosityLevel::value()),
+                    $bool->describe(VerbosityLevel::value()),
+                ))->identifier(sprintf('%s.alwaysFalse', $condition instanceof Node\Expr\BinaryOp\Identical ? 'identical' : 'notIdentical'))->build(),
+            ];
+        }
+        return [];
+    }
+}

tests/src/Rules/AccessResultConditionRuleTest.php

@@ -0,0 +1,35 @@
+<?php
+
+declare(strict_types=1);
+
+namespace mglaman\PHPStanDrupal\Tests\Rules;
+
+use mglaman\PHPStanDrupal\Rules\Drupal\AccessResultConditionRule;
+use mglaman\PHPStanDrupal\Tests\DrupalRuleTestCase;
+use PHPStan\Rules\Rule;
+
+final class AccessResultConditionRuleTest extends DrupalRuleTestCase
+{
+
+    protected function getRule(): Rule
+    {
+        return new AccessResultConditionRule(true);
+    }
+
+    public function testRule(): void
+    {
+        $this->analyse(
+            [__DIR__.'/data/access-result-condition-check.php'],
+            [
+                [
+                    'Strict comparison using === between false and false will always evaluate to true.',
+                    14
+                ],
+                [
+                    'Strict comparison using !== between false and false will always evaluate to false.',
+                    18
+                ],
+            ]
+        );
+    }
+}

tests/src/Rules/data/access-result-condition-check.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace AccessResultConditionCheck;
+
+use Drupal\Core\Access\AccessResult;
+use Drupal\Core\Access\AccessResultInterface;
+
+function foo(bool $x): AccessResultInterface
+{
+    return AccessResult::allowedIf($x);
+}
+function bar(): AccessResultInterface
+{
+    return AccessResult::allowedIf(false === false);
+}
+function baz(): AccessResultInterface
+{
+    return AccessResult::allowedIf(false !== false);
+}