Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow arbitrary configuration options to be overridden via the CLI #9599

Merged
merged 61 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
d73a453
WIP
AlexWaygood Jan 19, 2024
159fada
Get it working with `--isolated`
AlexWaygood Jan 19, 2024
7cd900c
Get it working without `--isolated`
AlexWaygood Jan 21, 2024
3cdbec7
i can spell
AlexWaygood Jan 21, 2024
0f2cc2c
Fix a couple build/test failures
AlexWaygood Jan 21, 2024
18daf07
Simplify `Configuration.transform()`
AlexWaygood Jan 22, 2024
721653d
Remove unnecessary special casing
AlexWaygood Jan 22, 2024
7bb72f5
Just pass a `Vec` to `from_cli_options`
AlexWaygood Jan 22, 2024
e7a1d89
Misc review comments
AlexWaygood Jan 23, 2024
7c05cd5
More docs cleanup
AlexWaygood Jan 23, 2024
26b7dc8
use an `Arc`
AlexWaygood Jan 23, 2024
59e6e0a
Use `clap` a little more for slightly prettier errors
AlexWaygood Jan 23, 2024
f33e91e
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Jan 24, 2024
cd6b4ce
regen docs
AlexWaygood Jan 24, 2024
3fb5d21
Improve error messages
AlexWaygood Jan 25, 2024
2568265
Improve error messages more
AlexWaygood Jan 25, 2024
d6a18fe
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Jan 25, 2024
8379890
Add tests for some failure cases
AlexWaygood Jan 25, 2024
366cd1b
Also test the formatter
AlexWaygood Jan 25, 2024
2583136
Many more tests
AlexWaygood Jan 25, 2024
981210c
spelling
AlexWaygood Jan 25, 2024
3bfcae2
Simplify some code
AlexWaygood Jan 25, 2024
e32dd0a
Further improve error messages
AlexWaygood Jan 26, 2024
aca4af5
cargo fmt
AlexWaygood Jan 26, 2024
446517b
couple more tests
AlexWaygood Jan 26, 2024
c39fbd6
Docs
AlexWaygood Jan 26, 2024
ebd7286
Apply suggestions from code review
AlexWaygood Jan 29, 2024
d771243
add a newline
AlexWaygood Jan 29, 2024
be48c4f
Fix compile; address more Micha comments
AlexWaygood Jan 29, 2024
8f79c65
Couple more comments
AlexWaygood Jan 29, 2024
1cc613c
Return `&Path` from `config_file()`
AlexWaygood Jan 29, 2024
c12bb11
Improve docs for `--config`
AlexWaygood Jan 29, 2024
a880691
regen docs
AlexWaygood Jan 29, 2024
59e07c3
Delete `Configuration::transform()`
AlexWaygood Jan 29, 2024
7ec9e8a
Rename the enum and its variants
AlexWaygood Jan 29, 2024
de846ba
More consistent naming for arguments that take instances of `ConfigAr…
AlexWaygood Jan 29, 2024
893f214
Better dev docs for the `ConfigArguments` struct
AlexWaygood Jan 29, 2024
0ccc608
Rework docs; improve test
AlexWaygood Jan 29, 2024
b12a9c7
cargo fmt
AlexWaygood Jan 29, 2024
1501eeb
Don't return `clap::Error` after the initial argument-parsing phase
AlexWaygood Jan 29, 2024
e46c06a
Various cleanups
AlexWaygood Feb 1, 2024
e39aa39
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Feb 1, 2024
e1a3328
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Feb 1, 2024
d83cfa5
Fixup some issues after merging in `main`
AlexWaygood Feb 1, 2024
138c6c3
Fix remaining test failures
AlexWaygood Feb 1, 2024
e500b39
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Feb 1, 2024
acca909
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Feb 6, 2024
5e83ef8
Fixup after merge
AlexWaygood Feb 6, 2024
18da11b
Merge branch 'arbitrary-cli-overrides' of https://github.com/AlexWayg…
AlexWaygood Feb 6, 2024
0b0917b
``
AlexWaygood Feb 6, 2024
5074bff
Address some Zanie comments
AlexWaygood Feb 6, 2024
c6019f7
Merge branch 'arbitrary-cli-overrides' of https://github.com/AlexWayg…
AlexWaygood Feb 6, 2024
7d6f9dd
Add a test for overriding a deprecated CLI option via the CLI
AlexWaygood Feb 6, 2024
bb2e6ee
regen docs
AlexWaygood Feb 6, 2024
7739440
Use `insta-filters` for the tests where the tempdir path is included …
AlexWaygood Feb 7, 2024
88e7273
Multiline tips instead of multiple tips
AlexWaygood Feb 7, 2024
bccab4e
Make more concise
AlexWaygood Feb 7, 2024
6eea797
Differentiate between TOML syntax errors and valid TOML that we don't…
AlexWaygood Feb 7, 2024
983e5aa
Address Charlie's docs review
AlexWaygood Feb 9, 2024
789d1d0
Add a transitional sentence
AlexWaygood Feb 9, 2024
0f8a583
Merge branch 'main' into arbitrary-cli-overrides
AlexWaygood Feb 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/ruff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ serde_json = { workspace = true }
shellexpand = { workspace = true }
strum = { workspace = true, features = [] }
thiserror = { workspace = true }
toml = { workspace = true }
tracing = { workspace = true, features = ["log"] }
walkdir = { workspace = true }
wild = { workspace = true }
Expand Down
524 changes: 397 additions & 127 deletions crates/ruff/src/args.rs

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions crates/ruff/src/commands/add_noqa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,17 @@ use ruff_linter::warn_user_once;
use ruff_python_ast::{PySourceType, SourceType};
use ruff_workspace::resolver::{python_files_in_path, PyprojectConfig, ResolvedFile};

use crate::args::CliOverrides;
use crate::args::ConfigArguments;

/// Add `noqa` directives to a collection of files.
pub(crate) fn add_noqa(
files: &[PathBuf],
pyproject_config: &PyprojectConfig,
overrides: &CliOverrides,
config_arguments: &ConfigArguments,
) -> Result<usize> {
// Collect all the files to check.
let start = Instant::now();
let (paths, resolver) = python_files_in_path(files, pyproject_config, overrides)?;
let (paths, resolver) = python_files_in_path(files, pyproject_config, config_arguments)?;
let duration = start.elapsed();
debug!("Identified files to lint in: {:?}", duration);

Expand Down
10 changes: 5 additions & 5 deletions crates/ruff/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use ruff_workspace::resolver::{
match_exclusion, python_files_in_path, PyprojectConfig, ResolvedFile,
};

use crate::args::CliOverrides;
use crate::args::ConfigArguments;
use crate::cache::{Cache, PackageCacheMap, PackageCaches};
use crate::diagnostics::Diagnostics;
use crate::panic::catch_unwind;
Expand All @@ -34,15 +34,15 @@ use crate::panic::catch_unwind;
pub(crate) fn check(
files: &[PathBuf],
pyproject_config: &PyprojectConfig,
overrides: &CliOverrides,
config_arguments: &ConfigArguments,
cache: flags::Cache,
noqa: flags::Noqa,
fix_mode: flags::FixMode,
unsafe_fixes: UnsafeFixes,
) -> Result<Diagnostics> {
// Collect all the Python files to check.
let start = Instant::now();
let (paths, resolver) = python_files_in_path(files, pyproject_config, overrides)?;
let (paths, resolver) = python_files_in_path(files, pyproject_config, config_arguments)?;
debug!("Identified files to lint in: {:?}", start.elapsed());

if paths.is_empty() {
Expand Down Expand Up @@ -233,7 +233,7 @@ mod test {
use ruff_workspace::resolver::{PyprojectConfig, PyprojectDiscoveryStrategy};
use ruff_workspace::Settings;

use crate::args::CliOverrides;
use crate::args::ConfigArguments;

use super::check;

Expand Down Expand Up @@ -272,7 +272,7 @@ mod test {
// Notebooks are not included by default
&[tempdir.path().to_path_buf(), notebook],
&pyproject_config,
&CliOverrides::default(),
&ConfigArguments::default(),
flags::Cache::Disabled,
flags::Noqa::Disabled,
flags::FixMode::Generate,
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff/src/commands/check_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@ use ruff_linter::packaging;
use ruff_linter::settings::flags;
use ruff_workspace::resolver::{match_exclusion, python_file_at_path, PyprojectConfig, Resolver};

use crate::args::CliOverrides;
use crate::args::ConfigArguments;
use crate::diagnostics::{lint_stdin, Diagnostics};
use crate::stdin::{parrot_stdin, read_from_stdin};

/// Run the linter over a single file, read from `stdin`.
pub(crate) fn check_stdin(
filename: Option<&Path>,
pyproject_config: &PyprojectConfig,
overrides: &CliOverrides,
overrides: &ConfigArguments,
noqa: flags::Noqa,
fix_mode: flags::FixMode,
) -> Result<Diagnostics> {
Expand Down
9 changes: 4 additions & 5 deletions crates/ruff/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use ruff_text_size::{TextLen, TextRange, TextSize};
use ruff_workspace::resolver::{match_exclusion, python_files_in_path, ResolvedFile, Resolver};
use ruff_workspace::FormatterSettings;

use crate::args::{CliOverrides, FormatArguments, FormatRange};
use crate::args::{ConfigArguments, FormatArguments, FormatRange};
use crate::cache::{Cache, FileCacheKey, PackageCacheMap, PackageCaches};
use crate::panic::{catch_unwind, PanicError};
use crate::resolve::resolve;
Expand Down Expand Up @@ -60,18 +60,17 @@ impl FormatMode {
/// Format a set of files, and return the exit status.
pub(crate) fn format(
cli: FormatArguments,
overrides: &CliOverrides,
config_arguments: &ConfigArguments,
log_level: LogLevel,
) -> Result<ExitStatus> {
let pyproject_config = resolve(
cli.isolated,
cli.config.as_deref(),
overrides,
config_arguments,
cli.stdin_filename.as_deref(),
)?;
let mode = FormatMode::from_cli(&cli);
let files = resolve_default_files(cli.files, false);
let (paths, resolver) = python_files_in_path(&files, &pyproject_config, overrides)?;
let (paths, resolver) = python_files_in_path(&files, &pyproject_config, config_arguments)?;

if paths.is_empty() {
warn_user_once!("No Python files found under the given path(s)");
Expand Down
12 changes: 7 additions & 5 deletions crates/ruff/src/commands/format_stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use ruff_python_ast::{PySourceType, SourceType};
use ruff_workspace::resolver::{match_exclusion, python_file_at_path, Resolver};
use ruff_workspace::FormatterSettings;

use crate::args::{CliOverrides, FormatArguments, FormatRange};
use crate::args::{ConfigArguments, FormatArguments, FormatRange};
use crate::commands::format::{
format_source, warn_incompatible_formatter_settings, FormatCommandError, FormatMode,
FormatResult, FormattedSource,
Expand All @@ -19,11 +19,13 @@ use crate::stdin::{parrot_stdin, read_from_stdin};
use crate::ExitStatus;

/// Run the formatter over a single file, read from `stdin`.
pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> Result<ExitStatus> {
pub(crate) fn format_stdin(
cli: &FormatArguments,
config_arguments: &ConfigArguments,
) -> Result<ExitStatus> {
let pyproject_config = resolve(
cli.isolated,
cli.config.as_deref(),
overrides,
config_arguments,
cli.stdin_filename.as_deref(),
)?;

Expand All @@ -34,7 +36,7 @@ pub(crate) fn format_stdin(cli: &FormatArguments, overrides: &CliOverrides) -> R

if resolver.force_exclude() {
if let Some(filename) = cli.stdin_filename.as_deref() {
if !python_file_at_path(filename, &mut resolver, overrides)? {
if !python_file_at_path(filename, &mut resolver, config_arguments)? {
if mode.is_write() {
parrot_stdin()?;
}
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/commands/show_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,17 @@ use itertools::Itertools;
use ruff_linter::warn_user_once;
use ruff_workspace::resolver::{python_files_in_path, PyprojectConfig, ResolvedFile};

use crate::args::CliOverrides;
use crate::args::ConfigArguments;

/// Show the list of files to be checked based on current settings.
pub(crate) fn show_files(
files: &[PathBuf],
pyproject_config: &PyprojectConfig,
overrides: &CliOverrides,
config_arguments: &ConfigArguments,
writer: &mut impl Write,
) -> Result<()> {
// Collect all files in the hierarchy.
let (paths, _resolver) = python_files_in_path(files, pyproject_config, overrides)?;
let (paths, _resolver) = python_files_in_path(files, pyproject_config, config_arguments)?;

if paths.is_empty() {
warn_user_once!("No Python files found under the given path(s)");
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff/src/commands/show_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ use itertools::Itertools;

use ruff_workspace::resolver::{python_files_in_path, PyprojectConfig, ResolvedFile};

use crate::args::CliOverrides;
use crate::args::ConfigArguments;

/// Print the user-facing configuration settings.
pub(crate) fn show_settings(
files: &[PathBuf],
pyproject_config: &PyprojectConfig,
overrides: &CliOverrides,
config_arguments: &ConfigArguments,
writer: &mut impl Write,
) -> Result<()> {
// Collect all files in the hierarchy.
let (paths, resolver) = python_files_in_path(files, pyproject_config, overrides)?;
let (paths, resolver) = python_files_in_path(files, pyproject_config, config_arguments)?;

// Print the list of files.
let Some(path) = paths
Expand Down
39 changes: 24 additions & 15 deletions crates/ruff/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,24 +204,23 @@ pub fn run(
}

fn format(args: FormatCommand, log_level: LogLevel) -> Result<ExitStatus> {
let (cli, overrides) = args.partition();
let (cli, config_arguments) = args.partition()?;

if is_stdin(&cli.files, cli.stdin_filename.as_deref()) {
commands::format_stdin::format_stdin(&cli, &overrides)
commands::format_stdin::format_stdin(&cli, &config_arguments)
} else {
commands::format::format(cli, &overrides, log_level)
commands::format::format(cli, &config_arguments, log_level)
}
}

pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
let (cli, overrides) = args.partition();
let (cli, config_arguments) = 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_config = resolve::resolve(
cli.isolated,
cli.config.as_deref(),
&overrides,
&config_arguments,
cli.stdin_filename.as_deref(),
)?;

Expand All @@ -239,11 +238,21 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
let files = resolve_default_files(cli.files, is_stdin);

if cli.show_settings {
commands::show_settings::show_settings(&files, &pyproject_config, &overrides, &mut writer)?;
commands::show_settings::show_settings(
&files,
&pyproject_config,
&config_arguments,
&mut writer,
)?;
return Ok(ExitStatus::Success);
}
if cli.show_files {
commands::show_files::show_files(&files, &pyproject_config, &overrides, &mut writer)?;
commands::show_files::show_files(
&files,
&pyproject_config,
&config_arguments,
&mut writer,
)?;
return Ok(ExitStatus::Success);
}

Expand Down Expand Up @@ -302,7 +311,8 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
if !fix_mode.is_generate() {
warn_user!("--fix is incompatible with --add-noqa.");
}
let modifications = commands::add_noqa::add_noqa(&files, &pyproject_config, &overrides)?;
let modifications =
commands::add_noqa::add_noqa(&files, &pyproject_config, &config_arguments)?;
if modifications > 0 && log_level >= LogLevel::Default {
let s = if modifications == 1 { "" } else { "s" };
#[allow(clippy::print_stderr)]
Expand Down Expand Up @@ -352,7 +362,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
let messages = commands::check::check(
&files,
&pyproject_config,
&overrides,
&config_arguments,
cache.into(),
noqa.into(),
fix_mode,
Expand All @@ -374,8 +384,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
if matches!(change_kind, ChangeKind::Configuration) {
pyproject_config = resolve::resolve(
cli.isolated,
cli.config.as_deref(),
&overrides,
&config_arguments,
cli.stdin_filename.as_deref(),
)?;
}
Expand All @@ -385,7 +394,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
let messages = commands::check::check(
&files,
&pyproject_config,
&overrides,
&config_arguments,
cache.into(),
noqa.into(),
fix_mode,
Expand All @@ -402,15 +411,15 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
commands::check_stdin::check_stdin(
cli.stdin_filename.map(fs::normalize_path).as_deref(),
&pyproject_config,
&overrides,
&config_arguments,
noqa.into(),
fix_mode,
)?
} else {
commands::check::check(
&files,
&pyproject_config,
&overrides,
&config_arguments,
cache.into(),
noqa.into(),
fix_mode,
Expand Down
18 changes: 9 additions & 9 deletions crates/ruff/src/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,18 @@ use ruff_workspace::resolver::{
Relativity,
};

use crate::args::CliOverrides;
use crate::args::ConfigArguments;

/// Resolve the relevant settings strategy and defaults for the current
/// invocation.
pub fn resolve(
isolated: bool,
config: Option<&Path>,
overrides: &CliOverrides,
config_arguments: &ConfigArguments,
stdin_filename: Option<&Path>,
) -> Result<PyprojectConfig> {
// First priority: if we're running in isolated mode, use the default settings.
if isolated {
let config = overrides.transform(Configuration::default());
let config = config_arguments.transform(Configuration::default());
let settings = config.into_settings(&path_dedot::CWD)?;
debug!("Isolated mode, not reading any pyproject.toml");
return Ok(PyprojectConfig::new(
Expand All @@ -36,12 +35,13 @@ pub fn resolve(
// Second priority: the user specified a `pyproject.toml` file. Use that
// `pyproject.toml` for _all_ configuration, and resolve paths relative to the
// current working directory. (This matches ESLint's behavior.)
if let Some(pyproject) = config
if let Some(pyproject) = config_arguments
.config_file()
.map(|config| config.display().to_string())
.map(|config| shellexpand::full(&config).map(|config| PathBuf::from(config.as_ref())))
.transpose()?
{
let settings = resolve_root_settings(&pyproject, Relativity::Cwd, overrides)?;
let settings = resolve_root_settings(&pyproject, Relativity::Cwd, config_arguments)?;
debug!(
"Using user-specified configuration file at: {}",
pyproject.display()
Expand All @@ -67,7 +67,7 @@ pub fn resolve(
"Using configuration file (via parent) at: {}",
pyproject.display()
);
let settings = resolve_root_settings(&pyproject, Relativity::Parent, overrides)?;
let settings = resolve_root_settings(&pyproject, Relativity::Parent, config_arguments)?;
return Ok(PyprojectConfig::new(
PyprojectDiscoveryStrategy::Hierarchical,
settings,
Expand All @@ -84,7 +84,7 @@ pub fn resolve(
"Using configuration file (via cwd) at: {}",
pyproject.display()
);
let settings = resolve_root_settings(&pyproject, Relativity::Cwd, overrides)?;
let settings = resolve_root_settings(&pyproject, Relativity::Cwd, config_arguments)?;
return Ok(PyprojectConfig::new(
PyprojectDiscoveryStrategy::Hierarchical,
settings,
Expand All @@ -97,7 +97,7 @@ pub fn resolve(
// "closest" `pyproject.toml` file for every Python file later on, so these act
// as the "default" settings.)
debug!("Using Ruff default settings");
let config = overrides.transform(Configuration::default());
let config = config_arguments.transform(Configuration::default());
let settings = config.into_settings(&path_dedot::CWD)?;
Ok(PyprojectConfig::new(
PyprojectDiscoveryStrategy::Hierarchical,
Expand Down
Loading
Loading