some edge-case tests and prevention

src/Exceptions/CallableIssue.php

@@ -0,0 +1,18 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Dakujem\Oliva\Exceptions;
+
+use LogicException;
+
+/**
+ * A problem caused by an exception thrown while running a callable.
+ * This exception is provided to enable context for external exceptions.
+ *
+ * @author Andrej Rypak <xrypak@gmail.com>
+ */
+final class CallableIssue extends LogicException implements FailsIntegrity, AcceptsDebugContext
+{
+    use AcceptDebugContext;
+}

src/MaterializedPath/Path.php

@@ -61,6 +61,9 @@ public static function delimited(string $delimiter, callable $accessor): callabl
      */
     public static function fixed(int $levelWidth, callable $accessor): callable
     {
+        if ($levelWidth < 1) {
+            throw new ConfigurationIssue('The level width must be a positive integer.');
+        }
         return function (mixed $data, mixed $inputIndex = null, ?TreeNodeContract $node = null) use (
             $levelWidth,
             $accessor,

src/Simple/TreeWrapper.php

@@ -5,10 +5,12 @@
 namespace Dakujem\Oliva\Simple;
 
 use Dakujem\Oliva\Exceptions\AcceptsDebugContext;
+use Dakujem\Oliva\Exceptions\CallableIssue;
 use Dakujem\Oliva\Exceptions\ExtractorReturnValueIssue;
 use Dakujem\Oliva\Exceptions\InvalidNodeFactoryReturnValue;
 use Dakujem\Oliva\MovableNodeContract;
 use Dakujem\Oliva\TreeNodeContract;
+use Throwable;
 
 /**
  * Simple tree builder.
@@ -55,22 +57,36 @@ private function wrapNode(
         callable $nodeFactory,
         callable $childrenExtractor,
     ): MovableNodeContract {
-        // Create a node using the provided factory.
-        $node = $nodeFactory($data);
+        try {
+            // Create a node using the provided factory.
+            $node = $nodeFactory($data);
 
-        // Check for consistency.
-        if (!$node instanceof MovableNodeContract) {
-            throw (new InvalidNodeFactoryReturnValue())
-                ->tag('node', $node)
-                ->tag('data', $data);
+            // Check for consistency.
+            if (!$node instanceof MovableNodeContract) {
+                throw (new InvalidNodeFactoryReturnValue())
+                    ->tag('node', $node)
+                    ->tag('data', $data);
+            }
+
+            $childrenData = $childrenExtractor($data, $node);
+        } catch (Throwable $e) {
+            if (!$e instanceof AcceptsDebugContext) {
+                // wrap the exception so that it supports context decoration
+                $e = (new CallableIssue(
+                    message: $e->getMessage(),
+                    code: $e->getCode(),
+                    previous: $e,
+                ));
+            }
+            throw $e->push('nodes', $node ?? null);
         }
 
-        $childrenData = $childrenExtractor($data, $node);
         if (null !== $childrenData && !is_iterable($childrenData)) {
             throw (new ExtractorReturnValueIssue('Children data extractor must return an iterable collection containing children data.'))
                 ->tag('children', $childrenData)
                 ->tag('parent', $node)
-                ->tag('data', $data);
+                ->tag('data', $data)
+                ->push('nodes', $node);
         }
         foreach ($childrenData ?? [] as $key => $childData) {
             try {

tests/nodes.phpt

@@ -4,6 +4,8 @@ declare(strict_types=1);
 
 namespace Dakujem\Test;
 
+use Dakujem\Oliva\Exceptions\AcceptsDebugContext;
+use Dakujem\Oliva\Exceptions\Context;
 use Dakujem\Oliva\Node;
 use Dakujem\Oliva\Simple\NodeBuilder;
 use Dakujem\Oliva\Simple\TreeWrapper;
@@ -78,11 +80,65 @@ require_once __DIR__ . '/setup.php';
             ],
         ],
     ];
-    $wrapper = new TreeWrapper(fn(array $raw) => new Node($raw['data']), fn(array $raw) => $raw['children'] ?? null);
+    $wrapper = new TreeWrapper(
+        fn(array $raw) => new Node($raw['data']),
+        fn(array $raw): ?iterable => $raw['children'] ?? null,
+    );
     $root = $wrapper->wrap($data);
 
     // pre-order
     Assert::same('FBADCEGIH', TreeTesterTool::flatten($root));
+
+    $faulty = $data;
+    $thrown = false;
+    try {
+        $faulty['children'][1]['children'][] = ['data' => 'this will cause type error inside the extractor', 'children' => 42];
+        $wrapper->wrap($faulty);
+    } catch (AcceptsDebugContext $e) {
+        /** @var Context $context */
+        $context = $e->context;
+        Assert::count(3, $context->bag['nodes'] ?? []);
+        Assert::same(
+            ['this will cause type error inside the extractor', 'G', 'F'],
+            array_map(fn(Node $node) => $node->data(), $context->bag['nodes']),
+        );
+        $thrown = true;
+    }
+    Assert::true($thrown, 'The catch block has to run');
+
+    $faultyWrapper = new TreeWrapper(
+        fn(array $raw) => new Node($raw['data']),
+        function (array $raw): mixed {
+            if (isset($raw['children']) && !is_iterable($raw['children'] ?? null)) {
+                return 'fault';
+            }
+            return $raw['children'] ?? null;
+        },
+    );
+
+    $thrown = false;
+    try {
+        $faultyWrapper->wrap($faulty);
+    } catch (AcceptsDebugContext $e) {
+        /** @var Context $context */
+        $context = $e->context;
+        Assert::count(3, $context->bag['nodes'] ?? []);
+        Assert::same(
+            ['this will cause type error inside the extractor', 'G', 'F'],
+            array_map(fn(Node $node) => $node->data(), $context->bag['nodes']),
+        );
+        // the "children" extracted by the extractor
+        Assert::same('fault', $context->bag['children'] ?? null);
+        // the data that was present
+        Assert::same([
+            'data' => 'this will cause type error inside the extractor',
+            'children' => 42,
+        ], $context->bag['data'] ?? null);
+        // ref to the parent node
+        Assert::type(Node::class, $context->bag['parent'] ?? null);
+        $thrown = true;
+    }
+    Assert::true($thrown, 'The catch block has to run');
 })();
 
 

