From 319c66a2a4284c682d9575fe7aad5e4b4bf89365 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Tue, 12 Jul 2016 17:36:11 +0200 Subject: [PATCH] lint on implementing `visit_string` without also implementing `visit_str` --- CHANGELOG.md | 1 + Cargo.toml | 1 + README.md | 3 +- clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/serde.rs | 55 +++++++++++++++++++++++++++++++++++++ tests/compile-fail/serde.rs | 39 ++++++++++++++++++++++++++ 6 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/serde.rs create mode 100644 tests/compile-fail/serde.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index ca3c67e4e8f..e6ceb413169 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -246,6 +246,7 @@ All notable changes to this project will be documented in this file. [`result_unwrap_used`]: https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used [`reverse_range_loop`]: https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop [`search_is_some`]: https://github.com/Manishearth/rust-clippy/wiki#search_is_some +[`serde_api_misuse`]: https://github.com/Manishearth/rust-clippy/wiki#serde_api_misuse [`shadow_reuse`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse [`shadow_same`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_same [`shadow_unrelated`]: https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated diff --git a/Cargo.toml b/Cargo.toml index 79aefdfce61..9790937a307 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ lazy_static = "0.1.15" regex = "0.1.71" rustc-serialize = "0.3" clippy-mini-macro-test = { version = "0.1", path = "mini-macro" } +serde = "0.7" [features] diff --git a/README.md b/README.md index f37d242e434..c62edbe03ae 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Table of contents: ## Lints -There are 158 lints included in this crate: +There are 159 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -136,6 +136,7 @@ name [result_unwrap_used](https://github.com/Manishearth/rust-clippy/wiki#result_unwrap_used) | allow | using `Result.unwrap()`, which might be better handled [reverse_range_loop](https://github.com/Manishearth/rust-clippy/wiki#reverse_range_loop) | warn | Iterating over an empty range, such as `10..0` or `5..5` [search_is_some](https://github.com/Manishearth/rust-clippy/wiki#search_is_some) | warn | using an iterator search followed by `is_some()`, which is more succinctly expressed as a call to `any()` +[serde_api_misuse](https://github.com/Manishearth/rust-clippy/wiki#serde_api_misuse) | warn | Various things that will negatively affect your serde experience [shadow_reuse](https://github.com/Manishearth/rust-clippy/wiki#shadow_reuse) | allow | rebinding a name to an expression that re-uses the original value, e.g. `let x = x + 1` [shadow_same](https://github.com/Manishearth/rust-clippy/wiki#shadow_same) | allow | rebinding a name to itself, e.g. `let mut x = &mut x` [shadow_unrelated](https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated) | allow | The name is re-bound without even using the original value diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index c873ef8cc57..4a8ecdda6f0 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -114,6 +114,7 @@ pub mod ptr_arg; pub mod ranges; pub mod regex; pub mod returns; +pub mod serde; pub mod shadow; pub mod strings; pub mod swap; @@ -167,6 +168,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { store.register_removed("string_to_string", "using `string::to_string` is common even today and specialization will likely happen soon"); // 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 types::TypePass); reg.register_late_lint_pass(box booleans::NonminimalBool); reg.register_late_lint_pass(box misc::TopLevelRefPass); @@ -399,6 +401,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { regex::TRIVIAL_REGEX, returns::LET_AND_RETURN, returns::NEEDLESS_RETURN, + serde::SERDE_API_MISUSE, strings::STRING_LIT_AS_BYTES, swap::ALMOST_SWAPPED, swap::MANUAL_SWAP, diff --git a/clippy_lints/src/serde.rs b/clippy_lints/src/serde.rs new file mode 100644 index 00000000000..c916ad3c514 --- /dev/null +++ b/clippy_lints/src/serde.rs @@ -0,0 +1,55 @@ +use rustc::lint::*; +use rustc::hir::*; +use utils::{span_lint, get_trait_def_id}; + +/// **What it does:** This lint checks for mis-uses of the serde API +/// +/// **Why is this bad?** Serde is very finnicky about how its API should be used, but the type system can't be used to enforce it (yet) +/// +/// **Known problems:** None. +/// +/// **Example:** implementing `Visitor::visit_string` but not `Visitor::visit_str` +declare_lint! { + pub SERDE_API_MISUSE, Warn, + "Various things that will negatively affect your serde experience" +} + + +#[derive(Copy, Clone)] +pub struct Serde; + +impl LintPass for Serde { + fn get_lints(&self) -> LintArray { + lint_array!(SERDE_API_MISUSE) + } +} + +impl LateLintPass for Serde { + fn check_item(&mut self, cx: &LateContext, item: &Item) { + if let ItemImpl(_, _, _, Some(ref trait_ref), _, ref items) = item.node { + let did = cx.tcx.expect_def(trait_ref.ref_id).def_id(); + if let Some(visit_did) = get_trait_def_id(cx, &["serde", "de", "Visitor"]) { + if did == visit_did { + let mut seen_str = None; + let mut seen_string = None; + for item in items { + match &*item.name.as_str() { + "visit_str" => seen_str = Some(item.span), + "visit_string" => seen_string = Some(item.span), + _ => {}, + } + } + if let Some(span) = seen_string { + if seen_str.is_none() { + span_lint(cx, + SERDE_API_MISUSE, + span, + "you should not implement `visit_string` without also implementing `visit_str`", + ); + } + } + } + } + } + } +} diff --git a/tests/compile-fail/serde.rs b/tests/compile-fail/serde.rs new file mode 100644 index 00000000000..d5099edbc0c --- /dev/null +++ b/tests/compile-fail/serde.rs @@ -0,0 +1,39 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![deny(serde_api_misuse)] +#![allow(dead_code)] + +extern crate serde; + +struct A; + +impl serde::de::Visitor for A { + type Value = (); + fn visit_str(&mut self, _v: &str) -> Result + where E: serde::Error, + { + unimplemented!() + } + + fn visit_string(&mut self, _v: String) -> Result + where E: serde::Error, + { + unimplemented!() + } +} + +struct B; + +impl serde::de::Visitor for B { + type Value = (); + + fn visit_string(&mut self, _v: String) -> Result + //~^ ERROR you should not implement `visit_string` without also implementing `visit_str` + where E: serde::Error, + { + unimplemented!() + } +} + +fn main() { +}