Skip to content

Conversation

@kocsismate
Copy link
Member

Based on the discussion of https://externals.io/message/118554

@kocsismate kocsismate force-pushed the readonly-class-inheritance branch from bf87586 to 7c88d56 Compare October 25, 2022 11:23
@derickr
Copy link
Member

derickr commented Oct 31, 2022

I don't think that readonly classes should be inheritable by non-readonly classes, as this would surely invalidate LSP compliance?

@deleugpn
Copy link

@derickr LSP is mostly about external factors about a class while readonly is about internals of a class. If one type-hints a readonly class, one shouldn't be doing $class->foo = 'bar'; anyway. Doing so means the user of the class is breaking LSP and relying on non-readonly children. I can't find any LSP reasoning that this should be prohibited.

Overall, I am against this change and I think it shouldn't be allowed, but I can't find any justification for why I feel this way.

@bdsl
Copy link

bdsl commented Dec 2, 2022

Agreeing with @deleugpn I don't see an LSP problem with a non-readonly class extending from readonly. As Wikipedia puts it: "Thus, one can define a circle with immutable center and mutable radius as a subtype of an immutable point without violating the history constraint."

Any additional fields added by a child class should generally be invisible to code that depends only on the parent class.

@kocsismate
Copy link
Member Author

@kocsismate kocsismate closed this May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants