Fix version_compare return type

Author: VincentLanglet

resources/functionMap.php

@@ -12725,7 +12725,7 @@
 'VarnishStat::__construct' => ['void', 'args='=>'array'],
 'VarnishStat::getSnapshot' => ['array'],
 'version_compare' => ['int', 'version1'=>'string', 'version2'=>'string'],
-'version_compare\'1' => ['bool', 'version1'=>'string', 'version2'=>'string', 'operator'=>'string|null'],
+'version_compare\'1' => ['__benevolent<bool|null>', 'version1'=>'string', 'version2'=>'string', 'operator'=>'string|null'],
 'vfprintf' => ['int', 'stream'=>'resource', 'format'=>'string', 'args'=>'array<__stringAndStringable|int|float|null|bool>'],
 'virtual' => ['bool', 'uri'=>'string'],
 'Volatile::__construct' => ['void'],

src/Type/Php/VersionCompareFunctionDynamicReturnTypeExtension.php

@@ -10,29 +10,50 @@
 use PHPStan\Php\ComposerPhpVersionFactory;
 use PHPStan\Php\PhpVersion;
 use PHPStan\Reflection\FunctionReflection;
+use PHPStan\Type\BenevolentUnionType;
 use PHPStan\Type\BooleanType;
 use PHPStan\Type\Constant\ConstantBooleanType;
 use PHPStan\Type\Constant\ConstantIntegerType;
 use PHPStan\Type\Constant\ConstantStringType;
 use PHPStan\Type\DynamicFunctionReturnTypeExtension;
+use PHPStan\Type\NullType;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
 use function array_filter;
 use function count;
+use function in_array;
 use function is_array;
 use function version_compare;
 
 #[AutowiredService]
 final class VersionCompareFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
 {
 
+	private const VALID_OPERATORS = [
+		'<',
+		'lt',
+		'<=',
+		'le',
+		'>',
+		'gt',
+		'>=',
+		'ge',
+		'==',
+		'=',
+		'eq',
+		'!=',
+		'<>',
+		'ne',
+	];
+
 	/**
 	 * @param int|array{min: int, max: int}|null $configPhpVersion
 	 */
 	public function __construct(
 		#[AutowiredParameter(ref: '%phpVersion%')]
 		private int|array|null $configPhpVersion,
 		private ComposerPhpVersionFactory $composerPhpVersionFactory,
+		private PhpVersion $phpVersion,
 	)
 	{
 	}
@@ -63,7 +84,9 @@ public function getTypeFromFunctionCall(
 		if (isset($args[2])) {
 			$operatorStrings = $scope->getType($args[2]->value)->getConstantStrings();
 			$counts[] = count($operatorStrings);
-			$returnType = new BooleanType();
+			$returnType = $this->phpVersion->throwsValueErrorForInternalFunctions()
+				? new BooleanType()
+				: new BenevolentUnionType([new BooleanType(), new NullType()]);
 		} else {
 			$returnType = TypeCombinator::union(
 				new ConstantIntegerType(-1),
@@ -81,11 +104,21 @@ public function getTypeFromFunctionCall(
 		}
 
 		$types = [];
+		$canBeNull = false;
 		foreach ($version1Strings as $version1String) {
 			foreach ($version2Strings as $version2String) {
 				if (isset($operatorStrings)) {
 					foreach ($operatorStrings as $operatorString) {
-						$value = version_compare($version1String->getValue(), $version2String->getValue(), $operatorString->getValue());
+						$operatorValue = $operatorString->getValue();
+						if (!in_array($operatorValue, self::VALID_OPERATORS, true)) {
+							if (!$this->phpVersion->throwsValueErrorForInternalFunctions()) {
+								$canBeNull = true;
+							}
+
+							continue;
+						}
+
+						$value = version_compare($version1String->getValue(), $version2String->getValue(), $operatorValue);
 						$types[$value] = new ConstantBooleanType($value);
 					}
 				} else {
@@ -94,6 +127,11 @@ public function getTypeFromFunctionCall(
 				}
 			}
 		}
+
+		if ($canBeNull) {
+			$types[] = new NullType();
+		}
+
 		return TypeCombinator::union(...$types);
 	}
 

tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php

@@ -5237,11 +5237,11 @@ public static function dataFunctions(): array
 				'$versionCompare6',
 			],
 			[
-				'bool',
+				PHP_VERSION_ID < 80000 ? '(bool|null)' : 'bool',
 				'$versionCompare7',
 			],
 			[
-				'bool',
+				PHP_VERSION_ID < 80000 ? '(bool|null)' : 'bool',
 				'$versionCompare8',
 			],
 			[

tests/PHPStan/Analyser/nsrt/version-compare-php7.php

@@ -0,0 +1,32 @@
+<?php // lint < 8.0
+
+declare(strict_types=1);
+
+namespace VersionComparePHP7;
+
+use function PHPStan\Testing\assertType;
+
+class Foo
+{
+	/**
+	 * @param string  $string
+	 * @param '<'|'>' $unionValid
+	 * @param '<'|'a' $unionBoth
+	 * @param 'a'|'b' $unionInvalid
+	 */
+	public function fgetss(
+		string $string,
+		string $unionValid,
+		string $unionBoth,
+		string $unionInvalid,
+	) : void
+	{
+		assertType('(bool|null)', \version_compare($string, $string, $string));
+
+		assertType('false', \version_compare('Foo','Bar','<'));
+		assertType('(bool|null)', \version_compare('Foo','Bar', $string));
+		assertType('false', \version_compare('Foo','Bar', $unionValid));
+		assertType('false|null', \version_compare('Foo','Bar', $unionBoth));
+		assertType('null', \version_compare('Foo','Bar', $unionInvalid));
+	}
+}

tests/PHPStan/Analyser/nsrt/version-compare-php8.php

@@ -0,0 +1,32 @@
+<?php // lint >= 8.0
+
+declare(strict_types=1);
+
+namespace VersionComparePHP8;
+
+use function PHPStan\Testing\assertType;
+
+class Foo
+{
+	/**
+	 * @param string  $string
+	 * @param '<'|'>' $unionValid
+	 * @param '<'|'a' $unionBoth
+	 * @param 'a'|'b' $unionInvalid
+	 */
+	public function fgetss(
+		string $string,
+		string $unionValid,
+		string $unionBoth,
+		string $unionInvalid,
+	) : void
+	{
+		assertType('bool', \version_compare($string, $string, $string));
+
+		assertType('false', \version_compare('Foo','Bar','<'));
+		assertType('bool', \version_compare('Foo','Bar', $string));
+		assertType('false', \version_compare('Foo','Bar', $unionValid));
+		assertType('false', \version_compare('Foo','Bar', $unionBoth));
+		assertType('*NEVER*', \version_compare('Foo','Bar', $unionInvalid));
+	}
+}