A small diagnostic improvement for dropping_copy_types

fixes #125189
rust-lang · May 24, 2024 · 09c8e39 · 09c8e39
1 parent 9f432d7
commit 09c8e39
Show file tree

Hide file tree

Showing 11 changed files with 169 additions and 11 deletions.
diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl
@@ -197,6 +197,7 @@ lint_drop_trait_constraints =
 lint_dropping_copy_types = calls to `std::mem::drop` with a value that implements `Copy` does nothing
     .label = argument has type `{$arg_ty}`
     .note = use `let _ = ...` to ignore the expression or result
+    .suggestion = use `let _ = ...` to ignore the expression or result
 
 lint_dropping_references = calls to `std::mem::drop` with a reference instead of an owned value does nothing
     .label = argument has type `{$arg_ty}`

diff --git a/compiler/rustc_lint/src/drop_forget_useless.rs b/compiler/rustc_lint/src/drop_forget_useless.rs
@@ -1,11 +1,11 @@
-use rustc_hir::{Arm, Expr, ExprKind, Node};
+use rustc_hir::{Arm, Expr, ExprKind, Node, StmtKind};
 use rustc_middle::ty;
 use rustc_span::sym;
 
 use crate::{
     lints::{
-        DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, UndroppedManuallyDropsDiag,
-        UndroppedManuallyDropsSuggestion,
+        DropCopyDiag, DropCopySuggestion, DropRefDiag, ForgetCopyDiag, ForgetRefDiag,
+        UndroppedManuallyDropsDiag, UndroppedManuallyDropsSuggestion,
     },
     LateContext, LateLintPass, LintContext,
 };
@@ -163,10 +163,23 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless {
                     );
                 }
                 sym::mem_drop if is_copy && !drop_is_single_call_in_arm => {
+                    let sugg = if let Some((_, node)) = cx.tcx.hir().parent_iter(expr.hir_id).nth(0)
+                        && let Node::Stmt(stmt) = node
+                        && let StmtKind::Semi(e) = stmt.kind
+                        && e.hir_id == expr.hir_id
+                    {
+                        DropCopySuggestion::Suggestion {
+                            start_span: expr.span.shrink_to_lo().until(arg.span),
+                            end_span: arg.span.shrink_to_hi().until(expr.span.shrink_to_hi()),
+                        }
+                    } else {
+                        DropCopySuggestion::Note
+                    };
+
                     cx.emit_span_lint(
                         DROPPING_COPY_TYPES,
                         expr.span,
-                        DropCopyDiag { arg_ty, label: arg.span },
+                        DropCopyDiag { arg_ty, label: arg.span, sugg },
                     );
                 }
                 sym::mem_forget if is_copy => {

diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs
@@ -669,11 +669,25 @@ pub struct DropRefDiag<'a> {
 
 #[derive(LintDiagnostic)]
 #[diag(lint_dropping_copy_types)]
-#[note]
 pub struct DropCopyDiag<'a> {
     pub arg_ty: Ty<'a>,
     #[label]
     pub label: Span,
+    #[subdiagnostic]
+    pub sugg: DropCopySuggestion,
+}
+
+#[derive(Subdiagnostic)]
+pub enum DropCopySuggestion {
+    #[note(lint_note)]
+    Note,
+    #[multipart_suggestion(lint_suggestion, style = "verbose", applicability = "maybe-incorrect")]
+    Suggestion {
+        #[suggestion_part(code = "let _ = ")]
+        start_span: Span,
+        #[suggestion_part(code = "")]
+        end_span: Span,
+    },
 }
 
 #[derive(LintDiagnostic)]

diff --git a/tests/ui/associated-types/defaults-unsound-62211-1.next.stderr b/tests/ui/associated-types/defaults-unsound-62211-1.next.stderr
@@ -6,8 +6,12 @@ LL |     drop(origin);
    |          |
    |          argument has type `<T as UncheckedCopy>::Output`
    |
-   = note: use `let _ = ...` to ignore the expression or result
    = note: `#[warn(dropping_copy_types)]` on by default
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(origin);
+LL +     let _ = origin;
+   |
 
 warning: 1 warning emitted
 
diff --git a/tests/ui/associated-types/defaults-unsound-62211-2.next.stderr b/tests/ui/associated-types/defaults-unsound-62211-2.next.stderr
@@ -6,8 +6,12 @@ LL |     drop(origin);
    |          |
    |          argument has type `<T as UncheckedCopy>::Output`
    |
-   = note: use `let _ = ...` to ignore the expression or result
    = note: `#[warn(dropping_copy_types)]` on by default
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(origin);
+LL +     let _ = origin;
+   |
 
 warning: 1 warning emitted
 
