feat: catch imports of private definitions in resolution

compiler/noirc_frontend/src/ast/mod.rs

@@ -359,6 +359,18 @@ pub enum ItemVisibility {
     Public,
     Private,
     PublicCrate,
+    PublicSuper,
+}
+
+impl std::fmt::Display for ItemVisibility {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::Public => write!(f, "pub"),
+            Self::Private => write!(f, "priv"),
+            Self::PublicCrate => write!(f, "pub(crate)"),
+            Self::PublicSuper => write!(f, "pub(super)"),
+        }
+    }
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]

compiler/noirc_frontend/src/ast/statement.rs

@@ -5,8 +5,8 @@ use crate::lexer::token::SpannedToken;
 use crate::parser::{ParserError, ParserErrorReason};
 use crate::token::Token;
 use crate::{
-    BlockExpression, Expression, ExpressionKind, IndexExpression, MemberAccessExpression,
-    MethodCallExpression, UnresolvedType,
+    BlockExpression, Expression, ExpressionKind, IndexExpression, ItemVisibility,
+    MemberAccessExpression, MethodCallExpression, UnresolvedType,
 };
 use acvm::FieldElement;
 use iter_extended::vecmap;
@@ -243,10 +243,15 @@ pub trait Recoverable {
 #[derive(Debug, PartialEq, Eq, Clone)]
 pub struct ModuleDeclaration {
     pub ident: Ident,
+    pub visibility: ItemVisibility,
 }
 
 impl std::fmt::Display for ModuleDeclaration {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        if self.visibility != ItemVisibility::Private {
+            write!(f, "{} ", self.visibility)?;
+        };
+
         write!(f, "mod {}", self.ident)
     }
 }

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

@@ -4,7 +4,7 @@ use crate::graph::CrateId;
 use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
 use crate::hir::resolution::errors::ResolverError;
 
