mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improve needless_borrow lint.

Browse Source 
* Lint when a borrow is auto dereferenced more than once
* Lint when the expression is used as the expression of a block for a match arm

Moves `needless_borrow` and `ref_binding_to_reference` to `dereference`
lint pass in preperation for `explicit_auto_deref` lint.
This commit is contained in:
Jason Newcomb 2021-11-15 11:58:25 -05:00
parent f51fb341dd
commit 8ded385ddc
11 changed files with 425 additions and 306 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clippy_lints/src/bytecount.rs
View File

@ -74,10 +74,10 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if (p == sym::iter || p == sym!(iter_mut)) && args.len() == 1 {
&args[0]
} else {
&filter_recv
filter_recv
}
} else {
&filter_recv
filter_recv
};
let mut applicability = Applicability::MaybeIncorrect;
span_lint_and_sugg(

335
clippy_lints/src/dereference.rs
View File

@ -1,14 +1,20 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_context;
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::{snippet_with_applicability, snippet_with_context};
use clippy_utils::ty::peel_mid_ty_refs;
use clippy_utils::{get_parent_node, is_lint_allowed};
use rustc_ast::util::parser::PREC_PREFIX;
use clippy_utils::{get_parent_expr, get_parent_node, is_lint_allowed, path_to_local};
use rustc_ast::util::parser::{PREC_POSTFIX, PREC_PREFIX};
use rustc_data_structures::fx::FxIndexMap;
use rustc_errors::Applicability;
use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node, UnOp};
use rustc_hir::{
BindingAnnotation, Body, BodyId, BorrowKind, Destination, Expr, ExprKind, HirId, MatchSource, Mutability, Node,
Pat, PatKind, UnOp,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow};
use rustc_middle::ty::{self, Ty, TyCtxt, TyS, TypeckResults};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{symbol::sym, Span};
use std::iter;
declare_clippy_lint! {
/// ### What it does
@ -40,8 +46,64 @@
"Explicit use of deref or deref_mut method while not in a method chain."
}
declare_clippy_lint! {
/// ### What it does
/// Checks for address of operations (`&`) that are going to
/// be dereferenced immediately by the compiler.
///
/// ### Why is this bad?
/// Suggests that the receiver of the expression borrows
/// the expression.
///
/// ### Example
/// ```rust
/// fn fun(_a: &i32) {}
///
/// // Bad
/// let x: &i32 = &&&&&&5;
/// fun(&x);
///
/// // Good
/// let x: &i32 = &5;
/// fun(x);
/// ```
#[clippy::version = "pre 1.29.0"]
pub NEEDLESS_BORROW,
style,
"taking a reference that is going to be automatically dereferenced"
}
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.
///
/// ### 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
/// }
/// ```
#[clippy::version = "1.54.0"]
pub REF_BINDING_TO_REFERENCE,
pedantic,
"`ref` binding to a reference"
}
impl_lint_pass!(Dereferencing => [
EXPLICIT_DEREF_METHODS,
NEEDLESS_BORROW,
REF_BINDING_TO_REFERENCE,
]);
#[derive(Default)]
@ -52,6 +114,18 @@ pub struct Dereferencing {
// expression. This is to store the id of that expression so it can be skipped when
// `check_expr` is called for it.
skip_expr: Option<HirId>,
/// 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<BodyId>,
/// 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<HirId, Option<RefPat>>,
}
struct StateData {
@ -68,6 +142,9 @@ enum State {
ty_changed_count: usize,
is_final_ufcs: bool,
},
DerefedBorrow {
count: u32,
},
}
// A reference operation considered by this lint pass
@ -77,13 +154,29 @@ enum RefOp {
AddrOf,
}
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<Span>,
/// The applicability of this suggestion.
app: Applicability,
/// All the replacements which need to be made.
replacements: Vec<(Span, String)>,
}
impl<'tcx> LateLintPass<'tcx> for Dereferencing {
#[allow(clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
// Skip path expressions from deref calls. e.g. `Deref::deref(e)`
if Some(expr.hir_id) == self.skip_expr.take() {
return;
}
if let Some(local) = path_to_local(expr) {
self.check_local_usage(cx, expr, local);
}
// Stop processing sub expressions when a macro call is seen
if expr.span.from_expansion() {
if let Some((state, data)) = self.state.take() {
@ -128,6 +221,48 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
},
));
},
RefOp::AddrOf => {
// Find the number of times the borrow is auto-derefed.
let mut iter = find_adjustments(cx.tcx, typeck, expr).iter();
if let Some((i, adjust)) = iter.by_ref().enumerate().find_map(|(i, adjust)| {
if !matches!(adjust.kind, Adjust::Deref(_)) {
Some((i, adjust))
} else if !adjust.target.is_ref() {
// Add one to the number of references found.
Some((i + 1, adjust))
} else {
None
}
}) {
// Found two consecutive derefs. At least one can be removed.
if i > 1 {
let target_mut = iter::once(adjust)
.chain(iter)
.find_map(|adjust| match adjust.kind {
Adjust::Borrow(AutoBorrow::Ref(_, m)) => Some(m.into()),
_ => None,
})
// This default should never happen. Auto-deref always reborrows.
.unwrap_or(Mutability::Not);
self.state = Some((
// Subtract one for the current borrow expression, and one to cover the last
// reference which can't be removed (it's either reborrowed, or needed for
// auto-deref to happen).
State::DerefedBorrow {
count:
// Truncation here would require more than a `u32::MAX` level reference. The compiler
// does not support this.
#[allow(clippy::cast_possible_truncation)]
{ i as u32 - 2 }
},
StateData {
span: expr.span,
target_mut,
},
));
}
}
},
_ => (),
}
},
@ -144,10 +279,80 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
data,
));
},
(Some((State::DerefedBorrow { count }, data)), RefOp::AddrOf) if count != 0 => {
self.state = Some((State::DerefedBorrow { count: count - 1 }, data));
},
(Some((state, data)), _) => report(cx, expr, state, data),
}
}
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
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 pat.span.from_expansion() {
// 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 !pat.span.from_expansion();
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())],
}),
);
}
}
}
}
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,
if pat.always_deref {
NEEDLESS_BORROW
} else {
REF_BINDING_TO_REFERENCE
},
pat.spans,
"this pattern creates a reference to a reference",
|diag| {
diag.multipart_suggestion("try this", replacements, app);
},
);
}
self.current_body = None;
}
}
}
fn try_parse_ref_op(
@ -251,6 +456,48 @@ fn is_linted_explicit_deref_position(parent: Option<Node<'_>>, child_id: HirId,
}
}
/// Adjustments are sometimes made in the parent block rather than the expression itself.
fn find_adjustments(
tcx: TyCtxt<'tcx>,
typeck: &'tcx TypeckResults<'_>,
expr: &'tcx Expr<'_>,
) -> &'tcx [Adjustment<'tcx>] {
let map = tcx.hir();
let mut iter = map.parent_iter(expr.hir_id);
let mut prev = expr;
loop {
match typeck.expr_adjustments(prev) {
[] => (),
a => break a,
};
match iter.next().map(|(_, x)| x) {
Some(Node::Block(_)) => {
if let Some((_, Node::Expr(e))) = iter.next() {
prev = e;
} else {
// This shouldn't happen. Blocks are always contained in an expression.
break &[];
}
},
Some(Node::Expr(&Expr {
kind: ExprKind::Break(Destination { target_id: Ok(id), .. }, _),
..
})) => {
if let Some(Node::Expr(e)) = map.find(id) {
prev = e;
iter = map.parent_iter(id);
} else {
// This shouldn't happen. The destination should exist.
break &[];
}
},
_ => break &[],
}
}
}
#[allow(clippy::needless_pass_by_value)]
fn report(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) {
match state {
@ -301,5 +548,83 @@ fn report(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: Stat
app,
);
},
State::DerefedBorrow { .. } => {
let mut app = Applicability::MachineApplicable;
let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0;
span_lint_and_sugg(
cx,
NEEDLESS_BORROW,
data.span,
&format!(
"this expression borrows a reference (`{}`) that is immediately dereferenced by the compiler",
cx.typeck_results().expr_ty(expr),
),
"change this to",
snip.into(),
app,
);
},
}
}
impl Dereferencing {
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 !span.from_expansion() => {
// 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 !parent.span.from_expansion() => {
// 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 !e.span.from_expansion() => {
// 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,
}
}
}
}
}
}

