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

Make the trait sealed by default #521

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Fokko
Copy link

@Fokko Fokko commented Nov 10, 2019

Fixes #501

I'd like to suggest making the trait sealed by default. A sealed trait requires to have all the classes that it extends in the same file. Which is the case with scalaxb. Having a sealed trait makes it easier to spot errors because the compiler will drop warnings.

@eed3si9n
Copy link
Owner

Maybe this should be opt-in. I think there are situations where scalaxb would produce datatypes crossing multiple files when there are multiple schemas involved, and also I'd love to move code generation that's more modular, similar to what was done in https://github.com/sbt/contraband.

@Fokko
Copy link
Author

Fokko commented Nov 14, 2019

Good point, I'll make it configurable, somewhere this weekend probably.

@dalegaspi
Copy link

i am revisiting this from #501 and the code change can't be like this, because this assumes that all generated traits are sealed...however, any generated traits that employ inheritance will not compile (as shown in the failed test) because (some?) of the traits are generated in different source file which violates the sealed property (sealed traits and its inherited objects/traits need to be in the same source file).

I am haven't looked at this in a while so I have no clue as to how to make this work...maybe add an option to configure the types to have a root trait as sealed and ensure that they are all generated in the same source file?

@deenar
Copy link
Contributor

deenar commented Nov 3, 2020

@dalegaspi @Fokko I was looking into this too as I was some of the typelevel Cats type classes and run into the same issue. I would too prefer if making all generated traits sealed was an opt-in.

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.

Use Sealed Trait As Base Class?
4 participants