mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-10 23:24:24 +00:00
Merge pull request #125 from Manishearth/parent
added parent method, also changed match-if-let note to help
This commit is contained in:
commit
50ebdaa79d
3 changed files with 139 additions and 117 deletions
181
src/misc.rs
181
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<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 }
|
||||
}
|
||||
|
|
71
src/utils.rs
71
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<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);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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),
|
||||
_ => {}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue