Skip to content

Commit a4fb1a2

Browse files
janedbaljanedbal
authored
UnusedVisitors: use Stmt\Expression instead of long allowlists (#353)
1 parent 5004b35 commit a4fb1a2

3 files changed

Lines changed: 24 additions & 89 deletions

File tree

‎src/Visitor/UnusedExceptionVisitor.php‎

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,10 @@
33
namespace ShipMonk\PHPStan\Visitor;
44

55
use PhpParser\Node;
6-
use PhpParser\Node\Arg;
7-
use PhpParser\Node\ArrayItem;
8-
use PhpParser\Node\Expr\Assign;
9-
use PhpParser\Node\Expr\BinaryOp\Coalesce;
10-
use PhpParser\Node\Expr\ClassConstFetch;
116
use PhpParser\Node\Expr\MethodCall;
127
use PhpParser\Node\Expr\New_;
13-
use PhpParser\Node\Expr\NullsafeMethodCall;
148
use PhpParser\Node\Expr\StaticCall;
15-
use PhpParser\Node\Expr\Ternary;
16-
use PhpParser\Node\Expr\Throw_;
17-
use PhpParser\Node\Expr\Yield_;
18-
use PhpParser\Node\MatchArm;
19-
use PhpParser\Node\Stmt\Return_;
9+
use PhpParser\Node\Stmt\Expression;
2010
use PhpParser\NodeVisitorAbstract;
2111
use function array_pop;
2212
use function end;
@@ -43,18 +33,11 @@ public function beforeTraverse(array $nodes): ?array
4333

4434
public function enterNode(Node $node): ?Node
4535
{
46-
if ($this->stack !== []) {
47-
$parent = end($this->stack);
48-
49-
if ($this->isNodeInInterest($node) && $this->isUsed($parent)) {
50-
$node->setAttribute(self::RESULT_USED, true);
51-
}
52-
}
53-
54-
if ($this->shouldBuildStack($node)) {
55-
$this->stack[] = $node;
36+
if ($this->isNodeInInterest($node) && !$this->hasUnusedResult()) {
37+
$node->setAttribute(self::RESULT_USED, true);
5638
}
5739

40+
$this->stack[] = $node;
5841
return null;
5942
}
6043

@@ -75,28 +58,10 @@ private function isNodeInInterest(Node $node): bool
7558
|| $node instanceof StaticCall;
7659
}
7760

78-
private function shouldBuildStack(Node $node): bool
79-
{
80-
return $this->stack !== [] || $this->isUsed($node);
81-
}
82-
83-
/**
84-
* Those parent nodes are marking the exception as used
85-
*/
86-
private function isUsed(Node $parent): bool
61+
private function hasUnusedResult(): bool
8762
{
88-
return $parent instanceof Assign
89-
|| $parent instanceof MethodCall
90-
|| $parent instanceof Return_
91-
|| $parent instanceof Arg
92-
|| $parent instanceof Coalesce
93-
|| $parent instanceof ArrayItem
94-
|| $parent instanceof NullsafeMethodCall
95-
|| $parent instanceof Ternary
96-
|| $parent instanceof Yield_
97-
|| $parent instanceof Throw_
98-
|| $parent instanceof ClassConstFetch
99-
|| $parent instanceof MatchArm;
63+
$parent = end($this->stack);
64+
return $parent === false || $parent instanceof Expression;
10065
}
10166

10267
}

‎src/Visitor/UnusedMatchVisitor.php‎

Lines changed: 7 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,8 @@
33
namespace ShipMonk\PHPStan\Visitor;
44

55
use PhpParser\Node;
6-
use PhpParser\Node\Arg;
7-
use PhpParser\Node\ArrayItem;
8-
use PhpParser\Node\Expr\ArrowFunction;
9-
use PhpParser\Node\Expr\Assign;
10-
use PhpParser\Node\Expr\AssignOp;
11-
use PhpParser\Node\Expr\BinaryOp\Coalesce;
126
use PhpParser\Node\Expr\Match_;
13-
use PhpParser\Node\Expr\MethodCall;
14-
use PhpParser\Node\Expr\NullsafeMethodCall;
15-
use PhpParser\Node\Expr\Ternary;
16-
use PhpParser\Node\Expr\Throw_;
17-
use PhpParser\Node\Expr\Yield_;
18-
use PhpParser\Node\Expr\YieldFrom;
19-
use PhpParser\Node\MatchArm;
20-
use PhpParser\Node\Stmt\Return_;
7+
use PhpParser\Node\Stmt\Expression;
218
use PhpParser\NodeVisitorAbstract;
229
use function array_pop;
2310
use function end;
@@ -44,18 +31,11 @@ public function beforeTraverse(array $nodes): ?array
4431

4532
public function enterNode(Node $node): ?Node
4633
{
47-
if ($this->stack !== []) {
48-
$parent = end($this->stack);
49-
50-
if ($node instanceof Match_ && $this->isUsed($parent)) {
51-
$node->setAttribute(self::MATCH_RESULT_USED, true);
52-
}
53-
}
54-
55-
if ($this->shouldBuildStack($node)) {
56-
$this->stack[] = $node;
34+
if ($node instanceof Match_ && !$this->hasUnusedResult()) {
35+
$node->setAttribute(self::MATCH_RESULT_USED, true);
5736
}
5837

38+
$this->stack[] = $node;
5939
return null;
6040
}
6141

@@ -65,30 +45,10 @@ public function leaveNode(Node $node): ?Node
6545
return null;
6646
}
6747

68-
private function shouldBuildStack(Node $node): bool
69-
{
70-
return $this->stack !== [] || $this->isUsed($node);
71-
}
72-
73-
/**
74-
* Those parent nodes are marking the match as used
75-
*/
76-
private function isUsed(Node $parent): bool
48+
private function hasUnusedResult(): bool
7749
{
78-
return $parent instanceof Throw_
79-
|| $parent instanceof Assign
80-
|| $parent instanceof AssignOp
81-
|| $parent instanceof MethodCall
82-
|| $parent instanceof Return_
83-
|| $parent instanceof Arg
84-
|| $parent instanceof Coalesce
85-
|| $parent instanceof ArrayItem
86-
|| $parent instanceof NullsafeMethodCall
87-
|| $parent instanceof Ternary
88-
|| $parent instanceof MatchArm
89-
|| $parent instanceof Yield_
90-
|| $parent instanceof YieldFrom
91-
|| $parent instanceof ArrowFunction;
50+
$parent = end($this->stack);
51+
return $parent === false || $parent instanceof Expression;
9252
}
9353

9454
}

‎tests/Rule/data/ForbidUnusedMatchResultRule/code.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,16 @@ function ($int) {
8282
1 => 'y',
8383
};
8484

85+
$c = 'The number is ' . match ($int) {
86+
0 => 'even',
87+
1 => 'odd',
88+
};
89+
90+
!match ($int) {
91+
0 => true,
92+
1 => false,
93+
};
94+
8595
return match ($bool) {
8696
false => 1,
8797
true => 2,

0 commit comments

Comments
 (0)