Skip to content

Commit

Permalink
refactor - show file path in error message when parsing config from toml
Browse files Browse the repository at this point in the history
fixes 6302

Display the path to the toml config file that rustfmt failed to parse
configurations from.
  • Loading branch information
rufevean authored and ytmimi committed Sep 12, 2024
1 parent 16f0ecc commit 0127491
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 38 deletions.
69 changes: 35 additions & 34 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,14 +291,8 @@ impl Config {
let mut file = File::open(&file_path)?;
let mut toml = String::new();
file.read_to_string(&mut toml)?;
Config::from_toml_for_style_edition(
&toml,
file_path.parent().unwrap(),
edition,
style_edition,
version,
)
.map_err(|err| Error::new(ErrorKind::InvalidData, err))
Config::from_toml_for_style_edition(&toml, file_path, edition, style_edition, version)
.map_err(|err| Error::new(ErrorKind::InvalidData, err))
}

/// Resolves the config for input in `dir`.
Expand Down Expand Up @@ -370,13 +364,13 @@ impl Config {
}

#[allow(dead_code)]
pub(super) fn from_toml(toml: &str, dir: &Path) -> Result<Config, String> {
Self::from_toml_for_style_edition(toml, dir, None, None, None)
pub(super) fn from_toml(toml: &str, file_path: &Path) -> Result<Config, String> {
Self::from_toml_for_style_edition(toml, file_path, None, None, None)
}

pub(crate) fn from_toml_for_style_edition(
toml: &str,
dir: &Path,
file_path: &Path,
edition: Option<Edition>,
style_edition: Option<StyleEdition>,
version: Option<Version>,
Expand All @@ -400,13 +394,19 @@ impl Config {
if !err.is_empty() {
eprint!("{err}");
}
let dir = file_path.parent().ok_or_else(|| {
format!("failed to get parent directory for {}", file_path.display())
})?;

Ok(parsed_config.to_parsed_config(style_edition, edition, version, dir))
}
Err(e) => {
err.push_str("Error: Decoding config file failed:\n");
err.push_str(format!("{e}\n").as_str());
err.push_str("Please check your config file.");
Err(err)
let err_msg = format!(
"The file `{}` failed to parse.\nError details: {e}",
file_path.display()
);
err.push_str(&err_msg);
Err(err_msg)
}
}
}
Expand Down Expand Up @@ -674,7 +674,7 @@ mod test {

#[test]
fn test_was_set() {
let config = Config::from_toml("hard_tabs = true", Path::new("")).unwrap();
let config = Config::from_toml("hard_tabs = true", Path::new("./rustfmt.toml")).unwrap();

assert_eq!(config.was_set().hard_tabs(), true);
assert_eq!(config.was_set().verbose(), false);
Expand Down Expand Up @@ -933,7 +933,8 @@ make_backup = false
#[nightly_only_test]
#[test]
fn test_unstable_from_toml() {
let config = Config::from_toml("unstable_features = true", Path::new("")).unwrap();
let config =
Config::from_toml("unstable_features = true", Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.was_set().unstable_features(), true);
assert_eq!(config.unstable_features(), true);
}
Expand Down Expand Up @@ -963,7 +964,7 @@ make_backup = false
unstable_features = true
merge_imports = true
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.imports_granularity(), ImportGranularity::Crate);
}

Expand All @@ -975,7 +976,7 @@ make_backup = false
merge_imports = true
imports_granularity = "Preserve"
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.imports_granularity(), ImportGranularity::Preserve);
}

