added parent method, also changed match-if-let note to help

This commit is contained in:
llogiq 2015-08-11 15:07:21 +02:00
parent 228f06a960
commit f0eb36c2af
3 changed files with 139 additions and 117 deletions

View file

@ -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<Expr>]) -> 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 }
}

View file

@ -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<T> and return a ref on the result
@ -55,13 +66,21 @@ pub fn de_p<T>(p: &P<T>) -> &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);
}
}

View file

@ -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),
_ => {}
}