X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fmethods%2Fmod.rs;h=b820b7409304562ce75a4d7045e571eb393b2770;hb=b97784fd07b1981292703fb136cf6e4f7cddc113;hp=7047aa4c69b5d714be3301fa56dad31b7d1a8bd4;hpb=6fba89751bfacf59109d37331ec79a8f87ee0aaf;p=rust.git diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7047aa4c69b..b820b740930 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -9,6 +9,7 @@ mod clone_on_copy; mod clone_on_ref_ptr; mod cloned_instead_of_copied; +mod err_expect; mod expect_fun_call; mod expect_used; mod extend_with_drain; @@ -20,11 +21,13 @@ mod flat_map_identity; mod flat_map_option; mod from_iter_instead_of_collect; +mod get_last_with_len; mod get_unwrap; mod implicit_clone; mod inefficient_to_string; mod inspect_for_each; mod into_iter_on_ref; +mod is_digit_ascii_radix; mod iter_cloned_collect; mod iter_count; mod iter_next_slice; @@ -40,6 +43,9 @@ mod map_flatten; mod map_identity; mod map_unwrap_or; +mod needless_option_as_deref; +mod needless_option_take; +mod no_effect_replace; mod ok_expect; mod option_as_ref_deref; mod option_map_or_none; @@ -292,15 +298,15 @@ /// Checks for methods with certain name prefixes and which /// doesn't match how self is taken. The actual rules are: /// - /// |Prefix |Postfix |`self` taken | `self` type | - /// |-------|------------|-----------------------|--------------| - /// |`as_` | none |`&self` or `&mut self` | any | - /// |`from_`| none | none | any | - /// |`into_`| none |`self` | any | - /// |`is_` | none |`&self` or none | any | - /// |`to_` | `_mut` |`&mut self` | any | - /// |`to_` | not `_mut` |`self` | `Copy` | - /// |`to_` | not `_mut` |`&self` | not `Copy` | + /// |Prefix |Postfix |`self` taken | `self` type | + /// |-------|------------|-------------------------------|--------------| + /// |`as_` | none |`&self` or `&mut self` | any | + /// |`from_`| none | none | any | + /// |`into_`| none |`self` | any | + /// |`is_` | none |`&mut self` or `&self` or none | any | + /// |`to_` | `_mut` |`&mut self` | any | + /// |`to_` | not `_mut` |`self` | `Copy` | + /// |`to_` | not `_mut` |`&self` | not `Copy` | /// /// Note: Clippy doesn't trigger methods with `to_` prefix in: /// - Traits definition. @@ -362,6 +368,29 @@ "using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `.err().expect()` calls on the `Result` type. + /// + /// ### Why is this bad? + /// `.expect_err()` can be called directly to avoid the extra type conversion from `err()`. + /// + /// ### Example + /// ```should_panic + /// let x: Result = Ok(10); + /// x.err().expect("Testing err().expect()"); + /// ``` + /// Use instead: + /// ```should_panic + /// let x: Result = Ok(10); + /// x.expect_err("Testing expect_err"); + /// ``` + #[clippy::version = "1.61.0"] + pub ERR_EXPECT, + style, + r#"using `.err().expect("")` when `.expect_err("")` can be used"# +} + declare_clippy_lint! { /// ### What it does /// Checks for usages of `_.unwrap_or_else(Default::default)` on `Option` and @@ -1182,6 +1211,38 @@ "replace `.drain(..)` with `.into_iter()`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for using `x.get(x.len() - 1)` instead of + /// `x.last()`. + /// + /// ### Why is this bad? + /// Using `x.last()` is easier to read and has the same + /// result. + /// + /// Note that using `x[x.len() - 1]` is semantically different from + /// `x.last()`. Indexing into the array will panic on out-of-bounds + /// accesses, while `x.get()` and `x.last()` will return `None`. + /// + /// There is another lint (get_unwrap) that covers the case of using + /// `x.get(index).unwrap()` instead of `x[index]`. + /// + /// ### Example + /// ```rust + /// // Bad + /// let x = vec![2, 3, 5]; + /// let last_element = x.get(x.len() - 1); + /// + /// // Good + /// let x = vec![2, 3, 5]; + /// let last_element = x.last(); + /// ``` + #[clippy::version = "1.37.0"] + pub GET_LAST_WITH_LEN, + complexity, + "Using `x.get(x.len() - 1)` when `x.last()` is correct and simpler" +} + declare_clippy_lint! { /// ### What it does /// Checks for use of `.get().unwrap()` (or @@ -1240,7 +1301,7 @@ #[clippy::version = "1.55.0"] pub EXTEND_WITH_DRAIN, perf, - "using vec.append(&mut vec) to move the full range of a vecor to another" + "using vec.append(&mut vec) to move the full range of a vector to another" } declare_clippy_lint! { @@ -1536,7 +1597,7 @@ #[clippy::version = "1.39.0"] pub MANUAL_SATURATING_ARITHMETIC, style, - "`.chcked_add/sub(x).unwrap_or(MAX/MIN)`" + "`.checked_add/sub(x).unwrap_or(MAX/MIN)`" } declare_clippy_lint! { @@ -1749,8 +1810,6 @@ /// /// ### Example /// ```rust - /// use std::iter::FromIterator; - /// /// let five_fives = std::iter::repeat(5).take(5); /// /// let v = Vec::from_iter(five_fives); @@ -1982,13 +2041,27 @@ /// ### Example /// ```rust,ignore /// // Bad - /// let (key, value) = _.splitn(2, '=').next_tuple()?; - /// let value = _.splitn(2, '=').nth(1)?; + /// let s = "key=value=add"; + /// let (key, value) = s.splitn(2, '=').next_tuple()?; + /// let value = s.splitn(2, '=').nth(1)?; /// + /// let mut parts = s.splitn(2, '='); + /// let key = parts.next()?; + /// let value = parts.next()?; + /// ``` + /// Use instead: + /// ```rust,ignore /// // Good - /// let (key, value) = _.split_once('=')?; - /// let value = _.split_once('=')?.1; + /// let s = "key=value=add"; + /// let (key, value) = s.split_once('=')?; + /// let value = s.split_once('=')?.1; + /// + /// let (key, value) = s.split_once('=')?; /// ``` + /// + /// ### Limitations + /// The multiple statement variant currently only detects `iter.next()?`/`iter.next().unwrap()` + /// in two separate `let` statements that immediately follow the `splitn()` #[clippy::version = "1.57.0"] pub MANUAL_SPLIT_ONCE, complexity, @@ -2055,7 +2128,7 @@ /// Checks for use of `.collect::>().join("")` on iterators. /// /// ### Why is this bad? - /// `.collect::()` is more concise and usually more performant + /// `.collect::()` is more concise and might be more performant /// /// ### Example /// ```rust @@ -2070,26 +2143,130 @@ /// println!("{}", output); /// ``` /// ### Known problems - /// While `.collect::()` is more performant in most cases, there are cases where + /// While `.collect::()` is sometimes more performant, there are cases where /// using `.collect::()` over `.collect::>().join("")` /// will prevent loop unrolling and will result in a negative performance impact. + /// + /// Additionally, differences have been observed between aarch64 and x86_64 assembly output, + /// with aarch64 tending to producing faster assembly in more cases when using `.collect::()` #[clippy::version = "1.61.0"] pub UNNECESSARY_JOIN, pedantic, "using `.collect::>().join(\"\")` on an iterator" } +declare_clippy_lint! { + /// ### What it does + /// Checks for no-op uses of `Option::{as_deref, as_deref_mut}`, + /// for example, `Option<&T>::as_deref()` returns the same type. + /// + /// ### Why is this bad? + /// Redundant code and improving readability. + /// + /// ### Example + /// ```rust + /// let a = Some(&1); + /// let b = a.as_deref(); // goes from Option<&i32> to Option<&i32> + /// ``` + /// Could be written as: + /// ```rust + /// let a = Some(&1); + /// let b = a; + /// ``` + #[clippy::version = "1.57.0"] + pub NEEDLESS_OPTION_AS_DEREF, + complexity, + "no-op use of `deref` or `deref_mut` method to `Option`." +} + +declare_clippy_lint! { + /// ### What it does + /// Finds usages of [`char::is_digit`] + /// (https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_digit) that + /// can be replaced with [`is_ascii_digit`] + /// (https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_ascii_digit) or + /// [`is_ascii_hexdigit`] + /// (https://doc.rust-lang.org/stable/std/primitive.char.html#method.is_ascii_hexdigit). + /// + /// ### Why is this bad? + /// `is_digit(..)` is slower and requires specifying the radix. + /// + /// ### Example + /// ```rust + /// let c: char = '6'; + /// c.is_digit(10); + /// c.is_digit(16); + /// ``` + /// Use instead: + /// ```rust + /// let c: char = '6'; + /// c.is_ascii_digit(); + /// c.is_ascii_hexdigit(); + /// ``` + #[clippy::version = "1.61.0"] + pub IS_DIGIT_ASCII_RADIX, + style, + "use of `char::is_digit(..)` with literal radix of 10 or 16" +} + +declare_clippy_lint! { + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```rust + /// let x = Some(3); + /// x.as_ref().take(); + /// ``` + /// Use instead: + /// ```rust + /// let x = Some(3); + /// x.as_ref(); + /// ``` + #[clippy::version = "1.61.0"] + pub NEEDLESS_OPTION_TAKE, + complexity, + "using `.as_ref().take()` on a temporary value" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for `replace` statements which have no effect. + /// + /// ### Why is this bad? + /// It's either a mistake or confusing. + /// + /// ### Example + /// ```rust + /// "1234".replace("12", "12"); + /// "1234".replacen("12", "12", 1); + /// ``` + #[clippy::version = "1.62.0"] + pub NO_EFFECT_REPLACE, + suspicious, + "replace with no effect" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, + allow_expect_in_tests: bool, + allow_unwrap_in_tests: bool, } impl Methods { #[must_use] - pub fn new(avoid_breaking_exported_api: bool, msrv: Option) -> Self { + pub fn new( + avoid_breaking_exported_api: bool, + msrv: Option, + allow_expect_in_tests: bool, + allow_unwrap_in_tests: bool, + ) -> Self { Self { avoid_breaking_exported_api, msrv, + allow_expect_in_tests, + allow_unwrap_in_tests, } } } @@ -2139,6 +2316,7 @@ pub fn new(avoid_breaking_exported_api: bool, msrv: Option) -> Sel BYTES_NTH, ITER_SKIP_NEXT, GET_UNWRAP, + GET_LAST_WITH_LEN, STRING_EXTEND_CHARS, ITER_CLONED_COLLECT, ITER_WITH_DRAIN, @@ -2165,6 +2343,11 @@ pub fn new(avoid_breaking_exported_api: bool, msrv: Option) -> Sel NEEDLESS_SPLITN, UNNECESSARY_TO_OWNED, UNNECESSARY_JOIN, + ERR_EXPECT, + NEEDLESS_OPTION_AS_DEREF, + IS_DIGIT_ASCII_RADIX, + NEEDLESS_OPTION_TAKE, + NO_EFFECT_REPLACE, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -2184,7 +2367,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { return; } - check_methods(cx, expr, self.msrv.as_ref()); + self.check_methods(cx, expr); match expr.kind { hir::ExprKind::Call(func, args) => { @@ -2200,7 +2383,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { single_char_add_str::check(cx, expr, args); into_iter_on_ref::check(cx, expr, method_span, method_call.ident.name, args); single_char_pattern::check(cx, expr, method_call.ident.name, args); - unnecessary_to_owned::check(cx, expr, method_call.ident.name, args); + unnecessary_to_owned::check(cx, expr, method_call.ident.name, args, self.msrv); }, hir::ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => { let mut info = BinaryExprInfo { @@ -2383,190 +2566,205 @@ fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx TraitItem<'_> extract_msrv_attr!(LateContext); } -#[allow(clippy::too_many_lines)] -fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Option<&RustcVersion>) { - if let Some((name, [recv, args @ ..], span)) = method_call(expr) { - match (name, args) { - ("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => { - zst_offset::check(cx, expr, recv); - }, - ("and_then", [arg]) => { - let biom_option_linted = bind_instead_of_map::OptionAndThenSome::check(cx, expr, recv, arg); - let biom_result_linted = bind_instead_of_map::ResultAndThenOk::check(cx, expr, recv, arg); - if !biom_option_linted && !biom_result_linted { - unnecessary_lazy_eval::check(cx, expr, recv, arg, "and"); - } - }, - ("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv), - ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv), - ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv), - ("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, msrv), - ("collect", []) => match method_call(recv) { - Some((name @ ("cloned" | "copied"), [recv2], _)) => { - iter_cloned_collect::check(cx, name, expr, recv2); +impl Methods { + #[allow(clippy::too_many_lines)] + fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if let Some((name, [recv, args @ ..], span)) = method_call(expr) { + match (name, args) { + ("add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub", [_arg]) => { + zst_offset::check(cx, expr, recv); + }, + ("and_then", [arg]) => { + let biom_option_linted = bind_instead_of_map::OptionAndThenSome::check(cx, expr, recv, arg); + let biom_result_linted = bind_instead_of_map::ResultAndThenOk::check(cx, expr, recv, arg); + if !biom_option_linted && !biom_result_linted { + unnecessary_lazy_eval::check(cx, expr, recv, arg, "and"); + } }, - Some(("map", [m_recv, m_arg], _)) => { - map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv); + ("as_deref" | "as_deref_mut", []) => { + needless_option_as_deref::check(cx, expr, recv, name); + }, + ("as_mut", []) => useless_asref::check(cx, expr, "as_mut", recv), + ("as_ref", []) => useless_asref::check(cx, expr, "as_ref", recv), + ("assume_init", []) => uninit_assumed_init::check(cx, expr, recv), + ("cloned", []) => cloned_instead_of_copied::check(cx, expr, recv, span, self.msrv), + ("collect", []) => match method_call(recv) { + Some((name @ ("cloned" | "copied"), [recv2], _)) => { + iter_cloned_collect::check(cx, name, expr, recv2); + }, + Some(("map", [m_recv, m_arg], _)) => { + map_collect_result_unit::check(cx, expr, m_recv, m_arg, recv); + }, + Some(("take", [take_self_arg, take_arg], _)) => { + if meets_msrv(self.msrv, msrvs::STR_REPEAT) { + manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg); + } + }, + _ => {}, + }, + (name @ "count", args @ []) => match method_call(recv) { + Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => { + iter_count::check(cx, expr, recv2, name2); + }, + Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg), + _ => {}, + }, + ("drain", [arg]) => { + iter_with_drain::check(cx, expr, recv, span, arg); + }, + ("expect", [_]) => match method_call(recv) { + Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv), + Some(("err", [recv], err_span)) => err_expect::check(cx, expr, recv, self.msrv, span, err_span), + _ => expect_used::check(cx, expr, recv, self.allow_expect_in_tests), + }, + ("extend", [arg]) => { + string_extend_chars::check(cx, expr, recv, arg); + extend_with_drain::check(cx, expr, recv, arg); + }, + ("filter_map", [arg]) => { + unnecessary_filter_map::check(cx, expr, arg, name); + filter_map_identity::check(cx, expr, arg, span); + }, + ("find_map", [arg]) => { + unnecessary_filter_map::check(cx, expr, arg, name); + }, + ("flat_map", [arg]) => { + flat_map_identity::check(cx, expr, arg, span); + flat_map_option::check(cx, expr, arg, span); + }, + (name @ "flatten", args @ []) => match method_call(recv) { + Some(("map", [recv, map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span), + Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + _ => {}, }, - Some(("take", [take_self_arg, take_arg], _)) => { - if meets_msrv(msrv, &msrvs::STR_REPEAT) { - manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg); + ("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span), + ("for_each", [_]) => { + if let Some(("inspect", [_, _], span2)) = method_call(recv) { + inspect_for_each::check(cx, expr, span2); } }, - _ => {}, - }, - (name @ "count", args @ []) => match method_call(recv) { - Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), - Some((name2 @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => { - iter_count::check(cx, expr, recv2, name2); + ("get", [arg]) => get_last_with_len::check(cx, expr, recv, arg), + ("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"), + ("is_file", []) => filetype_is_file::check(cx, expr, recv), + ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, self.msrv), + ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), + ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), + ("join", [join_arg]) => { + if let Some(("collect", _, span)) = method_call(recv) { + unnecessary_join::check(cx, expr, recv, join_arg, span); + } }, - Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg), - _ => {}, - }, - ("drain", [arg]) => { - iter_with_drain::check(cx, expr, recv, span, arg); - }, - ("expect", [_]) => match method_call(recv) { - Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv), - _ => expect_used::check(cx, expr, recv), - }, - ("extend", [arg]) => { - string_extend_chars::check(cx, expr, recv, arg); - extend_with_drain::check(cx, expr, recv, arg); - }, - ("filter_map", [arg]) => { - unnecessary_filter_map::check(cx, expr, arg, name); - filter_map_identity::check(cx, expr, arg, span); - }, - ("find_map", [arg]) => { - unnecessary_filter_map::check(cx, expr, arg, name); - }, - ("flat_map", [arg]) => { - flat_map_identity::check(cx, expr, arg, span); - flat_map_option::check(cx, expr, arg, span); - }, - (name @ "flatten", args @ []) => match method_call(recv) { - Some(("map", [recv, map_arg], map_span)) => map_flatten::check(cx, expr, recv, map_arg, map_span), - Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), - _ => {}, - }, - ("fold", [init, acc]) => unnecessary_fold::check(cx, expr, init, acc, span), - ("for_each", [_]) => { - if let Some(("inspect", [_, _], span2)) = method_call(recv) { - inspect_for_each::check(cx, expr, span2); - } - }, - ("get_or_insert_with", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "get_or_insert"), - ("is_file", []) => filetype_is_file::check(cx, expr, recv), - ("is_none", []) => check_is_some_is_none(cx, expr, recv, false), - ("is_some", []) => check_is_some_is_none(cx, expr, recv, true), - ("join", [join_arg]) => { - if let Some(("collect", _, span)) = method_call(recv) { - unnecessary_join::check(cx, expr, recv, join_arg, span); - } - }, - ("last", args @ []) | ("skip", args @ [_]) => { - if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { - if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv2, name, args); + ("last", args @ []) | ("skip", args @ [_]) => { + if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { + if let ("cloned", []) = (name2, args2) { + iter_overeager_cloned::check(cx, expr, recv2, name, args); + } } - } - }, - (name @ ("map" | "map_err"), [m_arg]) => { - if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) { - match (name, args) { - ("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, msrv), - ("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, msrv), - ("filter", [f_arg]) => { - filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, false); - }, - ("find", [f_arg]) => filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, true), - _ => {}, + }, + (name @ ("map" | "map_err"), [m_arg]) => { + if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) { + match (name, args) { + ("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, self.msrv), + ("as_ref", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, false, self.msrv), + ("filter", [f_arg]) => { + filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, false); + }, + ("find", [f_arg]) => { + filter_map::check(cx, expr, recv2, f_arg, span2, recv, m_arg, span, true); + }, + _ => {}, + } } - } - map_identity::check(cx, expr, recv, m_arg, name, span); - }, - ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map), - (name @ "next", args @ []) => { - if let Some((name2, [recv2, args2 @ ..], _)) = method_call(recv) { - match (name2, args2) { - ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv2, name, args), - ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg), - ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, msrv), - ("iter", []) => iter_next_slice::check(cx, expr, recv2), - ("skip", [arg]) => iter_skip_next::check(cx, expr, recv2, arg), - ("skip_while", [_]) => skip_while_next::check(cx, expr), - _ => {}, + map_identity::check(cx, expr, recv, m_arg, name, span); + }, + ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map), + (name @ "next", args @ []) => { + if let Some((name2, [recv2, args2 @ ..], _)) = method_call(recv) { + match (name2, args2) { + ("cloned", []) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + ("filter", [arg]) => filter_next::check(cx, expr, recv2, arg), + ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv2, arg, self.msrv), + ("iter", []) => iter_next_slice::check(cx, expr, recv2), + ("skip", [arg]) => iter_skip_next::check(cx, expr, recv2, arg), + ("skip_while", [_]) => skip_while_next::check(cx, expr), + _ => {}, + } } - } - }, - ("nth", args @ [n_arg]) => match method_call(recv) { - Some(("bytes", [recv2], _)) => bytes_nth::check(cx, expr, recv2, n_arg), - Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), - Some(("iter", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false), - Some(("iter_mut", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true), - _ => iter_nth_zero::check(cx, expr, recv, n_arg), - }, - ("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"), - ("or_else", [arg]) => { - if !bind_instead_of_map::ResultOrElseErrInfo::check(cx, expr, recv, arg) { - unnecessary_lazy_eval::check(cx, expr, recv, arg, "or"); - } - }, - ("splitn" | "rsplitn", [count_arg, pat_arg]) => { - if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { - suspicious_splitn::check(cx, name, expr, recv, count); - str_splitn::check(cx, name, expr, recv, pat_arg, count, msrv); - } - }, - ("splitn_mut" | "rsplitn_mut", [count_arg, _]) => { - if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { - suspicious_splitn::check(cx, name, expr, recv, count); - } - }, - ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), - ("take", args @ [_arg]) => { - if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { - if let ("cloned", []) = (name2, args2) { - iter_overeager_cloned::check(cx, expr, recv2, name, args); + }, + ("nth", args @ [n_arg]) => match method_call(recv) { + Some(("bytes", [recv2], _)) => bytes_nth::check(cx, expr, recv2, n_arg), + Some(("cloned", [recv2], _)) => iter_overeager_cloned::check(cx, expr, recv2, name, args), + Some(("iter", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false), + Some(("iter_mut", [recv2], _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true), + _ => iter_nth_zero::check(cx, expr, recv, n_arg), + }, + ("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"), + ("or_else", [arg]) => { + if !bind_instead_of_map::ResultOrElseErrInfo::check(cx, expr, recv, arg) { + unnecessary_lazy_eval::check(cx, expr, recv, arg, "or"); } - } - }, - ("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => { - implicit_clone::check(cx, name, expr, recv); - }, - ("unwrap", []) => { - match method_call(recv) { - Some(("get", [recv, get_arg], _)) => { - get_unwrap::check(cx, expr, recv, get_arg, false); - }, - Some(("get_mut", [recv, get_arg], _)) => { - get_unwrap::check(cx, expr, recv, get_arg, true); + }, + ("splitn" | "rsplitn", [count_arg, pat_arg]) => { + if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { + suspicious_splitn::check(cx, name, expr, recv, count); + str_splitn::check(cx, name, expr, recv, pat_arg, count, self.msrv); + } + }, + ("splitn_mut" | "rsplitn_mut", [count_arg, _]) => { + if let Some((Constant::Int(count), _)) = constant(cx, cx.typeck_results(), count_arg) { + suspicious_splitn::check(cx, name, expr, recv, count); + } + }, + ("step_by", [arg]) => iterator_step_by_zero::check(cx, expr, arg), + ("take", args @ [_arg]) => { + if let Some((name2, [recv2, args2 @ ..], _span2)) = method_call(recv) { + if let ("cloned", []) = (name2, args2) { + iter_overeager_cloned::check(cx, expr, recv2, name, args); + } + } + }, + ("take", []) => needless_option_take::check(cx, expr, recv), + ("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => { + implicit_clone::check(cx, name, expr, recv); + }, + ("unwrap", []) => { + match method_call(recv) { + Some(("get", [recv, get_arg], _)) => { + get_unwrap::check(cx, expr, recv, get_arg, false); + }, + Some(("get_mut", [recv, get_arg], _)) => { + get_unwrap::check(cx, expr, recv, get_arg, true); + }, + Some(("or", [recv, or_arg], or_span)) => { + or_then_unwrap::check(cx, expr, recv, or_arg, or_span); + }, + _ => {}, + } + unwrap_used::check(cx, expr, recv, self.allow_unwrap_in_tests); + }, + ("unwrap_or", [u_arg]) => match method_call(recv) { + Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), [lhs, rhs], _)) => { + manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]); }, - Some(("or", [recv, or_arg], or_span)) => { - or_then_unwrap::check(cx, expr, recv, or_arg, or_span); + Some(("map", [m_recv, m_arg], span)) => { + option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span); }, _ => {}, - } - unwrap_used::check(cx, expr, recv); - }, - ("unwrap_or", [u_arg]) => match method_call(recv) { - Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), [lhs, rhs], _)) => { - manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]); }, - Some(("map", [m_recv, m_arg], span)) => { - option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span); + ("unwrap_or_else", [u_arg]) => match method_call(recv) { + Some(("map", [recv, map_arg], _)) + if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, self.msrv) => {}, + _ => { + unwrap_or_else_default::check(cx, expr, recv, u_arg); + unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); + }, }, - _ => {}, - }, - ("unwrap_or_else", [u_arg]) => match method_call(recv) { - Some(("map", [recv, map_arg], _)) if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, msrv) => {}, - _ => { - unwrap_or_else_default::check(cx, expr, recv, u_arg); - unnecessary_lazy_eval::check(cx, expr, recv, u_arg, "unwrap_or"); + ("replace" | "replacen", [arg1, arg2] | [arg1, arg2, _]) => { + no_effect_replace::check(cx, expr, arg1, arg2); }, - }, - _ => {}, + _ => {}, + } } } } @@ -2693,7 +2891,7 @@ fn lifetime_param_cond(&self, impl_item: &hir::ImplItem<'_>) -> bool { ShouldImplTraitCase::new("std::ops::Sub", "sub", 2, FN_HEADER, SelfKind::Value, OutType::Any, true), ]; -#[derive(Clone, Copy, PartialEq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] enum SelfKind { Value, Ref,