diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs
index 26b53ab5d68..825ec84b4a8 100644
--- a/clippy_lints/src/manual_assert.rs
+++ b/clippy_lints/src/manual_assert.rs
@@ -1,7 +1,8 @@
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use crate::rustc_lint::LintContext;
+use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{root_macro_call, FormatArgsExpn};
use clippy_utils::source::snippet_with_applicability;
-use clippy_utils::{peel_blocks_with_stmt, sugg};
+use clippy_utils::{peel_blocks_with_stmt, span_extract_comment, sugg};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
@@ -50,20 +51,36 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert {
let mut applicability = Applicability::MachineApplicable;
let format_args_snip = snippet_with_applicability(cx, format_args.inputs_span(), "..", &mut applicability);
let cond = cond.peel_drop_temps();
+ let mut comments = span_extract_comment(cx.sess().source_map(), expr.span);
+ if !comments.is_empty() {
+ comments += "\n";
+ }
let (cond, not) = match cond.kind {
ExprKind::Unary(UnOp::Not, e) => (e, ""),
_ => (cond, "!"),
};
let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par();
let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip});");
- span_lint_and_sugg(
+ // we show to the user the suggestion without the comments, but when applicating the fix, include the comments in the block
+ span_lint_and_then(
cx,
MANUAL_ASSERT,
expr.span,
"only a `panic!` in `if`-then statement",
- "try",
- sugg,
- Applicability::MachineApplicable,
+ |diag| {
+ // comments can be noisy, do not show them to the user
+ diag.tool_only_span_suggestion(
+ expr.span.shrink_to_lo(),
+ "add comments back",
+ comments,
+ applicability);
+ diag.span_suggestion(
+ expr.span,
+ "try instead",
+ sugg,
+ applicability);
+ }
+
);
}
}
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index c2c52d08a3c..d67ceaec035 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -2295,6 +2295,29 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool {
});
}
+/// Return all the comments a given span contains
+/// Comments are returned wrapped with their relevant delimiters
+pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String {
+ let snippet = sm.span_to_snippet(span).unwrap_or_default();
+ let mut comments_buf: Vec<String> = Vec::new();
+ let mut index: usize = 0;
+
+ for token in tokenize(&snippet) {
+ let token_range = index..(index + token.len as usize);
+ index += token.len as usize;
+ match token.kind {
+ TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => {
+ if let Some(comment) = snippet.get(token_range) {
+ comments_buf.push(comment.to_string());
+ }
+ },
+ _ => (),
+ }
+ }
+
+ comments_buf.join("\n")
+}
+
macro_rules! op_utils {
($($name:ident $assign:ident)*) => {
/// Binary operation traits like `LangItem::Add`
diff --git a/tests/ui/manual_assert.edition2018.fixed b/tests/ui/manual_assert.edition2018.fixed
index 65598f1eacc..84f6855f338 100644
--- a/tests/ui/manual_assert.edition2018.fixed
+++ b/tests/ui/manual_assert.edition2018.fixed
@@ -4,7 +4,7 @@
// run-rustfix
#![warn(clippy::manual_assert)]
-#![allow(clippy::nonminimal_bool)]
+#![allow(dead_code, unused_doc_comments, clippy::nonminimal_bool)]
macro_rules! one {
() => {
@@ -50,3 +50,14 @@ fn main() {
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
}
+
+fn issue7730(a: u8) {
+ // Suggestion should preserve comment
+ // comment
+/* this is a
+ multiline
+ comment */
+/// Doc comment
+// comment after `panic!`
+assert!(!(a > 2), "panic with comment");
+}
diff --git a/tests/ui/manual_assert.edition2018.stderr b/tests/ui/manual_assert.edition2018.stderr
index a0f31afd6eb..dbd21be2da9 100644
--- a/tests/ui/manual_assert.edition2018.stderr
+++ b/tests/ui/manual_assert.edition2018.stderr
@@ -4,9 +4,13 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
- | |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
+ | |_____^
|
= note: `-D clippy::manual-assert` implied by `-D warnings`
+help: try instead
+ |
+LL | assert!(a.is_empty(), "qaqaq{:?}", a);
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:33:5
@@ -14,7 +18,12 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
- | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(a.is_empty(), "qwqwq");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:50:5
@@ -22,7 +31,12 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
- | |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!b.is_empty(), "panic1");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:53:5
@@ -30,7 +44,12 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
- | |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(b.is_empty() && a.is_empty()), "panic2");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:56:5
@@ -38,7 +57,12 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
- | |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:59:5
@@ -46,7 +70,12 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
- | |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(b.is_empty() || a.is_empty()), "panic4");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:62:5
@@ -54,7 +83,12 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
- | |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:65:5
@@ -62,7 +96,29 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() {
LL | | panic!("with expansion {}", one!())
LL | | }
- | |_____^ help: try: `assert!(!a.is_empty(), "with expansion {}", one!());`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!a.is_empty(), "with expansion {}", one!());
+ |
-error: aborting due to 8 previous errors
+error: only a `panic!` in `if`-then statement
+ --> $DIR/manual_assert.rs:72:5
+ |
+LL | / if a > 2 {
+LL | | // comment
+LL | | /* this is a
+LL | | multiline
+... |
+LL | | panic!("panic with comment") // comment after `panic!`
+LL | | }
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(a > 2), "panic with comment");
+ |
+
+error: aborting due to 9 previous errors
diff --git a/tests/ui/manual_assert.edition2021.fixed b/tests/ui/manual_assert.edition2021.fixed
index 65598f1eacc..84f6855f338 100644
--- a/tests/ui/manual_assert.edition2021.fixed
+++ b/tests/ui/manual_assert.edition2021.fixed
@@ -4,7 +4,7 @@
// run-rustfix
#![warn(clippy::manual_assert)]
-#![allow(clippy::nonminimal_bool)]
+#![allow(dead_code, unused_doc_comments, clippy::nonminimal_bool)]
macro_rules! one {
() => {
@@ -50,3 +50,14 @@ fn main() {
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
}
+
+fn issue7730(a: u8) {
+ // Suggestion should preserve comment
+ // comment
+/* this is a
+ multiline
+ comment */
+/// Doc comment
+// comment after `panic!`
+assert!(!(a > 2), "panic with comment");
+}
diff --git a/tests/ui/manual_assert.edition2021.stderr b/tests/ui/manual_assert.edition2021.stderr
index a0f31afd6eb..dbd21be2da9 100644
--- a/tests/ui/manual_assert.edition2021.stderr
+++ b/tests/ui/manual_assert.edition2021.stderr
@@ -4,9 +4,13 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qaqaq{:?}", a);
LL | | }
- | |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);`
+ | |_____^
|
= note: `-D clippy::manual-assert` implied by `-D warnings`
+help: try instead
+ |
+LL | assert!(a.is_empty(), "qaqaq{:?}", a);
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:33:5
@@ -14,7 +18,12 @@ error: only a `panic!` in `if`-then statement
LL | / if !a.is_empty() {
LL | | panic!("qwqwq");
LL | | }
- | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(a.is_empty(), "qwqwq");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:50:5
@@ -22,7 +31,12 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() {
LL | | panic!("panic1");
LL | | }
- | |_____^ help: try: `assert!(!b.is_empty(), "panic1");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!b.is_empty(), "panic1");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:53:5
@@ -30,7 +44,12 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() && a.is_empty() {
LL | | panic!("panic2");
LL | | }
- | |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(b.is_empty() && a.is_empty()), "panic2");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:56:5
@@ -38,7 +57,12 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() && !b.is_empty() {
LL | | panic!("panic3");
LL | | }
- | |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:59:5
@@ -46,7 +70,12 @@ error: only a `panic!` in `if`-then statement
LL | / if b.is_empty() || a.is_empty() {
LL | | panic!("panic4");
LL | | }
- | |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(b.is_empty() || a.is_empty()), "panic4");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:62:5
@@ -54,7 +83,12 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() || !b.is_empty() {
LL | | panic!("panic5");
LL | | }
- | |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5");
+ |
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:65:5
@@ -62,7 +96,29 @@ error: only a `panic!` in `if`-then statement
LL | / if a.is_empty() {
LL | | panic!("with expansion {}", one!())
LL | | }
- | |_____^ help: try: `assert!(!a.is_empty(), "with expansion {}", one!());`
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!a.is_empty(), "with expansion {}", one!());
+ |
-error: aborting due to 8 previous errors
+error: only a `panic!` in `if`-then statement
+ --> $DIR/manual_assert.rs:72:5
+ |
+LL | / if a > 2 {
+LL | | // comment
+LL | | /* this is a
+LL | | multiline
+... |
+LL | | panic!("panic with comment") // comment after `panic!`
+LL | | }
+ | |_____^
+ |
+help: try instead
+ |
+LL | assert!(!(a > 2), "panic with comment");
+ |
+
+error: aborting due to 9 previous errors
diff --git a/tests/ui/manual_assert.rs b/tests/ui/manual_assert.rs
index 4d2706dd621..14abf94965a 100644
--- a/tests/ui/manual_assert.rs
+++ b/tests/ui/manual_assert.rs
@@ -4,7 +4,7 @@
// run-rustfix
#![warn(clippy::manual_assert)]
-#![allow(clippy::nonminimal_bool)]
+#![allow(dead_code, unused_doc_comments, clippy::nonminimal_bool)]
macro_rules! one {
() => {
@@ -66,3 +66,15 @@ fn main() {
panic!("with expansion {}", one!())
}
}
+
+fn issue7730(a: u8) {
+ // Suggestion should preserve comment
+ if a > 2 {
+ // comment
+ /* this is a
+ multiline
+ comment */
+ /// Doc comment
+ panic!("panic with comment") // comment after `panic!`
+ }
+}