protobuf.js icon indicating copy to clipboard operation
protobuf.js copied to clipboard

fix!: nested namespace resolution

Open N2D4 opened this issue 5 years ago • 2 comments

When resolving types, we shouldn't check nested namespaces. Example:

message Foo {
    required Bar b = 1;
    required Qux q = 2;

    message Bar {
        required Corge c = 1;
    }

    message Qux {
        required Corge c = 1;

        message Corge {
            required string someString = 1;
        }
    }
}

message Corge {
    required int64 someInt = 1;
}

In this case, Foo.Bar.c should be of type .Corge, and hence have an integer field someInt (see docs). However, by running pbjs -t static -o out.js example.proto we can see that this is not the case, and the type resolves to .Foo.Qux.Corge instead:

// ...
    Foo.Bar = (function() {

        /**
         * Properties of a Bar.
         * @memberof Foo
         * @interface IBar
         * @property {Foo.Qux.ICorge} c Bar c
         */
// ...

protoc generates it correctly, as seen in this generated C++ code:

size_t Foo_Bar::ByteSizeLong() const {
// @@protoc_insertion_point(message_byte_size_start:Foo.Bar)
  size_t total_size = 0;

  // required .Corge c = 1;
  if (_internal_has_c()) {
    total_size += 1 +
      ::PROTOBUF_NAMESPACE_ID::internal::WireFormatLite::MessageSize(
        *c_);
  }

This PR fixes that.

N2D4 avatar Jun 24 '20 23:06 N2D4

This is technically a breaking change so let's leave it here unmerged until we're ready to go to 7.0.0.

alexander-fenster avatar Jul 10 '20 22:07 alexander-fenster

This problem still here in 7.2.4 or higher

xanahopper avatar May 24 '24 06:05 xanahopper