feat: catch imports of private definitions in resolution

compiler/noirc_frontend/src/ast/mod.rs

@@ -359,6 +359,7 @@ pub enum ItemVisibility {
     Public,
     Private,
     PublicCrate,
+    PublicSuper,
 }
 
 #[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,11 +243,17 @@ 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 {
-        write!(f, "mod {}", self.ident)
+        write!(
+            f,
+            "{}mod {}",
+            if self.visibility == ItemVisibility::Public { "pub " } else { "" },
+            self.ident
+        )
     }
 }
 

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

@@ -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);
@@ -280,7 +283,14 @@ impl DefCollector {
         // Resolve unresolved imports collected from the crate, one by one.
         for collected_import in def_collector.collected_imports {
             match resolve_import(crate_id, &collected_import, &context.def_maps) {
-                Ok(resolved_import) => {
+                Ok((resolved_import, warning)) => {
+                    if let Some(warning) = 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();
 
@@ -409,11 +419,12 @@ 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((module_def, ident)) = path_resolver::resolve_path(
             &context.def_maps,
             ModuleId { krate: crate_id, local_id: crate_root },
             path,
         ) {
+            assert!(ident.is_none(), "Tried to add private item to prelude");
             let module_id = module_def.as_module().expect("std::prelude should be a module");
             let prelude = context.module(module_id).scope().names();
 

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

@@ -284,7 +284,13 @@
             };
 
             // 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 @@
             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));
@@ -470,7 +482,7 @@
                         }
                     }
                     TraitItem::Type { name } => {
                         // TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()
                         if let Err((first_def, second_def)) = self.def_collector.def_map.modules
                             [trait_id.0.local_id.0]
                             .declare_type_alias(name.clone(), TypeAliasId::dummy_id())
@@ -510,7 +522,13 @@
     ) -> 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 @@
         );
 
         // 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 @@
     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 @@
                 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")]