From d77dabadbb691d6eb530f8af989478378ab86c73 Mon Sep 17 00:00:00 2001
From: Axel Chalon <tnf_chordmaster@hotmail.fr>
Date: Fri, 13 Oct 2017 12:20:57 +0200
Subject: [PATCH] CLI: Reject invalid argument values rather than ignore them
 (#6723)

* CLI: Reject invalid argument values rather than ignore them

* Fix grumbles
---
 parity/cli/mod.rs   | 10 +++++++++
 parity/cli/usage.rs | 52 ++++++++++++++++++++++++++++-----------------
 2 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs
index 18d800546..774256d02 100644
--- a/parity/cli/mod.rs
+++ b/parity/cli/mod.rs
@@ -1203,6 +1203,16 @@ mod tests {
 	use toml;
 	use clap::{ErrorKind as ClapErrorKind};
 
+
+	#[test]
+	fn should_reject_invalid_values() {
+		let args = Args::parse(&["parity", "--cache=20"]);
+		assert!(args.is_ok());
+
+		let args = Args::parse(&["parity", "--cache=asd"]);
+		assert!(args.is_err());
+	}
+
 	#[test]
 	fn should_parse_args_and_flags() {
 		let args = Args::parse(&["parity", "--no-warp"]).unwrap();
diff --git a/parity/cli/usage.rs b/parity/cli/usage.rs
index dd370bdbc..83b878ee1 100644
--- a/parity/cli/usage.rs
+++ b/parity/cli/usage.rs
@@ -32,6 +32,20 @@ macro_rules! otry {
 	)
 }
 
+macro_rules! return_if_parse_error {
+	($e:expr) => (
+		match $e {
+			Err(clap_error @ ClapError { kind: ClapErrorKind::ValueValidation, .. }) => {
+				return Err(clap_error);
+			},
+
+			// Otherwise, if $e is ClapErrorKind::ArgumentNotFound or Ok(),
+			// then convert to Option
+			_ => $e.ok()
+		}
+	)
+}
+
 macro_rules! if_option {
 	(Option<$type:ty>, THEN {$($then:tt)*} ELSE {$($otherwise:tt)*}) => (
 		$($then)*
@@ -139,7 +153,7 @@ macro_rules! usage {
 		use std::{fs, io, process};
 		use std::io::{Read, Write};
 		use util::version;
-		use clap::{Arg, App, SubCommand, AppSettings, Error as ClapError};
+		use clap::{Arg, App, SubCommand, AppSettings, Error as ClapError, ErrorKind as ClapErrorKind};
 		use helpers::replace_home;
 		use std::ffi::OsStr;
 		use std::collections::HashMap;
@@ -563,23 +577,23 @@ macro_rules! usage {
 						raw_args.$flag = matches.is_present(stringify!($flag));
 					)*
 					$(
-						raw_args.$arg = if_option!(
+						raw_args.$arg = return_if_parse_error!(if_option!(
 							$($arg_type_tt)+,
 							THEN {
 								if_option_vec!(
 									$($arg_type_tt)+,
-									THEN { values_t!(matches, stringify!($arg), inner_option_vec_type!($($arg_type_tt)+)).ok() }
-									ELSE { value_t!(matches, stringify!($arg), inner_option_type!($($arg_type_tt)+)).ok() }
+									THEN { values_t!(matches, stringify!($arg), inner_option_vec_type!($($arg_type_tt)+)) }
+									ELSE { value_t!(matches, stringify!($arg), inner_option_type!($($arg_type_tt)+)) }
 								)
 							}
 							ELSE {
 								if_vec!(
 									$($arg_type_tt)+,
-									THEN { values_t!(matches, stringify!($arg), inner_vec_type!($($arg_type_tt)+)).ok() }
-									ELSE { value_t!(matches, stringify!($arg), $($arg_type_tt)+).ok() }
+									THEN { values_t!(matches, stringify!($arg), inner_vec_type!($($arg_type_tt)+)) }
+									ELSE { value_t!(matches, stringify!($arg), $($arg_type_tt)+) }
 								)
 							}
-						);
+						));
 					)*
 				)*
 
@@ -594,23 +608,23 @@ macro_rules! usage {
 						)*
 						// Subcommand arguments
 						$(
-							raw_args.$subc_arg = if_option!(
+							raw_args.$subc_arg = return_if_parse_error!(if_option!(
 										$($subc_arg_type_tt)+,
 										THEN {
 											if_option_vec!(
 												$($subc_arg_type_tt)+,
-												THEN { values_t!(submatches, stringify!($subc_arg), inner_option_vec_type!($($subc_arg_type_tt)+)).ok() }
-												ELSE { value_t!(submatches, stringify!($subc_arg), inner_option_type!($($subc_arg_type_tt)+)).ok() }
+												THEN { values_t!(submatches, stringify!($subc_arg), inner_option_vec_type!($($subc_arg_type_tt)+)) }
+												ELSE { value_t!(submatches, stringify!($subc_arg), inner_option_type!($($subc_arg_type_tt)+)) }
 											)
 										}
 										ELSE {
 											if_vec!(
 												$($subc_arg_type_tt)+,
-												THEN { values_t!(submatches, stringify!($subc_arg), inner_vec_type!($($subc_arg_type_tt)+)).ok() }
-												ELSE { value_t!(submatches, stringify!($subc_arg), $($subc_arg_type_tt)+).ok() }
+												THEN { values_t!(submatches, stringify!($subc_arg), inner_vec_type!($($subc_arg_type_tt)+)) }
+												ELSE { value_t!(submatches, stringify!($subc_arg), $($subc_arg_type_tt)+) }
 											)
 										}
-							);
+							));
 						)*
 
 						// Sub-subcommands
@@ -624,23 +638,23 @@ macro_rules! usage {
 								)*
 								// Sub-subcommand arguments
 								$(
-									raw_args.$subc_subc_arg = if_option!(
+									raw_args.$subc_subc_arg = return_if_parse_error!(if_option!(
 										$($subc_subc_arg_type_tt)+,
 										THEN {
 											if_option_vec!(
 												$($subc_subc_arg_type_tt)+,
-												THEN { values_t!(subsubmatches, stringify!($subc_subc_arg), inner_option_vec_type!($($subc_subc_arg_type_tt)+)).ok() }
-												ELSE { value_t!(subsubmatches, stringify!($subc_subc_arg), inner_option_type!($($subc_subc_arg_type_tt)+)).ok() }
+												THEN { values_t!(subsubmatches, stringify!($subc_subc_arg), inner_option_vec_type!($($subc_subc_arg_type_tt)+)) }
+												ELSE { value_t!(subsubmatches, stringify!($subc_subc_arg), inner_option_type!($($subc_subc_arg_type_tt)+)) }
 											)
 										}
 										ELSE {
 											if_vec!(
 												$($subc_subc_arg_type_tt)+,
-												THEN { values_t!(subsubmatches, stringify!($subc_subc_arg), inner_vec_type!($($subc_subc_arg_type_tt)+)).ok() }
-												ELSE { value_t!(subsubmatches, stringify!($subc_subc_arg), $($subc_subc_arg_type_tt)+).ok() }
+												THEN { values_t!(subsubmatches, stringify!($subc_subc_arg), inner_vec_type!($($subc_subc_arg_type_tt)+)) }
+												ELSE { value_t!(subsubmatches, stringify!($subc_subc_arg), $($subc_subc_arg_type_tt)+) }
 											)
 										}
-									);
+									));
 								)*
 							}
 							else {