Skip to content

Commit a647052

Browse files
ondrejmirtesondrejmirtes
committed
Bleeding edge - NoopRule - take advantage of impure points
1 parent d7579c4 commit a647052

13 files changed

+360
-101
lines changed

Diff for: ‎conf/bleedingEdge.neon

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ parameters:
3030
alwaysCheckTooWideReturnTypeFinalMethods: true
3131
duplicateStubs: true
3232
logicalXor: true
33+
betterNoop: true
3334
invarianceComposition: true
3435
alwaysTrueAlwaysReported: true
3536
disableUnreachableBranchesRules: true

Diff for: ‎conf/config.level4.neon

+6-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ conditionalTags:
2727
phpstan.rules.rule: %featureToggles.notAnalysedTrait%
2828
PHPStan\Rules\Comparison\LogicalXorConstantConditionRule:
2929
phpstan.rules.rule: %featureToggles.logicalXor%
30+
PHPStan\Rules\DeadCode\BetterNoopRule:
31+
phpstan.rules.rule: %featureToggles.betterNoop%
3032
PHPStan\Rules\TooWideTypehints\TooWideFunctionParameterOutTypeRule:
3133
phpstan.rules.rule: %featureToggles.paramOutType%
3234
PHPStan\Rules\TooWideTypehints\TooWideMethodParameterOutTypeRule:
@@ -74,7 +76,7 @@ services:
7476
-
7577
class: PHPStan\Rules\DeadCode\NoopRule
7678
arguments:
77-
logicalXor: %featureToggles.logicalXor%
79+
better: %featureToggles.betterNoop%
7880
tags:
7981
- phpstan.rules.rule
8082

@@ -142,6 +144,9 @@ services:
142144
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
143145
reportAlwaysTrueInLastCondition: %reportAlwaysTrueInLastCondition%
144146

147+
-
148+
class: PHPStan\Rules\DeadCode\BetterNoopRule
149+
145150
-
146151
class: PHPStan\Rules\Comparison\MatchExpressionRule
147152
arguments:

Diff for: ‎conf/config.neon

+1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ parameters:
6565
alwaysCheckTooWideReturnTypeFinalMethods: false
6666
duplicateStubs: false
6767
logicalXor: false
68+
betterNoop: false
6869
invarianceComposition: false
6970
alwaysTrueAlwaysReported: false
7071
disableUnreachableBranchesRules: false

Diff for: ‎conf/parametersSchema.neon

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ parametersSchema:
6060
alwaysCheckTooWideReturnTypeFinalMethods: bool()
6161
duplicateStubs: bool()
6262
logicalXor: bool()
63+
betterNoop: bool()
6364
invarianceComposition: bool()
6465
alwaysTrueAlwaysReported: bool()
6566
disableUnreachableBranchesRules: bool()

Diff for: ‎src/Analyser/ImpurePoint.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use PHPStan\Node\VirtualNode;
77

88
/**
9-
* @phpstan-type ImpurePointIdentifier = 'echo'|'die'|'exit'|'propertyAssign'|'methodCall'|'new'|'functionCall'|'include'|'require'|'print'|'eval'|'superglobal'
9+
* @phpstan-type ImpurePointIdentifier = 'echo'|'die'|'exit'|'propertyAssign'|'methodCall'|'new'|'functionCall'|'include'|'require'|'print'|'eval'|'superglobal'|'yield'|'yieldFrom'
1010
* @api
1111
*/
1212
class ImpurePoint

Diff for: ‎src/Analyser/NodeScopeResolver.php

