From 7b56904e56d95b88cefcbf3862e822fd3b1c8730 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 26 Nov 2024 15:19:49 +0000 Subject: [PATCH] fix!: Disallow `#[export]` on associated methods (#6626) --- .../src/hir/def_collector/dc_mod.rs | 13 ++ .../src/hir/def_collector/errors.rs | 12 +- compiler/noirc_frontend/src/tests.rs | 120 ++++++++++++------ 3 files changed, 105 insertions(+), 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 index 2a58e1049da..e7953aab5a4 100644 --- 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 index c08b4ff2062..cafbc670e32 100644 --- 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 index cb45bf37c83..605236c8dda 100644 --- 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 { .. } + ) + )); + }); }