Repository: uber/prototool
Pull request #264 (2019-01-03 15:14:44)

Fix proto2 format bug, unintended rewrite from repeated to optional
Hello and happy new year from Sweden!

Please let me know if you'd like me to add more tests or make any other changes to this bugfix.

Feel free to append/rewrite commits as you see fit!

Cheers

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

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

```diff
@@            Coverage Diff             @@
##              dev     #264      +/-   ##
==========================================
- Coverage   71.37%   70.74%   -0.63%     
==========================================
  Files          90       91       +1     
  Lines        4527     4574      +47     
==========================================
+ Hits         3231     3236       +5     
- Misses        962     1004      +42     
  Partials      334      334
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/264?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/format/main\_visitor.go](https://codecov.io/gh/uber/prototool/pull/264/diff?src=pr&el=tree#diff-aW50ZXJuYWwvZm9ybWF0L21haW5fdmlzaXRvci5nbw==) | `87.15% <100%> (ø)` | :arrow_up: |
| [...ernal/reflect/gen/uber/proto/reflect/reflect.pb.go](https://codecov.io/gh/uber/prototool/pull/264/diff?src=pr&el=tree#diff-aW50ZXJuYWwvcmVmbGVjdC9nZW4vdWJlci9wcm90by9yZWZsZWN0L3JlZmxlY3QucGIuZ28=) | `10.63% <0%> (ø)` | |

------

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

ohlsont: What an amazing PR, would be nice to see a release with this. @peter-edge a review maybe? :rainbow: 
peter-edge: Quick pro tip: it's all good, but as a general courtesy, don't tag individual maintainers on GitHub for reviews for company repositories - we have our own review process - all good though!

LGTM. Note we're in a bit of flux so we won't have an actual release for a while, our apologies, but there's instructions in the README.md on how to install using Golang 1.11+ from a specific branch/commit. Use https://github.com/myitcv/gobin

```
# Install to /path/to/bin
GOBIN=/path/to/bin gobin github.com/uber/prototool/cmd/prototool@COMMIT
```

Patch:
diff --git a/internal/format/main_visitor.go b/internal/format/main_visitor.go
index 9213d83..236e817 100644
--- a/internal/format/main_visitor.go
+++ b/internal/format/main_visitor.go
@@ -132,9 +132,6 @@ func (v *mainVisitor) VisitImport(element *proto.Import) {
 func (v *mainVisitor) VisitNormalField(element *proto.NormalField) {
 	v.haveHitNonComment = true
 	prefix := ""
-	if element.Repeated {
-		prefix = "repeated "
-	}
 	if v.isProto2 {
 		// technically these are only set if the file is proto2
 		// but doing this just to make sure
@@ -144,6 +141,9 @@ func (v *mainVisitor) VisitNormalField(element *proto.NormalField) {
 			prefix = "optional "
 		}
 	}
+	if element.Repeated {
+		prefix = "repeated "
+	}
 	v.PField(prefix, element.Type, element.Field)
 }
 


Test patch:
diff --git a/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto b/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto
index a5aa5be..3dbdd78 100644
--- a/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto
+++ b/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto
@@ -41,6 +41,7 @@ message FooProto2 {
     required int64 bar = 3;
     optional int64 bar2 = 4;
   }
+  repeated int64 baz = 5;
 }
 
 message FooSomething {}
diff --git a/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto.golden b/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto.golden
index a4ae7bf..a9091a0 100644
--- a/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto.golden
+++ b/internal/cmd/testdata/format/proto2/foo/foo_proto2.proto.golden
@@ -37,6 +37,7 @@ message FooProto2 {
     required int64 bar = 3;
     optional int64 bar2 = 4;
   }
+  repeated int64 baz = 5;
 }
 
 message FooSomething {}