Repository: uber/prototool
Pull request #298 (2019-01-13 22:51:50)

Add linters to verify that enums, messages, services, and service methods have complete sentence comments
These linters do not require Golang style `// TypeName ...`, but do require that the comments start with an uppercase letter and contain a period.

peter-edge: Re: #265.
codecov-io: # [Codecov](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=h1) Report
> Merging [#298](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=desc) into [dev](https://codecov.io/gh/uber/prototool/commit/94181cc993a1afce6cbae06eb45c3a81f46027d6?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `74.68%`.

[![Impacted file tree graph](https://codecov.io/gh/uber/prototool/pull/298/graphs/tree.svg?width=650&token=A9V0DApqZg&height=150&src=pr)](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=tree)

```diff
@@            Coverage Diff            @@
##              dev    #298      +/-   ##
=========================================
+ Coverage   72.88%   72.9%   +0.01%     
=========================================
  Files         131     135       +4     
  Lines        5908    5982      +74     
=========================================
+ Hits         4306    4361      +55     
- Misses       1198    1207       +9     
- Partials      404     414      +10
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/lint/check\_messages\_have\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19tZXNzYWdlc19oYXZlX2NvbW1lbnRzLmdv) | `77.77% <100%> (ø)` | :arrow_up: |
| [...ges\_have\_comments\_except\_request\_response\_types.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19tZXNzYWdlc19oYXZlX2NvbW1lbnRzX2V4Y2VwdF9yZXF1ZXN0X3Jlc3BvbnNlX3R5cGVzLmdv) | `94.28% <100%> (ø)` | :arrow_up: |
| [internal/lint/check\_services\_have\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19zZXJ2aWNlc19oYXZlX2NvbW1lbnRzLmdv) | `100% <100%> (ø)` | :arrow_up: |
| [internal/lint/check\_rpcs\_have\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19ycGNzX2hhdmVfY29tbWVudHMuZ28=) | `100% <100%> (ø)` | :arrow_up: |
| [internal/lint/check\_enums\_have\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19lbnVtc19oYXZlX2NvbW1lbnRzLmdv) | `100% <100%> (ø)` | :arrow_up: |
| [...rnal/lint/check\_services\_have\_sentence\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19zZXJ2aWNlc19oYXZlX3NlbnRlbmNlX2NvbW1lbnRzLmdv) | `60% <60%> (ø)` | |
| [internal/lint/lint.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9saW50Lmdv) | `72.14% <61.11%> (-0.81%)` | :arrow_down: |
| [internal/lint/check\_rpcs\_have\_sentence\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19ycGNzX2hhdmVfc2VudGVuY2VfY29tbWVudHMuZ28=) | `75% <75%> (ø)` | |
| [...nternal/lint/check\_enums\_have\_sentence\_comments.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19lbnVtc19oYXZlX3NlbnRlbmNlX2NvbW1lbnRzLmdv) | `75% <75%> (ø)` | |
| [...sentence\_comments\_except\_request\_response\_types.go](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19tZXNzYWdlc19oYXZlX3NlbnRlbmNlX2NvbW1lbnRzX2V4Y2VwdF9yZXF1ZXN0X3Jlc3BvbnNlX3R5cGVzLmdv) | `80% <80%> (ø)` | |
| ... and [5 more](https://codecov.io/gh/uber/prototool/pull/298/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=footer). Last update [94181cc...f42408c](https://codecov.io/gh/uber/prototool/pull/298?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/example/gen/go/uber/bar/v1/bar.pb.go b/example/gen/go/uber/bar/v1/bar.pb.go
index b029f9e..dfb5963 100644
--- a/example/gen/go/uber/bar/v1/bar.pb.go
+++ b/example/gen/go/uber/bar/v1/bar.pb.go
@@ -18,6 +18,7 @@ var _ = math.Inf
 // proto package needs to be updated.
 const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package
 
+// Hello is a hello.
 type Hello struct {
 	Hello                int64    `protobuf:"varint,1,opt,name=hello,proto3" json:"hello,omitempty"`
 	XXX_NoUnkeyedLiteral struct{} `json:"-"`
@@ -29,7 +30,7 @@ func (m *Hello) Reset()         { *m = Hello{} }
 func (m *Hello) String() string { return proto.CompactTextString(m) }
 func (*Hello) ProtoMessage()    {}
 func (*Hello) Descriptor() ([]byte, []int) {
-	return fileDescriptor_bar_ab45b574cbbc9a08, []int{0}
+	return fileDescriptor_bar_7da55e4d7db32d57, []int{0}
 }
 func (m *Hello) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_Hello.Unmarshal(m, b)
@@ -60,9 +61,9 @@ func init() {
 	proto.RegisterType((*Hello)(nil), "uber.bar.v1.Hello")
 }
 
-func init() { proto.RegisterFile("uber/bar/v1/bar.proto", fileDescriptor_bar_ab45b574cbbc9a08) }
+func init() { proto.RegisterFile("uber/bar/v1/bar.proto", fileDescriptor_bar_7da55e4d7db32d57) }
 
-var fileDescriptor_bar_ab45b574cbbc9a08 = []byte{
+var fileDescriptor_bar_7da55e4d7db32d57 = []byte{
 	// 138 bytes of a gzipped FileDescriptorProto
 	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x12, 0x2d, 0x4d, 0x4a, 0x2d,
 	0xd2, 0x4f, 0x4a, 0x2c, 0xd2, 0x2f, 0x33, 0x04, 0x51, 0x7a, 0x05, 0x45, 0xf9, 0x25, 0xf9, 0x42,
diff --git a/example/gen/go/uber/foo/v1/hello_api.pb.go b/example/gen/go/uber/foo/v1/hello_api.pb.go
index fe737ae..f6fa586 100644
--- a/example/gen/go/uber/foo/v1/hello_api.pb.go
+++ b/example/gen/go/uber/foo/v1/hello_api.pb.go
@@ -33,7 +33,7 @@ func (m *FooRequest) Reset()         { *m = FooRequest{} }
 func (m *FooRequest) String() string { return proto.CompactTextString(m) }
 func (*FooRequest) ProtoMessage()    {}
 func (*FooRequest) Descriptor() ([]byte, []int) {
-	return fileDescriptor_hello_api_d217dc40b042ecd5, []int{0}
+	return fileDescriptor_hello_api_8ba890b48aa56f70, []int{0}
 }
 func (m *FooRequest) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_FooRequest.Unmarshal(m, b)
@@ -63,7 +63,7 @@ func (m *FooResponse) Reset()         { *m = FooResponse{} }
 func (m *FooResponse) String() string { return proto.CompactTextString(m) }
 func (*FooResponse) ProtoMessage()    {}
 func (*FooResponse) Descriptor() ([]byte, []int) {
-	return fileDescriptor_hello_api_d217dc40b042ecd5, []int{1}
+	return fileDescriptor_hello_api_8ba890b48aa56f70, []int{1}
 }
 func (m *FooResponse) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_FooResponse.Unmarshal(m, b)
@@ -94,7 +94,7 @@ func (m *BarRequest) Reset()         { *m = BarRequest{} }
 func (m *BarRequest) String() string { return proto.CompactTextString(m) }
 func (*BarRequest) ProtoMessage()    {}
 func (*BarRequest) Descriptor() ([]byte, []int) {
-	return fileDescriptor_hello_api_d217dc40b042ecd5, []int{2}
+	return fileDescriptor_hello_api_8ba890b48aa56f70, []int{2}
 }
 func (m *BarRequest) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_BarRequest.Unmarshal(m, b)
@@ -131,7 +131,7 @@ func (m *BarResponse) Reset()         { *m = BarResponse{} }
 func (m *BarResponse) String() string { return proto.CompactTextString(m) }
 func (*BarResponse) ProtoMessage()    {}
 func (*BarResponse) Descriptor() ([]byte, []int) {
-	return fileDescriptor_hello_api_d217dc40b042ecd5, []int{3}
+	return fileDescriptor_hello_api_8ba890b48aa56f70, []int{3}
 }
 func (m *BarResponse) XXX_Unmarshal(b []byte) error {
 	return xxx_messageInfo_BarResponse.Unmarshal(m, b)
@@ -268,10 +268,10 @@ var _HelloAPI_serviceDesc = grpc.ServiceDesc{
 }
 
 func init() {
-	proto.RegisterFile("uber/foo/v1/hello_api.proto", fileDescriptor_hello_api_d217dc40b042ecd5)
+	proto.RegisterFile("uber/foo/v1/hello_api.proto", fileDescriptor_hello_api_8ba890b48aa56f70)
 }
 
-var fileDescriptor_hello_api_d217dc40b042ecd5 = []byte{
+var fileDescriptor_hello_api_8ba890b48aa56f70 = []byte{
 	// 227 bytes of a gzipped FileDescriptorProto
 	0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x92, 0x2e, 0x4d, 0x4a, 0x2d,
 	0xd2, 0x4f, 0xcb, 0xcf, 0xd7, 0x2f, 0x33, 0xd4, 0xcf, 0x48, 0xcd, 0xc9, 0xc9, 0x8f, 0x4f, 0x2c,
diff --git a/example/proto/uber/bar/v1/bar.proto b/example/proto/uber/bar/v1/bar.proto
index 256f544..5ca8d13 100644
--- a/example/proto/uber/bar/v1/bar.proto
+++ b/example/proto/uber/bar/v1/bar.proto
@@ -30,6 +30,7 @@ option java_package = "com.uber.bar.v1";
 option objc_class_prefix = "UBX";
 option php_namespace = "Uber\\Bar\\V1";
 
+// Hello is a hello.
 message Hello {
   int64 hello = 1;
 }
diff --git a/example/proto/uber/foo/v1/hello_api.proto b/example/proto/uber/foo/v1/hello_api.proto
index 70c4e7c..cf3dd75 100644
--- a/example/proto/uber/foo/v1/hello_api.proto
+++ b/example/proto/uber/foo/v1/hello_api.proto
@@ -30,6 +30,7 @@ option java_package = "com.uber.foo.v1";
 option objc_class_prefix = "UFX";
 option php_namespace = "Uber\\Foo\\V1";
 
+// HelloAPI controls Foos and Bars.
 service HelloAPI {
   // Foo does a foo.
   rpc Foo(FooRequest) returns (FooResponse);
diff --git a/internal/lint/check_enums_have_comments.go b/internal/lint/check_enums_have_comments.go
index acb82a5..0ebb8e5 100644
--- a/internal/lint/check_enums_have_comments.go
+++ b/internal/lint/check_enums_have_comments.go
@@ -21,9 +21,6 @@
 package lint
 
 import (
-	"fmt"
-	"strings"
-
 	"github.com/emicklei/proto"
 	"github.com/uber/prototool/internal/text"
 )
@@ -50,7 +47,7 @@ func (v enumsHaveCommentsVisitor) VisitMessage(message *proto.Message) {
 }
 
 func (v enumsHaveCommentsVisitor) VisitEnum(enum *proto.Enum) {
-	if enum.Comment == nil || len(enum.Comment.Lines) == 0 || !strings.HasPrefix(enum.Comment.Lines[0], fmt.Sprintf(" %s ", enum.Name)) {
+	if !hasGolangStyleComment(enum.Comment, enum.Name) {
 		v.AddFailuref(enum.Position, `Enum %q needs a comment of the form "// %s ..."`, enum.Name, enum.Name)
 	}
 }
diff --git a/internal/lint/check_enums_have_sentence_comments.go b/internal/lint/check_enums_have_sentence_comments.go
new file mode 100644
index 0000000..eac963a
--- /dev/null
+++ b/internal/lint/check_enums_have_sentence_comments.go
@@ -0,0 +1,53 @@
+// 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 enumsHaveSentenceCommentsLinter = NewLinter(
+	"ENUMS_HAVE_SENTENCE_COMMENTS",
+	`Verifies that all enums have a comment that contains at least one complete sentence.`,
+	checkEnumsHaveSentenceComments,
+)
+
+func checkEnumsHaveSentenceComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(enumsHaveSentenceCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type enumsHaveSentenceCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v enumsHaveSentenceCommentsVisitor) VisitMessage(message *proto.Message) {
+	// for nested enums
+	for _, child := range message.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v enumsHaveSentenceCommentsVisitor) VisitEnum(enum *proto.Enum) {
+	if !hasCompleteSentenceComment(enum.Comment) {
+		v.AddFailuref(enum.Position, `Enum %q needs a comment with a complete sentence that starts on the first line of the comment.`, enum.Name)
+	}
+}
diff --git a/internal/lint/check_messages_have_comments.go b/internal/lint/check_messages_have_comments.go
index 3b348f2..9820846 100644
--- a/internal/lint/check_messages_have_comments.go
+++ b/internal/lint/check_messages_have_comments.go
@@ -21,9 +21,6 @@
 package lint
 
 import (
-	"fmt"
-	"strings"
-
 	"github.com/emicklei/proto"
 	"github.com/uber/prototool/internal/text"
 )
@@ -50,7 +47,7 @@ func (v messagesHaveCommentsVisitor) VisitMessage(message *proto.Message) {
 	if message.IsExtend {
 		return
 	}
-	if message.Comment == nil || len(message.Comment.Lines) == 0 || !strings.HasPrefix(message.Comment.Lines[0], fmt.Sprintf(" %s ", message.Name)) {
+	if !hasGolangStyleComment(message.Comment, message.Name) {
 		v.AddFailuref(message.Position, `Message %q needs a comment of the form "// %s ..."`, message.Name, message.Name)
 	}
 }
diff --git a/internal/lint/check_messages_have_comments_except_request_response_types.go b/internal/lint/check_messages_have_comments_except_request_response_types.go
index d679c05..d0d2fb0 100644
--- a/internal/lint/check_messages_have_comments_except_request_response_types.go
+++ b/internal/lint/check_messages_have_comments_except_request_response_types.go
@@ -21,7 +21,6 @@
 package lint
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/emicklei/proto"
@@ -90,7 +89,7 @@ func (v *messagesHaveCommentsExceptRequestResponseTypesVisitor) Finally() error
 	for messageName, message := range v.messageNameToMessage {
 		if !message.IsExtend {
 			if _, ok := v.requestResponseTypes[messageName]; !ok {
-				if message.Comment == nil || len(message.Comment.Lines) == 0 || !strings.HasPrefix(message.Comment.Lines[0], fmt.Sprintf(" %s ", message.Name)) {
+				if !hasGolangStyleComment(message.Comment, message.Name) {
 					v.AddFailuref(message.Position, `Message %q needs a comment of the form "// %s ..."`, message.Name, message.Name)
 				}
 			}
diff --git a/internal/lint/check_messages_have_sentence_comments_except_request_response_types.go b/internal/lint/check_messages_have_sentence_comments_except_request_response_types.go
new file mode 100644
index 0000000..eebb547
--- /dev/null
+++ b/internal/lint/check_messages_have_sentence_comments_except_request_response_types.go
@@ -0,0 +1,99 @@
+// 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 messagesHaveSentenceCommentsExceptRequestResponseTypesLinter = NewLinter(
+	"MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES",
+	`Verifies that all non-extended messages except for request and response types have a comment that contains at least one complete sentence.`,
+	checkMessagesHaveSentenceCommentsExceptRequestResponseTypes,
+)
+
+func checkMessagesHaveSentenceCommentsExceptRequestResponseTypes(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(&messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor struct {
+	baseAddVisitor
+	messageNameToMessage map[string]*proto.Message
+	requestResponseTypes map[string]struct{}
+	nestedMessageNames   []string
+}
+
+func (v *messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor) OnStart(*FileDescriptor) error {
+	v.messageNameToMessage = nil
+	v.requestResponseTypes = nil
+	v.nestedMessageNames = nil
+	return nil
+}
+
+func (v *messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor) VisitMessage(message *proto.Message) {
+	v.nestedMessageNames = append(v.nestedMessageNames, message.Name)
+	for _, child := range message.Elements {
+		child.Accept(v)
+	}
+	v.nestedMessageNames = v.nestedMessageNames[:len(v.nestedMessageNames)-1]
+
+	if v.messageNameToMessage == nil {
+		v.messageNameToMessage = make(map[string]*proto.Message)
+	}
+	if len(v.nestedMessageNames) > 0 {
+		v.messageNameToMessage[strings.Join(v.nestedMessageNames, ".")+"."+message.Name] = message
+	} else {
+		v.messageNameToMessage[message.Name] = message
+	}
+}
+
+func (v *messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor) VisitService(service *proto.Service) {
+	for _, child := range service.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v *messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor) VisitRPC(rpc *proto.RPC) {
+	if v.requestResponseTypes == nil {
+		v.requestResponseTypes = make(map[string]struct{})
+	}
+	v.requestResponseTypes[rpc.RequestType] = struct{}{}
+	v.requestResponseTypes[rpc.ReturnsType] = struct{}{}
+}
+
+func (v *messagesHaveSentenceCommentsExceptRequestResponseTypesVisitor) Finally() error {
+	if v.messageNameToMessage == nil {
+		v.messageNameToMessage = make(map[string]*proto.Message)
+	}
+	for messageName, message := range v.messageNameToMessage {
+		if !message.IsExtend {
+			if _, ok := v.requestResponseTypes[messageName]; !ok {
+				if !hasCompleteSentenceComment(message.Comment) {
+					v.AddFailuref(message.Position, `Message %q needs a comment with a complete sentence that starts on the first line of the comment.`, message.Name)
+				}
+			}
+		}
+	}
+	return nil
+}
diff --git a/internal/lint/check_rpcs_have_comments.go b/internal/lint/check_rpcs_have_comments.go
index e2520b5..2828b4a 100644
--- a/internal/lint/check_rpcs_have_comments.go
+++ b/internal/lint/check_rpcs_have_comments.go
@@ -21,9 +21,6 @@
 package lint
 
 import (
-	"fmt"
-	"strings"
-
 	"github.com/emicklei/proto"
 	"github.com/uber/prototool/internal/text"
 )
@@ -49,7 +46,7 @@ func (v rpcsHaveCommentsVisitor) VisitService(service *proto.Service) {
 }
 
 func (v rpcsHaveCommentsVisitor) VisitRPC(rpc *proto.RPC) {
-	if rpc.Comment == nil || len(rpc.Comment.Lines) == 0 || !strings.HasPrefix(rpc.Comment.Lines[0], fmt.Sprintf(" %s ", rpc.Name)) {
+	if !hasGolangStyleComment(rpc.Comment, rpc.Name) {
 		v.AddFailuref(rpc.Position, `RPC %q needs a comment of the form "// %s ..."`, rpc.Name, rpc.Name)
 	}
 }
diff --git a/internal/lint/check_rpcs_have_sentence_comments.go b/internal/lint/check_rpcs_have_sentence_comments.go
new file mode 100644
index 0000000..95fe3a1
--- /dev/null
+++ b/internal/lint/check_rpcs_have_sentence_comments.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 (
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var rpcsHaveSentenceCommentsLinter = NewLinter(
+	"RPCS_HAVE_SENTENCE_COMMENTS",
+	`Verifies that all rpcs have a comment that contains at least one complete sentence.`,
+	checkRPCsHaveSentenceComments,
+)
+
+func checkRPCsHaveSentenceComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(rpcsHaveSentenceCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type rpcsHaveSentenceCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v rpcsHaveSentenceCommentsVisitor) VisitService(service *proto.Service) {
+	for _, child := range service.Elements {
+		child.Accept(v)
+	}
+}
+
+func (v rpcsHaveSentenceCommentsVisitor) VisitRPC(rpc *proto.RPC) {
+	if !hasCompleteSentenceComment(rpc.Comment) {
+		v.AddFailuref(rpc.Position, `RPC %q needs a comment with a complete sentence that starts on the first line of the comment.`, rpc.Name)
+	}
+}
diff --git a/internal/lint/check_services_have_comments.go b/internal/lint/check_services_have_comments.go
index 51c3a83..7846350 100644
--- a/internal/lint/check_services_have_comments.go
+++ b/internal/lint/check_services_have_comments.go
@@ -21,9 +21,6 @@
 package lint
 
 import (
-	"fmt"
-	"strings"
-
 	"github.com/emicklei/proto"
 	"github.com/uber/prototool/internal/text"
 )
@@ -43,7 +40,7 @@ type servicesHaveCommentsVisitor struct {
 }
 
 func (v servicesHaveCommentsVisitor) VisitService(service *proto.Service) {
-	if service.Comment == nil || len(service.Comment.Lines) == 0 || !strings.HasPrefix(service.Comment.Lines[0], fmt.Sprintf(" %s ", service.Name)) {
+	if !hasGolangStyleComment(service.Comment, service.Name) {
 		v.AddFailuref(service.Position, `Service %q needs a comment of the form "// %s ..."`, service.Name, service.Name)
 	}
 }
diff --git a/internal/lint/check_services_have_sentence_comments.go b/internal/lint/check_services_have_sentence_comments.go
new file mode 100644
index 0000000..a89bb96
--- /dev/null
+++ b/internal/lint/check_services_have_sentence_comments.go
@@ -0,0 +1,46 @@
+// 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 servicesHaveSentenceCommentsLinter = NewLinter(
+	"SERVICES_HAVE_SENTENCE_COMMENTS",
+	`Verifies that all services have a comment that contains at least one complete sentence.`,
+	checkServicesHaveSentenceComments,
+)
+
+func checkServicesHaveSentenceComments(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(servicesHaveSentenceCommentsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type servicesHaveSentenceCommentsVisitor struct {
+	baseAddVisitor
+}
+
+func (v servicesHaveSentenceCommentsVisitor) VisitService(service *proto.Service) {
+	if !hasCompleteSentenceComment(service.Comment) {
+		v.AddFailuref(service.Position, `Service %q needs a comment with a complete sentence that starts on the first line of the comment.`, service.Name)
+	}
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index af528cd..2b0cc52 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -26,6 +26,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"unicode"
 
 	"github.com/emicklei/proto"
 	"github.com/uber/prototool/internal/file"
@@ -48,6 +49,7 @@ var (
 		enumZeroValuesInvalidLinter,
 		enumZeroValuesInvalidExceptMessageLinter,
 		enumsHaveCommentsLinter,
+		enumsHaveSentenceCommentsLinter,
 		enumsNoAllowAliasLinter,
 		fieldsNotReservedLinter,
 		fileHeaderLinter,
@@ -86,6 +88,7 @@ var (
 		messageNamesCapitalizedLinter,
 		messagesHaveCommentsLinter,
 		messagesHaveCommentsExceptRequestResponseTypesLinter,
+		messagesHaveSentenceCommentsExceptRequestResponseTypesLinter,
 		oneofNamesLowerSnakeCaseLinter,
 		packageIsDeclaredLinter,
 		packageLowerSnakeCaseLinter,
@@ -93,6 +96,7 @@ var (
 		packageNoKeywordsLinter,
 		packagesSameInDirLinter,
 		rpcsHaveCommentsLinter,
+		rpcsHaveSentenceCommentsLinter,
 		rpcNamesCamelCaseLinter,
 		rpcNamesCapitalizedLinter,
 		rpcOptionsNoGoogleAPIHTTPLinter,
@@ -102,6 +106,7 @@ var (
 		requestResponseTypesUniqueLinter,
 		requestResponseNamesMatchRPCLinter,
 		servicesHaveCommentsLinter,
+		servicesHaveSentenceCommentsLinter,
 		serviceNamesAPISuffixLinter,
 		serviceNamesCamelCaseLinter,
 		serviceNamesCapitalizedLinter,
@@ -179,6 +184,7 @@ var (
 		enumNamesCamelCaseLinter,
 		enumNamesCapitalizedLinter,
 		enumZeroValuesInvalidExceptMessageLinter,
+		enumsHaveSentenceCommentsLinter,
 		enumsNoAllowAliasLinter,
 		fieldsNotReservedLinter,
 		fileHeaderLinter,
@@ -206,6 +212,7 @@ var (
 		gogoNotImportedLinter,
 		importsNotPublicLinter,
 		importsNotWeakLinter,
+		messagesHaveSentenceCommentsExceptRequestResponseTypesLinter,
 		messageFieldNamesLowerSnakeCaseLinter,
 		messageFieldsNoJSONNameLinter,
 		messageFieldsNotFloatsLinter,
@@ -217,6 +224,7 @@ var (
 		packageMajorBetaVersionedLinter,
 		packageNoKeywordsLinter,
 		packagesSameInDirLinter,
+		rpcsHaveSentenceCommentsLinter,
 		rpcNamesCamelCaseLinter,
 		rpcNamesCapitalizedLinter,
 		rpcOptionsNoGoogleAPIHTTPLinter,
@@ -225,6 +233,7 @@ var (
 		requestResponseTypesInSameFileLinter,
 		requestResponseTypesOnlyInFileLinter,
 		requestResponseTypesUniqueLinter,
+		servicesHaveSentenceCommentsLinter,
 		serviceNamesAPISuffixLinter,
 		serviceNamesCamelCaseLinter,
 		serviceNamesCapitalizedLinter,
@@ -477,6 +486,39 @@ func checkLintID(lintID string) error {
 	return nil
 }
 
+func hasGolangStyleComment(comment *proto.Comment, name string) bool {
+	return comment != nil && len(comment.Lines) > 0 && strings.HasPrefix(comment.Lines[0], fmt.Sprintf(" %s ", name))
+}
+
+func hasCompleteSentenceComment(comment *proto.Comment) bool {
+	return commentStartsWithUppercaseLetter(comment) && commentContainsPeriod(comment)
+}
+
+func commentStartsWithUppercaseLetter(comment *proto.Comment) bool {
+	if comment == nil || len(comment.Lines) == 0 {
+		return false
+	}
+	firstLine := strings.TrimSpace(comment.Lines[0])
+	if firstLine == "" {
+		return false
+	}
+	return unicode.IsUpper(rune(firstLine[0]))
+}
+
+func commentContainsPeriod(comment *proto.Comment) bool {
+	if comment == nil || len(comment.Lines) == 0 {
+		return false
+	}
+	// very primitive check, could make better with NLP but this is hard with comments
+	// since comments can contain code examples, links, etc.
+	for _, line := range comment.Lines {
+		if strings.Contains(line, ".") {
+			return true
+		}
+	}
+	return false
+}
+
 func isSuppressed(comment *proto.Comment, annotation string) bool {
 	if comment == nil {
 		return false


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index db261d2..a11d82e 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -425,7 +425,7 @@ func TestLint(t *testing.T) {
 	assertDoLintFile(
 		t,
 		false,
-		`17:1:REQUEST_RESPONSE_TYPES_AFTER_SERVICE`,
+		`19:1:REQUEST_RESPONSE_TYPES_AFTER_SERVICE`,
 		"testdata/lint/afterservice/foo/v1/hello3api.proto",
 	)
 
diff --git a/internal/cmd/testdata/lint/afterservice/foo/v1/hello2api.proto b/internal/cmd/testdata/lint/afterservice/foo/v1/hello2api.proto
index 62325ba..73a66fa 100644
--- a/internal/cmd/testdata/lint/afterservice/foo/v1/hello2api.proto
+++ b/internal/cmd/testdata/lint/afterservice/foo/v1/hello2api.proto
@@ -14,6 +14,8 @@ message SendFoo2Request {}
 
 message SendFoo2Response {}
 
+// Hello2API is the second HelloAPI.
 service Hello2API {
+  // SendFoo2 sends foos.
   rpc SendFoo2(SendFoo2Request) returns (SendFoo2Response);
 }
diff --git a/internal/cmd/testdata/lint/afterservice/foo/v1/hello3api.proto b/internal/cmd/testdata/lint/afterservice/foo/v1/hello3api.proto
index e0ebe24..ca57bad 100644
--- a/internal/cmd/testdata/lint/afterservice/foo/v1/hello3api.proto
+++ b/internal/cmd/testdata/lint/afterservice/foo/v1/hello3api.proto
@@ -10,7 +10,9 @@ option java_package = "com.foo.v1";
 option objc_class_prefix = "FXX";
 option php_namespace = "Foo\\V1";
 
+// Hello2API is the third HelloAPI.
 service Hello3API {
+  // SendFoo3 sends foos.
   rpc SendFoo3(SendFoo3Request) returns (SendFoo3Response);
 }
 
diff --git a/internal/cmd/testdata/lint/afterservice/foo/v1/hello_api.proto b/internal/cmd/testdata/lint/afterservice/foo/v1/hello_api.proto
index a7ff857..7844039 100644
--- a/internal/cmd/testdata/lint/afterservice/foo/v1/hello_api.proto
+++ b/internal/cmd/testdata/lint/afterservice/foo/v1/hello_api.proto
@@ -12,7 +12,9 @@ option php_namespace = "Foo\\V1";
 
 message SendFooRequest {}
 
+// HelloAPI is the first HelloAPI.
 service HelloAPI {
+  // SendFoo sends foos.
   rpc SendFoo(SendFooRequest) returns (SendFooResponse);
 }
 
diff --git a/internal/cmd/testdata/lint/floats/prototool.yaml b/internal/cmd/testdata/lint/floats/prototool.yaml
index 092dbf5..ee80b43 100644
--- a/internal/cmd/testdata/lint/floats/prototool.yaml
+++ b/internal/cmd/testdata/lint/floats/prototool.yaml
@@ -1,2 +1,8 @@
 lint:
   group: uber2
+  rules:
+    remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS
diff --git a/internal/cmd/testdata/lint/inlinecomments/prototool.yaml b/internal/cmd/testdata/lint/inlinecomments/prototool.yaml
index 092dbf5..ee80b43 100644
--- a/internal/cmd/testdata/lint/inlinecomments/prototool.yaml
+++ b/internal/cmd/testdata/lint/inlinecomments/prototool.yaml
@@ -1,2 +1,8 @@
 lint:
   group: uber2
+  rules:
+    remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS
diff --git a/internal/cmd/testdata/lint/nogoogleapihttp/prototool.yaml b/internal/cmd/testdata/lint/nogoogleapihttp/prototool.yaml
index f93e4d0..0f56249 100644
--- a/internal/cmd/testdata/lint/nogoogleapihttp/prototool.yaml
+++ b/internal/cmd/testdata/lint/nogoogleapihttp/prototool.yaml
@@ -4,3 +4,9 @@ lint:
     - id: PACKAGE_MAJOR_BETA_VERSIONED
       files:
         - google/api/api.proto
+  rules:
+    remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS
diff --git a/internal/cmd/testdata/lint/noplurals/prototool.yaml b/internal/cmd/testdata/lint/noplurals/prototool.yaml
index f055333..7abb392 100644
--- a/internal/cmd/testdata/lint/noplurals/prototool.yaml
+++ b/internal/cmd/testdata/lint/noplurals/prototool.yaml
@@ -2,4 +2,8 @@ lint:
   group: uber2
   rules:
     remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS
       - SERVICE_NAMES_MATCH_FILE_NAME
diff --git a/internal/cmd/testdata/lint/onlyinfile/prototool.yaml b/internal/cmd/testdata/lint/onlyinfile/prototool.yaml
index 092dbf5..ee80b43 100644
--- a/internal/cmd/testdata/lint/onlyinfile/prototool.yaml
+++ b/internal/cmd/testdata/lint/onlyinfile/prototool.yaml
@@ -1,2 +1,8 @@
 lint:
   group: uber2
+  rules:
+    remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS
diff --git a/internal/cmd/testdata/lint/servicenamefilename/prototool.yaml b/internal/cmd/testdata/lint/servicenamefilename/prototool.yaml
index 092dbf5..ee80b43 100644
--- a/internal/cmd/testdata/lint/servicenamefilename/prototool.yaml
+++ b/internal/cmd/testdata/lint/servicenamefilename/prototool.yaml
@@ -1,2 +1,8 @@
 lint:
   group: uber2
+  rules:
+    remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS