Add ignore-unknown-fields flag

[codesoap]

I'm happily picking up the suggestion from #143 to add an ignoreUnknownFields flag with this pull request. Similarly to the mempool flag, it can either be used as an option in the .proto file, or as a flag on the CLI.

I was able to test the change with a command line flag on the CLI, but was unable to test it with the option in the .proto file. Unfortunately I still don't fully understand how to use the ext.proto file in a project (the import google/protobuf/descriptor.proto failed).

I'm also unsure whether I should add new test cases for the feature and if so, how.

[vmg]

Good start! We should definitely add some tests for individual messages. https://github.com/planetscale/vtprotobuf/blob/main/testproto/pool/pool.proto is an example of testing an ext flag; simply adding another package under testproto and ensuring that the generated code compiles successfully will be enough to test this feature, as all it's doing is removing fields from the codegen.

[codesoap]

Thanks for the pointer! I have added a simple .proto file and adapted the Makefile to generate Go code from it as well as compile the generated code (through go test [...]).

I have named the new package "ignore_unknown_fields". This is not idiomatic, as Go package names should be short. I felt that it's OK in this case, since this is not a package anyone is going to import.

[vmg]

Great! Now to wrap this up, I think we should make the flag more consistent and ignore unknown fields everywhere (in marshalling and size calculations), not only when unmarshalling. I've commented on the leftover usages that can be removed from codegen when the flag is set.

[codesoap]

I had not changed the code you mentioned, because it wouldn't do anything, if unknownFields hadn't been filled during the unmarshalling. If unknownFields was still present, someone must have used the "regular" Unmarshal instead of UnmarshalVT or maybe had stored an old message, from whence the ignoreUnknownFields feature was not active.

I wasn't quite sure what the best approach in such a scenario would be and thought it might be best to assume the unknownFields were present for a good reason. However, I'm fine with your approach as well and am now stripping the unknownFields out of every message when calculating size, cloning and marshalling.

I have not extended the documentation for the newly adapted methods, as I feel like the changes won't be interesting for most of the users.

PS: I assume you are aware, but just to be sure: The EqualVT method still reports equality only if the unknownFields match.

[vmg]

PS: I assume you are aware, but just to be sure: The EqualVT method still reports equality only if the unknownFields match.

I think it makes a lot of sense to not fail equality when you're working on a message where you've explicitly opted in to ignore unknown fields. Likewise, if you've Unmarshalled with protobuf, we still don't want to emit unknown fields when you serialize with this library, and so on. Any other behavior would be IMO inconsistent, so fully cleaning up the codegen of all unknownFields references seems sensible to me.

PS: thanks again for all your effort here!

[vmg]

Thank you!

[codesoap]

Thanks a lot for your help and patience!