Be stricter in strict mode

Now the code actually starts to use the Strictness flag that was added in the earlier commit! Well, the *code* doesn’t, but the tests do: the macros that create the test cases now have a parameter for which tests they should run. It’s usually ‘Both’ for both strict mode and default mode, but can be specified to only run in one, for when the results differ (usually when options override one another)

The downside to strict mode is that, now, *any* call to `matches.has` or `matches.get` could fail, because an option could have been specified twice, and this is the place where those are checked for. This makes the code a little less ergonomic in places, but that’s what the ? operator is for. The only place this has really had an effect is in `Classify::deduce`, which used to just return a boolean but can now fail.

In order to more thoroughly test the mode, some of the older parts of the code can now act more strict. For example, `TerminalColours::deduce` will now use the last-given option rather than searching for “colours” before “colors”.

Help and Version continue doing their own thing.

src/options/dir_action.rs

@@ -8,12 +8,12 @@ impl DirAction {
 
     /// Determine which action to perform when trying to list a directory.
     pub fn deduce(matches: &MatchedFlags) -> Result<DirAction, Misfire> {
-        let recurse = matches.has(&flags::RECURSE);
-        let list    = matches.has(&flags::LIST_DIRS);
-        let tree    = matches.has(&flags::TREE);
+        let recurse = matches.has(&flags::RECURSE)?;
+        let list    = matches.has(&flags::LIST_DIRS)?;
+        let tree    = matches.has(&flags::TREE)?;
 
         // Early check for --level when it wouldn’t do anything
-        if !recurse && !tree && matches.get(&flags::LEVEL).is_some() {
+        if !recurse && !tree && matches.get(&flags::LEVEL)?.is_some() {
             return Err(Misfire::Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE));
         }
 
@@ -37,7 +37,7 @@ impl RecurseOptions {
 
     /// Determine which files should be recursed into.
     pub fn deduce(matches: &MatchedFlags, tree: bool) -> Result<RecurseOptions, Misfire> {
-        let max_depth = if let Some(level) = matches.get(&flags::LEVEL) {
+        let max_depth = if let Some(level) = matches.get(&flags::LEVEL)? {
             match level.to_string_lossy().parse() {
                 Ok(l)   => Some(l),
                 Err(e)  => return Err(Misfire::FailedParse(e)),
@@ -56,9 +56,10 @@ impl RecurseOptions {
 mod test {
     use super::*;
     use options::flags;
+    use options::parser::Flag;
 
     macro_rules! test {
-        ($name:ident: $type:ident <- $inputs:expr => $result:expr) => {
+        ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => {
             #[test]
             fn $name() {
                 use options::parser::Arg;
@@ -66,32 +67,36 @@ mod test {
                 use options::test::Strictnesses::*;
 
                 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)
+                assert_parses($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf), $result)
             }
         };
     }
 
 
     // Default behaviour
-    test!(empty:           DirAction <- []               => Ok(DirAction::List));
+    test!(empty:           DirAction <- [];               Both => Ok(DirAction::List));
 
     // Listing files as directories
-    test!(dirs_short:      DirAction <- ["-d"]           => Ok(DirAction::AsFile));
-    test!(dirs_long:       DirAction <- ["--list-dirs"]  => Ok(DirAction::AsFile));
+    test!(dirs_short:      DirAction <- ["-d"];           Both => Ok(DirAction::AsFile));
+    test!(dirs_long:       DirAction <- ["--list-dirs"];  Both => Ok(DirAction::AsFile));
 
     // Recursing
     use self::DirAction::Recurse;
-    test!(rec_short:       DirAction <- ["-R"]                           => Ok(Recurse(RecurseOptions { tree: false, max_depth: None })));
-    test!(rec_long:        DirAction <- ["--recurse"]                    => Ok(Recurse(RecurseOptions { tree: false, max_depth: None })));
-    test!(rec_lim_short:   DirAction <- ["-RL4"]                         => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(4) })));
-    test!(rec_lim_short_2: DirAction <- ["-RL=5"]                        => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(5) })));
-    test!(rec_lim_long:    DirAction <- ["--recurse", "--level", "666"]  => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(666) })));
-    test!(rec_lim_long_2:  DirAction <- ["--recurse", "--level=0118"]    => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(118) })));
-    test!(rec_tree:        DirAction <- ["--recurse", "--tree"]          => Ok(Recurse(RecurseOptions { tree: true,  max_depth: None })));
-    test!(rec_short_tree:  DirAction <- ["--tree", "--recurse"]          => Ok(Recurse(RecurseOptions { tree: true,  max_depth: None })));
+    test!(rec_short:       DirAction <- ["-R"];                           Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: None })));
+    test!(rec_long:        DirAction <- ["--recurse"];                    Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: None })));
+    test!(rec_lim_short:   DirAction <- ["-RL4"];                         Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(4) })));
+    test!(rec_lim_short_2: DirAction <- ["-RL=5"];                        Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(5) })));
+    test!(rec_lim_long:    DirAction <- ["--recurse", "--level", "666"];  Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(666) })));
+    test!(rec_lim_long_2:  DirAction <- ["--recurse", "--level=0118"];    Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(118) })));
+    test!(rec_tree:        DirAction <- ["--recurse", "--tree"];          Both => Ok(Recurse(RecurseOptions { tree: true,  max_depth: None })));
+    test!(rec_short_tree:  DirAction <- ["--tree", "--recurse"];          Both => Ok(Recurse(RecurseOptions { tree: true,  max_depth: None })));
 
     // Errors
-    test!(error:           DirAction <- ["--list-dirs", "--recurse"]  => Err(Misfire::Conflict(&flags::RECURSE, &flags::LIST_DIRS)));
-    test!(error_2:         DirAction <- ["--list-dirs", "--tree"]     => Err(Misfire::Conflict(&flags::TREE,    &flags::LIST_DIRS)));
-    test!(underwaterlevel: DirAction <- ["--level=4"]                 => Err(Misfire::Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE)));
+    test!(error:           DirAction <- ["--list-dirs", "--recurse"];     Both => Err(Misfire::Conflict(&flags::RECURSE, &flags::LIST_DIRS)));
+    test!(error_2:         DirAction <- ["--list-dirs", "--tree"];        Both => Err(Misfire::Conflict(&flags::TREE,    &flags::LIST_DIRS)));
+    test!(underwaterlevel: DirAction <- ["--level=4"];                    Both => Err(Misfire::Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE)));
+
+    // Overriding
+    test!(overriding_1:    DirAction <- ["-RL=6", "-L=7"];                Last => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(7) })));
+    test!(overriding_2:    DirAction <- ["-RL=6", "-L=7"];            Complain => Err(Misfire::Duplicate(Flag::Short(b'L'), Flag::Short(b'L'))));
 }

src/options/filter.rs

