fix!: Disallow #[export] on associated methods (#6626)

noir-lang · Nov 26, 2024 · 7b56904 · 7b56904
1 parent 42b9dac
commit 7b56904
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 40 deletions.
diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
@@ -216,6 +216,13 @@ impl<'a> ModCollector<'a> {
                     errors.push((error.into(), self.file_id));
                 }
 
+                if noir_function.def.attributes.has_export() {
+                    let error = DefCollectorErrorKind::ExportOnAssociatedFunction {
+                        span: noir_function.name_ident().span(),
+                    };
+                    errors.push((error.into(), self.file_id));
+                }
+
                 let location = Location::new(noir_function.def.span, self.file_id);
                 context.def_interner.push_function(*func_id, &noir_function.def, module, location);
             }
@@ -1088,6 +1095,12 @@ pub fn collect_impl(
             errors.push((error.into(), file_id));
             continue;
         }
+        if method.def.attributes.has_export() {
+            let error = DefCollectorErrorKind::ExportOnAssociatedFunction {
+                span: method.name_ident().span(),
+            };
+            errors.push((error.into(), file_id));
+        }
 
         let func_id = interner.push_empty_fn();
         method.def.where_clause.extend(r#impl.where_clause.clone());

diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs
@@ -84,6 +84,8 @@ pub enum DefCollectorErrorKind {
     UnsupportedNumericGenericType(#[from] UnsupportedNumericGenericType),
     #[error("The `#[test]` attribute may only be used on a non-associated function")]
     TestOnAssociatedFunction { span: Span },
+    #[error("The `#[export]` attribute may only be used on a non-associated function")]
+    ExportOnAssociatedFunction { span: Span },
 }
 
 impl DefCollectorErrorKind {
@@ -182,8 +184,8 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
             DefCollectorErrorKind::PathResolutionError(error) => error.into(),
             DefCollectorErrorKind::CannotReexportItemWithLessVisibility{item_name, desired_visibility} => {
                 Diagnostic::simple_warning(
-                    format!("cannot re-export {item_name} because it has less visibility than this use statement"), 
-                    format!("consider marking {item_name} as {desired_visibility}"), 
+                    format!("cannot re-export {item_name} because it has less visibility than this use statement"),
+                    format!("consider marking {item_name} as {desired_visibility}"),
                     item_name.span())
             }
             DefCollectorErrorKind::NonStructTypeInImpl { span } => Diagnostic::simple_error(
@@ -298,7 +300,11 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic {
                 String::new(),
                 *span,
             ),
-
+            DefCollectorErrorKind::ExportOnAssociatedFunction { span } => Diagnostic::simple_error(
+                "The `#[export]` attribute is disallowed on `impl` methods".into(),
+                String::new(),
+                *span,
+            ),
         }
     }
 }
diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs
@@ -3752,50 +3752,96 @@ fn allows_struct_with_generic_infix_type_as_main_input_3() {
     assert_no_errors(src);
 }
 
-#[test]
-fn disallows_test_attribute_on_impl_method() {
-    let src = r#"
-    pub struct Foo {}
-    impl Foo {
-        #[test]
-        fn foo() {}
-    }
+fn test_disallows_attribute_on_impl_method(
+    attr: &str,
+    check_error: impl FnOnce(&CompilationError),
+) {
+    let src = format!(
+        "
+        pub struct Foo {{ }}
 
-    fn main() {}
-    "#;
-    let errors = get_program_errors(src);
+        impl Foo {{
+            #[{attr}]
+            fn foo() {{ }}
+        }}
+
+        fn main() {{ }}
+    "
+    );
+    let errors = get_program_errors(&src);
     assert_eq!(errors.len(), 1);
+    check_error(&errors[0].0);
+}
 
-    assert!(matches!(
-        errors[0].0,
-        CompilationError::DefinitionError(DefCollectorErrorKind::TestOnAssociatedFunction {
-            span: _
-        })
-    ));
+fn test_disallows_attribute_on_trait_impl_method(
+    attr: &str,
+    check_error: impl FnOnce(&CompilationError),
+) {
+    let src = format!(
+        "
+        pub trait Trait {{
+            fn foo() {{ }}
+        }}
+
+        pub struct Foo {{ }}
+
+        impl Trait for Foo {{
+            #[{attr}]
+            fn foo() {{ }}
+        }}
+
+        fn main() {{ }}
+    "
+    );
+    let errors = get_program_errors(&src);
+    assert_eq!(errors.len(), 1);
+    check_error(&errors[0].0);
 }
 
 #[test]
-fn disallows_test_attribute_on_trait_impl_method() {
-    let src = r#"
-    pub trait Trait {
-        fn foo() {}
-    }
+fn disallows_test_attribute_on_impl_method() {
+    test_disallows_attribute_on_impl_method("test", |error| {
+        assert!(matches!(
+            error,
+            CompilationError::DefinitionError(
+                DefCollectorErrorKind::TestOnAssociatedFunction { .. }
+            )
+        ));
+    });
+}
 
-    pub struct Foo {}
-    impl Trait for Foo {
-        #[test]
-        fn foo() {}
-    }
+#[test]
+fn disallows_test_attribute_on_trait_impl_method() {
+    test_disallows_attribute_on_trait_impl_method("test", |error| {
+        assert!(matches!(
+            error,
+            CompilationError::DefinitionError(
+                DefCollectorErrorKind::TestOnAssociatedFunction { .. }
+            )
+        ));
+    });
+}
 
-    fn main() {}
-    "#;
-    let errors = get_program_errors(src);
-    assert_eq!(errors.len(), 1);
+#[test]
+fn disallows_export_attribute_on_impl_method() {
+    test_disallows_attribute_on_impl_method("export", |error| {
+        assert!(matches!(
+            error,
+            CompilationError::DefinitionError(
+                DefCollectorErrorKind::ExportOnAssociatedFunction { .. }
+            )
+        ));
+    });
+}
 
-    assert!(matches!(
-        errors[0].0,
-        CompilationError::DefinitionError(DefCollectorErrorKind::TestOnAssociatedFunction {
-            span: _
-        })
-    ));
+#[test]
+fn disallows_export_attribute_on_trait_impl_method() {
+    test_disallows_attribute_on_trait_impl_method("export", |error| {
+        assert!(matches!(
+            error,
+            CompilationError::DefinitionError(
+                DefCollectorErrorKind::ExportOnAssociatedFunction { .. }
+            )
+        ));
+    });
 }