+29-2
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@
106106
use PHPStan\Node\MatchExpressionNode;
107107
use PHPStan\Node\MethodCallableNode;
108108
use PHPStan\Node\MethodReturnStatementsNode;
109+
use PHPStan\Node\NoopExpressionNode;
109110
use PHPStan\Node\PropertyAssignNode;
110111
use PHPStan\Node\ReturnStatement;
111112
use PHPStan\Node\StaticMethodCallableNode;
@@ -746,6 +747,17 @@ private function processStmtNode(
746747
} elseif ($stmt instanceof Node\Stmt\Expression) {
747748
$earlyTerminationExpr = $this->findEarlyTerminatingExpr($stmt->expr, $scope);
748749
$result = $this->processExprNode($stmt, $stmt->expr, $scope, $nodeCallback, ExpressionContext::createTopLevel());
750+
$throwPoints = array_filter($result->getThrowPoints(), static fn ($throwPoint) => $throwPoint->isExplicit());
751+
if (
752+
count($result->getImpurePoints()) === 0
753+
&& count($throwPoints) === 0
754+
&& !$stmt->expr instanceof Expr\PostInc
755+
&& !$stmt->expr instanceof Expr\PreInc
756+
&& !$stmt->expr instanceof Expr\PostDec
757+
&& !$stmt->expr instanceof Expr\PreDec
758+
) {
759+
$nodeCallback(new NoopExpressionNode($stmt->expr), $scope);
760+
}
749761
$scope = $result->getScope();
750762
$scope = $scope->filterBySpecifiedTypes($this->typeSpecifier->specifyTypesInCondition(
751763
$scope,
@@ -2991,6 +3003,13 @@ static function (): void {
29913003
$throwPoints = $result->getThrowPoints();
29923004
$throwPoints[] = ThrowPoint::createImplicit($scope, $expr);
29933005
$impurePoints = $result->getImpurePoints();
3006+
$impurePoints[] = new ImpurePoint(
3007+
$scope,
3008+
$expr,
3009+
'yieldFrom',
3010+
'yield from',
3011+
true,
3012+
);
29943013
$hasYield = true;
29953014

29963015
$scope = $result->getScope();
@@ -3226,12 +3245,20 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
32263245
$throwPoints = [
32273246
ThrowPoint::createImplicit($scope, $expr),
32283247
];
3229-
$impurePoints = [];
3248+
$impurePoints = [
3249+
new ImpurePoint(
3250+
$scope,
3251+
$expr,
3252+
'yield',
3253+
'yield',
3254+
true,
3255+
),
3256+
];
32303257
if ($expr->key !== null) {
32313258
$keyResult = $this->processExprNode($stmt, $expr->key, $scope, $nodeCallback, $context->enterDeep());
32323259
$scope = $keyResult->getScope();
32333260
$throwPoints = $keyResult->getThrowPoints();
3234-
$impurePoints = $keyResult->getImpurePoints();
3261+
$impurePoints = array_merge($impurePoints, $keyResult->getImpurePoints());
32353262
}
32363263
if ($expr->value !== null) {
32373264
$valueResult = $this->processExprNode($stmt, $expr->value, $scope, $nodeCallback, $context->enterDeep());

Diff for: ‎src/Node/NoopExpressionNode.php

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Node;
4+
5+
use PhpParser\Node\Expr;
6+
use PhpParser\NodeAbstract;
7+
8+
class NoopExpressionNode extends NodeAbstract implements VirtualNode
9+
{
10+
11+
public function __construct(private Expr $originalExpr)
12+
{
13+
parent::__construct($this->originalExpr->getAttributes());
14+
}
15+
16+
public function getOriginalExpr(): Expr
17+
{
18+
return $this->originalExpr;
19+
}
20+
21+
public function getType(): string
22+
{
23+
return 'PHPStan_Node_NoopExpressionNode';
24+
}
25+
26+
/**
27+
* @return string[]
28+
*/
29+
public function getSubNodeNames(): array
30+
{
31+
return [];
32+
}
33+
34+
}

Diff for: ‎src/Rules/DeadCode/BetterNoopRule.php

+110
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\DeadCode;
4+
5+
use PhpParser\Node;
6+
use PHPStan\Analyser\Scope;
7+
use PHPStan\Node\NoopExpressionNode;
8+
use PHPStan\Node\Printer\ExprPrinter;
9+
use PHPStan\Rules\Rule;
10+
use PHPStan\Rules\RuleErrorBuilder;
11+
use function sprintf;
12+
13+
/**
14+
* @implements Rule<NoopExpressionNode>
15+
*/
16+
class BetterNoopRule implements Rule
17+
{
18+
19+
public function __construct(private ExprPrinter $exprPrinter)
20+
{
21+
}
22+
23+
public function getNodeType(): string
24+
{
25+
return NoopExpressionNode::class;
26+
}
27+
28+
public function processNode(Node $node, Scope $scope): array
29+
{
30+
$expr = $node->getOriginalExpr();
31+
if ($expr instanceof Node\Expr\BinaryOp\LogicalXor) {
32+
return [
33+
RuleErrorBuilder::message(
34+
'Unused result of "xor" operator.',
35+
)->line($expr->getStartLine())
36+
->tip('This operator has unexpected precedence, try disambiguating the logic with parentheses ().')
37+
->identifier('logicalXor.resultUnused')
38+
->build(),
39+
];
40+
}
41+
if ($expr instanceof Node\Expr\BinaryOp\LogicalAnd || $expr instanceof Node\Expr\BinaryOp\LogicalOr) {
42+
$identifierType = $expr instanceof Node\Expr\BinaryOp\LogicalAnd ? 'logicalAnd' : 'logicalOr';
43+
44+
return [
45+
RuleErrorBuilder::message(sprintf(
46+
'Unused result of "%s" operator.',
47+
$expr->getOperatorSigil(),
48+
))->line($expr->getStartLine())
49+
->tip('This operator has unexpected precedence, try disambiguating the logic with parentheses ().')
50+
->identifier(sprintf('%s.resultUnused', $identifierType))
51+
->build(),
52+
];
53+
}
54+
55+
if ($expr instanceof Node\Expr\BinaryOp\BooleanAnd || $expr instanceof Node\Expr\BinaryOp\BooleanOr) {
56+
$identifierType = $expr instanceof Node\Expr\BinaryOp\BooleanAnd ? 'booleanAnd' : 'booleanOr';
57+
58+
return [
59+
RuleErrorBuilder::message(sprintf(
60+
'Unused result of "%s" operator.',
61+
$expr->getOperatorSigil(),
62+
))->line($expr->getStartLine())
63+
->identifier(sprintf('%s.resultUnused', $identifierType))
64+
->build(),
65+
];
66+
}
67+
68+
if ($expr instanceof Node\Expr\Ternary) {
69+
return [
70+
RuleErrorBuilder::message('Unused result of ternary operator.')
71+
->line($expr->getStartLine())
72+
->identifier('ternary.resultUnused')
73+
->build(),
74+
];
75+
}
76+
77+
if (
78+
$expr instanceof Node\Expr\FuncCall
79+
|| $expr instanceof Node\Expr\NullsafeMethodCall
80+
|| $expr instanceof Node\Expr\MethodCall
81+
|| $expr instanceof Node\Expr\New_
82+
|| $expr instanceof Node\Expr\StaticCall
83+
) {
84+
// handled by *WithoutSideEffectsRule rules
85+
return [];
86+
}
87+
88+
if (
89+
$expr instanceof Node\Expr\Assign
90+
|| $expr instanceof Node\Expr\AssignOp
91+
|| $expr instanceof Node\Expr\AssignRef
92+
) {
93+
return [];
94+
}
95+
96+
if ($expr instanceof Node\Expr\Closure) {
97+
return [];
98+
}
99+
100+
return [
101+
RuleErrorBuilder::message(sprintf(
102+
'Expression "%s" on a separate line does not do anything.',
103+
$this->exprPrinter->printExpr($expr),
104+
))->line($expr->getStartLine())
105+
->identifier('expr.resultUnused')
106+
->build(),
107+
];
108+
}
109+
110+
}

Diff for: ‎src/Rules/DeadCode/NoopRule.php

+5-64
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
class NoopRule implements Rule
1616
{
1717

18-
public function __construct(private ExprPrinter $exprPrinter, private bool $logicalXor)
18+
public function __construct(private ExprPrinter $exprPrinter, private bool $better)
1919
{
2020
}
2121

@@ -26,6 +26,10 @@ public function getNodeType(): string
2626

2727
public function processNode(Node $node, Scope $scope): array
2828
{
29+
if ($this->better) {
30+
// disabled in bleeding edge
31+
return [];
32+
}
2933
$originalExpr = $node->expr;
3034
$expr = $originalExpr;
3135
if (
@@ -36,70 +40,7 @@ public function processNode(Node $node, Scope $scope): array
3640
) {
3741
$expr = $expr->expr;
3842
}
39-
if ($this->logicalXor) {
40-
if ($expr instanceof Node\Expr\BinaryOp\LogicalXor) {
41-
return [
42-
RuleErrorBuilder::message(
43-
'Unused result of "xor" operator.',
44-
)->line($expr->getStartLine())
45-
->tip('This operator has unexpected precedence, try disambiguating the logic with parentheses ().')
46-
->identifier('logicalXor.resultUnused')
47-
->build(),
48-
];
49-
}
50-
if ($expr instanceof Node\Expr\BinaryOp\LogicalAnd || $expr instanceof Node\Expr\BinaryOp\LogicalOr) {
51-
if (!$this->isNoopExpr($expr->right)) {
52-
return [];
53-
}
54-
55-
$identifierType = $expr instanceof Node\Expr\BinaryOp\LogicalAnd ? 'logicalAnd' : 'logicalOr';
56-
57-
return [
58-
RuleErrorBuilder::message(sprintf(
59-
'Unused result of "%s" operator.',
60-
$expr->getOperatorSigil(),
61-
))->line($expr->getStartLine())
62-
->tip('This operator has unexpected precedence, try disambiguating the logic with parentheses ().')
63-
->identifier(sprintf('%s.resultUnused', $identifierType))
64-
->build(),
65-
];
66-
}
67-
68-
if ($expr instanceof Node\Expr\BinaryOp\BooleanAnd || $expr instanceof Node\Expr\BinaryOp\BooleanOr) {
69-
if (!$this->isNoopExpr($expr->right)) {
70-
return [];
71-
}
7243

73-
$identifierType = $expr instanceof Node\Expr\BinaryOp\BooleanAnd ? 'booleanAnd' : 'booleanOr';
74-
75-
return [
76-
RuleErrorBuilder::message(sprintf(
77-
'Unused result of "%s" operator.',
78-
$expr->getOperatorSigil(),
79-
))->line($expr->getStartLine())
80-
->identifier(sprintf('%s.resultUnused', $identifierType))
81-
->build(),
82-
];
83-
}
84-
85-
if ($expr instanceof Node\Expr\Ternary) {
86-
$if = $expr->if;
87-
if ($if === null) {
88-
$if = $expr->cond;
89-
}
90-
91-
if (!$this->isNoopExpr($if) || !$this->isNoopExpr($expr->else)) {
92-
return [];
93-
}
94-
95-
return [
96-
RuleErrorBuilder::message('Unused result of ternary operator.')
97-
->line($expr->getStartLine())
98-
->identifier('ternary.resultUnused')
99-
->build(),
100-
];
101-
}
102-
}
10344
if (!$this->isNoopExpr($expr)) {
10445
return [];
10546
}

Diff for: ‎tests/PHPStan/Analyser/data/conditional-expression-infinite-loop.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class test
55
{
66
public function test2(bool $isFoo, bool $isBar): void
77
{
8-
match (true) {
8+
$a = match (true) {
99
$isFoo && $isBar => $foo = 1,
1010
$isFoo || $isBar => $foo = 2,
1111
default => $foo = null,

0 commit comments

Comments
 (0)