Skip to content

Commit ea53cad

Browse files
ondrejmirtesondrejmirtes
committed
Adjust NoopRule and more for pipe operator
1 parent b490e0e commit ea53cad

14 files changed

+246
-85
lines changed

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2604,7 +2604,7 @@ static function (): void {
26042604
$functionReflection->getVariants(),
26052605
$functionReflection->getNamedArgumentsVariants(),
26062606
);
2607-
$impurePoint = SimpleImpurePoint::createFromVariant($functionReflection, $parametersAcceptor);
2607+
$impurePoint = SimpleImpurePoint::createFromVariant($functionReflection, $parametersAcceptor, $scope, $expr->getArgs());
26082608
if ($impurePoint !== null) {
26092609
$impurePoints[] = new ImpurePoint($scope, $expr, $impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain());
26102610
}
@@ -2890,7 +2890,7 @@ static function (): void {
28902890
}
28912891

28922892
if ($methodReflection !== null) {
2893-
$impurePoint = SimpleImpurePoint::createFromVariant($methodReflection, $parametersAcceptor);
2893+
$impurePoint = SimpleImpurePoint::createFromVariant($methodReflection, $parametersAcceptor, $scope, $expr->getArgs());
28942894
if ($impurePoint !== null) {
28952895
$impurePoints[] = new ImpurePoint($scope, $expr, $impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain());
28962896
}
@@ -3082,7 +3082,7 @@ static function (): void {
30823082
}
30833083

30843084
if ($methodReflection !== null) {
3085-
$impurePoint = SimpleImpurePoint::createFromVariant($methodReflection, $parametersAcceptor);
3085+
$impurePoint = SimpleImpurePoint::createFromVariant($methodReflection, $parametersAcceptor, $scope, $expr->getArgs());
30863086
if ($impurePoint !== null) {
30873087
$impurePoints[] = new ImpurePoint($scope, $expr, $impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain());
30883088
}

‎src/Reflection/Callables/SimpleImpurePoint.php‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,13 @@
22

33
namespace PHPStan\Reflection\Callables;
44

5+
use PhpParser\Node\Arg;
56
use PHPStan\Analyser\ImpurePoint;
7+
use PHPStan\Analyser\Scope;
68
use PHPStan\Reflection\ExtendedMethodReflection;
79
use PHPStan\Reflection\FunctionReflection;
810
use PHPStan\Reflection\ParametersAcceptor;
11+
use PHPStan\Type\Type;
912
use function sprintf;
1013

1114
/**
@@ -14,6 +17,13 @@
1417
final class SimpleImpurePoint
1518
{
1619

20+
private const SIDE_EFFECT_FLIP_PARAMETERS = [
21+
// functionName => [name, pos, testName]
22+
'print_r' => ['return', 1, 'isTruthy'],
23+
'var_export' => ['return', 1, 'isTruthy'],
24+
'highlight_string' => ['return', 1, 'isTruthy'],
25+
];
26+
1727
/**
1828
* @param ImpurePointIdentifier $identifier
1929
*/
@@ -25,7 +35,10 @@ public function __construct(
2535
{
2636
}
2737

28-
public static function createFromVariant(FunctionReflection|ExtendedMethodReflection $function, ?ParametersAcceptor $variant): ?self
38+
/**
39+
* @param Arg[] $args
40+
*/
41+
public static function createFromVariant(FunctionReflection|ExtendedMethodReflection $function, ?ParametersAcceptor $variant, ?Scope $scope = null, array $args = []): ?self
2942
{
3043
if (!$function->hasSideEffects()->no()) {
3144
$certain = $function->isPure()->no();
@@ -34,6 +47,45 @@ public static function createFromVariant(FunctionReflection|ExtendedMethodReflec
3447
}
3548

3649
if ($function instanceof FunctionReflection) {
50+
if (isset(self::SIDE_EFFECT_FLIP_PARAMETERS[$function->getName()]) && $scope !== null) {
51+
[
52+
$flipParameterName,
53+
$flipParameterPosition,
54+
$testName,
55+
] = self::SIDE_EFFECT_FLIP_PARAMETERS[$function->getName()];
56+
57+
$sideEffectFlipped = false;
58+
$hasNamedParameter = false;
59+
$checker = [
60+
'isNotNull' => static fn (Type $type) => $type->isNull()->no(),
61+
'isTruthy' => static fn (Type $type) => $type->toBoolean()->isTrue()->yes(),
62+
][$testName];
63+
64+
foreach ($args as $i => $arg) {
65+
$isFlipParameter = false;
66+
67+
if ($arg->name !== null) {
68+
$hasNamedParameter = true;
69+
if ($arg->name->name === $flipParameterName) {
70+
$isFlipParameter = true;
71+
}
72+
}
73+
74+
if (!$hasNamedParameter && $i === $flipParameterPosition) {
75+
$isFlipParameter = true;
76+
}
77+
78+
if ($isFlipParameter) {
79+
$sideEffectFlipped = $checker($scope->getType($arg->value));
80+
break;
81+
}
82+
}
83+
84+
if ($sideEffectFlipped) {
85+
return null;
86+
}
87+
}
88+
3789
return new SimpleImpurePoint(
3890
'functionCall',
3991
sprintf('call to function %s()', $function->getName()),

‎src/Rules/DeadCode/NoopRule.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ public function processNode(Node $node, Scope $scope): array
8383
];
8484
}
8585

86+
if ($expr instanceof Node\Expr\BinaryOp\Pipe) {
87+
$expr = $expr->right;
88+
}
89+
8690
if ($expr instanceof Node\Expr\FuncCall) {
8791
if ($expr->name instanceof Node\Name) {
8892
// handled by CallToFunctionStatementWithoutSideEffectsRule

‎src/Rules/Functions/CallToFunctionStatementWithoutSideEffectsRule.php‎

Lines changed: 18 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -3,33 +3,24 @@
33
namespace PHPStan\Rules\Functions;
44

55
use PhpParser\Node;
6-
use PhpParser\Node\Arg;
76
use PHPStan\Analyser\Scope;
87
use PHPStan\DependencyInjection\RegisteredRule;
8+
use PHPStan\Node\NoopExpressionNode;
99
use PHPStan\Reflection\ReflectionProvider;
1010
use PHPStan\Rules\Rule;
1111
use PHPStan\Rules\RuleErrorBuilder;
1212
use PHPStan\Type\NeverType;
13-
use PHPStan\Type\Type;
1413
use function count;
1514
use function in_array;
1615
use function sprintf;
1716

1817
/**
19-
* @implements Rule<Node\Stmt\Expression>
18+
* @implements Rule<NoopExpressionNode>
2019
*/
2120
#[RegisteredRule(level: 4)]
2221
final class CallToFunctionStatementWithoutSideEffectsRule implements Rule
2322
{
2423

25-
private const SIDE_EFFECT_FLIP_PARAMETERS = [
26-
// functionName => [name, pos, testName]
27-
'print_r' => ['return', 1, 'isTruthy'],
28-
'var_export' => ['return', 1, 'isTruthy'],
29-
'highlight_string' => ['return', 1, 'isTruthy'],
30-
31-
];
32-
3324
public const PHPSTAN_TESTING_FUNCTIONS = [
3425
'PHPStan\\dumpNativeType',
3526
'PHPStan\\dumpType',
@@ -47,16 +38,20 @@ public function __construct(private ReflectionProvider $reflectionProvider)
4738

4839
public function getNodeType(): string
4940
{
50-
return Node\Stmt\Expression::class;
41+
return NoopExpressionNode::class;
5142
}
5243

5344
public function processNode(Node $node, Scope $scope): array
5445
{
55-
if (!$node->expr instanceof Node\Expr\FuncCall) {
46+
$expr = $node->getOriginalExpr();
47+
if ($expr instanceof Node\Expr\BinaryOp\Pipe) {
48+
$expr = $expr->right;
49+
}
50+
if (!$expr instanceof Node\Expr\FuncCall) {
5651
return [];
5752
}
5853

59-
$funcCall = $node->expr;
54+
$funcCall = $expr;
6055
if (!($funcCall->name instanceof Node\Name)) {
6156
return [];
6257
}
@@ -71,79 +66,21 @@ public function processNode(Node $node, Scope $scope): array
7166
}
7267

7368
$functionName = $function->getName();
74-
$functionHasSideEffects = !$function->hasSideEffects()->no();
75-
7669
if (in_array($functionName, self::PHPSTAN_TESTING_FUNCTIONS, true)) {
7770
return [];
7871
}
7972

80-
if (isset(self::SIDE_EFFECT_FLIP_PARAMETERS[$functionName])) {
81-
[
82-
$flipParameterName,
83-
$flipParameterPosition,
84-
$testName,
85-
] = self::SIDE_EFFECT_FLIP_PARAMETERS[$functionName];
86-
87-
$sideEffectFlipped = false;
88-
$hasNamedParameter = false;
89-
$checker = [
90-
'isNotNull' => static fn (Type $type) => $type->isNull()->no(),
91-
'isTruthy' => static fn (Type $type) => $type->toBoolean()->isTrue()->yes(),
92-
][$testName];
93-
94-
foreach ($funcCall->getRawArgs() as $i => $arg) {
95-
if (!$arg instanceof Arg) {
96-
return [];
97-
}
98-
99-
$isFlipParameter = false;
100-
101-
if ($arg->name !== null) {
102-
$hasNamedParameter = true;
103-
if ($arg->name->name === $flipParameterName) {
104-
$isFlipParameter = true;
105-
}
106-
}
107-
108-
if (!$hasNamedParameter && $i === $flipParameterPosition) {
109-
$isFlipParameter = true;
110-
}
111-
112-
if ($isFlipParameter) {
113-
$sideEffectFlipped = $checker($scope->getType($arg->value));
114-
break;
115-
}
116-
}
117-
118-
if (!$sideEffectFlipped) {
119-
return [];
120-
}
121-
122-
$functionHasSideEffects = false;
123-
}
124-
125-
if (!$functionHasSideEffects || $node->expr->isFirstClassCallable()) {
126-
if (!$node->expr->isFirstClassCallable()) {
127-
$throwsType = $function->getThrowType();
128-
if ($throwsType !== null && !$throwsType->isVoid()->yes()) {
129-
return [];
130-
}
131-
}
132-
133-
$functionResult = $scope->getType($funcCall);
134-
if ($functionResult instanceof NeverType && $functionResult->isExplicit()) {
135-
return [];
136-
}
137-
138-
return [
139-
RuleErrorBuilder::message(sprintf(
140-
'Call to function %s() on a separate line has no effect.',
141-
$function->getName(),
142-
))->identifier('function.resultUnused')->build(),
143-
];
73+
$functionResult = $scope->getType($funcCall);
74+
if ($functionResult instanceof NeverType && $functionResult->isExplicit()) {
75+
return [];
14476
}
14577

146-
return [];
78+
return [
79+
RuleErrorBuilder::message(sprintf(
80+
'Call to function %s() on a separate line has no effect.',
81+
$function->getName(),
82+
))->identifier('function.resultUnused')->build(),
83+
];
14784
}
14885

14986
}

‎src/Rules/Methods/CallToMethodStatementWithoutSideEffectsRule.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ public function getNodeType(): string
3535
public function processNode(Node $node, Scope $scope): array
3636
{
3737
$methodCall = $node->getOriginalExpr();
38+
if ($methodCall instanceof Node\Expr\BinaryOp\Pipe) {
39+
$methodCall = $methodCall->right;
40+
}
3841
if ($methodCall instanceof Node\Expr\NullsafeMethodCall) {
3942
$scope = $scope->filterByTruthyValue(new Node\Expr\BinaryOp\NotIdentical($methodCall->var, new Node\Expr\ConstFetch(new Node\Name('null'))));
4043
} elseif (!$methodCall instanceof Node\Expr\MethodCall) {

‎src/Rules/Methods/CallToStaticMethodStatementWithoutSideEffectsRule.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ public function getNodeType(): string
4141
public function processNode(Node $node, Scope $scope): array
4242
{
4343
$staticCall = $node->getOriginalExpr();
44+
if ($staticCall instanceof Node\Expr\BinaryOp\Pipe) {
45+
$staticCall = $staticCall->right;
46+
}
4447
if (!$staticCall instanceof Node\Expr\StaticCall) {
4548
return [];
4649
}

‎tests/PHPStan/Rules/DeadCode/NoopRuleTest.php‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PHPStan\Node\Printer\Printer;
77
use PHPStan\Rules\Rule;
88
use PHPStan\Testing\RuleTestCase;
9+
use PHPUnit\Framework\Attributes\RequiresPhp;
910

1011
/**
1112
* @extends RuleTestCase<NoopRule>
@@ -172,4 +173,15 @@ public function testBug13698(): void
172173
]);
173174
}
174175

176+
#[RequiresPhp('>= 8.5')]
177+
public function testPipeOperator(): void
178+
{
179+
$this->analyse([__DIR__ . '/data/noop-pipe.php'], [
180+
[
181+
'Expression "\'doFoo\'" on a separate line does not do anything.',
182+
13,
183+
],
184+
]);
185+
}
186+
175187
}

‎tests/PHPStan/Rules/DeadCode/data/noop-pipe.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php // lint >= 8.5
2+
3+
namespace NoopPipe;
4+
5+
/** @phpstan-pure */
6+
function doFoo(int $i): int
7+
{
8+
return 5;
9+
}
10+
11+
5 |> doFoo(...); // reported by a specific rule
12+
13+
5 |> 'doFoo'; // error

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPStan\Rules\Rule;
66
use PHPStan\Testing\RuleTestCase;
7+
use PHPUnit\Framework\Attributes\RequiresPhp;
78
use const PHP_VERSION_ID;
89

910
/**
@@ -118,4 +119,15 @@ public function testFirstClassCallables(): void
118119
]);
119120
}
120121

122+
#[RequiresPhp('>= 8.5')]
123+
public function testPipeOperator(): void
124+
{
125+
$this->analyse([__DIR__ . '/data/function-call-without-side-effect-pipe.php'], [
126+
[
127+
'Call to function FunctionCallWithoutSideEffectPipe\pureFunc() on a separate line has no effect.',
128+
28,
129+
],
130+
]);
131+
}
132+
121133
}

‎tests/PHPStan/Rules/Functions/data/function-call-without-side-effect-pipe.php‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php // lint >= 8.5
2+
3+
namespace FunctionCallWithoutSideEffectPipe;
4+
5+
/**
6+
* @phpstan-pure
7+
*/
8+
function pureFunc(int $i): int
9+
{
10+
return $i;
11+
}
12+
13+
function maybePureFunc(int $i): int
14+
{
15+
return $i;
16+
}
17+
18+
/**
19+
* @phpstan-impure
20+
*/
21+
function impurePureFunc(int $i): int
22+
{
23+
echo '5';
24+
return $i;
25+
}
26+
27+
function (): void {
28+
5 |> pureFunc(...);
29+
5 |> maybePureFunc(...);
30+
5 |> impurePureFunc(...);
31+
};

0 commit comments

Comments
 (0)