fix: update attribute syntax to require following element (#5325)

This change makes is so that attributes must only exist preceding
another syntax that allows attributes. Attributes can now always be
found as part of the base node of the attached element (i.e. package
clause or import declaration or statement).

docs/SPEC.md

@@ -1024,15 +1024,12 @@ Flux source is organized into packages.
 A package consists of one or more source files.
 Each source file is parsed individually and composed into a single package.
 
-    File = [ Attributes ] [ PackageClause ] [ ImportList ] StatementList .
+    File = [ PackageClause ] [ ImportList ] StatementList .
     ImportList = { ImportDeclaration } .
 
-The first attributes defined before the package clause or import lists whether they exists or not apply to the entire package.
-A package clause or import list is required to associate attributes with the first statement in a file.
-
 #### Package clause
 
-    PackageClause = "package" identifier .
+    PackageClause = [ Attributes ] "package" identifier .
 
 A package clause defines the name for the current package.
 Package names must be valid Flux identifiers.
@@ -1052,7 +1049,7 @@ The _main_ package is special for a few reasons:
 
 #### Import declaration
 
-    ImportDeclaration = "import" [identifier] string_lit
+    ImportDeclaration = [ Attributes ] "import" [identifier] string_lit
 
 Associated with every package is a package name and an import path.
 The import statement takes a package's import path and brings all of the identifiers defined in that package into the current scope under a namespace.
@@ -1081,7 +1078,7 @@ Every statement contained in an imported package is evaluated.
 
 ### Attributes
 
-Attributes define a set of properties on the subsequent source code element.
+Attributes define a set of properties on source code elements.
 
     Attributes             = { Attribute } .
     Attribute              = "@" identifier AttributeParameters .

libflux/flux-core/src/formatter/mod.rs

@@ -401,8 +401,10 @@ impl<'doc> Formatter<'doc> {
     fn format_package_clause(&mut self, pkg: &'doc ast::PackageClause) -> Doc<'doc> {
         let arena = self.arena;
         if !pkg.name.name.is_empty() {
+            let attrs = self.format_attribute_list(&pkg.base.attributes);
             docs![
                 arena,
+                attrs,
                 self.format_comments(&pkg.base.comments),
                 "package ",
                 self.format_identifier(&pkg.name),
@@ -681,8 +683,10 @@ impl<'doc> Formatter<'doc> {
 
     fn format_import_declaration(&mut self, n: &'doc ast::ImportDeclaration) -> Doc<'doc> {
         let arena = self.arena;
+        let attrs = self.format_attribute_list(&n.base.attributes);
         docs![
             arena,
+            attrs,
             self.format_comments(&n.base.comments),
             "import ",
             if let Some(alias) = &n.alias {
@@ -795,7 +799,7 @@ impl<'doc> Formatter<'doc> {
         let arena = self.arena;
         docs![
             arena,
-            self.format_attribute_list(&s.base().attributes),
+            self.format_attribute_list(&s.base().attributes,),
             match s {
                 Statement::Expr(s) => self.format_expression_statement(s),
                 Statement::Variable(s) => self.format_variable_assignment(s),

libflux/flux-core/src/formatter/tests.rs

@@ -320,8 +320,33 @@ identity = (x) => x"#,
 )
 package foo
 "#,
+    );
+    assert_unchanged(
+        r#"// Attributes on imports
+@edition("2022.1")
+import "foo""#,
     );
     assert_format("@edition package foo", "@edition\npackage foo\n");
+
+    // All the attributes
+    assert_unchanged(
+        r#"// Package comments
+@mount("fluxlang.dev", "https://fluxlang.dev/api/modules")
+@two
+@three
+@four
+package foo
+
+
+// Comments for import
+@registry("fluxlang.dev")
+@double
+import "date"
+
+// x is one
+@deprecated("0.123.0")
+x = 1"#,
+    );
 }
 
 #[test]

libflux/flux-core/src/parser/mod.rs

@@ -304,6 +304,20 @@ impl<'input> Parser<'input> {
         base.set_comments(comments_from.comments.clone());
         base
     }
+    fn base_node_from_other_end_c_a(
+        &mut self,
+        start: &Token,
+        end: &BaseNode,
+        comments_from: &Token,
+        attributes: Vec<Attribute>,
+    ) -> BaseNode {
+        let mut base = self.base_node(
+            self.source_location(&ast::Position::from(&start.start_pos), &end.location.end),
+        );
+        base.set_comments(comments_from.comments.clone());
+        base.attributes = attributes;
+        base
+    }
 
     fn base_node_from_others(&mut self, start: &BaseNode, end: &BaseNode) -> BaseNode {
         self.base_node_from_pos(&start.location.start, &end.location.end)
@@ -363,18 +377,28 @@ impl<'input> Parser<'input> {
         let start_pos = ast::Position::from(&self.peek().start_pos);
         let mut end = ast::Position::invalid();
 
-        // Parse attributes at the beginning of the file.
-        let attributes = self.parse_attribute_list();
+        // Parse inner attributes at the beginning of the file and hand them off to the first
+        // clause, declaration or statement that exists
+        let inner_attributes = self.parse_attribute_inner_list();
 
-        let pkg = self.parse_package_clause();
+        let (pkg, inner_attributes) = self.parse_package_clause(inner_attributes);
         if let Some(pkg) = &pkg {
             end = pkg.base.location.end;
         }
-        let imports = self.parse_import_list();
+        let (imports, inner_attributes) = self.parse_import_list(inner_attributes);
         if let Some(import) = imports.last() {
             end = import.base.location.end;
         }
-        let body = self.parse_statement_list();
+        let (mut body, inner_attributes) = self.parse_statement_list(inner_attributes);
+        if let Some(attrs) = inner_attributes {
+            if !attrs.is_empty() {
+                // We have left over attributes from the beginning of the file.
+                body.push(Statement::Bad(Box::new(BadStmt {
+                    base: self.base_node_from_others(&attrs[0].base, &attrs[attrs.len() - 1].base),
+                    text: "extra attributes not associated with anything".to_string(),
+                })));
+            }
+        }
         if let Some(stmt) = body.last() {
             end = stmt.base().location.end;
         }
@@ -383,7 +407,6 @@ impl<'input> Parser<'input> {
         File {
             base: BaseNode {
                 location: self.source_location(&start_pos, &end),
-                attributes,
                 ..BaseNode::default()
             },
             name: self.fname.clone(),
@@ -395,58 +418,84 @@ impl<'input> Parser<'input> {
         }
     }
 
-    fn parse_package_clause(&mut self) -> Option<PackageClause> {
+    fn parse_package_clause(
+        &mut self,
+        attributes: Vec<Attribute>,
+    ) -> (Option<PackageClause>, Option<Vec<Attribute>>) {
         let t = self.peek();
         if t.tok == TokenType::Package {
             let t = self.consume();
             let ident = self.parse_identifier();
-            return Some(PackageClause {
-                base: self.base_node_from_other_end_c(&t, &ident.base, &t),
-                name: ident,
-            });
+            let base = self.base_node_from_other_end_c_a(&t, &ident.base, &t, attributes);
+            return (Some(PackageClause { base, name: ident }), None);
         }
-        None
+        (None, Some(attributes))
     }
 
-    fn parse_import_list(&mut self) -> Vec<ImportDeclaration> {
+    fn parse_import_list(
+        &mut self,
+        attributes: Option<Vec<Attribute>>,
+    ) -> (Vec<ImportDeclaration>, Option<Vec<Attribute>>) {
         let mut imports: Vec<ImportDeclaration> = Vec::new();
+        let mut attrs = attributes;
         loop {
             let t = self.peek();
-            if t.tok != TokenType::Import {
-                return imports;
+            match t.tok {
+                TokenType::Attribute => {
+                    if attrs.is_some() {
+                        self.errs.push("found multiple attribute lists".to_string());
+                    }
+                    attrs = Some(self.parse_attribute_inner_list());
+                }
+                TokenType::Import => {
+                    imports.push(self.parse_import_declaration(attrs));
+                    attrs = None;
+                }
+                _ => {
+                    return (imports, attrs);
+                }
             }
-            imports.push(self.parse_import_declaration())
         }
     }
 
-    fn parse_import_declaration(&mut self) -> ImportDeclaration {
+    fn parse_import_declaration(
+        &mut self,
+        attributes: Option<Vec<Attribute>>,
+    ) -> ImportDeclaration {
+        let attrs = if let Some(attributes) = attributes {
+            attributes
+        } else {
+            self.parse_attribute_inner_list()
+        };
         let t = self.expect(TokenType::Import);
         let alias = if self.peek().tok == TokenType::Ident {
             Some(self.parse_identifier())
         } else {
             None
         };
         let path = self.parse_string_literal();
-        ImportDeclaration {
-            base: self.base_node_from_other_end_c(&t, &path.base, &t),
-            alias,
-            path,
-        }
+        let base = self.base_node_from_other_end_c_a(&t, &path.base, &t, attrs);
+        ImportDeclaration { base, alias, path }
     }
 
-    fn parse_statement_list(&mut self) -> Vec<Statement> {
+    fn parse_statement_list(
+        &mut self,
+        attributes: Option<Vec<Attribute>>,
+    ) -> (Vec<Statement>, Option<Vec<Attribute>>) {
         let mut stmts: Vec<Statement> = Vec::new();
+        let mut attrs = attributes;
         loop {
             if !self.more() {
-                return stmts;
+                return (stmts, attrs);
             }
-            stmts.push(self.parse_statement());
+            stmts.push(self.parse_statement(attrs));
+            attrs = None;
         }
     }
 
     /// Parses a flux statement
-    pub fn parse_statement(&mut self) -> Statement {
-        self.depth_guard(|this| this.parse_statement_inner())
+    pub fn parse_statement(&mut self, attributes: Option<Vec<Attribute>>) -> Statement {
+        self.depth_guard(|this| this.parse_statement_inner(attributes))
             .unwrap_or_else(|| {
                 let t = self.consume();
                 Statement::Bad(Box::new(BadStmt {
@@ -456,8 +505,12 @@ impl<'input> Parser<'input> {
             })
     }
 
-    fn parse_statement_inner(&mut self) -> Statement {
-        let attributes = self.parse_attribute_list();
+    fn parse_statement_inner(&mut self, attributes: Option<Vec<Attribute>>) -> Statement {
+        let attributes = if let Some(attributes) = attributes {
+            attributes
+        } else {
+            self.parse_attribute_inner_list()
+        };
 
         let t = self.peek();
         let mut stmt = match t.tok {
@@ -925,7 +978,7 @@ impl<'input> Parser<'input> {
     }
     fn parse_block(&mut self) -> Block {
         let start = self.open(TokenType::LBrace, TokenType::RBrace);
-        let stmts = self.parse_statement_list();
+        let (stmts, _) = self.parse_statement_list(None);
         let end = self.close(TokenType::RBrace);
         Block {
             base: self.base_node_from_tokens(&start, &end),
@@ -2307,18 +2360,20 @@ impl<'input> Parser<'input> {
         }
     }
 
-    fn parse_attribute_list(&mut self) -> Vec<Attribute> {
+    fn parse_attribute_inner_list(&mut self) -> Vec<Attribute> {
         let mut attributes = Vec::new();
         while self.peek().tok == TokenType::Attribute {
-            attributes.push(self.parse_attribute());
+            attributes.push(self.parse_attribute_inner());
         }
         attributes
     }
 
-    fn parse_attribute(&mut self) -> Attribute {
+    fn parse_attribute_inner(&mut self) -> Attribute {
         let tok = self.expect(TokenType::Attribute);
         let name = tok.lit.trim_start_matches('@').to_string();
-
+        self.parse_attribute_rest(tok, name)
+    }
+    fn parse_attribute_rest(&mut self, tok: Token, name: String) -> Attribute {
         // Parenthesis are optional. No parenthesis means no parameters.
         if self.peek().tok != TokenType::LParen {
             return Attribute {