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

proposal: protoc-gen-go: support generating unexported types #501

Closed
rogpeppe opened this issue Jan 30, 2018 · 10 comments
Closed

proposal: protoc-gen-go: support generating unexported types #501

rogpeppe opened this issue Jan 30, 2018 · 10 comments
Labels

Comments

@rogpeppe
Copy link

Currently the names of the types generated for the protobuf messages are always capitalised, and hence exported.

Sometimes it is not desirable to advertise the fact that a package is using protobuf to encode its data. We may wish to export types with a more restrictive set of methods, or perhaps not export them at all if (for example) they're only used as an internal serialisation detail.

Currently we're working around this by generating the protobuf code inside an internal package and then having the public package import that, but this is heavyweight and not ideal when a simple option, say:

  option go_autoexport = false

could work as just as well without the necessity to add two directory levels and an extra package.

@puellanivis
Copy link
Collaborator

A central problem to your idea here is that due to the rules of reflection in Go, the protobuf library cannot marshal or unmarshal to unexported fields.

Working around this would be a) too clever, and b) require malicious use of the unsafe library

@rogpeppe
Copy link
Author

A central problem to your idea here is that due to the rules of reflection in Go, the protobuf library cannot marshal or unmarshal to unexported fields.

I wasn't suggesting that the fields be unexported, merely that the types themselves would be unexported. There's no problem marshaling and unmarshaling to exported fields in unexported types in Go (for example https://play.golang.org/p/cSa3nBncQh0)

@puellanivis
Copy link
Collaborator

Ah yes, indeed…

Although, next how would you access the types at all then? Or are you thinking of putting your code in the same directory/package as the pb.go generated file itself?

That seems… unusual to me… but then I’m perhaps overly accustomed to always isolating protobuf packages from everything else…

@rogpeppe
Copy link
Author

Yes, I'd like to be able to put the code in the same package as the pb.go generated file itself. I don't see that there's any particular need for protobuf packages to be isolated, other than the fact that the generated files produce so much godoc noise - this issue is about being able to silence that.

@puellanivis
Copy link
Collaborator

Well, the reason I rigorously isolate my proto packages is because they are by their nature individual/isolatable concepts, and I try to practice defensive isolation in general…

I can’t really say that breaking this isolation would be wrong… it’s just… I dunno… weird to me. ;)

But ok, I understand what’s going now, and will bow out, as I have no further useful purpose to this thread.

@awalterschulze
Copy link

Despite my perceived tone, I don't want to pick a fight, I am truly interested.
Currently I recommend the the leaky approach to generated protobufs for safety and performance reasons, but maybe I am wrong.
I actually do not know how to isolate the generated proto code from the rest of my code.
If I copy from the generated proto struct to another struct inside my code, then that will result in an error prone and expensive copy function, which is unacceptable to me.
So is there some other way to keep it isolated and still keep the speed and maintainability?

@puellanivis
Copy link
Collaborator

@awalterschulze I think it is unclear as to whom you are speaking.

Though to be clear about what I mean when I say “isolate”, I mean that the only .go files in any given protobuf package are the .pb.go files alone. Everything else then imports that package and uses pointers to the protobuf structs/types/etc.

@awalterschulze
Copy link

@puellanivis I am speaking to you or anyone who can answer my question.

Ah ok so there are two definitions of isolate:

  1. isolate in a package, but not from the rest of the code (your definition)
  2. isolate from the rest of the code and create a barrier (my definition)

So yes 1 is very much possible and not inefficient and still safe, but I struggle to not see the benefit of not allowing to add extra methods to the struct, by adding more code to the same package? To me it is just another struct. For example, when I use a protobuf as an AST ("parse tree") it is nice to also add constructors and Walk methods to the same package.
https://github.com/katydid/katydid/tree/master/relapse/ast

In general if someone knows how to do 2 efficiently and make it easy to switch from protobuf to a newer serialization format in future, that will also be interesting, but I don't expect an answer.

@rogpeppe
Copy link
Author

All I'm asking is that we get the choice whether to hide protobuf types. I'm not saying that it's appropriate to do so in all cases.

I actually do not know how to isolate the generated proto code from the rest of my code.

I've been doing that by putting the protobuf code into an internal package (for example https://godoc.org/gopkg.in/macaroon-bakery.v2-unstable/bakery/internal/macaroonpb), but really I don't see why that should be mandated.

I'd like to be able to have a protobuf-generated type that isn't exported so the type isn't visible anywhere else. Then I can use it as I wish. For example I could put it into a wrapper that only implements the methods I wish to support. The fact that the code uses protobufs becomes a hidden implementation detail.

type ExportedType struct {
	inner unexportedProtobufType
}

func (t *ExportedType) UnmarshalBinary(data []byte) error {
	return proto.Unmarshal(data, &t.inner)
}

func (t *ExportedType) MarshalBinary() ([]byte, error) {
	return proto.Marshal(&t.inner)
}

func (t *ExportedType) Foo() int {
	return t.inner.Foo
}

@dsnet dsnet added the proposal label Feb 13, 2018
@dsnet dsnet changed the title allow generation of unexported types proposal: protoc-gen-go: allow generation of unexported types Feb 13, 2018
@dsnet dsnet changed the title proposal: protoc-gen-go: allow generation of unexported types proposal: protoc-gen-go: support generating unexported types Feb 13, 2018
@dsnet
Copy link
Member

dsnet commented Mar 9, 2018

I don't think we will ever provide an option to intentionally unexport all enum and message types. This seems too niche of use case.

However, I filed #555 to provide support for specifying the Go name, which allows you to functionally do this for message and enum types. There are some other exported stuff that is visible, and it's an open question what (if anything) can be done there.

@dsnet dsnet closed this as completed Mar 9, 2018
@golang golang locked and limited conversation to collaborators Jun 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants