diff --git a/src/uu/test/src/parser.rs b/src/uu/test/src/parser.rs index 5eec781ba..7b7d77469 100644 --- a/src/uu/test/src/parser.rs +++ b/src/uu/test/src/parser.rs @@ -10,6 +10,21 @@ use std::ffi::OsString; use std::iter::Peekable; +/// Represents one of the binary comparison operators for strings, integers, or files +#[derive(Debug, PartialEq)] +pub enum Operator { + String(OsString), + Int(OsString), + File(OsString), +} + +/// Represents one of the unary test operators for strings or files +#[derive(Debug, PartialEq)] +pub enum UnaryOperator { + StrlenOp(OsString), + FiletestOp(OsString), +} + /// Represents a parsed token from a test expression #[derive(Debug, PartialEq)] pub enum Symbol { @@ -17,11 +32,8 @@ pub enum Symbol { Bang, BoolOp(OsString), Literal(OsString), - StringOp(OsString), - IntOp(OsString), - FileOp(OsString), - StrlenOp(OsString), - FiletestOp(OsString), + Op(Operator), + UnaryOp(UnaryOperator), None, } @@ -35,12 +47,14 @@ impl Symbol { "(" => Symbol::LParen, "!" => Symbol::Bang, "-a" | "-o" => Symbol::BoolOp(s), - "=" | "==" | "!=" => Symbol::StringOp(s), - "-eq" | "-ge" | "-gt" | "-le" | "-lt" | "-ne" => Symbol::IntOp(s), - "-ef" | "-nt" | "-ot" => Symbol::FileOp(s), - "-n" | "-z" => Symbol::StrlenOp(s), + "=" | "==" | "!=" => Symbol::Op(Operator::String(s)), + "-eq" | "-ge" | "-gt" | "-le" | "-lt" | "-ne" => Symbol::Op(Operator::Int(s)), + "-ef" | "-nt" | "-ot" => Symbol::Op(Operator::File(s)), + "-n" | "-z" => Symbol::UnaryOp(UnaryOperator::StrlenOp(s)), "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-G" | "-h" | "-k" | "-L" | "-O" - | "-p" | "-r" | "-s" | "-S" | "-t" | "-u" | "-w" | "-x" => Symbol::FiletestOp(s), + | "-p" | "-r" | "-s" | "-S" | "-t" | "-u" | "-w" | "-x" => { + Symbol::UnaryOp(UnaryOperator::FiletestOp(s)) + } _ => Symbol::Literal(s), }, None => Symbol::None, @@ -60,11 +74,11 @@ impl Symbol { Symbol::Bang => OsString::from("!"), Symbol::BoolOp(s) | Symbol::Literal(s) - | Symbol::StringOp(s) - | Symbol::IntOp(s) - | Symbol::FileOp(s) - | Symbol::StrlenOp(s) - | Symbol::FiletestOp(s) => s, + | Symbol::Op(Operator::String(s)) + | Symbol::Op(Operator::Int(s)) + | Symbol::Op(Operator::File(s)) + | Symbol::UnaryOp(UnaryOperator::StrlenOp(s)) + | Symbol::UnaryOp(UnaryOperator::FiletestOp(s)) => s, Symbol::None => panic!(), }) } @@ -78,7 +92,6 @@ impl Symbol { /// /// EXPR → TERM | EXPR BOOLOP EXPR /// TERM → ( EXPR ) -/// TERM → ( ) /// TERM → ! EXPR /// TERM → UOP str /// UOP → STRLEN | FILETEST @@ -113,6 +126,20 @@ impl Parser { Symbol::new(self.tokens.next()) } + /// Consume the next token & verify that it matches the provided value. + /// + /// # Panics + /// + /// Panics if the next token does not match the provided value. + /// + /// TODO: remove panics and convert Parser to return error messages. + fn expect(&mut self, value: &str) { + match self.next_token() { + Symbol::Literal(s) if s == value => (), + _ => panic!("expected ‘{}’", value), + } + } + /// Peek at the next token from the input stream, returning it as a Symbol. /// The stream is unchanged and will return the same Symbol on subsequent /// calls to `next()` or `peek()`. @@ -144,8 +171,7 @@ impl Parser { match symbol { Symbol::LParen => self.lparen(), Symbol::Bang => self.bang(), - Symbol::StrlenOp(_) => self.uop(symbol), - Symbol::FiletestOp(_) => self.uop(symbol), + Symbol::UnaryOp(_) => self.uop(symbol), Symbol::None => self.stack.push(symbol), literal => self.literal(literal), } @@ -154,21 +180,75 @@ impl Parser { /// Parse a (possibly) parenthesized expression. /// /// test has no reserved keywords, so "(" will be interpreted as a literal - /// if it is followed by nothing or a comparison operator OP. + /// in certain cases: + /// + /// * when found at the end of the token stream + /// * when followed by a binary operator that is not _itself_ interpreted + /// as a literal + /// fn lparen(&mut self) { - match self.peek() { - // lparen is a literal when followed by nothing or comparison - Symbol::None | Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) => { + // Look ahead up to 3 tokens to determine if the lparen is being used + // as a grouping operator or should be treated as a literal string + let peek3: Vec = self + .tokens + .clone() + .take(3) + .map(|token| Symbol::new(Some(token))) + .collect(); + + match peek3.as_slice() { + // case 1: lparen is a literal when followed by nothing + [] => self.literal(Symbol::LParen.into_literal()), + + // case 2: error if end of stream is `( ` + [symbol] => { + eprintln!("test: missing argument after ‘{:?}’", symbol); + std::process::exit(2); + } + + // case 3: `( uop )` → parenthesized unary operation; + // this case ensures we don’t get confused by `( -f ) )` + // or `( -f ( )`, for example + [Symbol::UnaryOp(_), _, Symbol::Literal(s)] if s == ")" => { + let symbol = self.next_token(); + self.uop(symbol); + self.expect(")"); + } + + // case 4: binary comparison of literal lparen, e.g. `( != )` + [Symbol::Op(_), Symbol::Literal(s)] | [Symbol::Op(_), Symbol::Literal(s), _] + if s == ")" => + { self.literal(Symbol::LParen.into_literal()); } - // empty parenthetical - Symbol::Literal(s) if s == ")" => {} + + // case 5: after handling the prior cases, any single token inside + // parentheses is a literal, e.g. `( -f )` + [_, Symbol::Literal(s)] | [_, Symbol::Literal(s), _] if s == ")" => { + let symbol = self.next_token(); + self.literal(symbol); + self.expect(")"); + } + + // case 6: two binary ops in a row, treat the first op as a literal + [Symbol::Op(_), Symbol::Op(_), _] => { + let symbol = self.next_token(); + self.literal(symbol); + self.expect(")"); + } + + // case 7: if earlier cases didn’t match, `( op …` + // indicates binary comparison of literal lparen with + // anything _except_ ")" (case 4) + [Symbol::Op(_), _] | [Symbol::Op(_), _, _] => { + self.literal(Symbol::LParen.into_literal()); + } + + // Otherwise, lparen indicates the start of a parenthesized + // expression _ => { self.expr(); - match self.next_token() { - Symbol::Literal(s) if s == ")" => (), - _ => panic!("expected ')'"), - } + self.expect(")"); } } } @@ -192,7 +272,7 @@ impl Parser { /// fn bang(&mut self) { match self.peek() { - Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) | Symbol::BoolOp(_) => { + Symbol::Op(_) | Symbol::BoolOp(_) => { // we need to peek ahead one more token to disambiguate the first // three cases listed above let peek2 = Symbol::new(self.tokens.clone().nth(1)); @@ -200,7 +280,7 @@ impl Parser { match peek2 { // case 1: `! ` // case 3: `! = OP str` - Symbol::StringOp(_) | Symbol::None => { + Symbol::Op(_) | Symbol::None => { // op is literal let op = self.next_token().into_literal(); self.literal(op); @@ -293,18 +373,15 @@ impl Parser { self.stack.push(token.into_literal()); // EXPR → str OP str - match self.peek() { - Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) => { - let op = self.next_token(); + if let Symbol::Op(_) = self.peek() { + let op = self.next_token(); - match self.next_token() { - Symbol::None => panic!("missing argument after {:?}", op), - token => self.stack.push(token.into_literal()), - } - - self.stack.push(op); + match self.next_token() { + Symbol::None => panic!("missing argument after {:?}", op), + token => self.stack.push(token.into_literal()), } - _ => {} + + self.stack.push(op); } } diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 6eea00080..50563ba49 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -11,7 +11,7 @@ mod parser; use clap::{crate_version, App, AppSettings}; -use parser::{parse, Symbol}; +use parser::{parse, Operator, Symbol, UnaryOperator}; use std::ffi::{OsStr, OsString}; use std::path::Path; @@ -159,19 +159,19 @@ fn eval(stack: &mut Vec) -> Result { Ok(!result) } - Some(Symbol::StringOp(op)) => { + Some(Symbol::Op(Operator::String(op))) => { let b = stack.pop(); let a = stack.pop(); Ok(if op == "!=" { a != b } else { a == b }) } - Some(Symbol::IntOp(op)) => { + Some(Symbol::Op(Operator::Int(op))) => { let b = pop_literal!(); let a = pop_literal!(); Ok(integers(&a, &b, &op)?) } - Some(Symbol::FileOp(_op)) => unimplemented!(), - Some(Symbol::StrlenOp(op)) => { + Some(Symbol::Op(Operator::File(_op))) => unimplemented!(), + Some(Symbol::UnaryOp(UnaryOperator::StrlenOp(op))) => { let s = match stack.pop() { Some(Symbol::Literal(s)) => s, Some(Symbol::None) => OsString::from(""), @@ -189,7 +189,7 @@ fn eval(stack: &mut Vec) -> Result { !s.is_empty() }) } - Some(Symbol::FiletestOp(op)) => { + Some(Symbol::UnaryOp(UnaryOperator::FiletestOp(op))) => { let op = op.to_string_lossy(); let f = pop_literal!(); diff --git a/tests/by-util/test_test.rs b/tests/by-util/test_test.rs index c5f1e43ed..23facd610 100644 --- a/tests/by-util/test_test.rs +++ b/tests/by-util/test_test.rs @@ -35,6 +35,35 @@ fn test_solo_and_or_or_is_a_literal() { new_ucmd!().arg("-o").succeeds(); } +#[test] +fn test_some_literals() { + let scenario = TestScenario::new(util_name!()); + let tests = [ + "a string", + "(", + ")", + "-", + "--", + "-0", + "-f", + "--help", + "--version", + "-eq", + "-lt", + "-ef", + "[", + ]; + + for test in &tests { + scenario.ucmd().arg(test).succeeds(); + } + + // run the inverse of all these tests + for test in &tests { + scenario.ucmd().arg("!").arg(test).run().status_code(1); + } +} + #[test] fn test_double_not_is_false() { new_ucmd!().args(&["!", "!"]).run().status_code(1); @@ -99,21 +128,6 @@ fn test_zero_len_of_empty() { new_ucmd!().args(&["-z", ""]).succeeds(); } -#[test] -fn test_solo_parenthesis_is_literal() { - let scenario = TestScenario::new(util_name!()); - let tests = [["("], [")"]]; - - for test in &tests { - scenario.ucmd().args(&test[..]).succeeds(); - } -} - -#[test] -fn test_solo_empty_parenthetical_is_error() { - new_ucmd!().args(&["(", ")"]).run().status_code(2); -} - #[test] fn test_zero_len_equals_zero_len() { new_ucmd!().args(&["", "=", ""]).succeeds(); @@ -139,6 +153,7 @@ fn test_string_comparison() { ["contained\nnewline", "=", "contained\nnewline"], ["(", "=", "("], ["(", "!=", ")"], + ["(", "!=", "="], ["!", "=", "!"], ["=", "=", "="], ]; @@ -199,11 +214,13 @@ fn test_a_bunch_of_not() { #[test] fn test_pseudofloat_equal() { + // string comparison; test(1) doesn't support comparison of actual floats new_ucmd!().args(&["123.45", "=", "123.45"]).succeeds(); } #[test] fn test_pseudofloat_not_equal() { + // string comparison; test(1) doesn't support comparison of actual floats new_ucmd!().args(&["123.45", "!=", "123.450"]).succeeds(); } @@ -230,6 +247,16 @@ fn test_some_int_compares() { for test in &tests { scenario.ucmd().args(&test[..]).succeeds(); } + + // run the inverse of all these tests + for test in &tests { + scenario + .ucmd() + .arg("!") + .args(&test[..]) + .run() + .status_code(1); + } } #[test] @@ -257,6 +284,16 @@ fn test_negative_int_compare() { for test in &tests { scenario.ucmd().args(&test[..]).succeeds(); } + + // run the inverse of all these tests + for test in &tests { + scenario + .ucmd() + .arg("!") + .args(&test[..]) + .run() + .status_code(1); + } } #[test] @@ -499,6 +536,93 @@ fn test_file_is_not_sticky() { .status_code(1); } +#[test] +fn test_solo_empty_parenthetical_is_error() { + new_ucmd!().args(&["(", ")"]).run().status_code(2); +} + +#[test] +fn test_parenthesized_literal() { + let scenario = TestScenario::new(util_name!()); + let tests = [ + "a string", + "(", + ")", + "-", + "--", + "-0", + "-f", + "--help", + "--version", + "-e", + "-t", + "!", + "-n", + "-z", + "[", + "-a", + "-o", + ]; + + for test in &tests { + scenario.ucmd().arg("(").arg(test).arg(")").succeeds(); + } + + // run the inverse of all these tests + for test in &tests { + scenario + .ucmd() + .arg("!") + .arg("(") + .arg(test) + .arg(")") + .run() + .status_code(1); + } +} + +#[test] +fn test_parenthesized_op_compares_literal_parenthesis() { + // ensure we aren’t treating this case as “string length of literal equal + // sign” + new_ucmd!().args(&["(", "=", ")"]).run().status_code(1); +} + +#[test] +fn test_parenthesized_string_comparison() { + let scenario = TestScenario::new(util_name!()); + let tests = [ + ["(", "foo", "!=", "bar", ")"], + ["(", "contained\nnewline", "=", "contained\nnewline", ")"], + ["(", "(", "=", "(", ")"], + ["(", "(", "!=", ")", ")"], + ["(", "!", "=", "!", ")"], + ["(", "=", "=", "=", ")"], + ]; + + for test in &tests { + scenario.ucmd().args(&test[..]).succeeds(); + } + + // run the inverse of all these tests + for test in &tests { + scenario + .ucmd() + .arg("!") + .args(&test[..]) + .run() + .status_code(1); + } +} + +#[test] +fn test_parenthesized_right_parenthesis_as_literal() { + new_ucmd!() + .args(&["(", "-f", ")", ")"]) + .run() + .status_code(1); +} + #[test] #[cfg(not(windows))] fn test_file_owned_by_euid() {