Empty array passed to foreach

[dbu]

Since updating from 0.10.6 to 0.11.1 we get false positives about supposedly empty arrays.

A minimal example to reproduce the problem is this:

<?php

namespace Test;

class Foo
{
    public $bar;

    public function baz(Foo $foo)
    {
        $foo->bar = ['a', 'b'];
    }

    public static function doStuff()
    {
        $foo = new Foo();
        $foo->bar = [];
        $foo->baz($foo);
        // phpstan 0.11.1 reports on this line that $foo->bar is always empty
        foreach ($foo->bar as $value) {
            var_dump($value);
        }
    }
}

IMHO the information that $foo->bar is empty should be discarded when we pass $foo to a method because that method might change the value of the object.

The same is when we call a method on the object itself:

<?php

namespace Test;

class Foo
{
    public $bar;

    public function baz()
    {
        $this->bar = ['a', 'b'];
    }

    public function doStuff()
    {
        $this->bar = [];
        $this->baz();
        // phpstan 0.11.1 reports on this line that $this->bar is always empty
        foreach ($this->bar as $value) {
            var_dump($value);
        }
    }
}

[muglug]

This is essentially a dupe of #929.

The only easy option is to allow people who write code this way to remove all knowledge of property types after any function or method call (but it needs to be a config that's off by default).

[ondrejmirtes]

I plan to solve this with pure methods that will have two features:

1. They always return the same thing, unless an impure method is called meanwhile. That also allows the $this->getName() ? $this->getName() : 'foo' case.
2. All properties are reset when an impure method is called.

Getters will be pure by default, additional methods will be configurable.

[muglug]

I don't think that approach is practical.

There are lots of complicated use cases that don't have an obvious answer e.g.

class C {
  public ?int $x;
  public function foo(int $x) : int {
    $this->x = 5;
    $this->maybePure();
    return $this->x;
  }
  public function maybePure() : void {}
  public function setNull() : void {
    $this->x = null;
  }
}
class D extends C {
  public function maybePure() : void {
    $this->setNull();
  }
}

[muglug]

There's also so many ways that functions can be called in PHP (e.g. magic getters, destructors) that aren't transparently obvious unless you have full type coverage.

[ondrejmirtes]

I think that your example verifies that my approach is useful, PHPStan would understand the code exactly as it behaves. But of course, if you put side-effects into your getters, it would be worse.

[dbu]

if you put side-effects into your getters, it would be worse.

if phpstan does semantics by method names, it would be consistent to also complain about side effects in getters. one side effect that i can think of off my head would be in a caching scenario, e.g. this code: https://github.com/symfony-cmf/Routing/blob/4762e60a04d9a1b4c825db0a0a864b307bbc8ec5/src/ChainRouter.php#L103-L119 (that method is not called get but it is used like one, and the caching is a speed optimisation for repeated calls)

[ondrejmirtes]

Yes, that's a great idea! Although side-effects are really hard to detect. For example - if there's some lazy initialization in the getter, it looks like a side-effect, but isn't one.

[ondrejmirtes]

The assignment is now forgotten if baz() method is void or is tagged with @phpstan-impure.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.