clippy_lints/src/significant_drop_in_scrutinee.rs

   1 use crate::FxHashSet;
   2 use clippy_utils::diagnostics::span_lint_and_then;
   3 use clippy_utils::get_attr;
   4 use clippy_utils::source::{indent_of, snippet};
   5 use rustc_errors::{Applicability, Diagnostic};
   6 use rustc_hir::intravisit::{walk_expr, Visitor};
   7 use rustc_hir::{Expr, ExprKind};
   8 use rustc_lint::{LateContext, LateLintPass, LintContext};
   9 use rustc_middle::ty::subst::GenericArgKind;
  10 use rustc_middle::ty::{Ty, TypeAndMut};
  11 use rustc_session::{declare_lint_pass, declare_tool_lint};
  12 use rustc_span::Span;
  13
  14 declare_clippy_lint! {
  15     /// ### What it does
  16     /// Check for temporaries returned from function calls in a match scrutinee that have the
  17     /// `clippy::has_significant_drop` attribute.
  18     ///
  19     /// ### Why is this bad?
  20     /// The `clippy::has_significant_drop` attribute can be added to types whose Drop impls have
  21     /// an important side-effect, such as unlocking a mutex, making it important for users to be
  22     /// able to accurately understand their lifetimes. When a temporary is returned in a function
  23     /// call in a match scrutinee, its lifetime lasts until the end of the match block, which may
  24     /// be surprising.
  25     ///
  26     /// For `Mutex`es this can lead to a deadlock. This happens when the match scrutinee uses a
  27     /// function call that returns a `MutexGuard` and then tries to lock again in one of the match
  28     /// arms. In that case the `MutexGuard` in the scrutinee will not be dropped until the end of
  29     /// the match block and thus will not unlock.
  30     ///
  31     /// ### Example
  32     /// ```rust.ignore
  33     /// # use std::sync::Mutex;
  34     ///
  35     /// # struct State {}
  36     ///
  37     /// # impl State {
  38     /// #     fn foo(&self) -> bool {
  39     /// #         true
  40     /// #     }
  41     ///
  42     /// #     fn bar(&self) {}
  43     /// # }
  44     ///
  45     ///
  46     /// let mutex = Mutex::new(State {});
  47     ///
  48     /// match mutex.lock().unwrap().foo() {
  49     ///     true => {
  50     ///         mutex.lock().unwrap().bar(); // Deadlock!
  51     ///     }
  52     ///     false => {}
  53     /// };
  54     ///
  55     /// println!("All done!");
  56     ///
  57     /// ```
  58     /// Use instead:
  59     /// ```rust
  60     /// # use std::sync::Mutex;
  61     ///
  62     /// # struct State {}
  63     ///
  64     /// # impl State {
  65     /// #     fn foo(&self) -> bool {
  66     /// #         true
  67     /// #     }
  68     ///
  69     /// #     fn bar(&self) {}
  70     /// # }
  71     ///
  72     /// let mutex = Mutex::new(State {});
  73     ///
  74     /// let is_foo = mutex.lock().unwrap().foo();
  75     /// match is_foo {
  76     ///     true => {
  77     ///         mutex.lock().unwrap().bar();
  78     ///     }
  79     ///     false => {}
  80     /// };
  81     ///
  82     /// println!("All done!");
  83     /// ```
  84     #[clippy::version = "1.60.0"]
  85     pub SIGNIFICANT_DROP_IN_SCRUTINEE,
  86     suspicious,
  87     "warns when a temporary of a type with a drop with a significant side-effect might have a surprising lifetime"
  88 }
  89
  90 declare_lint_pass!(SignificantDropInScrutinee => [SIGNIFICANT_DROP_IN_SCRUTINEE]);
  91
  92 impl<'tcx> LateLintPass<'tcx> for SignificantDropInScrutinee {
  93     fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
  94         if let Some(suggestions) = has_significant_drop_in_scrutinee(cx, expr) {
  95             for found in suggestions {
  96                 span_lint_and_then(
  97                     cx,
  98                     SIGNIFICANT_DROP_IN_SCRUTINEE,
  99                     found.found_span,
 100                     "temporary with significant drop in match scrutinee",
 101                     |diag| set_diagnostic(diag, cx, expr, found),
 102                 );
 103             }
 104         }
 105     }
 106 }
 107
 108 fn set_diagnostic<'tcx>(diag: &mut Diagnostic, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, found: FoundSigDrop) {
 109     if found.lint_suggestion == LintSuggestion::MoveAndClone {
 110         // If our suggestion is to move and clone, then we want to leave it to the user to
 111         // decide how to address this lint, since it may be that cloning is inappropriate.
 112         // Therefore, we won't to emit a suggestion.
 113         return;
 114     }
 115
 116     let original = snippet(cx, found.found_span, "..");
 117     let trailing_indent = " ".repeat(indent_of(cx, found.found_span).unwrap_or(0));
 118
 119     let replacement = if found.lint_suggestion == LintSuggestion::MoveAndDerefToCopy {
 120         format!("let value = *{};\n{}", original, trailing_indent)
 121     } else if found.is_unit_return_val {
 122         // If the return value of the expression to be moved is unit, then we don't need to
 123         // capture the result in a temporary -- we can just replace it completely with `()`.
 124         format!("{};\n{}", original, trailing_indent)
 125     } else {
 126         format!("let value = {};\n{}", original, trailing_indent)
 127     };
 128
 129     let suggestion_message = if found.lint_suggestion == LintSuggestion::MoveOnly {
 130         "try moving the temporary above the match"
 131     } else {
 132         "try moving the temporary above the match and create a copy"
 133     };
 134
 135     let scrutinee_replacement = if found.is_unit_return_val {
 136         "()".to_owned()
 137     } else {
 138         "value".to_owned()
 139     };
 140
 141     diag.multipart_suggestion(
 142         suggestion_message,
 143         vec![
 144             (expr.span.shrink_to_lo(), replacement),
 145             (found.found_span, scrutinee_replacement),
 146         ],
 147         Applicability::MaybeIncorrect,
 148     );
 149 }
 150
 151 /// If the expression is an `ExprKind::Match`, check if the scrutinee has a significant drop that
 152 /// may have a surprising lifetime.
 153 fn has_significant_drop_in_scrutinee<'tcx, 'a>(
 154     cx: &'a LateContext<'tcx>,
 155     expr: &'tcx Expr<'tcx>,
 156 ) -> Option<Vec<FoundSigDrop>> {
 157     let mut helper = SigDropHelper::new(cx);
 158     match expr.kind {
 159         ExprKind::Match(match_expr, _, _) => helper.find_sig_drop(match_expr),
 160         _ => None,
 161     }
 162 }
 163
 164 struct SigDropHelper<'a, 'tcx> {
 165     cx: &'a LateContext<'tcx>,
 166     is_chain_end: bool,
 167     seen_types: FxHashSet<Ty<'tcx>>,
 168     has_significant_drop: bool,
 169     current_sig_drop: Option<FoundSigDrop>,
 170     sig_drop_spans: Option<Vec<FoundSigDrop>>,
 171     special_handling_for_binary_op: bool,
 172 }
 173
 174 #[expect(clippy::enum_variant_names)]
 175 #[derive(Debug, PartialEq, Eq, Clone, Copy)]
 176 enum LintSuggestion {
 177     MoveOnly,
 178     MoveAndDerefToCopy,
 179     MoveAndClone,
 180 }
 181
 182 #[derive(Clone, Copy)]
 183 struct FoundSigDrop {
 184     found_span: Span,
 185     is_unit_return_val: bool,
 186     lint_suggestion: LintSuggestion,
 187 }
 188
 189 impl<'a, 'tcx> SigDropHelper<'a, 'tcx> {
 190     fn new(cx: &'a LateContext<'tcx>) -> SigDropHelper<'a, 'tcx> {
 191         SigDropHelper {
 192             cx,
 193             is_chain_end: true,
 194             seen_types: FxHashSet::default(),
 195             has_significant_drop: false,
 196             current_sig_drop: None,
 197             sig_drop_spans: None,
 198             special_handling_for_binary_op: false,
 199         }
 200     }
 201
 202     fn find_sig_drop(&mut self, match_expr: &'tcx Expr<'_>) -> Option<Vec<FoundSigDrop>> {
 203         self.visit_expr(match_expr);
 204
 205         // If sig drop spans is empty but we found a significant drop, it means that we didn't find
 206         // a type that was trivially copyable as we moved up the chain after finding a significant
 207         // drop, so move the entire scrutinee.
 208         if self.has_significant_drop && self.sig_drop_spans.is_none() {
 209             self.try_setting_current_suggestion(match_expr, true);
 210             self.move_current_suggestion();
 211         }
 212
 213         self.sig_drop_spans.take()
 214     }
 215
 216     /// This will try to set the current suggestion (so it can be moved into the suggestions vec
 217     /// later). If `allow_move_and_clone` is false, the suggestion *won't* be set -- this gives us
 218     /// an opportunity to look for another type in the chain that will be trivially copyable.
 219     /// However, if we are at the the end of the chain, we want to accept whatever is there. (The
 220     /// suggestion won't actually be output, but the diagnostic message will be output, so the user
 221     /// can determine the best way to handle the lint.)
 222     fn try_setting_current_suggestion(&mut self, expr: &'tcx Expr<'_>, allow_move_and_clone: bool) {
 223         if self.current_sig_drop.is_some() {
 224             return;
 225         }
 226         let ty = self.get_type(expr);
 227         if ty.is_ref() {
 228             // We checked that the type was ref, so builtin_deref will return Some TypeAndMut,
 229             // but let's avoid any chance of an ICE
 230             if let Some(TypeAndMut { ty, .. }) = ty.builtin_deref(true) {
 231                 if ty.is_trivially_pure_clone_copy() {
 232                     self.current_sig_drop.replace(FoundSigDrop {
 233                         found_span: expr.span,
 234                         is_unit_return_val: false,
 235                         lint_suggestion: LintSuggestion::MoveAndDerefToCopy,
 236                     });
 237                 } else if allow_move_and_clone {
 238                     self.current_sig_drop.replace(FoundSigDrop {
 239                         found_span: expr.span,
 240                         is_unit_return_val: false,
 241                         lint_suggestion: LintSuggestion::MoveAndClone,
 242                     });
 243                 }
 244             }
 245         } else if ty.is_trivially_pure_clone_copy() {
 246             self.current_sig_drop.replace(FoundSigDrop {
 247                 found_span: expr.span,
 248                 is_unit_return_val: false,
 249                 lint_suggestion: LintSuggestion::MoveOnly,
 250             });
 251         }
 252     }
 253
 254     fn move_current_suggestion(&mut self) {
 255         if let Some(current) = self.current_sig_drop.take() {
 256             self.sig_drop_spans.get_or_insert_with(Vec::new).push(current);
 257         }
 258     }
 259
 260     fn get_type(&self, ex: &'tcx Expr<'_>) -> Ty<'tcx> {
 261         self.cx.typeck_results().expr_ty(ex)
 262     }
 263
 264     fn has_seen_type(&mut self, ty: Ty<'tcx>) -> bool {
 265         !self.seen_types.insert(ty)
 266     }
 267
 268     fn visit_exprs_for_binary_ops(
 269         &mut self,
 270         left: &'tcx Expr<'_>,
 271         right: &'tcx Expr<'_>,
 272         is_unit_return_val: bool,
 273         span: Span,
 274     ) {
 275         self.special_handling_for_binary_op = true;
 276         self.visit_expr(left);
 277         self.visit_expr(right);
 278
 279         // If either side had a significant drop, suggest moving the entire scrutinee to avoid
 280         // unnecessary copies and to simplify cases where both sides have significant drops.
 281         if self.has_significant_drop {
 282             self.current_sig_drop.replace(FoundSigDrop {
 283                 found_span: span,
 284                 is_unit_return_val,
 285                 lint_suggestion: LintSuggestion::MoveOnly,
 286             });
 287         }
 288
 289         self.special_handling_for_binary_op = false;
 290     }
 291
 292     fn has_sig_drop_attr(&mut self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
 293         if let Some(adt) = ty.ty_adt_def() {
 294             if get_attr(cx.sess(), cx.tcx.get_attrs_unchecked(adt.did()), "has_significant_drop").count() > 0 {
 295                 return true;
 296             }
 297         }
 298
 299         match ty.kind() {
 300             rustc_middle::ty::Adt(a, b) => {
 301                 for f in a.all_fields() {
 302                     let ty = f.ty(cx.tcx, b);
 303                     if !self.has_seen_type(ty) && self.has_sig_drop_attr(cx, ty) {
 304                         return true;
 305                     }
 306                 }
 307
 308                 for generic_arg in b.iter() {
 309                     if let GenericArgKind::Type(ty) = generic_arg.unpack() {
 310                         if self.has_sig_drop_attr(cx, ty) {
 311                             return true;
 312                         }
 313                     }
 314                 }
 315                 false
 316             },
 317             rustc_middle::ty::Array(ty, _)
 318             | rustc_middle::ty::RawPtr(TypeAndMut { ty, .. })
 319             | rustc_middle::ty::Ref(_, ty, _)
 320             | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(cx, *ty),
 321             _ => false,
 322         }
 323     }
 324 }
 325
 326 impl<'a, 'tcx> Visitor<'tcx> for SigDropHelper<'a, 'tcx> {
 327     fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
 328         if !self.is_chain_end && self.has_sig_drop_attr(self.cx, self.get_type(ex)) {
 329             self.has_significant_drop = true;
 330             return;
 331         }
 332         self.is_chain_end = false;
 333
 334         match ex.kind {
 335             ExprKind::MethodCall(_, [ref expr, ..], _) => {
 336                 self.visit_expr(expr);
 337             }
 338             ExprKind::Binary(_, left, right) => {
 339                 self.visit_exprs_for_binary_ops(left, right, false, ex.span);
 340             }
 341             ExprKind::Assign(left, right, _) | ExprKind::AssignOp(_, left, right) => {
 342                 self.visit_exprs_for_binary_ops(left, right, true, ex.span);
 343             }
 344             ExprKind::Tup(exprs) => {
 345                 for expr in exprs {
 346                     self.visit_expr(expr);
 347                     if self.has_significant_drop {
 348                         // We may have not have set current_sig_drop if all the suggestions were
 349                         // MoveAndClone, so add this tuple item's full expression in that case.
 350                         if self.current_sig_drop.is_none() {
 351                             self.try_setting_current_suggestion(expr, true);
 352                         }
 353
 354                         // Now we are guaranteed to have something, so add it to the final vec.
 355                         self.move_current_suggestion();
 356                     }
 357                     // Reset `has_significant_drop` after each tuple expression so we can look for
 358                     // additional cases.
 359                     self.has_significant_drop = false;
 360                 }
 361                 if self.sig_drop_spans.is_some() {
 362                     self.has_significant_drop = true;
 363                 }
 364             }
 365             ExprKind::Box(..) |
 366                 ExprKind::Array(..) |
 367                 ExprKind::Call(..) |
 368                 ExprKind::Unary(..) |
 369                 ExprKind::If(..) |
 370                 ExprKind::Match(..) |
 371                 ExprKind::Field(..) |
 372                 ExprKind::Index(..) |
 373                 ExprKind::Ret(..) |
 374                 ExprKind::Repeat(..) |
 375                 ExprKind::Yield(..) |
 376                 ExprKind::MethodCall(..) => walk_expr(self, ex),
 377             ExprKind::AddrOf(_, _, _) |
 378                 ExprKind::Block(_, _) |
 379                 ExprKind::Break(_, _) |
 380                 ExprKind::Cast(_, _) |
 381                 // Don't want to check the closure itself, only invocation, which is covered by MethodCall
 382                 ExprKind::Closure(_, _, _, _, _) |
 383                 ExprKind::ConstBlock(_) |
 384                 ExprKind::Continue(_) |
 385                 ExprKind::DropTemps(_) |
 386                 ExprKind::Err |
 387                 ExprKind::InlineAsm(_) |
 388                 ExprKind::Let(_) |
 389                 ExprKind::Lit(_) |
 390                 ExprKind::Loop(_, _, _, _) |
 391                 ExprKind::Path(_) |
 392                 ExprKind::Struct(_, _, _) |
 393                 ExprKind::Type(_, _) => {
 394                 return;
 395             }
 396         }
 397
 398         // Once a significant temporary has been found, we need to go back up at least 1 level to
 399         // find the span to extract for replacement, so the temporary gets dropped. However, for
 400         // binary ops, we want to move the whole scrutinee so we avoid unnecessary copies and to
 401         // simplify cases where both sides have significant drops.
 402         if self.has_significant_drop && !self.special_handling_for_binary_op {
 403             self.try_setting_current_suggestion(ex, false);
 404         }
 405     }
 406 }