Repository: googleapis/api-linter
Pull request #227 (2019-10-11 04:07:34)

Standard method synonyms
This adds synonym checks for Get, Create, Update.


Patch:
diff --git a/rules/aip0131/aip0131.go b/rules/aip0131/aip0131.go
index bd1abbe..0c2c97b 100644
--- a/rules/aip0131/aip0131.go
+++ b/rules/aip0131/aip0131.go
@@ -32,6 +32,7 @@ func AddRules(r lint.RuleRegistry) {
 		responseMessageName,
 		requestMessageName,
 		standardFields,
+		synonyms,
 		unknownFields,
 	)
 }
diff --git a/rules/aip0131/method_rules.go b/rules/aip0131/method_rules.go
index 308e3b9..e5822c8 100644
--- a/rules/aip0131/method_rules.go
+++ b/rules/aip0131/method_rules.go
@@ -16,9 +16,10 @@ package aip0131
 
 import (
 	"fmt"
+	"strings"
 
-	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 )
 
@@ -129,3 +130,26 @@ var httpBody = &lint.MethodRule{
 		return nil
 	},
 }
+
+// Get methods should not generally use synonyms for "get".
+var synonyms = &lint.MethodRule{
+	Name: lint.NewRuleName("core", "0131", "synonyms"),
+	URI:  "https://aip.dev/131#guidance",
+	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
+		name := m.GetName()
+		for _, syn := range []string{"Acquire", "Fetch", "Lookup", "Read", "Retrieve"} {
+			if strings.HasPrefix(name, syn) {
+				return []lint.Problem{{
+					Message: fmt.Sprintf(
+						`%q can be a synonym for "Get". Should this be a Get method?`,
+						syn,
+					),
+					Suggestion: strings.Replace(name, syn, "Get", 1),
+					Descriptor: m,
+					Location:   lint.DescriptorNameLocation(m),
+				}}
+			}
+		}
+		return nil
+	},
+}
diff --git a/rules/aip0133/aip0133.go b/rules/aip0133/aip0133.go
index a2c06b5..cd0c74e 100644
--- a/rules/aip0133/aip0133.go
+++ b/rules/aip0133/aip0133.go
@@ -27,13 +27,14 @@ import (
 // this AIP's rules to it.
 func AddRules(r lint.RuleRegistry) {
 	r.Register(
-		httpUriField,
+		httpURIField,
 		httpVerb,
 		httpBody,
 		inputName,
 		outputName,
 		resourceField,
 		parentField,
+		synonyms,
 		unknownFields,
 	)
 }
@@ -59,9 +60,8 @@ func getResourceMsgName(m *desc.MethodDescriptor) string {
 	// method or output naming doesn't follow the right principles)
 	if strings.Contains(m.GetName()[6:], m.GetOutputType().GetName()) {
 		return m.GetOutputType().GetName()
-	} else {
-		return m.GetName()[6:]
 	}
+	return m.GetName()[6:]
 }
 
 // get resource message type name from request message
diff --git a/rules/aip0133/method_rules.go b/rules/aip0133/method_rules.go
index 36feb5c..e46ed93 100644
--- a/rules/aip0133/method_rules.go
+++ b/rules/aip0133/method_rules.go
@@ -44,7 +44,7 @@ var httpVerb = &lint.MethodRule{
 }
 
 // Create methods should have a proper HTTP pattern.
-var httpUriField = &lint.MethodRule{
+var httpURIField = &lint.MethodRule{
 	Name:   lint.NewRuleName("core", "0133", "http-uri"),
 	URI:    "https://aip.dev/133#guidance",
 	OnlyIf: isCreateMethod,
@@ -169,3 +169,26 @@ var outputName = &lint.MethodRule{
 		return nil
 	},
 }
+
+// Create methods should use "create", not synonyms.
+var synonyms = &lint.MethodRule{
+	Name: lint.NewRuleName("core", "0133", "synonyms"),
+	URI:  "https://aip.dev/133",
+	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
+		name := m.GetName()
+		for _, syn := range []string{"Insert", "Make", "Post"} {
+			if strings.HasPrefix(name, syn) {
+				return []lint.Problem{{
+					Message: fmt.Sprintf(
+						`%q can be a synonym for "Create". Should this be a Create method?`,
+						syn,
+					),
+					Descriptor: m,
+					Location:   lint.DescriptorNameLocation(m),
+					Suggestion: strings.Replace(name, syn, "Create", 1),
+				}}
+			}
+		}
+		return nil
+	},
+}
diff --git a/rules/aip0134/aip0134.go b/rules/aip0134/aip0134.go
index e1ca2cf..2cd8e1f 100644
--- a/rules/aip0134/aip0134.go
+++ b/rules/aip0134/aip0134.go
@@ -32,6 +32,7 @@ func AddRules(r lint.RuleRegistry) {
 		responseMessageName,
 		requestMessageName,
 		standardFields,
+		synonyms,
 		unknownFields,
 	)
 }
