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 08a2153f0a..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 @@ -293,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() diff --git a/src/Rules/Functions/UselessFunctionReturnValueRule.php b/src/Rules/Functions/UselessFunctionReturnValueRule.php new file mode 100644 index 0000000000..b5af3ebd1c --- /dev/null +++ b/src/Rules/Functions/UselessFunctionReturnValueRule.php @@ -0,0 +1,82 @@ + + */ +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', 'show_source'], 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( + 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(), + ]; + } + + return []; + } + +} diff --git a/tests/PHPStan/Levels/data/unreachable-0.json b/tests/PHPStan/Levels/data/unreachable-0.json new file mode 100644 index 0000000000..5091fec153 --- /dev/null +++ b/tests/PHPStan/Levels/data/unreachable-0.json @@ -0,0 +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.", + "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.", + "line": 89, + "ignorable": true + } +] diff --git a/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php new file mode 100644 index 0000000000..12cecc952f --- /dev/null +++ b/tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php @@ -0,0 +1,58 @@ + + */ +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 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.', + 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, + ], + ]); + } + + public function testUselessReturnValuePhp8(): void + { + if (PHP_VERSION_ID < 80000) { + $this->markTestSkipped('Test requires PHP 8.0.'); + } + + $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.', + 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..ae376e8cce --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/useless-fn-return.php @@ -0,0 +1,76 @@ + 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) + ); + } + + public function showSource(string $s): void + { + error_log("Email-Template couldn't be found by parameters:" . show_source($s)); + } + +}