diff --git a/src/misc.rs b/src/misc.rs index 73b94875b..0f0405a83 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -7,14 +7,13 @@ use rustc::lint::{Context, LintPass, LintArray, Lint, Level}; use rustc::middle::ty; use syntax::codemap::{Span, Spanned}; -use types::span_note_and_lint; -use utils::{match_path, snippet, span_lint}; +use utils::{match_path, snippet, span_lint, span_help_and_lint}; pub fn walk_ty<'t>(ty: ty::Ty<'t>) -> ty::Ty<'t> { - match ty.sty { - ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => walk_ty(tm.ty), - _ => ty - } + match ty.sty { + ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => walk_ty(tm.ty), + _ => ty + } } /// Handles uncategorized lints @@ -42,9 +41,12 @@ impl LintPass for MiscPass { } // In some cases, an exhaustive match is preferred to catch situations when // an enum is extended. So we only consider cases where a `_` wildcard is used - if arms[1].pats[0].node == PatWild(PatWildSingle) && arms[0].pats.len() == 1 { - span_note_and_lint(cx, SINGLE_MATCH, expr.span, - "You seem to be trying to use match for destructuring a single type. Did you mean to use `if let`?", + if arms[1].pats[0].node == PatWild(PatWildSingle) && + arms[0].pats.len() == 1 { + span_help_and_lint(cx, SINGLE_MATCH, expr.span, + "You seem to be trying to use match for \ + destructuring a single type. Did you mean to \ + use `if let`?", &*format!("Try if let {} = {} {{ ... }}", snippet(cx, arms[0].pats[0].span, ".."), snippet(cx, ex.span, "..")) @@ -79,9 +81,9 @@ impl LintPass for StrToStringPass { fn is_str(cx: &Context, expr: &ast::Expr) -> bool { match walk_ty(cx.tcx.expr_ty(expr)).sty { - ty::TyStr => true, - _ => false - } + ty::TyStr => true, + _ => false + } } } } @@ -116,123 +118,124 @@ declare_lint!(pub CMP_NAN, Deny, "Deny comparisons to std::f32::NAN or std::f64: pub struct CmpNan; impl LintPass for CmpNan { - fn get_lints(&self) -> LintArray { + fn get_lints(&self) -> LintArray { lint_array!(CMP_NAN) - } - - fn check_expr(&mut self, cx: &Context, expr: &Expr) { - if let ExprBinary(ref cmp, ref left, ref right) = expr.node { - if is_comparison_binop(cmp.node) { - if let &ExprPath(_, ref path) = &left.node { - check_nan(cx, path, expr.span); - } - if let &ExprPath(_, ref path) = &right.node { - check_nan(cx, path, expr.span); - } - } - } - } + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let ExprBinary(ref cmp, ref left, ref right) = expr.node { + if is_comparison_binop(cmp.node) { + if let &ExprPath(_, ref path) = &left.node { + check_nan(cx, path, expr.span); + } + if let &ExprPath(_, ref path) = &right.node { + check_nan(cx, path, expr.span); + } + } + } + } } fn check_nan(cx: &Context, path: &Path, span: Span) { path.segments.last().map(|seg| if seg.identifier.name == "NAN" { - span_lint(cx, CMP_NAN, span, "Doomed comparison with NAN, use std::{f32,f64}::is_nan instead"); + span_lint(cx, CMP_NAN, span, + "Doomed comparison with NAN, use std::{f32,f64}::is_nan instead"); }); } declare_lint!(pub FLOAT_CMP, Warn, - "Warn on ==/!= comparison of floaty values"); - + "Warn on ==/!= comparison of floaty values"); + #[derive(Copy,Clone)] pub struct FloatCmp; impl LintPass for FloatCmp { - fn get_lints(&self) -> LintArray { + fn get_lints(&self) -> LintArray { lint_array!(FLOAT_CMP) - } - - fn check_expr(&mut self, cx: &Context, expr: &Expr) { - if let ExprBinary(ref cmp, ref left, ref right) = expr.node { - let op = cmp.node; - if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) { - span_lint(cx, FLOAT_CMP, expr.span, &format!( - "{}-Comparison of f32 or f64 detected. You may want to change this to 'abs({} - {}) < epsilon' for some suitable value of epsilon", - binop_to_string(op), snippet(cx, left.span, ".."), - snippet(cx, right.span, ".."))); - } - } - } + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let ExprBinary(ref cmp, ref left, ref right) = expr.node { + let op = cmp.node; + if (op == BiEq || op == BiNe) && (is_float(cx, left) || is_float(cx, right)) { + span_lint(cx, FLOAT_CMP, expr.span, &format!( + "{}-Comparison of f32 or f64 detected. You may want to change this to 'abs({} - {}) < epsilon' for some suitable value of epsilon", + binop_to_string(op), snippet(cx, left.span, ".."), + snippet(cx, right.span, ".."))); + } + } + } } fn is_float(cx: &Context, expr: &Expr) -> bool { - if let ty::TyFloat(_) = walk_ty(cx.tcx.expr_ty(expr)).sty { - true - } else { - false - } + if let ty::TyFloat(_) = walk_ty(cx.tcx.expr_ty(expr)).sty { + true + } else { + false + } } declare_lint!(pub PRECEDENCE, Warn, - "Warn on mixing bit ops with integer arithmetic without parenthesis"); - + "Warn on mixing bit ops with integer arithmetic without parenthesis"); + #[derive(Copy,Clone)] pub struct Precedence; impl LintPass for Precedence { - fn get_lints(&self) -> LintArray { + fn get_lints(&self) -> LintArray { lint_array!(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_lint(cx, PRECEDENCE, expr.span, - "Operator precedence can trip the unwary. Consider adding parenthesis to the subexpression."); - } - } - } + } + + 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_lint(cx, PRECEDENCE, expr.span, + "Operator precedence can trip the unwary. Consider adding parenthesis to the subexpression."); + } + } + } } fn is_arith_expr(expr : &Expr) -> bool { - match expr.node { - ExprBinary(Spanned { node: op, ..}, _, _) => is_arith_op(op), - _ => false - } + match expr.node { + ExprBinary(Spanned { node: op, ..}, _, _) => is_arith_op(op), + _ => false + } } fn is_bit_op(op : BinOp_) -> bool { - match op { - BiBitXor | BiBitAnd | BiBitOr | BiShl | BiShr => true, - _ => false - } + match op { + BiBitXor | BiBitAnd | BiBitOr | BiShl | BiShr => true, + _ => false + } } fn is_arith_op(op : BinOp_) -> bool { - match op { - BiAdd | BiSub | BiMul | BiDiv | BiRem => true, - _ => false - } + match op { + BiAdd | BiSub | BiMul | BiDiv | BiRem => true, + _ => false + } } declare_lint!(pub CMP_OWNED, Warn, - "Warn on creating an owned string just for comparison"); - + "Warn on creating an owned string just for comparison"); + #[derive(Copy,Clone)] pub struct CmpOwned; impl LintPass for CmpOwned { - fn get_lints(&self) -> LintArray { + fn get_lints(&self) -> LintArray { lint_array!(CMP_OWNED) - } - - fn check_expr(&mut self, cx: &Context, expr: &Expr) { - if let ExprBinary(ref cmp, ref left, ref right) = expr.node { - if is_comparison_binop(cmp.node) { - check_to_owned(cx, left, right.span); - check_to_owned(cx, right, left.span) - } - } - } + } + + fn check_expr(&mut self, cx: &Context, expr: &Expr) { + if let ExprBinary(ref cmp, ref left, ref right) = expr.node { + if is_comparison_binop(cmp.node) { + check_to_owned(cx, left, right.span); + check_to_owned(cx, right, left.span) + } + } + } } fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) { @@ -263,6 +266,6 @@ fn check_to_owned(cx: &Context, expr: &Expr, other_span: Span) { } fn is_str_arg(cx: &Context, args: &[P]) -> bool { - args.len() == 1 && if let ty::TyStr = - walk_ty(cx.tcx.expr_ty(&*args[0])).sty { true } else { false } + args.len() == 1 && if let ty::TyStr = + walk_ty(cx.tcx.expr_ty(&*args[0])).sty { true } else { false } } diff --git a/src/utils.rs b/src/utils.rs index 4a87f4b3a..a231171ae 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,7 +1,8 @@ -use rustc::lint::{Context, Lint}; -use syntax::ast::{DefId, Name, Path}; +use rustc::lint::{Context, Lint, Level}; +use syntax::ast::{DefId, Expr, Name, NodeId, Path}; use syntax::codemap::{ExpnInfo, Span}; use syntax::ptr::P; +use rustc::ast_map::Node::NodeExpr; use rustc::middle::ty; use std::borrow::{Cow, IntoCow}; use std::convert::From; @@ -9,45 +10,55 @@ use std::convert::From; /// returns true if the macro that expanded the crate was outside of /// the current crate or was a compiler plugin pub fn in_macro(cx: &Context, opt_info: Option<&ExpnInfo>) -> bool { - // no ExpnInfo = no macro - opt_info.map_or(false, |info| { - // no span for the callee = external macro - info.callee.span.map_or(true, |span| { - // no snippet = external macro or compiler-builtin expansion - cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| - // macro doesn't start with "macro_rules" - // = compiler plugin - !code.starts_with("macro_rules") - ) - }) - }) + // no ExpnInfo = no macro + opt_info.map_or(false, |info| { + // no span for the callee = external macro + info.callee.span.map_or(true, |span| { + // no snippet = external macro or compiler-builtin expansion + cx.sess().codemap().span_to_snippet(span).ok().map_or(true, |code| + // macro doesn't start with "macro_rules" + // = compiler plugin + !code.starts_with("macro_rules") + ) + }) + }) } /// invokes in_macro with the expansion info of the given span pub fn in_external_macro(cx: &Context, span: Span) -> bool { - cx.sess().codemap().with_expn_info(span.expn_id, - |info| in_macro(cx, info)) + cx.sess().codemap().with_expn_info(span.expn_id, + |info| in_macro(cx, info)) } /// check if a DefId's path matches the given absolute type path /// usage e.g. with /// `match_def_path(cx, id, &["core", "option", "Option"])` pub fn match_def_path(cx: &Context, def_id: DefId, path: &[&str]) -> bool { - cx.tcx.with_path(def_id, |iter| iter.map(|elem| elem.name()) - .zip(path.iter()).all(|(nm, p)| &nm.as_str() == p)) + cx.tcx.with_path(def_id, |iter| iter.map(|elem| elem.name()) + .zip(path.iter()).all(|(nm, p)| &nm.as_str() == p)) } /// match a Path against a slice of segment string literals, e.g. /// `match_path(path, &["std", "rt", "begin_unwind"])` pub fn match_path(path: &Path, segments: &[&str]) -> bool { - path.segments.iter().rev().zip(segments.iter().rev()).all( - |(a,b)| &a.identifier.name.as_str() == b) + path.segments.iter().rev().zip(segments.iter().rev()).all( + |(a,b)| &a.identifier.name.as_str() == b) } /// convert a span to a code snippet if available, otherwise use default, e.g. /// `snippet(cx, expr.span, "..")` pub fn snippet<'a>(cx: &Context, span: Span, default: &'a str) -> Cow<'a, str> { - cx.sess().codemap().span_to_snippet(span).map(From::from).unwrap_or(Cow::Borrowed(default)) + cx.sess().codemap().span_to_snippet(span).map(From::from).unwrap_or(Cow::Borrowed(default)) +} + +/// get a parent expr if any – this is useful to constrain a lint +pub fn get_parent_expr<'c>(cx: &'c Context, e: &Expr) -> Option<&'c Expr> { + let map = &cx.tcx.map; + let node_id : NodeId = e.id; + let parent_id : NodeId = map.get_parent_node(node_id); + if node_id == parent_id { return None; } + map.find(parent_id).and_then(|node| + if let NodeExpr(parent) = node { Some(parent) } else { None } ) } /// dereference a P and return a ref on the result @@ -55,13 +66,21 @@ pub fn de_p(p: &P) -> &T { &*p } #[cfg(not(feature="structured_logging"))] pub fn span_lint(cx: &Context, lint: &'static Lint, sp: Span, msg: &str) { - cx.span_lint(lint, sp, msg); + cx.span_lint(lint, sp, msg); } #[cfg(feature="structured_logging")] pub fn span_lint(cx: &Context, lint: &'static Lint, sp: Span, msg: &str) { - // lint.name / lint.desc is can give details of the lint - // 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); + // lint.name / lint.desc is can give details of the lint + // 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); +} + +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().span_help(span, help); + } } diff --git a/tests/compile-fail/match_if_let.rs b/tests/compile-fail/match_if_let.rs index b03c6e114..f1864f9d7 100644 --- a/tests/compile-fail/match_if_let.rs +++ b/tests/compile-fail/match_if_let.rs @@ -6,7 +6,7 @@ fn main(){ let x = Some(1u8); match x { //~ ERROR You seem to be trying to use match - //~^ NOTE Try if let Some(y) = x { ... } + //~^ HELP Try if let Some(y) = x { ... } Some(y) => println!("{:?}", y), _ => () } @@ -17,7 +17,7 @@ fn main(){ } let z = (1u8,1u8); match z { //~ ERROR You seem to be trying to use match - //~^ NOTE Try if let (2...3, 7...9) = z { ... } + //~^ HELP Try if let (2...3, 7...9) = z { ... } (2...3, 7...9) => println!("{:?}", z), _ => {} }