Address review comments

smoelius · smoelius · commit 5ad9b72dbbea · 2022-10-03T17:08:24.000-04:00
diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs
@@ -20,16 +20,15 @@ use rustc_hir::{
 };
 use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
-use rustc_lint::{LateContext, LateLintPass, LintArray, LintPass};
-use rustc_lint_defs::lint_array;
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::mir::{Rvalue, StatementKind};
 use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
 use rustc_middle::ty::{
     self, Binder, BoundVariableKind, EarlyBinder, FnSig, GenericArgKind, List, ParamTy, PredicateKind,
     ProjectionPredicate, Ty, TyCtxt, TypeVisitable, TypeckResults,
 };
 use rustc_semver::RustcVersion;
-use rustc_session::declare_tool_lint;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::{symbol::sym, Span, Symbol, DUMMY_SP};
 use rustc_trait_selection::infer::InferCtxtExt as _;
 use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
@@ -146,24 +145,12 @@ declare_clippy_lint! {
     "dereferencing when the compiler would automatically dereference"
 }
 
-#[expect(rustc::internal)]
-impl<'tcx> LintPass for Dereferencing<'tcx> {
-    fn name(&self) -> &'static str {
-        stringify!($ty)
-    }
-}
-
-impl<'tcx> Dereferencing<'tcx> {
-    #[expect(dead_code)]
-    pub fn get_lints() -> LintArray {
-        lint_array!(
-            EXPLICIT_DEREF_METHODS,
-            NEEDLESS_BORROW,
-            REF_BINDING_TO_REFERENCE,
-            EXPLICIT_AUTO_DEREF,
-        )
-    }
-}
+impl_lint_pass!(Dereferencing<'_> => [
+    EXPLICIT_DEREF_METHODS,
+    NEEDLESS_BORROW,
+    REF_BINDING_TO_REFERENCE,
+    EXPLICIT_AUTO_DEREF,
+]);
 
 #[derive(Default)]
 pub struct Dereferencing<'tcx> {
@@ -189,7 +176,7 @@ pub struct Dereferencing<'tcx> {
 
     /// Map from body owners to `PossibleBorrowerMap`s. Used by `needless_borrow_impl_arg_position`
     /// to determine when a borrowed expression can instead be moved.
-    possible_borrowers: FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
+    possible_borrowers: Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
 
     // `IntoIterator` for arrays requires Rust 1.53.
     msrv: Option<RustcVersion>,
