test: implement parenthesized expressions, additional tests

- Replace the parser with a recursive descent implementation that handles
  parentheses and produces a stack of operations in postfix order.

  Parsing now operates directly on OsStrings passed by the uucore framework.

- Replace the dispatch mechanism with a stack machine operating on the
  symbol stack produced by the parser.

- Add tests for parenthesized expressions.

- Begin testing character encoding handling.

src/uu/test/src/parser.rs

@@ -0,0 +1,292 @@
+// This file is part of the uutils coreutils package.
+//
+// (c) Daniel Rocco <drocco@gmail.com>
+//
+// For the full copyright and license information, please view the LICENSE
+// file that was distributed with this source code.
+
+use std::ffi::OsString;
+use std::iter::Peekable;
+
+/// Represents a parsed token from a test expression
+#[derive(Debug, PartialEq)]
+pub enum Symbol {
+    LParen,
+    Bang,
+    BoolOp(OsString),
+    Literal(OsString),
+    StringOp(OsString),
+    IntOp(OsString),
+    FileOp(OsString),
+    StrlenOp(OsString),
+    FiletestOp(OsString),
+    None,
+}
+
+impl Symbol {
+    /// Create a new Symbol from an OsString.
+    ///
+    /// Returns Symbol::None in place of None
+    fn new(token: Option<OsString>) -> Symbol {
+        match token {
+            Some(s) => match s.to_string_lossy().as_ref() {
+                "(" => 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),
+                "-b" | "-c" | "-d" | "-e" | "-f" | "-g" | "-G" | "-h" | "-k" | "-L" | "-O"
+                | "-p" | "-r" | "-s" | "-S" | "-t" | "-u" | "-w" | "-x" => Symbol::FiletestOp(s),
+                _ => Symbol::Literal(s),
+            },
+            None => Symbol::None,
+        }
+    }
+
+    /// Convert this Symbol into a Symbol::Literal, useful for cases where
+    /// test treats an operator as a string operand (test has no reserved
+    /// words).
+    ///
+    /// # Panics
+    ///
+    /// Panics if `self` is Symbol::None
+    fn into_literal(self) -> Symbol {
+        Symbol::Literal(match self {
+            Symbol::LParen => OsString::from("("),
+            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::None => panic!(),
+        })
+    }
+}
+
+/// Recursive descent parser for test, which converts a list of OsStrings
+/// (typically command line arguments) into a stack of Symbols in postfix
+/// order.
+///
+/// Grammar:
+///
+///   EXPR → TERM | EXPR BOOLOP EXPR
+///   TERM → ( EXPR )
+///   TERM → ( )
+///   TERM → ! EXPR
+///   TERM → UOP str
+///   UOP → STRLEN | FILETEST
+///   TERM → str OP str
+///   TERM → str | 𝜖
+///   OP → STRINGOP | INTOP | FILEOP
+///   STRINGOP → = | !=
+///   INTOP → -eq | -ge | -gt | -le | -lt | -ne
+///   FILEOP → -ef | -nt | -ot
+///   STRLEN → -n | -z
+///   FILETEST → -b | -c | -d | -e | -f | -g | -G | -h | -k | -L | -O | -p |
+///               -r | -s | -S | -t | -u | -w | -x
+///   BOOLOP → -a | -o
+///
+#[derive(Debug)]
+struct Parser {
+    tokens: Peekable<std::vec::IntoIter<OsString>>,
+    pub stack: Vec<Symbol>,
+}
+
+impl Parser {
+    /// Construct a new Parser from a `Vec<OsString>` of tokens.
+    fn new(tokens: Vec<OsString>) -> Parser {
+        Parser {
+            tokens: tokens.into_iter().peekable(),
+            stack: vec![],
+        }
+    }
+
+    /// Fetch the next token from the input stream as a Symbol.
+    fn next_token(&mut self) -> Symbol {
+        Symbol::new(self.tokens.next())
+    }
+
+    /// 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()`.
+    fn peek(&mut self) -> Symbol {
+        Symbol::new(self.tokens.peek().map(|s| s.to_os_string()))
+    }
+
+    /// Test if the next token in the stream is a BOOLOP (-a or -o), without
+    /// removing the token from the stream.
+    fn peek_is_boolop(&mut self) -> bool {
+        if let Symbol::BoolOp(_) = self.peek() {
+            true
+        } else {
+            false
+        }
+    }
+
+    /// Parse an expression.
+    ///
+    ///   EXPR → TERM | EXPR BOOLOP EXPR
+    fn expr(&mut self) {
+        if !self.peek_is_boolop() {
+            self.term();
+        }
+        self.maybe_boolop();
+    }
+
+    /// Parse a term token and possible subsequent symbols: "(", "!", UOP,
+    /// literal, or None.
+    fn term(&mut self) {
+        let symbol = self.next_token();
+
+        match symbol {
+            Symbol::LParen => self.lparen(),
+            Symbol::Bang => self.bang(),
+            Symbol::StrlenOp(_) => self.uop(symbol),
+            Symbol::FiletestOp(_) => self.uop(symbol),
+            Symbol::None => self.stack.push(symbol),
+            literal => self.literal(literal),
+        }
+    }
+
+    /// 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.
+    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(_) => {
+                self.literal(Symbol::Literal(OsString::from("(")));
+            }
+            // empty parenthetical
+            Symbol::Literal(s) if s == ")" => {}
+            _ => {
+                self.expr();
+                match self.next_token() {
+                    Symbol::Literal(s) if s == ")" => (),
+                    _ => panic!("expected ‘)’"),
+                }
+            }
+        }
+    }
+
+    /// Parse a (possibly) negated expression.
+    ///
+    /// Example cases:
+    ///
+    /// * `! =`: negate the result of the implicit string length test of `=`
+    /// * `! = foo`: compare the literal strings `!` and `foo`
+    /// * `! <expr>`: negate the result of the expression
+    ///
+    fn bang(&mut self) {
+        if let Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) = self.peek() {
+            // we need to peek ahead one more token to disambiguate the first
+            // two cases listed above: case 1 — `! <OP as literal>` — and
+            // case 2: `<! as literal> OP str`.
+            let peek2 = self.tokens.clone().nth(1);
+
+            if peek2.is_none() {
+                // op is literal
+                let op = self.next_token().into_literal();
+                self.stack.push(op);
+                self.stack.push(Symbol::Bang);
+            } else {
+                // bang is literal; parsing continues with op
+                self.literal(Symbol::Literal(OsString::from("!")));
+            }
+        } else {
+            self.expr();
+            self.stack.push(Symbol::Bang);
+        }
+    }
+
+    /// Peek at the next token and parse it as a BOOLOP or string literal,
+    /// as appropriate.
+    fn maybe_boolop(&mut self) {
+        if self.peek_is_boolop() {
+            let token = self.tokens.next().unwrap(); // safe because we peeked
+
+            // BoolOp by itself interpreted as Literal
+            if let Symbol::None = self.peek() {
+                self.literal(Symbol::Literal(token))
+            } else {
+                self.boolop(Symbol::BoolOp(token))
+            }
+        }
+    }
+
+    /// Parse a Boolean expression.
+    ///
+    /// Logical and (-a) has higher precedence than or (-o), so in an
+    /// expression like `foo -o '' -a ''`, the and subexpression is evaluated
+    /// first.
+    fn boolop(&mut self, op: Symbol) {
+        if op == Symbol::BoolOp(OsString::from("-a")) {
+            self.term();
+            self.stack.push(op);
+            self.maybe_boolop();
+        } else {
+            self.expr();
+            self.stack.push(op);
+        }
+    }
+
+    /// Parse a (possible) unary argument test (string length or file
+    /// attribute check).
+    ///
+    /// If a UOP is followed by nothing it is interpreted as a literal string.
+    fn uop(&mut self, op: Symbol) {
+        match self.next_token() {
+            Symbol::None => self.stack.push(op.into_literal()),
+            symbol => {
+                self.stack.push(symbol.into_literal());
+                self.stack.push(op);
+            }
+        }
+    }
+
+    /// Parse a string literal, optionally followed by a comparison operator
+    /// and a second string literal.
+    fn literal(&mut self, token: Symbol) {
+        self.stack.push(token.into_literal());
+
+        // EXPR → str OP str
+        match self.peek() {
+            Symbol::StringOp(_) | Symbol::IntOp(_) | Symbol::FileOp(_) => {
+                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);
+            }
+            _ => {}
+        }
+    }
+
+    /// Parser entry point: parse the token stream `self.tokens`, storing the
+    /// resulting `Symbol` stack in `self.stack`.
+    fn parse(&mut self) -> Result<(), String> {
+        self.expr();
+
+        match self.tokens.next() {
+            Some(token) => Err(format!("extra argument ‘{}’", token.to_string_lossy())),
+            None => Ok(()),
+        }
+    }
+}
+
+/// Parse the token stream `args`, returning a `Symbol` stack representing the
+/// operations to perform in postfix order.
+pub fn parse(args: Vec<OsString>) -> Result<Vec<Symbol>, String> {
+    let mut p = Parser::new(args);
+    p.parse()?;
+    Ok(p.stack)
+}

