refactor: Account for idents in function signatures in reorganize_definitions

Previously only idents used in the function body were accounted for, which led to errors if there was a type referenced in the signature but not the body.

Author: randomPoison

c2rust-refactor/src/transform/reorganize_definitions.rs

@@ -247,30 +247,23 @@ impl<'a, 'tcx> Reorganizer<'a, 'tcx> {
         fn keep_items(items: &[P<Item>]) -> HashSet<NodeId> {
             let mut keep_items = HashSet::new();
             let mut used_idents = HashSet::new();
+            let mut collect_used_idents = |item: &Item| {
+                visit_nodes(item, |path: &Path| {
+                    if let [segment] = &path.segments[..] {
+                        used_idents.insert(segment.ident);
+                    }
+                });
+            };
             for item in &items[..] {
                 match &item.kind {
-                    ItemKind::Fn(box Fn {
-                        body: Some(ref body),
-                        ..
-                    }) => {
+                    ItemKind::Fn(box Fn { body: Some(_), .. }) => {
                         keep_items.insert(item.id);
-                        visit_nodes(&**body, |path: &Path| {
-                            if let [segment] = &path.segments[..] {
-                                used_idents.insert(segment.ident);
-                            }
-                        });
+                        collect_used_idents(item);
                     }
 
-                    ItemKind::Static(_, _, init) if !is_exported(item) => {
+                    ItemKind::Static(_, _, _) if !is_exported(item) => {
                         keep_items.insert(item.id);
-
-                        if let Some(init) = init {
-                            visit_nodes(&**init, |path: &Path| {
-                                if let [segment] = &path.segments[..] {
-                                    used_idents.insert(segment.ident);
-                                }
-                            });
-                        }
+                        collect_used_idents(item);
                     }
 
                     _ => {}

c2rust-refactor/tests/snapshots.rs

@@ -459,6 +459,13 @@ fn test_reorganize_foreign_types() {
         .test();
 }
 
+#[test]
+fn test_reorganize_forward_decl_with_local_definition() {
+    refactor("reorganize_definitions")
+        .named("reorganize_forward_decl_with_local_definition.rs")
+        .test();
+}
+
 #[test]
 fn test_sink_lets() {
     refactor("sink_lets").test();

c2rust-refactor/tests/snapshots/reorganize_forward_decl_with_local_definition.rs

@@ -0,0 +1,72 @@
+#![feature(extern_types)]
+#![feature(register_tool)]
+#![register_tool(c2rust)]
+#![allow(dead_code)]
+#![allow(non_camel_case_types)]
+
+pub mod repository {
+    #[c2rust::header_src = "attrcache.h:1"]
+    pub mod attrcache_h {
+        extern "C" {
+            #[c2rust::src_loc = "1:1"]
+            pub type git_attr_cache;
+        }
+    }
+
+    #[c2rust::header_src = "repository.h:2"]
+    pub mod repository_h {
+        pub use super::attrcache_h::git_attr_cache;
+
+        #[derive(Copy, Clone)]
+        #[repr(C)]
+        #[c2rust::src_loc = "2:1"]
+        pub struct git_repository {
+            pub attrcache: *mut git_attr_cache,
+        }
+    }
+
+    use repository_h::git_repository;
+}
+
+pub mod attrcache {
+    #[c2rust::header_src = "attrcache.h:1"]
+    pub mod attrcache_h {
+        #[c2rust::src_loc = "1:1"]
+        pub const GIT_ATTR_CONFIG: i32 = 0;
+    }
+
+    #[c2rust::header_src = "repository.h:2"]
+    pub mod repository_h {
+        use super::git_attr_cache;
+
+        #[derive(Copy, Clone)]
+        #[repr(C)]
+        #[c2rust::src_loc = "2:1"]
+        pub struct git_repository {
+            pub attrcache: *mut git_attr_cache,
+        }
+
+        #[c2rust::src_loc = "3:1"]
+        pub unsafe extern "C" fn git_repository_attr_cache(
+            repo: *mut git_repository,
+        ) -> *mut git_attr_cache {
+            (*repo).attrcache
+        }
+    }
+
+    use repository_h::git_repository;
+
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct git_attr_cache {
+        pub x: i32,
+    }
+
+    unsafe extern "C" fn attrcache_source_function(
+        repo: *mut git_repository,
+    ) -> *mut git_attr_cache {
+        repository_h::git_repository_attr_cache(repo)
+    }
+}
+
+fn main() {}

c2rust-refactor/tests/snapshots/snapshots__refactor-reorganize_definitions-reorganize_forward_decl_with_local_definition.rs.snap

@@ -0,0 +1,52 @@
+---
+source: c2rust-refactor/tests/snapshots.rs
+assertion_line: 212
+expression: c2rust-refactor reorganize_definitions --rewrite-mode alongside -- tests/snapshots/reorganize_forward_decl_with_local_definition.rs --edition 2021
+---
+#![feature(extern_types)]
+#![feature(register_tool)]
+#![register_tool(c2rust)]
+#![allow(dead_code)]
+#![allow(non_camel_case_types)]
+
+pub mod repository {
+
+    // =============== BEGIN repository_h ================
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct git_repository {
+        pub attrcache: *mut crate::attrcache::git_attr_cache,
+    }
+}
+
+pub mod attrcache {
+
+    // =============== BEGIN attrcache_h ================
+    pub const GIT_ATTR_CONFIG: i32 = 0;
+
+    pub mod repository_h {
+        use crate::attrcache::git_attr_cache;
+
+        pub unsafe extern "C" fn git_repository_attr_cache(
+            repo: *mut crate::repository::git_repository,
+        ) -> *mut git_attr_cache {
+            (*repo).attrcache
+        }
+    }
+
+    use crate::repository::git_repository;
+
+    #[derive(Copy, Clone)]
+    #[repr(C)]
+    pub struct git_attr_cache {
+        pub x: i32,
+    }
+
+    unsafe extern "C" fn attrcache_source_function(
+        repo: *mut crate::repository::git_repository,
+    ) -> *mut git_attr_cache {
+        repository_h::git_repository_attr_cache(repo)
+    }
+}
+
+fn main() {}