Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Executable lines: fix empty constructor and match expression regressions #906

Merged
merged 3 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions src/StaticAnalysis/ExecutableLinesFindingVisitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,19 @@
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\PropertyFetch;
use PhpParser\Node\Expr\StaticPropertyFetch;
use PhpParser\Node\Expr\Ternary;
use PhpParser\Node\MatchArm;
use PhpParser\Node\Scalar\Encapsed;
use PhpParser\Node\Stmt\Break_;
use PhpParser\Node\Stmt\Case_;
use PhpParser\Node\Stmt\Catch_;
use PhpParser\Node\Stmt\Class_;
use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\Node\Stmt\Continue_;
use PhpParser\Node\Stmt\Do_;
use PhpParser\Node\Stmt\Echo_;
Expand Down Expand Up @@ -118,6 +122,30 @@ private function getLines(Node $node): array
return [$node->dim->getStartLine()];
}

if ($node instanceof ClassMethod) {
if ($node->name->name !== '__construct') {
return [];
}

$existsAPromotedProperty = false;

foreach ($node->getParams() as $param) {
if (0 !== ($param->flags & Class_::VISIBILITY_MODIFIER_MASK)) {
$existsAPromotedProperty = true;

break;
}
}

if ($existsAPromotedProperty) {
// Only the line with `function` keyword should be listed here
// but `nikic/php-parser` doesn't provide a way to fetch it
return range($node->getStartLine(), $node->name->getEndLine());
}

return [];
}

if ($node instanceof MethodCall) {
return [$node->name->getStartLine()];
}
Expand All @@ -134,6 +162,22 @@ private function getLines(Node $node): array
return $lines;
}

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

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

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

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

Expand All @@ -147,6 +191,7 @@ private function isExecutable(Node $node): bool
$node instanceof Case_ ||
$node instanceof Cast ||
$node instanceof Catch_ ||
$node instanceof ClassMethod ||
$node instanceof Closure ||
$node instanceof Continue_ ||
$node instanceof Do_ ||
Expand All @@ -160,6 +205,8 @@ private function isExecutable(Node $node): bool
$node instanceof Foreach_ ||
$node instanceof Goto_ ||
$node instanceof If_ ||
$node instanceof Match_ ||
$node instanceof MatchArm ||
$node instanceof MethodCall ||
$node instanceof NullsafePropertyFetch ||
$node instanceof PropertyFetch ||
Expand Down
32 changes: 32 additions & 0 deletions tests/_files/source_with_empty_constructor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

class Foo
{
public
function
__construct
(
public
$var
=
1
)
{

}
}

class Bar
{
public
function
__construct
(
$var
=
1
)
{
$var = 1;
}
}
3 changes: 0 additions & 3 deletions tests/_files/source_with_heavy_indentation.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,6 @@ function
)
;

$var[]
;

(string)
$var
;
Expand Down
30 changes: 30 additions & 0 deletions tests/_files/source_with_match_expression.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

class Foo
{
public
function
__construct(
string
$bar
)
{
match
(
$bar
)
{
'a',
'b'
=>
1
,
// Some comments
default
=>
throw new Exception()
,
}
;
}
}
37 changes: 33 additions & 4 deletions tests/tests/Data/RawCodeCoverageDataTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -366,12 +366,41 @@ public function testHeavyIndentationIsHandledCorrectly(): void
135,
139,
143,
147,
149,
153,
158,
161,
162,
159,
],
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
);
}

public function testEmtpyConstructorIsMarkedAsExecutable(): void
{
$file = TEST_FILES_PATH . 'source_with_empty_constructor.php';

$this->assertEquals(
[
5,
6,
7,
30,
],
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
);
}

/**
* @requires PHP 8.0
*/
public function testEachCaseInMatchExpressionIsMarkedAsExecutable(): void
{
$file = TEST_FILES_PATH . 'source_with_match_expression.php';

$this->assertEquals(
[
14,
20,
25,
],
array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file])
);
Expand Down