vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.49k stars 655 forks source link

InaccessibleProperty when using fclose in destructor of a readonly class #10308

Open vudaltsov opened 8 months ago

vudaltsov commented 8 months ago

https://psalm.dev/r/edb33c5ba5 https://psalm.dev/r/2540af9e0d

This code works fine: https://3v4l.org/ZuaRD https://3v4l.org/QBFIh

psalm-github-bot[bot] commented 8 months ago

I found these snippets:

https://psalm.dev/r/edb33c5ba5 ```php resource = fopen($file, 'r'); } public function close(): void { fclose($this->resource); } public function rewind(): void { rewind($this->resource); } } (new File(__FILE__))->close(); ``` ``` Psalm output (using commit 75fcfe3): ERROR: InaccessibleProperty - 17:16 - File::$resource is marked readonly ERROR: InvalidPropertyAssignmentValue - 17:16 - $this->resource with declared type 'resource' cannot be assigned type 'closed-resource' ```
robchett commented 8 months ago

While valid it does open you up to throwing a typeError

https://3v4l.org/kljvE#v8.2.11

Maybe a __destruct is a safer way to do this?

https://psalm.dev/r/edb33c5ba5

psalm-github-bot[bot] commented 8 months ago

I found these snippets:

https://psalm.dev/r/edb33c5ba5 ```php resource = fopen($file, 'r'); } public function close(): void { fclose($this->resource); } public function rewind(): void { rewind($this->resource); } } (new File(__FILE__))->close(); ``` ``` Psalm output (using commit 147505c): ERROR: InaccessibleProperty - 17:16 - File::$resource is marked readonly ERROR: InvalidPropertyAssignmentValue - 17:16 - $this->resource with declared type 'resource' cannot be assigned type 'closed-resource' ```
vudaltsov commented 8 months ago

In my code I have it in __destruct, of course. I copied example above after some experiments. So, yeah, your version is better.

Technically I understand why Psalm is complaining: from Psalm's perspective type changes from resource to closed-resource. From PHP's it doesn't.