From cccc7500464177018f573b9ce9b3b454be50e11b Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 13 Jun 2022 21:10:30 -0400 Subject: [PATCH] Fix `clap` deprecation warnings --- clippy_dev/src/lint.rs | 2 +- clippy_dev/src/main.rs | 212 +++++++++++++++---------------------- clippy_dev/src/new_lint.rs | 7 +- clippy_dev/src/serve.rs | 2 +- lintcheck/src/config.rs | 64 +++++------ 5 files changed, 120 insertions(+), 167 deletions(-) diff --git a/clippy_dev/src/lint.rs b/clippy_dev/src/lint.rs index 9e463aa741c..71005449b4d 100644 --- a/clippy_dev/src/lint.rs +++ b/clippy_dev/src/lint.rs @@ -13,7 +13,7 @@ fn exit_if_err(status: io::Result) { } } -pub fn run<'a>(path: &str, args: impl Iterator) { +pub fn run<'a>(path: &str, args: impl Iterator) { let is_file = match fs::metadata(path) { Ok(metadata) => metadata.is_file(), Err(e) => { diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index ee535b1d3be..2c27a0bcaf9 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -2,7 +2,7 @@ // warn on lints, that are included in `rust-lang/rust`s bootstrap #![warn(rust_2018_idioms, unused_lifetimes)] -use clap::{Arg, ArgMatches, Command}; +use clap::{Arg, ArgAction, ArgMatches, Command, PossibleValue}; use clippy_dev::{bless, fmt, lint, new_lint, serve, setup, update_lints}; use indoc::indoc; fn main() { @@ -10,15 +10,15 @@ fn main() { match matches.subcommand() { Some(("bless", matches)) => { - bless::bless(matches.is_present("ignore-timestamp")); + bless::bless(matches.contains_id("ignore-timestamp")); }, Some(("fmt", matches)) => { - fmt::run(matches.is_present("check"), matches.is_present("verbose")); + fmt::run(matches.contains_id("check"), matches.contains_id("verbose")); }, Some(("update_lints", matches)) => { - if matches.is_present("print-only") { + if matches.contains_id("print-only") { update_lints::print_lints(); - } else if matches.is_present("check") { + } else if matches.contains_id("check") { update_lints::update(update_lints::UpdateMode::Check); } else { update_lints::update(update_lints::UpdateMode::Change); @@ -26,10 +26,10 @@ fn main() { }, Some(("new_lint", matches)) => { match new_lint::create( - matches.value_of("pass"), - matches.value_of("name"), - matches.value_of("category"), - matches.is_present("msrv"), + matches.get_one::("pass"), + matches.get_one::("name"), + matches.get_one::("category"), + matches.contains_id("msrv"), ) { Ok(_) => update_lints::update(update_lints::UpdateMode::Change), Err(e) => eprintln!("Unable to create lint: {}", e), @@ -37,28 +37,28 @@ fn main() { }, Some(("setup", sub_command)) => match sub_command.subcommand() { Some(("intellij", matches)) => { - if matches.is_present("remove") { + if matches.contains_id("remove") { setup::intellij::remove_rustc_src(); } else { setup::intellij::setup_rustc_src( matches - .value_of("rustc-repo-path") + .get_one::("rustc-repo-path") .expect("this field is mandatory and therefore always valid"), ); } }, Some(("git-hook", matches)) => { - if matches.is_present("remove") { + if matches.contains_id("remove") { setup::git_hook::remove_hook(); } else { - setup::git_hook::install_hook(matches.is_present("force-override")); + setup::git_hook::install_hook(matches.contains_id("force-override")); } }, Some(("vscode-tasks", matches)) => { - if matches.is_present("remove") { + if matches.contains_id("remove") { setup::vscode::remove_tasks(); } else { - setup::vscode::install_tasks(matches.is_present("force-override")); + setup::vscode::install_tasks(matches.contains_id("force-override")); } }, _ => {}, @@ -70,19 +70,19 @@ fn main() { _ => {}, }, Some(("serve", matches)) => { - let port = matches.value_of("port").unwrap().parse().unwrap(); - let lint = matches.value_of("lint"); + let port = *matches.get_one::("port").unwrap(); + let lint = matches.get_one::("lint"); serve::run(port, lint); }, Some(("lint", matches)) => { - let path = matches.value_of("path").unwrap(); - let args = matches.values_of("args").into_iter().flatten(); + let path = matches.get_one::("path").unwrap(); + let args = matches.get_many::("args").into_iter().flatten(); lint::run(path, args); }, Some(("rename_lint", matches)) => { - let old_name = matches.value_of("old_name").unwrap(); - let new_name = matches.value_of("new_name").unwrap_or(old_name); - let uplift = matches.is_present("uplift"); + let old_name = matches.get_one::("old_name").unwrap(); + let new_name = matches.get_one::("new_name").unwrap_or(old_name); + let uplift = matches.contains_id("uplift"); update_lints::rename(old_name, new_name, uplift); }, _ => {}, @@ -92,98 +92,86 @@ fn main() { fn get_clap_config() -> ArgMatches { Command::new("Clippy developer tooling") .arg_required_else_help(true) - .subcommand( + .subcommands([ Command::new("bless").about("bless the test output changes").arg( Arg::new("ignore-timestamp") .long("ignore-timestamp") .help("Include files updated before clippy was built"), ), - ) - .subcommand( Command::new("fmt") .about("Run rustfmt on all projects and tests") - .arg(Arg::new("check").long("check").help("Use the rustfmt --check option")) - .arg(Arg::new("verbose").short('v').long("verbose").help("Echo commands run")), - ) - .subcommand( + .args([ + Arg::new("check").long("check").help("Use the rustfmt --check option"), + Arg::new("verbose").short('v').long("verbose").help("Echo commands run"), + ]), Command::new("update_lints") .about("Updates lint registration and information from the source code") .long_about( "Makes sure that:\n \ - * the lint count in README.md is correct\n \ - * the changelog contains markdown link references at the bottom\n \ - * all lint groups include the correct lints\n \ - * lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod`\n \ - * all lints are registered in the lint store", + * the lint count in README.md is correct\n \ + * the changelog contains markdown link references at the bottom\n \ + * all lint groups include the correct lints\n \ + * lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod`\n \ + * all lints are registered in the lint store", ) - .arg(Arg::new("print-only").long("print-only").help( - "Print a table of lints to STDOUT. \ - This does not include deprecated and internal lints. \ - (Does not modify any files)", - )) - .arg( + .args([ + Arg::new("print-only").long("print-only").help( + "Print a table of lints to STDOUT. \ + This does not include deprecated and internal lints. \ + (Does not modify any files)", + ), Arg::new("check") .long("check") .help("Checks that `cargo dev update_lints` has been run. Used on CI."), - ), - ) - .subcommand( + ]), Command::new("new_lint") .about("Create new lint and run `cargo dev update_lints`") - .arg( + .args([ Arg::new("pass") .short('p') .long("pass") .help("Specify whether the lint runs during the early or late pass") .takes_value(true) - .possible_values(&["early", "late"]) + .value_parser([PossibleValue::new("early"), PossibleValue::new("late")]) .required(true), - ) - .arg( Arg::new("name") .short('n') .long("name") .help("Name of the new lint in snake case, ex: fn_too_long") .takes_value(true) .required(true), - ) - .arg( Arg::new("category") .short('c') .long("category") .help("What category the lint belongs to") .default_value("nursery") - .possible_values(&[ - "style", - "correctness", - "suspicious", - "complexity", - "perf", - "pedantic", - "restriction", - "cargo", - "nursery", - "internal", - "internal_warn", + .value_parser([ + PossibleValue::new("style"), + PossibleValue::new("correctness"), + PossibleValue::new("suspicious"), + PossibleValue::new("complexity"), + PossibleValue::new("perf"), + PossibleValue::new("pedantic"), + PossibleValue::new("restriction"), + PossibleValue::new("cargo"), + PossibleValue::new("nursery"), + PossibleValue::new("internal"), + PossibleValue::new("internal_warn"), ]) .takes_value(true), - ) - .arg(Arg::new("msrv").long("msrv").help("Add MSRV config code to the lint")), - ) - .subcommand( + Arg::new("msrv").long("msrv").help("Add MSRV config code to the lint"), + ]), Command::new("setup") .about("Support for setting up your personal development environment") .arg_required_else_help(true) - .subcommand( + .subcommands([ Command::new("intellij") .about("Alter dependencies so Intellij Rust can find rustc internals") - .arg( + .args([ Arg::new("remove") .long("remove") .help("Remove the dependencies added with 'cargo dev setup intellij'") .required(false), - ) - .arg( Arg::new("rustc-repo-path") .long("repo-path") .short('r') @@ -192,67 +180,53 @@ fn get_clap_config() -> ArgMatches { .value_name("path") .conflicts_with("remove") .required(true), - ), - ) - .subcommand( + ]), Command::new("git-hook") .about("Add a pre-commit git hook that formats your code to make it look pretty") - .arg( + .args([ Arg::new("remove") .long("remove") .help("Remove the pre-commit hook added with 'cargo dev setup git-hook'") .required(false), - ) - .arg( Arg::new("force-override") .long("force-override") .short('f') .help("Forces the override of an existing git pre-commit hook") .required(false), - ), - ) - .subcommand( + ]), Command::new("vscode-tasks") .about("Add several tasks to vscode for formatting, validation and testing") - .arg( + .args([ Arg::new("remove") .long("remove") .help("Remove the tasks added with 'cargo dev setup vscode-tasks'") .required(false), - ) - .arg( Arg::new("force-override") .long("force-override") .short('f') .help("Forces the override of existing vscode tasks") .required(false), - ), - ), - ) - .subcommand( + ]), + ]), Command::new("remove") .about("Support for undoing changes done by the setup command") .arg_required_else_help(true) - .subcommand(Command::new("git-hook").about("Remove any existing pre-commit git hook")) - .subcommand(Command::new("vscode-tasks").about("Remove any existing vscode tasks")) - .subcommand( + .subcommands([ + Command::new("git-hook").about("Remove any existing pre-commit git hook"), + Command::new("vscode-tasks").about("Remove any existing vscode tasks"), Command::new("intellij").about("Removes rustc source paths added via `cargo dev setup intellij`"), - ), - ) - .subcommand( + ]), Command::new("serve") .about("Launch a local 'ALL the Clippy Lints' website in a browser") - .arg( + .args([ Arg::new("port") .long("port") .short('p') .help("Local port for the http server") .default_value("8000") - .validator_os(serve::validate_port), - ) - .arg(Arg::new("lint").help("Which lint's page to load initially (optional)")), - ) - .subcommand( + .value_parser(clap::value_parser!(u16)), + Arg::new("lint").help("Which lint's page to load initially (optional)"), + ]), Command::new("lint") .about("Manually run clippy on a file or package") .after_help(indoc! {" @@ -271,37 +245,27 @@ fn get_clap_config() -> ArgMatches { cargo dev lint file.rs -- -W clippy::pedantic cargo dev lint ~/my-project -- -- -W clippy::pedantic "}) - .arg( + .args([ Arg::new("path") .required(true) .help("The path to a file or package directory to lint"), - ) - .arg( Arg::new("args") - .multiple_occurrences(true) + .action(ArgAction::Append) .help("Pass extra arguments to cargo/clippy-driver"), - ), - ) - .subcommand( - Command::new("rename_lint") - .about("Renames the given lint") - .arg( - Arg::new("old_name") - .index(1) - .required(true) - .help("The name of the lint to rename"), - ) - .arg( - Arg::new("new_name") - .index(2) - .required_unless_present("uplift") - .help("The new name of the lint"), - ) - .arg( - Arg::new("uplift") - .long("uplift") - .help("This lint will be uplifted into rustc"), - ), - ) + ]), + Command::new("rename_lint").about("Renames the given lint").args([ + Arg::new("old_name") + .index(1) + .required(true) + .help("The name of the lint to rename"), + Arg::new("new_name") + .index(2) + .required_unless_present("uplift") + .help("The new name of the lint"), + Arg::new("uplift") + .long("uplift") + .help("This lint will be uplifted into rustc"), + ]), + ]) .get_matches() } diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 07d19638788..748d73c0801 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -34,7 +34,12 @@ fn context>(self, text: C) -> Self { /// # Errors /// /// This function errors out if the files couldn't be created or written to. -pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>, msrv: bool) -> io::Result<()> { +pub fn create( + pass: Option<&String>, + lint_name: Option<&String>, + category: Option<&String>, + msrv: bool, +) -> io::Result<()> { let lint = LintData { pass: pass.expect("`pass` argument is validated by clap"), name: lint_name.expect("`name` argument is validated by clap"), diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index d55b1a354d0..f15f24da946 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -8,7 +8,7 @@ /// # Panics /// /// Panics if the python commands could not be spawned -pub fn run(port: u16, lint: Option<&str>) -> ! { +pub fn run(port: u16, lint: Option<&String>) -> ! { let mut url = Some(match lint { None => format!("http://localhost:{}", port), Some(lint) => format!("http://localhost:{}/#{}", port, lint), diff --git a/lintcheck/src/config.rs b/lintcheck/src/config.rs index a6f93d2a1c0..1742cf677c0 100644 --- a/lintcheck/src/config.rs +++ b/lintcheck/src/config.rs @@ -1,50 +1,40 @@ -use clap::{Arg, ArgMatches, Command}; +use clap::{Arg, ArgAction, ArgMatches, Command}; use std::env; use std::path::PathBuf; fn get_clap_config() -> ArgMatches { Command::new("lintcheck") .about("run clippy on a set of crates and check output") - .arg( + .args([ Arg::new("only") - .takes_value(true) + .action(ArgAction::Set) .value_name("CRATE") .long("only") .help("Only process a single crate of the list"), - ) - .arg( Arg::new("crates-toml") - .takes_value(true) + .action(ArgAction::Set) .value_name("CRATES-SOURCES-TOML-PATH") .long("crates-toml") .help("Set the path for a crates.toml where lintcheck should read the sources from"), - ) - .arg( Arg::new("threads") - .takes_value(true) + .action(ArgAction::Set) .value_name("N") + .value_parser(clap::value_parser!(usize)) .short('j') .long("jobs") .help("Number of threads to use, 0 automatic choice"), - ) - .arg( Arg::new("fix") - .long("--fix") + .long("fix") .help("Runs cargo clippy --fix and checks if all suggestions apply"), - ) - .arg( Arg::new("filter") - .long("--filter") - .takes_value(true) - .multiple_occurrences(true) + .long("filter") + .action(ArgAction::Append) .value_name("clippy_lint_name") .help("Apply a filter to only collect specified lints, this also overrides `allow` attributes"), - ) - .arg( Arg::new("markdown") - .long("--markdown") + .long("markdown") .help("Change the reports table to use markdown links"), - ) + ]) .get_matches() } @@ -75,13 +65,13 @@ pub fn new() -> Self { // if not, use the default "lintcheck/lintcheck_crates.toml" let sources_toml = env::var("LINTCHECK_TOML").unwrap_or_else(|_| { clap_config - .value_of("crates-toml") - .clone() + .get_one::("crates-toml") + .map(|s| &**s) .unwrap_or("lintcheck/lintcheck_crates.toml") - .to_string() + .into() }); - let markdown = clap_config.is_present("markdown"); + let markdown = clap_config.contains_id("markdown"); let sources_toml_path = PathBuf::from(sources_toml); // for the path where we save the lint results, get the filename without extension (so for @@ -96,25 +86,19 @@ pub fn new() -> Self { // look at the --threads arg, if 0 is passed, ask rayon rayon how many threads it would spawn and // use half of that for the physical core count // by default use a single thread - let max_jobs = match clap_config.value_of("threads") { - Some(threads) => { - let threads: usize = threads - .parse() - .unwrap_or_else(|_| panic!("Failed to parse '{}' to a digit", threads)); - if threads == 0 { - // automatic choice - // Rayon seems to return thread count so half that for core count - (rayon::current_num_threads() / 2) as usize - } else { - threads - } + let max_jobs = match clap_config.get_one::("threads") { + Some(&0) => { + // automatic choice + // Rayon seems to return thread count so half that for core count + (rayon::current_num_threads() / 2) as usize }, + Some(&threads) => threads, // no -j passed, use a single thread None => 1, }; let lint_filter: Vec = clap_config - .values_of("filter") + .get_many::("filter") .map(|iter| { iter.map(|lint_name| { let mut filter = lint_name.replace('_', "-"); @@ -131,8 +115,8 @@ pub fn new() -> Self { max_jobs, sources_toml_path, lintcheck_results_path, - only: clap_config.value_of("only").map(String::from), - fix: clap_config.is_present("fix"), + only: clap_config.get_one::("only").map(String::from), + fix: clap_config.contains_id("fix"), lint_filter, markdown, }