@@ -11,8 +11,8 @@ impl FileFilter {
     /// command-line arguments.
     pub fn deduce(matches: &MatchedFlags) -> Result<FileFilter, Misfire> {
         Ok(FileFilter {
-            list_dirs_first: matches.has(&flags::DIRS_FIRST),
-            reverse:         matches.has(&flags::REVERSE),
+            list_dirs_first: matches.has(&flags::DIRS_FIRST)?,
+            reverse:         matches.has(&flags::REVERSE)?,
             sort_field:      SortField::deduce(matches)?,
             dot_filter:      DotFilter::deduce(matches)?,
             ignore_patterns: IgnorePatterns::deduce(matches)?,
@@ -38,7 +38,7 @@ impl SortField {
     /// argument. This will return `Err` if the option is there, but does not
     /// correspond to a valid field.
     fn deduce(matches: &MatchedFlags) -> Result<SortField, Misfire> {
-        let word = match matches.get(&flags::SORT) {
+        let word = match matches.get(&flags::SORT)? {
             Some(w)  => w,
             None     => return Ok(SortField::default()),
         };
@@ -85,11 +85,22 @@ impl SortField {
 
 impl DotFilter {
     pub fn deduce(matches: &MatchedFlags) -> Result<DotFilter, Misfire> {
-        match matches.count(&flags::ALL) {
-            0 => Ok(DotFilter::JustFiles),
-            1 => Ok(DotFilter::Dotfiles),
-            _ => if matches.has(&flags::TREE) { Err(Misfire::TreeAllAll) }
-                                         else { Ok(DotFilter::DotfilesAndDots) }
+        let count = matches.count(&flags::ALL);
+
+        if count == 0 {
+            Ok(DotFilter::JustFiles)
+        }
+        else if count == 1 {
+            Ok(DotFilter::Dotfiles)
+        }
+        else if matches.count(&flags::TREE) > 0 {
+            Err(Misfire::TreeAllAll)
+        }
+        else if count >= 3 && matches.is_strict() {
+            Err(Misfire::Conflict(&flags::ALL, &flags::ALL))
+        }
+        else {
+            Ok(DotFilter::DotfilesAndDots)
         }
     }
 }
@@ -101,7 +112,7 @@ impl IgnorePatterns {
     /// command-line arguments.
     pub fn deduce(matches: &MatchedFlags) -> Result<IgnorePatterns, Misfire> {
 
-        let inputs = match matches.get(&flags::IGNORE_GLOB) {
+        let inputs = match matches.get(&flags::IGNORE_GLOB)? {
             None => return Ok(IgnorePatterns::empty()),
             Some(is) => is,
         };
@@ -126,6 +137,7 @@ mod test {
     use super::*;
     use std::ffi::OsString;
     use options::flags;
+    use options::parser::Flag;
 
     pub fn os(input: &'static str) -> OsString {
         let mut os = OsString::new();
@@ -134,7 +146,7 @@ mod test {
     }
 
     macro_rules! test {
-        ($name:ident: $type:ident <- $inputs:expr => $result:expr) => {
+        ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => {
             #[test]
             fn $name() {
                 use options::parser::Arg;
@@ -142,7 +154,7 @@ mod test {
                 use options::test::Strictnesses::*;
 
                 static TEST_ARGS: &[&Arg] = &[ &flags::SORT, &flags::ALL, &flags::TREE, &flags::IGNORE_GLOB ];
-                assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result)
+                assert_parses($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf), $result)
             }
         };
     }
@@ -151,21 +163,23 @@ mod test {
         use super::*;
 
         // Default behaviour
-        test!(empty:         SortField <- []                  => Ok(SortField::default()));
+        test!(empty:         SortField <- [];                  Both => Ok(SortField::default()));
 
         // Sort field arguments
-        test!(one_arg:       SortField <- ["--sort=cr"]       => Ok(SortField::CreatedDate));
-        test!(one_long:      SortField <- ["--sort=size"]     => Ok(SortField::Size));
-        test!(one_short:     SortField <- ["-saccessed"]      => Ok(SortField::AccessedDate));
-        test!(lowercase:     SortField <- ["--sort", "name"]  => Ok(SortField::Name(SortCase::Sensitive)));
-        test!(uppercase:     SortField <- ["--sort", "Name"]  => Ok(SortField::Name(SortCase::Insensitive)));
+        test!(one_arg:       SortField <- ["--sort=cr"];       Both => Ok(SortField::CreatedDate));
+        test!(one_long:      SortField <- ["--sort=size"];     Both => Ok(SortField::Size));
+        test!(one_short:     SortField <- ["-saccessed"];      Both => Ok(SortField::AccessedDate));
+        test!(lowercase:     SortField <- ["--sort", "name"];  Both => Ok(SortField::Name(SortCase::Sensitive)));
+        test!(uppercase:     SortField <- ["--sort", "Name"];  Both => Ok(SortField::Name(SortCase::Insensitive)));
 
         // Errors
-        test!(error:         SortField <- ["--sort=colour"]   => Err(Misfire::bad_argument(&flags::SORT, &os("colour"), super::SORTS)));
+        test!(error:         SortField <- ["--sort=colour"];   Both => Err(Misfire::bad_argument(&flags::SORT, &os("colour"), super::SORTS)));
 
         // Overriding
-        test!(overridden:    SortField <- ["--sort=cr",       "--sort", "mod"]     => Ok(SortField::ModifiedDate));
-        test!(overridden_2:  SortField <- ["--sort", "none",  "--sort=Extension"]  => Ok(SortField::Extension(SortCase::Insensitive)));
+        test!(overridden:    SortField <- ["--sort=cr",       "--sort", "mod"];     Last => Ok(SortField::ModifiedDate));
+        test!(overridden_2:  SortField <- ["--sort", "none",  "--sort=Extension"];  Last => Ok(SortField::Extension(SortCase::Insensitive)));
+        test!(overridden_3:  SortField <- ["--sort=cr",       "--sort", "mod"];     Complain => Err(Misfire::Duplicate(Flag::Long("sort"), Flag::Long("sort"))));
+        test!(overridden_4:  SortField <- ["--sort", "none",  "--sort=Extension"];  Complain => Err(Misfire::Duplicate(Flag::Long("sort"), Flag::Long("sort"))));
     }
 
 
@@ -173,16 +187,20 @@ mod test {
         use super::*;
 
         // Default behaviour
-        test!(empty:      DotFilter <- []               => Ok(DotFilter::JustFiles));
+        test!(empty:      DotFilter <- [];               Both => Ok(DotFilter::JustFiles));
 
         // --all
-        test!(all:        DotFilter <- ["--all"]        => Ok(DotFilter::Dotfiles));
-        test!(all_all:    DotFilter <- ["--all", "-a"]  => Ok(DotFilter::DotfilesAndDots));
-        test!(all_all_2:  DotFilter <- ["-aa"]          => Ok(DotFilter::DotfilesAndDots));
+        test!(all:        DotFilter <- ["--all"];        Both => Ok(DotFilter::Dotfiles));
+        test!(all_all:    DotFilter <- ["--all", "-a"];  Both => Ok(DotFilter::DotfilesAndDots));
+        test!(all_all_2:  DotFilter <- ["-aa"];          Both => Ok(DotFilter::DotfilesAndDots));
+
+        test!(all_all_3:  DotFilter <- ["-aaa"];         Last => Ok(DotFilter::DotfilesAndDots));
+        test!(all_all_4:  DotFilter <- ["-aaa"];         Complain => Err(Misfire::Conflict(&flags::ALL, &flags::ALL)));
 
         // --all and --tree
-        test!(tree_a:     DotFilter <- ["-Ta"]          => Ok(DotFilter::Dotfiles));
-        test!(tree_aa:    DotFilter <- ["-Taa"]         => Err(Misfire::TreeAllAll));
+        test!(tree_a:     DotFilter <- ["-Ta"];          Both => Ok(DotFilter::Dotfiles));
+        test!(tree_aa:    DotFilter <- ["-Taa"];         Both => Err(Misfire::TreeAllAll));
+        test!(tree_aaa:   DotFilter <- ["-Taaa"];        Both => Err(Misfire::TreeAllAll));
     }
 
 
@@ -196,9 +214,15 @@ mod test {
         }
 
         // Various numbers of globs
-        test!(none:   IgnorePatterns <- []                             => Ok(IgnorePatterns::empty()));
-        test!(one:    IgnorePatterns <- ["--ignore-glob", "*.ogg"]     => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg") ])));
-        test!(two:    IgnorePatterns <- ["--ignore-glob=*.ogg|*.MP3"]  => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg"), pat("*.MP3") ])));
-        test!(loads:  IgnorePatterns <- ["-I*|?|.|*"]                  => Ok(IgnorePatterns::from_iter(vec![ pat("*"), pat("?"), pat("."), pat("*") ])));
+        test!(none:   IgnorePatterns <- [];                             Both => Ok(IgnorePatterns::empty()));
+        test!(one:    IgnorePatterns <- ["--ignore-glob", "*.ogg"];     Both => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg") ])));
+        test!(two:    IgnorePatterns <- ["--ignore-glob=*.ogg|*.MP3"];  Both => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg"), pat("*.MP3") ])));
+        test!(loads:  IgnorePatterns <- ["-I*|?|.|*"];                  Both => Ok(IgnorePatterns::from_iter(vec![ pat("*"), pat("?"), pat("."), pat("*") ])));
+
+        // Overriding
+        test!(overridden:   IgnorePatterns <- ["-I=*.ogg",    "-I", "*.mp3"];  Last => Ok(IgnorePatterns::from_iter(vec![ pat("*.mp3") ])));
+        test!(overridden_2: IgnorePatterns <- ["-I", "*.OGG", "-I*.MP3"];      Last => Ok(IgnorePatterns::from_iter(vec![ pat("*.MP3") ])));
+        test!(overridden_3: IgnorePatterns <- ["-I=*.ogg",    "-I", "*.mp3"];  Complain => Err(Misfire::Duplicate(Flag::Short(b'I'), Flag::Short(b'I'))));
+        test!(overridden_4: IgnorePatterns <- ["-I", "*.OGG", "-I*.MP3"];      Complain => Err(Misfire::Duplicate(Flag::Short(b'I'), Flag::Short(b'I'))));
     }
 }

src/options/help.rs

@@ -72,9 +72,13 @@ impl HelpString {
     /// Determines how to show help, if at all, based on the user’s
     /// command-line arguments. This one works backwards from the other
     /// ‘deduce’ functions, returning Err if help needs to be shown.
+    ///
+    /// We don’t do any strict-mode error checking here: it’s OK to give
+    /// the --help or --long flags more than once. Actually checking for
+    /// errors when the user wants help is kind of petty!
     pub fn deduce(matches: &MatchedFlags) -> Result<(), HelpString> {
-        if matches.has(&flags::HELP) {
-            let only_long = matches.has(&flags::LONG);
+        if matches.count(&flags::HELP) > 0 {
+            let only_long = matches.count(&flags::LONG) > 0;
             let git       = cfg!(feature="git");
             let xattrs    = xattr::ENABLED;
             Err(HelpString { only_long, git, xattrs })

src/options/misfire.rs

@@ -5,7 +5,7 @@ use std::num::ParseIntError;
 use glob;
 
 use options::{HelpString, VersionString};
-use options::parser::{Arg, ParseError};
+use options::parser::{Arg, Flag, ParseError};
 
 
 /// A list of legal choices for an argument-taking option
@@ -36,6 +36,9 @@ pub enum Misfire {
     /// The user wanted the version number.
     Version(VersionString),
 
+    /// An option was given twice or more in strict mode.
+    Duplicate(Flag, Flag),
+
     /// Two options were given that conflict with one another.
     Conflict(&'static Arg, &'static Arg),
 
@@ -93,6 +96,7 @@ impl fmt::Display for Misfire {
             Help(ref text)                   => write!(f, "{}", text),
             Version(ref version)             => write!(f, "{}", version),
             Conflict(ref a, ref b)           => write!(f, "Option {} conflicts with option {}.", a, b),
+            Duplicate(ref a, ref b)          => write!(f, "Flag {:?} conflicts with flag {:?}.", a, b),
             Useless(ref a, false, ref b)     => write!(f, "Option {} is useless without option {}.", a, b),
             Useless(ref a, true, ref b)      => write!(f, "Option {} is useless given option {}.", a, b),
             Useless2(ref a, ref b1, ref b2)  => write!(f, "Option {} is useless without options {} or {}.", a, b1, b2),

src/options/mod.rs

@@ -170,6 +170,9 @@ pub mod test {
         Both,
     }
 
+    /// This function gets used by the other testing modules.
+    /// It can run with one or both strictness values: if told to run with
+    /// both, then both should resolve to the same result.
     pub fn assert_parses<T, F>(inputs: &[&str], args: &'static [&'static Arg], strictnesses: Strictnesses, get: F, result: T)
     where  T: PartialEq + Debug,  F: Fn(&MatchedFlags) -> T
     {

src/options/parser.rs

@@ -31,6 +31,8 @@
 use std::ffi::{OsStr, OsString};
 use std::fmt;
 
+use options::Misfire;
+
 
 /// A **short argument** is a single ASCII character.
 pub type ShortArg = u8;
@@ -50,7 +52,7 @@ pub enum Flag {
 }
 
 impl Flag {
-    fn matches(&self, arg: &Arg) -> bool {
+    pub fn matches(&self, arg: &Arg) -> bool {
         match *self {
             Flag::Short(short)  => arg.short == Some(short),
             Flag::Long(long)    => arg.long == long,
@@ -312,21 +314,55 @@ pub struct MatchedFlags<'args> {
     strictness: Strictness,
 }
 
+use self::Strictness::*;
 impl<'a> MatchedFlags<'a> {
 
     /// Whether the given argument was specified.
-    pub fn has(&self, arg: &Arg) -> bool {
-        self.flags.iter().rev()
-            .find(|tuple| tuple.1.is_none() && tuple.0.matches(arg))
-            .is_some()
+    pub fn has(&self, arg: &'static Arg) -> Result<bool, Misfire> {
+        match self.strictness {
+            UseLastArguments => {
+                let any = self.flags.iter().rev()
+                              .find(|tuple| tuple.1.is_none() && tuple.0.matches(arg))
+                              .is_some();
+                Ok(any)
+            }
+            ComplainAboutRedundantArguments => {
+                let all = self.flags.iter()
+                              .filter(|tuple| tuple.1.is_none() && tuple.0.matches(arg))
+                              .collect::<Vec<_>>();
+
+                if all.len() < 2 { Ok(all.len() == 1) }
+                             else { Err(Misfire::Conflict(arg, arg)) }
+            }
+        }
+    }
+
+    // This code could probably be better.
+
+    pub fn get(&self, arg: &'static Arg) -> Result<Option<&OsStr>, Misfire> {
+        self.get_where(|flag| flag.matches(arg))
     }
 
     /// If the given argument was specified, return its value.
     /// The value is not guaranteed to be valid UTF-8.
-    pub fn get(&self, arg: &Arg) -> Option<&OsStr> {
-        self.flags.iter().rev()
-            .find(|tuple| tuple.1.is_some() && tuple.0.matches(arg))
-            .map(|tuple| tuple.1.unwrap())
+    pub fn get_where<P>(&self, predicate: P) -> Result<Option<&OsStr>, Misfire>
+    where P: Fn(&Flag) -> bool {
+        match self.strictness {
+            UseLastArguments => {
+                let found = self.flags.iter().rev()
+                                .find(|tuple| tuple.1.is_some() && predicate(&tuple.0))
+                                .map(|tuple| tuple.1.unwrap());
+                Ok(found)
+            }
+            ComplainAboutRedundantArguments => {
+                let those = self.flags.iter()
+                                .filter(|tuple| tuple.1.is_some() && predicate(&tuple.0))
+                                .collect::<Vec<_>>();
+
+                if those.len() < 2 { Ok(those.first().cloned().map(|t| t.1.unwrap())) }
+                               else { Err(Misfire::Duplicate(those[0].0.clone(), those[1].0.clone())) }
+            }
+        }
     }
 
     // It’s annoying that ‘has’ and ‘get’ won’t work when accidentally given
@@ -338,6 +374,12 @@ impl<'a> MatchedFlags<'a> {
             .filter(|tuple| tuple.0.matches(arg))
             .count()
     }
+
+    /// Checks whether strict mode is on. This is usually done from within
+    /// ‘has’ and ‘get’, but it’s available in an emergency.
+    pub fn is_strict(&self) -> bool {
+        self.strictness == Strictness::ComplainAboutRedundantArguments
+    }
 }
 
 
@@ -547,7 +589,7 @@ mod matches_test {
                     strictness: Strictness::UseLastArguments,
                 };
 
-                assert_eq!(flags.has(&$param), $result);
+                assert_eq!(flags.has(&$param), Ok($result));
             }
         };
     }
@@ -573,7 +615,7 @@ mod matches_test {
             strictness: Strictness::UseLastArguments,
         };
 
-        assert_eq!(flags.get(&COUNT), Some(&*everything));
+        assert_eq!(flags.get(&COUNT), Ok(Some(&*everything)));
     }
 
     #[test]
@@ -587,13 +629,13 @@ mod matches_test {
             strictness: Strictness::UseLastArguments,
         };
 
-        assert_eq!(flags.get(&COUNT), Some(&*nothing));
+        assert_eq!(flags.get(&COUNT), Ok(Some(&*nothing)));
     }
 
     #[test]
     fn no_count() {
         let flags = MatchedFlags { flags: Vec::new(), strictness: Strictness::UseLastArguments };
 
-        assert!(!flags.has(&COUNT));
+        assert!(!flags.has(&COUNT).unwrap());
     }
 }

src/options/version.rs

@@ -17,8 +17,10 @@ impl VersionString {
     /// Determines how to show the version, if at all, based on the user’s
     /// command-line arguments. This one works backwards from the other
     /// ‘deduce’ functions, returning Err if help needs to be shown.
+    ///
+    /// Like --help, this doesn’t bother checking for errors.
     pub fn deduce(matches: &MatchedFlags) -> Result<(), VersionString> {
-        if matches.has(&flags::VERSION) {
+        if matches.count(&flags::VERSION) > 0 {
             Err(VersionString { cargo: env!("CARGO_PKG_VERSION") })
         }
         else {

src/options/view.rs

@@ -19,7 +19,7 @@ impl View {
     pub fn deduce(matches: &MatchedFlags) -> Result<View, Misfire> {
         let mode = Mode::deduce(matches)?;
         let colours = Colours::deduce(matches)?;
-        let style = FileStyle::deduce(matches);
+        let style = FileStyle::deduce(matches)?;
         Ok(View { mode, colours, style })
     }
 }
@@ -32,17 +32,17 @@ impl Mode {
         use options::misfire::Misfire::*;
 
         let long = || {
-            if matches.has(&flags::ACROSS) && !matches.has(&flags::GRID) {
+            if matches.has(&flags::ACROSS)? && !matches.has(&flags::GRID)? {
                 Err(Useless(&flags::ACROSS, true, &flags::LONG))
             }
-            else if matches.has(&flags::ONE_LINE) {
+            else if matches.has(&flags::ONE_LINE)? {
                 Err(Useless(&flags::ONE_LINE, true, &flags::LONG))
             }
             else {
                 Ok(details::Options {
                     table: Some(TableOptions::deduce(matches)?),
-                    header: matches.has(&flags::HEADER),
-                    xattr: xattr::ENABLED && matches.has(&flags::EXTENDED),
+                    header: matches.has(&flags::HEADER)?,
+                    xattr: xattr::ENABLED && matches.has(&flags::EXTENDED)?,
                 })
             }
         };
@@ -50,15 +50,15 @@ impl Mode {
         let long_options_scan = || {
             for option in &[ &flags::BINARY, &flags::BYTES, &flags::INODE, &flags::LINKS,
                              &flags::HEADER, &flags::BLOCKS, &flags::TIME, &flags::GROUP ] {
-                if matches.has(option) {
+                if matches.has(option)? {
                     return Err(Useless(*option, false, &flags::LONG));
                 }
             }
 
-            if cfg!(feature="git") && matches.has(&flags::GIT) {
+            if cfg!(feature="git") && matches.has(&flags::GIT)? {
                 Err(Useless(&flags::GIT, false, &flags::LONG))
             }
-            else if matches.has(&flags::LEVEL) && !matches.has(&flags::RECURSE) && !matches.has(&flags::TREE) {
+            else if matches.has(&flags::LEVEL)? && !matches.has(&flags::RECURSE)? && !matches.has(&flags::TREE)? {
                 Err(Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE))
             }
             else {
@@ -68,26 +68,26 @@ impl Mode {
 
         let other_options_scan = || {
             if let Some(width) = TerminalWidth::deduce()?.width() {
-                if matches.has(&flags::ONE_LINE) {
-                    if matches.has(&flags::ACROSS) {
+                if matches.has(&flags::ONE_LINE)? {
+                    if matches.has(&flags::ACROSS)? {
                         Err(Useless(&flags::ACROSS, true, &flags::ONE_LINE))
                     }
                     else {
                         Ok(Mode::Lines)
                     }
                 }
-                else if matches.has(&flags::TREE) {
+                else if matches.has(&flags::TREE)? {
                     let details = details::Options {
                         table: None,
                         header: false,
-                        xattr: xattr::ENABLED && matches.has(&flags::EXTENDED),
+                        xattr: xattr::ENABLED && matches.has(&flags::EXTENDED)?,
                     };
 
                     Ok(Mode::Details(details))
                 }
                 else {
                     let grid = grid::Options {
-                        across: matches.has(&flags::ACROSS),
+                        across: matches.has(&flags::ACROSS)?,
                         console_width: width,
                     };
 
@@ -99,11 +99,11 @@ impl Mode {
                 // as the program’s stdout being connected to a file, then
                 // fallback to the lines view.
 
-                if matches.has(&flags::TREE) {
+                if matches.has(&flags::TREE)? {
                     let details = details::Options {
                         table: None,
                         header: false,
-                        xattr: xattr::ENABLED && matches.has(&flags::EXTENDED),
+                        xattr: xattr::ENABLED && matches.has(&flags::EXTENDED)?,
                     };
 
                     Ok(Mode::Details(details))
@@ -114,9 +114,9 @@ impl Mode {
             }
         };
 
-        if matches.has(&flags::LONG) {
+        if matches.has(&flags::LONG)? {
             let details = long()?;
-            if matches.has(&flags::GRID) {
+            if matches.has(&flags::GRID)? {
                 match other_options_scan()? {
                     Mode::Grid(grid)  => return Ok(Mode::GridDetails(grid, details)),
                     others            => return Ok(others),
@@ -185,11 +185,11 @@ impl TableOptions {
             time_format: TimeFormat::deduce(matches)?,
             size_format: SizeFormat::deduce(matches)?,
             time_types:  TimeTypes::deduce(matches)?,
-            inode:  matches.has(&flags::INODE),
-            links:  matches.has(&flags::LINKS),
-            blocks: matches.has(&flags::BLOCKS),
-            group:  matches.has(&flags::GROUP),
-            git:    cfg!(feature="git") && matches.has(&flags::GIT),
+            inode:  matches.has(&flags::INODE)?,
+            links:  matches.has(&flags::LINKS)?,
+            blocks: matches.has(&flags::BLOCKS)?,
+            group:  matches.has(&flags::GROUP)?,
+            git:    cfg!(feature="git") && matches.has(&flags::GIT)?,
         })
     }
 }
@@ -206,8 +206,8 @@ impl SizeFormat {
     /// involves the `--binary` or `--bytes` flags, and these conflict with
     /// each other.
     fn deduce(matches: &MatchedFlags) -> Result<SizeFormat, Misfire> {
-        let binary = matches.has(&flags::BINARY);
-        let bytes  = matches.has(&flags::BYTES);
+        let binary = matches.has(&flags::BINARY)?;
+        let bytes  = matches.has(&flags::BYTES)?;
 
         match (binary, bytes) {
             (true,  true )  => Err(Misfire::Conflict(&flags::BINARY, &flags::BYTES)),
@@ -226,7 +226,7 @@ impl TimeFormat {
         pub use output::time::{DefaultFormat, ISOFormat};
         const STYLES: &[&str] = &["default", "long-iso", "full-iso", "iso"];
 
-        let word = match matches.get(&flags::TIME_STYLE) {
+        let word = match matches.get(&flags::TIME_STYLE)? {
             Some(w) => w,
             None    => return Ok(TimeFormat::DefaultFormat(DefaultFormat::new())),
         };
@@ -265,10 +265,10 @@ impl TimeTypes {
     /// option, but passing *no* options means that the user just wants to
     /// see the default set.
     fn deduce(matches: &MatchedFlags) -> Result<TimeTypes, Misfire> {
-        let possible_word = matches.get(&flags::TIME);
-        let modified = matches.has(&flags::MODIFIED);
-        let created  = matches.has(&flags::CREATED);
-        let accessed = matches.has(&flags::ACCESSED);
+        let possible_word = matches.get(&flags::TIME)?;
+        let modified = matches.has(&flags::MODIFIED)?;
+        let created  = matches.has(&flags::CREATED)?;
+        let accessed = matches.has(&flags::ACCESSED)?;
 
         if let Some(word) = possible_word {
             if modified {
@@ -329,13 +329,14 @@ impl Default for TerminalColours {
     }
 }
 
+const COLOURS: &[&str] = &["always", "auto", "never"];
+
 impl TerminalColours {
 
     /// Determine which terminal colour conditions to use.
     fn deduce(matches: &MatchedFlags) -> Result<TerminalColours, Misfire> {
-        const COLOURS: &[&str] = &["always", "auto", "never"];
 
-        let word = match matches.get(&flags::COLOR).or_else(|| matches.get(&flags::COLOUR)) {
+        let word = match matches.get_where(|f| f.matches(&flags::COLOR) || f.matches(&flags::COLOUR))? {
             Some(w) => w,
             None    => return Ok(TerminalColours::default()),
         };
@@ -362,7 +363,7 @@ impl Colours {
 
         let tc = TerminalColours::deduce(matches)?;
         if tc == Always || (tc == Automatic && TERM_WIDTH.is_some()) {
-            let scale = matches.has(&flags::COLOR_SCALE) || matches.has(&flags::COLOUR_SCALE);
+            let scale = matches.has(&flags::COLOR_SCALE)? || matches.has(&flags::COLOUR_SCALE)?;
             Ok(Colours::colourful(scale))
         }
         else {
@@ -374,17 +375,19 @@ impl Colours {
 
 
 impl FileStyle {
-    fn deduce(matches: &MatchedFlags) -> FileStyle {
-        let classify = Classify::deduce(matches);
+    fn deduce(matches: &MatchedFlags) -> Result<FileStyle, Misfire> {
+        let classify = Classify::deduce(matches)?;
         let exts = FileExtensions;
-        FileStyle { classify, exts }
+        Ok(FileStyle { classify, exts })
     }
 }
 
 impl Classify {
-    fn deduce(matches: &MatchedFlags) -> Classify {
-        if matches.has(&flags::CLASSIFY) { Classify::AddFileIndicators }
-                                    else { Classify::JustFilenames }
+    fn deduce(matches: &MatchedFlags) -> Result<Classify, Misfire> {
+        let flagged = matches.has(&flags::CLASSIFY)?;
+
+        Ok(if flagged { Classify::AddFileIndicators }
+                 else { Classify::JustFilenames })
     }
 }
 
@@ -407,10 +410,18 @@ lazy_static! {
 #[cfg(test)]
 mod test {
     use super::*;
+    use std::ffi::OsString;
     use options::flags;
+    use options::parser::Flag;
+
+    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) => {
+        ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => {
             #[test]
             fn $name() {
                 use options::parser::Arg;
@@ -418,9 +429,10 @@ mod test {
                 use options::test::Strictnesses::*;
 
                 static TEST_ARGS: &[&Arg] = &[ &flags::BINARY, &flags::BYTES,
-                                               &flags::TIME, &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED ];
+                                               &flags::TIME,   &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED,
+                                               &flags::COLOR,  &flags::COLOUR ];
 
-                assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result)
+                assert_parses($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf), $result)
             }
         };
     }
@@ -429,49 +441,75 @@ mod test {
     mod size_formats {
         use super::*;
 
-        test!(empty:   SizeFormat <- []                       => Ok(SizeFormat::DecimalBytes));
-        test!(binary:  SizeFormat <- ["--binary"]             => Ok(SizeFormat::BinaryBytes));
-        test!(bytes:   SizeFormat <- ["--bytes"]              => Ok(SizeFormat::JustBytes));
-        test!(both:    SizeFormat <- ["--binary", "--bytes"]  => Err(Misfire::Conflict(&flags::BINARY, &flags::BYTES)));
+        test!(empty:   SizeFormat <- [];                       Both => Ok(SizeFormat::DecimalBytes));
+        test!(binary:  SizeFormat <- ["--binary"];             Both => Ok(SizeFormat::BinaryBytes));
+        test!(bytes:   SizeFormat <- ["--bytes"];              Both => Ok(SizeFormat::JustBytes));
+        test!(both:    SizeFormat <- ["--binary", "--bytes"];  Both => Err(Misfire::Conflict(&flags::BINARY, &flags::BYTES)));
     }
 
 
     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()));
-        test!(modified:  TimeTypes <- ["--modified"]          => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
-        test!(m:         TimeTypes <- ["-m"]                  => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
-        test!(time_mod:  TimeTypes <- ["--time=modified"]     => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
-        test!(time_m:    TimeTypes <- ["-tmod"]               => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
-
-        test!(acc:       TimeTypes <- ["--accessed"]          => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
-        test!(a:         TimeTypes <- ["-u"]                  => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
-        test!(time_acc:  TimeTypes <- ["--time", "accessed"]  => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
-        test!(time_a:    TimeTypes <- ["-t", "acc"]           => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
-
-        test!(cr:        TimeTypes <- ["--created"]           => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
-        test!(c:         TimeTypes <- ["-U"]                  => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
-        test!(time_cr:   TimeTypes <- ["--time=created"]      => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
-        test!(time_c:    TimeTypes <- ["-tcr"]                => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
+        test!(empty:     TimeTypes <- [];                      Both => Ok(TimeTypes::default()));
+        test!(modified:  TimeTypes <- ["--modified"];          Both => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
+        test!(m:         TimeTypes <- ["-m"];                  Both => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
+        test!(time_mod:  TimeTypes <- ["--time=modified"];     Both => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
+        test!(time_m:    TimeTypes <- ["-tmod"];               Both => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
+
+        test!(acc:       TimeTypes <- ["--accessed"];          Both => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
+        test!(a:         TimeTypes <- ["-u"];                  Both => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
+        test!(time_acc:  TimeTypes <- ["--time", "accessed"];  Both => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
+        test!(time_a:    TimeTypes <- ["-t", "acc"];           Both => Ok(TimeTypes { accessed: true,   modified: false,  created: false }));
+
+        test!(cr:        TimeTypes <- ["--created"];           Both => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
+        test!(c:         TimeTypes <- ["-U"];                  Both => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
+        test!(time_cr:   TimeTypes <- ["--time=created"];      Both => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
+        test!(time_c:    TimeTypes <- ["-tcr"];                Both => Ok(TimeTypes { accessed: false,  modified: false,  created: true  }));
 
         // Multiples
-        test!(time_uu:    TimeTypes <- ["-uU"]                => Ok(TimeTypes { accessed: true,   modified: false,  created: true  }));
+        test!(time_uu:   TimeTypes <- ["-uU"];                 Both => Ok(TimeTypes { accessed: true,   modified: false,  created: true  }));
+
+        // Errors
+        test!(time_tea:  TimeTypes <- ["--time=tea"];          Both => Err(Misfire::bad_argument(&flags::TIME, &os("tea"), super::TIMES)));
+        test!(time_ea:   TimeTypes <- ["-tea"];                Both => Err(Misfire::bad_argument(&flags::TIME, &os("ea"), super::TIMES)));
 
         // Overriding
-        test!(time_mc:    TimeTypes <- ["-tcr", "-tmod"]      => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
+        test!(overridden:   TimeTypes <- ["-tcr", "-tmod"];    Last => Ok(TimeTypes { accessed: false,  modified: true,   created: false }));
+        test!(overridden_2: TimeTypes <- ["-tcr", "-tmod"];    Complain => Err(Misfire::Duplicate(Flag::Short(b't'), Flag::Short(b't'))));
+    }
+
+
+    mod colourses {
+        use super::*;
+
+        // Default
+        test!(empty:        TerminalColours <- [];                     Both => Ok(TerminalColours::default()));
+
+        // --colour
+        test!(u_always:     TerminalColours <- ["--colour=always"];    Both => Ok(TerminalColours::Always));
+        test!(u_auto:       TerminalColours <- ["--colour", "auto"];   Both => Ok(TerminalColours::Automatic));
+        test!(u_never:      TerminalColours <- ["--colour=never"];     Both => Ok(TerminalColours::Never));
+
+        // --color
+        test!(no_u_always:  TerminalColours <- ["--color", "always"];  Both => Ok(TerminalColours::Always));
+        test!(no_u_auto:    TerminalColours <- ["--color=auto"];       Both => Ok(TerminalColours::Automatic));
+        test!(no_u_never:   TerminalColours <- ["--color", "never"];   Both => Ok(TerminalColours::Never));
 
         // Errors
-        test!(time_tea:  TimeTypes <- ["--time=tea"]  => Err(Misfire::bad_argument(&flags::TIME, &os("tea"), super::TIMES)));
-        test!(time_ea:   TimeTypes <- ["-tea"]        => Err(Misfire::bad_argument(&flags::TIME, &os("ea"), super::TIMES)));
+        test!(no_u_error:   TerminalColours <- ["--color=upstream"];   Both => Err(Misfire::bad_argument(&flags::COLOR, &os("upstream"), super::COLOURS)));  // the error is for --color
+        test!(u_error:      TerminalColours <- ["--colour=lovers"];    Both => Err(Misfire::bad_argument(&flags::COLOR, &os("lovers"),   super::COLOURS)));  // and so is this one!
+
+        // Overriding
+        test!(overridden_1: TerminalColours <- ["--colour=auto", "--colour=never"];  Last => Ok(TerminalColours::Never));
+        test!(overridden_2: TerminalColours <- ["--color=auto",  "--colour=never"];  Last => Ok(TerminalColours::Never));
+        test!(overridden_3: TerminalColours <- ["--colour=auto", "--color=never"];   Last => Ok(TerminalColours::Never));
+        test!(overridden_4: TerminalColours <- ["--color=auto",  "--color=never"];   Last => Ok(TerminalColours::Never));
+
+        test!(overridden_5: TerminalColours <- ["--colour=auto", "--colour=never"];  Complain => Err(Misfire::Duplicate(Flag::Long("colour"), Flag::Long("colour"))));
+        test!(overridden_6: TerminalColours <- ["--color=auto",  "--colour=never"];  Complain => Err(Misfire::Duplicate(Flag::Long("color"),  Flag::Long("colour"))));
+        test!(overridden_7: TerminalColours <- ["--colour=auto", "--color=never"];   Complain => Err(Misfire::Duplicate(Flag::Long("colour"), Flag::Long("color"))));
+        test!(overridden_8: TerminalColours <- ["--color=auto",  "--color=never"];   Complain => Err(Misfire::Duplicate(Flag::Long("color"),  Flag::Long("color"))));
     }
 }