Fix some minor issues with references. #7684
Conversation
References assigned to properties on `$this` should never be unused. Using a `@var` docblock before a reference should be allowed if it targets a variable instead of the assignment statement.
AndrolGenhald
added
the
release:fix
| @@ -1004,6 +1005,15 @@ public static function analyzeAssignmentRef( | |||
| : "property-assignment-as-reference" | |||
| ); | |||
| } | |||
|
|
|||
| if ($root_var_id === '$this') { | |||
| // Variables on `$this` are always used | |||
There was a problem hiding this comment.
Do you mean that $this->whatever = $var constitutes a use of $var (but not necessarily $this->whatever)?
There was a problem hiding this comment.
No, it constitutes use of $this->whatever as well. We don't currently seem to track unused properties, and this caused me some issues.
There was a problem hiding this comment.
Reference assignment actually catches more than regular assignment for properties. I'm not too sure if it'd be better to just make it work the same or if non-reference assignment should be improved, so I decided to start small and make $this a special case.
There was a problem hiding this comment.
We don't currently seem to track unused properties,
We actually do, but you have to use the class itself and enable Detect unused classes and methods checkbox on psalm.dev: https://psalm.dev/r/031e79174f
There was a problem hiding this comment.
Ah, I missed that the class had to be used. That means this PR causes this to not mark Foo::$bar as unused, but I think that's an acceptable tradeoff for now. To fix it we'd need to somehow mark the assignment as used, but still keep the property itself as unused.
There was a problem hiding this comment.
Seems ok if it only involves references
|
Thanks! |
References assigned to properties on
$thisshould never be unused.Using a
@vardocblock before a reference should be allowed if it targets a variable instead of the assignment statement.