Bleeding edge level 4 - ConstantLooseComparisonRule

Author: ondrejmirtes

conf/bleedingEdge.neon

@@ -12,3 +12,4 @@ parameters:
 		illegalConstructorMethodCall: true
 		disableCheckMissingIterableValueType: true
 		strictUnnecessaryNullsafePropertyFetch: true
+		looseComparison: true

conf/config.level4.neon

@@ -19,6 +19,10 @@ rules:
 	- PHPStan\Rules\TooWideTypehints\TooWideClosureReturnTypehintRule
 	- PHPStan\Rules\TooWideTypehints\TooWideFunctionReturnTypehintRule
 
+conditionalTags:
+	PHPStan\Rules\Comparison\ConstantLooseComparisonRule:
+		phpstan.rules.rule: %featureToggles.looseComparison%
+
 parameters:
 	checkAdvancedIsset: true
 
@@ -120,6 +124,11 @@ services:
 		tags:
 			- phpstan.rules.rule
 
+	-
+		class: PHPStan\Rules\Comparison\ConstantLooseComparisonRule
+		arguments:
+			checkAlwaysTrueLooseComparison: %checkAlwaysTrueLooseComparison%
+
 	-
 		class: PHPStan\Rules\Comparison\TernaryOperatorConstantConditionRule
 		arguments:

conf/config.neon

@@ -36,12 +36,14 @@ parameters:
 		illegalConstructorMethodCall: false
 		disableCheckMissingIterableValueType: false
 		strictUnnecessaryNullsafePropertyFetch: false
+		looseComparison: false
 	fileExtensions:
 		- php
 	checkAdvancedIsset: false
 	checkAlwaysTrueCheckTypeFunctionCall: false
 	checkAlwaysTrueInstanceof: false
 	checkAlwaysTrueStrictComparison: false
+	checkAlwaysTrueLooseComparison: false
 	checkClassCaseSensitivity: false
 	checkExplicitMixed: false
 	checkFunctionArgumentTypes: false
@@ -228,12 +230,14 @@ parametersSchema:
 		illegalConstructorMethodCall: bool(),
 		disableCheckMissingIterableValueType: bool(),
 		strictUnnecessaryNullsafePropertyFetch: bool(),
+		looseComparison: bool()
 	])
 	fileExtensions: listOf(string())
 	checkAdvancedIsset: bool()
 	checkAlwaysTrueCheckTypeFunctionCall: bool()
 	checkAlwaysTrueInstanceof: bool()
 	checkAlwaysTrueStrictComparison: bool()
+	checkAlwaysTrueLooseComparison: bool()
 	checkClassCaseSensitivity: bool()
 	checkExplicitMixed: bool()
 	checkFunctionArgumentTypes: bool()

src/Rules/Comparison/ConstantLooseComparisonRule.php

@@ -0,0 +1,65 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Comparison;
+
+use PhpParser\Node;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\Constant\ConstantBooleanType;
+use PHPStan\Type\VerbosityLevel;
+use function sprintf;
+
+/**
+ * @implements Rule<Node\Expr\BinaryOp>
+ */
+class ConstantLooseComparisonRule implements Rule
+{
+
+	public function __construct(private bool $checkAlwaysTrueLooseComparison)
+	{
+	}
+
+	public function getNodeType(): string
+	{
+		return Node\Expr\BinaryOp::class;
+	}
+
+	public function processNode(Node $node, Scope $scope): array
+	{
+		if (!$node instanceof Node\Expr\BinaryOp\Equal && !$node instanceof Node\Expr\BinaryOp\NotEqual) {
+			return [];
+		}
+
+		$nodeType = $scope->getType($node);
+		if (!$nodeType instanceof ConstantBooleanType) {
+			return [];
+		}
+
+		$leftType = $scope->getType($node->left);
+		$rightType = $scope->getType($node->right);
+
+		if (!$nodeType->getValue()) {
+			return [
+				RuleErrorBuilder::message(sprintf(
+					'Loose comparison using %s between %s and %s will always evaluate to false.',
+					$node instanceof Node\Expr\BinaryOp\Equal ? '==' : '!=',
+					$leftType->describe(VerbosityLevel::value()),
+					$rightType->describe(VerbosityLevel::value()),
+				))->build(),
+			];
+		} elseif ($this->checkAlwaysTrueLooseComparison) {
+			return [
+				RuleErrorBuilder::message(sprintf(
+					'Loose comparison using %s between %s and %s will always evaluate to true.',
+					$node instanceof Node\Expr\BinaryOp\Equal ? '==' : '!=',
+					$leftType->describe(VerbosityLevel::value()),
+					$rightType->describe(VerbosityLevel::value()),
+				))->build(),
+			];
+		}
+
+		return [];
+	}
+
+}

tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php

@@ -0,0 +1,47 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Comparison;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+
+/**
+ * @extends RuleTestCase<ConstantLooseComparisonRule>
+ */
+class ConstantLooseComparisonRuleTest extends RuleTestCase
+{
+
+	private bool $checkAlwaysTrueStrictComparison;
+
+	protected function getRule(): Rule
+	{
+		return new ConstantLooseComparisonRule($this->checkAlwaysTrueStrictComparison);
+	}
+
+	public function testRule(): void
+	{
+		$this->checkAlwaysTrueStrictComparison = false;
+		$this->analyse([__DIR__ . '/data/loose-comparison.php'], [
+			[
+				"Loose comparison using == between 0 and '1' will always evaluate to false.",
+				20,
+			],
+		]);
+	}
+
+	public function testRuleAlwaysTrue(): void
+	{
+		$this->checkAlwaysTrueStrictComparison = true;
+		$this->analyse([__DIR__ . '/data/loose-comparison.php'], [
+			[
+				"Loose comparison using == between 0 and '0' will always evaluate to true.",
+				16,
+			],
+			[
+				"Loose comparison using == between 0 and '1' will always evaluate to false.",
+				20,
+			],
+		]);
+	}
+
+}

tests/PHPStan/Rules/Comparison/data/loose-comparison.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace ConstantLooseComparison;
+
+class Foo
+{
+
+	public function doFoo(string $s, string $i): void
+	{
+		if ($s == $i) {
+
+		}
+		if ($s != $i) {
+
+		}
+		if (0 == "0") {
+
+		}
+
+		if (0 == "1") {
+
+		}
+	}
+
+}