From ddad101b8a5029e1ed1ee346dd91d7ebb4651bef Mon Sep 17 00:00:00 2001 From: flip1995 Date: Mon, 17 Jan 2022 13:29:07 +0100 Subject: [PATCH] Merge commit '8d14c94b5c0a66241b4244f1c60ac5859cec1d97' into clippyup --- CHANGELOG.md | 1 + COPYRIGHT | 2 +- LICENSE-APACHE | 2 +- LICENSE-MIT | 2 +- README.md | 2 +- clippy_lints/src/bit_mask.rs | 24 +++--- clippy_lints/src/copies.rs | 29 ++++--- clippy_lints/src/format.rs | 19 +++- .../src/functions/not_unsafe_ptr_arg_deref.rs | 11 ++- .../src/iter_not_returning_iterator.rs | 2 +- clippy_lints/src/lib.register_all.rs | 2 +- clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_pedantic.rs | 1 + clippy_lints/src/lib.register_perf.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 - clippy_lints/src/methods/implicit_clone.rs | 21 +++-- .../src/methods/iter_overeager_cloned.rs | 62 +++++++++++++ clippy_lints/src/methods/mod.rs | 86 +++++++++++++++---- clippy_lints/src/methods/or_fun_call.rs | 26 ++++-- clippy_lints/src/misc.rs | 21 ++++- clippy_lints/src/return_self_not_must_use.rs | 2 +- rustc_tools_util/README.md | 2 +- tests/ui/cmp_owned/comparison_flip.fixed | 29 +++++++ tests/ui/cmp_owned/comparison_flip.rs | 29 +++++++ tests/ui/cmp_owned/comparison_flip.stderr | 18 ++++ tests/ui/format.fixed | 6 ++ tests/ui/format.rs | 6 ++ tests/ui/format.stderr | 14 ++- tests/ui/functions.rs | 8 ++ tests/ui/functions.stderr | 26 +++++- tests/ui/if_same_then_else2.rs | 17 ++++ tests/ui/implicit_clone.rs | 9 ++ tests/ui/implicit_clone.stderr | 54 +++++++----- tests/ui/iter_not_returning_iterator.rs | 7 ++ tests/ui/iter_overeager_cloned.fixed | 45 ++++++++++ tests/ui/iter_overeager_cloned.rs | 47 ++++++++++ tests/ui/iter_overeager_cloned.stderr | 58 +++++++++++++ tests/ui/or_fun_call.fixed | 48 +++++++++++ tests/ui/or_fun_call.rs | 50 +++++++++++ tests/ui/or_fun_call.stderr | 54 +++++++++++- tests/ui/return_self_not_must_use.rs | 1 + tests/ui/return_self_not_must_use.stderr | 6 +- util/gh-pages/index.html | 2 +- 43 files changed, 754 insertions(+), 100 deletions(-) create mode 100644 clippy_lints/src/methods/iter_overeager_cloned.rs create mode 100644 tests/ui/cmp_owned/comparison_flip.fixed create mode 100644 tests/ui/cmp_owned/comparison_flip.rs create mode 100644 tests/ui/cmp_owned/comparison_flip.stderr create mode 100644 tests/ui/iter_overeager_cloned.fixed create mode 100644 tests/ui/iter_overeager_cloned.rs create mode 100644 tests/ui/iter_overeager_cloned.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f4da9a38279..258a8256f531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3049,6 +3049,7 @@ Released 2018-09-13 [`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator [`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero +[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero [`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits diff --git a/COPYRIGHT b/COPYRIGHT index 238c919b69d6..a6be75b5e310 100644 --- a/COPYRIGHT +++ b/COPYRIGHT @@ -1,4 +1,4 @@ -Copyright 2014-2021 The Rust Project Developers +Copyright 2014-2022 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/LICENSE-APACHE b/LICENSE-APACHE index 04169a42b8be..0d62c37278e5 100644 --- a/LICENSE-APACHE +++ b/LICENSE-APACHE @@ -186,7 +186,7 @@ APPENDIX: How to apply the Apache License to your work. same "printed page" as the copyright notice for easier identification within third-party archives. -Copyright 2014-2021 The Rust Project Developers +Copyright 2014-2022 The Rust Project Developers Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/LICENSE-MIT b/LICENSE-MIT index 90a2d3950d19..b724b24aa830 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2014-2021 The Rust Project Developers +Copyright (c) 2014-2022 The Rust Project Developers Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated diff --git a/README.md b/README.md index f001a42d917d..edbc626e354b 100644 --- a/README.md +++ b/README.md @@ -238,7 +238,7 @@ If you want to contribute to Clippy, you can find more information in [CONTRIBUT ## License -Copyright 2014-2021 The Rust Project Developers +Copyright 2014-2022 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/clippy_lints/src/bit_mask.rs b/clippy_lints/src/bit_mask.rs index 0977cf22b2c4..ca4af66cad16 100644 --- a/clippy_lints/src/bit_mask.rs +++ b/clippy_lints/src/bit_mask.rs @@ -18,14 +18,14 @@ declare_clippy_lint! { /// {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following /// table: /// - /// |Comparison |Bit Op|Example |is always|Formula | - /// |------------|------|------------|---------|----------------------| - /// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | - /// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | - /// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | - /// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | - /// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | - /// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | + /// |Comparison |Bit Op|Example |is always|Formula | + /// |------------|------|-------------|---------|----------------------| + /// |`==` or `!=`| `&` |`x & 2 == 3` |`false` |`c & m != c` | + /// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | + /// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | + /// |`==` or `!=`| `\|` |`x \| 1 == 0`|`false` |`c \| m != c` | + /// |`<` or `>=`| `\|` |`x \| 1 < 1` |`false` |`m >= c` | + /// |`<=` or `>` | `\|` |`x \| 1 > 0` |`true` |`m > c` | /// /// ### Why is this bad? /// If the bits that the comparison cares about are always @@ -53,10 +53,10 @@ declare_clippy_lint! { /// without changing the outcome. The basic structure can be seen in the /// following table: /// - /// |Comparison| Bit Op |Example |equals | - /// |----------|---------|-----------|-------| - /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`| - /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`| + /// |Comparison| Bit Op |Example |equals | + /// |----------|----------|------------|-------| + /// |`>` / `<=`|`\|` / `^`|`x \| 2 > 3`|`x > 3`| + /// |`<` / `>=`|`\|` / `^`|`x ^ 1 < 4` |`x < 4`| /// /// ### Why is this bad? /// Not equally evil as [`bad_bit_mask`](#bad_bit_mask), diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 73ce656ad151..f44d370a8fd0 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -183,7 +183,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste { lint_same_cond(cx, &conds); lint_same_fns_in_if_cond(cx, &conds); // Block duplication - lint_same_then_else(cx, &blocks, conds.len() == blocks.len(), expr); + lint_same_then_else(cx, &conds, &blocks, conds.len() == blocks.len(), expr); } } } @@ -192,6 +192,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste { /// Implementation of `BRANCHES_SHARING_CODE` and `IF_SAME_THEN_ELSE` if the blocks are equal. fn lint_same_then_else<'tcx>( cx: &LateContext<'tcx>, + conds: &[&'tcx Expr<'_>], blocks: &[&Block<'tcx>], has_conditional_else: bool, expr: &'tcx Expr<'_>, @@ -204,7 +205,7 @@ fn lint_same_then_else<'tcx>( // Check if each block has shared code let has_expr = blocks[0].expr.is_some(); - let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, blocks) { + let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, conds, blocks) { (block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq) } else { return; @@ -316,14 +317,14 @@ struct BlockEqual { /// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to /// abort any further processing and avoid duplicate lint triggers. -fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option { +fn scan_block_for_eq(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> Option { let mut start_eq = usize::MAX; let mut end_eq = usize::MAX; let mut expr_eq = true; - let mut iter = blocks.windows(2); - while let Some(&[win0, win1]) = iter.next() { - let l_stmts = win0.stmts; - let r_stmts = win1.stmts; + let mut iter = blocks.windows(2).enumerate(); + while let Some((i, &[block0, block1])) = iter.next() { + let l_stmts = block0.stmts; + let r_stmts = block1.stmts; // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752. // The comparison therefore needs to be done in a way that builds the correct context. @@ -340,22 +341,26 @@ fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option LateLintPass<'tcx> for UselessFormat { ExprKind::MethodCall(path, ..) => path.ident.name.as_str() == "to_string", _ => false, }; - let sugg = if is_new_string { + let sugg = if format_args.format_string_span.contains(value.span) { + // Implicit argument. e.g. `format!("{x}")` span points to `{x}` + let spdata = value.span.data(); + let span = Span::new( + spdata.lo + BytePos(1), + spdata.hi - BytePos(1), + spdata.ctxt, + spdata.parent + ); + let snip = snippet_with_applicability(cx, span, "..", &mut applicability); + if is_new_string { + snip.into() + } else { + format!("{snip}.to_string()") + } + } else if is_new_string { snippet_with_applicability(cx, value.span, "..", &mut applicability).into_owned() } else { let sugg = Sugg::hir_with_applicability(cx, value, "", &mut applicability); diff --git a/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs index 6d829a18b2e0..834f6d2425e9 100644 --- a/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs +++ b/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs @@ -42,8 +42,7 @@ fn check_raw_ptr<'tcx>( let expr = &body.value; if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) { let raw_ptrs = iter_input_pats(decl, body) - .zip(decl.inputs.iter()) - .filter_map(|(arg, ty)| raw_ptr_arg(arg, ty)) + .filter_map(|arg| raw_ptr_arg(cx, arg)) .collect::(); if !raw_ptrs.is_empty() { @@ -59,8 +58,12 @@ fn check_raw_ptr<'tcx>( } } -fn raw_ptr_arg(arg: &hir::Param<'_>, ty: &hir::Ty<'_>) -> Option { - if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.kind, &ty.kind) { +fn raw_ptr_arg(cx: &LateContext<'_>, arg: &hir::Param<'_>) -> Option { + if let (&hir::PatKind::Binding(_, id, _, _), Some(&ty::RawPtr(_))) = ( + &arg.pat.kind, + cx.maybe_typeck_results() + .map(|typeck_results| typeck_results.pat_ty(arg.pat).kind()), + ) { Some(id) } else { None diff --git a/clippy_lints/src/iter_not_returning_iterator.rs b/clippy_lints/src/iter_not_returning_iterator.rs index d3bdc819a9f2..b56d87c5348c 100644 --- a/clippy_lints/src/iter_not_returning_iterator.rs +++ b/clippy_lints/src/iter_not_returning_iterator.rs @@ -66,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for IterNotReturningIterator { fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) { if sig.decl.implicit_self.has_implicit_self() { - let ret_ty = cx.tcx.fn_sig(fn_id).skip_binder().output(); + let ret_ty = cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(fn_id).output()); let ret_ty = cx .tcx .try_normalize_erasing_regions(cx.param_env, ret_ty) diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 26fb4259952b..87fd7f99748a 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -156,6 +156,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::ITER_NEXT_SLICE), LintId::of(methods::ITER_NTH), LintId::of(methods::ITER_NTH_ZERO), + LintId::of(methods::ITER_OVEREAGER_CLONED), LintId::of(methods::ITER_SKIP_NEXT), LintId::of(methods::MANUAL_FILTER_MAP), LintId::of(methods::MANUAL_FIND_MAP), @@ -249,7 +250,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(reference::REF_IN_DEREF), LintId::of(regex::INVALID_REGEX), LintId::of(repeat_once::REPEAT_ONCE), - LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(returns::LET_AND_RETURN), LintId::of(returns::NEEDLESS_RETURN), LintId::of(self_assignment::SELF_ASSIGNMENT), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 746bdb19c3d9..56146a0fd3a7 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -288,6 +288,7 @@ store.register_lints(&[ methods::ITER_NEXT_SLICE, methods::ITER_NTH, methods::ITER_NTH_ZERO, + methods::ITER_OVEREAGER_CLONED, methods::ITER_SKIP_NEXT, methods::MANUAL_FILTER_MAP, methods::MANUAL_FIND_MAP, diff --git a/clippy_lints/src/lib.register_pedantic.rs b/clippy_lints/src/lib.register_pedantic.rs index 1744b7c82507..1292675f4a96 100644 --- a/clippy_lints/src/lib.register_pedantic.rs +++ b/clippy_lints/src/lib.register_pedantic.rs @@ -81,6 +81,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![ LintId::of(ranges::RANGE_PLUS_ONE), LintId::of(redundant_else::REDUNDANT_ELSE), LintId::of(ref_option_ref::REF_OPTION_REF), + LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED), LintId::of(strings::STRING_ADD_ASSIGN), LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS), diff --git a/clippy_lints/src/lib.register_perf.rs b/clippy_lints/src/lib.register_perf.rs index f9ffd4cf829f..c44ef124bfa0 100644 --- a/clippy_lints/src/lib.register_perf.rs +++ b/clippy_lints/src/lib.register_perf.rs @@ -14,6 +14,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(methods::EXPECT_FUN_CALL), LintId::of(methods::EXTEND_WITH_DRAIN), LintId::of(methods::ITER_NTH), + LintId::of(methods::ITER_OVEREAGER_CLONED), LintId::of(methods::MANUAL_STR_REPEAT), LintId::of(methods::OR_FUN_CALL), LintId::of(methods::SINGLE_CHAR_PATTERN), diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 8594338ffa5a..10f8ae4b7f7f 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -16,7 +16,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec! LintId::of(methods::SUSPICIOUS_MAP), LintId::of(mut_key::MUTABLE_KEY_TYPE), LintId::of(octal_escapes::OCTAL_ESCAPES), - LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE), LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL), ]) diff --git a/clippy_lints/src/methods/implicit_clone.rs b/clippy_lints/src/methods/implicit_clone.rs index 90492ffda3cc..865e7702b715 100644 --- a/clippy_lints/src/methods/implicit_clone.rs +++ b/clippy_lints/src/methods/implicit_clone.rs @@ -1,31 +1,40 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_context; +use clippy_utils::ty::peel_mid_ty_refs; use clippy_utils::{is_diag_item_method, is_diag_trait_item}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_middle::ty::TyS; -use rustc_span::{sym, Span}; +use rustc_span::sym; use super::IMPLICIT_CLONE; -pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, span: Span) { +pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) { if_chain! { if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); if is_clone_like(cx, method_name, method_def_id); let return_type = cx.typeck_results().expr_ty(expr); - let input_type = cx.typeck_results().expr_ty(recv).peel_refs(); + let input_type = cx.typeck_results().expr_ty(recv); + let (input_type, ref_count) = peel_mid_ty_refs(input_type); if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did)); if TyS::same_type(return_type, input_type); then { + let mut app = Applicability::MachineApplicable; + let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0; span_lint_and_sugg( cx, IMPLICIT_CLONE, - span, + expr.span, &format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_name), "consider using", - "clone".to_string(), - Applicability::MachineApplicable + if ref_count > 1 { + format!("({}{}).clone()", "*".repeat(ref_count - 1), recv_snip) + } else { + format!("{}.clone()", recv_snip) + }, + app, ); } } diff --git a/clippy_lints/src/methods/iter_overeager_cloned.rs b/clippy_lints/src/methods/iter_overeager_cloned.rs new file mode 100644 index 000000000000..ca33bfc643da --- /dev/null +++ b/clippy_lints/src/methods/iter_overeager_cloned.rs @@ -0,0 +1,62 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use clippy_utils::ty::{get_iterator_item_ty, is_copy}; +use itertools::Itertools; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::LateContext; +use rustc_middle::ty; +use std::ops::Not; + +use super::ITER_OVEREAGER_CLONED; +use crate::redundant_clone::REDUNDANT_CLONE; + +/// lint overeager use of `cloned()` for `Iterator`s +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx hir::Expr<'_>, + recv: &'tcx hir::Expr<'_>, + name: &str, + map_arg: &[hir::Expr<'_>], +) { + // Check if it's iterator and get type associated with `Item`. + let inner_ty = match get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv)) { + Some(ty) => ty, + _ => return, + }; + + match inner_ty.kind() { + ty::Ref(_, ty, _) if !is_copy(cx, ty) => {}, + _ => return, + }; + + let (lint, preserve_cloned) = match name { + "count" => (REDUNDANT_CLONE, false), + _ => (ITER_OVEREAGER_CLONED, true), + }; + let wildcard_params = map_arg.is_empty().not().then(|| "...").unwrap_or_default(); + let msg = format!( + "called `cloned().{}({})` on an `Iterator`. It may be more efficient to call `{}({}){}` instead", + name, + wildcard_params, + name, + wildcard_params, + preserve_cloned.then(|| ".cloned()").unwrap_or_default(), + ); + + span_lint_and_sugg( + cx, + lint, + expr.span, + &msg, + "try this", + format!( + "{}.{}({}){}", + snippet(cx, recv.span, ".."), + name, + map_arg.iter().map(|a| snippet(cx, a.span, "..")).join(", "), + preserve_cloned.then(|| ".cloned()").unwrap_or_default(), + ), + Applicability::MachineApplicable, + ); +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index ed5136e7d00f..7357219f7b47 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -30,6 +30,7 @@ mod iter_count; mod iter_next_slice; mod iter_nth; mod iter_nth_zero; +mod iter_overeager_cloned; mod iter_skip_next; mod iterator_step_by_zero; mod manual_saturating_arithmetic; @@ -106,6 +107,41 @@ declare_clippy_lint! { "used `cloned` where `copied` could be used instead" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `_.cloned().()` where call to `.cloned()` can be postponed. + /// + /// ### Why is this bad? + /// It's often inefficient to clone all elements of an iterator, when eventually, only some + /// of them will be consumed. + /// + /// ### Examples + /// ```rust + /// # let vec = vec!["string".to_string()]; + /// + /// // Bad + /// vec.iter().cloned().take(10); + /// + /// // Good + /// vec.iter().take(10).cloned(); + /// + /// // Bad + /// vec.iter().cloned().last(); + /// + /// // Good + /// vec.iter().last().cloned(); + /// + /// ``` + /// ### Known Problems + /// This `lint` removes the side of effect of cloning items in the iterator. + /// A code that relies on that side-effect could fail. + /// + #[clippy::version = "1.59.0"] + pub ITER_OVEREAGER_CLONED, + perf, + "using `cloned()` early with `Iterator::iter()` can lead to some performance inefficiencies" +} + declare_clippy_lint! { /// ### What it does /// Checks for usages of `Iterator::flat_map()` where `filter_map()` could be @@ -1950,6 +1986,7 @@ impl_lint_pass!(Methods => [ CLONE_ON_COPY, CLONE_ON_REF_PTR, CLONE_DOUBLE_REF, + ITER_OVEREAGER_CLONED, CLONED_INSTEAD_OF_COPIED, FLAT_MAP_OPTION, INEFFICIENT_TO_STRING, @@ -2243,9 +2280,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio }, _ => {}, }, - ("count", []) => match method_call(recv) { - Some((name @ ("into_iter" | "iter" | "iter_mut"), [recv2], _)) => { - iter_count::check(cx, expr, recv2, name); + (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), _ => {}, @@ -2266,10 +2304,10 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio flat_map_identity::check(cx, expr, arg, span); flat_map_option::check(cx, expr, arg, span); }, - ("flatten", []) => { - if let Some(("map", [recv, map_arg], _)) = method_call(recv) { - map_flatten::check(cx, expr, recv, map_arg); - } + (name @ "flatten", args @ []) => match method_call(recv) { + Some(("map", [recv, map_arg], _)) => map_flatten::check(cx, expr, recv, map_arg), + 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", [_]) => { @@ -2281,6 +2319,13 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio ("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), + ("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); + } + } + }, ("map", [m_arg]) => { if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) { match (name, args) { @@ -2296,20 +2341,22 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio map_identity::check(cx, expr, recv, m_arg, span); }, ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map), - ("next", []) => { - if let Some((name, [recv, args @ ..], _)) = method_call(recv) { - match (name, args) { - ("filter", [arg]) => filter_next::check(cx, expr, recv, arg), - ("filter_map", [arg]) => filter_map_next::check(cx, expr, recv, arg, msrv), - ("iter", []) => iter_next_slice::check(cx, expr, recv), - ("skip", [arg]) => iter_skip_next::check(cx, expr, recv, arg), + (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), _ => {}, } } }, - ("nth", [n_arg]) => match method_call(recv) { + ("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), @@ -2337,8 +2384,15 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio } }, ("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); + } + } + }, ("to_os_string" | "to_owned" | "to_path_buf" | "to_vec", []) => { - implicit_clone::check(cx, name, expr, recv, span); + 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), diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 4e4653dadcaf..448dc4e6147f 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -4,6 +4,7 @@ use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_mac use clippy_utils::ty::{implements_trait, match_type}; use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths}; use if_chain::if_chain; +use rustc_errors::emitter::MAX_SUGGESTION_HIGHLIGHT_LINES; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; @@ -23,6 +24,7 @@ pub(super) fn check<'tcx>( args: &'tcx [hir::Expr<'_>], ) { /// Checks for `unwrap_or(T::new())` or `unwrap_or(T::default())`. + #[allow(clippy::too_many_arguments)] fn check_unwrap_or_default( cx: &LateContext<'_>, name: &str, @@ -31,6 +33,7 @@ pub(super) fn check<'tcx>( arg: &hir::Expr<'_>, or_has_args: bool, span: Span, + method_span: Span, ) -> bool { let is_default_default = || is_trait_item(cx, fun, sym::Default); @@ -52,16 +55,27 @@ pub(super) fn check<'tcx>( then { let mut applicability = Applicability::MachineApplicable; + let hint = "unwrap_or_default()"; + let mut sugg_span = span; + + let mut sugg: String = format!( + "{}.{}", + snippet_with_applicability(cx, self_expr.span, "..", &mut applicability), + hint + ); + + if sugg.lines().count() > MAX_SUGGESTION_HIGHLIGHT_LINES { + sugg_span = method_span.with_hi(span.hi()); + sugg = hint.to_string(); + } + span_lint_and_sugg( cx, OR_FUN_CALL, - span, + sugg_span, &format!("use of `{}` followed by a call to `{}`", name, path), "try this", - format!( - "{}.unwrap_or_default()", - snippet_with_applicability(cx, self_expr.span, "..", &mut applicability) - ), + sugg, applicability, ); @@ -164,7 +178,7 @@ pub(super) fn check<'tcx>( match inner_arg.kind { hir::ExprKind::Call(fun, or_args) => { let or_has_args = !or_args.is_empty(); - if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) { + if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span, method_span) { let fun_span = if or_has_args { None } else { Some(fun.span) }; check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span); } diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 21b3f81d5d98..8db71d1e9676 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -548,6 +548,7 @@ fn is_array(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { matches!(&cx.typeck_results().expr_ty(expr).peel_refs().kind(), ty::Array(_, _)) } +#[allow(clippy::too_many_lines)] fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool) { #[derive(Default)] struct EqImpl { @@ -644,10 +645,26 @@ fn check_to_owned(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: hint = expr_snip; } else { span = expr.span.to(other.span); + + let cmp_span = if other.span < expr.span { + other.span.between(expr.span) + } else { + expr.span.between(other.span) + }; if eq_impl.ty_eq_other { - hint = format!("{} == {}", expr_snip, snippet(cx, other.span, "..")); + hint = format!( + "{}{}{}", + expr_snip, + snippet(cx, cmp_span, ".."), + snippet(cx, other.span, "..") + ); } else { - hint = format!("{} == {}", snippet(cx, other.span, ".."), expr_snip); + hint = format!( + "{}{}{}", + snippet(cx, other.span, ".."), + snippet(cx, cmp_span, ".."), + expr_snip + ); } } diff --git a/clippy_lints/src/return_self_not_must_use.rs b/clippy_lints/src/return_self_not_must_use.rs index 5dafd08cf3be..79f104eac0be 100644 --- a/clippy_lints/src/return_self_not_must_use.rs +++ b/clippy_lints/src/return_self_not_must_use.rs @@ -60,7 +60,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.59.0"] pub RETURN_SELF_NOT_MUST_USE, - suspicious, + pedantic, "missing `#[must_use]` annotation on a method returning `Self`" } diff --git a/rustc_tools_util/README.md b/rustc_tools_util/README.md index 6027538dc4ab..01891b51d3b0 100644 --- a/rustc_tools_util/README.md +++ b/rustc_tools_util/README.md @@ -53,7 +53,7 @@ This gives the following output in clippy: ## License -Copyright 2014-2020 The Rust Project Developers +Copyright 2014-2022 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/tests/ui/cmp_owned/comparison_flip.fixed b/tests/ui/cmp_owned/comparison_flip.fixed new file mode 100644 index 000000000000..44e41bdd1148 --- /dev/null +++ b/tests/ui/cmp_owned/comparison_flip.fixed @@ -0,0 +1,29 @@ +// run-rustfix + +use std::fmt::{self, Display}; + +fn main() { + let a = Foo; + + if a != "bar" { + println!("foo"); + } + + if a != "bar" { + println!("foo"); + } +} + +struct Foo; + +impl Display for Foo { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "foo") + } +} + +impl PartialEq<&str> for Foo { + fn eq(&self, other: &&str) -> bool { + "foo" == *other + } +} diff --git a/tests/ui/cmp_owned/comparison_flip.rs b/tests/ui/cmp_owned/comparison_flip.rs new file mode 100644 index 000000000000..662673abb62d --- /dev/null +++ b/tests/ui/cmp_owned/comparison_flip.rs @@ -0,0 +1,29 @@ +// run-rustfix + +use std::fmt::{self, Display}; + +fn main() { + let a = Foo; + + if a.to_string() != "bar" { + println!("foo"); + } + + if "bar" != a.to_string() { + println!("foo"); + } +} + +struct Foo; + +impl Display for Foo { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "foo") + } +} + +impl PartialEq<&str> for Foo { + fn eq(&self, other: &&str) -> bool { + "foo" == *other + } +} diff --git a/tests/ui/cmp_owned/comparison_flip.stderr b/tests/ui/cmp_owned/comparison_flip.stderr new file mode 100644 index 000000000000..e4d0d822bb1e --- /dev/null +++ b/tests/ui/cmp_owned/comparison_flip.stderr @@ -0,0 +1,18 @@ +error: this creates an owned instance just for comparison + --> $DIR/comparison_flip.rs:8:8 + | +LL | if a.to_string() != "bar" { + | ^^^^^^^^^^^^^ help: try: `a` + | + = note: `-D clippy::cmp-owned` implied by `-D warnings` + +error: this creates an owned instance just for comparison + --> $DIR/comparison_flip.rs:12:17 + | +LL | if "bar" != a.to_string() { + | ---------^^^^^^^^^^^^^ + | | + | help: try: `a != "bar"` + +error: aborting due to 2 previous errors + diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index 64cb7b1cfb80..78d2bfd474e4 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -73,4 +73,10 @@ fn main() { let _s: String = (&*v.join("\n")).to_string(); format!("prepend {:+}", "s"); + + // Issue #8290 + let x = "foo"; + let _ = x.to_string(); + let _ = format!("{x:?}"); // Don't lint on debug + let _ = x.to_string(); } diff --git a/tests/ui/format.rs b/tests/ui/format.rs index a065b1b5683c..009c1aa216fc 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -75,4 +75,10 @@ fn main() { let _s: String = format!("{}", &*v.join("\n")); format!("prepend {:+}", "s"); + + // Issue #8290 + let x = "foo"; + let _ = format!("{x}"); + let _ = format!("{x:?}"); // Don't lint on debug + let _ = format!("{y}", y = x); } diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index 58ad7499bb26..660be57585e3 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -99,5 +99,17 @@ error: useless use of `format!` LL | let _s: String = format!("{}", &*v.join("/n")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `(&*v.join("/n")).to_string()` -error: aborting due to 15 previous errors +error: useless use of `format!` + --> $DIR/format.rs:81:13 + | +LL | let _ = format!("{x}"); + | ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()` + +error: useless use of `format!` + --> $DIR/format.rs:83:13 + | +LL | let _ = format!("{y}", y = x); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()` + +error: aborting due to 17 previous errors diff --git a/tests/ui/functions.rs b/tests/ui/functions.rs index 271754cb06ee..5521870eaecf 100644 --- a/tests/ui/functions.rs +++ b/tests/ui/functions.rs @@ -78,6 +78,14 @@ pub fn public(p: *const u8) { unsafe { std::ptr::read(p) }; } +type Alias = *const u8; + +pub fn type_alias(p: Alias) { + println!("{}", unsafe { *p }); + println!("{:?}", unsafe { p.as_ref() }); + unsafe { std::ptr::read(p) }; +} + impl Bar { fn private(self, p: *const u8) { println!("{}", unsafe { *p }); diff --git a/tests/ui/functions.stderr b/tests/ui/functions.stderr index a2b8c2a384b0..8ebd4997f4f6 100644 --- a/tests/ui/functions.stderr +++ b/tests/ui/functions.stderr @@ -69,22 +69,40 @@ LL | unsafe { std::ptr::read(p) }; | ^ error: this public function might dereference a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:87:34 + --> $DIR/functions.rs:84:30 + | +LL | println!("{}", unsafe { *p }); + | ^ + +error: this public function might dereference a raw pointer but is not marked `unsafe` + --> $DIR/functions.rs:85:31 + | +LL | println!("{:?}", unsafe { p.as_ref() }); + | ^ + +error: this public function might dereference a raw pointer but is not marked `unsafe` + --> $DIR/functions.rs:86:29 + | +LL | unsafe { std::ptr::read(p) }; + | ^ + +error: this public function might dereference a raw pointer but is not marked `unsafe` + --> $DIR/functions.rs:95:34 | LL | println!("{}", unsafe { *p }); | ^ error: this public function might dereference a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:88:35 + --> $DIR/functions.rs:96:35 | LL | println!("{:?}", unsafe { p.as_ref() }); | ^ error: this public function might dereference a raw pointer but is not marked `unsafe` - --> $DIR/functions.rs:89:33 + --> $DIR/functions.rs:97:33 | LL | unsafe { std::ptr::read(p) }; | ^ -error: aborting due to 13 previous errors +error: aborting due to 16 previous errors diff --git a/tests/ui/if_same_then_else2.rs b/tests/ui/if_same_then_else2.rs index 69189d9e0c00..0016009a02f5 100644 --- a/tests/ui/if_same_then_else2.rs +++ b/tests/ui/if_same_then_else2.rs @@ -138,6 +138,23 @@ fn if_same_then_else2() -> Result<&'static str, ()> { let (y, x) = (1, 2); return Ok(&foo[x..y]); } + + // Issue #7579 + let _ = if let Some(0) = None { 0 } else { 0 }; + + if true { + return Err(()); + } else if let Some(0) = None { + return Err(()); + } + + let _ = if let Some(0) = None { + 0 + } else if let Some(1) = None { + 0 + } else { + 0 + }; } fn main() {} diff --git a/tests/ui/implicit_clone.rs b/tests/ui/implicit_clone.rs index cef71cf79d79..639fecb8927b 100644 --- a/tests/ui/implicit_clone.rs +++ b/tests/ui/implicit_clone.rs @@ -105,4 +105,13 @@ fn main() { let os_str = OsStr::new("foo"); let _ = os_str.to_owned(); let _ = os_str.to_os_string(); + + // issue #8227 + let pathbuf_ref = &pathbuf; + let pathbuf_ref = &pathbuf_ref; + let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&PathBuf` + let _ = pathbuf_ref.to_path_buf(); + let pathbuf_ref = &pathbuf_ref; + let _ = pathbuf_ref.to_owned(); // Don't lint. Returns `&&PathBuf` + let _ = pathbuf_ref.to_path_buf(); } diff --git a/tests/ui/implicit_clone.stderr b/tests/ui/implicit_clone.stderr index e6f7527b6721..0f4124241907 100644 --- a/tests/ui/implicit_clone.stderr +++ b/tests/ui/implicit_clone.stderr @@ -1,64 +1,76 @@ error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:65:17 + --> $DIR/implicit_clone.rs:65:13 | LL | let _ = vec.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^ help: consider using: `vec.clone()` | = note: `-D clippy::implicit-clone` implied by `-D warnings` error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type - --> $DIR/implicit_clone.rs:66:17 + --> $DIR/implicit_clone.rs:66:13 | LL | let _ = vec.to_vec(); - | ^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^ help: consider using: `vec.clone()` error: implicitly cloning a `Vec` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:70:21 + --> $DIR/implicit_clone.rs:70:13 | LL | let _ = vec_ref.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^ help: consider using: `vec_ref.clone()` error: implicitly cloning a `Vec` by calling `to_vec` on its dereferenced type - --> $DIR/implicit_clone.rs:71:21 + --> $DIR/implicit_clone.rs:71:13 | LL | let _ = vec_ref.to_vec(); - | ^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^ help: consider using: `vec_ref.clone()` error: implicitly cloning a `String` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:83:17 + --> $DIR/implicit_clone.rs:83:13 | LL | let _ = str.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^ help: consider using: `str.clone()` error: implicitly cloning a `Kitten` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:87:20 + --> $DIR/implicit_clone.rs:87:13 | LL | let _ = kitten.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^ help: consider using: `kitten.clone()` error: implicitly cloning a `PathBuf` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:97:21 + --> $DIR/implicit_clone.rs:97:13 | LL | let _ = pathbuf.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^ help: consider using: `pathbuf.clone()` error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type - --> $DIR/implicit_clone.rs:98:21 + --> $DIR/implicit_clone.rs:98:13 | LL | let _ = pathbuf.to_path_buf(); - | ^^^^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `pathbuf.clone()` error: implicitly cloning a `OsString` by calling `to_owned` on its dereferenced type - --> $DIR/implicit_clone.rs:101:23 + --> $DIR/implicit_clone.rs:101:13 | LL | let _ = os_string.to_owned(); - | ^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `os_string.clone()` error: implicitly cloning a `OsString` by calling `to_os_string` on its dereferenced type - --> $DIR/implicit_clone.rs:102:23 + --> $DIR/implicit_clone.rs:102:13 | LL | let _ = os_string.to_os_string(); - | ^^^^^^^^^^^^ help: consider using: `clone` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `os_string.clone()` -error: aborting due to 10 previous errors +error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type + --> $DIR/implicit_clone.rs:113:13 + | +LL | let _ = pathbuf_ref.to_path_buf(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(*pathbuf_ref).clone()` + +error: implicitly cloning a `PathBuf` by calling `to_path_buf` on its dereferenced type + --> $DIR/implicit_clone.rs:116:13 + | +LL | let _ = pathbuf_ref.to_path_buf(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(**pathbuf_ref).clone()` + +error: aborting due to 12 previous errors diff --git a/tests/ui/iter_not_returning_iterator.rs b/tests/ui/iter_not_returning_iterator.rs index 2c91e02e8422..cce216fc649b 100644 --- a/tests/ui/iter_not_returning_iterator.rs +++ b/tests/ui/iter_not_returning_iterator.rs @@ -64,4 +64,11 @@ impl S { } } +struct S2([u8]); +impl S2 { + fn iter(&self) -> core::slice::Iter { + self.0.iter() + } +} + fn main() {} diff --git a/tests/ui/iter_overeager_cloned.fixed b/tests/ui/iter_overeager_cloned.fixed new file mode 100644 index 000000000000..a9041671101b --- /dev/null +++ b/tests/ui/iter_overeager_cloned.fixed @@ -0,0 +1,45 @@ +// run-rustfix +#![warn(clippy::iter_overeager_cloned, clippy::redundant_clone, clippy::filter_next)] + +fn main() { + let vec = vec!["1".to_string(), "2".to_string(), "3".to_string()]; + + let _: Option = vec.iter().last().cloned(); + + let _: Option = vec.iter().chain(vec.iter()).next().cloned(); + + let _: usize = vec.iter().filter(|x| x == &"2").count(); + + let _: Vec<_> = vec.iter().take(2).cloned().collect(); + + let _: Vec<_> = vec.iter().skip(2).cloned().collect(); + + let _ = vec.iter().filter(|x| x == &"2").nth(2).cloned(); + + let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))] + .iter().flatten().cloned(); + + // Not implemented yet + let _ = vec.iter().cloned().filter(|x| x.starts_with('2')); + + // Not implemented yet + let _ = vec.iter().cloned().map(|x| x.len()); + + // This would fail if changed. + let _ = vec.iter().cloned().map(|x| x + "2"); + + // Not implemented yet + let _ = vec.iter().cloned().find(|x| x == "2"); + + // Not implemented yet + let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty())); + + // Not implemented yet + let _ = vec.iter().cloned().all(|x| x.len() == 1); + + // Not implemented yet + let _ = vec.iter().cloned().any(|x| x.len() == 1); + + // Should probably stay as it is. + let _ = [0, 1, 2, 3, 4].iter().cloned().take(10); +} diff --git a/tests/ui/iter_overeager_cloned.rs b/tests/ui/iter_overeager_cloned.rs new file mode 100644 index 000000000000..dd04e33a4b3a --- /dev/null +++ b/tests/ui/iter_overeager_cloned.rs @@ -0,0 +1,47 @@ +// run-rustfix +#![warn(clippy::iter_overeager_cloned, clippy::redundant_clone, clippy::filter_next)] + +fn main() { + let vec = vec!["1".to_string(), "2".to_string(), "3".to_string()]; + + let _: Option = vec.iter().cloned().last(); + + let _: Option = vec.iter().chain(vec.iter()).cloned().next(); + + let _: usize = vec.iter().filter(|x| x == &"2").cloned().count(); + + let _: Vec<_> = vec.iter().cloned().take(2).collect(); + + let _: Vec<_> = vec.iter().cloned().skip(2).collect(); + + let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2); + + let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))] + .iter() + .cloned() + .flatten(); + + // Not implemented yet + let _ = vec.iter().cloned().filter(|x| x.starts_with('2')); + + // Not implemented yet + let _ = vec.iter().cloned().map(|x| x.len()); + + // This would fail if changed. + let _ = vec.iter().cloned().map(|x| x + "2"); + + // Not implemented yet + let _ = vec.iter().cloned().find(|x| x == "2"); + + // Not implemented yet + let _ = vec.iter().cloned().for_each(|x| assert!(!x.is_empty())); + + // Not implemented yet + let _ = vec.iter().cloned().all(|x| x.len() == 1); + + // Not implemented yet + let _ = vec.iter().cloned().any(|x| x.len() == 1); + + // Should probably stay as it is. + let _ = [0, 1, 2, 3, 4].iter().cloned().take(10); +} diff --git a/tests/ui/iter_overeager_cloned.stderr b/tests/ui/iter_overeager_cloned.stderr new file mode 100644 index 000000000000..e36b0e36fbdf --- /dev/null +++ b/tests/ui/iter_overeager_cloned.stderr @@ -0,0 +1,58 @@ +error: called `cloned().last()` on an `Iterator`. It may be more efficient to call `last().cloned()` instead + --> $DIR/iter_overeager_cloned.rs:7:29 + | +LL | let _: Option = vec.iter().cloned().last(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().last().cloned()` + | + = note: `-D clippy::iter-overeager-cloned` implied by `-D warnings` + +error: called `cloned().next()` on an `Iterator`. It may be more efficient to call `next().cloned()` instead + --> $DIR/iter_overeager_cloned.rs:9:29 + | +LL | let _: Option = vec.iter().chain(vec.iter()).cloned().next(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().chain(vec.iter()).next().cloned()` + +error: called `cloned().count()` on an `Iterator`. It may be more efficient to call `count()` instead + --> $DIR/iter_overeager_cloned.rs:11:20 + | +LL | let _: usize = vec.iter().filter(|x| x == &"2").cloned().count(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().filter(|x| x == &"2").count()` + | + = note: `-D clippy::redundant-clone` implied by `-D warnings` + +error: called `cloned().take(...)` on an `Iterator`. It may be more efficient to call `take(...).cloned()` instead + --> $DIR/iter_overeager_cloned.rs:13:21 + | +LL | let _: Vec<_> = vec.iter().cloned().take(2).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().take(2).cloned()` + +error: called `cloned().skip(...)` on an `Iterator`. It may be more efficient to call `skip(...).cloned()` instead + --> $DIR/iter_overeager_cloned.rs:15:21 + | +LL | let _: Vec<_> = vec.iter().cloned().skip(2).collect(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().skip(2).cloned()` + +error: called `cloned().nth(...)` on an `Iterator`. It may be more efficient to call `nth(...).cloned()` instead + --> $DIR/iter_overeager_cloned.rs:17:13 + | +LL | let _ = vec.iter().filter(|x| x == &"2").cloned().nth(2); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `vec.iter().filter(|x| x == &"2").nth(2).cloned()` + +error: called `cloned().flatten()` on an `Iterator`. It may be more efficient to call `flatten().cloned()` instead + --> $DIR/iter_overeager_cloned.rs:19:13 + | +LL | let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))] + | _____________^ +LL | | .iter() +LL | | .cloned() +LL | | .flatten(); + | |__________________^ + | +help: try this + | +LL ~ let _ = [Some(Some("str".to_string())), Some(Some("str".to_string()))] +LL ~ .iter().flatten().cloned(); + | + +error: aborting due to 7 previous errors + diff --git a/tests/ui/or_fun_call.fixed b/tests/ui/or_fun_call.fixed index 87cdb3ace47c..3208048e0d53 100644 --- a/tests/ui/or_fun_call.fixed +++ b/tests/ui/or_fun_call.fixed @@ -176,4 +176,52 @@ mod issue6675 { } } +mod issue8239 { + fn more_than_max_suggestion_highest_lines_0() { + let frames = Vec::new(); + frames + .iter() + .map(|f: &String| f.to_lowercase()) + .reduce(|mut acc, f| { + acc.push_str(&f); + acc + }) + .unwrap_or_default(); + } + + fn more_to_max_suggestion_highest_lines_1() { + let frames = Vec::new(); + let iter = frames.iter(); + iter.map(|f: &String| f.to_lowercase()) + .reduce(|mut acc, f| { + let _ = ""; + let _ = ""; + acc.push_str(&f); + acc + }) + .unwrap_or_default(); + } + + fn equal_to_max_suggestion_highest_lines() { + let frames = Vec::new(); + let iter = frames.iter(); + iter.map(|f: &String| f.to_lowercase()) + .reduce(|mut acc, f| { + let _ = ""; + acc.push_str(&f); + acc + }).unwrap_or_default(); + } + + fn less_than_max_suggestion_highest_lines() { + let frames = Vec::new(); + let iter = frames.iter(); + let map = iter.map(|f: &String| f.to_lowercase()); + map.reduce(|mut acc, f| { + acc.push_str(&f); + acc + }).unwrap_or_default(); + } +} + fn main() {} diff --git a/tests/ui/or_fun_call.rs b/tests/ui/or_fun_call.rs index 3f69cef301c8..57ab5f03ee28 100644 --- a/tests/ui/or_fun_call.rs +++ b/tests/ui/or_fun_call.rs @@ -176,4 +176,54 @@ mod issue6675 { } } +mod issue8239 { + fn more_than_max_suggestion_highest_lines_0() { + let frames = Vec::new(); + frames + .iter() + .map(|f: &String| f.to_lowercase()) + .reduce(|mut acc, f| { + acc.push_str(&f); + acc + }) + .unwrap_or(String::new()); + } + + fn more_to_max_suggestion_highest_lines_1() { + let frames = Vec::new(); + let iter = frames.iter(); + iter.map(|f: &String| f.to_lowercase()) + .reduce(|mut acc, f| { + let _ = ""; + let _ = ""; + acc.push_str(&f); + acc + }) + .unwrap_or(String::new()); + } + + fn equal_to_max_suggestion_highest_lines() { + let frames = Vec::new(); + let iter = frames.iter(); + iter.map(|f: &String| f.to_lowercase()) + .reduce(|mut acc, f| { + let _ = ""; + acc.push_str(&f); + acc + }) + .unwrap_or(String::new()); + } + + fn less_than_max_suggestion_highest_lines() { + let frames = Vec::new(); + let iter = frames.iter(); + let map = iter.map(|f: &String| f.to_lowercase()); + map.reduce(|mut acc, f| { + acc.push_str(&f); + acc + }) + .unwrap_or(String::new()); + } +} + fn main() {} diff --git a/tests/ui/or_fun_call.stderr b/tests/ui/or_fun_call.stderr index 9d0c42b10c27..549b00ae3c45 100644 --- a/tests/ui/or_fun_call.stderr +++ b/tests/ui/or_fun_call.stderr @@ -108,5 +108,57 @@ error: use of `unwrap_or` followed by a function call LL | None.unwrap_or( unsafe { ptr_to_ref(s) } ); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })` -error: aborting due to 18 previous errors +error: use of `unwrap_or` followed by a call to `new` + --> $DIR/or_fun_call.rs:189:14 + | +LL | .unwrap_or(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` + +error: use of `unwrap_or` followed by a call to `new` + --> $DIR/or_fun_call.rs:202:14 + | +LL | .unwrap_or(String::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_default()` + +error: use of `unwrap_or` followed by a call to `new` + --> $DIR/or_fun_call.rs:208:9 + | +LL | / iter.map(|f: &String| f.to_lowercase()) +LL | | .reduce(|mut acc, f| { +LL | | let _ = ""; +LL | | acc.push_str(&f); +LL | | acc +LL | | }) +LL | | .unwrap_or(String::new()); + | |_____________________________________^ + | +help: try this + | +LL ~ iter.map(|f: &String| f.to_lowercase()) +LL + .reduce(|mut acc, f| { +LL + let _ = ""; +LL + acc.push_str(&f); +LL + acc +LL ~ }).unwrap_or_default(); + | + +error: use of `unwrap_or` followed by a call to `new` + --> $DIR/or_fun_call.rs:221:9 + | +LL | / map.reduce(|mut acc, f| { +LL | | acc.push_str(&f); +LL | | acc +LL | | }) +LL | | .unwrap_or(String::new()); + | |_________________________________^ + | +help: try this + | +LL ~ map.reduce(|mut acc, f| { +LL + acc.push_str(&f); +LL + acc +LL ~ }).unwrap_or_default(); + | + +error: aborting due to 22 previous errors diff --git a/tests/ui/return_self_not_must_use.rs b/tests/ui/return_self_not_must_use.rs index 7dd5742dae9f..9b33ad6d3f6b 100644 --- a/tests/ui/return_self_not_must_use.rs +++ b/tests/ui/return_self_not_must_use.rs @@ -1,4 +1,5 @@ #![crate_type = "lib"] +#![warn(clippy::return_self_not_must_use)] #[derive(Clone)] pub struct Bar; diff --git a/tests/ui/return_self_not_must_use.stderr b/tests/ui/return_self_not_must_use.stderr index 8af10cb65c40..94be87dfa31c 100644 --- a/tests/ui/return_self_not_must_use.stderr +++ b/tests/ui/return_self_not_must_use.stderr @@ -1,5 +1,5 @@ error: missing `#[must_use]` attribute on a method returning `Self` - --> $DIR/return_self_not_must_use.rs:7:5 + --> $DIR/return_self_not_must_use.rs:8:5 | LL | fn what(&self) -> Self; | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -8,7 +8,7 @@ LL | fn what(&self) -> Self; = help: consider adding the `#[must_use]` attribute to the method or directly to the `Self` type error: missing `#[must_use]` attribute on a method returning `Self` - --> $DIR/return_self_not_must_use.rs:17:5 + --> $DIR/return_self_not_must_use.rs:18:5 | LL | / pub fn foo(&self) -> Self { LL | | Self @@ -18,7 +18,7 @@ LL | | } = help: consider adding the `#[must_use]` attribute to the method or directly to the `Self` type error: missing `#[must_use]` attribute on a method returning `Self` - --> $DIR/return_self_not_must_use.rs:20:5 + --> $DIR/return_self_not_must_use.rs:21:5 | LL | / pub fn bar(self) -> Self { LL | | self diff --git a/util/gh-pages/index.html b/util/gh-pages/index.html index f175700a3f47..5b7e61a349d9 100644 --- a/util/gh-pages/index.html +++ b/util/gh-pages/index.html @@ -368,7 +368,7 @@

Fork me on Github - +