Skip to content

Commit ceedb51

Browse files
mvorisekmvorisek
committed
fix
1 parent 13bc953 commit ceedb51

File tree

3 files changed

+68
-79
lines changed

3 files changed

+68
-79
lines changed

‎src/StaticAnalysis/ExecutableLinesFindingVisitor.php

+54-46
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,11 @@
1515
use PhpParser\Node\Expr\Assign;
1616
use PhpParser\Node\Expr\BinaryOp;
1717
use PhpParser\Node\Expr\CallLike;
18-
use PhpParser\Node\Expr\Cast;
1918
use PhpParser\Node\Expr\Closure;
2019
use PhpParser\Node\Expr\Match_;
2120
use PhpParser\Node\Expr\MethodCall;
2221
use PhpParser\Node\Expr\NullsafePropertyFetch;
22+
use PhpParser\Node\Expr\Print_;
2323
use PhpParser\Node\Expr\PropertyFetch;
2424
use PhpParser\Node\Expr\StaticPropertyFetch;
2525
use PhpParser\Node\Expr\Ternary;
@@ -35,7 +35,6 @@
3535
use PhpParser\Node\Stmt\Echo_;
3636
use PhpParser\Node\Stmt\Else_;
3737
use PhpParser\Node\Stmt\ElseIf_;
38-
use PhpParser\Node\Stmt\Expression;
3938
use PhpParser\Node\Stmt\Finally_;
4039
use PhpParser\Node\Stmt\For_;
4140
use PhpParser\Node\Stmt\Foreach_;
@@ -67,12 +66,16 @@ final class ExecutableLinesFindingVisitor extends NodeVisitorAbstract
6766
private $propertyLines = [];
6867

6968
/**
70-
* @psalm-var array<int, Return_>
69+
* @psalm-var array<int, Return_|Assign|Array_>
7170
*/
7271
private $returns = [];
7372

