From 987c61e935a7d73485b4d73aef7a17a4c1e2e325 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@googlemail.com>
Date: Fri, 3 Feb 2017 22:34:27 +0100
Subject: [PATCH] Drop support for false return value in NodeTraverser

---
 UPGRADE-4.0.md                           |  4 +++-
 doc/2_Usage_of_basic_components.markdown |  5 +++--
 lib/PhpParser/NodeTraverser.php          |  9 +++++++--
 lib/PhpParser/NodeVisitor.php            |  2 +-
 test/PhpParser/NodeTraverserTest.php     | 10 ++++++++--
 5 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/UPGRADE-4.0.md b/UPGRADE-4.0.md
index ea54f59..3ba5f76 100644
--- a/UPGRADE-4.0.md
+++ b/UPGRADE-4.0.md
@@ -20,4 +20,6 @@ source code, while running on a newer version.
 ### Removed functionality
 
 * Removed `type` subnode on `Class`, `ClassMethod` and `Property` nodes. Use `flags` instead.
-* The `ClassConst::isStatic()` method has been removed. Constants cannot have a static modifier.
\ No newline at end of file
+* The `ClassConst::isStatic()` method has been removed. Constants cannot have a static modifier.
+* The `NodeTraverser` no longer accepts `false` as a return value from a `leaveNode()` method.
+  `NodeTraverser::REMOVE_NODE` should be returned instead.
\ No newline at end of file
diff --git a/doc/2_Usage_of_basic_components.markdown b/doc/2_Usage_of_basic_components.markdown
index f91a5bb..06cc123 100644
--- a/doc/2_Usage_of_basic_components.markdown
+++ b/doc/2_Usage_of_basic_components.markdown
@@ -410,6 +410,7 @@ The last thing we need to do is remove the `namespace` and `use` statements:
 ```php
 use PhpParser\Node;
 use PhpParser\Node\Stmt;
+use PhpParser\NodeTraverser;
 
 class NodeVisitor_NamespaceConverter extends \PhpParser\NodeVisitorAbstract
 {
@@ -428,8 +429,8 @@ class NodeVisitor_NamespaceConverter extends \PhpParser\NodeVisitorAbstract
             // returning an array merges is into the parent array
             return $node->stmts;
         } elseif ($node instanceof Stmt\Use_) {
-            // returning false removed the node altogether
-            return false;
+            // return use nodes altogether
+            return NodeTraverser::REMOVE_NODE;
         }
     }
 }
diff --git a/lib/PhpParser/NodeTraverser.php b/lib/PhpParser/NodeTraverser.php
index 3810be3..fd257b3 100644
--- a/lib/PhpParser/NodeTraverser.php
+++ b/lib/PhpParser/NodeTraverser.php
@@ -28,7 +28,7 @@ class NodeTraverser implements NodeTraverserInterface
      * For subsequent visitors leaveNode() will still be invoked for the
      * removed node.
      */
-    const REMOVE_NODE = false;
+    const REMOVE_NODE = 3;
 
     /** @var NodeVisitor[] Visitors */
     protected $visitors;
@@ -211,9 +211,14 @@ class NodeTraverser implements NodeTraverserInterface
                         } elseif (self::REMOVE_NODE === $return) {
                             $doNodes[] = array($i, array());
                             break;
-                        } else if (self::STOP_TRAVERSAL === $return) {
+                        } elseif (self::STOP_TRAVERSAL === $return) {
                             $this->stopTraversal = true;
                             break 2;
+                        } elseif (false === $return) {
+                            throw new \LogicException(
+                                'bool(false) return from leaveNode() no longer supported. ' .
+                                'Return NodeTraverser::REMOVE_NODE instead'
+                            );
                         } else {
                             throw new \LogicException(
                                 'leaveNode() returned invalid value of type ' . gettype($return)
diff --git a/lib/PhpParser/NodeVisitor.php b/lib/PhpParser/NodeVisitor.php
index d56e6d7..254ecc0 100644
--- a/lib/PhpParser/NodeVisitor.php
+++ b/lib/PhpParser/NodeVisitor.php
@@ -53,7 +53,7 @@ interface NodeVisitor
      *
      * @param Node $node Node
      *
-     * @return null|false|int|Node|Node[] Replacement node (or special return value)
+     * @return null|int|Node|Node[] Replacement node (or special return value)
      */
     public function leaveNode(Node $node);
 
diff --git a/test/PhpParser/NodeTraverserTest.php b/test/PhpParser/NodeTraverserTest.php
index 0ca485d..dfce05e 100644
--- a/test/PhpParser/NodeTraverserTest.php
+++ b/test/PhpParser/NodeTraverserTest.php
@@ -85,7 +85,7 @@ class NodeTraverserTest extends \PHPUnit_Framework_TestCase
 
         // remove the string1 node, leave the string2 node
         $visitor->expects($this->at(2))->method('leaveNode')->with($str1Node)
-                ->will($this->returnValue(false));
+                ->will($this->returnValue(NodeTraverser::REMOVE_NODE));
 
         $traverser = new NodeTraverser;
         $traverser->addVisitor($visitor);
@@ -280,7 +280,12 @@ class NodeTraverserTest extends \PHPUnit_Framework_TestCase
             ->will($this->returnValue('foobar'));
 
         $visitor5 = $this->getMockBuilder('PhpParser\NodeVisitor')->getMock();
-        $visitor5->method('leaveNode')->willReturn(array(new Node\Scalar\DNumber(42.0)));
+        $visitor5->expects($this->at(3))->method('leaveNode')
+            ->willReturn(array(new Node\Scalar\DNumber(42.0)));
+
+        $visitor6 = $this->getMockBuilder('PhpParser\NodeVisitor')->getMock();
+        $visitor6->expects($this->at(4))->method('leaveNode')
+            ->willReturn(false);
 
         return [
             [$visitor1, 'enterNode() returned invalid value of type string'],
@@ -288,6 +293,7 @@ class NodeTraverserTest extends \PHPUnit_Framework_TestCase
             [$visitor3, 'leaveNode() returned invalid value of type string'],
             [$visitor4, 'leaveNode() returned invalid value of type string'],
             [$visitor5, 'leaveNode() may only return an array if the parent structure is an array'],
+            [$visitor6, 'bool(false) return from leaveNode() no longer supported. Return NodeTraverser::REMOVE_NODE instead'],
         ];
     }
 }