fix: require generic trait impls to be in scope to call them (#6913)

Co-authored-by: Tom French <[email protected]>

Author: asterite

compiler/noirc_frontend/src/elaborator/types.rs

@@ -1361,7 +1361,7 @@ impl<'context> Elaborator<'context> {
         span: Span,
         has_self_arg: bool,
     ) -> Option<HirMethodReference> {
-        // First search in the struct methods
+        // First search in the type methods. If there is one, that's the one.
         if let Some(method_id) =
             self.interner.lookup_direct_method(object_type, method_name, has_self_arg)
         {
@@ -1372,43 +1372,55 @@ impl<'context> Elaborator<'context> {
         let trait_methods =
             self.interner.lookup_trait_methods(object_type, method_name, has_self_arg);
 
-        if trait_methods.is_empty() {
-            // If we couldn't find any trait methods, search in
-            // impls for all types `T`, e.g. `impl<T> Foo for T`
-            if let Some(func_id) =
-                self.interner.lookup_generic_method(object_type, method_name, has_self_arg)
-            {
-                return Some(HirMethodReference::FuncId(func_id));
-            }
+        // If there's at least one matching trait method we need to see if only one is in scope.
+        if !trait_methods.is_empty() {
+            return self.return_trait_method_in_scope(&trait_methods, method_name, span);
+        }
 
-            if let Type::Struct(struct_type, _) = object_type {
-                let has_field_with_function_type =
-                    struct_type.borrow().get_fields_as_written().into_iter().any(|field| {
-                        field.name.0.contents == method_name && field.typ.is_function()
-                    });
-                if has_field_with_function_type {
-                    self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
-                        method_name: method_name.to_string(),
-                        object_type: object_type.clone(),
-                        span,
-                    });
-                } else {
-                    self.push_err(TypeCheckError::UnresolvedMethodCall {
-                        method_name: method_name.to_string(),
-                        object_type: object_type.clone(),
-                        span,
-                    });
-                }
-                return None;
+        // If we couldn't find any trait methods, search in
+        // impls for all types `T`, e.g. `impl<T> Foo for T`
+        let generic_methods =
+            self.interner.lookup_generic_methods(object_type, method_name, has_self_arg);
+        if !generic_methods.is_empty() {
+            return self.return_trait_method_in_scope(&generic_methods, method_name, span);
+        }
+
+        if let Type::Struct(struct_type, _) = object_type {
+            let has_field_with_function_type = struct_type
+                .borrow()
+                .get_fields_as_written()
+                .into_iter()
+                .any(|field| field.name.0.contents == method_name && field.typ.is_function());
+            if has_field_with_function_type {
+                self.push_err(TypeCheckError::CannotInvokeStructFieldFunctionType {
+                    method_name: method_name.to_string(),
+                    object_type: object_type.clone(),
+                    span,
+                });
             } else {
-                // It could be that this type is a composite type that is bound to a trait,
-                // for example `x: (T, U) ... where (T, U): SomeTrait`
-                // (so this case is a generalization of the NamedGeneric case)
-                return self.lookup_method_in_trait_constraints(object_type, method_name, span);
+                self.push_err(TypeCheckError::UnresolvedMethodCall {
+                    method_name: method_name.to_string(),
+                    object_type: object_type.clone(),
+                    span,
+                });
             }
+            None
+        } else {
+            // It could be that this type is a composite type that is bound to a trait,
+            // for example `x: (T, U) ... where (T, U): SomeTrait`
+            // (so this case is a generalization of the NamedGeneric case)
+            self.lookup_method_in_trait_constraints(object_type, method_name, span)
         }
+    }
 
-        // We found some trait methods... but is only one of them currently in scope?
+    /// Given a list of functions and the trait they belong to, returns the one function
+    /// that is in scope.
+    fn return_trait_method_in_scope(
+        &mut self,
+        trait_methods: &[(FuncId, TraitId)],
+        method_name: &str,
+        span: Span,
+    ) -> Option<HirMethodReference> {
         let module_id = self.module_id();
         let module_data = self.get_module(module_id);
 
@@ -1490,7 +1502,7 @@ impl<'context> Elaborator<'context> {
     fn trait_hir_method_reference(
         &self,
         trait_id: TraitId,
-        trait_methods: Vec<(FuncId, TraitId)>,
+        trait_methods: &[(FuncId, TraitId)],
         method_name: &str,
         span: Span,
     ) -> HirMethodReference {

compiler/noirc_frontend/src/node_interner.rs

@@ -1746,7 +1746,7 @@ impl NodeInterner {
         Ok(())
     }
 
-    /// Looks up a method that's directly defined in the given struct.
+    /// Looks up a method that's directly defined in the given type.
     pub fn lookup_direct_method(
         &self,
         typ: &Type,
@@ -1761,7 +1761,7 @@ impl NodeInterner {
             .and_then(|methods| methods.find_direct_method(typ, has_self_arg, self))
     }
 
-    /// Looks up a methods that apply to the given struct but are defined in traits.
+    /// Looks up a methods that apply to the given type but are defined in traits.
     pub fn lookup_trait_methods(
         &self,
         typ: &Type,
@@ -1780,43 +1780,18 @@ impl NodeInterner {
         }
     }
 
-    /// Select the 1 matching method with an object type matching `typ`
-    fn find_matching_method(
-        &self,
-        typ: &Type,
-        methods: Option<&Methods>,
-        method_name: &str,
-        has_self_arg: bool,
-    ) -> Option<FuncId> {
-        if let Some(method) = methods.and_then(|m| m.find_matching_method(typ, has_self_arg, self))
-        {
-            Some(method)
-        } else {
-            self.lookup_generic_method(typ, method_name, has_self_arg)
-        }
-    }
-
-    /// Looks up a method at impls for all types `T`, e.g. `impl<T> Foo for T`
-    pub fn lookup_generic_method(
+    /// Looks up methods at impls for all types `T`, e.g. `impl<T> Foo for T`
+    pub fn lookup_generic_methods(
         &self,
         typ: &Type,
         method_name: &str,
         has_self_arg: bool,
-    ) -> Option<FuncId> {
-        let global_methods = self.methods.get(&TypeMethodKey::Generic)?.get(method_name)?;
-        global_methods.find_matching_method(typ, has_self_arg, self)
-    }
-
-    /// Looks up a given method name on the given primitive type.
-    pub fn lookup_primitive_method(
-        &self,
-        typ: &Type,
-        method_name: &str,
-        has_self_arg: bool,
-    ) -> Option<FuncId> {
-        let key = get_type_method_key(typ)?;
-        let methods = self.methods.get(&key)?.get(method_name)?;
-        self.find_matching_method(typ, Some(methods), method_name, has_self_arg)
+    ) -> Vec<(FuncId, TraitId)> {
+        self.methods
+            .get(&TypeMethodKey::Generic)
+            .and_then(|h| h.get(method_name))
+            .map(|methods| methods.find_trait_methods(typ, has_self_arg, self))
+            .unwrap_or_default()
     }
 
     /// Returns what the next trait impl id is expected to be.

compiler/noirc_frontend/src/tests/traits.rs

@@ -879,6 +879,43 @@ fn errors_if_multiple_trait_methods_are_in_scope_for_function_call() {
     assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]);
 }
 
+#[test]
+fn warns_if_trait_is_not_in_scope_for_method_call_and_there_is_only_one_trait_method() {
+    let src = r#"
+    fn main() {
+        let bar = Bar { x: 42 };
+        let _ = bar.foo();
+    }
+
+    pub struct Bar {
+        x: i32,
+    }
+
+    mod private_mod {
+        pub trait Foo {
+            fn foo(self) -> i32;
+        }
+
+        impl Foo for super::Bar {
+            fn foo(self) -> i32 {
+                self.x
+            }
+        }
+    }
+    "#;
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::PathResolutionError(
+        PathResolutionError::TraitMethodNotInScope { ident, trait_name },
+    )) = &errors[0].0
+    else {
+        panic!("Expected a 'trait method not in scope' error");
+    };
+    assert_eq!(ident.to_string(), "foo");
+    assert_eq!(trait_name, "private_mod::Foo");
+}
+
 #[test]
 fn calls_trait_method_if_it_is_in_scope() {
     let src = r#"
@@ -1166,3 +1203,36 @@ fn warns_if_trait_is_not_in_scope_for_primitive_method_call_and_there_is_only_on
     assert_eq!(ident.to_string(), "foo");
     assert_eq!(trait_name, "private_mod::Foo");
 }
+
+#[test]
+fn warns_if_trait_is_not_in_scope_for_generic_function_call_and_there_is_only_one_trait_method() {
+    let src = r#"
+    fn main() {
+        let x: i32 = 1;
+        let _ = x.foo();
+    }
+
+    mod private_mod {
+        pub trait Foo<T> {
+            fn foo(self) -> i32;
+        }
+
+        impl<T> Foo<T> for T {
+            fn foo(self) -> i32 {
+                42
+            }
+        }
+    }
+    "#;
+    let errors = get_program_errors(src);
+    assert_eq!(errors.len(), 1);
+
+    let CompilationError::ResolverError(ResolverError::PathResolutionError(
+        PathResolutionError::TraitMethodNotInScope { ident, trait_name },
+    )) = &errors[0].0
+    else {
+        panic!("Expected a 'trait method not in scope' error");
+    };
+    assert_eq!(ident.to_string(), "foo");
+    assert_eq!(trait_name, "private_mod::Foo");
+}