Skip to content

Commit

Permalink
Add linters for enum field and message field comments (uber#416)
Browse files Browse the repository at this point in the history
  • Loading branch information
bufdev authored Apr 1, 2019
1 parent 1c2c34f commit c79e86b
Show file tree
Hide file tree
Showing 12 changed files with 392 additions and 1 deletion.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions internal/cmd/cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
41 changes: 41 additions & 0 deletions internal/cmd/testdata/lint/fieldscomments/foo/v1/foo.proto
Original file line number Diff line number Diff line change
@@ -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;
}
}
6 changes: 6 additions & 0 deletions internal/cmd/testdata/lint/fieldscomments/prototool.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
lint:
group: uber2
rules:
add:
- ENUM_FIELDS_HAVE_COMMENTS
- MESSAGE_FIELDS_HAVE_COMMENTS
41 changes: 41 additions & 0 deletions internal/cmd/testdata/lint/fieldssentencecomments/foo/v1/foo.proto
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
lint:
group: uber2
rules:
add:
- ENUM_FIELDS_HAVE_SENTENCE_COMMENTS
- MESSAGE_FIELDS_HAVE_SENTENCE_COMMENTS
4 changes: 4 additions & 0 deletions internal/lint/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
59 changes: 59 additions & 0 deletions internal/lint/check_enum_fields_have_comments.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
59 changes: 59 additions & 0 deletions internal/lint/check_enum_fields_have_sentence_comments.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
70 changes: 70 additions & 0 deletions internal/lint/check_message_fields_have_comments.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
Loading

0 comments on commit c79e86b

Please sign in to comment.