fix: doc comments on functions warn unexpectedly

compiler/noirc_frontend/src/parser/errors.rs

@@ -44,6 +44,8 @@ pub enum ParserErrorReason {
     InvalidPattern,
     #[error("Documentation comment does not document anything")]
     DocCommentDoesNotDocumentAnything,
+    #[error("Documentation comments cannot be applied to function parameters")]
+    DocCommentCannotBeAppliedToFunctionParameters,
 
     #[error("Missing type for function parameter")]
     MissingTypeForFunctionParameter,

compiler/noirc_frontend/src/parser/parser/doc_comments.rs

@@ -19,12 +19,13 @@ impl Parser<'_> {
         })
     }
 
-    /// OuterDocComments = outer_doc_comments*
+    /// OuterDocComments = OuterDocComment*
     pub(super) fn parse_outer_doc_comments(&mut self) -> Vec<String> {
         self.parse_many("outer doc comments", without_separator(), Self::parse_outer_doc_comment)
     }
 
-    fn parse_outer_doc_comment(&mut self) -> Option<String> {
+    /// OuterDocComment = outer_doc_comment
+    pub(super) fn parse_outer_doc_comment(&mut self) -> Option<String> {
         self.eat_kind(TokenKind::OuterDocComment).map(|token| match token.into_token() {
             Token::LineComment(comment, Some(DocStyle::Outer))
             | Token::BlockComment(comment, Some(DocStyle::Outer)) => comment,
@@ -34,13 +35,20 @@ impl Parser<'_> {
 
     /// Skips any outer doc comments but produces a warning saying that they don't document anything.
     pub(super) fn warn_on_outer_doc_comments(&mut self) {
+        self.skip_doc_comments_with_reason(ParserErrorReason::DocCommentDoesNotDocumentAnything);
+    }
+
+    /// Skips any outer doc comments but produces an error saying that they can't be applied to parameters
+    pub(super) fn error_on_outer_doc_comments_on_parameter(&mut self) {
+        let reason = ParserErrorReason::DocCommentCannotBeAppliedToFunctionParameters;
+        self.skip_doc_comments_with_reason(reason);
+    }
+
+    fn skip_doc_comments_with_reason(&mut self, reason: ParserErrorReason) {
         let location_before_doc_comments = self.current_token_location;
         let doc_comments = self.parse_outer_doc_comments();
         if !doc_comments.is_empty() {
-            self.push_error(
-                ParserErrorReason::DocCommentDoesNotDocumentAnything,
-                location_before_doc_comments,
-            );
+            self.push_error(reason, self.location_since(location_before_doc_comments));
         }
     }
 }

compiler/noirc_frontend/src/parser/parser/function.rs

@@ -171,6 +171,8 @@ impl Parser<'_> {
 
     fn parse_function_parameter(&mut self, allow_self: bool) -> Option<Param> {
         loop {
+            self.error_on_outer_doc_comments_on_parameter();
+
             let start_location = self.current_token_location;
 
             let pattern_or_self = if allow_self {
@@ -608,4 +610,19 @@ mod tests {
         assert!(matches!(call.object.kind, ExpressionKind::If(_)));
         assert_eq!(call.method_name.to_string(), "bar");
     }
+
+    #[test]
+    fn errors_on_doc_comments_on_parameter() {
+        let src = "
+        fn foo(
+            /// Doc comment
+            x: Field,
+        )
+        ";
+        let (_module, errors) = parse_program_with_dummy_file(src);
+        assert_eq!(errors.len(), 1);
+
+        let reason = errors[0].reason().unwrap();
+        assert_eq!(reason, &ParserErrorReason::DocCommentCannotBeAppliedToFunctionParameters);
+    }
 }

compiler/noirc_frontend/src/parser/parser/item.rs

@@ -1,8 +1,9 @@
 use iter_extended::vecmap;
+use noirc_errors::Location;
 
 use crate::{
     parser::{Item, ItemKind, ParserErrorReason, labels::ParsingRuleLabel},
-    token::{Keyword, Token},
+    token::{Attribute, Keyword, Token},
 };
 
 use super::{Parser, impls::Impl, parse_many::without_separator};
@@ -87,11 +88,24 @@ impl<'a> Parser<'a> {
         }
     }
 
-    /// Item = OuterDocComments ItemKind
+    /// Item = ( Attribute | OuterDocComments )* ItemKind
     fn parse_item(&mut self) -> Vec<Item> {
         let start_location = self.current_token_location;
-        let doc_comments = self.parse_outer_doc_comments();
-        let kinds = self.parse_item_kind();
+
+        // Attributes and doc comments can come in any order, and can even be interspersed
+        let mut doc_comments = Vec::new();
+        let mut attributes = Vec::new();
+        loop {
+            if let Some(doc_comment) = self.parse_outer_doc_comment() {
+                doc_comments.push(doc_comment);
+            } else if let Some(attribute) = self.parse_attribute() {
+                attributes.push(attribute);
+            } else {
+                break;
+            }
+        }
+
+        let kinds = self.parse_item_kind(attributes);
         let location = self.location_since(start_location);
 
         if kinds.is_empty() && !doc_comments.is_empty() {
@@ -118,13 +132,12 @@ impl<'a> Parser<'a> {
     ///         | TypeAlias
     ///         | Function
     ///         )
-    fn parse_item_kind(&mut self) -> Vec<ItemKind> {
+    fn parse_item_kind(&mut self, attributes: Vec<(Attribute, Location)>) -> Vec<ItemKind> {
         if let Some(kind) = self.parse_inner_attribute() {
             return vec![ItemKind::InnerAttribute(kind)];
         }
 
         let start_location = self.current_token_location;
-        let attributes = self.parse_attributes();
 
         let modifiers = self.parse_modifiers(
             true, // allow mut
@@ -236,7 +249,10 @@ impl<'a> Parser<'a> {
 mod tests {
     use crate::{
         parse_program_with_dummy_file,
-        parser::parser::tests::{get_single_error, get_source_with_error_span},
+        parser::{
+            ItemKind,
+            parser::tests::{get_single_error, get_source_with_error_span},
+        },
     };
 
     #[test]
@@ -278,4 +294,33 @@ mod tests {
         let error = get_single_error(&errors, span);
         assert!(error.to_string().contains("This doc comment doesn't document anything"));
     }
+
+    #[test]
+    fn parse_item_with_mixed_attributes_and_doc_comments() {
+        let src = "
+        /// One
+        #[one]
+        /// Two
+        #[two]
+        /// Three
+        fn foo() {}
+        ";
+
+        let (module, errors) = parse_program_with_dummy_file(src);
+        assert!(errors.is_empty());
+
+        assert_eq!(module.items.len(), 1);
+        let item = &module.items[0];
+        assert_eq!(
+            item.doc_comments,
+            vec![" One".to_string(), " Two".to_string(), " Three".to_string(),]
+        );
+        let ItemKind::Function(func) = &item.kind else {
+            panic!("Expected function");
+        };
+        let attributes = &func.attributes().secondary;
+        assert_eq!(attributes.len(), 2);
+        assert_eq!(attributes[0].to_string(), "#[one]");
+        assert_eq!(attributes[1].to_string(), "#[two]");
+    }
 }

noir_stdlib/src/ecdsa_secp256k1.nr

@@ -1,21 +1,20 @@
-// TODO(https://github.com/noir-lang/noir/issues/7711): Switch the regular comments to doc comments once they do not trigger a warning anymore
 #[foreign(ecdsa_secp256k1)]
 // docs:start:ecdsa_secp256k1
-// Verifies a ECDSA signature over the secp256k1 curve.
-// - inputs:
-//     - x coordinate of public key as 32 bytes
-//     - y coordinate of public key as 32 bytes
-//     - the signature, as a 64 bytes array
-//       The signature internally will be represented as `(r, s)`,
-//       where `r` and `s` are fixed-sized big endian scalar values.
-//       As the `secp256k1` has a 256-bit modulus, we have a 64 byte signature
-//       while `r` and `s` will both be 32 bytes.
-//       We expect `s` to be normalized. This means given the curve's order,
-//       `s` should be less than or equal to `order / 2`.
-//       This is done to prevent malleability.
-//       For more context regarding malleability you can reference BIP 0062.
-//     - the hash of the message, as a vector of bytes
-// - output: false for failure and true for success
+/// Verifies a ECDSA signature over the secp256k1 curve.
+/// - inputs:
+///     - x coordinate of public key as 32 bytes
+///     - y coordinate of public key as 32 bytes
+///     - the signature, as a 64 bytes array
+///       The signature internally will be represented as `(r, s)`,
+///       where `r` and `s` are fixed-sized big endian scalar values.
+///       As the `secp256k1` has a 256-bit modulus, we have a 64 byte signature
+///       while `r` and `s` will both be 32 bytes.
+///       We expect `s` to be normalized. This means given the curve's order,
+///       `s` should be less than or equal to `order / 2`.
+///       This is done to prevent malleability.
+///       For more context regarding malleability you can reference BIP 0062.
+///     - the hash of the message, as a vector of bytes
+/// - output: false for failure and true for success
 pub fn verify_signature<let N: u32>(
     public_key_x: [u8; 32],
     public_key_y: [u8; 32],

tooling/nargo_fmt/src/formatter/attribute.rs

@@ -15,9 +15,14 @@ impl Formatter<'_> {
         if let Some((function_attribute, index)) = attributes.function {
             all_attributes.insert(index, Attribute::Function(function_attribute));
         }
+
+        // Attributes and doc comments can be mixed, so between each attribute
+        // we format potential doc comments
         for attribute in all_attributes {
+            self.format_outer_doc_comments();
             self.format_attribute(attribute);
         }
+        self.format_outer_doc_comments();
     }
 
     pub(super) fn format_secondary_attributes(&mut self, attributes: Vec<SecondaryAttribute>) {

tooling/nargo_fmt/src/formatter/item.rs

@@ -179,3 +179,28 @@ struct ImportGroup {
     visibility: ItemVisibility,
     span_end: u32,
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::assert_format;
+
+    #[test]
+    fn formats_item_with_mixed_attributes_and_doc_comments() {
+        let src = "
+        /// One
+        #[one]
+        /// Two
+        #[two]
+        /// Three
+        fn  foo( ) { }
+        ";
+        let expected = "/// One
+#[one]
+/// Two
+#[two]
+/// Three
+fn foo() {}
+";
+        assert_format(src, expected);
+    }
+}