diff --git a/README.md b/README.md index 012b81c1d19e..478bb25272f9 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ A collection of lints that give helpful tips to newbies and catch oversights. ##Lints -There are 48 lints included in this crate: +There are 49 lints included in this crate: name | default | meaning -----------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- @@ -47,6 +47,7 @@ name [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) | warn | The name is re-bound without even using the original value +[should_implement_trait](https://github.com/Manishearth/rust-clippy/wiki#should_implement_trait) | warn | defining a method that should be implementing a std trait [single_match](https://github.com/Manishearth/rust-clippy/wiki#single_match) | warn | a match statement with a single nontrivial arm (i.e, where the other arm is `_ => {}`) is used; recommends `if let` instead [str_to_string](https://github.com/Manishearth/rust-clippy/wiki#str_to_string) | warn | using `to_string()` on a str, which should be `to_owned()` [string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead diff --git a/src/approx_const.rs b/src/approx_const.rs index 0ec2f94cab8a..d307c7dd0563 100644 --- a/src/approx_const.rs +++ b/src/approx_const.rs @@ -3,7 +3,7 @@ use syntax::ast::*; use syntax::codemap::Span; use std::f64::consts as f64; -use utils::span_help_and_lint; +use utils::span_lint; declare_lint! { pub APPROX_CONSTANT, @@ -40,7 +40,7 @@ fn check_lit(cx: &Context, lit: &Lit, span: Span) { match lit.node { LitFloat(ref str, TyF32) => check_known_consts(cx, span, str, "f32"), LitFloat(ref str, TyF64) => check_known_consts(cx, span, str, "f64"), - LitFloatUnsuffixed(ref str) => + LitFloatUnsuffixed(ref str) => check_known_consts(cx, span, str, "f{32, 64}"), _ => () } @@ -50,18 +50,16 @@ fn check_known_consts(cx: &Context, span: Span, str: &str, module: &str) { if let Ok(value) = str.parse::() { for &(constant, name) in KNOWN_CONSTS { if within_epsilon(constant, value) { - span_help_and_lint(cx, APPROX_CONSTANT, span, &format!( + span_lint(cx, APPROX_CONSTANT, span, &format!( "approximate value of `{}::{}` found. \ - Consider using it directly", module, &name), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#approx_constant"); + Consider using it directly", module, &name)); } } } } fn within_epsilon(target: f64, value: f64) -> bool { - f64::abs(value - target) < f64::abs(if target > value { - target + f64::abs(value - target) < f64::abs(if target > value { + target } else { value }) / EPSILON_DIVISOR } diff --git a/src/attrs.rs b/src/attrs.rs index a9ee9402e284..ad021f28a4d6 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -4,7 +4,7 @@ use rustc::lint::*; use syntax::ast::*; use syntax::codemap::ExpnInfo; -use utils::{in_macro, match_path, span_help_and_lint}; +use utils::{in_macro, match_path, span_lint}; declare_lint! { pub INLINE_ALWAYS, Warn, "`#[inline(always)]` is a bad idea in most cases" } @@ -98,12 +98,10 @@ fn check_attrs(cx: &Context, info: Option<&ExpnInfo>, ident: &Ident, if values.len() != 1 || inline != &"inline" { continue; } if let MetaWord(ref always) = values[0].node { if always != &"always" { continue; } - span_help_and_lint(cx, INLINE_ALWAYS, attr.span, &format!( + span_lint(cx, INLINE_ALWAYS, attr.span, &format!( "you have declared `#[inline(always)]` on `{}`. This \ is usually a bad idea. Are you sure?", - ident.name), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#inline_always"); + ident.name)); } } } diff --git a/src/bit_mask.rs b/src/bit_mask.rs index 6817dd3d97b7..465b772da5ce 100644 --- a/src/bit_mask.rs +++ b/src/bit_mask.rs @@ -5,7 +5,7 @@ use syntax::ast::*; use syntax::ast_util::is_comparison_binop; use syntax::codemap::Span; -use utils::span_help_and_lint; +use utils::span_lint; declare_lint! { pub BAD_BIT_MASK, @@ -100,50 +100,36 @@ fn check_bit_mask(cx: &Context, bit_op: BinOp_, cmp_op: BinOp_, BiEq | BiNe => match bit_op { BiBitAnd => if mask_value & cmp_value != mask_value { if cmp_value != 0 { - span_help_and_lint(cx, BAD_BIT_MASK, *span, &format!( + span_lint(cx, BAD_BIT_MASK, *span, &format!( "incompatible bit mask: `_ & {}` can never be equal to `{}`", - mask_value, cmp_value), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + mask_value, cmp_value)); } } else { if mask_value == 0 { - span_help_and_lint(cx, BAD_BIT_MASK, *span, - "&-masking with zero", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); } }, BiBitOr => if mask_value | cmp_value != cmp_value { - span_help_and_lint(cx, BAD_BIT_MASK, *span, &format!( + span_lint(cx, BAD_BIT_MASK, *span, &format!( "incompatible bit mask: `_ | {}` can never be equal to `{}`", - mask_value, cmp_value), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + mask_value, cmp_value)); }, _ => () }, BiLt | BiGe => match bit_op { BiBitAnd => if mask_value < cmp_value { - span_help_and_lint(cx, BAD_BIT_MASK, *span, &format!( + span_lint(cx, BAD_BIT_MASK, *span, &format!( "incompatible bit mask: `_ & {}` will always be lower than `{}`", - mask_value, cmp_value), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + mask_value, cmp_value)); } else { if mask_value == 0 { - span_help_and_lint(cx, BAD_BIT_MASK, *span, - "&-masking with zero", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); } }, BiBitOr => if mask_value >= cmp_value { - span_help_and_lint(cx, BAD_BIT_MASK, *span, &format!( + span_lint(cx, BAD_BIT_MASK, *span, &format!( "incompatible bit mask: `_ | {}` will never be lower than `{}`", - mask_value, cmp_value), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + mask_value, cmp_value)); } else { check_ineffective_lt(cx, *span, mask_value, cmp_value, "|"); }, @@ -153,25 +139,18 @@ fn check_bit_mask(cx: &Context, bit_op: BinOp_, cmp_op: BinOp_, }, BiLe | BiGt => match bit_op { BiBitAnd => if mask_value <= cmp_value { - span_help_and_lint(cx, BAD_BIT_MASK, *span, &format!( + span_lint(cx, BAD_BIT_MASK, *span, &format!( "incompatible bit mask: `_ & {}` will never be higher than `{}`", - mask_value, cmp_value), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + mask_value, cmp_value)); } else { if mask_value == 0 { - span_help_and_lint(cx, BAD_BIT_MASK, *span, - "&-masking with zero", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + span_lint(cx, BAD_BIT_MASK, *span, "&-masking with zero"); } }, BiBitOr => if mask_value > cmp_value { - span_help_and_lint(cx, BAD_BIT_MASK, *span, &format!( + span_lint(cx, BAD_BIT_MASK, *span, &format!( "incompatible bit mask: `_ | {}` will always be higher than `{}`", - mask_value, cmp_value), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#bad_bit_mask"); + mask_value, cmp_value)); } else { check_ineffective_gt(cx, *span, mask_value, cmp_value, "|"); }, @@ -185,21 +164,17 @@ fn check_bit_mask(cx: &Context, bit_op: BinOp_, cmp_op: BinOp_, fn check_ineffective_lt(cx: &Context, span: Span, m: u64, c: u64, op: &str) { if c.is_power_of_two() && m < c { - span_help_and_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!( + span_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!( "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", - op, m, c), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#ineffective_bit_mask"); + op, m, c)); } } fn check_ineffective_gt(cx: &Context, span: Span, m: u64, c: u64, op: &str) { if (c + 1).is_power_of_two() && m <= c { - span_help_and_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!( + span_lint(cx, INEFFECTIVE_BIT_MASK, span, &format!( "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", - op, m, c), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#ineffective_bit_mask"); + op, m, c)); } } diff --git a/src/lib.rs b/src/lib.rs index 33788190fd3f..32fc953d1c32 100755 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,6 +100,7 @@ pub fn plugin_registrar(reg: &mut Registry) { matches::SINGLE_MATCH, methods::OPTION_UNWRAP_USED, methods::RESULT_UNWRAP_USED, + methods::SHOULD_IMPLEMENT_TRAIT, methods::STR_TO_STRING, methods::STRING_TO_STRING, misc::CMP_NAN, diff --git a/src/methods.rs b/src/methods.rs index bfe2f7984a90..50f3512b4f00 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -4,9 +4,12 @@ use rustc::middle::ty; use std::iter; use std::borrow::Cow; -use utils::{snippet, span_lint, match_type, walk_ptrs_ty_depth}; +use utils::{snippet, span_lint, match_path, match_type, walk_ptrs_ty_depth}; use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH}; +use self::SelfKind::*; +use self::OutType::*; + #[derive(Copy,Clone)] pub struct MethodsPass; @@ -18,10 +21,13 @@ declare_lint!(pub STR_TO_STRING, Warn, "using `to_string()` on a str, which should be `to_owned()`"); declare_lint!(pub STRING_TO_STRING, Warn, "calling `String.to_string()` which is a no-op"); +declare_lint!(pub SHOULD_IMPLEMENT_TRAIT, Warn, + "defining a method that should be implementing a std trait"); impl LintPass for MethodsPass { fn get_lints(&self) -> LintArray { - lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING) + lint_array!(OPTION_UNWRAP_USED, RESULT_UNWRAP_USED, STR_TO_STRING, STRING_TO_STRING, + SHOULD_IMPLEMENT_TRAIT) } fn check_expr(&mut self, cx: &Context, expr: &Expr) { @@ -57,4 +63,112 @@ impl LintPass for MethodsPass { } } } + + fn check_item(&mut self, cx: &Context, item: &Item) { + if let ItemImpl(_, _, _, None, _, ref items) = item.node { + for item in items { + let name = item.ident.name; + for &(method_name, n_args, self_kind, out_type, trait_name) in &TRAIT_METHODS { + if_let_chain! { + [ + name == method_name, + let MethodImplItem(ref sig, _) = item.node, + sig.decl.inputs.len() == n_args, + out_type.matches(&sig.decl.output), + self_kind.matches(&sig.explicit_self.node) + ], { + span_lint(cx, SHOULD_IMPLEMENT_TRAIT, item.span, &format!( + "defining a method called `{}` on this type; consider implementing \ + the `{}` trait or choosing a less ambiguous name", name, trait_name)); + } + } + } + } + } + } +} + +const TRAIT_METHODS: [(&'static str, usize, SelfKind, OutType, &'static str); 30] = [ + ("add", 2, ValueSelf, AnyType, "std::ops::Add`"), + ("sub", 2, ValueSelf, AnyType, "std::ops::Sub"), + ("mul", 2, ValueSelf, AnyType, "std::ops::Mul"), + ("div", 2, ValueSelf, AnyType, "std::ops::Div"), + ("rem", 2, ValueSelf, AnyType, "std::ops::Rem"), + ("shl", 2, ValueSelf, AnyType, "std::ops::Shl"), + ("shr", 2, ValueSelf, AnyType, "std::ops::Shr"), + ("bitand", 2, ValueSelf, AnyType, "std::ops::BitAnd"), + ("bitor", 2, ValueSelf, AnyType, "std::ops::BitOr"), + ("bitxor", 2, ValueSelf, AnyType, "std::ops::BitXor"), + ("neg", 1, ValueSelf, AnyType, "std::ops::Neg"), + ("not", 1, ValueSelf, AnyType, "std::ops::Not"), + ("drop", 1, RefMutSelf, UnitType, "std::ops::Drop"), + ("index", 2, RefSelf, RefType, "std::ops::Index"), + ("index_mut", 2, RefMutSelf, RefType, "std::ops::IndexMut"), + ("deref", 1, RefSelf, RefType, "std::ops::Deref"), + ("deref_mut", 1, RefMutSelf, RefType, "std::ops::DerefMut"), + ("clone", 1, RefSelf, AnyType, "std::clone::Clone"), + ("borrow", 1, RefSelf, RefType, "std::borrow::Borrow"), + ("borrow_mut", 1, RefMutSelf, RefType, "std::borrow::BorrowMut"), + ("as_ref", 1, RefSelf, RefType, "std::convert::AsRef"), + ("as_mut", 1, RefMutSelf, RefType, "std::convert::AsMut"), + ("eq", 2, RefSelf, BoolType, "std::cmp::PartialEq"), + ("cmp", 2, RefSelf, AnyType, "std::cmp::Ord"), + ("default", 0, NoSelf, AnyType, "std::default::Default"), + ("hash", 2, RefSelf, UnitType, "std::hash::Hash"), + ("next", 1, RefMutSelf, AnyType, "std::iter::Iterator"), + ("into_iter", 1, ValueSelf, AnyType, "std::iter::IntoIterator"), + ("from_iter", 1, NoSelf, AnyType, "std::iter::FromIterator"), + ("from_str", 1, NoSelf, AnyType, "std::str::FromStr"), +]; + +#[derive(Clone, Copy)] +enum SelfKind { + ValueSelf, + RefSelf, + RefMutSelf, + NoSelf +} + +impl SelfKind { + fn matches(&self, slf: &ExplicitSelf_) -> bool { + match (self, slf) { + (&ValueSelf, &SelfValue(_)) => true, + (&RefSelf, &SelfRegion(_, Mutability::MutImmutable, _)) => true, + (&RefMutSelf, &SelfRegion(_, Mutability::MutMutable, _)) => true, + (&NoSelf, &SelfStatic) => true, + _ => false + } + } +} + +#[derive(Clone, Copy)] +enum OutType { + UnitType, + BoolType, + AnyType, + RefType, +} + +impl OutType { + fn matches(&self, ty: &FunctionRetTy) -> bool { + match (self, ty) { + (&UnitType, &DefaultReturn(_)) => true, + (&UnitType, &Return(ref ty)) if ty.node == TyTup(vec![]) => true, + (&BoolType, &Return(ref ty)) if is_bool(ty) => true, + (&AnyType, &Return(ref ty)) if ty.node != TyTup(vec![]) => true, + (&RefType, &Return(ref ty)) => { + if let TyRptr(_, _) = ty.node { true } else { false } + } + _ => false + } + } +} + +fn is_bool(ty: &Ty) -> bool { + if let TyPath(None, ref p) = ty.node { + if match_path(p, &["bool"]) { + return true; + } + } + false } diff --git a/src/misc.rs b/src/misc.rs index 9692d464ea53..6e4384072169 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -6,7 +6,7 @@ use syntax::codemap::{Span, Spanned}; use syntax::visit::FnKind; use rustc::middle::ty; -use utils::{match_path, snippet, span_lint, span_help_and_lint, walk_ptrs_ty}; +use utils::{match_path, snippet, span_lint, walk_ptrs_ty}; use consts::constant; declare_lint!(pub TOPLEVEL_REF_ARG, Warn, @@ -65,10 +65,8 @@ impl LintPass for CmpNan { fn check_nan(cx: &Context, path: &Path, span: Span) { path.segments.last().map(|seg| if seg.identifier.name == "NAN" { - span_help_and_lint(cx, CMP_NAN, span, - "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cmp_nan"); + span_lint(cx, CMP_NAN, span, + "doomed comparison with NAN, use `std::{f32,f64}::is_nan()` instead"); }); } @@ -126,11 +124,9 @@ impl LintPass for Precedence { fn check_expr(&mut self, cx: &Context, expr: &Expr) { if let ExprBinary(Spanned { node: op, ..}, ref left, ref right) = expr.node { if is_bit_op(op) && (is_arith_expr(left) || is_arith_expr(right)) { - span_help_and_lint(cx, PRECEDENCE, expr.span, + span_lint(cx, PRECEDENCE, expr.span, "operator precedence can trip the unwary. Consider adding parentheses \ - to the subexpression", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#precedence"); + to the subexpression"); } } } diff --git a/src/shadow.rs b/src/shadow.rs index 717c06a4c14e..eba7a9da1875 100644 --- a/src/shadow.rs +++ b/src/shadow.rs @@ -6,7 +6,7 @@ use syntax::visit::FnKind; use rustc::lint::{Context, LintArray, LintPass}; use rustc::middle::def::Def::{DefVariant, DefStruct}; -use utils::{in_external_macro, snippet, span_help_and_lint}; +use utils::{in_external_macro, snippet, span_lint}; declare_lint!(pub SHADOW_SAME, Allow, "rebinding a name to itself, e.g. `let mut x = &mut x`"); @@ -72,7 +72,7 @@ fn is_binding(cx: &Context, pat: &Pat) -> bool { } } -fn check_pat(cx: &Context, pat: &Pat, init: &Option, span: Span, +fn check_pat(cx: &Context, pat: &Pat, init: &Option, span: Span, bindings: &mut Vec) where T: Deref { //TODO: match more stuff / destructuring match pat.node { @@ -94,7 +94,7 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option, span: Span, PatBox(ref inner) => { if let Some(ref initp) = *init { match initp.node { - ExprBox(_, ref inner_init) => + ExprBox(_, ref inner_init) => check_pat(cx, inner, &Some(&**inner_init), span, bindings), //TODO: ExprCall on Box::new _ => check_pat(cx, inner, init, span, bindings), @@ -110,38 +110,30 @@ fn check_pat(cx: &Context, pat: &Pat, init: &Option, span: Span, } } -fn lint_shadow(cx: &Context, name: Name, span: Span, lspan: Span, init: +fn lint_shadow(cx: &Context, name: Name, span: Span, lspan: Span, init: &Option) where T: Deref { if let &Some(ref expr) = init { if is_self_shadow(name, expr) { - span_help_and_lint(cx, SHADOW_SAME, span, &format!( + span_lint(cx, SHADOW_SAME, span, &format!( "{} is shadowed by itself in {}", snippet(cx, lspan, "_"), - snippet(cx, expr.span, "..")), - "for further information see \ - https://github.com/Manishearth/rust-clippy/wiki#shadow_same"); + snippet(cx, expr.span, ".."))); } else { if contains_self(name, expr) { - span_help_and_lint(cx, SHADOW_REUSE, span, &format!( + span_lint(cx, SHADOW_REUSE, span, &format!( "{} is shadowed by {} which reuses the original value", snippet(cx, lspan, "_"), - snippet(cx, expr.span, "..")), - "for further information see https://\ - github.com/Manishearth/rust-clippy/wiki#shadow_reuse"); + snippet(cx, expr.span, ".."))); } else { - span_help_and_lint(cx, SHADOW_UNRELATED, span, &format!( + span_lint(cx, SHADOW_UNRELATED, span, &format!( "{} is shadowed by {} in this declaration", snippet(cx, lspan, "_"), - snippet(cx, expr.span, "..")), - "for further information see https://github.com\ - /Manishearth/rust-clippy/wiki#shadow_unrelated"); + snippet(cx, expr.span, ".."))); } } } else { - span_help_and_lint(cx, SHADOW_UNRELATED, span, &format!( - "{} is shadowed in this declaration", snippet(cx, lspan, "_")), - "for further information see \ - https://github.com/Manishearth/rust-clippy/wiki#shadow_unrelated"); + span_lint(cx, SHADOW_UNRELATED, span, &format!( + "{} is shadowed in this declaration", snippet(cx, lspan, "_"))); } } @@ -218,7 +210,7 @@ fn is_self_shadow(name: Name, expr: &Expr) -> bool { } fn path_eq_name(name: Name, path: &Path) -> bool { - !path.global && path.segments.len() == 1 && + !path.global && path.segments.len() == 1 && path.segments[0].identifier.name == name } @@ -242,8 +234,8 @@ fn contains_self(name: Name, expr: &Expr) -> bool { otherwise.as_ref().map_or(false, |ref e| contains_self(name, e)), ExprWhile(ref e, ref block, _) => contains_self(name, e) || contains_block_self(name, block), - ExprMatch(ref e, ref arms, _) => - arms.iter().any(|ref arm| arm.pats.iter().any(|ref pat| + ExprMatch(ref e, ref arms, _) => + arms.iter().any(|ref arm| arm.pats.iter().any(|ref pat| contains_pat_self(name, pat))) || contains_self(name, e), ExprPath(_, ref path) => path_eq_name(name, path), _ => false @@ -274,7 +266,7 @@ fn contains_pat_self(name: Name, pat: &Pat) -> bool { match pat.node { PatIdent(_, ref ident, ref inner) => name == ident.node.name || inner.as_ref().map_or(false, |ref p| contains_pat_self(name, p)), - PatEnum(_, ref opats) => opats.as_ref().map_or(false, + PatEnum(_, ref opats) => opats.as_ref().map_or(false, |pats| pats.iter().any(|p| contains_pat_self(name, p))), PatQPath(_, ref path) => path_eq_name(name, path), PatStruct(_, ref fieldpats, _) => fieldpats.iter().any( @@ -282,10 +274,10 @@ fn contains_pat_self(name: Name, pat: &Pat) -> bool { PatTup(ref ps) => ps.iter().any(|ref p| contains_pat_self(name, p)), PatBox(ref p) | PatRegion(ref p, _) => contains_pat_self(name, p), - PatRange(ref from, ref until) => + PatRange(ref from, ref until) => contains_self(name, from) || contains_self(name, until), PatVec(ref pre, ref opt, ref post) => - pre.iter().any(|ref p| contains_pat_self(name, p)) || + pre.iter().any(|ref p| contains_pat_self(name, p)) || opt.as_ref().map_or(false, |ref p| contains_pat_self(name, p)) || post.iter().any(|ref p| contains_pat_self(name, p)), _ => false, diff --git a/src/strings.rs b/src/strings.rs index 8e10cfaa72cf..fc8a2d238bb3 100644 --- a/src/strings.rs +++ b/src/strings.rs @@ -8,7 +8,7 @@ use syntax::ast::*; use syntax::codemap::Spanned; use eq_op::is_exp_equal; -use utils::{match_type, span_help_and_lint, walk_ptrs_ty, get_parent_expr}; +use utils::{match_type, span_lint, walk_ptrs_ty, get_parent_expr}; use utils::STRING_PATH; declare_lint! { @@ -45,19 +45,15 @@ impl LintPass for StringAdd { } } } - span_help_and_lint(cx, STRING_ADD, e.span, + span_lint(cx, STRING_ADD, e.span, "you added something to a string. \ - Consider using `String::push_str()` instead", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#string_add") + Consider using `String::push_str()` instead") } } else if let &ExprAssign(ref target, ref src) = &e.node { if is_string(cx, target) && is_add(cx, src, target) { - span_help_and_lint(cx, STRING_ADD_ASSIGN, e.span, + span_lint(cx, STRING_ADD_ASSIGN, e.span, "you assigned the result of adding something to this string. \ - Consider using `String::push_str()` instead", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#string_add_assign") + Consider using `String::push_str()` instead") } } } diff --git a/src/types.rs b/src/types.rs index 12f31ae89ddb..3c4d1441f177 100644 --- a/src/types.rs +++ b/src/types.rs @@ -31,17 +31,14 @@ impl LintPass for TypePass { span_help_and_lint( cx, BOX_VEC, ast_ty.span, "you seem to be trying to use `Box>`. Did you mean to use `Vec`?", - "`Vec` is already on the heap, `Box>` makes an extra allocation. \ - for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#box_vec"); + "`Vec` is already on the heap, `Box>` makes an extra allocation."); } } else if match_type(cx, ty, &LL_PATH) { span_help_and_lint( cx, LINKEDLIST, ast_ty.span, "I see you're using a LinkedList! Perhaps you meant some other data structure?", - "a RingBuf might work; for further information see \ - https://github.com/Manishearth/rust-clippy/wiki#ineffective_bit_mask"); + "a RingBuf might work"); } } } @@ -141,15 +138,12 @@ fn span_precision_loss_lint(cx: &Context, expr: &Expr, cast_from: &ty::TyS, cast let from_nbits_str = if arch_dependent {"64".to_owned()} else if is_isize_or_usize(cast_from) {"32 or 64".to_owned()} else {int_ty_to_nbits(cast_from).to_string()}; - span_help_and_lint(cx, CAST_PRECISION_LOSS, expr.span, + span_lint(cx, CAST_PRECISION_LOSS, expr.span, &format!("casting {0} to {1} causes a loss of precision {2}\ ({0} is {3} bits wide, but {1}'s mantissa is only {4} bits wide)", cast_from, if cast_to_f64 {"f64"} else {"f32"}, if arch_dependent {arch_dependent_str} else {""}, - from_nbits_str, - mantissa_nbits), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_precision_loss"); + from_nbits_str, mantissa_nbits)); } enum ArchSuffix { @@ -183,26 +177,22 @@ fn check_truncation_and_wrapping(cx: &Context, expr: &Expr, cast_from: &ty::TyS, ), }; if span_truncation { - span_help_and_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, + span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &format!("casting {} to {} may truncate the value{}", cast_from, cast_to, match suffix_truncation { ArchSuffix::_32 => arch_32_suffix, ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "" }), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_possible_truncation"); + ArchSuffix::None => "" })); } if span_wrap { - span_help_and_lint(cx, CAST_POSSIBLE_WRAP, expr.span, + span_lint(cx, CAST_POSSIBLE_WRAP, expr.span, &format!("casting {} to {} may wrap around the value{}", cast_from, cast_to, match suffix_wrap { ArchSuffix::_32 => arch_32_suffix, ArchSuffix::_64 => arch_64_suffix, - ArchSuffix::None => "" }), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_possible_wrap"); + ArchSuffix::None => "" })); } } @@ -227,37 +217,29 @@ impl LintPass for CastPass { } }, (false, true) => { - span_help_and_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, - &format!("casting {} to {} may truncate the value", - cast_from, cast_to), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_possible_truncation"); + span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, + &format!("casting {} to {} may truncate the value", + cast_from, cast_to)); if !cast_to.is_signed() { - span_help_and_lint(cx, CAST_SIGN_LOSS, expr.span, - &format!("casting {} to {} may lose the sign of the value", - cast_from, cast_to), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_sign_loss"); + span_lint(cx, CAST_SIGN_LOSS, expr.span, + &format!("casting {} to {} may lose the sign of the value", + cast_from, cast_to)); } }, (true, true) => { if cast_from.is_signed() && !cast_to.is_signed() { - span_help_and_lint(cx, CAST_SIGN_LOSS, expr.span, - &format!("casting {} to {} may lose the sign of the value", - cast_from, cast_to), - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_sign_loss"); + span_lint(cx, CAST_SIGN_LOSS, expr.span, + &format!("casting {} to {} may lose the sign of the value", + cast_from, cast_to)); } check_truncation_and_wrapping(cx, expr, cast_from, cast_to); } (false, false) => { if let (&ty::TyFloat(ast::TyF64), &ty::TyFloat(ast::TyF32)) = (&cast_from.sty, &cast_to.sty) { - span_help_and_lint(cx, CAST_POSSIBLE_TRUNCATION, - expr.span, - "casting f64 to f32 may truncate the value", - "for further information see https://github.com/\ - Manishearth/rust-clippy/wiki#cast_possible_truncation"); + span_lint(cx, CAST_POSSIBLE_TRUNCATION, + expr.span, + "casting f64 to f32 may truncate the value"); } } } diff --git a/src/utils.rs b/src/utils.rs index ece1eee9050b..69f5e22c48fe 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -141,6 +141,11 @@ pub fn get_parent_expr<'c>(cx: &'c Context, e: &Expr) -> Option<&'c Expr> { #[cfg(not(feature="structured_logging"))] pub fn span_lint(cx: &Context, lint: &'static Lint, sp: Span, msg: &str) { cx.span_lint(lint, sp, msg); + if cx.current_level(lint) != Level::Allow { + cx.sess().fileline_help(sp, &format!("for further information visit \ + https://github.com/Manishearth/rust-clippy/wiki#{}", + lint.name_lower())) + } } #[cfg(feature="structured_logging")] @@ -149,13 +154,20 @@ pub fn span_lint(cx: &Context, lint: &'static Lint, sp: Span, msg: &str) { // cx.sess().codemap() has all these nice functions for line/column/snippet details // http://doc.rust-lang.org/syntax/codemap/struct.CodeMap.html#method.span_to_string cx.span_lint(lint, sp, msg); + if cx.current_level(lint) != Level::Allow { + cx.sess().fileline_help(sp, &format!("for further information visit \ + https://github.com/Manishearth/rust-clippy/wiki#{}", + lint.name_lower())) + } } pub fn span_help_and_lint(cx: &Context, lint: &'static Lint, span: Span, msg: &str, help: &str) { span_lint(cx, lint, span, msg); if cx.current_level(lint) != Level::Allow { - cx.sess().fileline_help(span, help); + cx.sess().fileline_help(span, &format!("{}\nfor further information \ + visit https://github.com/Manishearth/rust-clippy/wiki#{}", + help, lint.name_lower())) } } diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index 811c44ef85cb..05f77c1511e8 100755 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -1,8 +1,27 @@ #![feature(plugin)] #![plugin(clippy)] -#[deny(option_unwrap_used, result_unwrap_used)] -#[deny(str_to_string, string_to_string)] +#![allow(unused)] +#![deny(clippy)] + +use std::ops::Mul; + +struct T; + +impl T { + fn add(self, other: T) -> T { self } //~ERROR defining a method called `add` + fn drop(&mut self) { } //~ERROR defining a method called `drop` + + fn sub(&self, other: T) -> &T { self } // no error, self is a ref + fn div(self) -> T { self } // no error, different #arguments + fn rem(self, other: T) { } // no error, wrong return type +} + +impl Mul for T { + type Output = T; + fn mul(self, other: T) -> T { self } // no error, obviously +} + fn main() { let opt = Some(0); let _ = opt.unwrap(); //~ERROR used unwrap() on an Option