Repository: uber/prototool
Pull request #317 (2019-01-30 15:41:42)

Add linter to check for file_name
```
$ prototool lint internal/cmd/testdata/lint/fieldnamesfilename
internal/cmd/testdata/lint/fieldnamesfilename/hello.proto:16:7:Field name "sfile_name" should be "sfilename".
internal/cmd/testdata/lint/fieldnamesfilename/hello.proto:18:9:Field name "file_name" should be "filename".
internal/cmd/testdata/lint/fieldnamesfilename/hello.proto:21:5:Field name "sfile_names" should be "sfilenames".
internal/cmd/testdata/lint/fieldnamesfilename/hello.proto:23:3:Field name "file_names" should be "filenames".
internal/cmd/testdata/lint/fieldnamesfilename/hello.proto:24:3:Field name "map_file_name" should be "map_filename".
```

Re: #301.

codecov-io: # [Codecov](https://codecov.io/gh/uber/prototool/pull/317?src=pr&el=h1) Report
> Merging [#317](https://codecov.io/gh/uber/prototool/pull/317?src=pr&el=desc) into [dev](https://codecov.io/gh/uber/prototool/commit/fdf400bb873cacbb10d95d663c93d9ef27476c57?src=pr&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##              dev     #317      +/-   ##
==========================================
+ Coverage   73.36%   73.44%   +0.07%     
==========================================
  Files         139      140       +1     
  Lines        6252     6269      +17     
==========================================
+ Hits         4587     4604      +17     
  Misses       1246     1246              
  Partials      419      419
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/317?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/lint/lint.go](https://codecov.io/gh/uber/prototool/pull/317/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9saW50Lmdv) | `71.01% <ø> (ø)` | :arrow_up: |
| [...nternal/lint/check\_message\_field\_names\_filename.go](https://codecov.io/gh/uber/prototool/pull/317/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19tZXNzYWdlX2ZpZWxkX25hbWVzX2ZpbGVuYW1lLmdv) | `100% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber/prototool/pull/317?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/317?src=pr&el=footer). Last update [fdf400b...3d6cc86](https://codecov.io/gh/uber/prototool/pull/317?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/internal/lint/BUILD.bazel b/internal/lint/BUILD.bazel
index 3297a50..481c995 100644
--- a/internal/lint/BUILD.bazel
+++ b/internal/lint/BUILD.bazel
@@ -29,6 +29,7 @@ go_library(
         "check_gogo_not_imported.go",
         "check_imports_not_public.go",
         "check_imports_not_weak.go",
+        "check_message_field_names_filename.go",
         "check_message_field_names_lower_snake_case.go",
         "check_message_field_names_lowercase.go",
         "check_message_fields_no_json_name.go",
diff --git a/internal/lint/check_message_field_names_filename.go b/internal/lint/check_message_field_names_filename.go
new file mode 100644
index 0000000..d961ddd
--- /dev/null
+++ b/internal/lint/check_message_field_names_filename.go
@@ -0,0 +1,73 @@
+// 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 messageFieldNamesFilenameLinter = NewLinter(
+	"MESSAGE_FIELD_NAMES_FILENAME",
+	`Verifies that all message field names do not contain "file_name" as "filename" should be used instead.`,
+	checkMessageFieldNamesFilename,
+)
+
+func checkMessageFieldNamesFilename(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(messageFieldNamesFilenameVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type messageFieldNamesFilenameVisitor struct {
+	baseAddVisitor
+}
+
+func (v messageFieldNamesFilenameVisitor) VisitMessage(message *proto.Message) {
+	for _, element := range message.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldNamesFilenameVisitor) VisitOneof(oneof *proto.Oneof) {
+	for _, element := range oneof.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldNamesFilenameVisitor) VisitNormalField(field *proto.NormalField) {
+	v.handleField(field.Field)
+}
+
+func (v messageFieldNamesFilenameVisitor) VisitOneofField(field *proto.OneOfField) {
+	v.handleField(field.Field)
+}
+
+func (v messageFieldNamesFilenameVisitor) VisitMapField(field *proto.MapField) {
+	v.handleField(field.Field)
+}
+
+func (v messageFieldNamesFilenameVisitor) handleField(field *proto.Field) {
+	if strings.Contains(field.Name, "file_name") {
+		// This might break down in practice but let's try it out
+		v.AddFailuref(field.Position, "Field name %q should be %q.", field.Name, strings.Replace(field.Name, "file_name", "filename", -1))
+	}
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index b742cf6..a562d25 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -83,6 +83,7 @@ var (
 		importsNotWeakLinter,
 		messageFieldsNotFloatsLinter,
 		messageFieldsNoJSONNameLinter,
+		messageFieldNamesFilenameLinter,
 		messageFieldNamesLowerSnakeCaseLinter,
 		messageFieldNamesLowercaseLinter,
 		messageNamesCamelCaseLinter,
@@ -220,6 +221,7 @@ var (
 		importsNotWeakLinter,
 		messagesHaveSentenceCommentsExceptRequestResponseTypesLinter,
 		messagesNotEmptyExceptRequestResponseTypesLinter,
+		messageFieldNamesFilenameLinter,
 		messageFieldNamesLowerSnakeCaseLinter,
 		messageFieldsNoJSONNameLinter,
 		messageFieldsNotFloatsLinter,


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index 8162994..2764329 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -492,6 +492,17 @@ func TestLint(t *testing.T) {
 		"testdata/lint/notempty/foo/v1/hello_api.proto",
 	)
 
+	assertDoLintFile(
+		t,
+		false,
+		`16:7:MESSAGE_FIELD_NAMES_FILENAME
+		18:9:MESSAGE_FIELD_NAMES_FILENAME
+		21:5:MESSAGE_FIELD_NAMES_FILENAME
+		23:3:MESSAGE_FIELD_NAMES_FILENAME
+		24:3:MESSAGE_FIELD_NAMES_FILENAME`,
+		"testdata/lint/fieldnamesfilename/hello.proto",
+	)
+
 	assertDoLintFile(
 		t,
 		false,
diff --git a/internal/cmd/testdata/lint/fieldnamesfilename/hello.proto b/internal/cmd/testdata/lint/fieldnamesfilename/hello.proto
new file mode 100644
index 0000000..2e8bcc5
--- /dev/null
+++ b/internal/cmd/testdata/lint/fieldnamesfilename/hello.proto
@@ -0,0 +1,25 @@
+syntax = "proto3";
+
+package foo.v1;
+
+option csharp_namespace = "Foo.V1";
+option go_package = "foov1";
+option java_multiple_files = true;
+option java_outer_classname = "HelloProto";
+option java_package = "com.foo.v1";
+option objc_class_prefix = "FXX";
+option php_namespace = "Foo\\V1";
+
+message Foo {
+  message NestedFoo {
+    message NestedNestedFoo {
+      string sfile_name = 1;
+      oneof hello {
+        string file_name = 2;
+      }
+    }
+    string sfile_names = 1;
+  }
+  string file_names = 1;
+  map<string, int64> map_file_name = 2;
+}
diff --git a/internal/cmd/testdata/lint/fieldnamesfilename/prototool.yaml b/internal/cmd/testdata/lint/fieldnamesfilename/prototool.yaml
new file mode 100644
index 0000000..ee80b43
--- /dev/null
+++ b/internal/cmd/testdata/lint/fieldnamesfilename/prototool.yaml
@@ -0,0 +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