Merge pull request #1738 from Manishearth/serde

Update serde to 1.0

Cargo.toml

@@ -32,15 +32,15 @@ test = false
 # begin automatic update
 clippy_lints = { version = "0.0.131", path = "clippy_lints" }
 # end automatic update
-cargo_metadata = "0.1.1"
+cargo_metadata = "0.2"
 
 [dev-dependencies]
 compiletest_rs = "0.2.5"
-lazy_static = "0.1.15"
+lazy_static = "0.2"
 regex = "0.2"
-serde_derive = "0.9.1"
+serde_derive = "1.0"
 clippy-mini-macro-test = { version = "0.1", path = "mini-macro" }
-serde = "0.9.1"
+serde = "1.0"
 
 [features]
 debugging = []

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 = []

clippy_lints/src/lib.rs

@@ -44,6 +44,13 @@ extern crate rustc_const_math;
 #[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 }
@@ -124,7 +131,7 @@ pub mod ranges;
 pub mod reference;
 pub mod regex;
 pub mod returns;
-pub mod serde;
+pub mod serde_api;
 pub mod shadow;
 pub mod should_assert_eq;
 pub mod strings;
@@ -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")
         }
     };
 
@@ -202,7 +209,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     );
     // end deprecated lints, do not remove this comment, it’s used in `update_lints`
 
-    reg.register_late_lint_pass(box serde::Serde);
+    reg.register_late_lint_pass(box serde_api::Serde);
     reg.register_early_lint_pass(box utils::internal_lints::Clippy);
     reg.register_late_lint_pass(box utils::internal_lints::LintWithoutLintPass::default());
     reg.register_late_lint_pass(box utils::inspector::Pass);
@@ -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,

clippy_lints/src/misc_early.rs

@@ -4,7 +4,7 @@ use std::char;
 use syntax::ast::*;
 use syntax::codemap::Span;
 use syntax::visit::FnKind;
-use utils::{constants, span_lint, span_help_and_lint, snippet, snippet_opt, span_lint_and_then};
+use utils::{constants, span_lint, span_help_and_lint, snippet, snippet_opt, span_lint_and_then, in_external_macro};
 
 /// **What it does:** Checks for structure field patterns bound to wildcards.
 ///
@@ -267,6 +267,9 @@ impl EarlyLintPass for MiscEarly {
     }
 
     fn check_expr(&mut self, cx: &EarlyContext, expr: &Expr) {
+        if in_external_macro(cx, expr.span) {
+            return;
+        }
         match expr.node {
             ExprKind::Call(ref paren, _) => {
                 if let ExprKind::Paren(ref closure) = paren.node {
@@ -290,7 +293,38 @@ impl EarlyLintPass for MiscEarly {
                               "`--x` could be misinterpreted as pre-decrement by C programmers, is usually a no-op");
                 }
             },
-            ExprKind::Lit(ref lit) => {
+            ExprKind::Lit(ref lit) => self.check_lit(cx, lit),
+            _ => (),
+        }
+    }
+
+    fn check_block(&mut self, cx: &EarlyContext, block: &Block) {
+        for w in block.stmts.windows(2) {
+            if_let_chain! {[
+                let StmtKind::Local(ref local) = w[0].node,
+                let Option::Some(ref t) = local.init,
+                let ExprKind::Closure(_, _, _, _) = t.node,
+                let PatKind::Ident(_, sp_ident, _) = local.pat.node,
+                let StmtKind::Semi(ref second) = w[1].node,
+                let ExprKind::Assign(_, ref call) = second.node,
+                let ExprKind::Call(ref closure, _) = call.node,
+                let ExprKind::Path(_, ref path) = closure.node
+            ], {
+                if sp_ident.node == (&path.segments[0]).identifier {
+                    span_lint(
+                        cx,
+                        REDUNDANT_CLOSURE_CALL,
+                        second.span,
+                        "Closure called just once immediately after it was declared",
+                    );
+                }
+            }}
+        }
+    }
+}
+
+impl MiscEarly {
+    fn check_lit(&self, cx: &EarlyContext, lit: &Lit) {
         if_let_chain! {[
             let LitKind::Int(value, ..) = lit.node,
             let Some(src) = snippet_opt(cx, lit.span),
@@ -361,32 +395,5 @@ impl EarlyLintPass for MiscEarly {
                 prev = ch;
             }
         }}
-            },
-            _ => (),
-        }
-    }
-
-    fn check_block(&mut self, cx: &EarlyContext, block: &Block) {
-        for w in block.stmts.windows(2) {
-            if_let_chain! {[
-                let StmtKind::Local(ref local) = w[0].node,
-                let Option::Some(ref t) = local.init,
-                let ExprKind::Closure(_, _, _, _) = t.node,
-                let PatKind::Ident(_, sp_ident, _) = local.pat.node,
-                let StmtKind::Semi(ref second) = w[1].node,
-                let ExprKind::Assign(_, ref call) = second.node,
-                let ExprKind::Call(ref closure, _) = call.node,
-                let ExprKind::Path(_, ref path) = closure.node
-            ], {
-                if sp_ident.node == (&path.segments[0]).identifier {
-                    span_lint(
-                        cx,
-                        REDUNDANT_CLOSURE_CALL,
-                        second.span,
-                        "Closure called just once immediately after it was declared",
-                    );
-                }
-            }}
-        }
     }
 }