7473
public function enterNode(Node $node): void
7574
{
75+
if (!$node instanceof NodeAbstract) {
76+
return;
77+
}
78+
7679
$this->savePropertyLines($node);
7780

7881
if (!$this->isExecutable($node)) {
@@ -111,17 +114,23 @@ private function savePropertyLines(Node $node): void
111114

112115
private function computeReturns(): void
113116
{
114-
foreach ($this->returns as $return) {
115-
foreach (range($return->getStartLine(), $return->getEndLine()) as $loc) {
116-
if (isset($this->executableLines[$loc])) {
117+
foreach ($this->returns as $node) {
118+
foreach (range($node->getStartLine(), $node->getEndLine()) as $index) {
119+
if (isset($this->executableLines[$index])) {
117120
continue 2;
118121
}
119122
}
120123

121-
if ($return->expr !== null) {
122-
$line = $this->getNodeStartLine($return->expr);
124+
if ($node instanceof Return_) {
125+
if ($node->expr === null) {
126+
$line = $node->getEndLine();
127+
} else {
128+
$line = $this->getNodeStartLine($node->expr);
129+
}
130+
} elseif ($node instanceof Assign) {
131+
$line = $this->getNodeStartLine($node->expr);
123132
} else {
124-
$line = $return->getEndLine();
133+
$line = $this->getNodeStartLine($node);
125134
}
126135

127136
$this->executableLines[$line] = $line;
@@ -131,33 +140,30 @@ private function computeReturns(): void
131140
/**
132141
* @return int[]
133142
*/
134-
private function getLines(Node $node): array
143+
private function getLines(NodeAbstract $node): array
135144
{
145+
if ($node instanceof Return_ || $node instanceof Assign || $node instanceof Array_) {
146+
$this->returns[] = $node;
147+
148+
return [];
149+
}
150+
136151
if ($node instanceof BinaryOp) {
137152
return [];
138153
}
139154

140-
if ($node instanceof Cast ||
141-
$node instanceof PropertyFetch ||
155+
if ($node instanceof PropertyFetch ||
142156
$node instanceof NullsafePropertyFetch ||
143157
$node instanceof StaticPropertyFetch) {
144-
return [$node->getEndLine()];
158+
return [$this->getNodeStartLine($node->name)];
145159
}
146160

147161
if ($node instanceof ArrayDimFetch) {
148162
if (null === $node->dim) {
149163
return [];
150164
}
151165

152-
return [$node->dim->getStartLine()];
153-
}
154-
155-
if ($node instanceof Array_) {
156-
if ([] === $node->items) {
157-
return [$node->getEndLine()];
158-
}
159-
160-
return [];
166+
return [$this->getNodeStartLine($node->dim)];
161167
}
162168

163169
if ($node instanceof ClassMethod) {
@@ -185,49 +191,45 @@ private function getLines(Node $node): array
185191
}
186192

187193
if ($node instanceof MethodCall) {
188-
return [$node->name->getStartLine()];
194+
return [$this->getNodeStartLine($node->name)];
189195
}
190196

191197
if ($node instanceof Ternary) {
192-
$lines = [$node->cond->getStartLine()];
198+
$lines = [$this->getNodeStartLine($node->cond)];
193199

194200
if (null !== $node->if) {
195-
$lines[] = $node->if->getStartLine();
201+
$lines[] = $this->getNodeStartLine($node->if);
196202
}
197203

198-
$lines[] = $node->else->getStartLine();
204+
$lines[] = $this->getNodeStartLine($node->else);
199205

200206
return $lines;
201207
}
202208

203209
if ($node instanceof Match_) {
204-
return [$node->cond->getStartLine()];
210+
return [$this->getNodeStartLine($node->cond)];
205211
}
206212

207213
if ($node instanceof MatchArm) {
208-
return [$node->body->getStartLine()];
214+
return [$this->getNodeStartLine($node->body)];
209215
}
210216

211-
if ($node instanceof Expression && (
212-
$node->expr instanceof Cast ||
213-
$node->expr instanceof Match_ ||
214-
$node->expr instanceof MethodCall
215-
)) {
216-
return [];
217-
}
218-
219-
if ($node instanceof Return_) {
220-
$this->returns[] = $node;
221-
222-
return [];
217+
// TODO this concept should be extended for every statement class like Foreach_
218+
if ($node instanceof If_ ||
219+
$node instanceof ElseIf_) {
220+
return [$this->getNodeStartLine($node->cond)];
223221
}
224222

225-
return [$node->getStartLine()];
223+
return [$this->getNodeStartLine($node)];
226224
}
227225

228226
private function getNodeStartLine(NodeAbstract $node): int
229227
{
230-
if ($node instanceof Node\Expr\BooleanNot) {
228+
if ($node instanceof Node\Expr\Cast ||
229+
$node instanceof Node\Expr\BooleanNot ||
230+
$node instanceof Node\Expr\UnaryMinus ||
231+
$node instanceof Node\Expr\UnaryPlus
232+
) {
231233
return $this->getNodeStartLine($node->expr);
232234
}
233235

@@ -242,19 +244,25 @@ private function getNodeStartLine(NodeAbstract $node): int
242244
return $node->getStartLine() + 1;
243245
}
244246

245-
return $node->getStartLine();
247+
if ($node instanceof Array_) {
248+
if ([] === $node->items || $node->items[0] === null) {
249+
return $node->getEndLine();
250+
}
251+
252+
return $this->getNodeStartLine($node->items[0]);
253+
}
254+
255+
return $node->getStartLine(); // $node should be only a scalar here
246256
}
247257

248258
private function isExecutable(Node $node): bool
249259
{
250260
return $node instanceof Assign ||
251261
$node instanceof ArrayDimFetch ||
252-
$node instanceof Array_ ||
253262
$node instanceof BinaryOp ||
254263
$node instanceof Break_ ||
255264
$node instanceof CallLike ||
256265
$node instanceof Case_ ||
257-
$node instanceof Cast ||
258266
$node instanceof Catch_ ||
259267
$node instanceof ClassMethod ||
260268
$node instanceof Closure ||
@@ -264,7 +272,6 @@ private function isExecutable(Node $node): bool
264272
$node instanceof ElseIf_ ||
265273
$node instanceof Else_ ||
266274
$node instanceof Encapsed ||
267-
$node instanceof Expression ||
268275
$node instanceof Finally_ ||
269276
$node instanceof For_ ||
270277
$node instanceof Foreach_ ||
@@ -274,6 +281,7 @@ private function isExecutable(Node $node): bool
274281
$node instanceof MatchArm ||
275282
$node instanceof MethodCall ||
276283
$node instanceof NullsafePropertyFetch ||
284+
$node instanceof Print_ ||
277285
$node instanceof PropertyFetch ||
278286
$node instanceof Return_ ||
279287
$node instanceof StaticPropertyFetch ||

‎tests/_files/source_with_multiline_constant_return.php

+1-3
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,7 @@ public function unaryMinusWithNotConstInTheMiddle(): string
407407

408408
public function unaryCastWithNotConstInTheMiddle(): int
409409
{
410-
return (
411-
int
412-
)
410+
return (int)
413411
(
414412
''
415413
.

‎tests/tests/Data/RawCodeCoverageDataTest.php

+13-30
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,6 @@ public function testHeavyIndentationIsHandledCorrectly(): void
331331

332332
$this->assertEquals(
333333
[
334-
9, // TODO not in xdebug output
335334
12,
336335
16,
337336
18,
@@ -340,40 +339,25 @@ public function testHeavyIndentationIsHandledCorrectly(): void
340339
25,
341340
28,
342341
31,
343-
// 36,
344-
40, // TODO not in xdebug output
342+
42,
345343
46,
346-
48, // TODO not in xdebug output
344+
50,
347345
54,
348346
60,
349-
64,
350347
71,
351-
83, // TODO not in xdebug output
352348
85,
353-
87, // TODO not in xdebug output
354349
89,
355350
91,
356351
93,
357-
95, // TODO not in xdebug output
358352
97,
359-
99, // TODO not in xdebug output
360353
101,
361-
116, // TODO not in xdebug output
362-
120, // TODO not in xdebug output
363-
123, // TODO not in xdebug output
364354
125, // This shouldn't be marked as LoC, but it's high unlikely to happen IRL to have $var = []; on multiple lines
355+
128,
365356
132,
366357
135,
367-
139, // TODO not in xdebug output
368358
143,
369359
149,
370360
153,
371-
159,
372-
373-
// xdebug lines:
374-
// 12, 16, 18, 19, 20, 21, 22, 24, 25, 26, 27, 28, 29, 31, 33, 36, 38, 42, 46,
375-
// 50, 52, 54, 60, 64, 71, 72, 73, 74, 85, 89, 91, 93, 97, 101, 103, 118, 125,
376-
// 132, 135, 143, 149, 153, 159, 162
377361
],
378362
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
379363
);
@@ -417,9 +401,9 @@ public function testReturnStatementWithOnlyAnArrayWithScalarReturnsTheFirstEleme
417401

418402
$this->assertEquals(
419403
[
420-
8, // TODO this line is correct and the only one reported by xdebug
404+
8,
421405
15,
422-
24, // TODO this line is correct and the only one reported by xdebug
406+
24,
423407
30,
424408
40,
425409
47,
@@ -481,16 +465,15 @@ public function testReturnStatementWithConstantExprOnlyReturnTheLineOfLast(): vo
481465
377,
482466
390,
483467
402,
484-
411,
485-
416,
486-
427,
487-
436,
468+
414,
469+
425,
470+
434,
471+
439,
488472
441,
489-
443,
490-
458,
491-
468,
492-
480,
493-
491,
473+
456,
474+
466,
475+
478,
476+
489,
494477
],
495478
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
496479
);

0 commit comments

Comments
 (0)