reset() modifies the array, but Psalm does not detect it

[BenMorel]

The following code:

final readonly class Foo
{
    private array $values;
    
    public function __construct(array $values)
    {
        $this->values = $values;
    }

    public function bar(): mixed
    {
        return reset($this->values);
    }
}

$x = new Foo([]);
$x->bar();

Yields the following error:

Fatal error: Uncaught Error: Cannot modify readonly property Foo::$values

But Psalm does not detect it: https://psalm.dev/r/e9d0177183
Same with array_shift(): https://psalm.dev/r/b37bb7be31

This probably applies to all functions with a parameter passed by reference.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/e9d0177183

<?php

final readonly class Foo
{
    private array $values;
    
    public function __construct(array $values)
    {
        $this->values = $values;
    }

    public function bar(): mixed
    {
        return reset($this->values);
    }
}

$x = new Foo([]);
$x->bar();

Psalm output (using commit 2bbfca6):

No issues!

https://psalm.dev/r/b37bb7be31

<?php

final readonly class Foo
{
    private array $values;
    
    public function __construct(array $values)
    {
        $this->values = $values;
    }

    public function bar(): mixed
    {
        return array_shift($this->values);
    }
}

$x = new Foo(['1']);
$x->bar();

Psalm output (using commit 2bbfca6):

No issues!

[orklah]

Oh that's a tough one.

Internal function is one thing but any reference on a readonly property is dangerous (but not necessarily, there are performance reasons why you would want references, even on a readonly property)

Maybe some kind of RiskyReferenceUsage would be needed here. That would warn for everything and it would be easier than to have some difference between references on internal function and reference on userland functions (and I'm pretty sure we would end up finding a perfectly fine case with an internal function that takes a reference but don't crash on readonly properties :p)

[BenMorel]

I’d personally be fine with RiskyReferenceUsage for any reference to a readonly property! 👍

I’m not aware of performance improvements when using references as PHP uses copy-on-write, so I don’t think there are many legitimate use cases anyway?

[orklah]

I’m not aware of performance improvements when using references as PHP uses copy-on-write, so I don’t think there are many legitimate use cases anyway?

Really? I was convinced that it made a difference! Good to know!