Add option for reporting lint errors as warnings

[cheister]

I'm trying to add buf linting to our CI process with the protoc-gen-buf-lint plugin but it fails the build when it gets the first error. I'd like to be able to run it with new lint rules and see how many errors there will be all at once.

Can you add an option that allows reporting linting errors as warnings instead of errors? This would also be useful so some of the lint rules could be printed as warnings during the build instead of errors.

[amckinney]

I'm not sure that's accurate - protoc-gen-buf-lint should return all of the lint failures with respect to the target input.

For example, if I add a lint failure in two files, I will see both of them in a single invocation:

$ buf protoc -I proto proto/buf/alpha/image/v1/image.proto proto/buf/alpha/module/v1alpha1/module.proto --buf-lint_out=.
--buf-lint_out: buf/alpha/image/v1/image.proto:28:22:Field name "File" should be lower_snake_case, such as "file".
buf/alpha/module/v1alpha1/module.proto:40:3:Enum zero value name "FOO_SOMETHING" should be suffixed with "_UNSPECIFIED".

Are you sure that you're specifying all of the files you want to lint? Maybe that's why you're not seeing everything you expect.

Plus, you really ought to using buf lint for a better experience (instead of protoc-gen-buf-lint). You can get the same effect with the following:

$ buf lint proto
proto/buf/alpha/image/v1/image.proto:28:22:Field name "File" should be lower_snake_case, such as "file".
proto/buf/alpha/module/v1alpha1/module.proto:40:3:Enum zero value name "FOO_SOMETHING" should be suffixed with "_UNSPECIFIED".

If you're using GitHub Actions, check out https://github.com/bufbuild/buf-lint-action. Otherwise, check out our more general documentation on CI/CD.

[cheister]

I'm adding this to a Bazel build and don't have the option to use buf lint or specify all of the proto files in one invocation. Bazel builds each proto file as a separate target. It would be great if I could specify an option so the lint warning is printed but it doesn't fail the build.

[amckinney]

Got it, thanks for clarifying that you're using Bazel.

I'm still not convinced that we need to expose any new configuration though - why would emitting it as a warning (i.e. a zero exit code) change anything in your case? All of the failures contained in a single file should be included in the output, not just the first one like you've described (I've tested this locally). What's the value in running the linter, but having a successful build if there are actually lint failures?

For what it's worth, another team put together rules-proto-grpc which provides Bazel rules to run protoc-gen-buf-lint. Please let us know what you think!

[cheister]

Right, but only seeing the failures for one proto file is not helpful since the build fails and there isn't a good way to see all the errors or warnings for for all of the proto files.

Mainly it's for migrating a code base to using the linter. It's hard to figure out how much work it will be to migrate to using the linter or a new rule if I don't know how many errors a linting rule will generate for the whole build. I also can't fix them all at once because they are only reported one failure at time.

I would also like to use some of the linting rules as guidelines instead of hard failures if possible.

[amckinney]

Ok, this sounds like a problem with with your rule implementation. When you mentioned that you're only seeing one lint failure at a time, you're probably referring to Bazel stopping after it encounters the first build error, right?

I imagine that this could be controlled within the build rule that you're authoring. Could you invoke buf but ignore the non-zero exit code so that other build rules continue to run? In the absence of a flag (or some other configuration that controls whether a warning or error is emitted), I think you can can do it with a run_shell + protoc-gen-lint || true (or something along those lines).

Also note that https://rules-proto-grpc.com/en/latest/lang/buf.html#buf-proto-lint-test is able to get around all of this by simply writing their rules as tests. So, for every proto rule you would have a test rule. This means that the whole build will continue to work, and only when you run the tests some targets will fail (as expected), but all of them will be run.

I understand that it'd be much easier for you to configure something to simply say, "don't return a non-zero exit code", but we'll need to carefully consider whether or not we include a configuration option like this and whether or not this is a whole new lint section in the buf.yaml. Given that this is only useful for such a specific use case right now, I'd prefer if we went with one of the other options above for now.

[bufdev]

I don't think we're going to change the exit code behavior of buf - our view is that when you make lint errors into warnings, they just get ignored, which defeats the purpose of a linter. Almost all linters will exit with a non-zero code on failure. Let us know if you have other issues though, happy to discuss, including on our Slack workspace at https://buf.build/links/slack