Improve "unknown field" error messages

smoelius · smoelius · commit 564725775bd2 · 2022-05-18T05:21:10.000-04:00
diff --git a/Cargo.toml b/Cargo.toml
@@ -25,6 +25,7 @@ clippy_lints = { path = "clippy_lints" }
 semver = "1.0"
 rustc_tools_util = { path = "rustc_tools_util" }
 tempfile = { version = "3.2", optional = true }
+termize = "0.1"
 
 [dev-dependencies]
 compiletest_rs = { version = "0.7.1", features = ["tmp"] }
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -418,7 +418,7 @@ mod zero_sized_map_values;
 // end lints modules, do not remove this comment, it’s used in `update_lints`
 
 pub use crate::utils::conf::Conf;
-use crate::utils::conf::TryConf;
+use crate::utils::conf::{format_error, TryConf};
 
 /// Register all pre expansion lints
 ///
@@ -463,7 +463,7 @@ pub fn read_conf(sess: &Session) -> Conf {
         sess.struct_err(&format!(
             "error reading Clippy's configuration file `{}`: {}",
             file_name.display(),
-            error
+            format_error(error)
         ))
         .emit();
     }
diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -6,7 +6,8 @@ use serde::de::{Deserializer, IgnoredAny, IntoDeserializer, MapAccess, Visitor};
 use serde::Deserialize;
 use std::error::Error;
 use std::path::{Path, PathBuf};
-use std::{env, fmt, fs, io};
+use std::str::FromStr;
+use std::{cmp, env, fmt, fs, io, iter};
 
 /// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint.
 #[derive(Clone, Debug, Deserialize)]