diff --git a/rules/aip0134/method_rules.go b/rules/aip0134/method_rules.go
index b45d162..2148b97 100644
--- a/rules/aip0134/method_rules.go
+++ b/rules/aip0134/method_rules.go
@@ -16,9 +16,10 @@ package aip0134
 
 import (
 	"fmt"
+	"strings"
 
-	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
 	"github.com/jhump/protoreflect/desc"
 	"github.com/stoewer/go-strcase"
 )
@@ -106,7 +107,7 @@ var httpNameField = &lint.MethodRule{
 		// Establish that the RPC has expected HTTP pattern.
 		for _, httpRule := range utils.GetHTTPRules(m) {
 			if uri := httpRule.GetPatch(); uri != "" {
-				matches := updateURINameRegexp.FindStringSubmatch(uri);
+				matches := updateURINameRegexp.FindStringSubmatch(uri)
 				if matches == nil || matches[1] != fieldName {
 					return []lint.Problem{{
 						Message:    fmt.Sprintf("Update methods should include the `%s.name` field in the URI.", fieldName),
@@ -140,3 +141,26 @@ var httpBody = &lint.MethodRule{
 		return nil
 	},
 }
+
+// Update methods should use the word "update", not synonyms.
+var synonyms = &lint.MethodRule{
+	Name: lint.NewRuleName("core", "0134", "synonyms"),
+	URI:  "https://aip.dev/134#guidance",
+	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
+		name := m.GetName()
+		for _, syn := range []string{"Patch", "Put", "Set"} {
+			if strings.HasPrefix(name, syn) {
+				return []lint.Problem{{
+					Message: fmt.Sprintf(
+						`%q can be a synonym for "Update". Should this be a Update method?`,
+						syn,
+					),
+					Descriptor: m,
+					Location:   lint.DescriptorNameLocation(m),
+					Suggestion: strings.Replace(name, syn, "Update", 1),
+				}}
+			}
+		}
+		return nil
+	},
+}


Test patch:
diff --git a/rules/aip0131/method_rules_test.go b/rules/aip0131/method_rules_test.go
index 8690bd9..b143ff3 100644
--- a/rules/aip0131/method_rules_test.go
+++ b/rules/aip0131/method_rules_test.go
@@ -247,3 +247,30 @@ func TestHttpNameField(t *testing.T) {
 		})
 	}
 }
+
+func TestSynonyms(t *testing.T) {
+	tests := []struct {
+		MethodName string
+		problems   testutils.Problems
+	}{
+		{"GetBook", testutils.Problems{}},
+		{"AcquireBook", testutils.Problems{{Suggestion: "GetBook"}}},
+		{"FetchBook", testutils.Problems{{Suggestion: "GetBook"}}},
+		{"LookupBook", testutils.Problems{{Suggestion: "GetBook"}}},
+		{"ReadBook", testutils.Problems{{Suggestion: "GetBook"}}},
+		{"RetrieveBook", testutils.Problems{{Suggestion: "GetBook"}}},
+	}
+	for _, test := range tests {
+		file := testutils.ParseProto3Tmpl(t, `
+			service Library {
+				rpc {{.MethodName}}({{.MethodName}}Request) returns (Book);
+			}
+			message {{.MethodName}}Request {}
+			message Book {}
+		`, test)
+		m := file.GetServices()[0].GetMethods()[0]
+		if diff := test.problems.SetDescriptor(m).Diff(synonyms.Lint(file)); diff != "" {
+			t.Errorf(diff)
+		}
+	}
+}
diff --git a/rules/aip0133/method_rules_test.go b/rules/aip0133/method_rules_test.go
index 45d67b4..39f0bc4 100644
--- a/rules/aip0133/method_rules_test.go
+++ b/rules/aip0133/method_rules_test.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/golang/protobuf/proto"
 	dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
+	"github.com/googleapis/api-linter/rules/internal/testutils"
 	"github.com/jhump/protoreflect/desc/builder"
 	"google.golang.org/genproto/googleapis/api/annotations"
 )
