From 31b2bf4ab9b3adcc8b7fbb8da30facd0c0be23a8 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 9 May 2017 15:23:38 +0200 Subject: [PATCH] Update our config reading to serde 1.0 --- clippy_lints/Cargo.toml | 5 +- clippy_lints/src/lib.rs | 13 +- clippy_lints/src/non_expressive_names.rs | 4 +- clippy_lints/src/utils/conf.rs | 148 +++++++++-------------- tests/ui/conf_bad_toml.stderr | 2 +- tests/ui/conf_bad_type.stderr | 2 +- tests/ui/conf_unknown_key.stderr | 2 +- 7 files changed, 78 insertions(+), 98 deletions(-) diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index 64bd412f604..ed5b1e61b92 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -19,9 +19,12 @@ keywords = ["clippy", "lint", "plugin"] matches = "0.1.2" regex-syntax = "0.4.0" semver = "0.6.0" -toml = "0.2" +toml = "0.4" unicode-normalization = "0.1" quine-mc_cluskey = "0.2.2" +serde = "1.0" +serde_derive = "1.0" +lazy_static = "0.2.8" [features] debugging = [] diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 79c05b5e01a..fb0114f8f68 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -44,6 +44,13 @@ #[macro_use] extern crate matches as matches_macro; +#[macro_use] +extern crate serde_derive; +extern crate serde; + +#[macro_use] +extern crate lazy_static; + macro_rules! declare_restriction_lint { { pub $name:tt, $description:tt } => { declare_lint! { pub $name, Allow, $description } @@ -175,7 +182,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.sess.struct_span_err(span, err) .span_note(span, "Clippy will use default configuration") .emit(); - utils::conf::Conf::default() + toml::from_str("").expect("we never error on empty config files") } }; @@ -266,7 +273,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box print::Pass); reg.register_late_lint_pass(box vec::Pass); reg.register_early_lint_pass(box non_expressive_names::NonExpressiveNames { - max_single_char_names: conf.max_single_char_names, + single_char_binding_names_threshold: conf.single_char_binding_names_threshold, }); reg.register_late_lint_pass(box drop_forget_ref::Pass); reg.register_late_lint_pass(box empty_enum::EmptyEnum); @@ -488,7 +495,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { regex::TRIVIAL_REGEX, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, - serde::SERDE_API_MISUSE, + serde_api::SERDE_API_MISUSE, should_assert_eq::SHOULD_ASSERT_EQ, strings::STRING_LIT_AS_BYTES, swap::ALMOST_SWAPPED, diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 67f1aaa27ab..6b97ffee6db 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -43,7 +43,7 @@ } pub struct NonExpressiveNames { - pub max_single_char_names: u64, + pub single_char_binding_names_threshold: u64, } impl LintPass for NonExpressiveNames { @@ -127,7 +127,7 @@ fn check_short_name(&mut self, c: char, span: Span) { return; } self.0.single_char_names.push(c); - if self.0.single_char_names.len() as u64 >= self.0.lint.max_single_char_names { + if self.0.single_char_names.len() as u64 >= self.0.lint.single_char_binding_names_threshold { span_lint(self.0.cx, MANY_SINGLE_CHAR_NAMES, span, diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 983a0ae44bf..deef05f4ef9 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -6,6 +6,7 @@ use std::io::Read; use syntax::{ast, codemap}; use toml; +use std::sync::Mutex; /// Get the configuration file from arguments. pub fn file_from_args(args: &[codemap::Spanned]) @@ -34,8 +35,8 @@ pub fn file_from_args(args: &[codemap::Spanned]) pub enum Error { /// An I/O error. Io(io::Error), - /// The file is not valid TOML. - Toml(Vec), + /// Not valid toml or doesn't fit the expected conf format + Toml(String), /// Type error. Type(/// The name of the key. &'static str, @@ -51,19 +52,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { match *self { Error::Io(ref err) => err.fmt(f), - Error::Toml(ref errs) => { - let mut first = true; - for err in errs { - if !first { - try!(", ".fmt(f)); - first = false; - } - - try!(err.fmt(f)); - } - - Ok(()) - }, + Error::Toml(ref err) => err.fmt(f), Error::Type(key, expected, got) => { write!(f, "`{}` is expected to be a `{}` but is a `{}`", key, expected, got) }, @@ -78,55 +67,45 @@ fn from(e: io::Error) -> Self { } } -macro_rules! define_Conf { - ($(#[$doc: meta] ($toml_name: tt, $rust_name: ident, $default: expr => $($ty: tt)+),)+) => { - /// Type used to store lint configuration. - pub struct Conf { - $(#[$doc] pub $rust_name: define_Conf!(TY $($ty)+),)+ - } +lazy_static! { + static ref ERRORS: Mutex> = Mutex::new(Vec::new()); +} - impl Default for Conf { - fn default() -> Conf { - Conf { - $($rust_name: define_Conf!(DEFAULT $($ty)+, $default),)+ - } +macro_rules! define_Conf { + ($(#[$doc: meta] ($rust_name: ident, $rust_name_str: expr, $default: expr => $($ty: tt)+),)+) => { + pub use self::helpers::Conf; + mod helpers { + /// Type used to store lint configuration. + #[derive(Deserialize)] + #[serde(rename_all="kebab-case")] + #[serde(deny_unknown_fields)] + pub struct Conf { + $(#[$doc] #[serde(default=$rust_name_str)] #[serde(with=$rust_name_str)] pub $rust_name: define_Conf!(TY $($ty)+),)+ + #[allow(dead_code)] + #[serde(default)] + third_party: Option<::toml::Value>, } - } - - impl Conf { - /// Set the property `name` (which must be the `toml` name) to the given value - #[allow(cast_sign_loss)] - fn set(&mut self, name: String, value: toml::Value) -> Result<(), Error> { - match name.as_str() { - $( - define_Conf!(PAT $toml_name) => { - if let Some(value) = define_Conf!(CONV $($ty)+, value) { - self.$rust_name = value; - } - else { - return Err(Error::Type(define_Conf!(EXPR $toml_name), - stringify!($($ty)+), - value.type_str())); - } - }, - )+ - "third-party" => { - // for external tools such as clippy-service - return Ok(()); - } - _ => { - return Err(Error::UnknownKey(name)); + $( + mod $rust_name { + use serde; + use serde::Deserialize; + pub fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result { + type T = define_Conf!(TY $($ty)+); + Ok(T::deserialize(deserializer).unwrap_or_else(|e| { + ::utils::conf::ERRORS.lock().expect("no threading here").push(::utils::conf::Error::Toml(e.to_string())); + super::$rust_name() + })) } } - Ok(()) - } + fn $rust_name() -> define_Conf!(TY $($ty)+) { + define_Conf!(DEFAULT $($ty)+, $default) + } + )+ } }; // hack to convert tts - (PAT $pat: pat) => { $pat }; - (EXPR $e: expr) => { $e }; (TY $ty: ty) => { $ty }; // how to read the value? @@ -139,7 +118,7 @@ fn set(&mut self, name: String, value: toml::Value) -> Result<(), Error> { }; (CONV String, $value: expr) => { $value.as_str().map(Into::into) }; (CONV Vec, $value: expr) => {{ - let slice = $value.as_slice(); + let slice = $value.as_array(); if let Some(slice) = slice { if slice.iter().any(|v| v.as_str().is_none()) { @@ -159,11 +138,11 @@ fn set(&mut self, name: String, value: toml::Value) -> Result<(), Error> { define_Conf! { /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about - ("blacklisted-names", blacklisted_names, ["foo", "bar", "baz", "quux"] => Vec), + (blacklisted_names, "blacklisted_names", ["foo", "bar", "baz", "quux"] => Vec), /// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have - ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25 => u64), + (cyclomatic_complexity_threshold, "cyclomatic_complexity_threshold", 25 => u64), /// Lint: DOC_MARKDOWN. The list of words this lint should not consider as identifiers needing ticks - ("doc-valid-idents", doc_valid_idents, [ + (doc_valid_idents, "doc_valid_idents", [ "KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", "ECMAScript", @@ -180,17 +159,17 @@ fn set(&mut self, name: String, value: toml::Value) -> Result<(), Error> { "MinGW", ] => Vec), /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have - ("too-many-arguments-threshold", too_many_arguments_threshold, 7 => u64), + (too_many_arguments_threshold, "too_many_arguments_threshold", 7 => u64), /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have - ("type-complexity-threshold", type_complexity_threshold, 250 => u64), + (type_complexity_threshold, "type_complexity_threshold", 250 => u64), /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have - ("single-char-binding-names-threshold", max_single_char_names, 5 => u64), + (single_char_binding_names_threshold, "single_char_binding_names_threshold", 5 => u64), /// Lint: BOXED_LOCAL. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap - ("too-large-for-stack", too_large_for_stack, 200 => u64), + (too_large_for_stack, "too_large_for_stack", 200 => u64), /// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger - ("enum-variant-name-threshold", enum_variant_name_threshold, 3 => u64), + (enum_variant_name_threshold, "enum_variant_name_threshold", 3 => u64), /// Lint: LARGE_ENUM_VARIANT. The maximum size of a emum's variant to avoid box suggestion - ("enum-variant-size-threshold", enum_variant_size_threshold, 200 => u64), + (enum_variant_size_threshold, "enum_variant_size_threshold", 200 => u64), } /// Search for the configuration file. @@ -225,17 +204,18 @@ pub fn lookup_conf_file() -> io::Result> { } } +fn default(errors: Vec) -> (Conf, Vec) { + (toml::from_str("").expect("we never error on empty config files"), errors) +} + /// Read the `toml` configuration file. /// /// In case of error, the function tries to continue as much as possible. pub fn read(path: Option<&path::Path>) -> (Conf, Vec) { - let mut conf = Conf::default(); - let mut errors = Vec::new(); - let path = if let Some(path) = path { path } else { - return (conf, errors); + return default(Vec::new()) }; let file = match fs::File::open(path) { @@ -243,31 +223,21 @@ pub fn read(path: Option<&path::Path>) -> (Conf, Vec) { let mut buf = String::new(); if let Err(err) = file.read_to_string(&mut buf) { - errors.push(err.into()); - return (conf, errors); + return default(vec![err.into()]) } buf }, - Err(err) => { - errors.push(err.into()); - return (conf, errors); - }, + Err(err) => return default(vec![err.into()]), }; - let mut parser = toml::Parser::new(&file); - let toml = if let Some(toml) = parser.parse() { - toml - } else { - errors.push(Error::Toml(parser.errors)); - return (conf, errors); - }; - - for (key, value) in toml { - if let Err(err) = conf.set(key, value) { - errors.push(err); - } + assert!(ERRORS.lock().expect("no threading -> mutex always safe").is_empty()); + match toml::from_str(&file) { + Ok(toml) => (toml, ERRORS.lock().expect("no threading -> mutex always safe").split_off(0)), + Err(e) => { + let mut errors = ERRORS.lock().expect("no threading -> mutex always safe").split_off(0); + errors.push(Error::Toml(e.to_string())); + default(errors) + }, } - - (conf, errors) } diff --git a/tests/ui/conf_bad_toml.stderr b/tests/ui/conf_bad_toml.stderr index a56cfea266c..8ee392f8924 100644 --- a/tests/ui/conf_bad_toml.stderr +++ b/tests/ui/conf_bad_toml.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file: expected `=`, but found `t` +error: error reading Clippy's configuration file: expected an equals, found an identifier at line 1 error: aborting due to previous error diff --git a/tests/ui/conf_bad_type.stderr b/tests/ui/conf_bad_type.stderr index 015d9ca2a3a..5cb4d05afef 100644 --- a/tests/ui/conf_bad_type.stderr +++ b/tests/ui/conf_bad_type.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file: `blacklisted-names` is expected to be a `Vec < String >` but is a `integer` +error: error reading Clippy's configuration file: invalid type: integer `42`, expected a sequence error: aborting due to previous error diff --git a/tests/ui/conf_unknown_key.stderr b/tests/ui/conf_unknown_key.stderr index 536950fec31..bd16dfd47da 100644 --- a/tests/ui/conf_unknown_key.stderr +++ b/tests/ui/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file: unknown key `foobar` +error: error reading Clippy's configuration file: unknown field `foobar`, expected one of `blacklisted-names`, `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`, `third-party` error: aborting due to previous error -- 2.44.0