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

Scala support #843

Closed
wants to merge 6 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@JannikArndt

JannikArndt commented May 4, 2018

I added scala as an output format.

JannikArndt added some commits May 4, 2018

@schani

This comment has been minimized.

Collaborator

schani commented May 4, 2018

@JannikArndt That's amazing! Thank you.

Do you have plans to support union types eventually?

@schani

This comment has been minimized.

Collaborator

schani commented May 4, 2018

There are a few compile errors:

src/language/Scala.ts(132,26): error TS6138: Property '_packageName' is declared but its value is never read.
src/language/Scala.ts(133,26): error TS6138: Property '_justTypes' is declared but its value is never read.

Also, would you mind adding Scala to the test suite? See languages.ts in test, and the very bottom of fixtures.ts. Basically you'll have to write a little driver program (in test/fixtures/scala) that deserializes JSON and then serializes it back again, and you'll have to tell the test suite how to build and run it. The Dockerfile will also have to install Scala, I guess. Or maybe Maven does that automatically?

@JannikArndt

This comment has been minimized.

JannikArndt commented May 4, 2018

Union Types
There is no obvious and universal way to represent union types in Scala (that's the beauty of the language 😉). A colleague of mine suggested nested Either (okay for two types, ugly for more) or shapeless' hlist, which would add another dependency.
So to answer your question, if I plan to support union types eventually: If I find an adequate solution, I will definitely add it, but I'd rather not support them then add a somewhat broken support. And I am still tinkering with possible solutions.

Tests
Yeah, I did see that one coming when I looked at #838 😉 It's on my list.

Compile warnings
Fixed that 👍

@schani

This comment has been minimized.

Collaborator

schani commented May 4, 2018

I thought cases classes is how you represent unions in Scala? Isn't that the equivalent to discriminated unions in ML-like languages?

We'll wait for the test suite before merging.

@schani

This comment has been minimized.

Collaborator

schani commented May 4, 2018

Btw, if you need any help, it's probably quicker if you join us on Slack.

@schani

This comment has been minimized.

Collaborator

schani commented May 16, 2018

@JannikArndt What's the status on this? Can we help?

@JannikArndt

This comment has been minimized.

JannikArndt commented May 16, 2018

Hey there, sorry, I didn't have the time to write the tests, yet :(

@schani schani referenced this pull request Jun 11, 2018

Closed

Scala support #894

@hderms

This comment has been minimized.

hderms commented Jun 11, 2018

@JannikArndt is there anything wrong with just having a sealed trait of one argument case classes to represent union types? I suppose it would play weird with singleton types like None or something so I suppose that partially answers the question.

Dotty won't become Scala 3 until 2020 as far as I've heard, so I feel like there are precious limited options in the short term.

@hderms

This comment has been minimized.

hderms commented Jun 13, 2018

@JannikArndt any opposition to handling this like https://scalapb.github.io/generated-code.html

specifically the section on oneOf?

@JannikArndt

This comment has been minimized.

JannikArndt commented Jun 17, 2018

@hderms Sounds good, I personally like the ScalaPB style. I still didn't find the time to implement anything further, though :(

@schani

This comment has been minimized.

Collaborator

schani commented Jul 5, 2018

@JannikArndt @hderms I'd really like to get this closed. I'm happy to take over the work if you can give a short summary of what still needs to be done, from your point of view. If you could write a test driver (see test/fixtures/* for other language's drivers) I'd be much obliged, but I can figure it out myself if you don't have the time at all.

@hderms

This comment has been minimized.

hderms commented Jul 5, 2018

@schani I'm not sure what needs to be done still but I felt like imitating the way ScalaPB does union types would be a reasonable way to do oneOf

@schani

This comment has been minimized.

Collaborator

schani commented Aug 11, 2018

Doesn't look like this is moving forward, so I'm closing it for now. Please revive when/if there is new work.

@schani schani closed this Aug 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment