From 00379cce635185aa768c47679b263e4cc6b38cb4 Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Tue, 8 Aug 2017 09:18:17 +0100 Subject: [PATCH] Thread Strictness through the parser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The value is ignored, but this broke quite a lot of tests that assumed MatchedFlags had only one field. Parsing tests have to have OsStr flags because I couldn’t get that part working right, but in general, some tests now re-use common functionality too. --- src/options/dir_action.rs | 19 +++-------- src/options/filter.rs | 10 +++--- src/options/mod.rs | 38 +++++++++++++++++++--- src/options/parser.rs | 68 ++++++++++++++++++++++++--------------- src/options/view.rs | 24 +++++++------- 5 files changed, 97 insertions(+), 62 deletions(-) diff --git a/src/options/dir_action.rs b/src/options/dir_action.rs index a15f439..1e6a90f 100644 --- a/src/options/dir_action.rs +++ b/src/options/dir_action.rs @@ -55,27 +55,18 @@ impl RecurseOptions { #[cfg(test)] mod test { use super::*; - use std::ffi::OsString; use options::flags; - pub fn os(input: &'static str) -> OsString { - let mut os = OsString::new(); - os.push(input); - os - } - macro_rules! test { ($name:ident: $type:ident <- $inputs:expr => $result:expr) => { #[test] fn $name() { - use options::parser::{Args, Arg}; - use std::ffi::OsString; + use options::parser::Arg; + use options::test::assert_parses; + use options::test::Strictnesses::*; - static TEST_ARGS: &[&Arg] = &[ &flags::RECURSE, &flags::LIST_DIRS, &flags::TREE, &flags::LEVEL ]; - - let bits = $inputs.as_ref().into_iter().map(|&o| os(o)).collect::>(); - let results = Args(TEST_ARGS).parse(bits.iter()); - assert_eq!($type::deduce(&results.unwrap().flags), $result); + static TEST_ARGS: &[&Arg] = &[&flags::RECURSE, &flags::LIST_DIRS, &flags::TREE, &flags::LEVEL ]; + assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result) } }; } diff --git a/src/options/filter.rs b/src/options/filter.rs index 7e56d5a..0832db0 100644 --- a/src/options/filter.rs +++ b/src/options/filter.rs @@ -137,14 +137,12 @@ mod test { ($name:ident: $type:ident <- $inputs:expr => $result:expr) => { #[test] fn $name() { - use options::parser::{Args, Arg}; - use std::ffi::OsString; + use options::parser::Arg; + use options::test::assert_parses; + use options::test::Strictnesses::*; static TEST_ARGS: &[&Arg] = &[ &flags::SORT, &flags::ALL, &flags::TREE, &flags::IGNORE_GLOB ]; - - let bits = $inputs.as_ref().into_iter().map(|&o| os(o)).collect::>(); - let results = Args(TEST_ARGS).parse(bits.iter()); - assert_eq!($type::deduce(&results.unwrap().flags), $result); + assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result) } }; } diff --git a/src/options/mod.rs b/src/options/mod.rs index d488381..8a9d805 100644 --- a/src/options/mod.rs +++ b/src/options/mod.rs @@ -116,9 +116,9 @@ impl Options { #[allow(unused_results)] pub fn getopts<'args, I>(args: I) -> Result<(Options, Vec<&'args OsStr>), Misfire> where I: IntoIterator { - use options::parser::Matches; + use options::parser::{Matches, Strictness}; - let Matches { flags, frees } = match flags::ALL_ARGS.parse(args) { + let Matches { flags, frees } = match flags::ALL_ARGS.parse(args, Strictness::UseLastArguments) { Ok(m) => m, Err(e) => return Err(Misfire::InvalidOptions(e)), }; @@ -155,14 +155,44 @@ impl Options { #[cfg(test)] -mod test { +pub mod test { use super::{Options, Misfire, flags}; + use options::parser::{Arg, MatchedFlags}; use std::ffi::OsString; use fs::filter::{SortField, SortCase}; + use std::fmt::Debug; + + + #[derive(PartialEq, Debug)] + pub enum Strictnesses { + Last, + Complain, + Both, + } + + pub fn assert_parses(inputs: &[&str], args: &'static [&'static Arg], strictnesses: Strictnesses, get: F, result: T) + where T: PartialEq + Debug, F: Fn(&MatchedFlags) -> T + { + use self::Strictnesses::*; + use options::parser::{Args, Strictness}; + use std::ffi::OsString; + + let bits = inputs.into_iter().map(|&o| os(o)).collect::>(); + + if strictnesses == Last || strictnesses == Both { + let results = Args(args).parse(bits.iter(), Strictness::UseLastArguments); + assert_eq!(get(&results.unwrap().flags), result); + } + + if strictnesses == Complain || strictnesses == Both { + let results = Args(args).parse(bits.iter(), Strictness::ComplainAboutRedundantArguments); + assert_eq!(get(&results.unwrap().flags), result); + } + } /// Creates an `OSStr` (used in tests) #[cfg(test)] - fn os(input: &'static str) -> OsString { + fn os(input: &str) -> OsString { let mut os = OsString::new(); os.push(input); os diff --git a/src/options/parser.rs b/src/options/parser.rs index 27aeb20..55ed97c 100644 --- a/src/options/parser.rs +++ b/src/options/parser.rs @@ -60,8 +60,7 @@ impl Flag { /// Whether redundant arguments should be considered a problem. -#[derive(PartialEq, Debug)] -#[allow(dead_code)] // until strict mode is actually implemented +#[derive(PartialEq, Debug, Copy, Clone)] pub enum Strictness { /// Throw an error when an argument doesn’t do anything, either because @@ -122,7 +121,7 @@ impl Args { /// Iterates over the given list of command-line arguments and parses /// them into a list of matched flags and free strings. - pub fn parse<'args, I>(&self, inputs: I) -> Result, ParseError> + pub fn parse<'args, I>(&self, inputs: I, strictness: Strictness) -> Result, ParseError> where I: IntoIterator { use std::os::unix::ffi::OsStrExt; use self::TakesValue::*; @@ -267,17 +266,17 @@ impl Args { } } - Ok(Matches { frees, flags: MatchedFlags { flags: result_flags } }) + Ok(Matches { frees, flags: MatchedFlags { flags: result_flags, strictness } }) } - fn lookup_short<'a>(&self, short: ShortArg) -> Result<&Arg, ParseError> { + fn lookup_short(&self, short: ShortArg) -> Result<&Arg, ParseError> { match self.0.into_iter().find(|arg| arg.short == Some(short)) { Some(arg) => Ok(arg), None => Err(ParseError::UnknownShortArgument { attempt: short }) } } - fn lookup_long<'a>(&self, long: &'a OsStr) -> Result<&Arg, ParseError> { + fn lookup_long<'b>(&self, long: &'b OsStr) -> Result<&Arg, ParseError> { match self.0.into_iter().find(|arg| arg.long == long) { Some(arg) => Ok(arg), None => Err(ParseError::UnknownArgument { attempt: long.to_os_string() }) @@ -308,6 +307,9 @@ pub struct MatchedFlags<'args> { /// we usually want the one nearest the end to count, and to know this, /// we need to know where they are in relation to one another. flags: Vec<(Flag, Option<&'args OsStr>)>, + + /// Whether to check for duplicate or redundant arguments. + strictness: Strictness, } impl<'a> MatchedFlags<'a> { @@ -433,6 +435,13 @@ mod split_test { mod parse_test { use super::*; + pub fn os(input: &'static str) -> OsString { + let mut os = OsString::new(); + os.push(input); + os + } + + macro_rules! test { ($name:ident: $inputs:expr => frees: $frees:expr, flags: $flags:expr) => { #[test] @@ -445,15 +454,11 @@ mod parse_test { let frees: Vec = $frees.as_ref().into_iter().map(|&o| os(o)).collect(); let frees: Vec<&OsStr> = frees.iter().map(|os| os.as_os_str()).collect(); - // And again for the flags - let flags: Vec<(Flag, Option<&OsStr>)> = $flags - .as_ref() - .into_iter() - .map(|&(ref f, ref os): &(Flag, Option<&'static str>)| (f.clone(), os.map(OsStr::new))) - .collect(); + let flags = <[_]>::into_vec(Box::new($flags)); - let got = Args(TEST_ARGS).parse(inputs.iter()); - let expected = Ok(Matches { frees, flags: MatchedFlags { flags } }); + let strictness = Strictness::UseLastArguments; // this isn’t even used + let got = Args(TEST_ARGS).parse(inputs.iter(), strictness); + let expected = Ok(Matches { frees, flags: MatchedFlags { flags, strictness } }); assert_eq!(got, expected); } }; @@ -463,8 +468,9 @@ mod parse_test { fn $name() { use self::ParseError::*; + let strictness = Strictness::UseLastArguments; // this isn’t even used let bits = $inputs.as_ref().into_iter().map(|&o| os(o)).collect::>(); - let got = Args(TEST_ARGS).parse(bits.iter()); + let got = Args(TEST_ARGS).parse(bits.iter(), strictness); assert_eq!(got, Err($error)); } @@ -498,8 +504,8 @@ mod parse_test { // Long args with values test!(bad_equals: ["--long=equals"] => error ForbiddenValue { flag: Flag::Long("long") }); test!(no_arg: ["--count"] => error NeedsValue { flag: Flag::Long("count") }); - test!(arg_equals: ["--count=4"] => frees: [], flags: [ (Flag::Long("count"), Some("4")) ]); - test!(arg_then: ["--count", "4"] => frees: [], flags: [ (Flag::Long("count"), Some("4")) ]); + test!(arg_equals: ["--count=4"] => frees: [], flags: [ (Flag::Long("count"), Some(OsStr::new("4"))) ]); + test!(arg_then: ["--count", "4"] => frees: [], flags: [ (Flag::Long("count"), Some(OsStr::new("4"))) ]); // Short args @@ -511,11 +517,11 @@ mod parse_test { // Short args with values test!(bad_short: ["-l=equals"] => error ForbiddenValue { flag: Flag::Short(b'l') }); test!(short_none: ["-c"] => error NeedsValue { flag: Flag::Short(b'c') }); - test!(short_arg_eq: ["-c=4"] => frees: [], flags: [(Flag::Short(b'c'), Some("4")) ]); - test!(short_arg_then: ["-c", "4"] => frees: [], flags: [(Flag::Short(b'c'), Some("4")) ]); - test!(short_two_together: ["-lctwo"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some("two")) ]); - test!(short_two_equals: ["-lc=two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some("two")) ]); - test!(short_two_next: ["-lc", "two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some("two")) ]); + test!(short_arg_eq: ["-c=4"] => frees: [], flags: [(Flag::Short(b'c'), Some(OsStr::new("4"))) ]); + test!(short_arg_then: ["-c", "4"] => frees: [], flags: [(Flag::Short(b'c'), Some(OsStr::new("4"))) ]); + test!(short_two_together: ["-lctwo"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]); + test!(short_two_equals: ["-lc=two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]); + test!(short_two_next: ["-lc", "two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]); // Unknown args @@ -536,7 +542,11 @@ mod matches_test { ($name:ident: $input:expr, has $param:expr => $result:expr) => { #[test] fn $name() { - let flags = MatchedFlags { flags: $input.to_vec() }; + let flags = MatchedFlags { + flags: $input.to_vec(), + strictness: Strictness::UseLastArguments, + }; + assert_eq!(flags.has(&$param), $result); } }; @@ -557,7 +567,12 @@ mod matches_test { #[test] fn only_count() { let everything = os("everything"); - let flags = MatchedFlags { flags: vec![ (Flag::Short(b'c'), Some(&*everything)) ] }; + + let flags = MatchedFlags { + flags: vec![ (Flag::Short(b'c'), Some(&*everything)) ], + strictness: Strictness::UseLastArguments, + }; + assert_eq!(flags.get(&COUNT), Some(&*everything)); } @@ -568,7 +583,8 @@ mod matches_test { let flags = MatchedFlags { flags: vec![ (Flag::Short(b'c'), Some(&*everything)), - (Flag::Short(b'c'), Some(&*nothing)) ] + (Flag::Short(b'c'), Some(&*nothing)) ], + strictness: Strictness::UseLastArguments, }; assert_eq!(flags.get(&COUNT), Some(&*nothing)); @@ -576,7 +592,7 @@ mod matches_test { #[test] fn no_count() { - let flags = MatchedFlags { flags: Vec::new() }; + let flags = MatchedFlags { flags: Vec::new(), strictness: Strictness::UseLastArguments }; assert!(!flags.has(&COUNT)); } diff --git a/src/options/view.rs b/src/options/view.rs index e9e81fc..58202a8 100644 --- a/src/options/view.rs +++ b/src/options/view.rs @@ -407,28 +407,20 @@ lazy_static! { #[cfg(test)] mod test { use super::*; - use std::ffi::OsString; use options::flags; - pub fn os(input: &'static str) -> OsString { - let mut os = OsString::new(); - os.push(input); - os - } - macro_rules! test { ($name:ident: $type:ident <- $inputs:expr => $result:expr) => { #[test] fn $name() { - use options::parser::{Args, Arg}; - use std::ffi::OsString; + use options::parser::Arg; + use options::test::assert_parses; + use options::test::Strictnesses::*; static TEST_ARGS: &[&Arg] = &[ &flags::BINARY, &flags::BYTES, &flags::TIME, &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED ]; - let bits = $inputs.as_ref().into_iter().map(|&o| os(o)).collect::>(); - let results = Args(TEST_ARGS).parse(bits.iter()); - assert_eq!($type::deduce(&results.unwrap().flags), $result); + assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result) } }; } @@ -446,6 +438,14 @@ mod test { mod time_types { use super::*; + use std::ffi::OsString; + + fn os(input: &'static str) -> OsString { + let mut os = OsString::new(); + os.push(input); + os + } + // Default behaviour test!(empty: TimeTypes <- [] => Ok(TimeTypes::default()));