//!
//! This lint is **warn** by default
-use crate::utils::{is_automatically_derived, snippet_opt, span_lint_and_then};
+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, 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, Item, 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::def_id::LocalDefId;
+use rustc_span::Span;
declare_clippy_lint! {
- /// **What it does:** Checks for address of operations (`&`) that are going to
+ /// ### 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
+ /// ### Why is this bad?
+ /// Suggests that the receiver of the expression borrows
/// the expression.
///
- /// **Known problems:** None.
- ///
- /// **Example:**
+ /// ### Example
/// ```rust
/// // Bad
/// let x: &i32 = &&&&&&5;
/// let x: &i32 = &5;
/// ```
pub NEEDLESS_BORROW,
- nursery,
+ 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
+ /// }
+ /// ```
+ 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 {
- derived_item: Option<LocalDefId>,
+ /// 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>>,
}
-impl_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW]);
+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 e.span.from_expansion() || self.derived_item.is_some() {
+ 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::Not, ref inner) = e.kind {
+ if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, 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 {
}
}
}
+
fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
- if pat.span.from_expansion() || self.derived_item.is_some() {
- 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;
}
}
-
- fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
- let attrs = cx.tcx.hir().attrs(item.hir_id());
- if is_automatically_derived(attrs) {
- debug_assert!(self.derived_item.is_none());
- self.derived_item = Some(item.def_id);
- }
- }
-
- fn check_item_post(&mut self, _: &LateContext<'tcx>, item: &'tcx Item<'_>) {
- if let Some(id) = self.derived_item {
- if item.def_id == id {
- self.derived_item = 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,
+ }
+ }
}
}
}
projects / rust.git / blobdiff