From cb95ce738158808ca7be0aad862baad7e25593ad Mon Sep 17 00:00:00 2001 From: Yohei Yoshimuta Date: Sun, 14 Apr 2024 21:02:01 +0900 Subject: [PATCH] fix: RPC_NAMES_UPPER_CAMEL_CASE disable is ignored when RPC has embedded comments --- .../disabled_rpc_names_upper_camel_case.proto | 9 +++++++++ .../invalid_rpc_names_upper_camel_case.proto | 9 +++++++++ .../disable_this_left_curly.proto | 9 +++++++++ go.mod | 2 +- go.sum | 4 ++-- .../addon/rules/rpcNamesUpperCamelCaseRule_test.go | 12 ++++++++++++ linter/visitor/extendedDisableRuleVisitor.go | 5 ++++- 7 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 _example/proto/issue_368/disabled_rpc_names_upper_camel_case.proto create mode 100644 _example/proto/issue_368/invalid_rpc_names_upper_camel_case.proto create mode 100644 _testdata/rules/rpcNamesUpperCamelCase/disable_this_left_curly.proto diff --git a/_example/proto/issue_368/disabled_rpc_names_upper_camel_case.proto b/_example/proto/issue_368/disabled_rpc_names_upper_camel_case.proto new file mode 100644 index 00000000..7187b5b7 --- /dev/null +++ b/_example/proto/issue_368/disabled_rpc_names_upper_camel_case.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +service OrgSubscriptionGrpcManager { + rpc findCurrentOrgSubscription (FindCurrentOrgSubscriptionRequest) returns (OneOrgSubscriptionResponse) { // protolint:disable:this RPC_NAMES_UPPER_CAMEL_CASE // + } + rpc findPartialPayEligibleSubscription (FindPartialPayEligibleSubscriptionRequest) // protolint:disable:this RPC_NAMES_UPPER_CAMEL_CASE + returns (OneOrgSubscriptionResponse) { + } +} diff --git a/_example/proto/issue_368/invalid_rpc_names_upper_camel_case.proto b/_example/proto/issue_368/invalid_rpc_names_upper_camel_case.proto new file mode 100644 index 00000000..1e938025 --- /dev/null +++ b/_example/proto/issue_368/invalid_rpc_names_upper_camel_case.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +service OrgSubscriptionGrpcManager { + rpc findCurrentOrgSubscription (FindCurrentOrgSubscriptionRequest) returns (OneOrgSubscriptionResponse) { + } + rpc findPartialPayEligibleSubscription (FindPartialPayEligibleSubscriptionRequest) + returns (OneOrgSubscriptionResponse) { + } +} diff --git a/_testdata/rules/rpcNamesUpperCamelCase/disable_this_left_curly.proto b/_testdata/rules/rpcNamesUpperCamelCase/disable_this_left_curly.proto new file mode 100644 index 00000000..1592b834 --- /dev/null +++ b/_testdata/rules/rpcNamesUpperCamelCase/disable_this_left_curly.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; + +service OrgSubscriptionGrpcManager { + rpc findCurrentOrgSubscription (FindCurrentOrgSubscriptionRequest) returns (OneOrgSubscriptionResponse) { // protolint:disable:this RPC_NAMES_UPPER_CAMEL_CASE + } + rpc findPartialPayEligibleSubscription (FindPartialPayEligibleSubscriptionRequest) // protolint:disable:this RPC_NAMES_UPPER_CAMEL_CASE + returns (OneOrgSubscriptionResponse) { + } +} diff --git a/go.mod b/go.mod index d98243e2..9a1dcf06 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/golang/protobuf v1.5.2 github.com/hashicorp/go-hclog v1.2.0 github.com/hashicorp/go-plugin v1.4.3 - github.com/yoheimuta/go-protoparser/v4 v4.7.0 + github.com/yoheimuta/go-protoparser/v4 v4.10.0 google.golang.org/grpc v1.46.0 google.golang.org/protobuf v1.27.1 gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index eb3ba696..b62ea56b 100644 --- a/go.sum +++ b/go.sum @@ -92,8 +92,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -github.com/yoheimuta/go-protoparser/v4 v4.7.0 h1:80LGfVM25sCoNDD08hv9O0ShQMjoTrIE76j5ON+gq3U= -github.com/yoheimuta/go-protoparser/v4 v4.7.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8= +github.com/yoheimuta/go-protoparser/v4 v4.10.0 h1:AcFzoUwzO7NmuluJvVnNjjKjDNVWkq/+3RHU9t0lnKs= +github.com/yoheimuta/go-protoparser/v4 v4.10.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= diff --git a/internal/addon/rules/rpcNamesUpperCamelCaseRule_test.go b/internal/addon/rules/rpcNamesUpperCamelCaseRule_test.go index 7eeb1248..bce95c5e 100644 --- a/internal/addon/rules/rpcNamesUpperCamelCaseRule_test.go +++ b/internal/addon/rules/rpcNamesUpperCamelCaseRule_test.go @@ -179,6 +179,18 @@ func TestRPCNamesUpperCamelCaseRule_Apply_disable(t *testing.T) { inputPlacementType: autodisable.ThisThenNext, wantFilename: "disable_this.proto", }, + { + name: "apply nothing to already disabled lines", + inputFilename: "disable_this.proto", + inputPlacementType: autodisable.ThisThenNext, + wantFilename: "disable_this.proto", + }, + { + name: "apply nothing to already disabled lines behind a left curly. See #368", + inputFilename: "disable_this_left_curly.proto", + inputPlacementType: autodisable.ThisThenNext, + wantFilename: "disable_this_left_curly.proto", + }, } for _, test := range tests { diff --git a/linter/visitor/extendedDisableRuleVisitor.go b/linter/visitor/extendedDisableRuleVisitor.go index 084f16a9..42e5f1a1 100644 --- a/linter/visitor/extendedDisableRuleVisitor.go +++ b/linter/visitor/extendedDisableRuleVisitor.go @@ -137,7 +137,10 @@ func (v extendedDisableRuleVisitor) VisitReserved(r *parser.Reserved) (next bool } func (v extendedDisableRuleVisitor) VisitRPC(r *parser.RPC) (next bool) { - if v.interpreter.Interpret(r.Comments, r.InlineComment) { + var inlines []*parser.Comment + inlines = append(inlines, r.InlineComment, r.InlineCommentBehindLeftCurly) + inlines = append(inlines, r.EmbeddedComments...) + if v.interpreter.Interpret(r.Comments, inlines...) { return true } return v.inner.VisitRPC(r)