From 2f01b4ff75098dabef5c59795a01b5ba1c4d0d33 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Mar 2024 19:04:24 -0500 Subject: [PATCH 1/2] Move shell expansion into --config lookup --- crates/ruff/src/args.rs | 13 ++++++----- crates/ruff/src/resolve.rs | 13 ++++------- crates/ruff/tests/lint.rs | 44 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index 022d5c6f2452f..f56d75ecda6fb 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -812,15 +812,18 @@ impl TypedValueParser for ConfigArgumentParser { arg: Option<&clap::Arg>, value: &std::ffi::OsStr, ) -> Result { - let path_to_config_file = PathBuf::from(value); - if path_to_config_file.exists() { - return Ok(SingleConfigArgument::FilePath(path_to_config_file)); - } - let value = value .to_str() .ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?; + if let Ok(path_to_config_file) = + shellexpand::full(value).map(|config| PathBuf::from(config.as_ref())) + { + if path_to_config_file.is_file() { + return Ok(SingleConfigArgument::FilePath(path_to_config_file)); + } + } + let config_parse_error = match toml::Table::from_str(value) { Ok(table) => match table.try_into::() { Ok(option) => { diff --git a/crates/ruff/src/resolve.rs b/crates/ruff/src/resolve.rs index f1dbf5aa17ffd..89796ee156507 100644 --- a/crates/ruff/src/resolve.rs +++ b/crates/ruff/src/resolve.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::path::Path; use anyhow::Result; use log::debug; @@ -34,13 +34,8 @@ 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_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, config_arguments)?; + if let Some(pyproject) = config_arguments.config_file() { + let settings = resolve_root_settings(pyproject, Relativity::Cwd, config_arguments)?; debug!( "Using user-specified configuration file at: {}", pyproject.display() @@ -48,7 +43,7 @@ pub fn resolve( return Ok(PyprojectConfig::new( PyprojectDiscoveryStrategy::Fixed, settings, - Some(pyproject), + Some(pyproject.to_path_buf()), )); } diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 900fc5382f24b..23aa96ea4cca5 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -1126,3 +1126,47 @@ import os Ok(()) } + +/// Expand environment variables in `--config` paths provided via the CLI. +#[test] +fn config_expand() -> Result<()> { + let tempdir = TempDir::new()?; + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + ruff_toml, + r#" +[lint] +select = ["F"] +ignore = ["F841"] +"#, + )?; + + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg("${NAME}.toml") + .env("NAME", "ruff") + .arg("-") + .current_dir(tempdir.path()) + .pass_stdin(r#" +import os + +def func(): + x = 1 +"#), @r###" + success: false + exit_code: 1 + ----- stdout ----- + -:2:8: F401 [*] `os` imported but unused + Found 1 error. + [*] 1 fixable with the `--fix` option. + + ----- stderr ----- + "###); + }); + + Ok(()) +} From 06f3006cd86b5b7a26f191d57540e58eed19bab1 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 4 Mar 2024 12:22:09 -0500 Subject: [PATCH 2/2] Review feedback --- crates/ruff/src/args.rs | 17 ++++++++++++----- crates/ruff/tests/format.rs | 2 +- crates/ruff/tests/lint.rs | 6 +----- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/crates/ruff/src/args.rs b/crates/ruff/src/args.rs index f56d75ecda6fb..8aea65056394a 100644 --- a/crates/ruff/src/args.rs +++ b/crates/ruff/src/args.rs @@ -812,12 +812,19 @@ impl TypedValueParser for ConfigArgumentParser { arg: Option<&clap::Arg>, value: &std::ffi::OsStr, ) -> Result { - let value = value - .to_str() - .ok_or_else(|| clap::Error::new(clap::error::ErrorKind::InvalidUtf8))?; + // Convert to UTF-8. + let Some(value) = value.to_str() else { + // But respect non-UTF-8 paths. + let path_to_config_file = PathBuf::from(value); + if path_to_config_file.is_file() { + return Ok(SingleConfigArgument::FilePath(path_to_config_file)); + } + return Err(clap::Error::new(clap::error::ErrorKind::InvalidUtf8)); + }; + // Expand environment variables and tildes. if let Ok(path_to_config_file) = - shellexpand::full(value).map(|config| PathBuf::from(config.as_ref())) + shellexpand::full(value).map(|config| PathBuf::from(&*config)) { if path_to_config_file.is_file() { return Ok(SingleConfigArgument::FilePath(path_to_config_file)); @@ -890,7 +897,7 @@ A `--config` flag must either be a path to a `.toml` configuration file " It looks like you were trying to pass a path to a configuration file. -The path `{value}` does not exist" +The path `{value}` does not point to a configuration file" )); } } else if value.contains('=') { diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 0a63670cf676c..8301e789abcd2 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -111,7 +111,7 @@ fn nonexistent_config_file() { option It looks like you were trying to pass a path to a configuration file. - The path `foo.toml` does not exist + The path `foo.toml` does not point to a configuration file For more information, try '--help'. "###); diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 23aa96ea4cca5..1dc89484a907a 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -522,7 +522,7 @@ fn nonexistent_config_file() { option It looks like you were trying to pass a path to a configuration file. - The path `foo.toml` does not exist + The path `foo.toml` does not point to a configuration file For more information, try '--help'. "###); @@ -1141,9 +1141,6 @@ ignore = ["F841"] "#, )?; - insta::with_settings!({ - filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] - }, { assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) .args(STDIN_BASE_OPTIONS) .arg("--config") @@ -1166,7 +1163,6 @@ def func(): ----- stderr ----- "###); - }); Ok(()) }