uber/prototool

Your Swiss Army Knife for Protocol Buffers

Prototool

Update: We recommend checking out Buf, which is under active development. There are a ton of docs for getting started, including for migration from Prototool.

Protobuf is one of the best interface description languages out there - it's widely adopted, and after over 15 years of use, it's practically bulletproof. However, working with Protobuf and maintaining consistency across your Protobuf files can be a pain - protoc, while being a tool that has stood the test of time, is non-trivial to use, and the Protobuf community has not developed common standards with regards to stub generation. Prototool aims to solve this by making working with Protobuf much simpler.

Prototool lets you:

• Handle installation of protoc and the import of all of the Well-Known Types behind the scenes in a platform-independent manner.
• Standardize building of your Protobuf files with a common configuration.
• Lint your Protobuf files with common linting rules according to Google' Style Guide, Uber's V1 Style Guide, Uber's V2 Style Guide, or your own set of configured lint rules.
• Format your Protobuf files in a consistent manner.
• Create Protobuf files from a template that passes lint, taking care of package naming for you.
• Generate stubs using any plugin based on a simple configuration file, including handling imports of all the Well-Known Types.
• Call gRPC endpoints with ease, taking care of the JSON to binary conversion for you.
• Check for breaking changes on a per-package basis, verifying that your API never breaks.
• Output errors and lint failures in a common file:line:column:message format, making integration with editors possible, Vim integration is provided out of the box.

Prototool accomplishes this by downloading and calling protoc on the fly for you, handing error messages from protoc and your plugins, and using the generated FileDescriptorSets for internal functionality, as well as wrapping a few great external libraries already in the Protobuf ecosystem. Compiling, linting and formatting commands run in around 3/100ths of second for a single Protobuf file, or under a second for a larger number (500+) of Protobuf files.

Table Of Contents

• Installation
• Quick Start
• Full Example
• Configuration
• File Discovery
• Command Overview
  • prototool config init
  • prototool compile
  • prototool generate
  • prototool lint
  • prototool format
  • prototool create
  • prototool files
  • prototool break check
  • prototool descriptor-set
  • prototool grpc
• Tips and Tricks
• Vim Integration
• Stability
• Development
• FAQ
• Special Thanks

Installation

Prototool can be installed on Mac OS X or Linux through a variety of methods.

See install.md for full instructions.

Quick Start

We'll start with a general overview of the commands. There are more commands, and we will get into] usage below, but this shows the basic functionality.

prototool help
prototool lint idl/uber # search for all .proto files recursively, obeying exclude_paths in prototool.yaml or prototool.json files
prototool lint # same as "prototool lint .", by default the current directory is used in directory mode
prototool create foo.proto # create the file foo.proto from a template that passes lint
prototool files idl/uber # list the files that will be used after applying exclude_paths from corresponding prototool.yaml or prototool.json files
prototool lint --list-linters # list all current lint rules being used
prototool lint --list-all-lint-groups # list all available lint groups, currently "google" and "uber"
prototool compile idl/uber # make sure all .proto files in idl/uber compile, but do not generate stubs
prototool generate idl/uber # generate stubs, see the generation directives in the config file example
prototool grpc idl/uber --address 0.0.0.0:8080 --method foo.ExcitedService/Exclamation --data '{"value":"hello"}' # call the foo.ExcitedService method Exclamation with the given data on 0.0.0.0:8080
prototool descriptor-set --include-imports idl/uber # generate a FileDescriptorSet for all files under idl/uber, outputting to stdout, a given file, or a temporary file
prototool break check idl/uber --git-branch master # check for breaking changes as compared to the Protobuf definitions in idl/uber on the master branch

Full Example

See the example directory.

The make command make example runs prototool while installing the necessary plugins.

Configuration

Prototool operates using a config file named either prototool.yaml or prototool.json. Only one of prototool.yaml or prototool.json can exist in a given directory. For non-trivial use, you should have a config file checked in to at least the root of your repository. It is important because the directory of an associated config file is passed to protoc as an include directory with -I, so this is the logical location your Protobuf file imports should start from.

Recommended base config file:

protoc:
  version: 3.11.0
lint:
  group: uber2

See protoc.md for how Prototool handles working with protoc.

The command prototool config init will generate a config file in the current directory with the currently recommended options set.

When specifying a directory or set of files for Prototool to operate on, Prototool will search for config files for each directory starting at the given path, and going up a directory until hitting root. If no config file is found, Prototool will use default values and operate as if there was a config file in the current directory, including the current directory with -I to protoc.

If multiple prototool.yaml or prototool.json files are found that match the input directory or files, an error will be returned.

See etc/config/example/prototool.yaml all available options.

File Discovery

In most Prototool commands, you will see help along the following lines:

$ prototool help lint
Lint proto files and compile with protoc to check for failures.

Usage:
  prototool lint [dirOrFile] [flags]

dirOrFile can take two forms:

• You can specify exactly one directory. If this is done, Prototool goes up until it finds a prototool.yaml or prototool.json file (or uses the current directory if none is found), and then uses this config for all .proto files under the given directory recursively, except for files in the excludes lists in prototool.yaml or prototool.json files.
• You can specify exactly one file. This has the effect as if you specified the directory of this file (using the logic above), but errors are only printed for that file. This is useful for e.g. Vim integration.
• You can specify nothing. This has the effect as if you specified the current directory as the directory.

The idea with "directory builds" is that you often need more than just one file to do a protoc call, for example if you have types in other files in the same package that are not referenced by their fully-qualified name, and/or if you need to know what directories to specify with -I to protoc (by default, the directory of the prototool.yaml or prototool.json file is used).

Command Overview

Let's go over some of the basic commands.

prototool config init

Create a prototool.yaml file in the current directory with the currently recommended options set.

Pass the --document flag to generate a prototool.yaml file with all other options documented and commented out.

Pass the --uncomment flag to generate prototool.yaml file with all options documented but uncommented.

See etc/config/example/prototool.yaml for the config file that prototool config init --uncomment generates.

prototool compile

Compile your Protobuf files, but do not generate stubs. This has the effect of calling protoc with -o /dev/null.

Pass the --dry-run flag to see the protoc commands that Prototool runs behind the scenes.

prototool generate

Compile your Protobuf files and generate stubs according to the rules in your prototool.yaml or prototool.json file.

See etc/config/example/prototool.yaml for all available options. There are special options available for Golang plugins, and plugins that output a single file instead of a set of files. Specifically, you can output a single JAR for the built-in protoc java plugin, and you can output a file with the serialized FileDescriptorSet using the built-in protoc descriptor_set plugin, optionally also calling --include_imports and/or --include_source_info.

Pass the --dry-run flag to see the protoc commands that Prototool runs behind the scenes.

See example/proto/prototool.yaml for a full example.

prototool lint

Lint rules can be set using the configuration file. See the configuration at etc/config/example/prototool.yaml for all available options. There are three pre-configured groups of rules, the setting of which is integral to the prototool lint, prototool create, and prototool format commands:

