From a7df6f37eef14a76dc6047a956a7bd451774a812 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 | 20 ++------- src/Visitor/MutationsCollectorVisitor.php | 44 +++++++++++++------ tests/MutationTest.php | 38 ++++++++++++++-- .../Coverage/CodeCoverageDataTest.php | 12 +++-- tests/Visitor/MutatorVisitorTest.php | 9 ++-- 7 files changed, 98 insertions(+), 41 deletions(-) diff --git a/src/Mutation.php b/src/Mutation.php index 9c6244699c..02f562b9a2 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 250402d38f..b0403a9c05 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 6c43b8f425..90abfd1a6d 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' => [ @@ -261,7 +247,7 @@ private function getTestsForExecutedMethodOnLine(string $filePath, int $line): a $allLines = range($coverageInfo['startLine'], $coverageInfo['endLine']); foreach ($allLines as $lineInExecutedMethod) { - if (array_key_exists($lineInExecutedMethod, $this->getCoverage()[$filePath]['byLine'])) { + if (\array_key_exists($lineInExecutedMethod, $this->getCoverage()[$filePath]['byLine'])) { $tests[] = $this->getCoverage()[$filePath]['byLine'][$lineInExecutedMethod]; } } diff --git a/src/Visitor/MutationsCollectorVisitor.php b/src/Visitor/MutationsCollectorVisitor.php index 2153393e1e..9e7416cdd1 100644 --- a/src/Visitor/MutationsCollectorVisitor.php +++ b/src/Visitor/MutationsCollectorVisitor.php @@ -122,7 +122,24 @@ public function leaveNode(Node $node): void } } - $isCoveredByTest = $this->isCoveredByTest($isOnFunctionSignature, $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 + $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; @@ -142,7 +159,8 @@ public function leaveNode(Node $node): void $isOnFunctionSignature, $isCoveredByTest, $mutatedNode, - $mutationByMutatorIndex + $mutationByMutatorIndex, + $linerange ); } } @@ -156,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 2ea76ac9a6..5266dbd1fe 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 b55946133d..3a51c3fcb5 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 b5796318df..ad1d98b3fc 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) ), ]; }