Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2025-02-17 14:38:46 +00:00
Code Issues Projects Releases Packages Wiki Activity

[manual_assert]: Preserve comments in the suggestion

Browse source
This commit is contained in:
kraktus 2022-09-14 11:58:41 +02:00
parent 5d837b58f7
commit 2c04c1a188
7 changed files with 237 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

40
clippy_lints/src/manual_assert.rs
View file

@ -1,9 +1,10 @@
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_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;
@ -50,20 +51,33 @@ 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 (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(
let mut comments = span_extract_comment(cx.sess().source_map(), expr.span);
if !comments.is_empty() {
comments += "\n";
}
// we need to negate the <cond> expression because `assert!` panics when <cond> is `false`, wherease original pattern panicked when evaluating to `true`
let cond_sugg = !sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability);
let sugg = format!("assert!({cond_sugg}, {format_args_snip});");
// 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);
}
);
}
}

23
clippy_utils/src/lib.rs
View file

@ -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`

16
tests/ui/manual_assert.edition2018.fixed
View file

@ -5,6 +5,7 @@
#![warn(clippy::manual_assert)]
#![allow(clippy::nonminimal_bool)]
#![allow(dead_code)]
macro_rules! one {
() => {
@ -27,8 +28,8 @@ fn main() {
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
assert!(!(!a.is_empty()), "qaqaq{:?}", a);
assert!(!(!a.is_empty()), "qwqwq");
if a.len() == 3 {
println!("qwq");
println!("qwq");
@ -50,3 +51,14 @@ fn main() {
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
}
fn issue7730() {
// Suggestion should preserve comment
// comment
/* this is a
multiline
comment */
/// Doc comment
// comment after `panic!`
assert!(!true, "panic with comment");
}

90
tests/ui/manual_assert.edition2018.stderr
View file

@ -1,68 +1,124 @@
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:30:5
--> $DIR/manual_assert.rs:31:5
|
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
--> $DIR/manual_assert.rs:34:5
|
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
--> $DIR/manual_assert.rs:51:5
|
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
--> $DIR/manual_assert.rs:54:5
|
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
--> $DIR/manual_assert.rs:57:5
|
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
--> $DIR/manual_assert.rs:60:5
|
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
--> $DIR/manual_assert.rs:63:5
|
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
--> $DIR/manual_assert.rs:66:5
|
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:73:5
|
LL | / if true {
LL | | // comment
LL | | /* this is a
LL | | multiline
... |
LL | | panic!("panic with comment") // comment after `panic!`
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!true, "panic with comment");
|
error: aborting due to 9 previous errors

16
tests/ui/manual_assert.edition2021.fixed
View file

@ -5,6 +5,7 @@
#![warn(clippy::manual_assert)]
#![allow(clippy::nonminimal_bool)]
#![allow(dead_code)]
macro_rules! one {
() => {
@ -27,8 +28,8 @@ fn main() {
{
panic!("qaqaq{:?}", a);
}
assert!(a.is_empty(), "qaqaq{:?}", a);
assert!(a.is_empty(), "qwqwq");
assert!(!(!a.is_empty()), "qaqaq{:?}", a);
assert!(!(!a.is_empty()), "qwqwq");
if a.len() == 3 {
println!("qwq");
println!("qwq");
@ -50,3 +51,14 @@ fn main() {
assert!(!(a.is_empty() || !b.is_empty()), "panic5");
assert!(!a.is_empty(), "with expansion {}", one!());
}
fn issue7730() {
// Suggestion should preserve comment
// comment
/* this is a
multiline
comment */
/// Doc comment
// comment after `panic!`
assert!(!true, "panic with comment");
}

90
tests/ui/manual_assert.edition2021.stderr
View file

@ -1,68 +1,124 @@
error: only a `panic!` in `if`-then statement
--> $DIR/manual_assert.rs:30:5
--> $DIR/manual_assert.rs:31:5
|
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
--> $DIR/manual_assert.rs:34:5
|
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
--> $DIR/manual_assert.rs:51:5
|
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
--> $DIR/manual_assert.rs:54:5
|
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
--> $DIR/manual_assert.rs:57:5
|
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
--> $DIR/manual_assert.rs:60:5
|
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
--> $DIR/manual_assert.rs:63:5
|
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
--> $DIR/manual_assert.rs:66:5
|
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:73:5
|
LL | / if true {
LL | | // comment
LL | | /* this is a
LL | | multiline
... |
LL | | panic!("panic with comment") // comment after `panic!`
LL | | }
| |_____^
|
help: try instead
|
LL | assert!(!true, "panic with comment");
|
error: aborting due to 9 previous errors

13
tests/ui/manual_assert.rs
View file

@ -5,6 +5,7 @@
#![warn(clippy::manual_assert)]
#![allow(clippy::nonminimal_bool)]
#![allow(dead_code)]
macro_rules! one {
() => {
@ -66,3 +67,15 @@ fn main() {
panic!("with expansion {}", one!())
}
}
fn issue7730() {
// Suggestion should preserve comment
if true {
// comment
/* this is a
multiline
comment */
/// Doc comment
panic!("panic with comment") // comment after `panic!`
}
}