tests/path.phpt

@@ -0,0 +1,101 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Dakujem\Test;
+
+use Dakujem\Oliva\Exceptions\ConfigurationIssue;
+use Dakujem\Oliva\Exceptions\Context;
+use Dakujem\Oliva\Exceptions\InvalidTreePath;
+use Dakujem\Oliva\MaterializedPath\Path;
+use Dakujem\Oliva\Node;
+use Tester\Assert;
+
+require_once __DIR__ . '/setup.php';
+
+// Test some edge cases of Path::delimited
+(function () {
+    Assert::throws(function () {
+        Path::delimited('', fn() => null);
+    }, ConfigurationIssue::class, 'The delimiter must be a single character.');
+    Assert::throws(function () {
+        Path::delimited('42', fn() => null);
+    }, ConfigurationIssue::class, 'The delimiter must be a single character.');
+
+    Assert::throws(function () {
+        $extractor = Path::delimited('.', fn() => 42);
+        $extractor(null);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+
+    $extractor = Path::delimited('.', fn($data) => $data);
+    Assert::same([], $extractor(null));
+    Assert::same([], $extractor(''));
+    Assert::same(['a string'], $extractor('a string'));
+    Assert::same(['a', 'string'], $extractor('a.string'));
+    Assert::same(['  a', '  string  '], $extractor('  a.  string  ')); // no trim implemented
+
+    Assert::throws(function () use ($extractor) {
+        $extractor(42);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+    Assert::throws(function () use ($extractor) {
+        $extractor([]);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+    Assert::throws(function () use ($extractor) {
+        $extractor(4.2);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+    Assert::throws(function () use ($extractor) {
+        $extractor(new Node(null));
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+
+    $extractor = Path::delimited('.', fn() => 42);
+    $thrown = false;
+    $node = new Node('empty');
+    try {
+        $extractor('foo', 'bar', $node);
+    } catch (InvalidTreePath $e) {
+        /** @var Context $context */
+        $context = $e->context;
+        Assert::type(Context::class, $context);
+        Assert::same([
+            'path' => 42,
+            'data' => 'foo',
+            'index' => 'bar',
+            'node' => $node,
+        ], $context->bag);
+        $thrown = true;
+    }
+    Assert::true($thrown, 'The catch block has not been executed.');
+})();
+
+// Test some edge cases of Path::fixed
+(function () {
+    Assert::throws(function () {
+        Path::fixed(0, fn() => null);
+    }, ConfigurationIssue::class, 'The level width must be a positive integer.');
+    Assert::throws(function () {
+        Path::fixed(-1, fn() => null);
+    }, ConfigurationIssue::class, 'The level width must be a positive integer.');
+
+    $extractor = Path::fixed(1, fn($data) => $data);
+    Assert::same(['a', 'b', 'c'], $extractor('abc'));
+    Assert::same([], $extractor(''));
+
+    $extractor = Path::fixed(2, fn($data) => $data);
+    Assert::same(['ab', 'cd'], $extractor('abcd'));
+    Assert::same(['  ', 'ab', 'cd', '  '], $extractor('  abcd  ')); // no trim implemented
+    Assert::same([], $extractor(''));
+
+    Assert::throws(function () use ($extractor) {
+        $extractor(42);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+    Assert::throws(function () use ($extractor) {
+        $extractor([]);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+    Assert::throws(function () use ($extractor) {
+        $extractor(4.2);
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+    Assert::throws(function () use ($extractor) {
+        $extractor(new Node(null));
+    }, InvalidTreePath::class, 'Invalid tree path returned by the accessor. A string is required.');
+})();
+