Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report useless return values of function calls #3225

Merged
merged 12 commits into from
Jul 12, 2024
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,6 @@ parameters:
pure: true
checkParameterCastableToStringFunctions: true
narrowPregMatches: true
uselessReturnValue: true
stubFiles:
- ../stubs/bleedingEdge/Rule.stub
5 changes: 5 additions & 0 deletions conf/config.level0.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -293,3 +295,6 @@ services:

-
class: PHPStan\Rules\Constants\MagicConstantContextRule

-
class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ parameters:
pure: false
checkParameterCastableToStringFunctions: false
narrowPregMatches: false
uselessReturnValue: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ parametersSchema:
pure: bool()
checkParameterCastableToStringFunctions: bool()
narrowPregMatches: bool()
uselessReturnValue: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
82 changes: 82 additions & 0 deletions src/Rules/Functions/UselessFunctionReturnValueRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PhpParser\Node;
use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\ArgumentsNormalizer;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use function count;
use function in_array;
use function sprintf;

/**
* @implements Rule<Node\Expr\FuncCall>
*/
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 [];
}

}
12 changes: 12 additions & 0 deletions tests/PHPStan/Levels/data/unreachable-0.json
Original file line number Diff line number Diff line change
@@ -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
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Functions;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use const PHP_VERSION_ID;

/**
* @extends RuleTestCase<UselessFunctionReturnValueRule>
*/
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,
],
]);
}

}
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Functions/data/useless-fn-return-php8.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php // lint >= 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,
])
);
}
}
76 changes: 76 additions & 0 deletions tests/PHPStan/Rules/Functions/data/useless-fn-return.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php

namespace UselessFunctionReturn;

class FooClass
{
public function fine(bool $bool): void
{
error_log(
"Email-Template couldn't be found by parameters:" . print_r([
'template' => 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));
}

}
Loading