• uber2: This lint group follows the V2 Uber Style Guide, and makes some modifications to more closely follow the Google Cloud APIs file structure, as well as adding even more rules to enforce more consistent development patterns. This is the lint group we recommend using.
• uber1: This lint group follows the V1 Uber Style Guide. For backwards compatibility reasons, this is the default lint group, however we recommend using the uber2 lint group.
• google: This lint group follows the Google Style Guide. This is a small group of rules meant to enforce basic naming. The style guide is copied to etc/style/google/google.proto.

The flag --generate-ignores will help with migrating to a given lint group by generating the configuration to ignore existing lint failures on a per-file basis.

See lint.md for full instructions.

prototool format

Format a Protobuf file and print the formatted file to stdout. There are flags to perform different actions:

• -d Write a diff instead.
• -f Fix the file according to the Style Guide. This will have different behavior if the uber2 lint group is set.
• -l Write a lint error in the form file:line:column:message if a file is unformatted.
• -w Overwrite the existing file instead.

prototool create

Create Protobuf files from a template. With the provided Vim integration, this will automatically create new files that pass lint when a new file is opened.

See create.md for full instructions.

prototool files

Print the list of all files that will be used given the input dirOrFile. Useful for debugging.

prototool break check

Protobuf is a great way to represent your APIs and generate stubs in each language you develop with. As such, Protobuf APIs should be stable so as not to break consumers across repositories. Even in a monorepo context, making sure that your Protobuf APIs do not introduce breaking changes is important so that different deployed versions of your services do not have wire incompatibilities.

Prototool exposes a breaking change detector through the prototool break check command. This will check your current Protobuf definitions against a past version of your Protobuf definitions to see if there are any source or wire incompatible changes. Some notes on this command:

• The breaking change detection operates on a per-package basis, not per-file - definitions can be moved between files within the same Protobuf package without being considered breaking.
• The breaking change detector can either check against a given git branch or tag, or it can check against a previous state saved with the prototool break descriptor-set command.
• The breaking change detector understands the concept of beta vs. stable packages, discussed in the V2 Style Guide. By default, the breaking change detector will not check beta packages for breaking changes, and will not allow stable packages to depend on beta packages, however both of these options are configurable in your prototool.yaml file.

See breaking.md for full instructions.

prototool descriptor-set

Produce a serialized FileDescriptorSet for all Protobuf definitions. By default, the serialized FileDescriptorSet is printed to stdout. There are a few options:

• --include-imports, --include-source-info are analagous to protoc's --include_imports, --include_source_info flags.
• --json outputs the FileDescriptorSet as JSON instead of binary.
• -o writes the FileDescriptorSet to the given output file path.
• --tmp writes the FileDescriptorset to a temporary file and prints the file path.

The outputted FileDescriptorSet is a merge of all produced FileDescriptorSets for each Protobuf package compiled.

This command is useful in a few situations.

One such situation is with external gRPC tools such as grpcurl or ghz. Both tools take a path to a serialized FileDescriptorSet for use to figure out the request/response structure of RPCs when the gRPC reflection service is not available. prototool descriptor-set can be used to generate these FileDescriptorSets on the fly.

grpcurl -protoset $(prototool descriptor-set --include-imports --tmp) ...
ghz -protoset $(prototool descriptor-set --include-imports --tmp) ...

You can also just save the file once and not re-compile each time.

prototool descriptor-set --include-imports -o descriptor_set.bin
grpcurl -protoset descriptor_set.bin ...
ghz -protoset descriptor_set.bin ...

Another situation is to use jq to make arbitrary queries on your Protobuf definitions.

For example, if your Protobuf definitions are in path/to/proto, the following will print all message names.

prototool descriptor-set path/to/proto --json | \
  jq '.file[] | select(.messageType != null) | .messageType[] | .name' | \
  sort | uniq

prototool grpc

Call a gRPC endpoint using a JSON input. What this does behind the scenes:

• Compiles your Protobuf files with protoc, generating a FileDescriptorSet.
• Uses the FileDescriptorSet to figure out the request and response type for the endpoint, and to convert the JSON input to binary.
• Calls the gRPC endpoint.
• Uses the FileDescriptorSet to convert the resulting binary back to JSON, and prints it out for you.

See grpc.md for full instructions.

Tips and Tricks

Prototool is meant to help enforce a consistent development style for Protobuf, and as such you should follow some basic rules:

• Have all your imports start from the directory your prototool.yaml or prototool.json file is in. While there is a configuration option protoc.includes to denote extra include directories, this is not recommended.
• Have all Protobuf files in the same directory use the same package.
• Do not use long-form go_package values, ie use foopb, not github.com/bar/baz/foo;foopb. This helps prototool generate do the best job.

Vim Integration

This repository is a self-contained plugin for use with the ALE Lint Engine. The Vim integration will currently compile, provide lint errors, do generation of your stubs, and format your files on save. It will also optionally create new files from a template when opened.

See vim.md for full instructions.

Stability

Prototool is generally available, and conforms to SemVer, so Prototool will not have any breaking changes on a given major version, with some exceptions:

• Commands under the x top-level command are experimental, and may change or be deleted between minor versions of Prototool. We expect such commands to be promoted to stable within a few minor releases, however development is still in-progress.
• The output of the formatter may change between minor versions. This has not happened yet, but we may change the format in the future to reflect things such as max line lengths.
• The breaking change detector's output format currently does not output filename, line, or column. This is an expected upgrade in the future, so the output will likely change. This is viewed as purely an upgrade, so until this is done, do not parse prototool break check output in scripts.
• The breaking change detector may have additional checks added between minor versions, and therefore a change that might not have been breaking previously might become a breaking change. This may become stable in the near future, and at this time we'll denote that no more checks will be added.

Development

See development.md for concerns related to Prototool development.

See maintenance.md for maintenance-related tasks.

FAQ

See faq.md for answers to frequently asked questions.

Special Thanks

Prototool uses some external libraries that deserve special mention and thanks for their contribution to Prototool's functionality:

• github.com/emicklei/proto - The Golang Protobuf parsing library that started it all, and is still used for the linting and formatting functionality. We can't thank Ernest Micklei enough for his help and putting up with all the filed issues.
• github.com/jhump/protoreflect - Used for the JSON to binary and back conversion. Josh Humphries is an amazing developer, thank you so much.
• github.com/fullstorydev/grpcurl - Still used for the gRPC functionality. Again a thank you to Josh Humphries and the team over at FullStory for their work.

prototool's Issues

Well-Known Plugins/Plugin Groups

In Protoeasy, I took the tact that Protoeasy should know the entire world, and it will only handle known plugins. This worked well for basic usage, but proved too limiting, as I eventually had to add the --extra-plugin flag, and keeping up to date with all handled plugins was not scalable in any sense of the word. In prototyping Prototool, I took the compete opposite tact, that I like to think of as the "dependency injection approach", where Prototool is not aware of any plugins, instead leaving you to specify what to compile and where to output it. Through discussions with the Protobuf team at Google, it's become apparent that a middle-ground approach is desired, where we do handle "well-known plugins" in a consistent manner, but leave room to "break glass" if necessary.

The current setup requires something like this:

gen:
  go_options:
    import_path: github.com/uber/prototool/example/idl/uber
    extra_modifiers:
      google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
      google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
  plugins:
    - name: gogoslick
      type: gogo
      flags: plugins=grpc
      output: ../../gen/proto/go
    - name: yarpc-go
      type: gogo
      output: ../../gen/proto/go
    - name: java
      output: ../../gen/proto/java
    - name: grpc-gateway
      type: go
      output: ../../gen/proto/go

