diff --git a/bin/functionMetadata_original.php b/bin/functionMetadata_original.php index 82c316f0558..492b082e8bf 100644 --- a/bin/functionMetadata_original.php +++ b/bin/functionMetadata_original.php @@ -136,6 +136,11 @@ 'lchgrp' => ['hasSideEffects' => true], 'lchown' => ['hasSideEffects' => true], 'link' => ['hasSideEffects' => true], + // Pragmatic: stub is #[Pure(true)], but treat like other mbstring string functions. + // Result can change if the analysed program changes internal encoding + // (mb_internal_encoding() or default_charset); PHPStan does not track that. + // https://github.com/phpstan/phpstan/issues/15224 + 'mb_str_pad' => ['hasSideEffects' => false], 'mkdir' => ['hasSideEffects' => true], 'move_uploaded_file' => ['hasSideEffects' => true], 'mysqli_affected_rows' => ['hasSideEffects' => true], diff --git a/bin/generate-function-metadata.php b/bin/generate-function-metadata.php index 0018406d1c6..9137d316bef 100755 --- a/bin/generate-function-metadata.php +++ b/bin/generate-function-metadata.php @@ -75,6 +75,16 @@ public function enterNode(Node $node) break 2; } + // phpstorm-stubs marks mb_str_pad #[Pure(true)], unlike other mbstring string + // functions (#[Pure]). The result can change if the analysed program changes + // internal encoding (mb_internal_encoding() or default_charset); PHPStan does + // not track that. Treat as pure to match mb_strlen et al. + // https://github.com/phpstan/phpstan/issues/15224 + if ($functionName === 'mb_str_pad') { + $this->functions[] = $functionName; + break 2; + } + // PhpStorm stub's #[Pure(true)] means the function has side effects but its return value is important. // In PHPStan's criteria, these functions are simply considered as ['hasSideEffect' => true]. if (isset($attr->args[0]->value->name->name) && $attr->args[0]->value->name->name === 'true') { diff --git a/resources/functionMetadata.php b/resources/functionMetadata.php index 794d9ce6c3f..9218f25d746 100644 --- a/resources/functionMetadata.php +++ b/resources/functionMetadata.php @@ -1378,7 +1378,7 @@ 'mb_rtrim' => ['hasSideEffects' => false], 'mb_scrub' => ['hasSideEffects' => false], 'mb_split' => ['hasSideEffects' => false], - 'mb_str_pad' => ['hasSideEffects' => true], + 'mb_str_pad' => ['hasSideEffects' => false], 'mb_str_split' => ['hasSideEffects' => false], 'mb_strcut' => ['hasSideEffects' => false], 'mb_strimwidth' => ['hasSideEffects' => false], diff --git a/tests/PHPStan/Analyser/nsrt/bug-15224.php b/tests/PHPStan/Analyser/nsrt/bug-15224.php new file mode 100644 index 00000000000..b51e3260ebb --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15224.php @@ -0,0 +1,16 @@ += 8.3 + +namespace Bug15224; + +use function PHPStan\Testing\assertType; + +function rememberMbStrPad(string $s): void +{ + if (mb_str_pad($s, 5) === 'x') { + assertType("'x'", mb_str_pad($s, 5)); + } else { + assertType('string', mb_str_pad($s, 5)); + } + + assertType('string', mb_str_pad($s, 5)); +} diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php index f81bbcf3291..53ca14c93ed 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRuleTest.php @@ -167,6 +167,25 @@ public function testFirstClassCallables(): void ]); } + #[RequiresPhp('>= 8.3.0')] + public function testBug15224(): void + { + $this->analyse([__DIR__ . '/data/bug-15224-no-side-effects.php'], [ + [ + 'Call to function mb_str_pad() on a separate line has no effect.', + 7, + ], + [ + 'Call to function mb_str_pad() on a separate line has no effect.', + 8, + ], + [ + 'Call to function mb_str_pad() on a separate line has no effect.', + 11, + ], + ]); + } + #[RequiresPhp('>= 8.5.0')] public function testPipeOperator(): void { diff --git a/tests/PHPStan/Rules/Functions/data/bug-15224-no-side-effects.php b/tests/PHPStan/Rules/Functions/data/bug-15224-no-side-effects.php new file mode 100644 index 00000000000..858a0761fb4 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-15224-no-side-effects.php @@ -0,0 +1,12 @@ += 8.3 + +namespace Bug15224NoSideEffects; + +function doFoo(string $s): void +{ + mb_str_pad($s, 5); + mb_str_pad($s, 5, encoding: 'UTF-8'); + + $pad = mb_str_pad(...); + mb_str_pad(...); +} diff --git a/tests/PHPStan/Rules/Pure/Bug15224Php83PureFunctionRuleTest.php b/tests/PHPStan/Rules/Pure/Bug15224Php83PureFunctionRuleTest.php new file mode 100644 index 00000000000..a684c738425 --- /dev/null +++ b/tests/PHPStan/Rules/Pure/Bug15224Php83PureFunctionRuleTest.php @@ -0,0 +1,31 @@ + + */ +class Bug15224Php83PureFunctionRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new PureFunctionRule(new FunctionPurityCheck()); + } + + public function testBug15224(): void + { + $this->analyse([__DIR__ . '/data/bug-15224-php83.php'], []); + } + + public static function getAdditionalConfigFiles(): array + { + return [ + __DIR__ . '/data/bug-15224-php83.neon', + ]; + } + +} diff --git a/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php b/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php index 1a4d3c3fbba..8a7ae73fc53 100644 --- a/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php +++ b/tests/PHPStan/Rules/Pure/PureFunctionRuleTest.php @@ -230,6 +230,12 @@ public function testBug6574(): void $this->analyse([__DIR__ . '/data/bug-6574.php'], []); } + #[RequiresPhp('>= 8.4.0')] + public function testBug15224(): void + { + $this->analyse([__DIR__ . '/data/bug-15224.php'], []); + } + public function testPureUnlessCallableIsImpure(): void { $this->analyse([__DIR__ . '/data/pure-unless-callable-is-impure.php'], [ diff --git a/tests/PHPStan/Rules/Pure/data/bug-15224-php83.neon b/tests/PHPStan/Rules/Pure/data/bug-15224-php83.neon new file mode 100644 index 00000000000..845f8f63042 --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/bug-15224-php83.neon @@ -0,0 +1,2 @@ +parameters: + phpVersion: 80300 diff --git a/tests/PHPStan/Rules/Pure/data/bug-15224-php83.php b/tests/PHPStan/Rules/Pure/data/bug-15224-php83.php new file mode 100644 index 00000000000..35cc9473110 --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/bug-15224-php83.php @@ -0,0 +1,50 @@ += 8.3 +declare(strict_types = 1); + +namespace Bug15224Php83; + +/** + * @phpstan-pure + */ +function test1(): string +{ + return mb_strcut('foo', 1) . + mb_strcut('foo', 1, encoding: 'UTF-8') . + mb_str_pad('foo', 123, 'x') . + mb_str_pad('foo', 123, 'x', encoding: 'UTF-8') + ; +} + +/** + * @phpstan-pure + */ +function test2a(): int|false +{ + return mb_strpos('foo', 'x', 0); +} + +/** + * @phpstan-pure + */ +function test2b(): int|false +{ + return mb_strpos('foo', 'x', 0, encoding: 'UTF-8'); +} + +/** + * @phpstan-pure + * @return array + */ +function test3a(): array +{ + return mb_str_split('foo'); +} + +/** + * @phpstan-pure + * @return array + */ +function test3b(): array +{ + return mb_str_split('foo', encoding: 'UTF-8'); +} diff --git a/tests/PHPStan/Rules/Pure/data/bug-15224.php b/tests/PHPStan/Rules/Pure/data/bug-15224.php new file mode 100644 index 00000000000..c2effe46683 --- /dev/null +++ b/tests/PHPStan/Rules/Pure/data/bug-15224.php @@ -0,0 +1,67 @@ += 8.4 +declare(strict_types = 1); + +namespace Bug15224; + +/** + * @phpstan-pure + */ +function test1(): string +{ + return mb_trim('foo', 'x') . + mb_trim('foo', 'x', encoding: 'UTF-8') . + mb_strcut('foo', 1) . + mb_strcut('foo', 1, encoding: 'UTF-8') . + mb_ucfirst('foo') . + mb_ucfirst('foo', encoding: 'UTF-8') . + mb_str_pad('foo', 123, 'x') . + mb_str_pad('foo', 123, 'x', encoding: 'UTF-8') + ; +} + +/** + * @phpstan-pure + */ +function test2a(): int|false +{ + return mb_strpos('foo', 'x', 0); +} + +/** + * @phpstan-pure + */ +function test2b(): int|false +{ + return mb_strpos('foo', 'x', 0, encoding: 'UTF-8'); +} + +/** + * @phpstan-pure + * @return array + */ +function test3a(): array +{ + return mb_str_split('foo'); +} + +/** + * @phpstan-pure + * @return array + */ +function test3b(): array +{ + return mb_str_split('foo', encoding: 'UTF-8'); +} + +/** + * @phpstan-pure + */ +function test4(): string +{ + return mb_ltrim('foo') . + mb_ltrim('foo', encoding: 'UTF-8') . + mb_rtrim('foo') . + mb_rtrim('foo', encoding: 'UTF-8') . + mb_lcfirst('foo') . + mb_lcfirst('foo', encoding: 'UTF-8'); +}