From 68c26b325b35e6984b585d285b17dd5eadb3c860 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:16:25 -0700 Subject: [PATCH 01/26] Rustup to rustc 1.39.0-nightly (acf7b50c7 2019-09-25) - Addresses inference error - Updates compiletest --- Cargo.toml | 2 +- tests/ui/many_single_char_names.rs | 2 +- tests/ui/many_single_char_names.stderr | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8c70bb189397..7ab20320e7dc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,7 +46,7 @@ rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"} [dev-dependencies] cargo_metadata = "0.8.0" -compiletest_rs = { version = "0.3.22", features = ["tmp"] } +compiletest_rs = { version = "0.3.23", features = ["tmp"] } lazy_static = "1.0" clippy-mini-macro-test = { version = "0.2", path = "mini-macro" } serde = { version = "1.0", features = ["derive"] } diff --git a/tests/ui/many_single_char_names.rs b/tests/ui/many_single_char_names.rs index 81c0427305d2..80800e487248 100644 --- a/tests/ui/many_single_char_names.rs +++ b/tests/ui/many_single_char_names.rs @@ -29,7 +29,7 @@ fn bla() { fn bindings(a: i32, b: i32, c: i32, d: i32, e: i32, f: i32, g: i32, h: i32) {} fn bindings2() { - let (a, b, c, d, e, f, g, h) = unimplemented!(); + let (a, b, c, d, e, f, g, h): (bool, bool, bool, bool, bool, bool, bool, bool) = unimplemented!(); } fn shadowing() { diff --git a/tests/ui/many_single_char_names.stderr b/tests/ui/many_single_char_names.stderr index a746667baf0e..27e62e641ade 100644 --- a/tests/ui/many_single_char_names.stderr +++ b/tests/ui/many_single_char_names.stderr @@ -44,7 +44,7 @@ LL | fn bindings(a: i32, b: i32, c: i32, d: i32, e: i32, f: i32, g: i32, h: i32) error: 8 bindings with single-character names in scope --> $DIR/many_single_char_names.rs:32:10 | -LL | let (a, b, c, d, e, f, g, h) = unimplemented!(); +LL | let (a, b, c, d, e, f, g, h): (bool, bool, bool, bool, bool, bool, bool, bool) = unimplemented!(); | ^ ^ ^ ^ ^ ^ ^ ^ error: aborting due to 5 previous errors From 982c51e769c693644eec19ac9f1e339997bbe579 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 12:00:17 -0700 Subject: [PATCH 02/26] arm.pats -> arm.pat --- clippy_lints/src/cognitive_complexity.rs | 3 +- clippy_lints/src/copies.rs | 39 ++++----- clippy_lints/src/format.rs | 3 +- .../src/infallible_destructuring_match.rs | 4 +- clippy_lints/src/loops.rs | 6 +- clippy_lints/src/matches.rs | 83 +++++++------------ clippy_lints/src/ok_if_let.rs | 2 +- .../src/redundant_pattern_matching.rs | 66 +++++++-------- clippy_lints/src/shadow.rs | 16 ++-- clippy_lints/src/utils/author.rs | 7 +- clippy_lints/src/utils/hir_utils.rs | 2 +- clippy_lints/src/utils/inspector.rs | 4 +- clippy_lints/src/utils/mod.rs | 8 +- tests/ui/author/for_loop.stdout | 9 +- tests/ui/shadow.rs | 7 +- tests/ui/shadow.stderr | 46 +++++----- tests/ui/wildcard_enum_match_arm.fixed | 2 +- tests/ui/wildcard_enum_match_arm.rs | 2 +- 18 files changed, 137 insertions(+), 172 deletions(-) diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index 3e7f3c980fc8..33603b27b7a4 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -112,8 +112,7 @@ impl<'tcx> Visitor<'tcx> for CCHelper { walk_expr(self, e); match e.node { ExprKind::Match(_, ref arms, _) => { - let arms_n: u64 = arms.iter().map(|arm| arm.pats.len() as u64).sum(); - if arms_n > 1 { + if arms.len() > 1 { self.cc += 1; } self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64; diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index b01ce7eeb77d..bdef21b0e69b 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -193,7 +193,7 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) { (min_index..=max_index).all(|index| arms[index].guard.is_none()) && SpanlessEq::new(cx).eq_expr(&lhs.body, &rhs.body) && // all patterns should have the same bindings - same_bindings(cx, &bindings(cx, &lhs.pats[0]), &bindings(cx, &rhs.pats[0])) + same_bindings(cx, &bindings(cx, &lhs.pat), &bindings(cx, &rhs.pat)) }; let indexed_arms: Vec<(usize, &Arm)> = arms.iter().enumerate().collect(); @@ -213,27 +213,22 @@ fn lint_match_arms<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &Expr) { // span for the whole pattern, the suggestion is only shown when there is only // one pattern. The user should know about `|` if they are already using it… - if i.pats.len() == 1 && j.pats.len() == 1 { - let lhs = snippet(cx, i.pats[0].span, ""); - let rhs = snippet(cx, j.pats[0].span, ""); - - if let PatKind::Wild = j.pats[0].node { - // if the last arm is _, then i could be integrated into _ - // note that i.pats[0] cannot be _, because that would mean that we're - // hiding all the subsequent arms, and rust won't compile - db.span_note( - i.body.span, - &format!( - "`{}` has the same arm body as the `_` wildcard, consider removing it`", - lhs - ), - ); - } else { - db.span_help( - i.pats[0].span, - &format!("consider refactoring into `{} | {}`", lhs, rhs), - ); - } + let lhs = snippet(cx, i.pat.span, ""); + let rhs = snippet(cx, j.pat.span, ""); + + if let PatKind::Wild = j.pat.node { + // if the last arm is _, then i could be integrated into _ + // note that i.pat cannot be _, because that would mean that we're + // hiding all the subsequent arms, and rust won't compile + db.span_note( + i.body.span, + &format!( + "`{}` has the same arm body as the `_` wildcard, consider removing it`", + lhs + ), + ); + } else { + db.span_help(i.pat.span, &format!("consider refactoring into `{} | {}`", lhs, rhs)); } }, ); diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 1df6b9294d7b..4f763863ffde 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -84,9 +84,8 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm if let ExprKind::Path(ref qpath) = args[1].node; if let Some(did) = resolve_node(cx, qpath, args[1].hir_id).opt_def_id(); if match_def_path(cx, did, &paths::DISPLAY_FMT_METHOD); - if arms[0].pats.len() == 1; // check `(arg0,)` in match block - if let PatKind::Tuple(ref pats, None) = arms[0].pats[0].node; + if let PatKind::Tuple(ref pats, None) = arms[0].pat.node; if pats.len() == 1; then { let ty = walk_ptrs_ty(cx.tables.pat_ty(&pats[0])); diff --git a/clippy_lints/src/infallible_destructuring_match.rs b/clippy_lints/src/infallible_destructuring_match.rs index a977a827321d..2ae2e2006399 100644 --- a/clippy_lints/src/infallible_destructuring_match.rs +++ b/clippy_lints/src/infallible_destructuring_match.rs @@ -47,8 +47,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for InfallibleDestructingMatch { if_chain! { if let Some(ref expr) = local.init; if let ExprKind::Match(ref target, ref arms, MatchSource::Normal) = expr.node; - if arms.len() == 1 && arms[0].pats.len() == 1 && arms[0].guard.is_none(); - if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pats[0].node; + if arms.len() == 1 && arms[0].guard.is_none(); + if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pat.node; if args.len() == 1; if let Some(arg) = get_arg_name(&args[0]); let body = remove_blocks(&arms[0].body); diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 0032cfd1985b..94328a2b5e3d 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -517,9 +517,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops { match *source { MatchSource::Normal | MatchSource::IfLetDesugar { .. } => { if arms.len() == 2 - && arms[0].pats.len() == 1 && arms[0].guard.is_none() - && arms[1].pats.len() == 1 && arms[1].guard.is_none() && is_simple_break_expr(&arms[1].body) { @@ -541,7 +539,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops { "try", format!( "while let {} = {} {{ .. }}", - snippet_with_applicability(cx, arms[0].pats[0].span, "..", &mut applicability), + snippet_with_applicability(cx, arms[0].pat.span, "..", &mut applicability), snippet_with_applicability(cx, matchexpr.span, "..", &mut applicability), ), applicability, @@ -554,7 +552,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Loops { } } if let ExprKind::Match(ref match_expr, ref arms, MatchSource::WhileLetDesugar) = expr.node { - let pat = &arms[0].pats[0].node; + let pat = &arms[0].pat.node; if let ( &PatKind::TupleStruct(ref qpath, ref pat_args, _), &ExprKind::MethodCall(ref method_path, _, ref method_args), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 2a65ea000db3..3d765af21edb 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -14,7 +14,6 @@ use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; use std::cmp::Ordering; use std::collections::Bound; -use std::ops::Deref; use syntax::ast::LitKind; use syntax::source_map::Span; @@ -255,9 +254,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { #[rustfmt::skip] fn check_single_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) { - if arms.len() == 2 && - arms[0].pats.len() == 1 && arms[0].guard.is_none() && - arms[1].pats.len() == 1 && arms[1].guard.is_none() { + if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() { let els = remove_blocks(&arms[1].body); let els = if is_unit_expr(els) { None @@ -283,7 +280,7 @@ fn check_single_match_single_pattern( expr: &Expr, els: Option<&Expr>, ) { - if is_wild(&arms[1].pats[0]) { + if is_wild(&arms[1].pat) { report_single_match_single_pattern(cx, ex, arms, expr, els); } } @@ -308,7 +305,7 @@ fn report_single_match_single_pattern( "try this", format!( "if let {} = {} {}{}", - snippet(cx, arms[0].pats[0].span, ".."), + snippet(cx, arms[0].pat.span, ".."), snippet(cx, ex.span, ".."), expr_block(cx, &arms[0].body, None, ".."), els_str, @@ -336,7 +333,7 @@ fn check_single_match_opt_like( (&paths::RESULT, "Ok"), ]; - let path = match arms[1].pats[0].node { + let path = match arms[1].pat.node { PatKind::TupleStruct(ref path, ref inner, _) => { // Contains any non wildcard patterns (e.g., `Err(err)`)? if !inner.iter().all(is_wild) { @@ -365,9 +362,9 @@ fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Ex expr.span, "you seem to be trying to match on a boolean expression", move |db| { - if arms.len() == 2 && arms[0].pats.len() == 1 { + if arms.len() == 2 { // no guards - let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pats[0].node { + let exprs = if let PatKind::Lit(ref arm_bool) = arms[0].pat.node { if let ExprKind::Lit(ref lit) = arm_bool.node { match lit.node { LitKind::Bool(true) => Some((&*arms[0].body, &*arms[1].body)), @@ -446,7 +443,7 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) { let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex)); if match_type(cx, ex_ty, &paths::RESULT) { for arm in arms { - if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pats[0].node { + if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.node { let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false)); if_chain! { if path_str == "Err"; @@ -457,9 +454,9 @@ fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) { // `Err(_)` arm with `panic!` found span_note_and_lint(cx, MATCH_WILD_ERR_ARM, - arm.pats[0].span, + arm.pat.span, "Err(_) will match all errors, maybe not a good idea", - arm.pats[0].span, + arm.pat.span, "to remove this warning, match each error separately \ or use unreachable macro"); } @@ -482,13 +479,11 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) { let mut wildcard_span = None; let mut wildcard_ident = None; for arm in arms { - for pat in &arm.pats { - if let PatKind::Wild = pat.node { - wildcard_span = Some(pat.span); - } else if let PatKind::Binding(_, _, ident, None) = pat.node { - wildcard_span = Some(pat.span); - wildcard_ident = Some(ident); - } + if let PatKind::Wild = arm.pat.node { + wildcard_span = Some(arm.pat.span); + } else if let PatKind::Binding(_, _, ident, None) = arm.pat.node { + wildcard_span = Some(arm.pat.span); + wildcard_ident = Some(ident); } } @@ -510,15 +505,13 @@ fn check_wild_enum_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm]) { // covered by the set of guards that cover it, but that's really hard to do. continue; } - for pat in &arm.pats { - if let PatKind::Path(ref path) = pat.deref().node { - if let QPath::Resolved(_, p) = path { - missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id())); - } - } else if let PatKind::TupleStruct(ref path, ..) = pat.deref().node { - if let QPath::Resolved(_, p) = path { - missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id())); - } + if let PatKind::Path(ref path) = arm.pat.node { + if let QPath::Resolved(_, p) = path { + missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id())); + } + } else if let PatKind::TupleStruct(ref path, ..) = arm.pat.node { + if let QPath::Resolved(_, p) = path { + missing_variants.retain(|e| e.ctor_def_id != Some(p.res.def_id())); } } } @@ -588,9 +581,9 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: ) }; - suggs.extend(arms.iter().flat_map(|a| &a.pats).filter_map(|p| { - if let PatKind::Ref(ref refp, _) = p.node { - Some((p.span, snippet(cx, refp.span, "..").to_string())) + suggs.extend(arms.iter().filter_map(|a| { + if let PatKind::Ref(ref refp, _) = a.pat.node { + Some((a.pat.span, snippet(cx, refp.span, "..").to_string())) } else { None } @@ -605,12 +598,7 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: } fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) { - if arms.len() == 2 - && arms[0].pats.len() == 1 - && arms[0].guard.is_none() - && arms[1].pats.len() == 1 - && arms[1].guard.is_none() - { + if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() { let arm_ref: Option = if is_none_arm(&arms[0]) { is_ref_some_arm(&arms[1]) } else if is_none_arm(&arms[1]) { @@ -666,14 +654,9 @@ fn all_ranges<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm]) -> Vec(cx: &LateContext<'a, 'tcx>, arms: &'tcx [Arm]) -> Vec bool { // Checks if arm has the form `None => None` fn is_none_arm(arm: &Arm) -> bool { - match arm.pats[0].node { + match arm.pat.node { PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => true, _ => false, } @@ -752,7 +734,7 @@ fn is_none_arm(arm: &Arm) -> bool { // Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`) fn is_ref_some_arm(arm: &Arm) -> Option { if_chain! { - if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pats[0].node; + if let PatKind::TupleStruct(ref path, ref pats, _) = arm.pat.node; if pats.len() == 1 && match_qpath(path, &paths::OPTION_SOME); if let PatKind::Binding(rb, .., ident, _) = pats[0].node; if rb == BindingAnnotation::Ref || rb == BindingAnnotation::RefMut; @@ -772,9 +754,8 @@ fn is_ref_some_arm(arm: &Arm) -> Option { fn has_only_ref_pats(arms: &[Arm]) -> bool { let mapped = arms .iter() - .flat_map(|a| &a.pats) - .map(|p| { - match p.node { + .map(|a| { + match a.pat.node { PatKind::Ref(..) => Some(true), // &-patterns PatKind::Wild => Some(false), // an "anything" wildcard is also fine _ => None, // any other pattern is not fine diff --git a/clippy_lints/src/ok_if_let.rs b/clippy_lints/src/ok_if_let.rs index 01d41f679dbb..7105376d5983 100644 --- a/clippy_lints/src/ok_if_let.rs +++ b/clippy_lints/src/ok_if_let.rs @@ -42,7 +42,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet { if let ExprKind::Match(ref op, ref body, ref source) = expr.node; //test if expr is a match if let MatchSource::IfLetDesugar { .. } = *source; //test if it is an If Let if let ExprKind::MethodCall(_, _, ref result_types) = op.node; //check is expr.ok() has type Result.ok() - if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pats[0].node; //get operation + if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.node; //get operation if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized; then { diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs index 68862f838cb0..b8d1ea3851f6 100644 --- a/clippy_lints/src/redundant_pattern_matching.rs +++ b/clippy_lints/src/redundant_pattern_matching.rs @@ -57,50 +57,48 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching { } fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, op: &P, arms: &HirVec) { - if arms[0].pats.len() == 1 { - let good_method = match arms[0].pats[0].node { - PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => { - if let PatKind::Wild = patterns[0].node { - if match_qpath(path, &paths::RESULT_OK) { - "is_ok()" - } else if match_qpath(path, &paths::RESULT_ERR) { - "is_err()" - } else if match_qpath(path, &paths::OPTION_SOME) { - "is_some()" - } else { - return; - } + let good_method = match arms[0].pat.node { + PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => { + if let PatKind::Wild = patterns[0].node { + if match_qpath(path, &paths::RESULT_OK) { + "is_ok()" + } else if match_qpath(path, &paths::RESULT_ERR) { + "is_err()" + } else if match_qpath(path, &paths::OPTION_SOME) { + "is_some()" } else { return; } - }, + } else { + return; + } + }, - PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()", + PatKind::Path(ref path) if match_qpath(path, &paths::OPTION_NONE) => "is_none()", - _ => return, - }; + _ => return, + }; - span_lint_and_then( - cx, - REDUNDANT_PATTERN_MATCHING, - arms[0].pats[0].span, - &format!("redundant pattern matching, consider using `{}`", good_method), - |db| { - let span = expr.span.to(op.span); - db.span_suggestion( - span, - "try this", - format!("{}.{}", snippet(cx, op.span, "_"), good_method), - Applicability::MaybeIncorrect, // snippet - ); - }, - ); - } + span_lint_and_then( + cx, + REDUNDANT_PATTERN_MATCHING, + arms[0].pat.span, + &format!("redundant pattern matching, consider using `{}`", good_method), + |db| { + let span = expr.span.to(op.span); + db.span_suggestion( + span, + "try this", + format!("{}.{}", snippet(cx, op.span, "_"), good_method), + Applicability::MaybeIncorrect, // snippet + ); + }, + ); } fn find_sugg_for_match<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, op: &P, arms: &HirVec) { if arms.len() == 2 { - let node_pair = (&arms[0].pats[0].node, &arms[1].pats[0].node); + let node_pair = (&arms[0].pat.node, &arms[1].pat.node); let found_good_method = match node_pair { ( diff --git a/clippy_lints/src/shadow.rs b/clippy_lints/src/shadow.rs index b30f8d415b18..ebef0449cc98 100644 --- a/clippy_lints/src/shadow.rs +++ b/clippy_lints/src/shadow.rs @@ -328,17 +328,15 @@ fn check_expr<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, bindings: check_expr(cx, init, bindings); let len = bindings.len(); for arm in arms { - for pat in &arm.pats { - check_pat(cx, pat, Some(&**init), pat.span, bindings); - // This is ugly, but needed to get the right type - if let Some(ref guard) = arm.guard { - match guard { - Guard::If(if_expr) => check_expr(cx, if_expr, bindings), - } + check_pat(cx, &arm.pat, Some(&**init), arm.pat.span, bindings); + // This is ugly, but needed to get the right type + if let Some(ref guard) = arm.guard { + match guard { + Guard::If(if_expr) => check_expr(cx, if_expr, bindings), } - check_expr(cx, &arm.body, bindings); - bindings.truncate(len); } + check_expr(cx, &arm.body, bindings); + bindings.truncate(len); } }, _ => (), diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 87208dd4beb7..044e9838090a 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -354,11 +354,8 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { }, } } - println!(" if {}[{}].pats.len() == {};", arms_pat, i, arm.pats.len()); - for (j, pat) in arm.pats.iter().enumerate() { - self.current = format!("{}[{}].pats[{}]", arms_pat, i, j); - self.visit_pat(pat); - } + self.current = format!("{}[{}].pat", arms_pat, i); + self.visit_pat(&arm.pat); } }, ExprKind::Closure(ref _capture_clause, ref _func, _, _, _) => { diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 6fc5939a2160..deea5823f139 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -124,7 +124,7 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { && over(la, ra, |l, r| { self.eq_expr(&l.body, &r.body) && both(&l.guard, &r.guard, |l, r| self.eq_guard(l, r)) - && over(&l.pats, &r.pats, |l, r| self.eq_pat(l, r)) + && self.eq_pat(&l.pat, &r.pat) }) }, (&ExprKind::MethodCall(ref l_path, _, ref l_args), &ExprKind::MethodCall(ref r_path, _, ref r_args)) => { diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index ba0e56c9987a..d70f8bab8f73 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -101,9 +101,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DeepCodeInspector { if !has_attr(cx.sess(), &arm.attrs) { return; } - for pat in &arm.pats { - print_pat(cx, pat, 1); - } + print_pat(cx, &arm.pat, 1); if let Some(ref guard) = arm.guard { println!("guard:"); print_guard(cx, guard, 1); diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 9165f8d74d78..88229da8c1a6 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -909,7 +909,7 @@ pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator Option<&Expr> { fn is_ok(arm: &Arm) -> bool { if_chain! { - if let PatKind::TupleStruct(ref path, ref pat, None) = arm.pats[0].node; + if let PatKind::TupleStruct(ref path, ref pat, None) = arm.pat.node; if match_qpath(path, &paths::RESULT_OK[1..]); if let PatKind::Binding(_, hir_id, _, None) = pat[0].node; if let ExprKind::Path(QPath::Resolved(None, ref path)) = arm.body.node; @@ -923,7 +923,7 @@ pub fn is_try(expr: &Expr) -> Option<&Expr> { } fn is_err(arm: &Arm) -> bool { - if let PatKind::TupleStruct(ref path, _, _) = arm.pats[0].node { + if let PatKind::TupleStruct(ref path, _, _) = arm.pat.node { match_qpath(path, &paths::RESULT_ERR[1..]) } else { false @@ -938,8 +938,8 @@ pub fn is_try(expr: &Expr) -> Option<&Expr> { if_chain! { if arms.len() == 2; - if arms[0].pats.len() == 1 && arms[0].guard.is_none(); - if arms[1].pats.len() == 1 && arms[1].guard.is_none(); + if arms[0].guard.is_none(); + if arms[1].guard.is_none(); if (is_ok(&arms[0]) && is_err(&arms[1])) || (is_ok(&arms[1]) && is_err(&arms[0])); then { diff --git a/tests/ui/author/for_loop.stdout b/tests/ui/author/for_loop.stdout index 7ce04e44b03d..d195c7fdb958 100644 --- a/tests/ui/author/for_loop.stdout +++ b/tests/ui/author/for_loop.stdout @@ -31,14 +31,12 @@ if_chain! { if match_qpath(path4, &["__next"]); if let ExprKind::Path(ref path5) = value.node; if match_qpath(path5, &["val"]); - if arms1[0].pats.len() == 1; - if let PatKind::TupleStruct(ref path6, ref fields1, None) = arms1[0].pats[0].node; + if let PatKind::TupleStruct(ref path6, ref fields1, None) = arms1[0].pat.node; if match_qpath(path6, &["{{root}}", "std", "option", "Option", "Some"]); if fields1.len() == 1; // unimplemented: field checks if let ExprKind::Break(ref destination, None) = arms1[1].body.node; - if arms1[1].pats.len() == 1; - if let PatKind::Path(ref path7) = arms1[1].pats[0].node; + if let PatKind::Path(ref path7) = arms1[1].pat.node; if match_qpath(path7, &["{{root}}", "std", "option", "Option", "None"]); if let StmtKind::Local(ref local1) = body.stmts[2].node; if let Some(ref init) = local1.init; @@ -56,8 +54,7 @@ if_chain! { if match_qpath(path9, &["y"]); if let PatKind::Binding(BindingAnnotation::Unannotated, _, name2, None) = local2.pat.node; if name2.node.as_str() == "z"; - if arms[0].pats.len() == 1; - if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pats[0].node; + if let PatKind::Binding(BindingAnnotation::Mutable, _, name3, None) = arms[0].pat.node; if name3.node.as_str() == "iter"; then { // report your lint here diff --git a/tests/ui/shadow.rs b/tests/ui/shadow.rs index a9c77aca66f5..7a5da67e0266 100644 --- a/tests/ui/shadow.rs +++ b/tests/ui/shadow.rs @@ -5,7 +5,12 @@ clippy::shadow_reuse, clippy::shadow_unrelated )] -#![allow(unused_parens, unused_variables, clippy::missing_docs_in_private_items)] +#![allow( + unused_parens, + unused_variables, + clippy::missing_docs_in_private_items, + clippy::single_match +)] fn id(x: T) -> T { x diff --git a/tests/ui/shadow.stderr b/tests/ui/shadow.stderr index ada8b07d69b7..184e781ae43d 100644 --- a/tests/ui/shadow.stderr +++ b/tests/ui/shadow.stderr @@ -1,135 +1,135 @@ error: `x` is shadowed by itself in `&mut x` - --> $DIR/shadow.rs:20:5 + --> $DIR/shadow.rs:25:5 | LL | let x = &mut x; | ^^^^^^^^^^^^^^^ | = note: `-D clippy::shadow-same` implied by `-D warnings` note: previous binding is here - --> $DIR/shadow.rs:19:13 + --> $DIR/shadow.rs:24:13 | LL | let mut x = 1; | ^ error: `x` is shadowed by itself in `{ x }` - --> $DIR/shadow.rs:21:5 + --> $DIR/shadow.rs:26:5 | LL | let x = { x }; | ^^^^^^^^^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:20:9 + --> $DIR/shadow.rs:25:9 | LL | let x = &mut x; | ^ error: `x` is shadowed by itself in `(&*x)` - --> $DIR/shadow.rs:22:5 + --> $DIR/shadow.rs:27:5 | LL | let x = (&*x); | ^^^^^^^^^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:21:9 + --> $DIR/shadow.rs:26:9 | LL | let x = { x }; | ^ error: `x` is shadowed by `{ *x + 1 }` which reuses the original value - --> $DIR/shadow.rs:23:9 + --> $DIR/shadow.rs:28:9 | LL | let x = { *x + 1 }; | ^ | = note: `-D clippy::shadow-reuse` implied by `-D warnings` note: initialization happens here - --> $DIR/shadow.rs:23:13 + --> $DIR/shadow.rs:28:13 | LL | let x = { *x + 1 }; | ^^^^^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:22:9 + --> $DIR/shadow.rs:27:9 | LL | let x = (&*x); | ^ error: `x` is shadowed by `id(x)` which reuses the original value - --> $DIR/shadow.rs:24:9 + --> $DIR/shadow.rs:29:9 | LL | let x = id(x); | ^ | note: initialization happens here - --> $DIR/shadow.rs:24:13 + --> $DIR/shadow.rs:29:13 | LL | let x = id(x); | ^^^^^ note: previous binding is here - --> $DIR/shadow.rs:23:9 + --> $DIR/shadow.rs:28:9 | LL | let x = { *x + 1 }; | ^ error: `x` is shadowed by `(1, x)` which reuses the original value - --> $DIR/shadow.rs:25:9 + --> $DIR/shadow.rs:30:9 | LL | let x = (1, x); | ^ | note: initialization happens here - --> $DIR/shadow.rs:25:13 + --> $DIR/shadow.rs:30:13 | LL | let x = (1, x); | ^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:24:9 + --> $DIR/shadow.rs:29:9 | LL | let x = id(x); | ^ error: `x` is shadowed by `first(x)` which reuses the original value - --> $DIR/shadow.rs:26:9 + --> $DIR/shadow.rs:31:9 | LL | let x = first(x); | ^ | note: initialization happens here - --> $DIR/shadow.rs:26:13 + --> $DIR/shadow.rs:31:13 | LL | let x = first(x); | ^^^^^^^^ note: previous binding is here - --> $DIR/shadow.rs:25:9 + --> $DIR/shadow.rs:30:9 | LL | let x = (1, x); | ^ error: `x` is shadowed by `y` - --> $DIR/shadow.rs:28:9 + --> $DIR/shadow.rs:33:9 | LL | let x = y; | ^ | = note: `-D clippy::shadow-unrelated` implied by `-D warnings` note: initialization happens here - --> $DIR/shadow.rs:28:13 + --> $DIR/shadow.rs:33:13 | LL | let x = y; | ^ note: previous binding is here - --> $DIR/shadow.rs:26:9 + --> $DIR/shadow.rs:31:9 | LL | let x = first(x); | ^ error: `x` shadows a previous declaration - --> $DIR/shadow.rs:30:5 + --> $DIR/shadow.rs:35:5 | LL | let x; | ^^^^^^ | note: previous binding is here - --> $DIR/shadow.rs:28:9 + --> $DIR/shadow.rs:33:9 | LL | let x = y; | ^ diff --git a/tests/ui/wildcard_enum_match_arm.fixed b/tests/ui/wildcard_enum_match_arm.fixed index 01c861282dcb..af67f326f856 100644 --- a/tests/ui/wildcard_enum_match_arm.fixed +++ b/tests/ui/wildcard_enum_match_arm.fixed @@ -1,7 +1,7 @@ // run-rustfix #![deny(clippy::wildcard_enum_match_arm)] -#![allow(unreachable_code, unused_variables)] +#![allow(unreachable_code, unused_variables, dead_code)] #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { diff --git a/tests/ui/wildcard_enum_match_arm.rs b/tests/ui/wildcard_enum_match_arm.rs index d33c68a6c7dd..049596d342e8 100644 --- a/tests/ui/wildcard_enum_match_arm.rs +++ b/tests/ui/wildcard_enum_match_arm.rs @@ -1,7 +1,7 @@ // run-rustfix #![deny(clippy::wildcard_enum_match_arm)] -#![allow(unreachable_code, unused_variables)] +#![allow(unreachable_code, unused_variables, dead_code)] #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum Color { From b5cadd734eed6f4ba317550f8d6e3ab019fe1bcb Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 13:34:55 -0700 Subject: [PATCH 03/26] ignore single-match for or patterns --- clippy_lints/src/matches.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 3d765af21edb..ac4f012f35b3 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -255,6 +255,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches { #[rustfmt::skip] fn check_single_match(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &Expr) { if arms.len() == 2 && arms[0].guard.is_none() && arms[1].guard.is_none() { + if let PatKind::Or(..) = arms[0].pat.node { + // don't lint for or patterns for now, this makes + // the lint noisy in unnecessary situations + return; + } let els = remove_blocks(&arms[1].body); let els = if is_unit_expr(els) { None From 2c73cad87462d0642065d8bce6afa3c37ef099a3 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:29:39 -0700 Subject: [PATCH 04/26] Remove suggestion for complex map_entry cases --- clippy_lints/src/entry.rs | 18 +++++----- tests/ui/entry.stderr | 74 +++++++++++++++++++++++++++++++++++---- 2 files changed, 77 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index 16009d8ab86d..a590a8179c22 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -1,5 +1,6 @@ use crate::utils::SpanlessEq; -use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt, span_lint_and_then, walk_ptrs_ty}; +use crate::utils::{get_item_name, higher, match_type, paths, snippet, snippet_opt}; +use crate::utils::{snippet_with_applicability, span_lint_and_then, walk_ptrs_ty}; use if_chain::if_chain; use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc::hir::*; @@ -145,10 +146,11 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { span_lint_and_then(self.cx, MAP_ENTRY, self.span, &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| { if self.sole_expr { + let mut app = Applicability::MachineApplicable; let help = format!("{}.entry({}).or_insert({})", - snippet(self.cx, self.map.span, "map"), - snippet(self.cx, params[1].span, ".."), - snippet(self.cx, params[2].span, "..")); + snippet_with_applicability(self.cx, self.map.span, "map", &mut app), + snippet_with_applicability(self.cx, params[1].span, "..", &mut app), + snippet_with_applicability(self.cx, params[2].span, "..", &mut app)); db.span_suggestion( self.span, @@ -158,15 +160,13 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { ); } else { - let help = format!("{}.entry({})", + let help = format!("consider using `{}.entry({})`", snippet(self.cx, self.map.span, "map"), snippet(self.cx, params[1].span, "..")); - db.span_suggestion( + db.span_help( self.span, - "consider using", - help, - Applicability::MachineApplicable, // snippet + &help, ); } }); diff --git a/tests/ui/entry.stderr b/tests/ui/entry.stderr index efacec1e7778..d17456ef8524 100644 --- a/tests/ui/entry.stderr +++ b/tests/ui/entry.stderr @@ -15,7 +15,16 @@ LL | / if !m.contains_key(&k) { LL | | foo(); LL | | m.insert(k, v); LL | | } - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:16:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v); +LL | | } + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:23:5 @@ -25,7 +34,17 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:23:5 + | +LL | / if !m.contains_key(&k) { +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:31:5 @@ -35,7 +54,17 @@ LL | | None LL | | } else { LL | | m.insert(k, v) LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:31:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | m.insert(k, v) +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:39:5 @@ -46,7 +75,18 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:39:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry.rs:48:5 @@ -57,7 +97,18 @@ LL | | } else { LL | | foo(); LL | | m.insert(k, v) LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:48:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | foo(); +LL | | m.insert(k, v) +LL | | }; + | |_____^ error: usage of `contains_key` followed by `insert` on a `BTreeMap` --> $DIR/entry.rs:57:5 @@ -68,7 +119,18 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ help: consider using: `m.entry(k)` + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry.rs:57:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ error: aborting due to 7 previous errors From cfc4bba86479156800a28de1bf7226b1a42d38ee Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:31:46 -0700 Subject: [PATCH 05/26] Split map_entry tests into fixable and unfixable --- tests/ui/entry_fixable.rs | 21 +++++ tests/ui/entry_fixable.stderr | 12 +++ tests/ui/{entry.rs => entry_unfixable.rs} | 19 ---- tests/ui/entry_unfixable.stderr | 109 ++++++++++++++++++++++ tests/ui/string_lit_as_bytes.fixed | 2 +- tests/ui/string_lit_as_bytes.rs | 2 +- tests/ui/string_lit_as_bytes.stderr | 4 +- 7 files changed, 146 insertions(+), 23 deletions(-) create mode 100644 tests/ui/entry_fixable.rs create mode 100644 tests/ui/entry_fixable.stderr rename tests/ui/{entry.rs => entry_unfixable.rs} (77%) create mode 100644 tests/ui/entry_unfixable.stderr diff --git a/tests/ui/entry_fixable.rs b/tests/ui/entry_fixable.rs new file mode 100644 index 000000000000..82267cf34a78 --- /dev/null +++ b/tests/ui/entry_fixable.rs @@ -0,0 +1,21 @@ +#![allow(unused, clippy::needless_pass_by_value)] +#![warn(clippy::map_entry)] + +use std::collections::{BTreeMap, HashMap}; +use std::hash::Hash; + +fn foo() {} + +fn insert_if_absent0(m: &mut HashMap, k: K, v: V) { + if !m.contains_key(&k) { + m.insert(k, v); + } +} + +fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: V) { + if !m.contains_key(&k) { + m.insert(o, v); + } +} + +fn main() {} diff --git a/tests/ui/entry_fixable.stderr b/tests/ui/entry_fixable.stderr new file mode 100644 index 000000000000..59a20e5c00b8 --- /dev/null +++ b/tests/ui/entry_fixable.stderr @@ -0,0 +1,12 @@ +error: usage of `contains_key` followed by `insert` on a `HashMap` + --> $DIR/entry_fixable.rs:10:5 + | +LL | / if !m.contains_key(&k) { +LL | | m.insert(k, v); +LL | | } + | |_____^ help: consider using: `m.entry(k).or_insert(v)` + | + = note: `-D clippy::map-entry` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/tests/ui/entry.rs b/tests/ui/entry_unfixable.rs similarity index 77% rename from tests/ui/entry.rs rename to tests/ui/entry_unfixable.rs index 0c84cd325c4d..8859a3a2f9b4 100644 --- a/tests/ui/entry.rs +++ b/tests/ui/entry_unfixable.rs @@ -6,19 +6,6 @@ use std::hash::Hash; fn foo() {} -fn insert_if_absent0(m: &mut HashMap, k: K, v: V) { - if !m.contains_key(&k) { - m.insert(k, v); - } -} - -fn insert_if_absent1(m: &mut HashMap, k: K, v: V) { - if !m.contains_key(&k) { - foo(); - m.insert(k, v); - } -} - fn insert_if_absent2(m: &mut HashMap, k: K, v: V) { if !m.contains_key(&k) { m.insert(k, v) @@ -62,12 +49,6 @@ fn insert_in_btreemap(m: &mut BTreeMap, k: K, v: V) { }; } -fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: V) { - if !m.contains_key(&k) { - m.insert(o, v); - } -} - // should not trigger, because the one uses different HashMap from another one fn insert_from_different_map(m: HashMap, n: &mut HashMap, k: K, v: V) { if !m.contains_key(&k) { diff --git a/tests/ui/entry_unfixable.stderr b/tests/ui/entry_unfixable.stderr new file mode 100644 index 000000000000..ab655fcfead6 --- /dev/null +++ b/tests/ui/entry_unfixable.stderr @@ -0,0 +1,109 @@ +error: usage of `contains_key` followed by `insert` on a `HashMap` + --> $DIR/entry_unfixable.rs:10:5 + | +LL | / if !m.contains_key(&k) { +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + | + = note: `-D clippy::map-entry` implied by `-D warnings` +help: consider using `m.entry(k)` + --> $DIR/entry_unfixable.rs:10:5 + | +LL | / if !m.contains_key(&k) { +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + +error: usage of `contains_key` followed by `insert` on a `HashMap` + --> $DIR/entry_unfixable.rs:18:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | m.insert(k, v) +LL | | }; + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry_unfixable.rs:18:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | m.insert(k, v) +LL | | }; + | |_____^ + +error: usage of `contains_key` followed by `insert` on a `HashMap` + --> $DIR/entry_unfixable.rs:26:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry_unfixable.rs:26:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + +error: usage of `contains_key` followed by `insert` on a `HashMap` + --> $DIR/entry_unfixable.rs:35:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | foo(); +LL | | m.insert(k, v) +LL | | }; + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry_unfixable.rs:35:5 + | +LL | / if m.contains_key(&k) { +LL | | None +LL | | } else { +LL | | foo(); +LL | | m.insert(k, v) +LL | | }; + | |_____^ + +error: usage of `contains_key` followed by `insert` on a `BTreeMap` + --> $DIR/entry_unfixable.rs:44:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + | +help: consider using `m.entry(k)` + --> $DIR/entry_unfixable.rs:44:5 + | +LL | / if !m.contains_key(&k) { +LL | | foo(); +LL | | m.insert(k, v) +LL | | } else { +LL | | None +LL | | }; + | |_____^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/string_lit_as_bytes.fixed b/tests/ui/string_lit_as_bytes.fixed index 1922478165fd..c8d1479f1206 100644 --- a/tests/ui/string_lit_as_bytes.fixed +++ b/tests/ui/string_lit_as_bytes.fixed @@ -14,7 +14,7 @@ fn str_lit_as_bytes() { let strify = stringify!(foobar).as_bytes(); - let includestr = include_bytes!("entry.rs"); + let includestr = include_bytes!("entry_unfixable.rs"); } fn main() {} diff --git a/tests/ui/string_lit_as_bytes.rs b/tests/ui/string_lit_as_bytes.rs index 560cbcb657b8..f0066d5d177b 100644 --- a/tests/ui/string_lit_as_bytes.rs +++ b/tests/ui/string_lit_as_bytes.rs @@ -14,7 +14,7 @@ fn str_lit_as_bytes() { let strify = stringify!(foobar).as_bytes(); - let includestr = include_str!("entry.rs").as_bytes(); + let includestr = include_str!("entry_unfixable.rs").as_bytes(); } fn main() {} diff --git a/tests/ui/string_lit_as_bytes.stderr b/tests/ui/string_lit_as_bytes.stderr index 59aaec75bd22..d6c6c52709f8 100644 --- a/tests/ui/string_lit_as_bytes.stderr +++ b/tests/ui/string_lit_as_bytes.stderr @@ -15,8 +15,8 @@ LL | let bs = r###"raw string with 3# plus " ""###.as_bytes(); error: calling `as_bytes()` on `include_str!(..)` --> $DIR/string_lit_as_bytes.rs:17:22 | -LL | let includestr = include_str!("entry.rs").as_bytes(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("entry.rs")` +LL | let includestr = include_str!("entry_unfixable.rs").as_bytes(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `include_bytes!(..)` instead: `include_bytes!("entry_unfixable.rs")` error: aborting due to 3 previous errors From 001cebf47cc8c3231eb923adb365584942e4f67e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:38:17 -0700 Subject: [PATCH 06/26] map_entry test: Fix semicolon, add run-rustfix --- clippy_lints/src/entry.rs | 5 +-- tests/ui/entry_fixable.fixed | 15 ++++++++ tests/ui/entry_fixable.rs | 8 ++--- tests/ui/entry_fixable.stderr | 4 +-- tests/ui/entry_unfixable.rs | 7 ++++ tests/ui/entry_unfixable.stderr | 62 +++------------------------------ 6 files changed, 34 insertions(+), 67 deletions(-) create mode 100644 tests/ui/entry_fixable.fixed diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index a590a8179c22..c0eba516dee0 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -65,6 +65,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for HashMapPass { } else { true } + // XXXManishearth we can also check for if/else blocks containing `None`. }; let mut visitor = InsertVisitor { @@ -147,7 +148,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { &format!("usage of `contains_key` followed by `insert` on a `{}`", self.ty), |db| { if self.sole_expr { let mut app = Applicability::MachineApplicable; - let help = format!("{}.entry({}).or_insert({})", + let help = format!("{}.entry({}).or_insert({});", snippet_with_applicability(self.cx, self.map.span, "map", &mut app), snippet_with_applicability(self.cx, params[1].span, "..", &mut app), snippet_with_applicability(self.cx, params[2].span, "..", &mut app)); @@ -164,7 +165,7 @@ impl<'a, 'tcx, 'b> Visitor<'tcx> for InsertVisitor<'a, 'tcx, 'b> { snippet(self.cx, self.map.span, "map"), snippet(self.cx, params[1].span, "..")); - db.span_help( + db.span_label( self.span, &help, ); diff --git a/tests/ui/entry_fixable.fixed b/tests/ui/entry_fixable.fixed new file mode 100644 index 000000000000..dcdaae7e7243 --- /dev/null +++ b/tests/ui/entry_fixable.fixed @@ -0,0 +1,15 @@ +// run-rustfix + +#![allow(unused, clippy::needless_pass_by_value)] +#![warn(clippy::map_entry)] + +use std::collections::{BTreeMap, HashMap}; +use std::hash::Hash; + +fn foo() {} + +fn insert_if_absent0(m: &mut HashMap, k: K, v: V) { + m.entry(k).or_insert(v); +} + +fn main() {} diff --git a/tests/ui/entry_fixable.rs b/tests/ui/entry_fixable.rs index 82267cf34a78..55d5b21568d0 100644 --- a/tests/ui/entry_fixable.rs +++ b/tests/ui/entry_fixable.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![allow(unused, clippy::needless_pass_by_value)] #![warn(clippy::map_entry)] @@ -12,10 +14,4 @@ fn insert_if_absent0(m: &mut HashMap, k: K, v: V) { } } -fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: V) { - if !m.contains_key(&k) { - m.insert(o, v); - } -} - fn main() {} diff --git a/tests/ui/entry_fixable.stderr b/tests/ui/entry_fixable.stderr index 59a20e5c00b8..87403200ced5 100644 --- a/tests/ui/entry_fixable.stderr +++ b/tests/ui/entry_fixable.stderr @@ -1,10 +1,10 @@ error: usage of `contains_key` followed by `insert` on a `HashMap` - --> $DIR/entry_fixable.rs:10:5 + --> $DIR/entry_fixable.rs:12:5 | LL | / if !m.contains_key(&k) { LL | | m.insert(k, v); LL | | } - | |_____^ help: consider using: `m.entry(k).or_insert(v)` + | |_____^ help: consider using: `m.entry(k).or_insert(v);` | = note: `-D clippy::map-entry` implied by `-D warnings` diff --git a/tests/ui/entry_unfixable.rs b/tests/ui/entry_unfixable.rs index 8859a3a2f9b4..f530fc023cfb 100644 --- a/tests/ui/entry_unfixable.rs +++ b/tests/ui/entry_unfixable.rs @@ -49,6 +49,13 @@ fn insert_in_btreemap(m: &mut BTreeMap, k: K, v: V) { }; } +// should not trigger +fn insert_other_if_absent(m: &mut HashMap, k: K, o: K, v: V) { + if !m.contains_key(&k) { + m.insert(o, v); + } +} + // should not trigger, because the one uses different HashMap from another one fn insert_from_different_map(m: HashMap, n: &mut HashMap, k: K, v: V) { if !m.contains_key(&k) { diff --git a/tests/ui/entry_unfixable.stderr b/tests/ui/entry_unfixable.stderr index ab655fcfead6..e58c8d22dc45 100644 --- a/tests/ui/entry_unfixable.stderr +++ b/tests/ui/entry_unfixable.stderr @@ -6,18 +6,9 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` | = note: `-D clippy::map-entry` implied by `-D warnings` -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:10:5 - | -LL | / if !m.contains_key(&k) { -LL | | m.insert(k, v) -LL | | } else { -LL | | None -LL | | }; - | |_____^ error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry_unfixable.rs:18:5 @@ -27,17 +18,7 @@ LL | | None LL | | } else { LL | | m.insert(k, v) LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:18:5 - | -LL | / if m.contains_key(&k) { -LL | | None -LL | | } else { -LL | | m.insert(k, v) -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry_unfixable.rs:26:5 @@ -48,18 +29,7 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:26:5 - | -LL | / if !m.contains_key(&k) { -LL | | foo(); -LL | | m.insert(k, v) -LL | | } else { -LL | | None -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: usage of `contains_key` followed by `insert` on a `HashMap` --> $DIR/entry_unfixable.rs:35:5 @@ -70,18 +40,7 @@ LL | | } else { LL | | foo(); LL | | m.insert(k, v) LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:35:5 - | -LL | / if m.contains_key(&k) { -LL | | None -LL | | } else { -LL | | foo(); -LL | | m.insert(k, v) -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: usage of `contains_key` followed by `insert` on a `BTreeMap` --> $DIR/entry_unfixable.rs:44:5 @@ -92,18 +51,7 @@ LL | | m.insert(k, v) LL | | } else { LL | | None LL | | }; - | |_____^ - | -help: consider using `m.entry(k)` - --> $DIR/entry_unfixable.rs:44:5 - | -LL | / if !m.contains_key(&k) { -LL | | foo(); -LL | | m.insert(k, v) -LL | | } else { -LL | | None -LL | | }; - | |_____^ + | |_____^ consider using `m.entry(k)` error: aborting due to 5 previous errors From 4169cd431802b40599fb496c1a7d95ef3c8b8216 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:46:43 -0700 Subject: [PATCH 07/26] Remove large-digit-groups test from literals.rs --- tests/ui/literals.rs | 5 ++--- tests/ui/literals.stderr | 26 +++++++++----------------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/tests/ui/literals.rs b/tests/ui/literals.rs index be1d2ed2f4d6..7680d4e10d66 100644 --- a/tests/ui/literals.rs +++ b/tests/ui/literals.rs @@ -1,4 +1,5 @@ -#![warn(clippy::large_digit_groups)] +// tests no rustfixable lints + #![warn(clippy::mixed_case_hex_literals)] #![warn(clippy::zero_prefixed_literal)] #![allow(clippy::unseparated_literal_suffix)] @@ -28,8 +29,6 @@ fn main() { let ok16 = 0xFE_BAFE_ABAB_ABCD; let ok17 = 0x123_4567_8901_usize; - let fail13 = 0x1_23456_78901_usize; - let fail19 = 12_3456_21; let fail22 = 3__4___23; let fail23 = 3__16___23; diff --git a/tests/ui/literals.stderr b/tests/ui/literals.stderr index 33321440d831..4e1e8e4c246a 100644 --- a/tests/ui/literals.stderr +++ b/tests/ui/literals.stderr @@ -1,5 +1,5 @@ error: inconsistent casing in hexadecimal literal - --> $DIR/literals.rs:13:17 + --> $DIR/literals.rs:14:17 | LL | let fail1 = 0xabCD; | ^^^^^^ @@ -7,19 +7,19 @@ LL | let fail1 = 0xabCD; = note: `-D clippy::mixed-case-hex-literals` implied by `-D warnings` error: inconsistent casing in hexadecimal literal - --> $DIR/literals.rs:14:17 + --> $DIR/literals.rs:15:17 | LL | let fail2 = 0xabCD_u32; | ^^^^^^^^^^ error: inconsistent casing in hexadecimal literal - --> $DIR/literals.rs:15:17 + --> $DIR/literals.rs:16:17 | LL | let fail2 = 0xabCD_isize; | ^^^^^^^^^^^^ error: this is a decimal constant - --> $DIR/literals.rs:16:27 + --> $DIR/literals.rs:17:27 | LL | let fail_multi_zero = 000_123usize; | ^^^^^^^^^^^^ @@ -35,7 +35,7 @@ LL | let fail_multi_zero = 0o123usize; | ^^^^^^^^^^ error: this is a decimal constant - --> $DIR/literals.rs:20:17 + --> $DIR/literals.rs:21:17 | LL | let fail8 = 0123; | ^^^^ @@ -48,16 +48,8 @@ help: if you mean to use an octal constant, use `0o` LL | let fail8 = 0o123; | ^^^^^ -error: digit groups should be smaller - --> $DIR/literals.rs:31:18 - | -LL | let fail13 = 0x1_23456_78901_usize; - | ^^^^^^^^^^^^^^^^^^^^^ help: consider: `0x0123_4567_8901_usize` - | - = note: `-D clippy::large-digit-groups` implied by `-D warnings` - error: digits grouped inconsistently by underscores - --> $DIR/literals.rs:33:18 + --> $DIR/literals.rs:32:18 | LL | let fail19 = 12_3456_21; | ^^^^^^^^^^ help: consider: `12_345_621` @@ -65,16 +57,16 @@ LL | let fail19 = 12_3456_21; = note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings` error: digits grouped inconsistently by underscores - --> $DIR/literals.rs:34:18 + --> $DIR/literals.rs:33:18 | LL | let fail22 = 3__4___23; | ^^^^^^^^^ help: consider: `3_423` error: digits grouped inconsistently by underscores - --> $DIR/literals.rs:35:18 + --> $DIR/literals.rs:34:18 | LL | let fail23 = 3__16___23; | ^^^^^^^^^^ help: consider: `31_623` -error: aborting due to 9 previous errors +error: aborting due to 8 previous errors From 1171e65b9af8484b60245494c4d89b42f90dec29 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:49:38 -0700 Subject: [PATCH 08/26] Leave note on non-rustfixable tests --- tests/ui/eq_op.rs | 2 ++ tests/ui/eq_op.stderr | 54 ++++++++++++++++----------------- tests/ui/float_cmp_const.rs | 2 ++ tests/ui/float_cmp_const.stderr | 28 ++++++++--------- tests/ui/literals.rs | 2 +- 5 files changed, 46 insertions(+), 42 deletions(-) diff --git a/tests/ui/eq_op.rs b/tests/ui/eq_op.rs index c93f520bee24..e9a685b9c79a 100644 --- a/tests/ui/eq_op.rs +++ b/tests/ui/eq_op.rs @@ -1,3 +1,5 @@ +// does not test any rustfixable lints + #[rustfmt::skip] #[warn(clippy::eq_op)] #[allow(clippy::identity_op, clippy::double_parens, clippy::many_single_char_names)] diff --git a/tests/ui/eq_op.stderr b/tests/ui/eq_op.stderr index e37c0c22907e..5b80e6078eed 100644 --- a/tests/ui/eq_op.stderr +++ b/tests/ui/eq_op.stderr @@ -1,5 +1,5 @@ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:9:5 + --> $DIR/eq_op.rs:11:5 | LL | 1 == 1; | ^^^^^^ @@ -7,157 +7,157 @@ LL | 1 == 1; = note: `-D clippy::eq-op` implied by `-D warnings` error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:10:5 + --> $DIR/eq_op.rs:12:5 | LL | "no" == "no"; | ^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:12:5 + --> $DIR/eq_op.rs:14:5 | LL | false != false; | ^^^^^^^^^^^^^^ error: equal expressions as operands to `<` - --> $DIR/eq_op.rs:13:5 + --> $DIR/eq_op.rs:15:5 | LL | 1.5 < 1.5; | ^^^^^^^^^ error: equal expressions as operands to `>=` - --> $DIR/eq_op.rs:14:5 + --> $DIR/eq_op.rs:16:5 | LL | 1u64 >= 1u64; | ^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:17:5 + --> $DIR/eq_op.rs:19:5 | LL | (1 as u64) & (1 as u64); | ^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `^` - --> $DIR/eq_op.rs:18:5 + --> $DIR/eq_op.rs:20:5 | LL | 1 ^ ((((((1)))))); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `<` - --> $DIR/eq_op.rs:21:5 + --> $DIR/eq_op.rs:23:5 | LL | (-(2) < -(2)); | ^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:22:5 + --> $DIR/eq_op.rs:24:5 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:22:6 + --> $DIR/eq_op.rs:24:6 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&` - --> $DIR/eq_op.rs:22:27 + --> $DIR/eq_op.rs:24:27 | LL | ((1 + 1) & (1 + 1) == (1 + 1) & (1 + 1)); | ^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:23:5 + --> $DIR/eq_op.rs:25:5 | LL | (1 * 2) + (3 * 4) == 1 * 2 + 3 * 4; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:26:5 + --> $DIR/eq_op.rs:28:5 | LL | ([1] != [1]); | ^^^^^^^^^^^^ error: equal expressions as operands to `!=` - --> $DIR/eq_op.rs:27:5 + --> $DIR/eq_op.rs:29:5 | LL | ((1, 2) != (1, 2)); | ^^^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:31:5 + --> $DIR/eq_op.rs:33:5 | LL | 1 + 1 == 2; | ^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:32:5 + --> $DIR/eq_op.rs:34:5 | LL | 1 - 1 == 0; | ^^^^^^^^^^ error: equal expressions as operands to `-` - --> $DIR/eq_op.rs:32:5 + --> $DIR/eq_op.rs:34:5 | LL | 1 - 1 == 0; | ^^^^^ error: equal expressions as operands to `-` - --> $DIR/eq_op.rs:34:5 + --> $DIR/eq_op.rs:36:5 | LL | 1 - 1; | ^^^^^ error: equal expressions as operands to `/` - --> $DIR/eq_op.rs:35:5 + --> $DIR/eq_op.rs:37:5 | LL | 1 / 1; | ^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:36:5 + --> $DIR/eq_op.rs:38:5 | LL | true && true; | ^^^^^^^^^^^^ error: equal expressions as operands to `||` - --> $DIR/eq_op.rs:38:5 + --> $DIR/eq_op.rs:40:5 | LL | true || true; | ^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:44:5 + --> $DIR/eq_op.rs:46:5 | LL | a == b && b == a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:45:5 + --> $DIR/eq_op.rs:47:5 | LL | a != b && b != a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:46:5 + --> $DIR/eq_op.rs:48:5 | LL | a < b && b > a; | ^^^^^^^^^^^^^^ error: equal expressions as operands to `&&` - --> $DIR/eq_op.rs:47:5 + --> $DIR/eq_op.rs:49:5 | LL | a <= b && b >= a; | ^^^^^^^^^^^^^^^^ error: equal expressions as operands to `==` - --> $DIR/eq_op.rs:50:5 + --> $DIR/eq_op.rs:52:5 | LL | a == a; | ^^^^^^ error: equal expressions as operands to `/` - --> $DIR/eq_op.rs:60:20 + --> $DIR/eq_op.rs:62:20 | LL | const D: u32 = A / A; | ^^^^^ diff --git a/tests/ui/float_cmp_const.rs b/tests/ui/float_cmp_const.rs index 17ad274bd6df..8f4ad15720b0 100644 --- a/tests/ui/float_cmp_const.rs +++ b/tests/ui/float_cmp_const.rs @@ -1,3 +1,5 @@ +// does not test any rustfixable lints + #![warn(clippy::float_cmp_const)] #![allow(clippy::float_cmp)] #![allow(unused, clippy::no_effect, clippy::unnecessary_operation)] diff --git a/tests/ui/float_cmp_const.stderr b/tests/ui/float_cmp_const.stderr index 48e92fa44ab8..3f2ac9eee9be 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -1,84 +1,84 @@ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:18:5 + --> $DIR/float_cmp_const.rs:20:5 | LL | 1f32 == ONE; | ^^^^^^^^^^^ help: consider comparing them within some error: `(1f32 - ONE).abs() < error` | = note: `-D clippy::float-cmp-const` implied by `-D warnings` note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:18:5 + --> $DIR/float_cmp_const.rs:20:5 | LL | 1f32 == ONE; | ^^^^^^^^^^^ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:19:5 + --> $DIR/float_cmp_const.rs:21:5 | LL | TWO == ONE; | ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() < error` | note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:19:5 + --> $DIR/float_cmp_const.rs:21:5 | LL | TWO == ONE; | ^^^^^^^^^^ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:20:5 + --> $DIR/float_cmp_const.rs:22:5 | LL | TWO != ONE; | ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() > error` | note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:20:5 + --> $DIR/float_cmp_const.rs:22:5 | LL | TWO != ONE; | ^^^^^^^^^^ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:21:5 + --> $DIR/float_cmp_const.rs:23:5 | LL | ONE + ONE == TWO; | ^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(ONE + ONE - TWO).abs() < error` | note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:21:5 + --> $DIR/float_cmp_const.rs:23:5 | LL | ONE + ONE == TWO; | ^^^^^^^^^^^^^^^^ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:23:5 + --> $DIR/float_cmp_const.rs:25:5 | LL | x as f32 == ONE; | ^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(x as f32 - ONE).abs() < error` | note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:23:5 + --> $DIR/float_cmp_const.rs:25:5 | LL | x as f32 == ONE; | ^^^^^^^^^^^^^^^ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:26:5 + --> $DIR/float_cmp_const.rs:28:5 | LL | v == ONE; | ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() < error` | note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:26:5 + --> $DIR/float_cmp_const.rs:28:5 | LL | v == ONE; | ^^^^^^^^ error: strict comparison of f32 or f64 constant - --> $DIR/float_cmp_const.rs:27:5 + --> $DIR/float_cmp_const.rs:29:5 | LL | v != ONE; | ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() > error` | note: std::f32::EPSILON and std::f64::EPSILON are available. - --> $DIR/float_cmp_const.rs:27:5 + --> $DIR/float_cmp_const.rs:29:5 | LL | v != ONE; | ^^^^^^^^ diff --git a/tests/ui/literals.rs b/tests/ui/literals.rs index 7680d4e10d66..20bd872c638c 100644 --- a/tests/ui/literals.rs +++ b/tests/ui/literals.rs @@ -1,4 +1,4 @@ -// tests no rustfixable lints +// does not test any rustfixable lints #![warn(clippy::mixed_case_hex_literals)] #![warn(clippy::zero_prefixed_literal)] From 68277b53349abba3caf2a2923a7ab2a28a2990ef Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:50:23 -0700 Subject: [PATCH 09/26] map_flatten: make it a rustfix test --- tests/ui/map_flatten.fixed | 8 ++++++++ tests/ui/map_flatten.rs | 2 ++ tests/ui/map_flatten.stderr | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/ui/map_flatten.fixed diff --git a/tests/ui/map_flatten.fixed b/tests/ui/map_flatten.fixed new file mode 100644 index 000000000000..51732e02be4f --- /dev/null +++ b/tests/ui/map_flatten.fixed @@ -0,0 +1,8 @@ +// run-rustfix + +#![warn(clippy::all, clippy::pedantic)] +#![allow(clippy::missing_docs_in_private_items)] + +fn main() { + let _: Vec<_> = vec![5_i8; 6].into_iter().flat_map(|x| 0..x).collect(); +} diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs index d0720c419c8b..66137a50ae8d 100644 --- a/tests/ui/map_flatten.rs +++ b/tests/ui/map_flatten.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::all, clippy::pedantic)] #![allow(clippy::missing_docs_in_private_items)] diff --git a/tests/ui/map_flatten.stderr b/tests/ui/map_flatten.stderr index 822d27391f6e..478c7e780d96 100644 --- a/tests/ui/map_flatten.stderr +++ b/tests/ui/map_flatten.stderr @@ -1,5 +1,5 @@ error: called `map(..).flatten()` on an `Iterator`. This is more succinctly expressed by calling `.flat_map(..)` - --> $DIR/map_flatten.rs:5:21 + --> $DIR/map_flatten.rs:7:21 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using flat_map instead: `vec![5_i8; 6].into_iter().flat_map(|x| 0..x)` From 41d7eb7155f15ad96a52055cbc9ba1b65aafaad2 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 04:59:10 -0700 Subject: [PATCH 10/26] mem_discriminant: split test, make rustfixable --- tests/ui/mem_discriminant.fixed | 45 ++++++++++++++++++++++ tests/ui/mem_discriminant.rs | 6 +-- tests/ui/mem_discriminant.stderr | 44 ++++++++------------- tests/ui/mem_discriminant_unfixable.rs | 16 ++++++++ tests/ui/mem_discriminant_unfixable.stderr | 20 ++++++++++ 5 files changed, 99 insertions(+), 32 deletions(-) create mode 100644 tests/ui/mem_discriminant.fixed create mode 100644 tests/ui/mem_discriminant_unfixable.rs create mode 100644 tests/ui/mem_discriminant_unfixable.stderr diff --git a/tests/ui/mem_discriminant.fixed b/tests/ui/mem_discriminant.fixed new file mode 100644 index 000000000000..69a8f286d050 --- /dev/null +++ b/tests/ui/mem_discriminant.fixed @@ -0,0 +1,45 @@ +// run-rustfix + +#![deny(clippy::mem_discriminant_non_enum)] + +use std::mem; + +enum Foo { + One(usize), + Two(u8), +} + +fn main() { + // bad + mem::discriminant(&Some(2)); + mem::discriminant(&None::); + mem::discriminant(&Foo::One(5)); + mem::discriminant(&Foo::Two(5)); + + let ro = &Some(3); + let rro = &ro; + mem::discriminant(ro); + mem::discriminant(*rro); + mem::discriminant(*rro); + + macro_rules! mem_discriminant_but_in_a_macro { + ($param:expr) => { + mem::discriminant($param) + }; + } + + mem_discriminant_but_in_a_macro!(*rro); + + let rrrrro = &&&rro; + mem::discriminant(****rrrrro); + mem::discriminant(****rrrrro); + + // ok + mem::discriminant(&Some(2)); + mem::discriminant(&None::); + mem::discriminant(&Foo::One(5)); + mem::discriminant(&Foo::Two(5)); + mem::discriminant(ro); + mem::discriminant(*rro); + mem::discriminant(****rrrrro); +} diff --git a/tests/ui/mem_discriminant.rs b/tests/ui/mem_discriminant.rs index 81f1628861e3..55db50fcdc73 100644 --- a/tests/ui/mem_discriminant.rs +++ b/tests/ui/mem_discriminant.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![deny(clippy::mem_discriminant_non_enum)] use std::mem; @@ -7,16 +9,12 @@ enum Foo { Two(u8), } -struct A(Foo); - fn main() { // bad - mem::discriminant(&"hello"); mem::discriminant(&&Some(2)); mem::discriminant(&&None::); mem::discriminant(&&Foo::One(5)); mem::discriminant(&&Foo::Two(5)); - mem::discriminant(&A(Foo::One(0))); let ro = &Some(3); let rro = &ro; diff --git a/tests/ui/mem_discriminant.stderr b/tests/ui/mem_discriminant.stderr index 295545406ed4..0420c82abda5 100644 --- a/tests/ui/mem_discriminant.stderr +++ b/tests/ui/mem_discriminant.stderr @@ -1,25 +1,19 @@ -error: calling `mem::discriminant` on non-enum type `&str` +error: calling `mem::discriminant` on non-enum type `&std::option::Option` --> $DIR/mem_discriminant.rs:14:5 | -LL | mem::discriminant(&"hello"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | mem::discriminant(&&Some(2)); + | ^^^^^^^^^^^^^^^^^^---------^ + | | + | help: try dereferencing: `&Some(2)` | note: lint level defined here - --> $DIR/mem_discriminant.rs:1:9 + --> $DIR/mem_discriminant.rs:3:9 | LL | #![deny(clippy::mem_discriminant_non_enum)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: calling `mem::discriminant` on non-enum type `&std::option::Option` - --> $DIR/mem_discriminant.rs:15:5 - | -LL | mem::discriminant(&&Some(2)); - | ^^^^^^^^^^^^^^^^^^---------^ - | | - | help: try dereferencing: `&Some(2)` - error: calling `mem::discriminant` on non-enum type `&std::option::Option` - --> $DIR/mem_discriminant.rs:16:5 + --> $DIR/mem_discriminant.rs:15:5 | LL | mem::discriminant(&&None::); | ^^^^^^^^^^^^^^^^^^------------^ @@ -27,7 +21,7 @@ LL | mem::discriminant(&&None::); | help: try dereferencing: `&None::` error: calling `mem::discriminant` on non-enum type `&Foo` - --> $DIR/mem_discriminant.rs:17:5 + --> $DIR/mem_discriminant.rs:16:5 | LL | mem::discriminant(&&Foo::One(5)); | ^^^^^^^^^^^^^^^^^^-------------^ @@ -35,21 +29,15 @@ LL | mem::discriminant(&&Foo::One(5)); | help: try dereferencing: `&Foo::One(5)` error: calling `mem::discriminant` on non-enum type `&Foo` - --> $DIR/mem_discriminant.rs:18:5 + --> $DIR/mem_discriminant.rs:17:5 | LL | mem::discriminant(&&Foo::Two(5)); | ^^^^^^^^^^^^^^^^^^-------------^ | | | help: try dereferencing: `&Foo::Two(5)` -error: calling `mem::discriminant` on non-enum type `A` - --> $DIR/mem_discriminant.rs:19:5 - | -LL | mem::discriminant(&A(Foo::One(0))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - error: calling `mem::discriminant` on non-enum type `&std::option::Option` - --> $DIR/mem_discriminant.rs:23:5 + --> $DIR/mem_discriminant.rs:21:5 | LL | mem::discriminant(&ro); | ^^^^^^^^^^^^^^^^^^---^ @@ -57,7 +45,7 @@ LL | mem::discriminant(&ro); | help: try dereferencing: `ro` error: calling `mem::discriminant` on non-enum type `&std::option::Option` - --> $DIR/mem_discriminant.rs:24:5 + --> $DIR/mem_discriminant.rs:22:5 | LL | mem::discriminant(rro); | ^^^^^^^^^^^^^^^^^^---^ @@ -65,7 +53,7 @@ LL | mem::discriminant(rro); | help: try dereferencing: `*rro` error: calling `mem::discriminant` on non-enum type `&&std::option::Option` - --> $DIR/mem_discriminant.rs:25:5 + --> $DIR/mem_discriminant.rs:23:5 | LL | mem::discriminant(&rro); | ^^^^^^^^^^^^^^^^^^----^ @@ -73,7 +61,7 @@ LL | mem::discriminant(&rro); | help: try dereferencing: `*rro` error: calling `mem::discriminant` on non-enum type `&&std::option::Option` - --> $DIR/mem_discriminant.rs:29:13 + --> $DIR/mem_discriminant.rs:27:13 | LL | mem::discriminant($param) | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -85,7 +73,7 @@ LL | mem_discriminant_but_in_a_macro!(&rro); | in this macro invocation error: calling `mem::discriminant` on non-enum type `&&&&&std::option::Option` - --> $DIR/mem_discriminant.rs:36:5 + --> $DIR/mem_discriminant.rs:34:5 | LL | mem::discriminant(&rrrrro); | ^^^^^^^^^^^^^^^^^^-------^ @@ -93,12 +81,12 @@ LL | mem::discriminant(&rrrrro); | help: try dereferencing: `****rrrrro` error: calling `mem::discriminant` on non-enum type `&&&std::option::Option` - --> $DIR/mem_discriminant.rs:37:5 + --> $DIR/mem_discriminant.rs:35:5 | LL | mem::discriminant(*rrrrro); | ^^^^^^^^^^^^^^^^^^-------^ | | | help: try dereferencing: `****rrrrro` -error: aborting due to 12 previous errors +error: aborting due to 10 previous errors diff --git a/tests/ui/mem_discriminant_unfixable.rs b/tests/ui/mem_discriminant_unfixable.rs new file mode 100644 index 000000000000..e245d3257d55 --- /dev/null +++ b/tests/ui/mem_discriminant_unfixable.rs @@ -0,0 +1,16 @@ +#![deny(clippy::mem_discriminant_non_enum)] + +use std::mem; + +enum Foo { + One(usize), + Two(u8), +} + +struct A(Foo); + +fn main() { + // bad + mem::discriminant(&"hello"); + mem::discriminant(&A(Foo::One(0))); +} diff --git a/tests/ui/mem_discriminant_unfixable.stderr b/tests/ui/mem_discriminant_unfixable.stderr new file mode 100644 index 000000000000..f12a9b2d934d --- /dev/null +++ b/tests/ui/mem_discriminant_unfixable.stderr @@ -0,0 +1,20 @@ +error: calling `mem::discriminant` on non-enum type `&str` + --> $DIR/mem_discriminant_unfixable.rs:14:5 + | +LL | mem::discriminant(&"hello"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: lint level defined here + --> $DIR/mem_discriminant_unfixable.rs:1:9 + | +LL | #![deny(clippy::mem_discriminant_non_enum)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: calling `mem::discriminant` on non-enum type `A` + --> $DIR/mem_discriminant_unfixable.rs:15:5 + | +LL | mem::discriminant(&A(Foo::One(0))); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + From 693b5ecd4ddf24d25bff6a0b2490041f0cc31275 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 05:03:28 -0700 Subject: [PATCH 11/26] needless_borrow: allow other lints, make fixable --- tests/ui/needless_borrow.fixed | 62 +++++++++++++++++++++++++++++++++ tests/ui/needless_borrow.rs | 4 ++- tests/ui/needless_borrow.stderr | 24 +++---------- 3 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 tests/ui/needless_borrow.fixed diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed new file mode 100644 index 000000000000..50f9b7c7ba63 --- /dev/null +++ b/tests/ui/needless_borrow.fixed @@ -0,0 +1,62 @@ +// run-rustfix + +#![allow(clippy::needless_borrowed_reference)] + +#[allow(clippy::trivially_copy_pass_by_ref)] +fn x(y: &i32) -> i32 { + *y +} + +#[warn(clippy::all, clippy::needless_borrow)] +#[allow(unused_variables)] +fn main() { + let a = 5; + let b = x(&a); + let c = x(&a); + let s = &String::from("hi"); + let s_ident = f(&s); // should not error, because `&String` implements Copy, but `String` does not + let g_val = g(&Vec::new()); // should not error, because `&Vec` derefs to `&[T]` + let vec = Vec::new(); + let vec_val = g(&vec); // should not error, because `&Vec` derefs to `&[T]` + h(&"foo"); // should not error, because the `&&str` is required, due to `&Trait` + if let Some(cake) = Some(&5) {} + let garbl = match 42 { + 44 => &a, + 45 => { + println!("foo"); + &&a // FIXME: this should lint, too + }, + 46 => &a, + _ => panic!(), + }; +} + +fn f(y: &T) -> T { + *y +} + +fn g(y: &[u8]) -> u8 { + y[0] +} + +trait Trait {} + +impl<'a> Trait for &'a str {} + +fn h(_: &dyn Trait) {} +#[warn(clippy::needless_borrow)] +#[allow(dead_code)] +fn issue_1432() { + let mut v = Vec::::new(); + let _ = v.iter_mut().filter(|&ref a| a.is_empty()); + let _ = v.iter().filter(|&a| a.is_empty()); + + let _ = v.iter().filter(|&a| a.is_empty()); +} + +#[allow(dead_code)] +#[warn(clippy::needless_borrow)] +#[derive(Debug)] +enum Foo<'a> { + Str(&'a str), +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index d4ac2b89854d..8677b957e4c3 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -1,4 +1,6 @@ -use std::borrow::Cow; +// run-rustfix + +#![allow(clippy::needless_borrowed_reference)] #[allow(clippy::trivially_copy_pass_by_ref)] fn x(y: &i32) -> i32 { diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index 40744160f659..49df9cd072b3 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -1,5 +1,5 @@ error: this expression borrows a reference that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:13:15 + --> $DIR/needless_borrow.rs:15:15 | LL | let c = x(&&a); | ^^^ help: change this to: `&a` @@ -7,36 +7,22 @@ LL | let c = x(&&a); = note: `-D clippy::needless-borrow` implied by `-D warnings` error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:20:17 + --> $DIR/needless_borrow.rs:22:17 | LL | if let Some(ref cake) = Some(&5) {} | ^^^^^^^^ help: change this to: `cake` error: this expression borrows a reference that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:27:15 + --> $DIR/needless_borrow.rs:29:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` -error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrow.rs:49:34 - | -LL | let _ = v.iter_mut().filter(|&ref a| a.is_empty()); - | ^^^^^^ help: try removing the `&ref` part and just keep: `a` - | - = note: `-D clippy::needless-borrowed-reference` implied by `-D warnings` - -error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrow.rs:50:30 - | -LL | let _ = v.iter().filter(|&ref a| a.is_empty()); - | ^^^^^^ help: try removing the `&ref` part and just keep: `a` - error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:50:31 + --> $DIR/needless_borrow.rs:52:31 | LL | let _ = v.iter().filter(|&ref a| a.is_empty()); | ^^^^^ help: change this to: `a` -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors From e1bb188658f5099e029580987a3762622cabc4a4 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 05:19:09 -0700 Subject: [PATCH 12/26] needless_borrowed_ref: fix false positive, make rustfixable --- clippy_lints/src/needless_borrowed_ref.rs | 17 +++++++-- tests/ui/needless_borrowed_ref.fixed | 45 +++++++++++++++++++++++ tests/ui/needless_borrowed_ref.rs | 2 + tests/ui/needless_borrowed_ref.stderr | 22 +---------- 4 files changed, 62 insertions(+), 24 deletions(-) create mode 100644 tests/ui/needless_borrowed_ref.fixed diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index 714c746f68d6..9afa67dadd14 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -2,9 +2,9 @@ //! //! This lint is **warn** by default -use crate::utils::{snippet, span_lint_and_then}; +use crate::utils::{snippet_with_applicability, span_lint_and_then}; use if_chain::if_chain; -use rustc::hir::{BindingAnnotation, MutImmutable, Pat, PatKind}; +use rustc::hir::{BindingAnnotation, MutImmutable, Node, Pat, PatKind}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; @@ -65,16 +65,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrowedRef { // Check sub_pat got a `ref` keyword (excluding `ref mut`). if let PatKind::Binding(BindingAnnotation::Ref, .., spanned_name, _) = sub_pat.node; + let parent_id = cx.tcx.hir().get_parent_node(pat.hir_id); + if let Some(parent_node) = cx.tcx.hir().find(parent_id); then { + // do not recurse within patterns, as they may have other references + // XXXManishearth we can relax this constraint if we only check patterns + // with a single ref pattern inside them + if let Node::Pat(_) = parent_node { + return; + } + let mut applicability = Applicability::MachineApplicable; span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, pat.span, "this pattern takes a reference on something that is being de-referenced", |db| { - let hint = snippet(cx, spanned_name.span, "..").into_owned(); + let hint = snippet_with_applicability(cx, spanned_name.span, "..", &mut applicability).into_owned(); db.span_suggestion( pat.span, "try removing the `&ref` part and just keep", hint, - Applicability::MachineApplicable, // snippet + applicability, ); }); } diff --git a/tests/ui/needless_borrowed_ref.fixed b/tests/ui/needless_borrowed_ref.fixed new file mode 100644 index 000000000000..a0937a2c5f62 --- /dev/null +++ b/tests/ui/needless_borrowed_ref.fixed @@ -0,0 +1,45 @@ +// run-rustfix + +#[warn(clippy::needless_borrowed_reference)] +#[allow(unused_variables)] +fn main() { + let mut v = Vec::::new(); + let _ = v.iter_mut().filter(|a| a.is_empty()); + // ^ should be linted + + let var = 3; + let thingy = Some(&var); + if let Some(&ref v) = thingy { + // ^ should be linted + } + + let mut var2 = 5; + let thingy2 = Some(&mut var2); + if let Some(&mut ref mut v) = thingy2 { + // ^ should **not** be linted + // v is borrowed as mutable. + *v = 10; + } + if let Some(&mut ref v) = thingy2 { + // ^ should **not** be linted + // here, v is borrowed as immutable. + // can't do that: + //*v = 15; + } +} + +#[allow(dead_code)] +enum Animal { + Cat(u64), + Dog(u64), +} + +#[allow(unused_variables)] +#[allow(dead_code)] +fn foo(a: &Animal, b: &Animal) { + match (a, b) { + (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref' + // ^ and ^ should **not** be linted + (&Animal::Dog(ref a), &Animal::Dog(_)) => (), // ^ should **not** be linted + } +} diff --git a/tests/ui/needless_borrowed_ref.rs b/tests/ui/needless_borrowed_ref.rs index c76f4de9b07a..500ac448f0d5 100644 --- a/tests/ui/needless_borrowed_ref.rs +++ b/tests/ui/needless_borrowed_ref.rs @@ -1,3 +1,5 @@ +// run-rustfix + #[warn(clippy::needless_borrowed_reference)] #[allow(unused_variables)] fn main() { diff --git a/tests/ui/needless_borrowed_ref.stderr b/tests/ui/needless_borrowed_ref.stderr index 1b8067f1d6d6..0a5cfb3db0b1 100644 --- a/tests/ui/needless_borrowed_ref.stderr +++ b/tests/ui/needless_borrowed_ref.stderr @@ -1,28 +1,10 @@ error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrowed_ref.rs:5:34 + --> $DIR/needless_borrowed_ref.rs:7:34 | LL | let _ = v.iter_mut().filter(|&ref a| a.is_empty()); | ^^^^^^ help: try removing the `&ref` part and just keep: `a` | = note: `-D clippy::needless-borrowed-reference` implied by `-D warnings` -error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrowed_ref.rs:10:17 - | -LL | if let Some(&ref v) = thingy { - | ^^^^^^ help: try removing the `&ref` part and just keep: `v` - -error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrowed_ref.rs:39:27 - | -LL | (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref' - | ^^^^^^ help: try removing the `&ref` part and just keep: `k` - -error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrowed_ref.rs:39:38 - | -LL | (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref' - | ^^^^^^ help: try removing the `&ref` part and just keep: `k` - -error: aborting due to 4 previous errors +error: aborting due to previous error From a4e02cdc0e6b5729165744f19ecf30d8284bd4e9 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 05:25:31 -0700 Subject: [PATCH 13/26] needless_collect: fix suggestion, make test rustfixable --- clippy_lints/src/loops.rs | 9 +++++++-- tests/ui/needless_collect.fixed | 21 +++++++++++++++++++++ tests/ui/needless_collect.rs | 4 ++++ tests/ui/needless_collect.stderr | 10 +++++----- 4 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 tests/ui/needless_collect.fixed diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 94328a2b5e3d..0a315fe48b5d 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -2427,12 +2427,17 @@ fn check_needless_collect<'a, 'tcx>(expr: &'tcx Expr, cx: &LateContext<'a, 'tcx> let contains_arg = snippet(cx, args[1].span, "??"); let span = shorten_needless_collect_span(expr); span_lint_and_then(cx, NEEDLESS_COLLECT, span, NEEDLESS_COLLECT_MSG, |db| { + let (arg, pred) = if contains_arg.starts_with('&') { + ("x", &contains_arg[1..]) + } else { + ("&x", &*contains_arg) + }; db.span_suggestion( span, "replace with", format!( - ".any(|&x| x == {})", - if contains_arg.starts_with('&') { &contains_arg[1..] } else { &contains_arg } + ".any(|{}| x == {})", + arg, pred ), Applicability::MachineApplicable, ); diff --git a/tests/ui/needless_collect.fixed b/tests/ui/needless_collect.fixed new file mode 100644 index 000000000000..b4227eaf2f8b --- /dev/null +++ b/tests/ui/needless_collect.fixed @@ -0,0 +1,21 @@ +// run-rustfix + +#![allow(unused, clippy::suspicious_map)] + +use std::collections::{BTreeSet, HashMap, HashSet}; + +#[warn(clippy::needless_collect)] +#[allow(unused_variables, clippy::iter_cloned_collect)] +fn main() { + let sample = [1; 5]; + let len = sample.iter().count(); + if sample.iter().next().is_none() { + // Empty + } + sample.iter().cloned().any(|x| x == 1); + sample.iter().map(|x| (x, x)).count(); + // Notice the `HashSet`--this should not be linted + sample.iter().collect::>().len(); + // Neither should this + sample.iter().collect::>().len(); +} diff --git a/tests/ui/needless_collect.rs b/tests/ui/needless_collect.rs index d4815f60f513..7ee603afeb07 100644 --- a/tests/ui/needless_collect.rs +++ b/tests/ui/needless_collect.rs @@ -1,3 +1,7 @@ +// run-rustfix + +#![allow(unused, clippy::suspicious_map)] + use std::collections::{BTreeSet, HashMap, HashSet}; #[warn(clippy::needless_collect)] diff --git a/tests/ui/needless_collect.stderr b/tests/ui/needless_collect.stderr index 684c501c5b53..8884c8e16129 100644 --- a/tests/ui/needless_collect.stderr +++ b/tests/ui/needless_collect.stderr @@ -1,5 +1,5 @@ error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:7:28 + --> $DIR/needless_collect.rs:11:28 | LL | let len = sample.iter().collect::>().len(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()` @@ -7,19 +7,19 @@ LL | let len = sample.iter().collect::>().len(); = note: `-D clippy::needless-collect` implied by `-D warnings` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:8:21 + --> $DIR/needless_collect.rs:12:21 | LL | if sample.iter().collect::>().is_empty() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.next().is_none()` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:11:27 + --> $DIR/needless_collect.rs:15:27 | LL | sample.iter().cloned().collect::>().contains(&1); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|&x| x == 1)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.any(|x| x == 1)` error: avoid using `collect()` when not needed - --> $DIR/needless_collect.rs:12:34 + --> $DIR/needless_collect.rs:16:34 | LL | sample.iter().map(|x| (x, x)).collect::>().len(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `.count()` From 5aecb172d92bf37b0639667f21d42cd20e364b34 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 05:28:19 -0700 Subject: [PATCH 14/26] needless_return: add allow()s to test, make rustfixable --- tests/ui/needless_return.fixed | 78 +++++++++++++++++++++++++++++++++ tests/ui/needless_return.rs | 4 ++ tests/ui/needless_return.stderr | 24 +++++----- 3 files changed, 94 insertions(+), 12 deletions(-) create mode 100644 tests/ui/needless_return.fixed diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed new file mode 100644 index 000000000000..70af5c196141 --- /dev/null +++ b/tests/ui/needless_return.fixed @@ -0,0 +1,78 @@ +// run-rustfix + +#![allow(unused, clippy::needless_bool, clippy::match_bool)] +#![allow(clippy::if_same_then_else, clippy::single_match)] +#![warn(clippy::needless_return)] + +macro_rules! the_answer { + () => { + 42 + }; +} + +fn test_end_of_fn() -> bool { + if true { + // no error! + return true; + } + true +} + +fn test_no_semicolon() -> bool { + true +} + +fn test_if_block() -> bool { + if true { + true + } else { + false + } +} + +fn test_match(x: bool) -> bool { + match x { + true => false, + false => { + true + }, + } +} + +fn test_closure() { + let _ = || { + true + }; + let _ = || true; +} + +fn test_macro_call() -> i32 { + return the_answer!(); +} + +fn test_void_fun() { + +} + +fn test_void_if_fun(b: bool) { + if b { + + } else { + + } +} + +fn test_void_match(x: u32) { + match x { + 0 => (), + _ => {}, + } +} + +fn main() { + let _ = test_end_of_fn(); + let _ = test_no_semicolon(); + let _ = test_if_block(); + let _ = test_match(true); + test_closure(); +} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index e9f064ea3efe..a1f8321ac6e7 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -1,3 +1,7 @@ +// run-rustfix + +#![allow(unused, clippy::needless_bool, clippy::match_bool)] +#![allow(clippy::if_same_then_else, clippy::single_match)] #![warn(clippy::needless_return)] macro_rules! the_answer { diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index ee700ab84086..b29c47f7b7a7 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -1,5 +1,5 @@ error: unneeded return statement - --> $DIR/needless_return.rs:14:5 + --> $DIR/needless_return.rs:18:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` @@ -7,67 +7,67 @@ LL | return true; = note: `-D clippy::needless-return` implied by `-D warnings` error: unneeded return statement - --> $DIR/needless_return.rs:18:5 + --> $DIR/needless_return.rs:22:5 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement - --> $DIR/needless_return.rs:23:9 + --> $DIR/needless_return.rs:27:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement - --> $DIR/needless_return.rs:25:9 + --> $DIR/needless_return.rs:29:9 | LL | return false; | ^^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded return statement - --> $DIR/needless_return.rs:31:17 + --> $DIR/needless_return.rs:35:17 | LL | true => return false, | ^^^^^^^^^^^^ help: remove `return`: `false` error: unneeded return statement - --> $DIR/needless_return.rs:33:13 + --> $DIR/needless_return.rs:37:13 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement - --> $DIR/needless_return.rs:40:9 + --> $DIR/needless_return.rs:44:9 | LL | return true; | ^^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement - --> $DIR/needless_return.rs:42:16 + --> $DIR/needless_return.rs:46:16 | LL | let _ = || return true; | ^^^^^^^^^^^ help: remove `return`: `true` error: unneeded return statement - --> $DIR/needless_return.rs:50:5 + --> $DIR/needless_return.rs:54:5 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded return statement - --> $DIR/needless_return.rs:55:9 + --> $DIR/needless_return.rs:59:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded return statement - --> $DIR/needless_return.rs:57:9 + --> $DIR/needless_return.rs:61:9 | LL | return; | ^^^^^^^ help: remove `return` error: unneeded return statement - --> $DIR/needless_return.rs:64:14 + --> $DIR/needless_return.rs:68:14 | LL | _ => return, | ^^^^^^ help: replace `return` with an empty block: `{}` From 89230f6633aaef40d63de2e4029ac98033c211e8 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 05:39:56 -0700 Subject: [PATCH 15/26] non_copy_const: remove incorrect suggestion --- clippy_lints/src/non_copy_const.rs | 10 +---- tests/ui/non_copy_const.stderr | 66 ++++++++++-------------------- 2 files changed, 23 insertions(+), 53 deletions(-) diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 992baa05e78e..90a31f710f4d 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -10,7 +10,6 @@ use rustc::lint::{LateContext, LateLintPass, Lint, LintArray, LintPass}; use rustc::ty::adjustment::Adjust; use rustc::ty::{Ty, TypeFlags}; use rustc::{declare_lint_pass, declare_tool_lint}; -use rustc_errors::Applicability; use rustc_typeck::hir_ty_to_ty; use syntax_pos::{InnerSpan, Span, DUMMY_SP}; @@ -125,16 +124,11 @@ fn verify_ty_bound<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, source: S match source { Source::Item { .. } => { let const_kw_span = span.from_inner(InnerSpan::new(0, 5)); - db.span_suggestion( - const_kw_span, - "make this a static item", - "static".to_string(), - Applicability::MachineApplicable, - ); + db.span_label(const_kw_span, "make this a static item (maybe with lazy_static)"); }, Source::Assoc { ty: ty_span, .. } => { if ty.flags.contains(TypeFlags::HAS_FREE_LOCAL_NAMES) { - db.span_help(ty_span, &format!("consider requiring `{}` to be `Copy`", ty)); + db.span_label(ty_span, &format!("consider requiring `{}` to be `Copy`", ty)); } }, Source::Expr { .. } => { diff --git a/tests/ui/non_copy_const.stderr b/tests/ui/non_copy_const.stderr index 2f325f9e3dfd..0568386f889b 100644 --- a/tests/ui/non_copy_const.stderr +++ b/tests/ui/non_copy_const.stderr @@ -4,7 +4,7 @@ error: a const item should never be interior mutable LL | const ATOMIC: AtomicUsize = AtomicUsize::new(5); //~ ERROR interior mutable | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | help: make this a static item: `static` + | make this a static item (maybe with lazy_static) | = note: `#[deny(clippy::declare_interior_mutable_const)]` on by default @@ -14,7 +14,7 @@ error: a const item should never be interior mutable LL | const CELL: Cell = Cell::new(6); //~ ERROR interior mutable | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | help: make this a static item: `static` + | make this a static item (maybe with lazy_static) error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:11:1 @@ -22,7 +22,7 @@ error: a const item should never be interior mutable LL | const ATOMIC_TUPLE: ([AtomicUsize; 1], Vec, u8) = ([ATOMIC], Vec::new(), 7); | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | help: make this a static item: `static` + | make this a static item (maybe with lazy_static) error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:16:9 @@ -43,37 +43,25 @@ error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:44:5 | LL | const INPUT: T; - | ^^^^^^^^^^^^^^^ - | -help: consider requiring `T` to be `Copy` - --> $DIR/non_copy_const.rs:44:18 - | -LL | const INPUT: T; - | ^ + | ^^^^^^^^^^^^^-^ + | | + | consider requiring `T` to be `Copy` error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:47:5 | LL | const ASSOC: Self::NonCopyType; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: consider requiring `>::NonCopyType` to be `Copy` - --> $DIR/non_copy_const.rs:47:18 - | -LL | const ASSOC: Self::NonCopyType; - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^-----------------^ + | | + | consider requiring `>::NonCopyType` to be `Copy` error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:51:5 | LL | const AN_INPUT: T = Self::INPUT; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: consider requiring `T` to be `Copy` - --> $DIR/non_copy_const.rs:51:21 - | -LL | const AN_INPUT: T = Self::INPUT; - | ^ + | ^^^^^^^^^^^^^^^^-^^^^^^^^^^^^^^^ + | | + | consider requiring `T` to be `Copy` error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:16:9 @@ -88,13 +76,9 @@ error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:60:5 | LL | const SELF_2: Self; - | ^^^^^^^^^^^^^^^^^^^ - | -help: consider requiring `Self` to be `Copy` - --> $DIR/non_copy_const.rs:60:19 - | -LL | const SELF_2: Self; - | ^^^^ + | ^^^^^^^^^^^^^^----^ + | | + | consider requiring `Self` to be `Copy` error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:81:5 @@ -106,25 +90,17 @@ error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:84:5 | LL | const U_SELF: U = U::SELF_2; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: consider requiring `U` to be `Copy` - --> $DIR/non_copy_const.rs:84:19 - | -LL | const U_SELF: U = U::SELF_2; - | ^ + | ^^^^^^^^^^^^^^-^^^^^^^^^^^^^ + | | + | consider requiring `U` to be `Copy` error: a const item should never be interior mutable --> $DIR/non_copy_const.rs:87:5 | LL | const T_ASSOC: T::NonCopyType = T::ASSOC; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: consider requiring `>::NonCopyType` to be `Copy` - --> $DIR/non_copy_const.rs:87:20 - | -LL | const T_ASSOC: T::NonCopyType = T::ASSOC; - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^--------------^^^^^^^^^^^^ + | | + | consider requiring `>::NonCopyType` to be `Copy` error: a const item with interior mutability should not be borrowed --> $DIR/non_copy_const.rs:94:5 From e25beabdfeaa3a5632c5e57bb318e2bf9b1b6840 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 08:39:45 -0700 Subject: [PATCH 16/26] map_unit_fn: rename tests to fixable --- ...it_fn.rs => option_map_unit_fn_fixable.rs} | 0 ...derr => option_map_unit_fn_fixable.stderr} | 50 +++++++++---------- ...it_fn.rs => result_map_unit_fn_fixable.rs} | 0 ...derr => result_map_unit_fn_fixable.stderr} | 46 ++++++++--------- 4 files changed, 48 insertions(+), 48 deletions(-) rename tests/ui/{option_map_unit_fn.rs => option_map_unit_fn_fixable.rs} (100%) rename tests/ui/{option_map_unit_fn.stderr => option_map_unit_fn_fixable.stderr} (86%) rename tests/ui/{result_map_unit_fn.rs => result_map_unit_fn_fixable.rs} (100%) rename tests/ui/{result_map_unit_fn.stderr => result_map_unit_fn_fixable.stderr} (86%) diff --git a/tests/ui/option_map_unit_fn.rs b/tests/ui/option_map_unit_fn_fixable.rs similarity index 100% rename from tests/ui/option_map_unit_fn.rs rename to tests/ui/option_map_unit_fn_fixable.rs diff --git a/tests/ui/option_map_unit_fn.stderr b/tests/ui/option_map_unit_fn_fixable.stderr similarity index 86% rename from tests/ui/option_map_unit_fn.stderr rename to tests/ui/option_map_unit_fn_fixable.stderr index 16e355ad0b2d..4b619495b7b9 100644 --- a/tests/ui/option_map_unit_fn.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -1,5 +1,5 @@ error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:32:5 + --> $DIR/option_map_unit_fn_fixable.rs:32:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -9,7 +9,7 @@ LL | x.field.map(do_nothing); = note: `-D clippy::option-map-unit-fn` implied by `-D warnings` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:34:5 + --> $DIR/option_map_unit_fn_fixable.rs:34:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -17,7 +17,7 @@ LL | x.field.map(do_nothing); | help: try this: `if let Some(x_field) = x.field { do_nothing(...) }` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:36:5 + --> $DIR/option_map_unit_fn_fixable.rs:36:5 | LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- @@ -25,7 +25,7 @@ LL | x.field.map(diverge); | help: try this: `if let Some(x_field) = x.field { diverge(...) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:42:5 + --> $DIR/option_map_unit_fn_fixable.rs:42:5 | LL | x.field.map(|value| x.do_option_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -33,7 +33,7 @@ LL | x.field.map(|value| x.do_option_nothing(value + captured)); | help: try this: `if let Some(value) = x.field { x.do_option_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:44:5 + --> $DIR/option_map_unit_fn_fixable.rs:44:5 | LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -41,7 +41,7 @@ LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { x.do_option_plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:47:5 + --> $DIR/option_map_unit_fn_fixable.rs:47:5 | LL | x.field.map(|value| do_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -49,7 +49,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:49:5 + --> $DIR/option_map_unit_fn_fixable.rs:49:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -57,7 +57,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:51:5 + --> $DIR/option_map_unit_fn_fixable.rs:51:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:53:5 + --> $DIR/option_map_unit_fn_fixable.rs:53:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -73,7 +73,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:56:5 + --> $DIR/option_map_unit_fn_fixable.rs:56:5 | LL | x.field.map(|value| diverge(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -81,7 +81,7 @@ LL | x.field.map(|value| diverge(value + captured)); | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:58:5 + --> $DIR/option_map_unit_fn_fixable.rs:58:5 | LL | x.field.map(|value| { diverge(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -89,7 +89,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:60:5 + --> $DIR/option_map_unit_fn_fixable.rs:60:5 | LL | x.field.map(|value| { diverge(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -97,7 +97,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:62:5 + --> $DIR/option_map_unit_fn_fixable.rs:62:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -105,7 +105,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:67:5 + --> $DIR/option_map_unit_fn_fixable.rs:67:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -113,7 +113,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { let y = plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:69:5 + --> $DIR/option_map_unit_fn_fixable.rs:69:5 | LL | x.field.map(|value| { plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -121,7 +121,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:71:5 + --> $DIR/option_map_unit_fn_fixable.rs:71:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -129,7 +129,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:74:5 + --> $DIR/option_map_unit_fn_fixable.rs:74:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -137,7 +137,7 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); | help: try this: `if let Some(ref value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:77:5 + --> $DIR/option_map_unit_fn_fixable.rs:77:5 | LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -145,7 +145,7 @@ LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); | help: try this: `if let Some(value) = x.field { ... }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:79:5 + --> $DIR/option_map_unit_fn_fixable.rs:79:5 | LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -153,7 +153,7 @@ LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) | help: try this: `if let Some(value) = x.field { ... }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:83:5 + --> $DIR/option_map_unit_fn_fixable.rs:83:5 | LL | x.field.map(|value| { | _____^ @@ -167,7 +167,7 @@ LL | || }); | error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn.rs:87:5 + --> $DIR/option_map_unit_fn_fixable.rs:87:5 | LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -175,7 +175,7 @@ LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); | help: try this: `if let Some(value) = x.field { ... }` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:90:5 + --> $DIR/option_map_unit_fn_fixable.rs:90:5 | LL | Some(42).map(diverge); | ^^^^^^^^^^^^^^^^^^^^^- @@ -183,7 +183,7 @@ LL | Some(42).map(diverge); | help: try this: `if let Some(_) = Some(42) { diverge(...) }` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:91:5 + --> $DIR/option_map_unit_fn_fixable.rs:91:5 | LL | "12".parse::().ok().map(diverge); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -191,7 +191,7 @@ LL | "12".parse::().ok().map(diverge); | help: try this: `if let Some(_) = "12".parse::().ok() { diverge(...) }` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:92:5 + --> $DIR/option_map_unit_fn_fixable.rs:92:5 | LL | Some(plus_one(1)).map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -199,7 +199,7 @@ LL | Some(plus_one(1)).map(do_nothing); | help: try this: `if let Some(_) = Some(plus_one(1)) { do_nothing(...) }` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn.rs:96:5 + --> $DIR/option_map_unit_fn_fixable.rs:96:5 | LL | y.map(do_nothing); | ^^^^^^^^^^^^^^^^^- diff --git a/tests/ui/result_map_unit_fn.rs b/tests/ui/result_map_unit_fn_fixable.rs similarity index 100% rename from tests/ui/result_map_unit_fn.rs rename to tests/ui/result_map_unit_fn_fixable.rs diff --git a/tests/ui/result_map_unit_fn.stderr b/tests/ui/result_map_unit_fn_fixable.stderr similarity index 86% rename from tests/ui/result_map_unit_fn.stderr rename to tests/ui/result_map_unit_fn_fixable.stderr index 9f9025152e24..29d1bd6dc18c 100644 --- a/tests/ui/result_map_unit_fn.stderr +++ b/tests/ui/result_map_unit_fn_fixable.stderr @@ -1,5 +1,5 @@ error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn.rs:34:5 + --> $DIR/result_map_unit_fn_fixable.rs:34:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -9,7 +9,7 @@ LL | x.field.map(do_nothing); = note: `-D clippy::result-map-unit-fn` implied by `-D warnings` error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn.rs:36:5 + --> $DIR/result_map_unit_fn_fixable.rs:36:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -17,7 +17,7 @@ LL | x.field.map(do_nothing); | help: try this: `if let Ok(x_field) = x.field { do_nothing(...) }` error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn.rs:38:5 + --> $DIR/result_map_unit_fn_fixable.rs:38:5 | LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- @@ -25,7 +25,7 @@ LL | x.field.map(diverge); | help: try this: `if let Ok(x_field) = x.field { diverge(...) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:44:5 + --> $DIR/result_map_unit_fn_fixable.rs:44:5 | LL | x.field.map(|value| x.do_result_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -33,7 +33,7 @@ LL | x.field.map(|value| x.do_result_nothing(value + captured)); | help: try this: `if let Ok(value) = x.field { x.do_result_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:46:5 + --> $DIR/result_map_unit_fn_fixable.rs:46:5 | LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -41,7 +41,7 @@ LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); | help: try this: `if let Ok(value) = x.field { x.do_result_plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:49:5 + --> $DIR/result_map_unit_fn_fixable.rs:49:5 | LL | x.field.map(|value| do_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -49,7 +49,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:51:5 + --> $DIR/result_map_unit_fn_fixable.rs:51:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -57,7 +57,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:53:5 + --> $DIR/result_map_unit_fn_fixable.rs:53:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:55:5 + --> $DIR/result_map_unit_fn_fixable.rs:55:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -73,7 +73,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:58:5 + --> $DIR/result_map_unit_fn_fixable.rs:58:5 | LL | x.field.map(|value| diverge(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -81,7 +81,7 @@ LL | x.field.map(|value| diverge(value + captured)); | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:60:5 + --> $DIR/result_map_unit_fn_fixable.rs:60:5 | LL | x.field.map(|value| { diverge(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -89,7 +89,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:62:5 + --> $DIR/result_map_unit_fn_fixable.rs:62:5 | LL | x.field.map(|value| { diverge(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -97,7 +97,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:64:5 + --> $DIR/result_map_unit_fn_fixable.rs:64:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -105,7 +105,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:69:5 + --> $DIR/result_map_unit_fn_fixable.rs:69:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -113,7 +113,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | help: try this: `if let Ok(value) = x.field { let y = plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:71:5 + --> $DIR/result_map_unit_fn_fixable.rs:71:5 | LL | x.field.map(|value| { plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -121,7 +121,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:73:5 + --> $DIR/result_map_unit_fn_fixable.rs:73:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -129,7 +129,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:76:5 + --> $DIR/result_map_unit_fn_fixable.rs:76:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -137,7 +137,7 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); | help: try this: `if let Ok(ref value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:79:5 + --> $DIR/result_map_unit_fn_fixable.rs:79:5 | LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -145,7 +145,7 @@ LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); | help: try this: `if let Ok(value) = x.field { ... }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:81:5 + --> $DIR/result_map_unit_fn_fixable.rs:81:5 | LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -153,7 +153,7 @@ LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) | help: try this: `if let Ok(value) = x.field { ... }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:85:5 + --> $DIR/result_map_unit_fn_fixable.rs:85:5 | LL | x.field.map(|value| { | _____^ @@ -167,7 +167,7 @@ LL | || }); | error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn.rs:89:5 + --> $DIR/result_map_unit_fn_fixable.rs:89:5 | LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -175,7 +175,7 @@ LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); | help: try this: `if let Ok(value) = x.field { ... }` error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn.rs:93:5 + --> $DIR/result_map_unit_fn_fixable.rs:93:5 | LL | "12".parse::().map(diverge); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -183,7 +183,7 @@ LL | "12".parse::().map(diverge); | help: try this: `if let Ok(_) = "12".parse::() { diverge(...) }` error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn.rs:99:5 + --> $DIR/result_map_unit_fn_fixable.rs:99:5 | LL | y.map(do_nothing); | ^^^^^^^^^^^^^^^^^- From 1b226853258a923911d03bc73dc68c16b7626913 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 08:48:09 -0700 Subject: [PATCH 17/26] option_map_unit_fn: Split into fixable/unfixable --- tests/ui/option_map_unit_fn_fixable.rs | 25 +------ tests/ui/option_map_unit_fn_fixable.stderr | 74 +------------------- tests/ui/option_map_unit_fn_unfixable.rs | 39 +++++++++++ tests/ui/option_map_unit_fn_unfixable.stderr | 27 +++++++ tests/ui/result_map_unit_fn_fixable.rs | 23 ------ tests/ui/result_map_unit_fn_fixable.stderr | 56 +-------------- tests/ui/result_map_unit_fn_unfixable.rs | 40 +++++++++++ tests/ui/result_map_unit_fn_unfixable.stderr | 27 +++++++ 8 files changed, 137 insertions(+), 174 deletions(-) create mode 100644 tests/ui/option_map_unit_fn_unfixable.rs create mode 100644 tests/ui/option_map_unit_fn_unfixable.stderr create mode 100644 tests/ui/result_map_unit_fn_unfixable.rs create mode 100644 tests/ui/result_map_unit_fn_unfixable.stderr diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index 1d2a3a17ee00..01aa6259855a 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -71,29 +71,6 @@ fn option_map_unit_fn() { x.field.map(|value| { { plus_one(value + captured); } }); - x.field.map(|ref value| { do_nothing(value + captured) }); - - - x.field.map(|value| { do_nothing(value); do_nothing(value) }); - - x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); - - // Suggestion for the let block should be `{ ... }` as it's too difficult to build a - // proper suggestion for these cases - x.field.map(|value| { - do_nothing(value); - do_nothing(value) - }); - x.field.map(|value| { do_nothing(value); do_nothing(value); }); - - // The following should suggest `if let Some(_X) ...` as it's difficult to generate a proper let variable name for them - Some(42).map(diverge); - "12".parse::().ok().map(diverge); - Some(plus_one(1)).map(do_nothing); - - // Should suggest `if let Some(_y) ...` to not override the existing foo variable - let y = Some(42); - y.map(do_nothing); -} + x.field.map(|ref value| { do_nothing(value + captured) });} fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index 4b619495b7b9..7c7ad39c363e 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -131,80 +131,10 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); error: called `map(f)` on an Option value where `f` is a unit closure --> $DIR/option_map_unit_fn_fixable.rs:74:5 | -LL | x.field.map(|ref value| { do_nothing(value + captured) }); +LL | x.field.map(|ref value| { do_nothing(value + captured) });} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(ref value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:77:5 - | -LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(value) = x.field { ... }` - -error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:79:5 - | -LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(value) = x.field { ... }` - -error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:83:5 - | -LL | x.field.map(|value| { - | _____^ - | |_____| - | || -LL | || do_nothing(value); -LL | || do_nothing(value) -LL | || }); - | ||______^- help: try this: `if let Some(value) = x.field { ... }` - | |_______| - | - -error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:87:5 - | -LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(value) = x.field { ... }` - -error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:90:5 - | -LL | Some(42).map(diverge); - | ^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(_) = Some(42) { diverge(...) }` - -error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:91:5 - | -LL | "12".parse::().ok().map(diverge); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(_) = "12".parse::().ok() { diverge(...) }` - -error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:92:5 - | -LL | Some(plus_one(1)).map(do_nothing); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(_) = Some(plus_one(1)) { do_nothing(...) }` - -error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:96:5 - | -LL | y.map(do_nothing); - | ^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Some(_y) = y { do_nothing(...) }` - -error: aborting due to 25 previous errors +error: aborting due to 17 previous errors diff --git a/tests/ui/option_map_unit_fn_unfixable.rs b/tests/ui/option_map_unit_fn_unfixable.rs new file mode 100644 index 000000000000..20e6c15b18d5 --- /dev/null +++ b/tests/ui/option_map_unit_fn_unfixable.rs @@ -0,0 +1,39 @@ +#![warn(clippy::option_map_unit_fn)] +#![allow(unused)] + +fn do_nothing(_: T) {} + +fn diverge(_: T) -> ! { + panic!() +} + +fn plus_one(value: usize) -> usize { + value + 1 +} + +#[rustfmt::skip] +fn option_map_unit_fn() { + + x.field.map(|value| { do_nothing(value); do_nothing(value) }); + + x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); + + // Suggestion for the let block should be `{ ... }` as it's too difficult to build a + // proper suggestion for these cases + x.field.map(|value| { + do_nothing(value); + do_nothing(value) + }); + x.field.map(|value| { do_nothing(value); do_nothing(value); }); + + // The following should suggest `if let Some(_X) ...` as it's difficult to generate a proper let variable name for them + Some(42).map(diverge); + "12".parse::().ok().map(diverge); + Some(plus_one(1)).map(do_nothing); + + // Should suggest `if let Some(_y) ...` to not override the existing foo variable + let y = Some(42); + y.map(do_nothing); +} + +fn main() {} diff --git a/tests/ui/option_map_unit_fn_unfixable.stderr b/tests/ui/option_map_unit_fn_unfixable.stderr new file mode 100644 index 000000000000..a53f5889c58d --- /dev/null +++ b/tests/ui/option_map_unit_fn_unfixable.stderr @@ -0,0 +1,27 @@ +error[E0425]: cannot find value `x` in this scope + --> $DIR/option_map_unit_fn_unfixable.rs:17:5 + | +LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/option_map_unit_fn_unfixable.rs:19:5 + | +LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/option_map_unit_fn_unfixable.rs:23:5 + | +LL | x.field.map(|value| { + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/option_map_unit_fn_unfixable.rs:27:5 + | +LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); + | ^ not found in this scope + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0425`. diff --git a/tests/ui/result_map_unit_fn_fixable.rs b/tests/ui/result_map_unit_fn_fixable.rs index a8e891d8db02..b0377fbcd16d 100644 --- a/tests/ui/result_map_unit_fn_fixable.rs +++ b/tests/ui/result_map_unit_fn_fixable.rs @@ -74,29 +74,6 @@ fn result_map_unit_fn() { x.field.map(|ref value| { do_nothing(value + captured) }); - - - x.field.map(|value| { do_nothing(value); do_nothing(value) }); - - x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); - - // Suggestion for the let block should be `{ ... }` as it's too difficult to build a - // proper suggestion for these cases - x.field.map(|value| { - do_nothing(value); - do_nothing(value) - }); - x.field.map(|value| { do_nothing(value); do_nothing(value); }); - - // The following should suggest `if let Ok(_X) ...` as it's difficult to generate a proper let variable name for them - let res: Result = Ok(42).map(diverge); - "12".parse::().map(diverge); - - let res: Result<(), usize> = Ok(plus_one(1)).map(do_nothing); - - // Should suggest `if let Ok(_y) ...` to not override the existing foo variable - let y: Result = Ok(42); - y.map(do_nothing); } fn main() {} diff --git a/tests/ui/result_map_unit_fn_fixable.stderr b/tests/ui/result_map_unit_fn_fixable.stderr index 29d1bd6dc18c..e48fb7c14942 100644 --- a/tests/ui/result_map_unit_fn_fixable.stderr +++ b/tests/ui/result_map_unit_fn_fixable.stderr @@ -136,59 +136,5 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); | | | help: try this: `if let Ok(ref value) = x.field { do_nothing(value + captured) }` -error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:79:5 - | -LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Ok(value) = x.field { ... }` - -error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:81:5 - | -LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Ok(value) = x.field { ... }` - -error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:85:5 - | -LL | x.field.map(|value| { - | _____^ - | |_____| - | || -LL | || do_nothing(value); -LL | || do_nothing(value) -LL | || }); - | ||______^- help: try this: `if let Ok(value) = x.field { ... }` - | |_______| - | - -error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:89:5 - | -LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Ok(value) = x.field { ... }` - -error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn_fixable.rs:93:5 - | -LL | "12".parse::().map(diverge); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Ok(_) = "12".parse::() { diverge(...) }` - -error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn_fixable.rs:99:5 - | -LL | y.map(do_nothing); - | ^^^^^^^^^^^^^^^^^- - | | - | help: try this: `if let Ok(_y) = y { do_nothing(...) }` - -error: aborting due to 23 previous errors +error: aborting due to 17 previous errors diff --git a/tests/ui/result_map_unit_fn_unfixable.rs b/tests/ui/result_map_unit_fn_unfixable.rs new file mode 100644 index 000000000000..7d597332eafc --- /dev/null +++ b/tests/ui/result_map_unit_fn_unfixable.rs @@ -0,0 +1,40 @@ +#![feature(never_type)] +#![warn(clippy::result_map_unit_fn)] +#![allow(unused)] + +fn do_nothing(_: T) {} + +fn diverge(_: T) -> ! { + panic!() +} + +fn plus_one(value: usize) -> usize { + value + 1 +} + +#[rustfmt::skip] +fn result_map_unit_fn() { + x.field.map(|value| { do_nothing(value); do_nothing(value) }); + + x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); + + // Suggestion for the let block should be `{ ... }` as it's too difficult to build a + // proper suggestion for these cases + x.field.map(|value| { + do_nothing(value); + do_nothing(value) + }); + x.field.map(|value| { do_nothing(value); do_nothing(value); }); + + // The following should suggest `if let Ok(_X) ...` as it's difficult to generate a proper let variable name for them + let res: Result = Ok(42).map(diverge); + "12".parse::().map(diverge); + + let res: Result<(), usize> = Ok(plus_one(1)).map(do_nothing); + + // Should suggest `if let Ok(_y) ...` to not override the existing foo variable + let y: Result = Ok(42); + y.map(do_nothing); +} + +fn main() {} diff --git a/tests/ui/result_map_unit_fn_unfixable.stderr b/tests/ui/result_map_unit_fn_unfixable.stderr new file mode 100644 index 000000000000..949c22946793 --- /dev/null +++ b/tests/ui/result_map_unit_fn_unfixable.stderr @@ -0,0 +1,27 @@ +error[E0425]: cannot find value `x` in this scope + --> $DIR/result_map_unit_fn_unfixable.rs:17:5 + | +LL | x.field.map(|value| { do_nothing(value); do_nothing(value) }); + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/result_map_unit_fn_unfixable.rs:19:5 + | +LL | x.field.map(|value| if value > 0 { do_nothing(value); do_nothing(value) }); + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/result_map_unit_fn_unfixable.rs:23:5 + | +LL | x.field.map(|value| { + | ^ not found in this scope + +error[E0425]: cannot find value `x` in this scope + --> $DIR/result_map_unit_fn_unfixable.rs:27:5 + | +LL | x.field.map(|value| { do_nothing(value); do_nothing(value); }); + | ^ not found in this scope + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0425`. From a2e1ff42df31fdf7d1892ea989dc97a1f1df39a9 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 08:49:23 -0700 Subject: [PATCH 18/26] map_unit_fn: fix applicability --- clippy_lints/src/map_unit_fn.rs | 12 ++++++------ tests/ui/option_map_unit_fn_fixable.stderr | 6 +++--- tests/ui/result_map_unit_fn_fixable.stderr | 6 +++--- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index 0df14c2664ae..a888834551c9 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -217,15 +217,15 @@ fn lint_map_unit_fn(cx: &LateContext<'_, '_>, stmt: &hir::Stmt, expr: &hir::Expr if is_unit_function(cx, fn_arg) { let msg = suggestion_msg("function", map_type); let suggestion = format!( - "if let {0}({1}) = {2} {{ {3}(...) }}", + "if let {0}({binding}) = {1} {{ {2}({binding}) }}", variant, - let_binding_name(cx, var_arg), snippet(cx, var_arg.span, "_"), - snippet(cx, fn_arg.span, "_") + snippet(cx, fn_arg.span, "_"), + binding = let_binding_name(cx, var_arg) ); span_lint_and_then(cx, lint, expr.span, &msg, |db| { - db.span_suggestion(stmt.span, "try this", suggestion, Applicability::Unspecified); + db.span_suggestion(stmt.span, "try this", suggestion, Applicability::MachineApplicable); }); } else if let Some((binding, closure_expr)) = unit_closure(cx, fn_arg) { let msg = suggestion_msg("closure", map_type); @@ -250,9 +250,9 @@ fn lint_map_unit_fn(cx: &LateContext<'_, '_>, stmt: &hir::Stmt, expr: &hir::Expr "if let {0}({1}) = {2} {{ ... }}", variant, snippet(cx, binding.pat.span, "_"), - snippet(cx, var_arg.span, "_") + snippet(cx, var_arg.span, "_"), ); - db.span_suggestion(stmt.span, "try this", suggestion, Applicability::Unspecified); + db.span_suggestion(stmt.span, "try this", suggestion, Applicability::HasPlaceholders); } }); } diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index 7c7ad39c363e..a15be882525c 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -4,7 +4,7 @@ error: called `map(f)` on an Option value where `f` is a unit function LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- | | - | help: try this: `if let Some(x_field) = x.field { do_nothing(...) }` + | help: try this: `if let Some(x_field) = x.field { do_nothing(x_field) }` | = note: `-D clippy::option-map-unit-fn` implied by `-D warnings` @@ -14,7 +14,7 @@ error: called `map(f)` on an Option value where `f` is a unit function LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- | | - | help: try this: `if let Some(x_field) = x.field { do_nothing(...) }` + | help: try this: `if let Some(x_field) = x.field { do_nothing(x_field) }` error: called `map(f)` on an Option value where `f` is a unit function --> $DIR/option_map_unit_fn_fixable.rs:36:5 @@ -22,7 +22,7 @@ error: called `map(f)` on an Option value where `f` is a unit function LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- | | - | help: try this: `if let Some(x_field) = x.field { diverge(...) }` + | help: try this: `if let Some(x_field) = x.field { diverge(x_field) }` error: called `map(f)` on an Option value where `f` is a unit closure --> $DIR/option_map_unit_fn_fixable.rs:42:5 diff --git a/tests/ui/result_map_unit_fn_fixable.stderr b/tests/ui/result_map_unit_fn_fixable.stderr index e48fb7c14942..94ed079762b5 100644 --- a/tests/ui/result_map_unit_fn_fixable.stderr +++ b/tests/ui/result_map_unit_fn_fixable.stderr @@ -4,7 +4,7 @@ error: called `map(f)` on an Result value where `f` is a unit function LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- | | - | help: try this: `if let Ok(x_field) = x.field { do_nothing(...) }` + | help: try this: `if let Ok(x_field) = x.field { do_nothing(x_field) }` | = note: `-D clippy::result-map-unit-fn` implied by `-D warnings` @@ -14,7 +14,7 @@ error: called `map(f)` on an Result value where `f` is a unit function LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- | | - | help: try this: `if let Ok(x_field) = x.field { do_nothing(...) }` + | help: try this: `if let Ok(x_field) = x.field { do_nothing(x_field) }` error: called `map(f)` on an Result value where `f` is a unit function --> $DIR/result_map_unit_fn_fixable.rs:38:5 @@ -22,7 +22,7 @@ error: called `map(f)` on an Result value where `f` is a unit function LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- | | - | help: try this: `if let Ok(x_field) = x.field { diverge(...) }` + | help: try this: `if let Ok(x_field) = x.field { diverge(x_field) }` error: called `map(f)` on an Result value where `f` is a unit closure --> $DIR/result_map_unit_fn_fixable.rs:44:5 From 3a854a3022e11dc6f8320ff98a7d65e9289577b4 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 09:03:32 -0700 Subject: [PATCH 19/26] map_unit_fn: make test rustfixable --- tests/ui/option_map_unit_fn_fixable.fixed | 78 +++++++++++++++++++++ tests/ui/option_map_unit_fn_fixable.rs | 2 + tests/ui/option_map_unit_fn_fixable.stderr | 34 ++++----- tests/ui/result_map_unit_fn_fixable.fixed | 81 ++++++++++++++++++++++ tests/ui/result_map_unit_fn_fixable.rs | 2 + tests/ui/result_map_unit_fn_fixable.stderr | 34 ++++----- 6 files changed, 197 insertions(+), 34 deletions(-) create mode 100644 tests/ui/option_map_unit_fn_fixable.fixed create mode 100644 tests/ui/result_map_unit_fn_fixable.fixed diff --git a/tests/ui/option_map_unit_fn_fixable.fixed b/tests/ui/option_map_unit_fn_fixable.fixed new file mode 100644 index 000000000000..ad153e4fc194 --- /dev/null +++ b/tests/ui/option_map_unit_fn_fixable.fixed @@ -0,0 +1,78 @@ +// run-rustfix + +#![warn(clippy::option_map_unit_fn)] +#![allow(unused)] + +fn do_nothing(_: T) {} + +fn diverge(_: T) -> ! { + panic!() +} + +fn plus_one(value: usize) -> usize { + value + 1 +} + +struct HasOption { + field: Option, +} + +impl HasOption { + fn do_option_nothing(self: &Self, value: usize) {} + + fn do_option_plus_one(self: &Self, value: usize) -> usize { + value + 1 + } +} +#[rustfmt::skip] +fn option_map_unit_fn() { + let x = HasOption { field: Some(10) }; + + x.field.map(plus_one); + let _ : Option<()> = x.field.map(do_nothing); + + if let Some(x_field) = x.field { do_nothing(x_field) } + + if let Some(x_field) = x.field { do_nothing(x_field) } + + if let Some(x_field) = x.field { diverge(x_field) } + + let captured = 10; + if let Some(value) = x.field { do_nothing(value + captured) }; + let _ : Option<()> = x.field.map(|value| do_nothing(value + captured)); + + if let Some(value) = x.field { x.do_option_nothing(value + captured) } + + if let Some(value) = x.field { x.do_option_plus_one(value + captured); } + + + if let Some(value) = x.field { do_nothing(value + captured) } + + if let Some(value) = x.field { do_nothing(value + captured) } + + if let Some(value) = x.field { do_nothing(value + captured); } + + if let Some(value) = x.field { do_nothing(value + captured); } + + + if let Some(value) = x.field { diverge(value + captured) } + + if let Some(value) = x.field { diverge(value + captured) } + + if let Some(value) = x.field { diverge(value + captured); } + + if let Some(value) = x.field { diverge(value + captured); } + + + x.field.map(|value| plus_one(value + captured)); + x.field.map(|value| { plus_one(value + captured) }); + if let Some(value) = x.field { let y = plus_one(value + captured); } + + if let Some(value) = x.field { plus_one(value + captured); } + + if let Some(value) = x.field { plus_one(value + captured); } + + + if let Some(ref value) = x.field { do_nothing(value + captured) }} + +fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index 01aa6259855a..6926498341ac 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::option_map_unit_fn)] #![allow(unused)] diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index a15be882525c..6e87b070909e 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -1,5 +1,5 @@ error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:32:5 + --> $DIR/option_map_unit_fn_fixable.rs:34:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -9,7 +9,7 @@ LL | x.field.map(do_nothing); = note: `-D clippy::option-map-unit-fn` implied by `-D warnings` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:34:5 + --> $DIR/option_map_unit_fn_fixable.rs:36:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -17,7 +17,7 @@ LL | x.field.map(do_nothing); | help: try this: `if let Some(x_field) = x.field { do_nothing(x_field) }` error: called `map(f)` on an Option value where `f` is a unit function - --> $DIR/option_map_unit_fn_fixable.rs:36:5 + --> $DIR/option_map_unit_fn_fixable.rs:38:5 | LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- @@ -25,7 +25,7 @@ LL | x.field.map(diverge); | help: try this: `if let Some(x_field) = x.field { diverge(x_field) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:42:5 + --> $DIR/option_map_unit_fn_fixable.rs:44:5 | LL | x.field.map(|value| x.do_option_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -33,7 +33,7 @@ LL | x.field.map(|value| x.do_option_nothing(value + captured)); | help: try this: `if let Some(value) = x.field { x.do_option_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:44:5 + --> $DIR/option_map_unit_fn_fixable.rs:46:5 | LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -41,7 +41,7 @@ LL | x.field.map(|value| { x.do_option_plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { x.do_option_plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:47:5 + --> $DIR/option_map_unit_fn_fixable.rs:49:5 | LL | x.field.map(|value| do_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -49,7 +49,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:49:5 + --> $DIR/option_map_unit_fn_fixable.rs:51:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -57,7 +57,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:51:5 + --> $DIR/option_map_unit_fn_fixable.rs:53:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:53:5 + --> $DIR/option_map_unit_fn_fixable.rs:55:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -73,7 +73,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | help: try this: `if let Some(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:56:5 + --> $DIR/option_map_unit_fn_fixable.rs:58:5 | LL | x.field.map(|value| diverge(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -81,7 +81,7 @@ LL | x.field.map(|value| diverge(value + captured)); | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:58:5 + --> $DIR/option_map_unit_fn_fixable.rs:60:5 | LL | x.field.map(|value| { diverge(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -89,7 +89,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | help: try this: `if let Some(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:60:5 + --> $DIR/option_map_unit_fn_fixable.rs:62:5 | LL | x.field.map(|value| { diverge(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -97,7 +97,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:62:5 + --> $DIR/option_map_unit_fn_fixable.rs:64:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -105,7 +105,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | help: try this: `if let Some(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:67:5 + --> $DIR/option_map_unit_fn_fixable.rs:69:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -113,7 +113,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { let y = plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:69:5 + --> $DIR/option_map_unit_fn_fixable.rs:71:5 | LL | x.field.map(|value| { plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -121,7 +121,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:71:5 + --> $DIR/option_map_unit_fn_fixable.rs:73:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -129,7 +129,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | help: try this: `if let Some(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Option value where `f` is a unit closure - --> $DIR/option_map_unit_fn_fixable.rs:74:5 + --> $DIR/option_map_unit_fn_fixable.rs:76:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) });} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- diff --git a/tests/ui/result_map_unit_fn_fixable.fixed b/tests/ui/result_map_unit_fn_fixable.fixed new file mode 100644 index 000000000000..64d39516be71 --- /dev/null +++ b/tests/ui/result_map_unit_fn_fixable.fixed @@ -0,0 +1,81 @@ +// run-rustfix + +#![feature(never_type)] +#![warn(clippy::result_map_unit_fn)] +#![allow(unused)] + +fn do_nothing(_: T) {} + +fn diverge(_: T) -> ! { + panic!() +} + +fn plus_one(value: usize) -> usize { + value + 1 +} + +struct HasResult { + field: Result, +} + +impl HasResult { + fn do_result_nothing(self: &Self, value: usize) {} + + fn do_result_plus_one(self: &Self, value: usize) -> usize { + value + 1 + } +} + +#[rustfmt::skip] +fn result_map_unit_fn() { + let x = HasResult { field: Ok(10) }; + + x.field.map(plus_one); + let _: Result<(), usize> = x.field.map(do_nothing); + + if let Ok(x_field) = x.field { do_nothing(x_field) } + + if let Ok(x_field) = x.field { do_nothing(x_field) } + + if let Ok(x_field) = x.field { diverge(x_field) } + + let captured = 10; + if let Ok(value) = x.field { do_nothing(value + captured) }; + let _: Result<(), usize> = x.field.map(|value| do_nothing(value + captured)); + + if let Ok(value) = x.field { x.do_result_nothing(value + captured) } + + if let Ok(value) = x.field { x.do_result_plus_one(value + captured); } + + + if let Ok(value) = x.field { do_nothing(value + captured) } + + if let Ok(value) = x.field { do_nothing(value + captured) } + + if let Ok(value) = x.field { do_nothing(value + captured); } + + if let Ok(value) = x.field { do_nothing(value + captured); } + + + if let Ok(value) = x.field { diverge(value + captured) } + + if let Ok(value) = x.field { diverge(value + captured) } + + if let Ok(value) = x.field { diverge(value + captured); } + + if let Ok(value) = x.field { diverge(value + captured); } + + + x.field.map(|value| plus_one(value + captured)); + x.field.map(|value| { plus_one(value + captured) }); + if let Ok(value) = x.field { let y = plus_one(value + captured); } + + if let Ok(value) = x.field { plus_one(value + captured); } + + if let Ok(value) = x.field { plus_one(value + captured); } + + + if let Ok(ref value) = x.field { do_nothing(value + captured) } +} + +fn main() {} diff --git a/tests/ui/result_map_unit_fn_fixable.rs b/tests/ui/result_map_unit_fn_fixable.rs index b0377fbcd16d..bf4aba8a7cc1 100644 --- a/tests/ui/result_map_unit_fn_fixable.rs +++ b/tests/ui/result_map_unit_fn_fixable.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![feature(never_type)] #![warn(clippy::result_map_unit_fn)] #![allow(unused)] diff --git a/tests/ui/result_map_unit_fn_fixable.stderr b/tests/ui/result_map_unit_fn_fixable.stderr index 94ed079762b5..db72c64d52e2 100644 --- a/tests/ui/result_map_unit_fn_fixable.stderr +++ b/tests/ui/result_map_unit_fn_fixable.stderr @@ -1,5 +1,5 @@ error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn_fixable.rs:34:5 + --> $DIR/result_map_unit_fn_fixable.rs:36:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -9,7 +9,7 @@ LL | x.field.map(do_nothing); = note: `-D clippy::result-map-unit-fn` implied by `-D warnings` error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn_fixable.rs:36:5 + --> $DIR/result_map_unit_fn_fixable.rs:38:5 | LL | x.field.map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^- @@ -17,7 +17,7 @@ LL | x.field.map(do_nothing); | help: try this: `if let Ok(x_field) = x.field { do_nothing(x_field) }` error: called `map(f)` on an Result value where `f` is a unit function - --> $DIR/result_map_unit_fn_fixable.rs:38:5 + --> $DIR/result_map_unit_fn_fixable.rs:40:5 | LL | x.field.map(diverge); | ^^^^^^^^^^^^^^^^^^^^- @@ -25,7 +25,7 @@ LL | x.field.map(diverge); | help: try this: `if let Ok(x_field) = x.field { diverge(x_field) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:44:5 + --> $DIR/result_map_unit_fn_fixable.rs:46:5 | LL | x.field.map(|value| x.do_result_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -33,7 +33,7 @@ LL | x.field.map(|value| x.do_result_nothing(value + captured)); | help: try this: `if let Ok(value) = x.field { x.do_result_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:46:5 + --> $DIR/result_map_unit_fn_fixable.rs:48:5 | LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -41,7 +41,7 @@ LL | x.field.map(|value| { x.do_result_plus_one(value + captured); }); | help: try this: `if let Ok(value) = x.field { x.do_result_plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:49:5 + --> $DIR/result_map_unit_fn_fixable.rs:51:5 | LL | x.field.map(|value| do_nothing(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -49,7 +49,7 @@ LL | x.field.map(|value| do_nothing(value + captured)); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:51:5 + --> $DIR/result_map_unit_fn_fixable.rs:53:5 | LL | x.field.map(|value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -57,7 +57,7 @@ LL | x.field.map(|value| { do_nothing(value + captured) }); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:53:5 + --> $DIR/result_map_unit_fn_fixable.rs:55:5 | LL | x.field.map(|value| { do_nothing(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -65,7 +65,7 @@ LL | x.field.map(|value| { do_nothing(value + captured); }); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:55:5 + --> $DIR/result_map_unit_fn_fixable.rs:57:5 | LL | x.field.map(|value| { { do_nothing(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -73,7 +73,7 @@ LL | x.field.map(|value| { { do_nothing(value + captured); } }); | help: try this: `if let Ok(value) = x.field { do_nothing(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:58:5 + --> $DIR/result_map_unit_fn_fixable.rs:60:5 | LL | x.field.map(|value| diverge(value + captured)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -81,7 +81,7 @@ LL | x.field.map(|value| diverge(value + captured)); | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:60:5 + --> $DIR/result_map_unit_fn_fixable.rs:62:5 | LL | x.field.map(|value| { diverge(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -89,7 +89,7 @@ LL | x.field.map(|value| { diverge(value + captured) }); | help: try this: `if let Ok(value) = x.field { diverge(value + captured) }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:62:5 + --> $DIR/result_map_unit_fn_fixable.rs:64:5 | LL | x.field.map(|value| { diverge(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -97,7 +97,7 @@ LL | x.field.map(|value| { diverge(value + captured); }); | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:64:5 + --> $DIR/result_map_unit_fn_fixable.rs:66:5 | LL | x.field.map(|value| { { diverge(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -105,7 +105,7 @@ LL | x.field.map(|value| { { diverge(value + captured); } }); | help: try this: `if let Ok(value) = x.field { diverge(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:69:5 + --> $DIR/result_map_unit_fn_fixable.rs:71:5 | LL | x.field.map(|value| { let y = plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -113,7 +113,7 @@ LL | x.field.map(|value| { let y = plus_one(value + captured); }); | help: try this: `if let Ok(value) = x.field { let y = plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:71:5 + --> $DIR/result_map_unit_fn_fixable.rs:73:5 | LL | x.field.map(|value| { plus_one(value + captured); }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -121,7 +121,7 @@ LL | x.field.map(|value| { plus_one(value + captured); }); | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:73:5 + --> $DIR/result_map_unit_fn_fixable.rs:75:5 | LL | x.field.map(|value| { { plus_one(value + captured); } }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- @@ -129,7 +129,7 @@ LL | x.field.map(|value| { { plus_one(value + captured); } }); | help: try this: `if let Ok(value) = x.field { plus_one(value + captured); }` error: called `map(f)` on an Result value where `f` is a unit closure - --> $DIR/result_map_unit_fn_fixable.rs:76:5 + --> $DIR/result_map_unit_fn_fixable.rs:78:5 | LL | x.field.map(|ref value| { do_nothing(value + captured) }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- From 884fbf4660f6a06a78ce0553a53290a4fd7d3075 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 09:30:27 -0700 Subject: [PATCH 20/26] redundant_closure_call: split tests into fixable --- clippy_lints/src/misc_early.rs | 14 ++++++-------- tests/ui/redundant_closure_call.rs | 4 ++-- tests/ui/redundant_closure_call.stderr | 8 +------- tests/ui/redundant_closure_call_fixable.fixed | 8 ++++++++ tests/ui/redundant_closure_call_fixable.rs | 8 ++++++++ tests/ui/redundant_closure_call_fixable.stderr | 10 ++++++++++ 6 files changed, 35 insertions(+), 17 deletions(-) create mode 100644 tests/ui/redundant_closure_call_fixable.fixed create mode 100644 tests/ui/redundant_closure_call_fixable.rs create mode 100644 tests/ui/redundant_closure_call_fixable.stderr diff --git a/clippy_lints/src/misc_early.rs b/clippy_lints/src/misc_early.rs index acc83d28897a..588f5f75c1e7 100644 --- a/clippy_lints/src/misc_early.rs +++ b/clippy_lints/src/misc_early.rs @@ -1,5 +1,6 @@ use crate::utils::{ - constants, snippet, snippet_opt, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, + constants, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, + span_lint_and_then, }; use if_chain::if_chain; use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintContext, LintPass}; @@ -414,13 +415,10 @@ impl EarlyLintPass for MiscEarlyLints { "Try not to call a closure in the expression where it is declared.", |db| { if decl.inputs.is_empty() { - let hint = snippet(cx, block.span, "..").into_owned(); - db.span_suggestion( - expr.span, - "Try doing something like: ", - hint, - Applicability::MachineApplicable, // snippet - ); + let mut app = Applicability::MachineApplicable; + let hint = + snippet_with_applicability(cx, block.span, "..", &mut app).into_owned(); + db.span_suggestion(expr.span, "Try doing something like: ", hint, app); } }, ); diff --git a/tests/ui/redundant_closure_call.rs b/tests/ui/redundant_closure_call.rs index 2e81eea44003..bacd67db7c30 100644 --- a/tests/ui/redundant_closure_call.rs +++ b/tests/ui/redundant_closure_call.rs @@ -1,8 +1,8 @@ +// non rustfixable, see redundant_closure_call_fixable.rs + #![warn(clippy::redundant_closure_call)] fn main() { - let a = (|| 42)(); - let mut i = 1; let mut k = (|m| m + 1)(i); diff --git a/tests/ui/redundant_closure_call.stderr b/tests/ui/redundant_closure_call.stderr index 9c827fd8f17c..68c1416bb6b1 100644 --- a/tests/ui/redundant_closure_call.stderr +++ b/tests/ui/redundant_closure_call.stderr @@ -12,12 +12,6 @@ error: Closure called just once immediately after it was declared LL | i = closure(3); | ^^^^^^^^^^^^^^ -error: Try not to call a closure in the expression where it is declared. - --> $DIR/redundant_closure_call.rs:4:13 - | -LL | let a = (|| 42)(); - | ^^^^^^^^^ help: Try doing something like: : `42` - error: Try not to call a closure in the expression where it is declared. --> $DIR/redundant_closure_call.rs:7:17 | @@ -30,5 +24,5 @@ error: Try not to call a closure in the expression where it is declared. LL | k = (|a, b| a * b)(1, 5); | ^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: aborting due to 4 previous errors diff --git a/tests/ui/redundant_closure_call_fixable.fixed b/tests/ui/redundant_closure_call_fixable.fixed new file mode 100644 index 000000000000..0abca6fca061 --- /dev/null +++ b/tests/ui/redundant_closure_call_fixable.fixed @@ -0,0 +1,8 @@ +// run-rustfix + +#![warn(clippy::redundant_closure_call)] +#![allow(unused)] + +fn main() { + let a = 42; +} diff --git a/tests/ui/redundant_closure_call_fixable.rs b/tests/ui/redundant_closure_call_fixable.rs new file mode 100644 index 000000000000..f8b9d37a5cc4 --- /dev/null +++ b/tests/ui/redundant_closure_call_fixable.rs @@ -0,0 +1,8 @@ +// run-rustfix + +#![warn(clippy::redundant_closure_call)] +#![allow(unused)] + +fn main() { + let a = (|| 42)(); +} diff --git a/tests/ui/redundant_closure_call_fixable.stderr b/tests/ui/redundant_closure_call_fixable.stderr new file mode 100644 index 000000000000..e7737f9dd856 --- /dev/null +++ b/tests/ui/redundant_closure_call_fixable.stderr @@ -0,0 +1,10 @@ +error: Try not to call a closure in the expression where it is declared. + --> $DIR/redundant_closure_call_fixable.rs:7:13 + | +LL | let a = (|| 42)(); + | ^^^^^^^^^ help: Try doing something like: : `42` + | + = note: `-D clippy::redundant-closure-call` implied by `-D warnings` + +error: aborting due to previous error + From 2df41d5f0a3d172a251b63954db03ac3cb557f7f Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 10:01:21 -0700 Subject: [PATCH 21/26] redundant_pattern_matching: make rustfixable --- .../src/redundant_pattern_matching.rs | 16 ++++- tests/ui/redundant_pattern_matching.fixed | 60 +++++++++++++++++++ tests/ui/redundant_pattern_matching.rs | 9 ++- tests/ui/redundant_pattern_matching.stderr | 46 +++++++------- 4 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 tests/ui/redundant_pattern_matching.fixed diff --git a/clippy_lints/src/redundant_pattern_matching.rs b/clippy_lints/src/redundant_pattern_matching.rs index b8d1ea3851f6..8fea20dba67c 100644 --- a/clippy_lints/src/redundant_pattern_matching.rs +++ b/clippy_lints/src/redundant_pattern_matching.rs @@ -49,14 +49,22 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPatternMatching { if let ExprKind::Match(ref op, ref arms, ref match_source) = expr.node { match match_source { MatchSource::Normal => find_sugg_for_match(cx, expr, op, arms), - MatchSource::IfLetDesugar { .. } => find_sugg_for_if_let(cx, expr, op, arms), + MatchSource::IfLetDesugar { contains_else_clause } => { + find_sugg_for_if_let(cx, expr, op, arms, *contains_else_clause) + }, _ => return, } } } } -fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, op: &P, arms: &HirVec) { +fn find_sugg_for_if_let<'a, 'tcx>( + cx: &LateContext<'a, 'tcx>, + expr: &'tcx Expr, + op: &P, + arms: &HirVec, + has_else: bool, +) { let good_method = match arms[0].pat.node { PatKind::TupleStruct(ref path, ref patterns, _) if patterns.len() == 1 => { if let PatKind::Wild = patterns[0].node { @@ -79,6 +87,8 @@ fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, _ => return, }; + let maybe_semi = if has_else { "" } else { ";" }; + span_lint_and_then( cx, REDUNDANT_PATTERN_MATCHING, @@ -89,7 +99,7 @@ fn find_sugg_for_if_let<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, db.span_suggestion( span, "try this", - format!("{}.{}", snippet(cx, op.span, "_"), good_method), + format!("{}.{}{}", snippet(cx, op.span, "_"), good_method, maybe_semi), Applicability::MaybeIncorrect, // snippet ); }, diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed new file mode 100644 index 000000000000..776c9444566b --- /dev/null +++ b/tests/ui/redundant_pattern_matching.fixed @@ -0,0 +1,60 @@ +// run-rustfix + +#![warn(clippy::all)] +#![warn(clippy::redundant_pattern_matching)] +#![allow(clippy::unit_arg, clippy::let_unit_value, unused_must_use)] + +fn main() { + Ok::(42).is_ok(); + + Err::(42).is_err(); + + None::<()>.is_none(); + + Some(42).is_some(); + + if Ok::(42).is_ok() {} + + if Err::(42).is_err() {} + + if None::.is_none() {} + + if Some(42).is_some() {} + + if let Ok(x) = Ok::(42) { + println!("{}", x); + } + + Ok::(42).is_ok(); + + Ok::(42).is_err(); + + Err::(42).is_err(); + + Err::(42).is_ok(); + + Some(42).is_some(); + + None::<()>.is_none(); + + let _ = None::<()>.is_none(); + + let _ = Ok::(4).is_ok(); + + let _ = does_something(); + let _ = returns_unit(); + + let opt = Some(false); + let x = opt.is_some(); + takes_bool(x); +} + +fn takes_bool(_: bool) {} + +fn does_something() -> bool { + Ok::(4).is_ok() +} + +fn returns_unit() { + Ok::(4).is_ok(); +} diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 2bea7d8d9618..2b2d5b1c1ec6 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -1,6 +1,8 @@ +// run-rustfix + #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::unit_arg, clippy::let_unit_value)] +#![allow(clippy::unit_arg, clippy::let_unit_value, unused_must_use)] fn main() { if let Ok(_) = Ok::(42) {} @@ -66,12 +68,9 @@ fn main() { let opt = Some(false); let x = if let Some(_) = opt { true } else { false }; takes_bool(x); - let y = if let Some(_) = opt {}; - takes_unit(y); } -fn takes_bool(x: bool) {} -fn takes_unit(x: ()) {} +fn takes_bool(_: bool) {} fn does_something() -> bool { if let Ok(_) = Ok::(4) { diff --git a/tests/ui/redundant_pattern_matching.stderr b/tests/ui/redundant_pattern_matching.stderr index df12b7e169a1..5a4a69b12200 100644 --- a/tests/ui/redundant_pattern_matching.stderr +++ b/tests/ui/redundant_pattern_matching.stderr @@ -1,31 +1,31 @@ error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:6:12 + --> $DIR/redundant_pattern_matching.rs:8:12 | LL | if let Ok(_) = Ok::(42) {} - | -------^^^^^------------------------ help: try this: `Ok::(42).is_ok()` + | -------^^^^^------------------------ help: try this: `Ok::(42).is_ok();` | = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:8:12 + --> $DIR/redundant_pattern_matching.rs:10:12 | LL | if let Err(_) = Err::(42) {} - | -------^^^^^^------------------------- help: try this: `Err::(42).is_err()` + | -------^^^^^^------------------------- help: try this: `Err::(42).is_err();` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:10:12 + --> $DIR/redundant_pattern_matching.rs:12:12 | LL | if let None = None::<()> {} - | -------^^^^---------------- help: try this: `None::<()>.is_none()` + | -------^^^^---------------- help: try this: `None::<()>.is_none();` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:12:12 + --> $DIR/redundant_pattern_matching.rs:14:12 | LL | if let Some(_) = Some(42) {} - | -------^^^^^^^-------------- help: try this: `Some(42).is_some()` + | -------^^^^^^^-------------- help: try this: `Some(42).is_some();` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:26:5 + --> $DIR/redundant_pattern_matching.rs:28:5 | LL | / match Ok::(42) { LL | | Ok(_) => true, @@ -34,7 +34,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_ok()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:31:5 + --> $DIR/redundant_pattern_matching.rs:33:5 | LL | / match Ok::(42) { LL | | Ok(_) => false, @@ -43,7 +43,7 @@ LL | | }; | |_____^ help: try this: `Ok::(42).is_err()` error: redundant pattern matching, consider using `is_err()` - --> $DIR/redundant_pattern_matching.rs:36:5 + --> $DIR/redundant_pattern_matching.rs:38:5 | LL | / match Err::(42) { LL | | Ok(_) => false, @@ -52,7 +52,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_err()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:41:5 + --> $DIR/redundant_pattern_matching.rs:43:5 | LL | / match Err::(42) { LL | | Ok(_) => true, @@ -61,7 +61,7 @@ LL | | }; | |_____^ help: try this: `Err::(42).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:46:5 + --> $DIR/redundant_pattern_matching.rs:48:5 | LL | / match Some(42) { LL | | Some(_) => true, @@ -70,7 +70,7 @@ LL | | }; | |_____^ help: try this: `Some(42).is_some()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:51:5 + --> $DIR/redundant_pattern_matching.rs:53:5 | LL | / match None::<()> { LL | | Some(_) => false, @@ -79,7 +79,7 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_none()` - --> $DIR/redundant_pattern_matching.rs:56:13 + --> $DIR/redundant_pattern_matching.rs:58:13 | LL | let _ = match None::<()> { | _____________^ @@ -89,25 +89,19 @@ LL | | }; | |_____^ help: try this: `None::<()>.is_none()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:61:20 + --> $DIR/redundant_pattern_matching.rs:63:20 | LL | let _ = if let Ok(_) = Ok::(4) { true } else { false }; | -------^^^^^--------------------------------------------- help: try this: `Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:67:20 + --> $DIR/redundant_pattern_matching.rs:69:20 | LL | let x = if let Some(_) = opt { true } else { false }; | -------^^^^^^^------------------------------ help: try this: `opt.is_some()` -error: redundant pattern matching, consider using `is_some()` - --> $DIR/redundant_pattern_matching.rs:69:20 - | -LL | let y = if let Some(_) = opt {}; - | -------^^^^^^^--------- help: try this: `opt.is_some()` - error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:77:12 + --> $DIR/redundant_pattern_matching.rs:76:12 | LL | if let Ok(_) = Ok::(4) { | _____- ^^^^^ @@ -118,7 +112,7 @@ LL | | } | |_____- help: try this: `Ok::(4).is_ok()` error: redundant pattern matching, consider using `is_ok()` - --> $DIR/redundant_pattern_matching.rs:85:12 + --> $DIR/redundant_pattern_matching.rs:84:12 | LL | if let Ok(_) = Ok::(4) { | _____- ^^^^^ @@ -128,5 +122,5 @@ LL | | false LL | | }; | |_____- help: try this: `Ok::(4).is_ok()` -error: aborting due to 16 previous errors +error: aborting due to 15 previous errors From cad0196f55691f7abf9b1750df113b53e7c7bbc3 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 10:08:19 -0700 Subject: [PATCH 22/26] renamed_builtin_attr: make test rustfixable --- tests/ui/renamed_builtin_attr.fixed | 4 ++++ tests/ui/renamed_builtin_attr.rs | 2 ++ tests/ui/renamed_builtin_attr.stderr | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 tests/ui/renamed_builtin_attr.fixed diff --git a/tests/ui/renamed_builtin_attr.fixed b/tests/ui/renamed_builtin_attr.fixed new file mode 100644 index 000000000000..cb91b841d2cb --- /dev/null +++ b/tests/ui/renamed_builtin_attr.fixed @@ -0,0 +1,4 @@ +// run-rustfix + +#[clippy::cognitive_complexity = "1"] +fn main() {} diff --git a/tests/ui/renamed_builtin_attr.rs b/tests/ui/renamed_builtin_attr.rs index fdb425363e81..b3ce2758067c 100644 --- a/tests/ui/renamed_builtin_attr.rs +++ b/tests/ui/renamed_builtin_attr.rs @@ -1,2 +1,4 @@ +// run-rustfix + #[clippy::cyclomatic_complexity = "1"] fn main() {} diff --git a/tests/ui/renamed_builtin_attr.stderr b/tests/ui/renamed_builtin_attr.stderr index cf6cccd36884..a399ff52fb8b 100644 --- a/tests/ui/renamed_builtin_attr.stderr +++ b/tests/ui/renamed_builtin_attr.stderr @@ -1,5 +1,5 @@ error: Usage of deprecated attribute - --> $DIR/renamed_builtin_attr.rs:1:11 + --> $DIR/renamed_builtin_attr.rs:3:11 | LL | #[clippy::cyclomatic_complexity = "1"] | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `cognitive_complexity` From 99437a5caa3e7d9e6143549c9a7139ceb2915def Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 10:21:08 -0700 Subject: [PATCH 23/26] redundant_static_lifetimes: split test, make rustfixable --- tests/ui/redundant_static_lifetimes.fixed | 56 +++++++++++++ tests/ui/redundant_static_lifetimes.rs | 23 +----- tests/ui/redundant_static_lifetimes.stderr | 82 +++---------------- .../ui/redundant_static_lifetimes_multiple.rs | 13 +++ ...redundant_static_lifetimes_multiple.stderr | 64 +++++++++++++++ 5 files changed, 148 insertions(+), 90 deletions(-) create mode 100644 tests/ui/redundant_static_lifetimes.fixed create mode 100644 tests/ui/redundant_static_lifetimes_multiple.rs create mode 100644 tests/ui/redundant_static_lifetimes_multiple.stderr diff --git a/tests/ui/redundant_static_lifetimes.fixed b/tests/ui/redundant_static_lifetimes.fixed new file mode 100644 index 000000000000..921249606ad2 --- /dev/null +++ b/tests/ui/redundant_static_lifetimes.fixed @@ -0,0 +1,56 @@ +// run-rustfix + +#![allow(unused)] + +#[derive(Debug)] +struct Foo {} + +const VAR_ONE: &str = "Test constant #1"; // ERROR Consider removing 'static. + +const VAR_TWO: &str = "Test constant #2"; // This line should not raise a warning. + +const VAR_THREE: &[&str] = &["one", "two"]; // ERROR Consider removing 'static + +const VAR_FOUR: (&str, (&str, &str), &str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static + +const VAR_SIX: &u8 = &5; + +const VAR_HEIGHT: &Foo = &Foo {}; + +const VAR_SLICE: &[u8] = b"Test constant #1"; // ERROR Consider removing 'static. + +const VAR_TUPLE: &(u8, u8) = &(1, 2); // ERROR Consider removing 'static. + +const VAR_ARRAY: &[u8; 1] = b"T"; // ERROR Consider removing 'static. + +static STATIC_VAR_ONE: &str = "Test static #1"; // ERROR Consider removing 'static. + +static STATIC_VAR_TWO: &str = "Test static #2"; // This line should not raise a warning. + +static STATIC_VAR_THREE: &[&str] = &["one", "two"]; // ERROR Consider removing 'static + +static STATIC_VAR_SIX: &u8 = &5; + +static STATIC_VAR_HEIGHT: &Foo = &Foo {}; + +static STATIC_VAR_SLICE: &[u8] = b"Test static #3"; // ERROR Consider removing 'static. + +static STATIC_VAR_TUPLE: &(u8, u8) = &(1, 2); // ERROR Consider removing 'static. + +static STATIC_VAR_ARRAY: &[u8; 1] = b"T"; // ERROR Consider removing 'static. + +fn main() { + let false_positive: &'static str = "test"; +} + +trait Bar { + const TRAIT_VAR: &'static str; +} + +impl Foo { + const IMPL_VAR: &'static str = "var"; +} + +impl Bar for Foo { + const TRAIT_VAR: &'static str = "foo"; +} diff --git a/tests/ui/redundant_static_lifetimes.rs b/tests/ui/redundant_static_lifetimes.rs index fc9f0e066d43..4d4b249d076f 100644 --- a/tests/ui/redundant_static_lifetimes.rs +++ b/tests/ui/redundant_static_lifetimes.rs @@ -1,3 +1,7 @@ +// run-rustfix + +#![allow(unused)] + #[derive(Debug)] struct Foo {} @@ -9,12 +13,8 @@ const VAR_THREE: &[&'static str] = &["one", "two"]; // ERROR Consider removing ' const VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static -const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static - const VAR_SIX: &'static u8 = &5; -const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; - const VAR_HEIGHT: &'static Foo = &Foo {}; const VAR_SLICE: &'static [u8] = b"Test constant #1"; // ERROR Consider removing 'static. @@ -29,14 +29,8 @@ static STATIC_VAR_TWO: &str = "Test static #2"; // This line should not raise a static STATIC_VAR_THREE: &[&'static str] = &["one", "two"]; // ERROR Consider removing 'static -static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static - -static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static - static STATIC_VAR_SIX: &'static u8 = &5; -static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; - static STATIC_VAR_HEIGHT: &'static Foo = &Foo {}; static STATIC_VAR_SLICE: &'static [u8] = b"Test static #3"; // ERROR Consider removing 'static. @@ -47,15 +41,6 @@ static STATIC_VAR_ARRAY: &'static [u8; 1] = b"T"; // ERROR Consider removing 'st fn main() { let false_positive: &'static str = "test"; - println!("{}", VAR_ONE); - println!("{}", VAR_TWO); - println!("{:?}", VAR_THREE); - println!("{:?}", VAR_FOUR); - println!("{:?}", VAR_FIVE); - println!("{:?}", VAR_SIX); - println!("{:?}", VAR_SEVEN); - println!("{:?}", VAR_HEIGHT); - println!("{}", false_positive); } trait Bar { diff --git a/tests/ui/redundant_static_lifetimes.stderr b/tests/ui/redundant_static_lifetimes.stderr index da6175d1debc..3c3d2eacd8d9 100644 --- a/tests/ui/redundant_static_lifetimes.stderr +++ b/tests/ui/redundant_static_lifetimes.stderr @@ -1,5 +1,5 @@ error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:4:17 + --> $DIR/redundant_static_lifetimes.rs:8:17 | LL | const VAR_ONE: &'static str = "Test constant #1"; // ERROR Consider removing 'static. | -^^^^^^^---- help: consider removing `'static`: `&str` @@ -7,53 +7,29 @@ LL | const VAR_ONE: &'static str = "Test constant #1"; // ERROR Consider removin = note: `-D clippy::redundant-static-lifetimes` implied by `-D warnings` error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:8:21 + --> $DIR/redundant_static_lifetimes.rs:12:21 | LL | const VAR_THREE: &[&'static str] = &["one", "two"]; // ERROR Consider removing 'static | -^^^^^^^---- help: consider removing `'static`: `&str` error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:10:32 + --> $DIR/redundant_static_lifetimes.rs:14:32 | LL | const VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static | -^^^^^^^---- help: consider removing `'static`: `&str` error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:10:47 + --> $DIR/redundant_static_lifetimes.rs:14:47 | LL | const VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static | -^^^^^^^---- help: consider removing `'static`: `&str` error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:12:18 - | -LL | const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static - | -^^^^^^^------------------ help: consider removing `'static`: `&[&[&'static str]]` - -error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:12:30 - | -LL | const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static - | -^^^^^^^---- help: consider removing `'static`: `&str` - -error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:14:17 + --> $DIR/redundant_static_lifetimes.rs:16:17 | LL | const VAR_SIX: &'static u8 = &5; | -^^^^^^^--- help: consider removing `'static`: `&u8` -error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:16:29 - | -LL | const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; - | -^^^^^^^--------------- help: consider removing `'static`: `&[&'static str]` - -error: Constants have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:16:39 - | -LL | const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; - | -^^^^^^^---- help: consider removing `'static`: `&str` - error: Constants have by default a `'static` lifetime --> $DIR/redundant_static_lifetimes.rs:18:20 | @@ -91,70 +67,34 @@ LL | static STATIC_VAR_THREE: &[&'static str] = &["one", "two"]; // ERROR Consid | -^^^^^^^---- help: consider removing `'static`: `&str` error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:32:40 - | -LL | static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static - | -^^^^^^^---- help: consider removing `'static`: `&str` - -error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:32:55 - | -LL | static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static - | -^^^^^^^---- help: consider removing `'static`: `&str` - -error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:34:26 - | -LL | static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static - | -^^^^^^^------------------ help: consider removing `'static`: `&[&[&'static str]]` - -error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:34:38 - | -LL | static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static - | -^^^^^^^---- help: consider removing `'static`: `&str` - -error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:36:25 + --> $DIR/redundant_static_lifetimes.rs:32:25 | LL | static STATIC_VAR_SIX: &'static u8 = &5; | -^^^^^^^--- help: consider removing `'static`: `&u8` error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:38:37 - | -LL | static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; - | -^^^^^^^--------------- help: consider removing `'static`: `&[&'static str]` - -error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:38:47 - | -LL | static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; - | -^^^^^^^---- help: consider removing `'static`: `&str` - -error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:40:28 + --> $DIR/redundant_static_lifetimes.rs:34:28 | LL | static STATIC_VAR_HEIGHT: &'static Foo = &Foo {}; | -^^^^^^^---- help: consider removing `'static`: `&Foo` error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:42:27 + --> $DIR/redundant_static_lifetimes.rs:36:27 | LL | static STATIC_VAR_SLICE: &'static [u8] = b"Test static #3"; // ERROR Consider removing 'static. | -^^^^^^^----- help: consider removing `'static`: `&[u8]` error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:44:27 + --> $DIR/redundant_static_lifetimes.rs:38:27 | LL | static STATIC_VAR_TUPLE: &'static (u8, u8) = &(1, 2); // ERROR Consider removing 'static. | -^^^^^^^--------- help: consider removing `'static`: `&(u8, u8)` error: Statics have by default a `'static` lifetime - --> $DIR/redundant_static_lifetimes.rs:46:27 + --> $DIR/redundant_static_lifetimes.rs:40:27 | LL | static STATIC_VAR_ARRAY: &'static [u8; 1] = b"T"; // ERROR Consider removing 'static. | -^^^^^^^-------- help: consider removing `'static`: `&[u8; 1]` -error: aborting due to 26 previous errors +error: aborting due to 16 previous errors diff --git a/tests/ui/redundant_static_lifetimes_multiple.rs b/tests/ui/redundant_static_lifetimes_multiple.rs new file mode 100644 index 000000000000..f57dd58e230a --- /dev/null +++ b/tests/ui/redundant_static_lifetimes_multiple.rs @@ -0,0 +1,13 @@ +// these are rustfixable, but run-rustfix tests cannot handle them + +const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + +const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + +static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static + +static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + +static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + +fn main() {} diff --git a/tests/ui/redundant_static_lifetimes_multiple.stderr b/tests/ui/redundant_static_lifetimes_multiple.stderr new file mode 100644 index 000000000000..afc853dcfce8 --- /dev/null +++ b/tests/ui/redundant_static_lifetimes_multiple.stderr @@ -0,0 +1,64 @@ +error: Constants have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:3:18 + | +LL | const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + | -^^^^^^^------------------ help: consider removing `'static`: `&[&[&'static str]]` + | + = note: `-D clippy::redundant-static-lifetimes` implied by `-D warnings` + +error: Constants have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:3:30 + | +LL | const VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + | -^^^^^^^---- help: consider removing `'static`: `&str` + +error: Constants have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:5:29 + | +LL | const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + | -^^^^^^^--------------- help: consider removing `'static`: `&[&'static str]` + +error: Constants have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:5:39 + | +LL | const VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + | -^^^^^^^---- help: consider removing `'static`: `&str` + +error: Statics have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:7:40 + | +LL | static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static + | -^^^^^^^---- help: consider removing `'static`: `&str` + +error: Statics have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:7:55 + | +LL | static STATIC_VAR_FOUR: (&str, (&str, &'static str), &'static str) = ("on", ("th", "th"), "on"); // ERROR Consider removing 'static + | -^^^^^^^---- help: consider removing `'static`: `&str` + +error: Statics have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:9:26 + | +LL | static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + | -^^^^^^^------------------ help: consider removing `'static`: `&[&[&'static str]]` + +error: Statics have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:9:38 + | +LL | static STATIC_VAR_FIVE: &'static [&[&'static str]] = &[&["test"], &["other one"]]; // ERROR Consider removing 'static + | -^^^^^^^---- help: consider removing `'static`: `&str` + +error: Statics have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:11:37 + | +LL | static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + | -^^^^^^^--------------- help: consider removing `'static`: `&[&'static str]` + +error: Statics have by default a `'static` lifetime + --> $DIR/redundant_static_lifetimes_multiple.rs:11:47 + | +LL | static STATIC_VAR_SEVEN: &[&(&str, &'static [&'static str])] = &[&("one", &["other one"])]; + | -^^^^^^^---- help: consider removing `'static`: `&str` + +error: aborting due to 10 previous errors + From 6c10f2c939735f1bf38618bd44f9c173526ce231 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 10:33:48 -0700 Subject: [PATCH 24/26] string_add, string_add_assign: split tests, make one rustfixable --- tests/ui/string_add.rs | 19 +++++++++++ tests/ui/string_add.stderr | 30 +++++++++++++++++ tests/ui/string_add_assign.fixed | 21 ++++++++++++ tests/ui/string_add_assign.rs | 21 ++++++++++++ tests/ui/string_add_assign.stderr | 24 +++++++++++++ tests/ui/strings.rs | 55 ------------------------------ tests/ui/strings.stderr | 56 ------------------------------- 7 files changed, 115 insertions(+), 111 deletions(-) create mode 100644 tests/ui/string_add.rs create mode 100644 tests/ui/string_add.stderr create mode 100644 tests/ui/string_add_assign.fixed create mode 100644 tests/ui/string_add_assign.rs create mode 100644 tests/ui/string_add_assign.stderr delete mode 100644 tests/ui/strings.rs delete mode 100644 tests/ui/strings.stderr diff --git a/tests/ui/string_add.rs b/tests/ui/string_add.rs new file mode 100644 index 000000000000..c9dd13eea8a3 --- /dev/null +++ b/tests/ui/string_add.rs @@ -0,0 +1,19 @@ +#[warn(clippy::string_add)] +#[allow(clippy::string_add_assign, unused)] +fn main() { + // ignores assignment distinction + let mut x = "".to_owned(); + + for _ in 1..3 { + x = x + "."; + } + + let y = "".to_owned(); + let z = y + "..."; + + assert_eq!(&x, &z); + + let mut x = 1; + x = x + 1; + assert_eq!(2, x); +} diff --git a/tests/ui/string_add.stderr b/tests/ui/string_add.stderr new file mode 100644 index 000000000000..8345c50f9710 --- /dev/null +++ b/tests/ui/string_add.stderr @@ -0,0 +1,30 @@ +error: manual implementation of an assign operation + --> $DIR/string_add.rs:8:9 + | +LL | x = x + "."; + | ^^^^^^^^^^^ help: replace it with: `x += "."` + | + = note: `-D clippy::assign-op-pattern` implied by `-D warnings` + +error: you added something to a string. Consider using `String::push_str()` instead + --> $DIR/string_add.rs:8:13 + | +LL | x = x + "."; + | ^^^^^^^ + | + = note: `-D clippy::string-add` implied by `-D warnings` + +error: you added something to a string. Consider using `String::push_str()` instead + --> $DIR/string_add.rs:12:13 + | +LL | let z = y + "..."; + | ^^^^^^^^^ + +error: manual implementation of an assign operation + --> $DIR/string_add.rs:17:5 + | +LL | x = x + 1; + | ^^^^^^^^^ help: replace it with: `x += 1` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/string_add_assign.fixed b/tests/ui/string_add_assign.fixed new file mode 100644 index 000000000000..db71bab1e521 --- /dev/null +++ b/tests/ui/string_add_assign.fixed @@ -0,0 +1,21 @@ +// run-rustfix + +#[allow(clippy::string_add, unused)] +#[warn(clippy::string_add_assign)] +fn main() { + // ignores assignment distinction + let mut x = "".to_owned(); + + for _ in 1..3 { + x += "."; + } + + let y = "".to_owned(); + let z = y + "..."; + + assert_eq!(&x, &z); + + let mut x = 1; + x += 1; + assert_eq!(2, x); +} diff --git a/tests/ui/string_add_assign.rs b/tests/ui/string_add_assign.rs new file mode 100644 index 000000000000..644991945cbe --- /dev/null +++ b/tests/ui/string_add_assign.rs @@ -0,0 +1,21 @@ +// run-rustfix + +#[allow(clippy::string_add, unused)] +#[warn(clippy::string_add_assign)] +fn main() { + // ignores assignment distinction + let mut x = "".to_owned(); + + for _ in 1..3 { + x = x + "."; + } + + let y = "".to_owned(); + let z = y + "..."; + + assert_eq!(&x, &z); + + let mut x = 1; + x = x + 1; + assert_eq!(2, x); +} diff --git a/tests/ui/string_add_assign.stderr b/tests/ui/string_add_assign.stderr new file mode 100644 index 000000000000..7676175c1b82 --- /dev/null +++ b/tests/ui/string_add_assign.stderr @@ -0,0 +1,24 @@ +error: you assigned the result of adding something to this string. Consider using `String::push_str()` instead + --> $DIR/string_add_assign.rs:10:9 + | +LL | x = x + "."; + | ^^^^^^^^^^^ + | + = note: `-D clippy::string-add-assign` implied by `-D warnings` + +error: manual implementation of an assign operation + --> $DIR/string_add_assign.rs:10:9 + | +LL | x = x + "."; + | ^^^^^^^^^^^ help: replace it with: `x += "."` + | + = note: `-D clippy::assign-op-pattern` implied by `-D warnings` + +error: manual implementation of an assign operation + --> $DIR/string_add_assign.rs:19:5 + | +LL | x = x + 1; + | ^^^^^^^^^ help: replace it with: `x += 1` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/strings.rs b/tests/ui/strings.rs deleted file mode 100644 index 766e23c744a5..000000000000 --- a/tests/ui/strings.rs +++ /dev/null @@ -1,55 +0,0 @@ -#[warn(clippy::string_add)] -#[allow(clippy::string_add_assign)] -fn add_only() { - // ignores assignment distinction - let mut x = "".to_owned(); - - for _ in 1..3 { - x = x + "."; - } - - let y = "".to_owned(); - let z = y + "..."; - - assert_eq!(&x, &z); -} - -#[warn(clippy::string_add_assign)] -fn add_assign_only() { - let mut x = "".to_owned(); - - for _ in 1..3 { - x = x + "."; - } - - let y = "".to_owned(); - let z = y + "..."; - - assert_eq!(&x, &z); -} - -#[warn(clippy::string_add, clippy::string_add_assign)] -fn both() { - let mut x = "".to_owned(); - - for _ in 1..3 { - x = x + "."; - } - - let y = "".to_owned(); - let z = y + "..."; - - assert_eq!(&x, &z); -} - -#[allow(clippy::assign_op_pattern)] -fn main() { - add_only(); - add_assign_only(); - both(); - - // the add is only caught for `String` - let mut x = 1; - x = x + 1; - assert_eq!(2, x); -} diff --git a/tests/ui/strings.stderr b/tests/ui/strings.stderr deleted file mode 100644 index 7f684fe63555..000000000000 --- a/tests/ui/strings.stderr +++ /dev/null @@ -1,56 +0,0 @@ -error: manual implementation of an assign operation - --> $DIR/strings.rs:8:9 - | -LL | x = x + "."; - | ^^^^^^^^^^^ help: replace it with: `x += "."` - | - = note: `-D clippy::assign-op-pattern` implied by `-D warnings` - -error: you added something to a string. Consider using `String::push_str()` instead - --> $DIR/strings.rs:8:13 - | -LL | x = x + "."; - | ^^^^^^^ - | - = note: `-D clippy::string-add` implied by `-D warnings` - -error: you added something to a string. Consider using `String::push_str()` instead - --> $DIR/strings.rs:12:13 - | -LL | let z = y + "..."; - | ^^^^^^^^^ - -error: you assigned the result of adding something to this string. Consider using `String::push_str()` instead - --> $DIR/strings.rs:22:9 - | -LL | x = x + "."; - | ^^^^^^^^^^^ - | - = note: `-D clippy::string-add-assign` implied by `-D warnings` - -error: manual implementation of an assign operation - --> $DIR/strings.rs:22:9 - | -LL | x = x + "."; - | ^^^^^^^^^^^ help: replace it with: `x += "."` - -error: you assigned the result of adding something to this string. Consider using `String::push_str()` instead - --> $DIR/strings.rs:36:9 - | -LL | x = x + "."; - | ^^^^^^^^^^^ - -error: manual implementation of an assign operation - --> $DIR/strings.rs:36:9 - | -LL | x = x + "."; - | ^^^^^^^^^^^ help: replace it with: `x += "."` - -error: you added something to a string. Consider using `String::push_str()` instead - --> $DIR/strings.rs:40:13 - | -LL | let z = y + "..."; - | ^^^^^^^^^ - -error: aborting due to 8 previous errors - From 90ce4f5dd73f23ad3ace0b7cdcf8a67fc3f88d50 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 10:46:14 -0700 Subject: [PATCH 25/26] unnecessary_clone: split rustfixable lint out into separate test --- tests/ui/iter_cloned_collect.fixed | 22 ++++++++++++++++++++++ tests/ui/iter_cloned_collect.rs | 25 +++++++++++++++++++++++++ tests/ui/iter_cloned_collect.stderr | 26 ++++++++++++++++++++++++++ tests/ui/unnecessary_clone.rs | 23 ++--------------------- tests/ui/unnecessary_clone.stderr | 28 ++-------------------------- 5 files changed, 77 insertions(+), 47 deletions(-) create mode 100644 tests/ui/iter_cloned_collect.fixed create mode 100644 tests/ui/iter_cloned_collect.rs create mode 100644 tests/ui/iter_cloned_collect.stderr diff --git a/tests/ui/iter_cloned_collect.fixed b/tests/ui/iter_cloned_collect.fixed new file mode 100644 index 000000000000..2773227e26bc --- /dev/null +++ b/tests/ui/iter_cloned_collect.fixed @@ -0,0 +1,22 @@ +// run-rustfix + +#![allow(unused)] + +use std::collections::HashSet; +use std::collections::VecDeque; + +fn main() { + let v = [1, 2, 3, 4, 5]; + let v2: Vec = v.to_vec(); + let v3: HashSet = v.iter().cloned().collect(); + let v4: VecDeque = v.iter().cloned().collect(); + + // Handle macro expansion in suggestion + let _: Vec = vec![1, 2, 3].to_vec(); + + // Issue #3704 + unsafe { + let _: Vec = std::ffi::CStr::from_ptr(std::ptr::null()) + .to_bytes().to_vec(); + } +} diff --git a/tests/ui/iter_cloned_collect.rs b/tests/ui/iter_cloned_collect.rs new file mode 100644 index 000000000000..60a4eac23c79 --- /dev/null +++ b/tests/ui/iter_cloned_collect.rs @@ -0,0 +1,25 @@ +// run-rustfix + +#![allow(unused)] + +use std::collections::HashSet; +use std::collections::VecDeque; + +fn main() { + let v = [1, 2, 3, 4, 5]; + let v2: Vec = v.iter().cloned().collect(); + let v3: HashSet = v.iter().cloned().collect(); + let v4: VecDeque = v.iter().cloned().collect(); + + // Handle macro expansion in suggestion + let _: Vec = vec![1, 2, 3].iter().cloned().collect(); + + // Issue #3704 + unsafe { + let _: Vec = std::ffi::CStr::from_ptr(std::ptr::null()) + .to_bytes() + .iter() + .cloned() + .collect(); + } +} diff --git a/tests/ui/iter_cloned_collect.stderr b/tests/ui/iter_cloned_collect.stderr new file mode 100644 index 000000000000..b90a1e6c9196 --- /dev/null +++ b/tests/ui/iter_cloned_collect.stderr @@ -0,0 +1,26 @@ +error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable + --> $DIR/iter_cloned_collect.rs:10:27 + | +LL | let v2: Vec = v.iter().cloned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()` + | + = note: `-D clippy::iter-cloned-collect` implied by `-D warnings` + +error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable + --> $DIR/iter_cloned_collect.rs:15:38 + | +LL | let _: Vec = vec![1, 2, 3].iter().cloned().collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()` + +error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable + --> $DIR/iter_cloned_collect.rs:20:24 + | +LL | .to_bytes() + | ________________________^ +LL | | .iter() +LL | | .cloned() +LL | | .collect(); + | |______________________^ help: try: `.to_vec()` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/unnecessary_clone.rs b/tests/ui/unnecessary_clone.rs index 4ff835115909..a0a50fee1808 100644 --- a/tests/ui/unnecessary_clone.rs +++ b/tests/ui/unnecessary_clone.rs @@ -1,9 +1,9 @@ +// does not test any rustfixable lints + #![warn(clippy::clone_on_ref_ptr)] #![allow(unused)] use std::cell::RefCell; -use std::collections::HashSet; -use std::collections::VecDeque; use std::rc::{self, Rc}; use std::sync::{self, Arc}; @@ -66,25 +66,6 @@ fn clone_on_double_ref() { println!("{:p} {:p}", *y, z); } -fn iter_clone_collect() { - let v = [1, 2, 3, 4, 5]; - let v2: Vec = v.iter().cloned().collect(); - let v3: HashSet = v.iter().cloned().collect(); - let v4: VecDeque = v.iter().cloned().collect(); - - // Handle macro expansion in suggestion - let _: Vec = vec![1, 2, 3].iter().cloned().collect(); - - // Issue #3704 - unsafe { - let _: Vec = std::ffi::CStr::from_ptr(std::ptr::null()) - .to_bytes() - .iter() - .cloned() - .collect(); - } -} - mod many_derefs { struct A; struct B; diff --git a/tests/ui/unnecessary_clone.stderr b/tests/ui/unnecessary_clone.stderr index b1388044c42c..7ed1df8d703e 100644 --- a/tests/ui/unnecessary_clone.stderr +++ b/tests/ui/unnecessary_clone.stderr @@ -78,35 +78,11 @@ help: or try being explicit about what type to clone LL | let z: &Vec<_> = &std::vec::Vec::clone(y); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable - --> $DIR/unnecessary_clone.rs:71:27 - | -LL | let v2: Vec = v.iter().cloned().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()` - | - = note: `-D clippy::iter-cloned-collect` implied by `-D warnings` - -error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable - --> $DIR/unnecessary_clone.rs:76:38 - | -LL | let _: Vec = vec![1, 2, 3].iter().cloned().collect(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.to_vec()` - -error: called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and more readable - --> $DIR/unnecessary_clone.rs:81:24 - | -LL | .to_bytes() - | ________________________^ -LL | | .iter() -LL | | .cloned() -LL | | .collect(); - | |______________________^ help: try: `.to_vec()` - error: using `clone` on a `Copy` type - --> $DIR/unnecessary_clone.rs:119:20 + --> $DIR/unnecessary_clone.rs:100:20 | LL | let _: E = a.clone(); | ^^^^^^^^^ help: try dereferencing it: `*****a` -error: aborting due to 15 previous errors +error: aborting due to 12 previous errors From 472745cf1715c763fc3fb41e792035bcf2782056 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 25 Sep 2019 10:49:56 -0700 Subject: [PATCH 26/26] unnecessary_operation: make test rustfixable --- tests/ui/unnecessary_operation.fixed | 79 +++++++++++++++++++++++++++ tests/ui/unnecessary_operation.rs | 12 +++- tests/ui/unnecessary_operation.stderr | 48 ++++++++-------- 3 files changed, 112 insertions(+), 27 deletions(-) create mode 100644 tests/ui/unnecessary_operation.fixed diff --git a/tests/ui/unnecessary_operation.fixed b/tests/ui/unnecessary_operation.fixed new file mode 100644 index 000000000000..2fca96c4cd55 --- /dev/null +++ b/tests/ui/unnecessary_operation.fixed @@ -0,0 +1,79 @@ +// run-rustfix + +#![feature(box_syntax)] +#![allow(clippy::deref_addrof, dead_code, unused, clippy::no_effect)] +#![warn(clippy::unnecessary_operation)] + +struct Tuple(i32); +struct Struct { + field: i32, +} +enum Enum { + Tuple(i32), + Struct { field: i32 }, +} +struct DropStruct { + field: i32, +} +impl Drop for DropStruct { + fn drop(&mut self) {} +} +struct DropTuple(i32); +impl Drop for DropTuple { + fn drop(&mut self) {} +} +enum DropEnum { + Tuple(i32), + Struct { field: i32 }, +} +impl Drop for DropEnum { + fn drop(&mut self) {} +} +struct FooString { + s: String, +} + +fn get_number() -> i32 { + 0 +} + +fn get_usize() -> usize { + 0 +} +fn get_struct() -> Struct { + Struct { field: 0 } +} +fn get_drop_struct() -> DropStruct { + DropStruct { field: 0 } +} + +fn main() { + get_number(); + get_number(); + get_struct(); + get_number(); + get_number(); + 5;get_number(); + get_number(); + get_number(); + 5;6;get_number(); + get_number(); + get_number(); + get_number(); + 5;get_number(); + 42;get_number(); + [42, 55];get_usize(); + 42;get_number(); + get_number(); + [42; 55];get_usize(); + get_number(); + String::from("blah"); + + // Do not warn + DropTuple(get_number()); + DropStruct { field: get_number() }; + DropStruct { field: get_number() }; + DropStruct { ..get_drop_struct() }; + DropEnum::Tuple(get_number()); + DropEnum::Struct { field: get_number() }; +} diff --git a/tests/ui/unnecessary_operation.rs b/tests/ui/unnecessary_operation.rs index 3c6796fea7b3..08cb9ab522ee 100644 --- a/tests/ui/unnecessary_operation.rs +++ b/tests/ui/unnecessary_operation.rs @@ -1,5 +1,7 @@ +// run-rustfix + #![feature(box_syntax)] -#![allow(clippy::deref_addrof)] +#![allow(clippy::deref_addrof, dead_code, unused, clippy::no_effect)] #![warn(clippy::unnecessary_operation)] struct Tuple(i32); @@ -34,6 +36,10 @@ struct FooString { fn get_number() -> i32 { 0 } + +fn get_usize() -> usize { + 0 +} fn get_struct() -> Struct { Struct { field: 0 } } @@ -56,10 +62,10 @@ fn main() { ..get_number(); 5..get_number(); [42, get_number()]; - [42, 55][get_number() as usize]; + [42, 55][get_usize()]; (42, get_number()).1; [get_number(); 55]; - [42; 55][get_number() as usize]; + [42; 55][get_usize()]; { get_number() }; diff --git a/tests/ui/unnecessary_operation.stderr b/tests/ui/unnecessary_operation.stderr index 826bf6e2c288..f88c9f9908be 100644 --- a/tests/ui/unnecessary_operation.stderr +++ b/tests/ui/unnecessary_operation.stderr @@ -1,5 +1,5 @@ error: statement can be reduced - --> $DIR/unnecessary_operation.rs:45:5 + --> $DIR/unnecessary_operation.rs:51:5 | LL | Tuple(get_number()); | ^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` @@ -7,109 +7,109 @@ LL | Tuple(get_number()); = note: `-D clippy::unnecessary-operation` implied by `-D warnings` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:46:5 + --> $DIR/unnecessary_operation.rs:52:5 | LL | Struct { field: get_number() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:47:5 + --> $DIR/unnecessary_operation.rs:53:5 | LL | Struct { ..get_struct() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_struct();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:48:5 + --> $DIR/unnecessary_operation.rs:54:5 | LL | Enum::Tuple(get_number()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:49:5 + --> $DIR/unnecessary_operation.rs:55:5 | LL | Enum::Struct { field: get_number() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:50:5 + --> $DIR/unnecessary_operation.rs:56:5 | LL | 5 + get_number(); | ^^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:51:5 + --> $DIR/unnecessary_operation.rs:57:5 | LL | *&get_number(); | ^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:52:5 + --> $DIR/unnecessary_operation.rs:58:5 | LL | &get_number(); | ^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:53:5 + --> $DIR/unnecessary_operation.rs:59:5 | LL | (5, 6, get_number()); | ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `5;6;get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:54:5 + --> $DIR/unnecessary_operation.rs:60:5 | LL | box get_number(); | ^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:55:5 + --> $DIR/unnecessary_operation.rs:61:5 | LL | get_number()..; | ^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:56:5 + --> $DIR/unnecessary_operation.rs:62:5 | LL | ..get_number(); | ^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:57:5 + --> $DIR/unnecessary_operation.rs:63:5 | LL | 5..get_number(); | ^^^^^^^^^^^^^^^^ help: replace it with: `5;get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:58:5 + --> $DIR/unnecessary_operation.rs:64:5 | LL | [42, get_number()]; | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:59:5 + --> $DIR/unnecessary_operation.rs:65:5 | -LL | [42, 55][get_number() as usize]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42, 55];get_number() as usize;` +LL | [42, 55][get_usize()]; + | ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42, 55];get_usize();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:60:5 + --> $DIR/unnecessary_operation.rs:66:5 | LL | (42, get_number()).1; | ^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `42;get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:61:5 + --> $DIR/unnecessary_operation.rs:67:5 | LL | [get_number(); 55]; | ^^^^^^^^^^^^^^^^^^^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:62:5 + --> $DIR/unnecessary_operation.rs:68:5 | -LL | [42; 55][get_number() as usize]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42; 55];get_number() as usize;` +LL | [42; 55][get_usize()]; + | ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `[42; 55];get_usize();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:63:5 + --> $DIR/unnecessary_operation.rs:69:5 | LL | / { LL | | get_number() @@ -117,7 +117,7 @@ LL | | }; | |______^ help: replace it with: `get_number();` error: statement can be reduced - --> $DIR/unnecessary_operation.rs:66:5 + --> $DIR/unnecessary_operation.rs:72:5 | LL | / FooString { LL | | s: String::from("blah"),