This will output stubs for github.com/gogo/protobuf including grpc-go stubs, github.com/yarpc/yarpc-go, java (the built-in "plugin" for java), and github.com/grpc-ecosystem/grpc-gateway. We denote the type for a Golang plugin to say whether to do Well-Known Type imports from github.com/golang/protobuf or github.com/gogo/protobuf. The import_path will probably be needed in some form regardless, unless we get too cute. extra_modifiers are needed for grpc-gateway, and all the plugins need to know where to output their result.

One of the goals of Prototool is for users to not have to think about these things, and for output to be consistent - a user can start using Prototool, and all output will be in the same location. Roughly, I have a few potential ideas in my head of how to accomplish this.

• Add a base_output option to gen. In the example above, base_output would be ../../gen/proto. This says the directory to put all output to. The default value might be gen/proto.

• Add the concept of Well-Known Plugins. This would be the set of the most commonly used plugins , and we would define default values for the type, flags, output values. We might also overload the concept of name to have some Well-Known Plugins do gRPC generation. As an instructive example, the following would be the equivalent (except for the java override):

gen:
  base_output: ../../gen/proto
  plugins:
    - name: gogoslick-grpc
    - name: yarpc-go
    - name: java
       output: java/example
    - name: grpc-gateway

Where we say output is go for gogoslick-grpc, yarpc-go, grpc-gateway, and the output is java/example for java (the default would have been java).

The Well-Known Plugins off the top of my head would be all the builtins (cpp, csharp, java, etc), all the golang/protobuf and gogo/protobuf plugins, grpc variants of all of these that we can, along with grpc-gateway and yarpc-go (yarpc-go is needed for Uber).

