Allow long defaults to parameter name (#3041)

Author: casey

README.md

@@ -2842,6 +2842,14 @@ $ just foo --bar=hello
 bar=hello
 ```
 
+The value of `long` can be omitted, in which case the option defaults to the
+name of the parameter:
+
+```just
+[arg("bar", long)]
+foo bar:
+```
+
 The `[arg(ARG, short=OPTION)]`<sup>master</sup> attribute can be used to make a
 parameter a short option.
 

src/attribute.rs

@@ -13,6 +13,8 @@ pub(crate) enum Attribute<'src> {
   Arg {
     help: Option<StringLiteral<'src>>,
     long: Option<StringLiteral<'src>>,
+    #[serde(skip)]
+    long_key: Option<Token<'src>>,
     name: StringLiteral<'src>,
     pattern: Option<Pattern<'src>>,
     short: Option<StringLiteral<'src>>,
@@ -91,7 +93,7 @@ impl<'src> Attribute<'src> {
   pub(crate) fn new(
     name: Name<'src>,
     arguments: Vec<StringLiteral<'src>>,
-    mut keyword_arguments: BTreeMap<&'src str, (Name<'src>, StringLiteral<'src>)>,
+    mut keyword_arguments: BTreeMap<&'src str, (Name<'src>, Option<StringLiteral<'src>>)>,
   ) -> CompileResult<'src, Self> {
     let discriminant = name
       .lexeme()
@@ -117,25 +119,29 @@ impl<'src> Attribute<'src> {
 
     let attribute = match discriminant {
       AttributeDiscriminant::Arg => {
-        let name = arguments.into_iter().next().unwrap();
+        let arg = arguments.into_iter().next().unwrap();
 
-        let long = keyword_arguments
+        let (long, long_key) = keyword_arguments
           .remove("long")
-          .map(|(_name, literal)| {
-            Self::check_option_name(&name, &literal)?;
-            Ok(literal)
+          .map(|(name, literal)| {
+            if let Some(literal) = literal {
+              Self::check_option_name(&arg, &literal)?;
+              Ok((Some(literal), None))
+            } else {
+              Ok((Some(arg.clone()), Some(*name)))
+            }
           })
-          .transpose()?;
+          .transpose()?
+          .unwrap_or((None, None));
 
-        let short = keyword_arguments
-          .remove("short")
-          .map(|(_name, literal)| {
-            Self::check_option_name(&name, &literal)?;
+        let short = Self::remove_required(&mut keyword_arguments, "short")?
+          .map(|(_key, literal)| {
+            Self::check_option_name(&arg, &literal)?;
 
             if literal.cooked.chars().count() != 1 {
               return Err(literal.token.error(
                 CompileErrorKind::ShortOptionWithMultipleCharacters {
-                  parameter: name.cooked.clone(),
+                  parameter: arg.cooked.clone(),
                 },
               ));
             }
@@ -144,29 +150,27 @@ impl<'src> Attribute<'src> {
           })
           .transpose()?;
 
-        let pattern = keyword_arguments
-          .remove("pattern")
-          .map(|(_name, literal)| Pattern::new(&literal))
+        let pattern = Self::remove_required(&mut keyword_arguments, "pattern")?
+          .map(|(_key, literal)| Pattern::new(&literal))
           .transpose()?;
 
-        let value = keyword_arguments
-          .remove("value")
-          .map(|(name, literal)| {
+        let value = Self::remove_required(&mut keyword_arguments, "value")?
+          .map(|(key, literal)| {
             if long.is_none() && short.is_none() {
-              return Err(name.error(CompileErrorKind::ArgAttributeValueRequiresOption));
+              return Err(key.error(CompileErrorKind::ArgAttributeValueRequiresOption));
             }
             Ok(literal)
           })
           .transpose()?;
 
-        let help = keyword_arguments
-          .remove("help")
-          .map(|(_name, literal)| literal);
+        let help =
+          Self::remove_required(&mut keyword_arguments, "help")?.map(|(_key, literal)| literal);
 
         Self::Arg {
           help,
           long,
-          name,
+          long_key,
+          name: arg,
           pattern,
           short,
           value,
@@ -214,6 +218,20 @@ impl<'src> Attribute<'src> {
     Ok(attribute)
   }
 
+  fn remove_required(
+    keyword_arguments: &mut BTreeMap<&'src str, (Name<'src>, Option<StringLiteral<'src>>)>,
+    key: &'src str,
+  ) -> CompileResult<'src, Option<(Name<'src>, StringLiteral<'src>)>> {
+    let Some((key, literal)) = keyword_arguments.remove(key) else {
+      return Ok(None);
+    };
+
+    let literal =
+      literal.ok_or_else(|| key.error(CompileErrorKind::AttributeKeyMissingValue { key }))?;
+
+    Ok(Some((key, literal)))
+  }
+
   pub(crate) fn discriminant(&self) -> AttributeDiscriminant {
     self.into()
   }
@@ -238,6 +256,7 @@ impl Display for Attribute<'_> {
       Self::Arg {
         help,
         long,
+        long_key: _,
         name,
         pattern,
         short,

src/compile_error.rs

@@ -337,6 +337,12 @@ impl Display for CompileError<'_> {
       UnknownAliasTarget { alias, target } => {
         write!(f, "Alias `{alias}` has an unknown target `{target}`")
       }
+      AttributeKeyMissingValue { key } => {
+        write!(
+          f,
+          "Attribute key `{key}` requires value",
+        )
+      }
       UnknownAttributeKeyword { attribute, keyword } => {
         write!(f, "Unknown keyword `{keyword}` for `{attribute}` attribute")
       }

src/compile_error_kind.rs

@@ -12,6 +12,9 @@ pub(crate) enum CompileErrorKind<'src> {
     min: usize,
     max: usize,
   },
+  AttributeKeyMissingValue {
+    key: Name<'src>,
+  },
   AttributePositionalFollowsKeyword,
   BacktickShebang,
   CircularRecipeDependency {

src/parser.rs

@@ -1022,6 +1022,7 @@ impl<'run, 'src> Parser<'run, 'src> {
       let Attribute::Arg {
         help,
         long,
+        long_key,
         name: arg,
         pattern,
         short,
@@ -1034,10 +1035,14 @@ impl<'run, 'src> Parser<'run, 'src> {
 
       if let Some(option) = long {
         if !longs.insert(&option.cooked) {
-          return Err(option.token.error(CompileErrorKind::DuplicateOption {
-            option: Switch::Long(option.cooked.clone()),
-            recipe: name.lexeme(),
-          }));
+          return Err(
+            long_key
+              .unwrap_or(option.token)
+              .error(CompileErrorKind::DuplicateOption {
+                option: Switch::Long(option.cooked.clone()),
+                recipe: name.lexeme(),
+              }),
+          );
         }
       }
 
@@ -1385,13 +1390,14 @@ impl<'run, 'src> Parser<'run, 'src> {
         } else if self.accepted(ParenL)? {
           loop {
             if self.next_is(Identifier) && !self.next_is_shell_expanded_string() {
-              let name = self.parse_name()?;
-
-              self.expect(Equals)?;
+              let key = self.parse_name()?;
 
-              let value = self.parse_string_literal()?;
+              let value = self
+                .accepted(Equals)?
+                .then(|| self.parse_string_literal())
+                .transpose()?;
 
-              keyword_arguments.insert(name.lexeme(), (name, value));
+              keyword_arguments.insert(key.lexeme(), (key, value));
             } else {
               let literal = self.parse_string_literal()?;
 

tests/arg_attribute.rs

@@ -354,3 +354,69 @@ fn pattern_mismatch_variadic() {
     .status(EXIT_FAILURE)
     .run();
 }
+
+#[test]
+fn pattern_requires_value() {
+  Test::new()
+    .justfile(
+      "
+        [arg('bar', pattern)]
+        foo bar:
+      ",
+    )
+    .stderr(
+      "
+        error: Attribute key `pattern` requires value
+         ——▶ justfile:1:13
+          │
+        1 │ [arg('bar', pattern)]
+          │             ^^^^^^^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+#[test]
+fn short_requires_value() {
+  Test::new()
+    .justfile(
+      "
+        [arg('bar', short)]
+        foo bar:
+      ",
+    )
+    .stderr(
+      "
+        error: Attribute key `short` requires value
+         ——▶ justfile:1:13
+          │
+        1 │ [arg('bar', short)]
+          │             ^^^^^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+#[test]
+fn value_requires_value() {
+  Test::new()
+    .justfile(
+      "
+        [arg('bar', long, value)]
+        foo bar:
+      ",
+    )
+    .stderr(
+      "
+        error: Attribute key `value` requires value
+         ——▶ justfile:1:19
+          │
+        1 │ [arg('bar', long, value)]
+          │                   ^^^^^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}

tests/options.rs

@@ -84,6 +84,21 @@ fn parameters_may_be_passed_with_long_options() {
     .run();
 }
 
+#[test]
+fn long_option_defaults_to_parameter_name() {
+  Test::new()
+    .justfile(
+      "
+        [arg('bar', long)]
+        @foo bar:
+          echo bar={{bar}}
+      ",
+    )
+    .args(["foo", "--bar", "baz"])
+    .stdout("bar=baz\n")
+    .run();
+}
+
 #[test]
 fn parameters_may_be_passed_with_short_options() {
   Test::new()
@@ -172,6 +187,32 @@ fn duplicate_long_option_attributes_are_forbidden() {
     .run();
 }
 
+#[test]
+fn defaulted_duplicate_long_option() {
+  Test::new()
+    .justfile(
+      "
+        [arg(
+          'aaa',
+          long='bar'
+        )]
+        [arg(      'bar', long)]
+        foo aaa bar:
+      ",
+    )
+    .stderr(
+      "
+        error: Recipe `foo` defines option `--bar` multiple times
+         ——▶ justfile:5:19
+          │
+        5 │ [arg(      'bar', long)]
+          │                   ^^^^
+      ",
+    )
+    .status(EXIT_FAILURE)
+    .run();
+}
+
 #[test]
 fn duplicate_short_option_attributes_are_forbidden() {
   Test::new()