Fix substr() type narrowing for possibly single char result

Author: staabm

src/Type/Php/SubstrDynamicReturnTypeExtension.php

@@ -49,10 +49,12 @@ public function getTypeFromFunctionCall(
 			$zeroOffset = (new ConstantIntegerType(0))->isSuperTypeOf($offset)->yes();
 			$length = null;
 			$positiveLength = false;
+			$maybeOneLength = false;
 
 			if (count($args) === 3) {
 				$length = $scope->getType($args[2]->value);
 				$positiveLength = IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($length)->yes();
+				$maybeOneLength = !(new ConstantIntegerType(1))->isSuperTypeOf($length)->no();
 			}
 
 			$constantStrings = $string->getConstantStrings();
@@ -88,7 +90,7 @@ public function getTypeFromFunctionCall(
 			}
 
 			if ($string->isNonEmptyString()->yes() && ($negativeOffset || $zeroOffset && $positiveLength)) {
-				if ($string->isNonFalsyString()->yes()) {
+				if ($string->isNonFalsyString()->yes() && !$maybeOneLength) {
 					return new IntersectionType([
 						new StringType(),
 						new AccessoryNonFalsyStringType(),

tests/PHPStan/Analyser/NodeScopeResolverTest.php

@@ -1490,6 +1490,7 @@ public function dataFileAsserts(): iterable
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6613.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10187.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10834.php');
+		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-11035.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10952.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-10952b.php');
 		yield from $this->gatherAssertTypes(__DIR__ . '/data/case-insensitive-parent.php');

tests/PHPStan/Analyser/data/bug-11035.php

@@ -0,0 +1,41 @@
+<?php declare(strict_types = 1);
+
+namespace Bug11035;
+
+use function PHPStan\Testing\assertType;
+
+function phone_number_starts_with_zero(string $phone): string
+{
+	if (
+		(12 === strlen($phone) and '380' === substr($phone, 0, 3)) or
+		(11 === strlen($phone) and '80' === substr($phone, 0, 2)) or
+		(10 === strlen($phone) and '0' === substr($phone, 0, 1))
+	) {
+		$phone = '+';
+	} else {
+		$phone = '';
+	}
+	return $phone;
+}
+
+/**
+ * @param int<1,3> $maybeOne
+ * @param int<2,10> $neverOne
+ */
+function lengthTypes(string $phone, int $maybeOne, int $neverOne): string
+{
+	if (
+		10 === strlen($phone)
+	) {
+		assertType('non-falsy-string', $phone);
+
+		assertType('non-empty-string', substr($phone, 0, 1));
+		assertType('bool', '0' === substr($phone, 0, 1));
+
+		assertType('non-empty-string', substr($phone, 0, $maybeOne));
+		assertType('bool', '0' === substr($phone, 0, $maybeOne));
+
+		assertType('non-falsy-string', substr($phone, 0, $neverOne));
+		assertType('false', '0' === substr($phone, 0, $neverOne));
+	}
+}

tests/PHPStan/Analyser/data/non-empty-string-substr.php

@@ -54,7 +54,7 @@ public function doMbSubstr(string $s, $nonEmpty, $positiveInt, $postiveRange, $n
 		assertType('string', mb_substr($s, 0, $positiveInt));
 		assertType('non-empty-string', mb_substr($nonEmpty, 0, $positiveInt));
 
-		assertType('non-falsy-string', mb_substr("déjà_vu", 0, $positiveInt));
+		assertType('non-empty-string', mb_substr("déjà_vu", 0, $positiveInt));
 		assertType("'déjà_vu'", mb_substr("déjà_vu", 0));
 		assertType("'déj'", mb_substr("déjà_vu", 0, 3));
 	}

tests/PHPStan/Analyser/data/non-falsy-string.php

@@ -126,9 +126,9 @@ public function doSubstr($nonFalsey, $positiveInt, $postiveRange, $negativeRange
 		assertType('non-falsy-string', substr($nonFalsey, $negativeRange));
 
 		assertType('non-falsy-string', substr($nonFalsey, 0, 5));
-		assertType('non-falsy-string', substr($nonFalsey, 0, $postiveRange));
+		assertType('non-empty-string', substr($nonFalsey, 0, $postiveRange));
 
-		assertType('non-falsy-string', substr($nonFalsey, 0, $positiveInt));
+		assertType('non-empty-string', substr($nonFalsey, 0, $positiveInt));
 	}
 
 	function numericIntoFalsy(string $s): void

tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

@@ -1022,4 +1022,22 @@ public function testBug3300(): void
 		$this->analyse([__DIR__ . '/../../Analyser/data/bug-3300.php'], []);
 	}
 
+	public function testBug11035(): void
+	{
+		$this->checkAlwaysTrueStrictComparison = true;
+		$this->analyse([__DIR__ . '/../../Analyser/data/bug-11035.php'], [
+			[
+				"Strict comparison using === between '0' and non-falsy-string will always evaluate to false.",
+				39,
+				'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%</>.',
+			],
+		]);
+	}
+
+	public function testBug9804(): void
+	{
+		$this->checkAlwaysTrueStrictComparison = true;
+		$this->analyse([__DIR__ . '/data/bug-9804.php'], []);
+	}
+
 }

tests/PHPStan/Rules/Comparison/data/bug-9804.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Bug9804;
+
+class Test
+{
+	public function error(?string $someString): void
+	{
+		// Line below is the only difference to "pass" method
+		if (!$someString) {
+			return;
+		}
+
+		// Strict comparison using === between int<min, -1>|int<1, max> and 0 will always evaluate to false.
+		$firstLetterAsInt = (int)substr($someString, 0, 1);
+		if ($firstLetterAsInt === 0) {
+			return;
+		}
+	}
+
+	public function pass(?string $someString): void
+	{
+		// Line below is the only difference to "error" method
+		if ($someString === null) {
+			return;
+		}
+
+		// All ok
+		$firstLetterAsInt = (int)substr($someString, 0, 1);
+		if ($firstLetterAsInt === 0) {
+			return;
+		}
+	}
+}