{"action":"created","issue":{"url":"https://api.github.com/repos/scalapb/ScalaPB/issues/458","repository_url":"https://api.github.com/repos/scalapb/ScalaPB","labels_url":"https://api.github.com/repos/scalapb/ScalaPB/issues/458/labels{/name}","comments_url":"https://api.github.com/repos/scalapb/ScalaPB/issues/458/comments","events_url":"https://api.github.com/repos/scalapb/ScalaPB/issues/458/events","html_url":"https://github.com/scalapb/ScalaPB/pull/458","id":332052325,"node_id":"MDExOlB1bGxSZXF1ZXN0MTk0NjMwNTA5","number":458,"title":"Introduce new \"sealed oneof\" encoding for a subset of oneofs","user":{"login":"olafurpg","id":1408093,"node_id":"MDQ6VXNlcjE0MDgwOTM=","avatar_url":"https://avatars0.githubusercontent.com/u/1408093?v=4","gravatar_id":"","url":"https://api.github.com/users/olafurpg","html_url":"https://github.com/olafurpg","followers_url":"https://api.github.com/users/olafurpg/followers","following_url":"https://api.github.com/users/olafurpg/following{/other_user}","gists_url":"https://api.github.com/users/olafurpg/gists{/gist_id}","starred_url":"https://api.github.com/users/olafurpg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/olafurpg/subscriptions","organizations_url":"https://api.github.com/users/olafurpg/orgs","repos_url":"https://api.github.com/users/olafurpg/repos","events_url":"https://api.github.com/users/olafurpg/events{/privacy}","received_events_url":"https://api.github.com/users/olafurpg/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-06-13T15:39:15Z","updated_at":"2018-06-19T16:12:24Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/scalapb/ScalaPB/pulls/458","html_url":"https://github.com/scalapb/ScalaPB/pull/458","diff_url":"https://github.com/scalapb/ScalaPB/pull/458.diff","patch_url":"https://github.com/scalapb/ScalaPB/pull/458.patch"},"body":"This PR changes the code generation for a subset of oneofs that can be translated to idiomatic Scala sealed traits. Messages containing such oneofs are called \"sealed oneof\" and must meet the following criteria:\r\n\r\n- all oneof fields are defined in the same file\r\n- all oneof field types are distinct top-level message types\r\n- the oneof name must be `sealed_value`\r\n- the containing type of the oneof has no fields outside of the oneof\r\n\r\nFor example, given the following proto definition\r\n```proto\r\nmessage Left { string message = 1; }\r\nmessage Right { int32 number = 1; }\r\nmessage Either {\r\n  oneof sealed_value {\r\n    Left left = 1;\r\n    Right right = 2;\r\n  }\r\n}\r\n```\r\nthe generated code will look roughly like this\r\n```scala\r\npackage a\r\ncase class Left(message: String) extends Either with scalapb.Message[Left]\r\ncase class Right(number: Int) extends Either with scalapb.Message[Right]\r\nsealed trait Either {\r\n  def toEitherMessage: EitherMessage\r\n}\r\nobject Either {\r\n  case object Empty extends Either\r\n  val defaultInstance: Either = Empty\r\n}\r\ncase class EitherMessage(sealedValue: EitherMessage.Value) extends scalapb.Message[EitherMessage] {\r\n  def asEither: Either\r\n  // ...\r\n}\r\nobject EitherMessage {\r\n  sealed trait Value\r\n  object Value {\r\n    case object Empty extends Value\r\n    case class Left(value: a.Left) extends Value\r\n    case class Right(value: a.Right extends Value\r\n  // normal oneof encoding for `message Either` ...\r\n}\r\n```\r\n\r\nAdditionally, when fields have a sealed oneof type they automatically get the `(scalapb.field).no_box = true` option. This allow you to concisely construct values for recursive ADTs.\r\n\r\nFor example, given the ADT below\r\n\r\n```proto\r\nmessage Lit {\r\n    int32 value = 1;\r\n}\r\n\r\nmessage Add {\r\n    Expr lhs = 1;\r\n    Expr rhs = 2;\r\n}\r\n\r\nmessage Expr {\r\n    oneof sealed_value {\r\n        Lit lit = 1;\r\n        Add add = 2;\r\n    }\r\n}\r\n```\r\n\r\nYou can construct a value like `Add(Lit(1), Add(Lit(2), Lit(3))`.\r\n\r\nFixes #455 .\r\n\r\nTODOs:\r\n- [ ] Decide on sealed oneof detection, should there be an option `(scalapb.message).sealed_oneof = true`;?\r\n- [ ] Add documentation to scalapb website\r\n- [ ] Add scalacheck generator for sealed oneofs, I would appreciate help here"},"comment":{"url":"https://api.github.com/repos/scalapb/ScalaPB/issues/comments/398456791","html_url":"https://github.com/scalapb/ScalaPB/pull/458#issuecomment-398456791","issue_url":"https://api.github.com/repos/scalapb/ScalaPB/issues/458","id":398456791,"node_id":"MDEyOklzc3VlQ29tbWVudDM5ODQ1Njc5MQ==","user":{"login":"thesamet","id":307373,"node_id":"MDQ6VXNlcjMwNzM3Mw==","avatar_url":"https://avatars0.githubusercontent.com/u/307373?v=4","gravatar_id":"","url":"https://api.github.com/users/thesamet","html_url":"https://github.com/thesamet","followers_url":"https://api.github.com/users/thesamet/followers","following_url":"https://api.github.com/users/thesamet/following{/other_user}","gists_url":"https://api.github.com/users/thesamet/gists{/gist_id}","starred_url":"https://api.github.com/users/thesamet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thesamet/subscriptions","organizations_url":"https://api.github.com/users/thesamet/orgs","repos_url":"https://api.github.com/users/thesamet/repos","events_url":"https://api.github.com/users/thesamet/events{/privacy}","received_events_url":"https://api.github.com/users/thesamet/received_events","type":"User","site_admin":false},"created_at":"2018-06-19T16:12:24Z","updated_at":"2018-06-19T16:12:24Z","author_association":"CONTRIBUTOR","body":"Hey @olafurpg , thanks for this PR, it looks really good. I started to work on this locally and make the necessary changes to get it merged. Great work there, the changes are mostly around my personal preferences.\r\n\r\nOne thought in the meantime: what's the reason behind restricting the oneof cases to only top-level messages? Anything prevents them to be anywhere in the file?  I am thinking that (at least for the first version), we can restrict the oneof messages to be nested inside the sealed oneof message. This ensures they get generated within the companion object and not into separate files. This eliminates the need to have a special case where single_file is false, and the need for a sealed oneofs cache (since the search for a parent would be constant time). If we do this, then the format for sealed oneofs would then be limited to this:\r\n\r\n```\r\nmessage Either {\r\n  message Left { string message = 1; }\r\n  message Right { int32 number = 1; }\r\n\r\n  oneof sealed_value {\r\n    Left left = 1;\r\n    Right right = 2;\r\n  }\r\n}\r\n```\r\n\r\nWDYT? Would that be too restrictive for your use case?\r\n\r\nAnother change I am making is that when there exists a oneof named `sealed_value`, code generation would abort if it doesn't pass the validation rules we defined (distinct message types, no other fields, etc)"}}
{"actor":{"display_login":"thesamet"}}