Skip to content

Commit

Permalink
Move shell expansion into --config lookup (astral-sh#10219)
Browse files Browse the repository at this point in the history
## Summary

When users provide configurations via `--config`, we use `shellexpand`
to ensure that we expand signifiers like `~` and environment variables.

In astral-sh#9599, we modified `--config`
to accept either a path or an arbitrary setting. However, the detection
(to determine whether the value is a path or a setting) was lacking the
`shellexpand` behavior -- it was downstream. So we were always treating
paths like `~/ruff.toml` as values, not paths.

Closes astral-sh/ruff-vscode#413.
  • Loading branch information
charliermarsh authored and nkxxll committed Mar 10, 2024
1 parent d7e4d94 commit 6274155
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 19 deletions.
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);
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.
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() {
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(())
}

0 comments on commit 6274155

Please sign in to comment.