Skip to content

Commit ebce317

Browse files
staabmstaabm
authored
Report useless return values of function calls
1 parent 892eb2e commit ebce317

File tree

9 files changed

+260
-0
lines changed

9 files changed

+260
-0
lines changed

‎conf/bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,5 +55,6 @@ parameters:
5555
pure: true
5656
checkParameterCastableToStringFunctions: true
5757
narrowPregMatches: true
58+
uselessReturnValue: true
5859
stubFiles:
5960
- ../stubs/bleedingEdge/Rule.stub

‎conf/config.level0.neon

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ conditionalTags:
2424
phpstan.rules.rule: %featureToggles.missingMagicSerializationRule%
2525
PHPStan\Rules\Constants\MagicConstantContextRule:
2626
phpstan.rules.rule: %featureToggles.magicConstantOutOfContext%
27+
PHPStan\Rules\Functions\UselessFunctionReturnValueRule:
28+
phpstan.rules.rule: %featureToggles.uselessReturnValue%
2729

2830
rules:
2931
- PHPStan\Rules\Api\ApiInstantiationRule
@@ -293,3 +295,6 @@ services:
293295

294296
-
295297
class: PHPStan\Rules\Constants\MagicConstantContextRule
298+
299+
-
300+
class: PHPStan\Rules\Functions\UselessFunctionReturnValueRule

‎conf/config.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ parameters:
9090
pure: false
9191
checkParameterCastableToStringFunctions: false
9292
narrowPregMatches: false
93+
uselessReturnValue: false
9394
fileExtensions:
9495
- php
9596
checkAdvancedIsset: false

‎conf/parametersSchema.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ parametersSchema:
8585
pure: bool()
8686
checkParameterCastableToStringFunctions: bool()
8787
narrowPregMatches: bool()
88+
uselessReturnValue: bool()
8889
])
8990
fileExtensions: listOf(string())
9091
checkAdvancedIsset: bool()

‎src/Rules/Functions/UselessFunctionReturnValueRule.php

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use PhpParser\Node;
6+
use PhpParser\Node\Expr\FuncCall;
7+
use PHPStan\Analyser\ArgumentsNormalizer;
8+
use PHPStan\Analyser\Scope;
9+
use PHPStan\Reflection\ParametersAcceptorSelector;
10+
use PHPStan\Reflection\ReflectionProvider;
11+
use PHPStan\Rules\Rule;
12+
use PHPStan\Rules\RuleErrorBuilder;
13+
use function count;
14+
use function in_array;
15+
use function sprintf;
16+
17+
/**
18+
* @implements Rule<Node\Expr\FuncCall>
19+
*/
20+
class UselessFunctionReturnValueRule implements Rule
21+
{
22+
23+
public function __construct(private ReflectionProvider $reflectionProvider)
24+
{
25+
}
26+
27+
public function getNodeType(): string
28+
{
29+
return FuncCall::class;
30+
}
31+
32+
public function processNode(Node $funcCall, Scope $scope): array
33+
{
34+
if (!($funcCall->name instanceof Node\Name) || $scope->isInFirstLevelStatement()) {
35+
return [];
36+
}
37+
38+
if (!$this->reflectionProvider->hasFunction($funcCall->name, $scope)) {
39+
return [];
40+
}
41+
42+
$functionReflection = $this->reflectionProvider->getFunction($funcCall->name, $scope);
43+
44+
if (!in_array($functionReflection->getName(), ['var_export', 'print_r', 'highlight_string', 'highlight_file', 'show_source'], true)) {
45+
return [];
46+
}
47+
$parametersAcceptor = ParametersAcceptorSelector::selectFromArgs(
48+
$scope,
49+
$funcCall->getArgs(),
50+
$functionReflection->getVariants(),
51+
$functionReflection->getNamedArgumentsVariants(),
52+
);
53+
54+
$reorderedFuncCall = ArgumentsNormalizer::reorderFuncArguments(
55+
$parametersAcceptor,
56+
$funcCall,
57+
);
58+
59+
if ($reorderedFuncCall === null) {
60+
return [];
61+
}
62+
$reorderedArgs = $reorderedFuncCall->getArgs();
63+
64+
if (count($reorderedArgs) === 1 || (count($reorderedArgs) >= 2 && $scope->getType($reorderedArgs[1]->value)->isFalse()->yes())) {
65+
return [RuleErrorBuilder::message(
66+
sprintf(
67+
'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.',
68+
$functionReflection->getName(),
69+
2,
70+
$parametersAcceptor->getParameters()[1]->getName(),
71+
),
72+
)
73+
->identifier('function.uselessReturnValue')
74+
->line($funcCall->getStartLine())
75+
->build(),
76+
];
77+
}
78+
79+
return [];
80+
}
81+
82+
}

