From ec575188c4859618a63d4fb2502b0adf2551382e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 28 Aug 2023 11:28:22 -0400 Subject: [PATCH] Narrow the supported options on the format CLI (#6944) ## Summary Ensures that we only show supported options: Screen Shot 2023-08-28 at 11 03 16 AM For now, I'm not super focused on DRYing up the CLI. --- crates/ruff_cli/src/args.rs | 143 +++++++++++++++++-------- crates/ruff_cli/src/commands/format.rs | 6 +- crates/ruff_cli/src/lib.rs | 19 +--- crates/ruff_dev/src/format_dev.rs | 48 +++++---- crates/ruff_dev/src/main.rs | 2 +- 5 files changed, 135 insertions(+), 83 deletions(-) diff --git a/crates/ruff_cli/src/args.rs b/crates/ruff_cli/src/args.rs index 6de964031e..1013486153 100644 --- a/crates/ruff_cli/src/args.rs +++ b/crates/ruff_cli/src/args.rs @@ -34,7 +34,7 @@ pub struct Args { #[derive(Debug, clap::Subcommand)] pub enum Command { /// Run Ruff on the given files or directories (default). - Check(CheckArgs), + Check(CheckCommand), /// Explain a rule (or all rules). #[clap(alias = "--explain")] #[command(group = clap::ArgGroup::new("selector").multiple(false).required(true))] @@ -51,9 +51,9 @@ pub enum Command { #[arg(long, value_enum, default_value = "text")] format: HelpFormat, }, - /// List or describe the available configuration options + /// List or describe the available configuration options. Config { option: Option }, - /// List all supported upstream linters + /// List all supported upstream linters. Linter { /// Output format #[arg(long, value_enum, default_value = "text")] @@ -65,19 +65,16 @@ pub enum Command { /// Generate shell completion. #[clap(alias = "--generate-shell-completion", hide = true)] GenerateShellCompletion { shell: clap_complete_command::Shell }, - /// Format the given files, or stdin when using `-`. + /// Run the Ruff formatter on the given files or directories. #[doc(hidden)] #[clap(hide = true)] - Format { - /// List of files or directories to format or `-` for stdin - files: Vec, - }, + Format(FormatCommand), } // The `Parser` derive is for ruff_dev, for ruff_cli `Args` would be sufficient #[derive(Clone, Debug, clap::Parser)] -#[allow(clippy::struct_excessive_bools, clippy::module_name_repetitions)] -pub struct CheckArgs { +#[allow(clippy::struct_excessive_bools)] +pub struct CheckCommand { /// List of files or directories to check. pub files: Vec, /// Attempt to automatically fix lint violations. @@ -95,15 +92,13 @@ pub struct CheckArgs { show_fixes: bool, #[clap(long, overrides_with("show_fixes"), hide = true)] no_show_fixes: bool, - /// Avoid writing any fixed files back; instead, output a diff for each - /// changed file to stdout. Implies `--fix-only`. + /// Avoid writing any fixed files back; instead, output a diff for each changed file to stdout. Implies `--fix-only`. #[arg(long, conflicts_with = "show_fixes")] pub diff: bool, /// Run in watch mode by re-running whenever files change. #[arg(short, long)] pub watch: bool, - /// Fix any fixable lint violations, but don't report on leftover - /// violations. Implies `--fix`. + /// Fix any fixable lint violations, but don't report on leftover violations. Implies `--fix`. #[arg(long, overrides_with("no_fix_only"))] fix_only: bool, #[clap(long, overrides_with("fix_only"), hide = true)] @@ -124,8 +119,7 @@ pub struct CheckArgs { /// configuration. #[arg(long, conflicts_with = "isolated")] pub config: Option, - /// Comma-separated list of rule codes to enable (or ALL, to enable all - /// rules). + /// Comma-separated list of rule codes to enable (or ALL, to enable all rules). #[arg( long, value_delimiter = ',', @@ -145,8 +139,7 @@ pub struct CheckArgs { hide_possible_values = true )] pub ignore: Option>, - /// Like --select, but adds additional rule codes on top of those already - /// specified. + /// Like --select, but adds additional rule codes on top of those already specified. #[arg( long, value_delimiter = ',', @@ -169,8 +162,7 @@ pub struct CheckArgs { /// List of mappings from file pattern to code to exclude. #[arg(long, value_delimiter = ',', help_heading = "Rule selection")] pub per_file_ignores: Option>, - /// Like `--per-file-ignores`, but adds additional ignores on top of - /// those already specified. + /// Like `--per-file-ignores`, but adds additional ignores on top of those already specified. #[arg(long, value_delimiter = ',', help_heading = "Rule selection")] pub extend_per_file_ignores: Option>, /// List of paths, used to omit files and/or directories from analysis. @@ -181,8 +173,7 @@ pub struct CheckArgs { help_heading = "File selection" )] pub exclude: Option>, - /// Like --exclude, but adds additional files and directories on top of - /// those already excluded. + /// Like --exclude, but adds additional files and directories on top of those already excluded. #[arg( long, value_delimiter = ',', @@ -190,8 +181,7 @@ pub struct CheckArgs { help_heading = "File selection" )] pub extend_exclude: Option>, - /// List of rule codes to treat as eligible for autofix. Only applicable - /// when autofix itself is enabled (e.g., via `--fix`). + /// List of rule codes to treat as eligible for autofix. Only applicable when autofix itself is enabled (e.g., via `--fix`). #[arg( long, value_delimiter = ',', @@ -201,8 +191,7 @@ pub struct CheckArgs { hide_possible_values = true )] pub fixable: Option>, - /// List of rule codes to treat as ineligible for autofix. Only applicable - /// when autofix itself is enabled (e.g., via `--fix`). + /// List of rule codes to treat as ineligible for autofix. Only applicable when autofix itself is enabled (e.g., via `--fix`). #[arg( long, value_delimiter = ',', @@ -212,8 +201,7 @@ pub struct CheckArgs { hide_possible_values = true )] pub unfixable: Option>, - /// Like --fixable, but adds additional rule codes on top of those already - /// specified. + /// Like --fixable, but adds additional rule codes on top of those already specified. #[arg( long, value_delimiter = ',', @@ -233,8 +221,7 @@ pub struct CheckArgs { hide = true )] pub extend_unfixable: Option>, - /// Respect file exclusions via `.gitignore` and other standard ignore - /// files. + /// Respect file exclusions via `.gitignore` and other standard ignore files. #[arg( long, overrides_with("no_respect_gitignore"), @@ -243,8 +230,7 @@ pub struct CheckArgs { respect_gitignore: bool, #[clap(long, overrides_with("respect_gitignore"), hide = true)] no_respect_gitignore: bool, - /// Enforce exclusions, even for paths passed to Ruff directly on the - /// command-line. + /// Enforce exclusions, even for paths passed to Ruff directly on the command-line. #[arg( long, overrides_with("no_force_exclude"), @@ -253,8 +239,7 @@ pub struct CheckArgs { force_exclude: bool, #[clap(long, overrides_with("force_exclude"), hide = true)] no_force_exclude: bool, - /// Set the line-length for length-associated rules and automatic - /// formatting. + /// Set the line-length for length-associated rules and automatic formatting. #[arg(long, help_heading = "Rule configuration", hide = true)] pub line_length: Option, /// Regular expression matching the name of dummy variables. @@ -280,8 +265,7 @@ pub struct CheckArgs { conflicts_with = "exit_non_zero_on_fix" )] pub exit_zero: bool, - /// Exit with a non-zero status code if any files were modified via - /// autofix, even if no lint violations remain. + /// Exit with a non-zero status code if any files were modified via autofix, even if no lint violations remain. #[arg(long, help_heading = "Miscellaneous", conflicts_with = "exit_zero")] pub exit_non_zero_on_fix: bool, /// Show counts for every rule with at least one violation. @@ -340,6 +324,46 @@ pub struct CheckArgs { pub ecosystem_ci: bool, } +#[derive(Clone, Debug, clap::Parser)] +#[allow(clippy::struct_excessive_bools)] +pub struct FormatCommand { + /// List of files or directories to format. + pub files: Vec, + /// The minimum Python version that should be supported. + #[arg(long, value_enum)] + pub target_version: Option, + /// Path to the `pyproject.toml` or `ruff.toml` file to use for configuration. + #[arg(long, conflicts_with = "isolated")] + pub config: Option, + /// Respect file exclusions via `.gitignore` and other standard ignore files. + #[arg( + long, + overrides_with("no_respect_gitignore"), + help_heading = "File selection" + )] + respect_gitignore: bool, + #[clap(long, overrides_with("respect_gitignore"), hide = true)] + no_respect_gitignore: bool, + /// Enforce exclusions, even for paths passed to Ruff directly on the command-line. + #[arg( + long, + overrides_with("no_force_exclude"), + help_heading = "File selection" + )] + force_exclude: bool, + #[clap(long, overrides_with("force_exclude"), hide = true)] + no_force_exclude: bool, + /// Set the line-length. + #[arg(long, help_heading = "Rule configuration", hide = true)] + pub line_length: Option, + /// Ignore all configuration files. + #[arg(long, conflicts_with = "config", help_heading = "Miscellaneous")] + pub isolated: bool, + /// The name of the file when passing it through stdin. + #[arg(long, help_heading = "Miscellaneous")] + pub stdin_filename: Option, +} + #[derive(Debug, Clone, Copy, clap::ValueEnum)] pub enum HelpFormat { Text, @@ -367,8 +391,7 @@ pub struct LogLevelArgs { help_heading = "Log levels" )] pub quiet: bool, - /// Disable all logging (but still exit with status code "1" upon detecting - /// lint violations). + /// Disable all logging (but still exit with status code "1" upon detecting lint violations). #[arg( short, long, @@ -393,12 +416,12 @@ impl From<&LogLevelArgs> for LogLevel { } } -impl CheckArgs { +impl CheckCommand { /// Partition the CLI into command-line arguments and configuration /// overrides. - pub fn partition(self) -> (Arguments, Overrides) { + pub fn partition(self) -> (CheckArguments, Overrides) { ( - Arguments { + CheckArguments { add_noqa: self.add_noqa, config: self.config, diff: self.diff, @@ -448,6 +471,32 @@ impl CheckArgs { } } +impl FormatCommand { + /// Partition the CLI into command-line arguments and configuration + /// overrides. + pub fn partition(self) -> (FormatArguments, Overrides) { + ( + FormatArguments { + config: self.config, + files: self.files, + isolated: self.isolated, + stdin_filename: self.stdin_filename, + }, + Overrides { + line_length: self.line_length, + respect_gitignore: resolve_bool_arg( + self.respect_gitignore, + self.no_respect_gitignore, + ), + force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude), + target_version: self.target_version, + // Unsupported on the formatter CLI, but required on `Overrides`. + ..Overrides::default() + }, + ) + } +} + fn parse_rule_selector(env: &str) -> Result { RuleSelector::from_str(env) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e)) @@ -465,7 +514,7 @@ fn resolve_bool_arg(yes: bool, no: bool) -> Option { /// CLI settings that are distinct from configuration (commands, lists of files, /// etc.). #[allow(clippy::struct_excessive_bools)] -pub struct Arguments { +pub struct CheckArguments { pub add_noqa: bool, pub config: Option, pub diff: bool, @@ -484,6 +533,16 @@ pub struct Arguments { pub watch: bool, } +/// CLI settings that are distinct from configuration (commands, lists of files, +/// etc.). +#[allow(clippy::struct_excessive_bools)] +pub struct FormatArguments { + pub config: Option, + pub files: Vec, + pub isolated: bool, + pub stdin_filename: Option, +} + /// CLI settings that function as configuration overrides. #[derive(Clone, Default)] #[allow(clippy::struct_excessive_bools)] diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs index fa4b4b56ba..0fceab4909 100644 --- a/crates/ruff_cli/src/commands/format.rs +++ b/crates/ruff_cli/src/commands/format.rs @@ -16,12 +16,12 @@ use ruff_python_ast::{PySourceType, SourceType}; use ruff_python_formatter::{format_module, FormatModuleError, PyFormatOptions}; use ruff_workspace::resolver::python_files_in_path; -use crate::args::{Arguments, Overrides}; +use crate::args::{FormatArguments, Overrides}; use crate::resolve::resolve; use crate::ExitStatus; /// Format a set of files, and return the exit status. -pub(crate) fn format(cli: &Arguments, overrides: &Overrides) -> Result { +pub(crate) fn format(cli: &FormatArguments, overrides: &Overrides) -> Result { let pyproject_config = resolve( cli.isolated, cli.config.as_deref(), @@ -59,7 +59,7 @@ pub(crate) fn format(cli: &Arguments, overrides: &Overrides) -> Result, _>>(); + .collect::>(); if result.is_ok() { Ok(ExitStatus::Success) diff --git a/crates/ruff_cli/src/lib.rs b/crates/ruff_cli/src/lib.rs index 5c76909249..a95af9820c 100644 --- a/crates/ruff_cli/src/lib.rs +++ b/crates/ruff_cli/src/lib.rs @@ -6,7 +6,6 @@ use std::sync::mpsc::channel; use anyhow::Result; use clap::CommandFactory; -use clap::FromArgMatches; use log::warn; use notify::{recommended_watcher, RecursiveMode, Watcher}; @@ -16,7 +15,7 @@ use ruff::settings::{flags, CliSettings}; use ruff::{fs, warn_user_once}; use ruff_python_formatter::{format_module, PyFormatOptions}; -use crate::args::{Args, CheckArgs, Command}; +use crate::args::{Args, CheckCommand, Command, FormatCommand}; use crate::commands::run_stdin::read_from_stdin; use crate::printer::{Flags as PrinterFlags, Printer}; @@ -149,27 +148,19 @@ quoting the executed command, along with the relevant file contents and `pyproje shell.generate(&mut Args::command(), &mut stdout()); } Command::Check(args) => return check(args, log_level), - Command::Format { files } => return format(&files), + Command::Format(args) => return format(args), } Ok(ExitStatus::Success) } -fn format(paths: &[PathBuf]) -> Result { +fn format(args: FormatCommand) -> Result { warn_user_once!( "`ruff format` is a work-in-progress, subject to change at any time, and intended only for \ experimentation." ); - // We want to use the same as `ruff check `, but we don't actually want to allow - // any of the linter settings. - // TODO(konstin): Refactor this to allow getting config and resolver without going - // though clap. - let args_matches = CheckArgs::command() - .no_binary_name(true) - .get_matches_from(paths); - let check_args: CheckArgs = CheckArgs::from_arg_matches(&args_matches)?; - let (cli, overrides) = check_args.partition(); + let (cli, overrides) = args.partition(); if is_stdin(&cli.files, cli.stdin_filename.as_deref()) { let unformatted = read_from_stdin()?; @@ -186,7 +177,7 @@ fn format(paths: &[PathBuf]) -> Result { } } -pub fn check(args: CheckArgs, log_level: LogLevel) -> Result { +pub fn check(args: CheckCommand, log_level: LogLevel) -> Result { let (cli, overrides) = args.partition(); // Construct the "default" settings. These are used when no `pyproject.toml` diff --git a/crates/ruff_dev/src/format_dev.rs b/crates/ruff_dev/src/format_dev.rs index 115ff15e4a..604327fc78 100644 --- a/crates/ruff_dev/src/format_dev.rs +++ b/crates/ruff_dev/src/format_dev.rs @@ -1,23 +1,3 @@ -use anyhow::{bail, format_err, Context, Error}; -use clap::{CommandFactory, FromArgMatches}; -use ignore::DirEntry; -use imara_diff::intern::InternedInput; -use imara_diff::sink::Counter; -use imara_diff::{diff, Algorithm}; -use indicatif::ProgressStyle; -#[cfg_attr(feature = "singlethreaded", allow(unused_imports))] -use rayon::iter::{IntoParallelIterator, ParallelIterator}; -use ruff::logging::LogLevel; -use ruff::settings::types::{FilePattern, FilePatternSet}; -use ruff_cli::args::{CheckArgs, LogLevelArgs}; -use ruff_cli::resolve::resolve; -use ruff_formatter::{FormatError, LineWidth, PrintError}; -use ruff_python_formatter::{ - format_module, FormatModuleError, MagicTrailingComma, PyFormatOptions, -}; -use ruff_workspace::resolver::python_files_in_path; -use serde::Deserialize; -use similar::{ChangeTag, TextDiff}; use std::fmt::{Display, Formatter}; use std::fs::File; use std::io::{BufWriter, Write}; @@ -28,6 +8,18 @@ use std::path::{Path, PathBuf}; use std::process::ExitCode; use std::time::{Duration, Instant}; use std::{fmt, fs, io, iter}; + +use anyhow::{bail, format_err, Context, Error}; +use clap::{CommandFactory, FromArgMatches}; +use ignore::DirEntry; +use imara_diff::intern::InternedInput; +use imara_diff::sink::Counter; +use imara_diff::{diff, Algorithm}; +use indicatif::ProgressStyle; +#[cfg_attr(feature = "singlethreaded", allow(unused_imports))] +use rayon::iter::{IntoParallelIterator, ParallelIterator}; +use serde::Deserialize; +use similar::{ChangeTag, TextDiff}; use tempfile::NamedTempFile; use tracing::{debug, error, info, info_span}; use tracing_indicatif::span_ext::IndicatifSpanExt; @@ -36,13 +28,23 @@ use tracing_subscriber::layer::SubscriberExt; use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::EnvFilter; +use ruff::logging::LogLevel; +use ruff::settings::types::{FilePattern, FilePatternSet}; +use ruff_cli::args::{FormatCommand, LogLevelArgs}; +use ruff_cli::resolve::resolve; +use ruff_formatter::{FormatError, LineWidth, PrintError}; +use ruff_python_formatter::{ + format_module, FormatModuleError, MagicTrailingComma, PyFormatOptions, +}; +use ruff_workspace::resolver::python_files_in_path; + /// Find files that ruff would check so we can format them. Adapted from `ruff_cli`. fn ruff_check_paths(dirs: &[PathBuf]) -> anyhow::Result>> { - let args_matches = CheckArgs::command() + let args_matches = FormatCommand::command() .no_binary_name(true) .get_matches_from(dirs); - let check_args: CheckArgs = CheckArgs::from_arg_matches(&args_matches)?; - let (cli, overrides) = check_args.partition(); + let arguments: FormatCommand = FormatCommand::from_arg_matches(&args_matches)?; + let (cli, overrides) = arguments.partition(); let mut pyproject_config = resolve( cli.isolated, cli.config.as_deref(), diff --git a/crates/ruff_dev/src/main.rs b/crates/ruff_dev/src/main.rs index c981d8c8a2..e3857332cb 100644 --- a/crates/ruff_dev/src/main.rs +++ b/crates/ruff_dev/src/main.rs @@ -56,7 +56,7 @@ enum Command { /// Run a ruff command n times for profiling/benchmarking Repeat { #[clap(flatten)] - args: ruff_cli::args::CheckArgs, + args: ruff_cli::args::CheckCommand, #[clap(flatten)] log_level_args: ruff_cli::args::LogLevelArgs, /// Run this many times