Never rely on dynamic properties #44037
Conversation
0d01bd6 to
4de0371
Compare
|
(PR is ready) |
| @@ -18,6 +18,9 @@ | |||
| */ | |||
| class SMimePart extends AbstractPart | |||
| { | |||
| /** @internal */ | |||
| public $_headers; | |||
There was a problem hiding this comment.
I've made it protected and all similar ones too.
I prefer to not make them private to be extra sure not to break inheritance.
@internal+protected looks safe enough to me.
4de0371 to
dc8f70c
Compare
dc8f70c to
aa4fb0c
Compare
|
build failures seems related |
|
Failures are not related to me. They happen because 3.4 is checked out and fails in flipped tests. |
|
Thank you @nicolas-grekas. |
|
Hello guys, We have an issue with this PR and our usage of PhpSpec when asserting for Exception (using the For instance, we are using it on an assertion like $this->shouldThrow(Symfony\Component\Security\Core\Exception\AuthenticationException::class`)The PhpSpec matcher will then iterate on the reflected Exception class properties to compare them, but willl crash here because the property does not exists anymore because it was unset in the constructor. Do you have an idea how we should deal with this issue? Regards, |
|
I think it can create issues with every tool using reflection. |
|
@jmleroux thanks for the feedback. From what you describe this looks like an issue with the tool, that doesn't properly use reflection since it doesn't (yet) know how to deal with a situation that can exist with any class. Isn't it? |
I could agree 🙂 but it is also very unusual to unset a property as the first constructor action, isn't it? To be honest, I didn't understand this |
|
I understand. It's still something that is legal and has a clear purpose in PHP. |
|
I didn't see this use case was documented in the "Backward Incompatible Changes". 😶 Thank you for quick and clear answers @nicolas-grekas ! |
As described in https://wiki.php.net/rfc/deprecate_dynamic_properties