"getNodeById" returns non-existent nodes

[sebastianstucke87]

\BlueM\Tree::getNodeById() returns non-existent nodes:

$data = [];
$sut = new BlueM\Tree($data);

// #1: returns a "\BlueM\Tree\Node" (incorrect)
var_dump($sut->getNodeById(0));

// #2: throws InvalidArgumentException (correct)
var_dump($sut->getNodeById(1));

Expected behavior: when an id does not exist, an \InvalidArgumentException should be thrown.

---

Details

There seems to be a problem with 0 (int), '0' (string) and false (bool):

// arrange
$data = [];
$sut = new BlueM\Tree($data);

$cases = [-1, 0, 1, '-1', '0', '1', PHP_INT_MIN, PHP_INT_MAX, true, false, null];
foreach ($cases as $case) {
    try {
        // act
        $result = $sut->getNodeById($case)->toArray();

        // assert
        assert(!is_array($result), 'fail: FIXME');

    } catch (InvalidArgumentException $e) {
        var_dump(['case' => $case, 'result' => 'pass']);

    } catch (Throwable $e) {
        var_dump(['case' => $case, 'result' => $e->getMessage()]);
    }
}

Result:

array (size=2)
  'case' => int -1
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => int 0
  'result' => string 'fail: FIXME' (length=11)

array (size=2)
  'case' => int 1
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => string '-1' (length=2)
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => string '0' (length=1)
  'result' => string 'fail: FIXME' (length=11)

array (size=2)
  'case' => string '1' (length=1)
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => int -9223372036854775808
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => int 9223372036854775807
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => boolean true
  'result' => string 'pass' (length=4)

array (size=2)
  'case' => boolean false
  'result' => string 'fail: FIXME' (length=11)

array (size=2)
  'case' => null
  'result' => string 'pass' (length=4)

[BlueM]

0 is the default root node ID (see https://github.com/BlueM/Tree/blob/master/src/Tree.php#L29). So $sut->getNodeById(0) returns the root node, and this is the expected behaviour. Getting the root node for '0' and false can be explained thru implicit type coercion – which could of course be prevented using strict types, but these are edge cases, and I guess in the vast majority of cases, this would be rather counterproductive.

On the other hand, the library is inconsistent – there is a $rootId, but the node with that ID is none of the nodes returned by getRootNodes(), which is a surprising behavior. Basically, the mere existence of a root ID is an implementation detail, and probably it would be better if there was none, as it doesn’t have any practical use. I’ll look into this.

[BlueM]

As part of a large number of other changes I just pushed to master, there is no longer a root node – only the top-level nodes. (Which can be fetched by callling getRootNodes(). This was confusing, so ditching the root node is beneficial also in this regard.) Please check if this solves your problem; I think it does.