UselessFunctionReturnValueRule: More precise error message

phpstan · Aug 1, 2024 · 35a2f57 · 35a2f57
1 parent 7328128
commit 35a2f57
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 13 deletions.
diff --git 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
@@ -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
@@ -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));
-	}
-
 }