Get Debugger tests passing with minor updates.

This breaks several internals of Debugger. While discouraged this is
allowed by our backwards compatibility guide as long as the removals are
documented.

Remove special casing for <5.4 as we no longer support that version of
PHP.

src/Error/Debugger.php

@@ -25,6 +25,7 @@ use Cake\Error\DumpNode\NodeInterface;
 use Cake\Error\DumpNode\PropertyNode;
 use Cake\Error\DumpNode\ReferenceNode;
 use Cake\Error\DumpNode\ScalarNode;
+use Cake\Error\DumpNode\SpecialNode;
 use Cake\Log\Log;
 use Cake\Utility\Hash;
 use Cake\Utility\Security;
@@ -504,7 +505,7 @@ class Debugger
     public static function exportVar($var, int $maxDepth = 3): string
     {
         $context = new DumpContext($maxDepth);
-        $node = static::_export($var, $context);
+        $node = static::export($var, $context);
 
         $formatter = new TextFormatter();
         return $formatter->dump($node);
@@ -517,7 +518,7 @@ class Debugger
      * @param \Cake\Error\DumpContext $context Dump context
      * @return \Cake\Error\DumpNode\NodeInterface The dumped variable.
      */
-    protected static function _export($var, DumpContext $context): NodeInterface
+    protected static function export($var, DumpContext $context): NodeInterface
     {
         switch (static::getType($var)) {
             case 'boolean':
@@ -529,15 +530,15 @@ class Debugger
             case 'string':
                 return new ScalarNode('string', $var);
             case 'array':
-                return static::_array($var, $context->withAddedDepthAndIndent());
+                return static::exportArray($var, $context->withAddedDepth());
             case 'resource':
                 return new ScalarNode('resource', gettype($var));
             case 'null':
                 return new ScalarNode('null', null);
             case 'unknown':
-                return new ScalarNode('unknown', null);
+                return new SpecialNode('(unknown)');
             default:
-                return static::_object($var, $context->withAddedDepthAndIndent());
+                return static::exportObject($var, $context->withAddedDepth());
         }
     }
 
@@ -558,7 +559,7 @@ class Debugger
      * @param \Cake\Error\DumpContext $context The current dump context.
      * @return \Cake\Error\DumpNode\ArrayNode Exported array.
      */
-    protected static function _array(array $var, DumpContext $context): ArrayNode
+    protected static function exportArray(array $var, DumpContext $context): ArrayNode
     {
         $items = [];
 
@@ -566,18 +567,22 @@ class Debugger
         if ($remaining >= 0) {
             $outputMask = (array)static::outputMask();
             foreach ($var as $key => $val) {
-                // Sniff for globals as !== explodes in < 5.4
-                if ($key === 'GLOBALS' && is_array($val) && isset($val['GLOBALS'])) {
-                    $node = new ScalarNode('string', '[recursion]');
-                } elseif (array_key_exists($key, $outputMask)) {
+                if (array_key_exists($key, $outputMask)) {
                     $node = new ScalarNode('string', $outputMask[$key]);
                 } elseif ($val !== $var) {
-                    $node = static::_export($val, $context);
+                    // Dump all the items without increasing depth.
+                    $node = static::export($val, $context);
+                } else {
+                    // Likely recursion, so we increase depth.
+                    $node = static::export($val, $context->withAddedDepth());
                 }
-                $items[] = new ItemNode($key, $node);
+                $items[] = new ItemNode(static::export($key, $context), $node);
             }
         } else {
-            $items[] = new ItemNode('', new ScalarNode('string', '[maximum depth reached]'));
+            $items[] = new ItemNode(
+                new ScalarNode('string', ''),
+                new SpecialNode('[maximum depth reached]')
+            );
         }
 
         return new ArrayNode($items);
@@ -588,10 +593,10 @@ class Debugger
      *
      * @param object $var Object to convert.
      * @param \Cake\Error\DumpContext $context The dump context.
-     * @return ClassNode|ReferenceNode
+     * @return \Cake\Error\DumpNode\NodeInterface
      * @see \Cake\Error\Debugger::exportVar()
      */
-    protected static function _object(object $var, DumpContext $context): NodeInterface
+    protected static function exportObject(object $var, DumpContext $context): NodeInterface
     {
         $isRef = $context->hasReference($var);
         $refNum = $context->getReferenceId($var);
@@ -603,16 +608,16 @@ class Debugger
         $node = new ClassNode($className, $refNum);
 
         $remaining = $context->remainingDepth();
-        if ($remaining > 0 && $isRef === false) {
+        if ($remaining > 0) {
             if (method_exists($var, '__debugInfo')) {
                 try {
                     foreach ($var->__debugInfo() as $key => $val) {
-                        $node->addProperty(new PropertyNode($key, null, static::_export($val, $context)));
+                        $node->addProperty(new PropertyNode("'{$key}'", null, static::export($val, $context)));
                     }
                 } catch (Exception $e) {
                     $message = $e->getMessage();
 
-                    return new ScalarNode('string', "(unable to export object: $message)");
+                    return new SpecialNode("(unable to export object: $message)");
                 }
             }
 
@@ -623,7 +628,7 @@ class Debugger
                     $value = $outputMask[$key];
                 }
                 $node->addProperty(
-                    new PropertyNode($key, 'public', static::_export($value, $context->withAddedDepth()))
+                    new PropertyNode($key, 'public', static::export($value, $context->withAddedDepth()))
                 );
             }
 
@@ -640,7 +645,11 @@ class Debugger
                     $value = $reflectionProperty->getValue($var);
 
                     $node->addProperty(
-                        new PropertyNode($key, $visibility, static::_export($value, $context->withAddedDepth()))
+                        new PropertyNode(
+                            $reflectionProperty->getName(),
+                            $visibility,
+                            static::export($value, $context->withAddedDepth())
+                        )
                     );
                 }
             }

src/Error/DumpContext.php

@@ -21,9 +21,8 @@ use SplObjectStorage;
 /**
  * Context tracking for Debugger::exportVar()
  *
- * This class is used by Debugger to track element depth,
- * and indentation. In the longer term indentation should be extracted
- * into a formatter (cli/html).
+ * This class is used by Debugger to track element depth, and
+ * prevent cyclic references from being traversed multiple times.
  *
  * @internal
  */
@@ -40,11 +39,6 @@ class DumpContext
     private $depth = 0;
 
     /**
-     * @var int
-     */
-    private $indent = 0;
-
-    /**
      * @var \SplObjectStorage
      */
     private $refs;
@@ -74,30 +68,6 @@ class DumpContext
     }
 
     /**
-     * Return a clone with increased depth and indent
-     *
-     * @return static
-     */
-    public function withAddedDepthAndIndent()
-    {
-        $new = clone $this;
-        $new->depth += 1;
-        $new->indent += 1;
-
-        return $new;
-    }
-
-    /**
-     * Get the current indent level.
-     *
-     * @return int
-     */
-    public function getIndent(): int
-    {
-        return $this->indent;
-    }
-
-    /**
      * Get the remaining depth levels
      *
      * @return int

src/Error/DumpFormatter/TextFormatter.php

@@ -8,6 +8,7 @@ use Cake\Error\DumpNode\ClassNode;
 use Cake\Error\DumpNode\NodeInterface;
 use Cake\Error\DumpNode\ReferenceNode;
 use Cake\Error\DumpNode\ScalarNode;
+use Cake\Error\DumpNode\SpecialNode;
 use RuntimeException;
 
 class TextFormatter
@@ -18,7 +19,7 @@ class TextFormatter
         return $this->_export($node, $indent);
     }
 
-    protected function _export($var, int $indent): string
+    protected function _export(NodeInterface $var, int $indent): string
     {
         if ($var instanceof ScalarNode) {
             switch ($var->getType()) {
@@ -38,6 +39,9 @@ class TextFormatter
         if ($var instanceof ClassNode || $var instanceof ReferenceNode) {
             return $this->_object($var, $indent + 1);
         }
+        if ($var instanceof SpecialNode) {
+            return (string)$var->getValue();
+        }
         throw new RuntimeException('Unknown node received ' . get_class($var));
     }
 
@@ -51,23 +55,19 @@ class TextFormatter
     protected function _array(ArrayNode $var, int $indent): string
     {
         $out = '[';
-        $break = $end = '';
-        if (!empty($var)) {
-            $break = "\n" . str_repeat("\t", $indent);
-            $end = "\n" . str_repeat("\t", $indent - 1);
-        }
+        $break = "\n" . str_repeat("  ", $indent);
+        $end = "\n" . str_repeat("  ", $indent - 1);
         $vars = [];
 
         foreach ($var->getChildren() as $item) {
             $val = $item->getValue();
-            // Sniff for globals as !== explodes in < 5.4
-            if ($item->getKey() === 'GLOBALS' && is_array($val) && isset($val['GLOBALS'])) {
-                $val = '[recursion]';
-            }
-            $vars[] = $break . $item->getKey() . ' => ' . $this->_export($val, $indent);
+            $vars[] = $break . $this->_export($item->getKey(), $indent) . ' => ' . $this->_export($val, $indent);
+        }
+        if (count($vars)) {
+            return $out . implode(',', $vars) . $end . ']';
         }
 
-        return $out . implode(',', $vars) . $end . ']';
+        return $out . ']';
     }
 
     /**
@@ -88,20 +88,23 @@ class TextFormatter
         }
 
         /* @var \Cake\Error\DumpNode\ClassNode $var */
-        $out .= "object({$var->getClass()}) id:{$var->getId()} {";
-        $break = "\n" . str_repeat("\t", $indent);
-        $end = "\n" . str_repeat("\t", $indent - 1);
+        $out .= "object({$var->getValue()}) id:{$var->getId()} {";
+        $break = "\n" . str_repeat("  ", $indent);
+        $end = "\n" . str_repeat("  ", $indent - 1) . '}';
 
         foreach ($var->getChildren() as $property) {
             $visibility = $property->getVisibility();
             $name = $property->getName();
-            if ($visibility) {
+            if ($visibility && $visibility !== 'public') {
                 $props[] = "[{$visibility}] {$name} => " . $this->_export($property->getValue(), $indent);
             } else {
                 $props[] = "{$name} => " . $this->_export($property->getValue(), $indent);
             }
         }
+        if (count($props)) {
+            return $out . $break . implode($break, $props) . $end;
+        }
 
-        return $out . $break . implode($break, $props) . $end;
+        return $out . '}';
     }
 }

