From 8c79f7840d399e3de52c63b1951a46adf6dda515 Mon Sep 17 00:00:00 2001 From: dswij Date: Tue, 7 Nov 2023 17:57:31 +0800 Subject: [PATCH 01/71] `read_zero_byte_vec` refactor for better heuristics --- clippy_lints/src/read_zero_byte_vec.rs | 118 +++++++++++++++---------- tests/ui/read_zero_byte_vec.rs | 29 ++++-- tests/ui/read_zero_byte_vec.stderr | 34 ++++--- 3 files changed, 112 insertions(+), 69 deletions(-) diff --git a/clippy_lints/src/read_zero_byte_vec.rs b/clippy_lints/src/read_zero_byte_vec.rs index b27d4cc6e4f8c..f5a8287d2e09f 100644 --- a/clippy_lints/src/read_zero_byte_vec.rs +++ b/clippy_lints/src/read_zero_byte_vec.rs @@ -1,11 +1,13 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; +use clippy_utils::get_enclosing_block; use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; use clippy_utils::source::snippet; -use clippy_utils::visitors::for_each_expr; -use core::ops::ControlFlow; -use hir::{Expr, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind}; + +use hir::{Expr, ExprKind, HirId, Local, PatKind, PathSegment, QPath, StmtKind}; use rustc_errors::Applicability; use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -49,57 +51,40 @@ declare_lint_pass!(ReadZeroByteVec => [READ_ZERO_BYTE_VEC]); impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { fn check_block(&mut self, cx: &LateContext<'tcx>, block: &hir::Block<'tcx>) { - for (idx, stmt) in block.stmts.iter().enumerate() { - if !stmt.span.from_expansion() - // matches `let v = Vec::new();` - && let StmtKind::Local(local) = stmt.kind - && let Local { pat, init: Some(init), .. } = local - && let PatKind::Binding(_, _, ident, _) = pat.kind + for stmt in block.stmts { + if stmt.span.from_expansion() { + return; + } + + if let StmtKind::Local(local) = stmt.kind + && let Local { + pat, init: Some(init), .. + } = local + && let PatKind::Binding(_, id, ident, _) = pat.kind && let Some(vec_init_kind) = get_vec_init_kind(cx, init) { - let visitor = |expr: &Expr<'_>| { - if let ExprKind::MethodCall(path, _, [arg], _) = expr.kind - && let PathSegment { - ident: read_or_read_exact, - .. - } = *path - && matches!(read_or_read_exact.as_str(), "read" | "read_exact") - && let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind - && let ExprKind::Path(QPath::Resolved(None, inner_path)) = inner.kind - && let [inner_seg] = inner_path.segments - && ident.name == inner_seg.ident.name - { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } + let mut visitor = ReadVecVisitor { + local_id: id, + read_zero_expr: None, + has_resize: false, }; - let (read_found, next_stmt_span) = if let Some(next_stmt) = block.stmts.get(idx + 1) { - // case { .. stmt; stmt; .. } - (for_each_expr(next_stmt, visitor).is_some(), next_stmt.span) - } else if let Some(e) = block.expr { - // case { .. stmt; expr } - (for_each_expr(e, visitor).is_some(), e.span) - } else { + let Some(enclosing_block) = get_enclosing_block(cx, id) else { return; }; + visitor.visit_block(enclosing_block); - if read_found && !next_stmt_span.from_expansion() { + if let Some(expr) = visitor.read_zero_expr { let applicability = Applicability::MaybeIncorrect; match vec_init_kind { VecInitKind::WithConstCapacity(len) => { span_lint_and_sugg( cx, READ_ZERO_BYTE_VEC, - next_stmt_span, + expr.span, "reading zero byte data to `Vec`", "try", - format!( - "{}.resize({len}, 0); {}", - ident.as_str(), - snippet(cx, next_stmt_span, "..") - ), + format!("{}.resize({len}, 0); {}", ident.as_str(), snippet(cx, expr.span, "..")), applicability, ); }, @@ -108,25 +93,20 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { span_lint_and_sugg( cx, READ_ZERO_BYTE_VEC, - next_stmt_span, + expr.span, "reading zero byte data to `Vec`", "try", format!( "{}.resize({}, 0); {}", ident.as_str(), snippet(cx, e.span, ".."), - snippet(cx, next_stmt_span, "..") + snippet(cx, expr.span, "..") ), applicability, ); }, _ => { - span_lint( - cx, - READ_ZERO_BYTE_VEC, - next_stmt_span, - "reading zero byte data to `Vec`", - ); + span_lint(cx, READ_ZERO_BYTE_VEC, expr.span, "reading zero byte data to `Vec`"); }, } } @@ -134,3 +114,47 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { } } } + +struct ReadVecVisitor<'tcx> { + local_id: HirId, + read_zero_expr: Option<&'tcx Expr<'tcx>>, + has_resize: bool, +} + +impl<'tcx> Visitor<'tcx> for ReadVecVisitor<'tcx> { + fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) { + if let ExprKind::MethodCall(path, receiver, args, _) = e.kind { + let PathSegment { ident, .. } = *path; + + match ident.as_str() { + "read" | "read_exact" => { + let [arg] = args else { return }; + if let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind + && let ExprKind::Path(QPath::Resolved(None, inner_path)) = inner.kind + && let [inner_seg] = inner_path.segments + && let Res::Local(res_id) = inner_seg.res + && self.local_id == res_id + { + self.read_zero_expr = Some(e); + return; + } + }, + "resize" => { + // If the Vec is resized, then it's a valid read + if let ExprKind::Path(QPath::Resolved(_, inner_path)) = receiver.kind + && let Res::Local(res_id) = inner_path.res + && self.local_id == res_id + { + self.has_resize = true; + return; + } + }, + _ => {}, + } + } + + if !self.has_resize && self.read_zero_expr.is_none() { + walk_expr(self, e); + } + } +} diff --git a/tests/ui/read_zero_byte_vec.rs b/tests/ui/read_zero_byte_vec.rs index 76b9b98185119..fd5a88a37a66c 100644 --- a/tests/ui/read_zero_byte_vec.rs +++ b/tests/ui/read_zero_byte_vec.rs @@ -55,14 +55,6 @@ fn test() -> io::Result<()> { let mut buf = [0u8; 100]; f.read(&mut buf)?; - // should not lint - let mut empty = vec![]; - let mut data7 = vec![]; - f.read(&mut empty); - - // should not lint - f.read(&mut data7); - // should not lint let mut data8 = Vec::new(); data8.resize(100, 0); @@ -75,6 +67,27 @@ fn test() -> io::Result<()> { Ok(()) } +fn test_nested() -> io::Result<()> { + let cap = 1000; + let mut f = File::open("foo.txt").unwrap(); + + // Issue #9274 + // Should not lint + let mut v = Vec::new(); + { + v.resize(10, 0); + f.read(&mut v)?; + } + + let mut v = Vec::new(); + { + f.read(&mut v)?; + //~^ ERROR: reading zero byte data to `Vec` + } + + Ok(()) +} + async fn test_futures(r: &mut R) { // should lint let mut data = Vec::new(); diff --git a/tests/ui/read_zero_byte_vec.stderr b/tests/ui/read_zero_byte_vec.stderr index 523ecb2948df3..e85aa051c34bc 100644 --- a/tests/ui/read_zero_byte_vec.stderr +++ b/tests/ui/read_zero_byte_vec.stderr @@ -2,7 +2,7 @@ error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:21:5 | LL | f.read_exact(&mut data).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data).unwrap();` + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data)` | = note: `-D clippy::read-zero-byte-vec` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::read_zero_byte_vec)]` @@ -11,19 +11,19 @@ error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:27:5 | LL | f.read_exact(&mut data2)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); f.read_exact(&mut data2)?;` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); f.read_exact(&mut data2)` error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:32:5 | LL | f.read_exact(&mut data3)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:37:5 + --> $DIR/read_zero_byte_vec.rs:37:13 | LL | let _ = f.read(&mut data4)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:43:9 @@ -38,28 +38,34 @@ LL | f.read(&mut data6) | ^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:81:5 + --> $DIR/read_zero_byte_vec.rs:84:9 + | +LL | f.read(&mut v)?; + | ^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:94:5 | LL | r.read(&mut data).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:86:5 + --> $DIR/read_zero_byte_vec.rs:99:5 | LL | r.read_exact(&mut data2).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:93:5 + --> $DIR/read_zero_byte_vec.rs:106:5 | LL | r.read(&mut data).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:98:5 + --> $DIR/read_zero_byte_vec.rs:111:5 | LL | r.read_exact(&mut data2).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 10 previous errors +error: aborting due to 11 previous errors From 543d56938e23865eb70b49f153c71a2057668c79 Mon Sep 17 00:00:00 2001 From: Aneesh Kadiyala <143342960+ARandomDev99@users.noreply.github.com> Date: Tue, 2 Jan 2024 11:30:20 +0530 Subject: [PATCH 02/71] Make `HirEqInterExpr::eq_block` take comments into account This commit: - now makes `HirEqInterExpr::eq_block` take comments into account. Identical code with varying comments will no longer be considered equal. - makes necessary adjustments to UI tests. --- clippy_utils/src/hir_utils.rs | 6 +- .../ui/branches_sharing_code/shared_at_top.rs | 8 +-- .../shared_at_top.stderr | 6 +- .../branches_sharing_code/valid_if_blocks.rs | 12 ++-- .../valid_if_blocks.stderr | 18 ++--- tests/ui/if_same_then_else.rs | 28 +++----- tests/ui/if_same_then_else.stderr | 69 +++++++------------ tests/ui/if_same_then_else2.rs | 18 ++--- tests/ui/if_same_then_else2.stderr | 37 ++++------ tests/ui/match_same_arms2.rs | 4 +- tests/ui/match_same_arms2.stderr | 5 +- 11 files changed, 73 insertions(+), 138 deletions(-) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index e610ed930505b..ea8d3631b0d27 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -134,7 +134,7 @@ impl HirEqInterExpr<'_, '_, '_> { /// Checks whether two blocks are the same. #[expect(clippy::similar_names)] fn eq_block(&mut self, left: &Block<'_>, right: &Block<'_>) -> bool { - use TokenKind::{BlockComment, LineComment, Semi, Whitespace}; + use TokenKind::{Semi, Whitespace}; if left.stmts.len() != right.stmts.len() { return false; } @@ -177,7 +177,7 @@ impl HirEqInterExpr<'_, '_, '_> { return false; } if !eq_span_tokens(self.inner.cx, lstart..lstmt_span.lo, rstart..rstmt_span.lo, |t| { - !matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi) + !matches!(t, Whitespace | Semi) }) { return false; } @@ -212,7 +212,7 @@ impl HirEqInterExpr<'_, '_, '_> { return false; } eq_span_tokens(self.inner.cx, lstart..lend, rstart..rend, |t| { - !matches!(t, Whitespace | LineComment { .. } | BlockComment { .. } | Semi) + !matches!(t, Whitespace | Semi) }) } diff --git a/tests/ui/branches_sharing_code/shared_at_top.rs b/tests/ui/branches_sharing_code/shared_at_top.rs index 44f8b2eabce3b..9af81f6f7cdda 100644 --- a/tests/ui/branches_sharing_code/shared_at_top.rs +++ b/tests/ui/branches_sharing_code/shared_at_top.rs @@ -9,17 +9,16 @@ fn simple_examples() { // Simple if true { - //~^ ERROR: all if blocks contain the same code at the start println!("Hello World!"); println!("I'm branch nr: 1"); } else { println!("Hello World!"); println!("I'm branch nr: 2"); } + //~^^^^^^^ ERROR: all if blocks contain the same code at the start // Else if if x == 0 { - //~^ ERROR: all if blocks contain the same code at the start let y = 9; println!("The value y was set to: `{}`", y); let _z = y; @@ -38,6 +37,7 @@ fn simple_examples() { println!("Ha, Pascal allows you to start the array where you want") } + //~^^^^^^^^^^^^^^^^^^^ ERROR: all if blocks contain the same code at the start // Return a value let _ = if x == 7 { @@ -60,7 +60,6 @@ fn simple_but_suggestion_is_invalid() { // Can't be automatically moved because used_value_name is getting used again let used_value_name = 19; if x == 10 { - //~^ ERROR: all if blocks contain the same code at the start let used_value_name = "Different type"; println!("Str: {}", used_value_name); let _ = 1; @@ -69,6 +68,7 @@ fn simple_but_suggestion_is_invalid() { println!("Str: {}", used_value_name); let _ = 2; } + //~^^^^^^^^^ ERROR: all if blocks contain the same code at the start let _ = used_value_name; // This can be automatically moved as `can_be_overridden` is not used again @@ -101,11 +101,11 @@ fn check_if_same_than_else_mask() { } if x == 2019 { - //~^ ERROR: this `if` has identical blocks println!("This should trigger `IS_SAME_THAN_ELSE` as usual"); } else { println!("This should trigger `IS_SAME_THAN_ELSE` as usual"); } + //~^^^^^ ERROR: this `if` has identical blocks } #[allow(clippy::vec_init_then_push)] diff --git a/tests/ui/branches_sharing_code/shared_at_top.stderr b/tests/ui/branches_sharing_code/shared_at_top.stderr index 9d4d42fb689e4..317d1577226ca 100644 --- a/tests/ui/branches_sharing_code/shared_at_top.stderr +++ b/tests/ui/branches_sharing_code/shared_at_top.stderr @@ -2,7 +2,6 @@ error: all if blocks contain the same code at the start --> $DIR/shared_at_top.rs:11:5 | LL | / if true { -LL | | LL | | println!("Hello World!"); | |_________________________________^ | @@ -21,7 +20,6 @@ error: all if blocks contain the same code at the start --> $DIR/shared_at_top.rs:21:5 | LL | / if x == 0 { -LL | | LL | | let y = 9; LL | | println!("The value y was set to: `{}`", y); LL | | let _z = y; @@ -54,7 +52,6 @@ error: all if blocks contain the same code at the start --> $DIR/shared_at_top.rs:62:5 | LL | / if x == 10 { -LL | | LL | | let used_value_name = "Different type"; LL | | println!("Str: {}", used_value_name); | |_____________________________________________^ @@ -105,13 +102,12 @@ error: this `if` has identical blocks | LL | if x == 2019 { | __________________^ -LL | | LL | | println!("This should trigger `IS_SAME_THAN_ELSE` as usual"); LL | | } else { | |_____^ | note: same as this - --> $DIR/shared_at_top.rs:106:12 + --> $DIR/shared_at_top.rs:105:12 | LL | } else { | ____________^ diff --git a/tests/ui/branches_sharing_code/valid_if_blocks.rs b/tests/ui/branches_sharing_code/valid_if_blocks.rs index 2aeacb89c0cb6..b63819d7c3932 100644 --- a/tests/ui/branches_sharing_code/valid_if_blocks.rs +++ b/tests/ui/branches_sharing_code/valid_if_blocks.rs @@ -107,9 +107,9 @@ fn valid_examples() { // Let's test empty blocks if false { - //~^ ERROR: this `if` has identical blocks } else { } + //~^^^ ERROR: this `if` has identical blocks } /// This makes sure that the `if_same_then_else` masks the `shared_code_in_if_blocks` lint @@ -119,7 +119,6 @@ fn trigger_other_lint() { // Same block if x == 0 { - //~^ ERROR: this `if` has identical blocks let u = 19; println!("How are u today?"); let _ = "This is a string"; @@ -128,6 +127,7 @@ fn trigger_other_lint() { println!("How are u today?"); let _ = "This is a string"; } + //~^^^^^^^^^ ERROR: this `if` has identical blocks // Only same expression let _ = if x == 6 { 7 } else { 7 }; @@ -138,28 +138,24 @@ fn trigger_other_lint() { println!("Well I'm the most important block"); "I'm a pretty string" } else if x == 68 { - //~^ ERROR: this `if` has identical blocks println!("I'm a doppelgänger"); - // Don't listen to my clone below if y == 90 { "=^.^=" } else { ":D" } } else { - // Don't listen to my clone above println!("I'm a doppelgänger"); if y == 90 { "=^.^=" } else { ":D" } }; + //~^^^^^^^^^ ERROR: this `if` has identical blocks if x == 0 { println!("I'm single"); } else if x == 68 { - //~^ ERROR: this `if` has identical blocks println!("I'm a doppelgänger"); - // Don't listen to my clone below } else { - // Don't listen to my clone above println!("I'm a doppelgänger"); } + //~^^^^^ ERROR: this `if` has identical blocks } fn main() {} diff --git a/tests/ui/branches_sharing_code/valid_if_blocks.stderr b/tests/ui/branches_sharing_code/valid_if_blocks.stderr index fcbf12235aa16..0daf2ff6967cd 100644 --- a/tests/ui/branches_sharing_code/valid_if_blocks.stderr +++ b/tests/ui/branches_sharing_code/valid_if_blocks.stderr @@ -3,12 +3,11 @@ error: this `if` has identical blocks | LL | if false { | ______________^ -LL | | LL | | } else { | |_____^ | note: same as this - --> $DIR/valid_if_blocks.rs:111:12 + --> $DIR/valid_if_blocks.rs:110:12 | LL | } else { | ____________^ @@ -25,7 +24,6 @@ error: this `if` has identical blocks | LL | if x == 0 { | _______________^ -LL | | LL | | let u = 19; LL | | println!("How are u today?"); LL | | let _ = "This is a string"; @@ -33,7 +31,7 @@ LL | | } else { | |_____^ | note: same as this - --> $DIR/valid_if_blocks.rs:126:12 + --> $DIR/valid_if_blocks.rs:125:12 | LL | } else { | ____________^ @@ -60,20 +58,17 @@ error: this `if` has identical blocks | LL | } else if x == 68 { | _______________________^ -LL | | LL | | println!("I'm a doppelgänger"); -LL | | // Don't listen to my clone below LL | | LL | | if y == 90 { "=^.^=" } else { ":D" } LL | | } else { | |_____^ | note: same as this - --> $DIR/valid_if_blocks.rs:146:12 + --> $DIR/valid_if_blocks.rs:144:12 | LL | } else { | ____________^ -LL | | // Don't listen to my clone above LL | | println!("I'm a doppelgänger"); LL | | LL | | if y == 90 { "=^.^=" } else { ":D" } @@ -81,22 +76,19 @@ LL | | }; | |_____^ error: this `if` has identical blocks - --> $DIR/valid_if_blocks.rs:155:23 + --> $DIR/valid_if_blocks.rs:153:23 | LL | } else if x == 68 { | _______________________^ -LL | | LL | | println!("I'm a doppelgänger"); -LL | | // Don't listen to my clone below LL | | } else { | |_____^ | note: same as this - --> $DIR/valid_if_blocks.rs:159:12 + --> $DIR/valid_if_blocks.rs:155:12 | LL | } else { | ____________^ -LL | | // Don't listen to my clone above LL | | println!("I'm a doppelgänger"); LL | | } | |_____^ diff --git a/tests/ui/if_same_then_else.rs b/tests/ui/if_same_then_else.rs index e84b20e9fef07..d53e1383d845c 100644 --- a/tests/ui/if_same_then_else.rs +++ b/tests/ui/if_same_then_else.rs @@ -21,7 +21,6 @@ fn foo() -> bool { fn if_same_then_else() { if true { - //~^ ERROR: this `if` has identical blocks Foo { bar: 42 }; 0..10; ..; @@ -38,6 +37,7 @@ fn if_same_then_else() { 0..=10; foo(); } + //~^^^^^^^^^^^^^^^^^ ERROR: this `if` has identical blocks if true { Foo { bar: 42 }; @@ -64,19 +64,11 @@ fn if_same_then_else() { foo(); } - let _ = if true { - //~^ ERROR: this `if` has identical blocks - 0.0 - } else { - 0.0 - }; + let _ = if true { 0.0 } else { 0.0 }; + //~^ ERROR: this `if` has identical blocks - let _ = if true { - //~^ ERROR: this `if` has identical blocks - -0.0 - } else { - -0.0 - }; + let _ = if true { -0.0 } else { -0.0 }; + //~^ ERROR: this `if` has identical blocks let _ = if true { 0.0 } else { -0.0 }; @@ -87,15 +79,10 @@ fn if_same_then_else() { foo(); } - let _ = if true { - //~^ ERROR: this `if` has identical blocks - 42 - } else { - 42 - }; + let _ = if true { 42 } else { 42 }; + //~^ ERROR: this `if` has identical blocks if true { - //~^ ERROR: this `if` has identical blocks let bar = if true { 42 } else { 43 }; while foo() { @@ -110,6 +97,7 @@ fn if_same_then_else() { } bar + 1; } + //~^^^^^^^^^^^^^^^ ERROR: this `if` has identical blocks if true { let _ = match 42 { diff --git a/tests/ui/if_same_then_else.stderr b/tests/ui/if_same_then_else.stderr index fb33e94e6c3de..281f30f88b46d 100644 --- a/tests/ui/if_same_then_else.stderr +++ b/tests/ui/if_same_then_else.stderr @@ -3,16 +3,16 @@ error: this `if` has identical blocks | LL | if true { | _____________^ -LL | | LL | | Foo { bar: 42 }; LL | | 0..10; +LL | | ..; ... | LL | | foo(); LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else.rs:32:12 + --> $DIR/if_same_then_else.rs:31:12 | LL | } else { | ____________^ @@ -29,75 +29,54 @@ LL | | } error: this `if` has identical blocks --> $DIR/if_same_then_else.rs:67:21 | -LL | let _ = if true { - | _____________________^ -LL | | -LL | | 0.0 -LL | | } else { - | |_____^ +LL | let _ = if true { 0.0 } else { 0.0 }; + | ^^^^^^^ | note: same as this - --> $DIR/if_same_then_else.rs:70:12 + --> $DIR/if_same_then_else.rs:67:34 | -LL | } else { - | ____________^ -LL | | 0.0 -LL | | }; - | |_____^ +LL | let _ = if true { 0.0 } else { 0.0 }; + | ^^^^^^^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:74:21 + --> $DIR/if_same_then_else.rs:70:21 | -LL | let _ = if true { - | _____________________^ -LL | | -LL | | -0.0 -LL | | } else { - | |_____^ +LL | let _ = if true { -0.0 } else { -0.0 }; + | ^^^^^^^^ | note: same as this - --> $DIR/if_same_then_else.rs:77:12 + --> $DIR/if_same_then_else.rs:70:35 | -LL | } else { - | ____________^ -LL | | -0.0 -LL | | }; - | |_____^ +LL | let _ = if true { -0.0 } else { -0.0 }; + | ^^^^^^^^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:90:21 + --> $DIR/if_same_then_else.rs:82:21 | -LL | let _ = if true { - | _____________________^ -LL | | -LL | | 42 -LL | | } else { - | |_____^ +LL | let _ = if true { 42 } else { 42 }; + | ^^^^^^ | note: same as this - --> $DIR/if_same_then_else.rs:93:12 + --> $DIR/if_same_then_else.rs:82:33 | -LL | } else { - | ____________^ -LL | | 42 -LL | | }; - | |_____^ +LL | let _ = if true { 42 } else { 42 }; + | ^^^^^^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:97:13 + --> $DIR/if_same_then_else.rs:85:13 | LL | if true { | _____________^ -LL | | LL | | let bar = if true { 42 } else { 43 }; LL | | +LL | | while foo() { ... | LL | | bar + 1; LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else.rs:105:12 + --> $DIR/if_same_then_else.rs:92:12 | LL | } else { | ____________^ @@ -110,7 +89,7 @@ LL | | } | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else.rs:250:14 + --> $DIR/if_same_then_else.rs:238:14 | LL | if x { | ______________^ @@ -119,7 +98,7 @@ LL | | } else { | |_________^ | note: same as this - --> $DIR/if_same_then_else.rs:252:16 + --> $DIR/if_same_then_else.rs:240:16 | LL | } else { | ________________^ diff --git a/tests/ui/if_same_then_else2.rs b/tests/ui/if_same_then_else2.rs index 0b171f21d0cc4..e23c77b082744 100644 --- a/tests/ui/if_same_then_else2.rs +++ b/tests/ui/if_same_then_else2.rs @@ -13,7 +13,6 @@ fn if_same_then_else2() -> Result<&'static str, ()> { if true { - //~^ ERROR: this `if` has identical blocks for _ in &[42] { let foo: &Option<_> = &Some::(42); if foo.is_some() { @@ -32,20 +31,21 @@ fn if_same_then_else2() -> Result<&'static str, ()> { } } } + //~^^^^^^^^^^^^^^^^^^^ ERROR: this `if` has identical blocks if true { - //~^ ERROR: this `if` has identical blocks if let Some(a) = Some(42) {} } else { if let Some(a) = Some(42) {} } + //~^^^^^ ERROR: this `if` has identical blocks if true { - //~^ ERROR: this `if` has identical blocks if let (1, .., 3) = (1, 2, 3) {} } else { if let (1, .., 3) = (1, 2, 3) {} } + //~^^^^^ ERROR: this `if` has identical blocks if true { if let (1, .., 3) = (1, 2, 3) {} @@ -90,19 +90,15 @@ fn if_same_then_else2() -> Result<&'static str, ()> { } // Same NaNs - let _ = if true { - //~^ ERROR: this `if` has identical blocks - f32::NAN - } else { - f32::NAN - }; + let _ = if true { f32::NAN } else { f32::NAN }; + //~^ ERROR: this `if` has identical blocks if true { - //~^ ERROR: this `if` has identical blocks Ok("foo")?; } else { Ok("foo")?; } + //~^^^^^ ERROR: this `if` has identical blocks if true { let foo = ""; @@ -122,13 +118,13 @@ fn if_same_then_else2() -> Result<&'static str, ()> { let foo = "bar"; return Ok(&foo[0..]); } else if true { - //~^ ERROR: this `if` has identical blocks let foo = ""; return Ok(&foo[0..]); } else { let foo = ""; return Ok(&foo[0..]); } + //~^^^^^^^ ERROR: this `if` has identical blocks // False positive `if_same_then_else`: `let (x, y)` vs. `let (y, x)`; see issue #3559. if true { diff --git a/tests/ui/if_same_then_else2.stderr b/tests/ui/if_same_then_else2.stderr index fe68ef2711b59..4e7a7c87dc54b 100644 --- a/tests/ui/if_same_then_else2.stderr +++ b/tests/ui/if_same_then_else2.stderr @@ -3,16 +3,16 @@ error: this `if` has identical blocks | LL | if true { | _____________^ -LL | | LL | | for _ in &[42] { LL | | let foo: &Option<_> = &Some::(42); +LL | | if foo.is_some() { ... | LL | | } LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:25:12 + --> $DIR/if_same_then_else2.rs:24:12 | LL | } else { | ____________^ @@ -31,13 +31,12 @@ error: this `if` has identical blocks | LL | if true { | _____________^ -LL | | LL | | if let Some(a) = Some(42) {} LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:39:12 + --> $DIR/if_same_then_else2.rs:38:12 | LL | } else { | ____________^ @@ -50,13 +49,12 @@ error: this `if` has identical blocks | LL | if true { | _____________^ -LL | | LL | | if let (1, .., 3) = (1, 2, 3) {} LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:46:12 + --> $DIR/if_same_then_else2.rs:45:12 | LL | } else { | ____________^ @@ -67,34 +65,26 @@ LL | | } error: this `if` has identical blocks --> $DIR/if_same_then_else2.rs:93:21 | -LL | let _ = if true { - | _____________________^ -LL | | -LL | | f32::NAN -LL | | } else { - | |_____^ +LL | let _ = if true { f32::NAN } else { f32::NAN }; + | ^^^^^^^^^^^^ | note: same as this - --> $DIR/if_same_then_else2.rs:96:12 + --> $DIR/if_same_then_else2.rs:93:39 | -LL | } else { - | ____________^ -LL | | f32::NAN -LL | | }; - | |_____^ +LL | let _ = if true { f32::NAN } else { f32::NAN }; + | ^^^^^^^^^^^^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:100:13 + --> $DIR/if_same_then_else2.rs:96:13 | LL | if true { | _____________^ -LL | | LL | | Ok("foo")?; LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:103:12 + --> $DIR/if_same_then_else2.rs:98:12 | LL | } else { | ____________^ @@ -103,18 +93,17 @@ LL | | } | |_____^ error: this `if` has identical blocks - --> $DIR/if_same_then_else2.rs:124:20 + --> $DIR/if_same_then_else2.rs:120:20 | LL | } else if true { | ____________________^ -LL | | LL | | let foo = ""; LL | | return Ok(&foo[0..]); LL | | } else { | |_____^ | note: same as this - --> $DIR/if_same_then_else2.rs:128:12 + --> $DIR/if_same_then_else2.rs:123:12 | LL | } else { | ____________^ diff --git a/tests/ui/match_same_arms2.rs b/tests/ui/match_same_arms2.rs index 525a355f40355..3428ff4590602 100644 --- a/tests/ui/match_same_arms2.rs +++ b/tests/ui/match_same_arms2.rs @@ -13,7 +13,6 @@ fn foo() -> bool { fn match_same_arms() { let _ = match 42 { 42 => { - //~^ ERROR: this match arm has an identical body to the `_` wildcard arm foo(); let mut a = 42 + [23].len() as i32; if true { @@ -32,6 +31,7 @@ fn match_same_arms() { a }, }; + //~^^^^^^^^^^^^^^^^^^^ ERROR: this match arm has an identical body to the `_` wildcard arm let _ = match 42 { 42 => foo(), @@ -146,13 +146,13 @@ fn match_same_arms() { empty!(0); }, 1 => { - //~^ ERROR: this match arm has an identical body to another arm empty!(0); }, x => { empty!(x); }, } + //~^^^^^^^ ERROR: this match arm has an identical body to another arm match_expr_like_matches_macro_priority(); } diff --git a/tests/ui/match_same_arms2.stderr b/tests/ui/match_same_arms2.stderr index 40b20c7e16d28..512ca7413a795 100644 --- a/tests/ui/match_same_arms2.stderr +++ b/tests/ui/match_same_arms2.stderr @@ -2,9 +2,9 @@ error: this match arm has an identical body to the `_` wildcard arm --> $DIR/match_same_arms2.rs:15:9 | LL | / 42 => { -LL | | LL | | foo(); LL | | let mut a = 42 + [23].len() as i32; +LL | | if true { ... | LL | | a LL | | }, @@ -12,7 +12,7 @@ LL | | }, | = help: or try changing either arm body note: `_` wildcard arm here - --> $DIR/match_same_arms2.rs:25:9 + --> $DIR/match_same_arms2.rs:24:9 | LL | / _ => { LL | | foo(); @@ -122,7 +122,6 @@ LL | 1 => { | ^ help: try merging the arm patterns: `1 | 0` | _________| | | -LL | | LL | | empty!(0); LL | | }, | |_________^ From c2d071c11c008b08d841d30bae40f8acfdac0c6e Mon Sep 17 00:00:00 2001 From: Eugene Date: Tue, 9 Jan 2024 06:50:17 -0800 Subject: [PATCH 03/71] Documentation edits for arc_with_non_send_sync Arc's documentation uses the term "thread", aligning to that terminology. Fix typo in "Rc". --- clippy_lints/src/arc_with_non_send_sync.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/arc_with_non_send_sync.rs b/clippy_lints/src/arc_with_non_send_sync.rs index 657d52d0e9e17..58738d5287883 100644 --- a/clippy_lints/src/arc_with_non_send_sync.rs +++ b/clippy_lints/src/arc_with_non_send_sync.rs @@ -14,8 +14,8 @@ declare_clippy_lint! { /// This lint warns when you use `Arc` with a type that does not implement `Send` or `Sync`. /// /// ### Why is this bad? - /// `Arc` is an Atomic `RC` and guarantees that updates to the reference counter are - /// Atomic. This is useful in multiprocessing scenarios. To send an `Arc` across processes + /// `Arc` is an Atomic `Rc` and guarantees that updates to the reference counter are + /// Atomic. This is useful in multithreading scenarios. To send an `Arc` across threads /// and make use of the atomic ref counter, `T` must be [both `Send` and `Sync`](https://doc.rust-lang.org/std/sync/struct.Arc.html#impl-Send-for-Arc%3CT%3E), /// either `T` should be made `Send + Sync` or an `Rc` should be used instead of an `Arc` /// From 7c389acd277eada4d2e0856707361cce24c9b23f Mon Sep 17 00:00:00 2001 From: cocodery Date: Thu, 11 Jan 2024 14:52:03 +0800 Subject: [PATCH 04/71] Fix error warning span for issue12045 --- clippy_lints/src/no_effect.rs | 2 +- tests/ui/no_effect.stderr | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 6bbe427ea2988..2a2c2b331ba79 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -155,7 +155,7 @@ fn check_no_effect(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { cx, NO_EFFECT_UNDERSCORE_BINDING, init.hir_id, - stmt.span, + ident.span, "binding to `_` prefixed variable with no side-effect", ); return true; diff --git a/tests/ui/no_effect.stderr b/tests/ui/no_effect.stderr index f5ba234b4cbf6..8140ba1feee9c 100644 --- a/tests/ui/no_effect.stderr +++ b/tests/ui/no_effect.stderr @@ -152,31 +152,31 @@ LL | FooString { s: s }; | ^^^^^^^^^^^^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:175:5 + --> $DIR/no_effect.rs:175:9 | LL | let _unused = 1; - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^ | = note: `-D clippy::no-effect-underscore-binding` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::no_effect_underscore_binding)]` error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:178:5 + --> $DIR/no_effect.rs:178:9 | LL | let _penguin = || println!("Some helpful closure"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:180:5 + --> $DIR/no_effect.rs:180:9 | LL | let _duck = Struct { field: 0 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ error: binding to `_` prefixed variable with no side-effect - --> $DIR/no_effect.rs:182:5 + --> $DIR/no_effect.rs:182:9 | LL | let _cat = [2, 4, 6, 8][2]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ error: aborting due to 29 previous errors From fb83ef649587413c4e3a8ec9e364b828ec250a56 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 11 Jan 2024 14:22:44 +1100 Subject: [PATCH 05/71] Stop using `DiagnosticBuilder::buffer` in the parser. One consequence is that errors returned by `maybe_new_parser_from_source_str` now must be consumed, so a bunch of places that previously ignored those errors now cancel them. (Most of them explicitly dropped the errors before. I guess that was to indicate "we are explicitly ignoring these", though I'm not 100% sure.) --- clippy_lints/src/doc/needless_doctest_main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/doc/needless_doctest_main.rs b/clippy_lints/src/doc/needless_doctest_main.rs index a744b69ecb478..8b018220c1715 100644 --- a/clippy_lints/src/doc/needless_doctest_main.rs +++ b/clippy_lints/src/doc/needless_doctest_main.rs @@ -53,7 +53,7 @@ pub fn check( let mut parser = match maybe_new_parser_from_source_str(&sess, filename, code) { Ok(p) => p, Err(errs) => { - drop(errs); + errs.into_iter().for_each(|err| err.cancel()); return (false, test_attr_spans); }, }; From aa220c7ee7a51ab39bccd03ee90fa1c9365a589a Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Thu, 11 Jan 2024 17:27:03 +0100 Subject: [PATCH 06/71] Merge commit '26ac6aab023393c94edf42f38f6ad31196009643' --- .github/workflows/clippy.yml | 2 +- .github/workflows/clippy_bors.yml | 10 +- .github/workflows/clippy_dev.yml | 2 +- .github/workflows/deploy.yml | 4 +- .github/workflows/remark.yml | 2 +- CHANGELOG.md | 7 + COPYRIGHT | 2 +- LICENSE-APACHE | 2 +- LICENSE-MIT | 2 +- README.md | 4 +- book/src/README.md | 2 +- book/src/SUMMARY.md | 1 + .../continuous_integration/github_actions.md | 2 +- book/src/continuous_integration/gitlab.md | 16 + book/src/development/macro_expansions.md | 2 +- book/src/development/type_checking.md | 4 +- book/src/lint_configuration.md | 10 + clippy_config/src/conf.rs | 7 +- clippy_config/src/metadata.rs | 3 + clippy_config/src/msrvs.rs | 2 +- clippy_config/src/types.rs | 6 + clippy_dev/src/new_lint.rs | 2 +- clippy_lints/src/casts/cast_sign_loss.rs | 141 ++++-- clippy_lints/src/declared_lints.rs | 8 +- clippy_lints/src/default_numeric_fallback.rs | 41 +- ...ith_brackets.rs => empty_with_brackets.rs} | 56 ++- clippy_lints/src/if_then_some_else_none.rs | 7 +- clippy_lints/src/instant_subtraction.rs | 5 +- clippy_lints/src/item_name_repetitions.rs | 5 + clippy_lints/src/iter_without_into_iter.rs | 9 +- clippy_lints/src/lib.rs | 17 +- clippy_lints/src/lines_filter_map_ok.rs | 3 +- clippy_lints/src/methods/filter_map.rs | 3 +- clippy_lints/src/methods/iter_filter.rs | 206 +++++++-- .../src/methods/manual_is_variant_and.rs | 59 +++ clippy_lints/src/methods/map_clone.rs | 142 ++++-- clippy_lints/src/methods/mod.rs | 115 ++++- .../src/methods/option_as_ref_cloned.rs | 24 + .../src/methods/option_map_unwrap_or.rs | 2 +- .../src/methods/path_ends_with_ext.rs | 2 +- clippy_lints/src/methods/redundant_as_str.rs | 6 +- clippy_lints/src/methods/str_split.rs | 38 ++ .../src/methods/unnecessary_to_owned.rs | 24 +- clippy_lints/src/methods/useless_asref.rs | 123 +++++- clippy_lints/src/methods/utils.rs | 1 + .../src/missing_asserts_for_indexing.rs | 2 +- .../src/missing_enforced_import_rename.rs | 14 +- clippy_lints/src/mutex_atomic.rs | 26 +- clippy_lints/src/no_effect.rs | 2 +- .../src/non_octal_unix_permissions.rs | 12 +- clippy_lints/src/operators/identity_op.rs | 208 +++++---- clippy_lints/src/pub_underscore_fields.rs | 83 ++++ clippy_lints/src/question_mark.rs | 7 +- clippy_lints/src/serde_api.rs | 2 +- ...ead_local_initializer_can_be_made_const.rs | 102 +++++ clippy_lints/src/transmute/eager_transmute.rs | 53 ++- .../src/transmute/transmute_undefined_repr.rs | 4 +- clippy_lints/src/unconditional_recursion.rs | 413 +++++++++++++++--- .../src/undocumented_unsafe_blocks.rs | 10 +- clippy_lints/src/unit_types/let_unit_value.rs | 21 +- clippy_lints/src/vec.rs | 17 +- clippy_lints/src/wildcard_imports.rs | 2 +- clippy_utils/src/eager_or_lazy.rs | 13 +- clippy_utils/src/hir_utils.rs | 2 +- clippy_utils/src/macros.rs | 2 +- clippy_utils/src/ty/type_certainty/mod.rs | 6 +- rust-toolchain | 2 +- rustc_tools_util/README.md | 2 +- .../all_pub_fields/clippy.toml | 1 + .../exported/clippy.toml | 1 + ...ub_underscore_fields.all_pub_fields.stderr | 60 +++ .../pub_underscore_fields.exported.stderr | 12 + .../pub_underscore_fields.rs | 66 +++ .../toml_unknown_key/conf_unknown_key.stderr | 2 + tests/ui/as_conversions.rs | 2 +- tests/ui/cast.rs | 49 +++ tests/ui/cast.stderr | 74 +++- tests/ui/crashes/ice-8821.fixed | 10 - tests/ui/crashes/ice-8821.rs | 2 - tests/ui/crashes/ice-8821.stderr | 11 - tests/ui/default_numeric_fallback_f64.fixed | 4 +- tests/ui/default_numeric_fallback_f64.rs | 2 - tests/ui/default_numeric_fallback_f64.stderr | 26 +- tests/ui/default_numeric_fallback_i32.fixed | 36 +- tests/ui/default_numeric_fallback_i32.rs | 34 +- tests/ui/default_numeric_fallback_i32.stderr | 44 +- tests/ui/eager_transmute.fixed | 35 +- tests/ui/eager_transmute.rs | 35 +- tests/ui/eager_transmute.stderr | 117 ++++- .../empty_enum_variants_with_brackets.fixed | 27 ++ tests/ui/empty_enum_variants_with_brackets.rs | 27 ++ .../empty_enum_variants_with_brackets.stderr | 36 ++ tests/ui/identity_op.fixed | 94 +++- tests/ui/identity_op.rs | 94 +++- tests/ui/identity_op.stderr | 154 +++++-- tests/ui/if_then_some_else_none.rs | 5 + tests/ui/into_iter_without_iter.rs | 39 ++ tests/ui/into_iter_without_iter.stderr | 38 +- tests/ui/iter_filter_is_ok.fixed | 193 +++++++- tests/ui/iter_filter_is_ok.rs | 193 +++++++- tests/ui/iter_filter_is_ok.stderr | 74 +++- tests/ui/iter_filter_is_some.fixed | 231 +++++++++- tests/ui/iter_filter_is_some.rs | 231 +++++++++- tests/ui/iter_filter_is_some.stderr | 62 ++- tests/ui/iter_without_into_iter.rs | 31 ++ tests/ui/iter_without_into_iter.stderr | 40 +- tests/ui/let_unit.fixed | 50 +-- tests/ui/let_unit.rs | 48 +- tests/ui/let_unit.stderr | 56 +-- tests/ui/manual_is_variant_and.fixed | 51 +++ tests/ui/manual_is_variant_and.rs | 57 +++ tests/ui/manual_is_variant_and.stderr | 82 ++++ tests/ui/map_clone.fixed | 24 + tests/ui/map_clone.rs | 24 + tests/ui/map_clone.stderr | 44 +- tests/ui/must_use_candidates.fixed | 6 +- tests/ui/must_use_candidates.rs | 6 +- tests/ui/must_use_candidates.stderr | 10 +- tests/ui/mutex_atomic.rs | 19 +- tests/ui/mutex_atomic.stderr | 30 +- tests/ui/non_octal_unix_permissions.fixed | 4 + tests/ui/non_octal_unix_permissions.rs | 4 + tests/ui/non_octal_unix_permissions.stderr | 2 +- tests/ui/option_as_ref_cloned.fixed | 21 + tests/ui/option_as_ref_cloned.rs | 21 + tests/ui/option_as_ref_cloned.stderr | 37 ++ tests/ui/redundant_as_str.fixed | 2 +- tests/ui/redundant_as_str.rs | 2 +- tests/ui/regex.rs | 2 +- tests/ui/single_char_pattern.fixed | 2 + tests/ui/single_char_pattern.rs | 2 + tests/ui/single_char_pattern.stderr | 26 +- tests/ui/str_split.fixed | 145 ++++++ tests/ui/str_split.rs | 145 ++++++ tests/ui/str_split.stderr | 65 +++ tests/ui/struct_fields.rs | 24 +- tests/ui/struct_fields.stderr | 28 +- ..._local_initializer_can_be_made_const.fixed | 30 ++ ...ead_local_initializer_can_be_made_const.rs | 30 ++ ...local_initializer_can_be_made_const.stderr | 29 ++ tests/ui/unconditional_recursion.rs | 108 ++++- tests/ui/unconditional_recursion.stderr | 95 +++- tests/ui/unnecessary_lazy_eval.fixed | 13 + tests/ui/unnecessary_lazy_eval.rs | 13 + tests/ui/unnecessary_lazy_eval.stderr | 126 +++--- tests/ui/unnecessary_safety_comment.rs | 21 + tests/ui/unnecessary_to_owned_on_split.fixed | 18 +- tests/ui/unnecessary_to_owned_on_split.rs | 18 +- tests/ui/unnecessary_to_owned_on_split.stderr | 24 +- tests/ui/useless_asref.fixed | 14 +- tests/ui/useless_asref.rs | 14 +- tests/ui/useless_asref.stderr | 42 +- tests/ui/useless_conversion.fixed | 4 +- tests/ui/useless_conversion.rs | 4 +- tests/ui/vec.fixed | 7 + tests/ui/vec.rs | 7 + triagebot.toml | 1 - 157 files changed, 5166 insertions(+), 819 deletions(-) create mode 100644 book/src/continuous_integration/gitlab.md rename clippy_lints/src/{empty_structs_with_brackets.rs => empty_with_brackets.rs} (62%) create mode 100644 clippy_lints/src/methods/manual_is_variant_and.rs create mode 100644 clippy_lints/src/methods/option_as_ref_cloned.rs create mode 100644 clippy_lints/src/methods/str_split.rs create mode 100644 clippy_lints/src/pub_underscore_fields.rs create mode 100644 clippy_lints/src/thread_local_initializer_can_be_made_const.rs create mode 100644 tests/ui-toml/pub_underscore_fields/all_pub_fields/clippy.toml create mode 100644 tests/ui-toml/pub_underscore_fields/exported/clippy.toml create mode 100644 tests/ui-toml/pub_underscore_fields/pub_underscore_fields.all_pub_fields.stderr create mode 100644 tests/ui-toml/pub_underscore_fields/pub_underscore_fields.exported.stderr create mode 100644 tests/ui-toml/pub_underscore_fields/pub_underscore_fields.rs delete mode 100644 tests/ui/crashes/ice-8821.fixed delete mode 100644 tests/ui/crashes/ice-8821.stderr create mode 100644 tests/ui/empty_enum_variants_with_brackets.fixed create mode 100644 tests/ui/empty_enum_variants_with_brackets.rs create mode 100644 tests/ui/empty_enum_variants_with_brackets.stderr create mode 100644 tests/ui/manual_is_variant_and.fixed create mode 100644 tests/ui/manual_is_variant_and.rs create mode 100644 tests/ui/manual_is_variant_and.stderr create mode 100644 tests/ui/option_as_ref_cloned.fixed create mode 100644 tests/ui/option_as_ref_cloned.rs create mode 100644 tests/ui/option_as_ref_cloned.stderr create mode 100644 tests/ui/str_split.fixed create mode 100644 tests/ui/str_split.rs create mode 100644 tests/ui/str_split.stderr create mode 100644 tests/ui/thread_local_initializer_can_be_made_const.fixed create mode 100644 tests/ui/thread_local_initializer_can_be_made_const.rs create mode 100644 tests/ui/thread_local_initializer_can_be_made_const.stderr diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 99d80bec0255f..5ba960db66dde 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -38,7 +38,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain diff --git a/.github/workflows/clippy_bors.yml b/.github/workflows/clippy_bors.yml index 73c2555074286..012797e5ca72a 100644 --- a/.github/workflows/clippy_bors.yml +++ b/.github/workflows/clippy_bors.yml @@ -26,7 +26,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ github.ref }} @@ -72,7 +72,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install i686 dependencies if: matrix.host == 'i686-unknown-linux-gnu' @@ -151,7 +151,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain @@ -175,7 +175,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain @@ -231,7 +231,7 @@ jobs: github_token: "${{ secrets.github_token }}" - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install toolchain run: rustup show active-toolchain diff --git a/.github/workflows/clippy_dev.yml b/.github/workflows/clippy_dev.yml index 0f0e3f2db925c..37f18a4c08740 100644 --- a/.github/workflows/clippy_dev.yml +++ b/.github/workflows/clippy_dev.yml @@ -24,7 +24,7 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 # Run - name: Build diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 999ee7acfe761..94f494b65c47c 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -21,10 +21,10 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: ref: ${{ env.TARGET_BRANCH }} path: 'out' diff --git a/.github/workflows/remark.yml b/.github/workflows/remark.yml index 30bd476332f75..05e1b3b9202b4 100644 --- a/.github/workflows/remark.yml +++ b/.github/workflows/remark.yml @@ -16,7 +16,7 @@ jobs: steps: # Setup - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Node.js uses: actions/setup-node@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index f82421a687b82..4d32bbec914a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5105,6 +5105,7 @@ Released 2018-09-13 [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else [`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum +[`empty_enum_variants_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum_variants_with_brackets [`empty_line_after_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_doc_comments [`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr [`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop @@ -5294,6 +5295,7 @@ Released 2018-09-13 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite [`manual_is_infinite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_infinite +[`manual_is_variant_and`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_variant_and [`manual_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else [`manual_main_separator_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_main_separator_str [`manual_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_map @@ -5440,6 +5442,7 @@ Released 2018-09-13 [`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref [`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some +[`option_as_ref_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_cloned [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap [`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used @@ -5483,6 +5486,7 @@ Released 2018-09-13 [`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names +[`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields [`pub_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_use [`pub_with_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_with_shorthand [`pub_without_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_without_shorthand @@ -5580,6 +5584,7 @@ Released 2018-09-13 [`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive [`std_instead_of_alloc`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc [`std_instead_of_core`]: https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_core +[`str_split_at_newline`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_split_at_newline [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add [`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign @@ -5612,6 +5617,7 @@ Released 2018-09-13 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr [`test_attr_in_doctest`]: https://rust-lang.github.io/rust-clippy/master/index.html#test_attr_in_doctest [`tests_outside_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#tests_outside_test_module +[`thread_local_initializer_can_be_made_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#thread_local_initializer_can_be_made_const [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some [`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display [`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args @@ -5810,4 +5816,5 @@ Released 2018-09-13 [`allowed-dotfiles`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allowed-dotfiles [`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow [`check-private-items`]: https://doc.rust-lang.org/clippy/lint_configuration.html#check-private-items +[`pub-underscore-fields-behavior`]: https://doc.rust-lang.org/clippy/lint_configuration.html#pub-underscore-fields-behavior diff --git a/COPYRIGHT b/COPYRIGHT index 82703b18fd7d2..219693d63d973 100644 --- a/COPYRIGHT +++ b/COPYRIGHT @@ -1,6 +1,6 @@ // REUSE-IgnoreStart -Copyright 2014-2022 The Rust Project Developers +Copyright 2014-2024 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/LICENSE-APACHE b/LICENSE-APACHE index 0d62c37278e58..506582c31d6d5 100644 --- a/LICENSE-APACHE +++ b/LICENSE-APACHE @@ -186,7 +186,7 @@ APPENDIX: How to apply the Apache License to your work. same "printed page" as the copyright notice for easier identification within third-party archives. -Copyright 2014-2022 The Rust Project Developers +Copyright 2014-2024 The Rust Project Developers Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/LICENSE-MIT b/LICENSE-MIT index b724b24aa8309..6d8ee9afb6163 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,6 +1,6 @@ MIT License -Copyright (c) 2014-2022 The Rust Project Developers +Copyright (c) 2014-2024 The Rust Project Developers Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated diff --git a/README.md b/README.md index 5d490645d8979..fa18447090c19 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. @@ -278,7 +278,7 @@ If you want to contribute to Clippy, you can find more information in [CONTRIBUT -Copyright 2014-2023 The Rust Project Developers +Copyright 2014-2024 The Rust Project Developers Licensed under the Apache License, Version 2.0 or the MIT license diff --git a/book/src/README.md b/book/src/README.md index 486ea3df70420..e7972b0db19ce 100644 --- a/book/src/README.md +++ b/book/src/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 650 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 700 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index b02457307d743..a048fbbd8acf9 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -9,6 +9,7 @@ - [Clippy's Lints](lints.md) - [Continuous Integration](continuous_integration/README.md) - [GitHub Actions](continuous_integration/github_actions.md) + - [GitLab CI](continuous_integration/gitlab.md) - [Travis CI](continuous_integration/travis.md) - [Development](development/README.md) - [Basics](development/basics.md) diff --git a/book/src/continuous_integration/github_actions.md b/book/src/continuous_integration/github_actions.md index 339287a7dd95f..b588c8f0f02c4 100644 --- a/book/src/continuous_integration/github_actions.md +++ b/book/src/continuous_integration/github_actions.md @@ -15,7 +15,7 @@ jobs: clippy_check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Run Clippy run: cargo clippy --all-targets --all-features ``` diff --git a/book/src/continuous_integration/gitlab.md b/book/src/continuous_integration/gitlab.md new file mode 100644 index 0000000000000..bb3ef246c2faa --- /dev/null +++ b/book/src/continuous_integration/gitlab.md @@ -0,0 +1,16 @@ +# GitLab CI + +You can add Clippy to GitLab CI by using the latest stable [rust docker image](https://hub.docker.com/_/rust), +as it is shown in the `.gitlab-ci.yml` CI configuration file below, + +```yml +# Make sure CI fails on all warnings, including Clippy lints +variables: + RUSTFLAGS: "-Dwarnings" + +clippy_check: + image: rust:latest + script: + - rustup component add clippy + - cargo clippy --all-targets --all-features +``` diff --git a/book/src/development/macro_expansions.md b/book/src/development/macro_expansions.md index c5eb000272d39..aecca9ef72e98 100644 --- a/book/src/development/macro_expansions.md +++ b/book/src/development/macro_expansions.md @@ -102,7 +102,7 @@ let x: Option = Some(42); m!(x, x.unwrap()); ``` -If the `m!(x, x.unwrapp());` line is expanded, we would get two expanded +If the `m!(x, x.unwrap());` line is expanded, we would get two expanded expressions: - `x.is_some()` (from the `$a.is_some()` line in the `m` macro) diff --git a/book/src/development/type_checking.md b/book/src/development/type_checking.md index a8c9660da4c85..dc29ab5d08dad 100644 --- a/book/src/development/type_checking.md +++ b/book/src/development/type_checking.md @@ -133,7 +133,7 @@ in this chapter: - [Type checking](https://rustc-dev-guide.rust-lang.org/type-checking.html) - [Ty module](https://rustc-dev-guide.rust-lang.org/ty.html) -[Adt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/sty/enum.TyKind.html#variant.Adt +[Adt]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/ty_kind/enum.TyKind.html#variant.Adt [AdtDef]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/adt/struct.AdtDef.html [expr_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.expr_ty [node_type]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html#method.node_type @@ -144,7 +144,7 @@ in this chapter: [LateLintPass]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.LateLintPass.html [pat_ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/typeck_results/struct.TypeckResults.html#method.pat_ty [Ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.Ty.html -[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/sty/enum.TyKind.html +[TyKind]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_type_ir/ty_kind/enum.TyKind.html [TypeckResults]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/struct.TypeckResults.html [middle_ty]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_middle/ty/struct.Ty.html [hir_ty]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_hir/struct.Ty.html diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 7c9a8eb1bfba1..3b62ae0524ab0 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -805,3 +805,13 @@ for _ in &mut *rmvec {} * [`missing_errors_doc`](https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc) +## `pub-underscore-fields-behavior` + + +**Default Value:** `"PublicallyExported"` + +--- +**Affected lints:** +* [`pub_underscore_fields`](https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields) + + diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index a4f368397cede..5477d9b83a72c 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -1,5 +1,5 @@ use crate::msrvs::Msrv; -use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, Rename}; +use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename}; use crate::ClippyConfiguration; use rustc_data_structures::fx::FxHashSet; use rustc_session::Session; @@ -547,6 +547,11 @@ define_Conf! { /// /// Whether to also run the listed lints on private items. (check_private_items: bool = false), + /// Lint: PUB_UNDERSCORE_FIELDS + /// + /// Lint "public" fields in a struct that are prefixed with an underscore based on their + /// exported visibility, or whether they are marked as "pub". + (pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PublicallyExported), } /// Search for the configuration file. diff --git a/clippy_config/src/metadata.rs b/clippy_config/src/metadata.rs index 2451fbc91e898..3ba2796e18d3c 100644 --- a/clippy_config/src/metadata.rs +++ b/clippy_config/src/metadata.rs @@ -96,6 +96,9 @@ fn parse_config_field_doc(doc_comment: &str) -> Option<(Vec, String)> { doc_comment.make_ascii_lowercase(); let lints: Vec = doc_comment .split_off(DOC_START.len()) + .lines() + .next() + .unwrap() .split(", ") .map(str::to_string) .collect(); diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index dae9f09ec00a0..72d5b9aff28df 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -17,7 +17,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { 1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE } - 1,70,0 { OPTION_IS_SOME_AND, BINARY_HEAP_RETAIN } + 1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN } 1,68,0 { PATH_MAIN_SEPARATOR_STR } 1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS } 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE } diff --git a/clippy_config/src/types.rs b/clippy_config/src/types.rs index df48cc3f5e391..baee09629ac44 100644 --- a/clippy_config/src/types.rs +++ b/clippy_config/src/types.rs @@ -126,3 +126,9 @@ unimplemented_serialize! { Rename, MacroMatcher, } + +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] +pub enum PubUnderscoreFieldsBehaviour { + PublicallyExported, + AllPubFields, +} diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index 31a42734c1310..5d9cde06cd86b 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -67,7 +67,7 @@ pub fn create( if pass == "early" { println!( "\n\ - NOTE: Use a late pass unless you need something specific from\ + NOTE: Use a late pass unless you need something specific from\n\ an early pass, as they lack many features and utilities" ); } diff --git a/clippy_lints/src/casts/cast_sign_loss.rs b/clippy_lints/src/casts/cast_sign_loss.rs index bd12ee406284b..1df5a25f674d7 100644 --- a/clippy_lints/src/casts/cast_sign_loss.rs +++ b/clippy_lints/src/casts/cast_sign_loss.rs @@ -1,12 +1,14 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::{method_chain_args, sext}; -use rustc_hir::{Expr, ExprKind}; +use clippy_utils::{clip, method_chain_args, sext}; +use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_middle::ty::{self, Ty}; +use rustc_middle::ty::{self, Ty, UintTy}; use super::CAST_SIGN_LOSS; +const METHODS_RET_POSITIVE: &[&str] = &["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; + pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { if should_lint(cx, cast_op, cast_from, cast_to) { span_lint( @@ -25,33 +27,28 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast return false; } - // Don't lint for positive constants. - let const_val = constant(cx, cx.typeck_results(), cast_op); - if let Some(Constant::Int(n)) = const_val - && let ty::Int(ity) = *cast_from.kind() - && sext(cx.tcx, n, ity) >= 0 - { + // Don't lint if `cast_op` is known to be positive. + if let Sign::ZeroOrPositive = expr_sign(cx, cast_op, cast_from) { return false; } - // Don't lint for the result of methods that always return non-negative values. - if let ExprKind::MethodCall(path, ..) = cast_op.kind { - let mut method_name = path.ident.name.as_str(); - let allowed_methods = ["abs", "checked_abs", "rem_euclid", "checked_rem_euclid"]; - - if method_name == "unwrap" - && let Some(arglist) = method_chain_args(cast_op, &["unwrap"]) - && let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind - { - method_name = inner_path.ident.name.as_str(); - } - - if allowed_methods.iter().any(|&name| method_name == name) { - return false; - } + let (mut uncertain_count, mut negative_count) = (0, 0); + // Peel off possible binary expressions, e.g. x * x * y => [x, x, y] + let Some(exprs) = exprs_with_selected_binop_peeled(cast_op) else { + // Assume cast sign lose if we cannot determine the sign of `cast_op` + return true; + }; + for expr in exprs { + let ty = cx.typeck_results().expr_ty(expr); + match expr_sign(cx, expr, ty) { + Sign::Negative => negative_count += 1, + Sign::Uncertain => uncertain_count += 1, + Sign::ZeroOrPositive => (), + }; } - true + // Lint if there are odd number of uncertain or negative results + uncertain_count % 2 == 1 || negative_count % 2 == 1 }, (false, true) => !cast_to.is_signed(), @@ -59,3 +56,97 @@ fn should_lint(cx: &LateContext<'_>, cast_op: &Expr<'_>, cast_from: Ty<'_>, cast (_, _) => false, } } + +fn get_const_int_eval(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Option { + if let Constant::Int(n) = constant(cx, cx.typeck_results(), expr)? + && let ty::Int(ity) = *ty.kind() + { + return Some(sext(cx.tcx, n, ity)); + } + None +} + +enum Sign { + ZeroOrPositive, + Negative, + Uncertain, +} + +fn expr_sign(cx: &LateContext<'_>, expr: &Expr<'_>, ty: Ty<'_>) -> Sign { + // Try evaluate this expr first to see if it's positive + if let Some(val) = get_const_int_eval(cx, expr, ty) { + return if val >= 0 { Sign::ZeroOrPositive } else { Sign::Negative }; + } + // Calling on methods that always return non-negative values. + if let ExprKind::MethodCall(path, caller, args, ..) = expr.kind { + let mut method_name = path.ident.name.as_str(); + + if method_name == "unwrap" + && let Some(arglist) = method_chain_args(expr, &["unwrap"]) + && let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind + { + method_name = inner_path.ident.name.as_str(); + } + + if method_name == "pow" + && let [arg] = args + { + return pow_call_result_sign(cx, caller, arg); + } else if METHODS_RET_POSITIVE.iter().any(|&name| method_name == name) { + return Sign::ZeroOrPositive; + } + } + + Sign::Uncertain +} + +/// Return the sign of the `pow` call's result. +/// +/// If the caller is a positive number, the result is always positive, +/// If the `power_of` is a even number, the result is always positive as well, +/// Otherwise a [`Sign::Uncertain`] will be returned. +fn pow_call_result_sign(cx: &LateContext<'_>, caller: &Expr<'_>, power_of: &Expr<'_>) -> Sign { + let caller_ty = cx.typeck_results().expr_ty(caller); + if let Some(caller_val) = get_const_int_eval(cx, caller, caller_ty) + && caller_val >= 0 + { + return Sign::ZeroOrPositive; + } + + if let Some(Constant::Int(n)) = constant(cx, cx.typeck_results(), power_of) + && clip(cx.tcx, n, UintTy::U32) % 2 == 0 + { + return Sign::ZeroOrPositive; + } + + Sign::Uncertain +} + +/// Peels binary operators such as [`BinOpKind::Mul`], [`BinOpKind::Div`] or [`BinOpKind::Rem`], +/// which the result could always be positive under certain condition. +/// +/// Other operators such as `+`/`-` causing the result's sign hard to determine, which we will +/// return `None` +fn exprs_with_selected_binop_peeled<'a>(expr: &'a Expr<'_>) -> Option>> { + #[inline] + fn collect_operands<'a>(expr: &'a Expr<'a>, operands: &mut Vec<&'a Expr<'a>>) -> Option<()> { + match expr.kind { + ExprKind::Binary(op, lhs, rhs) => { + if matches!(op.node, BinOpKind::Mul | BinOpKind::Div | BinOpKind::Rem) { + collect_operands(lhs, operands); + operands.push(rhs); + } else { + // Things are complicated when there are other binary ops exist, + // abort checking by returning `None` for now. + return None; + } + }, + _ => operands.push(expr), + } + Some(()) + } + + let mut res = vec![]; + collect_operands(expr, &mut res)?; + Some(res) +} diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index eae9dfac064e8..20230106d5366 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -150,7 +150,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO, crate::empty_drop::EMPTY_DROP_INFO, crate::empty_enum::EMPTY_ENUM_INFO, - crate::empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO, + crate::empty_with_brackets::EMPTY_ENUM_VARIANTS_WITH_BRACKETS_INFO, + crate::empty_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS_INFO, crate::endian_bytes::BIG_ENDIAN_BYTES_INFO, crate::endian_bytes::HOST_ENDIAN_BYTES_INFO, crate::endian_bytes::LITTLE_ENDIAN_BYTES_INFO, @@ -385,6 +386,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::JOIN_ABSOLUTE_PATHS_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, + crate::methods::MANUAL_IS_VARIANT_AND_INFO, crate::methods::MANUAL_NEXT_BACK_INFO, crate::methods::MANUAL_OK_OR_INFO, crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO, @@ -408,6 +410,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::NO_EFFECT_REPLACE_INFO, crate::methods::OBFUSCATED_IF_ELSE_INFO, crate::methods::OK_EXPECT_INFO, + crate::methods::OPTION_AS_REF_CLONED_INFO, crate::methods::OPTION_AS_REF_DEREF_INFO, crate::methods::OPTION_FILTER_MAP_INFO, crate::methods::OPTION_MAP_OR_ERR_OK_INFO, @@ -433,6 +436,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::STABLE_SORT_PRIMITIVE_INFO, crate::methods::STRING_EXTEND_CHARS_INFO, crate::methods::STRING_LIT_CHARS_ANY_INFO, + crate::methods::STR_SPLIT_AT_NEWLINE_INFO, crate::methods::SUSPICIOUS_COMMAND_ARG_SPACE_INFO, crate::methods::SUSPICIOUS_MAP_INFO, crate::methods::SUSPICIOUS_SPLITN_INFO, @@ -576,6 +580,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::ptr::MUT_FROM_REF_INFO, crate::ptr::PTR_ARG_INFO, crate::ptr_offset_with_cast::PTR_OFFSET_WITH_CAST_INFO, + crate::pub_underscore_fields::PUB_UNDERSCORE_FIELDS_INFO, crate::pub_use::PUB_USE_INFO, crate::question_mark::QUESTION_MARK_INFO, crate::question_mark_used::QUESTION_MARK_USED_INFO, @@ -648,6 +653,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::tabs_in_doc_comments::TABS_IN_DOC_COMMENTS_INFO, crate::temporary_assignment::TEMPORARY_ASSIGNMENT_INFO, crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO, + crate::thread_local_initializer_can_be_made_const::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST_INFO, crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO, crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO, crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO, diff --git a/clippy_lints/src/default_numeric_fallback.rs b/clippy_lints/src/default_numeric_fallback.rs index 64a924a776a65..712bc075650fa 100644 --- a/clippy_lints/src/default_numeric_fallback.rs +++ b/clippy_lints/src/default_numeric_fallback.rs @@ -4,7 +4,7 @@ use clippy_utils::{get_parent_node, numeric_literal}; use rustc_ast::ast::{LitFloatType, LitIntType, LitKind}; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, walk_stmt, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, HirId, ItemKind, Lit, Node, Stmt, StmtKind}; +use rustc_hir::{Block, Body, Expr, ExprKind, FnRetTy, HirId, ItemKind, Lit, Node, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, FloatTy, IntTy, PolyFnSig, Ty}; @@ -122,13 +122,42 @@ impl<'a, 'tcx> NumericFallbackVisitor<'a, 'tcx> { impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { match &expr.kind { + ExprKind::Block( + Block { + stmts, expr: Some(_), .. + }, + _, + ) => { + if let Some(parent) = self.cx.tcx.hir().find_parent(expr.hir_id) + && let Some(fn_sig) = parent.fn_sig() + && let FnRetTy::Return(_ty) = fn_sig.decl.output + { + // We cannot check the exact type since it's a `hir::Ty`` which does not implement `is_numeric` + self.ty_bounds.push(ExplicitTyBound(true)); + for stmt in *stmts { + self.visit_stmt(stmt); + } + self.ty_bounds.pop(); + // Ignore return expr since we know its type was inferred from return ty + return; + } + }, + + // Ignore return expr since we know its type was inferred from return ty + ExprKind::Ret(_) => return, + ExprKind::Call(func, args) => { if let Some(fn_sig) = fn_sig_opt(self.cx, func.hir_id) { for (expr, bound) in iter::zip(*args, fn_sig.skip_binder().inputs()) { - // Push found arg type, then visit arg. - self.ty_bounds.push((*bound).into()); - self.visit_expr(expr); - self.ty_bounds.pop(); + // If is from macro, try to use last bound type (typically pushed when visiting stmt), + // otherwise push found arg type, then visit arg, + if expr.span.from_expansion() { + self.visit_expr(expr); + } else { + self.ty_bounds.push((*bound).into()); + self.visit_expr(expr); + self.ty_bounds.pop(); + } } return; } @@ -137,7 +166,7 @@ impl<'a, 'tcx> Visitor<'tcx> for NumericFallbackVisitor<'a, 'tcx> { ExprKind::MethodCall(_, receiver, args, _) => { if let Some(def_id) = self.cx.typeck_results().type_dependent_def_id(expr.hir_id) { let fn_sig = self.cx.tcx.fn_sig(def_id).instantiate_identity().skip_binder(); - for (expr, bound) in iter::zip(std::iter::once(*receiver).chain(args.iter()), fn_sig.inputs()) { + for (expr, bound) in iter::zip(iter::once(*receiver).chain(args.iter()), fn_sig.inputs()) { self.ty_bounds.push((*bound).into()); self.visit_expr(expr); self.ty_bounds.pop(); diff --git a/clippy_lints/src/empty_structs_with_brackets.rs b/clippy_lints/src/empty_with_brackets.rs similarity index 62% rename from clippy_lints/src/empty_structs_with_brackets.rs rename to clippy_lints/src/empty_with_brackets.rs index 3cf67b3ecbfa4..969df6d85b5a0 100644 --- a/clippy_lints/src/empty_structs_with_brackets.rs +++ b/clippy_lints/src/empty_with_brackets.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_opt; -use rustc_ast::ast::{Item, ItemKind, VariantData}; +use rustc_ast::ast::{Item, ItemKind, Variant, VariantData}; use rustc_errors::Applicability; use rustc_lexer::TokenKind; use rustc_lint::{EarlyContext, EarlyLintPass}; @@ -27,9 +27,38 @@ declare_clippy_lint! { restriction, "finds struct declarations with empty brackets" } -declare_lint_pass!(EmptyStructsWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS]); -impl EarlyLintPass for EmptyStructsWithBrackets { +declare_clippy_lint! { + /// ### What it does + /// Finds enum variants without fields that are declared with empty brackets. + /// + /// ### Why is this bad? + /// Empty brackets while defining enum variants are redundant and can be omitted. + /// + /// ### Example + /// ```no_run + /// enum MyEnum { + /// HasData(u8), + /// HasNoData(), // redundant parentheses + /// } + /// ``` + /// + /// Use instead: + /// ```no_run + /// enum MyEnum { + /// HasData(u8), + /// HasNoData, + /// } + /// ``` + #[clippy::version = "1.77.0"] + pub EMPTY_ENUM_VARIANTS_WITH_BRACKETS, + restriction, + "finds enum variants with empty brackets" +} + +declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]); + +impl EarlyLintPass for EmptyWithBrackets { fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) { let span_after_ident = item.span.with_lo(item.ident.span.hi()); @@ -53,6 +82,27 @@ impl EarlyLintPass for EmptyStructsWithBrackets { ); } } + + fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) { + let span_after_ident = variant.span.with_lo(variant.ident.span.hi()); + + if has_brackets(&variant.data) && has_no_fields(cx, &variant.data, span_after_ident) { + span_lint_and_then( + cx, + EMPTY_ENUM_VARIANTS_WITH_BRACKETS, + span_after_ident, + "enum variant has empty brackets", + |diagnostic| { + diagnostic.span_suggestion_hidden( + span_after_ident, + "remove the brackets", + "", + Applicability::MaybeIncorrect, + ); + }, + ); + } + } } fn has_no_ident_token(braces_span_str: &str) -> bool { diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index cd6c46a71a8e1..8f48941c4a91e 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/clippy_lints/src/if_then_some_else_none.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::eager_or_lazy::switch_to_eager_eval; use clippy_utils::source::snippet_with_context; use clippy_utils::sugg::Sugg; -use clippy_utils::{contains_return, higher, is_else_clause, is_res_lang_ctor, path_res, peel_blocks}; +use clippy_utils::{contains_return, higher, in_constant, is_else_clause, is_res_lang_ctor, path_res, peel_blocks}; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::{Expr, ExprKind}; @@ -74,6 +74,11 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone { return; } + // `bool::then()` and `bool::then_some()` are not const + if in_constant(cx, expr.hir_id) { + return; + } + let ctxt = expr.span.ctxt(); if let Some(higher::If { diff --git a/clippy_lints/src/instant_subtraction.rs b/clippy_lints/src/instant_subtraction.rs index 655f4b82aa4f1..17b6256f982b2 100644 --- a/clippy_lints/src/instant_subtraction.rs +++ b/clippy_lints/src/instant_subtraction.rs @@ -40,7 +40,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Lints subtraction between an [`Instant`] and a [`Duration`]. + /// Lints subtraction between an `Instant` and a `Duration`. /// /// ### Why is this bad? /// Unchecked subtraction could cause underflow on certain platforms, leading to @@ -57,9 +57,6 @@ declare_clippy_lint! { /// # use std::time::{Instant, Duration}; /// let time_passed = Instant::now().checked_sub(Duration::from_secs(5)); /// ``` - /// - /// [`Duration`]: std::time::Duration - /// [`Instant::now()`]: std::time::Instant::now; #[clippy::version = "1.67.0"] pub UNCHECKED_DURATION_SUBTRACTION, pedantic, diff --git a/clippy_lints/src/item_name_repetitions.rs b/clippy_lints/src/item_name_repetitions.rs index a9f1612ff05ed..276c1abb60cd5 100644 --- a/clippy_lints/src/item_name_repetitions.rs +++ b/clippy_lints/src/item_name_repetitions.rs @@ -1,6 +1,7 @@ //! lint on enum variants that are prefixed or suffixed by the same characters use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_hir}; +use clippy_utils::is_bool; use clippy_utils::macros::span_is_local; use clippy_utils::source::is_present_in_source; use clippy_utils::str_utils::{camel_case_split, count_match_end, count_match_start, to_camel_case, to_snake_case}; @@ -231,6 +232,10 @@ fn check_fields(cx: &LateContext<'_>, threshold: u64, item: &Item<'_>, fields: & (false, _) => ("pre", prefix), (true, false) => ("post", postfix), }; + if fields.iter().all(|field| is_bool(field.ty)) { + // If all fields are booleans, we don't want to emit this lint. + return; + } span_lint_and_help( cx, STRUCT_FIELD_NAMES, diff --git a/clippy_lints/src/iter_without_into_iter.rs b/clippy_lints/src/iter_without_into_iter.rs index c9dc48668f29d..903d3a2ab8960 100644 --- a/clippy_lints/src/iter_without_into_iter.rs +++ b/clippy_lints/src/iter_without_into_iter.rs @@ -5,7 +5,8 @@ use clippy_utils::ty::{implements_trait, make_normalized_projection}; use rustc_ast::Mutability; use rustc_errors::Applicability; use rustc_hir::{FnRetTy, ImplItemKind, ImplicitSelfKind, ItemKind, TyKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; use rustc_middle::ty::{self, Ty}; use rustc_session::declare_lint_pass; use rustc_span::{sym, Symbol}; @@ -152,7 +153,8 @@ fn adt_has_inherent_method(cx: &LateContext<'_>, ty: Ty<'_>, method_name: Symbol impl LateLintPass<'_> for IterWithoutIntoIter { fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) { - if let ItemKind::Impl(imp) = item.kind + if !in_external_macro(cx.sess(), item.span) + && let ItemKind::Impl(imp) = item.kind && let TyKind::Ref(_, self_ty_without_ref) = &imp.self_ty.kind && let Some(trait_ref) = imp.of_trait && trait_ref @@ -219,7 +221,8 @@ impl {self_ty_without_ref} {{ _ => return, }; - if let ImplItemKind::Fn(sig, _) = item.kind + if !in_external_macro(cx.sess(), item.span) + && let ImplItemKind::Fn(sig, _) = item.kind && let FnRetTy::Return(ret) = sig.decl.output && is_nameable_in_impl_trait(ret) && cx.tcx.generics_of(item_did).params.is_empty() diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 755a4ff525d2e..efdd392594977 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -115,7 +115,7 @@ mod duplicate_mod; mod else_if_without_else; mod empty_drop; mod empty_enum; -mod empty_structs_with_brackets; +mod empty_with_brackets; mod endian_bytes; mod entry; mod enum_clike; @@ -272,6 +272,7 @@ mod permissions_set_readonly_false; mod precedence; mod ptr; mod ptr_offset_with_cast; +mod pub_underscore_fields; mod pub_use; mod question_mark; mod question_mark_used; @@ -322,6 +323,7 @@ mod swap_ptr_to_ref; mod tabs_in_doc_comments; mod temporary_assignment; mod tests_outside_test_module; +mod thread_local_initializer_can_be_made_const; mod to_digit_is_some; mod trailing_empty_array; mod trait_bounds; @@ -571,6 +573,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { verbose_bit_mask_threshold, warn_on_all_wildcard_imports, check_private_items, + pub_underscore_fields_behavior, blacklisted_names: _, cyclomatic_complexity_threshold: _, @@ -947,7 +950,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { }) }); store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef)); - store.register_early_pass(|| Box::new(empty_structs_with_brackets::EmptyStructsWithBrackets)); + store.register_early_pass(|| Box::new(empty_with_brackets::EmptyWithBrackets)); store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings)); store.register_early_pass(|| Box::new(pub_use::PubUse)); store.register_late_pass(|_| Box::new(format_push_string::FormatPushString)); @@ -1080,7 +1083,15 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity)); store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences)); store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions)); - store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion)); + store.register_late_pass(|_| Box::::default()); + store.register_late_pass(move |_| { + Box::new(pub_underscore_fields::PubUnderscoreFields { + behavior: pub_underscore_fields_behavior, + }) + }); + store.register_late_pass(move |_| { + Box::new(thread_local_initializer_can_be_made_const::ThreadLocalInitializerCanBeMadeConst::new(msrv())) + }); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/lines_filter_map_ok.rs b/clippy_lints/src/lines_filter_map_ok.rs index 8a0955147bb54..29957e423b0b9 100644 --- a/clippy_lints/src/lines_filter_map_ok.rs +++ b/clippy_lints/src/lines_filter_map_ok.rs @@ -96,8 +96,7 @@ fn should_lint(cx: &LateContext<'_>, args: &[Expr<'_>], method_str: &str) -> boo ExprKind::Path(qpath) => cx .qpath_res(qpath, fm_arg.hir_id) .opt_def_id() - .map(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)) - .unwrap_or_default(), + .is_some_and(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)), // Detect `|x| x.ok()` ExprKind::Closure(Closure { body, .. }) => { if let Body { diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 7cfd3d346b638..e489899c19e34 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -26,13 +26,14 @@ fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> hir::ExprKind::Closure(&hir::Closure { body, .. }) => { let body = cx.tcx.hir().body(body); let closure_expr = peel_blocks(body.value); - let arg_id = body.params[0].pat.hir_id; match closure_expr.kind { hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, receiver, ..) => { if ident.name == method_name && let hir::ExprKind::Path(path) = &receiver.kind && let Res::Local(ref local) = cx.qpath_res(path, receiver.hir_id) + && !body.params.is_empty() { + let arg_id = body.params[0].pat.hir_id; return arg_id == *local; } false diff --git a/clippy_lints/src/methods/iter_filter.rs b/clippy_lints/src/methods/iter_filter.rs index ade8e3155fae3..9f84321ced4ae 100644 --- a/clippy_lints/src/methods/iter_filter.rs +++ b/clippy_lints/src/methods/iter_filter.rs @@ -1,87 +1,197 @@ +use clippy_utils::ty::get_iterator_item_ty; +use hir::ExprKind; use rustc_lint::{LateContext, LintContext}; use super::{ITER_FILTER_IS_OK, ITER_FILTER_IS_SOME}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline}; -use clippy_utils::{is_trait_method, peel_blocks, span_contains_comment}; +use clippy_utils::{get_parent_expr, is_trait_method, peel_blocks, span_contains_comment}; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_hir::def::Res; use rustc_hir::QPath; -use rustc_span::symbol::{sym, Symbol}; +use rustc_span::symbol::{sym, Ident, Symbol}; use rustc_span::Span; use std::borrow::Cow; -fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> bool { - match &expr.kind { - hir::ExprKind::Path(QPath::TypeRelative(_, mname)) => mname.ident.name == method_name, - hir::ExprKind::Path(QPath::Resolved(_, segments)) => { - segments.segments.last().unwrap().ident.name == method_name +/// +/// Returns true if the expression is a method call to `method_name` +/// e.g. `a.method_name()` or `Option::method_name`. +/// +/// The type-checker verifies for us that the method accepts the right kind of items +/// (e.g. `Option::is_some` accepts `Option<_>`), so we don't need to check that. +/// +/// How to capture each case: +/// +/// `.filter(|a| { std::option::Option::is_some(a) })` +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ <- this is a closure, getting unwrapped and +/// recursively checked. +/// `std::option::Option::is_some(a)` +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ <- this is a call. It unwraps to a path with +/// `QPath::TypeRelative`. Since this is a type relative path, we need to check the method name, the +/// type, and that the parameter of the closure is passed in the call. This part is the dual of +/// `receiver.method_name()` below. +/// +/// `filter(std::option::Option::is_some);` +/// ^^^^^^^^^^^^^^^^^^^^^^^^^^^ <- this is a type relative path, like above, we check the +/// type and the method name. +/// +/// `filter(|a| a.is_some());` +/// ^^^^^^^^^^^^^^^ <- this is a method call inside a closure, +/// we check that the parameter of the closure is the receiver of the method call and don't allow +/// any other parameters. +fn is_method( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + type_symbol: Symbol, + method_name: Symbol, + params: &[&hir::Pat<'_>], +) -> bool { + fn pat_is_recv(ident: Ident, param: &hir::Pat<'_>) -> bool { + match param.kind { + hir::PatKind::Binding(_, _, other, _) => ident == other, + hir::PatKind::Ref(pat, _) => pat_is_recv(ident, pat), + _ => false, + } + } + match expr.kind { + hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, recv, ..) => { + // compare the identifier of the receiver to the parameter + // we are in a filter => closure has a single parameter and a single, non-block + // expression, this means that the parameter shadows all outside variables with + // the same name => avoid FPs. If the parameter is not the receiver, then this hits + // outside variables => avoid FP + if ident.name == method_name + && let ExprKind::Path(QPath::Resolved(None, path)) = recv.kind + && let &[seg] = path.segments + && params.iter().any(|p| pat_is_recv(seg.ident, p)) + { + return true; + } + false + }, + // This is used to check for complete paths via `|a| std::option::Option::is_some(a)` + // this then unwraps to a path with `QPath::TypeRelative` + // we pass the params as they've been passed to the current call through the closure + hir::ExprKind::Call(expr, [param]) => { + // this will hit the `QPath::TypeRelative` case and check that the method name is correct + if is_method(cx, expr, type_symbol, method_name, params) + // we then check that this is indeed passing the parameter of the closure + && let ExprKind::Path(QPath::Resolved(None, path)) = param.kind + && let &[seg] = path.segments + && params.iter().any(|p| pat_is_recv(seg.ident, p)) + { + return true; + } + false + }, + hir::ExprKind::Path(QPath::TypeRelative(ty, mname)) => { + let ty = cx.typeck_results().node_type(ty.hir_id); + if let Some(did) = cx.tcx.get_diagnostic_item(type_symbol) + && ty.ty_adt_def() == cx.tcx.type_of(did).skip_binder().ty_adt_def() + { + return mname.ident.name == method_name; + } + false }, - hir::ExprKind::MethodCall(segment, _, _, _) => segment.ident.name == method_name, hir::ExprKind::Closure(&hir::Closure { body, .. }) => { let body = cx.tcx.hir().body(body); let closure_expr = peel_blocks(body.value); - let arg_id = body.params[0].pat.hir_id; - match closure_expr.kind { - hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, receiver, ..) => { - if ident.name == method_name - && let hir::ExprKind::Path(path) = &receiver.kind - && let Res::Local(ref local) = cx.qpath_res(path, receiver.hir_id) - { - return arg_id == *local; - } - false - }, - _ => false, - } + let params = body.params.iter().map(|param| param.pat).collect::>(); + is_method(cx, closure_expr, type_symbol, method_name, params.as_slice()) }, _ => false, } } fn parent_is_map(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { - if let hir::Node::Expr(parent_expr) = cx.tcx.hir().get_parent(expr.hir_id) { - is_method(cx, parent_expr, rustc_span::sym::map) - } else { - false + if let Some(expr) = get_parent_expr(cx, expr) + && is_trait_method(cx, expr, sym::Iterator) + && let hir::ExprKind::MethodCall(path, _, _, _) = expr.kind + && path.ident.name == rustc_span::sym::map + { + return true; } + false } -#[allow(clippy::too_many_arguments)] -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir::Expr<'_>, filter_span: Span) { - let is_iterator = is_trait_method(cx, expr, sym::Iterator); - let parent_is_not_map = !parent_is_map(cx, expr); +enum FilterType { + IsSome, + IsOk, +} - if is_iterator - && parent_is_not_map - && is_method(cx, filter_arg, sym!(is_some)) - && !span_contains_comment(cx.sess().source_map(), filter_span.with_hi(expr.span.hi())) +/// Returns the `FilterType` of the expression if it is a filter over an Iter