From 886acf2f953fab3ce5bc2c968c17c1a5a105907b Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Sat, 12 Nov 2016 09:15:13 -0800 Subject: [PATCH] Let recipes take default arguments (#77) Looks like this: ```make recipe argument default-argument='default value': echo argument is {{argument}} echo default-argument is {{default-argument}} ``` Thanks @deckarep for the feature request! Fixes #49 --- GRAMMAR.md | 6 +- README.md | 24 +++++ src/integration.rs | 102 +++++++++++++++++++++ src/lib.rs | 216 ++++++++++++++++++++++++++++++++------------- src/unit.rs | 134 ++++++++++++++++++++++++++-- 5 files changed, 414 insertions(+), 68 deletions(-) diff --git a/GRAMMAR.md b/GRAMMAR.md index 231d8d1..f5f8e65 100644 --- a/GRAMMAR.md +++ b/GRAMMAR.md @@ -48,9 +48,11 @@ expression : STRING | BACKTICK | expression '+' expression -recipe : NAME arguments? ':' dependencies? body? +recipe : NAME argument* ':' dependencies? body? -arguments : NAME+ +argument : NAME + | NAME '=' STRING + | NAME '=' RAW_STRING dependencies : NAME+ diff --git a/README.md b/README.md index 28c9694..e282ca1 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,30 @@ Building my-awesome-project... cd my-awesome-project && make ``` +Parameters may have default values: + +```make +test target tests='all': + @echo 'Testing {{target}}:{{tests}}...' + ./test --tests {{tests}} {{target}} +``` + +Parameters with default values may be omitted: + +```sh +$ just test server +Testing server:all... +./test --tests all server +``` + +Or supplied: + +```sh +$ just test server unit +Testing server:unit... +./test --tests unit server +``` + Variables can be exported to recipes as environment variables: ```make diff --git a/src/integration.rs b/src/integration.rs index 8f1a6c6..0dfae43 100644 --- a/src/integration.rs +++ b/src/integration.rs @@ -862,6 +862,7 @@ foo A B: "error: Recipe `foo` got 3 arguments but only takes 2\n", ); } + #[test] fn argument_mismatch_fewer() { integration_test( @@ -876,6 +877,34 @@ foo A B: ); } +#[test] +fn argument_mismatch_more_with_default() { + integration_test( + &["foo", "ONE", "TWO", "THREE"], + " +foo A B='B': + echo A:{{A}} B:{{B}} + ", + 255, + "", + "error: Recipe `foo` got 3 arguments but takes at most 2\n", + ); +} + +#[test] +fn argument_mismatch_fewer_with_default() { + integration_test( + &["foo", "bar"], + " +foo A B C='C': + echo A:{{A}} B:{{B}} C:{{C}} + ", + 255, + "", + "error: Recipe `foo` got 1 argument but takes at least 2\n", + ); +} + #[test] fn unknown_recipe() { integration_test( @@ -969,3 +998,76 @@ recipe: ); } +#[test] +fn required_after_default() { + integration_test( + &[], + "bar:\nhello baz arg='foo' bar:", + 255, + "", + "error: non-default parameter `bar` follows default parameter + | +2 | hello baz arg='foo' bar: + | ^^^ +", + ); +} + +#[test] +fn use_string_default() { + integration_test( + &["hello", "ABC"], + r#" +bar: +hello baz arg="XYZ\t\" ": + echo '{{baz}}...{{arg}}' +"#, + 0, + "ABC...XYZ\t\"\t\n", + "echo 'ABC...XYZ\t\"\t'\n", + ); +} + + +#[test] +fn use_raw_string_default() { + integration_test( + &["hello", "ABC"], + r#" +bar: +hello baz arg='XYZ\t\" ': + echo '{{baz}}...{{arg}}' +"#, + 0, + "ABC...XYZ\t\\\"\t\n", + "echo 'ABC...XYZ\\t\\\"\t'\n", + ); +} + +#[test] +fn supply_use_default() { + integration_test( + &["hello", "0", "1"], + r#" +hello a b='B' c='C': + echo {{a}} {{b}} {{c}} +"#, + 0, + "0 1 C\n", + "echo 0 1 C\n", + ); +} + +#[test] +fn supply_defaults() { + integration_test( + &["hello", "0", "1", "2"], + r#" +hello a b='B' c='C': + echo {{a}} {{b}} {{c}} +"#, + 0, + "0 1 2\n", + "echo 0 1 2\n", + ); +} diff --git a/src/lib.rs b/src/lib.rs index 8834471..db3d314 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,6 +19,7 @@ extern crate unicode_width; use std::io::prelude::*; use std::{fs, fmt, process, io}; +use std::ops::Range; use std::fmt::Display; use regex::Regex; use std::collections::{BTreeMap as Map, BTreeSet as Set}; @@ -57,6 +58,10 @@ fn re(pattern: &str) -> Regex { Regex::new(pattern).unwrap() } +fn contains(range: &Range, i: T) -> bool { + i >= range.start && i < range.end +} + #[derive(PartialEq, Debug)] struct Recipe<'a> { line_number: usize, @@ -64,11 +69,27 @@ struct Recipe<'a> { lines: Vec>>, dependencies: Vec<&'a str>, dependency_tokens: Vec>, - parameters: Vec<&'a str>, - parameter_tokens: Vec>, + parameters: Vec>, shebang: bool, } +#[derive(PartialEq, Debug)] +struct Parameter<'a> { + name: &'a str, + default: Option, + token: Token<'a>, +} + +impl<'a> Display for Parameter<'a> { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!(f, "{}", self.name)?; + if let Some(ref default) = self.default { + write!(f, r#"="{}""#, default)?; + } + Ok(()) + } +} + #[derive(PartialEq, Debug)] enum Fragment<'a> { Text{text: Token<'a>}, @@ -78,7 +99,7 @@ enum Fragment<'a> { #[derive(PartialEq, Debug)] enum Expression<'a> { Variable{name: &'a str, token: Token<'a>}, - String{raw: &'a str, cooked: String}, + String{cooked_string: CookedString<'a>}, Backtick{raw: &'a str, token: Token<'a>}, Concatination{lhs: Box>, rhs: Box>}, } @@ -114,10 +135,10 @@ impl<'a> Iterator for Variables<'a> { impl<'a> Display for Expression<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { match *self { - Expression::Backtick {raw, .. } => write!(f, "`{}`", raw)?, - Expression::Concatination{ref lhs, ref rhs} => write!(f, "{} + {}", lhs, rhs)?, - Expression::String {raw, .. } => write!(f, "\"{}\"", raw)?, - Expression::Variable {name, .. } => write!(f, "{}", name)?, + Expression::Backtick {raw, .. } => write!(f, "`{}`", raw)?, + Expression::Concatination{ref lhs, ref rhs } => write!(f, "{} + {}", lhs, rhs)?, + Expression::String {ref cooked_string} => write!(f, "\"{}\"", cooked_string.raw)?, + Expression::Variable {name, .. } => write!(f, "{}", name)?, } Ok(()) } @@ -225,6 +246,12 @@ fn run_backtick<'a>( } impl<'a> Recipe<'a> { + fn argument_range(&self) -> Range { + self.parameters.iter().filter(|p| !p.default.is_some()).count() + .. + self.parameters.len() + 1 + } + fn run( &self, arguments: &[&'a str], @@ -232,8 +259,14 @@ impl<'a> Recipe<'a> { exports: &Set<&'a str>, options: &RunOptions, ) -> Result<(), RunError<'a>> { - let argument_map = arguments .iter().enumerate() - .map(|(i, argument)| (self.parameters[i], *argument)).collect(); + let argument_map = self.parameters.iter().enumerate() + .map(|(i, parameter)| if i < arguments.len() { + (parameter.name, arguments[i]) + } else if let Some(ref default) = parameter.default { + (parameter.name, default.as_str()) + } else { + panic!(); // FIXME internal error + }).collect(); let mut evaluator = Evaluator { evaluated: Map::new(), @@ -407,7 +440,9 @@ fn resolve_recipes<'a>( if let Fragment::Expression{ref expression, ..} = *fragment { for variable in expression.variables() { let name = variable.lexeme; - if !(assignments.contains_key(name) || recipe.parameters.contains(&name)) { + let undefined = !assignments.contains_key(name) + && !recipe.parameters.iter().any(|p| p.name == name); + if undefined { // There's a borrow issue here that seems too difficult to solve. // The error derived from the variable token has too short a lifetime, // so we create a new error from its contents, which do live long @@ -644,7 +679,7 @@ impl<'a, 'b> Evaluator<'a, 'b> { }); } } - Expression::String{ref cooked, ..} => cooked.clone(), + Expression::String{ref cooked_string} => cooked_string.cooked.clone(), Expression::Backtick{raw, ref token} => { run_backtick(raw, token, &self.scope, &self.exports, self.quiet)? } @@ -683,6 +718,7 @@ enum ErrorKind<'a> { MixedLeadingWhitespace{whitespace: &'a str}, OuterShebang, ParameterShadowsVariable{parameter: &'a str}, + RequiredParameterFollowsDefaultParameter{parameter: &'a str}, UndefinedVariable{variable: &'a str}, UnexpectedToken{expected: Vec, found: TokenKind}, UnknownDependency{recipe: &'a str, unknown: &'a str}, @@ -729,6 +765,49 @@ fn ticks(ts: &[T]) -> Vec> { ts.iter().map(Tick).collect() } +#[derive(PartialEq, Debug)] +struct CookedString<'a> { + raw: &'a str, + cooked: String, +} + +fn cook_string<'a>(token: &Token<'a>) -> Result, Error<'a>> { + let raw = &token.lexeme[1..token.lexeme.len()-1]; + + if let RawString = token.kind { + Ok(CookedString{raw: raw, cooked: raw.to_string()}) + } else if let StringToken = token.kind { + let mut cooked = String::new(); + let mut escape = false; + for c in raw.chars() { + if escape { + match c { + 'n' => cooked.push('\n'), + 'r' => cooked.push('\r'), + 't' => cooked.push('\t'), + '\\' => cooked.push('\\'), + '"' => cooked.push('"'), + other => return Err(token.error(ErrorKind::InvalidEscapeSequence { + character: other, + })), + } + escape = false; + continue; + } + if c == '\\' { + escape = true; + continue; + } + cooked.push(c); + } + Ok(CookedString{raw: raw, cooked: cooked}) + } else { + Err(token.error(ErrorKind::InternalError{ + message: "cook_string() called on non-string token".to_string() + })) + } +} + struct And<'a, T: 'a + Display>(&'a [T]); struct Or <'a, T: 'a + Display>(&'a [T]); @@ -900,6 +979,9 @@ impl<'a> Display for Error<'a> { ErrorKind::ParameterShadowsVariable{parameter} => { writeln!(f, "parameter `{}` shadows variable of the same name", parameter)?; } + ErrorKind::RequiredParameterFollowsDefaultParameter{parameter} => { + writeln!(f, "non-default parameter `{}` follows default parameter", parameter)?; + } ErrorKind::MixedLeadingWhitespace{whitespace} => { writeln!(f, "found a mix of tabs and spaces in leading whitespace: `{}`\n\ @@ -1011,11 +1093,13 @@ impl<'a, 'b> Justfile<'a> where 'a: 'b { return Err(RunError::NonLeadingRecipeWithParameters{recipe: recipe.name}); } let rest = &arguments[1..]; - if recipe.parameters.len() != rest.len() { + let argument_range = recipe.argument_range(); + if !contains(&argument_range, rest.len()) { return Err(RunError::ArgumentCountMismatch { recipe: recipe.name, - found: rest.len(), - expected: recipe.parameters.len(), + found: rest.len(), + min: argument_range.start, + max: argument_range.end - 1, }); } return self.run_recipe(recipe, rest, &scope, &mut ran, options); @@ -1089,7 +1173,7 @@ impl<'a> Display for Justfile<'a> { #[derive(Debug)] enum RunError<'a> { - ArgumentCountMismatch{recipe: &'a str, found: usize, expected: usize}, + ArgumentCountMismatch{recipe: &'a str, found: usize, min: usize, max: usize}, Code{recipe: &'a str, code: i32}, InternalError{message: String}, IoError{recipe: &'a str, io_error: io::Error}, @@ -1128,10 +1212,19 @@ impl<'a> Display for RunError<'a> { write!(f, "Recipe `{}` takes arguments and so must be the first and only recipe \ specified on the command line", recipe)?; }, - RunError::ArgumentCountMismatch{recipe, found, expected} => { - write!(f, "Recipe `{}` got {} argument{} but {}takes {}", - recipe, found, maybe_s(found), - if expected < found { "only " } else { "" }, expected)?; + RunError::ArgumentCountMismatch{recipe, found, min, max} => { + if min == max { + let expected = min; + write!(f, "Recipe `{}` got {} argument{} but {}takes {}", + recipe, found, maybe_s(found), + if expected < found { "only " } else { "" }, expected)?; + } else if found < min { + write!(f, "Recipe `{}` got {} argument{} but takes at least {}", + recipe, found, maybe_s(found), min)?; + } else if found > max { + write!(f, "Recipe `{}` got {} argument{} but takes at most {}", + recipe, found, maybe_s(found), max)?; + } }, RunError::Code{recipe, code} => { write!(f, "Recipe `{}` failed with exit code {}", recipe, code)?; @@ -1566,6 +1659,15 @@ impl<'a> Parser<'a> { } } + fn accept_any(&mut self, kinds: &[TokenKind]) -> Option> { + for kind in kinds { + if self.peek(*kind) { + return self.tokens.next(); + } + } + None + } + fn accepted(&mut self, kind: TokenKind) -> bool { self.accept(kind).is_some() } @@ -1605,16 +1707,40 @@ impl<'a> Parser<'a> { })); } - let mut parameters = vec![]; - let mut parameter_tokens = vec![]; + let mut parsed_parameter_with_default = false; + let mut parameters: Vec = vec![]; while let Some(parameter) = self.accept(Name) { - if parameters.contains(¶meter.lexeme) { + if parameters.iter().any(|p| p.name == parameter.lexeme) { return Err(parameter.error(ErrorKind::DuplicateParameter { recipe: name.lexeme, parameter: parameter.lexeme })); } - parameters.push(parameter.lexeme); - parameter_tokens.push(parameter); + + let default; + if self.accepted(Equals) { + if let Some(string) = self.accept_any(&[StringToken, RawString]) { + default = Some(cook_string(&string)?.cooked); + } else { + let unexpected = self.tokens.next().unwrap(); + return Err(self.unexpected_token(&unexpected, &[StringToken, RawString])); + } + } else { + default = None + } + + if parsed_parameter_with_default && default.is_none() { + return Err(parameter.error(ErrorKind::RequiredParameterFollowsDefaultParameter{ + parameter: parameter.lexeme, + })); + } + + parsed_parameter_with_default |= default.is_some(); + + parameters.push(Parameter { + name: parameter.lexeme, + default: default, + token: parameter, + }); } if let Some(token) = self.expect(Colon) { @@ -1694,7 +1820,6 @@ impl<'a> Parser<'a> { dependencies: dependencies, dependency_tokens: dependency_tokens, parameters: parameters, - parameter_tokens: parameter_tokens, lines: lines, shebang: shebang, }); @@ -1702,7 +1827,6 @@ impl<'a> Parser<'a> { Ok(()) } - fn expression(&mut self, interpolation: bool) -> Result, Error<'a>> { let first = self.tokens.next().unwrap(); let lhs = match first.kind { @@ -1711,36 +1835,8 @@ impl<'a> Parser<'a> { raw: &first.lexeme[1..first.lexeme.len()-1], token: first }, - RawString => { - let raw = &first.lexeme[1..first.lexeme.len() - 1]; - Expression::String{raw: raw, cooked: raw.to_string()} - } - StringToken => { - let raw = &first.lexeme[1..first.lexeme.len() - 1]; - let mut cooked = String::new(); - let mut escape = false; - for c in raw.chars() { - if escape { - match c { - 'n' => cooked.push('\n'), - 'r' => cooked.push('\r'), - 't' => cooked.push('\t'), - '\\' => cooked.push('\\'), - '"' => cooked.push('"'), - other => return Err(first.error(ErrorKind::InvalidEscapeSequence { - character: other, - })), - } - escape = false; - continue; - } - if c == '\\' { - escape = true; - continue; - } - cooked.push(c); - } - Expression::String{raw: raw, cooked: cooked} + RawString | StringToken => { + Expression::String{cooked_string: cook_string(&first)?} } _ => return Err(self.unexpected_token(&first, &[Name, StringToken])), }; @@ -1820,10 +1916,10 @@ impl<'a> Parser<'a> { resolve_recipes(&self.recipes, &self.assignments, self.text)?; for recipe in self.recipes.values() { - for parameter in &recipe.parameter_tokens { - if self.assignments.contains_key(parameter.lexeme) { - return Err(parameter.error(ErrorKind::ParameterShadowsVariable { - parameter: parameter.lexeme + for parameter in &recipe.parameters { + if self.assignments.contains_key(parameter.token.lexeme) { + return Err(parameter.token.error(ErrorKind::ParameterShadowsVariable { + parameter: parameter.token.lexeme })); } } diff --git a/src/unit.rs b/src/unit.rs index 7da2a73..2620876 100644 --- a/src/unit.rs +++ b/src/unit.rs @@ -255,6 +255,26 @@ fn parse_empty() { ", ""); } +#[test] +fn parse_string_default() { + parse_summary(r#" + +foo a="b\t": + + + "#, r#"foo a="b ":"#); +} + +#[test] +fn parse_raw_string_default() { + parse_summary(r#" + +foo a='b\t': + + + "#, r#"foo a="b\t":"#); +} + #[test] fn parse_export() { parse_summary(r#" @@ -372,6 +392,71 @@ fn missing_colon() { }); } +#[test] +fn missing_default_eol() { + let text = "hello arg=\n"; + parse_error(text, Error { + text: text, + index: 10, + line: 0, + column: 10, + width: Some(1), + kind: ErrorKind::UnexpectedToken{expected: vec![StringToken, RawString], found: Eol}, + }); +} + +#[test] +fn missing_default_eof() { + let text = "hello arg="; + parse_error(text, Error { + text: text, + index: 10, + line: 0, + column: 10, + width: Some(0), + kind: ErrorKind::UnexpectedToken{expected: vec![StringToken, RawString], found: Eof}, + }); +} + +#[test] +fn missing_default_colon() { + let text = "hello arg=:"; + parse_error(text, Error { + text: text, + index: 10, + line: 0, + column: 10, + width: Some(1), + kind: ErrorKind::UnexpectedToken{expected: vec![StringToken, RawString], found: Colon}, + }); +} + +#[test] +fn missing_default_backtick() { + let text = "hello arg=`hello`"; + parse_error(text, Error { + text: text, + index: 10, + line: 0, + column: 10, + width: Some(7), + kind: ErrorKind::UnexpectedToken{expected: vec![StringToken, RawString], found: Backtick}, + }); +} + +#[test] +fn required_after_default() { + let text = "hello arg='foo' bar:"; + parse_error(text, Error { + text: text, + index: 16, + line: 0, + column: 16, + width: Some(3), + kind: ErrorKind::RequiredParameterFollowsDefaultParameter{parameter: "bar"}, + }); +} + #[test] fn missing_eol() { let text = "a b c: z ="; @@ -614,6 +699,15 @@ fn conjoin_and() { assert_eq!("1, 2, 3, and 4", super::And(&[1,2,3,4]).to_string()); } +#[test] +fn range() { + assert!(super::contains(&(0..1), 0)); + assert!(super::contains(&(10..20), 15)); + assert!(!super::contains(&(0..0), 0)); + assert!(!super::contains(&(1..10), 0)); + assert!(!super::contains(&(1..10), 10)); +} + #[test] fn unknown_recipes() { match parse_success("a:\nb:\nc:").run(&["a", "x", "y", "z"], &Default::default()).unwrap_err() { @@ -778,25 +872,53 @@ a return code: } #[test] -fn missing_args() { +fn missing_some_arguments() { match parse_success("a b c d:").run(&["a", "b", "c"], &Default::default()).unwrap_err() { - RunError::ArgumentCountMismatch{recipe, found, expected} => { + RunError::ArgumentCountMismatch{recipe, found, min, max} => { assert_eq!(recipe, "a"); assert_eq!(found, 2); - assert_eq!(expected, 3); + assert_eq!(min, 3); + assert_eq!(max, 3); }, other => panic!("expected an code run error, but got: {}", other), } } #[test] -fn missing_default() { +fn missing_all_arguments() { match parse_success("a b c d:\n echo {{b}}{{c}}{{d}}") .run(&["a"], &Default::default()).unwrap_err() { - RunError::ArgumentCountMismatch{recipe, found, expected} => { + RunError::ArgumentCountMismatch{recipe, found, min, max} => { assert_eq!(recipe, "a"); assert_eq!(found, 0); - assert_eq!(expected, 3); + assert_eq!(min, 3); + assert_eq!(max, 3); + }, + other => panic!("expected an code run error, but got: {}", other), + } +} + +#[test] +fn missing_some_defaults() { + match parse_success("a b c d='hello':").run(&["a", "b"], &Default::default()).unwrap_err() { + RunError::ArgumentCountMismatch{recipe, found, min, max} => { + assert_eq!(recipe, "a"); + assert_eq!(found, 1); + assert_eq!(min, 2); + assert_eq!(max, 3); + }, + other => panic!("expected an code run error, but got: {}", other), + } +} + +#[test] +fn missing_all_defaults() { + match parse_success("a b c='r' d='h':").run(&["a"], &Default::default()).unwrap_err() { + RunError::ArgumentCountMismatch{recipe, found, min, max} => { + assert_eq!(recipe, "a"); + assert_eq!(found, 0); + assert_eq!(min, 1); + assert_eq!(max, 3); }, other => panic!("expected an code run error, but got: {}", other), }