From fb3f83e8f601b31a6eb5a12d58211dfced629821 Mon Sep 17 00:00:00 2001
From: Ondrej Mirtes <ondrej@mirtes.cz>
Date: Tue, 21 Sep 2021 15:43:56 +0200
Subject: [PATCH] While loop - condition always true

---
 conf/config.level4.neon                       |  7 +++
 src/Analyser/NodeScopeResolver.php            |  7 ++-
 src/Node/BreaklessWhileLoopNode.php           | 38 +++++++++++
 .../WhileLoopAlwaysTrueConditionRule.php      | 63 +++++++++++++++++++
 .../WhileLoopAlwaysTrueConditionRuleTest.php  | 50 +++++++++++++++
 .../Rules/Comparison/data/while-loop-true.php | 54 ++++++++++++++++
 6 files changed, 218 insertions(+), 1 deletion(-)
 create mode 100644 src/Node/BreaklessWhileLoopNode.php
 create mode 100644 src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php
 create mode 100644 tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php
 create mode 100644 tests/PHPStan/Rules/Comparison/data/while-loop-true.php

diff --git a/conf/config.level4.neon b/conf/config.level4.neon
index 31b3cf0d0d..43fe1bddf6 100644
--- a/conf/config.level4.neon
+++ b/conf/config.level4.neon
@@ -144,6 +144,13 @@ services:
 		tags:
 			- phpstan.rules.rule
 
+	-
+		class: PHPStan\Rules\Comparison\WhileLoopAlwaysTrueConditionRule
+		arguments:
+			treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
+		tags:
+			- phpstan.rules.rule
+
 	-
 		class: PHPStan\Rules\TooWideTypehints\TooWideMethodReturnTypehintRule
 		arguments:
diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php
index ad1ca635e3..8c6dc0db93 100644
--- a/src/Analyser/NodeScopeResolver.php
+++ b/src/Analyser/NodeScopeResolver.php
@@ -56,6 +56,7 @@
 use PHPStan\File\FileReader;
 use PHPStan\Node\BooleanAndNode;
 use PHPStan\Node\BooleanOrNode;
+use PHPStan\Node\BreaklessWhileLoopNode;
 use PHPStan\Node\CatchWithUnthrownExceptionNode;
 use PHPStan\Node\ClassConstantsNode;
 use PHPStan\Node\ClassMethodsNode;
