feat: warn on unused functions (noir-lang#5892)

# Description

## Problem

Now that we warn on unused imports, doing that for functions too is
relatively straight-forward.

## Summary

Now unused private or `pub(crate)` functions will be reported as unused.

## Additional Context

I'd like to try this on some Aztec-Packages projects, but for that it
would be nice to merge noir-lang#5895 first so I could get all warnings/errors
for a package inside VS Code.

We can eventually do the same thing with globals, traits, etc., once we
track their visibility.

I also think we could warn on unused `pub` functions in a "bin" or
"contract" package, not sure... but if we decide to do that, it could be
a separate PR.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

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

@@ -8,6 +8,7 @@ use crate::hir::resolution::errors::ResolverError;
 use crate::hir::resolution::path_resolver;
 use crate::hir::type_check::TypeCheckError;
 use crate::token::SecondaryAttribute;
+use crate::usage_tracker::UnusedItem;
 use crate::{Generics, Type};
 
 use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution};
@@ -271,7 +272,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![];
@@ -406,20 +407,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;
@@ -494,26 +489,29 @@ 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(
+    fn check_unused_items(
         context: &Context,
         crate_id: CrateId,
         errors: &mut Vec<(CompilationError, FileId)>,
     ) {
-        let unused_imports = context.def_interner.usage_tracker.unused_imports().iter();
+        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(),
+                });
                 (error, module.location.file)
             })
         }));

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

@@ -18,6 +18,7 @@ use crate::hir::resolution::errors::ResolverError;
 use crate::macros_api::{Expression, NodeInterner, StructId, UnresolvedType, UnresolvedTypeData};
 use crate::node_interner::ModuleAttributes;
 use crate::token::SecondaryAttribute;
+use crate::usage_tracker::UnusedItem;
 use crate::{
     graph::CrateId,
     hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait},
@@ -36,7 +37,7 @@ use super::{
     },
     errors::{DefCollectorErrorKind, DuplicateType},
 };
-use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData, ModuleId};
+use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleData, ModuleId, MAIN_FUNCTION};
 use crate::hir::resolution::import::ImportDirective;
 use crate::hir::Context;
 
@@ -833,6 +834,16 @@ pub fn collect_function(
             return None;
         }
     }
+
+    let module_data = &mut def_map.modules[module.local_id.0];
+
+    let is_test = function.def.attributes.is_test_function();
+    let is_entry_point_function = if module_data.is_contract {
+        function.attributes().is_contract_entry_point()
+    } else {
+        function.name() == MAIN_FUNCTION
+    };
+
     let name = function.name_ident().clone();
     let func_id = interner.push_empty_fn();
     let visibility = function.def.visibility;
@@ -841,6 +852,13 @@ pub fn collect_function(
     if interner.is_in_lsp_mode() && !function.def.is_test() {
         interner.register_function(func_id, &function.def);
     }
+
+    if !is_test && !is_entry_point_function {
+        let item = UnusedItem::Function(func_id);
+        interner.usage_tracker.add_unused_item(module, name.clone(), item, visibility);
+    }
+
+    // Add function to scope/ns of the module
     let result = def_map.modules[module.local_id.0].declare_function(name, visibility, func_id);
     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: &'static str },
     #[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(),
         }
     }
 }