src/Error/DumpNode/ArrayNode.php

@@ -5,6 +5,9 @@ namespace Cake\Error\DumpNode;
 
 class ArrayNode implements NodeInterface
 {
+    /**
+     * @var \Cake\Error\DumpNode\ItemNode[]
+     */
     private $items;
 
     public function __construct(array $items = [])
@@ -15,14 +18,12 @@ class ArrayNode implements NodeInterface
         }
     }
 
-    public function add(ItemNode $node)
+    public function add(ItemNode $node): void
     {
         $this->items[] = $node;
-
-        return $this;
     }
 
-    public function getValue()
+    public function getValue(): array
     {
         return $this->items;
     }

src/Error/DumpNode/ClassNode.php

@@ -5,8 +5,19 @@ namespace Cake\Error\DumpNode;
 
 class ClassNode implements NodeInterface
 {
+    /**
+     * @var string
+     */
     private $class;
+
+    /**
+     * @var int
+     */
     private $id;
+
+    /**
+     * @var \Cake\Error\DumpNode\PropertyNode[]
+     */
     private $properties = [];
 
     public function __construct(string $class, int $id)
@@ -18,11 +29,9 @@ class ClassNode implements NodeInterface
     public function addProperty(PropertyNode $node)
     {
         $this->properties[] = $node;
-
-        return $this;
     }
 
-    public function getClass(): string
+    public function getValue(): string
     {
         return $this->class;
     }

src/Error/DumpNode/ItemNode.php

@@ -8,7 +8,7 @@ class ItemNode implements NodeInterface
     private $key;
     private $value;
 
-    public function __construct($key, NodeInterface $value)
+    public function __construct(NodeInterface $key, NodeInterface $value)
     {
         $this->key = $key;
         $this->value = $value;

src/Error/DumpNode/NodeInterface.php

@@ -6,4 +6,6 @@ namespace Cake\Error\DumpNode;
 interface NodeInterface
 {
     public function getChildren(): array;
+
+    public function getValue();
 }

src/Error/DumpNode/PropertyNode.php

@@ -16,7 +16,7 @@ class PropertyNode implements NodeInterface
         $this->value = $value;
     }
 
-    public function getValue()
+    public function getValue(): NodeInterface
     {
         return $this->value;
     }

src/Error/DumpNode/ScalarNode.php

@@ -5,7 +5,14 @@ namespace Cake\Error\DumpNode;
 
 class ScalarNode implements NodeInterface
 {
+    /**
+     * @var string
+     */
     private $type;
+
+    /**
+     * @var string|int|float|bool|null
+     */
     private $value;
 
     public function __construct(string $type, $value)

src/Error/DumpNode/SpecialNode.php

@@ -0,0 +1,27 @@
+<?php
+declare(strict_types=1);
+
+namespace Cake\Error\DumpNode;
+
+class SpecialNode implements NodeInterface
+{
+    /**
+     * @var string
+     */
+    private $value;
+
+    public function __construct(string $value)
+    {
+        $this->value = $value;
+    }
+
+    public function getValue(): string
+    {
+        return $this->value;
+    }
+
+    public function getChildren(): array
+    {
+        return [];
+    }
+}

tests/TestCase/Error/DebuggerTest.php

@@ -312,55 +312,55 @@ class DebuggerTest extends TestCase
         $result = Debugger::exportVar($View);
         $expected = <<<TEXT
 object(Cake\View\View) id:0 {
-	Html => object(Cake\View\Helper\HtmlHelper) id:1 {}
-	Form => object(Cake\View\Helper\FormHelper) id:2 {}
-	int => (int) 2
-	float => (float) 1.333
-	string => '  '
-	[protected] _helpers => object(Cake\View\HelperRegistry) id:3 {}
-	[protected] Blocks => object(Cake\View\ViewBlock) id:4 {}
-	[protected] plugin => null
-	[protected] name => ''
-	[protected] helpers => [
-		(int) 0 => 'Html',
-		(int) 1 => 'Form'
-	]
-	[protected] templatePath => null
-	[protected] template => null
-	[protected] layout => 'default'
-	[protected] layoutPath => ''
-	[protected] autoLayout => true
-	[protected] viewVars => []
-	[protected] _ext => '.php'
-	[protected] subDir => ''
-	[protected] theme => null
-	[protected] request => object(Cake\Http\ServerRequest) id:5 {}
-	[protected] response => object(Cake\Http\Response) id:6 {}
-	[protected] elementCache => 'default'
-	[protected] _passedVars => [
-		(int) 0 => 'viewVars',
-		(int) 1 => 'autoLayout',
-		(int) 2 => 'helpers',
-		(int) 3 => 'template',
-		(int) 4 => 'layout',
-		(int) 5 => 'name',
-		(int) 6 => 'theme',
-		(int) 7 => 'layoutPath',
-		(int) 8 => 'templatePath',
-		(int) 9 => 'plugin'
-	]
-	[protected] _defaultConfig => []
-	[protected] _paths => []
-	[protected] _pathsForPlugin => []
-	[protected] _parents => []
-	[protected] _current => null
-	[protected] _currentType => ''
-	[protected] _stack => []
-	[protected] _viewBlockClass => 'Cake\View\ViewBlock'
-	[protected] _eventManager => object(Cake\Event\EventManager) id:7 {}
-	[protected] _eventClass => 'Cake\Event\Event'
-	[protected] _config => []
-	[protected] _configInitialized => true
+  Html => object(Cake\View\Helper\HtmlHelper) id:1 {}
+  Form => object(Cake\View\Helper\FormHelper) id:2 {}
+  int => (int) 2
+  float => (float) 1.333
+  string => '  '
+  [protected] _helpers => object(Cake\View\HelperRegistry) id:3 {}
+  [protected] Blocks => object(Cake\View\ViewBlock) id:4 {}
+  [protected] plugin => null
+  [protected] name => ''
+  [protected] helpers => [
+    (int) 0 => 'Html',
+    (int) 1 => 'Form'
+  ]
+  [protected] templatePath => null
+  [protected] template => null
+  [protected] layout => 'default'
+  [protected] layoutPath => ''
+  [protected] autoLayout => true
+  [protected] viewVars => []
+  [protected] _ext => '.php'
+  [protected] subDir => ''
+  [protected] theme => null
+  [protected] request => object(Cake\Http\ServerRequest) id:5 {}
+  [protected] response => object(Cake\Http\Response) id:6 {}
+  [protected] elementCache => 'default'
+  [protected] _passedVars => [
+    (int) 0 => 'viewVars',
+    (int) 1 => 'autoLayout',
+    (int) 2 => 'helpers',
+    (int) 3 => 'template',
+    (int) 4 => 'layout',
+    (int) 5 => 'name',
+    (int) 6 => 'theme',
+    (int) 7 => 'layoutPath',
+    (int) 8 => 'templatePath',
+    (int) 9 => 'plugin'
+  ]
+  [protected] _defaultConfig => []
+  [protected] _paths => []
+  [protected] _pathsForPlugin => []
+  [protected] _parents => []
+  [protected] _current => null
+  [protected] _currentType => ''
+  [protected] _stack => []
+  [protected] _viewBlockClass => 'Cake\View\ViewBlock'
+  [protected] _eventManager => object(Cake\Event\EventManager) id:7 {}
+  [protected] _eventClass => 'Cake\Event\Event'
+  [protected] _config => []
+  [protected] _configInitialized => true
 }
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -372,8 +372,8 @@ TEXT;
         $result = Debugger::exportVar($data);
         $expected = <<<TEXT
 [
-	(int) 1 => 'Index one',
-	(int) 5 => 'Index five'
+  (int) 1 => 'Index one',
+  (int) 5 => 'Index five'
 ]
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -386,9 +386,9 @@ TEXT;
         $result = Debugger::exportVar($data, 1);
         $expected = <<<TEXT
 [
-	'key' => [
-		[maximum depth reached]
-	]
+  'key' => [
+    '' => [maximum depth reached]
+  ]
 ]
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -403,7 +403,7 @@ TEXT;
         $file = fopen('php://output', 'w');
         fclose($file);
         $result = Debugger::exportVar($file);
-        $this->assertTextEquals('unknown', $result);
+        $this->assertTextEquals('(unknown)', $result);
     }
 
     /**
@@ -423,11 +423,11 @@ TEXT;
         $result = Debugger::exportVar($data);
         $expected = <<<TEXT
 [
-	'nothing' => '',
-	'null' => null,
-	'false' => false,
-	'szero' => '0',
-	'zero' => (int) 0
+  'nothing' => '',
+  'null' => null,
+  'false' => false,
+  'szero' => '0',
+  'zero' => (int) 0
 ]
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -451,11 +451,11 @@ TEXT;
         $result = Debugger::exportVar($parent, 6);
         $expected = <<<TEXT
 object(stdClass) id:0 {
-	name => 'cake'
-	child => object(stdClass) id:1 {
-		name => 'php'
-		child => object(stdClass) id:0 {}
-	}
+  name => 'cake'
+  child => object(stdClass) id:1 {
+    name => 'php'
+    child => object(stdClass) id:0 {}
+  }
 }
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -535,18 +535,18 @@ TEXT;
         $close = "\n\n";
         $expected = <<<TEXT
 {$open}[
-	'People' => [
-		(int) 0 => [
-			'name' => 'joeseph',
-			'coat' => 'technicolor',
-			'hair_color' => 'brown'
-		],
-		(int) 1 => [
-			'name' => 'Shaft',
-			'coat' => 'black',
-			'hair' => 'black'
-		]
-	]
+  'People' => [
+    (int) 0 => [
+      'name' => 'joeseph',
+      'coat' => 'technicolor',
+      'hair_color' => 'brown'
+    ],
+    (int) 1 => [
+      'name' => 'Shaft',
+      'coat' => 'black',
+      'hair' => 'black'
+    ]
+  ]
 ]{$close}
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -557,9 +557,9 @@ TEXT;
 
         $expected = <<<TEXT
 {$open}[
-	'People' => [
-		[maximum depth reached]
-	]
+  'People' => [
+    '' => [maximum depth reached]
+  ]
 ]{$close}
 TEXT;
         $this->assertTextEquals($expected, $result);
@@ -586,14 +586,14 @@ TEXT;
     }
 
     /**
-     * Test that exportVar() doesn't loop through recursive structures.
+     * Test that exportVar() will stop traversing recursive arrays like GLOBALS.
      *
      * @return void
      */
     public function testExportVarRecursion()
     {
         $output = Debugger::exportVar($GLOBALS);
-        $this->assertStringContainsString("'GLOBALS' => [recursion]", $output);
+        $this->assertRegExp("/'GLOBALS' => \[\s+'' \=\> \[maximum depth reached\]/", $output);
     }
 
     /**
@@ -623,10 +623,8 @@ TEXT;
         $result = Debugger::exportVar($object, 2);
         $expected = <<<eos
 object(TestApp\Error\Thing\DebuggableThing) id:0 {
-
-	'foo' => 'bar',
-	'inner' => object(TestApp\Error\Thing\DebuggableThing) id:1 {}
-
+  'foo' => 'bar'
+  'inner' => object(TestApp\Error\Thing\DebuggableThing) id:1 {}
 }
 eos;
         $this->assertEquals($expected, $result);
@@ -656,7 +654,7 @@ eos;
     {
         Debugger::setOutputMask(['password' => '[**********]']);
         $result = Debugger::exportVar(['password' => 'pass1234']);
-        $expected = "['password'=>[**********]]";
+        $expected = "['password'=>'[**********]']";
         $this->assertEquals($expected, preg_replace('/\s+/', '', $result));
     }
 
@@ -670,7 +668,7 @@ eos;
         Debugger::setOutputMask(['password' => '[**********]']);
         $object = new SecurityThing();
         $result = Debugger::exportVar($object);
-        $expected = 'object(TestApp\\Error\\Thing\\SecurityThing)id:0{password=>[**********]}';
+        $expected = "object(TestApp\\Error\\Thing\\SecurityThing)id:0{password=>'[**********]'}";
         $this->assertEquals($expected, preg_replace('/\s+/', '', $result));
     }