Skip to content

Commit 2bda882

Browse files
mvorisekSlamdunk
authored and
sebastianbergmann
committed
Fix executable lines analysis
Co-authored-by: Filippo Tessarotto <zoeslam@gmail.com>
1 parent 858b0f2 commit 2bda882

File tree

5 files changed

+714
-122
lines changed

5 files changed

+714
-122
lines changed

‎src/StaticAnalysis/ExecutableLinesFindingVisitor.php‎

Lines changed: 150 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@
1212
use PhpParser\Node;
1313
use PhpParser\Node\Expr\Array_;
1414
use PhpParser\Node\Expr\ArrayDimFetch;
15-
use PhpParser\Node\Expr\ArrayItem;
1615
use PhpParser\Node\Expr\Assign;
1716
use PhpParser\Node\Expr\BinaryOp;
1817
use PhpParser\Node\Expr\CallLike;
19-
use PhpParser\Node\Expr\Cast;
2018
use PhpParser\Node\Expr\Closure;
2119
use PhpParser\Node\Expr\Match_;
2220
use PhpParser\Node\Expr\MethodCall;
2321
use PhpParser\Node\Expr\NullsafePropertyFetch;
22+
use PhpParser\Node\Expr\Print_;
2423
use PhpParser\Node\Expr\PropertyFetch;
2524
use PhpParser\Node\Expr\StaticPropertyFetch;
2625
use PhpParser\Node\Expr\Ternary;
@@ -37,18 +36,17 @@
3736
use PhpParser\Node\Stmt\Else_;
3837
use PhpParser\Node\Stmt\ElseIf_;
3938
use PhpParser\Node\Stmt\Expression;
40-
use PhpParser\Node\Stmt\Finally_;
4139
use PhpParser\Node\Stmt\For_;
4240
use PhpParser\Node\Stmt\Foreach_;
41+
use PhpParser\Node\Stmt\Function_;
4342
use PhpParser\Node\Stmt\Goto_;
4443
use PhpParser\Node\Stmt\If_;
4544
use PhpParser\Node\Stmt\Property;
4645
use PhpParser\Node\Stmt\Return_;
47-
use PhpParser\Node\Stmt\Switch_;
4846
use PhpParser\Node\Stmt\Throw_;
49-
use PhpParser\Node\Stmt\TryCatch;
5047
use PhpParser\Node\Stmt\Unset_;
5148
use PhpParser\Node\Stmt\While_;
49+
use PhpParser\NodeAbstract;
5250
use PhpParser\NodeVisitorAbstract;
5351

5452
/**
@@ -67,19 +65,23 @@ final class ExecutableLinesFindingVisitor extends NodeVisitorAbstract
6765
private $propertyLines = [];
6866

6967
/**
70-
* @psalm-var array<int, Return_>
68+
* @psalm-var array<int, Function_|ClassMethod|Return_|Expression|Assign|Array_>
7169
*/
7270
private $returns = [];
7371

