First pass at linting for .any expressed as a .fold
This commit is contained in:
parent
b863a00a4e
commit
f6e56d2559
|
@ -12,7 +12,7 @@ use syntax::ast;
|
|||
use syntax::codemap::Span;
|
||||
use utils::{get_trait_def_id, implements_trait, in_external_macro, in_macro, is_copy, is_self, is_self_ty,
|
||||
iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method,
|
||||
match_type, method_chain_args, return_ty, same_tys, single_segment_path, snippet, span_lint,
|
||||
match_type, method_chain_args, return_ty, remove_blocks, same_tys, single_segment_path, snippet, span_lint,
|
||||
span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth};
|
||||
use utils::paths;
|
||||
use utils::sugg;
|
||||
|
@ -623,6 +623,23 @@ declare_lint! {
|
|||
"using `as_ref` where the types before and after the call are the same"
|
||||
}
|
||||
|
||||
|
||||
/// **What it does:** Checks for using `fold` to implement `any`.
|
||||
///
|
||||
/// **Why is this bad?** Readability.
|
||||
///
|
||||
/// **Known problems:** Changes semantics - the suggested replacement is short-circuiting.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// let _ = (0..3).fold(false, |acc, x| acc || x > 2);
|
||||
/// ```
|
||||
declare_lint! {
|
||||
pub FOLD_ANY,
|
||||
Warn,
|
||||
"TODO"
|
||||
}
|
||||
|
||||
impl LintPass for Pass {
|
||||
fn get_lints(&self) -> LintArray {
|
||||
lint_array!(
|
||||
|
@ -653,7 +670,8 @@ impl LintPass for Pass {
|
|||
GET_UNWRAP,
|
||||
STRING_EXTEND_CHARS,
|
||||
ITER_CLONED_COLLECT,
|
||||
USELESS_ASREF
|
||||
USELESS_ASREF,
|
||||
FOLD_ANY
|
||||
)
|
||||
}
|
||||
}
|
||||
|
@ -717,6 +735,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
|
|||
lint_asref(cx, expr, "as_ref", arglists[0]);
|
||||
} else if let Some(arglists) = method_chain_args(expr, &["as_mut"]) {
|
||||
lint_asref(cx, expr, "as_mut", arglists[0]);
|
||||
} else if let Some(arglists) = method_chain_args(expr, &["fold"]) {
|
||||
lint_fold_any(cx, expr, arglists[0]);
|
||||
}
|
||||
|
||||
lint_or_fun_call(cx, expr, &method_call.name.as_str(), args);
|
||||
|
@ -1105,6 +1125,70 @@ fn lint_iter_cloned_collect(cx: &LateContext, expr: &hir::Expr, iter_args: &[hir
|
|||
}
|
||||
}
|
||||
|
||||
// DONOTMERGE: copy-pasted from map_clone
|
||||
fn get_arg_name(pat: &hir::Pat) -> Option<ast::Name> {
|
||||
match pat.node {
|
||||
hir::PatKind::Binding(_, _, name, None) => Some(name.node),
|
||||
hir::PatKind::Ref(ref subpat, _) => get_arg_name(subpat),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn lint_fold_any(cx: &LateContext, expr: &hir::Expr, fold_args: &[hir::Expr]) {
|
||||
// DONOTMERGE: What if this is just some other method called fold?
|
||||
assert!(fold_args.len() == 3,
|
||||
"Expected fold_args to have three entries - the receiver, the initial value and the closure");
|
||||
|
||||
if let hir::ExprLit(ref lit) = fold_args[1].node {
|
||||
if let ast::LitKind::Bool(ref b) = lit.node {
|
||||
let initial_value = b.to_string();
|
||||
|
||||
if let hir::ExprClosure(_, ref decl, body_id, _, _) = fold_args[2].node {
|
||||
let closure_body = cx.tcx.hir.body(body_id);
|
||||
let closure_expr = remove_blocks(&closure_body.value);
|
||||
|
||||
let first_arg = &closure_body.arguments[0];
|
||||
let arg_ident = get_arg_name(&first_arg.pat).unwrap();
|
||||
|
||||
let second_arg = &closure_body.arguments[1];
|
||||
let second_arg_ident = get_arg_name(&second_arg.pat).unwrap();
|
||||
|
||||
if let hir::ExprBinary(ref bin_op, ref left_expr, ref right_expr) = closure_expr.node {
|
||||
if bin_op.node != hir::BinOp_::BiOr {
|
||||
return;
|
||||
}
|
||||
if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = left_expr.node {
|
||||
if path.segments.len() == 1 {
|
||||
let left_name = &path.segments[0].name;
|
||||
let right_source = cx.sess().codemap().span_to_snippet(right_expr.span).unwrap();
|
||||
|
||||
if left_name == &arg_ident {
|
||||
span_lint(
|
||||
cx,
|
||||
FOLD_ANY,
|
||||
expr.span,
|
||||
// TODO: don't suggest .any(|x| f(x)) if we can suggest .any(f)
|
||||
// TODO: these have difference semantics - original code might be deliberately avoiding short-circuiting
|
||||
&format!(
|
||||
".fold(false, |{f}, {s}| {f} || {r})) is more succinctly expressed as .any(|{s}| {r})",
|
||||
f = arg_ident,
|
||||
s = second_arg_ident,
|
||||
r = right_source
|
||||
),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} else{
|
||||
panic!("DONOTMERGE: can this happen?");
|
||||
}
|
||||
}
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
fn lint_iter_nth(cx: &LateContext, expr: &hir::Expr, iter_args: &[hir::Expr], is_mut: bool) {
|
||||
let mut_str = if is_mut { "_mut" } else { "" };
|
||||
let caller_type = if derefs_to_slice(cx, &iter_args[0], cx.tables.expr_ty(&iter_args[0])).is_some() {
|
||||
|
|
|
@ -385,6 +385,11 @@ fn iter_skip_next() {
|
|||
let _ = foo.filter().skip(42).next();
|
||||
}
|
||||
|
||||
/// Checks implementation of the `FOLD_ANY` lint
|
||||
fn fold_any() {
|
||||
let _ = (0..3).fold(false, |acc, x| acc || x > 2);
|
||||
}
|
||||
|
||||
#[allow(similar_names)]
|
||||
fn main() {
|
||||
let opt = Some(0);
|
||||
|
|
|
@ -493,10 +493,18 @@ error: called `skip(x).next()` on an iterator. This is more succinctly expressed
|
|||
382 | let _ = &some_vec[..].iter().skip(3).next();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
|
||||
--> $DIR/methods.rs:391:13
|
||||
error: .fold(false, |acc, x| acc || x > 2)) is more succinctly expressed as .any(|x| x > 2)
|
||||
--> $DIR/methods.rs:390:13
|
||||
|
|
||||
391 | let _ = opt.unwrap();
|
||||
390 | let _ = (0..3).fold(false, |acc, x| acc || x > 2);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D fold-any` implied by `-D warnings`
|
||||
|
||||
error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message
|
||||
--> $DIR/methods.rs:396:13
|
||||
|
|
||||
396 | let _ = opt.unwrap();
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
= note: `-D option-unwrap-used` implied by `-D warnings`
|
||||
|
|
Loading…
Reference in New Issue