diff --git a/src/Rules/Functions/PrintfHelper.php b/src/Rules/Functions/PrintfHelper.php index 411972885d..b9443a451b 100644 --- a/src/Rules/Functions/PrintfHelper.php +++ b/src/Rules/Functions/PrintfHelper.php @@ -5,12 +5,14 @@ use Nette\Utils\Strings; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Php\PhpVersion; +use ValueError; use function array_filter; use function array_keys; use function count; use function in_array; use function max; use function sprintf; +use function sscanf; use function strlen; use const PREG_SET_ORDER; @@ -26,24 +28,36 @@ public function __construct(private PhpVersion $phpVersion) public function getPrintfPlaceholdersCount(string $format): ?int { - return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format, false); + return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format); } /** @phpstan-return array> parameter index => placeholders */ public function getPrintfPlaceholders(string $format): ?array { - return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format, false); + return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format); } public function getScanfPlaceholdersCount(string $format): ?int { - return $this->getPlaceholdersCount('(?[cdDeEfinosuxX%s]|\[[^\]]+\])', $format, true); + if ($this->phpVersion->throwsValueErrorForInternalFunctions()) { + try { + $result = sscanf('', '%*n' . $format); + } catch (ValueError) { + return null; + } + } else { + $result = @sscanf('', '%*n' . $format); + } + if ($result === null) { + return null; + } + return count($result); } /** * @phpstan-return array>|null parameter index => placeholders */ - private function parsePlaceholders(string $specifiersPattern, string $format, bool $isScanf): ?array + private function parsePlaceholders(string $specifiersPattern, string $format): ?array { $addSpecifier = ''; if ($this->phpVersion->supportsHhPrintfSpecifier()) { @@ -72,10 +86,6 @@ private function parsePlaceholders(string $specifiersPattern, string $format, bo $showValueSuffix = false; if (isset($placeholder['width']) && $placeholder['width'] !== '') { - if ($isScanf) { - // In scanf, * means assignment suppression - skip this placeholder entirely - continue; - } $parsedPlaceholders[] = new PrintfPlaceholder( sprintf('"%s" (width)', $placeholder[0]), $parameterIdx++, @@ -136,9 +146,9 @@ private function getAcceptingTypeBySpecifier(string $specifier): string return 'mixed'; } - private function getPlaceholdersCount(string $specifiersPattern, string $format, bool $isScanf): ?int + private function getPlaceholdersCount(string $specifiersPattern, string $format): ?int { - $placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format, $isScanf); + $placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format); if ($placeholdersMap === null) { return null; } diff --git a/tests/PHPStan/Rules/Functions/PrintfHelperTest.php b/tests/PHPStan/Rules/Functions/PrintfHelperTest.php new file mode 100644 index 0000000000..8aa3498ad9 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/PrintfHelperTest.php @@ -0,0 +1,72 @@ +printf = $this->getPhpVersionIdAwareHelper(PHP_VERSION_ID); + } + + #[RequiresPhp('< 8.0.0')] + public function testReturnsNullForInvalidPatternOnLegacyPhpVersion(): void + { + $this->assertNull($this->printf->getScanfPlaceholdersCount('%a')); + } + + #[RequiresPhp('>= 8.0.0')] + public function testReturnsNullForInvalidPatternOnPhp8(): void + { + $this->assertNull($this->printf->getScanfPlaceholdersCount('%a')); + } + + #[RequiresPhp('>= 8.0.0')] + #[DataProvider('dataLegacyVersionIds')] + public function testLegacyVersionStillThrowsValueErrorOnPhp8(int $versionId): void + { + $helper = $this->getPhpVersionIdAwareHelper($versionId); + $this->expectException(ValueError::class); + $this->expectExceptionMessage('Bad scan conversion character "a"'); + $helper->getScanfPlaceholdersCount('%a'); + $this->fail('check your phpunit'); + } + + private function getPhpVersionIdAwareHelper(int $versionId): PrintfHelper + { + return new PrintfHelper($this->getPhpVersion($versionId)); + } + + private function getPhpVersion(int $versionId): PhpVersion + { + return new PhpVersion($versionId); + } + + public static function dataLegacyVersionIds(): array + { + return [ + 'PHP 4.3.14' => [40314], + 'PHP 5.0.20' => [50020], + 'PHP 5.3.23' => [50323], + 'PHP 5.5.36' => [50536], + 'PHP 7.0.16' => [70016], + 'PHP 7.1.0' => [70100], + 'PHP 7.2.22' => [70222], + 'PHP 7.3.0' => [70300], + 'PHP 7.4.33' => [70433], + ]; + } + +} diff --git a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php index 1e3ab9ddd7..15e8c997db 100644 --- a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php @@ -84,6 +84,14 @@ public function testFile(): void 'Call to sprintf contains 2 placeholders, 1 value given.', 29, ], + [ + 'Call to sscanf contains an invalid placeholder.', + 38, + ], + [ + 'Call to fscanf contains an invalid placeholder.', + 39, + ], [ 'Call to sprintf contains 2 placeholders, 1 value given.', 45, diff --git a/tests/PHPStan/Rules/Functions/data/printf.php b/tests/PHPStan/Rules/Functions/data/printf.php index b423630397..136687292b 100644 --- a/tests/PHPStan/Rules/Functions/data/printf.php +++ b/tests/PHPStan/Rules/Functions/data/printf.php @@ -35,8 +35,8 @@ sscanf($str, "%20[^abcde]a%d", $string, $number); // ok printf("%.E", 3.14159); // ok sprintf("%.E", 3.14159); // ok -sscanf($str, '%.E', $number); // ok -fscanf($str, '%.E', $number); // ok +sscanf($str, '%.E', $number); // bad scan conversion character '.' +fscanf($resource, '%.E', $number); // bad scan conversion character '.' sscanf($str, '%[A-Z]%d', $char, $number); // ok sprintf('%s %s %s', ...[1]); // do not detect unpacked arguments sprintf('%s %s %s', ...[1, 2, 3]); // ok