While loop - condition always true

Author: ondrejmirtes

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:

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 {

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 [];
+	}
+
+}

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 [];
+	}
+
+}

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%</>.',
+			],
+		]);
+	}
+
+}

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;
+			}
+		}
+	}
+
+}