From d5ebc9515e797ffbf00530541c8c43dd56e207c0 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sun, 14 Jul 2024 22:15:22 -0700 Subject: [PATCH] Allow `[doc]` annotation on modules (#2247) --- src/analyzer.rs | 27 +++++++++++++++------ src/compile_error.rs | 19 +++++++++------ src/compile_error_kind.rs | 12 ++++++---- src/item.rs | 1 + src/justfile.rs | 2 +- src/parser.rs | 45 +++++++++++++++++++++-------------- src/subcommand.rs | 2 +- tests/modules.rs | 50 +++++++++++++++++++++++++++++++++++++++ tests/no_exit_message.rs | 20 ++++++++++------ 9 files changed, 133 insertions(+), 45 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 48c17c7..b277b42 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -10,7 +10,7 @@ pub(crate) struct Analyzer<'src> { impl<'src> Analyzer<'src> { pub(crate) fn analyze( asts: &HashMap>, - doc: Option<&'src str>, + doc: Option, loaded: &[PathBuf], name: Option>, paths: &HashMap, @@ -22,7 +22,7 @@ impl<'src> Analyzer<'src> { fn justfile( mut self, asts: &HashMap>, - doc: Option<&'src str>, + doc: Option, loaded: &[PathBuf], name: Option>, paths: &HashMap, @@ -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(), })); } diff --git a/src/compile_error.rs b/src/compile_error.rs index 6d22704..1056f37 100644 --- a/src/compile_error.rs +++ b/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", diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs index 36a9843..7400b29 100644 --- a/src/compile_error_kind.rs +++ b/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, }, diff --git a/src/item.rs b/src/item.rs index 72c9185..08d723f 100644 --- a/src/item.rs +++ b/src/item.rs @@ -13,6 +13,7 @@ pub(crate) enum Item<'src> { relative: StringLiteral<'src>, }, Module { + attributes: BTreeSet>, absolute: Option, doc: Option<&'src str>, name: Name<'src>, diff --git a/src/justfile.rs b/src/justfile.rs index 52d9f01..90bfc5e 100644 --- a/src/justfile.rs +++ b/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, #[serde(rename = "first", serialize_with = "keyed::serialize_option")] pub(crate) default: Option>>, #[serde(skip)] diff --git a/src/parser.rs b/src/parser.rs index 6a18d5f..f337b90 100644 --- a/src/parser.rs +++ b/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>>> { + fn parse_attributes( + &mut self, + ) -> CompileResult<'src, Option<(Token<'src>, BTreeSet>)>> { 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()))) } } } diff --git a/src/subcommand.rs b/src/subcommand.rs index c9cce97..344667e 100644 --- a/src/subcommand.rs +++ b/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, ); diff --git a/tests/modules.rs b/tests/modules.rs index b01957d..14c3c01 100644 --- a/tests/modules.rs +++ b/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(); +} diff --git a/tests/no_exit_message.rs b/tests/no_exit_message.rs index 8e45607..6d27b25 100644 --- a/tests/no_exit_message.rs +++ b/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, }