• Another idea is to add the concept of Plugin Groups. This effectively extends the idea of Well-Known plugins to say you have a known group that you always generate. This works great if you define your Protobuf definitions in a different repository than your server implementation, something I want to move towards (and was attempting to prototype back in the day with https://github.com/protocafe). The Plugin Group named "uber" for example might be gogoslick-grpc, yarpc-go, java, cpp, python along with some gRPC variants. The Plugin Group named "default" or "google" might be much more extensive. Think the following:

gen:
  plugin_group:
    name: uber
    exclude:
      - cpp
      - python

Handle public imports in formatter

Prototool does not currently handle import public https://developers.google.com/protocol-buffers/docs/proto3#importing-definitions. We need to handle this before v1.0.

We also should handle import weak as it is part of the spec, even if not really used. https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#import_statement

Remove unneeded config options/Rename some config options

Similar to #11, we need a full review of the prototool.yaml config file. It exposes way too much functionality for v1.0 in my opinion. See etc/config/example/prototool.yaml for all options.

Things I nominate for potential deletion:

• no_default_excludes: I exclude "vendor" by default to make this easy to use with Golang projects, but this is probably overkill and too Golang-specific. Just removing the concept of default excludes altogether would be nice.
• protoc_include_wkt: Maybe just always include the Well-Known Types?
• format: The entire section. There shouldn't be options for formatting, ie "choose your spacing", no. I added this for testing.

Things I'm unsure of:

• lint groups: Will explain in a different issue.
• gen.plugins.type: Going with #2, do we need this? We might for Golang plugins that are not well-known, but this concept is weird.

Installation of known plugins

In #2 we describe the wanted functionality of handing known plugins. We install protoc for you within Prototool. It would be nice for Prototool to handle installation of things like protoc-gen-go as well. A problem here is that you want your library code to match your generated code, so it might be better for you to do:

.PHONY: gen
gen:
  go install ./vendor/github.com/golang/protobuf/protoc-gen-go
  prototool gen

So this might end up causing more problems than it solves. This is worth discussion though.

Handle allow_alias in enums

We probably want to outlaw enum aliases as part of linting by default, but this is a discussion item https://developers.google.com/protocol-buffers/docs/proto3#enum

Deal with exception-driven development for protoc errors

It's desirable to know all protoc errors and handle them separately, so we can output errors in a consistent manner with prototool ie filename:line:column:message. This was done by handing errors as they showed up during development:

var (
	// special cased
	pluginFailedRegexp       = regexp.MustCompile("^--.*_out: protoc-gen-(.*): Plugin failed with status code (.*).$")
	otherPluginFailureRegexp = regexp.MustCompile("^--(.*)_out: (.*)$")

	extraImportRegexp  = regexp.MustCompile("^(.*): warning: Import (.*) but not used.$")
	fileNotFoundRegexp = regexp.MustCompile("^(.*): File not found.$")
	// protoc outputs both this line and fileNotFound, so we end up ignoring this one
	// TODO figure out what the error is for errors in the import
	importNotFoundRegexp              = regexp.MustCompile("^(.*): Import (.*) was not found or had errors.$")
	noSyntaxSpecifiedRegexp           = regexp.MustCompile("No syntax specified for the proto file: (.*)\\. Please use")
	jsonCamelCaseRegexp               = regexp.MustCompile("^(.*): (The JSON camel-case name of field.*)$")
	isNotDefinedRegexp                = regexp.MustCompile("^(.*): (.*) is not defined.$")
	seemsToBeDefinedRegexp            = regexp.MustCompile(`^(.*): (".*" seems to be defined in ".*", which is not imported by ".*". To use it here, please add the necessary import.)$`)
	explicitDefaultValuesProto3Regexp = regexp.MustCompile("^(.*): Explicit default values are not allowed in proto3.$")
	optionValueRegexp                 = regexp.MustCompile("^(.*): Error while parsing option value for (.*)$")
	programNotFoundRegexp             = regexp.MustCompile("protoc-gen-(.*): program not found or is not executable$")
	firstEnumValueZeroRegexp          = regexp.MustCompile("^(.*): The first enum value must be zero in proto3.$")
)

This isn't great. We need to either figure out what all the errors or, or allow the catch-all to do something other than return a "system" error.

Switch java_package, java_multiple_files, java_outer_classname to match Google Cloud API format

https://cloud.google.com/apis/design/file_structure

Specifically:

• java_package will be com.PACKAGE instead of com.PACKAGE.pb
• java_multiple_files will be set to true
• java_outer_classname will be FILENAMEProto

go_package will not change as there's no guidance on that, and we won't do anything with csharp_namespace, objc_class_prefix, or php_namespace as of yet, as we don't use those and we don't need to overcomplicate this for now (we might want to do objc_class_prefix in the future though).

Changes that need to be made:

• Update the linters to enforce this
• Update the create package to create Protobuf files with this
• Update the formatter to set these values after #113 is merged

Windows compatibility

There's a lot of strings.HasPrefix and fmt.Sprintf("%s/%s", some, path) lying around Prototool that needs to be cleaned up into filepath.Join etc calls. It's not that difficult, but just needs to be done.

Add tool to check for breaking changes

https://github.com/nilslice/protolock just came out recently, and this is a nice tool, but it would be nice if similar functionality was built into Prototool. Additionally, Protolock generates a proto.lock file that stores information on the Protobuf files in a repository, which I'd like to get rid of. This is a new format and inherently can have bugs, but more to the point is duplicated information - the proto.lock file is generated via the checked-in Protobuf files, which in turn contain all the information you need. If you were to generate a FileDescriptorSet for the current Protobuf files, and a FileDescriptorSet for the old Protobuf files, and then compare the two, you could get the same thing. A proto.lock file could have just been the result of --descriptor_set_out as a further example, and then used https://github.com/jhump/protoreflect, but again instead of storing another file, we can just generate this on the fly. I'd also like errors to be printed in the Prototool standard filename:line:column:message format.

Example of what I'd imagine:

prototool check-compatibility --from dev # dev is the name of the base branch
a/b/c.proto:3:4:Cannot remove field "foo".
a/b/d.proto:1:1:Cannot change syntax.
...

Command-line file/directory specification

Right now, most commands take something along the lines of dirOrProtoFiles... as their first arguments. These commands also have a flag --dir-mode. The possibilities:

• You can specify multiple files. If this is done, these files will be explicitly used for protoc calls etc.
• You can specify exactly one directory. If this is done, Prototool goes up until it finds a prototool.yaml file (or uses the current directory if none is found), and then walks starting at this location for all .proto files, and these are used, except for files in the excludes lists in prototool.yaml files.
• You can specify exactly one file, along with --dir-mode. This has the effect as if you specified the directory of this file (using the logic above), but errors are only printed for that file. This is useful for e.g. Vim integration.

The idea with "directory builds" is that you often need more than just one file to do a protoc call, for example if you have types in other files in the same package that are not referenced by their fully-qualified name, and/or if you need to know what directories to specify with -I to protoc (by default, the directory of the prototool.yaml file is used).

In general practice, directory builds are what you always want to do. File builds were just added for convenience.

There's a proposal to do the following instead:

• Only one argument is specified. This has the side effect of making the number of arguments to many commands consistent.
• If a directory is speficied, do directory mode, as usual.
• If a file is specified, do the equivalent of --dir-mode above.

There are some side effects, for example you need to special-case the format command probably, but we probably want to do this. It's not as trivial as I first realized, and wanted to open this up for discussion before I do it.

Formatter adds java options when not requested

The latest version of prototool on dev adds java_options unbidden when running prototool format -w.

Mock gRPC servers

The Protobuf team at Google had a request for a tool that brings up a mock gRPC server, given a Protobuf definition. It would spit back dummy data for any endpoint. This isn't overly difficult, but needs spec'ing out as to what this means (options to return errors? sizes of dummy data? etc).

windows binary

Any chance to produce a windows binary as well ?

Multiple prototool.yaml files in the same project

We should consider the prototool.yaml hierarchy and determine whether or not we should stick to the current tree/override behavior. A single prototool.yaml at the project's root might be sufficient for now. This would also simplify some of the implementation details found in the settings package.

If there isn't an obvious need for the hierarchy, it might be worth deferring until after a 1.0 release.

Formatting order inconsistent with Google API Design Guidelines

Hi!

I'm raising this for discussion as I assume the formatting is already based on some formatting agreed within Uber. Anyway, the Google API Design guidelines explicitly list the following macro ordering:

The file structure must put higher level and more important definitions before lower level and less important items. In each proto file, the applicable sections should be in the following order:

1. Copyright and license notice if needed.
2. Proto syntax, package, option and import statements in that order.
3. The API overview documentation which prepares the readers for the rest of the file.
4. The API proto service definitions, in decreasing order of importance.
5. The RPC request and response message definitions, in the same order of the corresponding methods. Each request message must precede its corresponding response message, if any.
6. The resource message definitions. A parent resource must be defined before its child resource(s).

Now, I think specifically point 2 is violated by prototool format as it puts import statements above both options and package declarations.

Fail to curl prototool > Failed writing body (0 != 16360)

Hello,

When I try to install protool, I've got this error:

curl -sSL https://github.com/uber/prototool/releases/download/v0.1.0/prototool-$(uname -s)-$(uname -m) \ -o /usr/local/bin/prototool && \ chmod +x /usr/local/bin/prototool && \ prototool -h

return

curl: (23) Failed writing body (0 != 16360)

Factor out zap logging

We used github.com/uber-go/zap for logging, which is fine as long as this is just a CLI tool, but as we learned the hard way in github.com/grpc/grpc-go, it's not great to assume a given logging library unless you need to. The logging in Prototool is very basic, and almost always at the debug level, so creating a minimal package similar to https://godoc.org/google.golang.org/grpc/grpclog (but even simpler) might be nice.

Config file discovery semantics

Right now, config files are named prototool.yaml and searched for starting at the Protobuf file path, and going up a directory until hitting root. All paths in the config files can be relative or absolute, but if relative, the actual path used will start at the config file directory. If using directory mode, this means multiple prototool.yaml files corresponding to multiple found directories with Protobuf files may be used. After a config file is found, Prototool walks starting at that directory searching for Protobuf files, or if no config file is found, Prototool searches starting at the current directory.

A few things:

• Is prototool.yaml the right name?
• Should we terminate earlier? Right now for example, if you don't have a prototool.yaml file in your repo, but do in your home directory, prototool will go all the way down to your home directory, and then walk everything in your home directory, which can be annoying.
• Should we require a prototool.yaml file to use prototool? See the above. It's good practice to always have a prototool.yaml file to at least lock down the version of protoc you use, but this can be annoying for OSS users who are just starting out.
• The biggest thing: should we not allow multiple prototool.yaml files to be discovered? If multiple prototool.yaml files are discovered, this means that multiple calls to protoc must be done as they denote different builds, and more than one FileDescriptorSet is created. This gets bad when you do e.g. gRPC calls as foo.bar.BazService could be in more than one FileDescriptorSet, so you basically have to stop when you find the first one. All packages in Prototool have to handle multiple FileDescriptorSets, which leads to some other bad outcomes. Most people shouldn't have multiple prototool.yaml files, but it could be useful for compiling vendored prototool.yaml files.

Valid package names can't be parsed

input:

syntax = "proto3";

package group.v1;

command: prototool lint test.proto
output:

test.proto:3:14: found "." but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]

expected: No output

I found emicklei/proto#37 and this looks like the exact same issue. Does github.com/emicklei/proto need to updated?

Hoping to use this tool in my org's CI.

make failures

On a fresh clone of the repo:

~/go/src/github.com/uber/prototool (dev)  13:51:16
$ make
git status --short > /var/folders/nk/lrxg8bsn08l9c4w9ld_00v440000gn/T/checknodiffgenerated_pre.XXXXX.IE2l7UzP
/Applications/Xcode.app/Contents/Developer/usr/bin/make generate
go install ./vendor/go.uber.org/tools/update-license
can't load package: package github.com/uber/prototool/vendor/go.uber.org/tools/update-license: cannot find package "github.com/uber/prototool/vendor/go.uber.org/tools/update-license" in any of:
	/usr/local/Cellar/go/1.10.1/libexec/src/github.com/uber/prototool/vendor/go.uber.org/tools/update-license (from $GOROOT)
	/Users/blampe/go/src/github.com/uber/prototool/vendor/go.uber.org/tools/update-license (from $GOPATH)
