Bump clap to 4.0.x and adjust to best practices (#12381)

* Bump clap to 3.2.22

* Replace `from_os_str` with `value_parser`

* Replace `from_str` and `try_from_str` with `value_parser`

* Move possible_values to the new format

* Remove unwanted print

* Add missing match branch

* Update clap to 4.0.9 and make it compile

* Replace deprecated `clap` macro with `command` and `value`

* Move remaining `clap` attributes to `arg`

* Remove no-op value_parsers

* Adjust value_parser for state_version

* Remove "deprecated" feature flag and bump to 4.0.11

* Improve range

Co-authored-by: Bastian Köcher <git@kchr.de>

* Apply suggestions

* Trigger CI

* Fix unused error warning

* Fix doc errors

* Fix ArgGroup naming conflict

* Change default_value to default_value_t

* Use 1.. instead of 0..

Co-authored-by: Bastian Köcher <git@kchr.de>

substrate/utils/frame/try-runtime/cli/src/commands/execute_block.rs

@@ -33,11 +33,11 @@ use std::{fmt::Debug, str::FromStr};
 #[derive(Debug, Clone, clap::Parser)]
 pub struct ExecuteBlockCmd {
 	/// Overwrite the wasm code in state or not.
-	#[clap(long)]
+	#[arg(long)]
 	overwrite_wasm_code: bool,
 
 	/// If set the state root check is disabled.
-	#[clap(long)]
+	#[arg(long)]
 	no_state_root_check: bool,
 
 	/// Which try-state targets to execute when running this command.
@@ -49,17 +49,16 @@ pub struct ExecuteBlockCmd {
 	///   `Staking, System`).
 	/// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a
 	///   round-robin fashion.
-	#[clap(long, default_value = "none")]
+	#[arg(long, default_value = "none")]
 	try_state: frame_try_runtime::TryStateSelect,
 
 	/// The block hash at which to fetch the block.
 	///
 	/// If the `live` state type is being used, then this can be omitted, and is equal to whatever
 	/// the `state::at` is. Only use this (with care) when combined with a snapshot.
-	#[clap(
+	#[arg(
 		long,
-		multiple_values = false,
-		parse(try_from_str = crate::parse::hash)
+		value_parser = crate::parse::hash
 	)]
 	block_at: Option<String>,
 
@@ -67,10 +66,9 @@ pub struct ExecuteBlockCmd {
 	///
 	/// If the `live` state type is being used, then this can be omitted, and is equal to whatever
 	/// the `state::uri` is. Only use this (with care) when combined with a snapshot.
-	#[clap(
+	#[arg(
 		long,
-		multiple_values = false,
-		parse(try_from_str = crate::parse::url)
+		value_parser = crate::parse::url
 	)]
 	block_ws_uri: Option<String>,
 
@@ -79,7 +77,7 @@ pub struct ExecuteBlockCmd {
 	/// For this command only, if the `live` is used, then state of the parent block is fetched.
 	///
 	/// If `block_at` is provided, then the [`State::Live::at`] is being ignored.
-	#[clap(subcommand)]
+	#[command(subcommand)]
 	state: State,
 }
 

substrate/utils/frame/try-runtime/cli/src/commands/follow_chain.rs

@@ -43,11 +43,11 @@ const UN_SUB: &str = "chain_unsubscribeFinalizedHeads";
 #[derive(Debug, Clone, clap::Parser)]
 pub struct FollowChainCmd {
 	/// The url to connect to.
-	#[clap(short, long, parse(try_from_str = parse::url))]
+	#[arg(short, long, value_parser = parse::url)]
 	uri: String,
 
 	/// If set, then the state root check is enabled.
-	#[clap(long)]
+	#[arg(long)]
 	state_root_check: bool,
 
 	/// Which try-state targets to execute when running this command.
@@ -59,11 +59,11 @@ pub struct FollowChainCmd {
 	///   `Staking, System`).
 	/// - `rr-[x]` where `[x]` is a number. Then, the given number of pallets are checked in a
 	///   round-robin fashion.
-	#[clap(long, default_value = "none")]
+	#[arg(long, default_value = "none")]
 	try_state: frame_try_runtime::TryStateSelect,
 
 	/// If present, a single connection to a node will be kept and reused for fetching blocks.
-	#[clap(long)]
+	#[arg(long)]
 	keep_connection: bool,
 }
 

substrate/utils/frame/try-runtime/cli/src/commands/offchain_worker.rs

@@ -31,17 +31,16 @@ use std::{fmt::Debug, str::FromStr};
 #[derive(Debug, Clone, clap::Parser)]
 pub struct OffchainWorkerCmd {
 	/// Overwrite the wasm code in state or not.
-	#[clap(long)]
+	#[arg(long)]
 	overwrite_wasm_code: bool,
 
 	/// The block hash at which to fetch the header.
 	///
 	/// If the `live` state type is being used, then this can be omitted, and is equal to whatever
 	/// the `state::at` is. Only use this (with care) when combined with a snapshot.
-	#[clap(
+	#[arg(
 		long,
-		multiple_values = false,
-		parse(try_from_str = parse::hash)
+		value_parser = parse::hash
 	)]
 	header_at: Option<String>,
 
