feat: warn on unused functions

compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs

@@ -7,6 +7,7 @@ use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
 use crate::hir::resolution::path_resolver;
 use crate::hir::type_check::TypeCheckError;
+use crate::usage_tracker::UnusedItem;
 use crate::{Generics, Type};
 
 use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution};
@@ -253,7 +254,7 @@ impl DefCollector {
         root_file_id: FileId,
         debug_comptime_in_file: Option<&str>,
         enable_arithmetic_generics: bool,
-        error_on_usage_tracker: bool,
+        error_on_unused_items: bool,
         macro_processors: &[&dyn MacroProcessor],
     ) -> Vec<(CompilationError, FileId)> {
         let mut errors: Vec<(CompilationError, FileId)> = vec![];
@@ -388,20 +389,14 @@ impl DefCollector {
                         let result = current_def_map.modules[resolved_import.module_scope.0]
                             .import(name.clone(), visibility, module_def_id, is_prelude);
 
-                        // Empty spans could come from implicitly injected imports, and we don't want to track those
-                        if visibility != ItemVisibility::Public
-                            && name.span().start() < name.span().end()
-                        {
-                            let module_id = ModuleId {
-                                krate: crate_id,
-                                local_id: resolved_import.module_scope,
-                            };
-
-                            context
-                                .def_interner
-                                .usage_tracker
-                                .add_unused_import(module_id, name.clone());
-                        }
+                        let module_id =
+                            ModuleId { krate: crate_id, local_id: resolved_import.module_scope };
+                        context.def_interner.usage_tracker.add_unused_item(
+                            module_id,
+                            name.clone(),
+                            UnusedItem::Import,
+                            visibility,
+                        );
 
                         if visibility != ItemVisibility::Private {
                             let local_id = resolved_import.module_scope;
@@ -476,26 +471,34 @@ impl DefCollector {
             );
         }
 
-        if error_on_usage_tracker {
-            Self::check_usage_tracker(context, crate_id, &mut errors);
+        if error_on_unused_items {
+            Self::check_unused_items(context, crate_id, &mut errors);
         }
 
         errors
     }
 
-    fn check_usage_tracker(
-        context: &Context,
+    fn check_unused_items(
+        context: &mut Context,
         crate_id: CrateId,
         errors: &mut Vec<(CompilationError, FileId)>,
     ) {
-        let unused_imports = context.def_interner.usage_tracker.unused_imports().iter();
+        // Assume `main` is used so we don't warn on that
+        let root = ModuleId { krate: crate_id, local_id: context.def_maps[&crate_id].root };
+        let main = Ident::new("main".to_string(), Span::default());
+        context.def_interner.usage_tracker.mark_as_used(root, &main);
+
+        let unused_imports = context.def_interner.usage_tracker.unused_items().iter();
         let unused_imports = unused_imports.filter(|(module_id, _)| module_id.krate == crate_id);
 
         errors.extend(unused_imports.flat_map(|(module_id, usage_tracker)| {
             let module = &context.def_maps[&crate_id].modules()[module_id.local_id.0];
-            usage_tracker.iter().map(|ident| {
+            usage_tracker.iter().map(|(ident, unused_item)| {
                 let ident = ident.clone();
-                let error = CompilationError::ResolverError(ResolverError::UnusedImport { ident });
+                let error = CompilationError::ResolverError(ResolverError::UnusedItem {
+                    ident,
+                    item_type: unused_item.item_type().to_string(),
+                });
                 (error, module.location.file)
             })
         }));

compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

@@ -17,6 +17,7 @@ use crate::ast::{
 use crate::hir::resolution::errors::ResolverError;
 use crate::macros_api::{Expression, NodeInterner, UnresolvedType, UnresolvedTypeData};
 use crate::node_interner::ModuleAttributes;
+use crate::usage_tracker::UnusedItem;
 use crate::{
     graph::CrateId,
     hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
@@ -246,6 +247,8 @@ impl<'a> ModCollector<'a> {
                 context.def_interner.register_function(func_id, &function.def);
             }
 
+            let is_test = function.def.attributes.is_test_function();
+
             // Now link this func_id to a crate level map with the noir function and the module id
             // Encountering a NoirFunction, we retrieve it's module_data to get the namespace
             // Once we have lowered it to a HirFunction, we retrieve it's Id from the DefInterner
@@ -255,8 +258,17 @@ impl<'a> ModCollector<'a> {
             unresolved_functions.push_fn(self.module_id, func_id, function);
 
             // Add function to scope/ns of the module
-            let result = self.def_collector.def_map.modules[self.module_id.0]
-                .declare_function(name, visibility, func_id);
+            let module_data = &mut self.def_collector.def_map.modules[self.module_id.0];
+            let result = module_data.declare_function(name.clone(), visibility, func_id);
+
+            if !is_test {
+                let module_id = ModuleId { krate, local_id: self.module_id };
+                let item = UnusedItem::Function(func_id);
+                context
+                    .def_interner
+                    .usage_tracker
+                    .add_unused_item(module_id, name, item, visibility);
+            }
 
             if let Err((first_def, second_def)) = result {
                 let error = DefCollectorErrorKind::Duplicate {

compiler/noirc_frontend/src/hir/resolution/errors.rs

@@ -20,8 +20,8 @@ pub enum ResolverError {
     DuplicateDefinition { name: String, first_span: Span, second_span: Span },
     #[error("Unused variable")]
     UnusedVariable { ident: Ident },
-    #[error("Unused import")]
-    UnusedImport { ident: Ident },
+    #[error("Unused {item_type}")]
+    UnusedItem { ident: Ident, item_type: String },
     #[error("Could not find variable in this scope")]
     VariableNotDeclared { name: String, span: Span },
     #[error("path is not an identifier")]
@@ -158,12 +158,12 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
                 diagnostic.unnecessary = true;
                 diagnostic
             }
-            ResolverError::UnusedImport { ident } => {
+            ResolverError::UnusedItem { ident, item_type } => {
                 let name = &ident.0.contents;
 
                 let mut diagnostic = Diagnostic::simple_warning(
-                    format!("unused import {name}"),
-                    "unused import ".to_string(),
+                    format!("unused {item_type} {name}"),
+                    format!("unused {item_type}"),
                     ident.span(),
                 );
                 diagnostic.unnecessary = true;

compiler/noirc_frontend/src/node_interner.rs

@@ -655,7 +655,7 @@ impl Default for NodeInterner {
             auto_import_names: HashMap::default(),
             comptime_scopes: vec![HashMap::default()],
             trait_impl_associated_types: HashMap::default(),
-            usage_tracker: UsageTracker::default(),
+            usage_tracker: UsageTracker::new(),
         }
     }
 }

compiler/noirc_frontend/src/tests.rs

@@ -1362,7 +1362,7 @@ fn ban_mutable_globals() {
 fn deny_inline_attribute_on_unconstrained() {
     let src = r#"
         #[no_predicates]
-        unconstrained fn foo(x: Field, y: Field) {
+        unconstrained pub fn foo(x: Field, y: Field) {
             assert(x != y);
         }
     "#;
@@ -1378,7 +1378,7 @@ fn deny_inline_attribute_on_unconstrained() {
 fn deny_fold_attribute_on_unconstrained() {
     let src = r#"
         #[fold]
-        unconstrained fn foo(x: Field, y: Field) {
+        unconstrained pub fn foo(x: Field, y: Field) {
             assert(x != y);
         }
     "#;
@@ -1535,7 +1535,7 @@ fn struct_numeric_generic_in_function() {
         inner: u64
     }
 
-    fn bar<let N: Foo>() { }
+    pub fn bar<let N: Foo>() { }
     "#;
     let errors = get_program_errors(src);
     assert_eq!(errors.len(), 1);
@@ -1567,7 +1567,7 @@ fn struct_numeric_generic_in_struct() {
 #[test]
 fn bool_numeric_generic() {
     let src = r#"
-    fn read<let N: bool>() -> Field {
+    pub fn read<let N: bool>() -> Field {
         if N {
             0
         } else {
@@ -1586,7 +1586,7 @@ fn bool_numeric_generic() {
 #[test]
 fn numeric_generic_binary_operation_type_mismatch() {
     let src = r#"
-    fn foo<let N: Field>() -> bool {
+    pub fn foo<let N: Field>() -> bool {
         let mut check: bool = true;
         check = N;
         check
@@ -1603,7 +1603,7 @@ fn numeric_generic_binary_operation_type_mismatch() {
 #[test]
 fn bool_generic_as_loop_bound() {
     let src = r#"
-    fn read<let N: bool>() {
+    pub fn read<let N: bool>() {
         let mut fields = [0; N];
         for i in 0..N {
             fields[i] = i + 1;
@@ -1633,7 +1633,7 @@ fn bool_generic_as_loop_bound() {
 #[test]
 fn numeric_generic_in_function_signature() {
     let src = r#"
-    fn foo<let N: u8>(arr: [Field; N]) -> [Field; N] { arr }
+    pub fn foo<let N: u8>(arr: [Field; N]) -> [Field; N] { arr }
     "#;
     assert_no_errors(src);
 }
@@ -1675,7 +1675,7 @@ fn normal_generic_as_array_length() {
 #[test]
 fn numeric_generic_as_param_type() {
     let src = r#"
-    fn foo<let I: Field>(x: I) -> I {
+    pub fn foo<let I: Field>(x: I) -> I {
         let _q: I = 5;
         x
     }
@@ -1814,7 +1814,7 @@ fn numeric_generic_used_in_where_clause() {
         fn deserialize(fields: [Field; N]) -> Self;
     }
 
-    fn read<T, let N: u32>() -> T where T: Deserialize<N> {
+    pub fn read<T, let N: u32>() -> T where T: Deserialize<N> {
         let mut fields: [Field; N] = [0; N];
         for i in 0..N {
             fields[i] = i as Field + 1;
@@ -1828,12 +1828,12 @@ fn numeric_generic_used_in_where_clause() {
 #[test]
 fn numeric_generic_used_in_turbofish() {
     let src = r#"
-    fn double<let N: u32>() -> u32 {
+    pub fn double<let N: u32>() -> u32 {
         // Used as an expression
         N * 2
     }
 
-    fn double_numeric_generics_test() {
+    pub fn double_numeric_generics_test() {
         // Example usage of a numeric generic arguments.
         assert(double::<9>() == 18);
         assert(double::<7 + 8>() == 30);
@@ -1869,7 +1869,7 @@ fn normal_generic_used_when_numeric_expected_in_where_clause() {
         fn deserialize(fields: [Field; N]) -> Self;
     }
 
-    fn read<T, N>() -> T where T: Deserialize<N> {
+    pub fn read<T, N>() -> T where T: Deserialize<N> {
         T::deserialize([0, 1])
     }
     "#;
@@ -1885,7 +1885,7 @@ fn normal_generic_used_when_numeric_expected_in_where_clause() {
         fn deserialize(fields: [Field; N]) -> Self;
     }
 
-    fn read<T, N>() -> T where T: Deserialize<N> {
+    pub fn read<T, N>() -> T where T: Deserialize<N> {
         let mut fields: [Field; N] = [0; N];
         for i in 0..N {
             fields[i] = i as Field + 1;
@@ -2431,7 +2431,7 @@ fn use_super() {
     mod foo {
         use super::some_func;
 
-        fn bar() {
+        pub fn bar() {
             some_func();
         }
     }
@@ -2445,7 +2445,7 @@ fn use_super_in_path() {
     fn some_func() {}
 
     mod foo {
-        fn func() {
+        pub fn func() {
             super::some_func();
         }
     }
@@ -2736,7 +2736,7 @@ fn trait_constraint_on_tuple_type() {
             fn foo(self, x: A) -> bool;
         }
 
-        fn bar<T, U, V>(x: (T, U), y: V) -> bool where (T, U): Foo<V> {
+        pub fn bar<T, U, V>(x: (T, U), y: V) -> bool where (T, U): Foo<V> {
             x.foo(y)
         }
 
@@ -3072,7 +3072,7 @@ fn trait_impl_for_a_type_that_implements_another_trait() {
         }
     }
 
-    fn use_it<T>(t: T) -> i32 where T: Two {
+    pub fn use_it<T>(t: T) -> i32 where T: Two {
         Two::two(t)
     }
 
@@ -3112,7 +3112,7 @@ fn trait_impl_for_a_type_that_implements_another_trait_with_another_impl_used()
         }
     }
 
-    fn use_it(t: u32) -> i32 {
+    pub fn use_it(t: u32) -> i32 {
         Two::two(t)
     }
 
@@ -3224,12 +3224,14 @@ fn errors_on_unused_private_import() {
     let errors = get_program_errors(src);
     assert_eq!(errors.len(), 1);
 
-    let CompilationError::ResolverError(ResolverError::UnusedImport { ident }) = &errors[0].0
+    let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
+        &errors[0].0
     else {
-        panic!("Expected an unused import error");
+        panic!("Expected an unused item error");
     };
 
     assert_eq!(ident.to_string(), "bar");
+    assert_eq!(item_type, "import");
 }
 
 #[test]
@@ -3258,12 +3260,14 @@ fn errors_on_unused_pub_crate_import() {
     let errors = get_program_errors(src);
     assert_eq!(errors.len(), 1);
 
-    let CompilationError::ResolverError(ResolverError::UnusedImport { ident }) = &errors[0].0
+    let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
+        &errors[0].0
     else {
-        panic!("Expected an unused import error");
+        panic!("Expected an unused item error");
     };
 
     assert_eq!(ident.to_string(), "bar");
+    assert_eq!(item_type, "import");
 }
 
 #[test]
@@ -3276,7 +3280,7 @@ fn warns_on_use_of_private_exported_item() {
 
         use bar::baz;
 
-        fn qux() {
+        pub fn qux() {
             baz();
         }
     }
@@ -3341,3 +3345,26 @@ fn warns_on_re_export_of_item_with_less_visibility() {
         )
     ));
 }
+
+#[test]
+fn errors_on_unused_function() {
+    let src = r#"
+    fn foo() {
+        bar();
+    }
+
+    fn bar() {}
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::UnusedItem { ident, item_type }) =
+        &errors[0].0
+    else {
+        panic!("Expected an unused item error");
+    };
+
+    assert_eq!(ident.to_string(), "foo");
+    assert_eq!(item_type, "function");
+}