From 58aea8af0612459e27626353723a659b6582ca5f Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Wed, 22 Jan 2025 16:06:58 -0500 Subject: [PATCH 1/7] Add alias attribute --- src/alias.rs | 2 +- src/analyzer.rs | 12 ++++++++++++ src/attribute.rs | 31 ++++++++++++++++++++++++++++--- src/attribute_set.rs | 6 +++++- src/parser.rs | 4 ++-- 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/alias.rs b/src/alias.rs index 18280546fe..904a014af5 100644 --- a/src/alias.rs +++ b/src/alias.rs @@ -1,7 +1,7 @@ use super::*; /// An alias, e.g. `name := target` -#[derive(Debug, PartialEq, Clone, Serialize)] +#[derive(Debug, PartialEq, Ord, PartialOrd, Eq, Clone, Serialize)] pub(crate) struct Alias<'src, T = Rc>> { pub(crate) attributes: AttributeSet<'src>, pub(crate) name: Name<'src>, diff --git a/src/analyzer.rs b/src/analyzer.rs index c2c3da67ec..78f049b535 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -86,6 +86,18 @@ impl<'run, 'src> Analyzer<'run, 'src> { if recipe.enabled() { Self::analyze_recipe(recipe)?; self.recipes.push(recipe); + + for attribute in &recipe.attributes { + if let Attribute::Alias(name) = attribute { + Self::define(&mut definitions, *name, "alias", false)?; + let alias = Alias { + name: *name, + target: recipe.name, + attributes: AttributeSet::new(), + }; + self.aliases.insert(alias); + } + } } } Item::Set(set) => { diff --git a/src/attribute.rs b/src/attribute.rs index fb2fef4f89..95048e8c03 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -9,6 +9,7 @@ use super::*; #[strum_discriminants(derive(EnumString, Ord, PartialOrd))] #[strum_discriminants(strum(serialize_all = "kebab-case"))] pub(crate) enum Attribute<'src> { + Alias(Name<'src>), Confirm(Option>), Doc(Option>), ExitMessage, @@ -32,7 +33,7 @@ impl AttributeDiscriminant { fn argument_range(self) -> RangeInclusive { match self { Self::Confirm | Self::Doc => 0..=1, - Self::Group | Self::Extension | Self::WorkingDirectory => 1..=1, + Self::Alias | Self::Group | Self::Extension | Self::WorkingDirectory => 1..=1, Self::ExitMessage | Self::Linux | Self::Macos @@ -52,7 +53,7 @@ impl AttributeDiscriminant { impl<'src> Attribute<'src> { pub(crate) fn new( name: Name<'src>, - arguments: Vec>, + arguments: Vec<(Token<'src>, StringLiteral<'src>)>, ) -> CompileResult<'src, Self> { let discriminant = name .lexeme() @@ -77,7 +78,30 @@ impl<'src> Attribute<'src> { ); } + let alias_args = arguments.clone(); + let arguments = arguments.into_iter().map(|(_, arg)| arg); Ok(match discriminant { + AttributeDiscriminant::Alias => Self::Alias({ + let (token, string_literal) = alias_args.into_iter().next().unwrap(); + let delim = string_literal.kind.delimiter_len(); + let token = Token { + kind: TokenKind::Identifier, + column: token.column + delim, + length: token.length - (delim * 2), + offset: token.offset + delim, + ..token + }; + + if !token + .lexeme() + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_') + { + todo!("alias is not a valid identifier") + } + + Name::from_identifier(token) + }), AttributeDiscriminant::Confirm => Self::Confirm(arguments.into_iter().next()), AttributeDiscriminant::Doc => Self::Doc(arguments.into_iter().next()), AttributeDiscriminant::ExitMessage => Self::ExitMessage, @@ -115,7 +139,7 @@ impl<'src> Attribute<'src> { } pub(crate) fn repeatable(&self) -> bool { - matches!(self, Attribute::Group(_)) + matches!(self, Attribute::Group(_) | Attribute::Alias(_)) } } @@ -124,6 +148,7 @@ impl Display for Attribute<'_> { write!(f, "{}", self.name())?; match self { + Self::Alias(argument) => write!(f, "({argument})")?, Self::Confirm(Some(argument)) | Self::Doc(Some(argument)) | Self::Extension(argument) diff --git a/src/attribute_set.rs b/src/attribute_set.rs index a40b001d1a..d8504e9c70 100644 --- a/src/attribute_set.rs +++ b/src/attribute_set.rs @@ -1,9 +1,13 @@ use {super::*, std::collections}; -#[derive(Default, Debug, Clone, PartialEq, Serialize)] +#[derive(Default, Debug, Clone, PartialEq, PartialOrd, Ord, Eq, Serialize)] pub(crate) struct AttributeSet<'src>(BTreeSet>); impl<'src> AttributeSet<'src> { + pub(crate) fn new() -> Self { + Self(BTreeSet::new()) + } + pub(crate) fn len(&self) -> usize { self.0.len() } diff --git a/src/parser.rs b/src/parser.rs index 9305a42db5..f3ea34d2ed 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1181,10 +1181,10 @@ impl<'run, 'src> Parser<'run, 'src> { let mut arguments = Vec::new(); if self.accepted(Colon)? { - arguments.push(self.parse_string_literal()?); + arguments.push(self.parse_string_literal_token()?); } else if self.accepted(ParenL)? { loop { - arguments.push(self.parse_string_literal()?); + arguments.push(self.parse_string_literal_token()?); if !self.accepted(Comma)? { break; From 00d12306d0d6e1b1b21e285b3eb62b769ab8116c Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sat, 1 Feb 2025 12:40:27 -0500 Subject: [PATCH 2/7] Handle invalid aliases in alias attribute --- src/attribute.rs | 13 +++++++------ src/compile_error.rs | 4 ++++ src/compile_error_kind.rs | 3 +++ 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index 95048e8c03..9f653585fb 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -92,12 +92,13 @@ impl<'src> Attribute<'src> { ..token }; - if !token - .lexeme() - .chars() - .all(|c| c.is_ascii_alphanumeric() || c == '_') - { - todo!("alias is not a valid identifier") + let alias = token.lexeme(); + let valid_alias = alias.chars().all(|c| c.is_ascii_alphabetic() || c == '_'); + + if alias.is_empty() || !valid_alias { + return Err(token.error(CompileErrorKind::InvalidAliasName { + name: token.lexeme(), + })); } Name::from_identifier(token) diff --git a/src/compile_error.rs b/src/compile_error.rs index b09a1e5eb3..434566b8f6 100644 --- a/src/compile_error.rs +++ b/src/compile_error.rs @@ -170,6 +170,10 @@ 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" ), + InvalidAliasName { name } => write!( + f, + "`{name}` is not a valid alias. Aliases must only contain alphanumeric characters and underscores." + ), InvalidAttribute { item_name, item_kind, diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs index ca6f6fa7a6..b823fb9485 100644 --- a/src/compile_error_kind.rs +++ b/src/compile_error_kind.rs @@ -74,6 +74,9 @@ pub(crate) enum CompileErrorKind<'src> { Internal { message: String, }, + InvalidAliasName { + name: &'src str, + }, InvalidAttribute { item_kind: &'static str, item_name: &'src str, From d1f3c6445e0509d219dfc9db1b7401648416fb37 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sat, 1 Feb 2025 13:11:16 -0500 Subject: [PATCH 3/7] Refactor attribute argument assignment --- src/attribute.rs | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/attribute.rs b/src/attribute.rs index 9f653585fb..69bac88e26 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -78,12 +78,16 @@ impl<'src> Attribute<'src> { ); } - let alias_args = arguments.clone(); - let arguments = arguments.into_iter().map(|(_, arg)| arg); + let mut arguments = arguments.into_iter(); + let (token, argument) = arguments + .next() + .map(|(token, arg)| (Some(token), Some(arg))) + .unwrap_or_default(); + Ok(match discriminant { AttributeDiscriminant::Alias => Self::Alias({ - let (token, string_literal) = alias_args.into_iter().next().unwrap(); - let delim = string_literal.kind.delimiter_len(); + let delim = argument.unwrap().kind.delimiter_len(); + let token = token.unwrap(); let token = Token { kind: TokenKind::Identifier, column: token.column + delim, @@ -103,11 +107,11 @@ impl<'src> Attribute<'src> { Name::from_identifier(token) }), - AttributeDiscriminant::Confirm => Self::Confirm(arguments.into_iter().next()), - AttributeDiscriminant::Doc => Self::Doc(arguments.into_iter().next()), + AttributeDiscriminant::Confirm => Self::Confirm(argument), + AttributeDiscriminant::Doc => Self::Doc(argument), AttributeDiscriminant::ExitMessage => Self::ExitMessage, - AttributeDiscriminant::Extension => Self::Extension(arguments.into_iter().next().unwrap()), - AttributeDiscriminant::Group => Self::Group(arguments.into_iter().next().unwrap()), + AttributeDiscriminant::Extension => Self::Extension(argument.unwrap()), + AttributeDiscriminant::Group => Self::Group(argument.unwrap()), AttributeDiscriminant::Linux => Self::Linux, AttributeDiscriminant::Macos => Self::Macos, AttributeDiscriminant::NoCd => Self::NoCd, @@ -117,17 +121,14 @@ impl<'src> Attribute<'src> { AttributeDiscriminant::PositionalArguments => Self::PositionalArguments, AttributeDiscriminant::Private => Self::Private, AttributeDiscriminant::Script => Self::Script({ - let mut arguments = arguments.into_iter(); - arguments.next().map(|command| Interpreter { + argument.map(|command| Interpreter { command, - arguments: arguments.collect(), + arguments: arguments.map(|(_, arg)| arg).collect(), }) }), AttributeDiscriminant::Unix => Self::Unix, AttributeDiscriminant::Windows => Self::Windows, - AttributeDiscriminant::WorkingDirectory => { - Self::WorkingDirectory(arguments.into_iter().next().unwrap()) - } + AttributeDiscriminant::WorkingDirectory => Self::WorkingDirectory(argument.unwrap()), }) } From 4e651d87b7b3ef623a7cf2c1e1757140ecd6a9ef Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sat, 1 Feb 2025 13:12:29 -0500 Subject: [PATCH 4/7] Remove variable assignment --- src/analyzer.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index 78f049b535..db24cb56bd 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -90,12 +90,11 @@ impl<'run, 'src> Analyzer<'run, 'src> { for attribute in &recipe.attributes { if let Attribute::Alias(name) = attribute { Self::define(&mut definitions, *name, "alias", false)?; - let alias = Alias { + self.aliases.insert(Alias { name: *name, target: recipe.name, attributes: AttributeSet::new(), - }; - self.aliases.insert(alias); + }); } } } From ffb84d0f984f8c756d7cdfe66ec519e3acc29907 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sat, 1 Feb 2025 15:55:47 -0500 Subject: [PATCH 5/7] Store alias StringLiteral as well for dumping --- src/analyzer.rs | 2 +- src/attribute.rs | 17 +++++++++-------- tests/attributes.rs | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/analyzer.rs b/src/analyzer.rs index db24cb56bd..e12ae225a0 100644 --- a/src/analyzer.rs +++ b/src/analyzer.rs @@ -88,7 +88,7 @@ impl<'run, 'src> Analyzer<'run, 'src> { self.recipes.push(recipe); for attribute in &recipe.attributes { - if let Attribute::Alias(name) = attribute { + if let Attribute::Alias(name, _) = attribute { Self::define(&mut definitions, *name, "alias", false)?; self.aliases.insert(Alias { name: *name, diff --git a/src/attribute.rs b/src/attribute.rs index 69bac88e26..974c111bda 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -9,7 +9,7 @@ use super::*; #[strum_discriminants(derive(EnumString, Ord, PartialOrd))] #[strum_discriminants(strum(serialize_all = "kebab-case"))] pub(crate) enum Attribute<'src> { - Alias(Name<'src>), + Alias(Name<'src>, StringLiteral<'src>), Confirm(Option>), Doc(Option>), ExitMessage, @@ -85,8 +85,9 @@ impl<'src> Attribute<'src> { .unwrap_or_default(); Ok(match discriminant { - AttributeDiscriminant::Alias => Self::Alias({ - let delim = argument.unwrap().kind.delimiter_len(); + AttributeDiscriminant::Alias => { + let string_literal = argument.unwrap(); + let delim = string_literal.kind.delimiter_len(); let token = token.unwrap(); let token = Token { kind: TokenKind::Identifier, @@ -105,8 +106,8 @@ impl<'src> Attribute<'src> { })); } - Name::from_identifier(token) - }), + Self::Alias(Name::from_identifier(token), string_literal) + } AttributeDiscriminant::Confirm => Self::Confirm(argument), AttributeDiscriminant::Doc => Self::Doc(argument), AttributeDiscriminant::ExitMessage => Self::ExitMessage, @@ -141,7 +142,7 @@ impl<'src> Attribute<'src> { } pub(crate) fn repeatable(&self) -> bool { - matches!(self, Attribute::Group(_) | Attribute::Alias(_)) + matches!(self, Attribute::Group(_) | Attribute::Alias(_, _)) } } @@ -150,8 +151,8 @@ impl Display for Attribute<'_> { write!(f, "{}", self.name())?; match self { - Self::Alias(argument) => write!(f, "({argument})")?, - Self::Confirm(Some(argument)) + Self::Alias(_, argument) + | Self::Confirm(Some(argument)) | Self::Doc(Some(argument)) | Self::Extension(argument) | Self::Group(argument) diff --git a/tests/attributes.rs b/tests/attributes.rs index 80393f1aa2..e5a0927395 100644 --- a/tests/attributes.rs +++ b/tests/attributes.rs @@ -254,3 +254,17 @@ fn duplicate_non_repeatable_attributes_are_forbidden() { .status(EXIT_FAILURE) .run(); } + +#[test] +fn aliases_can_be_defined_as_attributes() { + Test::new() + .justfile( + " + [alias('bar')] + baz: + ", + ) + .arg("bar") + .status(EXIT_SUCCESS) + .run(); +} From ddfcd5f228dbbac8327952464e5b45319a664bee Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sat, 1 Feb 2025 16:13:00 -0500 Subject: [PATCH 6/7] Fix alias attribute validation --- src/attribute.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/attribute.rs b/src/attribute.rs index 974c111bda..1e99086779 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -98,7 +98,7 @@ impl<'src> Attribute<'src> { }; let alias = token.lexeme(); - let valid_alias = alias.chars().all(|c| c.is_ascii_alphabetic() || c == '_'); + let valid_alias = alias.chars().all(|c| c.is_ascii_alphanumeric() || c == '_'); if alias.is_empty() || !valid_alias { return Err(token.error(CompileErrorKind::InvalidAliasName { From 685bf6e7465e7e8b899e0c6323b7688c1c3dffb0 Mon Sep 17 00:00:00 2001 From: Marc Addeo Date: Sat, 1 Feb 2025 16:19:27 -0500 Subject: [PATCH 7/7] Add tests --- tests/attributes.rs | 154 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 151 insertions(+), 3 deletions(-) diff --git a/tests/attributes.rs b/tests/attributes.rs index e5a0927395..b755b89f0a 100644 --- a/tests/attributes.rs +++ b/tests/attributes.rs @@ -260,11 +260,159 @@ fn aliases_can_be_defined_as_attributes() { Test::new() .justfile( " - [alias('bar')] - baz: - ", + [alias('bar')] + baz: + ", ) .arg("bar") .status(EXIT_SUCCESS) .run(); } + +#[test] +fn multiple_aliases_can_be_defined_as_attributes() { + Test::new() + .justfile( + " + [alias('bar')] + [alias('foo')] + baz: + ", + ) + .arg("foo") + .status(EXIT_SUCCESS) + .run(); +} + +#[test] +fn duplicate_alias_attributes_are_forbidden() { + Test::new() + .justfile( + " + [alias('foo')] + [alias('foo')] + baz: + ", + ) + .arg("foo") + .stderr( + " + error: Alias `foo` first defined on line 1 is redefined on line 2 + ——▶ justfile:2:9 + │ + 2 │ [alias('foo')] + │ ^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} + +#[test] +fn alias_attributes_duplicating_alias_definition_is_forbidden() { + Test::new() + .justfile( + " + alias foo := baz + [alias('foo')] + baz: + ", + ) + .arg("foo") + .stderr( + " + error: Alias `foo` first defined on line 1 is redefined on line 2 + ——▶ justfile:2:9 + │ + 2 │ [alias('foo')] + │ ^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} + +#[test] +fn alias_definitions_duplicating_alias_attributes_is_forbidden() { + Test::new() + .justfile( + " + [alias('foo')] + baz: + + alias foo := baz + ", + ) + .arg("foo") + .stderr( + " + error: Alias `foo` first defined on line 1 is redefined on line 4 + ——▶ justfile:4:7 + │ + 4 │ alias foo := baz + │ ^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} + +#[test] +fn alphanumeric_and_underscores_are_valid_alias_attributes() { + Test::new() + .justfile( + " + [alias('alpha_numeric_123')] + baz: + ", + ) + .arg("alpha_numeric_123") + .status(EXIT_SUCCESS) + .run(); +} + +#[test] +fn nonalphanumeric_alias_attribute_is_forbidden() { + Test::new() + .justfile( + " + [alias('invalid name!')] + baz: + ", + ) + .arg("foo") + .stderr( + " + error: `invalid name!` is not a valid alias. Aliases must only contain alphanumeric characters and underscores. + ——▶ justfile:1:9 + │ + 1 │ [alias('invalid name!')] + │ ^^^^^^^^^^^^^ + ", + ) + .status(EXIT_FAILURE) + .run(); +} + +#[test] +fn empty_alias_attribute_is_forbidden() { + Test::new() + .justfile( + " + [alias('')] + baz: + ", + ) + .arg("baz") + .stderr( + " + error: `` is not a valid alias. Aliases must only contain alphanumeric characters and underscores. + ——▶ justfile:1:9 + │ + 1 │ [alias('')] + │ ^ + ", + ) + .status(EXIT_FAILURE) + .run(); +}