@@ -43,18 +44,33 @@ pub enum DisallowedType {
 #[derive(Default)]
 pub struct TryConf {
     pub conf: Conf,
-    pub errors: Vec<String>,
+    pub errors: Vec<Box<dyn Error>>,
 }
 
 impl TryConf {
-    fn from_error(error: impl Error) -> Self {
+    fn from_error(error: impl Error + 'static) -> Self {
         Self {
             conf: Conf::default(),
-            errors: vec![error.to_string()],
+            errors: vec![Box::new(error)],
         }
     }
 }
 
+#[derive(Debug)]
+struct ConfError(String);
+
+impl fmt::Display for ConfError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        <String as fmt::Display>::fmt(&self.0, f)
+    }
+}
+
+impl Error for ConfError {}
+
+fn conf_error(s: String) -> Box<dyn Error> {
+    Box::new(ConfError(s))
+}
+
 macro_rules! define_Conf {
     ($(
         $(#[doc = $doc:literal])+
@@ -103,11 +119,11 @@ macro_rules! define_Conf {
                 while let Some(name) = map.next_key::<&str>()? {
                     match Field::deserialize(name.into_deserializer())? {
                         $(Field::$name => {
-                            $(errors.push(format!("deprecated field `{}`. {}", name, $dep));)?
+                            $(errors.push(conf_error(format!("deprecated field `{}`. {}", name, $dep)));)?
                             match map.next_value() {
-                                Err(e) => errors.push(e.to_string()),
+                                Err(e) => errors.push(conf_error(e.to_string())),
                                 Ok(value) => match $name {
-                                    Some(_) => errors.push(format!("duplicate field `{}`", name)),
+                                    Some(_) => errors.push(conf_error(format!("duplicate field `{}`", name))),
                                     None => $name = Some(value),
                                 }
                             }
@@ -383,3 +399,102 @@ pub fn read(path: &Path) -> TryConf {
     };
     toml::from_str(&content).unwrap_or_else(TryConf::from_error)
 }
+
+const SEPARATOR_WIDTH: usize = 4;
+
+// Check whether the error is "unknown field" and, if so, list the available fields sorted and at
+// least one per line, more if `CLIPPY_TERMINAL_WIDTH` is set and allows it.
+pub fn format_error(error: Box<dyn Error>) -> String {
+    let s = error.to_string();
+
+    if_chain! {
+        if error.downcast::<toml::de::Error>().is_ok();
+        if let Some((prefix, mut fields, suffix)) = parse_unknown_field_message(&s);
+        then {
+            use fmt::Write;
+
+            fields.sort_unstable();
+
+            let (rows, column_widths) = calculate_dimensions(&fields);
+
+            let mut msg = String::from(prefix);
+            for row in 0..rows {
+                write!(msg, "\n").unwrap();
+                for (column, column_width) in column_widths.iter().copied().enumerate() {
+                    let index = column * rows + row;
+                    let field = fields.get(index).copied().unwrap_or_default();
+                    write!(
+                        msg,
+                        "{:separator_width$}{:field_width$}",
+                        " ",
+                        field,
+                        separator_width = SEPARATOR_WIDTH,
+                        field_width = column_width
+                    )
+                    .unwrap();
+                }
+            }
+            write!(msg, "\n{}", suffix).unwrap();
+            msg
+        } else {
+            s
+        }
+    }
+}
+
+// `parse_unknown_field_message` will become unnecessary if
+// https://github.com/alexcrichton/toml-rs/pull/364 is merged.
+fn parse_unknown_field_message(s: &str) -> Option<(&str, Vec<&str>, &str)> {
+    // An "unknown field" message has the following form:
+    //   unknown field `UNKNOWN`, expected one of `FIELD0`, `FIELD1`, ..., `FIELDN` at line X column Y
+    //                                           ^^      ^^^^                     ^^
+    if_chain! {
+        if s.starts_with("unknown field");
+        let slices = s.split("`, `").collect::<Vec<_>>();
+        let n = slices.len();
+        if n >= 2;
+        if let Some((prefix, first_field)) = slices[0].rsplit_once(" `");
+        if let Some((last_field, suffix)) = slices[n - 1].split_once("` ");
+        then {
+            let fields = iter::once(first_field)
+                .chain(slices[1..n - 1].iter().copied())
+                .chain(iter::once(last_field))
+                .collect::<Vec<_>>();
+            Some((prefix, fields, suffix))
+        } else {
+            None
+        }
+    }
+}
+
+fn calculate_dimensions(fields: &[&str]) -> (usize, Vec<usize>) {
+    let columns = env::var("CLIPPY_TERMINAL_WIDTH")
+        .ok()
+        .and_then(|s| <usize as FromStr>::from_str(&s).ok())
+        .map_or(1, |terminal_width| {
+            let max_field_width = fields.iter().map(|field| field.len()).max().unwrap();
+            cmp::max(1, terminal_width / (SEPARATOR_WIDTH + max_field_width))
+        });
+
+    let rows = (fields.len() + (columns - 1)) / columns;
+
+    let column_widths = (0..columns)
+        .map(|column| {
+            if column < columns - 1 {
+                (0..rows)
+                    .map(|row| {
+                        let index = column * rows + row;
+                        let field = fields.get(index).copied().unwrap_or_default();
+                        field.len()
+                    })
+                    .max()
+                    .unwrap()
+            } else {
+                // Avoid adding extra space to the last column.
+                0
+            }
+        })
+        .collect::<Vec<_>>();
+
+    (rows, column_widths)
+}
diff --git a/src/main.rs b/src/main.rs
@@ -123,8 +123,12 @@ impl ClippyCmd {
             .map(|arg| format!("{}__CLIPPY_HACKERY__", arg))
             .collect();
 
+        // Currently, `CLIPPY_TERMINAL_WIDTH` is used only to format "unknown field" error messages.
+        let terminal_width = termize::dimensions().map_or(0, |(w, _)| w);
+
         cmd.env("RUSTC_WORKSPACE_WRAPPER", Self::path())
             .env("CLIPPY_ARGS", clippy_args)
+            .env("CLIPPY_TERMINAL_WIDTH", terminal_width.to_string())
             .arg(self.cargo_subcommand)
             .args(&self.args);
 
diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -1,4 +1,43 @@
-error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `max-suggested-slice-pattern-length`, `await-holding-invalid-types`, `max-include-file-size`, `allow-expect-in-tests`, `allow-unwrap-in-tests`, `third-party` at line 5 column 1
+error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of
+           allow-expect-in-tests
+           allow-unwrap-in-tests
+           allowed-scripts
+           array-size-threshold
+           avoid-breaking-exported-api
+           await-holding-invalid-types
+           blacklisted-names
+           cargo-ignore-publish
+           cognitive-complexity-threshold
+           cyclomatic-complexity-threshold
+           disallowed-methods
+           disallowed-types
+           doc-valid-idents
+           enable-raw-pointer-heuristic-for-send
+           enforced-import-renames
+           enum-variant-name-threshold
+           enum-variant-size-threshold
+           literal-representation-threshold
+           max-fn-params-bools
+           max-include-file-size
+           max-struct-bools
+           max-suggested-slice-pattern-length
+           max-trait-bounds
+           msrv
+           pass-by-value-size-limit
+           single-char-binding-names-threshold
+           standard-macro-braces
+           third-party
+           too-large-for-stack
+           too-many-arguments-threshold
+           too-many-lines-threshold
+           trivial-copy-size-limit
+           type-complexity-threshold
+           unreadable-literal-lint-fractions
+           upper-case-acronyms-aggressive
+           vec-box-size-threshold
+           verbose-bit-mask-threshold
+           warn-on-all-wildcard-imports
+       at line 5 column 1
 
 error: aborting due to previous error
 
