From 35a2f577d14229e5f50ef83bd417f84ef56ee04e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 1 Aug 2024 10:11:16 +0200 Subject: [PATCH] UselessFunctionReturnValueRule: More precise error message --- .../Functions/UselessFunctionReturnValueRule.php | 13 ++++++++++--- .../UselessFunctionReturnValueRuleTest.php | 6 +----- .../Rules/Functions/data/useless-fn-return.php | 5 ----- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php index b5af3ebd1..102f9fec2 100644 --- a/src/Rules/Functions/UselessFunctionReturnValueRule.php +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -10,8 +10,8 @@ use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use function array_key_exists; use function count; -use function in_array; use function sprintf; /** @@ -20,6 +20,12 @@ class UselessFunctionReturnValueRule implements Rule { + private const USELESS_FUNCTIONS = [ + 'var_export' => 'null', + 'print_r' => 'true', + 'highlight_string' => 'true', + ]; + public function __construct(private ReflectionProvider $reflectionProvider) { } @@ -41,7 +47,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', 'show_source'], true)) { + if (!array_key_exists($functionReflection->getName(), self::USELESS_FUNCTIONS)) { return []; } $parametersAcceptor = ParametersAcceptorSelector::selectFromArgs( @@ -64,8 +70,9 @@ 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 %s and the result is printed instead of being returned. Pass in true as parameter #%d $%s to return the output instead.', $functionReflection->getName(), + self::USELESS_FUNCTIONS[$functionReflection->getName()], 2, $parametersAcceptor->getParameters()[1]->getName(), ), diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php index 12cecc952..422103bd3 100644 --- a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -27,17 +27,13 @@ public function testUselessReturnValue(): void 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 null 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.', 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 ae376e8cc..204371923 100644 --- a/tests/PHPStan/Rules/Functions/data/useless-fn-return.php +++ b/tests/PHPStan/Rules/Functions/data/useless-fn-return.php @@ -68,9 +68,4 @@ public function explicitNoReturn(): void ); } - public function showSource(string $s): void - { - error_log("Email-Template couldn't be found by parameters:" . show_source($s)); - } - }