fix(boundary): support reference impl owners without graph collisions

crates/sc-lint-boundary/src/analysis.rs

@@ -520,7 +520,8 @@ fn owner_id_for_node_id(node_id: &NodeId, node_kind: &str) -> Option<OwnerId> {
             .rsplit_once("::")
             .map(|(parent, _)| OwnerId::new(parent)),
         "method" => node_id
-            .rsplit_once("::")
+            .split_once("::impl::")
+            .or_else(|| node_id.rsplit_once("::"))
             .map(|(parent, _)| OwnerId::new(parent)),
         "variant" => node_id
             .rsplit_once("::variant::")

crates/sc-lint-boundary/src/graph/build.rs

@@ -487,7 +487,8 @@ fn ingest_module_items(
                 );
             }
             Item::Impl(item_impl) => {
-                let owner_name = impl_owner_name(&item_impl.self_ty)?;
+                let owner = impl_owner(&item_impl.self_ty)?;
+                let owner_name = &owner.name;
                 let owner_node_id = NodeId::new(format!("{parent_module_id}::{owner_name}"));
                 let trait_path = item_impl
                     .trait_
@@ -502,6 +503,22 @@ fn ingest_module_items(
                     NodeId::new(format!("{owner_node_id}::impl::inherent"))
                 };
 
+                // Keep established path-owner IDs stable. References share the
+                // target type, but must not merge their impls or methods with it.
+                let impl_node_id = if owner.is_reference {
+                    NodeId::new(format!(
+                        "{impl_node_id}::self::{}",
+                        hex_encode(owner.self_type.as_bytes())
+                    ))
+                } else {
+                    impl_node_id
+                };
+                let owner_label = if owner.is_reference {
+                    &owner.self_type
+                } else {
+                    owner_name
+                };
+
                 if !builder
                     .nodes
                     .iter()
@@ -533,8 +550,8 @@ fn ingest_module_items(
                     kind: NodeKind::Impl.as_str(),
                     label: trait_path
                         .as_ref()
-                        .map(|path| format!("impl {path} for {owner_name}"))
-                        .unwrap_or_else(|| format!("impl {owner_name}")),
+                        .map(|path| format!("impl {path} for {owner_label}"))
+                        .unwrap_or_else(|| format!("impl {owner_label}")),
                     visibility: None,
                     package: context.package_name.clone(),
                     target: Some(context.target_name.clone()),
@@ -585,8 +602,13 @@ fn ingest_module_items(
 
                 for impl_item in item_impl.items {
                     if let ImplItem::Fn(method) = impl_item {
+                        let method_owner = if owner.is_reference {
+                            &impl_node_id
+                        } else {
+                            &owner_node_id
+                        };
                         let method_id =
-                            NodeId::new(format!("{owner_node_id}::{}", method.sig.ident));
+                            NodeId::new(format!("{method_owner}::{}", method.sig.ident));
                         builder.add_node(GraphNode {
                             id: method_id.clone(),
                             kind: NodeKind::Method.as_str(),
@@ -622,7 +644,7 @@ fn ingest_module_items(
                             module_path,
                             collect_references_with(
                                 &local_owner_names,
-                                Some(&owner_name),
+                                Some(owner_name),
                                 &context.workspace_dependency_roots,
                                 |collector| {
                                     collector.visit_impl_item_fn(&method);

crates/sc-lint-boundary/src/graph/mod.rs

@@ -145,20 +145,44 @@ fn parse_rust_file(path: &Path) -> Result<File> {
         .with_context(|| format!("failed to parse Rust source {}", path.display()))
 }
 
-fn impl_owner_name(self_ty: &Type) -> Result<String> {
+struct ImplOwner {
+    name: String,
+    self_type: String,
+    is_reference: bool,
+}
+
+fn impl_owner(self_ty: &Type) -> Result<ImplOwner> {
     match self_ty {
         Type::Path(type_path) => {
-            // `syn::Type::Path` always stores at least one segment for a valid path type.
-            if let Some(segment) = type_path.path.segments.last() {
-                Ok(segment.ident.to_string())
+            let segment =
+                type_path.path.segments.last().ok_or_else(|| {
+                    anyhow::anyhow!("impl owner path is missing a terminal segment")
+                })?;
+            Ok(ImplOwner {
+                name: segment.ident.to_string(),
+                self_type: self_ty.to_token_stream().to_string(),
+                is_reference: false,
+            })
+        }
+        Type::Reference(reference) => {
+            let mut owner = impl_owner(&reference.elem)?;
+            let lifetime = reference
+                .lifetime
+                .as_ref()
+                .map_or_else(String::new, |value| format!("{} ", value.to_token_stream()));
+            let mutability = if reference.mutability.is_some() {
+                "mut "
             } else {
-                Err(anyhow::anyhow!(
-                    "impl owner path is missing a terminal segment"
-                ))
-            }
+                ""
+            };
+            owner.self_type = format!("&{lifetime}{mutability}{}", owner.self_type);
+            owner.is_reference = true;
+            Ok(owner)
         }
+        Type::Paren(paren) => impl_owner(&paren.elem),
+        Type::Group(group) => impl_owner(&group.elem),
         _ => anyhow::bail!(
-            "unsupported impl owner type `{}`; only path owners are supported",
+            "unsupported impl owner type `{}`; expected a path or reference to a path",
             self_ty.to_token_stream()
         ),
     }
@@ -205,3 +229,28 @@ pub(crate) fn load_metadata(root: &Path) -> Result<cargo_metadata::Metadata> {
         .exec()
         .with_context(|| format!("failed to load cargo metadata for {}", root.display()))
 }
+
+#[cfg(test)]
+mod owner_tests {
+    use super::*;
+
+    #[test]
+    fn parenthesized_and_grouped_owners_are_transparent() {
+        let path: Type = syn::parse_quote!(Owner);
+        let parenthesized: Type = syn::parse_quote!((Owner));
+        let grouped = Type::Group(syn::TypeGroup {
+            group_token: Default::default(),
+            elem: Box::new(parenthesized),
+        });
+        let plain = impl_owner(&path).unwrap();
+        let wrapped = impl_owner(&grouped).unwrap();
+        assert_eq!(plain.name, wrapped.name);
+        assert_eq!(plain.self_type, wrapped.self_type);
+        assert!(!wrapped.is_reference);
+        let reference: Type = syn::parse_quote!((&'a mut (Owner)));
+        let wrapped = impl_owner(&reference).unwrap();
+        assert_eq!(wrapped.name, "Owner");
+        assert_eq!(wrapped.self_type, "&'a mut Owner");
+        assert!(wrapped.is_reference);
+    }
+}

crates/sc-lint-boundary/src/tests.rs

@@ -2042,7 +2042,7 @@ fn preserves_full_trait_path_in_trait_impl_self_loop_messages() {
 }
 
 #[test]
-fn rejects_non_path_impl_owners() {
+fn rejects_impl_owners_without_a_named_target() {
     let fixture = WorkspaceFixture::new();
     fixture.write_workspace_root();
     fixture.write_package_manifest("example");
@@ -2052,7 +2052,7 @@ fn rejects_non_path_impl_owners() {
         r#"
                 pub struct Loop;
 
-                impl core::fmt::Display for &Loop {
+                impl core::fmt::Display for (Loop, Loop) {
                     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
                         write!(f, "loop")
                     }
@@ -2067,7 +2067,7 @@ fn rejects_non_path_impl_owners() {
 
     let message = format!("{error:#}");
     assert!(message.contains("unsupported impl owner type"));
-    assert!(message.contains("&Loop") || message.contains("& Loop"));
+    assert!(message.contains("(Loop , Loop)"));
 }
 
 #[test]
@@ -2633,3 +2633,146 @@ fn trim_indentation(input: &str) -> String {
     }
     output
 }
+
+#[test]
+fn supports_generic_field_value_reference_impl_owner() {
+    let fixture = WorkspaceFixture::new();
+    fixture.write_workspace_root();
+    fixture.write_package_manifest("example");
+    fixture.write_source(
+        "example",
+        "lib.rs",
+        r#"
+        mod serde { pub trait Serialize {} }
+        pub struct FieldValue<'a, T: ?Sized>(&'a T);
+        pub trait SerializeKindTag {}
+        impl<T: ?Sized + serde::Serialize> SerializeKindTag for &FieldValue<'_, T> {}
+    "#,
+    );
+    let graph = export_workspace_graph(&ExportGraphOptions {
+        root: fixture.root().to_path_buf(),
+    })
+    .unwrap();
+    let owner = graph
+        .nodes
+        .iter()
+        .find(|node| node.label == "FieldValue")
+        .unwrap();
+    let implementation = graph
+        .nodes
+        .iter()
+        .find(|node| node.impl_trait.as_deref() == Some("SerializeKindTag"))
+        .unwrap();
+    assert!(implementation.label.contains("&FieldValue"));
+    assert!(
+        graph
+            .edges
+            .iter()
+            .any(|edge| edge.kind == EdgeKind::Targets.as_str()
+                && edge.from == implementation.id
+                && edge.to == owner.id)
+    );
+}
+
+#[test]
+fn reference_impls_and_methods_do_not_collide_with_owned_impls() {
+    let fixture = WorkspaceFixture::new();
+    fixture.write_workspace_root();
+    fixture.write_package_manifest("example");
+    fixture.write_source(
+        "example",
+        "lib.rs",
+        r#"
+        pub struct Owner;
+        pub trait Action { fn act(&self); }
+        impl Action for Owner { fn act(&self) {} }
+        impl Action for &Owner { fn act(&self) {} }
+        impl Action for &mut Owner { fn act(&self) {} }
+        impl Action for &&Owner { fn act(&self) {} }
+    "#,
+    );
+    let graph = export_workspace_graph(&ExportGraphOptions {
+        root: fixture.root().to_path_buf(),
+    })
+    .unwrap();
+    let owner = graph
+        .nodes
+        .iter()
+        .find(|node| node.label == "Owner")
+        .unwrap();
+    let implementations: Vec<_> = graph
+        .nodes
+        .iter()
+        .filter(|node| node.kind == "impl")
+        .collect();
+    let methods: Vec<_> = graph
+        .nodes
+        .iter()
+        .filter(|node| node.kind == "method" && node.impl_trait.as_deref() == Some("Action"))
+        .collect();
+    assert_eq!(implementations.len(), 4);
+    assert_eq!(methods.len(), 4);
+    let owned = implementations
+        .iter()
+        .find(|node| node.label == "impl Action for Owner")
+        .unwrap();
+    assert_eq!(
+        owned.id.as_str(),
+        format!("{}::impl::416374696f6e", owner.id)
+    );
+    assert!(
+        methods
+            .iter()
+            .any(|node| node.id.as_str() == format!("{}::act", owner.id))
+    );
+    for implementation in implementations {
+        assert!(
+            graph
+                .edges
+                .iter()
+                .any(|edge| edge.kind == EdgeKind::Targets.as_str()
+                    && edge.from == implementation.id
+                    && edge.to == owner.id)
+        );
+        let method_edges: Vec<_> = graph
+            .edges
+            .iter()
+            .filter(|edge| {
+                edge.kind == EdgeKind::Contains.as_str()
+                    && edge.from == implementation.id
+                    && methods.iter().any(|method| method.id == edge.to)
+            })
+            .collect();
+        assert_eq!(method_edges.len(), 1);
+    }
+}
+
+#[test]
+fn reference_method_cycles_keep_the_underlying_type_owner() {
+    let fixture = WorkspaceFixture::new();
+    fixture.write_workspace_root();
+    fixture.write_package_manifest("example");
+    fixture.write_source(
+        "example",
+        "lib.rs",
+        r#"
+        pub struct Owner;
+        pub trait Action { fn act(&self); }
+        impl Action for &Owner { fn act(&self) { let _ = Owner; } }
+    "#,
+    );
+    let report = analyze_workspace(&AnalyzeOptions {
+        root: fixture.root().to_path_buf(),
+        format: OutputFormat::Json,
+        rule: Some(RuleFilter::Cycles),
+    })
+    .unwrap();
+    assert_eq!(report.findings.len(), 1);
+    assert_eq!(report.findings[0].rule_id, RuleId::ScbCycle003);
+    assert!(
+        report.findings[0]
+            .owner_ids
+            .iter()
+            .all(|owner| owner.ends_with("::Owner"))
+    );
+}