src/tools/clippy/clippy_lints/src/significant_drop_tightening.rs

   1 use clippy_utils::diagnostics::span_lint_and_then;
   2 use clippy_utils::source::{indent_of, snippet};
   3 use clippy_utils::{expr_or_init, get_attr, path_to_local, peel_hir_expr_unary};
   4 use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
   5 use rustc_errors::Applicability;
   6 use rustc_hir::def::{DefKind, Res};
   7 use rustc_hir::intravisit::{walk_expr, Visitor};
   8 use rustc_hir::{self as hir};
   9 use rustc_lint::{LateContext, LateLintPass, LintContext};
  10 use rustc_middle::ty::{GenericArgKind, Ty, TypeAndMut};
  11 use rustc_session::{declare_tool_lint, impl_lint_pass};
  12 use rustc_span::symbol::Ident;
  13 use rustc_span::{sym, Span, DUMMY_SP};
  14 use std::borrow::Cow;
  15
  16 declare_clippy_lint! {
  17     /// ### What it does
  18     ///
  19     /// Searches for elements marked with `#[clippy::has_significant_drop]` that could be early
  20     /// dropped but are in fact dropped at the end of their scopes. In other words, enforces the
  21     /// "tightening" of their possible lifetimes.
  22     ///
  23     /// ### Why is this bad?
  24     ///
  25     /// Elements marked with `#[clippy::has_significant_drop]` are generally synchronizing
  26     /// primitives that manage shared resources, as such, it is desired to release them as soon as
  27     /// possible to avoid unnecessary resource contention.
  28     ///
  29     /// ### Example
  30     ///
  31     /// ```rust,ignore
  32     /// fn main() {
  33     ///   let lock = some_sync_resource.lock();
  34     ///   let owned_rslt = lock.do_stuff_with_resource();
  35     ///   // Only `owned_rslt` is needed but `lock` is still held.
  36     ///   do_heavy_computation_that_takes_time(owned_rslt);
  37     /// }
  38     /// ```
  39     ///
  40     /// Use instead:
  41     ///
  42     /// ```rust,ignore
  43     /// fn main() {
  44     ///     let owned_rslt = some_sync_resource.lock().do_stuff_with_resource();
  45     ///     do_heavy_computation_that_takes_time(owned_rslt);
  46     /// }
  47     /// ```
  48     #[clippy::version = "1.69.0"]
  49     pub SIGNIFICANT_DROP_TIGHTENING,
  50     nursery,
  51     "Searches for elements marked with `#[clippy::has_significant_drop]` that could be early dropped but are in fact dropped at the end of their scopes"
  52 }
  53
  54 impl_lint_pass!(SignificantDropTightening<'_> => [SIGNIFICANT_DROP_TIGHTENING]);
  55
  56 #[derive(Default)]
  57 pub struct SignificantDropTightening<'tcx> {
  58     apas: FxIndexMap<hir::HirId, AuxParamsAttr>,
  59     /// Auxiliary structure used to avoid having to verify the same type multiple times.
  60     seen_types: FxHashSet<Ty<'tcx>>,
  61     type_cache: FxHashMap<Ty<'tcx>, bool>,
  62 }
  63
  64 impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
  65     fn check_fn(
  66         &mut self,
  67         cx: &LateContext<'tcx>,
  68         _: hir::intravisit::FnKind<'_>,
  69         _: &hir::FnDecl<'_>,
  70         body: &'tcx hir::Body<'_>,
  71         _: Span,
  72         _: hir::def_id::LocalDefId,
  73     ) {
  74         self.apas.clear();
  75         let initial_dummy_stmt = dummy_stmt_expr(body.value);
  76         let mut ap = AuxParams::new(&mut self.apas, &initial_dummy_stmt);
  77         StmtsChecker::new(&mut ap, cx, &mut self.seen_types, &mut self.type_cache).visit_body(body);
  78         for apa in ap.apas.values() {
  79             if apa.counter <= 1 || !apa.has_expensive_expr_after_last_attr {
  80                 continue;
  81             }
  82             span_lint_and_then(
  83                 cx,
  84                 SIGNIFICANT_DROP_TIGHTENING,
  85                 apa.first_bind_ident.span,
  86                 "temporary with significant `Drop` can be early dropped",
  87                 |diag| {
  88                     match apa.counter {
  89                         0 | 1 => {},
  90                         2 => {
  91                             let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
  92                             let init_method = snippet(cx, apa.first_method_span, "..");
  93                             let usage_method = snippet(cx, apa.last_method_span, "..");
  94                             let stmt = if apa.last_bind_ident == Ident::empty() {
  95                                 format!("\n{indent}{init_method}.{usage_method};")
  96                             } else {
  97                                 format!(
  98                                     "\n{indent}let {} = {init_method}.{usage_method};",
  99                                     snippet(cx, apa.last_bind_ident.span, ".."),
 100                                 )
 101                             };
 102                             diag.span_suggestion_verbose(
 103                                 apa.first_stmt_span,
 104                                 "merge the temporary construction with its single usage",
 105                                 stmt,
 106                                 Applicability::MaybeIncorrect,
 107                             );
 108                             diag.span_suggestion(
 109                                 apa.last_stmt_span,
 110                                 "remove separated single usage",
 111                                 "",
 112                                 Applicability::MaybeIncorrect,
 113                             );
 114                         },
 115                         _ => {
 116                             diag.span_suggestion(
 117                                 apa.last_stmt_span.shrink_to_hi(),
 118                                 "drop the temporary after the end of its last usage",
 119                                 format!(
 120                                     "\n{}drop({});",
 121                                     " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
 122                                     apa.first_bind_ident
 123                                 ),
 124                                 Applicability::MaybeIncorrect,
 125                             );
 126                         },
 127                     }
 128                     diag.note("this might lead to unnecessary resource contention");
 129                     diag.span_label(
 130                         apa.first_block_span,
 131                         format!(
 132                             "temporary `{}` is currently being dropped at the end of its contained scope",
 133                             apa.first_bind_ident
 134                         ),
 135                     );
 136                 },
 137             );
 138         }
 139     }
 140 }
 141
 142 /// Checks the existence of the `#[has_significant_drop]` attribute.
 143 struct AttrChecker<'cx, 'others, 'tcx> {
 144     cx: &'cx LateContext<'tcx>,
 145     seen_types: &'others mut FxHashSet<Ty<'tcx>>,
 146     type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
 147 }
 148
 149 impl<'cx, 'others, 'tcx> AttrChecker<'cx, 'others, 'tcx> {
 150     pub(crate) fn new(
 151         cx: &'cx LateContext<'tcx>,
 152         seen_types: &'others mut FxHashSet<Ty<'tcx>>,
 153         type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
 154     ) -> Self {
 155         seen_types.clear();
 156         Self {
 157             cx,
 158             seen_types,
 159             type_cache,
 160         }
 161     }
 162
 163     fn has_sig_drop_attr(&mut self, ty: Ty<'tcx>) -> bool {
 164         // The borrow checker prevents us from using something fancier like or_insert_with.
 165         if let Some(ty) = self.type_cache.get(&ty) {
 166             return *ty;
 167         }
 168         let value = self.has_sig_drop_attr_uncached(ty);
 169         self.type_cache.insert(ty, value);
 170         value
 171     }
 172
 173     fn has_sig_drop_attr_uncached(&mut self, ty: Ty<'tcx>) -> bool {
 174         if let Some(adt) = ty.ty_adt_def() {
 175             let mut iter = get_attr(
 176                 self.cx.sess(),
 177                 self.cx.tcx.get_attrs_unchecked(adt.did()),
 178                 "has_significant_drop",
 179             );
 180             if iter.next().is_some() {
 181                 return true;
 182             }
 183         }
 184         match ty.kind() {
 185             rustc_middle::ty::Adt(a, b) => {
 186                 for f in a.all_fields() {
 187                     let ty = f.ty(self.cx.tcx, b);
 188                     if !self.has_seen_ty(ty) && self.has_sig_drop_attr(ty) {
 189                         return true;
 190                     }
 191                 }
 192                 for generic_arg in *b {
 193                     if let GenericArgKind::Type(ty) = generic_arg.unpack() {
 194                         if self.has_sig_drop_attr(ty) {
 195                             return true;
 196                         }
 197                     }
 198                 }
 199                 false
 200             },
 201             rustc_middle::ty::Array(ty, _)
 202             | rustc_middle::ty::RawPtr(TypeAndMut { ty, .. })
 203             | rustc_middle::ty::Ref(_, ty, _)
 204             | rustc_middle::ty::Slice(ty) => self.has_sig_drop_attr(*ty),
 205             _ => false,
 206         }
 207     }
 208
 209     fn has_seen_ty(&mut self, ty: Ty<'tcx>) -> bool {
 210         !self.seen_types.insert(ty)
 211     }
 212 }
 213
 214 struct StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx> {
 215     ap: &'ap mut AuxParams<'others, 'stmt, 'tcx>,
 216     cx: &'lc LateContext<'tcx>,
 217     seen_types: &'others mut FxHashSet<Ty<'tcx>>,
 218     type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
 219 }
 220
 221 impl<'ap, 'lc, 'others, 'stmt, 'tcx> StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx> {
 222     fn new(
 223         ap: &'ap mut AuxParams<'others, 'stmt, 'tcx>,
 224         cx: &'lc LateContext<'tcx>,
 225         seen_types: &'others mut FxHashSet<Ty<'tcx>>,
 226         type_cache: &'others mut FxHashMap<Ty<'tcx>, bool>,
 227     ) -> Self {
 228         Self {
 229             ap,
 230             cx,
 231             seen_types,
 232             type_cache,
 233         }
 234     }
 235
 236     fn manage_has_expensive_expr_after_last_attr(&mut self) {
 237         let has_expensive_stmt = match self.ap.curr_stmt.kind {
 238             hir::StmtKind::Expr(expr) if is_inexpensive_expr(expr) => false,
 239             hir::StmtKind::Local(local)
 240                 if let Some(expr) = local.init
 241                     && let hir::ExprKind::Path(_) = expr.kind =>
 242             {
 243                 false
 244             },
 245             _ => true,
 246         };
 247         if has_expensive_stmt {
 248             for apa in self.ap.apas.values_mut() {
 249                 let last_stmt_is_not_dummy = apa.last_stmt_span != DUMMY_SP;
 250                 let last_stmt_is_not_curr = self.ap.curr_stmt.span != apa.last_stmt_span;
 251                 let block_equals_curr = self.ap.curr_block_hir_id == apa.first_block_hir_id;
 252                 let block_is_ancestor = self
 253                     .cx
 254                     .tcx
 255                     .hir()
 256                     .parent_iter(self.ap.curr_block_hir_id)
 257                     .any(|(id, _)| id == apa.first_block_hir_id);
 258                 if last_stmt_is_not_dummy && last_stmt_is_not_curr && (block_equals_curr || block_is_ancestor) {
 259                     apa.has_expensive_expr_after_last_attr = true;
 260                 }
 261             }
 262         }
 263     }
 264 }
 265
 266 impl<'ap, 'lc, 'others, 'stmt, 'tcx> Visitor<'tcx> for StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx> {
 267     fn visit_block(&mut self, block: &'tcx hir::Block<'tcx>) {
 268         self.ap.curr_block_hir_id = block.hir_id;
 269         self.ap.curr_block_span = block.span;
 270         for stmt in block.stmts {
 271             self.ap.curr_stmt = Cow::Borrowed(stmt);
 272             self.visit_stmt(stmt);
 273             self.ap.curr_block_hir_id = block.hir_id;
 274             self.ap.curr_block_span = block.span;
 275             self.manage_has_expensive_expr_after_last_attr();
 276         }
 277         if let Some(expr) = block.expr {
 278             self.ap.curr_stmt = Cow::Owned(dummy_stmt_expr(expr));
 279             self.visit_expr(expr);
 280             self.ap.curr_block_hir_id = block.hir_id;
 281             self.ap.curr_block_span = block.span;
 282             self.manage_has_expensive_expr_after_last_attr();
 283         }
 284     }
 285
 286     fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
 287         let modify_apa_params = |apa: &mut AuxParamsAttr| {
 288             apa.counter = apa.counter.wrapping_add(1);
 289             apa.has_expensive_expr_after_last_attr = false;
 290         };
 291         let mut ac = AttrChecker::new(self.cx, self.seen_types, self.type_cache);
 292         if ac.has_sig_drop_attr(self.cx.typeck_results().expr_ty(expr)) {
 293             if let hir::StmtKind::Local(local) = self.ap.curr_stmt.kind
 294                 && let hir::PatKind::Binding(_, hir_id, ident, _) = local.pat.kind
 295                 && !self.ap.apas.contains_key(&hir_id)
 296                 && {
 297                     if let Some(local_hir_id) = path_to_local(expr) {
 298                         local_hir_id == hir_id
 299                     } else {
 300                         true
 301                     }
 302                 }
 303             {
 304                 let mut apa = AuxParamsAttr {
 305                     first_bind_ident: ident,
 306                     first_block_hir_id: self.ap.curr_block_hir_id,
 307                     first_block_span: self.ap.curr_block_span,
 308                     first_method_span: {
 309                         let expr_or_init = expr_or_init(self.cx, expr);
 310                         if let hir::ExprKind::MethodCall(_, local_expr, _, span) = expr_or_init.kind {
 311                             local_expr.span.to(span)
 312                         } else {
 313                             expr_or_init.span
 314                         }
 315                     },
 316                     first_stmt_span: self.ap.curr_stmt.span,
 317                     ..Default::default()
 318                 };
 319                 modify_apa_params(&mut apa);
 320                 let _ = self.ap.apas.insert(hir_id, apa);
 321             } else {
 322                 let Some(hir_id) = path_to_local(expr) else {
 323                     return;
 324                 };
 325                 let Some(apa) = self.ap.apas.get_mut(&hir_id) else {
 326                     return;
 327                 };
 328                 match self.ap.curr_stmt.kind {
 329                     hir::StmtKind::Local(local) => {
 330                         if let hir::PatKind::Binding(_, _, ident, _) = local.pat.kind {
 331                             apa.last_bind_ident = ident;
 332                         }
 333                         if let Some(local_init) = local.init
 334                             && let hir::ExprKind::MethodCall(_, _, _, span) = local_init.kind
 335                         {
 336                             apa.last_method_span = span;
 337                         }
 338                     },
 339                     hir::StmtKind::Semi(semi_expr) => {
 340                         if has_drop(semi_expr, &apa.first_bind_ident, self.cx) {
 341                             apa.has_expensive_expr_after_last_attr = false;
 342                             apa.last_stmt_span = DUMMY_SP;
 343                             return;
 344                         }
 345                         if let hir::ExprKind::MethodCall(_, _, _, span) = semi_expr.kind {
 346                             apa.last_method_span = span;
 347                         }
 348                     },
 349                     _ => {},
 350                 }
 351                 apa.last_stmt_span = self.ap.curr_stmt.span;
 352                 modify_apa_params(apa);
 353             }
 354         }
 355         walk_expr(self, expr);
 356     }
 357 }
 358
 359 /// Auxiliary parameters used on each block check of an item
 360 struct AuxParams<'others, 'stmt, 'tcx> {
 361     //// See [AuxParamsAttr].
 362     apas: &'others mut FxIndexMap<hir::HirId, AuxParamsAttr>,
 363     /// The current block identifier that is being visited.
 364     curr_block_hir_id: hir::HirId,
 365     /// The current block span that is being visited.
 366     curr_block_span: Span,
 367     /// The current statement that is being visited.
 368     curr_stmt: Cow<'stmt, hir::Stmt<'tcx>>,
 369 }
 370
 371 impl<'others, 'stmt, 'tcx> AuxParams<'others, 'stmt, 'tcx> {
 372     fn new(apas: &'others mut FxIndexMap<hir::HirId, AuxParamsAttr>, curr_stmt: &'stmt hir::Stmt<'tcx>) -> Self {
 373         Self {
 374             apas,
 375             curr_block_hir_id: hir::HirId::INVALID,
 376             curr_block_span: DUMMY_SP,
 377             curr_stmt: Cow::Borrowed(curr_stmt),
 378         }
 379     }
 380 }
 381
 382 /// Auxiliary parameters used on expression created with `#[has_significant_drop]`.
 383 #[derive(Debug)]
 384 struct AuxParamsAttr {
 385     /// The number of times `#[has_significant_drop]` was referenced.
 386     counter: usize,
 387     /// If an expensive expression follows the last use of anything marked with
 388     /// `#[has_significant_drop]`.
 389     has_expensive_expr_after_last_attr: bool,
 390
 391     /// The identifier of the block that involves the first `#[has_significant_drop]`.
 392     first_block_hir_id: hir::HirId,
 393     /// The span of the block that involves the first `#[has_significant_drop]`.
 394     first_block_span: Span,
 395     /// The binding or variable that references the initial construction of the type marked with
 396     /// `#[has_significant_drop]`.
 397     first_bind_ident: Ident,
 398     /// Similar to `init_bind_ident` but encompasses the right-hand method call.
 399     first_method_span: Span,
 400     /// Similar to `init_bind_ident` but encompasses the whole contained statement.
 401     first_stmt_span: Span,
 402
 403     /// The last visited binding or variable span within a block that had any referenced inner type
 404     /// marked with `#[has_significant_drop]`.
 405     last_bind_ident: Ident,
 406     /// Similar to `last_bind_span` but encompasses the right-hand method call.
 407     last_method_span: Span,
 408     /// Similar to `last_bind_span` but encompasses the whole contained statement.
 409     last_stmt_span: Span,
 410 }
 411
 412 impl Default for AuxParamsAttr {
 413     fn default() -> Self {
 414         Self {
 415             counter: 0,
 416             has_expensive_expr_after_last_attr: false,
 417             first_block_hir_id: hir::HirId::INVALID,
 418             first_bind_ident: Ident::empty(),
 419             first_block_span: DUMMY_SP,
 420             first_method_span: DUMMY_SP,
 421             first_stmt_span: DUMMY_SP,
 422             last_bind_ident: Ident::empty(),
 423             last_method_span: DUMMY_SP,
 424             last_stmt_span: DUMMY_SP,
 425         }
 426     }
 427 }
 428
 429 fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {
 430     hir::Stmt {
 431         hir_id: hir::HirId::INVALID,
 432         kind: hir::StmtKind::Expr(expr),
 433         span: DUMMY_SP,
 434     }
 435 }
 436
 437 fn has_drop(expr: &hir::Expr<'_>, first_bind_ident: &Ident, lcx: &LateContext<'_>) -> bool {
 438     if let hir::ExprKind::Call(fun, args) = expr.kind
 439         && let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
 440         && let Res::Def(DefKind::Fn, did) = fun_path.res
 441         && lcx.tcx.is_diagnostic_item(sym::mem_drop, did)
 442         && let [first_arg, ..] = args
 443     {
 444         let has_ident = |local_expr: &hir::Expr<'_>| {
 445             if let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &local_expr.kind
 446                 && let [first_arg_ps, ..] = arg_path.segments
 447                 && &first_arg_ps.ident == first_bind_ident
 448             {
 449                 true
 450             } else {
 451                 false
 452             }
 453         };
 454         if has_ident(first_arg) {
 455             return true;
 456         }
 457         if let hir::ExprKind::Tup(value) = &first_arg.kind
 458             && value.iter().any(has_ident)
 459         {
 460             return true;
 461         }
 462     }
 463     false
 464 }
 465
 466 fn is_inexpensive_expr(expr: &hir::Expr<'_>) -> bool {
 467     let actual = peel_hir_expr_unary(expr).0;
 468     let is_path = matches!(actual.kind, hir::ExprKind::Path(_));
 469     let is_lit = matches!(actual.kind, hir::ExprKind::Lit(_));
 470     is_path || is_lit
 471 }