-use crate::hir::resolution::import::{resolve_import, ImportDirective};
+use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution};
 use crate::hir::resolution::{
     collect_impls, collect_trait_impls, path_resolver, resolve_free_functions, resolve_globals,
     resolve_impls, resolve_structs, resolve_trait_by_path, resolve_trait_impls, resolve_traits,
@@ -56,8 +56,11 @@ impl UnresolvedFunctions {
             for bound in &mut func.def.where_clause {
                 match resolve_trait_by_path(def_maps, module, bound.trait_bound.trait_path.clone())
                 {
-                    Ok(trait_id) => {
+                    Ok((trait_id, warning)) => {
                         bound.trait_bound.trait_id = Some(trait_id);
+                        if let Some(warning) = warning {
+                            errors.push(DefCollectorErrorKind::PathResolutionError(warning));
+                        }
                     }
                     Err(err) => {
                         errors.push(err);
@@ -281,6 +284,13 @@ impl DefCollector {
         for collected_import in def_collector.collected_imports {
             match resolve_import(crate_id, &collected_import, &context.def_maps) {
                 Ok(resolved_import) => {
+                    if let Some(warning) = resolved_import.warning {
+                        errors.push((
+                            DefCollectorErrorKind::PathResolutionError(warning).into(),
+                            root_file_id,
+                        ));
+                    }
+
                     // Populate module namespaces according to the imports used
                     let current_def_map = context.def_maps.get_mut(&crate_id).unwrap();
 
@@ -299,9 +309,9 @@ impl DefCollector {
                         }
                     }
                 }
-                Err((error, module_id)) => {
+                Err(error) => {
                     let current_def_map = context.def_maps.get(&crate_id).unwrap();
-                    let file_id = current_def_map.file_id(module_id);
+                    let file_id = current_def_map.file_id(collected_import.module_id);
                     let error = DefCollectorErrorKind::PathResolutionError(error);
                     errors.push((error.into(), file_id));
                 }
@@ -409,12 +419,13 @@ fn inject_prelude(
         Path { segments: segments.clone(), kind: crate::PathKind::Dep, span: Span::default() };
 
     if !crate_id.is_stdlib() {
-        if let Ok(module_def) = path_resolver::resolve_path(
+        if let Ok(PathResolution { module_def_id, warning }) = path_resolver::resolve_path(
             &context.def_maps,
             ModuleId { krate: crate_id, local_id: crate_root },
             path,
         ) {
-            let module_id = module_def.as_module().expect("std::prelude should be a module");
+            assert!(warning.is_none(), "Tried to add private item to prelude");
+            let module_id = module_def_id.as_module().expect("std::prelude should be a module");
             let prelude = context.module(module_id).scope().names();
 
             for path in prelude {

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

@@ -284,7 +284,13 @@ impl<'a> ModCollector<'a> {
             };
 
             // Create the corresponding module for the struct namespace
-            let id = match self.push_child_module(&name, self.file_id, false, false) {
+            let id = match self.push_child_module(
+                &name,
+                ItemVisibility::Public,
+                self.file_id,
+                false,
+                false,
+            ) {
                 Ok(local_id) => {
                     context.def_interner.new_struct(&unresolved, krate, local_id, self.file_id)
                 }
@@ -364,7 +370,13 @@ impl<'a> ModCollector<'a> {
             let name = trait_definition.name.clone();
 
             // Create the corresponding module for the trait namespace
-            let trait_id = match self.push_child_module(&name, self.file_id, false, false) {
+            let trait_id = match self.push_child_module(
+                &name,
+                ItemVisibility::Public,
+                self.file_id,
+                false,
+                false,
+            ) {
                 Ok(local_id) => TraitId(ModuleId { krate, local_id }),
                 Err(error) => {
                     errors.push((error.into(), self.file_id));
@@ -510,7 +522,13 @@ impl<'a> ModCollector<'a> {
     ) -> Vec<(CompilationError, FileId)> {
         let mut errors: Vec<(CompilationError, FileId)> = vec![];
         for submodule in submodules {
-            match self.push_child_module(&submodule.name, file_id, true, submodule.is_contract) {
+            match self.push_child_module(
+                &submodule.name,
+                ItemVisibility::Public,
+                file_id,
+                true,
+                submodule.is_contract,
+            ) {
                 Ok(child) => {
                     errors.extend(collect_defs(
                         self.def_collector,
@@ -593,7 +611,13 @@ impl<'a> ModCollector<'a> {
         );
 
         // Add module into def collector and get a ModuleId
-        match self.push_child_module(&mod_decl.ident, child_file_id, true, false) {
+        match self.push_child_module(
+            &mod_decl.ident,
+            mod_decl.visibility,
+            child_file_id,
+            true,
+            false,
+        ) {
             Ok(child_mod_id) => {
                 errors.extend(collect_defs(
                     self.def_collector,
@@ -617,6 +641,7 @@ impl<'a> ModCollector<'a> {
     fn push_child_module(
         &mut self,
         mod_name: &Ident,
+        visibility: ItemVisibility,
         file_id: FileId,
         add_to_parent_scope: bool,
         is_contract: bool,
@@ -644,9 +669,11 @@ impl<'a> ModCollector<'a> {
                 local_id: LocalModuleId(module_id),
             };
 
-            if let Err((first_def, second_def)) =
-                modules[self.module_id.0].declare_child_module(mod_name.to_owned(), mod_id)
-            {
+            if let Err((first_def, second_def)) = modules[self.module_id.0].declare_child_module(
+                mod_name.to_owned(),
+                visibility,
+                mod_id,
+            ) {
                 let err = DefCollectorErrorKind::Duplicate {
                     typ: DuplicateType::Module,
                     first_def,

compiler/noirc_frontend/src/hir/def_map/module_data.rs

@@ -105,9 +105,10 @@ impl ModuleData {
     pub fn declare_child_module(
         &mut self,
         name: Ident,
+        visibility: ItemVisibility,
         child_id: ModuleId,
     ) -> Result<(), (Ident, Ident)> {
-        self.declare(name, ItemVisibility::Public, child_id.into(), None)
+        self.declare(name, visibility, child_id.into(), None)
     }
 
     pub fn find_func_with_name(&self, name: &Ident) -> Option<FuncId> {

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

@@ -25,7 +25,7 @@ pub enum ResolverError {
     #[error("path is not an identifier")]
     PathIsNotIdent { span: Span },
     #[error("could not resolve path")]
-    PathResolutionError(PathResolutionError),
+    PathResolutionError(#[from] PathResolutionError),
     #[error("Expected")]
     Expected { span: Span, expected: String, got: String },
     #[error("Duplicate field in constructor")]