Repository: uber/prototool
Pull request #284 (2019-01-10 05:24:27)

 Add linters for suffixes for google.protobuf.Timestamp and google.protobuf.Duration
Re: #265.


Patch:
diff --git a/internal/lint/check_service_names_api_suffix.go b/internal/lint/check_service_names_api_suffix.go
new file mode 100644
index 0000000..d17072e
--- /dev/null
+++ b/internal/lint/check_service_names_api_suffix.go
@@ -0,0 +1,52 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package lint
+
+import (
+	"strings"
+
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var serviceNamesAPISuffixLinter = NewLinter(
+	"SERVICE_NAMES_API_SUFFIX",
+	`Verifies that all service names end with "API".`,
+	checkServiceNamesAPISuffix,
+)
+
+func checkServiceNamesAPISuffix(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
+	return runVisitor(serviceNamesAPISuffixVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type serviceNamesAPISuffixVisitor struct {
+	baseAddVisitor
+}
+
+func (v serviceNamesAPISuffixVisitor) VisitService(service *proto.Service) {
+	if !strings.HasSuffix(service.Name, "API") {
+		if strings.HasSuffix(service.Name, "Service") {
+			v.AddFailuref(service.Position, `Service name %q must end with "API". Since it currently ends with "Service", this likely means replacing "Service" with "API"`, service.Name)
+		} else {
+			v.AddFailuref(service.Position, `Service name %q must end with "API".`, service.Name)
+		}
+	}
+}
diff --git a/internal/lint/check_wkt_duration_suffix.go b/internal/lint/check_wkt_duration_suffix.go
new file mode 100644
index 0000000..4911f82
--- /dev/null
+++ b/internal/lint/check_wkt_duration_suffix.go
@@ -0,0 +1,68 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package lint
+
+import (
+	"strings"
+
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var wktDurationSuffixLinter = NewLinter(
+	"WKT_DURATION_SUFFIX",
+	"Verifies that all google.protobuf.Duration field names end in _duration.",
+	checkWKTDurationSuffix,
+)
+
+func checkWKTDurationSuffix(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
+	return runVisitor(wktDurationSuffixVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type wktDurationSuffixVisitor struct {
+	baseAddVisitor
+}
+
+func (v wktDurationSuffixVisitor) VisitMessage(message *proto.Message) {
+	for _, element := range message.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v wktDurationSuffixVisitor) VisitOneof(oneof *proto.Oneof) {
+	for _, element := range oneof.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v wktDurationSuffixVisitor) VisitNormalField(field *proto.NormalField) {
+	v.visitField(field.Field)
+}
+
+func (v wktDurationSuffixVisitor) VisitOneofField(field *proto.OneOfField) {
+	v.visitField(field.Field)
+}
+
+func (v wktDurationSuffixVisitor) visitField(field *proto.Field) {
+	if field.Type == "google.protobuf.Duration" && !strings.HasSuffix(field.Name, "_duration") {
+		v.AddFailuref(field.Position, `Field %q of type "google.protobuf.Duration" must end in "_duration".`, field.Name)
+	}
+}
diff --git a/internal/lint/check_wkt_timestamp_suffix.go b/internal/lint/check_wkt_timestamp_suffix.go
new file mode 100644
index 0000000..c78a510
--- /dev/null
+++ b/internal/lint/check_wkt_timestamp_suffix.go
@@ -0,0 +1,68 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package lint
+
+import (
+	"strings"
+
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var wktTimestampSuffixLinter = NewLinter(
+	"WKT_TIMESTAMP_SUFFIX",
+	"Verifies that all google.protobuf.Timestamp field names end in _time.",
+	checkWKTTimestampSuffix,
+)
+
+func checkWKTTimestampSuffix(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
+	return runVisitor(wktTimestampSuffixVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type wktTimestampSuffixVisitor struct {
+	baseAddVisitor
+}
+
+func (v wktTimestampSuffixVisitor) VisitMessage(message *proto.Message) {
+	for _, element := range message.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v wktTimestampSuffixVisitor) VisitOneof(oneof *proto.Oneof) {
+	for _, element := range oneof.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v wktTimestampSuffixVisitor) VisitNormalField(field *proto.NormalField) {
+	v.visitField(field.Field)
+}
+
+func (v wktTimestampSuffixVisitor) VisitOneofField(field *proto.OneOfField) {
+	v.visitField(field.Field)
+}
+
+func (v wktTimestampSuffixVisitor) visitField(field *proto.Field) {
+	if field.Type == "google.protobuf.Timestamp" && !strings.HasSuffix(field.Name, "_time") {
+		v.AddFailuref(field.Position, `Field %q of type "google.protobuf.Timestamp" must end in "_time".`, field.Name)
+	}
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index e7cbe2e..ca3a786 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -87,10 +87,13 @@ var (
 		requestResponseTypesUniqueLinter,
 		requestResponseNamesMatchRPCLinter,
 		servicesHaveCommentsLinter,
+		serviceNamesAPISuffixLinter,
 		serviceNamesCamelCaseLinter,
 		serviceNamesCapitalizedLinter,
 		syntaxProto3Linter,
 		wktDirectlyImportedLinter,
+		wktDurationSuffixLinter,
+		wktTimestampSuffixLinter,
 	}
 
 	// DefaultLinters is the slice of default Linters.
@@ -189,10 +192,13 @@ var (
 		requestResponseNamesMatchRPCLinter,
 		requestResponseTypesInSameFileLinter,
 		requestResponseTypesUniqueLinter,
+		serviceNamesAPISuffixLinter,
 		serviceNamesCamelCaseLinter,
 		serviceNamesCapitalizedLinter,
 		syntaxProto3Linter,
 		wktDirectlyImportedLinter,
+		wktDurationSuffixLinter,
+		wktTimestampSuffixLinter,
 	}
 
 	// GroupToLinters is the map from linter group to the corresponding slice of linters.


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index 57b964d..e2678fc 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -399,6 +399,14 @@ func TestLint(t *testing.T) {
 		"testdata/lint/noreserved/foo.proto",
 	)
 
+	assertDoLintFile(
+		t,
+		false,
+		`10:1:SERVICE_NAMES_API_SUFFIX
+		12:1:SERVICE_NAMES_API_SUFFIX`,
+		"testdata/lint/apisuffix/foo.proto",
+	)
+
 	assertDoLintFile(
 		t,
 		false,
@@ -448,6 +456,16 @@ func TestLint(t *testing.T) {
 		"testdata/lint/nokeywords/foo/public/public_suppressed.proto",
 	)
 
+	assertDoLintFile(
+		t,
+		false,
+		`14:3:WKT_TIMESTAMP_SUFFIX
+		15:3:WKT_DURATION_SUFFIX
+		17:5:WKT_TIMESTAMP_SUFFIX
+		18:5:WKT_DURATION_SUFFIX`,
+		"testdata/lint/wktsuffix/foo/foo.proto",
+	)
+
 	assertDoLintFile(
 		t,
 		false,
diff --git a/internal/cmd/testdata/lint/apisuffix/foo.proto b/internal/cmd/testdata/lint/apisuffix/foo.proto
new file mode 100644
index 0000000..94e7f81
--- /dev/null
+++ b/internal/cmd/testdata/lint/apisuffix/foo.proto
@@ -0,0 +1,12 @@
+syntax = "proto3";
+
+package foo;
+
+option go_package = "foopb";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo";
+
+service HelloService {}
+
+service Hello {}
diff --git a/internal/cmd/testdata/lint/apisuffix/prototool.yaml b/internal/cmd/testdata/lint/apisuffix/prototool.yaml
new file mode 100644
index 0000000..677f7b2
--- /dev/null
+++ b/internal/cmd/testdata/lint/apisuffix/prototool.yaml
@@ -0,0 +1,4 @@
+lint:
+  rules:
+    add:
+      - SERVICE_NAMES_API_SUFFIX
diff --git a/internal/cmd/testdata/lint/wktsuffix/foo/foo.proto b/internal/cmd/testdata/lint/wktsuffix/foo/foo.proto
new file mode 100644
index 0000000..77886b6
--- /dev/null
+++ b/internal/cmd/testdata/lint/wktsuffix/foo/foo.proto
@@ -0,0 +1,20 @@
+syntax = "proto3";
+
+package foo;
+
+option go_package = "foopb";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo";
+
+import "google/protobuf/duration.proto";
+import "google/protobuf/timestamp.proto";
+
+message Bar {
+  google.protobuf.Timestamp one = 1;
+  google.protobuf.Duration two = 2;
+  oneof test {
+    google.protobuf.Timestamp three = 3;
+    google.protobuf.Duration four = 4;
+  }
+}
diff --git a/internal/cmd/testdata/lint/wktsuffix/prototool.yaml b/internal/cmd/testdata/lint/wktsuffix/prototool.yaml
new file mode 100644
index 0000000..15247af
--- /dev/null
+++ b/internal/cmd/testdata/lint/wktsuffix/prototool.yaml
@@ -0,0 +1,5 @@
+lint:
+  rules:
+    add:
+      - WKT_DURATION_SUFFIX
+      - WKT_TIMESTAMP_SUFFIX