@@ -116,7 +117,7 @@ func TestHttpUriField(t *testing.T) {
 			}
 
 			// Run the method, ensure we get what we expect.
-			problems := httpUriField.Lint(service.GetFile())
+			problems := httpURIField.Lint(service.GetFile())
 			if test.msg == "" && len(problems) > 0 {
 				t.Errorf("Got %v, expected no problems.", problems)
 			} else if test.msg != "" && len(problems) == 0 {
@@ -263,3 +264,30 @@ func TestOutputMessageName(t *testing.T) {
 		})
 	}
 }
+
+func TestSynonyms(t *testing.T) {
+	tests := []struct {
+		MethodName string
+		problems   testutils.Problems
+	}{
+		{"CreateBook", testutils.Problems{}},
+		{"InsertBook", testutils.Problems{{Suggestion: "CreateBook"}}},
+		{"MakeBook", testutils.Problems{{Suggestion: "CreateBook"}}},
+		{"PostBook", testutils.Problems{{Suggestion: "CreateBook"}}},
+	}
+	for _, test := range tests {
+		t.Run(test.MethodName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+				service Library {
+					rpc {{.MethodName}}({{.MethodName}}Request) returns (Book);
+				}
+				message {{.MethodName}}Request {}
+				message Book {}
+			`, test)
+			m := file.GetServices()[0].GetMethods()[0]
+			if diff := test.problems.SetDescriptor(m).Diff(synonyms.Lint(file)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}
diff --git a/rules/aip0134/method_rules_test.go b/rules/aip0134/method_rules_test.go
index ca6b82f..aaa2a48 100644
--- a/rules/aip0134/method_rules_test.go
+++ b/rules/aip0134/method_rules_test.go
@@ -270,3 +270,30 @@ func TestHttpNameField(t *testing.T) {
 		})
 	}
 }
+
+func TestSynonyms(t *testing.T) {
+	tests := []struct {
+		MethodName string
+		problems   testutils.Problems
+	}{
+		{"UpdateBook", testutils.Problems{}},
+		{"PatchBook", testutils.Problems{{Suggestion: "UpdateBook"}}},
+		{"PutBook", testutils.Problems{{Suggestion: "UpdateBook"}}},
+		{"SetBook", testutils.Problems{{Suggestion: "UpdateBook"}}},
+	}
+	for _, test := range tests {
+		t.Run(test.MethodName, func(t *testing.T) {
+			file := testutils.ParseProto3Tmpl(t, `
+				service Library {
+					rpc {{.MethodName}}({{.MethodName}}Request) returns (Book);
+				}
+				message {{.MethodName}}Request {}
+				message Book {}
+			`, test)
+			m := file.GetServices()[0].GetMethods()[0]
+			if diff := test.problems.SetDescriptor(m).Diff(synonyms.Lint(file)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}