From 457e6e8ea000b33936ff0de013709cde2bcba8af Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Thu, 17 Feb 2022 08:59:48 +0100 Subject: [PATCH 1/3] Improve executable lines visitor accuracy --- .../ExecutableLinesFindingVisitor.php | 53 +++++++++++++------ .../_files/source_with_heavy_indentation.php | 24 +++++++++ tests/tests/RawCodeCoverageDataTest.php | 10 ++++ 3 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/StaticAnalysis/ExecutableLinesFindingVisitor.php b/src/StaticAnalysis/ExecutableLinesFindingVisitor.php index 2121466bc..d2a4bb4eb 100644 --- a/src/StaticAnalysis/ExecutableLinesFindingVisitor.php +++ b/src/StaticAnalysis/ExecutableLinesFindingVisitor.php @@ -10,8 +10,14 @@ namespace SebastianBergmann\CodeCoverage\StaticAnalysis; use PhpParser\Node; +use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\BinaryOp; use PhpParser\Node\Expr\CallLike; +use PhpParser\Node\Expr\Closure; +use PhpParser\Node\Expr\NullsafePropertyFetch; +use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\StaticPropertyFetch; +use PhpParser\Node\Expr\Ternary; use PhpParser\Node\Scalar; use PhpParser\Node\Stmt\Break_; use PhpParser\Node\Stmt\Case_; @@ -59,13 +65,13 @@ public function enterNode(Node $node): void return; } - $line = $this->getLine($node); + foreach ($this->getLines($node) as $line) { + if (isset($this->propertyLines[$line])) { + return; + } - if (isset($this->propertyLines[$line])) { - return; + $this->executableLines[$line] = $line; } - - $this->executableLines[$line] = $line; } /** @@ -87,26 +93,39 @@ private function savePropertyLines(Node $node): void } } - private function getLine(Node $node): int + /** + * @return int[] + */ + private function getLines(Node $node): array { if ( - $node instanceof Node\Expr\PropertyFetch || - $node instanceof Node\Expr\NullsafePropertyFetch || - $node instanceof Node\Expr\StaticPropertyFetch + $node instanceof PropertyFetch || + $node instanceof NullsafePropertyFetch || + $node instanceof StaticPropertyFetch ) { - return $node->getEndLine(); + return [$node->getEndLine()]; } - return $node->getStartLine(); + if ($node instanceof Ternary) { + return [ + $node->cond->getStartLine(), + $node->if->getStartLine(), + $node->else->getStartLine(), + ]; + } + + return [$node->getStartLine()]; } private function isExecutable(Node $node): bool { - return $node instanceof BinaryOp || + return $node instanceof Assign || + $node instanceof BinaryOp || $node instanceof Break_ || $node instanceof CallLike || $node instanceof Case_ || $node instanceof Catch_ || + $node instanceof Closure || $node instanceof Continue_ || $node instanceof Do_ || $node instanceof Echo_ || @@ -114,20 +133,20 @@ private function isExecutable(Node $node): bool $node instanceof Else_ || $node instanceof Expression || $node instanceof Finally_ || - $node instanceof Foreach_ || $node instanceof For_ || + $node instanceof Foreach_ || $node instanceof Goto_ || $node instanceof If_ || + $node instanceof NullsafePropertyFetch || + $node instanceof PropertyFetch || $node instanceof Return_ || $node instanceof Scalar || + $node instanceof StaticPropertyFetch || $node instanceof Switch_ || + $node instanceof Ternary || $node instanceof Throw_ || $node instanceof TryCatch || $node instanceof Unset_ || - $node instanceof Node\Expr\Assign || - $node instanceof Node\Expr\PropertyFetch || - $node instanceof Node\Expr\NullsafePropertyFetch || - $node instanceof Node\Expr\StaticPropertyFetch || $node instanceof While_; } } diff --git a/tests/_files/source_with_heavy_indentation.php b/tests/_files/source_with_heavy_indentation.php index bf4b123af..1bc431314 100644 --- a/tests/_files/source_with_heavy_indentation.php +++ b/tests/_files/source_with_heavy_indentation.php @@ -77,3 +77,27 @@ public function isTwo(): bool private static $staticState = 1; private const CONST_STATE = 1.1; } + +function &foo($bar) +{ + $baz + = + function () {} + ; + $a + = + true + ? + true + : + false + ; + $b + = + "{$a}" + ; + $c + = + "${b}" + ; +} diff --git a/tests/tests/RawCodeCoverageDataTest.php b/tests/tests/RawCodeCoverageDataTest.php index 5499f4439..725742322 100644 --- a/tests/tests/RawCodeCoverageDataTest.php +++ b/tests/tests/RawCodeCoverageDataTest.php @@ -348,6 +348,16 @@ public function testHeavyIndentationIsHandledCorrectly(): void 54, 60, 71, + 83, + 85, + 87, + 89, + 91, + 93, + 95, + 97, + 99, + 101, ], array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file]) ); From ae3e92348d7cb7994f9ff71c4a69d8b449c3f496 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Thu, 17 Feb 2022 09:17:23 +0100 Subject: [PATCH 2/3] Update test suite --- .../CoverageForClassWithAnonymousFunction/index.html | 4 ++-- .../source_with_class_and_anonymous_function.php.html | 8 ++++---- .../CoverageForClassWithAnonymousFunction/index.html | 4 ++-- .../source_with_class_and_anonymous_function.php.html | 8 ++++---- .../CoverageForClassWithAnonymousFunction/index.xml | 4 ++-- .../source_with_class_and_anonymous_function.php.xml | 9 ++++++--- .../CoverageForClassWithAnonymousFunction/index.xml | 4 ++-- .../source_with_class_and_anonymous_function.php.xml | 9 ++++++--- tests/_files/class-with-anonymous-function-clover.xml | 7 ++++--- tests/_files/class-with-anonymous-function-cobertura.xml | 4 +++- tests/_files/class-with-anonymous-function-text.txt | 4 ++-- tests/tests/RawCodeCoverageDataTest.php | 2 ++ 12 files changed, 39 insertions(+), 28 deletions(-) diff --git a/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.html b/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.html index 9626a7eea..577c610e3 100644 --- a/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.html +++ b/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.html @@ -50,7 +50,7 @@
100.00%
-
4 / 4
+
5 / 5
100.00% covered (success) @@ -78,7 +78,7 @@
100.00%
-
4 / 4
+
5 / 5
100.00% covered (success) diff --git a/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html b/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html index bbae23992..df5d7c6e5 100644 --- a/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html +++ b/tests/_files/Report/HTML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html @@ -67,7 +67,7 @@
100.00%
-
4 / 4
+
5 / 5
@@ -96,7 +96,7 @@
100.00%
-
4 / 4
+
5 / 5
@@ -117,7 +117,7 @@
100.00%
-
4 / 4
+
5 / 5
@@ -136,7 +136,7 @@ 8 9        array_walk( 10            $filter, - 11            function (&$val, $key) { + 11            function (&$val, $key) { 12                $val = preg_replace('|[^0-9]|', '', $val); 13            } 14        ); diff --git a/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.html b/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.html index 9626a7eea..577c610e3 100644 --- a/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.html +++ b/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.html @@ -50,7 +50,7 @@
100.00%
-
4 / 4
+
5 / 5
100.00% covered (success) @@ -78,7 +78,7 @@
100.00%
-
4 / 4
+
5 / 5
100.00% covered (success) diff --git a/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html b/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html index d1390f1e8..3bddd888f 100644 --- a/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html +++ b/tests/_files/Report/HTML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.html @@ -67,7 +67,7 @@
100.00%
-
4 / 4
+
5 / 5
@@ -96,7 +96,7 @@
100.00%
-
4 / 4
+
5 / 5
@@ -117,7 +117,7 @@
100.00%
-
4 / 4
+
5 / 5
@@ -136,7 +136,7 @@ 8 9        array_walk( 10            $filter, - 11            function (&$val, $key) { + 11            function (&$val, $key) { 12                $val = preg_replace('|[^0-9]|', '', $val); 13            } 14        ); diff --git a/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.xml b/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.xml index ad86c96f4..06490308d 100644 --- a/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.xml +++ b/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/index.xml @@ -10,7 +10,7 @@ - + @@ -18,7 +18,7 @@ - + diff --git a/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml b/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml index ab4f84275..76286e420 100644 --- a/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml +++ b/tests/_files/Report/XML/PHP80AndBelow/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml @@ -2,15 +2,15 @@ - + - + - + @@ -19,6 +19,9 @@ + + + diff --git a/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.xml b/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.xml index ad86c96f4..06490308d 100644 --- a/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.xml +++ b/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/index.xml @@ -10,7 +10,7 @@ - + @@ -18,7 +18,7 @@ - + diff --git a/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml b/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml index a767fa117..8137a2c05 100644 --- a/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml +++ b/tests/_files/Report/XML/PHP81AndUp/CoverageForClassWithAnonymousFunction/source_with_class_and_anonymous_function.php.xml @@ -2,15 +2,15 @@ - + - + - + @@ -19,6 +19,9 @@ + + + diff --git a/tests/_files/class-with-anonymous-function-clover.xml b/tests/_files/class-with-anonymous-function-clover.xml index 223ebebec..02b8660da 100644 --- a/tests/_files/class-with-anonymous-function-clover.xml +++ b/tests/_files/class-with-anonymous-function-clover.xml @@ -3,15 +3,16 @@ - + + - + - + diff --git a/tests/_files/class-with-anonymous-function-cobertura.xml b/tests/_files/class-with-anonymous-function-cobertura.xml index 874f85080..aa0fb6e3f 100644 --- a/tests/_files/class-with-anonymous-function-cobertura.xml +++ b/tests/_files/class-with-anonymous-function-cobertura.xml @@ -1,6 +1,6 @@ - + %s @@ -13,6 +13,7 @@ + @@ -21,6 +22,7 @@ + diff --git a/tests/_files/class-with-anonymous-function-text.txt b/tests/_files/class-with-anonymous-function-text.txt index ca6d0b965..602d0d8e4 100644 --- a/tests/_files/class-with-anonymous-function-text.txt +++ b/tests/_files/class-with-anonymous-function-text.txt @@ -6,7 +6,7 @@ Code Coverage Report: Summary: Classes: 100.00% (1/1) Methods: 100.00% (1/1) - Lines: 100.00% (4/4) + Lines: 100.00% (5/5) CoveredClassWithAnonymousFunctionInStaticMethod - Methods: 100.00% ( 1/ 1) Lines: 100.00% ( 4/ 4) + Methods: 100.00% ( 1/ 1) Lines: 100.00% ( 5/ 5) diff --git a/tests/tests/RawCodeCoverageDataTest.php b/tests/tests/RawCodeCoverageDataTest.php index 725742322..256b22081 100644 --- a/tests/tests/RawCodeCoverageDataTest.php +++ b/tests/tests/RawCodeCoverageDataTest.php @@ -271,6 +271,7 @@ public function testUseStatementsAreUncovered(): void 12, 14, 15, + 16, 18, ], array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file]) @@ -298,6 +299,7 @@ public function testInterfacesAreUncovered(): void 7, 9, 10, + 11, 13, ], array_keys(RawCodeCoverageData::fromUncoveredFile($file, new ParsingFileAnalyser(true, true))->lineCoverage()[$file]) From 4e77c76c459b1a050b8a3d84a1d76c0d315d62ca Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Thu, 17 Feb 2022 09:35:56 +0100 Subject: [PATCH 3/3] Ternary if condition is nullable --- .../ExecutableLinesFindingVisitor.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/StaticAnalysis/ExecutableLinesFindingVisitor.php b/src/StaticAnalysis/ExecutableLinesFindingVisitor.php index d2a4bb4eb..100064f6c 100644 --- a/src/StaticAnalysis/ExecutableLinesFindingVisitor.php +++ b/src/StaticAnalysis/ExecutableLinesFindingVisitor.php @@ -107,11 +107,15 @@ private function getLines(Node $node): array } if ($node instanceof Ternary) { - return [ - $node->cond->getStartLine(), - $node->if->getStartLine(), - $node->else->getStartLine(), - ]; + $lines = [$node->cond->getStartLine()]; + + if (null !== $node->if) { + $lines[] = $node->if->getStartLine(); + } + + $lines[] = $node->else->getStartLine(); + + return $lines; } return [$node->getStartLine()];