From 5b02e8e3c77b10732e69e7aad005946d06a431ad Mon Sep 17 00:00:00 2001
From: Gert de Pagter <BackEndTea@users.noreply.github.com>
Date: Sun, 10 Mar 2019 21:08:23 +0100
Subject: [PATCH] Stop traversal of interfaces and abstract methods (#656)

* Stop traversal of interfaces and abstract methods

* Remove interface and abstraction checks in public visitor
---
 src/Mutant/Generator/MutationsGenerator.php   |   2 +
 .../FunctionSignature/PublicVisibility.php    |  11 --
 src/Visitor/MutationsCollectorVisitor.php     |  15 +-
 .../NotMutableIgnoreVisitor.php}              |  25 ++--
 tests/Mutator/AbstractMutatorTestCase.php     |   2 +
 tests/Visitor/NotMutableIgnoreVisitorTest.php | 130 ++++++++++++++++++
 6 files changed, 144 insertions(+), 41 deletions(-)
 rename src/{Mutator/Util/InterfaceParentTrait.php => Visitor/NotMutableIgnoreVisitor.php} (75%)
 create mode 100644 tests/Visitor/NotMutableIgnoreVisitorTest.php

diff --git a/src/Mutant/Generator/MutationsGenerator.php b/src/Mutant/Generator/MutationsGenerator.php
index f911a0c0a..0f821680a 100644
--- a/src/Mutant/Generator/MutationsGenerator.php
+++ b/src/Mutant/Generator/MutationsGenerator.php
@@ -47,6 +47,7 @@
 use Infection\Traverser\PriorityNodeTraverser;
 use Infection\Visitor\FullyQualifiedClassNameVisitor;
 use Infection\Visitor\MutationsCollectorVisitor;
+use Infection\Visitor\NotMutableIgnoreVisitor;
 use Infection\Visitor\ParentConnectorVisitor;
 use Infection\Visitor\ReflectionVisitor;
 use PhpParser\Node;
@@ -159,6 +160,7 @@ private function getMutationsFromFile(SplFileInfo $file, bool $onlyCovered, arra
             $onlyCovered
         );
 
+        $traverser->addVisitor(new NotMutableIgnoreVisitor(), 50);
         $traverser->addVisitor(new ParentConnectorVisitor(), 40);
         $traverser->addVisitor(new FullyQualifiedClassNameVisitor(), 30);
         $traverser->addVisitor(new ReflectionVisitor(), 20);
diff --git a/src/Mutator/FunctionSignature/PublicVisibility.php b/src/Mutator/FunctionSignature/PublicVisibility.php
index bcd398005..df7c09a33 100644
--- a/src/Mutator/FunctionSignature/PublicVisibility.php
+++ b/src/Mutator/FunctionSignature/PublicVisibility.php
@@ -35,7 +35,6 @@
 
 namespace Infection\Mutator\FunctionSignature;
 
-use Infection\Mutator\Util\InterfaceParentTrait;
 use Infection\Mutator\Util\Mutator;
 use Infection\Visitor\ReflectionVisitor;
 use PhpParser\Node;
