yoheimuta / protolint

A pluggable linter and fixer to enforce Protocol Buffer style and conventions.
MIT License
547 stars 49 forks source link

Some rules do not support auto-disable #369

Open dduugg opened 5 months ago

dduugg commented 5 months ago

I'm attempting to apply the default rules set to existing proto files. My understanding is that applying -auto_disable should apply the appropriate comment for disabling. However, I still see violations for the following rules after using -auto_disable:

  - ENUM_FIELDS_HAVE_COMMENT
  - ENUMS_HAVE_COMMENT
  - FIELD_NAMES_EXCLUDE_PREPOSITIONS
  - FIELDS_HAVE_COMMENT
  - FILE_HAS_COMMENT
  - MAX_LINE_LENGTH
  - MESSAGE_NAMES_EXCLUDE_PREPOSITIONS
  - MESSAGES_HAVE_COMMENT
  - RPC_NAMES_UPPER_CAMEL_CASE
  - RPCS_HAVE_COMMENT
  - SERVICES_HAVE_COMMENT
yoheimuta commented 5 months ago

@dduugg Thank you for sponsoring me 😸

Let me make sure I've got your point. Regarding ENUM_FIELDS_HAVE_COMMENT, it's not mentioned in the official style guide, hence it's not enabled by default.

I was under the impression that users with files missing comments shouldn't activate ENUM_FIELDS_HAVE_COMMENT to begin with.

dduugg commented 5 months ago

Hi @yoheimuta, happy to sponsor the project! You've saved me a lot of time.

With all_default: true, I do in fact see ENUM_FIELDS_HAVE_COMMENT violations.

What I am trying to do, is enable the default rules so that new proto files going forward are strictly enforced. However, I want inline disable the legacy violations. Excluding the files entirely is sub-optimal, because many still actively see changes.

I'm also struggling to figure out how to auto-disable custom rules. From a quick glance, it looks like RuleGen may need to add autodisable autodisable.PlacementType support, or something to that effect.

dduugg commented 5 months ago

I took a quick stab at adding auto-disable support to custom rules (I'm new to Go, please be kind): https://github.com/yoheimuta/protolint/compare/master...dduugg:protolint:autodisable

I got stuck trying to build the proto files though, not sure what to make of this error:

$ make dev/build/proto
protoc -I _proto _proto/*.proto --go_out=plugins=grpc:internal/addon/plugin/proto
--go_out: protoc-gen-go: plugins are not supported; use 'protoc --go-grpc_out=...' to generate gRPC

See https://grpc.io/docs/languages/go/quickstart/#regenerate-grpc-code for more information.
make: *** [dev/build/proto] Error 1
dduugg commented 5 months ago

This ticket is now low-priority, I wrote a quick wrapper to apply disables to existing violations:

#!/usr/bin/env python3

import json
import subprocess

def disable_protolint_violations() -> None:
    cmd = ["protolint", "-reporter", "json", "."]
    result = subprocess.run(cmd, stderr=subprocess.PIPE)
    # parse the json output and disable the rule at each violation:
    lints = json.loads(result.stderr)["lints"]
    if lints:
        for lint in lints:
            with open(lint["filename"], "r") as f:
                lines = f.readlines()
                line_num = lint["line"] - 1
                line = lines[line_num]
                if "protolint:disable:this" in line:
                    new_line = line[:-1] + f" {lint['rule']}\n"
                else:
                    new_line = line[:-1] + f" // protolint:disable:this {lint['rule']}\n"
                lines[line_num] = new_line
            with open(lint["filename"], "w") as f:
                f.writelines(lines)

disable_protolint_violations()
# Need to call a second time to disable line length violations
# that result from the first invocation
disable_protolint_violations()
yoheimuta commented 5 months ago

@dduugg Sorry for the late response.

What I am trying to do, is enable the default rules so that new proto files going forward are strictly enforced. However, I want inline disable the legacy violations. Excluding the files entirely is sub-optimal, because many still actively see changes.

I get your point. I hadn't supported the others simply because I wasn't sure if there was enough demand for it.

I took a quick stab at adding auto-disable support to custom rules (I'm new to Go, please be kind): https://github.com/yoheimuta/protolint/compare/master...dduugg:protolint:autodisable

This change looks good to me 👍

It looks like the latest protoc behave differently. How about the following workaround for now?

20:19:04 ~/yoheimuta/protolint $ make dev/build/proto                                                                                                      git/master
# protoc -I _proto _proto/*.proto --go_out=plugins=grpc:internal/addon/plugin/proto
protoc -I _proto _proto/*.proto --go_out=. --go-grpc_out=.
rm -rf internal/addon/plugin/proto
mv github.com/yoheimuta/protolint/internal/addon/plugin/proto internal/addon/plugin/
diff --git a/Makefile b/Makefile
index 7e07d10..0cd1ee0 100644
--- a/Makefile
+++ b/Makefile
@@ -34,7 +34,11 @@ dev/install/dep:

 ## dev/build/proto builds proto files under the _proto directory.
 dev/build/proto:
-   protoc -I _proto _proto/*.proto --go_out=plugins=grpc:internal/addon/plugin/proto
+   # protoc -I _proto _proto/*.proto --go_out=plugins=grpc:internal/addon/plugin/proto
+   protoc -I _proto _proto/*.proto --go_out=. --go-grpc_out=.
+   rm -rf internal/addon/plugin/proto
+   mv github.com/yoheimuta/protolint/internal/addon/plugin/proto internal/addon/plugin/

 ## ARG is command arguments.
 ARG=lint _example/proto
diff --git a/internal/addon/plugin/proto/plugin.pb.go b/internal/addon/plugin/proto/plugin.pb.go
index cd09665..05171d6 100644
--- a/internal/addon/plugin/proto/plugin.pb.go
+++ b/internal/addon/plugin/proto/plugin.pb.go
@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 //     protoc-gen-go v1.30.0
-//     protoc        v3.12.4
+//     protoc        v4.25.3
 // source: plugin.proto

 package proto
diff --git a/internal/addon/plugin/proto/plugin_grpc.pb.go b/internal/addon/plugin/proto/plugin_grpc.pb.go
index 3a6559a..21523ed 100644
--- a/internal/addon/plugin/proto/plugin_grpc.pb.go
+++ b/internal/addon/plugin/proto/plugin_grpc.pb.go
@@ -1,7 +1,7 @@
 // Code generated by protoc-gen-go-grpc. DO NOT EDIT.
 // versions:
 // - protoc-gen-go-grpc v1.3.0
-// - protoc             v3.12.4
+// - protoc             v4.25.3
 // source: plugin.proto

 package proto