Only booleans in conditions - respect levels more

ondrejmirtes · ondrejmirtes · commit 142d99d55411 · 2018-06-03T10:41:53.000+02:00
diff --git a/rules.neon b/rules.neon
@@ -29,5 +29,7 @@ rules:
 	- PHPStan\Rules\SwitchConditions\MatchingTypeInSwitchCaseConditionRule
 
 services:
+	-
+		class: PHPStan\Rules\BooleansInConditions\BooleanRuleHelper
 	-
 		class: PHPStan\Rules\Operators\OperatorRuleHelper
diff --git a/src/Rules/BooleansInConditions/BooleanInBooleanAndRule.php b/src/Rules/BooleansInConditions/BooleanInBooleanAndRule.php
@@ -7,6 +7,14 @@
 class BooleanInBooleanAndRule implements \PHPStan\Rules\Rule
 {
 
+	/** @var BooleanRuleHelper */
+	private $helper;
+
+	public function __construct(BooleanRuleHelper $helper)
+	{
+		$this->helper = $helper;
+	}
+
 	public function getNodeType(): string
 	{
 		return \PhpParser\Node\Expr\BinaryOp\BooleanAnd::class;
@@ -19,18 +27,17 @@ public function getNodeType(): string
 	 */
 	public function processNode(\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope): array
 	{
-		$leftType = $scope->getType($node->left);
-
 		$messages = [];
-		if (!BooleanRuleHelper::passesAsBoolean($leftType)) {
+		if (!$this->helper->passesAsBoolean($scope, $node->left)) {
+			$leftType = $scope->getType($node->left);
 			$messages[] = sprintf(
 				'Only booleans are allowed in &&, %s given on the left side.',
 				$leftType->describe(VerbosityLevel::typeOnly())
 			);
 		}
 
 		$rightType = $scope->getType($node->right);
-		if (!BooleanRuleHelper::passesAsBoolean($rightType)) {
+		if (!$this->helper->passesAsBoolean($scope, $node->right)) {
 			$messages[] = sprintf(
 				'Only booleans are allowed in &&, %s given on the right side.',
 				$rightType->describe(VerbosityLevel::typeOnly())
diff --git a/src/Rules/BooleansInConditions/BooleanInBooleanNotRule.php b/src/Rules/BooleansInConditions/BooleanInBooleanNotRule.php
@@ -7,6 +7,14 @@
 class BooleanInBooleanNotRule implements \PHPStan\Rules\Rule
 {
 
+	/** @var BooleanRuleHelper */
+	private $helper;
+
+	public function __construct(BooleanRuleHelper $helper)
+	{
+		$this->helper = $helper;
+	}
+
 	public function getNodeType(): string
 	{
 		return \PhpParser\Node\Expr\BooleanNot::class;
@@ -19,11 +27,12 @@ public function getNodeType(): string
 	 */
 	public function processNode(\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope): array
 	{
-		$expressionType = $scope->getType($node->expr);
-		if (BooleanRuleHelper::passesAsBoolean($expressionType)) {
+		if ($this->helper->passesAsBoolean($scope, $node->expr)) {
 			return [];
 		}
 
+		$expressionType = $scope->getType($node->expr);
+
 		return [
 			sprintf(
 				'Only booleans are allowed in a negated boolean, %s given.',
diff --git a/src/Rules/BooleansInConditions/BooleanInBooleanOrRule.php b/src/Rules/BooleansInConditions/BooleanInBooleanOrRule.php
@@ -7,6 +7,14 @@
 class BooleanInBooleanOrRule implements \PHPStan\Rules\Rule
 {
 
+	/** @var BooleanRuleHelper */
+	private $helper;
+
+	public function __construct(BooleanRuleHelper $helper)
+	{
+		$this->helper = $helper;
+	}
+
 	public function getNodeType(): string
 	{
 		return \PhpParser\Node\Expr\BinaryOp\BooleanOr::class;
@@ -19,17 +27,17 @@ public function getNodeType(): string
 	 */
 	public function processNode(\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope): array
 	{
-		$leftType = $scope->getType($node->left);
 		$messages = [];
-		if (!BooleanRuleHelper::passesAsBoolean($leftType)) {
+		if (!$this->helper->passesAsBoolean($scope, $node->left)) {
+			$leftType = $scope->getType($node->left);
 			$messages[] = sprintf(
 				'Only booleans are allowed in ||, %s given on the left side.',
 				$leftType->describe(VerbosityLevel::typeOnly())
 			);
 		}
 
-		$rightType = $scope->getType($node->right);
-		if (!BooleanRuleHelper::passesAsBoolean($rightType)) {
+		if (!$this->helper->passesAsBoolean($scope, $node->right)) {
+			$rightType = $scope->getType($node->right);
 			$messages[] = sprintf(
 				'Only booleans are allowed in ||, %s given on the right side.',
 				$rightType->describe(VerbosityLevel::typeOnly())
diff --git a/src/Rules/BooleansInConditions/BooleanInElseIfConditionRule.php b/src/Rules/BooleansInConditions/BooleanInElseIfConditionRule.php
@@ -7,6 +7,14 @@
 class BooleanInElseIfConditionRule implements \PHPStan\Rules\Rule
 {
 
+	/** @var BooleanRuleHelper */
+	private $helper;
+
+	public function __construct(BooleanRuleHelper $helper)
+	{
+		$this->helper = $helper;
+	}
+
 	public function getNodeType(): string
 	{
 		return \PhpParser\Node\Stmt\ElseIf_::class;
@@ -19,11 +27,12 @@ public function getNodeType(): string
 	 */
 	public function processNode(\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope): array
 	{
-		$conditionExpressionType = $scope->getType($node->cond);
-		if (BooleanRuleHelper::passesAsBoolean($conditionExpressionType)) {
+		if ($this->helper->passesAsBoolean($scope, $node->cond)) {
 			return [];
 		}
 
+		$conditionExpressionType = $scope->getType($node->cond);
+
 		return [
 			sprintf(
 				'Only booleans are allowed in an elseif condition, %s given.',
diff --git a/src/Rules/BooleansInConditions/BooleanInIfConditionRule.php b/src/Rules/BooleansInConditions/BooleanInIfConditionRule.php
@@ -7,6 +7,14 @@
 class BooleanInIfConditionRule implements \PHPStan\Rules\Rule
 {
 
+	/** @var BooleanRuleHelper */
+	private $helper;
+
+	public function __construct(BooleanRuleHelper $helper)
+	{
+		$this->helper = $helper;
+	}
+
 	public function getNodeType(): string
 	{
 		return \PhpParser\Node\Stmt\If_::class;
@@ -19,11 +27,12 @@ public function getNodeType(): string
 	 */
 	public function processNode(\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope): array
 	{
-		$conditionExpressionType = $scope->getType($node->cond);
-		if (BooleanRuleHelper::passesAsBoolean($conditionExpressionType)) {
+		if ($this->helper->passesAsBoolean($scope, $node->cond)) {
 			return [];
 		}
 
+		$conditionExpressionType = $scope->getType($node->cond);
+
 		return [
 			sprintf(
 				'Only booleans are allowed in an if condition, %s given.',
diff --git a/src/Rules/BooleansInConditions/BooleanInTernaryOperatorRule.php b/src/Rules/BooleansInConditions/BooleanInTernaryOperatorRule.php
@@ -7,6 +7,14 @@
 class BooleanInTernaryOperatorRule implements \PHPStan\Rules\Rule
 {
 
+	/** @var BooleanRuleHelper */
+	private $helper;
+
+	public function __construct(BooleanRuleHelper $helper)
+	{
+		$this->helper = $helper;
+	}
+
 	public function getNodeType(): string
 	{
 		return \PhpParser\Node\Expr\Ternary::class;
@@ -23,11 +31,12 @@ public function processNode(\PhpParser\Node $node, \PHPStan\Analyser\Scope $scop
 			return []; // elvis ?:
 		}
 
-		$conditionExpressionType = $scope->getType($node->cond);
-		if (BooleanRuleHelper::passesAsBoolean($conditionExpressionType)) {
+		if ($this->helper->passesAsBoolean($scope, $node->cond)) {
 			return [];
 		}
 
+		$conditionExpressionType = $scope->getType($node->cond);
+
 		return [
 			sprintf(
 				'Only booleans are allowed in a ternary operator condition, %s given.',
diff --git a/src/Rules/BooleansInConditions/BooleanRuleHelper.php b/src/Rules/BooleansInConditions/BooleanRuleHelper.php
@@ -2,22 +2,45 @@
 
 namespace PHPStan\Rules\BooleansInConditions;
 
+use PhpParser\Node\Expr;
+use PHPStan\Analyser\Scope;
+use PHPStan\Rules\RuleLevelHelper;
+use PHPStan\Type\BooleanType;
+use PHPStan\Type\ErrorType;
+use PHPStan\Type\MixedType;
+use PHPStan\Type\Type;
+
 class BooleanRuleHelper
 {
 
-	public static function passesAsBoolean(\PHPStan\Type\Type $type): bool
+	/** @var \PHPStan\Rules\RuleLevelHelper */
+	private $ruleLevelHelper;
+
+	public function __construct(RuleLevelHelper $ruleLevelHelper)
 	{
-		if ($type instanceof \PHPStan\Type\BooleanType) {
-			return true;
+		$this->ruleLevelHelper = $ruleLevelHelper;
+	}
+
+	public function passesAsBoolean(Scope $scope, Expr $expr): bool
+	{
+		$type = $scope->getType($expr);
+		if ($type instanceof MixedType) {
+			return !$type->isExplicitMixed();
 		}
-		if (
-			$type instanceof \PHPStan\Type\MixedType
-			&& !$type->isExplicitMixed()
-		) {
+		$typeToCheck = $this->ruleLevelHelper->findTypeToCheck(
+			$scope,
+			$expr,
+			'',
+			function (Type $type): bool {
+				return $type instanceof BooleanType;
+			}
+		);
+		$foundType = $typeToCheck->getType();
+		if ($foundType instanceof ErrorType) {
 			return true;
 		}
 
-		return false;
+		return $foundType instanceof BooleanType;
 	}
 
 }
diff --git a/tests/Levels/LevelsIntegrationTest.php b/tests/Levels/LevelsIntegrationTest.php
@@ -12,6 +12,7 @@ public function dataTopics(): array
 	{
 		return [
 			['arithmeticOperators'],
+			['onlyBooleans'],
 		];
 	}
 
diff --git a/tests/Levels/data/onlyBooleans-0.json b/tests/Levels/data/onlyBooleans-0.json
@@ -0,0 +1,12 @@
+[
+    {
+        "message": "Method PHPStan\\Levels\\OnlyBooleans\\Foo::doFoo() has parameter $mixed with no typehint specified",
+        "line": 14,
+        "ignorable": true
+    },
+    {
+        "message": "Only booleans are allowed in an if condition, mixed given.",
+        "line": 32,
+        "ignorable": true
+    }
+]
diff --git a/tests/Levels/data/onlyBooleans-2.json b/tests/Levels/data/onlyBooleans-2.json
@@ -0,0 +1,12 @@
+[
+    {
+        "message": "Only booleans are allowed in an if condition, int given.",
+        "line": 26,
+        "ignorable": true
+    },
+    {
+        "message": "Only booleans are allowed in an if condition, float|string given.",
+        "line": 38,
+        "ignorable": true
+    }
+]
diff --git a/tests/Levels/data/onlyBooleans-6.json b/tests/Levels/data/onlyBooleans-6.json
@@ -0,0 +1,7 @@
+[
+    {
+        "message": "Only booleans are allowed in an if condition, int|false given.",
+        "line": 35,
+        "ignorable": true
+    }
+]
diff --git a/tests/Levels/data/onlyBooleans.php b/tests/Levels/data/onlyBooleans.php
@@ -0,0 +1,43 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Levels\OnlyBooleans;
+
+class Foo
+{
+
+	/**
+	 * @param bool $bool
+	 * @param mixed $explicitMixed
+	 * @param int|false $intOrFalse
+	 * @param float|string $floatOrString
+	 */
+	public function doFoo(
+		bool $bool,
+		int $int,
+		$mixed,
+		$explicitMixed,
+		$intOrFalse,
+		$floatOrString
+	): void
+	{
+		if ($bool) {
+
+		}
+		if ($int) {
+
+		}
+		if ($mixed) {
+
+		}
+		if ($explicitMixed) {
+
+		}
+		if ($intOrFalse) {
+
+		}
+		if ($floatOrString) {
+
+		}
+	}
+
+}
diff --git a/tests/Rules/BooleansInConditions/BooleanInBooleanAndRuleTest.php b/tests/Rules/BooleansInConditions/BooleanInBooleanAndRuleTest.php
@@ -3,13 +3,23 @@
 namespace PHPStan\Rules\BooleansInConditions;
 
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleLevelHelper;
 
 class BooleanInBooleanAndRuleTest extends \PHPStan\Testing\RuleTestCase
 {
 
 	protected function getRule(): Rule
 	{
-		return new BooleanInBooleanAndRule();
+		return new BooleanInBooleanAndRule(
+			new BooleanRuleHelper(
+				new RuleLevelHelper(
+					$this->createBroker(),
+					true,
+					false,
+					true
+				)
+			)
+		);
 	}
 
 	public function testRule(): void
diff --git a/tests/Rules/BooleansInConditions/BooleanInBooleanNotRuleTest.php b/tests/Rules/BooleansInConditions/BooleanInBooleanNotRuleTest.php
@@ -3,13 +3,23 @@
 namespace PHPStan\Rules\BooleansInConditions;
 
 use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleLevelHelper;
 
 class BooleanInBooleanNotRuleTest extends \PHPStan\Testing\RuleTestCase
 {
 
 	protected function getRule(): Rule
 	{
-		return new BooleanInBooleanNotRule();
+		return new BooleanInBooleanNotRule(
+			new BooleanRuleHelper(
+				new RuleLevelHelper(
+					$this->createBroker(),
+					true,
+					false,
+					true
+				)
+			)
+		);
 	}
 
 	public function testRule(): void
diff --git a/tests/Rules/BooleansInConditions/BooleanInBooleanOrRuleTest.php b/tests/Rules/BooleansInConditions/BooleanInBooleanOrRuleTest.php
diff --git a/tests/Rules/BooleansInConditions/BooleanInElseIfConditionRuleTest.php b/tests/Rules/BooleansInConditions/BooleanInElseIfConditionRuleTest.php
diff --git a/tests/Rules/BooleansInConditions/BooleanInIfConditionRuleTest.php b/tests/Rules/BooleansInConditions/BooleanInIfConditionRuleTest.php
diff --git a/tests/Rules/BooleansInConditions/BooleanInTernaryOperatorRuleTest.php b/tests/Rules/BooleansInConditions/BooleanInTernaryOperatorRuleTest.php

Original file line number	Diff line number	Diff line change
`@@ -29,5 +29,7 @@ rules:`
`29`	`29`	`- PHPStan\Rules\SwitchConditions\MatchingTypeInSwitchCaseConditionRule`
`30`	`30`
`31`	`31`	`services:`
	`32`	`+ -`
	`33`	`+ class: PHPStan\Rules\BooleansInConditions\BooleanRuleHelper`
`32`	`34`	`-`
`33`	`35`	`class: PHPStan\Rules\Operators\OperatorRuleHelper`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ public function dataTopics(): array`
`12`	`12`	`{`
`13`	`13`	`return [`
`14`	`14`	`['arithmeticOperators'],`
	`15`	`+ ['onlyBooleans'],`
`15`	`16`	`];`
`16`	`17`	`}`
`17`	`18`