From f45fbbd3c5082cbceb829765992594da967a1dda Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 14:17:42 +0200 Subject: [PATCH 01/12] Report useless return values of function calls --- src/Rules/FunctionCallParametersCheck.php | 28 +++++++- .../CallToFunctionParametersRule.php | 1 + .../CallToFunctionParametersRuleTest.php | 32 +++++++++ .../Functions/data/useless-fn-return-php8.php | 24 +++++++ .../Functions/data/useless-fn-return.php | 71 +++++++++++++++++++ 5 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Functions/data/useless-fn-return-php8.php create mode 100644 tests/PHPStan/Rules/Functions/data/useless-fn-return.php diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 4c34fb4f43..912bd45254 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -4,6 +4,7 @@ use PhpParser\Node; use PhpParser\Node\Expr; +use PHPStan\Analyser\ArgumentsNormalizer; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\Scope; use PHPStan\Php\PhpVersion; @@ -29,6 +30,7 @@ use function array_key_exists; use function count; use function implode; +use function in_array; use function is_string; use function max; use function sprintf; @@ -53,7 +55,7 @@ public function __construct( /** * @param Node\Expr\FuncCall|Node\Expr\MethodCall|Node\Expr\StaticCall|Node\Expr\New_ $funcCall - * @param array{0: string, 1: string, 2: string, 3: string, 4: string, 5: string, 6: string, 7: string, 8: string, 9: string, 10: string, 11: string, 12: string, 13?: string} $messages + * @param array{0: string, 1: string, 2: string, 3: string, 4: string, 5: string, 6: string, 7: string, 8: string, 9: string, 10: string, 11: string, 12: string, 13?: string, 14?: string} $messages * @param 'attribute'|'callable'|'method'|'staticMethod'|'function'|'new' $nodeType * @return list */ @@ -244,6 +246,30 @@ public function check( ->build(); } + if ( + $funcCall instanceof Expr\FuncCall + && isset($messages[14]) + && !$scope->isInFirstLevelStatement() + && $funcCall->name instanceof Node\Name + && in_array($funcCall->name->toString(), ['var_export', 'print_r', 'highlight_string', 'highlight_file'], true) + ) { + $reorderedFuncCall = ArgumentsNormalizer::reorderFuncArguments( + $parametersAcceptor, + $funcCall, + ); + + if ($reorderedFuncCall !== null) { + $reorderedArgs = $reorderedFuncCall->getArgs(); + + if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) { + $errors[] = RuleErrorBuilder::message($messages[14]) + ->identifier(sprintf('%s.uselessReturn', $nodeType)) + ->line($funcCall->getStartLine()) + ->build(); + } + } + } + [$addedErrors, $argumentsWithParameters] = $this->processArguments($parametersAcceptor, $funcCall->getStartLine(), $isBuiltin, $arguments, $hasNamedArguments, $messages[10], $messages[11]); foreach ($addedErrors as $error) { $errors[] = $error; diff --git a/src/Rules/Functions/CallToFunctionParametersRule.php b/src/Rules/Functions/CallToFunctionParametersRule.php index 01b80d7f68..d857500b11 100644 --- a/src/Rules/Functions/CallToFunctionParametersRule.php +++ b/src/Rules/Functions/CallToFunctionParametersRule.php @@ -64,6 +64,7 @@ public function processNode(Node $node, Scope $scope): array 'Unknown parameter $%s in call to function ' . $functionName . '.', 'Return type of call to function ' . $functionName . ' contains unresolvable type.', 'Parameter %s of function ' . $functionName . ' contains unresolvable type.', + 'Return value of call to function ' . $functionName . ' is useless.', ], 'function', ); diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 90859848d0..8c528a353b 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -1716,4 +1716,36 @@ public function testCountArrayShift(): void $this->analyse([__DIR__ . '/data/count-array-shift.php'], $errors); } + public function testUselessReturnValue(): void + { + $this->analyse([__DIR__ . '/data/useless-fn-return.php'], [ + [ + 'Return value of call to function print_r is useless.', + 47, + ], + [ + 'Return value of call to function var_export is useless.', + 56, + ], + [ + 'Return value of call to function print_r is useless.', + 64, + ], + ]); + } + + public function testUselessReturnValuePhp8(): void + { + if (PHP_VERSION_ID < 80000) { + return; + } + + $this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [ + [ + 'Return value of call to function print_r is useless.', + 18, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/useless-fn-return-php8.php b/tests/PHPStan/Rules/Functions/data/useless-fn-return-php8.php new file mode 100644 index 0000000000..2114e7152a --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/useless-fn-return-php8.php @@ -0,0 +1,24 @@ += 8.0 + +namespace UselessFunctionReturnPhp8; + +class FooClass +{ + public function explicitReturnNamed(): void + { + error_log("Email-Template couldn't be found by parameters:" . print_r(return: true, value: [ + 'template' => 1, + 'spracheid' => 2, + ]) + ); + } + + public function explicitNoReturnNamed(): void + { + error_log("Email-Template couldn't be found by parameters:" . print_r(return: false, value: [ + 'template' => 1, + 'spracheid' => 2, + ]) + ); + } +} diff --git a/tests/PHPStan/Rules/Functions/data/useless-fn-return.php b/tests/PHPStan/Rules/Functions/data/useless-fn-return.php new file mode 100644 index 0000000000..204371923b --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/useless-fn-return.php @@ -0,0 +1,71 @@ + 1, + 'spracheid' => 2, + ], true) + ); + + $x = print_r([ + 'template' => 1, + 'spracheid' => 2, + ], true); + + print_r([ + 'template' => 1, + 'spracheid' => 2, + ]); + + error_log( + "Email-Template couldn't be found by parameters:" . print_r([ + 'template' => 1, + 'spracheid' => 2, + ], $bool) + ); + + print_r([ + 'template' => 1, + 'spracheid' => 2, + ], $bool); + + $x = print_r([ + 'template' => 1, + 'spracheid' => 2, + ], $bool); + } + + public function missesReturn(): void + { + error_log( + "Email-Template couldn't be found by parameters:" . print_r([ + 'template' => 1, + 'spracheid' => 2, + ]) + ); + } + + public function missesReturnVarDump(): string + { + return "Email-Template couldn't be found by parameters:" . var_export([ + 'template' => 1, + 'spracheid' => 2, + ]); + } + + public function explicitNoReturn(): void + { + error_log("Email-Template couldn't be found by parameters:" . print_r([ + 'template' => 1, + 'spracheid' => 2, + ], false) + ); + } + +} From d728a1fe6ef1912c10b9b5c726720590ed55764c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 15:43:35 +0200 Subject: [PATCH 02/12] Extract new UselessFunctionReturnValueRule --- conf/config.level0.neon | 1 + src/Rules/FunctionCallParametersCheck.php | 28 +------ .../CallToFunctionParametersRule.php | 1 - .../UselessFunctionReturnValueRule.php | 74 +++++++++++++++++++ .../CallToFunctionParametersRuleTest.php | 32 -------- .../UselessFunctionReturnValueRuleTest.php | 54 ++++++++++++++ 6 files changed, 130 insertions(+), 60 deletions(-) create mode 100644 src/Rules/Functions/UselessFunctionReturnValueRule.php create mode 100644 tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 08a2153f0a..6d31c9a427 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -70,6 +70,7 @@ rules: - PHPStan\Rules\Functions\DefineParametersRule - PHPStan\Rules\Functions\ExistingClassesInArrowFunctionTypehintsRule - PHPStan\Rules\Functions\CallToFunctionParametersRule + - PHPStan\Rules\Functions\UselessFunctionReturnValueRule - PHPStan\Rules\Functions\ExistingClassesInClosureTypehintsRule - PHPStan\Rules\Functions\ExistingClassesInTypehintsRule - PHPStan\Rules\Functions\FunctionAttributesRule diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 912bd45254..4c34fb4f43 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -4,7 +4,6 @@ use PhpParser\Node; use PhpParser\Node\Expr; -use PHPStan\Analyser\ArgumentsNormalizer; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\Scope; use PHPStan\Php\PhpVersion; @@ -30,7 +29,6 @@ use function array_key_exists; use function count; use function implode; -use function in_array; use function is_string; use function max; use function sprintf; @@ -55,7 +53,7 @@ public function __construct( /** * @param Node\Expr\FuncCall|Node\Expr\MethodCall|Node\Expr\StaticCall|Node\Expr\New_ $funcCall - * @param array{0: string, 1: string, 2: string, 3: string, 4: string, 5: string, 6: string, 7: string, 8: string, 9: string, 10: string, 11: string, 12: string, 13?: string, 14?: string} $messages + * @param array{0: string, 1: string, 2: string, 3: string, 4: string, 5: string, 6: string, 7: string, 8: string, 9: string, 10: string, 11: string, 12: string, 13?: string} $messages * @param 'attribute'|'callable'|'method'|'staticMethod'|'function'|'new' $nodeType * @return list */ @@ -246,30 +244,6 @@ public function check( ->build(); } - if ( - $funcCall instanceof Expr\FuncCall - && isset($messages[14]) - && !$scope->isInFirstLevelStatement() - && $funcCall->name instanceof Node\Name - && in_array($funcCall->name->toString(), ['var_export', 'print_r', 'highlight_string', 'highlight_file'], true) - ) { - $reorderedFuncCall = ArgumentsNormalizer::reorderFuncArguments( - $parametersAcceptor, - $funcCall, - ); - - if ($reorderedFuncCall !== null) { - $reorderedArgs = $reorderedFuncCall->getArgs(); - - if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) { - $errors[] = RuleErrorBuilder::message($messages[14]) - ->identifier(sprintf('%s.uselessReturn', $nodeType)) - ->line($funcCall->getStartLine()) - ->build(); - } - } - } - [$addedErrors, $argumentsWithParameters] = $this->processArguments($parametersAcceptor, $funcCall->getStartLine(), $isBuiltin, $arguments, $hasNamedArguments, $messages[10], $messages[11]); foreach ($addedErrors as $error) { $errors[] = $error; diff --git a/src/Rules/Functions/CallToFunctionParametersRule.php b/src/Rules/Functions/CallToFunctionParametersRule.php index d857500b11..01b80d7f68 100644 --- a/src/Rules/Functions/CallToFunctionParametersRule.php +++ b/src/Rules/Functions/CallToFunctionParametersRule.php @@ -64,7 +64,6 @@ public function processNode(Node $node, Scope $scope): array 'Unknown parameter $%s in call to function ' . $functionName . '.', 'Return type of call to function ' . $functionName . ' contains unresolvable type.', 'Parameter %s of function ' . $functionName . ' contains unresolvable type.', - 'Return value of call to function ' . $functionName . ' is useless.', ], 'function', ); diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php new file mode 100644 index 0000000000..115d779cf8 --- /dev/null +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -0,0 +1,74 @@ + + */ +class UselessFunctionReturnValueRule implements Rule +{ + + public function __construct(private ReflectionProvider $reflectionProvider) + { + } + + public function getNodeType(): string + { + return FuncCall::class; + } + + public function processNode(Node $funcCall, Scope $scope): array + { + if (!($funcCall->name instanceof Node\Name) || $scope->isInFirstLevelStatement()) { + return []; + } + + if (!$this->reflectionProvider->hasFunction($funcCall->name, $scope)) { + return []; + } + + $functionReflection = $this->reflectionProvider->getFunction($funcCall->name, $scope); + + if (!in_array($functionReflection->getName(), ['var_export', 'print_r', 'highlight_string', 'highlight_file'], true)) { + return []; + } + $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( + $scope, + $funcCall->getArgs(), + $functionReflection->getVariants(), + $functionReflection->getNamedArgumentsVariants(), + ); + + $reorderedFuncCall = ArgumentsNormalizer::reorderFuncArguments( + $parametersAcceptor, + $funcCall, + ); + + if ($reorderedFuncCall === null) { + return []; + } + $reorderedArgs = $reorderedFuncCall->getArgs(); + + if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) { + return [RuleErrorBuilder::message('Return value of call to function ' . $functionReflection->getName() . ' is useless.') + ->identifier('functionReflection.uselessReturn') + ->line($funcCall->getStartLine()) + ->build(), + ]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php index 8c528a353b..90859848d0 100644 --- a/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php @@ -1716,36 +1716,4 @@ public function testCountArrayShift(): void $this->analyse([__DIR__ . '/data/count-array-shift.php'], $errors); } - public function testUselessReturnValue(): void - { - $this->analyse([__DIR__ . '/data/useless-fn-return.php'], [ - [ - 'Return value of call to function print_r is useless.', - 47, - ], - [ - 'Return value of call to function var_export is useless.', - 56, - ], - [ - 'Return value of call to function print_r is useless.', - 64, - ], - ]); - } - - public function testUselessReturnValuePhp8(): void - { - if (PHP_VERSION_ID < 80000) { - return; - } - - $this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [ - [ - 'Return value of call to function print_r is useless.', - 18, - ], - ]); - } - } diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php new file mode 100644 index 0000000000..081fc9609e --- /dev/null +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -0,0 +1,54 @@ + + */ +class UselessFunctionReturnValueRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new UselessFunctionReturnValueRule( + $this->createReflectionProvider(), + ); + } + + public function testUselessReturnValue(): void + { + $this->analyse([__DIR__ . '/data/useless-fn-return.php'], [ + [ + 'Return value of call to function print_r is useless.', + 47, + ], + [ + 'Return value of call to function var_export is useless.', + 56, + ], + [ + 'Return value of call to function print_r is useless.', + 64, + ], + ]); + } + + public function testUselessReturnValuePhp8(): void + { + if (PHP_VERSION_ID < 80000) { + return; + } + + $this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [ + [ + 'Return value of call to function print_r is useless.', + 18, + ], + ]); + } + +} From 14f3d68ed82b6de88c0adc791a9181090efdafdb Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 16:39:03 +0200 Subject: [PATCH 03/12] Create unreachable-0.json --- tests/PHPStan/Levels/data/unreachable-0.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 tests/PHPStan/Levels/data/unreachable-0.json diff --git a/tests/PHPStan/Levels/data/unreachable-0.json b/tests/PHPStan/Levels/data/unreachable-0.json new file mode 100644 index 0000000000..33dfb3565a --- /dev/null +++ b/tests/PHPStan/Levels/data/unreachable-0.json @@ -0,0 +1,12 @@ +[ + { + "message": "Return value of call to function print_r is useless.", + "line": 38, + "ignorable": true + }, + { + "message": "Return value of call to function print_r is useless.", + "line": 89, + "ignorable": true + } +] \ No newline at end of file From fcfa55dfb1e1b431a313c8843b63388f7f94164e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 11 Jul 2024 20:50:51 +0200 Subject: [PATCH 04/12] Update UselessFunctionReturnValueRule.php --- src/Rules/Functions/UselessFunctionReturnValueRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php index 115d779cf8..7c535c4253 100644 --- a/src/Rules/Functions/UselessFunctionReturnValueRule.php +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -62,7 +62,7 @@ public function processNode(Node $funcCall, Scope $scope): array if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) { return [RuleErrorBuilder::message('Return value of call to function ' . $functionReflection->getName() . ' is useless.') - ->identifier('functionReflection.uselessReturn') + ->identifier('function.uselessReturnValue') ->line($funcCall->getStartLine()) ->build(), ]; From 82966afc3b4912df2b7742959d925feceef719a5 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 08:44:39 +0200 Subject: [PATCH 05/12] enable only in bleeding edge --- conf/bleedingEdge.neon | 1 + conf/config.level0.neon | 6 +++++- conf/config.neon | 1 + conf/parametersSchema.neon | 1 + 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index ef0fba19f4..4d8707567e 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -55,5 +55,6 @@ parameters: pure: true checkParameterCastableToStringFunctions: true narrowPregMatches: true + uselessReturnValue: true stubFiles: - ../stubs/bleedingEdge/Rule.stub diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 6d31c9a427..844d09cdf1 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -24,6 +24,8 @@ conditionalTags: phpstan.rules.rule: %featureToggles.missingMagicSerializationRule% PHPStan\Rules\Constants\MagicConstantContextRule: phpstan.rules.rule: %featureToggles.magicConstantOutOfContext% + PHPStan\Rules\Functions\UselessFunctionReturnValueRule: + phpstan.rules.rule: %featureToggles.uselessReturnValue% rules: - PHPStan\Rules\Api\ApiInstantiationRule @@ -70,7 +72,6 @@ rules: - PHPStan\Rules\Functions\DefineParametersRule - PHPStan\Rules\Functions\ExistingClassesInArrowFunctionTypehintsRule - PHPStan\Rules\Functions\CallToFunctionParametersRule - - PHPStan\Rules\Functions\UselessFunctionReturnValueRule - PHPStan\Rules\Functions\ExistingClassesInClosureTypehintsRule - PHPStan\Rules\Functions\ExistingClassesInTypehintsRule - PHPStan\Rules\Functions\FunctionAttributesRule @@ -294,3 +295,6 @@ services: - class: PHPStan\Rules\Constants\MagicConstantContextRule + + - + class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule diff --git a/conf/config.neon b/conf/config.neon index 09d631bc8c..f5d21b7ee1 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -90,6 +90,7 @@ parameters: pure: false checkParameterCastableToStringFunctions: false narrowPregMatches: false + uselessReturnValue: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 0a63268a1b..0cd1faa9b1 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -85,6 +85,7 @@ parametersSchema: pure: bool() checkParameterCastableToStringFunctions: bool() narrowPregMatches: bool() + uselessReturnValue: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() From 0f9676839efbb2abea14bbc4472621dfbcccd45f Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 08:53:48 +0200 Subject: [PATCH 06/12] improve error message --- src/Rules/Functions/UselessFunctionReturnValueRule.php | 10 +++++++++- .../Functions/UselessFunctionReturnValueRuleTest.php | 8 ++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php index 7c535c4253..0e5f4bede7 100644 --- a/src/Rules/Functions/UselessFunctionReturnValueRule.php +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -12,6 +12,7 @@ use PHPStan\Rules\RuleErrorBuilder; use function count; use function in_array; +use function sprintf; /** * @implements Rule @@ -61,7 +62,14 @@ public function processNode(Node $funcCall, Scope $scope): array $reorderedArgs = $reorderedFuncCall->getArgs(); if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) { - return [RuleErrorBuilder::message('Return value of call to function ' . $functionReflection->getName() . ' is useless.') + return [RuleErrorBuilder::message( + sprintf( + 'Return value of function %s is always true and the result is printed instead of being returned. Pass in true as parameter #%d $%s to return the output instead.', + $functionReflection->getName(), + 2, + $parametersAcceptor->getParameters()[1]->getName(), + ), + ) ->identifier('function.uselessReturnValue') ->line($funcCall->getStartLine()) ->build(), diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php index 081fc9609e..ee68566ae3 100644 --- a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -23,15 +23,15 @@ public function testUselessReturnValue(): void { $this->analyse([__DIR__ . '/data/useless-fn-return.php'], [ [ - 'Return value of call to function print_r is useless.', + 'Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 47, ], [ - 'Return value of call to function var_export is useless.', + 'Return value of function var_export is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 56, ], [ - 'Return value of call to function print_r is useless.', + 'Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 64, ], ]); @@ -45,7 +45,7 @@ public function testUselessReturnValuePhp8(): void $this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [ [ - 'Return value of call to function print_r is useless.', + 'Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 18, ], ]); From 5bda9b8e28cec16d3dc1689498debdf3fdce87dd Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 09:02:50 +0200 Subject: [PATCH 07/12] debug risky php 7.2 test --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6670f1cea2..da55d5ffea 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ tests-levels: php vendor/bin/paratest --runner WrapperRunner --no-coverage --group levels tests-coverage: - php vendor/bin/paratest --runner WrapperRunner + php vendor/bin/paratest --runner WrapperRunner --verbose=VERBOSE tests-golden-reflection: php vendor/bin/paratest --runner WrapperRunner --no-coverage tests/PHPStan/Reflection/ReflectionProviderGoldenTest.php From 3fba6ea4858ed2fe3fa8f282e478724e337a59a2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 09:49:04 +0200 Subject: [PATCH 08/12] Update unreachable-0.json --- tests/PHPStan/Levels/data/unreachable-0.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/PHPStan/Levels/data/unreachable-0.json b/tests/PHPStan/Levels/data/unreachable-0.json index 33dfb3565a..71e493bb9b 100644 --- a/tests/PHPStan/Levels/data/unreachable-0.json +++ b/tests/PHPStan/Levels/data/unreachable-0.json @@ -1,11 +1,11 @@ [ { - "message": "Return value of call to function print_r is useless.", + "message": "Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.", "line": 38, "ignorable": true }, { - "message": "Return value of call to function print_r is useless.", + "message": "Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.", "line": 89, "ignorable": true } From 5c9c6feb3aa81b4ac6f3d0e8c8ba3bb118a9134a Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 13:55:42 +0200 Subject: [PATCH 09/12] Revert "debug risky php 7.2 test" This reverts commit 51d77837680354b704b9f419358f79ebe58e6b2e. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index da55d5ffea..6670f1cea2 100644 --- a/Makefile +++ b/Makefile @@ -12,7 +12,7 @@ tests-levels: php vendor/bin/paratest --runner WrapperRunner --no-coverage --group levels tests-coverage: - php vendor/bin/paratest --runner WrapperRunner --verbose=VERBOSE + php vendor/bin/paratest --runner WrapperRunner tests-golden-reflection: php vendor/bin/paratest --runner WrapperRunner --no-coverage tests/PHPStan/Reflection/ReflectionProviderGoldenTest.php From 35a2d24705734a0928b1a32b876cb049031caaed Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 13:57:15 +0200 Subject: [PATCH 10/12] Update UselessFunctionReturnValueRuleTest.php --- .../Rules/Functions/UselessFunctionReturnValueRuleTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php index ee68566ae3..9cdec5ea6a 100644 --- a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -40,7 +40,7 @@ public function testUselessReturnValue(): void public function testUselessReturnValuePhp8(): void { if (PHP_VERSION_ID < 80000) { - return; + $this->markTestSkipped('Test requires PHP 8.0.'); } $this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [ From 457452892b5017e93bd1d13247ed44de3c9dc72c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 14:26:57 +0200 Subject: [PATCH 11/12] add parenthesis --- src/Rules/Functions/UselessFunctionReturnValueRule.php | 2 +- tests/PHPStan/Levels/data/unreachable-0.json | 6 +++--- .../Functions/UselessFunctionReturnValueRuleTest.php | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php index 0e5f4bede7..6d90d86743 100644 --- a/src/Rules/Functions/UselessFunctionReturnValueRule.php +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -64,7 +64,7 @@ public function processNode(Node $funcCall, Scope $scope): array if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) { return [RuleErrorBuilder::message( sprintf( - 'Return value of function %s is always true and the result is printed instead of being returned. Pass in true as parameter #%d $%s to return the output instead.', + 'Return value of function %s() is always true and the result is printed instead of being returned. Pass in true as parameter #%d $%s to return the output instead.', $functionReflection->getName(), 2, $parametersAcceptor->getParameters()[1]->getName(), diff --git a/tests/PHPStan/Levels/data/unreachable-0.json b/tests/PHPStan/Levels/data/unreachable-0.json index 71e493bb9b..5091fec153 100644 --- a/tests/PHPStan/Levels/data/unreachable-0.json +++ b/tests/PHPStan/Levels/data/unreachable-0.json @@ -1,12 +1,12 @@ [ { - "message": "Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.", + "message": "Return value of function print_r() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.", "line": 38, "ignorable": true }, { - "message": "Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.", + "message": "Return value of function print_r() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.", "line": 89, "ignorable": true } -] \ No newline at end of file +] diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php index 9cdec5ea6a..07bc2d5b64 100644 --- a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -23,15 +23,15 @@ public function testUselessReturnValue(): void { $this->analyse([__DIR__ . '/data/useless-fn-return.php'], [ [ - 'Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', + 'Return value of function print_r() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 47, ], [ - 'Return value of function var_export is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', + 'Return value of function var_export() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 56, ], [ - 'Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', + 'Return value of function print_r() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 64, ], ]); @@ -45,7 +45,7 @@ public function testUselessReturnValuePhp8(): void $this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [ [ - 'Return value of function print_r is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', + 'Return value of function print_r() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 18, ], ]); From e4ca95d045486854bd2cd0dedacb63945086664d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 12 Jul 2024 14:42:00 +0200 Subject: [PATCH 12/12] cover show_source --- src/Rules/Functions/UselessFunctionReturnValueRule.php | 2 +- .../Rules/Functions/UselessFunctionReturnValueRuleTest.php | 4 ++++ tests/PHPStan/Rules/Functions/data/useless-fn-return.php | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php index 6d90d86743..b5af3ebd1c 100644 --- a/src/Rules/Functions/UselessFunctionReturnValueRule.php +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -41,7 +41,7 @@ public function processNode(Node $funcCall, Scope $scope): array $functionReflection = $this->reflectionProvider->getFunction($funcCall->name, $scope); - if (!in_array($functionReflection->getName(), ['var_export', 'print_r', 'highlight_string', 'highlight_file'], true)) { + if (!in_array($functionReflection->getName(), ['var_export', 'print_r', 'highlight_string', 'highlight_file', 'show_source'], true)) { return []; } $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php index 07bc2d5b64..12cecc952f 100644 --- a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -34,6 +34,10 @@ public function testUselessReturnValue(): void 'Return value of function print_r() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', 64, ], + [ + 'Return value of function show_source() is always true and the result is printed instead of being returned. Pass in true as parameter #2 $return to return the output instead.', + 73, + ], ]); } diff --git a/tests/PHPStan/Rules/Functions/data/useless-fn-return.php b/tests/PHPStan/Rules/Functions/data/useless-fn-return.php index 204371923b..ae376e8cce 100644 --- a/tests/PHPStan/Rules/Functions/data/useless-fn-return.php +++ b/tests/PHPStan/Rules/Functions/data/useless-fn-return.php @@ -68,4 +68,9 @@ public function explicitNoReturn(): void ); } + public function showSource(string $s): void + { + error_log("Email-Template couldn't be found by parameters:" . show_source($s)); + } + }