Drop support for false return value in NodeTraverser

easysoft · Feb 3, 2017 · 987c61e · 987c61e
1 parent 2b72bae
commit 987c61e
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 8 deletions.
diff --git 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.
+* 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.
diff --git 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
@@ -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 @@ protected function traverseArray(array $nodes) {
                         } 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
@@ -53,7 +53,7 @@ public function enterNode(Node $node);
      *
      * @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
@@ -85,7 +85,7 @@ public function testRemove() {
 
         // 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,14 +280,20 @@ public function provideTestInvalidReturn() {
             ->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'],
             [$visitor2, 'enterNode() returned invalid value of type string'],
             [$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'],
         ];
     }
 }