From f9dde4bbe9a013ca09b85b5b545a33285b9a0bdc Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sat, 1 Apr 2023 18:21:11 +0000 Subject: [PATCH 1/8] uplift `clippy::clone_double_ref` to rustc --- compiler/rustc_lint/messages.ftl | 7 ++ compiler/rustc_lint/src/clone_double_ref.rs | 86 +++++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 32 ++++++++ tests/ui/lint/clone-double-ref.rs | 23 ++++++ tests/ui/lint/clone-double-ref.stderr | 22 ++++++ 6 files changed, 173 insertions(+) create mode 100644 compiler/rustc_lint/src/clone_double_ref.rs create mode 100644 tests/ui/lint/clone-double-ref.rs create mode 100644 tests/ui/lint/clone-double-ref.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index db15b176df001..db8da08a84129 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -5,6 +5,13 @@ lint_array_into_iter = .use_explicit_into_iter_suggestion = or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value +lint_clone_double_ref = + using `clone` on a double-reference, which copies the reference of type `{$ty}` instead of cloning the inner type + +lint_clone_double_ref_try_deref = try dereferencing it + +lint_clone_double_ref_sugg_explicit = or try being explicit if you are sure, that you want to clone a reference + lint_enum_intrinsics_mem_discriminant = the return value of `mem::discriminant` is unspecified when called with a non-enum type .note = the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `{$ty_param}`, which is not an enum. diff --git a/compiler/rustc_lint/src/clone_double_ref.rs b/compiler/rustc_lint/src/clone_double_ref.rs new file mode 100644 index 0000000000000..e3b14fe4cf34a --- /dev/null +++ b/compiler/rustc_lint/src/clone_double_ref.rs @@ -0,0 +1,86 @@ +use crate::lints::{CloneDoubleRefExplicit, CloneDoubleRefTryDeref}; +use crate::{lints, LateLintPass, LintContext}; + +use rustc_hir as hir; +use rustc_hir::ExprKind; +use rustc_middle::ty; +use rustc_middle::ty::adjustment::Adjust; +use rustc_span::sym; + +declare_lint! { + /// The `clone_double_ref` lint checks for usage of `.clone()` on an `&&T`, + /// which copies the inner `&T`, instead of cloning the underlying `T` and + /// can be confusing. + pub CLONE_DOUBLE_REF, + Warn, + "using `clone` on `&&T`" +} + +declare_lint_pass!(CloneDoubleRef => [CLONE_DOUBLE_REF]); + +impl<'tcx> LateLintPass<'tcx> for CloneDoubleRef { + fn check_expr(&mut self, cx: &crate::LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) { + let ExprKind::MethodCall(path, receiver, args, _) = &e.kind else { return; }; + + if path.ident.name != sym::clone || !args.is_empty() { + return; + } + + let typeck_results = cx.typeck_results(); + + if typeck_results + .type_dependent_def_id(e.hir_id) + .and_then(|id| cx.tcx.trait_of_item(id)) + .zip(cx.tcx.lang_items().clone_trait()) + .map_or(true, |(x, y)| x != y) + { + return; + } + + let arg_adjustments = cx.typeck_results().expr_adjustments(receiver); + + // https://github.com/rust-lang/rust-clippy/issues/9272 + if arg_adjustments.iter().any(|adj| matches!(adj.kind, Adjust::Deref(Some(_)))) { + return; + } + + if !receiver.span.eq_ctxt(e.span) { + return; + } + + let arg_ty = arg_adjustments + .last() + .map_or_else(|| cx.typeck_results().expr_ty(receiver), |a| a.target); + + let ty = cx.typeck_results().expr_ty(e); + + if let ty::Ref(_, inner, _) = arg_ty.kind() + && let ty::Ref(_, innermost, _) = inner.kind() { + let mut inner_ty = innermost; + let mut n = 1; + while let ty::Ref(_, inner, _) = inner_ty.kind() { + inner_ty = inner; + n += 1; + } + let refs = "&".repeat(n); + let derefs = "*".repeat(n); + let start = e.span.with_hi(receiver.span.lo()); + let end = e.span.with_lo(receiver.span.hi()); + cx.emit_spanned_lint(CLONE_DOUBLE_REF, e.span, lints::CloneDoubleRef { + ty, + try_deref: CloneDoubleRefTryDeref { + start, + end, + refs: refs.clone(), + derefs, + }, + explicit: CloneDoubleRefExplicit { + start, + end, + refs, + ty: *inner_ty, + } + }); + } + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index b3578540516d0..099af93731932 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -50,6 +50,7 @@ extern crate tracing; mod array_into_iter; pub mod builtin; +mod clone_double_ref; mod context; mod deref_into_dyn_supertrait; mod early; @@ -95,6 +96,7 @@ use rustc_span::Span; use array_into_iter::ArrayIntoIter; use builtin::*; +use clone_double_ref::CloneDoubleRef; use deref_into_dyn_supertrait::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; use for_loops_over_fallibles::*; @@ -199,6 +201,7 @@ late_lint_methods!( [ BuiltinCombinedModuleLateLintPass, [ + CloneDoubleRef: CloneDoubleRef, ForLoopsOverFallibles: ForLoopsOverFallibles, DerefIntoDynSupertrait: DerefIntoDynSupertrait, HardwiredLints: HardwiredLints, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 1d5e02369f528..e115654cf7bcc 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -564,6 +564,38 @@ pub struct BuiltinUnexpectedCliConfigValue { pub value: Symbol, } +#[derive(LintDiagnostic)] +#[diag(lint_clone_double_ref)] +pub struct CloneDoubleRef<'a> { + pub ty: Ty<'a>, + #[subdiagnostic] + pub try_deref: CloneDoubleRefTryDeref, + #[subdiagnostic] + pub explicit: CloneDoubleRefExplicit<'a>, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_clone_double_ref_sugg_explicit, applicability = "maybe-incorrect")] +pub struct CloneDoubleRefExplicit<'a> { + #[suggestion_part(code = "<{refs}{ty}>::clone(")] + pub start: Span, + #[suggestion_part(code = ")")] + pub end: Span, + pub refs: String, + pub ty: Ty<'a>, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_clone_double_ref_try_deref, applicability = "maybe-incorrect")] +pub struct CloneDoubleRefTryDeref { + #[suggestion_part(code = "{refs}({derefs}")] + pub start: Span, + #[suggestion_part(code = ").clone()")] + pub end: Span, + pub refs: String, + pub derefs: String, +} + // deref_into_dyn_supertrait.rs #[derive(LintDiagnostic)] #[diag(lint_supertrait_as_deref_target)] diff --git a/tests/ui/lint/clone-double-ref.rs b/tests/ui/lint/clone-double-ref.rs new file mode 100644 index 0000000000000..00496b1ec0ea1 --- /dev/null +++ b/tests/ui/lint/clone-double-ref.rs @@ -0,0 +1,23 @@ +#![feature(once_cell)] +#![deny(clone_double_ref)] + +pub fn clone_on_double_ref() { + let x = vec![1]; + let y = &&x; + let z: &Vec<_> = y.clone(); + //~^ ERROR using `clone` on a double-reference, which copies the reference of type `Vec` + + println!("{:p} {:p}", *y, z); +} + +use std::sync::LazyLock; + +pub static STRS: LazyLock<&str> = LazyLock::new(|| "First"); + +// https://github.com/rust-lang/rust-clippy/issues/9272 +fn rust_clippy_issue_9272() { + let str = STRS.clone(); + println!("{str}") +} + +fn main() {} diff --git a/tests/ui/lint/clone-double-ref.stderr b/tests/ui/lint/clone-double-ref.stderr new file mode 100644 index 0000000000000..9276433b7429b --- /dev/null +++ b/tests/ui/lint/clone-double-ref.stderr @@ -0,0 +1,22 @@ +error: using `clone` on a double-reference, which copies the reference of type `Vec` instead of cloning the inner type + --> $DIR/clone-double-ref.rs:7:22 + | +LL | let z: &Vec<_> = y.clone(); + | ^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/clone-double-ref.rs:2:9 + | +LL | #![deny(clone_double_ref)] + | ^^^^^^^^^^^^^^^^ +help: try dereferencing it + | +LL | let z: &Vec<_> = &(*y).clone(); + | +++ ~~~~~~~~~ +help: or try being explicit if you are sure, that you want to clone a reference + | +LL | let z: &Vec<_> = <&Vec>::clone(y); + | +++++++++++++++++++ ~ + +error: aborting due to previous error + From f89f190d891ee5bfce091303886fc2b037f8c430 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sat, 1 Apr 2023 18:22:40 +0000 Subject: [PATCH 2/8] clippy changes to uplift and remove lint --- src/tools/clippy/clippy_dev/src/lib.rs | 2 +- .../clippy/clippy_lints/src/declared_lints.rs | 1 - .../clippy_lints/src/methods/clone_on_copy.rs | 40 +------- .../clippy/clippy_lints/src/methods/mod.rs | 24 ----- .../clippy/clippy_lints/src/renamed_lints.rs | 1 + src/tools/clippy/tests/ui/deprecated.rs | 1 + src/tools/clippy/tests/ui/deprecated.stderr | 8 +- .../tests/ui/explicit_deref_methods.fixed | 2 +- .../clippy/tests/ui/explicit_deref_methods.rs | 2 +- src/tools/clippy/tests/ui/rename.fixed | 2 + src/tools/clippy/tests/ui/rename.rs | 2 + src/tools/clippy/tests/ui/rename.stderr | 92 ++++++++++--------- .../clippy/tests/ui/unnecessary_clone.stderr | 46 +++++----- 13 files changed, 90 insertions(+), 133 deletions(-) diff --git a/src/tools/clippy/clippy_dev/src/lib.rs b/src/tools/clippy/clippy_dev/src/lib.rs index 3a8b070d73516..9d96bbc1f506c 100644 --- a/src/tools/clippy/clippy_dev/src/lib.rs +++ b/src/tools/clippy/clippy_dev/src/lib.rs @@ -12,7 +12,7 @@ extern crate rustc_lexer; use std::path::PathBuf; -pub mod bless; +pub mod bless pub mod dogfood; pub mod fmt; pub mod lint; diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index f24dab6278095..b87bdaceefdab 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/clippy_lints/src/declared_lints.rs @@ -312,7 +312,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::CHARS_NEXT_CMP_INFO, crate::methods::CLEAR_WITH_DRAIN_INFO, crate::methods::CLONED_INSTEAD_OF_COPIED_INFO, - crate::methods::CLONE_DOUBLE_REF_INFO, crate::methods::CLONE_ON_COPY_INFO, crate::methods::CLONE_ON_REF_PTR_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, diff --git a/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs b/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs index 3795c0ec25098..65fd50dff5846 100644 --- a/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs +++ b/src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs @@ -1,7 +1,6 @@ -use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::get_parent_node; use clippy_utils::source::snippet_with_context; -use clippy_utils::sugg; use clippy_utils::ty::is_copy; use rustc_errors::Applicability; use rustc_hir::{BindingAnnotation, ByRef, Expr, ExprKind, MatchSource, Node, PatKind, QPath}; @@ -9,7 +8,6 @@ use rustc_lint::LateContext; use rustc_middle::ty::{self, adjustment::Adjust, print::with_forced_trimmed_paths}; use rustc_span::symbol::{sym, Symbol}; -use super::CLONE_DOUBLE_REF; use super::CLONE_ON_COPY; /// Checks for the `CLONE_ON_COPY` lint. @@ -42,41 +40,7 @@ pub(super) fn check( let ty = cx.typeck_results().expr_ty(expr); if let ty::Ref(_, inner, _) = arg_ty.kind() { - if let ty::Ref(_, innermost, _) = inner.kind() { - span_lint_and_then( - cx, - CLONE_DOUBLE_REF, - expr.span, - &with_forced_trimmed_paths!(format!( - "using `clone` on a double-reference; \ - this will copy the reference of type `{ty}` instead of cloning the inner type" - )), - |diag| { - if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) { - let mut ty = innermost; - let mut n = 0; - while let ty::Ref(_, inner, _) = ty.kind() { - ty = inner; - n += 1; - } - let refs = "&".repeat(n + 1); - let derefs = "*".repeat(n); - let explicit = with_forced_trimmed_paths!(format!("<{refs}{ty}>::clone({snip})")); - diag.span_suggestion( - expr.span, - "try dereferencing it", - with_forced_trimmed_paths!(format!("{refs}({derefs}{}).clone()", snip.deref())), - Applicability::MaybeIncorrect, - ); - diag.span_suggestion( - expr.span, - "or try being explicit if you are sure, that you want to clone a reference", - explicit, - Applicability::MaybeIncorrect, - ); - } - }, - ); + if let ty::Ref(..) = inner.kind() { return; // don't report clone_on_copy } } diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 64bf55ba24c98..d6b981dcbb9c4 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -984,29 +984,6 @@ declare_clippy_lint! { "using 'clone' on a ref-counted pointer" } -declare_clippy_lint! { - /// ### What it does - /// Checks for usage of `.clone()` on an `&&T`. - /// - /// ### Why is this bad? - /// Cloning an `&&T` copies the inner `&T`, instead of - /// cloning the underlying `T`. - /// - /// ### Example - /// ```rust - /// fn main() { - /// let x = vec![1]; - /// let y = &&x; - /// let z = y.clone(); - /// println!("{:p} {:p}", *y, z); // prints out the same pointer - /// } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub CLONE_DOUBLE_REF, - correctness, - "using `clone` on `&&T`" -} - declare_clippy_lint! { /// ### What it does /// Checks for usage of `.to_string()` on an `&&T` where @@ -3258,7 +3235,6 @@ impl_lint_pass!(Methods => [ CHARS_LAST_CMP, CLONE_ON_COPY, CLONE_ON_REF_PTR, - CLONE_DOUBLE_REF, COLLAPSIBLE_STR_REPLACE, ITER_OVEREAGER_CLONED, CLONED_INSTEAD_OF_COPIED, diff --git a/src/tools/clippy/clippy_lints/src/renamed_lints.rs b/src/tools/clippy/clippy_lints/src/renamed_lints.rs index 9f487dedb8cb6..2410a2ebe50cd 100644 --- a/src/tools/clippy/clippy_lints/src/renamed_lints.rs +++ b/src/tools/clippy/clippy_lints/src/renamed_lints.rs @@ -30,6 +30,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::stutter", "clippy::module_name_repetitions"), ("clippy::to_string_in_display", "clippy::recursive_format_impl"), ("clippy::zero_width_space", "clippy::invisible_characters"), + ("clippy::clone_double_ref", "clone_double_ref"), ("clippy::drop_bounds", "drop_bounds"), ("clippy::for_loop_over_option", "for_loops_over_fallibles"), ("clippy::for_loop_over_result", "for_loops_over_fallibles"), diff --git a/src/tools/clippy/tests/ui/deprecated.rs b/src/tools/clippy/tests/ui/deprecated.rs index 07270bd76362a..7d0a02ff7bd89 100644 --- a/src/tools/clippy/tests/ui/deprecated.rs +++ b/src/tools/clippy/tests/ui/deprecated.rs @@ -18,5 +18,6 @@ #![warn(clippy::filter_map)] #![warn(clippy::pub_enum_variant_names)] #![warn(clippy::wrong_pub_self_convention)] +#![warn(clippy::clone_double_ref)] fn main() {} diff --git a/src/tools/clippy/tests/ui/deprecated.stderr b/src/tools/clippy/tests/ui/deprecated.stderr index 0e142ac8f20e7..10749e22d83a0 100644 --- a/src/tools/clippy/tests/ui/deprecated.stderr +++ b/src/tools/clippy/tests/ui/deprecated.stderr @@ -96,5 +96,11 @@ error: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid LL | #![warn(clippy::wrong_pub_self_convention)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 16 previous errors +error: lint `clippy::clone_double_ref` has been renamed to `clone_double_ref` + --> $DIR/deprecated.rs:21:9 + | +LL | #![warn(clippy::clone_double_ref)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clone_double_ref` + +error: aborting due to 17 previous errors diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.fixed b/src/tools/clippy/tests/ui/explicit_deref_methods.fixed index 6d32bbece1e57..bef4437c131c3 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.fixed +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.fixed @@ -3,7 +3,7 @@ #![allow(unused_variables)] #![allow( clippy::borrow_deref_ref, - clippy::clone_double_ref, + clone_double_ref, clippy::explicit_auto_deref, clippy::needless_borrow, clippy::uninlined_format_args diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.rs b/src/tools/clippy/tests/ui/explicit_deref_methods.rs index 779909e42380c..ffbdbb8d73bf2 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.rs +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.rs @@ -3,7 +3,7 @@ #![allow(unused_variables)] #![allow( clippy::borrow_deref_ref, - clippy::clone_double_ref, + clone_double_ref, clippy::explicit_auto_deref, clippy::needless_borrow, clippy::uninlined_format_args diff --git a/src/tools/clippy/tests/ui/rename.fixed b/src/tools/clippy/tests/ui/rename.fixed index 5076f61334d67..1ceb60dba2f79 100644 --- a/src/tools/clippy/tests/ui/rename.fixed +++ b/src/tools/clippy/tests/ui/rename.fixed @@ -27,6 +27,7 @@ #![allow(clippy::module_name_repetitions)] #![allow(clippy::recursive_format_impl)] #![allow(clippy::invisible_characters)] +#![allow(clone_double_ref)] #![allow(drop_bounds)] #![allow(for_loops_over_fallibles)] #![allow(array_into_iter)] @@ -67,6 +68,7 @@ #![warn(clippy::module_name_repetitions)] #![warn(clippy::recursive_format_impl)] #![warn(clippy::invisible_characters)] +#![warn(clone_double_ref)] #![warn(drop_bounds)] #![warn(for_loops_over_fallibles)] #![warn(for_loops_over_fallibles)] diff --git a/src/tools/clippy/tests/ui/rename.rs b/src/tools/clippy/tests/ui/rename.rs index 64bc1ca7116c5..81a01b7fb3a54 100644 --- a/src/tools/clippy/tests/ui/rename.rs +++ b/src/tools/clippy/tests/ui/rename.rs @@ -27,6 +27,7 @@ #![allow(clippy::module_name_repetitions)] #![allow(clippy::recursive_format_impl)] #![allow(clippy::invisible_characters)] +#![allow(clone_double_ref)] #![allow(drop_bounds)] #![allow(for_loops_over_fallibles)] #![allow(array_into_iter)] @@ -67,6 +68,7 @@ #![warn(clippy::stutter)] #![warn(clippy::to_string_in_display)] #![warn(clippy::zero_width_space)] +#![warn(clippy::clone_double_ref)] #![warn(clippy::drop_bounds)] #![warn(clippy::for_loop_over_option)] #![warn(clippy::for_loop_over_result)] diff --git a/src/tools/clippy/tests/ui/rename.stderr b/src/tools/clippy/tests/ui/rename.stderr index 27a0263292ef1..2fe5e320c67c6 100644 --- a/src/tools/clippy/tests/ui/rename.stderr +++ b/src/tools/clippy/tests/ui/rename.stderr @@ -1,5 +1,5 @@ error: lint `clippy::almost_complete_letter_range` has been renamed to `clippy::almost_complete_range` - --> $DIR/rename.rs:42:9 + --> $DIR/rename.rs:43:9 | LL | #![warn(clippy::almost_complete_letter_range)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::almost_complete_range` @@ -7,250 +7,256 @@ LL | #![warn(clippy::almost_complete_letter_range)] = note: `-D renamed-and-removed-lints` implied by `-D warnings` error: lint `clippy::blacklisted_name` has been renamed to `clippy::disallowed_names` - --> $DIR/rename.rs:43:9 + --> $DIR/rename.rs:44:9 | LL | #![warn(clippy::blacklisted_name)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_names` error: lint `clippy::block_in_if_condition_expr` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:44:9 + --> $DIR/rename.rs:45:9 | LL | #![warn(clippy::block_in_if_condition_expr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::block_in_if_condition_stmt` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:45:9 + --> $DIR/rename.rs:46:9 | LL | #![warn(clippy::block_in_if_condition_stmt)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::box_vec` has been renamed to `clippy::box_collection` - --> $DIR/rename.rs:46:9 + --> $DIR/rename.rs:47:9 | LL | #![warn(clippy::box_vec)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::box_collection` error: lint `clippy::const_static_lifetime` has been renamed to `clippy::redundant_static_lifetimes` - --> $DIR/rename.rs:47:9 + --> $DIR/rename.rs:48:9 | LL | #![warn(clippy::const_static_lifetime)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::redundant_static_lifetimes` error: lint `clippy::cyclomatic_complexity` has been renamed to `clippy::cognitive_complexity` - --> $DIR/rename.rs:48:9 + --> $DIR/rename.rs:49:9 | LL | #![warn(clippy::cyclomatic_complexity)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::cognitive_complexity` error: lint `clippy::derive_hash_xor_eq` has been renamed to `clippy::derived_hash_with_manual_eq` - --> $DIR/rename.rs:49:9 + --> $DIR/rename.rs:50:9 | LL | #![warn(clippy::derive_hash_xor_eq)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::derived_hash_with_manual_eq` error: lint `clippy::disallowed_method` has been renamed to `clippy::disallowed_methods` - --> $DIR/rename.rs:50:9 + --> $DIR/rename.rs:51:9 | LL | #![warn(clippy::disallowed_method)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_methods` error: lint `clippy::disallowed_type` has been renamed to `clippy::disallowed_types` - --> $DIR/rename.rs:51:9 + --> $DIR/rename.rs:52:9 | LL | #![warn(clippy::disallowed_type)] | ^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_types` error: lint `clippy::eval_order_dependence` has been renamed to `clippy::mixed_read_write_in_expression` - --> $DIR/rename.rs:52:9 + --> $DIR/rename.rs:53:9 | LL | #![warn(clippy::eval_order_dependence)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::mixed_read_write_in_expression` error: lint `clippy::identity_conversion` has been renamed to `clippy::useless_conversion` - --> $DIR/rename.rs:53:9 + --> $DIR/rename.rs:54:9 | LL | #![warn(clippy::identity_conversion)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::useless_conversion` error: lint `clippy::if_let_some_result` has been renamed to `clippy::match_result_ok` - --> $DIR/rename.rs:54:9 + --> $DIR/rename.rs:55:9 | LL | #![warn(clippy::if_let_some_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::match_result_ok` error: lint `clippy::logic_bug` has been renamed to `clippy::overly_complex_bool_expr` - --> $DIR/rename.rs:55:9 + --> $DIR/rename.rs:56:9 | LL | #![warn(clippy::logic_bug)] | ^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::overly_complex_bool_expr` error: lint `clippy::new_without_default_derive` has been renamed to `clippy::new_without_default` - --> $DIR/rename.rs:56:9 + --> $DIR/rename.rs:57:9 | LL | #![warn(clippy::new_without_default_derive)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::new_without_default` error: lint `clippy::option_and_then_some` has been renamed to `clippy::bind_instead_of_map` - --> $DIR/rename.rs:57:9 + --> $DIR/rename.rs:58:9 | LL | #![warn(clippy::option_and_then_some)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::bind_instead_of_map` error: lint `clippy::option_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:58:9 + --> $DIR/rename.rs:59:9 | LL | #![warn(clippy::option_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::option_map_unwrap_or` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:59:9 + --> $DIR/rename.rs:60:9 | LL | #![warn(clippy::option_map_unwrap_or)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:60:9 + --> $DIR/rename.rs:61:9 | LL | #![warn(clippy::option_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:61:9 + --> $DIR/rename.rs:62:9 | LL | #![warn(clippy::option_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::ref_in_deref` has been renamed to `clippy::needless_borrow` - --> $DIR/rename.rs:62:9 + --> $DIR/rename.rs:63:9 | LL | #![warn(clippy::ref_in_deref)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::needless_borrow` error: lint `clippy::result_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:63:9 + --> $DIR/rename.rs:64:9 | LL | #![warn(clippy::result_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::result_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:64:9 + --> $DIR/rename.rs:65:9 | LL | #![warn(clippy::result_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::result_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:65:9 + --> $DIR/rename.rs:66:9 | LL | #![warn(clippy::result_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::single_char_push_str` has been renamed to `clippy::single_char_add_str` - --> $DIR/rename.rs:66:9 + --> $DIR/rename.rs:67:9 | LL | #![warn(clippy::single_char_push_str)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::single_char_add_str` error: lint `clippy::stutter` has been renamed to `clippy::module_name_repetitions` - --> $DIR/rename.rs:67:9 + --> $DIR/rename.rs:68:9 | LL | #![warn(clippy::stutter)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::module_name_repetitions` error: lint `clippy::to_string_in_display` has been renamed to `clippy::recursive_format_impl` - --> $DIR/rename.rs:68:9 + --> $DIR/rename.rs:69:9 | LL | #![warn(clippy::to_string_in_display)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::recursive_format_impl` error: lint `clippy::zero_width_space` has been renamed to `clippy::invisible_characters` - --> $DIR/rename.rs:69:9 + --> $DIR/rename.rs:70:9 | LL | #![warn(clippy::zero_width_space)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::invisible_characters` +error: lint `clippy::clone_double_ref` has been renamed to `clone_double_ref` + --> $DIR/rename.rs:71:9 + | +LL | #![warn(clippy::clone_double_ref)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clone_double_ref` + error: lint `clippy::drop_bounds` has been renamed to `drop_bounds` - --> $DIR/rename.rs:70:9 + --> $DIR/rename.rs:72:9 | LL | #![warn(clippy::drop_bounds)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `drop_bounds` error: lint `clippy::for_loop_over_option` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:71:9 + --> $DIR/rename.rs:73:9 | LL | #![warn(clippy::for_loop_over_option)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::for_loop_over_result` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:72:9 + --> $DIR/rename.rs:74:9 | LL | #![warn(clippy::for_loop_over_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::for_loops_over_fallibles` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:73:9 + --> $DIR/rename.rs:75:9 | LL | #![warn(clippy::for_loops_over_fallibles)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter` - --> $DIR/rename.rs:74:9 + --> $DIR/rename.rs:76:9 | LL | #![warn(clippy::into_iter_on_array)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `array_into_iter` error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomic_ordering` - --> $DIR/rename.rs:75:9 + --> $DIR/rename.rs:77:9 | LL | #![warn(clippy::invalid_atomic_ordering)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering` error: lint `clippy::invalid_ref` has been renamed to `invalid_value` - --> $DIR/rename.rs:76:9 + --> $DIR/rename.rs:78:9 | LL | #![warn(clippy::invalid_ref)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_value` error: lint `clippy::let_underscore_drop` has been renamed to `let_underscore_drop` - --> $DIR/rename.rs:77:9 + --> $DIR/rename.rs:79:9 | LL | #![warn(clippy::let_underscore_drop)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `let_underscore_drop` error: lint `clippy::mem_discriminant_non_enum` has been renamed to `enum_intrinsics_non_enums` - --> $DIR/rename.rs:78:9 + --> $DIR/rename.rs:80:9 | LL | #![warn(clippy::mem_discriminant_non_enum)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `enum_intrinsics_non_enums` error: lint `clippy::panic_params` has been renamed to `non_fmt_panics` - --> $DIR/rename.rs:79:9 + --> $DIR/rename.rs:81:9 | LL | #![warn(clippy::panic_params)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `non_fmt_panics` error: lint `clippy::positional_named_format_parameters` has been renamed to `named_arguments_used_positionally` - --> $DIR/rename.rs:80:9 + --> $DIR/rename.rs:82:9 | LL | #![warn(clippy::positional_named_format_parameters)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `named_arguments_used_positionally` error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr` - --> $DIR/rename.rs:81:9 + --> $DIR/rename.rs:83:9 | LL | #![warn(clippy::temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr` error: lint `clippy::unknown_clippy_lints` has been renamed to `unknown_lints` - --> $DIR/rename.rs:82:9 + --> $DIR/rename.rs:84:9 | LL | #![warn(clippy::unknown_clippy_lints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unknown_lints` error: lint `clippy::unused_label` has been renamed to `unused_labels` - --> $DIR/rename.rs:83:9 + --> $DIR/rename.rs:85:9 | LL | #![warn(clippy::unused_label)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unused_labels` -error: aborting due to 42 previous errors +error: aborting due to 43 previous errors diff --git a/src/tools/clippy/tests/ui/unnecessary_clone.stderr b/src/tools/clippy/tests/ui/unnecessary_clone.stderr index 6022d9fa4c5c3..264da2a70c594 100644 --- a/src/tools/clippy/tests/ui/unnecessary_clone.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_clone.stderr @@ -44,29 +44,35 @@ error: using `clone` on type `Option` which implements the `Copy` trait LL | Some(t).clone(); | ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)` -error: using `clone` on a double-reference; this will copy the reference of type `&Vec` instead of cloning the inner type +error: using `clone` on type `E` which implements the `Copy` trait + --> $DIR/unnecessary_clone.rs:84:20 + | +LL | let _: E = a.clone(); + | ^^^^^^^^^ help: try dereferencing it: `*****a` + +error: using `.clone()` on a ref-counted pointer + --> $DIR/unnecessary_clone.rs:108:14 + | +LL | Some(try_opt!(Some(rc)).clone()) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Rc::::clone(&try_opt!(Some(rc)))` + +error: using `clone` on a double-reference, which copies the reference of type `std::vec::Vec` instead of cloning the inner type --> $DIR/unnecessary_clone.rs:48:22 | LL | let z: &Vec<_> = y.clone(); | ^^^^^^^^^ | - = note: `#[deny(clippy::clone_double_ref)]` on by default + = note: `-D clone-double-ref` implied by `-D warnings` help: try dereferencing it | LL | let z: &Vec<_> = &(*y).clone(); - | ~~~~~~~~~~~~~ + | +++ ~~~~~~~~~ help: or try being explicit if you are sure, that you want to clone a reference | -LL | let z: &Vec<_> = <&Vec>::clone(y); - | ~~~~~~~~~~~~~~~~~~~~~ +LL | let z: &Vec<_> = <&std::vec::Vec>::clone(y); + | +++++++++++++++++++++++++++++ ~ -error: using `clone` on type `E` which implements the `Copy` trait - --> $DIR/unnecessary_clone.rs:84:20 - | -LL | let _: E = a.clone(); - | ^^^^^^^^^ help: try dereferencing it: `*****a` - -error: using `clone` on a double-reference; this will copy the reference of type `&[u8]` instead of cloning the inner type +error: using `clone` on a double-reference, which copies the reference of type `[u8]` instead of cloning the inner type --> $DIR/unnecessary_clone.rs:89:22 | LL | let _ = &mut encoded.clone(); @@ -75,13 +81,13 @@ LL | let _ = &mut encoded.clone(); help: try dereferencing it | LL | let _ = &mut &(*encoded).clone(); - | ~~~~~~~~~~~~~~~~~~~ + | +++ ~~~~~~~~~ help: or try being explicit if you are sure, that you want to clone a reference | LL | let _ = &mut <&[u8]>::clone(encoded); - | ~~~~~~~~~~~~~~~~~~~~~~~ + | +++++++++++++++ ~ -error: using `clone` on a double-reference; this will copy the reference of type `&[u8]` instead of cloning the inner type +error: using `clone` on a double-reference, which copies the reference of type `[u8]` instead of cloning the inner type --> $DIR/unnecessary_clone.rs:90:18 | LL | let _ = &encoded.clone(); @@ -90,17 +96,11 @@ LL | let _ = &encoded.clone(); help: try dereferencing it | LL | let _ = &&(*encoded).clone(); - | ~~~~~~~~~~~~~~~~~~~ + | +++ ~~~~~~~~~ help: or try being explicit if you are sure, that you want to clone a reference | LL | let _ = &<&[u8]>::clone(encoded); - | ~~~~~~~~~~~~~~~~~~~~~~~ - -error: using `.clone()` on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:108:14 - | -LL | Some(try_opt!(Some(rc)).clone()) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Rc::::clone(&try_opt!(Some(rc)))` + | +++++++++++++++ ~ error: aborting due to 12 previous errors From 744b241f112f93ddd6dfd57055d118a827548b9f Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sat, 1 Apr 2023 19:18:26 +0000 Subject: [PATCH 3/8] fix it in librustdoc --- src/librustdoc/html/format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 1b445b8981e1a..297120da284b9 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -167,7 +167,7 @@ pub(crate) fn print_generic_bounds<'a, 'tcx: 'a>( display_fn(move |f| { let mut bounds_dup = FxHashSet::default(); - for (i, bound) in bounds.iter().filter(|b| bounds_dup.insert(b.clone())).enumerate() { + for (i, bound) in bounds.iter().filter(|b| bounds_dup.insert(*b)).enumerate() { if i > 0 { f.write_str(" + ")?; } From 63c5d3897641ef1d29535c9abd3a4ebbd01f3e8d Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Mon, 3 Apr 2023 06:56:28 +0000 Subject: [PATCH 4/8] temp --- tests/ui/lint/noop-method-call.rs | 5 ++++- tests/ui/trait-bounds/issue-94680.rs | 2 ++ tests/ui/trivial-bounds/issue-73021-impossible-inline.rs | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/ui/lint/noop-method-call.rs b/tests/ui/lint/noop-method-call.rs index 89b2966359542..e52dea2e32168 100644 --- a/tests/ui/lint/noop-method-call.rs +++ b/tests/ui/lint/noop-method-call.rs @@ -1,7 +1,7 @@ // check-pass #![allow(unused)] -#![warn(noop_method_call)] +#![warn(noop_method_call, clone_double_ref)] use std::borrow::Borrow; use std::ops::Deref; @@ -15,6 +15,7 @@ fn main() { let non_clone_type_ref = &PlainType(1u32); let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); //~^ WARNING call to `.clone()` on a reference in this situation does nothing + //~| WARNING using `clone` on a double-reference, which copies the reference of type `PlainType` let clone_type_ref = &CloneType(1u32); let clone_type_ref_clone: CloneType = clone_type_ref.clone(); @@ -23,6 +24,7 @@ fn main() { // peels the outer reference off let clone_type_ref = &&CloneType(1u32); let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); + //~^ WARNING using `clone` on a double-reference, which copies the reference of type `CloneType` let non_deref_type = &PlainType(1u32); let non_deref_type_deref: &PlainType = non_deref_type.deref(); @@ -42,6 +44,7 @@ fn main() { let xs = ["a", "b", "c"]; let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead + //~^ WARNING using `clone` on a double-reference, which copies the reference of type `str` } fn generic(non_clone_type: &PlainType) { diff --git a/tests/ui/trait-bounds/issue-94680.rs b/tests/ui/trait-bounds/issue-94680.rs index 58e892079e65f..7095be08afa65 100644 --- a/tests/ui/trait-bounds/issue-94680.rs +++ b/tests/ui/trait-bounds/issue-94680.rs @@ -1,5 +1,7 @@ // check-pass +#![allow(clone_double_ref)] + fn main() { println!("{:?}", { type T = (); diff --git a/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs b/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs index ab6677e911b24..a09bf4219fe90 100644 --- a/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs +++ b/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs @@ -2,7 +2,7 @@ // revisions: no-opt inline // [inline]compile-flags: -Zmir-opt-level=3 --emit=mir #![feature(trivial_bounds)] -#![allow(unused)] +#![allow(unused, clone_double_ref)] trait Foo { fn test(&self); From 8a919eb5da56f312193ea6f578757e3416d042d5 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Thu, 6 Apr 2023 11:56:45 +0000 Subject: [PATCH 5/8] fix clippy --- src/tools/clippy/clippy_dev/src/lib.rs | 2 +- src/tools/clippy/tests/ui/deprecated.rs | 1 - src/tools/clippy/tests/ui/deprecated.stderr | 8 +-- .../clippy/tests/ui/unnecessary_clone.rs | 13 ----- .../clippy/tests/ui/unnecessary_clone.stderr | 52 ++----------------- tests/ui/lint/clone-double-ref.rs | 5 ++ 6 files changed, 10 insertions(+), 71 deletions(-) diff --git a/src/tools/clippy/clippy_dev/src/lib.rs b/src/tools/clippy/clippy_dev/src/lib.rs index 9d96bbc1f506c..3a8b070d73516 100644 --- a/src/tools/clippy/clippy_dev/src/lib.rs +++ b/src/tools/clippy/clippy_dev/src/lib.rs @@ -12,7 +12,7 @@ extern crate rustc_lexer; use std::path::PathBuf; -pub mod bless +pub mod bless; pub mod dogfood; pub mod fmt; pub mod lint; diff --git a/src/tools/clippy/tests/ui/deprecated.rs b/src/tools/clippy/tests/ui/deprecated.rs index 7d0a02ff7bd89..07270bd76362a 100644 --- a/src/tools/clippy/tests/ui/deprecated.rs +++ b/src/tools/clippy/tests/ui/deprecated.rs @@ -18,6 +18,5 @@ #![warn(clippy::filter_map)] #![warn(clippy::pub_enum_variant_names)] #![warn(clippy::wrong_pub_self_convention)] -#![warn(clippy::clone_double_ref)] fn main() {} diff --git a/src/tools/clippy/tests/ui/deprecated.stderr b/src/tools/clippy/tests/ui/deprecated.stderr index 10749e22d83a0..0e142ac8f20e7 100644 --- a/src/tools/clippy/tests/ui/deprecated.stderr +++ b/src/tools/clippy/tests/ui/deprecated.stderr @@ -96,11 +96,5 @@ error: lint `clippy::wrong_pub_self_convention` has been removed: set the `avoid LL | #![warn(clippy::wrong_pub_self_convention)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: lint `clippy::clone_double_ref` has been renamed to `clone_double_ref` - --> $DIR/deprecated.rs:21:9 - | -LL | #![warn(clippy::clone_double_ref)] - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clone_double_ref` - -error: aborting due to 17 previous errors +error: aborting due to 16 previous errors diff --git a/src/tools/clippy/tests/ui/unnecessary_clone.rs b/src/tools/clippy/tests/ui/unnecessary_clone.rs index 8b1629b19a769..7ceed3c75fd85 100644 --- a/src/tools/clippy/tests/ui/unnecessary_clone.rs +++ b/src/tools/clippy/tests/ui/unnecessary_clone.rs @@ -42,14 +42,6 @@ fn clone_on_copy_generic(t: T) { Some(t).clone(); } -fn clone_on_double_ref() { - let x = vec![1]; - let y = &&x; - let z: &Vec<_> = y.clone(); - - println!("{:p} {:p}", *y, z); -} - mod many_derefs { struct A; struct B; @@ -84,11 +76,6 @@ mod many_derefs { let _: E = a.clone(); let _: E = *****a; } - - fn check(mut encoded: &[u8]) { - let _ = &mut encoded.clone(); - let _ = &encoded.clone(); - } } mod issue2076 { diff --git a/src/tools/clippy/tests/ui/unnecessary_clone.stderr b/src/tools/clippy/tests/ui/unnecessary_clone.stderr index 264da2a70c594..5686ab6b4531e 100644 --- a/src/tools/clippy/tests/ui/unnecessary_clone.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_clone.stderr @@ -45,62 +45,16 @@ LL | Some(t).clone(); | ^^^^^^^^^^^^^^^ help: try removing the `clone` call: `Some(t)` error: using `clone` on type `E` which implements the `Copy` trait - --> $DIR/unnecessary_clone.rs:84:20 + --> $DIR/unnecessary_clone.rs:76:20 | LL | let _: E = a.clone(); | ^^^^^^^^^ help: try dereferencing it: `*****a` error: using `.clone()` on a ref-counted pointer - --> $DIR/unnecessary_clone.rs:108:14 + --> $DIR/unnecessary_clone.rs:95:14 | LL | Some(try_opt!(Some(rc)).clone()) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `Rc::::clone(&try_opt!(Some(rc)))` -error: using `clone` on a double-reference, which copies the reference of type `std::vec::Vec` instead of cloning the inner type - --> $DIR/unnecessary_clone.rs:48:22 - | -LL | let z: &Vec<_> = y.clone(); - | ^^^^^^^^^ - | - = note: `-D clone-double-ref` implied by `-D warnings` -help: try dereferencing it - | -LL | let z: &Vec<_> = &(*y).clone(); - | +++ ~~~~~~~~~ -help: or try being explicit if you are sure, that you want to clone a reference - | -LL | let z: &Vec<_> = <&std::vec::Vec>::clone(y); - | +++++++++++++++++++++++++++++ ~ - -error: using `clone` on a double-reference, which copies the reference of type `[u8]` instead of cloning the inner type - --> $DIR/unnecessary_clone.rs:89:22 - | -LL | let _ = &mut encoded.clone(); - | ^^^^^^^^^^^^^^^ - | -help: try dereferencing it - | -LL | let _ = &mut &(*encoded).clone(); - | +++ ~~~~~~~~~ -help: or try being explicit if you are sure, that you want to clone a reference - | -LL | let _ = &mut <&[u8]>::clone(encoded); - | +++++++++++++++ ~ - -error: using `clone` on a double-reference, which copies the reference of type `[u8]` instead of cloning the inner type - --> $DIR/unnecessary_clone.rs:90:18 - | -LL | let _ = &encoded.clone(); - | ^^^^^^^^^^^^^^^ - | -help: try dereferencing it - | -LL | let _ = &&(*encoded).clone(); - | +++ ~~~~~~~~~ -help: or try being explicit if you are sure, that you want to clone a reference - | -LL | let _ = &<&[u8]>::clone(encoded); - | +++++++++++++++ ~ - -error: aborting due to 12 previous errors +error: aborting due to 9 previous errors diff --git a/tests/ui/lint/clone-double-ref.rs b/tests/ui/lint/clone-double-ref.rs index 00496b1ec0ea1..45dd91565da06 100644 --- a/tests/ui/lint/clone-double-ref.rs +++ b/tests/ui/lint/clone-double-ref.rs @@ -20,4 +20,9 @@ fn rust_clippy_issue_9272() { println!("{str}") } +fn check(mut encoded: &[u8]) { + let _ = &mut encoded.clone(); + let _ = &encoded.clone(); +} + fn main() {} From 0dbc32dd3426f41f8d687448faf0495de23cd0e6 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Fri, 7 Apr 2023 14:04:12 +0000 Subject: [PATCH 6/8] merge clone_double_ref with noop_method_call --- compiler/rustc_lint/messages.ftl | 6 +- compiler/rustc_lint/src/clone_double_ref.rs | 86 ------------------- compiler/rustc_lint/src/lib.rs | 3 - compiler/rustc_lint/src/lints.rs | 28 +----- compiler/rustc_lint/src/noop_method_call.rs | 68 +++++++++++---- library/alloc/src/str.rs | 4 +- tests/ui/issues/issue-11820.rs | 2 + tests/ui/lint/clone-double-ref.rs | 8 +- tests/ui/lint/clone-double-ref.stderr | 35 +++++--- tests/ui/lint/noop-method-call.rs | 9 +- tests/ui/lint/noop-method-call.stderr | 32 ++++++- tests/ui/underscore-imports/cycle.rs | 1 + tests/ui/underscore-imports/hygiene.rs | 1 + tests/ui/underscore-imports/macro-expanded.rs | 1 + 14 files changed, 119 insertions(+), 165 deletions(-) delete mode 100644 compiler/rustc_lint/src/clone_double_ref.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index db8da08a84129..b56f2e32d7956 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -6,11 +6,7 @@ lint_array_into_iter = or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value lint_clone_double_ref = - using `clone` on a double-reference, which copies the reference of type `{$ty}` instead of cloning the inner type - -lint_clone_double_ref_try_deref = try dereferencing it - -lint_clone_double_ref_sugg_explicit = or try being explicit if you are sure, that you want to clone a reference + using `.{$call}()` on a double reference, which copies `{$ty}` instead of {$op} the inner type lint_enum_intrinsics_mem_discriminant = the return value of `mem::discriminant` is unspecified when called with a non-enum type diff --git a/compiler/rustc_lint/src/clone_double_ref.rs b/compiler/rustc_lint/src/clone_double_ref.rs deleted file mode 100644 index e3b14fe4cf34a..0000000000000 --- a/compiler/rustc_lint/src/clone_double_ref.rs +++ /dev/null @@ -1,86 +0,0 @@ -use crate::lints::{CloneDoubleRefExplicit, CloneDoubleRefTryDeref}; -use crate::{lints, LateLintPass, LintContext}; - -use rustc_hir as hir; -use rustc_hir::ExprKind; -use rustc_middle::ty; -use rustc_middle::ty::adjustment::Adjust; -use rustc_span::sym; - -declare_lint! { - /// The `clone_double_ref` lint checks for usage of `.clone()` on an `&&T`, - /// which copies the inner `&T`, instead of cloning the underlying `T` and - /// can be confusing. - pub CLONE_DOUBLE_REF, - Warn, - "using `clone` on `&&T`" -} - -declare_lint_pass!(CloneDoubleRef => [CLONE_DOUBLE_REF]); - -impl<'tcx> LateLintPass<'tcx> for CloneDoubleRef { - fn check_expr(&mut self, cx: &crate::LateContext<'tcx>, e: &'tcx hir::Expr<'tcx>) { - let ExprKind::MethodCall(path, receiver, args, _) = &e.kind else { return; }; - - if path.ident.name != sym::clone || !args.is_empty() { - return; - } - - let typeck_results = cx.typeck_results(); - - if typeck_results - .type_dependent_def_id(e.hir_id) - .and_then(|id| cx.tcx.trait_of_item(id)) - .zip(cx.tcx.lang_items().clone_trait()) - .map_or(true, |(x, y)| x != y) - { - return; - } - - let arg_adjustments = cx.typeck_results().expr_adjustments(receiver); - - // https://github.com/rust-lang/rust-clippy/issues/9272 - if arg_adjustments.iter().any(|adj| matches!(adj.kind, Adjust::Deref(Some(_)))) { - return; - } - - if !receiver.span.eq_ctxt(e.span) { - return; - } - - let arg_ty = arg_adjustments - .last() - .map_or_else(|| cx.typeck_results().expr_ty(receiver), |a| a.target); - - let ty = cx.typeck_results().expr_ty(e); - - if let ty::Ref(_, inner, _) = arg_ty.kind() - && let ty::Ref(_, innermost, _) = inner.kind() { - let mut inner_ty = innermost; - let mut n = 1; - while let ty::Ref(_, inner, _) = inner_ty.kind() { - inner_ty = inner; - n += 1; - } - let refs = "&".repeat(n); - let derefs = "*".repeat(n); - let start = e.span.with_hi(receiver.span.lo()); - let end = e.span.with_lo(receiver.span.hi()); - cx.emit_spanned_lint(CLONE_DOUBLE_REF, e.span, lints::CloneDoubleRef { - ty, - try_deref: CloneDoubleRefTryDeref { - start, - end, - refs: refs.clone(), - derefs, - }, - explicit: CloneDoubleRefExplicit { - start, - end, - refs, - ty: *inner_ty, - } - }); - } - } -} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 099af93731932..b3578540516d0 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -50,7 +50,6 @@ extern crate tracing; mod array_into_iter; pub mod builtin; -mod clone_double_ref; mod context; mod deref_into_dyn_supertrait; mod early; @@ -96,7 +95,6 @@ use rustc_span::Span; use array_into_iter::ArrayIntoIter; use builtin::*; -use clone_double_ref::CloneDoubleRef; use deref_into_dyn_supertrait::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; use for_loops_over_fallibles::*; @@ -201,7 +199,6 @@ late_lint_methods!( [ BuiltinCombinedModuleLateLintPass, [ - CloneDoubleRef: CloneDoubleRef, ForLoopsOverFallibles: ForLoopsOverFallibles, DerefIntoDynSupertrait: DerefIntoDynSupertrait, HardwiredLints: HardwiredLints, diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index e115654cf7bcc..d6480d78cbc5a 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -567,33 +567,9 @@ pub struct BuiltinUnexpectedCliConfigValue { #[derive(LintDiagnostic)] #[diag(lint_clone_double_ref)] pub struct CloneDoubleRef<'a> { + pub call: Symbol, pub ty: Ty<'a>, - #[subdiagnostic] - pub try_deref: CloneDoubleRefTryDeref, - #[subdiagnostic] - pub explicit: CloneDoubleRefExplicit<'a>, -} - -#[derive(Subdiagnostic)] -#[multipart_suggestion(lint_clone_double_ref_sugg_explicit, applicability = "maybe-incorrect")] -pub struct CloneDoubleRefExplicit<'a> { - #[suggestion_part(code = "<{refs}{ty}>::clone(")] - pub start: Span, - #[suggestion_part(code = ")")] - pub end: Span, - pub refs: String, - pub ty: Ty<'a>, -} - -#[derive(Subdiagnostic)] -#[multipart_suggestion(lint_clone_double_ref_try_deref, applicability = "maybe-incorrect")] -pub struct CloneDoubleRefTryDeref { - #[suggestion_part(code = "{refs}({derefs}")] - pub start: Span, - #[suggestion_part(code = ").clone()")] - pub end: Span, - pub refs: String, - pub derefs: String, + pub op: &'static str, } // deref_into_dyn_supertrait.rs diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index d67a00619dd09..d5a0c1a6bf12d 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -1,10 +1,11 @@ use crate::context::LintContext; -use crate::lints::NoopMethodCallDiag; +use crate::lints::{CloneDoubleRef, NoopMethodCallDiag}; use crate::LateContext; use crate::LateLintPass; use rustc_hir::def::DefKind; use rustc_hir::{Expr, ExprKind}; use rustc_middle::ty; +use rustc_middle::ty::adjustment::Adjust; use rustc_span::symbol::sym; declare_lint! { @@ -31,18 +32,32 @@ declare_lint! { /// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything /// as references are copy. This lint detects these calls and warns the user about them. pub NOOP_METHOD_CALL, - Allow, + Warn, "detects the use of well-known noop methods" } -declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL]); +declare_lint! { + /// The `clone_double_ref` lint checks for usage of `.clone()` on an `&&T`, + /// which copies the inner `&T`, instead of cloning the underlying `T` and + /// can be confusing. + pub CLONE_DOUBLE_REF, + Warn, + "using `clone` on `&&T`" +} + +declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL, CLONE_DOUBLE_REF]); impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { // We only care about method calls. - let ExprKind::MethodCall(call, receiver, ..) = &expr.kind else { - return + let ExprKind::MethodCall(call, receiver, _, call_span) = &expr.kind else { + return; }; + + if call_span.from_expansion() { + return; + } + // We only care about method calls corresponding to the `Clone`, `Deref` and `Borrow` // traits and ignore any other method call. let did = match cx.typeck_results().type_dependent_def(expr.hir_id) { @@ -70,25 +85,40 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { }; // (Re)check that it implements the noop diagnostic. let Some(name) = cx.tcx.get_diagnostic_name(i.def_id()) else { return }; - if !matches!( - name, - sym::noop_method_borrow | sym::noop_method_clone | sym::noop_method_deref - ) { - return; - } + + let op = match name { + sym::noop_method_borrow => "borrowing", + sym::noop_method_clone => "cloning", + sym::noop_method_deref => "dereferencing", + _ => return, + }; + let receiver_ty = cx.typeck_results().expr_ty(receiver); let expr_ty = cx.typeck_results().expr_ty_adjusted(expr); - if receiver_ty != expr_ty { - // This lint will only trigger if the receiver type and resulting expression \ - // type are the same, implying that the method call is unnecessary. + + let arg_adjustments = cx.typeck_results().expr_adjustments(receiver); + + // If there is any user defined auto-deref step, then we don't want to warn. + // https://github.com/rust-lang/rust-clippy/issues/9272 + if arg_adjustments.iter().any(|adj| matches!(adj.kind, Adjust::Deref(Some(_)))) { return; } + let expr_span = expr.span; let span = expr_span.with_lo(receiver.span.hi()); - cx.emit_spanned_lint( - NOOP_METHOD_CALL, - span, - NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span }, - ); + + if receiver_ty == expr_ty { + cx.emit_spanned_lint( + NOOP_METHOD_CALL, + span, + NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span }, + ); + } else { + cx.emit_spanned_lint( + CLONE_DOUBLE_REF, + span, + CloneDoubleRef { call: call.ident.name, ty: expr_ty, op }, + ) + } } } diff --git a/library/alloc/src/str.rs b/library/alloc/src/str.rs index b87ef59f64a3b..a3b7d6e8df714 100644 --- a/library/alloc/src/str.rs +++ b/library/alloc/src/str.rs @@ -89,7 +89,7 @@ macro_rules! specialize_for_lengths { $num => { for s in iter { copy_slice_and_advance!(target, sep_bytes); - let content_bytes = s.borrow().as_ref(); + let content_bytes = s.as_ref(); copy_slice_and_advance!(target, content_bytes); } }, @@ -98,7 +98,7 @@ macro_rules! specialize_for_lengths { // arbitrary non-zero size fallback for s in iter { copy_slice_and_advance!(target, sep_bytes); - let content_bytes = s.borrow().as_ref(); + let content_bytes = s.as_ref(); copy_slice_and_advance!(target, content_bytes); } } diff --git a/tests/ui/issues/issue-11820.rs b/tests/ui/issues/issue-11820.rs index 7ffe9652797cf..dc6349b10ee58 100644 --- a/tests/ui/issues/issue-11820.rs +++ b/tests/ui/issues/issue-11820.rs @@ -1,6 +1,8 @@ // run-pass // pretty-expanded FIXME #23616 +#![allow(noop_method_call)] + struct NoClone; fn main() { diff --git a/tests/ui/lint/clone-double-ref.rs b/tests/ui/lint/clone-double-ref.rs index 45dd91565da06..f68dfed279dec 100644 --- a/tests/ui/lint/clone-double-ref.rs +++ b/tests/ui/lint/clone-double-ref.rs @@ -1,11 +1,11 @@ -#![feature(once_cell)] -#![deny(clone_double_ref)] +#![feature(lazy_cell)] +#![deny(clone_double_ref, noop_method_call)] pub fn clone_on_double_ref() { let x = vec![1]; let y = &&x; let z: &Vec<_> = y.clone(); - //~^ ERROR using `clone` on a double-reference, which copies the reference of type `Vec` + //~^ ERROR using `.clone()` on a double reference, which copies `&Vec` println!("{:p} {:p}", *y, z); } @@ -22,7 +22,9 @@ fn rust_clippy_issue_9272() { fn check(mut encoded: &[u8]) { let _ = &mut encoded.clone(); + //~^ ERROR call to `.clone()` on a reference in this situation does nothing let _ = &encoded.clone(); + //~^ ERROR call to `.clone()` on a reference in this situation does nothing } fn main() {} diff --git a/tests/ui/lint/clone-double-ref.stderr b/tests/ui/lint/clone-double-ref.stderr index 9276433b7429b..4d78cbabb7c71 100644 --- a/tests/ui/lint/clone-double-ref.stderr +++ b/tests/ui/lint/clone-double-ref.stderr @@ -1,22 +1,35 @@ -error: using `clone` on a double-reference, which copies the reference of type `Vec` instead of cloning the inner type - --> $DIR/clone-double-ref.rs:7:22 +error: using `.clone()` on a double reference, which copies `&Vec` instead of cloning the inner type + --> $DIR/clone-double-ref.rs:7:23 | LL | let z: &Vec<_> = y.clone(); - | ^^^^^^^^^ + | ^^^^^^^^ | note: the lint level is defined here --> $DIR/clone-double-ref.rs:2:9 | -LL | #![deny(clone_double_ref)] +LL | #![deny(clone_double_ref, noop_method_call)] | ^^^^^^^^^^^^^^^^ -help: try dereferencing it + +error: call to `.clone()` on a reference in this situation does nothing + --> $DIR/clone-double-ref.rs:24:25 + | +LL | let _ = &mut encoded.clone(); + | ^^^^^^^^ unnecessary method call + | + = note: the type `&[u8]` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed +note: the lint level is defined here + --> $DIR/clone-double-ref.rs:2:27 + | +LL | #![deny(clone_double_ref, noop_method_call)] + | ^^^^^^^^^^^^^^^^ + +error: call to `.clone()` on a reference in this situation does nothing + --> $DIR/clone-double-ref.rs:26:21 | -LL | let z: &Vec<_> = &(*y).clone(); - | +++ ~~~~~~~~~ -help: or try being explicit if you are sure, that you want to clone a reference +LL | let _ = &encoded.clone(); + | ^^^^^^^^ unnecessary method call | -LL | let z: &Vec<_> = <&Vec>::clone(y); - | +++++++++++++++++++ ~ + = note: the type `&[u8]` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed -error: aborting due to previous error +error: aborting due to 3 previous errors diff --git a/tests/ui/lint/noop-method-call.rs b/tests/ui/lint/noop-method-call.rs index e52dea2e32168..c624ff3cf17d5 100644 --- a/tests/ui/lint/noop-method-call.rs +++ b/tests/ui/lint/noop-method-call.rs @@ -15,24 +15,21 @@ fn main() { let non_clone_type_ref = &PlainType(1u32); let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clone(); //~^ WARNING call to `.clone()` on a reference in this situation does nothing - //~| WARNING using `clone` on a double-reference, which copies the reference of type `PlainType` let clone_type_ref = &CloneType(1u32); let clone_type_ref_clone: CloneType = clone_type_ref.clone(); - // Calling clone on a double reference doesn't warn since the method call itself - // peels the outer reference off let clone_type_ref = &&CloneType(1u32); let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); - //~^ WARNING using `clone` on a double-reference, which copies the reference of type `CloneType` + //~^ WARNING using `.clone()` on a double reference, which copies `&CloneType` let non_deref_type = &PlainType(1u32); let non_deref_type_deref: &PlainType = non_deref_type.deref(); //~^ WARNING call to `.deref()` on a reference in this situation does nothing - // Dereferencing a &&T does not warn since it has collapsed the double reference let non_deref_type = &&PlainType(1u32); let non_deref_type_deref: &PlainType = non_deref_type.deref(); + //~^ WARNING using `.deref()` on a double reference, which copies `&PlainType` let non_borrow_type = &PlainType(1u32); let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); @@ -44,7 +41,7 @@ fn main() { let xs = ["a", "b", "c"]; let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead - //~^ WARNING using `clone` on a double-reference, which copies the reference of type `str` + //~^ WARNING using `.clone()` on a double reference, which copies `&str` } fn generic(non_clone_type: &PlainType) { diff --git a/tests/ui/lint/noop-method-call.stderr b/tests/ui/lint/noop-method-call.stderr index 6a904d01abc8e..22e92323bd249 100644 --- a/tests/ui/lint/noop-method-call.stderr +++ b/tests/ui/lint/noop-method-call.stderr @@ -8,25 +8,49 @@ LL | let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clon note: the lint level is defined here --> $DIR/noop-method-call.rs:4:9 | -LL | #![warn(noop_method_call)] +LL | #![warn(noop_method_call, clone_double_ref)] | ^^^^^^^^^^^^^^^^ +warning: using `.clone()` on a double reference, which copies `&CloneType` instead of cloning the inner type + --> $DIR/noop-method-call.rs:23:63 + | +LL | let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); + | ^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/noop-method-call.rs:4:27 + | +LL | #![warn(noop_method_call, clone_double_ref)] + | ^^^^^^^^^^^^^^^^ + warning: call to `.deref()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:28:63 + --> $DIR/noop-method-call.rs:27:63 | LL | let non_deref_type_deref: &PlainType = non_deref_type.deref(); | ^^^^^^^^ unnecessary method call | = note: the type `&PlainType` which `deref` is being called on is the same as the type returned from `deref`, so the method call does not do anything and can be removed +warning: using `.deref()` on a double reference, which copies `&PlainType` instead of dereferencing the inner type + --> $DIR/noop-method-call.rs:31:63 + | +LL | let non_deref_type_deref: &PlainType = non_deref_type.deref(); + | ^^^^^^^^ + warning: call to `.borrow()` on a reference in this situation does nothing - --> $DIR/noop-method-call.rs:36:66 + --> $DIR/noop-method-call.rs:35:66 | LL | let non_borrow_type_borrow: &PlainType = non_borrow_type.borrow(); | ^^^^^^^^^ unnecessary method call | = note: the type `&PlainType` which `borrow` is being called on is the same as the type returned from `borrow`, so the method call does not do anything and can be removed +warning: using `.clone()` on a double reference, which copies `&str` instead of cloning the inner type + --> $DIR/noop-method-call.rs:43:44 + | +LL | let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // ok, but could use `*x` instead + | ^^^^^^^^ + warning: call to `.clone()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:48:19 | @@ -43,5 +67,5 @@ LL | non_clone_type.clone(); | = note: the type `&PlainType` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed -warning: 5 warnings emitted +warning: 8 warnings emitted diff --git a/tests/ui/underscore-imports/cycle.rs b/tests/ui/underscore-imports/cycle.rs index bacf9b2d5a96a..5452b38e97953 100644 --- a/tests/ui/underscore-imports/cycle.rs +++ b/tests/ui/underscore-imports/cycle.rs @@ -1,6 +1,7 @@ // Check that cyclic glob imports are allowed with underscore imports // check-pass +#![allow(noop_method_call)] mod x { pub use crate::y::*; diff --git a/tests/ui/underscore-imports/hygiene.rs b/tests/ui/underscore-imports/hygiene.rs index c4db65245386f..7795ccb797199 100644 --- a/tests/ui/underscore-imports/hygiene.rs +++ b/tests/ui/underscore-imports/hygiene.rs @@ -3,6 +3,7 @@ // check-pass #![feature(decl_macro)] +#![allow(noop_method_call)] mod x { pub use std::ops::Deref as _; diff --git a/tests/ui/underscore-imports/macro-expanded.rs b/tests/ui/underscore-imports/macro-expanded.rs index 43f527bc9a408..55e86e848558d 100644 --- a/tests/ui/underscore-imports/macro-expanded.rs +++ b/tests/ui/underscore-imports/macro-expanded.rs @@ -3,6 +3,7 @@ // check-pass #![feature(decl_macro, rustc_attrs)] +#![allow(noop_method_call)] mod x { pub use std::ops::Not as _; From 82fbdaa9fa8f2f40d583e2f7de3e7a5e06a0d869 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Fri, 7 Apr 2023 17:34:26 +0000 Subject: [PATCH 7/8] rename to suspicious_double_ref_op and fix CI --- compiler/rustc_codegen_ssa/src/back/link.rs | 4 ++-- compiler/rustc_lint/messages.ftl | 2 +- compiler/rustc_lint/src/lints.rs | 2 +- compiler/rustc_lint/src/noop_method_call.rs | 8 ++++---- .../src/traits/error_reporting/suggestions.rs | 3 +-- library/core/benches/iter.rs | 3 +-- library/core/tests/clone.rs | 1 + src/tools/clippy/clippy_lints/src/renamed_lints.rs | 2 +- src/tools/clippy/tests/ui/explicit_deref_methods.fixed | 2 +- src/tools/clippy/tests/ui/explicit_deref_methods.rs | 2 +- src/tools/clippy/tests/ui/rename.fixed | 4 ++-- src/tools/clippy/tests/ui/rename.rs | 2 +- src/tools/clippy/tests/ui/rename.stderr | 4 ++-- src/tools/compiletest/src/main.rs | 2 +- tests/ui/lint/clone-double-ref.rs | 2 +- tests/ui/lint/clone-double-ref.stderr | 10 +++++----- tests/ui/lint/noop-method-call.rs | 2 +- tests/ui/lint/noop-method-call.stderr | 6 +++--- tests/ui/trait-bounds/issue-94680.rs | 2 +- .../ui/trivial-bounds/issue-73021-impossible-inline.rs | 2 +- 20 files changed, 32 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 02e21e74fadc8..27e6144681a16 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -573,7 +573,7 @@ fn link_dwarf_object<'a>( impl ThorinSession { fn alloc_mmap(&self, data: Mmap) -> &Mmap { - (*self.arena_mmap.alloc(data)).borrow() + &*self.arena_mmap.alloc(data) } } @@ -583,7 +583,7 @@ fn link_dwarf_object<'a>( } fn alloc_relocation(&self, data: Relocations) -> &Relocations { - (*self.arena_relocations.alloc(data)).borrow() + &*self.arena_relocations.alloc(data) } fn read_input(&self, path: &Path) -> std::io::Result<&[u8]> { diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index b56f2e32d7956..6e186ddc0b374 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -5,7 +5,7 @@ lint_array_into_iter = .use_explicit_into_iter_suggestion = or use `IntoIterator::into_iter(..)` instead of `.into_iter()` to explicitly iterate by value -lint_clone_double_ref = +lint_suspicious_double_ref_op = using `.{$call}()` on a double reference, which copies `{$ty}` instead of {$op} the inner type lint_enum_intrinsics_mem_discriminant = diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index d6480d78cbc5a..be8fd4cabbd70 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -565,7 +565,7 @@ pub struct BuiltinUnexpectedCliConfigValue { } #[derive(LintDiagnostic)] -#[diag(lint_clone_double_ref)] +#[diag(lint_suspicious_double_ref_op)] pub struct CloneDoubleRef<'a> { pub call: Symbol, pub ty: Ty<'a>, diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index d5a0c1a6bf12d..1233b27817a94 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -37,15 +37,15 @@ declare_lint! { } declare_lint! { - /// The `clone_double_ref` lint checks for usage of `.clone()` on an `&&T`, + /// The `suspicious_double_ref_op` lint checks for usage of `.clone()` on an `&&T`, /// which copies the inner `&T`, instead of cloning the underlying `T` and /// can be confusing. - pub CLONE_DOUBLE_REF, + pub SUSPICIOUS_DOUBLE_REF_OP, Warn, "using `clone` on `&&T`" } -declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL, CLONE_DOUBLE_REF]); +declare_lint_pass!(NoopMethodCall => [NOOP_METHOD_CALL, SUSPICIOUS_DOUBLE_REF_OP]); impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { @@ -115,7 +115,7 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall { ); } else { cx.emit_spanned_lint( - CLONE_DOUBLE_REF, + SUSPICIOUS_DOUBLE_REF_OP, span, CloneDoubleRef { call: call.ident.name, ty: expr_ty, op }, ) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index fb75ec7672920..8b22ad10e6eba 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -39,7 +39,6 @@ use rustc_span::def_id::LocalDefId; use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span, DUMMY_SP}; use rustc_target::spec::abi; -use std::ops::Deref; use super::method_chain::CollectAllMismatches; use super::InferCtxtPrivExt; @@ -3571,7 +3570,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> { // trait_pred `S: Sum<::Item>` and predicate `i32: Sum<&()>` let mut type_diffs = vec![]; - if let ObligationCauseCode::ExprBindingObligation(def_id, _, _, idx) = parent_code.deref() + if let ObligationCauseCode::ExprBindingObligation(def_id, _, _, idx) = parent_code && let Some(node_substs) = typeck_results.node_substs_opt(call_hir_id) && let where_clauses = self.tcx.predicates_of(def_id).instantiate(self.tcx, node_substs) && let Some(where_pred) = where_clauses.predicates.get(*idx) diff --git a/library/core/benches/iter.rs b/library/core/benches/iter.rs index 9193c79bee875..a20ab52d38d1c 100644 --- a/library/core/benches/iter.rs +++ b/library/core/benches/iter.rs @@ -1,4 +1,3 @@ -use core::borrow::Borrow; use core::iter::*; use core::mem; use core::num::Wrapping; @@ -428,7 +427,7 @@ fn bench_trusted_random_access_chunks(b: &mut Bencher) { black_box(&v) .iter() // this shows that we're not relying on the slice::Iter specialization in Copied - .map(|b| *b.borrow()) + .map(|b| *b) .array_chunks::<{ mem::size_of::() }>() .map(|ary| { let d = u64::from_ne_bytes(ary); diff --git a/library/core/tests/clone.rs b/library/core/tests/clone.rs index 33ca9f2c6a3a1..aafe5ced2e97f 100644 --- a/library/core/tests/clone.rs +++ b/library/core/tests/clone.rs @@ -1,4 +1,5 @@ #[test] +#[cfg_attr(not(bootstrap), allow(suspicious_double_ref_op))] fn test_borrowed_clone() { let x = 5; let y: &i32 = &x; diff --git a/src/tools/clippy/clippy_lints/src/renamed_lints.rs b/src/tools/clippy/clippy_lints/src/renamed_lints.rs index 2410a2ebe50cd..5e81a01a461ab 100644 --- a/src/tools/clippy/clippy_lints/src/renamed_lints.rs +++ b/src/tools/clippy/clippy_lints/src/renamed_lints.rs @@ -30,7 +30,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::stutter", "clippy::module_name_repetitions"), ("clippy::to_string_in_display", "clippy::recursive_format_impl"), ("clippy::zero_width_space", "clippy::invisible_characters"), - ("clippy::clone_double_ref", "clone_double_ref"), + ("clippy::clone_double_ref", "suspicious_double_ref_op"), ("clippy::drop_bounds", "drop_bounds"), ("clippy::for_loop_over_option", "for_loops_over_fallibles"), ("clippy::for_loop_over_result", "for_loops_over_fallibles"), diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.fixed b/src/tools/clippy/tests/ui/explicit_deref_methods.fixed index bef4437c131c3..7f1f15e934a82 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.fixed +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.fixed @@ -2,8 +2,8 @@ #![warn(clippy::explicit_deref_methods)] #![allow(unused_variables)] #![allow( + noop_method_call, clippy::borrow_deref_ref, - clone_double_ref, clippy::explicit_auto_deref, clippy::needless_borrow, clippy::uninlined_format_args diff --git a/src/tools/clippy/tests/ui/explicit_deref_methods.rs b/src/tools/clippy/tests/ui/explicit_deref_methods.rs index ffbdbb8d73bf2..feaff117f70a1 100644 --- a/src/tools/clippy/tests/ui/explicit_deref_methods.rs +++ b/src/tools/clippy/tests/ui/explicit_deref_methods.rs @@ -2,8 +2,8 @@ #![warn(clippy::explicit_deref_methods)] #![allow(unused_variables)] #![allow( + noop_method_call, clippy::borrow_deref_ref, - clone_double_ref, clippy::explicit_auto_deref, clippy::needless_borrow, clippy::uninlined_format_args diff --git a/src/tools/clippy/tests/ui/rename.fixed b/src/tools/clippy/tests/ui/rename.fixed index 1ceb60dba2f79..6c07390887df2 100644 --- a/src/tools/clippy/tests/ui/rename.fixed +++ b/src/tools/clippy/tests/ui/rename.fixed @@ -27,7 +27,6 @@ #![allow(clippy::module_name_repetitions)] #![allow(clippy::recursive_format_impl)] #![allow(clippy::invisible_characters)] -#![allow(clone_double_ref)] #![allow(drop_bounds)] #![allow(for_loops_over_fallibles)] #![allow(array_into_iter)] @@ -37,6 +36,7 @@ #![allow(enum_intrinsics_non_enums)] #![allow(non_fmt_panics)] #![allow(named_arguments_used_positionally)] +#![allow(suspicious_double_ref_op)] #![allow(temporary_cstring_as_ptr)] #![allow(unknown_lints)] #![allow(unused_labels)] @@ -68,7 +68,7 @@ #![warn(clippy::module_name_repetitions)] #![warn(clippy::recursive_format_impl)] #![warn(clippy::invisible_characters)] -#![warn(clone_double_ref)] +#![warn(suspicious_double_ref_op)] #![warn(drop_bounds)] #![warn(for_loops_over_fallibles)] #![warn(for_loops_over_fallibles)] diff --git a/src/tools/clippy/tests/ui/rename.rs b/src/tools/clippy/tests/ui/rename.rs index 81a01b7fb3a54..082641798a88a 100644 --- a/src/tools/clippy/tests/ui/rename.rs +++ b/src/tools/clippy/tests/ui/rename.rs @@ -27,7 +27,6 @@ #![allow(clippy::module_name_repetitions)] #![allow(clippy::recursive_format_impl)] #![allow(clippy::invisible_characters)] -#![allow(clone_double_ref)] #![allow(drop_bounds)] #![allow(for_loops_over_fallibles)] #![allow(array_into_iter)] @@ -37,6 +36,7 @@ #![allow(enum_intrinsics_non_enums)] #![allow(non_fmt_panics)] #![allow(named_arguments_used_positionally)] +#![allow(suspicious_double_ref_op)] #![allow(temporary_cstring_as_ptr)] #![allow(unknown_lints)] #![allow(unused_labels)] diff --git a/src/tools/clippy/tests/ui/rename.stderr b/src/tools/clippy/tests/ui/rename.stderr index 2fe5e320c67c6..70d15408b9fc1 100644 --- a/src/tools/clippy/tests/ui/rename.stderr +++ b/src/tools/clippy/tests/ui/rename.stderr @@ -168,11 +168,11 @@ error: lint `clippy::zero_width_space` has been renamed to `clippy::invisible_ch LL | #![warn(clippy::zero_width_space)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::invisible_characters` -error: lint `clippy::clone_double_ref` has been renamed to `clone_double_ref` +error: lint `clippy::clone_double_ref` has been renamed to `suspicious_double_ref_op` --> $DIR/rename.rs:71:9 | LL | #![warn(clippy::clone_double_ref)] - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clone_double_ref` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `suspicious_double_ref_op` error: lint `clippy::drop_bounds` has been renamed to `drop_bounds` --> $DIR/rename.rs:72:9 diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 6a91d25a82436..a175fdc24aa43 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -1114,7 +1114,7 @@ fn check_overlapping_tests(found_paths: &BTreeSet) { for path in found_paths { for ancestor in path.ancestors().skip(1) { if found_paths.contains(ancestor) { - collisions.push((path, ancestor.clone())); + collisions.push((path, ancestor)); } } } diff --git a/tests/ui/lint/clone-double-ref.rs b/tests/ui/lint/clone-double-ref.rs index f68dfed279dec..1dec96a589157 100644 --- a/tests/ui/lint/clone-double-ref.rs +++ b/tests/ui/lint/clone-double-ref.rs @@ -1,5 +1,5 @@ #![feature(lazy_cell)] -#![deny(clone_double_ref, noop_method_call)] +#![deny(suspicious_double_ref_op, noop_method_call)] pub fn clone_on_double_ref() { let x = vec![1]; diff --git a/tests/ui/lint/clone-double-ref.stderr b/tests/ui/lint/clone-double-ref.stderr index 4d78cbabb7c71..cdc1d429ab483 100644 --- a/tests/ui/lint/clone-double-ref.stderr +++ b/tests/ui/lint/clone-double-ref.stderr @@ -7,8 +7,8 @@ LL | let z: &Vec<_> = y.clone(); note: the lint level is defined here --> $DIR/clone-double-ref.rs:2:9 | -LL | #![deny(clone_double_ref, noop_method_call)] - | ^^^^^^^^^^^^^^^^ +LL | #![deny(suspicious_double_ref_op, noop_method_call)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ error: call to `.clone()` on a reference in this situation does nothing --> $DIR/clone-double-ref.rs:24:25 @@ -18,10 +18,10 @@ LL | let _ = &mut encoded.clone(); | = note: the type `&[u8]` which `clone` is being called on is the same as the type returned from `clone`, so the method call does not do anything and can be removed note: the lint level is defined here - --> $DIR/clone-double-ref.rs:2:27 + --> $DIR/clone-double-ref.rs:2:35 | -LL | #![deny(clone_double_ref, noop_method_call)] - | ^^^^^^^^^^^^^^^^ +LL | #![deny(suspicious_double_ref_op, noop_method_call)] + | ^^^^^^^^^^^^^^^^ error: call to `.clone()` on a reference in this situation does nothing --> $DIR/clone-double-ref.rs:26:21 diff --git a/tests/ui/lint/noop-method-call.rs b/tests/ui/lint/noop-method-call.rs index c624ff3cf17d5..a0435d8322161 100644 --- a/tests/ui/lint/noop-method-call.rs +++ b/tests/ui/lint/noop-method-call.rs @@ -1,7 +1,7 @@ // check-pass #![allow(unused)] -#![warn(noop_method_call, clone_double_ref)] +#![warn(noop_method_call, suspicious_double_ref_op)] use std::borrow::Borrow; use std::ops::Deref; diff --git a/tests/ui/lint/noop-method-call.stderr b/tests/ui/lint/noop-method-call.stderr index 22e92323bd249..9261e70b22a0e 100644 --- a/tests/ui/lint/noop-method-call.stderr +++ b/tests/ui/lint/noop-method-call.stderr @@ -8,7 +8,7 @@ LL | let non_clone_type_ref_clone: &PlainType = non_clone_type_ref.clon note: the lint level is defined here --> $DIR/noop-method-call.rs:4:9 | -LL | #![warn(noop_method_call, clone_double_ref)] +LL | #![warn(noop_method_call, suspicious_double_ref_op)] | ^^^^^^^^^^^^^^^^ warning: using `.clone()` on a double reference, which copies `&CloneType` instead of cloning the inner type @@ -20,8 +20,8 @@ LL | let clone_type_ref_clone: &CloneType = clone_type_ref.clone(); note: the lint level is defined here --> $DIR/noop-method-call.rs:4:27 | -LL | #![warn(noop_method_call, clone_double_ref)] - | ^^^^^^^^^^^^^^^^ +LL | #![warn(noop_method_call, suspicious_double_ref_op)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ warning: call to `.deref()` on a reference in this situation does nothing --> $DIR/noop-method-call.rs:27:63 diff --git a/tests/ui/trait-bounds/issue-94680.rs b/tests/ui/trait-bounds/issue-94680.rs index 7095be08afa65..3b41dc8fd91c2 100644 --- a/tests/ui/trait-bounds/issue-94680.rs +++ b/tests/ui/trait-bounds/issue-94680.rs @@ -1,6 +1,6 @@ // check-pass -#![allow(clone_double_ref)] +#![allow(suspicious_double_ref_op)] fn main() { println!("{:?}", { diff --git a/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs b/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs index a09bf4219fe90..d933434dc164f 100644 --- a/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs +++ b/tests/ui/trivial-bounds/issue-73021-impossible-inline.rs @@ -2,7 +2,7 @@ // revisions: no-opt inline // [inline]compile-flags: -Zmir-opt-level=3 --emit=mir #![feature(trivial_bounds)] -#![allow(unused, clone_double_ref)] +#![allow(unused, suspicious_double_ref_op)] trait Foo { fn test(&self); From dc7f05544e63ce25529d59933868165a044cc2d1 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sat, 8 Apr 2023 09:10:14 +0000 Subject: [PATCH 8/8] add example, adjust --- compiler/rustc_lint/src/noop_method_call.rs | 23 +++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 1233b27817a94..e39df24d44764 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -16,7 +16,6 @@ declare_lint! { /// /// ```rust /// # #![allow(unused)] - /// #![warn(noop_method_call)] /// struct Foo; /// let foo = &Foo; /// let clone: &Foo = foo.clone(); @@ -37,9 +36,25 @@ declare_lint! { } declare_lint! { - /// The `suspicious_double_ref_op` lint checks for usage of `.clone()` on an `&&T`, - /// which copies the inner `&T`, instead of cloning the underlying `T` and - /// can be confusing. + /// The `suspicious_double_ref_op` lint checks for usage of `.clone()`/`.borrow()`/`.deref()` + /// on an `&&T` when `T: !Deref/Borrow/Clone`, which means the call will copy the inner `&T`, + /// instead of performing the operation on the underlying `T` and can be confusing. + /// + /// ### Example + /// + /// ```rust + /// # #![allow(unused)] + /// struct Foo; + /// let foo = &&Foo; + /// let clone: &Foo = foo.clone(); + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Since `Foo` doesn't implement `Clone`, running `.clone()` only dereferences the double + /// reference, instead of cloning the inner type which should be what was intended. pub SUSPICIOUS_DOUBLE_REF_OP, Warn, "using `clone` on `&&T`"