Skip to content
Open
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
5 changes: 5 additions & 0 deletions .changes/unreleased/fixed-20261029-relative-type-names.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
kind: Fixed
body: |-
**`DescriptorPool` resolves a type name without a leading dot as protoc does** (closes #635).
protoc always writes the leading dot, but a hand-built descriptor may leave it out. The pool looked such a name up as fully qualified, so `type_name: "M"` on a field of `a.Holder` failed with `UnresolvedTypeName`, or linked to a root-level `M` from an imported file with no package where protoc and protobuf-go link `a.M`. A field's `type_name`, an extension's `extendee` and a method's `input_type` and `output_type` without a leading dot are now resolved against each enclosing scope, innermost first, and then as written; a compound name `A.B` stops at the innermost scope that defines `A`, and a match in a file that is not imported is passed over for one further out. A name with a leading dot, which is everything protoc writes, links as before.
time: 2026-10-29T23:30:00+00:00
96 changes: 82 additions & 14 deletions buffa-descriptor/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,9 @@ pub enum PoolError {
/// A field with `type` unset takes its kind from the type the name
/// resolves to, so it reports this error as well.
///
/// The pool looks a `type_name` up as a fully-qualified name, with or
/// without the leading dot. A name relative to the field's scope, which
/// protoc resolves before it writes a descriptor, is reported here.
/// A `type_name` with a leading dot is fully qualified. One without is
/// resolved as protoc resolves it, against each scope enclosing the
/// field, innermost first, and then as written.
UnresolvedTypeName { type_name: String, field: String },
/// A field's `type_name` resolved to the wrong kind (e.g. a `TYPE_ENUM`
/// field referencing a message). Carries the name and the field.
Expand Down Expand Up @@ -1487,8 +1487,9 @@ impl DescriptorPool {
///
/// A field that leaves `type` unset takes its kind from the message or
/// enum that its `type_name` resolves to. With `type_name` also unset or
/// empty, the field links as `double`. Every `type_name` must be fully
/// qualified; the leading dot is optional.
/// empty, the field links as `double`. A `type_name`, `extendee`,
/// `input_type` or `output_type` without a leading dot is resolved
/// relative to the declaration that holds it, as protoc resolves it.
///
/// # Errors
///
Expand Down Expand Up @@ -2815,8 +2816,77 @@ impl DescriptorPool {
Ok(())
}

/// Resolve a method's `input_type`/`output_type` (a leading-dot FQN like
/// `.my.pkg.Request`) to a [`MessageIndex`].
/// Look `type_name` up as protoc does. A leading dot marks a
/// fully-qualified name. Any other name is relative to `referrer`, the
/// full name of the field, extension or method that holds it: each
/// enclosing scope is tried, innermost first, then the name on its own.
/// A compound name `A.B` stops at the innermost scope that defines `A`.
/// A definition in a file `scope` cannot see is passed over for a visible
/// one further out, and is returned only when none matches, so that the
/// caller reports it as not imported.
fn lookup_type_name(
&self,
type_name: &str,
referrer: &str,
scope: LinkScope<'_>,
) -> Option<Definition> {
if let Some(full_name) = type_name.strip_prefix('.') {
return self.by_name.get(full_name).copied();
}
let first = type_name.split('.').next().unwrap_or(type_name);
let compound = first.len() < type_name.len();
let mut hidden = None;
let mut enclosing = referrer;
while let Some((outer, _)) = enclosing.rsplit_once('.') {
enclosing = outer;
if compound {
if self.is_aggregate(&format!("{outer}.{first}")) {
return self.by_name.get(&format!("{outer}.{type_name}")).copied();
}
} else if let Some(&def) = self.by_name.get(&format!("{outer}.{type_name}")) {
if self.is_visible(scope, def) {
return Some(def);
}
hidden.get_or_insert(def);
}
}
match self.by_name.get(type_name).copied() {
Some(def) if self.is_visible(scope, def) => Some(def),
found => hidden.or(found),
}
}

/// Whether `name` is a message, enum, service or package, the symbols a
/// compound type name can continue into. A package is known only through
/// the symbols declared in it.
fn is_aggregate(&self, name: &str) -> bool {
if matches!(
self.symbols.get(name),
Some(SymbolKind::Message | SymbolKind::Enum | SymbolKind::Service)
) {
return true;
}
let prefix = format!("{name}.");
self.symbols
.range::<String, _>(&prefix..)
.next()
.is_some_and(|(next, _)| next.starts_with(&prefix))
}

/// Whether `scope` may reference `def`.
fn is_visible(&self, scope: LinkScope<'_>, def: Definition) -> bool {
let def_file = match def {
Definition::Message(midx) => self.message_file[midx.0 as usize],
Definition::Enum(eidx) => self.enum_file[eidx.0 as usize],
};
match scope.visible {
Some(visible) => visible.contains(&def_file),
None => true,
}
}

/// Resolve a method's `input_type`/`output_type`, or an extension's
/// `extendee`, to a [`MessageIndex`].
fn resolve_message_type_name(
&self,
type_name: Option<&str>,
Expand All @@ -2826,11 +2896,10 @@ impl DescriptorPool {
let tn = type_name.ok_or_else(|| PoolError::MissingTypeName {
field: method_fqn.to_string(),
})?;
let lookup = tn.strip_prefix('.').unwrap_or(tn);
match self.by_name.get(lookup) {
match self.lookup_type_name(tn, method_fqn, scope) {
Some(Definition::Message(midx)) => {
self.check_visible(scope, self.message_file[midx.0 as usize], tn, method_fqn)?;
Ok(*midx)
Ok(midx)
}
Some(Definition::Enum(_)) => Err(PoolError::WrongTypeKind {
type_name: tn.to_string(),
Expand Down Expand Up @@ -3282,20 +3351,19 @@ impl DescriptorPool {
let tn = type_name.ok_or_else(|| PoolError::MissingTypeName {
field: field_fqn.to_string(),
})?;
let lookup = tn.strip_prefix('.').unwrap_or(tn);
match self.by_name.get(lookup) {
match self.lookup_type_name(tn, field_fqn, scope) {
Some(Definition::Message(midx))
if matches!(
ty,
None | Some(ProtoType::TYPE_MESSAGE | ProtoType::TYPE_GROUP)
) =>
{
self.check_visible(scope, self.message_file[midx.0 as usize], tn, field_fqn)?;
Ok(SingularKind::Message(*midx))
Ok(SingularKind::Message(midx))
}
Some(Definition::Enum(eidx)) if matches!(ty, None | Some(ProtoType::TYPE_ENUM)) => {
self.check_visible(scope, self.enum_file[eidx.0 as usize], tn, field_fqn)?;
Ok(SingularKind::Enum(*eidx))
Ok(SingularKind::Enum(eidx))
}
Some(_) => Err(PoolError::WrongTypeKind {
type_name: tn.to_string(),
Expand Down
195 changes: 191 additions & 4 deletions buffa-descriptor/tests/pool_e2e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1869,15 +1869,13 @@ fn inferred_message_fields_follow_the_message_encoding_feature() {
assert!(delimited.is_delimited());
}

/// The pool looks a `type_name` up as written. A name that is relative to the
/// field's scope, here the field's own message, is unresolved like a name
/// that no type has.
/// A name that no type has is unresolved, written fully qualified or relative.
#[test]
fn unresolved_inferred_type_names_are_rejected_transactionally() {
use buffa_descriptor::generated::descriptor::field_descriptor_proto::Label;
use buffa_descriptor::generated::descriptor::DescriptorProto;

for type_name in [".invalid.test.Missing", "Holder"] {
for type_name in [".invalid.test.Missing", "Missing", "Holder.Missing"] {
assert_rejected_without_mutating_pool(
"inferred-unresolved.proto",
"invalid.test.Holder",
Expand All @@ -1900,6 +1898,195 @@ fn unresolved_inferred_type_names_are_rejected_transactionally() {
}
}

mod relative_type_names {
use buffa_descriptor::generated::descriptor::field_descriptor_proto::{Label, Type};
use buffa_descriptor::generated::descriptor::{
DescriptorProto, FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet,
MethodDescriptorProto, ServiceDescriptorProto,
};
use buffa_descriptor::{DescriptorPool, FieldKind, PoolError, SingularKind};

fn msg_field(name: &str, number: i32, type_name: &str) -> FieldDescriptorProto {
FieldDescriptorProto {
name: Some(name.into()),
number: Some(number),
label: Some(Label::LABEL_OPTIONAL),
r#type: Some(Type::TYPE_MESSAGE),
type_name: Some(type_name.into()),
..Default::default()
}
}

fn message(
name: &str,
field: Vec<FieldDescriptorProto>,
nested_type: Vec<DescriptorProto>,
) -> DescriptorProto {
DescriptorProto {
name: Some(name.into()),
field,
nested_type,
..Default::default()
}
}

fn file(
name: &str,
package: Option<&str>,
dependency: &[&str],
message_type: Vec<DescriptorProto>,
) -> FileDescriptorProto {
FileDescriptorProto {
name: Some(name.into()),
package: package.map(Into::into),
syntax: Some("proto3".into()),
dependency: dependency.iter().map(|d| (*d).to_string()).collect(),
message_type,
..Default::default()
}
}

fn empty(name: &str) -> DescriptorProto {
message(name, vec![], vec![])
}

fn link(files: Vec<FileDescriptorProto>) -> Result<DescriptorPool, PoolError> {
DescriptorPool::new(FileDescriptorSet {
file: files,
..Default::default()
})
}

/// Asserts that field `number` of `message` links to the message `target`.
fn assert_links(pool: &DescriptorPool, message: &str, number: u32, target: &str) {
let holder = pool.message_by_name(message).unwrap();
let expected = SingularKind::Message(pool.message_index(target).unwrap());
assert_eq!(
holder.field(number).unwrap().kind(),
FieldKind::Singular(expected)
);
}

/// protoc and protobuf-go try each enclosing scope, innermost first, so a
/// package-level `M` wins over a root-level `M` from a file with no
/// package.
#[test]
fn a_name_without_a_leading_dot_resolves_against_the_enclosing_scopes() {
let mut a = file(
"a.proto",
Some("a"),
&["root.proto"],
vec![
empty("M"),
message(
"Holder",
vec![msg_field("innermost_scope", 1, "M")],
vec![empty("M")],
),
message(
"Other",
vec![
msg_field("package_level", 1, "M"),
msg_field("nested", 2, "Holder.M"),
msg_field("dotless_full_name", 3, "a.M"),
msg_field("root", 4, ".M"),
],
vec![],
),
],
);
a.service = vec![ServiceDescriptorProto {
name: Some("Svc".into()),
method: vec![MethodDescriptorProto {
name: Some("Call".into()),
input_type: Some("M".into()),
output_type: Some("Holder.M".into()),
..Default::default()
}],
..Default::default()
}];
let root = file("root.proto", None, &[], vec![empty("M")]);
let pool = link(vec![root, a]).expect("relative names link");

assert_links(&pool, "a.Holder", 1, "a.Holder.M");
assert_links(&pool, "a.Other", 1, "a.M");
assert_links(&pool, "a.Other", 2, "a.Holder.M");
assert_links(&pool, "a.Other", 3, "a.M");
assert_links(&pool, "a.Other", 4, "M");
let call = &pool.service_by_name("a.Svc").unwrap().methods()[0];
assert_eq!(call.input(), pool.message_index("a.M").unwrap());
assert_eq!(call.output(), pool.message_index("a.Holder.M").unwrap());
}

/// A compound name stops at the innermost scope that defines its first
/// component, so `Bar.Baz` inside `Foo` looks in `Foo.Bar` only, as in
/// protoc.
#[test]
fn a_compound_name_stops_at_the_innermost_scope_of_its_first_component() {
let t = file(
"t.proto",
Some("t"),
&[],
vec![
message("Bar", vec![], vec![empty("Baz")]),
message(
"Foo",
vec![msg_field("nested_baz", 1, "Bar.Baz")],
vec![empty("Bar")],
),
],
);
let err = link(vec![t]).unwrap_err();
assert!(
matches!(
&err,
PoolError::UnresolvedTypeName { type_name, field }
if type_name == "Bar.Baz" && field == "t.Foo.nested_baz"
),
"unexpected error: {err}"
);
}

#[test]
fn a_partly_qualified_name_resolves_through_an_enclosing_package() {
let xy = file(
"xy.proto",
Some("x.y"),
&[],
vec![
empty("M"),
message("Holder", vec![msg_field("m", 1, "y.M")], vec![]),
],
);
let pool = link(vec![xy]).expect("partly qualified name links");
assert_links(&pool, "x.y.Holder", 1, "x.y.M");
}

/// protoc passes over a match in a file the referring file does not
/// import and keeps looking further out.
#[test]
fn a_match_in_a_file_that_is_not_imported_is_passed_over() {
let hidden = file("hidden.proto", Some("a"), &[], vec![empty("M")]);
let root = file("root.proto", None, &[], vec![empty("M")]);
let holder = message("Holder", vec![msg_field("m", 1, "M")], vec![]);
let user = |deps: &[&str]| file("user.proto", Some("a"), deps, vec![holder.clone()]);

let linked = link(vec![hidden.clone(), root, user(&["root.proto"])])
.expect("the imported root-level M links");
assert_links(&linked, "a.Holder", 1, "M");

let err = link(vec![hidden, user(&[])]).unwrap_err();
assert!(
matches!(
&err,
PoolError::TypeNotImported { field, defined_in, .. }
if field == "a.Holder.m" && defined_in == "hidden.proto"
),
"unexpected error: {err}"
);
}
}

#[test]
fn oneof_links() {
let p = pool();
Expand Down
Loading