starwing / lua-protobuf

A Lua module to work with Google protobuf
MIT License
1.71k stars 388 forks source link

Disallow ignorezero in Message field #259

Closed mxi-box closed 6 months ago

mxi-box commented 6 months ago

According to Application note: Field presence, a Singular Message field without optional adjective should also have Explicit Presence. Ignoring zero-length message field causes indistinguishable presence.

sonarcloud[bot] commented 6 months ago

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

starwing commented 6 months ago

"Empty" length-delimited values (such as empty strings) can be validly represented in serialized values: the field is "present," in the sense that it appears in the wire format. However, if the generated API does not track presence, then these values may not be re-serialized; i.e., the empty field may be "not present" after a serialization round-trip.

It seems messages with all optional fields zeroed can be ignored in wired format. (in the docs your provided)

coveralls commented 6 months ago

Coverage Status

coverage: 99.61%. remained the same when pulling 7791963c833dccecb58eafab6596cdfa6743cc54 on mxi-box:master into 3631ca69c55052c9308e8a583c761087bcb409d9 on starwing:master.

mxi-box commented 6 months ago

"Empty" length-delimited values (such as empty strings) can be validly represented in serialized values: the field is "present," in the sense that it appears in the wire format. However, if the generated API does not track presence, then these values may not be re-serialized; i.e., the empty field may be "not present" after a serialization round-trip.

It seems messages with all optional fields zeroed can be ignored in wired format. (in the docs your provided)

if the generated API does not track presence, ...

Then the table Presence in proto3 APIs below writes that singular message's presence must be tracked regardless of optional

starwing commented 6 months ago

Okay, you are right. Merged.