Expand All @@ -986,7 +987,7 @@ make_backup = false
unstable_features = true
merge_imports = true
"#;
let mut config = Config::from_toml(toml, Path::new("")).unwrap();
let mut config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
config.override_value("imports_granularity", "Preserve");
assert_eq!(config.imports_granularity(), ImportGranularity::Preserve);
}
Expand All @@ -998,7 +999,7 @@ make_backup = false
unstable_features = true
imports_granularity = "Module"
"#;
let mut config = Config::from_toml(toml, Path::new("")).unwrap();
let mut config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
config.override_value("merge_imports", "true");
// no effect: the new option always takes precedence
assert_eq!(config.imports_granularity(), ImportGranularity::Module);
Expand All @@ -1015,7 +1016,7 @@ make_backup = false
use_small_heuristics = "Default"
max_width = 200
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), 120);
assert_eq!(config.attr_fn_like_width(), 140);
assert_eq!(config.chain_width(), 120);
Expand All @@ -1031,7 +1032,7 @@ make_backup = false
use_small_heuristics = "Max"
max_width = 120
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), 120);
assert_eq!(config.attr_fn_like_width(), 120);
assert_eq!(config.chain_width(), 120);
Expand All @@ -1047,7 +1048,7 @@ make_backup = false
use_small_heuristics = "Off"
max_width = 100
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), usize::max_value());
assert_eq!(config.attr_fn_like_width(), usize::max_value());
assert_eq!(config.chain_width(), usize::max_value());
Expand All @@ -1069,7 +1070,7 @@ make_backup = false
struct_lit_width = 30
struct_variant_width = 34
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), 20);
assert_eq!(config.attr_fn_like_width(), 40);
assert_eq!(config.chain_width(), 20);
Expand All @@ -1091,7 +1092,7 @@ make_backup = false
struct_lit_width = 30
struct_variant_width = 34
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), 20);
assert_eq!(config.attr_fn_like_width(), 40);
assert_eq!(config.chain_width(), 20);
Expand All @@ -1113,7 +1114,7 @@ make_backup = false
struct_lit_width = 30
struct_variant_width = 34
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), 20);
assert_eq!(config.attr_fn_like_width(), 40);
assert_eq!(config.chain_width(), 20);
Expand All @@ -1129,7 +1130,7 @@ make_backup = false
max_width = 90
fn_call_width = 95
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.fn_call_width(), 90);
}

Expand All @@ -1139,7 +1140,7 @@ make_backup = false
max_width = 80
attr_fn_like_width = 90
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.attr_fn_like_width(), 80);
}

Expand All @@ -1149,7 +1150,7 @@ make_backup = false
max_width = 78
struct_lit_width = 90
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.struct_lit_width(), 78);
}

Expand All @@ -1159,7 +1160,7 @@ make_backup = false
max_width = 80
struct_variant_width = 90
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.struct_variant_width(), 80);
}

Expand All @@ -1169,7 +1170,7 @@ make_backup = false
max_width = 60
array_width = 80
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.array_width(), 60);
}

Expand All @@ -1179,7 +1180,7 @@ make_backup = false
max_width = 80
chain_width = 90
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.chain_width(), 80);
}

Expand All @@ -1189,7 +1190,7 @@ make_backup = false
max_width = 70
single_line_if_else_max_width = 90
"#;
let config = Config::from_toml(toml, Path::new("")).unwrap();
let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap();
assert_eq!(config.single_line_if_else_max_width(), 70);
}

Expand Down
9 changes: 6 additions & 3 deletions src/ignore_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,11 @@ mod test {
use crate::ignore_path::IgnorePathSet;
use std::path::{Path, PathBuf};

let config =
Config::from_toml(r#"ignore = ["foo.rs", "bar_dir/*"]"#, Path::new("")).unwrap();
let config = Config::from_toml(
r#"ignore = ["foo.rs", "bar_dir/*"]"#,
Path::new("./rustfmt.toml"),
)
.unwrap();
let ignore_path_set = IgnorePathSet::from_ignore_list(&config.ignore()).unwrap();

assert!(ignore_path_set.is_match(&FileName::Real(PathBuf::from("src/foo.rs"))));
Expand All @@ -59,7 +62,7 @@ mod test {

let config = Config::from_toml(
r#"ignore = ["foo.rs", "bar_dir/*", "!bar_dir/*/what.rs"]"#,
Path::new(""),
Path::new("./rustfmt.toml"),
)
.unwrap();
let ignore_path_set = IgnorePathSet::from_ignore_list(&config.ignore()).unwrap();
Expand Down
4 changes: 3 additions & 1 deletion src/parse/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,9 @@ mod tests {
}

fn get_ignore_list(config: &str) -> IgnoreList {
Config::from_toml(config, Path::new("")).unwrap().ignore()
Config::from_toml(config, Path::new("./rustfmt.toml"))
.unwrap()
.ignore()
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions tests/config/issue-6302.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
edition = 2019
13 changes: 13 additions & 0 deletions tests/rustfmt/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,3 +219,16 @@ fn rustfmt_generates_no_error_if_failed_format_code_in_doc_comments() {
assert!(stderr.is_empty());
assert!(stdout.is_empty());
}

#[test]
fn rustfmt_error_improvement_regarding_invalid_toml() {
// See also https://github.com/rust-lang/rustfmt/issues/6302
let invalid_toml_config = "tests/config/issue-6302.toml";
let args = ["--config-path", invalid_toml_config];
let (_stdout, stderr) = rustfmt(&args);

let toml_path = Path::new(invalid_toml_config).canonicalize().unwrap();
let expected_error_message = format!("The file `{}` failed to parse", toml_path.display());

assert!(stderr.contains(&expected_error_message));
}

0 comments on commit 0127491

Please sign in to comment.