fix: zero_repeat_side_effects misses curlies
This commit is contained in:
yanglsh
Linshu Yang
parent
a10cafebcd
commit
f2af204d17
4 changed files with 151 additions and 31 deletions
|
|
@ -4,10 +4,11 @@ use clippy_utils::source::{snippet, snippet_indent};
|
|||
use rustc_ast::LitKind;
|
||||
use rustc_data_structures::packed::Pu128;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{ConstArgKind, ExprKind, Node};
|
||||
use rustc_hir::{ConstArgKind, Expr, ExprKind, LetStmt, LocalSource, Node};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::IsSuggestable;
|
||||
use rustc_middle::ty::{IsSuggestable, Ty};
|
||||
use rustc_session::declare_lint_pass;
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
|
|
@ -44,7 +45,7 @@ declare_clippy_lint! {
|
|||
declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]);
|
||||
|
||||
impl LateLintPass<'_> for ZeroRepeatSideEffects {
|
||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &rustc_hir::Expr<'_>) {
|
||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
||||
if let Some(args) = VecArgs::hir(cx, expr)
|
||||
&& let VecArgs::Repeat(inner_expr, len) = args
|
||||
&& let ExprKind::Lit(l) = len.kind
|
||||
|
|
@ -69,7 +70,7 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects {
|
|||
}
|
||||
}
|
||||
|
||||
fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) {
|
||||
fn inner_check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, inner_expr: &'_ Expr<'_>, is_vec: bool) {
|
||||
// check if expr is a call or has a call inside it
|
||||
if inner_expr.can_have_side_effects() {
|
||||
let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id);
|
||||
|
|
@ -81,19 +82,22 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr:
|
|||
let vec = if is_vec { "vec!" } else { "" };
|
||||
|
||||
let (span, sugg) = match parent_hir_node {
|
||||
Node::LetStmt(l) => (
|
||||
l.span,
|
||||
format!(
|
||||
"{inner_expr};\n{indent}let {var_name}: {return_type} = {vec}[];",
|
||||
var_name = snippet(cx, l.pat.span.source_callsite(), "..")
|
||||
),
|
||||
),
|
||||
Node::LetStmt(l)
|
||||
if matches!(l.source, LocalSource::AssignDesugar)
|
||||
&& let mut parent_iter = cx.tcx.hir_parent_iter(l.hir_id)
|
||||
&& let Some((_, Node::Stmt(_))) = parent_iter.next()
|
||||
&& let Some((_, Node::Block(_))) = parent_iter.next()
|
||||
&& let Some((_, Node::Expr(x))) = parent_iter.next() =>
|
||||
{
|
||||
(
|
||||
x.span,
|
||||
assign_expr_suggestion(cx, x, l.pat.span, &inner_expr, return_type, vec),
|
||||
)
|
||||
},
|
||||
Node::LetStmt(l) => (l.span, let_stmt_suggestion(cx, l, &inner_expr, return_type, vec)),
|
||||
Node::Expr(x) if let ExprKind::Assign(l, _, _) = x.kind => (
|
||||
x.span,
|
||||
format!(
|
||||
"{inner_expr};\n{indent}{var_name} = {vec}[] as {return_type}",
|
||||
var_name = snippet(cx, l.span.source_callsite(), "..")
|
||||
),
|
||||
assign_expr_suggestion(cx, x, l.span, &inner_expr, return_type, vec),
|
||||
),
|
||||
// NOTE: don't use the stmt span to avoid touching the trailing semicolon
|
||||
Node::Stmt(_) => (expr.span, format!("{inner_expr};\n{indent}{vec}[] as {return_type}")),
|
||||
|
|
@ -131,3 +135,41 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr:
|
|||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn let_stmt_suggestion(
|
||||
cx: &LateContext<'_>,
|
||||
let_stmt: &LetStmt<'_>,
|
||||
inner_expr: &str,
|
||||
return_type: Ty<'_>,
|
||||
vec_str: &str,
|
||||
) -> String {
|
||||
let indent = snippet_indent(cx, let_stmt.span).unwrap_or_default();
|
||||
format!(
|
||||
"{inner_expr};\n{}let {var_name}: {return_type} = {vec_str}[];",
|
||||
indent,
|
||||
var_name = snippet(cx, let_stmt.pat.span.source_callsite(), "..")
|
||||
)
|
||||
}
|
||||
|
||||
fn assign_expr_suggestion(
|
||||
cx: &LateContext<'_>,
|
||||
outer_expr: &Expr<'_>,
|
||||
assign_expr_span: Span,
|
||||
inner_expr: &str,
|
||||
return_type: Ty<'_>,
|
||||
vec_str: &str,
|
||||
) -> String {
|
||||
let mut parent_hir_node = cx.tcx.parent_hir_node(outer_expr.hir_id);
|
||||
if let Node::Stmt(stmt) = parent_hir_node {
|
||||
parent_hir_node = cx.tcx.parent_hir_node(stmt.hir_id);
|
||||
}
|
||||
let needs_curly = !matches!(parent_hir_node, Node::Block(_));
|
||||
|
||||
let indent = snippet_indent(cx, outer_expr.span).unwrap_or_default();
|
||||
let var_name = snippet(cx, assign_expr_span.source_callsite(), "..");
|
||||
if needs_curly {
|
||||
format!("{{\n {indent}{inner_expr};\n {indent}{var_name} = {vec_str}[] as {return_type}\n{indent}}}",)
|
||||
} else {
|
||||
format!("{inner_expr};\n{indent}{var_name} = {vec_str}[] as {return_type}")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,11 @@
|
|||
#![warn(clippy::zero_repeat_side_effects)]
|
||||
#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)]
|
||||
#![allow(clippy::no_effect)] // only fires _after_ the fix
|
||||
#![allow(
|
||||
clippy::unnecessary_operation,
|
||||
clippy::useless_vec,
|
||||
clippy::needless_late_init,
|
||||
clippy::single_match,
|
||||
clippy::no_effect // only fires _after_ the fix
|
||||
)]
|
||||
|
||||
fn f() -> i32 {
|
||||
println!("side effect");
|
||||
|
|
@ -119,3 +124,26 @@ fn issue_14681() {
|
|||
});
|
||||
//~^ zero_repeat_side_effects
|
||||
}
|
||||
|
||||
fn issue_15824() {
|
||||
fn f() {}
|
||||
|
||||
match 0 {
|
||||
0 => {
|
||||
f();
|
||||
_ = [] as [(); 0]
|
||||
},
|
||||
//~^ zero_repeat_side_effects
|
||||
_ => {},
|
||||
}
|
||||
|
||||
let mut a = [(); 0];
|
||||
match 0 {
|
||||
0 => {
|
||||
f();
|
||||
a = [] as [(); 0]
|
||||
},
|
||||
//~^ zero_repeat_side_effects
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,11 @@
|
|||
#![warn(clippy::zero_repeat_side_effects)]
|
||||
#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)]
|
||||
#![allow(clippy::no_effect)] // only fires _after_ the fix
|
||||
#![allow(
|
||||
clippy::unnecessary_operation,
|
||||
clippy::useless_vec,
|
||||
clippy::needless_late_init,
|
||||
clippy::single_match,
|
||||
clippy::no_effect // only fires _after_ the fix
|
||||
)]
|
||||
|
||||
fn f() -> i32 {
|
||||
println!("side effect");
|
||||
|
|
@ -102,3 +107,20 @@ fn issue_14681() {
|
|||
foo(&[Some(Some(S::new())); 0]);
|
||||
//~^ zero_repeat_side_effects
|
||||
}
|
||||
|
||||
fn issue_15824() {
|
||||
fn f() {}
|
||||
|
||||
match 0 {
|
||||
0 => _ = [f(); 0],
|
||||
//~^ zero_repeat_side_effects
|
||||
_ => {},
|
||||
}
|
||||
|
||||
let mut a = [(); 0];
|
||||
match 0 {
|
||||
0 => a = [f(); 0],
|
||||
//~^ zero_repeat_side_effects
|
||||
_ => {},
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:17:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:22:5
|
||||
|
|
||||
LL | let a = [f(); 0];
|
||||
| ^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -13,7 +13,7 @@ LL + let a: [i32; 0] = [];
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:20:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:25:5
|
||||
|
|
||||
LL | b = [f(); 0];
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
@ -25,7 +25,7 @@ LL ~ b = [] as [i32; 0];
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:25:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:30:5
|
||||
|
|
||||
LL | let c = vec![f(); 0];
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -37,7 +37,7 @@ LL + let c: std::vec::Vec<i32> = vec![];
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:28:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:33:5
|
||||
|
|
||||
LL | d = vec![f(); 0];
|
||||
| ^^^^^^^^^^^^^^^^
|
||||
|
|
@ -49,7 +49,7 @@ LL ~ d = vec![] as std::vec::Vec<i32>;
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:32:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:37:5
|
||||
|
|
||||
LL | let e = [println!("side effect"); 0];
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -61,7 +61,7 @@ LL + let e: [(); 0] = [];
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:36:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:41:5
|
||||
|
|
||||
LL | let g = [{ f() }; 0];
|
||||
| ^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -73,7 +73,7 @@ LL + let g: [i32; 0] = [];
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:40:10
|
||||
--> tests/ui/zero_repeat_side_effects.rs:45:10
|
||||
|
|
||||
LL | drop(vec![f(); 0]);
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
@ -87,7 +87,7 @@ LL ~ });
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:44:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:49:5
|
||||
|
|
||||
LL | vec![f(); 0];
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
@ -99,7 +99,7 @@ LL ~ vec![] as std::vec::Vec<i32>;
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:46:5
|
||||
--> tests/ui/zero_repeat_side_effects.rs:51:5
|
||||
|
|
||||
LL | [f(); 0];
|
||||
| ^^^^^^^^
|
||||
|
|
@ -111,7 +111,7 @@ LL ~ [] as [i32; 0];
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:100:10
|
||||
--> tests/ui/zero_repeat_side_effects.rs:105:10
|
||||
|
|
||||
LL | foo(&[Some(f()); 0]);
|
||||
| ^^^^^^^^^^^^^^
|
||||
|
|
@ -125,7 +125,7 @@ LL ~ });
|
|||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:102:10
|
||||
--> tests/ui/zero_repeat_side_effects.rs:107:10
|
||||
|
|
||||
LL | foo(&[Some(Some(S::new())); 0]);
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
@ -138,5 +138,33 @@ LL + [] as [std::option::Option<std::option::Option<S>>; 0]
|
|||
LL ~ });
|
||||
|
|
||||
|
||||
error: aborting due to 11 previous errors
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:115:14
|
||||
|
|
||||
LL | 0 => _ = [f(); 0],
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
help: consider performing the side effect separately
|
||||
|
|
||||
LL ~ 0 => {
|
||||
LL + f();
|
||||
LL + _ = [] as [(); 0]
|
||||
LL ~ },
|
||||
|
|
||||
|
||||
error: expression with side effects as the initial value in a zero-sized array initializer
|
||||
--> tests/ui/zero_repeat_side_effects.rs:122:14
|
||||
|
|
||||
LL | 0 => a = [f(); 0],
|
||||
| ^^^^^^^^^^^^
|
||||
|
|
||||
help: consider performing the side effect separately
|
||||
|
|
||||
LL ~ 0 => {
|
||||
LL + f();
|
||||
LL + a = [] as [(); 0]
|
||||
LL ~ },
|
||||
|
|
||||
|
||||
error: aborting due to 13 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue