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.

Sign up for GitHub

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

Jump to bottom

Generated code is invalid for well-known types #70

Open
andrew-selvia opened this issue Oct 22, 2023 · 7 comments · May be fixed by #72
Open

Generated code is invalid for well-known types #70

andrew-selvia opened this issue Oct 22, 2023 · 7 comments · May be fixed by #72

Comments

@andrew-selvia
Copy link

Based on my experiments, if a message contains a well-known type, http4s-grpc produces invalid code. For example, this protobuf:

syntax = "proto3";
package com.selvia.andrew.demo;
import "google/protobuf/empty.proto";
import "google/protobuf/wrappers.proto";
service Foo {
  rpc Bar (google.protobuf.Empty) returns (stream google.protobuf.Int64Value) {}
}

produces the following code:

trait Foo[F[_]] { 
  def bar(request: _root_.com.google.protobuf.empty.Empty, ctx: _root_.org.http4s.Headers): _root_.fs2.Stream[F, _root_._root_.scala.Long]
}

Notice how the return type has the invalid type prefix _root_._root_.

ScalaPB seems to handle these alright. Will some additional sbt configuration make this work or is this a bug?
@ChristopherDavenport
Copy link
Member

This seems like a bug, I think we may have prefixed with root which ends up wrong. 🤔

@armanbilge armanbilge linked a pull request Oct 23, 2023 that will close this issue
Draft
@armanbilge
Copy link
Member

I have a fix for this in #72, but unfortunately more things are broken for well-known types 😕

For example this generated code is broken:

_root_.org.http4s.grpc.codecs.ScalaPb.codecForGenerated(_root_.scala.Long)

@armanbilge
Copy link
Member

Actually it's not clear to me at all how to use a Long as a message type 🤔 I think we are going to have to redirect through the Int64Value wrapper type somehow.

@andrew-selvia
Copy link
Author

ScalaPB seems to be unwrapping into Long automatically, though that really isn't a blocker. Routing through Int64Value is preferable to invalid code.

@armanbilge
Copy link
Member

armanbilge commented Oct 24, 2023 •
edited

Yes, to be clear, I'm saying we should still keep Long in the signature, just behind-the-scenes we need to encode that through the Int64Value wrapper to get a valid message.

@andrew-selvia
Copy link
Author

Just confirming, #72 isn't fully ready to merge due to the issue mentioned here, right?

@armanbilge
Copy link
Member

Ugh, yeah. ScalaPB does it so presumably we should be able to copy/adapt their approach.

@danicheg danicheg mentioned this issue May 7, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't prefix _root_ when referring to scala type http4s/http4s-grpc
3 participants
@armanbilge @andrew-selvia @ChristopherDavenport