Skip to content

Commit

Permalink
fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@mvorisek
mvorisek committed Oct 28, 2022
1 parent 13bc953 commit ceedb51
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 79 deletions.
100 changes: 54 additions & 46 deletions src/StaticAnalysis/ExecutableLinesFindingVisitor.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@
use PhpParser\Node\Expr\Assign;
use PhpParser\Node\Expr\BinaryOp;
use PhpParser\Node\Expr\CallLike;
use PhpParser\Node\Expr\Cast;
use PhpParser\Node\Expr\Closure;
use PhpParser\Node\Expr\Match_;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\NullsafePropertyFetch;
use PhpParser\Node\Expr\Print_;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Ternary;
Expand All @@ -35,7 +35,6 @@
use PhpParser\Node\Stmt\Echo_;
use PhpParser\Node\Stmt\Else_;
use PhpParser\Node\Stmt\ElseIf_;
use PhpParser\Node\Stmt\Expression;
use PhpParser\Node\Stmt\Finally_;
use PhpParser\Node\Stmt\For_;
use PhpParser\Node\Stmt\Foreach_;
Expand Down Expand Up @@ -67,12 +66,16 @@ final class ExecutableLinesFindingVisitor extends NodeVisitorAbstract
private $propertyLines = [];

/**
* @psalm-var array<int, Return_>
* @psalm-var array<int, Return_|Assign|Array_>
*/
private $returns = [];