make[1]: *** [license] Error 1
make: *** [checknodiffgenerated] Error 2

After running glide i I still get a failure:

$ make
git status --short > /var/folders/nk/lrxg8bsn08l9c4w9ld_00v440000gn/T/checknodiffgenerated_pre.XXXXX.o7dDQPDS
/Applications/Xcode.app/Contents/Developer/usr/bin/make generate
go install ./vendor/go.uber.org/tools/update-license
update-license ./cmd/prototool/main.go ./internal/cmd/cmd.go ./internal/cmd/cmd_test.go ./internal/desc/desc.go ./internal/diff/diff.go ./internal/exec/exec.go ./internal/exec/runner.go ./internal/extract/extract.go ./internal/extract/getter.go ./internal/file/file.go ./internal/file/proto_set_provider.go ./internal/file/proto_set_provider_test.go ./internal/format/base_visitor.go ./internal/format/first_pass_visitor.go ./internal/format/format.go ./internal/format/log_visitor.go ./internal/format/middle_visitor.go ./internal/format/printer.go ./internal/format/transformer.go ./internal/gen/gen-prototool-bash-completion/main.go ./internal/gen/gen-prototool-manpages/main.go ./internal/gen/gen-prototool-zsh-completion/main.go ./internal/grpc/grpc.go ./internal/grpc/handler.go ./internal/grpc/invocation_event_handler.go ./internal/lint/base_checker.go ./internal/lint/base_visitor.go ./internal/lint/check_comments_no_c_style.go ./internal/lint/check_enum_field_names_upper_snake_case.go ./internal/lint/check_enum_field_names_uppercase.go ./internal/lint/check_enum_field_prefixes.go ./internal/lint/check_enum_names_camel_case.go ./internal/lint/check_enum_names_capitalized.go ./internal/lint/check_enum_zero_values_invalid.go ./internal/lint/check_enums_have_comments.go ./internal/lint/check_file_options_equal.go ./internal/lint/check_file_options_required.go ./internal/lint/check_message_field_names_lower_snake_case.go ./internal/lint/check_message_field_names_lowercase.go ./internal/lint/check_message_fields_not_floats.go ./internal/lint/check_message_names_camel_case.go ./internal/lint/check_message_names_capitalized.go ./internal/lint/check_messages_have_comments.go ./internal/lint/check_messages_have_comments_except_request_response_types.go ./internal/lint/check_oneof_names_lower_snake_case.go ./internal/lint/check_package_lower_snake_case.go ./internal/lint/check_request_response_names_match_rpc.go ./internal/lint/check_request_response_types_in_same_file.go ./internal/lint/check_request_response_types_unique.go ./internal/lint/check_rpc_names_camel_case.go ./internal/lint/check_rpc_names_capitalized.go ./internal/lint/check_rpcs_have_comments.go ./internal/lint/check_service_names_camel_case.go ./internal/lint/check_service_names_capitalized.go ./internal/lint/check_services_have_comments.go ./internal/lint/check_syntax_proto3.go ./internal/lint/check_wkt_directly_imported.go ./internal/lint/lint.go ./internal/lint/runner.go ./internal/protoc/compiler.go ./internal/protoc/downloader.go ./internal/protoc/downloader_test.go ./internal/protoc/protoc.go ./internal/reflect/handler.go ./internal/reflect/reflect.go ./internal/settings/config_provider.go ./internal/settings/config_provider_test.go ./internal/settings/settings.go ./internal/strs/strs.go ./internal/strs/strs_test.go ./internal/text/text.go ./internal/text/text_test.go ./internal/wkt/wkt.go ./prototool.go
git log --format='%aN <%aE>' | sort -fu > CONTRIBUTORS
go install \
		-ldflags "-X 'github.com/uber/prototool.GitCommit=2f29e7013073f1209afe97db4682dfd931e760e3' -X 'github.com/uber/prototool.BuiltTimestamp=Mon Apr 16 20:57:53 UTC 2018'" \
		github.com/uber/prototool/cmd/prototool
for file in internal/cmd/testdata/format/bar/bar.proto.golden internal/cmd/testdata/format/bar/bar_proto2.proto.golden internal/cmd/testdata/format/foo/foo.proto.golden internal/cmd/testdata/format/foo/foo_proto2.proto.golden; do \
		rm -f ${file}; \
	done
for file in internal/cmd/testdata/format/bar/bar.proto internal/cmd/testdata/format/bar/bar_proto2.proto internal/cmd/testdata/format/foo/foo.proto internal/cmd/testdata/format/foo/foo_proto2.proto; do \
		prototool format ${file} > ${file}.golden || true; \
	done
go install ./vendor/github.com/gogo/protobuf/protoc-gen-gogoslick
go install ./vendor/github.com/golang/protobuf/protoc-gen-go
go install ./vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway
go install ./vendor/go.uber.org/yarpc/encoding/protobuf/protoc-gen-yarpc-go
rm -rf example/gen
prototool all example/idl/uber
go build ./example/gen/proto/go/foo
go build ./example/gen/proto/go/sub
go build ./example/cmd/excited/main.go
prototool lint etc/style
prototool gen internal/cmd/testdata/grpc
rm -rf etc/release
mkdir -p etc/release/etc/bash_completion.d
mkdir -p etc/release/etc/zsh_completion.d
mkdir -p etc/release/share/man/man1
go run internal/gen/gen-prototool-bash-completion/main.go > etc/release/etc/bash_completion.d/prototool
go run internal/gen/gen-prototool-zsh-completion/main.go > etc/release/etc/zsh_completion.d/prototool
go run internal/gen/gen-prototool-manpages/main.go etc/release/share/man/man1
git status --short > /var/folders/nk/lrxg8bsn08l9c4w9ld_00v440000gn/T/checknodiffgenerated_post.XXXXX.kzuns5G8
make generate produced a diff, make sure to check these in:
0a1,21
>  M etc/release/share/man/man1/prototool-all.1
>  M etc/release/share/man/man1/prototool-binary-to-json.1
>  M etc/release/share/man/man1/prototool-clean.1
>  M etc/release/share/man/man1/prototool-compile.1
>  M etc/release/share/man/man1/prototool-descriptor-proto.1
>  M etc/release/share/man/man1/prototool-download.1
>  M etc/release/share/man/man1/prototool-field-descriptor-proto.1
>  M etc/release/share/man/man1/prototool-files.1
>  M etc/release/share/man/man1/prototool-format.1
>  M etc/release/share/man/man1/prototool-gen.1
>  M etc/release/share/man/man1/prototool-grpc.1
>  M etc/release/share/man/man1/prototool-json-to-binary.1
>  M etc/release/share/man/man1/prototool-lint.1
>  M etc/release/share/man/man1/prototool-list-all-lint-groups.1
>  M etc/release/share/man/man1/prototool-list-all-linters.1
>  M etc/release/share/man/man1/prototool-list-lint-group.1
>  M etc/release/share/man/man1/prototool-list-linters.1
>  M etc/release/share/man/man1/prototool-protoc-commands.1
>  M etc/release/share/man/man1/prototool-service-descriptor-proto.1
>  M etc/release/share/man/man1/prototool-version.1
>  M etc/release/share/man/man1/prototool.1
make: *** [checknodiffgenerated] Error 1

