Skip to content

descriptor: report missing extendee and method types by name #808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bniladridas wants to merge 1 commit into
base: main
Choose a base branch
from
Open

descriptor: report missing extendee and method types by name #808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions .changes/unreleased/changed-20261029-missing-type-name-context.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
kind: Changed
body: |-
**`PoolError::MissingTypeName` no longer reports extensions and methods as fields** (#641). An extension with no `extendee` now reports `MissingExtendee { extension }` as `extension <name> has no extendee`, and a method with no `input_type` or `output_type` reports `MissingMethodInputType { method }` or `MissingMethodOutputType { method }` as `method <name> has no input_type` or `has no output_type`. `MissingTypeName { field }` keeps its existing shape and message and is now emitted only for fields. The new variants are additive on the non-exhaustive `PoolError` enum.
time: 2026-10-29T11:03:11+05:30
79 changes: 66 additions & 13 deletions buffa-descriptor/src/pool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,19 @@ pub enum PoolError {
defined_in: String,
},
/// A field had no `type_name` for a `TYPE_MESSAGE`/`TYPE_GROUP`/`TYPE_ENUM`.
///
/// An extension with no `extendee` reports
/// [`MissingExtendee`](Self::MissingExtendee), and a method with no
/// `input_type` or `output_type` reports
/// [`MissingMethodInputType`](Self::MissingMethodInputType) or
/// [`MissingMethodOutputType`](Self::MissingMethodOutputType).
MissingTypeName { field: String },
/// An extension had no `extendee`.
MissingExtendee { extension: String },
/// An RPC method had no `input_type`.
MissingMethodInputType { method: String },
/// An RPC method had no `output_type`.
MissingMethodOutputType { method: String },
/// A field whose `type` is set to a scalar type also has a non-empty
/// `type_name`. Only a message, group or enum field, or a field with
/// `type` unset, may have one.
Expand Down Expand Up @@ -731,6 +743,15 @@ impl core::fmt::Display for PoolError {
write!(f, "message field {field} declares a default value")
}
Self::MissingTypeName { field } => write!(f, "field {field} has no type_name"),
Self::MissingExtendee { extension } => {
write!(f, "extension {extension} has no extendee")
}
Self::MissingMethodInputType { method } => {
write!(f, "method {method} has no input_type")
}
Self::MissingMethodOutputType { method } => {
write!(f, "method {method} has no output_type")
}
Self::UnexpectedTypeName { field, type_name } => write!(
f,
"field {field} with scalar type has type_name {type_name:?}"
Expand Down Expand Up @@ -1255,6 +1276,15 @@ struct LinkScope<'a> {
visible: Option<&'a BTreeSet<usize>>,
}

/// What an unresolved `type_name` would have named, so a missing one can
/// report the declaration and attribute that actually lacked it.
#[derive(Clone, Copy)]
enum MessageTypeRole {
ExtensionExtendee,
MethodInput,
MethodOutput,
}

/// Whether `file` declares `syntax = "proto3"`.
fn is_proto3(file: &FileDescriptorProto) -> bool {
file.syntax.as_deref() == Some("proto3")
Expand Down Expand Up @@ -2655,10 +2685,18 @@ impl DescriptorPool {
}
let method_fqn = format!("{fqn}.{}", m.name.as_deref().unwrap_or(""));
self.register_symbol(&method_fqn, SymbolKind::Method)?;
let input =
self.resolve_message_type_name(m.input_type.as_deref(), &method_fqn, scope)?;
let output =
self.resolve_message_type_name(m.output_type.as_deref(), &method_fqn, scope)?;
let input = self.resolve_message_type_name(
m.input_type.as_deref(),
&method_fqn,
MessageTypeRole::MethodInput,
scope,
)?;
let output = self.resolve_message_type_name(
m.output_type.as_deref(),
&method_fqn,
MessageTypeRole::MethodOutput,
scope,
)?;
methods.push(MethodDescriptor {
name: mname,
input,
Expand Down Expand Up @@ -2698,7 +2736,12 @@ impl DescriptorPool {
format!("{scope_fqn}.{name}")
};
self.register_symbol(&fqn, SymbolKind::Extension)?;
let extendee = self.resolve_message_type_name(ext.extendee.as_deref(), &fqn, scope)?;
let extendee = self.resolve_message_type_name(
ext.extendee.as_deref(),
&fqn,
MessageTypeRole::ExtensionExtendee,
scope,
)?;
// A proto3 file declares extensions only to define custom options.
let extendee_name = &self.messages[extendee.0 as usize].full_name;
if scope.proto3 && !is_proto3_extendee(extendee_name) {
Expand Down Expand Up @@ -2815,30 +2858,40 @@ impl DescriptorPool {
Ok(())
}

/// Resolve a method's `input_type`/`output_type` (a leading-dot FQN like
/// `.my.pkg.Request`) to a [`MessageIndex`].
/// Resolve a method's `input_type`/`output_type` or an extension's
/// `extendee` (a leading-dot FQN like `.my.pkg.Request`) to a
/// [`MessageIndex`].
fn resolve_message_type_name(
&self,
type_name: Option<&str>,
method_fqn: &str,
owner_fqn: &str,
role: MessageTypeRole,
scope: LinkScope<'_>,
) -> Result<MessageIndex, PoolError> {
let tn = type_name.ok_or_else(|| PoolError::MissingTypeName {
field: method_fqn.to_string(),
let tn = type_name.ok_or_else(|| match role {
MessageTypeRole::ExtensionExtendee => PoolError::MissingExtendee {
extension: owner_fqn.to_string(),
},
MessageTypeRole::MethodInput => PoolError::MissingMethodInputType {
method: owner_fqn.to_string(),
},
MessageTypeRole::MethodOutput => PoolError::MissingMethodOutputType {
method: owner_fqn.to_string(),
},
})?;
let lookup = tn.strip_prefix('.').unwrap_or(tn);
match self.by_name.get(lookup) {
Some(Definition::Message(midx)) => {
self.check_visible(scope, self.message_file[midx.0 as usize], tn, method_fqn)?;
self.check_visible(scope, self.message_file[midx.0 as usize], tn, owner_fqn)?;
Ok(*midx)
}
Some(Definition::Enum(_)) => Err(PoolError::WrongTypeKind {
type_name: tn.to_string(),
field: method_fqn.to_string(),
field: owner_fqn.to_string(),
}),
None => Err(PoolError::UnresolvedTypeName {
type_name: tn.to_string(),
field: method_fqn.to_string(),
field: owner_fqn.to_string(),
}),
}
}
Expand Down
165 changes: 165 additions & 0 deletions buffa-descriptor/tests/pool_e2e.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,171 @@ fn scalar_extensions_with_type_name_are_rejected_transactionally() {
);
}

#[test]
fn missing_field_type_name_names_the_field() {
use buffa_descriptor::generated::descriptor::field_descriptor_proto::Type;
use buffa_descriptor::generated::descriptor::DescriptorProto;

assert_rejected_without_mutating_pool(
"missing-field-type-name.proto",
"invalid.test.MissingFieldTypeName",
DescriptorProto {
name: Some("MissingFieldTypeName".into()),
field: vec![scalar_field("value", 1, Type::TYPE_MESSAGE)],
..Default::default()
},
|err| {
assert!(
matches!(
err,
PoolError::MissingTypeName { field }
if field == "invalid.test.MissingFieldTypeName.value"
),
"unexpected error: {err}"
);
assert_eq!(
err.to_string(),
"field invalid.test.MissingFieldTypeName.value has no type_name"
);
},
);
}

#[test]
fn missing_extension_extendee_names_the_extension() {
use buffa_descriptor::generated::descriptor::field_descriptor_proto::Type;
use buffa_descriptor::generated::descriptor::{
FieldDescriptorProto, FileDescriptorProto, FileDescriptorSet,
};

let set = FileDescriptorSet {
file: vec![FileDescriptorProto {
name: Some("missing-extendee.proto".into()),
package: Some("invalid.test".into()),
syntax: Some("proto2".into()),
extension: vec![FieldDescriptorProto {
..scalar_field("flag", 100, Type::TYPE_BOOL)
}],
..Default::default()
}],
..Default::default()
};

assert_set_rejected_without_mutating_pool(
"missing-extendee.proto",
"invalid.test.flag",
set,
|err| {
assert!(
matches!(
err,
PoolError::MissingExtendee { extension }
if extension == "invalid.test.flag"
),
"unexpected error: {err}"
);
assert_eq!(
err.to_string(),
"extension invalid.test.flag has no extendee"
);
},
);
}

#[test]
fn missing_method_input_type_names_the_method() {
use buffa_descriptor::generated::descriptor::{
FileDescriptorProto, FileDescriptorSet, MethodDescriptorProto, ServiceDescriptorProto,
};

let set = FileDescriptorSet {
file: vec![FileDescriptorProto {
name: Some("missing-input-type.proto".into()),
package: Some("invalid.test".into()),
syntax: Some("proto3".into()),
dependency: vec!["reflect_test.proto".into()],
service: vec![ServiceDescriptorProto {
name: Some("Gateway".into()),
method: vec![MethodDescriptorProto {
name: Some("Call".into()),
output_type: Some(".reflect.test.Inner".into()),
..Default::default()
}],
..Default::default()
}],
..Default::default()
}],
..Default::default()
};

assert_set_rejected_without_mutating_pool(
"missing-input-type.proto",
"invalid.test.Gateway.Call",
set,
|err| {
assert!(
matches!(
err,
PoolError::MissingMethodInputType { method }
if method == "invalid.test.Gateway.Call"
),
"unexpected error: {err}"
);
assert_eq!(
err.to_string(),
"method invalid.test.Gateway.Call has no input_type"
);
},
);
}

#[test]
fn missing_method_output_type_names_the_method() {
use buffa_descriptor::generated::descriptor::{
FileDescriptorProto, FileDescriptorSet, MethodDescriptorProto, ServiceDescriptorProto,
};

let set = FileDescriptorSet {
file: vec![FileDescriptorProto {
name: Some("missing-output-type.proto".into()),
package: Some("invalid.test".into()),
syntax: Some("proto3".into()),
dependency: vec!["reflect_test.proto".into()],
service: vec![ServiceDescriptorProto {
name: Some("Gateway".into()),
method: vec![MethodDescriptorProto {
name: Some("Call".into()),
input_type: Some(".reflect.test.Inner".into()),
..Default::default()
}],
..Default::default()
}],
..Default::default()
}],
..Default::default()
};

assert_set_rejected_without_mutating_pool(
"missing-output-type.proto",
"invalid.test.Gateway.Call",
set,
|err| {
assert!(
matches!(
err,
PoolError::MissingMethodOutputType { method }
if method == "invalid.test.Gateway.Call"
),
"unexpected error: {err}"
);
assert_eq!(
err.to_string(),
"method invalid.test.Gateway.Call has no output_type"
);
},
);
}

#[test]
fn scalar_map_values_with_type_name_are_rejected_transactionally() {
use buffa_descriptor::generated::descriptor::field_descriptor_proto::{Label, Type};
Expand Down
Loading