descriptor: reject ambiguous field identities

[fallintoplace]

What this does

Rejects duplicate field numbers and collisions across proto and JSON field names before building the message lookup indexes.

It also rejects negative and out-of-range oneof_index values instead of silently leaving fields with oneof presence but no oneof membership. DescriptorPool stages additions on a clone, so each invalid descriptor is rejected without changing an existing pool.

Coverage

The regression tests cover duplicate field numbers with different wire kinds, duplicate proto names, duplicate JSON names, proto/JSON cross-collisions, positive and negative oneof indices, and transactional rollback.

Validation

• cargo fmt --all -- --check
• cargo clippy --workspace --all-targets -- -D warnings
• cargo test --workspace
• cargo test -p buffa-descriptor --features reflect
• cargo check --workspace --all-features

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[iainmcgin]

[claude code] Thanks for this — three of the four rules are exactly right, and the oneof_index one is a genuine catch: link_field silently dropped an out-of-u16 index to None while presence resolution keyed off the raw oneof_index.is_some(), so a field could end up with oneof presence and no oneof membership. That's a real latent bug, not just validation.

The JSON-name rule needed reshaping, and I've done that maintainer-side in 9b3aa07 rather than sending it back to you. It rejected descriptor sets protoc itself emits. Verified against protoc 33.5:

case	protoc 33.5	buf 1.70	this PR (before)
proto2, derived JSON-name conflict	warning, emits	emits	rejected
proto3 + deprecated_legacy_json_field_conflicts	accepts	rejects	rejected
proto3, no opt-out	error	error	rejected
proto2, explicit duplicate json_name	error	error	rejected

I fed both protoc-emitted sets to DescriptorPool::decode on this branch and both came back REJECTED. Since protoc hard-errors on every genuine conflict itself, the rule could only ever fire on sets protoc had deliberately blessed.

The failure mode made it sharper than a build break: buffa-codegen never constructs a pool, so such a schema still compiles clean. The panic lands at runtime on first descriptor_pool() access — from generated code whose doc comment says a panic there "indicates a codegen bug, not consumer input", which would be actively wrong.

The fix turned out to need no new API, because protobuf already models this distinction and buffa already resolves it: json_format is LegacyBestEffort for proto2 and Allow for proto3/editions — the same line protoc draws — and deprecated_legacy_json_field_conflicts is protoc's explicit per-message opt-out. So the rule is now gated on msg_features.json_format == JsonFormat::Allow && !deprecated_legacy_json_field_conflicts. protoc rejects everything that leaves through, so a hand-built or third-party set with genuinely ambiguous JSON names is still caught, while protoc's output always loads. One condition, ~10 lines, no new public surface.

Worth noting a constraint I hit: protoc populates json_name for every field, so a descriptor set cannot distinguish an explicit json_name from a derived one. Exact protoc parity is therefore unreachable — this rule is a deliberate superset, slightly more lenient than protoc for hand-built proto2 sets, which is the safe direction.

Also added: three tests pinning the new behaviour (proto2 conflicts accepted, the opt-out honoured, duplicate proto names still rejected in proto2 — the leniency is scoped to JSON names only). The first two fail against the strict rule, so they genuinely guard it. And a changelog fragment: this is a user-visible tightening shipping in 0.9.0 and needed one — check-changelog only verifies CHANGELOG.md is in sync with existing fragments, so its passing wasn't evidence one wasn't needed. Merged current main in as well; the branch predated today's five merges.

Your other three rules are untouched and land as written. Approving.