Repository: uber/prototool
Pull request #260 (2018-12-28 18:19:06)

Add linter to outlaw using the json_name field option
We don't want people adjusting the default behavior in our new style guide. This outlaws the built-in message field option `json_name`.


Patch:
diff --git a/internal/lint/check_message_fields_no_json_name.go b/internal/lint/check_message_fields_no_json_name.go
new file mode 100644
index 0000000..c32f50d
--- /dev/null
+++ b/internal/lint/check_message_fields_no_json_name.go
@@ -0,0 +1,72 @@
+// 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 messageFieldsNoJSONNameLinter = NewLinter(
+	"MESSAGE_FIELDS_NO_JSON_NAME",
+	`Verifies that no message field has the the "json_name" option set.`,
+	checkMessageFieldsNoJSONName,
+)
+
+func checkMessageFieldsNoJSONName(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
+	return runVisitor(messageFieldsNoJSONNameVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type messageFieldsNoJSONNameVisitor struct {
+	baseAddVisitor
+}
+
+func (v messageFieldsNoJSONNameVisitor) VisitMessage(message *proto.Message) {
+	for _, element := range message.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldsNoJSONNameVisitor) VisitOneof(oneof *proto.Oneof) {
+	for _, element := range oneof.Elements {
+		element.Accept(v)
+	}
+}
+
+func (v messageFieldsNoJSONNameVisitor) VisitNormalField(field *proto.NormalField) {
+	v.handleField(field.Field)
+}
+
+func (v messageFieldsNoJSONNameVisitor) VisitOneofField(field *proto.OneOfField) {
+	v.handleField(field.Field)
+}
+
+func (v messageFieldsNoJSONNameVisitor) VisitMapField(field *proto.MapField) {
+	v.handleField(field.Field)
+}
+
+func (v messageFieldsNoJSONNameVisitor) handleField(field *proto.Field) {
+	for _, option := range field.Options {
+		if option.Name == "json_name" {
+			v.AddFailuref(field.Position, `Field %q cannot have the "json_name" option set.`, field.Name)
+		}
+	}
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index f1cabbb..f3e41c2 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -63,6 +63,7 @@ var (
 		fileOptionsUnsetJavaOuterClassnameLinter,
 		gogoNotImportedLinter,
 		messageFieldsNotFloatsLinter,
+		messageFieldsNoJSONNameLinter,
 		messageFieldNamesLowerSnakeCaseLinter,
 		messageFieldNamesLowercaseLinter,
 		messageNamesCamelCaseLinter,


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index df9dbc6..924e457 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -377,12 +377,24 @@ func TestLint(t *testing.T) {
 		`5:1:FILE_OPTIONS_GO_PACKAGE_NOT_LONG_FORM`,
 		"testdata/lint/gopackagelongform/gopackagelongform.proto",
 	)
+	assertDoLintFile(
+		t,
+		false,
+		`11:3:MESSAGE_FIELDS_NO_JSON_NAME
+		12:12:MESSAGE_FIELDS_NO_JSON_NAME
+		13:3:MESSAGE_FIELDS_NO_JSON_NAME
+		15:5:MESSAGE_FIELDS_NO_JSON_NAME
+		16:5:MESSAGE_FIELDS_NO_JSON_NAME`,
+		"testdata/lint/nojsonname/foo.proto",
+	)
+
 	assertDoLintFile(
 		t,
 		false,
 		`10:1:GOGO_NOT_IMPORTED`,
 		"testdata/lint/gogonotimported/gogonotimported.proto",
 	)
+
 	assertDoLintFile(
 		t,
 		false,
diff --git a/internal/cmd/testdata/lint/nojsonname/foo.proto b/internal/cmd/testdata/lint/nojsonname/foo.proto
new file mode 100644
index 0000000..3747772
--- /dev/null
+++ b/internal/cmd/testdata/lint/nojsonname/foo.proto
@@ -0,0 +1,18 @@
+syntax = "proto3";
+
+package foo;
+
+option go_package = "foopb";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo";
+
+message Foo {
+  int64 one = 1 [json_name = "test"];
+  repeated int64 two = 2 [json_name = "test"];
+  map<int64, int64> three = 3 [json_name = "test"];
+  oneof oneof_foo {
+    int64 four = 4 [json_name = "test"];
+    int64 five = 5 [json_name = "test"];
+  }
+}
diff --git a/internal/cmd/testdata/lint/nojsonname/prototool.yaml b/internal/cmd/testdata/lint/nojsonname/prototool.yaml
new file mode 100644
index 0000000..bed2890
--- /dev/null
+++ b/internal/cmd/testdata/lint/nojsonname/prototool.yaml
@@ -0,0 +1,4 @@
+lint:
+  rules:
+    add:
+      - MESSAGE_FIELDS_NO_JSON_NAME