Repository: googleapis/api-linter
Pull request #290 (2019-11-03 22:18:13)

[AIP-140] Document AIP-140 rules.


Patch:
diff --git a/docs/rules/core/0140-abbreviations.md b/docs/rules/core/0140-abbreviations.md
new file mode 100644
index 0000000..756deb7
--- /dev/null
+++ b/docs/rules/core/0140-abbreviations.md
@@ -0,0 +1,66 @@
+---
+rule:
+  aip: 140
+  name: [core, '0140', abbreviations]
+  summary: Field names should use common abbreviations.
+---
+
+# Field names: Abbreviations
+
+This rule enforces that field names use common abbreviations, as mandated in
+[AIP-140][].
+
+## Details
+
+This rule checks every descriptor in the proto and complains if the long form
+of any of the following words are used instead of the abbreviation:
+
+- configuration
+- identifier
+- information
+- specification
+- statistics
+
+## Examples
+
+### Single word method
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message Book {
+  string name = 1;
+  string identifier = 2;  // Should be `id`.
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message Book {
+  string name = 1;
+  string id = 2;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0140::abbreviations=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message Book {
+  string name = 1;
+  string identifier = 2;  // Should be `id`.
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-140]: https://aip.dev/140
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/docs/rules/core/0140-base64.md b/docs/rules/core/0140-base64.md
new file mode 100644
index 0000000..89ea382
--- /dev/null
+++ b/docs/rules/core/0140-base64.md
@@ -0,0 +1,66 @@
+---
+rule:
+  aip: 140
+  name: [core, '0140', base64]
+  summary: Base64 fields should use the `bytes` type.
+---
+
+# Base64 fields
+
+This rule tries to enforce that base64 fields use the `bytes` type, as mandated
+by [AIP-140][].
+
+## Details
+
+This rule checks the comments over each field, and if "base64" is mentioned, and yet
+the field is a `string`, it suggests using `bytes` instead.
+
+## Examples
+
+### Single word method
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message Book {
+  string name = 1;
+
+  // The base64-encoded checksum.
+  string checksum = 2;  // Should be bytes.
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message Book {
+  string name = 1;
+
+  // The base64-encoded checksum.
+  bytes checksum = 2;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0140::base64=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message Book {
+  string name = 1;
+
+  // The base64-encoded checksum
+  string checksum = 2;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-140]: https://aip.dev/140
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/docs/rules/core/0140-lower-snake.md b/docs/rules/core/0140-lower-snake.md
new file mode 100644
index 0000000..15a3f05
--- /dev/null
+++ b/docs/rules/core/0140-lower-snake.md
@@ -0,0 +1,60 @@
+---
+rule:
+  aip: 140
+  name: [core, '0140', lower-snake]
+  summary: Field names should use `snake_case`.
+---
+
+# Field names: Abbreviations
+
+This rule enforces that field names use `snake_case`, as mandated in
+[AIP-140][].
+
+## Details
+
+This rule checks every field in the proto and complains if the field name
+contains a capital letter.
+
+## Examples
+
+### Single word method
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message Book {
+  string name = 1;
+  int32 pageCount = 2;  // Should be `page_count`.
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message Book {
+  string name = 1;
+  int32 page_count = 2;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0140::lower-snake=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message Book {
+  string name = 1;
+  string pageCount = 2;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-140]: https://aip.dev/140
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/docs/rules/core/0140-prepositions.md b/docs/rules/core/0140-prepositions.md
new file mode 100644
index 0000000..7d919dc
--- /dev/null
+++ b/docs/rules/core/0140-prepositions.md
@@ -0,0 +1,60 @@
+---
+rule:
+  aip: 140
+  name: [core, '0140', prepositions]
+  summary: Fields must not include prepositions in their names.
+---
+
+# Field names: Prepositions
+
+This rule enforces that field names do not include most prepositions, as
+mandated in [AIP-140][].
+
+## Details
+
+This rule looks at each field and complains if it sees any of the following
+words in the method's name:
+
+{% include prepositions.md %}
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+message Book {
+  string name = 1;
+  string written_by = 2;  // Should be `author`.
+}
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+message Book {
+  string name = 1;
+  string author = 2;
+}
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the method.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0140::prepositions=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+message Book {
+  string name = 1;
+  string written_by = 2;
+}
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-136]: https://aip.dev/136
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0140/abbreviations.go b/rules/aip0140/abbreviations.go
index 530a72a..3951a33 100644
--- a/rules/aip0140/abbreviations.go
+++ b/rules/aip0140/abbreviations.go
@@ -22,76 +22,49 @@ import (
 	"github.com/jhump/protoreflect/desc"
 )
 
-// checkAbbreviations checks for each long name in the descriptor's name,
-// and returns problems if one is found that suggests the abbreviation instead.
-func checkAbbreviations(d desc.Descriptor, caseFunc func(string) string) (problems []lint.Problem) {
-	abbv := map[string]string{
-		"configuration": "config",
-		"identifier":    "id",
-		"information":   "info",
-		"specification": "spec",
-		"statistics":    "stats",
-	}
-
-	// Iterate over each abbreviation and determine whether the descriptor's
-	// name includes the long name.
-	for long, short := range abbv {
-		if strings.Contains(d.GetName(), caseFunc(long)) {
-			problems = append(problems, lint.Problem{
-				Message: fmt.Sprintf(
-					"Use the common abbreviation %q instead of %q.",
-					caseFunc(short),
-					caseFunc(long),
-				),
-				Suggestion: strings.ReplaceAll(d.GetName(), caseFunc(long), caseFunc(short)),
-				Descriptor: d,
-				Location:   lint.DescriptorNameLocation(d),
-			})
-		}
-	}
-	return
+var expectedAbbreviations = map[string]string{
+	"configuration": "config",
+	"identifier":    "id",
+	"information":   "info",
+	"specification": "spec",
+	"statistics":    "stats",
 }
 
-var abbreviationsField = &lint.FieldRule{
-	Name: lint.NewRuleName("core", "0140", "field-names", "abbreviations"),
+var abbreviations = &lint.DescriptorRule{
+	Name: lint.NewRuleName("core", "0140", "abbreviations"),
 	URI:  "https://aip.dev/140#abbreviations",
-	LintField: func(f *desc.FieldDescriptor) []lint.Problem {
-		return checkAbbreviations(f, strings.ToLower)
-	},
-}
-
-var abbreviationsMessage = &lint.MessageRule{
-	Name: lint.NewRuleName("core", "0140", "message-names", "abbreviations"),
-	URI:  "https://aip.dev/140#abbreviations",
-	LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
-		return checkAbbreviations(m, strings.Title)
-	},
-}
-
-var abbreviationsEnum = &lint.EnumRule{
-	Name: lint.NewRuleName("core", "0140", "enum-names", "abbreviations"),
-	URI:  "https://aip.dev/140#abbreviations",
-	LintEnum: func(e *desc.EnumDescriptor) []lint.Problem {
-		problems := checkAbbreviations(e, strings.Title)
-		for _, ev := range e.GetValues() {
-			problems = append(problems, checkAbbreviations(ev, strings.ToUpper)...)
+	LintDescriptor: func(d desc.Descriptor) (problems []lint.Problem) {
+		// Determine the correct case function to use.
+		// Most things in protobuf are PascalCase; the two exceptions are
+		// fields (snake case) and enum values (UPPER_CAMEL_CASE).
+		//
+		// We do not need to worry about word separators though, since
+		// we are checking for single words only.
+		var caseFunc func(string) string = strings.Title
+		switch d.(type) {
+		case *desc.FieldDescriptor:
+			caseFunc = strings.ToLower
+		case *desc.EnumValueDescriptor:
+			caseFunc = strings.ToUpper
 		}
-		return problems
-	},
-}
 
-var abbreviationsService = &lint.ServiceRule{
-	Name: lint.NewRuleName("core", "0140", "serivice-names", "abbreviations"),
-	URI:  "https://aip.dev/140#abbreviations",
-	LintService: func(s *desc.ServiceDescriptor) []lint.Problem {
-		return checkAbbreviations(s, strings.Title)
-	},
-}
+		// Iterate over each abbreviation and determine whether the descriptor's
+		// name includes the long name.
+		for long, short := range expectedAbbreviations {
+			if strings.Contains(d.GetName(), caseFunc(long)) {
+				problems = append(problems, lint.Problem{
+					Message: fmt.Sprintf(
+						"Use the common abbreviation %q instead of %q.",
+						caseFunc(short),
+						caseFunc(long),
+					),
+					Suggestion: strings.ReplaceAll(d.GetName(), caseFunc(long), caseFunc(short)),
+					Descriptor: d,
+					Location:   lint.DescriptorNameLocation(d),
+				})
+			}
+		}
+		return
 
-var abbreviationsMethod = &lint.MethodRule{
-	Name: lint.NewRuleName("core", "0140", "method-names", "abbreviations"),
-	URI:  "https://aip.dev/140#abbreviations",
-	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		return checkAbbreviations(m, strings.Title)
 	},
 }
diff --git a/rules/aip0140/aip0140.go b/rules/aip0140/aip0140.go
index f6c4f78..c4c14f3 100644
--- a/rules/aip0140/aip0140.go
+++ b/rules/aip0140/aip0140.go
@@ -27,11 +27,7 @@ import (
 // AddRules adds all of the AIP-140 rules to the provided registry.
 func AddRules(r lint.RuleRegistry) {
 	r.Register(
-		abbreviationsEnum,
-		abbreviationsField,
-		abbreviationsMessage,
-		abbreviationsMethod,
-		abbreviationsService,
+		abbreviations,
 		base64,
 		lowerSnake,
 		noPrepositions,


Test patch:
diff --git a/rules/aip0140/abbreviations_test.go b/rules/aip0140/abbreviations_test.go
index 6ed780b..16aed86 100644
--- a/rules/aip0140/abbreviations_test.go
+++ b/rules/aip0140/abbreviations_test.go
@@ -18,7 +18,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/googleapis/api-linter/lint"
 	"github.com/googleapis/api-linter/rules/internal/testutils"
 	"github.com/jhump/protoreflect/desc"
 	"github.com/stoewer/go-strcase"
@@ -27,14 +26,12 @@ import (
 func TestAbbreviations(t *testing.T) {
 	ruleGroups := []struct {
 		name     string
-		rule     lint.ProtoRule
 		tmpl     string
 		caseFunc func(string) string
 		descFunc func(*desc.FileDescriptor) desc.Descriptor
 	}{
 		{
 			name:     "Field",
-			rule:     abbreviationsField,
 			tmpl:     `message Book { string {{.Name}} = 1; }`,
 			caseFunc: strcase.SnakeCase,
 			descFunc: func(f *desc.FileDescriptor) desc.Descriptor {
@@ -43,7 +40,6 @@ func TestAbbreviations(t *testing.T) {
 		},
 		{
 			name:     "Message",
-			rule:     abbreviationsMessage,
 			tmpl:     `message {{.Name}} {}`,
 			caseFunc: strcase.UpperCamelCase,
 			descFunc: func(f *desc.FileDescriptor) desc.Descriptor {
@@ -52,7 +48,6 @@ func TestAbbreviations(t *testing.T) {
 		},
 		{
 			name:     "Service",
-			rule:     abbreviationsService,
 			tmpl:     `service {{.Name}} {}`,
 			caseFunc: strcase.UpperCamelCase,
 			descFunc: func(f *desc.FileDescriptor) desc.Descriptor {
@@ -61,14 +56,13 @@ func TestAbbreviations(t *testing.T) {
 		},
 		{
 			name: "Method",
-			rule: abbreviationsMethod,
 			tmpl: `
 				service Library {
-					rpc {{.Name}}({{.Name}}Request) returns ({{.Name}}Response);
+					rpc {{.Name}}(Request) returns (Response);
 				}
 
-				message {{.Name}}Request {}
-				message {{.Name}}Response {}
+				message Request {}
+				message Response {}
 			`,
 			caseFunc: strcase.UpperCamelCase,
 			descFunc: func(f *desc.FileDescriptor) desc.Descriptor {
@@ -77,7 +71,6 @@ func TestAbbreviations(t *testing.T) {
 		},
 		{
 			name:     "Enum",
-			rule:     abbreviationsEnum,
 			tmpl:     `enum {{.Name}} { UNSPECIFIED = 0; }`,
 			caseFunc: strcase.UpperCamelCase,
 			descFunc: func(f *desc.FileDescriptor) desc.Descriptor {
@@ -86,7 +79,6 @@ func TestAbbreviations(t *testing.T) {
 		},
 		{
 			name: "EnumValue",
-			rule: abbreviationsEnum,
 			tmpl: `enum Thing { {{.Name}} = 0; }`,
 			caseFunc: func(s string) string {
 				return strings.ToUpper(strcase.SnakeCase(s))
@@ -106,7 +98,7 @@ func TestAbbreviations(t *testing.T) {
 					d := group.descFunc(file)
 
 					// Establish that we get the problems we expect.
-					problems := group.rule.Lint(file)
+					problems := abbreviations.Lint(file)
 					if diff := test.problems.SetDescriptor(d).Diff(problems); diff != "" {
 						t.Errorf(diff)
 					}