diff --git a/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs b/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs
@@ -0,0 +1,14 @@
+//@ check-fail
+
+#![deny(dropping_copy_types)]
+
+fn main() {
+    let y = 1;
+    let z = 2;
+    match y {
+        0 => drop(y), //~ ERROR calls to `std::mem::drop`
+        1 => drop(z), //~ ERROR calls to `std::mem::drop`
+        2 => drop(3), //~ ERROR calls to `std::mem::drop`
+        _ => {},
+    }
+}
diff --git a/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.stderr b/tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.stderr
@@ -0,0 +1,37 @@
+error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
+  --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:9:14
+   |
+LL |         0 => drop(y),
+   |              ^^^^^-^
+   |                   |
+   |                   argument has type `i32`
+   |
+   = note: use `let _ = ...` to ignore the expression or result
+note: the lint level is defined here
+  --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:3:9
+   |
+LL | #![deny(dropping_copy_types)]
+   |         ^^^^^^^^^^^^^^^^^^^
+
+error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
+  --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:10:14
+   |
+LL |         1 => drop(z),
+   |              ^^^^^-^
+   |                   |
+   |                   argument has type `i32`
+   |
+   = note: use `let _ = ...` to ignore the expression or result
+
+error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
+  --> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:11:14
+   |
+LL |         2 => drop(3),
+   |              ^^^^^-^
+   |                   |
+   |                   argument has type `i32`
+   |
+   = note: use `let _ = ...` to ignore the expression or result
+
+error: aborting due to 3 previous errors
+
diff --git a/tests/ui/lint/dropping_copy_types-issue-125189.fixed b/tests/ui/lint/dropping_copy_types-issue-125189.fixed
@@ -0,0 +1,10 @@
+//@ check-fail
+//@ run-rustfix
+
+#![deny(dropping_copy_types)]
+
+fn main() {
+    let y = 1;
+    let _ = 3.2; //~ ERROR calls to `std::mem::drop`
+    let _ = y; //~ ERROR calls to `std::mem::drop`
+}
diff --git a/tests/ui/lint/dropping_copy_types-issue-125189.rs b/tests/ui/lint/dropping_copy_types-issue-125189.rs
@@ -0,0 +1,10 @@
+//@ check-fail
+//@ run-rustfix
+
+#![deny(dropping_copy_types)]
+
+fn main() {
+    let y = 1;
+    drop(3.2); //~ ERROR calls to `std::mem::drop`
+    drop(y); //~ ERROR calls to `std::mem::drop`
+}
diff --git a/tests/ui/lint/dropping_copy_types-issue-125189.stderr b/tests/ui/lint/dropping_copy_types-issue-125189.stderr
@@ -0,0 +1,35 @@
+error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
+  --> $DIR/dropping_copy_types-issue-125189.rs:8:5
+   |
+LL |     drop(3.2);
+   |     ^^^^^---^
+   |          |
+   |          argument has type `f64`
+   |
+note: the lint level is defined here
+  --> $DIR/dropping_copy_types-issue-125189.rs:4:9
+   |
+LL | #![deny(dropping_copy_types)]
+   |         ^^^^^^^^^^^^^^^^^^^
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(3.2);
+LL +     let _ = 3.2;
+   |
+
+error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
+  --> $DIR/dropping_copy_types-issue-125189.rs:9:5
+   |
+LL |     drop(y);
+   |     ^^^^^-^
+   |          |
+   |          argument has type `i32`
+   |
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(y);
+LL +     let _ = y;
+   |
+
+error: aborting due to 2 previous errors
+
diff --git a/tests/ui/lint/dropping_copy_types.stderr b/tests/ui/lint/dropping_copy_types.stderr
@@ -6,12 +6,16 @@ LL |     drop(s1);
    |          |
    |          argument has type `SomeStruct`
    |
-   = note: use `let _ = ...` to ignore the expression or result
 note: the lint level is defined here
   --> $DIR/dropping_copy_types.rs:3:9
    |
 LL | #![warn(dropping_copy_types)]
    |         ^^^^^^^^^^^^^^^^^^^
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(s1);
+LL +     let _ = s1;
+   |
 
 warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
   --> $DIR/dropping_copy_types.rs:35:5
@@ -21,7 +25,11 @@ LL |     drop(s2);
    |          |
    |          argument has type `SomeStruct`
    |
-   = note: use `let _ = ...` to ignore the expression or result
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(s2);
+LL +     let _ = s2;
+   |
 
 warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing
   --> $DIR/dropping_copy_types.rs:36:5
@@ -42,7 +50,11 @@ LL |     drop(s4);
    |          |
    |          argument has type `SomeStruct`
    |
-   = note: use `let _ = ...` to ignore the expression or result
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -     drop(s4);
+LL +     let _ = s4;
+   |
 
 warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing
   --> $DIR/dropping_copy_types.rs:38:5
@@ -82,7 +94,11 @@ LL |             drop(println_and(13));
    |                  |
    |                  argument has type `i32`
    |
-   = note: use `let _ = ...` to ignore the expression or result
+help: use `let _ = ...` to ignore the expression or result
+   |
+LL -             drop(println_and(13));
+LL +             let _ = println_and(13);
+   |
 
 warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
   --> $DIR/dropping_copy_types.rs:74:14