fix: skip view generation on FooView name collision (#32)

buffa-codegen/src/lib.rs

@@ -424,47 +424,47 @@ 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<String> {
     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<String>,
+    ) {
         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() {
                 name.to_string()
             } 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(

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<String>,
 ) -> Result<(TokenStream, TokenStream, RegistryPaths), CodeGenError> {
     let scope = MessageScope {
         ctx,
@@ -72,14 +74,15 @@ 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(
     scope: MessageScope<'_>,
     msg: &DescriptorProto,
     rust_name: &str,
     resolver: &crate::imports::ImportResolver,
+    view_skip_fqns: &std::collections::HashSet<String>,
 ) -> 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::<Result<Vec<_>, _>>()?;
@@ -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! {}
         };

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}"
     );
 }