Fix notice returning reference in DotAccess::get

[NuclearDog]

Hey there!

Ran into an issue with the latest 0.4.x releases within Laravel. It presented as my watches seemingly registering and polling correctly, however they never actually called my callback with any events.

Tracking the problem down, I found in Watch::internal_iterator any exceptions thrown are being caught and not handled which led to no logging/output or other indication of a problem. Removing the try/catch, I was getting an exception thrown: Only variable references should be returned by reference.

This is likely to only apply to Laravel as it, by default, converts all PHP error logs into exceptions. Generally this would have gone to a log somewhere never to be seen again. However, it did uncover a potential problem.

The notice was being thrown by DotAccess::get where it's returning the value:

return $currentValue === null ? $default : $currentValue;

While the function's declared to return a reference to a variable, because of the ternary this can only ever return a reference to the result of an expression. That is, this is effectively equivalent to doing: function &add($n1, $n2) { return $n1 + $n2; }. The reference returned isn't actually to the $currentValue / value returned from propGet. This can be minimally demonstrated with:

<?php

class Example
{
    private array $my_data;

    public function __construct(array $my_data)
    {
        $this->my_data = $my_data;
    }

    public function &get(string $key, $default = null)
    {
        $value = &$this->my_data[$key];
        return $value === null ? $default : $value;
    }

}

$example = new Example(['a' => 1, 'b' => 2]);

$a = &$example->get('a'); // triggers a NOTICE
$a = 5;

var_dump($example->get('a')); // outputs 1

If you swap out the arrays in the example for objects, they exhibit the same issue. In both cases, the attached diff will correctly return the variable reference instead of a reference to the result of the expression--making the method match what appears to be intended behaviour and also allow watches to work again in Laravel and other frameworks that treat PHP's errors more severely.

Cheers, and thank you! Been using this lib for a bit to hack away at things on my home cluster. Appreciate the work you've put in.

[travisghansen]

Thanks! I’ll merge this in a bit and tag it.

I had to make major changes to support not decoding as an associative array (due to the ambiguity of how an empty array encodes). I ran into issues where I couldn’t retrieve a resource, modify it, and then patch it because of the array ambiguity so I had to normalize everything to support not caring about the type of decoded data. It seems mostly settled at this point but as you’ve seen may still have a few rough edges.

[NuclearDog]

Thanks a lot for getting that merged and released so quickly! Cheers!