Skip to content
Open
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
134 changes: 73 additions & 61 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::cell::Cell;
use std::ffi::OsStr;
use std::fs::File;
use std::io::{Error, ErrorKind, Read};
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -354,50 +355,7 @@ impl Config {
style_edition: Option<StyleEdition>,
version: Option<Version>,
) -> Result<(Config, Option<PathBuf>), Error> {
/// Try to find a project file in the given directory and its parents.
/// Returns the path of the nearest project file if one exists,
/// or `None` if no project file was found.
fn resolve_project_file(dir: &Path) -> Result<Option<PathBuf>, Error> {
let mut current = if dir.is_relative() {
env::current_dir()?.join(dir)
} else {
dir.to_path_buf()
};

current = fs::canonicalize(current)?;

loop {
match get_toml_path(&current) {
Ok(Some(path)) => return Ok(Some(path)),
Err(e) => return Err(e),
_ => (),
}

// If the current directory has no parent, we're done searching.
if !current.pop() {
break;
}
}

// If nothing was found, check in the home directory.
if let Some(home_dir) = dirs::home_dir() {
if let Some(path) = get_toml_path(&home_dir)? {
return Ok(Some(path));
}
}

// If none was found there either, check in the user's configuration directory.
if let Some(mut config_dir) = dirs::config_dir() {
config_dir.push("rustfmt");
if let Some(path) = get_toml_path(&config_dir)? {
return Ok(Some(path));
}
}

Ok(None)
}

match resolve_project_file(dir)? {
match resolve_project_file(dir, &CONFIG_FILE_NAMES, true)? {
None => Ok((
Config::default_for_possible_style_edition(style_edition, edition, version),
None,
Expand Down Expand Up @@ -455,6 +413,57 @@ impl Config {
}
}

/// Try to find a project file in the given directory and its parents.
/// Returns the path of the nearest project file if one exists,
/// or `None` if no project file was found.
fn resolve_project_file(
dir: &Path,
file_names: &[impl AsRef<OsStr>],
user_dirs: bool,
) -> Result<Option<PathBuf>, Error> {
let mut current = if dir.is_relative() {
env::current_dir()?.join(dir)
} else {
dir.to_path_buf()
};

current = fs::canonicalize(current)?;

loop {
match get_toml_path(&current, file_names) {
Ok(Some(path)) => return Ok(Some(path)),
Err(e) => return Err(e),
_ => (),
}

// If the current directory has no parent, we're done searching.
if !current.pop() {
break;
}
}

if !user_dirs {
return Ok(None);
}

// If nothing was found, check in the home directory.
if let Some(home_dir) = dirs::home_dir() {
if let Some(path) = get_toml_path(&home_dir, file_names)? {
return Ok(Some(path));
}
}

// If none was found there either, check in the user's configuration directory.
if let Some(mut config_dir) = dirs::config_dir() {
config_dir.push("rustfmt");
if let Some(path) = get_toml_path(&config_dir, file_names)? {
return Ok(Some(path));
}
}

Ok(None)
}

/// Loads a config by checking the client-supplied options and if appropriate, the
/// file system (including searching the file system for overrides).
pub fn load_config<O: CliOptions>(
Expand Down Expand Up @@ -494,10 +503,9 @@ pub fn load_config<O: CliOptions>(
// Check for the presence of known config file names (`rustfmt.toml`, `.rustfmt.toml`) in `dir`
//
// Return the path if a config file exists, empty if no file exists, and Error for IO errors
fn get_toml_path(dir: &Path) -> Result<Option<PathBuf>, Error> {
const CONFIG_FILE_NAMES: [&str; 2] = [".rustfmt.toml", "rustfmt.toml"];
for config_file_name in &CONFIG_FILE_NAMES {
let config_file = dir.join(config_file_name);
fn get_toml_path(dir: &Path, file_names: &[impl AsRef<OsStr>]) -> Result<Option<PathBuf>, Error> {
for config_file_name in file_names {
let config_file = dir.join(config_file_name.as_ref());
match fs::metadata(&config_file) {
// Only return if it's a file to handle the unlikely situation of a directory named
// `rustfmt.toml`.
Expand All @@ -506,12 +514,10 @@ fn get_toml_path(dir: &Path) -> Result<Option<PathBuf>, Error> {
// `NotFound` => file not found
// `NotADirectory` => rare case where expected directory is a file
// Otherwise, return the error
Err(e) => {
if !matches!(e.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) {
let ctx = format!("Failed to get metadata for config file {:?}", &config_file);
let err = anyhow::Error::new(e).context(ctx);
return Err(Error::new(ErrorKind::Other, err));
}
Err(e) if !matches!(e.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory) => {
let ctx = format!("Failed to get metadata for config file {:?}", config_file);
let err = anyhow::Error::new(e).context(ctx);
return Err(Error::new(ErrorKind::Other, err));
}
_ => {}
}
Expand All @@ -533,23 +539,29 @@ fn config_path(options: &dyn CliOptions) -> Result<Option<PathBuf>, Error> {
// Read the config_path and convert to parent dir if a file is provided.
// If a config file cannot be found from the given path, return error.
match options.config_path() {
Some(path) if !path.exists() => config_path_not_found(path.to_str().unwrap()),
Some(path) if path.is_dir() => {
let config_file_path = get_toml_path(path)?;
Some(path) => {
let config_file_path =
if path.as_os_str().as_encoded_bytes().last() == Some(&b'/') || path.is_dir() {

@matthewhughes934 matthewhughes934 Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is .is_dir not a sufficient check here?

View changes since the review

@rami3l rami3l Sep 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@matthewhughes934 Originally that had been largely sufficient, but with the new semantics, the path to be passed in through the base dir mode might not exist immediately, and has to be found via recursive search. In this case, a trailing / makes it clear that we are looking for a directory rather than a TOML file.

This should be able to match how rust-analyzer is calling rustfmt: it is passing the parent directory of the file, which itself is passed in via stdin.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but with the new semantics, the path to be passed in through the base dir mode might not exist immediately

Can you explain that case more? If someone passes as path that doesn't existing to --config-path I would expect it to error out.

My primary concern here is that / is not a path separator on Windows mostly because of Windows weirdness I expect simply checking for \ might also not be sufficient here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can you explain that case more? If someone passes as path that doesn't existing to --config-path I would expect it to error out.

@matthewhughes934 I totally get your point no, and my apologies for potential confusion. Here is the full background:

Firstly, when rust-analyzer tries to format dir/file.rs, it wants to pass --config-path=dir to it, and that directory may or may not have the expected .rustfmt.toml or rustfmt.toml, so a recursive search should be initiated starting from there. However, due to #4660 being unresolved, rust-analyzer has instead used the hack of cding to dir when running rustfmt.

From there, I'd agree that --config-path=dir should bail out when dir doesn't exist.

Secondly, however, if the user wants to use a different name for the config file, taking the example mentioned in #4660 (comment):

rustfmt = { extraArgs = { "+nightly", "--config-path=.rustfmt.unstable.toml" } },

The file (.rustfmt.unstable.toml in the above example) is better placed at CWD, but it may totally be at a different place. This time I OTOH don't want --config-path=file to bail out immediately because I may want to use ../../file etc.

The crux of the problem here, it seems to me, is that --config-path is overloaded with two meanings and this has made it hard for us to disambiguate, for which some manual intervention must be involved.

The trailing / is, as you said, probably not the best idea for disambiguation.

// If the path is a known directory, we interpret itself as the base directory.
resolve_project_file(path, &CONFIG_FILE_NAMES, false)
} else if let Some((dir, file)) = path.parent().zip(path.file_name()) {
// Otherwise, we search for the file's base name in its parent directory.
resolve_project_file(dir, &[file], false)
} else {
Ok(None)
}?;
if config_file_path.is_some() {
Ok(config_file_path)
} else {
config_path_not_found(path.to_str().unwrap())
}
}
Some(path) => Ok(Some(
// Canonicalize only after checking above that the `path.exists()`.
path.canonicalize()?,
)),
None => Ok(None),
}
}

const CONFIG_FILE_NAMES: [&str; 2] = [".rustfmt.toml", "rustfmt.toml"];

#[cfg(test)]
mod test {
use super::*;
Expand Down
7 changes: 7 additions & 0 deletions tests/config/issue_4660/Cargo.lock

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

2 changes: 2 additions & 0 deletions tests/config/issue_4660/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[workspace]
members = ["inner_lib"]
1 change: 1 addition & 0 deletions tests/config/issue_4660/inner_bin/.rustfmt.unstable.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
disable_all_formatting = true
6 changes: 6 additions & 0 deletions tests/config/issue_4660/inner_bin/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "inner_bin"
version = "0.1.0"
edition = "2021"

[dependencies]
3 changes: 3 additions & 0 deletions tests/config/issue_4660/inner_bin/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
6 changes: 6 additions & 0 deletions tests/config/issue_4660/inner_lib/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "inner_lib"
version = "0.1.0"
edition = "2021"

[dependencies]
1 change: 1 addition & 0 deletions tests/config/issue_4660/inner_lib/rustfmt.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
disable_all_formatting = true
1 change: 1 addition & 0 deletions tests/config/issue_4660/inner_lib/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub fn add(left:u64,right:u64)->u64{left+right}
40 changes: 40 additions & 0 deletions tests/rustfmt/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,46 @@ fn rustfmt_error_improvement_regarding_invalid_toml() {
assert!(stderr.contains(&expected_error_message));
}

#[test]
fn config_path_walks_parent_directories_with_dir_name() {
let src_dir = "tests/config/issue_4660/inner_lib/src";
let src_file = src_dir.to_owned() + "/lib.rs";
let args = ["--config-path", src_dir, &src_file];
let (stdout, stderr) = rustfmt(&args);

assert_eq!(stderr, "");
// Due to `disable_all_formatting = true` in `tests/config/issue_4660/inner_lib/rustfmt.toml`,
// the source file should not be modified.
assert_eq!(stdout, "");
}

#[test]
fn config_path_walks_parent_directories_with_toml_name() {
let toml_name = ".rustfmt.unstable.toml";
let src_name = "main.rs";
let src_dir = "tests/config/issue_4660/inner_bin/src";

let args = [
"--config-path",
&[src_dir, toml_name].join("/"),
&[src_dir, src_name].join("/"),
];
let (stdout, stderr) = rustfmt(&args);

assert_eq!(stderr, "");
// Due to `disable_all_formatting = true` in `tests/config/issue_4660/inner_bin/.rustfmt.unstable.toml`,
// the source file should not be modified.
assert_eq!(stdout, "");

let args = ["--config-path", toml_name, src_name];
let (stdout, stderr) = rustfmt_with_extra(&args, Some(src_dir), &[]);

assert_eq!(stderr, "");
// Due to `disable_all_formatting = true` in `tests/config/issue_4660/inner_bin/.rustfmt.unstable.toml`,
// the source file should not be modified.
assert_eq!(stdout, "");
}

#[test]
fn rustfmt_allow_not_a_dir_errors() {
// See also https://github.com/rust-lang/rustfmt/pull/6624
Expand Down