chore: standardize that doc comments on top of statements and express…

…ion are allowed but warn (#7450)
noir-lang · Feb 19, 2025 · c63cc73 · c63cc73
1 parent 093a8ec
commit c63cc73
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 20 deletions.
diff --git a/compiler/noirc_frontend/src/parser/parser/doc_comments.rs b/compiler/noirc_frontend/src/parser/parser/doc_comments.rs
@@ -1,4 +1,7 @@
-use crate::token::{DocStyle, Token, TokenKind};
+use crate::{
+    parser::ParserErrorReason,
+    token::{DocStyle, Token, TokenKind},
+};
 
 use super::{parse_many::without_separator, Parser};
 
@@ -28,6 +31,18 @@ impl<'a> Parser<'a> {
             _ => unreachable!(),
         })
     }
+
+    /// 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) {
+        let span_before_doc_comments = self.current_token_span;
+        let doc_comments = self.parse_outer_doc_comments();
+        if !doc_comments.is_empty() {
+            self.push_error(
+                ParserErrorReason::DocCommentDoesNotDocumentAnything,
+                span_before_doc_comments,
+            );
+        }
+    }
 }
 
 #[cfg(test)]

diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs
@@ -267,14 +267,8 @@ impl<'a> Parser<'a> {
     }
 
     fn parse_atom_kind(&mut self, allow_constructors: bool) -> Option<ExpressionKind> {
-        let span_before_doc_comments = self.current_token_span;
-        let doc_comments = self.parse_outer_doc_comments();
-        if !doc_comments.is_empty() {
-            self.push_error(
-                ParserErrorReason::DocCommentDoesNotDocumentAnything,
-                span_before_doc_comments,
-            );
-        }
+        // Like in Rust, we allow parsing doc comments on top of an expression but they always produce a warning.
+        self.warn_on_outer_doc_comments();
 
         if let Some(kind) = self.parse_unsafe_expr() {
             return Some(kind);

diff --git a/compiler/noirc_frontend/src/parser/parser/statement.rs b/compiler/noirc_frontend/src/parser/parser/statement.rs
@@ -25,14 +25,8 @@ impl<'a> Parser<'a> {
     /// Statement = Attributes StatementKind ';'?
     pub(crate) fn parse_statement(&mut self) -> Option<(Statement, (Option<Token>, Span))> {
         loop {
-            let span_before_doc_comments = self.current_token_span;
-            let doc_comments = self.parse_outer_doc_comments();
-            if !doc_comments.is_empty() {
-                self.push_error(
-                    ParserErrorReason::DocCommentDoesNotDocumentAnything,
-                    span_before_doc_comments,
-                );
-            }
+            // Like in Rust, we allow parsing doc comments on top of a statement but they always produce a warning.
+            self.warn_on_outer_doc_comments();
 
             if !self.current_token_comments.is_empty() {
                 self.statement_comments = Some(std::mem::take(&mut self.current_token_comments));

diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs
@@ -6,7 +6,7 @@ use noirc_frontend::{
         MemberAccessExpression, MethodCallExpression, PrefixExpression, TypePath, UnaryOp,
         UnresolvedTypeData,
     },
-    token::{Keyword, Token},
+    token::{Keyword, Token, TokenKind},
 };
 
 use crate::chunks::{Chunk, ChunkFormatter, ChunkGroup, GroupKind, GroupTag, TextChunk};
@@ -21,7 +21,19 @@ struct FormattedLambda {
 
 impl<'a, 'b> ChunkFormatter<'a, 'b> {
     pub(super) fn format_expression(&mut self, expression: Expression, group: &mut ChunkGroup) {
-        group.leading_comment(self.skip_comments_and_whitespace_chunk());
+        group.leading_comment(self.chunk(|formatter| {
+            // Doc comments for an expression could come before a potential non-doc comment
+            if formatter.token.kind() == TokenKind::OuterDocComment {
+                formatter.format_outer_doc_comments_checking_safety();
+            }
+
+            formatter.skip_comments_and_whitespace();
+
+            // Or doc comments could come after a potential non-doc comment
+            if formatter.token.kind() == TokenKind::OuterDocComment {
+                formatter.format_outer_doc_comments_checking_safety();
+            }
+        }));
 
         match expression.kind {
             ExpressionKind::Literal(literal) => self.format_literal(literal, group),
@@ -377,7 +389,6 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> {
     ) -> ChunkGroup {
         let mut group = ChunkGroup::new();
         group.text(self.chunk(|formatter| {
-            formatter.format_outer_doc_comments_checking_safety();
             formatter.write_keyword(Keyword::Unsafe);
             formatter.write_space();
         }));