Allow [doc] annotation on modules (#2247)

src/analyzer.rs

@@ -10,7 +10,7 @@ pub(crate) struct Analyzer<'src> {
 impl<'src> Analyzer<'src> {
   pub(crate) fn analyze(
     asts: &HashMap<PathBuf, Ast<'src>>,
-    doc: Option<&'src str>,
+    doc: Option<String>,
     loaded: &[PathBuf],
     name: Option<Name<'src>>,
     paths: &HashMap<PathBuf, PathBuf>,
@@ -22,7 +22,7 @@ impl<'src> Analyzer<'src> {
   fn justfile(
     mut self,
     asts: &HashMap<PathBuf, Ast<'src>>,
-    doc: Option<&'src str>,
+    doc: Option<String>,
     loaded: &[PathBuf],
     name: Option<Name<'src>>,
     paths: &HashMap<PathBuf, PathBuf>,
@@ -90,13 +90,27 @@ impl<'src> Analyzer<'src> {
             absolute,
             name,
             doc,
+            attributes,
             ..
           } => {
+            let mut doc_attr: Option<&str> = None;
+            for attribute in attributes {
+              if let Attribute::Doc(ref doc) = attribute {
+                doc_attr = doc.as_ref().map(|s| s.cooked.as_ref());
+              } else {
+                return Err(name.token.error(InvalidAttribute {
+                  item_kind: "Module",
+                  item_name: name.lexeme(),
+                  attribute: attribute.clone(),
+                }));
+              }
+            }
+
             if let Some(absolute) = absolute {
               define(*name, "module", false)?;
               modules.insert(Self::analyze(
                 asts,
-                *doc,
+                doc_attr.or(*doc).map(ToOwned::to_owned),
                 loaded,
                 Some(*name),
                 paths,
@@ -245,12 +259,11 @@ impl<'src> Analyzer<'src> {
   }
 
   fn analyze_alias(alias: &Alias<'src, Name<'src>>) -> CompileResult<'src> {
-    let name = alias.name.lexeme();
-
     for attribute in &alias.attributes {
       if *attribute != Attribute::Private {
-        return Err(alias.name.token.error(AliasInvalidAttribute {
-          alias: name,
+        return Err(alias.name.token.error(InvalidAttribute {
+          item_kind: "Alias",
+          item_name: alias.name.lexeme(),
           attribute: attribute.clone(),
         }));
       }

src/compile_error.rs

@@ -32,13 +32,6 @@ impl Display for CompileError<'_> {
     use CompileErrorKind::*;
 
     match &*self.kind {
-      AliasInvalidAttribute { alias, attribute } => {
-        write!(
-          f,
-          "Alias `{alias}` has invalid attribute `{}`",
-          attribute.name(),
-        )
-      }
       AliasShadowsRecipe { alias, recipe_line } => write!(
         f,
         "Alias `{alias}` defined on line {} shadows recipe `{alias}` defined on line {}",
@@ -150,6 +143,9 @@ impl Display for CompileError<'_> {
         write!(f, "Variable {variable} is both exported and unexported")
       }
       ExtraLeadingWhitespace => write!(f, "Recipe line has extra leading whitespace"),
+      ExtraneousAttributes { count } => {
+        write!(f, "Extraneous {}", Count("attribute", *count))
+      }
       FunctionArgumentCountMismatch {
         function,
         found,
@@ -176,6 +172,15 @@ impl Display for CompileError<'_> {
         "Internal error, this may indicate a bug in just: {message}\n\
            consider filing an issue: https://github.com/casey/just/issues/new"
       ),
+      InvalidAttribute {
+        item_name,
+        item_kind,
+        attribute,
+      } => write!(
+        f,
+        "{item_kind} `{item_name}` has invalid attribute `{}`",
+        attribute.name(),
+      ),
       InvalidEscapeSequence { character } => write!(
         f,
         "`\\{}` is not a valid escape sequence",

src/compile_error_kind.rs

@@ -2,10 +2,6 @@ use super::*;
 
 #[derive(Debug, PartialEq)]
 pub(crate) enum CompileErrorKind<'src> {
-  AliasInvalidAttribute {
-    alias: &'src str,
-    attribute: Attribute<'src>,
-  },
   AliasShadowsRecipe {
     alias: &'src str,
     recipe_line: usize,
@@ -63,6 +59,9 @@ pub(crate) enum CompileErrorKind<'src> {
     variable: &'src str,
   },
   ExtraLeadingWhitespace,
+  ExtraneousAttributes {
+    count: usize,
+  },
   FunctionArgumentCountMismatch {
     function: &'src str,
     found: usize,
@@ -76,6 +75,11 @@ pub(crate) enum CompileErrorKind<'src> {
   Internal {
     message: String,
   },
+  InvalidAttribute {
+    item_kind: &'static str,
+    item_name: &'src str,
+    attribute: Attribute<'src>,
+  },
   InvalidEscapeSequence {
     character: char,
   },

src/item.rs

@@ -13,6 +13,7 @@ pub(crate) enum Item<'src> {
     relative: StringLiteral<'src>,
   },
   Module {
+    attributes: BTreeSet<Attribute<'src>>,
     absolute: Option<PathBuf>,
     doc: Option<&'src str>,
     name: Name<'src>,

src/justfile.rs

@@ -13,7 +13,7 @@ struct Invocation<'src: 'run, 'run> {
 pub(crate) struct Justfile<'src> {
   pub(crate) aliases: Table<'src, Alias<'src>>,
   pub(crate) assignments: Table<'src, Assignment<'src>>,
-  pub(crate) doc: Option<&'src str>,
+  pub(crate) doc: Option<String>,
   #[serde(rename = "first", serialize_with = "keyed::serialize_option")]
   pub(crate) default: Option<Rc<Recipe<'src>>>,
   #[serde(skip)]

src/parser.rs

@@ -321,6 +321,14 @@ impl<'run, 'src> Parser<'run, 'src> {
     self.accept(ByteOrderMark)?;
 
     loop {
+      let mut attributes = self.parse_attributes()?;
+      let mut take_attributes = || {
+        attributes
+          .take()
+          .map(|(_token, attributes)| attributes)
+          .unwrap_or_default()
+      };
+
       let next = self.next()?;
 
       if let Some(comment) = self.accept(Comment)? {
@@ -334,7 +342,7 @@ impl<'run, 'src> Parser<'run, 'src> {
       } else if self.next_is(Identifier) {
         match Keyword::from_lexeme(next.lexeme()) {
           Some(Keyword::Alias) if self.next_are(&[Identifier, Identifier, ColonEquals]) => {
-            items.push(Item::Alias(self.parse_alias(BTreeSet::new())?));
+            items.push(Item::Alias(self.parse_alias(take_attributes())?));
           }
           Some(Keyword::Export) if self.next_are(&[Identifier, Identifier, ColonEquals]) => {
             self.presume_keyword(Keyword::Export)?;
@@ -388,6 +396,7 @@ impl<'run, 'src> Parser<'run, 'src> {
             };
 
             items.push(Item::Module {
+              attributes: take_attributes(),
               absolute: None,
               doc,
               name,
@@ -412,7 +421,7 @@ impl<'run, 'src> Parser<'run, 'src> {
               items.push(Item::Recipe(self.parse_recipe(
                 doc,
                 false,
-                BTreeSet::new(),
+                take_attributes(),
               )?));
             }
           }
@@ -422,23 +431,17 @@ impl<'run, 'src> Parser<'run, 'src> {
         items.push(Item::Recipe(self.parse_recipe(
           doc,
           true,
-          BTreeSet::new(),
+          take_attributes(),
         )?));
-      } else if let Some(attributes) = self.parse_attributes()? {
-        let next_keyword = Keyword::from_lexeme(self.next()?.lexeme());
-        match next_keyword {
-          Some(Keyword::Alias) if self.next_are(&[Identifier, Identifier, ColonEquals]) => {
-            items.push(Item::Alias(self.parse_alias(attributes)?));
-          }
-          _ => {
-            let quiet = self.accepted(At)?;
-            let doc = pop_doc_comment(&mut items, eol_since_last_comment);
-            items.push(Item::Recipe(self.parse_recipe(doc, quiet, attributes)?));
-          }
-        }
       } else {
         return Err(self.unexpected_token()?);
       }
+
+      if let Some((token, attributes)) = attributes {
+        return Err(token.error(CompileErrorKind::ExtraneousAttributes {
+          count: attributes.len(),
+        }));
+      }
     }
 
     if self.next_token == self.tokens.len() {
@@ -989,10 +992,16 @@ impl<'run, 'src> Parser<'run, 'src> {
   }
 
   /// Parse recipe attributes
-  fn parse_attributes(&mut self) -> CompileResult<'src, Option<BTreeSet<Attribute<'src>>>> {
+  fn parse_attributes(
+    &mut self,
+  ) -> CompileResult<'src, Option<(Token<'src>, BTreeSet<Attribute<'src>>)>> {
     let mut attributes = BTreeMap::new();
 
-    while self.accepted(BracketL)? {
+    let mut token = None;
+
+    while let Some(bracket) = self.accept(BracketL)? {
+      token.get_or_insert(bracket);
+
       loop {
         let name = self.parse_name()?;
 
@@ -1029,7 +1038,7 @@ impl<'run, 'src> Parser<'run, 'src> {
     if attributes.is_empty() {
       Ok(None)
     } else {
-      Ok(Some(attributes.into_keys().collect()))
+      Ok(Some((token.unwrap(), attributes.into_keys().collect())))
     }
   }
 }

src/subcommand.rs

@@ -620,7 +620,7 @@ impl Subcommand {
         format_doc(
           config,
           submodule.name(),
-          submodule.doc,
+          submodule.doc.as_deref(),
           max_signature_width,
           &signature_widths,
         );

tests/modules.rs

@@ -737,3 +737,53 @@ fn comments_can_follow_modules() {
     .stdout("FOO\n")
     .run();
 }
+
+#[test]
+fn doc_comment_on_module() {
+  Test::new()
+    .write("foo.just", "")
+    .justfile(
+      "
+        # Comment
+        mod foo
+      ",
+    )
+    .test_round_trip(false)
+    .arg("--list")
+    .stdout("Available recipes:\n    foo ... # Comment\n")
+    .run();
+}
+
+#[test]
+fn doc_attribute_on_module() {
+  Test::new()
+    .write("foo.just", "")
+    .justfile(
+      r#"
+        # Suppressed comment
+        [doc: "Comment"]
+        mod foo
+      "#,
+    )
+    .test_round_trip(false)
+    .arg("--list")
+    .stdout("Available recipes:\n    foo ... # Comment\n")
+    .run();
+}
+
+#[test]
+fn bad_module_attribute_fails() {
+  Test::new()
+    .write("foo.just", "")
+    .justfile(
+      r#"
+        [no-cd]
+        mod foo
+      "#,
+    )
+    .test_round_trip(false)
+    .arg("--list")
+    .stderr("error: Module `foo` has invalid attribute `no-cd`\n ——▶ justfile:2:5\n  │\n2 │ mod foo\n  │     ^^^\n")
+    .status(EXIT_FAILURE)
+    .run();
+}

tests/no_exit_message.rs

@@ -80,7 +80,7 @@ error: Expected identifier, but found ']'
 }
 
 test! {
-  name: unattached_attribute_before_comment,
+  name: extraneous_attribute_before_comment,
   justfile: r#"
 [no-exit-message]
 # This is a doc comment
@@ -88,25 +88,31 @@ hello:
   @exit 100
 "#,
   stderr: r#"
-error: Expected '@', '[', or identifier, but found comment
- ——▶ justfile:2:1
+error: Extraneous attribute
+ ——▶ justfile:1:1
   │
-2 │ # This is a doc comment
-  │ ^^^^^^^^^^^^^^^^^^^^^^^
+1 │ [no-exit-message]
+  │ ^
 "#,
 
   status: EXIT_FAILURE,
 }
 
 test! {
-  name: unattached_attribute_before_empty_line,
+  name: extraneous_attribute_before_empty_line,
   justfile: r#"
 [no-exit-message]
 
 hello:
   @exit 100
 "#,
-  stderr: "error: Expected '@', '[', or identifier, but found end of line\n ——▶ justfile:2:1\n  │\n2 │ \n  │ ^\n",
+  stderr: "
+    error: Extraneous attribute
+     ——▶ justfile:1:1
+      │
+    1 │ [no-exit-message]
+      │ ^
+  ",
   status: EXIT_FAILURE,
 }