Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ format changes.
target's, and a oneof field.
- **Error text** matches protowire-go's. Text a caller matched on changes.

- **`Violation.name` for `(pxf.required)` with `(pxf.default)` on one field is
the default's literal** (#30, trendvidia/protowire-go#211). It was the bare
field name, which `element` already holds. An empty default gives an empty
`name`. This matches the other ports. For `(pxf.required)` on a oneof member,
`name` is the oneof's name, as protowire-cpp reports it.

### Added

- **`Violation.kind` values `"default_option"`, `"retired_number"` and
Expand Down
1 change: 0 additions & 1 deletion src/_protowire/module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,6 @@ PxfValidateDescriptor(nb::bytes fds_bytes, const std::string& full_name) {
case protowire::pxf::ViolationKind::kKeyOption: kind = "key_option"; break;
case protowire::pxf::ViolationKind::kRequiredOption:
kind = "required_default";
name = v.element.substr(v.element.rfind('.') + 1); // cpp leaves name empty here.
break;
case protowire::pxf::ViolationKind::kDefaultOption: kind = "default_option"; break;
case protowire::pxf::ViolationKind::kRetiredNumber: kind = "retired_number"; break;
Expand Down
7 changes: 4 additions & 3 deletions src/protowire/pxf.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ class Violation:
"well_known_shape",
]
element: str # fully-qualified protobuf name, e.g. "trades.v1.Side.null"
# The reserved identifier, the (pxf.key) value, the field's name
# ("required_default"), the (pxf.default) literal ("default_option"), or
# the retired number ("retired_number"); empty for "well_known_shape".
# The reserved identifier, the (pxf.key) value, the (pxf.default) literal
# ("default_option", and "required_default" beside a default), the oneof's
# name ("required_default" on a oneof member), or the retired number
# ("retired_number"); empty for "well_known_shape".
name: str
file: str # .proto file path the element is declared in

Expand Down
25 changes: 23 additions & 2 deletions tests/test_pxf_required_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ def _one_field(options: bytes):
return _bind(fd)


def _assert_refused(fds: bytes, cls, element: str, file: str) -> None:
def _assert_refused(fds: bytes, cls, element: str, file: str, name: str = "x") -> None:
# A document supplying the field: the schema is wrong whatever the input.
doc = 'f = "y"\n' if cls.DESCRIPTOR.fields_by_name.get("f") else ""
for call in (
Expand All @@ -91,7 +91,7 @@ def _assert_refused(fds: bytes, cls, element: str, file: str) -> None:
pxf.Violation(
kind="required_default",
element=element,
name=element.rsplit(".", 1)[1],
name=name, # The (pxf.default) literal (protowire-go#211).
file=file,
)
]
Expand All @@ -102,6 +102,11 @@ def test_required_with_default_is_refused_at_bind():
_assert_refused(fds, cls, "t.M.f", "one.proto")


def test_empty_default_gives_an_empty_name():
fds, cls = _one_field(REQUIRED + _opt_str(_DEFAULT, ""))
_assert_refused(fds, cls, "t.M.f", "one.proto", name="")


def test_option_order_does_not_matter():
fds, cls = _one_field(DEFAULT + REQUIRED)
_assert_refused(fds, cls, "t.M.f", "one.proto")
Expand Down Expand Up @@ -151,3 +156,19 @@ def test_misplaced_key_option_is_reported_with_its_kind():
_add_field(fd.message_type.add(name="M"), "f", 1, _opt_str(_KEY, "id"))
_, cls = _bind(fd)
assert [v.kind for v in pxf.validate_descriptor(cls())] == ["key_option"]


def test_required_on_a_oneof_member_names_the_oneof():
# protowire-cpp v2.0.0 reports this under the same kind, with the oneof's
# name in `name` ("Oneof Members").
fd = _file("oneof.proto")
m = fd.message_type.add(name="M")
m.oneof_decl.add(name="pick")
_add_field(m, "a", 1, REQUIRED)
_add_field(m, "b", 2)
for f in m.field:
f.oneof_index = 0
_, cls = _bind(fd)
assert pxf.validate_descriptor(cls()) == [
pxf.Violation(kind="required_default", element="t.M.a", name="pick", file="oneof.proto")
]
Loading