protocolbuffers / protobuf

Protocol Buffers - Google's data interchange format
http://protobuf.dev
Other
64.5k stars 15.36k forks source link

Allow custom plugin codegenerators to use imports #8174

Open nipunn1313 opened 3 years ago

nipunn1313 commented 3 years ago

Describe the problem you are trying to solve. At Dropbox, we have a custom code generation plugin which makes use of custom options. For example

import "dropbox/api/errors.proto";

message Request {}
message Response {}
service MyService {
  rpc MyRPC(Request) returns (Response) {
    option (api_v2.method_error_type) = "errors.SpecialError";
  }
}

Our custom plugin generator currently (and ideally) depends on the import in order to bring in the special error type. However, protoc will warn

dropbox/api/my_service.proto:1:1: warning: Import dropbox/api/errors.proto is unused.

Describe the solution you'd like Ideally, there would be a way to suppress this warning in cases where the import is being used by the custom plugin, while still providing in the warning in true cases of unused imports. Perhaps the codegenerator response could include a list of imports that were used by the plugin - for unused import detection.

I could understand if this was an overgeneralization of this problem and I'm open to other ideas.

Describe alternatives you've considered We currently ignore unused import warnings for this reason - which is a possibility here, though suboptimal. Another idea might be to give a fully qualified path in the option string and teach the custom generator to pull in the import separately from protoc. This seems suboptimal as it's reimplementing import logic that protoc has already implemented, but it could work. Another idea might be to implement support for a warning-suppression comment into protoc eg:

import dropbox/api/errors.proto;  // ignore: unused

Another idea might be a file-level option for additional imports eg:

option (api.custom_generator.additional_imports) = "dropbox/api/my_service.proto";

Thanks!

acozzette commented 3 years ago

One easy workaround is to replace import with import public. That feature is intended to facilitate moving definitions from one .proto file to another, but it also has this side effect of disabling warnings about unused imports.

nipunn1313 commented 3 years ago

oh that's a neat workaround idea.

It is imperfect in a couple of ways

That being said, this is still probably better than the status quo.

fowles commented 1 year ago

The design sketched out here is good, but we don't have time to pursue it ourselves