@@ -47,8 +46,6 @@
  */
 final class PublicVisibility extends Mutator
 {
-    use InterfaceParentTrait;
-
     /**
      * Replaces "public function..." with "protected function ..."
      *
@@ -81,18 +78,10 @@ protected function mutatesNode(Node $node): bool
             return false;
         }
 
-        if ($node->isAbstract()) {
-            return false;
-        }
-
         if ($node->isMagic()) {
             return false;
         }
 
-        if ($this->isBelongsToInterface($node)) {
-            return false;
-        }
-
         return !$this->hasSamePublicParentMethod($node);
     }
 
diff --git a/src/Visitor/MutationsCollectorVisitor.php b/src/Visitor/MutationsCollectorVisitor.php
index 2153393e1..cc71dfc7b 100644
--- a/src/Visitor/MutationsCollectorVisitor.php
+++ b/src/Visitor/MutationsCollectorVisitor.php
@@ -50,7 +50,7 @@ final class MutationsCollectorVisitor extends NodeVisitorAbstract
     /**
      * @var Mutator[]
      */
-    private $mutators = [];
+    private $mutators;
 
     /**
      * @var Mutation[]
@@ -109,19 +109,6 @@ public function leaveNode(Node $node): void
                 continue;
             }
 
-            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;
-                }
-            }
-
             $isCoveredByTest = $this->isCoveredByTest($isOnFunctionSignature, $node);
 
             if ($this->onlyCovered && !$isCoveredByTest) {
diff --git a/src/Mutator/Util/InterfaceParentTrait.php b/src/Visitor/NotMutableIgnoreVisitor.php
similarity index 75%
rename from src/Mutator/Util/InterfaceParentTrait.php
rename to src/Visitor/NotMutableIgnoreVisitor.php
index 94cb9ca1b..2ba60e872 100644
--- a/src/Mutator/Util/InterfaceParentTrait.php
+++ b/src/Visitor/NotMutableIgnoreVisitor.php
@@ -33,32 +33,25 @@
 
 declare(strict_types=1);
 
-namespace Infection\Mutator\Util;
+namespace Infection\Visitor;
 
-use Infection\Visitor\ParentConnectorVisitor;
 use PhpParser\Node;
+use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitorAbstract;
 
 /**
- * Checks if given node belongs to Interface
- *
  * @internal
- *
- * @author Volodimir Melko <v.melko28@gmail.com>
  */
-trait InterfaceParentTrait
+final class NotMutableIgnoreVisitor extends NodeVisitorAbstract
 {
-    private function isBelongsToInterface(Node $node): bool
+    public function enterNode(Node $node)
     {
-        $parentNode = $node->getAttribute(ParentConnectorVisitor::PARENT_KEY);
-
-        if ($parentNode instanceof Node\Stmt\Interface_) {
-            return true;
+        if ($node instanceof Node\Stmt\Interface_) {
+            return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
         }
 
-        if ($parentNode instanceof Node) {
-            return $this->isBelongsToInterface($parentNode);
+        if ($node instanceof Node\Stmt\ClassMethod && $node->isAbstract()) {
+            return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN;
         }
-
-        return false;
     }
 }
diff --git a/tests/Mutator/AbstractMutatorTestCase.php b/tests/Mutator/AbstractMutatorTestCase.php
index 0510faf69..4b53fb02a 100644
--- a/tests/Mutator/AbstractMutatorTestCase.php
+++ b/tests/Mutator/AbstractMutatorTestCase.php
@@ -42,6 +42,7 @@
 use Infection\Tests\Fixtures\SimpleMutatorVisitor;
 use Infection\Visitor\CloneVisitor;
 use Infection\Visitor\FullyQualifiedClassNameVisitor;
+use Infection\Visitor\NotMutableIgnoreVisitor;
 use Infection\Visitor\ParentConnectorVisitor;
 use Infection\Visitor\ReflectionVisitor;
 use PhpParser\Lexer;
@@ -152,6 +153,7 @@ private function getMutationsFromCode(string $code, Parser $parser, array $setti
 
         $mutationsCollectorVisitor = new SimpleMutationsCollectorVisitor($this->getMutator($settings), $initialStatements);
 
+        $traverser->addVisitor(new NotMutableIgnoreVisitor());
         $traverser->addVisitor($mutationsCollectorVisitor);
         $traverser->addVisitor(new ParentConnectorVisitor());
         $traverser->addVisitor(new FullyQualifiedClassNameVisitor());
diff --git a/tests/Visitor/NotMutableIgnoreVisitorTest.php b/tests/Visitor/NotMutableIgnoreVisitorTest.php
new file mode 100644
index 000000000..afd8c31a9
--- /dev/null
+++ b/tests/Visitor/NotMutableIgnoreVisitorTest.php
@@ -0,0 +1,130 @@
+<?php
+/**
+ * This code is licensed under the BSD 3-Clause License.
+ *
+ * Copyright (c) 2017-2019, Maks Rafalko
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * * Redistributions of source code must retain the above copyright notice, this
+ *   list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright notice,
+ *   this list of conditions and the following disclaimer in the documentation
+ *   and/or other materials provided with the distribution.
+ *
+ * * Neither the name of the copyright holder nor the names of its
+ *   contributors may be used to endorse or promote products derived from
+ *   this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+declare(strict_types=1);
+
+namespace Infection\Tests\Visitor;
+
+use Infection\Visitor\NotMutableIgnoreVisitor;
+use PhpParser\Node;
+use PhpParser\NodeTraverser;
+use PhpParser\NodeVisitorAbstract;
+
+/**
+ * @internal
+ */
+final class NotMutableIgnoreVisitorTest extends AbstractBaseVisitorTest
+{
+    private $spyVisitor;
+
+    protected function setUp(): void
+    {
+        $this->spyVisitor = $this->getSpyVisitor();
+    }
+
+    public function test_it_does_not_traverse_interface_methods(): void
+    {
+        $code = <<<'PHP'
+<?php
+
+interface Foo
+{
+    public function foo(): array;
+    public function bar(int $number): string;
+}
+PHP;
+        $this->parseAndTraverse($code);
+        $this->assertSame(0, $this->spyVisitor->getNumberOfClassMethodsVisited());
+    }
+
+    public function test_it_does_not_traverse_abstract_methods(): void
+    {
+        $code = <<<'PHP'
+<?php
+
+abstract class Foo
+{
+    abstract public function foo(): array;
+    abstract public function bar(int $number): string;
+}
+PHP;
+        $this->parseAndTraverse($code);
+        $this->assertSame(0, $this->spyVisitor->getNumberOfClassMethodsVisited());
+    }
+
+    public function test_it_still_traverses_normal_methods_in_abstract_classes(): void
+    {
+        $code = <<<'PHP'
+<?php
+
+abstract class Foo
+{
+    abstract public function foo(): array;
+    public function bar(int $number): string { return ''; }
+}
+PHP;
+        $this->parseAndTraverse($code);
+        $this->assertSame(1, $this->spyVisitor->getNumberOfClassMethodsVisited());
+    }
+
+    private function getSpyVisitor()
+    {
+        return new class() extends NodeVisitorAbstract {
+            private $nodesVisitedCount = 0;
+
+            public function leaveNode(Node $node): void
+            {
+                if ($node instanceof Node\Stmt\ClassMethod) {
+                    ++$this->nodesVisitedCount;
+                }
+            }
+
+            public function getNumberOfClassMethodsVisited(): int
+            {
+                return $this->nodesVisitedCount;
+            }
+        };
+    }
+
+    private function parseAndTraverse(string $code): void
+    {
+        $nodes = $this->getNodes($code);
+
+        $traverser = new NodeTraverser();
+
+        $traverser->addVisitor(new NotMutableIgnoreVisitor());
+        $traverser->addVisitor($this->spyVisitor);
+
+        $traverser->traverse($nodes);
+    }
+}