From 1b0cc4d1f3c0d5d7673cd4416d3987699d12db5a Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 23 Sep 2024 14:35:51 -0300 Subject: [PATCH 1/5] Do not double error on import with error --- .../src/hir/def_collector/dc_crate.rs | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 6265d0e65f2..b9b94c8c9ec 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -381,6 +381,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(), @@ -410,14 +412,19 @@ 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 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, + ); + } if visibility != ItemVisibility::Private { let local_id = resolved_import.module_scope; From 828ce6e15f8eefdc4c184efe09c6ba5999e2a990 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 23 Sep 2024 14:40:15 -0300 Subject: [PATCH 2/5] Also don't suggest name for auto-import if it has errors --- .../src/hir/def_collector/dc_crate.rs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index b9b94c8c9ec..1bdfe5942d0 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -424,17 +424,17 @@ impl DefCollector { 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, - 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(); From e3996662cdefa5dd605457121786f90377f88651 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 23 Sep 2024 16:32:27 -0300 Subject: [PATCH 3/5] Add a test --- compiler/noirc_frontend/src/tests.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index dc54fd624e4..c40cc5dea51 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3748,3 +3748,26 @@ fn macro_result_type_mismatch() { CompilationError::TypeError(TypeCheckError::TypeMismatch { .. }) )); } + +#[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::ResolverError(ResolverError::PathResolutionError( + PathResolutionError::Private { .. } + )) + )); +} From d0d10519f49772cd3291de9a2727b91118ac78b4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 23 Sep 2024 17:27:02 -0300 Subject: [PATCH 4/5] Fix test --- compiler/noirc_frontend/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index c40cc5dea51..4b88f646548 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -3766,7 +3766,7 @@ fn errors_once_on_unused_import_that_is_not_accessible() { assert_eq!(errors.len(), 1); assert!(matches!( errors[0].0, - CompilationError::ResolverError(ResolverError::PathResolutionError( + CompilationError::DefinitionError(DefCollectorErrorKind::PathResolutionError( PathResolutionError::Private { .. } )) )); From 58c2cb31dc4010e8454780235513c2574a747e43 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 23 Sep 2024 17:30:30 -0300 Subject: [PATCH 5/5] Fix more tests --- .../code_action/remove_unused_import.rs | 40 +++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tooling/lsp/src/requests/code_action/remove_unused_import.rs b/tooling/lsp/src/requests/code_action/remove_unused_import.rs index c660dd57e47..f1e12d64ef5 100644 --- a/tooling/lsp/src/requests/code_action/remove_unused_import.rs +++ b/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>|||||