2
clippy_lints/src/lib.register_all.rs
View File

@ -33,6 +33,7 @@
LintId::of(copies::IFS_SAME_COND),
LintId::of(copies::IF_SAME_THEN_ELSE),
LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
LintId::of(dereference::NEEDLESS_BORROW),
LintId::of(derivable_impls::DERIVABLE_IMPLS),
LintId::of(derive::DERIVE_HASH_XOR_EQ),
LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD),
@ -203,7 +204,6 @@
LintId::of(needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
LintId::of(needless_bool::BOOL_COMPARISON),
LintId::of(needless_bool::NEEDLESS_BOOL),
LintId::of(needless_borrow::NEEDLESS_BORROW),
LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
LintId::of(needless_option_as_deref::NEEDLESS_OPTION_AS_DEREF),
LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),

4
clippy_lints/src/lib.register_lints.rs
View File

@ -92,6 +92,8 @@
default::FIELD_REASSIGN_WITH_DEFAULT,
default_numeric_fallback::DEFAULT_NUMERIC_FALLBACK,
dereference::EXPLICIT_DEREF_METHODS,
dereference::NEEDLESS_BORROW,
dereference::REF_BINDING_TO_REFERENCE,
derivable_impls::DERIVABLE_IMPLS,
derive::DERIVE_HASH_XOR_EQ,
derive::DERIVE_ORD_XOR_PARTIAL_ORD,
@ -356,8 +358,6 @@
needless_bitwise_bool::NEEDLESS_BITWISE_BOOL,
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,