Factor out emicklei/proto

The Golang Protobuf parsing library github.com/emicklei/proto has done wonders for Prototool, and this project would not exist without it - when I started Prototool, it was originally conceived as just a linter for Uber to keep us on a common standard, and the prototype solely used this library. I can't thank Ernest Micklei enough.

However, Prototool has grown into something much larger, and emicklei/proto does not fully implement the Protobuf spec - we explicitly need to check against protoc to verify a file compiles before linting or formatting, and cleaning up edge cases in emicklei/proto has been a case of exception-driven development. Along the way, the internal/x/protoc package picked up the ability to compile Protobuf files on the fly and provide a full FileDescriptorSet to Prototool packages, which a lot of the functionality within Prototool relies on. If SourceCodeInfo was also outputted (easily done by adding --include_imports --include_source_info to protoc calls) then we could effectively replicate the functionality of Prototool.

There's a few options here:

• Factor out emicklei/proto and build a new set of Message/Service/Enum/etc structs/interfaces based on FileDescriptorSets/SourceCodeInfo.
• Factor out emicklei/proto and use FileDescriptorSets/SourceCodeInfo to use the existing setup in https://godoc.org/github.com/jhump/protoreflect/desc. There's a few things missing in the descriptors there, like source representation for options and the ability to walk descriptors in order they appear in the file, but if we're going to factor out emicklei/proto this is probably the best option as we use jhump/protoreflect elsewhere and that probably isn't going away.
• Keep emicklei/proto and say it's "good enough". At least in the short-term, this might be the option we take. emicklei/proto now works against all of googleapis so it's pretty complete, and we only use it in internal/x/lint and internal/x/format, so we might just keep it for now.

Add lint option reference with rationale

Some of the lint options imply some sort of technical limitation, for example MESSAGE_FIELDS_NOT_FLOATS. I think the package should define a reference for the lint options and a rationale for each option so the user can make an informed decision about which ones to include.

Handle repeated options

This came up when fixing #27. Will fix in a follow-up PR.

Override uber.proto

I would like to override the following package related configuration included in uber.proto and used by default by the prototool

// The go package is always $(basename PACKAGE)pb.
// Do not use the "long-form" package name with a directory path.
option go_package = "uberpb";
// The java package is always com.PACKAGE.pb.
option java_package = "com.style.uber.pb";

Documentation mentions that

You can add or exclude lint rules in your prototool.yaml file.

but prototool.yaml has no configuration regarding linting of package names.

How can I do this?

Discuss enforcing packages in linting

https://developers.google.com/protocol-buffers/docs/proto3#packages says that packages are optional, which even having used Protobuf for a while, is news to me. We should enforce as part of linting that packages are required - we use them for gRPC paths, for fully-qualified references, and relate them to the go_package and java_package options. For consistency, we should always have them.

In https://github.com/uber/prototool/blob/dev/internal/x/lint/check_package_lower_snake_case.go#L64, we actually enforce this, but this should be split out into a separate linting rule, and we should add the Filename to the scanner.Position there.

Formatting nested options produces code that emicklei/proto cannot parse

When the formatter processes the following file:

syntax = "proto3";

import "google/api/annotations.proto";

service Test {
    rpc Test(Void) returns (Void) {
        option (google.api.http) = {
            get: "/api/v1/test"
            additional_bindings {
                post: "/api/v1/test"
                body: "*"
            }
        };
    }
}

message Void {}

It produces the following:

syntax = "proto3";

import "google/api/annotations.proto";

service Test {
	rpc Test(Void) returns (Void) {
		option (google.api.http) = {
			get: "/api/v1/test"
			additional_bindings.post: "/api/v1/test"
			additional_bindings.body: "*"
		};
	}
}

message Void {}