‎tests/PHPStan/Levels/data/unreachable-0.json

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
[
2+
{
3+
"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.",
4+
"line": 38,
5+
"ignorable": true
6+
},
7+
{
8+
"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.",
9+
"line": 89,
10+
"ignorable": true
11+
}
12+
]

‎tests/PHPStan/Rules/Functions/UselessFunctionReturnValueRuleTest.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use PHPStan\Rules\Rule;
6+
use PHPStan\Testing\RuleTestCase;
7+
use const PHP_VERSION_ID;
8+
9+
/**
10+
* @extends RuleTestCase<UselessFunctionReturnValueRule>
11+
*/
12+
class UselessFunctionReturnValueRuleTest extends RuleTestCase
13+
{
14+
15+
protected function getRule(): Rule
16+
{
17+
return new UselessFunctionReturnValueRule(
18+
$this->createReflectionProvider(),
19+
);
20+
}
21+
22+
public function testUselessReturnValue(): void
23+
{
24+
$this->analyse([__DIR__ . '/data/useless-fn-return.php'], [
25+
[
26+
'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.',
27+
47,
28+
],
29+
[
30+
'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.',
31+
56,
32+
],
33+
[
34+
'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.',
35+
64,
36+
],
37+
[
38+
'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.',
39+
73,
40+
],
41+
]);
42+
}
43+
44+
public function testUselessReturnValuePhp8(): void
45+
{
46+
if (PHP_VERSION_ID < 80000) {
47+
$this->markTestSkipped('Test requires PHP 8.0.');
48+
}
49+
50+
$this->analyse([__DIR__ . '/data/useless-fn-return-php8.php'], [
51+
[
52+
'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.',
53+
18,
54+
],
55+
]);
56+
}
57+
58+
}

‎tests/PHPStan/Rules/Functions/data/useless-fn-return-php8.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php // lint >= 8.0
2+
3+
namespace UselessFunctionReturnPhp8;
4+
5+
class FooClass
6+
{
7+
public function explicitReturnNamed(): void
8+
{
9+
error_log("Email-Template couldn't be found by parameters:" . print_r(return: true, value: [
10+
'template' => 1,
11+
'spracheid' => 2,
12+
])
13+
);
14+
}
15+
16+
public function explicitNoReturnNamed(): void
17+
{
18+
error_log("Email-Template couldn't be found by parameters:" . print_r(return: false, value: [
19+
'template' => 1,
20+
'spracheid' => 2,
21+
])
22+
);
23+
}
24+
}

‎tests/PHPStan/Rules/Functions/data/useless-fn-return.php

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
3+
namespace UselessFunctionReturn;
4+
5+
class FooClass
6+
{
7+
public function fine(bool $bool): void
8+
{
9+
error_log(
10+
"Email-Template couldn't be found by parameters:" . print_r([
11+
'template' => 1,
12+
'spracheid' => 2,
13+
], true)
14+
);
15+
16+
$x = print_r([
17+
'template' => 1,
18+
'spracheid' => 2,
19+
], true);
20+
21+
print_r([
22+
'template' => 1,
23+
'spracheid' => 2,
24+
]);
25+
26+
error_log(
27+
"Email-Template couldn't be found by parameters:" . print_r([
28+
'template' => 1,
29+
'spracheid' => 2,
30+
], $bool)
31+
);
32+
33+
print_r([
34+
'template' => 1,
35+
'spracheid' => 2,
36+
], $bool);
37+
38+
$x = print_r([
39+
'template' => 1,
40+
'spracheid' => 2,
41+
], $bool);
42+
}
43+
44+
public function missesReturn(): void
45+
{
46+
error_log(
47+
"Email-Template couldn't be found by parameters:" . print_r([
48+
'template' => 1,
49+
'spracheid' => 2,
50+
])
51+
);
52+
}
53+
54+
public function missesReturnVarDump(): string
55+
{
56+
return "Email-Template couldn't be found by parameters:" . var_export([
57+
'template' => 1,
58+
'spracheid' => 2,
59+
]);
60+
}
61+
62+
public function explicitNoReturn(): void
63+
{
64+
error_log("Email-Template couldn't be found by parameters:" . print_r([
65+
'template' => 1,
66+
'spracheid' => 2,
67+
], false)
68+
);
69+
}
70+
71+
public function showSource(string $s): void
72+
{
73+
error_log("Email-Template couldn't be found by parameters:" . show_source($s));
74+
}
75+
76+
}

0 commit comments

Comments
 (0)