Clean up deprecated options and add CHECK macro (#9036)

* CHECK macro to replace hard-coded checks

* Clean up deprecated options

* typo: FlAG -> FLAG

* Fix tests

* Deprecated non-used ui params default is None

* test: arg_ui_path is not deprecated

* arg_ui_port should be None

* Address grumbles

* Fix tests

parity/cli/mod.rs

@@ -449,6 +449,17 @@ usage! {
 			"--reserved-peers=[FILE]",
 			"Provide a file containing enodes, one per line. These nodes will always have a reserved slot on top of the normal maximum peers.",
 
+			CHECK |args: &Args| {
+				if let (Some(max_peers), Some(min_peers)) = (args.arg_max_peers, args.arg_min_peers) {
+					if min_peers > max_peers {
+						return Err(ArgsError::PeerConfiguration);
+					}
+				}
+
+				Ok(())
+			},
+
+
 		["API and Console Options – HTTP JSON-RPC"]
 			FLAG flag_no_jsonrpc: (bool) = false, or |c: &Config| c.rpc.as_ref()?.disable.clone(),
 			"--no-jsonrpc",
@@ -879,26 +890,43 @@ usage! {
 			"Target size of the whisper message pool in megabytes.",
 
 		["Legacy Options"]
-			FLAG flag_warp: (bool) = false, or |_| None,
+		    // Options that are hidden from config, but are still unique for its functionality.
-			"--warp",
-			"Does nothing; warp sync is enabled by default. Use --no-warp to disable.",
-
-			FLAG flag_dapps_apis_all: (bool) = false, or |_| None,
-			"--dapps-apis-all",
-			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-apis.",
 
 			FLAG flag_geth: (bool) = false, or |_| None,
 			"--geth",
 			"Run in Geth-compatibility mode. Sets the IPC path to be the same as Geth's. Overrides the --ipc-path and --ipcpath options. Alters RPCs to reflect Geth bugs. Includes the personal_ RPC by default.",
 
-			FLAG flag_testnet: (bool) = false, or |_| None,
-			"--testnet",
-			"Testnet mode. Equivalent to --chain testnet. Overrides the --keys-path option.",
-
 			FLAG flag_import_geth_keys: (bool) = false, or |_| None,
 			"--import-geth-keys",
 			"Attempt to import keys from Geth client.",
 
+			// Options that either do nothing, or are replaced by other options.
+			// FLAG Removed in 1.6 or before.
+
+			FLAG flag_warp: (bool) = false, or |_| None,
+			"--warp",
+			"Does nothing; warp sync is enabled by default. Use --no-warp to disable.",
+
+			FLAG flag_jsonrpc: (bool) = false, or |_| None,
+			"-j, --jsonrpc",
+			"Does nothing; HTTP JSON-RPC is on by default now.",
+
+			FLAG flag_rpc: (bool) = false, or |_| None,
+			"--rpc",
+			"Does nothing; HTTP JSON-RPC is on by default now.",
+
+			FLAG flag_jsonrpc_off: (bool) = false, or |_| None,
+			"--jsonrpc-off",
+			"Equivalent to --no-jsonrpc.",
+
+			FLAG flag_webapp: (bool) = false, or |_| None,
+			"-w, --webapp",
+			"Does nothing; dapps server has been removed.",
+
+			FLAG flag_dapps_off: (bool) = false, or |_| None,
+			"--dapps-off",
+			"Equivalent to --no-dapps.",
+
 			FLAG flag_ipcdisable: (bool) = false, or |_| None,
 			"--ipcdisable",
 			"Equivalent to --no-ipc.",
@@ -907,29 +935,21 @@ usage! {
 			"--ipc-off",
 			"Equivalent to --no-ipc.",
 
+			FLAG flag_testnet: (bool) = false, or |_| None,
+			"--testnet",
+			"Testnet mode. Equivalent to --chain testnet. Overrides the --keys-path option.",
+
 			FLAG flag_nodiscover: (bool) = false, or |_| None,
 			"--nodiscover",
 			"Equivalent to --no-discovery.",
 
-			FLAG flag_jsonrpc: (bool) = false, or |_| None,
+			// FLAG Removed in 1.7.
-			"-j, --jsonrpc",
-			"Does nothing; HTTP JSON-RPC is on by default now.",
 
-			FLAG flag_jsonrpc_off: (bool) = false, or |_| None,
+			FLAG flag_dapps_apis_all: (bool) = false, or |_| None,
-			"--jsonrpc-off",
+			"--dapps-apis-all",
-			"Equivalent to --no-jsonrpc.",
+			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-apis.",
 
-			FLAG flag_webapp: (bool) = false, or |_| None,
+			// FLAG Removed in 1.11.
-			"-w, --webapp",
-			"Does nothing; dapps server is on by default now.",
-
-			FLAG flag_dapps_off: (bool) = false, or |_| None,
-			"--dapps-off",
-			"Equivalent to --no-dapps.",
-
-			FLAG flag_rpc: (bool) = false, or |_| None,
-			"--rpc",
-			"Does nothing; HTTP JSON-RPC is on by default now.",
 
 			FLAG flag_public_node: (bool) = false, or |_| None,
 			"--public-node",
@@ -947,41 +967,15 @@ usage! {
 			"--ui-no-validation",
 			"Does nothing; UI is now a separate project.",
 
-			ARG arg_ui_interface: (String) = "local", or |_| None,
+			// ARG Removed in 1.6 or before.
-			"--ui-interface=[IP]",
-			"Does nothing; UI is now a separate project.",
 
-			ARG arg_ui_hosts: (String) = "none", or |_| None,
+			ARG arg_etherbase: (Option<String>) = None, or |_| None,
-			"--ui-hosts=[HOSTS]",
+			"--etherbase=[ADDRESS]",
-			"Does nothing; UI is now a separate project.",
+			"Equivalent to --author ADDRESS.",
 
-			ARG arg_ui_port: (u16) = 8180u16, or |_| None,
+			ARG arg_extradata: (Option<String>) = None, or |_| None,
-			"--ui-port=[PORT]",
+			"--extradata=[STRING]",
-			"Does nothing; UI is now a separate project.",
+			"Equivalent to --extra-data STRING.",
-
-			ARG arg_dapps_port: (Option<u16>) = None, or |c: &Config| c.dapps.as_ref()?.port.clone(),
-			"--dapps-port=[PORT]",
-			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-port.",
-
-			ARG arg_dapps_interface: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.interface.clone(),
-			"--dapps-interface=[IP]",
-			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-interface.",
-
-			ARG arg_dapps_hosts: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.hosts.as_ref().map(|vec| vec.join(",")),
-			"--dapps-hosts=[HOSTS]",
-			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-hosts.",
-
-			ARG arg_dapps_cors: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.cors.clone(),
-			"--dapps-cors=[URL]",
-			"Dapps server is merged with HTTP JSON-RPC server. Use --jsonrpc-cors.",
-
-			ARG arg_dapps_user: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.user.clone(),
-			"--dapps-user=[USERNAME]",
-			"Dapps server authentication has been removed.",
-
-			ARG arg_dapps_pass: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.pass.clone(),
-			"--dapps-pass=[PASSWORD]",
-			"Dapps server authentication has been removed.",
 
 			ARG arg_datadir: (Option<String>) = None, or |_| None,
 			"--datadir=[PATH]",
@@ -1027,23 +1021,55 @@ usage! {
 			"--gasprice=[WEI]",
 			"Equivalent to --min-gas-price WEI.",
 
-			ARG arg_etherbase: (Option<String>) = None, or |_| None,
-			"--etherbase=[ADDRESS]",
-			"Equivalent to --author ADDRESS.",
-
-			ARG arg_extradata: (Option<String>) = None, or |_| None,
-			"--extradata=[STRING]",
-			"Equivalent to --extra-data STRING.",
-
 			ARG arg_cache: (Option<u32>) = None, or |_| None,
 			"--cache=[MB]",
 			"Equivalent to --cache-size MB.",
 
-			ARG arg_tx_queue_ban_count: (u16) = 1u16, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_count.clone(),
+			// ARG Removed in 1.7.
+
+			ARG arg_dapps_port: (Option<u16>) = None, or |c: &Config| c.dapps.as_ref()?.port.clone(),
+			"--dapps-port=[PORT]",
+			"Does nothing; dapps server has been removed.",
+
+			ARG arg_dapps_interface: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.interface.clone(),
+			"--dapps-interface=[IP]",
+			"Does nothing; dapps server has been removed.",
+
+			ARG arg_dapps_hosts: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.hosts.as_ref().map(|vec| vec.join(",")),
+			"--dapps-hosts=[HOSTS]",
+			"Does nothing; dapps server has been removed.",
+
+			ARG arg_dapps_cors: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.cors.clone(),
+			"--dapps-cors=[URL]",
+			"Does nothing; dapps server has been removed.",
+
+			ARG arg_dapps_user: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.user.clone(),
+			"--dapps-user=[USERNAME]",
+			"Dapps server authentication has been removed.",
+
+			ARG arg_dapps_pass: (Option<String>) = None, or |c: &Config| c.dapps.as_ref()?.pass.clone(),
+			"--dapps-pass=[PASSWORD]",
+			"Dapps server authentication has been removed.",
+
+			// ARG removed in 1.11.
+
+			ARG arg_ui_interface: (Option<String>) = None, or |_| None,
+			"--ui-interface=[IP]",
+			"Does nothing; UI is now a separate project.",
+
+			ARG arg_ui_hosts: (Option<String>) = None, or |_| None,
+			"--ui-hosts=[HOSTS]",
+			"Does nothing; UI is now a separate project.",
+
+			ARG arg_ui_port: (Option<u16>) = None, or |_| None,
+			"--ui-port=[PORT]",
+			"Does nothing; UI is now a separate project.",
+
+			ARG arg_tx_queue_ban_count: (Option<u16>) = None, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_count.clone(),
 			"--tx-queue-ban-count=[C]",
 			"Not supported.",
 
-			ARG arg_tx_queue_ban_time: (u16) = 180u16, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_time.clone(),
+			ARG arg_tx_queue_ban_time: (Option<u16>) = None, or |c: &Config| c.mining.as_ref()?.tx_queue_ban_time.clone(),
 			"--tx-queue-ban-time=[SEC]",
 			"Not supported.",
 	}
@@ -1427,11 +1453,11 @@ mod tests {
 
 		let args = Args::parse(&["parity", "--password", "~/.safe/1", "--password", "~/.safe/2", "--ui-port", "8123"]).unwrap();
 		assert_eq!(args.arg_password, vec!["~/.safe/1".to_owned(), "~/.safe/2".to_owned()]);
-		assert_eq!(args.arg_ui_port, 8123);
+		assert_eq!(args.arg_ui_port, Some(8123));
 
 		let args = Args::parse(&["parity", "--password", "~/.safe/1,~/.safe/2", "--ui-port", "8123"]).unwrap();
 		assert_eq!(args.arg_password, vec!["~/.safe/1".to_owned(), "~/.safe/2".to_owned()]);
-		assert_eq!(args.arg_ui_port, 8123);
+		assert_eq!(args.arg_ui_port, Some(8123));
 	}
 
 	#[test]
@@ -1583,9 +1609,9 @@ mod tests {
 
 			flag_force_ui: false,
 			flag_no_ui: false,
-			arg_ui_port: 8180u16,
+			arg_ui_port: None,
-			arg_ui_interface: "local".into(),
+			arg_ui_interface: None,
-			arg_ui_hosts: "none".into(),
+			arg_ui_hosts: None,
 			arg_ui_path: "$HOME/.parity/signer".into(),
 			flag_ui_no_validation: false,
 
@@ -1692,8 +1718,8 @@ mod tests {
 			arg_tx_queue_mem_limit: 4u32,
 			arg_tx_queue_gas: "off".into(),
 			arg_tx_queue_strategy: "gas_factor".into(),
-			arg_tx_queue_ban_count: 1u16,
+			arg_tx_queue_ban_count: Some(1u16),
-			arg_tx_queue_ban_time: 180u16,
+			arg_tx_queue_ban_time: Some(180u16),
 			flag_remove_solved: false,
 			arg_notify_work: Some("http://localhost:3001".into()),
 			flag_refuse_service_transactions: false,

parity/cli/usage.rs

@@ -135,6 +135,9 @@ macro_rules! usage {
 				$(
 					ARG $arg:ident : ($($arg_type_tt:tt)+) = $arg_default:expr, or $arg_from_config:expr, $arg_usage:expr, $arg_help:expr,
 				)*
+				$(
+					CHECK $check:expr,
+				)*
 			)*
 		}
 	) => {
@@ -318,13 +321,6 @@ macro_rules! usage {
 			pub fn parse<S: AsRef<str>>(command: &[S]) -> Result<Self, ArgsError> {
 				let raw_args = RawArgs::parse(command)?;
 
-				if let (Some(max_peers), Some(min_peers)) = (raw_args.arg_max_peers, raw_args.arg_min_peers) {
-					// Invalid configuration pattern `mix_peers` > `max_peers`
-					if min_peers > max_peers {
-						return Err(ArgsError::PeerConfiguration);
-					}
-				}
-
 				// Skip loading config file if no_config flag is specified
 				if raw_args.flag_no_config {
 					return Ok(raw_args.into_args(Config::default()));
@@ -332,7 +328,8 @@ macro_rules! usage {
 
 				let config_file = raw_args.arg_config.clone().unwrap_or_else(|| raw_args.clone().into_args(Config::default()).arg_config);
 				let config_file = replace_home(&::dir::default_data_path(), &config_file);
-				match (fs::File::open(&config_file), raw_args.arg_config.clone()) {
+
+				let args = match (fs::File::open(&config_file), raw_args.arg_config.clone()) {
 					// Load config file
 					(Ok(mut file), _) => {
 						println_stderr!("Loading config file from {}", &config_file);
@@ -349,7 +346,15 @@ macro_rules! usage {
 							Err(e) => Err(ArgsError::Config(config_file, e))
 						}
 					},
-				}
+				}?;
+
+				$(
+					$(
+						$check(&args)?;
+					)*
+				)*
+
+				Ok(args)
 			}
 
 			#[cfg(test)]

parity/deprecated.rs

@@ -37,6 +37,8 @@ impl fmt::Display for Deprecated {
 pub fn find_deprecated(args: &Args) -> Vec<Deprecated> {
 	let mut result = vec![];
 
+	// Removed in 1.6 or before.
+
 	if args.flag_warp {
 		result.push(Deprecated::DoesNothing("--warp"));
 	}
@@ -77,21 +79,78 @@ pub fn find_deprecated(args: &Args) -> Vec<Deprecated> {
 		result.push(Deprecated::Replaced("--extradata", "--extra-data"));
 	}
 
-	// Removed in 1.7
+	if args.flag_testnet {
+		result.push(Deprecated::Replaced("--testnet", "--chain testnet"));
+	}
+
+	if args.flag_nodiscover {
+		result.push(Deprecated::Replaced("--nodiscover", "--no-discovery"));
+	}
+
+	if args.arg_datadir.is_some() {
+		result.push(Deprecated::Replaced("--datadir", "--base-path"));
+	}
+
+	if args.arg_networkid.is_some() {
+		result.push(Deprecated::Replaced("--networkid", "--network-id"));
+	}
+
+	if args.arg_peers.is_some() {
+		result.push(Deprecated::Replaced("--peers", "--min-peers"));
+	}
+
+	if args.arg_nodekey.is_some() {
+		result.push(Deprecated::Replaced("--nodekey", "--node-key"));
+	}
+
+	if args.arg_rpcaddr.is_some() {
+		result.push(Deprecated::Replaced("--rpcaddr", "--jsonrpc-interface"));
+	}
+
+	if args.arg_rpcport.is_some() {
+		result.push(Deprecated::Replaced("--rpcport", "--jsonrpc-port"));
+	}
+
+	if args.arg_rpcapi.is_some() {
+		result.push(Deprecated::Replaced("--rpcapi", "--jsonrpc-api"));
+	}
+
+	if args.arg_rpccorsdomain.is_some() {
+		result.push(Deprecated::Replaced("--rpccorsdomain", "--jsonrpc-cors"));
+	}
+
+	if args.arg_ipcapi.is_some() {
+		result.push(Deprecated::Replaced("--ipcapi", "--ipc-apis"));
+	}
+
+	if args.arg_ipcpath.is_some() {
+		result.push(Deprecated::Replaced("--ipcpath", "--ipc-path"));
+	}
+
+	if args.arg_gasprice.is_some() {
+		result.push(Deprecated::Replaced("--gasprice", "--min-gas-price"));
+	}
+
+	if args.arg_cache.is_some() {
+		result.push(Deprecated::Replaced("--cache", "--cache-size"));
+	}
+
+	// Removed in 1.7.
+
 	if args.arg_dapps_port.is_some() {
-		result.push(Deprecated::Replaced("--dapps-port", "--jsonrpc-port"));
+		result.push(Deprecated::Removed("--dapps-port"));
 	}
 
 	if args.arg_dapps_interface.is_some() {
-		result.push(Deprecated::Replaced("--dapps-interface", "--jsonrpc-interface"));
+		result.push(Deprecated::Removed("--dapps-interface"));
 	}
 
 	if args.arg_dapps_hosts.is_some() {
-		result.push(Deprecated::Replaced("--dapps-hosts", "--jsonrpc-hosts"));
+		result.push(Deprecated::Removed("--dapps-hosts"));
 	}
 
 	if args.arg_dapps_cors.is_some() {
-		result.push(Deprecated::Replaced("--dapps-cors", "--jsonrpc-cors"));
+		result.push(Deprecated::Removed("--dapps-cors"));
 	}
 
 	if args.arg_dapps_user.is_some() {
@@ -106,7 +165,43 @@ pub fn find_deprecated(args: &Args) -> Vec<Deprecated> {
 		result.push(Deprecated::Replaced("--dapps-apis-all", "--jsonrpc-apis"));
 	}
 
-	// Removed in 1.8
+	// Removed in 1.11.
+
+	if args.flag_public_node {
+		result.push(Deprecated::Removed("--public-node"));
+	}
+
+	if args.flag_force_ui {
+		result.push(Deprecated::Removed("--force-ui"));
+	}
+
+	if args.flag_no_ui {
+		result.push(Deprecated::Removed("--no-ui"));
+	}
+
+	if args.flag_ui_no_validation {
+		result.push(Deprecated::Removed("--ui-no-validation"));
+	}
+
+	if args.arg_ui_interface.is_some() {
+		result.push(Deprecated::Removed("--ui-interface"));
+	}
+
+	if args.arg_ui_hosts.is_some() {
+		result.push(Deprecated::Removed("--ui-hosts"));
+	}
+
+	if args.arg_ui_port.is_some() {
+		result.push(Deprecated::Removed("--ui-port"));
+	}
+
+	if args.arg_tx_queue_ban_count.is_some() {
+		result.push(Deprecated::Removed("--tx-queue-ban-count"));
+	}
+
+	if args.arg_tx_queue_ban_time.is_some() {
+		result.push(Deprecated::Removed("--tx-queue-ban-time"));
+	}
 
 	result
 }
@@ -150,10 +245,10 @@ mod tests {
 			Deprecated::Replaced("--ipc-off", "--no-ipc"),
 			Deprecated::Replaced("--etherbase", "--author"),
 			Deprecated::Replaced("--extradata", "--extra-data"),
-			Deprecated::Replaced("--dapps-port", "--jsonrpc-port"),
+			Deprecated::Removed("--dapps-port"),
-			Deprecated::Replaced("--dapps-interface", "--jsonrpc-interface"),
+			Deprecated::Removed("--dapps-interface"),
-			Deprecated::Replaced("--dapps-hosts", "--jsonrpc-hosts"),
+			Deprecated::Removed("--dapps-hosts"),
-			Deprecated::Replaced("--dapps-cors", "--jsonrpc-cors"),
+			Deprecated::Removed("--dapps-cors"),
 			Deprecated::Removed("--dapps-user"),
 			Deprecated::Removed("--dapps-pass"),
 			Deprecated::Replaced("--dapps-apis-all", "--jsonrpc-apis"),