From 3bae203e8d1247475375b3a1197a254c818931f7 Mon Sep 17 00:00:00 2001 From: Gert de Pagter Date: Fri, 8 Mar 2019 13:37:55 +0100 Subject: [PATCH] If node is part of an array, use those lines for coverage --- src/Mutation.php | 14 ++++- src/MutationInterface.php | 2 + .../Coverage/CodeCoverageData.php | 18 +----- src/Visitor/MutationsCollectorVisitor.php | 57 ++++++++++++++----- tests/MutationTest.php | 38 ++++++++++++- .../Coverage/CodeCoverageDataTest.php | 12 ++-- tests/Visitor/MutatorVisitorTest.php | 9 ++- 7 files changed, 110 insertions(+), 40 deletions(-) diff --git a/src/Mutation.php b/src/Mutation.php index 9c6244699..02f562b9a 100644 --- a/src/Mutation.php +++ b/src/Mutation.php @@ -93,6 +93,11 @@ final class Mutation implements MutationInterface */ private $mutationByMutatorIndex; + /** + * @var array|int[] + */ + private $lineRange; + public function __construct( string $originalFilePath, array $originalFileAst, @@ -102,7 +107,8 @@ public function __construct( bool $isOnFunctionSignature, bool $isCoveredByTest, $mutatedNode, - int $mutationByMutatorIndex + int $mutationByMutatorIndex, + array $lineRange ) { $this->originalFilePath = $originalFilePath; $this->originalFileAst = $originalFileAst; @@ -113,6 +119,7 @@ public function __construct( $this->isCoveredByTest = $isCoveredByTest; $this->mutatedNode = $mutatedNode; $this->mutationByMutatorIndex = $mutationByMutatorIndex; + $this->lineRange = $lineRange; } public function getMutator(): Mutator @@ -183,4 +190,9 @@ public function getMutatedNode() { return $this->mutatedNode; } + + public function getLineRange(): array + { + return $this->lineRange; + } } diff --git a/src/MutationInterface.php b/src/MutationInterface.php index 250402d38..b0403a9c0 100644 --- a/src/MutationInterface.php +++ b/src/MutationInterface.php @@ -65,4 +65,6 @@ public function isCoveredByTest(): bool; * @return Node|Node[] Node, array of Nodes */ public function getMutatedNode(); + + public function getLineRange(): array; } diff --git a/src/TestFramework/Coverage/CodeCoverageData.php b/src/TestFramework/Coverage/CodeCoverageData.php index 8de22c062..6fa887c97 100644 --- a/src/TestFramework/Coverage/CodeCoverageData.php +++ b/src/TestFramework/Coverage/CodeCoverageData.php @@ -147,7 +147,7 @@ private function getTestsForMutationOnFunctionSignature(MutationInterface $mutat { $filePath = $mutation->getOriginalFilePath(); - foreach ($this->linesForMutation($mutation) as $line) { + foreach ($mutation->getLineRange() as $line) { if ($this->hasExecutedMethodOnLine($filePath, $line)) { yield from $this->getTestsForExecutedMethodOnLine($filePath, $line); } @@ -158,27 +158,13 @@ private function getTestsForMutation(MutationInterface $mutation): \Generator { $filePath = $mutation->getOriginalFilePath(); - foreach ($this->linesForMutation($mutation) as $line) { + foreach ($mutation->getLineRange() as $line) { if ($this->hasTestsOnLine($filePath, $line)) { yield from $this->getCoverage()[$filePath]['byLine'][$line]; } } } - private function linesForMutation(MutationInterface $mutation): \Generator - { - /* - * If we only look for tests that cover only the very first line of our - * mutation, we may naturally miss other tests that may actually also - * cover this mutation, this all leading to false positives. - * - * Therefore we have to consider all lines of a mutation. - */ - for ($line = $mutation->getAttributes()['startLine']; $line <= $mutation->getAttributes()['endLine']; ++$line) { - yield $line; - } - } - /** * coverage[$sourceFilePath] = [ * 'byMethod' => [ diff --git a/src/Visitor/MutationsCollectorVisitor.php b/src/Visitor/MutationsCollectorVisitor.php index cc71dfc7b..9bee7f213 100644 --- a/src/Visitor/MutationsCollectorVisitor.php +++ b/src/Visitor/MutationsCollectorVisitor.php @@ -109,7 +109,37 @@ public function leaveNode(Node $node): void continue; } - $isCoveredByTest = $this->isCoveredByTest($isOnFunctionSignature, $node); + if ($isOnFunctionSignature + && $methodNode = $node->getAttribute(ReflectionVisitor::FUNCTION_SCOPE_KEY) + ) { + /** @var Node\Stmt\ClassMethod|Node\Expr\Closure $methodNode */ + if ($methodNode instanceof Node\Stmt\ClassMethod && $methodNode->isAbstract()) { + continue; + } + + if ($methodNode instanceof Node\Stmt\ClassMethod && $methodNode->getAttribute(ParentConnectorVisitor::PARENT_KEY) instanceof Node\Stmt\Interface_) { + continue; + } + } + + if ($isOnFunctionSignature) { + // hasExecutedMethodOnLine checks for all lines of a given method, + // therefore it isn't making any sense to check any other line but first + $isCoveredByTest = $this->codeCoverageData->hasExecutedMethodOnLine($this->filePath, $node->getLine()); + $linerange = range($node->getStartLine(), $node->getEndLine()); + } else { + $outerMostArrayNode = $this->getOuterMostArrayNode($node); + $isCoveredByTest = false; + + for ($line = $outerMostArrayNode->getStartLine(); $line <= $outerMostArrayNode->getEndLine(); ++$line) { + if ($this->codeCoverageData->hasTestsOnLine($this->filePath, $line)) { + $isCoveredByTest = true; + + break; + } + } + $linerange = range($outerMostArrayNode->getStartLine(), $outerMostArrayNode->getEndLine()); + } if ($this->onlyCovered && !$isCoveredByTest) { continue; @@ -129,7 +159,8 @@ public function leaveNode(Node $node): void $isOnFunctionSignature, $isCoveredByTest, $mutatedNode, - $mutationByMutatorIndex + $mutationByMutatorIndex, + $linerange ); } } @@ -143,20 +174,20 @@ public function getMutations(): array return $this->mutations; } - private function isCoveredByTest(bool $isOnFunctionSignature, Node $node): bool + /** + * If the node is part of an array, this will find the outermost array. + * Otherwise this will return the node itself + */ + private function getOuterMostArrayNode(Node $node): Node { - if ($isOnFunctionSignature) { - // hasExecutedMethodOnLine checks for all lines of a given method, - // therefore it isn't making any sense to check any other line but first - return $this->codeCoverageData->hasExecutedMethodOnLine($this->filePath, $node->getLine()); - } + $outerMostArrayParent = $node; - for ($line = $node->getStartLine(); $line <= $node->getEndLine(); ++$line) { - if ($this->codeCoverageData->hasTestsOnLine($this->filePath, $line)) { - return true; + do { + if ($node instanceof Node\Expr\Array_) { + $outerMostArrayParent = $node; } - } + } while ($node = $node->getAttribute(ParentConnectorVisitor::PARENT_KEY)); - return false; + return $outerMostArrayParent; } } diff --git a/tests/MutationTest.php b/tests/MutationTest.php index 2ea76ac9a..5266dbd1f 100644 --- a/tests/MutationTest.php +++ b/tests/MutationTest.php @@ -67,7 +67,8 @@ public function test_it_correctly_generates_hash(): void false, true, new Node\Scalar\LNumber(1), - 0 + 0, + [1, 2, 3] ); $this->assertSame('2930c05082a35248987760a81b9f9a08', $mutation->getHash()); @@ -94,7 +95,8 @@ public function test_it_correctly_sets_is_on_function_signature(): void false, true, new Node\Scalar\LNumber(1), - 0 + 0, + [1, 2, 3] ); $this->assertFalse($mutation->isOnFunctionSignature()); @@ -122,9 +124,39 @@ public function test_it_correctly_sets_original_file_ast(): void false, true, new Node\Scalar\LNumber(1), - 0 + 0, + [1, 2, 3] ); $this->assertSame($fileAst, $mutation->getOriginalFileAst()); } + + public function test_it_correctly_sets_line_range(): void + { + $mutator = new Plus(new MutatorConfig([])); + $attributes = [ + 'startLine' => 3, + 'endLine' => 5, + 'startTokenPos' => 21, + 'endTokenPos' => 31, + 'startFilePos' => 43, + 'endFilePos' => 53, + ]; + $range = [21, 22, 23, 24]; + + $mutation = new Mutation( + '/abc.php', + ['file' => 'ast'], + $mutator, + $attributes, + 'Interface_', + false, + true, + new Node\Scalar\LNumber(1), + 0, + $range + ); + + $this->assertSame($range, $mutation->getLineRange()); + } } diff --git a/tests/TestFramework/Coverage/CodeCoverageDataTest.php b/tests/TestFramework/Coverage/CodeCoverageDataTest.php index b55946133..3a51c3fcb 100644 --- a/tests/TestFramework/Coverage/CodeCoverageDataTest.php +++ b/tests/TestFramework/Coverage/CodeCoverageDataTest.php @@ -149,7 +149,8 @@ public function test_it_returns_zero_tests_for_not_covered_function_body_mutator false, true, new LNumber(1), - 0 + 0, + [1] ); $this->assertCount(0, $codeCoverageData->getAllTestsFor($mutation)); @@ -169,7 +170,8 @@ public function test_it_returns_tests_for_covered_function_body_mutator(): void false, true, new LNumber(1), - 0 + 0, + [26] ); $this->assertCount(2, $codeCoverageData->getAllTestsFor($mutation)); @@ -189,7 +191,8 @@ public function test_it_returns_zero_tests_for_not_covered_function_signature_mu true, true, new LNumber(1), - 0 + 0, + [1] ); $this->assertCount(0, $codeCoverageData->getAllTestsFor($mutation)); @@ -209,7 +212,8 @@ public function test_it_returns_tests_for_covered_function_signature_mutator(): true, true, new LNumber(1), - 0 + 0, + [24] ); $this->assertCount(6, $codeCoverageData->getAllTestsFor($mutation)); diff --git a/tests/Visitor/MutatorVisitorTest.php b/tests/Visitor/MutatorVisitorTest.php index b5796318d..ad1d98b3f 100644 --- a/tests/Visitor/MutatorVisitorTest.php +++ b/tests/Visitor/MutatorVisitorTest.php @@ -121,7 +121,8 @@ public function hello() : string true, true, new Nop(), - 0 + 0, + range(29, 48) ), ]; @@ -170,7 +171,8 @@ public function bye() : string true, true, new Nop(), - 0 + 0, + range(29, 50) ), ]; $badLexer = new Lexer\Emulative([ @@ -226,7 +228,8 @@ public function bye() : string true, true, new Nop(), - 0 + 0, + range(29, 48) ), ]; }