Demote LSP violation to warning even if parent implements abstract #2358
Conversation
|
Travis failure seems unrelated. |
| if (child->common.prototype && ( | ||
| child->common.prototype->common.fn_flags & ZEND_ACC_ABSTRACT | ||
| )) { | ||
| if (parent->common.fn_flags & ZEND_ACC_ABSTRACT) { |
There was a problem hiding this comment.
This doesn't look quite right to me. Doesn't think mean that if B extends A implements I, and B violates I (and not just A), then you will only get a warning, rather than a fatal error?
There was a problem hiding this comment.
If B extends A implements I and B violates I then you should get a fatal. The whole goal is to eventually fatal for all incompatibilties. The only reason I can see to not give a fatal is if we previously didn't fatal, in which case it should probably wait until 8.0.
There was a problem hiding this comment.
I see, so basically this test should still fatal. Correct?
There was a problem hiding this comment.
@pmmaga Correct. However I don't see a great way of implementing that, short of going two LSP checks, one against the interface and one against the class. Not sure if we want to do that...
|
This PR would demote any incompatibility with interfaces to a Warning if this interface was implemented by the parent of the current method. This is not the desired effect and the original behavior is preferred. |
After #2342, an LSP violation over the number of required parameters was promoted to a Fatal Error if the parent implements an interface or abstract.
This PR fixes that behaviour.