@@ -49,15 +48,14 @@ pub struct OffchainWorkerCmd {
 	///
 	/// If the `live` state type is being used, then this can be omitted, and is equal to whatever
 	/// the `state::uri` is. Only use this (with care) when combined with a snapshot.
-	#[clap(
+	#[arg(
 		long,
-		multiple_values = false,
-		parse(try_from_str = parse::url)
+		value_parser = parse::url
 	)]
 	header_ws_uri: Option<String>,
 
 	/// The state type to use.
-	#[clap(subcommand)]
+	#[command(subcommand)]
 	pub state: State,
 }
 

substrate/utils/frame/try-runtime/cli/src/commands/on_runtime_upgrade.rs

@@ -32,7 +32,7 @@ use crate::{
 #[derive(Debug, Clone, clap::Parser)]
 pub struct OnRuntimeUpgradeCmd {
 	/// The state type to use.
-	#[clap(subcommand)]
+	#[command(subcommand)]
 	pub state: State,
 }
 

substrate/utils/frame/try-runtime/cli/src/lib.rs

@@ -385,6 +385,7 @@ pub enum Command {
 
 /// Shared parameters of the `try-runtime` commands
 #[derive(Debug, Clone, clap::Parser)]
+#[group(skip)]
 pub struct SharedParams {
 	/// Shared parameters of substrate cli.
 	#[allow(missing_docs)]
@@ -392,41 +393,41 @@ pub struct SharedParams {
 	pub shared_params: sc_cli::SharedParams,
 
 	/// The execution strategy that should be used.
-	#[clap(long, value_name = "STRATEGY", arg_enum, ignore_case = true, default_value = "wasm")]
+	#[arg(long, value_name = "STRATEGY", value_enum, ignore_case = true, default_value_t = ExecutionStrategy::Wasm)]
 	pub execution: ExecutionStrategy,
 
 	/// Type of wasm execution used.
-	#[clap(
+	#[arg(
 		long = "wasm-execution",
 		value_name = "METHOD",
-		possible_values = WasmExecutionMethod::variants(),
+		value_enum,
 		ignore_case = true,
-		default_value = DEFAULT_WASM_EXECUTION_METHOD,
+		default_value_t = DEFAULT_WASM_EXECUTION_METHOD,
 	)]
 	pub wasm_method: WasmExecutionMethod,
 
 	/// The WASM instantiation method to use.
 	///
 	/// Only has an effect when `wasm-execution` is set to `compiled`.
-	#[clap(
+	#[arg(
 		long = "wasm-instantiation-strategy",
 		value_name = "STRATEGY",
 		default_value_t = DEFAULT_WASMTIME_INSTANTIATION_STRATEGY,
-		arg_enum,
+		value_enum,
 	)]
 	pub wasmtime_instantiation_strategy: WasmtimeInstantiationStrategy,
 
 	/// The number of 64KB pages to allocate for Wasm execution. Defaults to
 	/// [`sc_service::Configuration.default_heap_pages`].
-	#[clap(long)]
+	#[arg(long)]
 	pub heap_pages: Option<u64>,
 
 	/// When enabled, the spec check will not panic, and instead only show a warning.
-	#[clap(long)]
+	#[arg(long)]
 	pub no_spec_check_panic: bool,
 
 	/// State version that is used by the chain.
-	#[clap(long, default_value = "1", parse(try_from_str = parse::state_version))]
+	#[arg(long, default_value_t = StateVersion::V1, value_parser = parse::state_version)]
 	pub state_version: StateVersion,
 }
 
@@ -438,7 +439,7 @@ pub struct TryRuntimeCmd {
 	#[clap(flatten)]
 	pub shared: SharedParams,
 
-	#[clap(subcommand)]
+	#[command(subcommand)]
 	pub command: Command,
 }
 
@@ -449,17 +450,17 @@ pub enum State {
 	///
 	/// This can be crated by passing a value to [`State::Live::snapshot_path`].
 	Snap {
-		#[clap(short, long)]
+		#[arg(short, long)]
 		snapshot_path: PathBuf,
 	},
 
 	/// Use a live chain as the source of runtime state.
 	Live {
 		/// The url to connect to.
-		#[clap(
+		#[arg(
 			short,
 			long,
-			parse(try_from_str = parse::url),
+			value_parser = parse::url,
 		)]
 		uri: String,
 
@@ -467,21 +468,20 @@ pub enum State {
 		///
 		/// If non provided, then the latest finalized head is used. This is particularly useful
 		/// for [`Command::OnRuntimeUpgrade`].
-		#[clap(
+		#[arg(
 			short,
 			long,
-			multiple_values = false,
-			parse(try_from_str = parse::hash),
+			value_parser = parse::hash,
 		)]
 		at: Option<String>,
 
 		/// An optional state snapshot file to WRITE to. Not written if set to `None`.
-		#[clap(short, long)]
+		#[arg(short, long)]
 		snapshot_path: Option<PathBuf>,
 
 		/// A pallet to scrape. Can be provided multiple times. If empty, entire chain state will
 		/// be scraped.
-		#[clap(short, long, multiple_values = true)]
+		#[arg(short, long, num_args = 1..)]
 		pallet: Vec<String>,
 
 		/// Fetch the child-keys as well.
@@ -489,7 +489,7 @@ pub enum State {
 		/// Default is `false`, if specific `--pallets` are specified, `true` otherwise. In other
 		/// words, if you scrape the whole state the child tree data is included out of the box.
 		/// Otherwise, it must be enabled explicitly using this flag.
-		#[clap(long)]
+		#[arg(long)]
 		child_tree: bool,
 	},
 }