From 14d51659603af7fa034e05eae18ad7e7a1d243e5 Mon Sep 17 00:00:00 2001 From: rufevean Date: Sun, 1 Sep 2024 20:38:03 +0530 Subject: [PATCH] refactor - show file path in error message when parsing config from toml fixes 6302 Display the path to the toml config file that rustfmt failed to parse configurations from. --- src/config/mod.rs | 69 ++++++++++++++++++------------------ src/ignore_path.rs | 9 +++-- src/parse/session.rs | 4 ++- tests/config/issue-6302.toml | 1 + tests/rustfmt/main.rs | 13 +++++++ 5 files changed, 58 insertions(+), 38 deletions(-) create mode 100644 tests/config/issue-6302.toml diff --git a/src/config/mod.rs b/src/config/mod.rs index 80515077abd..8e67d452d43 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -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`. @@ -370,13 +364,13 @@ impl Config { } #[allow(dead_code)] - pub(super) fn from_toml(toml: &str, dir: &Path) -> Result { - Self::from_toml_for_style_edition(toml, dir, None, None, None) + pub(super) fn from_toml(toml: &str, file_path: &Path) -> Result { + 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, style_edition: Option, version: Option, @@ -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) } } } @@ -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); @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); @@ -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); @@ -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); @@ -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()); @@ -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); @@ -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); @@ -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); @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } @@ -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); } diff --git a/src/ignore_path.rs b/src/ignore_path.rs index 5c25f233ce3..2f804ef5a25 100644 --- a/src/ignore_path.rs +++ b/src/ignore_path.rs @@ -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")))); @@ -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(); diff --git a/src/parse/session.rs b/src/parse/session.rs index 4160e02b8f8..9f27a05939e 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -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] diff --git a/tests/config/issue-6302.toml b/tests/config/issue-6302.toml new file mode 100644 index 00000000000..8148b37b1f6 --- /dev/null +++ b/tests/config/issue-6302.toml @@ -0,0 +1 @@ +edition = 2019 diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs index e99890acd1b..a9f58b9328e 100644 --- a/tests/rustfmt/main.rs +++ b/tests/rustfmt/main.rs @@ -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)); +}