-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix assertions in tests #7831
Fix assertions in tests #7831
Conversation
e910a3f
to
3358429
Compare
src/Filter/Model/FilterData.php
Outdated
@@ -13,9 +13,6 @@ | |||
|
|||
namespace Sonata\AdminBundle\Filter\Model; | |||
|
|||
/** | |||
* @psalm-immutable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you removed this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because currently it's not really immutable
52 @readonly property Sonata\AdminBundle\Filter\Model\FilterData::$type
is assigned outside of the constructor.
56 @readonly property Sonata\AdminBundle\Filter\Model\FilterData::$value
is assigned outside of the constructor.
57 @readonly property
Sonata\AdminBundle\Filter\Model\FilterData::$hasValue is assigned
outside of the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is a bug in psalm, those properties are accessed in a named constructor, that doesn't make the class mutable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should ignore those false positives instead of removing the immutability
I don't consider this like a false positive. The
We're not writting them in the class-constructor but in a custom method I you want this class to be really immutable you should use the constructor ; it would be a BC PR since the constructor is private |
That's like if in the constructor you change the value multiple times, at the end is about getting a new object.
Being the constructor private is what prevents us from BC break, since we can change it and call it from |
8b22e70
to
8d9125e
Compare
No description provided.