From 29c0c2bb09b9b33f1e4e81db0e46bdd089dc2847 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sun, 21 Feb 2016 20:11:32 +0100 Subject: [PATCH 01/11] Start implementing a configuration file --- Cargo.toml | 1 + src/conf.rs | 184 +++++++++++++++++++++++ src/lib.rs | 26 +++- tests/compile-fail/conf_bad_arg.rs | 6 + tests/compile-fail/conf_non_existant.rs | 6 + tests/compile-fail/conf_unknown_key.rs | 6 + tests/compile-fail/conf_unknown_key.toml | 1 + 7 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 src/conf.rs create mode 100644 tests/compile-fail/conf_bad_arg.rs create mode 100644 tests/compile-fail/conf_non_existant.rs create mode 100644 tests/compile-fail/conf_unknown_key.rs create mode 100644 tests/compile-fail/conf_unknown_key.toml diff --git a/Cargo.toml b/Cargo.toml index 853815e049f7..a10164b5fcfb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ plugin = true regex-syntax = "0.2.2" regex_macros = { version = "0.1.28", optional = true } semver = "0.2.1" +toml = "0.1" unicode-normalization = "0.1" [dev-dependencies] diff --git a/src/conf.rs b/src/conf.rs new file mode 100644 index 000000000000..e275bd701bd5 --- /dev/null +++ b/src/conf.rs @@ -0,0 +1,184 @@ +use std::{fmt, fs, io}; +use std::io::Read; +use syntax::{ast, codemap, ptr}; +use syntax::parse::token; +use toml; + +/// Get the configuration file from arguments. +pub fn conf_file(args: &[ptr::P]) -> Result, (&'static str, codemap::Span)> { + for arg in args { + match arg.node { + ast::MetaItemKind::Word(ref name) | ast::MetaItemKind::List(ref name, _) => { + if name == &"conf_file" { + return Err(("`conf_file` must be a named value", arg.span)); + } + } + ast::MetaItemKind::NameValue(ref name, ref value) => { + if name == &"conf_file" { + return if let ast::LitKind::Str(ref file, _) = value.node { + Ok(Some(file.clone())) + } else { + Err(("`conf_file` value must be a string", value.span)) + } + } + } + } + } + + Ok(None) +} + +/// Error from reading a configuration file. +#[derive(Debug)] +pub enum ConfError { + IoError(io::Error), + TomlError(Vec), + TypeError(&'static str, &'static str, &'static str), + UnknownKey(String), +} + +impl fmt::Display for ConfError { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + match *self { + ConfError::IoError(ref err) => { + err.fmt(f) + } + ConfError::TomlError(ref errs) => { + let mut first = true; + for err in errs { + if !first { + try!(", ".fmt(f)); + first = false; + } + + try!(err.fmt(f)); + } + + Ok(()) + } + ConfError::TypeError(ref key, ref expected, ref got) => { + write!(f, "`{}` is expected to be a `{}` but is a `{}`", key, expected, got) + } + ConfError::UnknownKey(ref key) => { + write!(f, "unknown key `{}`", key) + } + } + } +} + +impl From for ConfError { + fn from(e: io::Error) -> Self { + ConfError::IoError(e) + } +} + +macro_rules! define_Conf { + ($(($toml_name: tt, $rust_name: ident, $default: expr, $ty: ident),)+) => { + /// Type used to store lint configuration. + pub struct Conf { + $(pub $rust_name: $ty,)+ + } + + impl Default for Conf { + fn default() -> Conf { + Conf { + $($rust_name: $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<(), ConfError> { + 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(ConfError::TypeError(define_Conf!(EXPR $toml_name), + stringify!($ty), + value.type_str())); + } + }, + )+ + _ => { + return Err(ConfError::UnknownKey(name)); + } + } + + Ok(()) + } + } + }; + + // hack to convert tts + (PAT $pat: pat) => { $pat }; + (EXPR $e: expr) => { $e }; + + // how to read the value? + (CONV i64, $value: expr) => { $value.as_integer() }; + (CONV u64, $value: expr) => { $value.as_integer().iter().filter_map(|&i| if i >= 0 { Some(i as u64) } else { None }).next() }; + (CONV String, $value: expr) => { $value.as_str().map(Into::into) }; + (CONV StringVec, $value: expr) => {{ + let slice = $value.as_slice(); + + if let Some(slice) = slice { + if slice.iter().any(|v| v.as_str().is_none()) { + None + } + else { + Some(slice.iter().map(|v| v.as_str().unwrap_or_else(|| unreachable!()).to_owned()).collect()) + } + } + else { + None + } + }}; +} + +/// To keep the `define_Conf!` macro simple +pub type StringVec = Vec; + +define_Conf! { + ("blacklisted-names", blacklisted_names, vec!["foo".to_owned(), "bar".to_owned(), "baz".to_owned()], StringVec), + ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25, u64), + ("too-many-arguments-threshold", too_many_arguments_threshold, 6, u64), + ("type-complexity-threshold", type_complexity_threshold, 250, u64), +} + +/// Read the `toml` configuration file. The function will ignore “File not found” errors iif +/// `!must_exist`, in which case, it will return the default configuration. +pub fn read_conf(path: &str, must_exist: bool) -> Result { + let mut conf = Conf::default(); + + let file = match fs::File::open(path) { + Ok(mut file) => { + let mut buf = String::new(); + try!(file.read_to_string(&mut buf)); + buf + } + Err(ref err) if !must_exist && err.kind() == io::ErrorKind::NotFound => { + return Ok(conf); + } + Err(err) => { + return Err(err.into()); + } + }; + + let mut parser = toml::Parser::new(&file); + let toml = if let Some(toml) = parser.parse() { + toml + } + else { + return Err(ConfError::TomlError(parser.errors)); + }; + + for (key, value) in toml { + try!(conf.set(key, value)); + } + + Ok(conf) +} diff --git a/src/lib.rs b/src/lib.rs index 4b9d5d8c9c4a..f3c3564796ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,8 @@ extern crate rustc; #[macro_use] extern crate rustc_front; +extern crate toml; + // Only for the compile time checking of paths extern crate core; extern crate collections; @@ -35,6 +37,7 @@ extern crate rustc_plugin; use rustc_plugin::Registry; +mod conf; pub mod consts; #[macro_use] pub mod utils; @@ -107,6 +110,27 @@ mod reexport { #[plugin_registrar] #[cfg_attr(rustfmt, rustfmt_skip)] pub fn plugin_registrar(reg: &mut Registry) { + let conferr = match conf::conf_file(reg.args()) { + Ok(Some(file_name)) => { + conf::read_conf(&file_name, true) + } + Ok(None) => { + conf::read_conf("Clippy.toml", false) + } + Err((err, span)) => { + reg.sess.struct_span_err(span, err).emit(); + return; + } + }; + + let conf = match conferr { + Ok(conf) => conf, + Err(err) => { + reg.sess.struct_err(&format!("error reading Clippy's configuration file: {}", err)).emit(); + return; + } + }; + reg.register_late_lint_pass(box types::TypePass); reg.register_late_lint_pass(box misc::TopLevelRefPass); reg.register_late_lint_pass(box misc::CmpNan); @@ -157,7 +181,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box map_clone::MapClonePass); reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass); reg.register_late_lint_pass(box transmute::UselessTransmute); - reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(25)); + reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold)); reg.register_late_lint_pass(box escape::EscapePass); reg.register_early_lint_pass(box misc_early::MiscEarly); reg.register_late_lint_pass(box misc::UsedUnderscoreBinding); diff --git a/tests/compile-fail/conf_bad_arg.rs b/tests/compile-fail/conf_bad_arg.rs new file mode 100644 index 000000000000..68b902719f6e --- /dev/null +++ b/tests/compile-fail/conf_bad_arg.rs @@ -0,0 +1,6 @@ +// error-pattern: `conf_file` must be a named value + +#![feature(plugin)] +#![plugin(clippy(conf_file))] + +fn main() {} diff --git a/tests/compile-fail/conf_non_existant.rs b/tests/compile-fail/conf_non_existant.rs new file mode 100644 index 000000000000..13ab7f6cebfd --- /dev/null +++ b/tests/compile-fail/conf_non_existant.rs @@ -0,0 +1,6 @@ +// error-pattern: error reading Clippy's configuration file: No such file or directory + +#![feature(plugin)] +#![plugin(clippy(conf_file="./tests/compile-fail/non_existant_conf.toml"))] + +fn main() {} diff --git a/tests/compile-fail/conf_unknown_key.rs b/tests/compile-fail/conf_unknown_key.rs new file mode 100644 index 000000000000..02131d94d527 --- /dev/null +++ b/tests/compile-fail/conf_unknown_key.rs @@ -0,0 +1,6 @@ +// error-pattern: error reading Clippy's configuration file: unknown key `foobar` + +#![feature(plugin)] +#![plugin(clippy(conf_file="./tests/compile-fail/conf_unknown_key.toml"))] + +fn main() {} diff --git a/tests/compile-fail/conf_unknown_key.toml b/tests/compile-fail/conf_unknown_key.toml new file mode 100644 index 000000000000..df298ea78d49 --- /dev/null +++ b/tests/compile-fail/conf_unknown_key.toml @@ -0,0 +1 @@ +foobar = 42 From 1841804d43b1ef3e919a3e9cb7fdb4e166f8fd64 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sun, 21 Feb 2016 21:01:30 +0100 Subject: [PATCH 02/11] Use configuration in the `TYPE_COMPLEXITY` lint --- src/lib.rs | 2 +- src/types.rs | 78 ++++++++++++++++++++++++++++++---------------------- 2 files changed, 46 insertions(+), 34 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f3c3564796ac..4c0a069a0905 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -168,7 +168,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box entry::HashMapLint); reg.register_late_lint_pass(box ranges::StepByZero); reg.register_late_lint_pass(box types::CastPass); - reg.register_late_lint_pass(box types::TypeComplexityPass); + reg.register_late_lint_pass(box types::TypeComplexityPass::new(conf.type_complexity_threshold)); reg.register_late_lint_pass(box matches::MatchPass); reg.register_late_lint_pass(box misc::PatternPass); reg.register_late_lint_pass(box minmax::MinMaxPass); diff --git a/src/types.rs b/src/types.rs index cf90155b1fa1..1aefe5a4d273 100644 --- a/src/types.rs +++ b/src/types.rs @@ -417,7 +417,17 @@ declare_lint! { } #[allow(missing_copy_implementations)] -pub struct TypeComplexityPass; +pub struct TypeComplexityPass { + threshold: u64, +} + +impl TypeComplexityPass { + pub fn new(threshold: u64) -> Self { + TypeComplexityPass { + threshold: threshold + } + } +} impl LintPass for TypeComplexityPass { fn get_lints(&self) -> LintArray { @@ -427,18 +437,18 @@ impl LintPass for TypeComplexityPass { impl LateLintPass for TypeComplexityPass { fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) { - check_fndecl(cx, decl); + self.check_fndecl(cx, decl); } fn check_struct_field(&mut self, cx: &LateContext, field: &StructField) { // enum variants are also struct fields now - check_type(cx, &field.ty); + self.check_type(cx, &field.ty); } fn check_item(&mut self, cx: &LateContext, item: &Item) { match item.node { ItemStatic(ref ty, _, _) | - ItemConst(ref ty, _) => check_type(cx, ty), + ItemConst(ref ty, _) => self.check_type(cx, ty), // functions, enums, structs, impls and traits are covered _ => (), } @@ -447,8 +457,8 @@ impl LateLintPass for TypeComplexityPass { fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) { match item.node { ConstTraitItem(ref ty, _) | - TypeTraitItem(_, Some(ref ty)) => check_type(cx, ty), - MethodTraitItem(MethodSig { ref decl, .. }, None) => check_fndecl(cx, decl), + TypeTraitItem(_, Some(ref ty)) => self.check_type(cx, ty), + MethodTraitItem(MethodSig { ref decl, .. }, None) => self.check_fndecl(cx, decl), // methods with default impl are covered by check_fn _ => (), } @@ -457,7 +467,7 @@ impl LateLintPass for TypeComplexityPass { fn check_impl_item(&mut self, cx: &LateContext, item: &ImplItem) { match item.node { ImplItemKind::Const(ref ty, _) | - ImplItemKind::Type(ref ty) => check_type(cx, ty), + ImplItemKind::Type(ref ty) => self.check_type(cx, ty), // methods are covered by check_fn _ => (), } @@ -465,47 +475,49 @@ impl LateLintPass for TypeComplexityPass { fn check_local(&mut self, cx: &LateContext, local: &Local) { if let Some(ref ty) = local.ty { - check_type(cx, ty); + self.check_type(cx, ty); } } } -fn check_fndecl(cx: &LateContext, decl: &FnDecl) { - for arg in &decl.inputs { - check_type(cx, &arg.ty); +impl TypeComplexityPass { + fn check_fndecl(&self, cx: &LateContext, decl: &FnDecl) { + for arg in &decl.inputs { + self.check_type(cx, &arg.ty); + } + if let Return(ref ty) = decl.output { + self.check_type(cx, ty); + } } - if let Return(ref ty) = decl.output { - check_type(cx, ty); - } -} -fn check_type(cx: &LateContext, ty: &Ty) { - if in_macro(cx, ty.span) { - return; - } - let score = { - let mut visitor = TypeComplexityVisitor { - score: 0, - nest: 1, + fn check_type(&self, cx: &LateContext, ty: &Ty) { + if in_macro(cx, ty.span) { + return; + } + let score = { + let mut visitor = TypeComplexityVisitor { + score: 0, + nest: 1, + }; + visitor.visit_ty(ty); + visitor.score }; - visitor.visit_ty(ty); - visitor.score - }; - if score > 250 { - span_lint(cx, - TYPE_COMPLEXITY, - ty.span, - "very complex type used. Consider factoring parts into `type` definitions"); + if score > self.threshold { + span_lint(cx, + TYPE_COMPLEXITY, + ty.span, + "very complex type used. Consider factoring parts into `type` definitions"); + } } } /// Walks a type and assigns a complexity score to it. struct TypeComplexityVisitor { /// total complexity score of the type - score: u32, + score: u64, /// current nesting level - nest: u32, + nest: u64, } impl<'v> Visitor<'v> for TypeComplexityVisitor { From 232710cd4312abce7476de2baa21b943a9763eb4 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 22 Feb 2016 14:25:51 +0100 Subject: [PATCH 03/11] Add configuration variables to wiki --- src/conf.rs | 32 +++++++++++++++++++------------- src/lib.rs | 1 + util/update_wiki.py | 40 ++++++++++++++++++++++++++++++++-------- 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/conf.rs b/src/conf.rs index e275bd701bd5..3b2a1dafad4b 100644 --- a/src/conf.rs +++ b/src/conf.rs @@ -73,16 +73,16 @@ impl From for ConfError { } macro_rules! define_Conf { - ($(($toml_name: tt, $rust_name: ident, $default: expr, $ty: ident),)+) => { + ($(#[$doc: meta] ($toml_name: tt, $rust_name: ident, $default: expr => $($ty: tt)+),)+) => { /// Type used to store lint configuration. pub struct Conf { - $(pub $rust_name: $ty,)+ + $(#[$doc] pub $rust_name: define_Conf!(TY $($ty)+),)+ } impl Default for Conf { fn default() -> Conf { Conf { - $($rust_name: $default,)+ + $($rust_name: define_Conf!(DEFAULT $($ty)+, $default),)+ } } } @@ -94,12 +94,12 @@ macro_rules! define_Conf { match name.as_str() { $( define_Conf!(PAT $toml_name) => { - if let Some(value) = define_Conf!(CONV $ty, value) { + if let Some(value) = define_Conf!(CONV $($ty)+, value) { self.$rust_name = value; } else { return Err(ConfError::TypeError(define_Conf!(EXPR $toml_name), - stringify!($ty), + stringify!($($ty)+), value.type_str())); } }, @@ -117,12 +117,13 @@ macro_rules! define_Conf { // hack to convert tts (PAT $pat: pat) => { $pat }; (EXPR $e: expr) => { $e }; + (TY $ty: ty) => { $ty }; // how to read the value? (CONV i64, $value: expr) => { $value.as_integer() }; (CONV u64, $value: expr) => { $value.as_integer().iter().filter_map(|&i| if i >= 0 { Some(i as u64) } else { None }).next() }; (CONV String, $value: expr) => { $value.as_str().map(Into::into) }; - (CONV StringVec, $value: expr) => {{ + (CONV Vec, $value: expr) => {{ let slice = $value.as_slice(); if let Some(slice) = slice { @@ -137,16 +138,21 @@ macro_rules! define_Conf { None } }}; + + // provide a nicer syntax to declare the default value of `Vec` variables + (DEFAULT Vec, $e: expr) => { $e.iter().map(|&e| e.to_owned()).collect() }; + (DEFAULT $ty: ty, $e: expr) => { $e }; } -/// To keep the `define_Conf!` macro simple -pub type StringVec = Vec; - define_Conf! { - ("blacklisted-names", blacklisted_names, vec!["foo".to_owned(), "bar".to_owned(), "baz".to_owned()], StringVec), - ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25, u64), - ("too-many-arguments-threshold", too_many_arguments_threshold, 6, u64), - ("type-complexity-threshold", type_complexity_threshold, 250, u64), + /// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about + ("blacklisted-names", blacklisted_names, ["foo", "bar", "baz"] => Vec), + /// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have + ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25 => u64), + /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have + ("too-many-arguments-threshold", too_many_arguments_threshold, 6 => u64), + /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have + ("type-complexity-threshold", type_complexity_threshold, 250 => u64), } /// Read the `toml` configuration file. The function will ignore “File not found” errors iif diff --git a/src/lib.rs b/src/lib.rs index 4c0a069a0905..b69fdf70c998 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +#![feature(type_macros)] #![feature(plugin_registrar, box_syntax)] #![feature(rustc_private, collections)] #![feature(iter_arith)] diff --git a/util/update_wiki.py b/util/update_wiki.py index d3467a410124..a10b3549a225 100755 --- a/util/update_wiki.py +++ b/util/update_wiki.py @@ -9,6 +9,8 @@ import sys level_re = re.compile(r'''(Forbid|Deny|Warn|Allow)''') +conf_re = re.compile(r'''define_Conf! {\n([^}]*)\n}''', re.MULTILINE) +confvar_re = re.compile(r'''/// Lint: (\w+). (.*).*\n *\("([^"]*)", (?:[^,]*), (.*) => (.*)\),''') def parse_path(p="src"): @@ -16,10 +18,23 @@ def parse_path(p="src"): for f in os.listdir(p): if f.endswith(".rs"): parse_file(d, os.path.join(p, f)) - return d + return (d, parse_conf(p)) -START = 0 -LINT = 1 + +def parse_conf(p): + c = {} + with open(p + '/conf.rs') as f: + f = f.read() + + m = re.search(conf_re, f) + m = m.groups()[0] + + m = re.findall(confvar_re, m) + + for (lint, doc, name, default, ty) in m: + c[lint.lower()] = (name, ty, doc, default) + + return c def parse_file(d, f): @@ -85,8 +100,14 @@ template = """\n# `%s` %s""" +conf_template = """ +**Configuration:** This lint has the following configuration variables: -def write_wiki_page(d, f): +* `%s: %s`: %s (defaults to `%s`). +""" + + +def write_wiki_page(d, c, f): keys = list(d.keys()) keys.sort() with open(f, "w") as w: @@ -102,8 +123,11 @@ def write_wiki_page(d, f): for k in keys: w.write(template % (k, d[k][0], "".join(d[k][1]))) + if k in c: + w.write(conf_template % c[k]) -def check_wiki_page(d, f): + +def check_wiki_page(d, c, f): errors = [] with open(f) as w: for line in w: @@ -122,11 +146,11 @@ def check_wiki_page(d, f): def main(): - d = parse_path() + (d, c) = parse_path() if "-c" in sys.argv: - check_wiki_page(d, "../rust-clippy.wiki/Home.md") + check_wiki_page(d, c, "../rust-clippy.wiki/Home.md") else: - write_wiki_page(d, "../rust-clippy.wiki/Home.md") + write_wiki_page(d, c, "../rust-clippy.wiki/Home.md") if __name__ == "__main__": main() From a3031e34f9db46f172d955cec607c6f4ef226ab4 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 22 Feb 2016 15:42:24 +0100 Subject: [PATCH 04/11] Add a `BLACKLISTED_NAME` lint --- README.md | 3 +- src/blacklisted_name.rs | 45 +++++++++++++++++++ src/lib.rs | 3 ++ tests/compile-fail/blacklisted_name.rs | 26 +++++++++++ tests/compile-fail/box_vec.rs | 1 + .../conf_french_blacklisted_name.rs | 26 +++++++++++ .../conf_french_blacklisted_name.toml | 1 + tests/compile-fail/copies.rs | 1 + tests/compile-fail/dlist.rs | 3 +- tests/compile-fail/methods.rs | 2 +- tests/compile-fail/mut_reference.rs | 12 ++--- tests/compile-fail/used_underscore_binding.rs | 2 + 12 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 src/blacklisted_name.rs create mode 100755 tests/compile-fail/blacklisted_name.rs create mode 100755 tests/compile-fail/conf_french_blacklisted_name.rs create mode 100644 tests/compile-fail/conf_french_blacklisted_name.toml diff --git a/README.md b/README.md index 48a106e3591d..1b18a62f7a4a 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A collection of lints to catch common mistakes and improve your Rust code. [Jump to link with clippy-service](#link-with-clippy-service) ##Lints -There are 134 lints included in this crate: +There are 135 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -19,6 +19,7 @@ name [almost_swapped](https://github.com/Manishearth/rust-clippy/wiki#almost_swapped) | warn | `foo = bar; bar = foo` sequence [approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant) | warn | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant [bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask) | warn | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have) +[blacklisted_name](https://github.com/Manishearth/rust-clippy/wiki#blacklisted_name) | warn | usage of a blacklisted/placeholder name [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr) | warn | braces can be eliminated in conditions that are expressions, e.g `if { true } ...` [block_in_if_condition_stmt](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_stmt) | warn | avoid complex blocks in conditions, instead move the block higher and bind it with 'let'; e.g: `if { let x = true; x } ...` [bool_comparison](https://github.com/Manishearth/rust-clippy/wiki#bool_comparison) | warn | comparing a variable to a boolean, e.g. `if x == true` diff --git a/src/blacklisted_name.rs b/src/blacklisted_name.rs new file mode 100644 index 000000000000..2d62cc44d264 --- /dev/null +++ b/src/blacklisted_name.rs @@ -0,0 +1,45 @@ +use rustc::lint::*; +use rustc_front::hir::*; +use utils::span_lint; + +/// **What it does:** This lints about usage of blacklisted names. +/// +/// **Why is this bad?** These names are usually placeholder names and should be avoided. +/// +/// **Known problems:** None. +/// +/// **Example:** `let foo = 3.14;` +declare_lint! { + pub BLACKLISTED_NAME, + Warn, + "usage of a blacklisted/placeholder name" +} + +#[derive(Clone, Debug)] +pub struct BlackListedName { + blacklist: Vec, +} + +impl BlackListedName { + pub fn new(blacklist: Vec) -> BlackListedName { + BlackListedName { + blacklist: blacklist + } + } +} + +impl LintPass for BlackListedName { + fn get_lints(&self) -> LintArray { + lint_array!(BLACKLISTED_NAME) + } +} + +impl LateLintPass for BlackListedName { + fn check_pat(&mut self, cx: &LateContext, pat: &Pat) { + if let PatKind::Ident(_, ref ident, _) = pat.node { + if self.blacklist.iter().any(|s| s == &*ident.node.name.as_str()) { + span_lint(cx, BLACKLISTED_NAME, pat.span, &format!("use of a blacklisted/placeholder name `{}`", ident.node.name)); + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index b69fdf70c998..66ae815d7e00 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -48,6 +48,7 @@ pub mod approx_const; pub mod array_indexing; pub mod attrs; pub mod bit_mask; +pub mod blacklisted_name; pub mod block_in_if_condition; pub mod collapsible_if; pub mod copies; @@ -204,6 +205,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box overflow_check_conditional::OverflowCheckConditional); reg.register_late_lint_pass(box unused_label::UnusedLabel); reg.register_late_lint_pass(box new_without_default::NewWithoutDefault); + reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names)); reg.register_lint_group("clippy_pedantic", vec![ array_indexing::INDEXING_SLICING, @@ -236,6 +238,7 @@ pub fn plugin_registrar(reg: &mut Registry) { attrs::INLINE_ALWAYS, bit_mask::BAD_BIT_MASK, bit_mask::INEFFECTIVE_BIT_MASK, + blacklisted_name::BLACKLISTED_NAME, block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR, block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT, collapsible_if::COLLAPSIBLE_IF, diff --git a/tests/compile-fail/blacklisted_name.rs b/tests/compile-fail/blacklisted_name.rs new file mode 100755 index 000000000000..efcb810a30ee --- /dev/null +++ b/tests/compile-fail/blacklisted_name.rs @@ -0,0 +1,26 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![allow(dead_code)] +#![allow(single_match)] +#![allow(unused_variables)] +#![deny(blacklisted_name)] + +fn test(foo: ()) {} //~ERROR use of a blacklisted/placeholder name `foo` + +fn main() { + let foo = 42; //~ERROR use of a blacklisted/placeholder name `foo` + let bar = 42; //~ERROR use of a blacklisted/placeholder name `bar` + let baz = 42; //~ERROR use of a blacklisted/placeholder name `baz` + + let barb = 42; + let barbaric = 42; + + match (42, Some(1337), Some(0)) { + (foo, Some(bar), baz @ Some(_)) => (), + //~^ ERROR use of a blacklisted/placeholder name `foo` + //~| ERROR use of a blacklisted/placeholder name `bar` + //~| ERROR use of a blacklisted/placeholder name `baz` + _ => (), + } +} diff --git a/tests/compile-fail/box_vec.rs b/tests/compile-fail/box_vec.rs index 044e7dffa794..071945a81b23 100644 --- a/tests/compile-fail/box_vec.rs +++ b/tests/compile-fail/box_vec.rs @@ -3,6 +3,7 @@ #![deny(clippy)] #![allow(boxed_local)] +#![allow(blacklisted_name)] macro_rules! boxit { ($init:expr, $x:ty) => { diff --git a/tests/compile-fail/conf_french_blacklisted_name.rs b/tests/compile-fail/conf_french_blacklisted_name.rs new file mode 100755 index 000000000000..b7e29eeef1f7 --- /dev/null +++ b/tests/compile-fail/conf_french_blacklisted_name.rs @@ -0,0 +1,26 @@ +#![feature(plugin)] +#![plugin(clippy(conf_file="./tests/compile-fail/conf_french_blacklisted_name.toml"))] + +#![allow(dead_code)] +#![allow(single_match)] +#![allow(unused_variables)] +#![deny(blacklisted_name)] + +fn test(toto: ()) {} //~ERROR use of a blacklisted/placeholder name `toto` + +fn main() { + let toto = 42; //~ERROR use of a blacklisted/placeholder name `toto` + let tata = 42; //~ERROR use of a blacklisted/placeholder name `tata` + let titi = 42; //~ERROR use of a blacklisted/placeholder name `titi` + + let tatab = 42; + let tatatataic = 42; + + match (42, Some(1337), Some(0)) { + (toto, Some(tata), titi @ Some(_)) => (), + //~^ ERROR use of a blacklisted/placeholder name `toto` + //~| ERROR use of a blacklisted/placeholder name `tata` + //~| ERROR use of a blacklisted/placeholder name `titi` + _ => (), + } +} diff --git a/tests/compile-fail/conf_french_blacklisted_name.toml b/tests/compile-fail/conf_french_blacklisted_name.toml new file mode 100644 index 000000000000..6abe5a3bbc27 --- /dev/null +++ b/tests/compile-fail/conf_french_blacklisted_name.toml @@ -0,0 +1 @@ +blacklisted-names = ["toto", "tata", "titi"] diff --git a/tests/compile-fail/copies.rs b/tests/compile-fail/copies.rs index c1e1ba68b3e7..66457e77f476 100755 --- a/tests/compile-fail/copies.rs +++ b/tests/compile-fail/copies.rs @@ -6,6 +6,7 @@ #![allow(needless_return)] #![allow(unused_variables)] #![allow(cyclomatic_complexity)] +#![allow(blacklisted_name)] fn bar(_: T) {} fn foo() -> bool { unimplemented!() } diff --git a/tests/compile-fail/dlist.rs b/tests/compile-fail/dlist.rs index a800c045a502..e79196191212 100644 --- a/tests/compile-fail/dlist.rs +++ b/tests/compile-fail/dlist.rs @@ -6,8 +6,7 @@ extern crate collections; use collections::linked_list::LinkedList; -pub fn test(foo: LinkedList) { //~ ERROR I see you're using a LinkedList! - println!("{:?}", foo) +pub fn test(_: LinkedList) { //~ ERROR I see you're using a LinkedList! } fn main(){ diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 46f14d5d921c..344016a3b909 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -2,7 +2,7 @@ #![plugin(clippy)] #![deny(clippy, clippy_pedantic)] -#![allow(unused, print_stdout, non_ascii_literal, new_without_default)] +#![allow(blacklisted_name, unused, print_stdout, non_ascii_literal, new_without_default)] use std::collections::BTreeMap; use std::collections::HashMap; diff --git a/tests/compile-fail/mut_reference.rs b/tests/compile-fail/mut_reference.rs index 1d81ed14e4ed..0bb59a318b83 100644 --- a/tests/compile-fail/mut_reference.rs +++ b/tests/compile-fail/mut_reference.rs @@ -20,8 +20,8 @@ impl MyStruct { fn main() { // Functions takes_an_immutable_reference(&mut 42); //~ERROR The function/method "takes_an_immutable_reference" doesn't need a mutable reference - let foo: fn(&i32) = takes_an_immutable_reference; - foo(&mut 42); //~ERROR The function/method "foo" doesn't need a mutable reference + let as_ptr: fn(&i32) = takes_an_immutable_reference; + as_ptr(&mut 42); //~ERROR The function/method "as_ptr" doesn't need a mutable reference // Methods let my_struct = MyStruct; @@ -32,12 +32,12 @@ fn main() { // Functions takes_an_immutable_reference(&42); - let foo: fn(&i32) = takes_an_immutable_reference; - foo(&42); + let as_ptr: fn(&i32) = takes_an_immutable_reference; + as_ptr(&42); takes_a_mutable_reference(&mut 42); - let foo: fn(&mut i32) = takes_a_mutable_reference; - foo(&mut 42); + let as_ptr: fn(&mut i32) = takes_a_mutable_reference; + as_ptr(&mut 42); let a = &mut 42; takes_an_immutable_reference(a); diff --git a/tests/compile-fail/used_underscore_binding.rs b/tests/compile-fail/used_underscore_binding.rs index 281d92c46df8..6bf4324e6237 100644 --- a/tests/compile-fail/used_underscore_binding.rs +++ b/tests/compile-fail/used_underscore_binding.rs @@ -2,6 +2,8 @@ #![plugin(clippy)] #![deny(clippy)] +#![allow(blacklisted_name)] + /// Test that we lint if we use a binding with a single leading underscore fn prefix_underscore(_foo: u32) -> u32 { _foo + 1 //~ ERROR used binding which is prefixed with an underscore From 578750aae145c59ce7a175c42ee1a6a7a32831e9 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 22 Feb 2016 15:50:40 +0100 Subject: [PATCH 05/11] Document the configuration file --- README.md | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1b18a62f7a4a..97e0a075dd2e 100644 --- a/README.md +++ b/README.md @@ -6,9 +6,12 @@ A collection of lints to catch common mistakes and improve your Rust code. -[Jump to usage instructions](#usage) - -[Jump to link with clippy-service](#link-with-clippy-service) +Table of contents: +* [Lint list](#lints) +* [Usage instructions](#usage) +* [Configuration](#configuration) +* [*clippy-service*](#link-with-clippy-service) +* [License](#license) ##Lints There are 135 lints included in this crate: @@ -231,6 +234,22 @@ And, in your `main.rs` or `lib.rs`: #![cfg_attr(feature="clippy", plugin(clippy))] ``` +## Configuration +Some lints can be configured in a `Clippy.toml` file. It contains basic `variable = value` mapping eg. + +```toml +blacklisted-names = ["toto", "tata", "titi"] +cyclomatic-complexity-threshold = 30 +``` + +See the wiki for more information about which lints can be configured and the +meaning of the variables. + +You can also specify the path to the configuration file with: +```rust +#![plugin(clippy(conf_file="path/to/clippy's/configuration"))] +``` + ##Link with clippy service `clippy-service` is a rust web initiative providing `rust-clippy` as a web service. From c7db94aee6996b0b707cb8c0b912056c1ede6011 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 29 Feb 2016 17:48:20 +0100 Subject: [PATCH 06/11] Rustfmt --- src/blacklisted_name.rs | 9 +++++---- src/conf.rs | 13 ++++--------- src/types.rs | 4 +--- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/blacklisted_name.rs b/src/blacklisted_name.rs index 2d62cc44d264..25c0bac2c374 100644 --- a/src/blacklisted_name.rs +++ b/src/blacklisted_name.rs @@ -22,9 +22,7 @@ pub struct BlackListedName { impl BlackListedName { pub fn new(blacklist: Vec) -> BlackListedName { - BlackListedName { - blacklist: blacklist - } + BlackListedName { blacklist: blacklist } } } @@ -38,7 +36,10 @@ impl LateLintPass for BlackListedName { fn check_pat(&mut self, cx: &LateContext, pat: &Pat) { if let PatKind::Ident(_, ref ident, _) = pat.node { if self.blacklist.iter().any(|s| s == &*ident.node.name.as_str()) { - span_lint(cx, BLACKLISTED_NAME, pat.span, &format!("use of a blacklisted/placeholder name `{}`", ident.node.name)); + span_lint(cx, + BLACKLISTED_NAME, + pat.span, + &format!("use of a blacklisted/placeholder name `{}`", ident.node.name)); } } } diff --git a/src/conf.rs b/src/conf.rs index 3b2a1dafad4b..0ceede3b5b8c 100644 --- a/src/conf.rs +++ b/src/conf.rs @@ -19,7 +19,7 @@ pub fn conf_file(args: &[ptr::P]) -> Result Result<(), fmt::Error> { match *self { - ConfError::IoError(ref err) => { - err.fmt(f) - } + ConfError::IoError(ref err) => err.fmt(f), ConfError::TomlError(ref errs) => { let mut first = true; for err in errs { @@ -59,9 +57,7 @@ impl fmt::Display for ConfError { ConfError::TypeError(ref key, ref expected, ref got) => { write!(f, "`{}` is expected to be a `{}` but is a `{}`", key, expected, got) } - ConfError::UnknownKey(ref key) => { - write!(f, "unknown key `{}`", key) - } + ConfError::UnknownKey(ref key) => write!(f, "unknown key `{}`", key), } } } @@ -177,8 +173,7 @@ pub fn read_conf(path: &str, must_exist: bool) -> Result { let mut parser = toml::Parser::new(&file); let toml = if let Some(toml) = parser.parse() { toml - } - else { + } else { return Err(ConfError::TomlError(parser.errors)); }; diff --git a/src/types.rs b/src/types.rs index 1aefe5a4d273..e64bf0105492 100644 --- a/src/types.rs +++ b/src/types.rs @@ -423,9 +423,7 @@ pub struct TypeComplexityPass { impl TypeComplexityPass { pub fn new(threshold: u64) -> Self { - TypeComplexityPass { - threshold: threshold - } + TypeComplexityPass { threshold: threshold } } } From 403c54ec5bfa6f2d944dff5a1b34fda3344dec66 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sun, 6 Mar 2016 14:40:25 +0100 Subject: [PATCH 07/11] White-list `third-party` in conf files --- src/conf.rs | 4 ++++ tests/compile-fail/conf_unknown_key.toml | 5 +++++ tests/run-pass/conf_unknown_key.rs | 4 ++++ tests/run-pass/conf_unknown_key.toml | 3 +++ 4 files changed, 16 insertions(+) create mode 100644 tests/run-pass/conf_unknown_key.rs create mode 100644 tests/run-pass/conf_unknown_key.toml diff --git a/src/conf.rs b/src/conf.rs index 0ceede3b5b8c..93caa1edca32 100644 --- a/src/conf.rs +++ b/src/conf.rs @@ -100,6 +100,10 @@ macro_rules! define_Conf { } }, )+ + "third-party" => { + // for external tools such as clippy-service + return Ok(()); + } _ => { return Err(ConfError::UnknownKey(name)); } diff --git a/tests/compile-fail/conf_unknown_key.toml b/tests/compile-fail/conf_unknown_key.toml index df298ea78d49..554b87cc50be 100644 --- a/tests/compile-fail/conf_unknown_key.toml +++ b/tests/compile-fail/conf_unknown_key.toml @@ -1 +1,6 @@ +# that one is an error foobar = 42 + +# that one is white-listed +[third-party] +clippy-feature = "nightly" diff --git a/tests/run-pass/conf_unknown_key.rs b/tests/run-pass/conf_unknown_key.rs new file mode 100644 index 000000000000..bb186d476300 --- /dev/null +++ b/tests/run-pass/conf_unknown_key.rs @@ -0,0 +1,4 @@ +#![feature(plugin)] +#![plugin(clippy(conf_file="./tests/run-pass/conf_unknown_key.toml"))] + +fn main() {} diff --git a/tests/run-pass/conf_unknown_key.toml b/tests/run-pass/conf_unknown_key.toml new file mode 100644 index 000000000000..9f87de20baff --- /dev/null +++ b/tests/run-pass/conf_unknown_key.toml @@ -0,0 +1,3 @@ +# this is ignored by Clippy, but allowed for other tools like clippy-service +[third-party] +clippy-feature = "nightly" From d118b27abb50fef41f7a9510aa9e3741ac036370 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sun, 6 Mar 2016 15:17:51 +0100 Subject: [PATCH 08/11] mv src/conf.rs src/utils --- src/lib.rs | 7 +++---- src/{ => utils}/conf.rs | 0 src/utils/mod.rs | 1 + 3 files changed, 4 insertions(+), 4 deletions(-) rename src/{ => utils}/conf.rs (100%) diff --git a/src/lib.rs b/src/lib.rs index 66ae815d7e00..a7fe2c1aab31 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -38,7 +38,6 @@ extern crate rustc_plugin; use rustc_plugin::Registry; -mod conf; pub mod consts; #[macro_use] pub mod utils; @@ -112,12 +111,12 @@ mod reexport { #[plugin_registrar] #[cfg_attr(rustfmt, rustfmt_skip)] pub fn plugin_registrar(reg: &mut Registry) { - let conferr = match conf::conf_file(reg.args()) { + let conferr = match utils::conf::conf_file(reg.args()) { Ok(Some(file_name)) => { - conf::read_conf(&file_name, true) + utils::conf::read_conf(&file_name, true) } Ok(None) => { - conf::read_conf("Clippy.toml", false) + utils::conf::read_conf("Clippy.toml", false) } Err((err, span)) => { reg.sess.struct_span_err(span, err).emit(); diff --git a/src/conf.rs b/src/utils/conf.rs similarity index 100% rename from src/conf.rs rename to src/utils/conf.rs diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 625e8da197d5..feeb70126ea9 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -14,6 +14,7 @@ use syntax::codemap::{ExpnInfo, Span, ExpnFormat}; use syntax::errors::DiagnosticBuilder; use syntax::ptr::P; +pub mod conf; mod hir; pub use self::hir::{SpanlessEq, SpanlessHash}; pub type MethodArgs = HirVec>; From 95e582a338068a840d0d5a9be6ae4b2b9c93c053 Mon Sep 17 00:00:00 2001 From: mcarton Date: Sun, 6 Mar 2016 15:48:56 +0100 Subject: [PATCH 09/11] =?UTF-8?q?Don=E2=80=99t=20make=20conf=20errors=20fa?= =?UTF-8?q?tal=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/lib.rs | 38 ++++++++++++++++++++++---------------- src/utils/conf.rs | 25 ++++++++++++++++++------- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a7fe2c1aab31..1088e4bc139a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -111,24 +111,30 @@ mod reexport { #[plugin_registrar] #[cfg_attr(rustfmt, rustfmt_skip)] pub fn plugin_registrar(reg: &mut Registry) { - let conferr = match utils::conf::conf_file(reg.args()) { - Ok(Some(file_name)) => { - utils::conf::read_conf(&file_name, true) - } - Ok(None) => { - utils::conf::read_conf("Clippy.toml", false) + let conf = match utils::conf::conf_file(reg.args()) { + Ok(file_name) => { + // if the user specified a file, it must exist, otherwise default to `Clippy.toml` but + // do not require the file to exist + let (ref file_name, must_exist) = if let Some(ref file_name) = file_name { + (&**file_name, true) + } else { + ("Clippy.toml", false) + }; + + let (conf, errors) = utils::conf::read_conf(&file_name, must_exist); + + // all conf errors are non-fatal, we just use the default conf in case of error + for error in errors { + reg.sess.struct_err(&format!("error reading Clippy's configuration file: {}", error)).emit(); + } + + conf } Err((err, span)) => { - reg.sess.struct_span_err(span, err).emit(); - return; - } - }; - - let conf = match conferr { - Ok(conf) => conf, - Err(err) => { - reg.sess.struct_err(&format!("error reading Clippy's configuration file: {}", err)).emit(); - return; + reg.sess.struct_span_err(span, err) + .span_note(span, "Clippy will use defaulf configuration") + .emit(); + utils::conf::Conf::default() } }; diff --git a/src/utils/conf.rs b/src/utils/conf.rs index 93caa1edca32..8c36570c8339 100644 --- a/src/utils/conf.rs +++ b/src/utils/conf.rs @@ -157,20 +157,28 @@ define_Conf! { /// Read the `toml` configuration file. The function will ignore “File not found” errors iif /// `!must_exist`, in which case, it will return the default configuration. -pub fn read_conf(path: &str, must_exist: bool) -> Result { +/// In case of error, the function tries to continue as much as possible. +pub fn read_conf(path: &str, must_exist: bool) -> (Conf, Vec) { let mut conf = Conf::default(); + let mut errors = Vec::new(); let file = match fs::File::open(path) { Ok(mut file) => { let mut buf = String::new(); - try!(file.read_to_string(&mut buf)); + + if let Err(err) = file.read_to_string(&mut buf) { + errors.push(err.into()); + return (conf, errors); + } + buf } Err(ref err) if !must_exist && err.kind() == io::ErrorKind::NotFound => { - return Ok(conf); + return (conf, errors); } Err(err) => { - return Err(err.into()); + errors.push(err.into()); + return (conf, errors); } }; @@ -178,12 +186,15 @@ pub fn read_conf(path: &str, must_exist: bool) -> Result { let toml = if let Some(toml) = parser.parse() { toml } else { - return Err(ConfError::TomlError(parser.errors)); + errors.push(ConfError::TomlError(parser.errors)); + return (conf, errors); }; for (key, value) in toml { - try!(conf.set(key, value)); + if let Err(err) = conf.set(key, value) { + errors.push(err); + } } - Ok(conf) + (conf, errors) } From aa4daea3646f1ff34362f6f52a6881fab91190a6 Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 9 Mar 2016 00:48:10 +0100 Subject: [PATCH 10/11] Lint function with too many arguments --- README.md | 3 +- src/functions.rs | 75 +++++++++++++++++++++++++++++++++ src/lib.rs | 3 ++ src/utils/conf.rs | 2 +- tests/compile-fail/functions.rs | 33 +++++++++++++++ 5 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 src/functions.rs create mode 100755 tests/compile-fail/functions.rs diff --git a/README.md b/README.md index 97e0a075dd2e..93edf4a81faa 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Table of contents: * [License](#license) ##Lints -There are 135 lints included in this crate: +There are 136 lints included in this crate: name | default | meaning ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ @@ -130,6 +130,7 @@ name [suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn | suspicious formatting of `*=`, `-=` or `!=` [suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting) | warn | suspicious formatting of `else if` [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries +[too_many_arguments](https://github.com/Manishearth/rust-clippy/wiki#too_many_arguments) | warn | functions with too many arguments [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`. [trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex) | warn | finds trivial regular expressions in `Regex::new(_)` invocations [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions diff --git a/src/functions.rs b/src/functions.rs new file mode 100644 index 000000000000..5ac5aae51a40 --- /dev/null +++ b/src/functions.rs @@ -0,0 +1,75 @@ +use rustc::lint::*; +use rustc_front::hir; +use rustc_front::intravisit; +use syntax::ast; +use syntax::codemap::Span; +use utils::span_lint; + +/// **What it does:** Check for functions with too many parameters. +/// +/// **Why is this bad?** Functions with lots of parameters are considered bad style and reduce +/// readability (“what does the 5th parameter means?”). Consider grouping some parameters into a +/// new type. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// +/// ``` +/// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) { .. } +/// ``` +declare_lint! { + pub TOO_MANY_ARGUMENTS, + Warn, + "functions with too many arguments" +} + +#[derive(Copy,Clone)] +pub struct Functions { + threshold: u64, +} + +impl Functions { + pub fn new(threshold: u64) -> Functions { + Functions { + threshold: threshold + } + } +} + +impl LintPass for Functions { + fn get_lints(&self) -> LintArray { + lint_array!(TOO_MANY_ARGUMENTS) + } +} + +impl LateLintPass for Functions { + fn check_fn(&mut self, cx: &LateContext, _: intravisit::FnKind, decl: &hir::FnDecl, _: &hir::Block, span: Span, nodeid: ast::NodeId) { + use rustc::front::map::Node::*; + + if let Some(NodeItem(ref item)) = cx.tcx.map.find(cx.tcx.map.get_parent_node(nodeid)) { + match item.node { + hir::ItemImpl(_, _, _, Some(_), _, _) | hir::ItemDefaultImpl(..) => return, + _ => (), + } + } + + self.check_arg_number(cx, decl, span); + } + + fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) { + if let hir::MethodTraitItem(ref sig, _) = item.node { + self.check_arg_number(cx, &sig.decl, item.span); + } + } +} + +impl Functions { + fn check_arg_number(&self, cx: &LateContext, decl: &hir::FnDecl, span: Span) { + let args = decl.inputs.len() as u64; + if args > self.threshold { + span_lint(cx, TOO_MANY_ARGUMENTS, span, + &format!("this function has to many arguments ({}/{})", args, self.threshold)); + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 1088e4bc139a..d65294b56b88 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -63,6 +63,7 @@ pub mod escape; pub mod eta_reduction; pub mod format; pub mod formatting; +pub mod functions; pub mod identity_op; pub mod if_not_else; pub mod items_after_statements; @@ -211,6 +212,7 @@ pub fn plugin_registrar(reg: &mut Registry) { reg.register_late_lint_pass(box unused_label::UnusedLabel); reg.register_late_lint_pass(box new_without_default::NewWithoutDefault); reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names)); + reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold)); reg.register_lint_group("clippy_pedantic", vec![ array_indexing::INDEXING_SLICING, @@ -263,6 +265,7 @@ pub fn plugin_registrar(reg: &mut Registry) { format::USELESS_FORMAT, formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, formatting::SUSPICIOUS_ELSE_FORMATTING, + functions::TOO_MANY_ARGUMENTS, identity_op::IDENTITY_OP, if_not_else::IF_NOT_ELSE, items_after_statements::ITEMS_AFTER_STATEMENTS, diff --git a/src/utils/conf.rs b/src/utils/conf.rs index 8c36570c8339..6636e30ab38a 100644 --- a/src/utils/conf.rs +++ b/src/utils/conf.rs @@ -150,7 +150,7 @@ define_Conf! { /// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25 => u64), /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have - ("too-many-arguments-threshold", too_many_arguments_threshold, 6 => 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), } diff --git a/tests/compile-fail/functions.rs b/tests/compile-fail/functions.rs new file mode 100755 index 000000000000..d3d5eee335a0 --- /dev/null +++ b/tests/compile-fail/functions.rs @@ -0,0 +1,33 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![deny(clippy)] +#![allow(dead_code)] + +fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {} + +fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) { + //~^ ERROR: this function has to many arguments (8/7) +} + +trait Foo { + fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool); + fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()); + //~^ ERROR: this function has to many arguments (8/7) +} + +struct Bar; + +impl Bar { + fn good_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {} + fn bad_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} + //~^ ERROR: this function has to many arguments (8/7) +} + +// ok, we don’t want to warn implementations +impl Foo for Bar { + fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {} + fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {} +} + +fn main() {} From 14dcb60bf8c68dda34bb22894534f4d9b83e60df Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 9 Mar 2016 11:48:55 +0100 Subject: [PATCH 11/11] s/Clippy.toml/clippy.toml --- README.md | 2 +- src/lib.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 93edf4a81faa..52223ef097b8 100644 --- a/README.md +++ b/README.md @@ -236,7 +236,7 @@ And, in your `main.rs` or `lib.rs`: ``` ## Configuration -Some lints can be configured in a `Clippy.toml` file. It contains basic `variable = value` mapping eg. +Some lints can be configured in a `clippy.toml` file. It contains basic `variable = value` mapping eg. ```toml blacklisted-names = ["toto", "tata", "titi"] diff --git a/src/lib.rs b/src/lib.rs index d65294b56b88..f5a3598a1ba0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,12 +114,12 @@ mod reexport { pub fn plugin_registrar(reg: &mut Registry) { let conf = match utils::conf::conf_file(reg.args()) { Ok(file_name) => { - // if the user specified a file, it must exist, otherwise default to `Clippy.toml` but + // if the user specified a file, it must exist, otherwise default to `clippy.toml` but // do not require the file to exist let (ref file_name, must_exist) = if let Some(ref file_name) = file_name { (&**file_name, true) } else { - ("Clippy.toml", false) + ("clippy.toml", false) }; let (conf, errors) = utils::conf::read_conf(&file_name, must_exist);