From 945d4cf69f7a2a63f17e82c5927bad75922a6005 Mon Sep 17 00:00:00 2001 From: BO41 Date: Mon, 26 Aug 2019 12:50:15 +0200 Subject: [PATCH 1/5] Dereference one less on search_is_some and make it auto-fixable --- clippy_lints/src/methods/mod.rs | 25 ++- tests/ui/methods.fixed | 306 ++++++++++++++++++++++++++++++++ tests/ui/methods.rs | 4 + tests/ui/methods.stderr | 65 ++++--- 4 files changed, 363 insertions(+), 37 deletions(-) create mode 100644 tests/ui/methods.fixed diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 81a8e69220c1..06126e02ed33 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2386,6 +2386,7 @@ fn lint_search_is_some<'a, 'tcx>( let search_snippet = snippet(cx, search_args[1].span, ".."); if search_snippet.lines().count() <= 1 { // suggest `any(|x| ..)` instead of `any(|&x| ..)` for `find(|&x| ..).is_some()` + // suggest `any(|..| *..)` instead of `any(|..| **..)` for `find(|..| **..).is_some()` let any_search_snippet = if_chain! { if search_method == "find"; if let hir::ExprKind::Closure(_, _, body_id, ..) = search_args[1].node; @@ -2393,24 +2394,32 @@ fn lint_search_is_some<'a, 'tcx>( if let Some(closure_arg) = closure_body.params.get(0); if let hir::PatKind::Ref(..) = closure_arg.pat.node; then { - Some(search_snippet.replacen('&', "", 1)) + match &closure_arg.pat.node { + hir::PatKind::Ref(..) => Some(search_snippet.replacen('&', "", 1)), + hir::PatKind::Binding(_, _, expr, _) => { + let closure_arg_snip = snippet(cx, expr.span, ".."); + Some(search_snippet.replace(&format!("*{}", closure_arg_snip), &closure_arg_snip)) + } + _ => None, + } } else { None } }; // add note if not multi-line - span_note_and_lint( + span_lint_and_sugg( cx, SEARCH_IS_SOME, expr.span, &msg, - expr.span, - &format!( - "replace `{0}({1}).is_some()` with `any({2})`", - search_method, - search_snippet, - any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str) + "try this", + format!( + "any({})", + any_search_snippet + .as_ref() + .map_or(&*search_snippet, String::as_str) ), + Applicability::MachineApplicable, ); } else { span_lint(cx, SEARCH_IS_SOME, expr.span, &msg); diff --git a/tests/ui/methods.fixed b/tests/ui/methods.fixed new file mode 100644 index 000000000000..f86da92bae32 --- /dev/null +++ b/tests/ui/methods.fixed @@ -0,0 +1,306 @@ +// aux-build:option_helpers.rs +// compile-flags: --edition 2018 +// run-rustfix + +#![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)] +#![allow( + clippy::blacklisted_name, + unused, + clippy::print_stdout, + clippy::non_ascii_literal, + clippy::new_without_default, + clippy::missing_docs_in_private_items, + clippy::needless_pass_by_value, + clippy::default_trait_access, + clippy::use_self, + clippy::useless_format, + clippy::wrong_self_convention +)] + +#[macro_use] +extern crate option_helpers; + +use std::collections::BTreeMap; +use std::collections::HashMap; +use std::collections::HashSet; +use std::collections::VecDeque; +use std::iter::FromIterator; +use std::ops::Mul; +use std::rc::{self, Rc}; +use std::sync::{self, Arc}; + +use option_helpers::IteratorFalsePositives; + +pub struct T; + +impl T { + pub fn add(self, other: T) -> T { + self + } + + // no error, not public interface + pub(crate) fn drop(&mut self) {} + + // no error, private function + fn neg(self) -> Self { + self + } + + // no error, private function + fn eq(&self, other: T) -> bool { + true + } + + // No error; self is a ref. + fn sub(&self, other: T) -> &T { + self + } + + // No error; different number of arguments. + fn div(self) -> T { + self + } + + // No error; wrong return type. + fn rem(self, other: T) {} + + // Fine + fn into_u32(self) -> u32 { + 0 + } + + fn into_u16(&self) -> u16 { + 0 + } + + fn to_something(self) -> u32 { + 0 + } + + fn new(self) -> Self { + unimplemented!(); + } +} + +struct Lt<'a> { + foo: &'a u32, +} + +impl<'a> Lt<'a> { + // The lifetime is different, but that’s irrelevant; see issue #734. + #[allow(clippy::needless_lifetimes)] + pub fn new<'b>(s: &'b str) -> Lt<'b> { + unimplemented!() + } +} + +struct Lt2<'a> { + foo: &'a u32, +} + +impl<'a> Lt2<'a> { + // The lifetime is different, but that’s irrelevant; see issue #734. + pub fn new(s: &str) -> Lt2 { + unimplemented!() + } +} + +struct Lt3<'a> { + foo: &'a u32, +} + +impl<'a> Lt3<'a> { + // The lifetime is different, but that’s irrelevant; see issue #734. + pub fn new() -> Lt3<'static> { + unimplemented!() + } +} + +#[derive(Clone, Copy)] +struct U; + +impl U { + fn new() -> Self { + U + } + // Ok because `U` is `Copy`. + fn to_something(self) -> u32 { + 0 + } +} + +struct V { + _dummy: T, +} + +impl V { + fn new() -> Option> { + None + } +} + +struct AsyncNew; + +impl AsyncNew { + async fn new() -> Option { + None + } +} + +struct BadNew; + +impl BadNew { + fn new() -> i32 { + 0 + } +} + +impl Mul for T { + type Output = T; + // No error, obviously. + fn mul(self, other: T) -> T { + self + } +} + +/// Checks implementation of the following lints: +/// * `OPTION_MAP_UNWRAP_OR` +/// * `OPTION_MAP_UNWRAP_OR_ELSE` +#[rustfmt::skip] +fn option_methods() { + let opt = Some(1); + + // Check `OPTION_MAP_UNWRAP_OR`. + // Single line case. + let _ = opt.map(|x| x + 1) + // Should lint even though this call is on a separate line. + .unwrap_or(0); + // Multi-line cases. + let _ = opt.map(|x| { + x + 1 + } + ).unwrap_or(0); + let _ = opt.map(|x| x + 1) + .unwrap_or({ + 0 + }); + // Single line `map(f).unwrap_or(None)` case. + let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); + // Multi-line `map(f).unwrap_or(None)` cases. + let _ = opt.map(|x| { + Some(x + 1) + } + ).unwrap_or(None); + let _ = opt + .map(|x| Some(x + 1)) + .unwrap_or(None); + // macro case + let _ = opt_map!(opt, |x| x + 1).unwrap_or(0); // should not lint + + // Should not lint if not copyable + let id: String = "identifier".to_string(); + let _ = Some("prefix").map(|p| format!("{}.{}", p, id)).unwrap_or(id); + // ...but DO lint if the `unwrap_or` argument is not used in the `map` + let id: String = "identifier".to_string(); + let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); + + // Check OPTION_MAP_UNWRAP_OR_ELSE + // single line case + let _ = opt.map(|x| x + 1) + // Should lint even though this call is on a separate line. + .unwrap_or_else(|| 0); + // Multi-line cases. + let _ = opt.map(|x| { + x + 1 + } + ).unwrap_or_else(|| 0); + let _ = opt.map(|x| x + 1) + .unwrap_or_else(|| + 0 + ); + // Macro case. + // Should not lint. + let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0); + + // Issue #4144 + { + let mut frequencies = HashMap::new(); + let word = "foo"; + + frequencies + .get_mut(word) + .map(|count| { + *count += 1; + }) + .unwrap_or_else(|| { + frequencies.insert(word.to_owned(), 1); + }); + } +} + +/// Checks implementation of `FILTER_NEXT` lint. +#[rustfmt::skip] +fn filter_next() { + let v = vec![3, 2, 1, 0, -1, -2, -3]; + + // Single-line case. + let _ = v.iter().filter(|&x| *x < 0).next(); + + // Multi-line case. + let _ = v.iter().filter(|&x| { + *x < 0 + } + ).next(); + + // Check that hat we don't lint if the caller is not an `Iterator`. + let foo = IteratorFalsePositives { foo: 0 }; + let _ = foo.filter().next(); +} + +/// Checks implementation of `SEARCH_IS_SOME` lint. +#[rustfmt::skip] +fn search_is_some() { + let v = vec![3, 2, 1, 0, -1, -2, -3]; + let y = &&42; + + // Check `find().is_some()`, single-line case. + let _ = any(|x| *x < 0); + let _ = any(|x| **y == x); // one dereference less + let _ = any(|x| x == 0); + + // Check `find().is_some()`, multi-line case. + let _ = v.iter().find(|&x| { + *x < 0 + } + ).is_some(); + + // Check `position().is_some()`, single-line case. + let _ = any(|&x| x < 0); + + // Check `position().is_some()`, multi-line case. + let _ = v.iter().position(|&x| { + x < 0 + } + ).is_some(); + + // Check `rposition().is_some()`, single-line case. + let _ = any(|&x| x < 0); + + // Check `rposition().is_some()`, multi-line case. + let _ = v.iter().rposition(|&x| { + x < 0 + } + ).is_some(); + + // Check that we don't lint if the caller is not an `Iterator`. + let foo = IteratorFalsePositives { foo: 0 }; + let _ = foo.find().is_some(); + let _ = foo.position().is_some(); + let _ = foo.rposition().is_some(); +} + +#[allow(clippy::similar_names)] +fn main() { + let opt = Some(0); + let _ = opt.unwrap(); +} diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index 53f9f82485d1..930b76a68677 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -1,5 +1,6 @@ // aux-build:option_helpers.rs // compile-flags: --edition 2018 +// run-rustfix #![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)] #![allow( @@ -260,9 +261,12 @@ fn filter_next() { #[rustfmt::skip] fn search_is_some() { let v = vec![3, 2, 1, 0, -1, -2, -3]; + let y = &&42; // Check `find().is_some()`, single-line case. let _ = v.iter().find(|&x| *x < 0).is_some(); + let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less + let _ = (0..1).find(|x| *x == 0).is_some(); // Check `find().is_some()`, multi-line case. let _ = v.iter().find(|&x| { diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index 504bb5e8253f..c35a74463ec8 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -1,5 +1,5 @@ error: defining a method called `add` on this type; consider implementing the `std::ops::Add` trait or choosing a less ambiguous name - --> $DIR/methods.rs:36:5 + --> $DIR/methods.rs:37:5 | LL | / pub fn add(self, other: T) -> T { LL | | self @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::should-implement-trait` implied by `-D warnings` error: methods called `new` usually return `Self` - --> $DIR/methods.rs:152:5 + --> $DIR/methods.rs:153:5 | LL | / fn new() -> i32 { LL | | 0 @@ -19,7 +19,7 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:174:13 + --> $DIR/methods.rs:175:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -31,7 +31,7 @@ LL | | .unwrap_or(0); = note: replace `map(|x| x + 1).unwrap_or(0)` with `map_or(0, |x| x + 1)` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:178:13 + --> $DIR/methods.rs:179:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -41,7 +41,7 @@ LL | | ).unwrap_or(0); | |____________________________^ error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:182:13 + --> $DIR/methods.rs:183:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -51,7 +51,7 @@ LL | | }); | |__________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:187:13 + --> $DIR/methods.rs:188:13 | LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:189:13 + --> $DIR/methods.rs:190:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -69,7 +69,7 @@ LL | | ).unwrap_or(None); | |_____________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:193:13 + --> $DIR/methods.rs:194:13 | LL | let _ = opt | _____________^ @@ -80,7 +80,7 @@ LL | | .unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:204:13 + --> $DIR/methods.rs:205:13 | LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); = note: replace `map(|p| format!("{}.", p)).unwrap_or(id)` with `map_or(id, |p| format!("{}.", p))` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:208:13 + --> $DIR/methods.rs:209:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -100,7 +100,7 @@ LL | | .unwrap_or_else(|| 0); = note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:212:13 + --> $DIR/methods.rs:213:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -110,7 +110,7 @@ LL | | ).unwrap_or_else(|| 0); | |____________________________________^ error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:216:13 + --> $DIR/methods.rs:217:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -120,7 +120,7 @@ LL | | ); | |_________________^ error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:246:13 + --> $DIR/methods.rs:247:13 | LL | let _ = v.iter().filter(|&x| *x < 0).next(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -129,7 +129,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next(); = note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)` error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:249:13 + --> $DIR/methods.rs:250:13 | LL | let _ = v.iter().filter(|&x| { | _____________^ @@ -139,17 +139,28 @@ LL | | ).next(); | |___________________________^ error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:265:13 + --> $DIR/methods.rs:267:13 | LL | let _ = v.iter().find(|&x| *x < 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` | = note: `-D clippy::search-is-some` implied by `-D warnings` - = note: replace `find(|&x| *x < 0).is_some()` with `any(|x| *x < 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. --> $DIR/methods.rs:268:13 | +LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` + +error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. + --> $DIR/methods.rs:269:13 + | +LL | let _ = (0..1).find(|x| *x == 0).is_some(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` + +error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. + --> $DIR/methods.rs:272:13 + | LL | let _ = v.iter().find(|&x| { | _____________^ LL | | *x < 0 @@ -158,15 +169,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:274:13 + --> $DIR/methods.rs:278:13 | LL | let _ = v.iter().position(|&x| x < 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: replace `position(|&x| x < 0).is_some()` with `any(|&x| x < 0)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:277:13 + --> $DIR/methods.rs:281:13 | LL | let _ = v.iter().position(|&x| { | _____________^ @@ -176,15 +185,13 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:283:13 + --> $DIR/methods.rs:287:13 | LL | let _ = v.iter().rposition(|&x| x < 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: replace `rposition(|&x| x < 0).is_some()` with `any(|&x| x < 0)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:286:13 + --> $DIR/methods.rs:290:13 | LL | let _ = v.iter().rposition(|&x| { | _____________^ @@ -194,12 +201,12 @@ LL | | ).is_some(); | |______________________________^ error: used unwrap() on an Option value. If you don't want to handle the None case gracefully, consider using expect() to provide a better panic message - --> $DIR/methods.rs:301:13 + --> $DIR/methods.rs:305:13 | LL | let _ = opt.unwrap(); | ^^^^^^^^^^^^ | = note: `-D clippy::option-unwrap-used` implied by `-D warnings` -error: aborting due to 21 previous errors +error: aborting due to 23 previous errors From 832c0830ec3d6be0f406cc76611975a1cb8bd8bc Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 28 Aug 2019 10:41:06 +0200 Subject: [PATCH 2/5] Also return the method spans in utils::method_calls --- clippy_lints/src/utils/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 8fb45899653c..9d88e020fd11 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -339,25 +339,27 @@ pub fn resolve_node(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> Res { /// Returns the method names and argument list of nested method call expressions that make up /// `expr`. -pub fn method_calls(expr: &Expr, max_depth: usize) -> (Vec, Vec<&[Expr]>) { +pub fn method_calls(expr: &Expr, max_depth: usize) -> (Vec, Vec<&[Expr]>, Vec) { let mut method_names = Vec::with_capacity(max_depth); let mut arg_lists = Vec::with_capacity(max_depth); + let mut spans = Vec::with_capacity(max_depth); let mut current = expr; for _ in 0..max_depth { - if let ExprKind::MethodCall(path, _, args) = ¤t.node { + if let ExprKind::MethodCall(path, span, args) = ¤t.node { if args.iter().any(|e| e.span.from_expansion()) { break; } method_names.push(path.ident.name); arg_lists.push(&**args); + spans.push(*span); current = &args[0]; } else { break; } } - (method_names, arg_lists) + (method_names, arg_lists, spans) } /// Matches an `Expr` against a chain of methods, and return the matched `Expr`s. From dac81d867b64ce043dbb7708cf1fdf8b6ef10560 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 28 Aug 2019 10:41:32 +0200 Subject: [PATCH 3/5] Use the spans returned by utils::method_calls --- clippy_lints/src/methods/mod.rs | 50 +++++++++++++----------- clippy_lints/src/utils/internal_lints.rs | 6 +-- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 06126e02ed33..3708a370936b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1000,7 +1000,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { return; } - let (method_names, arg_lists) = method_calls(expr, 2); + let (method_names, arg_lists, method_spans) = method_calls(expr, 2); let method_names: Vec = method_names.iter().map(|s| s.as_str()).collect(); let method_names: Vec<&str> = method_names.iter().map(std::convert::AsRef::as_ref).collect(); @@ -1020,7 +1020,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["map", "find"] => lint_find_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", "filter"] => lint_filter_flat_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]), - ["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0]), + ["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0], method_spans[0]), ["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]), ["is_some", "find"] => lint_search_is_some(cx, expr, "find", arg_lists[1], arg_lists[0]), ["is_some", "position"] => lint_search_is_some(cx, expr, "position", arg_lists[1], arg_lists[0]), @@ -1035,7 +1035,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["collect", "cloned"] => lint_iter_cloned_collect(cx, expr, arg_lists[1]), ["as_ref"] => lint_asref(cx, expr, "as_ref", arg_lists[0]), ["as_mut"] => lint_asref(cx, expr, "as_mut", arg_lists[0]), - ["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0]), + ["fold", ..] => lint_unnecessary_fold(cx, expr, arg_lists[0], method_spans[0]), ["filter_map", ..] => unnecessary_filter_map::lint(cx, expr, arg_lists[0]), ["count", "map"] => lint_suspicious_map(cx, expr), _ => {}, @@ -1712,11 +1712,12 @@ fn lint_iter_cloned_collect<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &hir::Ex } } -fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: &[hir::Expr]) { +fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: &[hir::Expr], fold_span: Span) { fn check_fold_with_op( cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: &[hir::Expr], + fold_span: Span, op: hir::BinOpKind, replacement_method_name: &str, replacement_has_args: bool, @@ -1738,8 +1739,6 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: if match_var(&*left_expr, first_arg_ident); if replacement_has_args || match_var(&*right_expr, second_arg_ident); - if let hir::ExprKind::MethodCall(_, span, _) = &expr.node; - then { let mut applicability = Applicability::MachineApplicable; let sugg = if replacement_has_args { @@ -1759,7 +1758,7 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: span_lint_and_sugg( cx, UNNECESSARY_FOLD, - span.with_hi(expr.span.hi()), + fold_span.with_hi(expr.span.hi()), // TODO #2371 don't suggest e.g., .any(|x| f(x)) if we can suggest .any(f) "this `.fold` can be written more succinctly using another method", "try", @@ -1783,10 +1782,18 @@ fn lint_unnecessary_fold(cx: &LateContext<'_, '_>, expr: &hir::Expr, fold_args: // Check if the first argument to .fold is a suitable literal if let hir::ExprKind::Lit(ref lit) = fold_args[1].node { match lit.node { - ast::LitKind::Bool(false) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::Or, "any", true), - ast::LitKind::Bool(true) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::And, "all", true), - ast::LitKind::Int(0, _) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::Add, "sum", false), - ast::LitKind::Int(1, _) => check_fold_with_op(cx, expr, fold_args, hir::BinOpKind::Mul, "product", false), + ast::LitKind::Bool(false) => { + check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Or, "any", true) + }, + ast::LitKind::Bool(true) => { + check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::And, "all", true) + }, + ast::LitKind::Int(0, _) => { + check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Add, "sum", false) + }, + ast::LitKind::Int(1, _) => { + check_fold_with_op(cx, expr, fold_args, fold_span, hir::BinOpKind::Mul, "product", false) + }, _ => (), } } @@ -2323,22 +2330,21 @@ fn lint_flat_map_identity<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, flat_map_args: &'tcx [hir::Expr], + flat_map_span: Span, ) { if match_trait_method(cx, expr, &paths::ITERATOR) { let arg_node = &flat_map_args[1].node; let apply_lint = |message: &str| { - if let hir::ExprKind::MethodCall(_, span, _) = &expr.node { - span_lint_and_sugg( - cx, - FLAT_MAP_IDENTITY, - span.with_hi(expr.span.hi()), - message, - "try", - "flatten()".to_string(), - Applicability::MachineApplicable, - ); - } + span_lint_and_sugg( + cx, + FLAT_MAP_IDENTITY, + flat_map_span.with_hi(expr.span.hi()), + message, + "try", + "flatten()".to_string(), + Applicability::MachineApplicable, + ); }; if_chain! { diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index 855b1e6eff3d..48400593b9d2 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -280,7 +280,7 @@ impl_lint_pass!(OuterExpnDataPass => [OUTER_EXPN_EXPN_DATA]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OuterExpnDataPass { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) { - let (method_names, arg_lists) = method_calls(expr, 2); + let (method_names, arg_lists, spans) = method_calls(expr, 2); let method_names: Vec = method_names.iter().map(|s| s.as_str()).collect(); let method_names: Vec<&str> = method_names.iter().map(std::convert::AsRef::as_ref).collect(); if_chain! { @@ -294,10 +294,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OuterExpnDataPass { span_lint_and_sugg( cx, OUTER_EXPN_EXPN_DATA, - expr.span.trim_start(self_arg.span).unwrap_or(expr.span), + spans[1].with_hi(expr.span.hi()), "usage of `outer_expn().expn_data()`", "try", - ".outer_expn_data()".to_string(), + "outer_expn_data()".to_string(), Applicability::MachineApplicable, ); } From 6bbf4187472bf2326de7256feda5d1fc4d0e651e Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 28 Aug 2019 10:52:23 +0200 Subject: [PATCH 4/5] Fix tests --- tests/ui/outer_expn_data.stderr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ui/outer_expn_data.stderr b/tests/ui/outer_expn_data.stderr index 6803b3feb8e6..15fb0ad2ede1 100644 --- a/tests/ui/outer_expn_data.stderr +++ b/tests/ui/outer_expn_data.stderr @@ -1,8 +1,8 @@ error: usage of `outer_expn().expn_data()` - --> $DIR/outer_expn_data.rs:21:33 + --> $DIR/outer_expn_data.rs:21:34 | LL | let _ = expr.span.ctxt().outer_expn().expn_data(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `.outer_expn_data()` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `outer_expn_data()` | note: lint level defined here --> $DIR/outer_expn_data.rs:3:9 From 64cd9e4d6087667aedcd786142755d82056b09d1 Mon Sep 17 00:00:00 2001 From: BO41 Date: Thu, 29 Aug 2019 10:06:56 +0200 Subject: [PATCH 5/5] Try to fix .fixed --- clippy_lints/src/methods/mod.rs | 31 ++-- clippy_lints/src/utils/mod.rs | 2 +- tests/ui/methods.fixed | 306 -------------------------------- tests/ui/methods.rs | 2 +- tests/ui/methods.stderr | 56 +++--- 5 files changed, 49 insertions(+), 348 deletions(-) delete mode 100644 tests/ui/methods.fixed diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3708a370936b..8272f8b8a061 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1022,9 +1022,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["flat_map", "filter_map"] => lint_filter_map_flat_map(cx, expr, arg_lists[1], arg_lists[0]), ["flat_map", ..] => lint_flat_map_identity(cx, expr, arg_lists[0], method_spans[0]), ["flatten", "map"] => lint_map_flatten(cx, expr, arg_lists[1]), - ["is_some", "find"] => lint_search_is_some(cx, expr, "find", arg_lists[1], arg_lists[0]), - ["is_some", "position"] => lint_search_is_some(cx, expr, "position", arg_lists[1], arg_lists[0]), - ["is_some", "rposition"] => lint_search_is_some(cx, expr, "rposition", arg_lists[1], arg_lists[0]), + ["is_some", "find"] => lint_search_is_some(cx, expr, "find", arg_lists[1], arg_lists[0], method_spans[1]), + ["is_some", "position"] => { + lint_search_is_some(cx, expr, "position", arg_lists[1], arg_lists[0], method_spans[1]) + }, + ["is_some", "rposition"] => { + lint_search_is_some(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1]) + }, ["extend", ..] => lint_extend(cx, expr, arg_lists[0]), ["as_ptr", "unwrap"] | ["as_ptr", "expect"] => { lint_cstring_as_ptr(cx, expr, &arg_lists[1][0], &arg_lists[0][0]) @@ -2381,6 +2385,7 @@ fn lint_search_is_some<'a, 'tcx>( search_method: &str, search_args: &'tcx [hir::Expr], is_some_args: &'tcx [hir::Expr], + method_span: Span, ) { // lint if caller of search is an Iterator if match_trait_method(cx, &is_some_args[0], &paths::ITERATOR) { @@ -2398,15 +2403,13 @@ fn lint_search_is_some<'a, 'tcx>( if let hir::ExprKind::Closure(_, _, body_id, ..) = search_args[1].node; let closure_body = cx.tcx.hir().body(body_id); if let Some(closure_arg) = closure_body.params.get(0); - if let hir::PatKind::Ref(..) = closure_arg.pat.node; then { - match &closure_arg.pat.node { - hir::PatKind::Ref(..) => Some(search_snippet.replacen('&', "", 1)), - hir::PatKind::Binding(_, _, expr, _) => { - let closure_arg_snip = snippet(cx, expr.span, ".."); - Some(search_snippet.replace(&format!("*{}", closure_arg_snip), &closure_arg_snip)) - } - _ => None, + if let hir::PatKind::Ref(..) = closure_arg.pat.node { + Some(search_snippet.replacen('&', "", 1)) + } else if let Some(name) = get_arg_name(&closure_arg.pat) { + Some(search_snippet.replace(&format!("*{}", name), &name.as_str())) + } else { + None } } else { None @@ -2416,14 +2419,12 @@ fn lint_search_is_some<'a, 'tcx>( span_lint_and_sugg( cx, SEARCH_IS_SOME, - expr.span, + method_span.with_hi(expr.span.hi()), &msg, "try this", format!( "any({})", - any_search_snippet - .as_ref() - .map_or(&*search_snippet, String::as_str) + any_search_snippet.as_ref().map_or(&*search_snippet, String::as_str) ), Applicability::MachineApplicable, ); diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 9d88e020fd11..2bb09d8e0909 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -338,7 +338,7 @@ pub fn resolve_node(cx: &LateContext<'_, '_>, qpath: &QPath, id: HirId) -> Res { } /// Returns the method names and argument list of nested method call expressions that make up -/// `expr`. +/// `expr`. method/span lists are sorted with the most recent call first. pub fn method_calls(expr: &Expr, max_depth: usize) -> (Vec, Vec<&[Expr]>, Vec) { let mut method_names = Vec::with_capacity(max_depth); let mut arg_lists = Vec::with_capacity(max_depth); diff --git a/tests/ui/methods.fixed b/tests/ui/methods.fixed deleted file mode 100644 index f86da92bae32..000000000000 --- a/tests/ui/methods.fixed +++ /dev/null @@ -1,306 +0,0 @@ -// aux-build:option_helpers.rs -// compile-flags: --edition 2018 -// run-rustfix - -#![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)] -#![allow( - clippy::blacklisted_name, - unused, - clippy::print_stdout, - clippy::non_ascii_literal, - clippy::new_without_default, - clippy::missing_docs_in_private_items, - clippy::needless_pass_by_value, - clippy::default_trait_access, - clippy::use_self, - clippy::useless_format, - clippy::wrong_self_convention -)] - -#[macro_use] -extern crate option_helpers; - -use std::collections::BTreeMap; -use std::collections::HashMap; -use std::collections::HashSet; -use std::collections::VecDeque; -use std::iter::FromIterator; -use std::ops::Mul; -use std::rc::{self, Rc}; -use std::sync::{self, Arc}; - -use option_helpers::IteratorFalsePositives; - -pub struct T; - -impl T { - pub fn add(self, other: T) -> T { - self - } - - // no error, not public interface - pub(crate) fn drop(&mut self) {} - - // no error, private function - fn neg(self) -> Self { - self - } - - // no error, private function - fn eq(&self, other: T) -> bool { - true - } - - // No error; self is a ref. - fn sub(&self, other: T) -> &T { - self - } - - // No error; different number of arguments. - fn div(self) -> T { - self - } - - // No error; wrong return type. - fn rem(self, other: T) {} - - // Fine - fn into_u32(self) -> u32 { - 0 - } - - fn into_u16(&self) -> u16 { - 0 - } - - fn to_something(self) -> u32 { - 0 - } - - fn new(self) -> Self { - unimplemented!(); - } -} - -struct Lt<'a> { - foo: &'a u32, -} - -impl<'a> Lt<'a> { - // The lifetime is different, but that’s irrelevant; see issue #734. - #[allow(clippy::needless_lifetimes)] - pub fn new<'b>(s: &'b str) -> Lt<'b> { - unimplemented!() - } -} - -struct Lt2<'a> { - foo: &'a u32, -} - -impl<'a> Lt2<'a> { - // The lifetime is different, but that’s irrelevant; see issue #734. - pub fn new(s: &str) -> Lt2 { - unimplemented!() - } -} - -struct Lt3<'a> { - foo: &'a u32, -} - -impl<'a> Lt3<'a> { - // The lifetime is different, but that’s irrelevant; see issue #734. - pub fn new() -> Lt3<'static> { - unimplemented!() - } -} - -#[derive(Clone, Copy)] -struct U; - -impl U { - fn new() -> Self { - U - } - // Ok because `U` is `Copy`. - fn to_something(self) -> u32 { - 0 - } -} - -struct V { - _dummy: T, -} - -impl V { - fn new() -> Option> { - None - } -} - -struct AsyncNew; - -impl AsyncNew { - async fn new() -> Option { - None - } -} - -struct BadNew; - -impl BadNew { - fn new() -> i32 { - 0 - } -} - -impl Mul for T { - type Output = T; - // No error, obviously. - fn mul(self, other: T) -> T { - self - } -} - -/// Checks implementation of the following lints: -/// * `OPTION_MAP_UNWRAP_OR` -/// * `OPTION_MAP_UNWRAP_OR_ELSE` -#[rustfmt::skip] -fn option_methods() { - let opt = Some(1); - - // Check `OPTION_MAP_UNWRAP_OR`. - // Single line case. - let _ = opt.map(|x| x + 1) - // Should lint even though this call is on a separate line. - .unwrap_or(0); - // Multi-line cases. - let _ = opt.map(|x| { - x + 1 - } - ).unwrap_or(0); - let _ = opt.map(|x| x + 1) - .unwrap_or({ - 0 - }); - // Single line `map(f).unwrap_or(None)` case. - let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); - // Multi-line `map(f).unwrap_or(None)` cases. - let _ = opt.map(|x| { - Some(x + 1) - } - ).unwrap_or(None); - let _ = opt - .map(|x| Some(x + 1)) - .unwrap_or(None); - // macro case - let _ = opt_map!(opt, |x| x + 1).unwrap_or(0); // should not lint - - // Should not lint if not copyable - let id: String = "identifier".to_string(); - let _ = Some("prefix").map(|p| format!("{}.{}", p, id)).unwrap_or(id); - // ...but DO lint if the `unwrap_or` argument is not used in the `map` - let id: String = "identifier".to_string(); - let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); - - // Check OPTION_MAP_UNWRAP_OR_ELSE - // single line case - let _ = opt.map(|x| x + 1) - // Should lint even though this call is on a separate line. - .unwrap_or_else(|| 0); - // Multi-line cases. - let _ = opt.map(|x| { - x + 1 - } - ).unwrap_or_else(|| 0); - let _ = opt.map(|x| x + 1) - .unwrap_or_else(|| - 0 - ); - // Macro case. - // Should not lint. - let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0); - - // Issue #4144 - { - let mut frequencies = HashMap::new(); - let word = "foo"; - - frequencies - .get_mut(word) - .map(|count| { - *count += 1; - }) - .unwrap_or_else(|| { - frequencies.insert(word.to_owned(), 1); - }); - } -} - -/// Checks implementation of `FILTER_NEXT` lint. -#[rustfmt::skip] -fn filter_next() { - let v = vec![3, 2, 1, 0, -1, -2, -3]; - - // Single-line case. - let _ = v.iter().filter(|&x| *x < 0).next(); - - // Multi-line case. - let _ = v.iter().filter(|&x| { - *x < 0 - } - ).next(); - - // Check that hat we don't lint if the caller is not an `Iterator`. - let foo = IteratorFalsePositives { foo: 0 }; - let _ = foo.filter().next(); -} - -/// Checks implementation of `SEARCH_IS_SOME` lint. -#[rustfmt::skip] -fn search_is_some() { - let v = vec![3, 2, 1, 0, -1, -2, -3]; - let y = &&42; - - // Check `find().is_some()`, single-line case. - let _ = any(|x| *x < 0); - let _ = any(|x| **y == x); // one dereference less - let _ = any(|x| x == 0); - - // Check `find().is_some()`, multi-line case. - let _ = v.iter().find(|&x| { - *x < 0 - } - ).is_some(); - - // Check `position().is_some()`, single-line case. - let _ = any(|&x| x < 0); - - // Check `position().is_some()`, multi-line case. - let _ = v.iter().position(|&x| { - x < 0 - } - ).is_some(); - - // Check `rposition().is_some()`, single-line case. - let _ = any(|&x| x < 0); - - // Check `rposition().is_some()`, multi-line case. - let _ = v.iter().rposition(|&x| { - x < 0 - } - ).is_some(); - - // Check that we don't lint if the caller is not an `Iterator`. - let foo = IteratorFalsePositives { foo: 0 }; - let _ = foo.find().is_some(); - let _ = foo.position().is_some(); - let _ = foo.rposition().is_some(); -} - -#[allow(clippy::similar_names)] -fn main() { - let opt = Some(0); - let _ = opt.unwrap(); -} diff --git a/tests/ui/methods.rs b/tests/ui/methods.rs index 930b76a68677..8f53b8cecbd3 100644 --- a/tests/ui/methods.rs +++ b/tests/ui/methods.rs @@ -1,6 +1,5 @@ // aux-build:option_helpers.rs // compile-flags: --edition 2018 -// run-rustfix #![warn(clippy::all, clippy::pedantic, clippy::option_unwrap_used)] #![allow( @@ -267,6 +266,7 @@ fn search_is_some() { let _ = v.iter().find(|&x| *x < 0).is_some(); let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less let _ = (0..1).find(|x| *x == 0).is_some(); + let _ = v.iter().find(|x| **x == 0).is_some(); // Check `find().is_some()`, multi-line case. let _ = v.iter().find(|&x| { diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index c35a74463ec8..b30371fa541f 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -1,5 +1,5 @@ error: defining a method called `add` on this type; consider implementing the `std::ops::Add` trait or choosing a less ambiguous name - --> $DIR/methods.rs:37:5 + --> $DIR/methods.rs:36:5 | LL | / pub fn add(self, other: T) -> T { LL | | self @@ -9,7 +9,7 @@ LL | | } = note: `-D clippy::should-implement-trait` implied by `-D warnings` error: methods called `new` usually return `Self` - --> $DIR/methods.rs:153:5 + --> $DIR/methods.rs:152:5 | LL | / fn new() -> i32 { LL | | 0 @@ -19,7 +19,7 @@ LL | | } = note: `-D clippy::new-ret-no-self` implied by `-D warnings` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:175:13 + --> $DIR/methods.rs:174:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -31,7 +31,7 @@ LL | | .unwrap_or(0); = note: replace `map(|x| x + 1).unwrap_or(0)` with `map_or(0, |x| x + 1)` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:179:13 + --> $DIR/methods.rs:178:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -41,7 +41,7 @@ LL | | ).unwrap_or(0); | |____________________________^ error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:183:13 + --> $DIR/methods.rs:182:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -51,7 +51,7 @@ LL | | }); | |__________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:188:13 + --> $DIR/methods.rs:187:13 | LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -59,7 +59,7 @@ LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:190:13 + --> $DIR/methods.rs:189:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -69,7 +69,7 @@ LL | | ).unwrap_or(None); | |_____________________^ error: called `map(f).unwrap_or(None)` on an Option value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/methods.rs:194:13 + --> $DIR/methods.rs:193:13 | LL | let _ = opt | _____________^ @@ -80,7 +80,7 @@ LL | | .unwrap_or(None); = note: replace `map(|x| Some(x + 1)).unwrap_or(None)` with `and_then(|x| Some(x + 1))` error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/methods.rs:205:13 + --> $DIR/methods.rs:204:13 | LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); = note: replace `map(|p| format!("{}.", p)).unwrap_or(id)` with `map_or(id, |p| format!("{}.", p))` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:209:13 + --> $DIR/methods.rs:208:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -100,7 +100,7 @@ LL | | .unwrap_or_else(|| 0); = note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)` error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:213:13 + --> $DIR/methods.rs:212:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -110,7 +110,7 @@ LL | | ).unwrap_or_else(|| 0); | |____________________________________^ error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/methods.rs:217:13 + --> $DIR/methods.rs:216:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -120,7 +120,7 @@ LL | | ); | |_________________^ error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:247:13 + --> $DIR/methods.rs:246:13 | LL | let _ = v.iter().filter(|&x| *x < 0).next(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -129,7 +129,7 @@ LL | let _ = v.iter().filter(|&x| *x < 0).next(); = note: replace `filter(|&x| *x < 0).next()` with `find(|&x| *x < 0)` error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. - --> $DIR/methods.rs:250:13 + --> $DIR/methods.rs:249:13 | LL | let _ = v.iter().filter(|&x| { | _____________^ @@ -139,24 +139,30 @@ LL | | ).next(); | |___________________________^ error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:267:13 + --> $DIR/methods.rs:266:22 | LL | let _ = v.iter().find(|&x| *x < 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x < 0)` | = note: `-D clippy::search-is-some` implied by `-D warnings` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:268:13 + --> $DIR/methods.rs:267:20 | LL | let _ = (0..1).find(|x| **y == *x).is_some(); // one dereference less - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| **y == x)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:269:13 + --> $DIR/methods.rs:268:20 | LL | let _ = (0..1).find(|x| *x == 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)` + +error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. + --> $DIR/methods.rs:269:22 + | +LL | let _ = v.iter().find(|x| **x == 0).is_some(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| *x == 0)` error: called `is_some()` after searching an `Iterator` with find. This is more succinctly expressed by calling `any()`. --> $DIR/methods.rs:272:13 @@ -169,10 +175,10 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:278:13 + --> $DIR/methods.rs:278:22 | LL | let _ = v.iter().position(|&x| x < 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with position. This is more succinctly expressed by calling `any()`. --> $DIR/methods.rs:281:13 @@ -185,10 +191,10 @@ LL | | ).is_some(); | |______________________________^ error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. - --> $DIR/methods.rs:287:13 + --> $DIR/methods.rs:287:22 | LL | let _ = v.iter().rposition(|&x| x < 0).is_some(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|&x| x < 0)` error: called `is_some()` after searching an `Iterator` with rposition. This is more succinctly expressed by calling `any()`. --> $DIR/methods.rs:290:13 @@ -208,5 +214,5 @@ LL | let _ = opt.unwrap(); | = note: `-D clippy::option-unwrap-used` implied by `-D warnings` -error: aborting due to 23 previous errors +error: aborting due to 24 previous errors