This output is not parsable by emicklei/proto at the moment (see emicklei/proto#80).

The options as I see it:

1. Change formatter to output the same code as in sample 1 above.
2. Wait for emicklei/proto#80 to be fixed.
3. Replace emicklei/proto with something else (see #1).

Consider using clang-format for Protobuf formatting

Instead of writing your own Protobuf formatter, why not use the popular clang-format tool which already supports Protobuf files?

Remove unneeded commands/Rename some commands

There's a long list of commands that prototool provides, some of which we probably should not have for v1.0. The full list:

$ prototool help
Usage:
  prototool [command]

Available Commands:
  all                      Compile, then format and overwrite, then re-compile and generate, then lint, stopping if any step fails.
  binary-to-json           Convert the data from json to binary for the message path and data.
  clean                    Delete the cache.
  compile                  Compile with protoc to check for failures.
  descriptor-proto         Get the descriptor proto for the message path.
  download                 Downlond the protobuf artifacts to a cache.
  field-descriptor-proto   Get the field descriptor proto for the field path.
  files                    Print all files that match the input arguments.
  format                   Format a proto file and compile with protoc to check for failures.
  gen                      Generate with protoc.
  grpc                     Call a gRPC endpoint.
  help                     Help about any command
  init                     Generate an initial config file in the current or given directory.
  json-to-binary           Convert the data from json to binary for the message path and data.
  lint                     Lint proto files and compile with protoc to check for failures.
  list-all-lint-groups     List all the available lint groups.
  list-all-linters         List all available linters.
  list-lint-group          List the linters in the given lint group.
  list-linters             List the configurerd linters.
  protoc-commands          Print the commands that would be run on compile or gen.
  service-descriptor-proto Get the service descriptor proto for the service path.
  version                  Print the version.

Things that I would nominate for deletion:

• binary-to-json: The functionality here is used for gRPC calls, I added this command for testing when developing Prototool, I would prefer we add binary-to-json separately later if needed.
• json-to-binary: Same.
• descriptor-proto: Was used for testing, not any obvious use outside of testing.
• field-descriptor-proto: Same.
• service-descriptor-proto: Same.

Things I'm not sure of the name of:

• compile vs gen: The split here isn't super obvious at first glance.
• format: Maybe fmt?
• protoc-commands: This is a little weird.
• download: This also is up for deletion, but I like it for it showing where the cache is.

More alignment with grpcurl

The library github.com/fullstorydev/grpcurl, built on top of github.com/jhump/protoreflect, enabled Prototool to get off-the-ground with gRPC support easily. However, Prototool does not implement all the features that grpcurl has. Right now:

• grpcurl has more features for gRPC (such as TLS) but requires you to generate FileDescriptorSets on your own if the gRPC reflection service is not available.
• Prototool handles FileDescriptorSets for you.

It'd be nice to merge this Venn diagram in some manner.

Override values in prototool.yaml

We have dynamic values in our prototool.yaml file. E.g. the path can change for grpc-ruby between each developer environment.

It would be nice if we were able to override the default values set in prototool.yaml from the command line and/or and overrides yaml file. E.g.

prototool gen --set 'gen.plugin_overrides.grpc-ruby:/usr/bin/grpc_tools_ruby_protoc_plugin'

Or

prototool gen --overrides overrides.yaml

Unexpected error when formatting valid proto definition

The following protofile produces a pretty hard-to-debug error when linting (and probably for other stuff as well):

syntax = "proto3";

import "google/api/annotations.proto";

service Test {
    rpc Test(Void) returns (Void) {
        option (google.api.http) = {
            post: "/api/v1/test";
            body: "*";
        };
    }
}

message Void {}

The error:

$ prototool lint test.proto
test.proto:12:1: found "}" but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]

Specifically, it's the semicolons inside the options object that the parser doesn't like.

This is, of course, a perfectly valid protofile definition (as protoc accepts it), though I admit I'm having trouble finding a definition for multi-value options. See https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#option, https://developers.google.com/protocol-buffers/docs/proto#customoptions for more information. The latter lists a single example of a multi-value option where all options are on the same line, without semicolon;

// usage:
message Bar {
  optional int32 a = 1 [(foo_options).opt1 = 123, (foo_options).opt2 = "baz"];
  // alternative aggregate syntax (uses TextFormat):
  optional int32 b = 2 [(foo_options) = { opt1: 123 opt2: "baz" }];
}

So perhaps this could be a linter option? In any case the parser should allow it.

Standardize failures: Define identifiers, types, and constructor(s)

The internal/text package is primarily used in Prototool's proto compilation and lint processes. Seeing as this package is mostly concerned with instantiating text.Failure, we should standardize how they are represented.

#87 proposes several changes that should be considered at a later time. Specifically,

• Rename text to failure (motivated by https://blog.golang.org/package-names)
• Introduce a failure.Type to differentiate failures found in protoc vs. lint
• Remove unnecessary functionality (potentially text.Failure.String -- if not required)
• Introduce a failure.LintIdentifier enum so that lint IDs are not free-form strings.

These items can be largely ignored for the time being, but should be iterated on as we continue to develop out the protoc and lint processes.

Client/server implementation for Prototool as a service

This probably won't happen, but just for discussion.

In Protoeasy, I made it possible to run Protoeasy as a server with a given docker container (which also had versions of all known plugins) so there was no need to install anything locally. This was useful when it was not as trivial to install protoc, and was also useful for installing things like golang/protobuf grpc-gateway in a consistent manner. This worked using https://github.com/peter-edge/protoeasy-go/blob/master/protoeasy.proto by tarring up all .proto files in your context, sending the tarball to the server, running the generation logic, then tarring up the entire result, and untarring in your local context.

Not sure if we want something similar here. Prototool handles installing protoc for you, which is easier now with GitHub releases. Depending on what we do with #2, we might want this to make it easy for known plugins to be handled as well.

Dependency Management

To get this off the ground, I did what I was used to and used Glide as my dependency management tool. I wanted this to be go-gettable, so all this does is specify all necessary dependencies without versions, which are then checked into vendor. This has been fine for development, as Prototool does not expose any external API, and this makes installing Prototool as easy as go get github.com/uber/prototool/cmd/prototool.

However, we eventually will want to expose parts of Prototool as a Golang library, and this isn't a very efficient way to do this. We probably don't want to check in vendor in this case, and we want to get up to date by using Dep. Installing Prototool will instead move to curl'ing binaries released to GitHub Releases.

Update: No longer checking in vendor, so probably all that is left here is to move from Glide to Dep.

Lint options probably too complicated

Going along with #3, we have a lot of options in the config file for linters. I'm not sure where we landed on this, but worth further discussion.

Print single-line field options on the same line from the formatter

If you have a field:

int woot = 5 [(bar.field_option) = true];

Print that out instead of the current:

int woot = 5 [
  (bar.field_option) = true
];

Handle signals when shelling out to protoc

Prototool shells out to protoc to do a lot of the heavy lifting. Right now, we are not handling OS signals if Prototool is sent a SIGTERM, SIGINT, SIGQUIT, SIGKILL, etc. This isn't difficult to add, just something we need to do before v1.0.

Move vim/prototool to separate repository

vim/prototool contains the Vim integration. Along with dense-analysis/ale#1382, we should either get all the code into ALE, or have a separate prototool.vim repository.

swagger annotations break linter

I'm trying to get the linter to pass on my swagger annotations however it seems like protoc is invoked differently for prototool lint command than for prototool gen. The modifications below to the example break lint but work for gen. Is this an issue with my configuration or are the swagger annotations not supported yet?

# prototool lint
[...]
2018-05-08T12:07:26.706-0400	DEBUG	running protoc	{"command": "/Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/bin/protoc -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I /Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/include -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber -o /dev/null /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber/foo/foo.proto"}
foo/foo.proto:18:1: found "}" but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]

# prototool gen
[...]
2018-05-08T12:07:05.189-0400	DEBUG	running protoc	{"command": "/Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/bin/protoc -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I /Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/include -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber --gogoslick_out=plugins=grpc,Mgoogle/protobuf/compiler/plugin.proto=github.com/gogo/protobuf/protoc-gen-gogo/plugin,Mgoogle/protobuf/descriptor.proto=github.com/gogo/protobuf/protoc-gen-gogo/descriptor,Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/source_context.proto=google.golang.org/genproto/protobuf/source_context,Mgoogle/protobuf/type.proto=google.golang.org/genproto/protobuf/ptype,Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/api.proto=google.golang.org/genproto/protobuf/api,Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/field_mask.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/struct.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/wrappers.proto=github.com/gogo/protobuf/types,Mgoogle/api/http.proto=google.golang.org/genproto/googleapis/api/annotations,Mprotoc-gen-swagger/options/annotations.proto=github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options/annotations.proto,Mgoogle/api/annotations.proto=google.golang.org/genproto/googleapis/api/annotations:/Users/lucasvo/Code/go/src/github.com/uber/prototool/example/gen/proto/go /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber/foo/foo.proto"}
2018-05-08T12:07:05.189-0400	DEBUG	running protoc	{"command": "/Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/bin/protoc -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I /Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/include -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber --java_out=/Users/lucasvo/Code/go/src/github.com/uber/prototool/example/gen/proto/java /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber/foo/foo.proto"}

The diff for the example:

diff --git a/example/idl/uber/foo/foo.proto b/example/idl/uber/foo/foo.proto
index 7e94743..0d8e47a 100644
--- a/example/idl/uber/foo/foo.proto
+++ b/example/idl/uber/foo/foo.proto
@@ -5,6 +5,7 @@ import "google/protobuf/timestamp.proto";
 
 import "foo/dep.proto";
 import "google/api/annotations.proto";
+import "protoc-gen-swagger/options/annotations.proto";
 import "sub/sub.proto";
 
 package foo;
@@ -12,6 +13,9 @@ package foo;
 option go_package = "foopb";
 option java_multiple_files = true;
 option java_package = "com.foo.pb";
+option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = {
+  host: "example.com"; 
+};
 
 // Hello is a hello.
 enum Hello {
diff --git a/example/idl/uber/prototool.yaml b/example/idl/uber/prototool.yaml
index 592b48e..5177db7 100644
--- a/example/idl/uber/prototool.yaml
+++ b/example/idl/uber/prototool.yaml
@@ -2,6 +2,7 @@
 protoc_includes:
   # note vendor is .gitignored
   - ../../../vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis
+  - ../../../vendor/github.com/grpc-ecosystem/grpc-gateway
 protoc_include_wkt: true
 lint:
   # run "prototool list-linters" to see the currently configured linters
@@ -17,6 +18,8 @@ gen:
     extra_modifiers:
       google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
       google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
+      protoc-gen-swagger/options/annotations.proto: github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options/annotations.proto
+
   plugins:
     - name: gogoslick
       type: gogo

External API

We need to agree on how to expose a Golang external API for library usage. We need this for yab for example. This will probably be something similar to internal/x/exec.Runner, but with actual inputs and return values, as opposed to printing to stdout.

Add the option to print out metadata for gRPC

See #98 and #104. #104 is very messy, needs to be cleaned up in a way. The default behavior should probably remain the same, but we want to add an option to print everything with json.

Add a non-strict Style Guide group

We've committed our draft Style Guide into etc/style/uber/uber.proto. This is the product of a lot of hashing out for what we want Protobuf files to look like within Uber. The rules inside this are inherently strict - we want there to be One Way To Do Things™ within Uber, and we want easy-to-follow rules that prevent users from making common mistakes. As Jisi on the Protobuf team at Google pointed out though, this style guide would require migration for most existing users.

What my hope is is that we have the new "recommended/strict Style Guide" that accomplishes our goals within Uber, which I think apply to the greater community - if we have a consistent way to do enums, a consistent naming convention for go_package, java_package, etc, everyone wins, and makes things like consistent output of stubs easier. We also have the "default Style Guide" that does the bare minimum enforcement.

For the record, here are the rules I have to ignore to get googleapis to pass lint:

lint:
  exclude_ids:
    - REQUEST_RESPONSE_TYPES_UNIQUE
    - REQUEST_RESPONSE_TYPES_IN_SAME_FILE
    - ENUM_FIELD_PREFIXES
    - ENUM_ZERO_VALUES_INVALID
    - FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX
    - FILE_OPTIONS_EQUAL_JAVA_PACKAGE_COM_PB
    - FILE_OPTIONS_REQUIRE_JAVA_PACKAGE

Action items:

• Go over the Style Guide as proposed more, and come to a common "strict" Style Guide.
• Figure out what belongs in the strict Style Guide vs the default Style Guide.

Copying Jisi's feedback from an email (hope this is ok Jisi!):

There are two parts in the style guide,

• Fromatting rules., e.g. spaces, empty lines, import orders. Those can be safely applied to the whole depot without causing any issues.
• Naming: e.g. packages, enums, etc. Those could affect the APIs.

For a global style guide, we should probably only enforce the format ones. For the naming, we can list several best-practices (we do have such a doc in google), but as protobuf is already widely used, each organization probably has its own naming style. For example, you can find Google API's style guide here. Another idea is we can have different profile for naming.

Some of the detailed feedback about the style guide:

• package: each organization probably has its own style of naming packages. I can image the migration cost for google to a converged package style would be impractical given the # of protos we have.
• enum default value: internally we recommend _UNSPECIFIED or _UNSET as 0. It's the proto3 design goal to not distinguish between unset and set-to-default. In current proto3 implementation, you won't be able to distinguish between the two -- has-bit is gone and setting a value to 0 is a no-op (values default to 0). The serialization will simply skip the field even if you set it to 0 value explicitly. This is also a API change and we should probably just give suggestions rather than enforce it.

Command to output location of generated files

Building on #2, there was a request from the Protobuf team at Google to derive the location of generated files for known plugins. For example, if you have foo.proto, used protoc-gen-go, and had an output directory of ../../gen/proto/go, the list of generated files would be ["../../gen/proto/go/foo.pb.go"]. This would help with integration into build systems.

Behavior is confusing if you do not specify an argument for data for grpc

Example: cat input.json | prototool grpc example 0.0.0.0:8080 foo.ExcitedService/Exclamation

The trailing - is missing to denote stdin reading, but this is technically a legal invocation where it assumes you mean the current directory, with address example, method name 0.0.0.0:8080, and data foo.ExcitedService/Exclamation. We need to fix this in some manner, perhaps flags for the input args.

make cover need jq

jq json parser is needed.

make cover
./etc/bin/cover.sh: line 72: jq: command not found

Maybe is it possible to add a check for jq on make init ?

Error attempting "make example" on macOS 10.12.6 with brew installed go

I'm not entirely sure why this is failing. I have installed prototool in /usr/local/bin and the command prototool -h works. However attempting to build the examples does not.

go install \
		-ldflags "-X 'github.com/uber/prototool/internal/x/vars.GitCommit=db0d67f111a6bf4731d904e86b94fb39bd72cf3c' -X 'github.com/uber/prototool/internal/x/vars.BuiltTimestamp=Wed 11 Apr 2018 20:01:07 UTC'" \
		github.com/uber/prototool/cmd/prototool
can't load package: package github.com/uber/prototool/cmd/prototool: cannot find package "github.com/uber/prototool/cmd/prototool" in any of:
	/usr/local/Cellar/go/1.9/libexec/src/github.com/uber/prototool/cmd/prototool (from $GOROOT)
	/Users/jameshar/go/src/github.com/uber/prototool/cmd/prototool (from $GOPATH)
make: *** [install] Error 1

brew update and brew upgrade reports that everything is upto date. I have protobuf 3.5.1 installed using brew.

Warnings emitted by protoc plugins are failing the generation process

My plugin emits a warning to stderr, but exits with code 0 and no Error in the MessageResponse - however, prototool treats it as unrecoverable error.

prototool all "endpoint/"
2018-06-24T22:11:33.431+0300    WARN    protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new       {"protocLine": "I0624 22:11:33.429160   27601 handler.go:81] service/itemapi/itemapipb/client.proto: no target service defined in the file"}
<input>:1:1:I0624 22:11:33.429160   27601 handler.go:81] service/itemapi/itemapipb/client.proto: no target service defined in the file
make: *** [Makefile:54: gen] Error 255

Formatter omits empty option incorrectly

When running prototool format on the following proto:

syntax = "proto3";

import "protoc-gen-swagger/options/annotations.proto";

option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = {
	info: {
		version: "1.0"
	}
	external_docs: {}
	schemes: HTTPS
};

It produces the following output:

syntax = "proto3";

import "protoc-gen-swagger/options/annotations.proto";

option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = {
	info: {
		version: "1.0"
	}
	external_docs:
	schemes: HTTPS
};

This does not compile with protoc, instead giving the error:

$  
test.proto: Error while parsing option value for "openapiv2_swagger": Expected "{", found "schemes".

The correct thing here would be to leave the {}.

grpc - temporary protobuf descriptors are not cleaned up

prototool grpc leaves behind temporary protobuf descriptors on every call.

To reproduce, make any call with grpc and inspect your OS's typical temp file location. For example:

$ prototool grpc foo.Bar/Baz ''
$ ls /tmp/prototool*
/tmp/prototool661509915  /tmp/prototool892888492

Lint failure on protocol buffers version 2.6.1

I am using protocol buffers version 2.6.1 and linting fails with the following error on debug

2018-05-06T23:59:52.701+0200	DEBUG	downloaded protobuf zip file	{"url": "https://github.com/google/protobuf/releases/download/v2.6.1/protoc-2.6.1-osx-x86_64.zip"}
zip: not a valid zip file

The URL indeed does not work and as you can see from the file prototool tries to download we are talking about an OSX system here.

Is prototool compatible with version 2 protos?