src/uu/test/src/test.rs

@@ -1,144 +1,154 @@
-//  * This file is part of the uutils coreutils package.
+// This file is part of the uutils coreutils package.
-//  *
+//
-//  * (c) mahkoh (ju.orth [at] gmail [dot] com)
+// (c) mahkoh (ju.orth [at] gmail [dot] com)
-//  *
+// (c) Daniel Rocco <drocco@gmail.com>
-//  * For the full copyright and license information, please view the LICENSE
+//
-//  * file that was distributed with this source code.
+// For the full copyright and license information, please view the LICENSE
+// file that was distributed with this source code.
 
 // spell-checker:ignore (ToDO) retval paren prec subprec cond
 
-use std::collections::HashMap;
+mod parser;
-use std::str::from_utf8;
 
-static NAME: &str = "test";
+use parser::{parse, Symbol};
+use std::ffi::{OsStr, OsString};
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
-    let args: Vec<_> = args.collect();
+    // TODO: handle being called as `[`
-    // This is completely disregarding valid windows paths that aren't valid unicode
+    let args: Vec<_> = args.skip(1).collect();
-    let args = args
-        .iter()
-        .map(|a| a.to_str().unwrap().as_bytes())
-        .collect::<Vec<&[u8]>>();
-    if args.is_empty() {
-        return 2;
-    }
-    let args = if !args[0].ends_with(NAME.as_bytes()) {
-        &args[1..]
-    } else {
-        &args[..]
-    };
-    let args = match args[0] {
-        b"[" => match args[args.len() - 1] {
-            b"]" => &args[1..args.len() - 1],
-            _ => return 2,
-        },
-        _ => &args[1..args.len()],
-    };
-    let mut error = false;
-    let retval = 1 - parse_expr(args, &mut error) as i32;
-    if error {
-        2
-    } else {
-        retval
-    }
-}
 
