vimeo / psalm

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

@psalm-immutable is not the same as adding @psalm-readonly/@psalm-mutation-free #9907

Open kkmuffme opened 1 year ago

kkmuffme commented 1 year ago

Used to annotate a class where every property is treated by consumers as @psalm-readonly and every instance method is treated as @psalm-mutation-free

But it seems this isn't true: https://psalm.dev/r/2d7cd6c403 When using with immutable, the errors are different: https://psalm.dev/r/c68a1c73d6

You could argue: the class might be extended and then that wouldn't be true anymore - however this is irrelevant for what is in the class for now, and has to be adjusted in the extending class. However, adding final to disallow extending, still gives the same errors: https://psalm.dev/r/67c22e0ca6

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/2d7cd6c403 ```php bar = $arg; } /** * @psalm-mutation-free * @psalm-pure * @return string */ public function get_bar() { return $this->bar; } } ``` ``` Psalm output (using commit 2c50745): ERROR: ImpureVariable - 20:14 - Cannot reference $this in a pure context ERROR: ImpurePropertyFetch - 20:14 - Cannot access a property on a mutable object from a pure context ```
https://psalm.dev/r/34441531c4 ```php bar = $arg; } /** * @psalm-pure * @return string */ public function get_bar() { return $this->bar; } } ``` ``` Psalm output (using commit 2c50745): ERROR: ImpureVariable - 19:14 - Cannot reference $this in a pure context ERROR: ImpurePropertyFetch - 19:14 - Cannot access a property on a mutable object from a pure context ```
https://psalm.dev/r/67c22e0ca6 ```php bar = $arg; } /** * @psalm-mutation-free * @psalm-pure * @return string */ public function get_bar() { return $this->bar; } } ``` ``` Psalm output (using commit 2c50745): ERROR: ImpureVariable - 20:14 - Cannot reference $this in a pure context ERROR: ImpurePropertyFetch - 20:14 - Cannot access a property on a mutable object from a pure context ```
orklah commented 1 year ago

Your two first link shows exactly the same errors

kkmuffme commented 1 year ago

Sorry, copied the wrong one, fixed now.

ygottschalk commented 1 year ago

@readonly @mutation-free: https://psalm.dev/r/232ed66d3b @immutable: https://psalm.dev/r/5c9610e802

Both show the same (aka no) errors.

You had a @pure on the method which caused the errors...

psalm-github-bot[bot] commented 1 year ago

I found these snippets:

https://psalm.dev/r/232ed66d3b ```php bar = $arg; } /** * @psalm-mutation-free * @return string */ public function get_bar() { return $this->bar; } } ``` ``` Psalm output (using commit 2c50745): No issues! ```
https://psalm.dev/r/5c9610e802 ```php bar = $arg; } /** * @return string */ public function get_bar() { return $this->bar; } } ``` ``` Psalm output (using commit 2c50745): No issues! ```
kkmuffme commented 1 year ago

I know that the @pure caused the errors, but why are the errors different depending on whether I use immutable on the class or readonly/mutation-free on all properties?

ygottschalk commented 1 year ago

bcs you can actually 'use' an immutable object in a pure context.

Since the @immutable annotation gives psalm the guarantee that really all properties are readonly and all methods do not mutate, the ImpurePropertyFetch issue will be gone.

Used to annotate a class where every property is treated by consumers as @psalm-readonly and every instance method is treated as @psalm-mutation-free

The docs never claim, that it is the same to use either @immutable or @readonly @mutation-free, but state that using @immutable will make every property be treated like having the @readonly and every method like having @mutation-free. BUT: Using @immutable gives knowlege about the 'bigger picture', therefore psalm does know more about the class itself.

ygottschalk commented 1 year ago

Another difference is, if you use @immutable, all child classes need to be marked as @immutable, too.