Type safe CLI implementation for clippy-dev

Use the derive feature of `clap` to generate CLI of clippy-dev. Adding new
commands will be easier in the future and we get better compile time checking
through exhaustive matching.
This commit is contained in:
Philipp Krones 2024-05-02 13:51:10 +02:00
parent 993d8ae2a7
commit a0d562a183
No known key found for this signature in database
GPG Key ID: 1CA0DF2AF59D68A5
5 changed files with 269 additions and 332 deletions

View File

@ -1,11 +1,12 @@
[package] [package]
name = "clippy_dev" name = "clippy_dev"
description = "Clippy developer tooling"
version = "0.0.1" version = "0.0.1"
edition = "2021" edition = "2021"
[dependencies] [dependencies]
aho-corasick = "1.0" aho-corasick = "1.0"
clap = "4.1.4" clap = { version = "4.1.4", features = ["derive"] }
indoc = "1.0" indoc = "1.0"
itertools = "0.12" itertools = "0.12"
opener = "0.6" opener = "0.6"

View File

@ -2,200 +2,150 @@
// warn on lints, that are included in `rust-lang/rust`s bootstrap // warn on lints, that are included in `rust-lang/rust`s bootstrap
#![warn(rust_2018_idioms, unused_lifetimes)] #![warn(rust_2018_idioms, unused_lifetimes)]
use clap::{Arg, ArgAction, ArgMatches, Command}; use clap::{Args, Parser, Subcommand};
use clippy_dev::{dogfood, fmt, lint, new_lint, serve, setup, update_lints}; use clippy_dev::{dogfood, fmt, lint, new_lint, serve, setup, update_lints};
use indoc::indoc;
use std::convert::Infallible; use std::convert::Infallible;
fn main() { fn main() {
let matches = get_clap_config(); let dev = Dev::parse();
match matches.subcommand() { match dev.command {
Some(("bless", _)) => { DevCommand::Bless => {
eprintln!("use `cargo bless` to automatically replace `.stderr` and `.fixed` files as tests are being run"); eprintln!("use `cargo bless` to automatically replace `.stderr` and `.fixed` files as tests are being run");
}, },
Some(("dogfood", matches)) => { DevCommand::Dogfood {
dogfood::dogfood( fix,
matches.get_flag("fix"), allow_dirty,
matches.get_flag("allow-dirty"), allow_staged,
matches.get_flag("allow-staged"), } => dogfood::dogfood(fix, allow_dirty, allow_staged),
); DevCommand::Fmt { check, verbose } => fmt::run(check, verbose),
}, DevCommand::UpdateLints { print_only, check } => {
Some(("fmt", matches)) => { if print_only {
fmt::run(matches.get_flag("check"), matches.get_flag("verbose"));
},
Some(("update_lints", matches)) => {
if matches.get_flag("print-only") {
update_lints::print_lints(); update_lints::print_lints();
} else if matches.get_flag("check") { } else if check {
update_lints::update(update_lints::UpdateMode::Check); update_lints::update(update_lints::UpdateMode::Check);
} else { } else {
update_lints::update(update_lints::UpdateMode::Change); update_lints::update(update_lints::UpdateMode::Change);
} }
}, },
Some(("new_lint", matches)) => { DevCommand::NewLint {
match new_lint::create( pass,
matches.get_one::<String>("pass").unwrap(), name,
matches.get_one::<String>("name"), category,
matches.get_one::<String>("category").map(String::as_str), r#type,
matches.get_one::<String>("type").map(String::as_str), msrv,
matches.get_flag("msrv"), } => match new_lint::create(&pass, &name, &category, r#type.as_deref(), msrv) {
) {
Ok(()) => update_lints::update(update_lints::UpdateMode::Change), Ok(()) => update_lints::update(update_lints::UpdateMode::Change),
Err(e) => eprintln!("Unable to create lint: {e}"), Err(e) => eprintln!("Unable to create lint: {e}"),
}
}, },
Some(("setup", sub_command)) => match sub_command.subcommand() { DevCommand::Setup(SetupCommand { subcommand }) => match subcommand {
Some(("git-hook", matches)) => { SetupSubcommand::Intellij { remove, repo_path } => {
if matches.get_flag("remove") { if remove {
setup::git_hook::remove_hook();
} else {
setup::git_hook::install_hook(matches.get_flag("force-override"));
}
},
Some(("intellij", matches)) => {
if matches.get_flag("remove") {
setup::intellij::remove_rustc_src(); setup::intellij::remove_rustc_src();
} else { } else {
setup::intellij::setup_rustc_src( setup::intellij::setup_rustc_src(&repo_path);
matches
.get_one::<String>("rustc-repo-path")
.expect("this field is mandatory and therefore always valid"),
);
} }
}, },
Some(("toolchain", matches)) => { SetupSubcommand::GitHook { remove, force_override } => {
setup::toolchain::create( if remove {
matches.get_flag("force"), setup::git_hook::remove_hook();
matches.get_flag("release"), } else {
matches.get_one::<String>("name").unwrap(), setup::git_hook::install_hook(force_override);
); }
}, },
Some(("vscode-tasks", matches)) => { SetupSubcommand::Toolchain { force, release, name } => setup::toolchain::create(force, release, &name),
if matches.get_flag("remove") { SetupSubcommand::VscodeTasks { remove, force_override } => {
if remove {
setup::vscode::remove_tasks(); setup::vscode::remove_tasks();
} else { } else {
setup::vscode::install_tasks(matches.get_flag("force-override")); setup::vscode::install_tasks(force_override);
} }
}, },
_ => {},
}, },
Some(("remove", sub_command)) => match sub_command.subcommand() { DevCommand::Remove(RemoveCommand { subcommand }) => match subcommand {
Some(("git-hook", _)) => setup::git_hook::remove_hook(), RemoveSubcommand::Intellij => setup::intellij::remove_rustc_src(),
Some(("intellij", _)) => setup::intellij::remove_rustc_src(), RemoveSubcommand::GitHook => setup::git_hook::remove_hook(),
Some(("vscode-tasks", _)) => setup::vscode::remove_tasks(), RemoveSubcommand::VscodeTasks => setup::vscode::remove_tasks(),
_ => {},
}, },
Some(("serve", matches)) => { DevCommand::Serve { port, lint } => serve::run(port, lint),
let port = *matches.get_one::<u16>("port").unwrap(); DevCommand::Lint { path, args } => lint::run(&path, args.iter()),
let lint = matches.get_one::<String>("lint"); DevCommand::RenameLint {
serve::run(port, lint); old_name,
}, new_name,
Some(("lint", matches)) => { uplift,
let path = matches.get_one::<String>("path").unwrap(); } => update_lints::rename(&old_name, new_name.as_ref().unwrap_or(&old_name), uplift),
let args = matches.get_many::<String>("args").into_iter().flatten(); DevCommand::Deprecate { name, reason } => update_lints::deprecate(&name, reason.as_deref()),
lint::run(path, args);
},
Some(("rename_lint", matches)) => {
let old_name = matches.get_one::<String>("old_name").unwrap();
let new_name = matches.get_one::<String>("new_name").unwrap_or(old_name);
let uplift = matches.get_flag("uplift");
update_lints::rename(old_name, new_name, uplift);
},
Some(("deprecate", matches)) => {
let name = matches.get_one::<String>("name").unwrap();
let reason = matches.get_one("reason");
update_lints::deprecate(name, reason);
},
_ => {},
} }
} }
fn get_clap_config() -> ArgMatches { #[derive(Parser)]
Command::new("Clippy developer tooling") #[command(name = "dev", about)]
.arg_required_else_help(true) struct Dev {
.subcommands([ #[command(subcommand)]
Command::new("bless").about("bless the test output changes").arg( command: DevCommand,
Arg::new("ignore-timestamp") }
.long("ignore-timestamp")
.action(ArgAction::SetTrue) #[derive(Subcommand)]
.help("Include files updated before clippy was built"), enum DevCommand {
), /// Bless the test output changes
Command::new("dogfood").about("Runs the dogfood test").args([ Bless,
Arg::new("fix") /// Runs the dogfood test
.long("fix") Dogfood {
.action(ArgAction::SetTrue) #[arg(long)]
.help("Apply the suggestions when possible"), /// Apply the suggestions when possible
Arg::new("allow-dirty") fix: bool,
.long("allow-dirty") #[arg(long, requires = "fix")]
.action(ArgAction::SetTrue) /// Fix code even if the working directory has changes
.help("Fix code even if the working directory has changes") allow_dirty: bool,
.requires("fix"), #[arg(long, requires = "fix")]
Arg::new("allow-staged") /// Fix code even if the working directory has staged changes
.long("allow-staged") allow_staged: bool,
.action(ArgAction::SetTrue) },
.help("Fix code even if the working directory has staged changes") /// Run rustfmt on all projects and tests
.requires("fix"), Fmt {
]), #[arg(long)]
Command::new("fmt") /// Use the rustfmt --check option
.about("Run rustfmt on all projects and tests") check: bool,
.args([ #[arg(short, long)]
Arg::new("check") /// Echo commands run
.long("check") verbose: bool,
.action(ArgAction::SetTrue) },
.help("Use the rustfmt --check option"), #[command(name = "update_lints")]
Arg::new("verbose") /// Updates lint registration and information from the source code
.short('v') ///
.long("verbose") /// Makes sure that: {n}
.action(ArgAction::SetTrue) /// * the lint count in README.md is correct {n}
.help("Echo commands run"), /// * the changelog contains markdown link references at the bottom {n}
]), /// * all lint groups include the correct lints {n}
Command::new("update_lints") /// * lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod` {n}
.about("Updates lint registration and information from the source code") /// * all lints are registered in the lint store
.long_about( UpdateLints {
"Makes sure that:\n \ #[arg(long)]
* the lint count in README.md is correct\n \ /// Print a table of lints to STDOUT
* the changelog contains markdown link references at the bottom\n \ ///
* all lint groups include the correct lints\n \ /// This does not include deprecated and internal lints. (Does not modify any files)
* lint modules in `clippy_lints/*` are visible in `src/lib.rs` via `pub mod`\n \ print_only: bool,
* all lints are registered in the lint store", #[arg(long)]
) /// Checks that `cargo dev update_lints` has been run. Used on CI.
.args([ check: bool,
Arg::new("print-only") },
.long("print-only") #[command(name = "new_lint")]
.action(ArgAction::SetTrue) /// Create a new lint and run `cargo dev update_lints`
.help( NewLint {
"Print a table of lints to STDOUT. \ #[arg(short, long, value_parser = ["early", "late"], conflicts_with = "type", default_value = "late")]
This does not include deprecated and internal lints. \ /// Specify whether the lint runs during the early or late pass
(Does not modify any files)", pass: String,
), #[arg(
Arg::new("check") short,
.long("check") long,
.action(ArgAction::SetTrue) value_parser = |name: &str| Ok::<_, Infallible>(name.replace('-', "_")),
.help("Checks that `cargo dev update_lints` has been run. Used on CI."), )]
]), /// Name of the new lint in snake case, ex: `fn_too_long`
Command::new("new_lint") name: String,
.about("Create new lint and run `cargo dev update_lints`") #[arg(
.args([ short,
Arg::new("pass") long,
.short('p') value_parser = [
.long("pass")
.help("Specify whether the lint runs during the early or late pass")
.value_parser(["early", "late"])
.conflicts_with("type")
.default_value("late"),
Arg::new("name")
.short('n')
.long("name")
.help("Name of the new lint in snake case, ex: fn_too_long")
.required(true)
.value_parser(|name: &str| Ok::<_, Infallible>(name.replace('-', "_"))),
Arg::new("category")
.short('c')
.long("category")
.help("What category the lint belongs to")
.default_value("nursery")
.value_parser([
"style", "style",
"correctness", "correctness",
"suspicious", "suspicious",
@ -206,146 +156,138 @@ fn get_clap_config() -> ArgMatches {
"cargo", "cargo",
"nursery", "nursery",
"internal", "internal",
]), ],
Arg::new("type").long("type").help("What directory the lint belongs in"), default_value = "nursery",
Arg::new("msrv") )]
.long("msrv") /// What category the lint belongs to
.action(ArgAction::SetTrue) category: String,
.help("Add MSRV config code to the lint"), #[arg(long)]
]), /// What directory the lint belongs in
Command::new("setup") r#type: Option<String>,
.about("Support for setting up your personal development environment") #[arg(long)]
.arg_required_else_help(true) /// Add MSRV config code to the lint
.subcommands([ msrv: bool,
Command::new("git-hook") },
.about("Add a pre-commit git hook that formats your code to make it look pretty") /// Support for setting up your personal development environment
.args([ Setup(SetupCommand),
Arg::new("remove") /// Support for removing changes done by the setup command
.long("remove") Remove(RemoveCommand),
.action(ArgAction::SetTrue) /// Launch a local 'ALL the Clippy Lints' website in a browser
.help("Remove the pre-commit hook added with 'cargo dev setup git-hook'"), Serve {
Arg::new("force-override") #[arg(short, long, default_value = "8000")]
.long("force-override") /// Local port for the http server
.short('f') port: u16,
.action(ArgAction::SetTrue) #[arg(long)]
.help("Forces the override of an existing git pre-commit hook"), /// Which lint's page to load initially (optional)
]), lint: Option<String>,
Command::new("intellij") },
.about("Alter dependencies so Intellij Rust can find rustc internals") #[allow(clippy::doc_markdown)]
.args([ /// Manually run clippy on a file or package
Arg::new("remove") ///
.long("remove") /// ## Examples
.action(ArgAction::SetTrue) ///
.help("Remove the dependencies added with 'cargo dev setup intellij'"), /// Lint a single file: {n}
Arg::new("rustc-repo-path") /// cargo dev lint tests/ui/attrs.rs
.long("repo-path") ///
.short('r') /// Lint a package directory: {n}
.help("The path to a rustc repo that will be used for setting the dependencies") /// cargo dev lint tests/ui-cargo/wildcard_dependencies/fail {n}
.value_name("path") /// cargo dev lint ~/my-project
.conflicts_with("remove") ///
.required(true), /// Run rustfix: {n}
]), /// cargo dev lint ~/my-project -- --fix
Command::new("toolchain") ///
.about("Install a rustup toolchain pointing to the local clippy build") /// Set lint levels: {n}
.args([ /// cargo dev lint file.rs -- -W clippy::pedantic {n}
Arg::new("force") /// cargo dev lint ~/my-project -- -- -W clippy::pedantic
.long("force") Lint {
.short('f') /// The path to a file or package directory to lint
.action(ArgAction::SetTrue) path: String,
.help("Override an existing toolchain"), /// Pass extra arguments to cargo/clippy-driver
Arg::new("release") args: Vec<String>,
.long("release") },
.short('r') #[command(name = "rename_lint")]
.action(ArgAction::SetTrue) /// Rename a lint
.help("Point to --release clippy binaries"), RenameLint {
Arg::new("name") /// The name of the lint to rename
.long("name") old_name: String,
.default_value("clippy") #[arg(required_unless_present = "uplift")]
.help("The name of the created toolchain"), /// The new name of the lint
]), new_name: Option<String>,
Command::new("vscode-tasks") #[arg(long)]
.about("Add several tasks to vscode for formatting, validation and testing") /// This lint will be uplifted into rustc
.args([ uplift: bool,
Arg::new("remove") },
.long("remove") /// Deprecate the given lint
.action(ArgAction::SetTrue) Deprecate {
.help("Remove the tasks added with 'cargo dev setup vscode-tasks'"), /// The name of the lint to deprecate
Arg::new("force-override") name: String,
.long("force-override") #[arg(long, short)]
.short('f') /// The reason for deprecation
.action(ArgAction::SetTrue) reason: Option<String>,
.help("Forces the override of existing vscode tasks"), },
]), }
]),
Command::new("remove") #[derive(Args)]
.about("Support for undoing changes done by the setup command") struct SetupCommand {
.arg_required_else_help(true) #[command(subcommand)]
.subcommands([ subcommand: SetupSubcommand,
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`"), #[derive(Subcommand)]
]), enum SetupSubcommand {
Command::new("serve") /// Alter dependencies so Intellij Rust can find rustc internals
.about("Launch a local 'ALL the Clippy Lints' website in a browser") Intellij {
.args([ #[arg(long)]
Arg::new("port") /// Remove the dependencies added with 'cargo dev setup intellij'
.long("port") remove: bool,
.short('p') #[arg(long, short, conflicts_with = "remove")]
.help("Local port for the http server") /// The path to a rustc repo that will be used for setting the dependencies
.default_value("8000") repo_path: String,
.value_parser(clap::value_parser!(u16)), },
Arg::new("lint").help("Which lint's page to load initially (optional)"), /// Add a pre-commit git hook that formats your code to make it look pretty
]), GitHook {
Command::new("lint") #[arg(long)]
.about("Manually run clippy on a file or package") /// Remove the pre-commit hook added with 'cargo dev setup git-hook'
.after_help(indoc! {" remove: bool,
EXAMPLES #[arg(long, short)]
Lint a single file: /// Forces the override of an existing git pre-commit hook
cargo dev lint tests/ui/attrs.rs force_override: bool,
},
Lint a package directory: /// Install a rustup toolchain pointing to the local clippy build
cargo dev lint tests/ui-cargo/wildcard_dependencies/fail Toolchain {
cargo dev lint ~/my-project #[arg(long, short)]
/// Override an existing toolchain
Run rustfix: force: bool,
cargo dev lint ~/my-project -- --fix #[arg(long, short)]
/// Point to --release clippy binary
Set lint levels: release: bool,
cargo dev lint file.rs -- -W clippy::pedantic #[arg(long, default_value = "clippy")]
cargo dev lint ~/my-project -- -- -W clippy::pedantic /// Name of the toolchain
"}) name: String,
.args([ },
Arg::new("path") /// Add several tasks to vscode for formatting, validation and testing
.required(true) VscodeTasks {
.help("The path to a file or package directory to lint"), #[arg(long)]
Arg::new("args") /// Remove the tasks added with 'cargo dev setup vscode-tasks'
.action(ArgAction::Append) remove: bool,
.help("Pass extra arguments to cargo/clippy-driver"), #[arg(long, short)]
]), /// Forces the override of existing vscode tasks
Command::new("rename_lint").about("Renames the given lint").args([ force_override: bool,
Arg::new("old_name") },
.index(1) }
.required(true)
.help("The name of the lint to rename"), #[derive(Args)]
Arg::new("new_name") struct RemoveCommand {
.index(2) #[command(subcommand)]
.required_unless_present("uplift") subcommand: RemoveSubcommand,
.help("The new name of the lint"), }
Arg::new("uplift")
.long("uplift") #[derive(Subcommand)]
.action(ArgAction::SetTrue) enum RemoveSubcommand {
.help("This lint will be uplifted into rustc"), /// Remove the dependencies added with 'cargo dev setup intellij'
]), Intellij,
Command::new("deprecate").about("Deprecates the given lint").args([ /// Remove the pre-commit git hook
Arg::new("name") GitHook,
.index(1) /// Remove the tasks added with 'cargo dev setup vscode-tasks'
.required(true) VscodeTasks,
.help("The name of the lint to deprecate"),
Arg::new("reason")
.long("reason")
.short('r')
.help("The reason for deprecation"),
]),
])
.get_matches()
} }

View File

@ -36,22 +36,16 @@ fn context<C: AsRef<str>>(self, text: C) -> Self {
/// # Errors /// # Errors
/// ///
/// This function errors out if the files couldn't be created or written to. /// This function errors out if the files couldn't be created or written to.
pub fn create( pub fn create(pass: &str, name: &str, category: &str, mut ty: Option<&str>, msrv: bool) -> io::Result<()> {
pass: &String, if category == "cargo" && ty.is_none() {
lint_name: Option<&String>,
category: Option<&str>,
mut ty: Option<&str>,
msrv: bool,
) -> io::Result<()> {
if category == Some("cargo") && ty.is_none() {
// `cargo` is a special category, these lints should always be in `clippy_lints/src/cargo` // `cargo` is a special category, these lints should always be in `clippy_lints/src/cargo`
ty = Some("cargo"); ty = Some("cargo");
} }
let lint = LintData { let lint = LintData {
pass, pass,
name: lint_name.expect("`name` argument is validated by clap"), name,
category: category.expect("`category` argument is validated by clap"), category,
ty, ty,
project_root: clippy_project_root(), project_root: clippy_project_root(),
}; };

View File

@ -8,7 +8,7 @@
/// # Panics /// # Panics
/// ///
/// Panics if the python commands could not be spawned /// Panics if the python commands could not be spawned
pub fn run(port: u16, lint: Option<&String>) -> ! { pub fn run(port: u16, lint: Option<String>) -> ! {
let mut url = Some(match lint { let mut url = Some(match lint {
None => format!("http://localhost:{port}"), None => format!("http://localhost:{port}"),
Some(lint) => format!("http://localhost:{port}/#{lint}"), Some(lint) => format!("http://localhost:{port}/#{lint}"),

View File

@ -314,7 +314,7 @@ pub fn rename(old_name: &str, new_name: &str, uplift: bool) {
/// # Panics /// # Panics
/// ///
/// If a file path could not read from or written to /// If a file path could not read from or written to
pub fn deprecate(name: &str, reason: Option<&String>) { pub fn deprecate(name: &str, reason: Option<&str>) {
fn finish( fn finish(
(lints, mut deprecated_lints, renamed_lints): (Vec<Lint>, Vec<DeprecatedLint>, Vec<RenamedLint>), (lints, mut deprecated_lints, renamed_lints): (Vec<Lint>, Vec<DeprecatedLint>, Vec<RenamedLint>),
name: &str, name: &str,
@ -335,7 +335,7 @@ fn finish(
println!("note: you must run `cargo uitest` to update the test results"); println!("note: you must run `cargo uitest` to update the test results");
} }
let reason = reason.map_or(DEFAULT_DEPRECATION_REASON, String::as_str); let reason = reason.unwrap_or(DEFAULT_DEPRECATION_REASON);
let name_lower = name.to_lowercase(); let name_lower = name.to_lowercase();
let name_upper = name.to_uppercase(); let name_upper = name.to_uppercase();