@@ -571,6 +558,12 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
     }
 
     fn check_body_post(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'_>) {
+        if self.possible_borrowers.last().map_or(false, |&(local_def_id, _)| {
+            local_def_id == cx.tcx.hir().body_owner_def_id(body.id())
+        }) {
+            self.possible_borrowers.pop();
+        }
+
         if Some(body.id()) == self.current_body {
             for pat in self.ref_locals.drain(..).filter_map(|(_, x)| x) {
                 let replacements = pat.replacements;
@@ -703,7 +696,7 @@ impl Position {
 #[expect(clippy::too_many_lines)]
 fn walk_parents<'tcx>(
     cx: &LateContext<'tcx>,
-    possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
+    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
     e: &'tcx Expr<'_>,
     msrv: Option<RustcVersion>,
 ) -> (Position, &'tcx [Adjustment<'tcx>]) {
@@ -1061,7 +1054,7 @@ fn ty_contains_infer(ty: &hir::Ty<'_>) -> bool {
 #[expect(clippy::too_many_arguments)]
 fn needless_borrow_impl_arg_position<'tcx>(
     cx: &LateContext<'tcx>,
-    possible_borrowers: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
+    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
     parent: &Expr<'tcx>,
     arg_index: usize,
     param_ty: ParamTy,
@@ -1202,7 +1195,7 @@ fn has_ref_mut_self_method(cx: &LateContext<'_>, trait_def_id: DefId) -> bool {
 
 fn referent_used_exactly_once<'a, 'tcx>(
     cx: &'a LateContext<'tcx>,
-    possible_borrower: &mut FxIndexMap<LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>>,
+    possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
     reference: &Expr<'tcx>,
 ) -> bool {
     let mir = enclosing_mir(cx.tcx, reference.hir_id);
@@ -1213,9 +1206,13 @@ fn referent_used_exactly_once<'a, 'tcx>(
         && !place.has_deref()
     {
         let body_owner_local_def_id = cx.tcx.hir().enclosing_body_owner(reference.hir_id);
-        let possible_borrower = possible_borrower
-            .entry(body_owner_local_def_id)
-            .or_insert_with(|| PossibleBorrowerMap::new(cx, mir));
+        if possible_borrowers
+            .last()
+            .map_or(true, |&(local_def_id, _)| local_def_id != body_owner_local_def_id)
+        {
+            possible_borrowers.push((body_owner_local_def_id, PossibleBorrowerMap::new(cx, mir)));
+        }
+        let possible_borrower = &mut possible_borrowers.last_mut().unwrap().1;
         // If `only_borrowers` were used here, the `copyable_iterator::warn` test would fail. The reason is
         // that `PossibleBorrowerVisitor::visit_terminator` considers `place.local` a possible borrower of
         // itself. See the comment in that method for an explanation as to why.
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -37,7 +37,6 @@ extern crate rustc_index;
 extern crate rustc_infer;
 extern crate rustc_lexer;
 extern crate rustc_lint;
-extern crate rustc_lint_defs;
 extern crate rustc_middle;
 extern crate rustc_parse;
 extern crate rustc_session;
diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs
@@ -429,86 +429,87 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass {
             return;
         }
 
-        match &item.kind {
-            hir::ItemKind::Static(ty, Mutability::Not, body_id) => {
-                let is_lint_ref_ty = is_lint_ref_type(cx, ty);
-                if is_deprecated_lint(cx, ty) || is_lint_ref_ty {
-                    check_invalid_clippy_version_attribute(cx, item);
-
-                    let expr = &cx.tcx.hir().body(*body_id).value;
-                    let fields;
-                    if is_lint_ref_ty {
-                        if let ExprKind::AddrOf(_, _, inner_exp) = expr.kind
-                            && let ExprKind::Struct(_, struct_fields, _) = inner_exp.kind {
-                                fields = struct_fields;
-                        } else {
-                            return;
-                        }
-                    } else if let ExprKind::Struct(_, struct_fields, _) = expr.kind {
-                        fields = struct_fields;
+        if let hir::ItemKind::Static(ty, Mutability::Not, body_id) = item.kind {
+            let is_lint_ref_ty = is_lint_ref_type(cx, ty);
+            if is_deprecated_lint(cx, ty) || is_lint_ref_ty {
+                check_invalid_clippy_version_attribute(cx, item);
+
+                let expr = &cx.tcx.hir().body(body_id).value;
+                let fields;
+                if is_lint_ref_ty {
+                    if let ExprKind::AddrOf(_, _, inner_exp) = expr.kind
+                        && let ExprKind::Struct(_, struct_fields, _) = inner_exp.kind {
+                            fields = struct_fields;
                     } else {
                         return;
                     }
+                } else if let ExprKind::Struct(_, struct_fields, _) = expr.kind {
+                    fields = struct_fields;
+                } else {
+                    return;
+                }
 
-                    let field = fields
-                        .iter()
-                        .find(|f| f.ident.as_str() == "desc")
-                        .expect("lints must have a description field");
-
-                    if let ExprKind::Lit(Spanned {
-                        node: LitKind::Str(ref sym, _),
-                        ..
-                    }) = field.expr.kind
-                    {
-                        let sym_str = sym.as_str();
-                        if is_lint_ref_ty {
-                            if sym_str == "default lint description" {
-                                span_lint(
-                                    cx,
-                                    DEFAULT_LINT,
-                                    item.span,
-                                    &format!("the lint `{}` has the default lint description", item.ident.name),
-                                );
-                            }
+                let field = fields
+                    .iter()
+                    .find(|f| f.ident.as_str() == "desc")
+                    .expect("lints must have a description field");
 
-                            self.declared_lints.insert(item.ident.name, item.span);
-                        } else if sym_str == "default deprecation note" {
+                if let ExprKind::Lit(Spanned {
+                    node: LitKind::Str(ref sym, _),
+                    ..
+                }) = field.expr.kind
+                {
+                    let sym_str = sym.as_str();
+                    if is_lint_ref_ty {
+                        if sym_str == "default lint description" {
                             span_lint(
                                 cx,
-                                DEFAULT_DEPRECATION_REASON,
+                                DEFAULT_LINT,
                                 item.span,
-                                &format!("the lint `{}` has the default deprecation reason", item.ident.name),
+                                &format!("the lint `{}` has the default lint description", item.ident.name),
                             );
                         }
+
+                        self.declared_lints.insert(item.ident.name, item.span);
+                    } else if sym_str == "default deprecation note" {
+                        span_lint(
+                            cx,
+                            DEFAULT_DEPRECATION_REASON,
+                            item.span,
+                            &format!("the lint `{}` has the default deprecation reason", item.ident.name),
+                        );
                     }
                 }
-            },
-            hir::ItemKind::Impl(hir::Impl {
+            }
+        } else if let Some(macro_call) = root_macro_call_first_node(cx, item) {
+            if !matches!(
+                cx.tcx.item_name(macro_call.def_id).as_str(),
+                "impl_lint_pass" | "declare_lint_pass"
+            ) {
+                return;
+            }
+            if let hir::ItemKind::Impl(hir::Impl {
                 of_trait: None,
                 items: impl_item_refs,
                 ..
-            }) => {
-                let macro_call = root_macro_call_first_node(cx, item);
-                if let Some(macro_call) = &macro_call
-                    && !matches!(
-                        cx.tcx.item_name(macro_call.def_id).as_str(),
-                        "impl_lint_pass" | "declare_lint_pass"
-                    )
-                {
-                    return;
-                }
-                if let Some(iiref) = impl_item_refs.iter().find(|iiref| iiref.ident.as_str() == "get_lints") {
-                    let mut collector = LintCollector {
-                        output: &mut self.registered_lints,
-                        cx,
-                    };
-                    let body_id = cx.tcx.hir().body_owned_by(iiref.id.def_id.def_id);
-                    collector.visit_expr(cx.tcx.hir().body(body_id).value);
-                } else {
-                    assert!(macro_call.is_none(), "LintPass needs to implement get_lints");
-                }
-            },
-            _ => {},
+            }) = item.kind
+            {
+                let mut collector = LintCollector {
+                    output: &mut self.registered_lints,
+                    cx,
+                };
+                let body_id = cx.tcx.hir().body_owned_by(
+                    cx.tcx.hir().local_def_id(
+                        impl_item_refs
+                            .iter()
+                            .find(|iiref| iiref.ident.as_str() == "get_lints")
+                            .expect("LintPass needs to implement get_lints")
+                            .id
+                            .hir_id(),
+                    ),
+                );
+                collector.visit_expr(cx.tcx.hir().body(body_id).value);
+            }
         }
     }
 
