From 6d4dc35882a55610e9da81237d62bea1c0c1634e Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 17 Apr 2021 20:35:39 -0400 Subject: [PATCH] Improve `needless_borrow` lint Suggest changing usages of ref bindings to match the new type Split out some cases into new lint `ref_binding_to_reference` --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 4 +- clippy_lints/src/needless_borrow.rs | 205 ++++++++++++++++++++--- clippy_utils/src/higher.rs | 2 +- clippy_utils/src/lib.rs | 4 +- clippy_utils/src/visitors.rs | 37 ++-- tests/ui/needless_borrow.fixed | 17 -- tests/ui/needless_borrow.rs | 17 -- tests/ui/needless_borrow.stderr | 16 +- tests/ui/needless_borrow_pat.rs | 151 +++++++++++++++++ tests/ui/needless_borrow_pat.stderr | 112 +++++++++++++ tests/ui/ref_binding_to_reference.rs | 76 +++++++++ tests/ui/ref_binding_to_reference.stderr | 88 ++++++++++ 13 files changed, 627 insertions(+), 103 deletions(-) create mode 100644 tests/ui/needless_borrow_pat.rs create mode 100644 tests/ui/needless_borrow_pat.stderr create mode 100644 tests/ui/ref_binding_to_reference.rs create mode 100644 tests/ui/ref_binding_to_reference.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index da5a0712c..abfe7f91f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2622,6 +2622,7 @@ Released 2018-09-13 [`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate [`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes +[`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5cd7d5f14..a263e8b0c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -841,6 +841,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: needless_bool::BOOL_COMPARISON, needless_bool::NEEDLESS_BOOL, needless_borrow::NEEDLESS_BORROW, + needless_borrow::REF_BINDING_TO_REFERENCE, needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE, needless_continue::NEEDLESS_CONTINUE, needless_for_each::NEEDLESS_FOR_EACH, @@ -1116,6 +1117,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(misc_early::UNSEPARATED_LITERAL_SUFFIX), LintId::of(mut_mut::MUT_MUT), LintId::of(needless_bitwise_bool::NEEDLESS_BITWISE_BOOL), + LintId::of(needless_borrow::REF_BINDING_TO_REFERENCE), LintId::of(needless_continue::NEEDLESS_CONTINUE), LintId::of(needless_for_each::NEEDLESS_FOR_EACH), LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE), @@ -1890,7 +1892,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box zero_div_zero::ZeroDiv); store.register_late_pass(|| box mutex_atomic::Mutex); store.register_late_pass(|| box needless_update::NeedlessUpdate); - store.register_late_pass(|| box needless_borrow::NeedlessBorrow); + store.register_late_pass(|| box needless_borrow::NeedlessBorrow::default()); store.register_late_pass(|| box needless_borrowed_ref::NeedlessBorrowedRef); store.register_late_pass(|| box no_effect::NoEffect); store.register_late_pass(|| box temporary_assignment::TemporaryAssignment); diff --git a/clippy_lints/src/needless_borrow.rs b/clippy_lints/src/needless_borrow.rs index 3adbbfb8e..965b13177 100644 --- a/clippy_lints/src/needless_borrow.rs +++ b/clippy_lints/src/needless_borrow.rs @@ -3,14 +3,18 @@ //! This lint is **warn** by default use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::{snippet_opt, snippet_with_applicability, snippet_with_context}; +use clippy_utils::{get_parent_expr, in_macro, path_to_local}; use if_chain::if_chain; +use rustc_ast::util::parser::PREC_POSTFIX; +use rustc_data_structures::fx::FxIndexMap; use rustc_errors::Applicability; -use rustc_hir::{BindingAnnotation, BorrowKind, Expr, ExprKind, Mutability, Pat, PatKind}; +use rustc_hir::{BindingAnnotation, Body, BodyId, BorrowKind, Expr, ExprKind, HirId, Mutability, Pat, PatKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_middle::ty::adjustment::{Adjust, Adjustment}; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; declare_clippy_lint! { /// **What it does:** Checks for address of operations (`&`) that are going to @@ -34,13 +38,65 @@ declare_clippy_lint! { "taking a reference that is going to be automatically dereferenced" } -#[derive(Default)] -pub struct NeedlessBorrow; +declare_clippy_lint! { + /// **What it does:** Checks for `ref` bindings which create a reference to a reference. + /// + /// **Why is this bad?** The address-of operator at the use site is clearer about the need for a reference. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// // Bad + /// let x = Some(""); + /// if let Some(ref x) = x { + /// // use `x` here + /// } + /// + /// // Good + /// let x = Some(""); + /// if let Some(x) = x { + /// // use `&x` here + /// } + /// ``` + pub REF_BINDING_TO_REFERENCE, + pedantic, + "`ref` binding to a reference" +} -impl_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW]); +impl_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW, REF_BINDING_TO_REFERENCE]); +#[derive(Default)] +pub struct NeedlessBorrow { + /// The body the first local was found in. Used to emit lints when the traversal of the body has + /// been finished. Note we can't lint at the end of every body as they can be nested within each + /// other. + current_body: Option, + /// The list of locals currently being checked by the lint. + /// If the value is `None`, then the binding has been seen as a ref pattern, but is not linted. + /// This is needed for or patterns where one of the branches can be linted, but another can not + /// be. + /// + /// e.g. `m!(x) | Foo::Bar(ref x)` + ref_locals: FxIndexMap>, +} + +struct RefPat { + /// Whether every usage of the binding is dereferenced. + always_deref: bool, + /// The spans of all the ref bindings for this local. + spans: Vec, + /// The applicability of this suggestion. + app: Applicability, + /// All the replacements which need to be made. + replacements: Vec<(Span, String)>, +} impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if let Some(local) = path_to_local(e) { + self.check_local_usage(cx, e, local); + } + if e.span.from_expansion() { return; } @@ -81,35 +137,132 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow { } } } + fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { - if pat.span.from_expansion() { - return; + if let PatKind::Binding(BindingAnnotation::Ref, id, name, _) = pat.kind { + if let Some(opt_prev_pat) = self.ref_locals.get_mut(&id) { + // This binding id has been seen before. Add this pattern to the list of changes. + if let Some(prev_pat) = opt_prev_pat { + if in_macro(pat.span) { + // Doesn't match the context of the previous pattern. Can't lint here. + *opt_prev_pat = None; + } else { + prev_pat.spans.push(pat.span); + prev_pat.replacements.push(( + pat.span, + snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut prev_pat.app) + .0 + .into(), + )); + } + } + return; + } + + if_chain! { + if !in_macro(pat.span); + if let ty::Ref(_, tam, _) = *cx.typeck_results().pat_ty(pat).kind(); + // only lint immutable refs, because borrowed `&mut T` cannot be moved out + if let ty::Ref(_, _, Mutability::Not) = *tam.kind(); + then { + let mut app = Applicability::MachineApplicable; + let snip = snippet_with_context(cx, name.span, pat.span.ctxt(), "..", &mut app).0; + self.current_body = self.current_body.or(cx.enclosing_body); + self.ref_locals.insert( + id, + Some(RefPat { + always_deref: true, + spans: vec![pat.span], + app, + replacements: vec![(pat.span, snip.into())], + }), + ); + } + } } - if_chain! { - if let PatKind::Binding(BindingAnnotation::Ref, .., name, _) = pat.kind; - if let ty::Ref(_, tam, mutbl) = *cx.typeck_results().pat_ty(pat).kind(); - if mutbl == Mutability::Not; - if let ty::Ref(_, _, mutbl) = *tam.kind(); - // only lint immutable refs, because borrowed `&mut T` cannot be moved out - if mutbl == Mutability::Not; - then { + } + + fn check_body_post(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) { + if Some(body.id()) == self.current_body { + for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) { + let replacements = pat.replacements; + let app = pat.app; span_lint_and_then( cx, - NEEDLESS_BORROW, - pat.span, + if pat.always_deref { + NEEDLESS_BORROW + } else { + REF_BINDING_TO_REFERENCE + }, + pat.spans, "this pattern creates a reference to a reference", |diag| { - if let Some(snippet) = snippet_opt(cx, name.span) { - diag.span_suggestion( - pat.span, - "change this to", - snippet, - Applicability::MachineApplicable, - ); - } - } + diag.multipart_suggestion("try this", replacements, app); + }, ) } + self.current_body = None; + } + } +} +impl NeedlessBorrow { + fn check_local_usage(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, local: HirId) { + if let Some(outer_pat) = self.ref_locals.get_mut(&local) { + if let Some(pat) = outer_pat { + // Check for auto-deref + if !matches!( + cx.typeck_results().expr_adjustments(e), + [ + Adjustment { + kind: Adjust::Deref(_), + .. + }, + Adjustment { + kind: Adjust::Deref(_), + .. + }, + .. + ] + ) { + match get_parent_expr(cx, e) { + // Field accesses are the same no matter the number of references. + Some(Expr { + kind: ExprKind::Field(..), + .. + }) => (), + Some(&Expr { + span, + kind: ExprKind::Unary(UnOp::Deref, _), + .. + }) if !in_macro(span) => { + // Remove explicit deref. + let snip = snippet_with_context(cx, e.span, span.ctxt(), "..", &mut pat.app).0; + pat.replacements.push((span, snip.into())); + }, + Some(parent) if !in_macro(parent.span) => { + // Double reference might be needed at this point. + if parent.precedence().order() == PREC_POSTFIX { + // Parentheses would be needed here, don't lint. + *outer_pat = None; + } else { + pat.always_deref = false; + let snip = snippet_with_context(cx, e.span, parent.span.ctxt(), "..", &mut pat.app).0; + pat.replacements.push((e.span, format!("&{}", snip))); + } + }, + _ if !in_macro(e.span) => { + // Double reference might be needed at this point. + pat.always_deref = false; + let snip = snippet_with_applicability(cx, e.span, "..", &mut pat.app); + pat.replacements.push((e.span, format!("&{}", snip))); + }, + // Edge case for macros. The span of the identifier will usually match the context of the + // binding, but not if the identifier was created in a macro. e.g. `concat_idents` and proc + // macros + _ => *outer_pat = None, + } + } + } } } } diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 0c0e4d3b4..25b449161 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -70,7 +70,7 @@ pub fn range<'a>(expr: &'a hir::Expr<'_>) -> Option> { limits: ast::RangeLimits::Closed, }) }, - hir::ExprKind::Struct(ref path, ref fields, None) => match path { + hir::ExprKind::Struct(path, ref fields, None) => match path { hir::QPath::LangItem(hir::LangItem::RangeFull, _) => Some(Range { start: None, end: None, diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 82250151a..2b9b214da 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1254,12 +1254,12 @@ pub fn match_function_call<'tcx>( path: &[&str], ) -> Option<&'tcx [Expr<'tcx>]> { if_chain! { - if let ExprKind::Call(ref fun, ref args) = expr.kind; + if let ExprKind::Call(ref fun, args) = expr.kind; if let ExprKind::Path(ref qpath) = fun.kind; if let Some(fun_def_id) = cx.qpath_res(qpath, fun.hir_id).opt_def_id(); if match_def_path(cx, fun_def_id, path); then { - return Some(&args) + return Some(args) } }; None diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs index ecdc666b5..73f132eef 100644 --- a/clippy_utils/src/visitors.rs +++ b/clippy_utils/src/visitors.rs @@ -189,34 +189,21 @@ impl<'v> Visitor<'v> for LocalUsedVisitor<'v> { } } +/// A type which can be visited. pub trait Visitable<'tcx> { - fn visit>(self, v: &mut V); + /// Calls the corresponding `visit_*` function on the visitor. + fn visit>(self, visitor: &mut V); } -impl Visitable<'tcx> for &'tcx Expr<'tcx> { - fn visit>(self, v: &mut V) { - v.visit_expr(self) - } -} -impl Visitable<'tcx> for &'tcx Block<'tcx> { - fn visit>(self, v: &mut V) { - v.visit_block(self) - } -} -impl<'tcx> Visitable<'tcx> for &'tcx Stmt<'tcx> { - fn visit>(self, v: &mut V) { - v.visit_stmt(self) - } -} -impl<'tcx> Visitable<'tcx> for &'tcx Body<'tcx> { - fn visit>(self, v: &mut V) { - v.visit_body(self) - } -} -impl<'tcx> Visitable<'tcx> for &'tcx Arm<'tcx> { - fn visit>(self, v: &mut V) { - v.visit_arm(self) - } +macro_rules! visitable_ref { + ($t:ident, $f:ident) => { + impl Visitable<'tcx> for &'tcx $t<'tcx> { + fn visit>(self, visitor: &mut V) { + visitor.$f(self); + } + } + }; } +visitable_ref!(Block, visit_block); /// Calls the given function for each break expression. pub fn visit_break_exprs<'tcx>( diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 5ae4a0e79..a87171dc3 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -18,7 +18,6 @@ fn main() { let vec = Vec::new(); let vec_val = g(&vec); // should not error, because `&Vec` derefs to `&[T]` h(&"foo"); // should not error, because the `&&str` is required, due to `&Trait` - if let Some(cake) = Some(&5) {} let garbl = match 42 { 44 => &a, 45 => { @@ -43,19 +42,3 @@ trait Trait {} impl<'a> Trait for &'a str {} fn h(_: &dyn Trait) {} -#[warn(clippy::needless_borrow)] -#[allow(dead_code)] -fn issue_1432() { - let mut v = Vec::::new(); - let _ = v.iter_mut().filter(|&ref a| a.is_empty()); - let _ = v.iter().filter(|&a| a.is_empty()); - - let _ = v.iter().filter(|&a| a.is_empty()); -} - -#[allow(dead_code)] -#[warn(clippy::needless_borrow)] -#[derive(Debug)] -enum Foo<'a> { - Str(&'a str), -} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 1e281316c..059dc8cea 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -18,7 +18,6 @@ fn main() { let vec = Vec::new(); let vec_val = g(&vec); // should not error, because `&Vec` derefs to `&[T]` h(&"foo"); // should not error, because the `&&str` is required, due to `&Trait` - if let Some(ref cake) = Some(&5) {} let garbl = match 42 { 44 => &a, 45 => { @@ -43,19 +42,3 @@ trait Trait {} impl<'a> Trait for &'a str {} fn h(_: &dyn Trait) {} -#[warn(clippy::needless_borrow)] -#[allow(dead_code)] -fn issue_1432() { - let mut v = Vec::::new(); - let _ = v.iter_mut().filter(|&ref a| a.is_empty()); - let _ = v.iter().filter(|&ref a| a.is_empty()); - - let _ = v.iter().filter(|&a| a.is_empty()); -} - -#[allow(dead_code)] -#[warn(clippy::needless_borrow)] -#[derive(Debug)] -enum Foo<'a> { - Str(&'a str), -} diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index bea4b41b8..6eddf26db 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -6,23 +6,11 @@ LL | let c = x(&&a); | = note: `-D clippy::needless-borrow` implied by `-D warnings` -error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:21:17 - | -LL | if let Some(ref cake) = Some(&5) {} - | ^^^^^^^^ help: change this to: `cake` - error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:28:15 + --> $DIR/needless_borrow.rs:27:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` -error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:51:31 - | -LL | let _ = v.iter().filter(|&ref a| a.is_empty()); - | ^^^^^ help: change this to: `a` - -error: aborting due to 4 previous errors +error: aborting due to 2 previous errors diff --git a/tests/ui/needless_borrow_pat.rs b/tests/ui/needless_borrow_pat.rs new file mode 100644 index 000000000..f09262207 --- /dev/null +++ b/tests/ui/needless_borrow_pat.rs @@ -0,0 +1,151 @@ +// edition:2018 +// FIXME: run-rustfix waiting on multi-span suggestions + +#![warn(clippy::needless_borrow)] +#![allow(clippy::needless_borrowed_reference)] + +fn f1(_: &str) {} +macro_rules! m1 { + ($e:expr) => { + f1($e); + }; +} +macro_rules! m3 { + ($i:ident) => { + Some(ref $i) + }; +} +macro_rules! if_chain { + (if $e:expr; $($rest:tt)*) => { + if $e { + if_chain!($($rest)*) + } + }; + + (if let $p:pat = $e:expr; $($rest:tt)*) => { + if let $p = $e { + if_chain!($($rest)*) + } + }; + + (then $b:block) => { + $b + }; +} + +#[allow(dead_code)] +fn main() { + let x = String::new(); + + // Ok, reference to a String. + let _: &String = match Some(x.clone()) { + Some(ref x) => x, + None => return, + }; + + // Ok, reference to a &mut String + let _: &&mut String = match Some(&mut x.clone()) { + Some(ref x) => x, + None => return, + }; + + // Ok, the pattern is from a macro + let _: &String = match Some(&x) { + m3!(x) => x, + None => return, + }; + + // Err, reference to a &String + let _: &String = match Some(&x) { + Some(ref x) => x, + None => return, + }; + + // Err, reference to a &String. + let _: &String = match Some(&x) { + Some(ref x) => *x, + None => return, + }; + + // Err, reference to a &String + let _: &String = match Some(&x) { + Some(ref x) => { + f1(x); + f1(*x); + x + }, + None => return, + }; + + // Err, reference to a &String + match Some(&x) { + Some(ref x) => m1!(x), + None => return, + }; + + // Err, reference to a &String + let _ = |&ref x: &&String| { + let _: &String = x; + }; + + // Err, reference to a &String + let (ref y,) = (&x,); + let _: &String = *y; + + let y = &&x; + // Ok, different y + let _: &String = *y; + + let x = (0, 0); + // Err, reference to a &u32. Don't suggest adding a reference to the field access. + let _: u32 = match Some(&x) { + Some(ref x) => x.0, + None => return, + }; + + enum E { + A(&'static u32), + B(&'static u32), + } + // Err, reference to &u32. + let _: &u32 = match E::A(&0) { + E::A(ref x) | E::B(ref x) => *x, + }; + + // Err, reference to &String. + if_chain! { + if true; + if let Some(ref x) = Some(&String::new()); + then { + f1(x); + } + } +} + +// Err, reference to a &String +fn f2<'a>(&ref x: &&'a String) -> &'a String { + let _: &String = x; + *x +} + +trait T1 { + // Err, reference to a &String + fn f(&ref x: &&String) { + let _: &String = x; + } +} + +struct S; +impl T1 for S { + // Err, reference to a &String + fn f(&ref x: &&String) { + let _: &String = *x; + } +} + +// Ok - used to error due to rustc bug +#[allow(dead_code)] +#[derive(Debug)] +enum Foo<'a> { + Str(&'a str), +} diff --git a/tests/ui/needless_borrow_pat.stderr b/tests/ui/needless_borrow_pat.stderr new file mode 100644 index 000000000..32913d59f --- /dev/null +++ b/tests/ui/needless_borrow_pat.stderr @@ -0,0 +1,112 @@ +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:60:14 + | +LL | Some(ref x) => x, + | ^^^^^ help: try this: `x` + | + = note: `-D clippy::needless-borrow` implied by `-D warnings` + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:66:14 + | +LL | Some(ref x) => *x, + | ^^^^^ + | +help: try this + | +LL | Some(x) => x, + | ^ ^ + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:72:14 + | +LL | Some(ref x) => { + | ^^^^^ + | +help: try this + | +LL | Some(x) => { +LL | f1(x); +LL | f1(x); + | + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:82:14 + | +LL | Some(ref x) => m1!(x), + | ^^^^^ help: try this: `x` + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:87:15 + | +LL | let _ = |&ref x: &&String| { + | ^^^^^ help: try this: `x` + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:92:10 + | +LL | let (ref y,) = (&x,); + | ^^^^^ + | +help: try this + | +LL | let (y,) = (&x,); +LL | let _: &String = y; + | + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:102:14 + | +LL | Some(ref x) => x.0, + | ^^^^^ help: try this: `x` + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:112:14 + | +LL | E::A(ref x) | E::B(ref x) => *x, + | ^^^^^ ^^^^^ + | +help: try this + | +LL | E::A(x) | E::B(x) => x, + | ^ ^ ^ + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:118:21 + | +LL | if let Some(ref x) = Some(&String::new()); + | ^^^^^ help: try this: `x` + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:126:12 + | +LL | fn f2<'a>(&ref x: &&'a String) -> &'a String { + | ^^^^^ + | +help: try this + | +LL | fn f2<'a>(&x: &&'a String) -> &'a String { +LL | let _: &String = x; +LL | x + | + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:133:11 + | +LL | fn f(&ref x: &&String) { + | ^^^^^ help: try this: `x` + +error: this pattern creates a reference to a reference + --> $DIR/needless_borrow_pat.rs:141:11 + | +LL | fn f(&ref x: &&String) { + | ^^^^^ + | +help: try this + | +LL | fn f(&x: &&String) { +LL | let _: &String = x; + | + +error: aborting due to 12 previous errors + diff --git a/tests/ui/ref_binding_to_reference.rs b/tests/ui/ref_binding_to_reference.rs new file mode 100644 index 000000000..c7235e1c2 --- /dev/null +++ b/tests/ui/ref_binding_to_reference.rs @@ -0,0 +1,76 @@ +// edition:2018 +// FIXME: run-rustfix waiting on multi-span suggestions + +#![warn(clippy::ref_binding_to_reference)] +#![allow(clippy::needless_borrowed_reference)] + +fn f1(_: &str) {} +macro_rules! m2 { + ($e:expr) => { + f1(*$e); + }; +} +macro_rules! m3 { + ($i:ident) => { + Some(ref $i) + }; +} + +#[allow(dead_code)] +fn main() { + let x = String::new(); + + // Ok, the pattern is from a macro + let _: &&String = match Some(&x) { + m3!(x) => x, + None => return, + }; + + // Err, reference to a &String + let _: &&String = match Some(&x) { + Some(ref x) => x, + None => return, + }; + + // Err, reference to a &String + let _: &&String = match Some(&x) { + Some(ref x) => { + f1(x); + f1(*x); + x + }, + None => return, + }; + + // Err, reference to a &String + match Some(&x) { + Some(ref x) => m2!(x), + None => return, + } + + // Err, reference to a &String + let _ = |&ref x: &&String| { + let _: &&String = x; + }; +} + +// Err, reference to a &String +fn f2<'a>(&ref x: &&'a String) -> &'a String { + let _: &&String = x; + *x +} + +trait T1 { + // Err, reference to a &String + fn f(&ref x: &&String) { + let _: &&String = x; + } +} + +struct S; +impl T1 for S { + // Err, reference to a &String + fn f(&ref x: &&String) { + let _: &&String = x; + } +} diff --git a/tests/ui/ref_binding_to_reference.stderr b/tests/ui/ref_binding_to_reference.stderr new file mode 100644 index 000000000..00aeff4fe --- /dev/null +++ b/tests/ui/ref_binding_to_reference.stderr @@ -0,0 +1,88 @@ +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:31:14 + | +LL | Some(ref x) => x, + | ^^^^^ + | + = note: `-D clippy::ref-binding-to-reference` implied by `-D warnings` +help: try this + | +LL | Some(x) => &x, + | ^ ^^ + +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:37:14 + | +LL | Some(ref x) => { + | ^^^^^ + | +help: try this + | +LL | Some(x) => { +LL | f1(x); +LL | f1(x); +LL | &x + | + +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:47:14 + | +LL | Some(ref x) => m2!(x), + | ^^^^^ + | +help: try this + | +LL | Some(x) => m2!(&x), + | ^ ^^ + +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:52:15 + | +LL | let _ = |&ref x: &&String| { + | ^^^^^ + | +help: try this + | +LL | let _ = |&x: &&String| { +LL | let _: &&String = &x; + | + +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:58:12 + | +LL | fn f2<'a>(&ref x: &&'a String) -> &'a String { + | ^^^^^ + | +help: try this + | +LL | fn f2<'a>(&x: &&'a String) -> &'a String { +LL | let _: &&String = &x; +LL | x + | + +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:65:11 + | +LL | fn f(&ref x: &&String) { + | ^^^^^ + | +help: try this + | +LL | fn f(&x: &&String) { +LL | let _: &&String = &x; + | + +error: this pattern creates a reference to a reference + --> $DIR/ref_binding_to_reference.rs:73:11 + | +LL | fn f(&ref x: &&String) { + | ^^^^^ + | +help: try this + | +LL | fn f(&x: &&String) { +LL | let _: &&String = &x; + | + +error: aborting due to 7 previous errors +