2
clippy_lints/src/lib.register_pedantic.rs
View File

@ -21,6 +21,7 @@
LintId::of(copy_iterator::COPY_ITERATOR),
LintId::of(default::DEFAULT_TRAIT_ACCESS),
LintId::of(dereference::EXPLICIT_DEREF_METHODS),
LintId::of(dereference::REF_BINDING_TO_REFERENCE),
LintId::of(derive::EXPL_IMPL_CLONE_ON_COPY),
LintId::of(derive::UNSAFE_DERIVE_DESERIALIZE),
LintId::of(doc::DOC_MARKDOWN),
@ -68,7 +69,6 @@
LintId::of(misc::USED_UNDERSCORE_BINDING),
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),

2
clippy_lints/src/lib.register_style.rs
View File

@ -15,6 +15,7 @@
LintId::of(collapsible_match::COLLAPSIBLE_MATCH),
LintId::of(comparison_chain::COMPARISON_CHAIN),
LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
LintId::of(dereference::NEEDLESS_BORROW),
LintId::of(doc::MISSING_SAFETY_DOC),
LintId::of(doc::NEEDLESS_DOCTEST_MAIN),
LintId::of(enum_variants::ENUM_VARIANT_NAMES),
@ -81,7 +82,6 @@
LintId::of(misc_early::REDUNDANT_PATTERN),
LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
LintId::of(needless_borrow::NEEDLESS_BORROW),
LintId::of(neg_multiply::NEG_MULTIPLY),
LintId::of(new_without_default::NEW_WITHOUT_DEFAULT),
LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),

2
clippy_lints/src/lib.rs
View File

@ -297,7 +297,6 @@ macro_rules! declare_clippy_lint {
mod needless_arbitrary_self_type;
mod needless_bitwise_bool;
mod needless_bool;
mod needless_borrow;
mod needless_borrowed_ref;
mod needless_continue;
mod needless_for_each;
@ -602,7 +601,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| Box::new(zero_div_zero::ZeroDiv));
store.register_late_pass(|| Box::new(mutex_atomic::Mutex));
store.register_late_pass(|| Box::new(needless_update::NeedlessUpdate));
store.register_late_pass(|| Box::new(needless_borrow::NeedlessBorrow::default()));
store.register_late_pass(|| Box::new(needless_borrowed_ref::NeedlessBorrowedRef));
store.register_late_pass(|| Box::new(no_effect::NoEffect));
store.register_late_pass(|| Box::new(temporary_assignment::TemporaryAssignment));

284
clippy_lints/src/needless_borrow.rs
View File

@ -1,284 +0,0 @@
//! Checks for needless address of operations (`&`)
//!
//! This lint is **warn** by default
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::{snippet_opt, snippet_with_applicability, snippet_with_context};
use clippy_utils::{get_parent_expr, 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, 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
/// be dereferenced immediately by the compiler.
///
/// ### Why is this bad?
/// Suggests that the receiver of the expression borrows
/// the expression.
///
/// ### Example
/// ```rust
/// fn fun(_a: &i32) {}
///
/// // Bad
/// let x: &i32 = &&&&&&5;
/// fun(&x);
///
/// // Good
/// let x: &i32 = &5;
/// fun(x);
/// ```
#[clippy::version = "pre 1.29.0"]
pub NEEDLESS_BORROW,
style,
"taking a reference that is going to be automatically dereferenced"
}
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.
///
/// ### 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
/// }
/// ```
#[clippy::version = "1.54.0"]
pub REF_BINDING_TO_REFERENCE,
pedantic,
"`ref` binding to a reference"
}
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<BodyId>,
/// 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<HirId, Option<RefPat>>,
}
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<Span>,
/// 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;
}
if let ExprKind::AddrOf(BorrowKind::Ref, mutability, inner) = e.kind {
if let ty::Ref(_, ty, _) = cx.typeck_results().expr_ty(inner).kind() {
for adj3 in cx.typeck_results().expr_adjustments(e).windows(3) {
if let [
Adjustment {
kind: Adjust::Deref(_), ..
},
Adjustment {
kind: Adjust::Deref(_), ..
},
Adjustment {
kind: Adjust::Borrow(_),
..
},
] = *adj3
{
let help_msg_ty = if matches!(mutability, Mutability::Not) {
format!("&{}", ty)
} else {
format!("&mut {}", ty)
};
span_lint_and_then(
cx,
NEEDLESS_BORROW,
e.span,
&format!(
"this expression borrows a reference (`{}`) that is immediately dereferenced \
by the compiler",
help_msg_ty
),
|diag| {
if let Some(snippet) = snippet_opt(cx, inner.span) {
diag.span_suggestion(
e.span,
"change this to",
snippet,
Applicability::MachineApplicable,
);
}
},
);
}
}
}
}
}
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
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 pat.span.from_expansion() {
// 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 !pat.span.from_expansion();
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())],
}),
);
}
}
}
}
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,
if pat.always_deref {
NEEDLESS_BORROW
} else {
REF_BINDING_TO_REFERENCE
},
pat.spans,
"this pattern creates a reference to a reference",
|diag| {
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 !span.from_expansion() => {
// 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 !parent.span.from_expansion() => {
// 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 !e.span.from_expansion() => {
// 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,
}
}
}
}
}
}

23
tests/ui/needless_borrow.fixed
View File

@ -1,9 +1,10 @@
// run-rustfix
#[warn(clippy::all, clippy::needless_borrow)]
#[allow(unused_variables)]
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
fn main() {
let a = 5;
let ref_a = &a;
let _ = x(&a); // no warning
let _ = x(&a); // warn
@ -21,11 +22,29 @@ fn main() {
44 => &a,
45 => {
println!("foo");
&&a // FIXME: this should lint, too
&a
},
46 => &a,
47 => {
println!("foo");
loop {
println!("{}", a);
if a == 25 {
break ref_a;
}
}
},
_ => panic!(),
};
let _ = x(&a);
let _ = x(&a);
let _ = x(&mut b);
let _ = x(ref_a);
{
let b = &mut b;
x(b);
}
}
#[allow(clippy::needless_borrowed_reference)]

23
tests/ui/needless_borrow.rs
View File

@ -1,9 +1,10 @@
// run-rustfix
#[warn(clippy::all, clippy::needless_borrow)]
#[allow(unused_variables)]
#[allow(unused_variables, clippy::unnecessary_mut_passed)]
fn main() {
let a = 5;
let ref_a = &a;
let _ = x(&a); // no warning
let _ = x(&&a); // warn
@ -21,11 +22,29 @@ fn main() {
44 => &a,
45 => {
println!("foo");
&&a // FIXME: this should lint, too
&&a
},
46 => &&a,
47 => {
println!("foo");
loop {
println!("{}", a);
if a == 25 {
break &ref_a;
}
}
},
_ => panic!(),
};
let _ = x(&&&a);
let _ = x(&mut &&a);
let _ = x(&&&mut b);
let _ = x(&&ref_a);
{
let b = &mut b;
x(&b);
}
}
#[allow(clippy::needless_borrowed_reference)]

50
tests/ui/needless_borrow.stderr
View File

@ -1,5 +1,5 @@
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:8:15
--> $DIR/needless_borrow.rs:9:15
|
LL | let _ = x(&&a); // warn
| ^^^ help: change this to: `&a`
@ -7,16 +7,58 @@ LL | let _ = x(&&a); // warn
= note: `-D clippy::needless-borrow` implied by `-D warnings`
error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:12:13
--> $DIR/needless_borrow.rs:13:13
|
LL | mut_ref(&mut &mut b); // warn
| ^^^^^^^^^^^ help: change this to: `&mut b`
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:26:15
--> $DIR/needless_borrow.rs:25:13
|
LL | &&a
| ^^^ help: change this to: `&a`
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:27:15
|
LL | 46 => &&a,
| ^^^ help: change this to: `&a`
error: aborting due to 3 previous errors
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:33:27
|
LL | break &ref_a;
| ^^^^^^ help: change this to: `ref_a`
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:40:15
|
LL | let _ = x(&&&a);
| ^^^^ help: change this to: `&a`
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:41:15
|
LL | let _ = x(&mut &&a);
| ^^^^^^^^ help: change this to: `&a`
error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:42:15
|
LL | let _ = x(&&&mut b);
| ^^^^^^^^ help: change this to: `&mut b`
error: this expression borrows a reference (`&i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:43:15
|
LL | let _ = x(&&ref_a);
| ^^^^^^^ help: change this to: `ref_a`
error: this expression borrows a reference (`&mut i32`) that is immediately dereferenced by the compiler
--> $DIR/needless_borrow.rs:46:11
|
LL | x(&b);
| ^^ help: change this to: `b`
error: aborting due to 10 previous errors