fix: don't warn twice when referring to private item

compiler/noirc_frontend/src/elaborator/patterns.rs

@@ -12,7 +12,7 @@ use crate::{
         type_check::{Source, TypeCheckError},
     },
     hir_def::{
-        expr::{HirExpression, HirIdent, HirMethodReference, ImplKind},
+        expr::{HirExpression, HirIdent, HirMethodReference, ImplKind, TraitMethod},
         stmt::HirPattern,
     },
     node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, TraitImplKind},
@@ -525,11 +525,15 @@ impl<'context> Elaborator<'context> {
     }
 
     fn resolve_variable(&mut self, path: Path) -> HirIdent {
-        if let Some((method, constraint, assumed)) = self.resolve_trait_generic_path(&path) {
+        if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) {
+            if let Some(error) = trait_path_resolution.error {
+                self.push_err(error);
+            }
+
             HirIdent {
                 location: Location::new(path.span, self.file),
-                id: self.interner.trait_method_id(method),
-                impl_kind: ImplKind::TraitMethod(method, constraint, assumed),
+                id: self.interner.trait_method_id(trait_path_resolution.method.method_id),
+                impl_kind: ImplKind::TraitMethod(trait_path_resolution.method),
             }
         } else {
             // If the Path is being used as an Expression, then it is referring to a global from a separate module
@@ -595,8 +599,12 @@ impl<'context> Elaborator<'context> {
         // We need to do this first since otherwise instantiating the type below
         // will replace each trait generic with a fresh type variable, rather than
         // the type used in the trait constraint (if it exists). See #4088.
-        if let ImplKind::TraitMethod(_, constraint, assumed) = &ident.impl_kind {
-            self.bind_generics_from_trait_constraint(constraint, *assumed, &mut bindings);
+        if let ImplKind::TraitMethod(method) = &ident.impl_kind {
+            self.bind_generics_from_trait_constraint(
+                &method.constraint,
+                method.assumed,
+                &mut bindings,
+            );
         }
 
         // An identifiers type may be forall-quantified in the case of generic functions.
@@ -634,18 +642,18 @@ impl<'context> Elaborator<'context> {
             }
         }
 
-        if let ImplKind::TraitMethod(_, mut constraint, assumed) = ident.impl_kind {
-            constraint.apply_bindings(&bindings);
-            if assumed {
-                let trait_generics = constraint.trait_generics.clone();
-                let object_type = constraint.typ;
+        if let ImplKind::TraitMethod(mut method) = ident.impl_kind {
+            method.constraint.apply_bindings(&bindings);
+            if method.assumed {
+                let trait_generics = method.constraint.trait_generics.clone();
+                let object_type = method.constraint.typ;
                 let trait_impl = TraitImplKind::Assumed { object_type, trait_generics };
                 self.interner.select_impl_for_expression(expr_id, trait_impl);
             } else {
                 // Currently only one impl can be selected per expr_id, so this
                 // constraint needs to be pushed after any other constraints so
                 // that monomorphization can resolve this trait method to the correct impl.
-                self.push_trait_constraint(constraint, expr_id);
+                self.push_trait_constraint(method.constraint, expr_id);
             }
         }
 
@@ -731,7 +739,7 @@ impl<'context> Elaborator<'context> {
                 let mut constraint =
                     self.interner.get_trait(method_id.trait_id).as_constraint(span);
                 constraint.trait_generics = generics;
-                ImplKind::TraitMethod(method_id, constraint, false)
+                ImplKind::TraitMethod(TraitMethod { method_id, constraint, assumed: false })
             }
         };
 

compiler/noirc_frontend/src/elaborator/types.rs

@@ -15,7 +15,7 @@ use crate::{
         comptime::{Interpreter, Value},
         def_collector::dc_crate::CompilationError,
         def_map::ModuleDefId,
-        resolution::errors::ResolverError,
+        resolution::{errors::ResolverError, import::PathResolutionError},
         type_check::{
             generics::{Generic, TraitGenerics},
             NoMatchingImplFoundError, Source, TypeCheckError,
@@ -24,15 +24,15 @@ use crate::{
     hir_def::{
         expr::{
             HirBinaryOp, HirCallExpression, HirExpression, HirLiteral, HirMemberAccess,
-            HirMethodReference, HirPrefixExpression,
+            HirMethodReference, HirPrefixExpression, TraitMethod,
         },
         function::{FuncMeta, Parameters},
         stmt::HirStatement,
         traits::{NamedType, TraitConstraint},
     },
     node_interner::{
-        DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, ImplSearchErrorKind, NodeInterner,
-        TraitId, TraitImplKind, TraitMethodId,
+        DefinitionKind, DependencyId, ExprId, GlobalId, ImplSearchErrorKind, NodeInterner, TraitId,
+        TraitImplKind, TraitMethodId,
     },
     token::SecondaryAttribute,
     Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, TypeVariable,
@@ -44,6 +44,11 @@ use super::{lints, Elaborator};
 pub const SELF_TYPE_NAME: &str = "Self";
 pub const WILDCARD_TYPE: &str = "_";
 
+pub(super) struct TraitPathResolution {
+    pub(super) method: TraitMethod,
+    pub(super) error: Option<PathResolutionError>,
+}
+
 impl<'context> Elaborator<'context> {
     /// Translates an UnresolvedType to a Type with a `TypeKind::Normal`
     pub(crate) fn resolve_type(&mut self, typ: UnresolvedType) -> Type {
@@ -522,10 +527,7 @@ impl<'context> Elaborator<'context> {
     //
     // Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not
     // E.g. `t.method()` with `where T: Foo<Bar>` in scope will return `(Foo::method, T, vec![Bar])`
-    fn resolve_trait_static_method_by_self(
-        &mut self,
-        path: &Path,
-    ) -> Option<(TraitMethodId, TraitConstraint, bool)> {
+    fn resolve_trait_static_method_by_self(&mut self, path: &Path) -> Option<TraitPathResolution> {
         let trait_impl = self.current_trait_impl?;
         let trait_id = self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id;
 
@@ -537,7 +539,10 @@ impl<'context> Elaborator<'context> {
                 let the_trait = self.interner.get_trait(trait_id);
                 let method = the_trait.find_method(method.0.contents.as_str())?;
                 let constraint = the_trait.as_constraint(path.span);
-                return Some((method, constraint, true));
+                return Some(TraitPathResolution {
+                    method: TraitMethod { method_id: method, constraint, assumed: true },
+                    error: None,
+                });
             }
         }
         None
@@ -547,16 +552,18 @@ impl<'context> Elaborator<'context> {
     //
     // Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not
     // E.g. `t.method()` with `where T: Foo<Bar>` in scope will return `(Foo::method, T, vec![Bar])`
-    fn resolve_trait_static_method(
-        &mut self,
-        path: &Path,
-    ) -> Option<(TraitMethodId, TraitConstraint, bool)> {
-        let func_id: FuncId = self.lookup(path.clone()).ok()?;
+    fn resolve_trait_static_method(&mut self, path: &Path) -> Option<TraitPathResolution> {
+        let path_resolution = self.resolve_path(path.clone()).ok()?;
+        let ModuleDefId::FunctionId(func_id) = path_resolution.module_def_id else { return None };
+
         let meta = self.interner.function_meta(&func_id);
         let the_trait = self.interner.get_trait(meta.trait_id?);
         let method = the_trait.find_method(path.last_name())?;
         let constraint = the_trait.as_constraint(path.span);
-        Some((method, constraint, false))
+        Some(TraitPathResolution {
+            method: TraitMethod { method_id: method, constraint, assumed: false },
+            error: path_resolution.error,
+        })
     }
 
     // This resolves a static trait method T::trait_method by iterating over the where clause
@@ -567,7 +574,7 @@ impl<'context> Elaborator<'context> {
     fn resolve_trait_method_by_named_generic(
         &mut self,
         path: &Path,
-    ) -> Option<(TraitMethodId, TraitConstraint, bool)> {
+    ) -> Option<TraitPathResolution> {
         if path.segments.len() != 2 {
             return None;
         }
@@ -581,7 +588,10 @@ impl<'context> Elaborator<'context> {
 
                 let the_trait = self.interner.get_trait(constraint.trait_id);
                 if let Some(method) = the_trait.find_method(path.last_name()) {
-                    return Some((method, constraint, true));
+                    return Some(TraitPathResolution {
+                        method: TraitMethod { method_id: method, constraint, assumed: true },
+                        error: None,
+                    });
                 }
             }
         }
@@ -595,7 +605,7 @@ impl<'context> Elaborator<'context> {
     pub(super) fn resolve_trait_generic_path(
         &mut self,
         path: &Path,
-    ) -> Option<(TraitMethodId, TraitConstraint, bool)> {
+    ) -> Option<TraitPathResolution> {
         self.resolve_trait_static_method_by_self(path)
             .or_else(|| self.resolve_trait_static_method(path))
             .or_else(|| self.resolve_trait_method_by_named_generic(path))

compiler/noirc_frontend/src/hir/comptime/interpreter.rs

@@ -244,7 +244,7 @@
             }
         } else {
             let name = self.elaborator.interner.function_name(&function);
             unreachable!("Non-builtin, lowlevel or oracle builtin fn '{name}'")
         }
     }
 
@@ -544,8 +544,8 @@
             InterpreterError::VariableNotInScope { location }
         })?;
 
-        if let ImplKind::TraitMethod(method, _, _) = ident.impl_kind {
-            let method_id = resolve_trait_method(self.elaborator.interner, method, id)?;
+        if let ImplKind::TraitMethod(method) = ident.impl_kind {
+            let method_id = resolve_trait_method(self.elaborator.interner, method.method_id, id)?;
             let typ = self.elaborator.interner.id_type(id).follow_bindings();
             let bindings = self.elaborator.interner.get_instantiation_bindings(id).clone();
             return Ok(Value::Function(method_id, typ, Rc::new(bindings)));

compiler/noirc_frontend/src/hir_def/expr.rs

@@ -70,7 +70,14 @@ pub enum ImplKind {
     /// and eventually linked to this id. The boolean indicates whether the impl
     /// is already assumed to exist - e.g. when resolving a path such as `T::default`
     /// when there is a corresponding `T: Default` constraint in scope.
-    TraitMethod(TraitMethodId, TraitConstraint, bool),
+    TraitMethod(TraitMethod),
+}
+
+#[derive(Debug, Clone)]
+pub struct TraitMethod {
+    pub method_id: TraitMethodId,
+    pub constraint: TraitConstraint,
+    pub assumed: bool,
 }
 
 impl Eq for HirIdent {}
@@ -247,7 +254,7 @@ impl HirMethodCallExpression {
                     trait_generics,
                     span: location.span,
                 };
-                (id, ImplKind::TraitMethod(method_id, constraint, false))
+                (id, ImplKind::TraitMethod(TraitMethod { method_id, constraint, assumed: false }))
             }
         };
         let func_var = HirIdent { location, id, impl_kind };

compiler/noirc_frontend/src/monomorphization/mod.rs

@@ -850,8 +850,8 @@ impl<'interner> Monomorphizer<'interner> {
     ) -> Result<ast::Expression, MonomorphizationError> {
         let typ = self.interner.id_type(expr_id);
 
-        if let ImplKind::TraitMethod(method, _, _) = ident.impl_kind {
-            return self.resolve_trait_method_expr(expr_id, typ, method);
+        if let ImplKind::TraitMethod(method) = ident.impl_kind {
+            return self.resolve_trait_method_expr(expr_id, typ, method.method_id);
         }
 
         // Ensure all instantiation bindings are bound.

compiler/noirc_frontend/src/tests/imports.rs

@@ -73,7 +73,7 @@ fn warns_on_use_of_private_exported_item() {
     "#;
 
     let errors = get_program_errors(src);
-    assert_eq!(errors.len(), 2); // An existing bug causes this error to be duplicated
+    assert_eq!(errors.len(), 1);
 
     assert!(matches!(
         &errors[0].0,

compiler/noirc_frontend/src/tests/visibility.rs

@@ -94,7 +94,7 @@ fn errors_if_trying_to_access_public_function_inside_private_module() {
     "#;
 
     let errors = get_program_errors(src);
-    assert_eq!(errors.len(), 2); // There's a bug that duplicates this error
+    assert_eq!(errors.len(), 1);
 
     let CompilationError::ResolverError(ResolverError::PathResolutionError(
         PathResolutionError::Private(ident),