Repository: uber/prototool (go)

Issue:
Verify if a message field has comments
We have Lint feature with MESSAGES_HAVE_COMMENTS which will verify if a message has a comment or not. In the same way can we have a feature to verify if a message field has a comment.

Thanks.

Pull request description:
Add linters for enum field and message field comments
This adds four non-default linters:

- `ENUM_FIELDS_HAVE_COMMENTS`
- `ENUM_FIELDS_HAVE_SENTENCE_COMMENTS`
- `MESSAGE_FIELDS_HAVE_COMMENTS`
- `MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS`

Fixes #413.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 39ddf7b..102a41f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,7 +5,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
-- No changes yet.
+- Add linters for enum field and message field comments. These linters are not
+  part of any lint group but can be manually added in a configuration file.
 
 
 ## [1.4.0] - 2019-03-19
diff --git a/internal/lint/BUILD.bazel b/internal/lint/BUILD.bazel
index 4e2da2e..a3e3664 100644
--- a/internal/lint/BUILD.bazel
+++ b/internal/lint/BUILD.bazel
@@ -11,6 +11,8 @@ go_library(
         "check_enum_field_names_uppercase.go",
         "check_enum_field_prefixes.go",
         "check_enum_field_prefixes_except_message.go",
+        "check_enum_fields_have_comments.go",
+        "check_enum_fields_have_sentence_comments.go",
         "check_enum_names_camel_case.go",
         "check_enum_names_capitalized.go",
         "check_enum_zero_values_invalid.go",
@@ -35,6 +37,8 @@ go_library(
         "check_message_field_names_lowercase.go",
         "check_message_field_names_no_descriptor.go",
         "check_message_fields_duration.go",
+        "check_message_fields_have_comments.go",
+        "check_message_fields_have_sentence_comments.go",
         "check_message_fields_no_json_name.go",
         "check_message_fields_not_floats.go",
         "check_message_fields_time.go",
diff --git a/internal/lint/check_enum_fields_have_comments.go b/internal/lint/check_enum_fields_have_comments.go
new file mode 100644
index 0000000..9b99828
--- /dev/null
+++ b/internal/lint/check_enum_fields_have_comments.go
@@ -0,0 +1,59 @@
+// 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 (
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var enumFieldsHaveCommentsLinter = NewLinter(
+	"ENUM_FIELDS_HAVE_COMMENTS",
+	`Verifies that all enum fields have a comment of the form "// FIELD_NAME ...".`,
+	checkEnumFieldsHaveComments,
+)
+
+func checkEnumFieldsHaveComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(enumFieldsHaveCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type enumFieldsHaveCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v enumFieldsHaveCommentsVisitor) VisitMessage(message *proto.Message) {
+	// for nested enums
+	for _, child := range message.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v enumFieldsHaveCommentsVisitor) VisitEnum(enum *proto.Enum) {
+	for _, child := range enum.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v enumFieldsHaveCommentsVisitor) VisitEnumField(enumField *proto.EnumField) {
+	if !hasGolangStyleComment(enumField.Comment, enumField.Name) {
+		v.AddFailuref(enumField.Position, `Enum field %q needs a comment of the form "// %s ..."`, enumField.Name, enumField.Name)
+	}
+}
diff --git a/internal/lint/check_enum_fields_have_sentence_comments.go b/internal/lint/check_enum_fields_have_sentence_comments.go
new file mode 100644
index 0000000..a9d2dac
--- /dev/null
+++ b/internal/lint/check_enum_fields_have_sentence_comments.go
@@ -0,0 +1,59 @@
+// 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 (
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var enumFieldsHaveSentenceCommentsLinter = NewLinter(
+	"ENUM_FIELDS_HAVE_SENTENCE_COMMENTS",
+	`Verifies that all enum fields have a comment that contains at least one complete sentence.`,
+	checkEnumFieldsHaveSentenceComments,
+)
+
+func checkEnumFieldsHaveSentenceComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(enumFieldsHaveSentenceCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type enumFieldsHaveSentenceCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v enumFieldsHaveSentenceCommentsVisitor) VisitMessage(message *proto.Message) {
+	// for nested enums
+	for _, child := range message.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v enumFieldsHaveSentenceCommentsVisitor) VisitEnum(enum *proto.Enum) {
+	for _, child := range enum.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v enumFieldsHaveSentenceCommentsVisitor) VisitEnumField(enumField *proto.EnumField) {
+	if !hasCompleteSentenceComment(enumField.Comment) {
+		v.AddFailuref(enumField.Position, `Enum field %q needs a comment with a complete sentence that starts on the first line of the comment.`, enumField.Name)
+	}
+}
diff --git a/internal/lint/check_message_fields_have_comments.go b/internal/lint/check_message_fields_have_comments.go
new file mode 100644
index 0000000..531409f
--- /dev/null
+++ b/internal/lint/check_message_fields_have_comments.go
@@ -0,0 +1,70 @@
+// 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 (
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var messageFieldsHaveCommentsLinter = NewLinter(
+	"MESSAGE_FIELDS_HAVE_COMMENTS",
+	`Verifies that all message fields have a comment of the form "// field_name ...".`,
+	checkMessageFieldsHaveComments,
+)
+
+func checkMessageFieldsHaveComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(messageFieldsHaveCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type messageFieldsHaveCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v messageFieldsHaveCommentsVisitor) VisitMessage(message *proto.Message) {
+	for _, element := range message.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldsHaveCommentsVisitor) VisitOneof(oneof *proto.Oneof) {
+	for _, element := range oneof.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldsHaveCommentsVisitor) VisitNormalField(field *proto.NormalField) {
+	v.visitField(field.Field)
+}
+
+func (v messageFieldsHaveCommentsVisitor) VisitOneofField(field *proto.OneOfField) {
+	v.visitField(field.Field)
+}
+
+func (v messageFieldsHaveCommentsVisitor) VisitMapField(field *proto.MapField) {
+	v.visitField(field.Field)
+}
+
+func (v messageFieldsHaveCommentsVisitor) visitField(field *proto.Field) {
+	if !hasGolangStyleComment(field.Comment, field.Name) {
+		v.AddFailuref(field.Position, `Message field %q needs a comment of the form "// %s ..."`, field.Name, field.Name)
+	}
+}
diff --git a/internal/lint/check_message_fields_have_sentence_comments.go b/internal/lint/check_message_fields_have_sentence_comments.go
new file mode 100644
index 0000000..7f77982
--- /dev/null
+++ b/internal/lint/check_message_fields_have_sentence_comments.go
@@ -0,0 +1,70 @@
+// 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 (
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var messageFieldsHaveSentenceCommentsLinter = NewLinter(
+	"MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS",
+	`Verifies that all message fields have a comment that contains at least one complete sentence.`,
+	checkMessageFieldsHaveSentenceComments,
+)
+
+func checkMessageFieldsHaveSentenceComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(messageFieldsHaveSentenceCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type messageFieldsHaveSentenceCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v messageFieldsHaveSentenceCommentsVisitor) VisitMessage(message *proto.Message) {
+	for _, element := range message.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldsHaveSentenceCommentsVisitor) VisitOneof(oneof *proto.Oneof) {
+	for _, element := range oneof.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldsHaveSentenceCommentsVisitor) VisitNormalField(field *proto.NormalField) {
+	v.visitField(field.Field)
+}
+
+func (v messageFieldsHaveSentenceCommentsVisitor) VisitOneofField(field *proto.OneOfField) {
+	v.visitField(field.Field)
+}
+
+func (v messageFieldsHaveSentenceCommentsVisitor) VisitMapField(field *proto.MapField) {
+	v.visitField(field.Field)
+}
+
+func (v messageFieldsHaveSentenceCommentsVisitor) visitField(field *proto.Field) {
+	if !hasCompleteSentenceComment(field.Comment) {
+		v.AddFailuref(field.Position, `Message field %q needs a comment with a complete sentence that starts on the first line of the comment.`, field.Name)
+	}
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index 19fb83f..9c241c9 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -44,6 +44,8 @@ var (
 		enumFieldNamesUpperSnakeCaseLinter,
 		enumFieldPrefixesLinter,
 		enumFieldPrefixesExceptMessageLinter,
+		enumFieldsHaveCommentsLinter,
+		enumFieldsHaveSentenceCommentsLinter,
 		enumNamesCamelCaseLinter,
 		enumNamesCapitalizedLinter,
 		enumZeroValuesInvalidLinter,
@@ -83,6 +85,8 @@ var (
 		importsNotPublicLinter,
 		importsNotWeakLinter,
 		messageFieldsDurationLinter,
+		messageFieldsHaveCommentsLinter,
+		messageFieldsHaveSentenceCommentsLinter,
 		messageFieldsNotFloatsLinter,
 		messageFieldsNoJSONNameLinter,
 		messageFieldsTimeLinter,


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index c38a067..e53359f 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -721,6 +721,36 @@ func TestLint(t *testing.T) {
 		51:7:ENUM_ZERO_VALUES_INVALID_EXCEPT_MESSAGE`,
 		"testdata/lint/enumexceptmessages/foo.proto",
 	)
+
+	assertDoLintFile(
+		t,
+		false,
+		`15:3:ENUM_FIELDS_HAVE_COMMENTS
+		24:7:ENUM_FIELDS_HAVE_COMMENTS
+		26:5:MESSAGE_FIELDS_HAVE_COMMENTS
+		27:5:MESSAGE_FIELDS_HAVE_COMMENTS
+		29:7:MESSAGE_FIELDS_HAVE_COMMENTS
+		34:5:ENUM_FIELDS_HAVE_COMMENTS
+		36:3:MESSAGE_FIELDS_HAVE_COMMENTS
+		37:3:MESSAGE_FIELDS_HAVE_COMMENTS
+		39:5:MESSAGE_FIELDS_HAVE_COMMENTS`,
+		"testdata/lint/fieldscomments/foo/v1/foo.proto",
+	)
+
+	assertDoLintFile(
+		t,
+		false,
+		`15:3:ENUM_FIELDS_HAVE_SENTENCE_COMMENTS
+		24:7:ENUM_FIELDS_HAVE_SENTENCE_COMMENTS
+		26:5:MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS
+		27:5:MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS
+		29:7:MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS
+		34:5:ENUM_FIELDS_HAVE_SENTENCE_COMMENTS
+		36:3:MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS
+		37:3:MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS
+		39:5:MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS`,
+		"testdata/lint/fieldssentencecomments/foo/v1/foo.proto",
+	)
 }
 
 func TestLintConfigDataOverride(t *testing.T) {
diff --git a/internal/cmd/testdata/lint/fieldscomments/foo/v1/foo.proto b/internal/cmd/testdata/lint/fieldscomments/foo/v1/foo.proto
new file mode 100644
index 0000000..c3a5835
--- /dev/null
+++ b/internal/cmd/testdata/lint/fieldscomments/foo/v1/foo.proto
@@ -0,0 +1,41 @@
+syntax = "proto3";
+
+package foo.v1;
+
+option csharp_namespace = "Foo.V1";
+option go_package = "foov1";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo.v1";
+option objc_class_prefix = "FXX";
+option php_namespace = "Foo\\V1";
+
+// A one.
+enum One {
+  ONE_INVALID = 0;
+}
+
+// A two.
+message Two {
+  // A three.
+  message Three {
+    // A four.
+    enum Four {
+      FOUR_INVALID = 0;
+    }
+    int64 six = 1;
+    map<int64, string> seven = 2;
+    oneof eight {
+      int64 nine = 3;
+    }
+  }
+  // A five.
+  enum Five {
+    FIVE_INVALID = 0;
+  }
+  int64 ten = 1;
+  map<int64, string> eleven = 2;
+  oneof twelve {
+    int64 fourteen = 3;
+  }
+}
diff --git a/internal/cmd/testdata/lint/fieldscomments/prototool.yaml b/internal/cmd/testdata/lint/fieldscomments/prototool.yaml
new file mode 100644
index 0000000..00e538f
--- /dev/null
+++ b/internal/cmd/testdata/lint/fieldscomments/prototool.yaml
@@ -0,0 +1,6 @@
+lint:
+  group: uber2
+  rules:
+    add:
+      - ENUM_FIELDS_HAVE_COMMENTS
+      - MESSAGE_FIELDS_HAVE_COMMENTS
diff --git a/internal/cmd/testdata/lint/fieldssentencecomments/foo/v1/foo.proto b/internal/cmd/testdata/lint/fieldssentencecomments/foo/v1/foo.proto
new file mode 100644
index 0000000..c3a5835
--- /dev/null
+++ b/internal/cmd/testdata/lint/fieldssentencecomments/foo/v1/foo.proto
@@ -0,0 +1,41 @@
+syntax = "proto3";
+
+package foo.v1;
+
+option csharp_namespace = "Foo.V1";
+option go_package = "foov1";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo.v1";
+option objc_class_prefix = "FXX";
+option php_namespace = "Foo\\V1";
+
+// A one.
+enum One {
+  ONE_INVALID = 0;
+}
+
+// A two.
+message Two {
+  // A three.
+  message Three {
+    // A four.
+    enum Four {
+      FOUR_INVALID = 0;
+    }
+    int64 six = 1;
+    map<int64, string> seven = 2;
+    oneof eight {
+      int64 nine = 3;
+    }
+  }
+  // A five.
+  enum Five {
+    FIVE_INVALID = 0;
+  }
+  int64 ten = 1;
+  map<int64, string> eleven = 2;
+  oneof twelve {
+    int64 fourteen = 3;
+  }
+}
diff --git a/internal/cmd/testdata/lint/fieldssentencecomments/prototool.yaml b/internal/cmd/testdata/lint/fieldssentencecomments/prototool.yaml
new file mode 100644
index 0000000..9ad7a5d
--- /dev/null
+++ b/internal/cmd/testdata/lint/fieldssentencecomments/prototool.yaml
@@ -0,0 +1,6 @@
+lint:
+  group: uber2
+  rules:
+    add:
+      - ENUM_FIELDS_HAVE_SENTENCE_COMMENTS
+      - MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS