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

Move shell expansion into --config lookup #10219

Merged
merged 2 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 18 additions & 8 deletions crates/ruff/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -812,14 +812,24 @@ impl TypedValueParser for ConfigArgumentParser {
arg: Option<&clap::Arg>,
value: &std::ffi::OsStr,
) -> Result<Self::Value, clap::Error> {
let path_to_config_file = PathBuf::from(value);
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
if path_to_config_file.exists() {
return Ok(SingleConfigArgument::FilePath(path_to_config_file));
}
// Convert to UTF-8.
let Some(value) = value.to_str() else {
// But respect non-UTF-8 paths.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth adding a test with a non-UTF-8 path?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm gonna defer it, I don't even know if Ruff as a whole will work with 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));
};

let value = value
.to_str()
.ok_or_else(|| 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))
{
if path_to_config_file.is_file() {
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
return Ok(SingleConfigArgument::FilePath(path_to_config_file));
}
}

let config_parse_error = match toml::Table::from_str(value) {
Ok(table) => match table.try_into::<Options>() {
Expand Down Expand Up @@ -887,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('=') {
Expand Down
13 changes: 4 additions & 9 deletions crates/ruff/src/resolve.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::path::{Path, PathBuf};
use std::path::Path;

use anyhow::Result;
use log::debug;
Expand Down Expand Up @@ -34,21 +34,16 @@ 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()
);
return Ok(PyprojectConfig::new(
PyprojectDiscoveryStrategy::Fixed,
settings,
Some(pyproject),
Some(pyproject.to_path_buf()),
));
}

Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'.
"###);
Expand Down
42 changes: 41 additions & 1 deletion crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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'.
"###);
Expand Down Expand Up @@ -1126,3 +1126,43 @@ 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"]
"#,
)?;

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(())
}
Loading