implement array counting

phpstan · Jun 15, 2024 · ed25873 · ed25873
1 parent 21617b0
commit ed25873
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 52 deletions.
diff --git a/src/Rules/Functions/PrintfArrayParametersRule.php b/src/Rules/Functions/PrintfArrayParametersRule.php
@@ -2,6 +2,7 @@
 
 namespace PHPStan\Rules\Functions;
 
+use Hoa\Stream\Test\Unit\IStream\In;
 use PhpParser\Node;
 use PhpParser\Node\Expr\FuncCall;
 use PHPStan\Analyser\Scope;
@@ -11,6 +12,7 @@
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\Constant\ConstantIntegerType;
 use PHPStan\Type\IntegerRangeType;
+use PHPStan\Type\TypeCombinator;
 use function count;
 use function in_array;
 use function sprintf;
@@ -56,85 +58,124 @@ public function processNode(Node $node, Scope $scope): array
 		}
 
 		$formatArgType = $scope->getType($args[0]->value);
-		$maxPlaceHoldersCount = null;
+		$placeHoldersCounts = [];
 		foreach ($formatArgType->getConstantStrings() as $formatString) {
 			$format = $formatString->getValue();
-			$tempPlaceHoldersCount = $this->printfHelper->getPrintfPlaceholdersCount($format);
-			if ($maxPlaceHoldersCount === null) {
-				$maxPlaceHoldersCount = $tempPlaceHoldersCount;
-			} elseif ($tempPlaceHoldersCount > $maxPlaceHoldersCount) {
-				$maxPlaceHoldersCount = $tempPlaceHoldersCount;
-			}
+
+			$placeHoldersCounts[] = $this->printfHelper->getPrintfPlaceholdersCount($format);
 		}
 