public function enterNode(Node $node): void
{
if (!$node instanceof NodeAbstract) {
return;
}

$this->savePropertyLines($node);

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

private function computeReturns(): void
{
foreach ($this->returns as $return) {
foreach (range($return->getStartLine(), $return->getEndLine()) as $loc) {
if (isset($this->executableLines[$loc])) {
foreach ($this->returns as $node) {
foreach (range($node->getStartLine(), $node->getEndLine()) as $index) {
if (isset($this->executableLines[$index])) {
continue 2;
}
}

if ($return->expr !== null) {
$line = $this->getNodeStartLine($return->expr);
if ($node instanceof Return_) {
if ($node->expr === null) {
$line = $node->getEndLine();
} else {
$line = $this->getNodeStartLine($node->expr);
}
} elseif ($node instanceof Assign) {
$line = $this->getNodeStartLine($node->expr);
} else {
$line = $return->getEndLine();
$line = $this->getNodeStartLine($node);
}

$this->executableLines[$line] = $line;
Expand All @@ -131,33 +140,30 @@ private function computeReturns(): void
/**
* @return int[]
*/
private function getLines(Node $node): array
private function getLines(NodeAbstract $node): array
{
if ($node instanceof Return_ || $node instanceof Assign || $node instanceof Array_) {
$this->returns[] = $node;

return [];
}

if ($node instanceof BinaryOp) {
return [];
}

if ($node instanceof Cast ||
$node instanceof PropertyFetch ||
if ($node instanceof PropertyFetch ||
$node instanceof NullsafePropertyFetch ||
$node instanceof StaticPropertyFetch) {
return [$node->getEndLine()];
return [$this->getNodeStartLine($node->name)];
}

if ($node instanceof ArrayDimFetch) {
if (null === $node->dim) {
return [];
}

return [$node->dim->getStartLine()];
}

if ($node instanceof Array_) {
if ([] === $node->items) {
return [$node->getEndLine()];
}

return [];
return [$this->getNodeStartLine($node->dim)];
}

if ($node instanceof ClassMethod) {
Expand Down Expand Up @@ -185,49 +191,45 @@ private function getLines(Node $node): array
}

if ($node instanceof MethodCall) {
return [$node->name->getStartLine()];
return [$this->getNodeStartLine($node->name)];
}

if ($node instanceof Ternary) {
$lines = [$node->cond->getStartLine()];
$lines = [$this->getNodeStartLine($node->cond)];

if (null !== $node->if) {
$lines[] = $node->if->getStartLine();
$lines[] = $this->getNodeStartLine($node->if);
}

$lines[] = $node->else->getStartLine();
$lines[] = $this->getNodeStartLine($node->else);

return $lines;
}

if ($node instanceof Match_) {
return [$node->cond->getStartLine()];
return [$this->getNodeStartLine($node->cond)];
}

if ($node instanceof MatchArm) {
return [$node->body->getStartLine()];
return [$this->getNodeStartLine($node->body)];
}

if ($node instanceof Expression && (
$node->expr instanceof Cast ||
$node->expr instanceof Match_ ||
$node->expr instanceof MethodCall
)) {
return [];
}

if ($node instanceof Return_) {
$this->returns[] = $node;

return [];
// TODO this concept should be extended for every statement class like Foreach_
if ($node instanceof If_ ||
$node instanceof ElseIf_) {
return [$this->getNodeStartLine($node->cond)];
}

return [$node->getStartLine()];
return [$this->getNodeStartLine($node)];
}

private function getNodeStartLine(NodeAbstract $node): int
{
if ($node instanceof Node\Expr\BooleanNot) {
if ($node instanceof Node\Expr\Cast ||
$node instanceof Node\Expr\BooleanNot ||
$node instanceof Node\Expr\UnaryMinus ||
$node instanceof Node\Expr\UnaryPlus
) {
return $this->getNodeStartLine($node->expr);
}

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

return $node->getStartLine();
if ($node instanceof Array_) {
if ([] === $node->items || $node->items[0] === null) {
return $node->getEndLine();
}

return $this->getNodeStartLine($node->items[0]);
}

return $node->getStartLine(); // $node should be only a scalar here
}

private function isExecutable(Node $node): bool
{
return $node instanceof Assign ||
$node instanceof ArrayDimFetch ||
$node instanceof Array_ ||
$node instanceof BinaryOp ||
$node instanceof Break_ ||
$node instanceof CallLike ||
$node instanceof Case_ ||
$node instanceof Cast ||
$node instanceof Catch_ ||
$node instanceof ClassMethod ||
$node instanceof Closure ||
Expand All @@ -264,7 +272,6 @@ private function isExecutable(Node $node): bool
$node instanceof ElseIf_ ||
$node instanceof Else_ ||
$node instanceof Encapsed ||
$node instanceof Expression ||
$node instanceof Finally_ ||
$node instanceof For_ ||
$node instanceof Foreach_ ||
Expand All @@ -274,6 +281,7 @@ private function isExecutable(Node $node): bool
$node instanceof MatchArm ||
$node instanceof MethodCall ||
$node instanceof NullsafePropertyFetch ||
$node instanceof Print_ ||
$node instanceof PropertyFetch ||
$node instanceof Return_ ||
$node instanceof StaticPropertyFetch ||
Expand Down
4 changes: 1 addition & 3 deletions tests/_files/source_with_multiline_constant_return.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,7 @@ public function unaryMinusWithNotConstInTheMiddle(): string

public function unaryCastWithNotConstInTheMiddle(): int
{
return (
int
)
return (int)
(
''
.
Expand Down
43 changes: 13 additions & 30 deletions tests/tests/Data/RawCodeCoverageDataTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,6 @@ public function testHeavyIndentationIsHandledCorrectly(): void

$this->assertEquals(
[
9, // TODO not in xdebug output
12,
16,
18,
Expand All @@ -340,40 +339,25 @@ public function testHeavyIndentationIsHandledCorrectly(): void
25,
28,
31,
// 36,
40, // TODO not in xdebug output
42,
46,
48, // TODO not in xdebug output
50,
54,
60,
64,
71,
83, // TODO not in xdebug output
85,
87, // TODO not in xdebug output
89,
91,
93,
95, // TODO not in xdebug output
97,
99, // TODO not in xdebug output
101,
116, // TODO not in xdebug output
120, // TODO not in xdebug output
123, // TODO not in xdebug output
125, // This shouldn't be marked as LoC, but it's high unlikely to happen IRL to have $var = []; on multiple lines
128,
132,
135,
139, // TODO not in xdebug output
143,
149,
153,
159,

// xdebug lines:
// 12, 16, 18, 19, 20, 21, 22, 24, 25, 26, 27, 28, 29, 31, 33, 36, 38, 42, 46,
// 50, 52, 54, 60, 64, 71, 72, 73, 74, 85, 89, 91, 93, 97, 101, 103, 118, 125,
// 132, 135, 143, 149, 153, 159, 162
],
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
);
Expand Down Expand Up @@ -417,9 +401,9 @@ public function testReturnStatementWithOnlyAnArrayWithScalarReturnsTheFirstEleme

$this->assertEquals(
[
8, // TODO this line is correct and the only one reported by xdebug
8,
15,
24, // TODO this line is correct and the only one reported by xdebug
24,
30,
40,
47,
Expand Down Expand Up @@ -481,16 +465,15 @@ public function testReturnStatementWithConstantExprOnlyReturnTheLineOfLast(): vo
377,
390,
402,
411,
416,
427,
436,
414,
425,
434,
439,
441,
443,
458,
468,
480,
491,
456,
466,
478,
489,
],
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
);
Expand Down

0 comments on commit ceedb51

Please sign in to comment.