Use subcommands for CLI instead of incompatible boolean flags

This commit greatly simplifies the implementation of the CLI,
as well as the user expierence (since --help no longer lists all
options even though many of them are in fact incompatible).

To preserve backwards-compatability as much as possible aliases have
been added for the new subcommands, so for example the following two
commands are equivalent:

    ruff explain E402 --format json
    ruff --explain E402 --format json

However for this to work the legacy-format double-dash command has to
come first, i.e. the following no longer works:

    ruff --format json --explain E402

Since ruff previously had an implicitly default subcommand,
this is preserved for backwards compatibility, i.e. the following two
commands are equivalent:

    ruff .
    ruff check .

Previously ruff didn't complain about several argument combinations that
should have never been allowed, e.g:

    ruff --explain RUF001 --line-length 33

previously worked but now rightfully fails since the explain command
doesn't support a `--line-length` option.
This commit is contained in:
Martin Fischer 2023-01-25 15:09:32 +01:00 committed by Charlie Marsh
parent 57a68f7c7d
commit eda2be6350
7 changed files with 173 additions and 188 deletions

View file

@ -15,12 +15,44 @@ use rustc_hash::FxHashMap;
#[command(
author,
name = "ruff",
about = "Ruff: An extremely fast Python linter."
about = "Ruff: An extremely fast Python linter.",
after_help = "To get help about a specific command, see 'ruff help <command>'."
)]
#[command(version)]
#[allow(clippy::struct_excessive_bools)]
pub struct Args {
#[arg(required_unless_present_any = ["clean", "explain", "generate_shell_completion"])]
#[command(subcommand)]
pub command: Command,
#[clap(flatten)]
pub log_level_args: LogLevelArgs,
}
#[allow(clippy::large_enum_variant)]
#[derive(Debug, clap::Subcommand)]
pub enum Command {
/// Run ruff on the given files or directories (this command is used by
/// default and may be omitted)
Check(CheckArgs),
/// Explain a rule.
#[clap(alias = "--explain")]
Explain {
#[arg(value_parser=Rule::from_code)]
rule: &'static Rule,
/// Output serialization format for violations.
#[arg(long, value_enum, env = "RUFF_FORMAT", default_value = "text")]
format: HelpFormat,
},
/// Clear any caches in the current directory or any subdirectories.
#[clap(alias = "--clean")]
Clean,
/// Generate shell completion
#[clap(alias = "--generate-shell-completion", hide = true)]
GenerateShellCompletion { shell: clap_complete_command::Shell },
}
#[derive(Debug, clap::Args)]
#[allow(clippy::struct_excessive_bools, clippy::module_name_repetitions)]
pub struct CheckArgs {
pub files: Vec<PathBuf>,
/// Attempt to automatically fix lint violations.
#[arg(long, overrides_with("no_fix"))]
@ -183,9 +215,6 @@ pub struct Args {
#[arg(
long,
// conflicts_with = "add_noqa",
conflicts_with = "clean",
conflicts_with = "explain",
conflicts_with = "generate_shell_completion",
conflicts_with = "show_files",
conflicts_with = "show_settings",
// Unsupported default-command arguments.
@ -193,64 +222,11 @@ pub struct Args {
conflicts_with = "watch",
)]
pub add_noqa: bool,
/// Explain a rule.
#[arg(
long,
value_parser=Rule::from_code,
help_heading="Subcommands",
// Fake subcommands.
conflicts_with = "add_noqa",
conflicts_with = "clean",
// conflicts_with = "explain",
conflicts_with = "generate_shell_completion",
conflicts_with = "show_files",
conflicts_with = "show_settings",
// Unsupported default-command arguments.
conflicts_with = "stdin_filename",
conflicts_with = "watch",
)]
pub explain: Option<&'static Rule>,
/// Clear any caches in the current directory or any subdirectories.
#[arg(
long,
help_heading="Subcommands",
// Fake subcommands.
conflicts_with = "add_noqa",
// conflicts_with = "clean",
conflicts_with = "explain",
conflicts_with = "generate_shell_completion",
conflicts_with = "show_files",
conflicts_with = "show_settings",
// Unsupported default-command arguments.
conflicts_with = "stdin_filename",
conflicts_with = "watch",
)]
pub clean: bool,
/// Generate shell completion
#[arg(
long,
hide = true,
value_name = "SHELL",
// Fake subcommands.
conflicts_with = "add_noqa",
conflicts_with = "clean",
conflicts_with = "explain",
// conflicts_with = "generate_shell_completion",
conflicts_with = "show_files",
conflicts_with = "show_settings",
// Unsupported default-command arguments.
conflicts_with = "stdin_filename",
conflicts_with = "watch",
)]
pub generate_shell_completion: Option<clap_complete_command::Shell>,
/// See the files Ruff will be run against with the current settings.
#[arg(
long,
// Fake subcommands.
conflicts_with = "add_noqa",
conflicts_with = "clean",
conflicts_with = "explain",
conflicts_with = "generate_shell_completion",
// conflicts_with = "show_files",
conflicts_with = "show_settings",
// Unsupported default-command arguments.
@ -263,9 +239,6 @@ pub struct Args {
long,
// Fake subcommands.
conflicts_with = "add_noqa",
conflicts_with = "clean",
conflicts_with = "explain",
conflicts_with = "generate_shell_completion",
conflicts_with = "show_files",
// conflicts_with = "show_settings",
// Unsupported default-command arguments.
@ -273,22 +246,44 @@ pub struct Args {
conflicts_with = "watch",
)]
pub show_settings: bool,
#[clap(flatten)]
pub log_level_args: LogLevelArgs,
}
#[derive(Debug, Clone, Copy, clap::ValueEnum)]
pub enum HelpFormat {
Text,
Json,
}
#[allow(clippy::module_name_repetitions)]
#[derive(Debug, clap::Args)]
pub struct LogLevelArgs {
/// Enable verbose logging.
#[arg(short, long, group = "verbosity", help_heading = "Log levels")]
#[arg(
short,
long,
global = true,
group = "verbosity",
help_heading = "Log levels"
)]
pub verbose: bool,
/// Print lint violations, but nothing else.
#[arg(short, long, group = "verbosity", help_heading = "Log levels")]
#[arg(
short,
long,
global = true,
group = "verbosity",
help_heading = "Log levels"
)]
pub quiet: bool,
/// Disable all logging (but still exit with status code "1" upon detecting
/// lint violations).
#[arg(short, long, group = "verbosity", help_heading = "Log levels")]
#[arg(
short,
long,
global = true,
group = "verbosity",
help_heading = "Log levels"
)]
pub silent: bool,
}
@ -306,20 +301,17 @@ impl From<&LogLevelArgs> for LogLevel {
}
}
impl Args {
impl CheckArgs {
/// Partition the CLI into command-line arguments and configuration
/// overrides.
pub fn partition(self) -> (Arguments, Overrides) {
(
Arguments {
add_noqa: self.add_noqa,
clean: self.clean,
config: self.config,
diff: self.diff,
exit_zero: self.exit_zero,
explain: self.explain,
files: self.files,
generate_shell_completion: self.generate_shell_completion,
isolated: self.isolated,
no_cache: self.no_cache,
show_files: self.show_files,
@ -371,13 +363,10 @@ fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
#[allow(clippy::struct_excessive_bools)]
pub struct Arguments {
pub add_noqa: bool,
pub clean: bool,
pub config: Option<PathBuf>,
pub diff: bool,
pub exit_zero: bool,
pub explain: Option<&'static Rule>,
pub files: Vec<PathBuf>,
pub generate_shell_completion: Option<clap_complete_command::Shell>,
pub isolated: bool,
pub no_cache: bool,
pub show_files: bool,

View file

@ -18,12 +18,11 @@ use ruff::message::{Location, Message};
use ruff::registry::{Linter, Rule, RuleNamespace};
use ruff::resolver::PyprojectDiscovery;
use ruff::settings::flags;
use ruff::settings::types::SerializationFormat;
use ruff::{fix, fs, packaging, resolver, warn_user_once, AutofixAvailability, IOError};
use serde::Serialize;
use walkdir::WalkDir;
use crate::args::Overrides;
use crate::args::{HelpFormat, Overrides};
use crate::cache;
use crate::diagnostics::{lint_path, lint_stdin, Diagnostics};
use crate::iterators::par_iter;
@ -269,10 +268,10 @@ struct Explanation<'a> {
}
/// Explain a `Rule` to the user.
pub fn explain(rule: &Rule, format: SerializationFormat) -> Result<()> {
pub fn explain(rule: &Rule, format: HelpFormat) -> Result<()> {
let (linter, _) = Linter::parse_code(rule.code()).unwrap();
match format {
SerializationFormat::Text | SerializationFormat::Grouped => {
HelpFormat::Text => {
println!("{}\n", rule.as_ref());
println!("Code: {} ({})\n", rule.code(), linter.name());
@ -290,7 +289,7 @@ pub fn explain(rule: &Rule, format: SerializationFormat) -> Result<()> {
println!("* {format}");
}
}
SerializationFormat::Json => {
HelpFormat::Json => {
println!(
"{}",
serde_json::to_string_pretty(&Explanation {
@ -300,24 +299,12 @@ pub fn explain(rule: &Rule, format: SerializationFormat) -> Result<()> {
})?
);
}
SerializationFormat::Junit => {
bail!("`--explain` does not support junit format")
}
SerializationFormat::Github => {
bail!("`--explain` does not support GitHub format")
}
SerializationFormat::Gitlab => {
bail!("`--explain` does not support GitLab format")
}
SerializationFormat::Pylint => {
bail!("`--explain` does not support pylint format")
}
};
Ok(())
}
/// Clear any caches in the current directory or any subdirectories.
pub fn clean(level: &LogLevel) -> Result<()> {
pub fn clean(level: LogLevel) -> Result<()> {
for entry in WalkDir::new(&*path_dedot::CWD)
.into_iter()
.filter_map(Result::ok)
@ -325,7 +312,7 @@ pub fn clean(level: &LogLevel) -> Result<()> {
{
let cache = entry.path().join(CACHE_DIR_NAME);
if cache.is_dir() {
if level >= &LogLevel::Default {
if level >= LogLevel::Default {
eprintln!("Removing cache at: {}", fs::relativize_path(&cache).bold());
}
remove_dir_all(&cache)?;

View file

@ -17,8 +17,8 @@ use ::ruff::resolver::PyprojectDiscovery;
use ::ruff::settings::types::SerializationFormat;
use ::ruff::{fix, fs, warn_user_once};
use anyhow::Result;
use args::Args;
use clap::{CommandFactory, Parser};
use args::{Args, CheckArgs, Command};
use clap::{CommandFactory, Parser, Subcommand};
use colored::Colorize;
use notify::{recommended_watcher, RecursiveMode, Watcher};
use printer::{Printer, Violations};
@ -35,8 +35,32 @@ mod resolve;
pub mod updates;
fn inner_main() -> Result<ExitCode> {
let mut args: Vec<_> = std::env::args_os().collect();
// Clap doesn't support default subcommands but we want to run `check` by
// default for convenience and backwards-compatibility, so we just
// preprocess the arguments accordingly before passing them to clap.
if let Some(arg1) = args.get(1).and_then(|s| s.to_str()) {
if !Command::has_subcommand(arg1)
&& !arg1
.strip_prefix("--")
.map(Command::has_subcommand)
.unwrap_or_default()
&& arg1 != "-h"
&& arg1 != "--help"
&& arg1 != "-v"
&& arg1 != "--version"
&& arg1 != "help"
{
args.insert(1, "check".into());
}
}
// Extract command-line arguments.
let args = Args::parse();
let Args {
command,
log_level_args,
} = Args::parse_from(args);
let default_panic_hook = std::panic::take_hook();
std::panic::set_hook(Box::new(move |info| {
@ -53,20 +77,25 @@ quoting the executed command, along with the relevant file contents and `pyproje
default_panic_hook(info);
}));
let log_level: LogLevel = (&args.log_level_args).into();
let log_level: LogLevel = (&log_level_args).into();
set_up_logging(&log_level)?;
let (cli, overrides) = args.partition();
match command {
Command::Explain { rule, format } => commands::explain(rule, format)?,
Command::Clean => commands::clean(log_level)?,
Command::GenerateShellCompletion { shell } => {
shell.generate(&mut Args::command(), &mut io::stdout());
}
if let Some(shell) = cli.generate_shell_completion {
shell.generate(&mut Args::command(), &mut io::stdout());
return Ok(ExitCode::SUCCESS);
}
if cli.clean {
commands::clean(&log_level)?;
return Ok(ExitCode::SUCCESS);
Command::Check(args) => return check(args, log_level),
}
Ok(ExitCode::SUCCESS)
}
fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitCode> {
let (cli, overrides) = args.partition();
// Construct the "default" settings. These are used when no `pyproject.toml`
// files are present, or files are injected from outside of the hierarchy.
let pyproject_strategy = resolve::resolve(
@ -76,6 +105,14 @@ quoting the executed command, along with the relevant file contents and `pyproje
cli.stdin_filename.as_deref(),
)?;
if cli.show_settings {
commands::show_settings(&cli.files, &pyproject_strategy, &overrides)?;
return Ok(ExitCode::SUCCESS);
}
if cli.show_files {
commands::show_files(&cli.files, &pyproject_strategy, &overrides)?;
}
// Extract options that are included in `Settings`, but only apply at the top
// level.
let CliSettings {
@ -89,19 +126,6 @@ quoting the executed command, along with the relevant file contents and `pyproje
PyprojectDiscovery::Hierarchical(settings) => settings.cli.clone(),
};
if let Some(rule) = cli.explain {
commands::explain(rule, format)?;
return Ok(ExitCode::SUCCESS);
}
if cli.show_settings {
commands::show_settings(&cli.files, &pyproject_strategy, &overrides)?;
return Ok(ExitCode::SUCCESS);
}
if cli.show_files {
commands::show_files(&cli.files, &pyproject_strategy, &overrides)?;
return Ok(ExitCode::SUCCESS);
}
// Autofix rules are as follows:
// - If `--fix` or `--fix-only` is set, always apply fixes to the filesystem (or
// print them to stdout, if we're reading from stdin).
@ -135,14 +159,17 @@ quoting the executed command, along with the relevant file contents and `pyproje
warn_user_once!("Detected debug build without --no-cache.");
}
let printer = Printer::new(&format, &log_level, &autofix, &violations);
if cli.add_noqa {
let modifications = commands::add_noqa(&cli.files, &pyproject_strategy, &overrides)?;
if modifications > 0 && log_level >= LogLevel::Default {
println!("Added {modifications} noqa directives.");
}
} else if cli.watch {
return Ok(ExitCode::SUCCESS);
}
let printer = Printer::new(&format, &log_level, &autofix, &violations);
if cli.watch {
if !matches!(autofix, fix::FixMode::None) {
warn_user_once!("--fix is not enabled in watch mode.");
}
@ -244,7 +271,6 @@ quoting the executed command, along with the relevant file contents and `pyproje
}
}
}
Ok(ExitCode::SUCCESS)
}

View file

@ -163,8 +163,8 @@ fn test_show_source() -> Result<()> {
#[test]
fn explain_status_codes() -> Result<()> {
let mut cmd = Command::cargo_bin(BIN_NAME)?;
cmd.args(["-", "--explain", "F401"]).assert().success();
cmd.args(["--explain", "F401"]).assert().success();
let mut cmd = Command::cargo_bin(BIN_NAME)?;
cmd.args(["-", "--explain", "RUF404"]).assert().failure();
cmd.args(["--explain", "RUF404"]).assert().failure();
Ok(())
}