Skip to content

Commit

Permalink
Report useless return values of function calls
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored Jul 12, 2024
1 parent 892eb2e commit ebce317
Show file tree
Hide file tree
Showing 9 changed files with 260 additions and 0 deletions.
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));
}

}

0 comments on commit ebce317

Please sign in to comment.