diff --git a/buffa-codegen/src/lib.rs b/buffa-codegen/src/lib.rs index b709aa9..258ec47 100644 --- a/buffa-codegen/src/lib.rs +++ b/buffa-codegen/src/lib.rs @@ -424,32 +424,32 @@ fn check_module_name_conflicts(file: &FileDescriptorProto) -> Result<(), CodeGen check_siblings(&file.message_type, package) } -/// Check that no message named `FooView` collides with the generated view -/// type for a sibling message `Foo`. -fn check_view_name_conflicts(file: &FileDescriptorProto) -> Result<(), CodeGenError> { +/// Collect proto FQNs of messages whose generated `{Name}View` type would +/// collide with a sibling message of that name. Views for these messages +/// are skipped rather than erroring, so the rest of the crate still compiles. +fn collect_view_skip_fqns(file: &FileDescriptorProto) -> std::collections::HashSet { use std::collections::HashSet; - fn check_siblings( + fn collect_siblings( messages: &[crate::generated::descriptor::DescriptorProto], scope: &str, - ) -> Result<(), CodeGenError> { - // Collect all message names at this level. + out: &mut HashSet, + ) { let names: HashSet<&str> = messages.iter().filter_map(|m| m.name.as_deref()).collect(); - // For each message Foo, check if FooView also exists. for msg in messages { let name = msg.name.as_deref().unwrap_or(""); let view_name = format!("{}View", name); if names.contains(view_name.as_str()) { - return Err(CodeGenError::ViewNameConflict { - scope: scope.to_string(), - owned_msg: name.to_string(), - view_msg: view_name, - }); + let fqn = if scope.is_empty() { + name.to_string() + } else { + format!("{}.{}", scope, name) + }; + out.insert(fqn); } } - // Recurse into nested messages. for msg in messages { let name = msg.name.as_deref().unwrap_or(""); let child_scope = if scope.is_empty() { @@ -457,14 +457,14 @@ fn check_view_name_conflicts(file: &FileDescriptorProto) -> Result<(), CodeGenEr } else { format!("{}.{}", scope, name) }; - check_siblings(&msg.nested_type, &child_scope)?; + collect_siblings(&msg.nested_type, &child_scope, out); } - - Ok(()) } let package = file.package.as_deref().unwrap_or(""); - check_siblings(&file.message_type, package) + let mut skip = HashSet::new(); + collect_siblings(&file.message_type, package, &mut skip); + skip } /// Generate Rust source for a single `.proto` file. @@ -475,9 +475,11 @@ fn generate_file( // Validate descriptors before generating code. check_reserved_field_names(file)?; check_module_name_conflicts(file)?; - if ctx.config.generate_views { - check_view_name_conflicts(file)?; - } + let view_skip_fqns = if ctx.config.generate_views { + collect_view_skip_fqns(file) + } else { + std::collections::HashSet::new() + }; let resolver = imports::ImportResolver::for_file(file); let mut tokens = resolver.generate_use_block(); @@ -520,6 +522,7 @@ fn generate_file( &proto_fqn, &features, &resolver, + &view_skip_fqns, )?; tokens.extend(msg_top); // Nested extension const paths are relative to the message's module @@ -537,7 +540,7 @@ fn generate_file( reg.json_any.extend(msg_reg.json_any); reg.text_any.extend(msg_reg.text_any); - let view_mod = if ctx.config.generate_views { + let view_mod = if ctx.config.generate_views && !view_skip_fqns.contains(&proto_fqn) { let (view_top, view_mod) = view::generate_view( ctx, message_type, @@ -692,17 +695,6 @@ pub enum CodeGenError { oneof_name: String, attempted: String, }, - /// A message named `FooView` collides with the generated view type for - /// message `Foo`. - #[error( - "name conflict in '{scope}': message '{view_msg}' collides with \ - the generated view type for message '{owned_msg}'" - )] - ViewNameConflict { - scope: String, - owned_msg: String, - view_msg: String, - }, /// The input contains a message with `option message_set_wire_format = true` /// but [`CodeGenConfig::allow_message_set`] was not set. #[error( diff --git a/buffa-codegen/src/message.rs b/buffa-codegen/src/message.rs index e792570..58ad6f8 100644 --- a/buffa-codegen/src/message.rs +++ b/buffa-codegen/src/message.rs @@ -56,6 +56,7 @@ impl RegistryPaths { /// to the per-message `__*_JSON_ANY` / `__*_TEXT_ANY` consts (relative to /// the struct's scope) and per-extension `__*_JSON_EXT` / `__*_TEXT_EXT` /// consts (relative to the `module_items` scope) for `register_types`. +#[allow(clippy::too_many_arguments)] pub fn generate_message( ctx: &CodeGenContext, msg: &DescriptorProto, @@ -64,6 +65,7 @@ pub fn generate_message( proto_fqn: &str, features: &ResolvedFeatures, resolver: &crate::imports::ImportResolver, + view_skip_fqns: &std::collections::HashSet, ) -> Result<(TokenStream, TokenStream, RegistryPaths), CodeGenError> { let scope = MessageScope { ctx, @@ -72,7 +74,7 @@ pub fn generate_message( features, nesting: 0, }; - generate_message_with_nesting(scope, msg, rust_name, resolver) + generate_message_with_nesting(scope, msg, rust_name, resolver, view_skip_fqns) } fn generate_message_with_nesting( @@ -80,6 +82,7 @@ fn generate_message_with_nesting( msg: &DescriptorProto, rust_name: &str, resolver: &crate::imports::ImportResolver, + view_skip_fqns: &std::collections::HashSet, ) -> Result<(TokenStream, TokenStream, RegistryPaths), CodeGenError> { let MessageScope { ctx, @@ -138,6 +141,7 @@ fn generate_message_with_nesting( nested, nested_proto_name, resolver, + view_skip_fqns, ) }) .collect::, _>>()?; @@ -516,23 +520,26 @@ fn generate_message_with_nesting( reg_paths.text_any.push(quote! { #mod_ident :: #p }); } - // Also generate views for nested messages if enabled. - // view_top (struct + impls) goes alongside the owned struct in the - // parent module; view_mod (oneof view enums) goes in the sub-module. + // Also generate views for nested messages if enabled and not skipped + // due to a sibling name collision (e.g. FooView message exists). let view_mod_items = if ctx.config.generate_views { let nested_name = nested_desc.name.as_deref().unwrap_or(""); let nested_fqn = format!("{}.{}", proto_fqn, nested_name); - let (view_top, view_mod) = crate::view::generate_view_with_nesting( - MessageScope { - proto_fqn: &nested_fqn, - nesting: nesting + 1, - ..scope - }, - nested_desc, - nested_name, - )?; - nested_items.extend(view_top); - view_mod + if view_skip_fqns.contains(&nested_fqn) { + quote! {} + } else { + let (view_top, view_mod) = crate::view::generate_view_with_nesting( + MessageScope { + proto_fqn: &nested_fqn, + nesting: nesting + 1, + ..scope + }, + nested_desc, + nested_name, + )?; + nested_items.extend(view_top); + view_mod + } } else { quote! {} }; diff --git a/buffa-codegen/src/tests/naming.rs b/buffa-codegen/src/tests/naming.rs index e751369..359e4d2 100644 --- a/buffa-codegen/src/tests/naming.rs +++ b/buffa-codegen/src/tests/naming.rs @@ -533,8 +533,9 @@ fn test_sibling_oneofs_get_distinct_names() { } #[test] -fn test_view_name_conflict_detected() { - // Messages "Foo" and "FooView" — Foo's view type collides with FooView struct. +fn test_view_name_collision_skips_view_generation() { + // Messages "Foo" and "FooView" — Foo's view type would collide with the + // FooView struct, so Foo's view is skipped rather than erroring. let mut file = proto3_file("test.proto"); file.package = Some("pkg".to_string()); file.message_type = vec![ @@ -550,12 +551,27 @@ fn test_view_name_conflict_detected() { let config = CodeGenConfig::default(); // views enabled by default let result = generate(&[file], &["test.proto".to_string()], &config); - assert!(result.is_err()); - let err = result.unwrap_err().to_string(); - assert!(err.contains("Foo"), "should mention owned message: {err}"); + let files = result.expect("view collision should be handled gracefully"); + let content = &files[0].content; + assert!( + content.contains("pub struct Foo"), + "owned Foo struct must exist: {content}" + ); + assert!( + content.contains("pub struct FooView"), + "FooView message struct must exist: {content}" + ); + // The generated view for Foo would be `pub struct FooView<'a>` with a + // `MessageView` impl — but that collides with the user-defined FooView + // message, so the view must be skipped. The user-defined FooView + // message's *own* view (`FooViewView<'a>`) should still exist. + assert!( + content.contains("FooViewView"), + "FooView message's own view (FooViewView) should still be generated: {content}" + ); assert!( - err.contains("FooView"), - "should mention view collision: {err}" + !content.contains("impl<'a> ::buffa::MessageView<'a> for FooView<'a>"), + "Foo's view impl should be skipped to avoid collision: {content}" ); }