clippy_lints/src/non_expressive_names.rs

@@ -43,7 +43,7 @@ declare_lint! {
 }
 
 pub struct NonExpressiveNames {
-    pub max_single_char_names: u64,
+    pub single_char_binding_names_threshold: u64,
 }
 
 impl LintPass for NonExpressiveNames {
@@ -127,7 +127,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
             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,

clippy_lints/src/utils/conf.rs

@@ -6,6 +6,7 @@ use std::{env, fmt, fs, io, path};
 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<ast::NestedMetaItemKind>])
@@ -34,8 +35,8 @@ pub fn file_from_args(args: &[codemap::Spanned<ast::NestedMetaItemKind>])
 pub enum Error {
     /// An I/O error.
     Io(io::Error),
-    /// The file is not valid TOML.
+    /// Not valid toml or doesn't fit the expected conf format
-    Toml(Vec<toml::ParserError>),
+    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) => {
+            Error::Toml(ref err) => err.fmt(f),
-                let mut first = true;
-                for err in errs {
-                    if !first {
-                        try!(", ".fmt(f));
-                        first = false;
-                    }
-
-                    try!(err.fmt(f));
-                }
-
-                Ok(())
-            },
             Error::Type(key, expected, got) => {
                 write!(f, "`{}` is expected to be a `{}` but is a `{}`", key, expected, got)
             },
@@ -78,55 +67,45 @@ impl From<io::Error> for Error {
     }
 }
 
+lazy_static! {
+    static ref ERRORS: Mutex<Vec<Error>> = Mutex::new(Vec::new());
+}
+
 macro_rules! define_Conf {
-    ($(#[$doc: meta] ($toml_name: tt, $rust_name: ident, $default: expr => $($ty: tt)+),)+) => {
+    ($(#[$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] pub $rust_name: define_Conf!(TY $($ty)+),)+
+                $(#[$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 Default for Conf {
-            fn default() -> Conf {
-                Conf {
-                    $($rust_name: define_Conf!(DEFAULT $($ty)+, $default),)+
-                }
-            }
-        }
-
-        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) => {
+                mod $rust_name {
-                            if let Some(value) = define_Conf!(CONV $($ty)+, value) {
+                    use serde;
-                                self.$rust_name = value;
+                    use serde::Deserialize;
-                            }
+                    pub fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<define_Conf!(TY $($ty)+), D::Error> {
-                            else {
+                        type T = define_Conf!(TY $($ty)+);
-                                return Err(Error::Type(define_Conf!(EXPR $toml_name),
+                        Ok(T::deserialize(deserializer).unwrap_or_else(|e| {
-                                                       stringify!($($ty)+),
+                            ::utils::conf::ERRORS.lock().expect("no threading here").push(::utils::conf::Error::Toml(e.to_string()));
-                                                       value.type_str()));
+                            super::$rust_name()
-                            }
+                        }))
-                        },
-                    )+
-                    "third-party" => {
-                        // for external tools such as clippy-service
-                        return Ok(());
-                    }
-                    _ => {
-                        return Err(Error::UnknownKey(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 @@ macro_rules! define_Conf {
     };
     (CONV String, $value: expr) => { $value.as_str().map(Into::into) };
     (CONV Vec<String>, $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 @@ macro_rules! define_Conf {
 
 define_Conf! {
     /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about
-    ("blacklisted-names", blacklisted_names, ["foo", "bar", "baz", "quux"] => Vec<String>),
+    (blacklisted_names, "blacklisted_names", ["foo", "bar", "baz", "quux"] => Vec<String>),
     /// 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 @@ define_Conf! {
         "MinGW",
     ] => Vec<String>),
     /// 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,21 @@ pub fn lookup_conf_file() -> io::Result<Option<path::PathBuf>> {
     }
 }
 
+/// Produces a `Conf` filled with the default values and forwards the errors
+///
+/// Used internally for convenience
+fn default(errors: Vec<Error>) -> (Conf, Vec<Error>) {
+    (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<Error>) {
-    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 +226,21 @@ pub fn read(path: Option<&path::Path>) -> (Conf, Vec<Error>) {
             let mut buf = String::new();
 
             if let Err(err) = file.read_to_string(&mut buf) {
-                errors.push(err.into());
+                return default(vec![err.into()])
-                return (conf, errors);
             }
 
             buf
         },
-        Err(err) => {
+        Err(err) => return default(vec![err.into()]),
-            errors.push(err.into());
+    };
-            return (conf, errors);
+
+    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)
         },
-    };
-
-    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);
     }
 }
-
-    (conf, errors)
-}

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
 

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
 

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
 

tests/ui/serde.rs

@@ -7,7 +7,7 @@ extern crate serde;
 
 struct A;
 
-impl serde::de::Visitor for A {
+impl<'de> serde::de::Visitor<'de> for A {
     type Value = ();
 
     fn expecting(&self, _: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
@@ -29,7 +29,7 @@ impl serde::de::Visitor for A {
 
 struct B;
 
-impl serde::de::Visitor for B {
+impl<'de> serde::de::Visitor<'de> for B {
     type Value = ();
 
     fn expecting(&self, _: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {