Weird error regarding type closed-resource

[ADmad]

https://psalm.dev/r/8581e64e34

Actual code here https://github.com/cakephp/cakephp/blob/c46238303a3bd8a711c90c503dd1f12ba3656ebe/src/Network/Socket.php#L421

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/8581e64e34

<?php
class Socket {
    /** @var resource|null */
    protected $connection;
  
    /** @var bool */
    protected $connected = false;
  
    /**
     * Disconnect the socket from the current connection.
     *
     * @return bool Success
     */
    public function disconnect(): bool
    {
        if (!is_resource($this->connection)) {
            $this->connected = false;

            return true;
        }
        $this->connected = !fclose($this->connection);

        if (!$this->connected) {
            $this->connection = null;
        }

        return !$this->connected;
    }
}

Psalm output (using commit d99f23e):

ERROR: InvalidPropertyAssignmentValue - 21:36 - $this->connection with declared type 'null|resource' cannot be assigned type 'closed-resource'

[muglug]

So, PHP is fun.

If you close a resource, it's not resource anymore – calling is_resource(...) on it will return false.

Therefore its property type @var resource is no longer valid. Psalm's telling you you're violating that contract. You can change that to @psalm-var resource|closed-resource and Psalm should be happy (but anything that expects it to be a valid resource will break).

Psalm doesn't warn when you close resources directly inside __destruct methods (as CakePHP does in a few places), because it (now) understands that you may want to tear down class state, prevent memory cycles etc.

[ADmad]

If you close a resource, it's not resource anymore – calling is_resource(...) on it will return false.

Yeah, hence the is_resource() check at beginning of the method.

Therefore its property type @var resource is no longer valid. Psalm's telling you you're violating that contract.

Okay, I understand that technically it's no longer valid but IMHO this error is useless in most practically valid use cases. The currently code is perfectly valid.

You can change that to @psalm-var resource|closed-resource and Psalm should be happy (but anything that expects it to be a valid resource will break).

That just ends up throwing more errors wherever the property is used.

[muglug]

Yeah, I understand it's frustrating.

The workaround (from Psalm's point of view) is to put the connection into a separate variable, close that, then assign the actual property to null: https://psalm.dev/r/5961c19533

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/5961c19533

<?php
class Socket {
    /** @var resource|null */
    protected $connection;
  
    /** @var bool */
    protected $connected = false;
  
    /**
     * Disconnect the socket from the current connection.
     *
     * @return bool Success
     */
    public function disconnect(): bool
    {
        if (!is_resource($this->connection)) {
            $this->connected = false;

            return true;
        }

        $connection = $this->connection;
        $this->connected = !fclose($connection);

        if (!$this->connected) {
            $this->connection = null;
        }

        return !$this->connected;
    }
}

Psalm output (using commit d99f23e):

No issues!

[muglug]

Hopefully someone will come up with a proper object-based replacement for resource, and we can leave these weird shenanigans behind.

[ADmad]

The workaround (from Psalm's point of view) is to put the connection into a separate variable, close that, then assign the actual property to null: https://psalm.dev/r/5961c19533

Thanks, but I am just going to use @psalm-suppress here instead of jumping through hoops 🙂.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/5961c19533

<?php
class Socket {
    /** @var resource|null */
    protected $connection;
  
    /** @var bool */
    protected $connected = false;
  
    /**
     * Disconnect the socket from the current connection.
     *
     * @return bool Success
     */
    public function disconnect(): bool
    {
        if (!is_resource($this->connection)) {
            $this->connected = false;

            return true;
        }

        $connection = $this->connection;
        $this->connected = !fclose($connection);

        if (!$this->connected) {
            $this->connection = null;
        }

        return !$this->connected;
    }
}

Psalm output (using commit d99f23e):

No issues!

[muglug]

Yeah, makes sense. FWIW I added the __destruct checks after seeing false-positives on CakePHP’s code.