feat: do not double error on import with error (noir-lang#6131)

# Description

## Problem

In a code like this:

```noir
mod moo {
    struct Foo {}
}

use moo::Foo;

fn main() {}
```

you'd previously get two errors:
1. `moo::Foo` is private
2. `moo::Foo` is unused

There's no need to give two errors here: saying that `moo::Foo` is
private is enough and it doesn't matter that it's used or not because
it's not even accessible.

## Summary

This PR makes it so that only one error is given in this case.

## Additional Context

None.

## 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.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>

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

@@ -372,6 +372,8 @@ impl DefCollector {
                     let current_def_map = context.def_maps.get_mut(&crate_id).unwrap();
                     let file_id = current_def_map.file_id(module_id);
 
+                    let has_path_resolution_error = resolved_import.error.is_some();
+
                     if let Some(error) = resolved_import.error {
                         errors.push((
                             DefCollectorErrorKind::PathResolutionError(error).into(),
@@ -401,24 +403,29 @@ impl DefCollector {
                         let result = current_def_map.modules[resolved_import.module_scope.0]
                             .import(name.clone(), visibility, module_def_id, is_prelude);
 
-                        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;
-                            let defining_module = ModuleId { krate: crate_id, local_id };
-                            context.def_interner.register_name_for_auto_import(
-                                name.to_string(),
-                                module_def_id,
+                        // If we error on path resolution don't also say it's unused (in case it ends up being unused)
+                        if !has_path_resolution_error {
+                            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,
-                                Some(defining_module),
                             );
+
+                            if visibility != ItemVisibility::Private {
+                                let local_id = resolved_import.module_scope;
+                                let defining_module = ModuleId { krate: crate_id, local_id };
+                                context.def_interner.register_name_for_auto_import(
+                                    name.to_string(),
+                                    module_def_id,
+                                    visibility,
+                                    Some(defining_module),
+                                );
+                            }
                         }
 
                         let last_segment = collected_import.path.last_ident();

compiler/noirc_frontend/src/tests.rs

@@ -3038,3 +3038,26 @@ fn use_numeric_generic_in_trait_method() {
     println!("{errors:?}");
     assert_eq!(errors.len(), 0);
 }
+
+#[test]
+fn errors_once_on_unused_import_that_is_not_accessible() {
+    // Tests that we don't get an "unused import" here given that the import is not accessible
+    let src = r#"
+        mod moo {
+            struct Foo {}
+        }
+
+        use moo::Foo;
+
+        fn main() {}
+    "#;
+
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+    assert!(matches!(
+        errors[0].0,
+        CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError(
+            PathResolutionError::Private { .. }
+        ))
+    ));
+}

tooling/lsp/src/requests/code_action/remove_unused_import.rs

@@ -163,7 +163,7 @@ mod tests {
 
         let src = r#"
         mod moo {
-            fn foo() {}
+            pub fn foo() {}
         }
         use moo::fo>|<o;
 
@@ -173,7 +173,7 @@ mod tests {
 
         let expected = r#"
         mod moo {
-            fn foo() {}
+            pub fn foo() {}
         }
         
 
@@ -190,7 +190,7 @@ mod tests {
 
         let src = r#"
         mod moo {
-            fn foo() {}
+            pub fn foo() {}
         }
 
         mod qux {
@@ -203,7 +203,7 @@ mod tests {
 
         let expected = r#"
         mod moo {
-            fn foo() {}
+            pub fn foo() {}
         }
 
         mod qux {
@@ -223,8 +223,8 @@ mod tests {
 
         let src = r#"
         mod moo {
-            fn foo() {}
-            fn bar() {}
+            pub fn foo() {}
+            pub fn bar() {}
         }
         use moo::{fo>|<o, bar};
 
@@ -235,8 +235,8 @@ mod tests {
 
         let expected = r#"
         mod moo {
-            fn foo() {}
-            fn bar() {}
+            pub fn foo() {}
+            pub fn bar() {}
         }
         use moo::bar;
 
@@ -254,9 +254,9 @@ mod tests {
 
         let src = r#"
         mod moo {
-            fn foo() {}
-            fn bar() {}
-            fn baz() {}
+            pub fn foo() {}
+            pub fn bar() {}
+            pub fn baz() {}
         }
         use moo::{fo>|<o, bar, baz};
 
@@ -267,9 +267,9 @@ mod tests {
 
         let expected = r#"
         mod moo {
-            fn foo() {}
-            fn bar() {}
-            fn baz() {}
+            pub fn foo() {}
+            pub fn bar() {}
+            pub fn baz() {}
         }
         use moo::bar;
 
@@ -287,10 +287,10 @@ mod tests {
 
         let src = r#"
         mod moo {
-            fn foo() {}
-            fn bar() {}
+            pub fn foo() {}
+            pub fn bar() {}
         }
-        pub use moo::{fo>|<o, bar};
+        pub(crate) use moo::{fo>|<o, bar};
 
         fn main() {
             bar();
@@ -299,10 +299,10 @@ mod tests {
 
         let expected = r#"
         mod moo {
-            fn foo() {}
-            fn bar() {}
+            pub fn foo() {}
+            pub fn bar() {}
         }
-        pub use moo::bar;
+        pub(crate) use moo::bar;
 
         fn main() {
             bar();