@@ -899,9 +900,13 @@ private function processStmtNode(
 			$isIterableAtLeastOnce = $beforeCondBooleanType instanceof ConstantBooleanType && $beforeCondBooleanType->getValue();
 			$alwaysIterates = $condBooleanType instanceof ConstantBooleanType && $condBooleanType->getValue();
 			$neverIterates = $condBooleanType instanceof ConstantBooleanType && !$condBooleanType->getValue();
+			$breakCount = count($finalScopeResult->getExitPointsByType(Break_::class));
+			if ($breakCount === 0) {
+				$nodeCallback(new BreaklessWhileLoopNode($stmt), $bodyScopeMaybeRan);
+			}
 
 			if ($alwaysIterates) {
-				$isAlwaysTerminating = count($finalScopeResult->getExitPointsByType(Break_::class)) === 0;
+				$isAlwaysTerminating = $breakCount === 0;
 			} elseif ($isIterableAtLeastOnce) {
 				$isAlwaysTerminating = $finalScopeResult->isAlwaysTerminating();
 			} else {
diff --git a/src/Node/BreaklessWhileLoopNode.php b/src/Node/BreaklessWhileLoopNode.php
new file mode 100644
index 0000000000..f662a139fe
--- /dev/null
+++ b/src/Node/BreaklessWhileLoopNode.php
@@ -0,0 +1,38 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node;
+
+use PhpParser\Node\Stmt\While_;
+use PhpParser\NodeAbstract;
+
+/** @api */
+class BreaklessWhileLoopNode extends NodeAbstract implements VirtualNode
+{
+
+	private While_ $originalNode;
+
+	public function __construct(While_ $originalNode)
+	{
+		parent::__construct($originalNode->getAttributes());
+		$this->originalNode = $originalNode;
+	}
+
+	public function getOriginalNode(): While_
+	{
+		return $this->originalNode;
+	}
+
+	public function getType(): string
+	{
+		return 'PHPStan_Node_BreaklessWhileLoop';
+	}
+
+	/**
+	 * @return string[]
+	 */
+	public function getSubNodeNames(): array
+	{
+		return [];
+	}
+
+}
diff --git a/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php b/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php
new file mode 100644
index 0000000000..15d696408b
--- /dev/null
+++ b/src/Rules/Comparison/WhileLoopAlwaysTrueConditionRule.php
@@ -0,0 +1,63 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Comparison;
+
+use PHPStan\Node\BreaklessWhileLoopNode;
+use PHPStan\Rules\RuleErrorBuilder;
+use PHPStan\Type\Constant\ConstantBooleanType;
+
+/**
+ * @implements \PHPStan\Rules\Rule<BreaklessWhileLoopNode>
+ */
+class WhileLoopAlwaysTrueConditionRule implements \PHPStan\Rules\Rule
+{
+
+	private ConstantConditionRuleHelper $helper;
+
+	private bool $treatPhpDocTypesAsCertain;
+
+	public function __construct(
+		ConstantConditionRuleHelper $helper,
+		bool $treatPhpDocTypesAsCertain
+	)
+	{
+		$this->helper = $helper;
+		$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
+	}
+
+	public function getNodeType(): string
+	{
+		return BreaklessWhileLoopNode::class;
+	}
+
+	public function processNode(
+		\PhpParser\Node $node,
+		\PHPStan\Analyser\Scope $scope
+	): array
+	{
+		$originalNode = $node->getOriginalNode();
+		$exprType = $this->helper->getBooleanType($scope, $originalNode->cond);
+		if ($exprType instanceof ConstantBooleanType && $exprType->getValue()) {
+			$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $originalNode): RuleErrorBuilder {
+				if (!$this->treatPhpDocTypesAsCertain) {
+					return $ruleErrorBuilder;
+				}
+
+				$booleanNativeType = $this->helper->getNativeBooleanType($scope, $originalNode->cond);
+				if ($booleanNativeType instanceof ConstantBooleanType) {
+					return $ruleErrorBuilder;
+				}
+
+				return $ruleErrorBuilder->tip('Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.');
+			};
+
+			return [
+				$addTip(RuleErrorBuilder::message('While loop condition is always true.'))->line($originalNode->cond->getLine())
+					->build(),
+			];
+		}
+
+		return [];
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php
new file mode 100644
index 0000000000..f827c11f93
--- /dev/null
+++ b/tests/PHPStan/Rules/Comparison/WhileLoopAlwaysTrueConditionRuleTest.php
@@ -0,0 +1,50 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Comparison;
+
+/**
+ * @extends \PHPStan\Testing\RuleTestCase<WhileLoopAlwaysTrueConditionRule>
+ */
+class WhileLoopAlwaysTrueConditionRuleTest extends \PHPStan\Testing\RuleTestCase
+{
+
+	/** @var bool */
+	private $treatPhpDocTypesAsCertain = true;
+
+	protected function getRule(): \PHPStan\Rules\Rule
+	{
+		return new WhileLoopAlwaysTrueConditionRule(
+			new ConstantConditionRuleHelper(
+				new ImpossibleCheckTypeHelper(
+					$this->createReflectionProvider(),
+					$this->getTypeSpecifier(),
+					[],
+					$this->treatPhpDocTypesAsCertain
+				),
+				$this->treatPhpDocTypesAsCertain
+			),
+			$this->treatPhpDocTypesAsCertain
+		);
+	}
+
+	protected function shouldTreatPhpDocTypesAsCertain(): bool
+	{
+		return $this->treatPhpDocTypesAsCertain;
+	}
+
+	public function testRule(): void
+	{
+		$this->analyse([__DIR__ . '/data/while-loop-true.php'], [
+			[
+				'While loop condition is always true.',
+				10,
+			],
+			[
+				'While loop condition is always true.',
+				20,
+				'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
+			],
+		]);
+	}
+
+}
diff --git a/tests/PHPStan/Rules/Comparison/data/while-loop-true.php b/tests/PHPStan/Rules/Comparison/data/while-loop-true.php
new file mode 100644
index 0000000000..717e753e9d
--- /dev/null
+++ b/tests/PHPStan/Rules/Comparison/data/while-loop-true.php
@@ -0,0 +1,54 @@
+<?php
+
+namespace WhileLoopTrue;
+
+class Foo
+{
+
+	public function doFoo(): void
+	{
+		while (true) {
+
+		}
+	}
+
+	/**
+	 * @param 1 $s
+	 */
+	public function doBar($s): void
+	{
+		while ($s) {
+
+		}
+	}
+
+	/**
+	 * @param string $s
+	 */
+	public function doBar2($s): void
+	{
+		while ($s === null) { // reported by StrictComparisonOfDifferentTypesRule
+
+		}
+	}
+
+	public function doBar3(): void
+	{
+		while (true) {
+			if (rand(0, 1)) {
+				break;
+			}
+		}
+	}
+
+	public function doBar4(): void
+	{
+		$b = true;
+		while ($b) {
+			if (rand(0, 1)) {
+				$b = false;
+			}
+		}
+	}
+
+}