Merge pull request #379 from yoheimuta/fix-rpc-names-upper-camel-disa…

…ble-comment fix: RPC_NAMES_UPPER_CAMEL_CASE disable is ignored when RPC has either leftcurly comment or embedded comments
yoheimuta · Apr 14, 2024 · ca174e0 · ca174e0
2 parents 46989b2 + cb95ce7
commit ca174e0
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 4 deletions.
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)