Repository: googleapis/api-linter
Pull request #389 (2019-11-25 22:44:15)

[refactor] Make utils.GetFieldBehavior better.
Instead of returning a slice of the obnoxious-to-use enum
annotation, we return a stringset instead. (Yay, `Contains`!)


Patch:
diff --git a/rules/aip0203/immutable.go b/rules/aip0203/immutable.go
index fde4366..76560e3 100644
--- a/rules/aip0203/immutable.go
+++ b/rules/aip0203/immutable.go
@@ -20,7 +20,6 @@ import (
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
-	"google.golang.org/genproto/googleapis/api/annotations"
 )
 
 var immutable = &lint.FieldRule{
@@ -34,10 +33,5 @@ var immutable = &lint.FieldRule{
 var immutableRegexp = regexp.MustCompile("(?i).*immutable.*")
 
 func withoutImmutableFieldBehavior(f *desc.FieldDescriptor) bool {
-	for _, v := range utils.GetFieldBehavior(f) {
-		if v == annotations.FieldBehavior_IMMUTABLE {
-			return false
-		}
-	}
-	return true
+	return !utils.GetFieldBehavior(f).Contains("IMMUTABLE")
 }
diff --git a/rules/aip0203/input_only.go b/rules/aip0203/input_only.go
index 71f0874..f41d5e0 100644
--- a/rules/aip0203/input_only.go
+++ b/rules/aip0203/input_only.go
@@ -20,7 +20,6 @@ import (
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
-	"google.golang.org/genproto/googleapis/api/annotations"
 )
 
 var inputOnly = &lint.FieldRule{
@@ -34,10 +33,5 @@ var inputOnly = &lint.FieldRule{
 var inputOnlyRegexp = regexp.MustCompile("(?i).*input.?only.*")
 
 func withoutInputOnlyFieldBehavior(f *desc.FieldDescriptor) bool {
-	for _, v := range utils.GetFieldBehavior(f) {
-		if v == annotations.FieldBehavior_INPUT_ONLY {
-			return false
-		}
-	}
-	return true
+	return !utils.GetFieldBehavior(f).Contains("INPUT_ONLY")
 }
diff --git a/rules/aip0203/optional.go b/rules/aip0203/optional.go
index c1a8af9..5227bb3 100644
--- a/rules/aip0203/optional.go
+++ b/rules/aip0203/optional.go
@@ -20,7 +20,6 @@ import (
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
-	"google.golang.org/genproto/googleapis/api/annotations"
 )
 
 var optional = &lint.FieldRule{
@@ -34,20 +33,13 @@ var optional = &lint.FieldRule{
 var optionalRegexp = regexp.MustCompile("(?i).*optional.*")
 
 func withoutOptionalFieldBehavior(f *desc.FieldDescriptor) bool {
-	for _, v := range utils.GetFieldBehavior(f) {
-		if v == annotations.FieldBehavior_OPTIONAL {
-			return false
-		}
-	}
-	return true
+	return !utils.GetFieldBehavior(f).Contains("OPTIONAL")
 }
 
 func messageHasOptionalFieldBehavior(m *desc.MessageDescriptor) bool {
 	for _, f := range m.GetFields() {
-		for _, v := range utils.GetFieldBehavior(f) {
-			if v == annotations.FieldBehavior_OPTIONAL {
-				return true
-			}
+		if utils.GetFieldBehavior(f).Contains("OPTIONAL") {
+			return true
 		}
 	}
 	return false
diff --git a/rules/aip0203/output_only.go b/rules/aip0203/output_only.go
index 468709f..c18c571 100644
--- a/rules/aip0203/output_only.go
+++ b/rules/aip0203/output_only.go
@@ -20,7 +20,6 @@ import (
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
-	"google.golang.org/genproto/googleapis/api/annotations"
 )
 
 var outputOnly = &lint.FieldRule{
@@ -34,10 +33,5 @@ var outputOnly = &lint.FieldRule{
 var outputOnlyRegexp = regexp.MustCompile("(?i).*output.?only.*")
 
 func withoutOutputOnlyFieldBehavior(f *desc.FieldDescriptor) bool {
-	for _, v := range utils.GetFieldBehavior(f) {
-		if v == annotations.FieldBehavior_OUTPUT_ONLY {
-			return false
-		}
-	}
-	return true
+	return !utils.GetFieldBehavior(f).Contains("OUTPUT_ONLY")
 }
diff --git a/rules/aip0203/required.go b/rules/aip0203/required.go
index ce10d07..a4f2fdd 100644
--- a/rules/aip0203/required.go
+++ b/rules/aip0203/required.go
@@ -20,7 +20,6 @@ import (
 	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
-	"google.golang.org/genproto/googleapis/api/annotations"
 )
 
 var required = &lint.FieldRule{
@@ -34,10 +33,5 @@ var required = &lint.FieldRule{
 var requiredRegexp = regexp.MustCompile("(?i).*required.*")
 
 func withoutRequiredFieldBehavior(f *desc.FieldDescriptor) bool {
-	for _, v := range utils.GetFieldBehavior(f) {
-		if v == annotations.FieldBehavior_REQUIRED {
-			return false
-		}
-	}
-	return true
+	return !utils.GetFieldBehavior(f).Contains("REQUIRED")
 }
diff --git a/rules/internal/utils/extension.go b/rules/internal/utils/extension.go
index ce606b2..1a10d0a 100644
--- a/rules/internal/utils/extension.go
+++ b/rules/internal/utils/extension.go
@@ -15,18 +15,23 @@
 package utils
 
 import (
+	"bitbucket.org/creachadair/stringset"
 	"github.com/golang/protobuf/proto"
 	"github.com/jhump/protoreflect/desc"
 	apb "google.golang.org/genproto/googleapis/api/annotations"
 	lrpb "google.golang.org/genproto/googleapis/longrunning"
 )
 
-// GetFieldBehavior returns a slice of FieldBehavior annotations for
+// GetFieldBehavior returns a stringset.Set of FieldBehavior annotations for
 // the given field.
-func GetFieldBehavior(f *desc.FieldDescriptor) []apb.FieldBehavior {
+func GetFieldBehavior(f *desc.FieldDescriptor) stringset.Set {
 	opts := f.GetFieldOptions()
 	if x, err := proto.GetExtension(opts, apb.E_FieldBehavior); err == nil {
-		return x.([]apb.FieldBehavior)
+		answer := stringset.New()
+		for _, fb := range x.([]apb.FieldBehavior) {
+			answer.Add(fb.String())
+		}
+		return answer
 	}
 	return nil
 }


Test patch:
diff --git a/rules/internal/utils/extension_test.go b/rules/internal/utils/extension_test.go
index d161aa0..0d08972 100644
--- a/rules/internal/utils/extension_test.go
+++ b/rules/internal/utils/extension_test.go
@@ -17,9 +17,9 @@ package utils
 import (
 	"testing"
 
+	"bitbucket.org/creachadair/stringset"
 	"github.com/google/go-cmp/cmp"
 	"github.com/googleapis/api-linter/rules/internal/testutils"
-	apb "google.golang.org/genproto/googleapis/api/annotations"
 )
 
 func TestGetFieldBehavior(t *testing.T) {
@@ -39,10 +39,10 @@ func TestGetFieldBehavior(t *testing.T) {
 	msg := fd.GetMessageTypes()[0]
 	tests := []struct {
 		fieldName      string
-		fieldBehaviors []apb.FieldBehavior
+		fieldBehaviors stringset.Set
 	}{
-		{"name", []apb.FieldBehavior{apb.FieldBehavior_IMMUTABLE, apb.FieldBehavior_OUTPUT_ONLY}},
-		{"title", []apb.FieldBehavior{apb.FieldBehavior_REQUIRED}},
+		{"name", stringset.New("IMMUTABLE", "OUTPUT_ONLY")},
+		{"title", stringset.New("REQUIRED")},
 		{"summary", nil},
 	}
 	for _, test := range tests {