-fn one(args: &[&[u8]]) -> bool {
+    let result = parse(args).and_then(|mut stack| eval(&mut stack));
-    !args[0].is_empty()
-}
 
-fn two(args: &[&[u8]], error: &mut bool) -> bool {
+    match result {
-    match args[0] {
+        Ok(result) => {
-        b"!" => !one(&args[1..]),
+            if result {
-        b"-b" => path(args[1], PathCondition::BlockSpecial),
+                0
-        b"-c" => path(args[1], PathCondition::CharacterSpecial),
+            } else {
-        b"-d" => path(args[1], PathCondition::Directory),
+                1
-        b"-e" => path(args[1], PathCondition::Exists),
-        b"-f" => path(args[1], PathCondition::Regular),
-        b"-g" => path(args[1], PathCondition::GroupIdFlag),
-        b"-h" => path(args[1], PathCondition::SymLink),
-        b"-L" => path(args[1], PathCondition::SymLink),
-        b"-n" => one(&args[1..]),
-        b"-p" => path(args[1], PathCondition::Fifo),
-        b"-r" => path(args[1], PathCondition::Readable),
-        b"-S" => path(args[1], PathCondition::Socket),
-        b"-s" => path(args[1], PathCondition::NonEmpty),
-        b"-t" => isatty(args[1]),
-        b"-u" => path(args[1], PathCondition::UserIdFlag),
-        b"-w" => path(args[1], PathCondition::Writable),
-        b"-x" => path(args[1], PathCondition::Executable),
-        b"-z" => !one(&args[1..]),
-        _ => {
-            *error = true;
-            false
-        }
-    }
-}
-
-fn three(args: &[&[u8]], error: &mut bool) -> bool {
-    match args[1] {
-        b"=" => args[0] == args[2],
-        b"==" => args[0] == args[2],
-        b"!=" => args[0] != args[2],
-        b"-eq" => integers(args[0], args[2], IntegerCondition::Equal),
-        b"-ne" => integers(args[0], args[2], IntegerCondition::Unequal),
-        b"-gt" => integers(args[0], args[2], IntegerCondition::Greater),
-        b"-ge" => integers(args[0], args[2], IntegerCondition::GreaterEqual),
-        b"-lt" => integers(args[0], args[2], IntegerCondition::Less),
-        b"-le" => integers(args[0], args[2], IntegerCondition::LessEqual),
-        _ => match args[0] {
-            b"!" => !two(&args[1..], error),
-            _ => {
-                *error = true;
-                false
             }
-        },
+        }
-    }
+        Err(e) => {
-}
+            eprintln!("test: {}", e);
-
+            2
-fn four(args: &[&[u8]], error: &mut bool) -> bool {
-    match args[0] {
-        b"!" => !three(&args[1..], error),
-        _ => {
-            *error = true;
-            false
         }
     }
 }
 
-enum IntegerCondition {
+/// Evaluate a stack of Symbols, returning the result of the evaluation or
-    Equal,
+/// an error message if evaluation failed.
-    Unequal,
+fn eval(stack: &mut Vec<Symbol>) -> Result<bool, String> {
-    Greater,
+    macro_rules! pop_literal {
-    GreaterEqual,
+        () => {
-    Less,
+            match stack.pop() {
-    LessEqual,
+                Some(Symbol::Literal(s)) => s,
-}
+                _ => panic!(),
+            }
+        };
+    }
 
-fn integers(a: &[u8], b: &[u8], cond: IntegerCondition) -> bool {
+    let s = stack.pop();
-    let (a, b): (&str, &str) = match (from_utf8(a), from_utf8(b)) {
+
-        (Ok(a), Ok(b)) => (a, b),
+    match s {
-        _ => return false,
+        Some(Symbol::Bang) => {
-    };
+            let result = eval(stack)?;
-    let (a, b): (i64, i64) = match (a.parse(), b.parse()) {
+
-        (Ok(a), Ok(b)) => (a, b),
+            Ok(!result)
-        _ => return false,
+        }
-    };
+        Some(Symbol::StringOp(op)) => {
-    match cond {
+            let b = stack.pop();
-        IntegerCondition::Equal => a == b,
+            let a = stack.pop();
-        IntegerCondition::Unequal => a != b,
+            Ok(if op == "=" { a == b } else { a != b })
-        IntegerCondition::Greater => a > b,
+        }
-        IntegerCondition::GreaterEqual => a >= b,
+        Some(Symbol::IntOp(op)) => {
-        IntegerCondition::Less => a < b,
+            let b = pop_literal!();
-        IntegerCondition::LessEqual => a <= b,
+            let a = pop_literal!();
+
+            Ok(integers(&a, &b, &op)?)
+        }
+        Some(Symbol::FileOp(_op)) => unimplemented!(),
+        Some(Symbol::StrlenOp(op)) => {
+            let s = match stack.pop() {
+                Some(Symbol::Literal(s)) => s,
+                Some(Symbol::None) => OsString::from(""),
+                None => {
+                    return Ok(true);
+                }
+                _ => {
+                    return Err(format!("missing argument after ‘{:?}’", op));
+                }
+            };
+
+            Ok(if op == "-z" {
+                s.is_empty()
+            } else {
+                !s.is_empty()
+            })
+        }
+        Some(Symbol::FiletestOp(op)) => {
+            let op = op.to_string_lossy();
+
+            let f = pop_literal!();
+
+            Ok(match op.as_ref() {
+                "-b" => path(&f, PathCondition::BlockSpecial),
+                "-c" => path(&f, PathCondition::CharacterSpecial),
+                "-d" => path(&f, PathCondition::Directory),
+                "-e" => path(&f, PathCondition::Exists),
+                "-f" => path(&f, PathCondition::Regular),
+                "-g" => path(&f, PathCondition::GroupIdFlag),
+                "-h" => path(&f, PathCondition::SymLink),
+                "-L" => path(&f, PathCondition::SymLink),
+                "-p" => path(&f, PathCondition::Fifo),
+                "-r" => path(&f, PathCondition::Readable),
+                "-S" => path(&f, PathCondition::Socket),
+                "-s" => path(&f, PathCondition::NonEmpty),
+                "-t" => isatty(&f)?,
+                "-u" => path(&f, PathCondition::UserIdFlag),
+                "-w" => path(&f, PathCondition::Writable),
+                "-x" => path(&f, PathCondition::Executable),
+                _ => panic!(),
+            })
+        }
+        Some(Symbol::Literal(s)) => Ok(!s.is_empty()),
+        Some(Symbol::None) => Ok(false),
+        Some(Symbol::BoolOp(op)) => {
+            let b = eval(stack)?;
+            let a = eval(stack)?;
+
+            Ok(if op == "-a" { a && b } else { a || b })
+        }
+        None => Ok(false),
+        _ => Err("expected value".to_string()),
     }
 }
 
-fn isatty(fd: &[u8]) -> bool {
+fn integers(a: &OsStr, b: &OsStr, cond: &OsStr) -> Result<bool, String> {
-    from_utf8(fd)
+    let format_err = |value| format!("invalid integer ‘{}’", value);
-        .ok()
+
-        .and_then(|s| s.parse().ok())
+    let a = a.to_string_lossy();
-        .map_or(false, |i| {
+    let a: i64 = a.parse().map_err(|_| format_err(a))?;
+
+    let b = b.to_string_lossy();
+    let b: i64 = b.parse().map_err(|_| format_err(b))?;
+
+    let cond = cond.to_string_lossy();
+    Ok(match cond.as_ref() {
+        "-eq" => a == b,
+        "-ne" => a != b,
+        "-gt" => a > b,
+        "-ge" => a >= b,
+        "-lt" => a < b,
+        "-le" => a <= b,
+        _ => return Err(format!("unknown operator ‘{}’", cond)),
+    })
+}
+
+fn isatty(fd: &OsStr) -> Result<bool, String> {
+    let fd = fd.to_string_lossy();
+
+    fd.parse()
+        .map_err(|_| format!("invalid integer ‘{}’", fd))
+        .map(|i| {
             #[cfg(not(target_os = "redox"))]
             unsafe {
                 libc::isatty(i) == 1
@@ -148,173 +158,6 @@ fn isatty(fd: &[u8]) -> bool {
         })
 }
 
-fn dispatch(args: &mut &[&[u8]], error: &mut bool) -> bool {
-    let (val, idx) = match args.len() {
-        0 => {
-            *error = true;
-            (false, 0)
-        }
-        1 => (one(*args), 1),
-        2 => dispatch_two(args, error),
-        3 => dispatch_three(args, error),
-        _ => dispatch_four(args, error),
-    };
-    *args = &(*args)[idx..];
-    val
-}
-
-fn dispatch_two(args: &mut &[&[u8]], error: &mut bool) -> (bool, usize) {
-    let val = two(*args, error);
-    if *error {
-        *error = false;
-        (one(*args), 1)
-    } else {
-        (val, 2)
-    }
-}
-
-fn dispatch_three(args: &mut &[&[u8]], error: &mut bool) -> (bool, usize) {
-    let val = three(*args, error);
-    if *error {
-        *error = false;
-        dispatch_two(args, error)
-    } else {
-        (val, 3)
-    }
-}
-
-fn dispatch_four(args: &mut &[&[u8]], error: &mut bool) -> (bool, usize) {
-    let val = four(*args, error);
-    if *error {
-        *error = false;
-        dispatch_three(args, error)
-    } else {
-        (val, 4)
-    }
-}
-
-#[derive(Clone, Copy)]
-enum Precedence {
-    Unknown = 0,
-    Paren, // FIXME: this is useless (parentheses have not been implemented)
-    Or,
-    And,
-    BUnOp,
-    BinOp,
-    UnOp,
-}
-
-fn parse_expr(mut args: &[&[u8]], error: &mut bool) -> bool {
-    if args.is_empty() {
-        false
-    } else {
-        let hashmap = setup_hashmap();
-        let lhs = dispatch(&mut args, error);
-
-        if !args.is_empty() {
-            parse_expr_helper(&hashmap, &mut args, lhs, Precedence::Unknown, error)
-        } else {
-            lhs
-        }
-    }
-}
-
-fn parse_expr_helper<'a>(
-    hashmap: &HashMap<&'a [u8], Precedence>,
-    args: &mut &[&'a [u8]],
-    mut lhs: bool,
-    min_prec: Precedence,
-    error: &mut bool,
-) -> bool {
-    let mut prec = *hashmap.get(&args[0]).unwrap_or_else(|| {
-        *error = true;
-        &min_prec
-    });
-    while !*error && !args.is_empty() && prec as usize >= min_prec as usize {
-        let op = args[0];
-        *args = &(*args)[1..];
-        let mut rhs = dispatch(args, error);
-        while !args.is_empty() {
-            let subprec = *hashmap.get(&args[0]).unwrap_or_else(|| {
-                *error = true;
-                &min_prec
-            });
-            if subprec as usize <= prec as usize || *error {
-                break;
-            }
-            rhs = parse_expr_helper(hashmap, args, rhs, subprec, error);
-        }
-        lhs = match prec {
-            Precedence::UnOp | Precedence::BUnOp => {
-                *error = true;
-                false
-            }
-            Precedence::And => lhs && rhs,
-            Precedence::Or => lhs || rhs,
-            Precedence::BinOp => three(
-                &[
-                    if lhs { b" " } else { b"" },
-                    op,
-                    if rhs { b" " } else { b"" },
-                ],
-                error,
-            ),
-            Precedence::Paren => unimplemented!(), // TODO: implement parentheses
-            _ => unreachable!(),
-        };
-        if !args.is_empty() {
-            prec = *hashmap.get(&args[0]).unwrap_or_else(|| {
-                *error = true;
-                &min_prec
-            });
-        }
-    }
-    lhs
-}
-
-#[inline]
-fn setup_hashmap<'a>() -> HashMap<&'a [u8], Precedence> {
-    let mut hashmap = HashMap::<&'a [u8], Precedence>::new();
-
-    hashmap.insert(b"-b", Precedence::UnOp);
-    hashmap.insert(b"-c", Precedence::UnOp);
-    hashmap.insert(b"-d", Precedence::UnOp);
-    hashmap.insert(b"-e", Precedence::UnOp);
-    hashmap.insert(b"-f", Precedence::UnOp);
-    hashmap.insert(b"-g", Precedence::UnOp);
-    hashmap.insert(b"-h", Precedence::UnOp);
-    hashmap.insert(b"-L", Precedence::UnOp);
-    hashmap.insert(b"-n", Precedence::UnOp);
-    hashmap.insert(b"-p", Precedence::UnOp);
-    hashmap.insert(b"-r", Precedence::UnOp);
-    hashmap.insert(b"-S", Precedence::UnOp);
-    hashmap.insert(b"-s", Precedence::UnOp);
-    hashmap.insert(b"-t", Precedence::UnOp);
-    hashmap.insert(b"-u", Precedence::UnOp);
-    hashmap.insert(b"-w", Precedence::UnOp);
-    hashmap.insert(b"-x", Precedence::UnOp);
-    hashmap.insert(b"-z", Precedence::UnOp);
-
-    hashmap.insert(b"=", Precedence::BinOp);
-    hashmap.insert(b"!=", Precedence::BinOp);
-    hashmap.insert(b"-eq", Precedence::BinOp);
-    hashmap.insert(b"-ne", Precedence::BinOp);
-    hashmap.insert(b"-gt", Precedence::BinOp);
-    hashmap.insert(b"-ge", Precedence::BinOp);
-    hashmap.insert(b"-lt", Precedence::BinOp);
-    hashmap.insert(b"-le", Precedence::BinOp);
-
-    hashmap.insert(b"!", Precedence::BUnOp);
-
-    hashmap.insert(b"-a", Precedence::And);
-    hashmap.insert(b"-o", Precedence::Or);
-
-    hashmap.insert(b"(", Precedence::Paren);
-    hashmap.insert(b")", Precedence::Paren);
-
-    hashmap
-}
-
 #[derive(Eq, PartialEq)]
 enum PathCondition {
     BlockSpecial,
@@ -334,14 +177,10 @@ enum PathCondition {
 }
 
 #[cfg(not(windows))]
-fn path(path: &[u8], cond: PathCondition) -> bool {
+fn path(path: &OsStr, cond: PathCondition) -> bool {
-    use std::ffi::OsStr;
     use std::fs::{self, Metadata};
-    use std::os::unix::ffi::OsStrExt;
     use std::os::unix::fs::{FileTypeExt, MetadataExt};
 
-    let path = OsStr::from_bytes(path);
-
     const S_ISUID: u32 = 0o4000;
     const S_ISGID: u32 = 0o2000;
 
@@ -403,13 +242,14 @@ fn path(path: &[u8], cond: PathCondition) -> bool {
 }
 
 #[cfg(windows)]
-fn path(path: &[u8], cond: PathCondition) -> bool {
+fn path(path: &OsStr, cond: PathCondition) -> bool {
     use std::fs::metadata;
-    let path = from_utf8(path).unwrap();
+
     let stat = match metadata(path) {
         Ok(s) => s,
         _ => return false,
     };
+
     match cond {
         PathCondition::BlockSpecial => false,
         PathCondition::CharacterSpecial => false,

tests/by-util/test_test.rs

@@ -39,7 +39,6 @@ fn test_double_not_is_false() {
 }
 
 #[test]
-#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"]
 fn test_and_not_is_false() {
     new_ucmd!().args(&["-a", "!"]).run().status_code(1);
 }
@@ -51,7 +50,6 @@ fn test_not_and_is_false() {
 }
 
 #[test]
-#[ignore = "fixme: parse/evaluation error (code 2); GNU returns 0"]
 fn test_not_and_not_succeeds() {
     new_ucmd!().args(&["!", "-a", "!"]).succeeds();
 }
@@ -62,7 +60,6 @@ fn test_simple_or() {
 }
 
 #[test]
-#[ignore = "fixme: parse/evaluation error (code 0); GNU returns 1"]
 fn test_negated_or() {
     new_ucmd!()
         .args(&["!", "foo", "-o", "bar"])
@@ -111,13 +108,8 @@ fn test_solo_paren_is_literal() {
 }
 
 #[test]
-#[ignore = "GNU considers this an error"]
 fn test_solo_empty_parenthetical_is_error() {
-    new_ucmd!()
+    new_ucmd!().args(&["(", ")"]).run().status_code(2);
-        .args(&["(", ")"])
-        .run()
-        .status_code(2)
-        .stderr_is("test: missing argument after ‘)’");
 }
 
 #[test]
@@ -248,7 +240,6 @@ fn test_negative_int_compare() {
 }
 
 #[test]
-#[ignore = "fixme: error reporting"]
 fn test_float_inequality_is_error() {
     new_ucmd!()
         .args(&["123.45", "-ge", "6"])
@@ -257,6 +248,32 @@ fn test_float_inequality_is_error() {
         .stderr_is("test: invalid integer ‘123.45’");
 }
 
+#[test]
+#[cfg(not(windows))]
+fn test_invalid_utf8_integer_compare() {
+    use std::ffi::OsStr;
+    use std::os::unix::ffi::OsStrExt;
+
+    let source = [0x66, 0x6f, 0x80, 0x6f];
+    let arg = OsStr::from_bytes(&source[..]);
+
+    let mut cmd = new_ucmd!();
+    cmd.arg("123").arg("-ne");
+    cmd.raw.arg(arg);
+
+    cmd.run()
+        .status_code(2)
+        .stderr_is("test: invalid integer ‘fo<66>o’");
+
+    let mut cmd = new_ucmd!();
+    cmd.raw.arg(arg);
+    cmd.arg("-eq").arg("456");
+
+    cmd.run()
+        .status_code(2)
+        .stderr_is("test: invalid integer ‘fo<66>o’");
+}
+
 #[test]
 #[ignore = "fixme: parse/evaluation error (code 2); GNU returns 1"]
 fn test_file_is_itself() {
@@ -445,6 +462,14 @@ fn test_op_prec_and_or_1() {
     new_ucmd!().args(&[" ", "-o", "", "-a", ""]).succeeds();
 }
 
+#[test]
+fn test_op_prec_and_or_1_overridden_by_parentheses() {
+    new_ucmd!()
+        .args(&["(", " ", "-o", "", ")", "-a", ""])
+        .run()
+        .status_code(1);
+}
+
 #[test]
 fn test_op_prec_and_or_2() {
     new_ucmd!()
@@ -452,6 +477,45 @@ fn test_op_prec_and_or_2() {
         .succeeds();
 }
 
+#[test]
+fn test_op_prec_and_or_2_overridden_by_parentheses() {
+    new_ucmd!()
+        .args(&["", "-a", "(", "", "-o", " ", ")", "-a", " "])
+        .run()
+        .status_code(1);
+}
+
+#[test]
+#[ignore = "fixme: error reporting"]
+fn test_dangling_parenthesis() {
+    new_ucmd!()
+        .args(&["(", "(", "a", "!=", "b", ")", "-o", "-n", "c"])
+        .run()
+        .status_code(2);
+    new_ucmd!()
+        .args(&["(", "(", "a", "!=", "b", ")", "-o", "-n", "c", ")"])
+        .succeeds();
+}
+
+#[test]
+fn test_complicated_parenthesized_expression() {
+    new_ucmd!()
+        .args(&[
+            "(", "(", "!", "(", "a", "=", "b", ")", "-o", "c", "=", "d", ")", "-a", "(", "q", "!=",
+            "r", ")", ")",
+        ])
+        .succeeds();
+}
+
+#[test]
+fn test_erroneous_parenthesized_expression() {
+    new_ucmd!()
+        .args(&["a", "!=", "(", "b", "-a", "b", ")", "!=", "c"])
+        .run()
+        .status_code(2)
+        .stderr_is("test: extra argument ‘b’");
+}
+
 #[test]
 fn test_or_as_filename() {
     new_ucmd!()