7472
public function enterNode(Node $node): void
7573
{
74+
if (!$node instanceof NodeAbstract) {
75+
return;
76+
}
77+
7678
$this->savePropertyLines($node);
7779

7880
if (!$this->isExecutable($node)) {
7981
return;
8082
}
8183

82-
foreach ($this->getLines($node) as $line) {
84+
foreach ($this->getLines($node, false) as $line) {
8385
if (isset($this->propertyLines[$line])) {
8486
return;
8587
}
@@ -88,170 +90,225 @@ public function enterNode(Node $node): void
8890
}
8991
}
9092

91-
/**
92-
* @psalm-return array<int, int>
93-
*/
94-
public function executableLines(): array
93+
public function afterTraverse(array $nodes): void
9594
{
9695
$this->computeReturns();
9796

9897
sort($this->executableLines);
98+
}
9999

100+
/**
101+
* @psalm-return array<int, int>
102+
*/
103+
public function executableLines(): array
104+
{
100105
return $this->executableLines;
101106
}
102107

103108
private function savePropertyLines(Node $node): void
104109
{
105-
if (!$node instanceof Property && !$node instanceof Node\Stmt\ClassConst) {
106-
return;
107-
}
108-
109-
foreach (range($node->getStartLine(), $node->getEndLine()) as $index) {
110-
$this->propertyLines[$index] = $index;
110+
if ($node instanceof Property) {
111+
foreach (range($node->getStartLine(), $node->getEndLine()) as $index) {
112+
$this->propertyLines[$index] = $index;
113+
}
111114
}
112115
}
113116

114117
private function computeReturns(): void
115118
{
116-
foreach ($this->returns as $return) {
117-
foreach (range($return->getStartLine(), $return->getEndLine()) as $loc) {
118-
if (isset($this->executableLines[$loc])) {
119-
continue 2;
119+
foreach (array_reverse($this->returns) as $node) {
120+
foreach (range($node->getStartLine(), $node->getEndLine()) as $index) {
121+
if (isset($this->executableLines[$index])) {
122+
continue;
120123
}
121124
}
122125

123-
$line = $return->getEndLine();
124-
125-
if ($return->expr !== null) {
126-
$line = $return->expr->getStartLine();
126+
foreach ($this->getLines($node, true) as $line) {
127+
$this->executableLines[$line] = $line;
127128
}
128-
129-
$this->executableLines[$line] = $line;
130129
}
131130
}
132131

133132
/**
134133
* @return int[]
135134
*/
136-
private function getLines(Node $node): array
135+
private function getLines(NodeAbstract $node, bool $fromReturns): array
137136
{
138-
if ($node instanceof BinaryOp) {
139-
if (($node->left instanceof Node\Scalar ||
140-
$node->left instanceof Node\Expr\ConstFetch) &&
141-
($node->right instanceof Node\Scalar ||
142-
$node->right instanceof Node\Expr\ConstFetch)) {
143-
return [$node->right->getStartLine()];
144-
}
145-
146-
return [];
147-
}
148-
149-
if ($node instanceof Cast ||
150-
$node instanceof PropertyFetch ||
151-
$node instanceof NullsafePropertyFetch ||
152-
$node instanceof StaticPropertyFetch) {
153-
return [$node->getEndLine()];
154-
}
137+
if ($node instanceof Function_ ||
138+
$node instanceof ClassMethod ||
139+
$node instanceof Return_ ||
140+
$node instanceof Expression ||
141+
$node instanceof Assign ||
142+
$node instanceof Array_
143+
) {
144+
if (!$fromReturns) {
145+
$this->returns[] = $node;
146+
147+
if ($node instanceof ClassMethod && $node->name->name === '__construct') {
148+
$existsAPromotedProperty = false;
149+
150+
foreach ($node->getParams() as $param) {
151+
if (0 !== ($param->flags & Class_::VISIBILITY_MODIFIER_MASK)) {
152+
$existsAPromotedProperty = true;
153+
154+
break;
155+
}
156+
}
157+
158+
if ($existsAPromotedProperty) {
159+
// Only the line with `function` keyword should be listed here
160+
// but `nikic/php-parser` doesn't provide a way to fetch it
161+
return range($node->getStartLine(), $node->name->getEndLine());
162+
}
163+
}
155164

156-
if ($node instanceof ArrayDimFetch) {
157-
if (null === $node->dim) {
158165
return [];
159166
}
160167

161-
return [$node->dim->getStartLine()];
162-
}
163-
164-
if ($node instanceof Array_) {
165-
$startLine = $node->getStartLine();
166-
167-
if (isset($this->executableLines[$startLine])) {
168-
return [];
168+
// ugly fix for non-fully AST based processing
169+
// self::afterTraverse()/self::computeReturns() should be rewritten using self::leaveNode()
170+
foreach (range($node->getStartLine(), $node->getEndLine()) as $index) {
171+
if (isset($this->executableLines[$index]) && !($node instanceof Assign)) {
172+
return [];
173+
}
169174
}
170175

171-
if ([] === $node->items) {
176+
// empty function
177+
if ($node instanceof Function_) {
172178
return [$node->getEndLine()];
173179
}
174180

175-
if ($node->items[0] instanceof ArrayItem) {
176-
return [$node->items[0]->getStartLine()];
181+
// empty method
182+
if ($node instanceof ClassMethod) {
183+
if (null === $node->stmts) { // method without body (interface prototype)
184+
return [];
185+
}
186+
187+
return [$node->getEndLine()];
177188
}
178189
}
179190

180-
if ($node instanceof ClassMethod) {
181-
if ($node->name->name !== '__construct') {
182-
return [];
191+
if ($node instanceof Return_) {
192+
if ($node->expr === null) {
193+
return [$node->getEndLine()];
183194
}
184195

185-
$existsAPromotedProperty = false;
196+
return $this->getLines($node->expr, $fromReturns);
197+
}
186198

187-
foreach ($node->getParams() as $param) {
188-
if (0 !== ($param->flags & Class_::VISIBILITY_MODIFIER_MASK)) {
189-
$existsAPromotedProperty = true;
199+
if ($node instanceof Expression) {
200+
return $this->getLines($node->expr, $fromReturns);
201+
}
190202

191-
break;
192-
}
193-
}
203+
if ($node instanceof Assign) {
204+
return [$this->getNodeStartLine($node->var)];
205+
}
194206

195-
if ($existsAPromotedProperty) {
196-
// Only the line with `function` keyword should be listed here
197-
// but `nikic/php-parser` doesn't provide a way to fetch it
198-
return range($node->getStartLine(), $node->name->getEndLine());
199-
}
207+
if ($node instanceof BinaryOp) {
208+
return $fromReturns ? $this->getLines($node->right, $fromReturns) : [];
209+
}
210+
211+
if ($node instanceof PropertyFetch ||
212+
$node instanceof NullsafePropertyFetch ||
213+
$node instanceof StaticPropertyFetch) {
214+
return [$this->getNodeStartLine($node->name)];
215+
}
200216

201-
return [];
217+
if ($node instanceof ArrayDimFetch && null !== $node->dim) {
218+
return [$this->getNodeStartLine($node->dim)];
202219
}
203220

204221
if ($node instanceof MethodCall) {
205-
return [$node->name->getStartLine()];
222+
return [$this->getNodeStartLine($node->name)];
206223
}
207224

208225
if ($node instanceof Ternary) {
209-
$lines = [$node->cond->getStartLine()];
226+
$lines = [$this->getNodeStartLine($node->cond)];
210227

211228
if (null !== $node->if) {
212-
$lines[] = $node->if->getStartLine();
229+
$lines[] = $this->getNodeStartLine($node->if);
213230
}
214231

215-
$lines[] = $node->else->getStartLine();
232+
$lines[] = $this->getNodeStartLine($node->else);
216233

217234
return $lines;
218235
}
219236

220237
if ($node instanceof Match_) {
221-
return [$node->cond->getStartLine()];
238+
return [$this->getNodeStartLine($node->cond)];
222239
}
223240

224241
if ($node instanceof MatchArm) {
225-
return [$node->body->getStartLine()];
242+
return [$this->getNodeStartLine($node->body)];
226243
}
227244

228-
if ($node instanceof Expression && (
229-
$node->expr instanceof Cast ||
230-
$node->expr instanceof Match_ ||
231-
$node->expr instanceof MethodCall
245+
// TODO this concept should be extended for every statement class like Foreach_, For_, ...
246+
if ($node instanceof If_ ||
247+
$node instanceof ElseIf_ ||
248+
$node instanceof While_ ||
249+
$node instanceof Do_) {
250+
return [$this->getNodeStartLine($node->cond)];
251+
}
252+
253+
if ($node instanceof Case_) {
254+
if (null === $node->cond) { // default case
255+
return [];
256+
}
257+
258+
return [$this->getNodeStartLine($node->cond)];
259+
}
260+
261+
if ($node instanceof Catch_) {
262+
return [$this->getNodeStartLine($node->types[0])];
263+
}
264+
265+
return [$this->getNodeStartLine($node)];
266+
}
267+
268+
private function getNodeStartLine(NodeAbstract $node): int
269+
{
270+
if ($node instanceof Node\Expr\Cast ||
271+
$node instanceof Node\Expr\BooleanNot ||
272+
$node instanceof Node\Expr\UnaryMinus ||
273+
$node instanceof Node\Expr\UnaryPlus
274+
) {
275+
return $this->getNodeStartLine($node->expr);
276+
}
277+
278+
if ($node instanceof BinaryOp) {
279+
return $this->getNodeStartLine($node->right);
280+
}
281+
282+
if ($node instanceof Node\Scalar\String_ && (
283+
$node->getAttribute('kind') === Node\Scalar\String_::KIND_HEREDOC ||
284+
$node->getAttribute('kind') === Node\Scalar\String_::KIND_NOWDOC
232285
)) {
233-
return [];
286+
return $node->getStartLine() + 1;
234287
}
235288

236-
if ($node instanceof Return_) {
237-
$this->returns[] = $node;
289+
if ($node instanceof Array_) {
290+
if ([] === $node->items || $node->items[0] === null) {
291+
return $node->getEndLine();
292+
}
293+
294+
return $this->getNodeStartLine($node->items[0]->value);
295+
}
238296

239-
return [];
297+
if ($node instanceof Assign) {
298+
return $this->getNodeStartLine($node->expr);
240299
}
241300

242-
return [$node->getStartLine()];
301+
return $node->getStartLine(); // $node should be only a scalar here
243302
}
244303

245304
private function isExecutable(Node $node): bool
246305
{
247306
return $node instanceof Assign ||
248307
$node instanceof ArrayDimFetch ||
249-
$node instanceof Array_ ||
250308
$node instanceof BinaryOp ||
251309
$node instanceof Break_ ||
252310
$node instanceof CallLike ||
253311
$node instanceof Case_ ||
254-
$node instanceof Cast ||
255312
$node instanceof Catch_ ||
256313
$node instanceof ClassMethod ||
257314
$node instanceof Closure ||
@@ -262,22 +319,21 @@ private function isExecutable(Node $node): bool
262319
$node instanceof Else_ ||
263320
$node instanceof Encapsed ||
264321
$node instanceof Expression ||
265-
$node instanceof Finally_ ||
266322
$node instanceof For_ ||
267323
$node instanceof Foreach_ ||
324+
$node instanceof Function_ ||
268325
$node instanceof Goto_ ||
269326
$node instanceof If_ ||
270327
$node instanceof Match_ ||
271328
$node instanceof MatchArm ||
272329
$node instanceof MethodCall ||
273330
$node instanceof NullsafePropertyFetch ||
331+
$node instanceof Print_ ||
274332
$node instanceof PropertyFetch ||
275333
$node instanceof Return_ ||
276334
$node instanceof StaticPropertyFetch ||
277-
$node instanceof Switch_ ||
278335
$node instanceof Ternary ||
279336
$node instanceof Throw_ ||
280-
$node instanceof TryCatch ||
281337
$node instanceof Unset_ ||
282338
$node instanceof While_;
283339
}

0 commit comments

Comments
 (0)