-		if ($maxPlaceHoldersCount === null) {
+		if ($placeHoldersCounts === []) {
 			return [];
 		}
 
-		$formatArgsCount = 0;
+		$minCount = min($placeHoldersCounts);
+		$maxCount = max($placeHoldersCounts);
+		if ($minCount === $maxCount) {
+			$placeHoldersCount = new ConstantIntegerType($minCount);
+		} else {
+			$placeHoldersCount = IntegerRangeType::fromInterval($minCount, $maxCount);
+		}
+
+
+		$formatArgsCounts = [];
 		if (isset($args[1])) {
 			$formatArgsType = $scope->getType($args[1]->value);
 
-			$size = null;
 			$constantArrays = $formatArgsType->getConstantArrays();
 			foreach ($constantArrays as $constantArray) {
-				$size = $constantArray->getArraySize();
-
-				if ($size instanceof IntegerRangeType) {
-					break;
-				}
-				if (!$size instanceof ConstantIntegerType) {
-					return [];
-				}
-				$formatArgsCount = $size->getValue();
+				$formatArgsCounts[] = $constantArray->getArraySize();
 			}
 
 			if ($constantArrays === []) {
-				$size = $formatArgsType->getArraySize();
+				$formatArgsCounts[] = $formatArgsType->getArraySize();
 			}
+		}
 
-			if ($size instanceof IntegerRangeType) {
-				if ($size->getMin() !== null && $size->getMax() !== null) {
-					$values = $size->getMin() . '-' . $size->getMax();
-				} elseif ($size->getMin() !== null) {
-					$values = $size->getMin() . ' or more';
-				} elseif ($size->getMax() !== null) {
-					$values = $size->getMax() . ' or less';
-				} else {
-					throw new ShouldNotHappenException();
-				}
-
-				return [
-					RuleErrorBuilder::message(sprintf(
-						sprintf(
-							'%s, %s.',
-							$maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders',
-							'%s values given',
-						),
-						$name,
-						$maxPlaceHoldersCount,
-						$values,
-					))->identifier(sprintf('argument.%s', $name))->build(),
-				];
-			}
+		if ($formatArgsCounts === []) {
+			$formatArgsCount = new ConstantIntegerType(0);
+		} else {
+			$formatArgsCount = TypeCombinator::union(...$formatArgsCounts);
 		}
 
-		if ($formatArgsCount !== $maxPlaceHoldersCount) {
+		if (!$this->placeholdersMatchesArgsCount($placeHoldersCount, $formatArgsCount)) {
+
+			if ($placeHoldersCount instanceof IntegerRangeType) {
+				$placeholders = $this->getIntegerRangeAsString($placeHoldersCount);
+				$singlePlaceholder = false;
+			} elseif ($placeHoldersCount instanceof ConstantIntegerType) {
+				$placeholders = $placeHoldersCount->getValue();
+				$singlePlaceholder = $placeholders === 1;
+			} else {
+				throw new ShouldNotHappenException();
+			}
+
+			if ($formatArgsCount instanceof IntegerRangeType) {
+				$values = $this->getIntegerRangeAsString($formatArgsCount);
+				$singleValue = false;
+			} elseif ($formatArgsCount instanceof ConstantIntegerType) {
+				$values = $formatArgsCount->getValue();
+				$singleValue = $values === 1;
+			} else {
+				throw new ShouldNotHappenException();
+			}
+
 			return [
 				RuleErrorBuilder::message(sprintf(
 					sprintf(
 						'%s, %s.',
-						$maxPlaceHoldersCount === 1 ? 'Call to %s contains %d placeholder' : 'Call to %s contains %d placeholders',
-						$formatArgsCount === 1 ? '%d value given' : '%d values given',
+						$singlePlaceholder ? 'Call to %s contains %d placeholder' : 'Call to %s contains %s placeholders',
+						$singleValue ? '%d value given' : '%s values given',
 					),
 					$name,
-					$maxPlaceHoldersCount,
-					$formatArgsCount,
+					$placeholders,
+					$values,
 				))->identifier(sprintf('argument.%s', $name))->build(),
 			];
 		}
 
 		return [];
 	}
 
+	private function placeholdersMatchesArgsCount(IntegerRangeType|ConstantIntegerType $placeHoldersCount, IntegerRangeType|ConstantIntegerType $formatArgsCount): bool
+	{
+		if ($placeHoldersCount instanceof ConstantIntegerType && $formatArgsCount instanceof ConstantIntegerType) {
+			return $placeHoldersCount->getValue() === $formatArgsCount->getValue();
+		}
+
+		// Zero placeholders + array
+		if ($placeHoldersCount instanceof ConstantIntegerType
+			&& $placeHoldersCount->getValue() === 0
+			&& $formatArgsCount instanceof IntegerRangeType
+		) {
+			return true;
+		}
+
+		if ($placeHoldersCount instanceof IntegerRangeType
+			&& $formatArgsCount instanceof IntegerRangeType
+		    && IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($placeHoldersCount)->yes()
+		) {
+			if ($formatArgsCount->getMin() !== null && $formatArgsCount->getMax() !== null) {
+				// constant array
+				return $placeHoldersCount->isSuperTypeOf($formatArgsCount)->yes();
+			}
+
+			return IntegerRangeType::fromInterval(1, null)->isSuperTypeOf($formatArgsCount)->yes();
+		}
+
+		return false;
+	}
+
+	private function getIntegerRangeAsString(IntegerRangeType $range): string {
+		if ($range->getMin() !== null && $range->getMax() !== null) {
+			return $range->getMin() . '-' . $range->getMax();
+		} elseif ($range->getMin() !== null) {
+			return $range->getMin() . ' or more';
+		} elseif ($range->getMax() !== null) {
+			return $range->getMax() . ' or less';
+		} else {
+			throw new ShouldNotHappenException();
+		}
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfArrayParametersRuleTest.php
@@ -69,8 +69,8 @@ public function testFile(): void
 				34,
 			],
 			[
-				'Call to vsprintf contains 0 placeholders, 1 or more values given.',
-				39,
+				'Call to vsprintf contains 1-2 placeholders, 0 or more values given.',
+				53,
 			],
 		]);
 	}

diff --git a/tests/PHPStan/Rules/Functions/data/vprintf.php b/tests/PHPStan/Rules/Functions/data/vprintf.php
@@ -34,8 +34,31 @@ function doFoo($message, array $arr) {
 	vprintf('%s %s', ['foo']); // one parameter missing
 	vprintf('abc'); // caught by CallToFunctionParametersRule
 	vsprintf('abc', []); // ok
+	vsprintf('abc', $arr); // ok
 
+	if (rand(0,1)) {
+		$format = '%s';
+		$args = ['foo'];
+	} else {
+		$format = '%s%s';
+		$args = ['foo', 'bar'];
+	}
+	vsprintf($format, $args); // ok
+
+	if (rand(0,1)) {
+		$format = '%s';
+	} else {
+		$format = '%s%s';
+	}
+	vsprintf($format, $arr); // need at least non-empty-array
+
+	if (rand(0,1)) {
+		$format = '%s';
+	} else {
+		$format = '%s%s';
+	}
 	if ($arr !== []) {
-		vsprintf('no-placeholder', $arr); // at least one parameter over
+		vsprintf($format, $arr); // ok
 	}
+
 }