Skip to content
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

Protected case class constructor -> non-public apply #14266

Merged
merged 1 commit into from Feb 21, 2022

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Jan 13, 2022

No description provided.

@dwijnand dwijnand linked an issue Jan 13, 2022 that may be closed by this pull request
@dwijnand dwijnand changed the title Protected case class constructor -> private apply Protected case class constructor -> non-public apply Jan 13, 2022
@dwijnand
Copy link
Member Author

dwijnand commented Jan 13, 2022

Coming back to protected apply: doesn't a protected apply method make the method available for subclasses of the case class? I guess I could write a test for that, either way. Done

@dwijnand dwijnand marked this pull request as ready for review January 14, 2022 10:00
@dwijnand dwijnand requested a review from odersky January 14, 2022 10:00
@dwijnand
Copy link
Member Author

@odersky do you consider it a bug that case classes with protected constructors are publicly constructible via the apply method?

Also, on a more technical note, is it preferable to keep things regular by just keeping things protected and qualified protected, or better to keep things sensical by making a protected constructor imply a private apply method?

@arturopala

This comment was marked as off-topic.

@dwijnand

This comment was marked as off-topic.

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a nice simplification for sure.

Is it true that the only effective change here is that if the constructor is protected, so is the apply method? Protected for the method does not really make sense here, since objects cannot be inherited. On the other hand protected also implies "package-private", and that's something we want. For clarity one might change this to a package private apply method instead. On the other hand protected is not wrong here, and it makes the code in desugar simpler.

@odersky odersky assigned dwijnand and unassigned odersky Jan 21, 2022
@dwijnand dwijnand merged commit c58f5c5 into scala:main Feb 21, 2022
@dwijnand dwijnand deleted the protected-apply branch February 21, 2022 14:05
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Case class with protected constructor has a public apply
4 participants