From 6b12829943aa617b17965b1c25d898d09406eab6 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sat, 23 Mar 2024 01:27:14 +0100 Subject: [PATCH 1/2] Move `is_parent_stmt` to `clippy_utils` --- clippy_lints/src/needless_bool.rs | 12 +++--------- clippy_utils/src/lib.rs | 9 +++++++++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 166a7f71d69..081d14c043c 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -6,11 +6,12 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::{ - higher, is_else_clause, is_expn_of, peel_blocks, peel_blocks_with_stmt, span_extract_comment, SpanlessEq, + higher, is_else_clause, is_expn_of, is_parent_stmt, peel_blocks, peel_blocks_with_stmt, span_extract_comment, + SpanlessEq, }; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Node, UnOp}; +use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use rustc_span::source_map::Spanned; @@ -135,13 +136,6 @@ fn condition_needs_parentheses(e: &Expr<'_>) -> bool { false } -fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool { - matches!( - cx.tcx.parent_hir_node(id), - Node::Stmt(..) | Node::Block(Block { stmts: &[], .. }) - ) -} - impl<'tcx> LateLintPass<'tcx> for NeedlessBool { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { use self::Expression::{Bool, RetBool}; diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index a6faac62f1c..5babc024a6e 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -3335,3 +3335,12 @@ fn maybe_get_relative_path(from: &DefPath, to: &DefPath, max_super: usize) -> St repeat(String::from("super")).take(go_up_by).chain(path).join("::") } } + +/// Returns true if the specified `HirId` is the top-level expression of a statement or the only +/// expression in a block. +pub fn is_parent_stmt(cx: &LateContext<'_>, id: HirId) -> bool { + matches!( + cx.tcx.parent_hir_node(id), + Node::Stmt(..) | Node::Block(Block { stmts: &[], .. }) + ) +} From c137c78ba2718134ad7937ee4ad5034c296b70df Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sat, 23 Mar 2024 01:32:25 +0100 Subject: [PATCH 2/2] `manual_assert`: do not add extra semicolon --- clippy_lints/src/manual_assert.rs | 5 +++-- tests/ui/manual_assert.edition2018.fixed | 8 ++++++++ tests/ui/manual_assert.edition2018.stderr | 11 ++++++++++- tests/ui/manual_assert.edition2021.fixed | 8 ++++++++ tests/ui/manual_assert.edition2021.stderr | 11 ++++++++++- tests/ui/manual_assert.rs | 10 ++++++++++ 6 files changed, 49 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs index 4f6a2cf017c..76edbe8b755 100644 --- a/clippy_lints/src/manual_assert.rs +++ b/clippy_lints/src/manual_assert.rs @@ -1,7 +1,7 @@ use crate::rustc_lint::LintContext; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::macros::root_macro_call; -use clippy_utils::{is_else_clause, peel_blocks_with_stmt, span_extract_comment, sugg}; +use clippy_utils::{is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; @@ -63,7 +63,8 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { _ => (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});"); + let semicolon = if is_parent_stmt(cx, expr.hir_id) { ";" } else { "" }; + let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip}){semicolon}"); // we show to the user the suggestion without the comments, but when applying the fix, include the // comments in the block span_lint_and_then( diff --git a/tests/ui/manual_assert.edition2018.fixed b/tests/ui/manual_assert.edition2018.fixed index 75beedfa450..75835780801 100644 --- a/tests/ui/manual_assert.edition2018.fixed +++ b/tests/ui/manual_assert.edition2018.fixed @@ -66,3 +66,11 @@ fn issue7730(a: u8) { // comment after `panic!` assert!(!(a > 2), "panic with comment"); } + +fn issue12505() { + struct Foo(T); + + impl Foo { + const BAR: () = assert!(!(N == 0), ); + } +} diff --git a/tests/ui/manual_assert.edition2018.stderr b/tests/ui/manual_assert.edition2018.stderr index 57015933d40..1eebe1bfe17 100644 --- a/tests/ui/manual_assert.edition2018.stderr +++ b/tests/ui/manual_assert.edition2018.stderr @@ -82,5 +82,14 @@ help: try instead LL | assert!(!(a > 2), "panic with comment"); | -error: aborting due to 9 previous errors +error: only a `panic!` in `if`-then statement + --> tests/ui/manual_assert.rs:91:25 + | +LL | const BAR: () = if N == 0 { + | _________________________^ +LL | | panic!() +LL | | }; + | |_________^ help: try instead: `assert!(!(N == 0), )` + +error: aborting due to 10 previous errors diff --git a/tests/ui/manual_assert.edition2021.fixed b/tests/ui/manual_assert.edition2021.fixed index 75beedfa450..75835780801 100644 --- a/tests/ui/manual_assert.edition2021.fixed +++ b/tests/ui/manual_assert.edition2021.fixed @@ -66,3 +66,11 @@ fn issue7730(a: u8) { // comment after `panic!` assert!(!(a > 2), "panic with comment"); } + +fn issue12505() { + struct Foo(T); + + impl Foo { + const BAR: () = assert!(!(N == 0), ); + } +} diff --git a/tests/ui/manual_assert.edition2021.stderr b/tests/ui/manual_assert.edition2021.stderr index 57015933d40..1eebe1bfe17 100644 --- a/tests/ui/manual_assert.edition2021.stderr +++ b/tests/ui/manual_assert.edition2021.stderr @@ -82,5 +82,14 @@ help: try instead LL | assert!(!(a > 2), "panic with comment"); | -error: aborting due to 9 previous errors +error: only a `panic!` in `if`-then statement + --> tests/ui/manual_assert.rs:91:25 + | +LL | const BAR: () = if N == 0 { + | _________________________^ +LL | | panic!() +LL | | }; + | |_________^ help: try instead: `assert!(!(N == 0), )` + +error: aborting due to 10 previous errors diff --git a/tests/ui/manual_assert.rs b/tests/ui/manual_assert.rs index 5979496ca83..363bafdf05d 100644 --- a/tests/ui/manual_assert.rs +++ b/tests/ui/manual_assert.rs @@ -83,3 +83,13 @@ fn issue7730(a: u8) { panic!("panic with comment") // comment after `panic!` } } + +fn issue12505() { + struct Foo(T); + + impl Foo { + const BAR: () = if N == 0 { + panic!() + }; + } +}