From d49b8aa7d2290241c2ea3ee246f3ffad8486dd35 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Wed, 30 Apr 2025 13:12:29 +0000 Subject: [PATCH 01/50] Remove `ClippyCtfe` pass --- clippy_lints/src/ctfe.rs | 26 --------------------- clippy_lints/src/lib.rs | 3 --- tests/ui/crashes/ice-9463.rs | 7 +++--- tests/ui/crashes/ice-9463.stderr | 29 ------------------------ tests/ui/indexing_slicing_index.rs | 1 - tests/ui/indexing_slicing_index.stderr | 31 ++++++++------------------ 6 files changed, 12 insertions(+), 85 deletions(-) delete mode 100644 clippy_lints/src/ctfe.rs delete mode 100644 tests/ui/crashes/ice-9463.stderr diff --git a/clippy_lints/src/ctfe.rs b/clippy_lints/src/ctfe.rs deleted file mode 100644 index 7bae04a10f10b..0000000000000 --- a/clippy_lints/src/ctfe.rs +++ /dev/null @@ -1,26 +0,0 @@ -use rustc_hir::def_id::LocalDefId; -use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, FnDecl}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; -use rustc_span::Span; - -declare_lint_pass! { - /// Ensures that Constant-time Function Evaluation is being done (specifically, MIR lint passes). - /// As Clippy deactivates codegen, this lint ensures that CTFE (used in hard errors) is still ran. - ClippyCtfe => [] -} - -impl<'tcx> LateLintPass<'tcx> for ClippyCtfe { - fn check_fn( - &mut self, - cx: &LateContext<'_>, - _: FnKind<'tcx>, - _: &'tcx FnDecl<'tcx>, - _: &'tcx Body<'tcx>, - _: Span, - defid: LocalDefId, - ) { - cx.tcx.ensure_ok().mir_drops_elaborated_and_const_checked(defid); // Lint - } -} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bc7fc60827a0b..1090b3ec045c7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -67,7 +67,6 @@ extern crate clippy_utils; mod utils; -pub mod ctfe; // Very important lint, do not remove (rust#125116) pub mod declared_lints; pub mod deprecated_lints; @@ -583,8 +582,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { let attrs = attr_storage.clone(); store.register_early_pass(move || Box::new(AttrCollector::new(attrs.clone()))); - store.register_late_pass(|_| Box::new(ctfe::ClippyCtfe)); - store.register_late_pass(move |_| Box::new(operators::arithmetic_side_effects::ArithmeticSideEffects::new(conf))); store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir)); store.register_late_pass(|_| Box::new(utils::author::Author)); diff --git a/tests/ui/crashes/ice-9463.rs b/tests/ui/crashes/ice-9463.rs index 93808e0f8923b..cfa6cdac6fa05 100644 --- a/tests/ui/crashes/ice-9463.rs +++ b/tests/ui/crashes/ice-9463.rs @@ -1,8 +1,7 @@ -#![deny(arithmetic_overflow)] +//@check-pass + fn main() { let _x = -1_i32 >> -1; - //~^ ERROR: this arithmetic operation will overflow + #[expect(overflowing_literals)] let _y = 1u32 >> 10000000000000u32; - //~^ ERROR: this arithmetic operation will overflow - //~| ERROR: literal out of range } diff --git a/tests/ui/crashes/ice-9463.stderr b/tests/ui/crashes/ice-9463.stderr deleted file mode 100644 index 9a3a5e444ade8..0000000000000 --- a/tests/ui/crashes/ice-9463.stderr +++ /dev/null @@ -1,29 +0,0 @@ -error: this arithmetic operation will overflow - --> tests/ui/crashes/ice-9463.rs:3:14 - | -LL | let _x = -1_i32 >> -1; - | ^^^^^^^^^^^^ attempt to shift right by `-1_i32`, which would overflow - | -note: the lint level is defined here - --> tests/ui/crashes/ice-9463.rs:1:9 - | -LL | #![deny(arithmetic_overflow)] - | ^^^^^^^^^^^^^^^^^^^ - -error: this arithmetic operation will overflow - --> tests/ui/crashes/ice-9463.rs:5:14 - | -LL | let _y = 1u32 >> 10000000000000u32; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to shift right by `1316134912_u32`, which would overflow - -error: literal out of range for `u32` - --> tests/ui/crashes/ice-9463.rs:5:22 - | -LL | let _y = 1u32 >> 10000000000000u32; - | ^^^^^^^^^^^^^^^^^ - | - = note: the literal `10000000000000u32` does not fit into the type `u32` whose range is `0..=4294967295` - = note: `#[deny(overflowing_literals)]` on by default - -error: aborting due to 3 previous errors - diff --git a/tests/ui/indexing_slicing_index.rs b/tests/ui/indexing_slicing_index.rs index cfa1c2f7c75fa..2510a023acd25 100644 --- a/tests/ui/indexing_slicing_index.rs +++ b/tests/ui/indexing_slicing_index.rs @@ -68,7 +68,6 @@ fn main() { // This should be linted, since `suppress-restriction-lint-in-const` default is false. const { &ARR[idx4()] }; //~^ ERROR: indexing may panic - //~| ERROR: evaluation of `main let y = &x; // Ok, referencing shouldn't affect this lint. See the issue 6021 diff --git a/tests/ui/indexing_slicing_index.stderr b/tests/ui/indexing_slicing_index.stderr index 50ee9b9edc75b..c68e1d53a9342 100644 --- a/tests/ui/indexing_slicing_index.stderr +++ b/tests/ui/indexing_slicing_index.stderr @@ -9,18 +9,6 @@ LL | const REF: &i32 = &ARR[idx()]; // This should be linted, since `suppress-re = note: `-D clippy::indexing-slicing` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::indexing_slicing)]` -error[E0080]: evaluation of `main::{constant#3}` failed - --> tests/ui/indexing_slicing_index.rs:69:14 - | -LL | const { &ARR[idx4()] }; - | ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4 - -note: erroneous constant encountered - --> tests/ui/indexing_slicing_index.rs:69:5 - | -LL | const { &ARR[idx4()] }; - | ^^^^^^^^^^^^^^^^^^^^^^ - error: indexing may panic --> tests/ui/indexing_slicing_index.rs:48:5 | @@ -63,13 +51,13 @@ LL | const { &ARR[idx4()] }; = note: the suggestion might not be applicable in constant blocks error: index is out of bounds - --> tests/ui/indexing_slicing_index.rs:77:5 + --> tests/ui/indexing_slicing_index.rs:76:5 | LL | y[4]; | ^^^^ error: indexing may panic - --> tests/ui/indexing_slicing_index.rs:81:5 + --> tests/ui/indexing_slicing_index.rs:80:5 | LL | v[0]; | ^^^^ @@ -77,7 +65,7 @@ LL | v[0]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> tests/ui/indexing_slicing_index.rs:83:5 + --> tests/ui/indexing_slicing_index.rs:82:5 | LL | v[10]; | ^^^^^ @@ -85,7 +73,7 @@ LL | v[10]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> tests/ui/indexing_slicing_index.rs:85:5 + --> tests/ui/indexing_slicing_index.rs:84:5 | LL | v[1 << 3]; | ^^^^^^^^^ @@ -93,13 +81,13 @@ LL | v[1 << 3]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: index is out of bounds - --> tests/ui/indexing_slicing_index.rs:93:5 + --> tests/ui/indexing_slicing_index.rs:92:5 | LL | x[N]; | ^^^^ error: indexing may panic - --> tests/ui/indexing_slicing_index.rs:97:5 + --> tests/ui/indexing_slicing_index.rs:96:5 | LL | v[N]; | ^^^^ @@ -107,7 +95,7 @@ LL | v[N]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> tests/ui/indexing_slicing_index.rs:99:5 + --> tests/ui/indexing_slicing_index.rs:98:5 | LL | v[M]; | ^^^^ @@ -115,11 +103,10 @@ LL | v[M]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: index is out of bounds - --> tests/ui/indexing_slicing_index.rs:103:13 + --> tests/ui/indexing_slicing_index.rs:102:13 | LL | let _ = x[4]; | ^^^^ -error: aborting due to 15 previous errors +error: aborting due to 14 previous errors -For more information about this error, try `rustc --explain E0080`. From a9225f82632c0e15c61bf858604860c7e0aa1abf Mon Sep 17 00:00:00 2001 From: Ryan van Polen Date: Fri, 16 May 2025 13:28:33 +0200 Subject: [PATCH 02/50] Updated feature flag and output of `clippy/tests/ui/map_flatten*` --- tests/ui/map_flatten.rs | 2 +- tests/ui/map_flatten_fixable.fixed | 1 - tests/ui/map_flatten_fixable.rs | 1 - tests/ui/map_flatten_fixable.stderr | 18 +++++++++--------- 4 files changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/ui/map_flatten.rs b/tests/ui/map_flatten.rs index d7e9c9d9900d9..0970da8039a48 100644 --- a/tests/ui/map_flatten.rs +++ b/tests/ui/map_flatten.rs @@ -1,5 +1,5 @@ #![warn(clippy::map_flatten)] -#![feature(result_flattening)] + //@no-rustfix // issue #8506, multi-line #[rustfmt::skip] diff --git a/tests/ui/map_flatten_fixable.fixed b/tests/ui/map_flatten_fixable.fixed index f8379ed23c5b2..6d8a27d3018dd 100644 --- a/tests/ui/map_flatten_fixable.fixed +++ b/tests/ui/map_flatten_fixable.fixed @@ -1,4 +1,3 @@ -#![feature(result_flattening)] #![allow( clippy::let_underscore_untyped, clippy::missing_docs_in_private_items, diff --git a/tests/ui/map_flatten_fixable.rs b/tests/ui/map_flatten_fixable.rs index 040a9ca85f647..845e3a79ae2b0 100644 --- a/tests/ui/map_flatten_fixable.rs +++ b/tests/ui/map_flatten_fixable.rs @@ -1,4 +1,3 @@ -#![feature(result_flattening)] #![allow( clippy::let_underscore_untyped, clippy::missing_docs_in_private_items, diff --git a/tests/ui/map_flatten_fixable.stderr b/tests/ui/map_flatten_fixable.stderr index fe68eb7e4ab44..05d4d9a6ad85c 100644 --- a/tests/ui/map_flatten_fixable.stderr +++ b/tests/ui/map_flatten_fixable.stderr @@ -1,5 +1,5 @@ error: called `map(..).flatten()` on `Iterator` - --> tests/ui/map_flatten_fixable.rs:17:47 + --> tests/ui/map_flatten_fixable.rs:16:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id)` @@ -8,43 +8,43 @@ LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id).flatten().coll = help: to override `-D warnings` add `#[allow(clippy::map_flatten)]` error: called `map(..).flatten()` on `Iterator` - --> tests/ui/map_flatten_fixable.rs:19:47 + --> tests/ui/map_flatten_fixable.rs:18:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_ref).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_ref)` error: called `map(..).flatten()` on `Iterator` - --> tests/ui/map_flatten_fixable.rs:21:47 + --> tests/ui/map_flatten_fixable.rs:20:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(option_id_closure).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(option_id_closure)` error: called `map(..).flatten()` on `Iterator` - --> tests/ui/map_flatten_fixable.rs:23:47 + --> tests/ui/map_flatten_fixable.rs:22:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| x.checked_add(1)).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `filter_map` and remove the `.flatten()`: `filter_map(|x| x.checked_add(1))` error: called `map(..).flatten()` on `Iterator` - --> tests/ui/map_flatten_fixable.rs:27:47 + --> tests/ui/map_flatten_fixable.rs:26:47 | LL | let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect(); | ^^^^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `flat_map` and remove the `.flatten()`: `flat_map(|x| 0..x)` error: called `map(..).flatten()` on `Option` - --> tests/ui/map_flatten_fixable.rs:31:40 + --> tests/ui/map_flatten_fixable.rs:30:40 | LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten(); | ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)` error: called `map(..).flatten()` on `Result` - --> tests/ui/map_flatten_fixable.rs:35:42 + --> tests/ui/map_flatten_fixable.rs:34:42 | LL | let _: Result<_, &str> = (Ok(Ok(1))).map(|x| x).flatten(); | ^^^^^^^^^^^^^^^^^^^^ help: try replacing `map` with `and_then` and remove the `.flatten()`: `and_then(|x| x)` error: called `map(..).flatten()` on `Iterator` - --> tests/ui/map_flatten_fixable.rs:45:10 + --> tests/ui/map_flatten_fixable.rs:44:10 | LL | .map(|n| match n { | __________^ @@ -74,7 +74,7 @@ LL ~ }); | error: called `map(..).flatten()` on `Option` - --> tests/ui/map_flatten_fixable.rs:66:10 + --> tests/ui/map_flatten_fixable.rs:65:10 | LL | .map(|_| { | __________^ From 0524773b5d9f2460b706b83c275bb5b5c28e358c Mon Sep 17 00:00:00 2001 From: yanglsh Date: Sat, 24 May 2025 14:31:52 +0800 Subject: [PATCH 03/50] fix: `unit_arg` suggests wrongly for `Default::default` --- clippy_lints/src/default.rs | 18 ++---------------- clippy_lints/src/unit_types/unit_arg.rs | 16 +++++++++------- clippy_utils/src/lib.rs | 13 +++++++++++++ ...pty_blocks.fixed => unit_arg_fixable.fixed} | 6 ++++++ ...arg_empty_blocks.rs => unit_arg_fixable.rs} | 6 ++++++ ...y_blocks.stderr => unit_arg_fixable.stderr} | 18 +++++++++++++----- 6 files changed, 49 insertions(+), 28 deletions(-) rename tests/ui/{unit_arg_empty_blocks.fixed => unit_arg_fixable.fixed} (87%) rename tests/ui/{unit_arg_empty_blocks.rs => unit_arg_fixable.rs} (84%) rename tests/ui/{unit_arg_empty_blocks.stderr => unit_arg_fixable.stderr} (69%) diff --git a/clippy_lints/src/default.rs b/clippy_lints/src/default.rs index 886c325b355fe..a48e4d2fbd57e 100644 --- a/clippy_lints/src/default.rs +++ b/clippy_lints/src/default.rs @@ -1,10 +1,9 @@ use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg}; use clippy_utils::source::snippet_with_context; use clippy_utils::ty::{has_drop, is_copy}; -use clippy_utils::{contains_name, get_parent_expr, in_automatically_derived, is_from_proc_macro}; +use clippy_utils::{contains_name, get_parent_expr, in_automatically_derived, is_expr_default, is_from_proc_macro}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; -use rustc_hir::def::Res; use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind, StructTailExpr}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; @@ -129,7 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for Default { // only take bindings to identifiers && let PatKind::Binding(_, binding_id, ident, _) = local.pat.kind // only when assigning `... = Default::default()` - && is_expr_default(expr, cx) + && is_expr_default(cx, expr) && let binding_type = cx.typeck_results().node_type(binding_id) && let ty::Adt(adt, args) = *binding_type.kind() && adt.is_struct() @@ -251,19 +250,6 @@ impl<'tcx> LateLintPass<'tcx> for Default { } } -/// Checks if the given expression is the `default` method belonging to the `Default` trait. -fn is_expr_default<'tcx>(expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> bool { - if let ExprKind::Call(fn_expr, []) = &expr.kind - && let ExprKind::Path(qpath) = &fn_expr.kind - && let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id) - { - // right hand side of assignment is `Default::default` - cx.tcx.is_diagnostic_item(sym::default_fn, def_id) - } else { - false - } -} - /// Returns the reassigned field and the assigning expression (right-hand side of assign). fn field_reassigned_by_stmt<'tcx>(this: &Stmt<'tcx>, binding_name: Symbol) -> Option<(Ident, &'tcx Expr<'tcx>)> { if let StmtKind::Semi(later_expr) = this.kind diff --git a/clippy_lints/src/unit_types/unit_arg.rs b/clippy_lints/src/unit_types/unit_arg.rs index 019ae16ca8591..e09b362800c0e 100644 --- a/clippy_lints/src/unit_types/unit_arg.rs +++ b/clippy_lints/src/unit_types/unit_arg.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::is_from_proc_macro; use clippy_utils::source::{SourceText, SpanRangeExt, indent_of, reindent_multiline}; +use clippy_utils::{is_expr_default, is_from_proc_macro}; use rustc_errors::Applicability; use rustc_hir::{Block, Expr, ExprKind, MatchSource, Node, StmtKind}; use rustc_lint::LateContext; @@ -59,7 +59,7 @@ fn is_questionmark_desugar_marked_call(expr: &Expr<'_>) -> bool { } } -fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Expr<'_>]) { +fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_recover: &[&'tcx Expr<'tcx>]) { let mut applicability = Applicability::MachineApplicable; let (singular, plural) = if args_to_recover.len() > 1 { ("", "s") @@ -102,9 +102,11 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp .iter() .filter_map(|arg| arg.span.get_source_text(cx)) .collect(); - let arg_snippets_without_empty_blocks: Vec<_> = args_to_recover + + // If the argument is an empty block or `Default::default()`, we can replace it with `()`. + let arg_snippets_without_redundant_exprs: Vec<_> = args_to_recover .iter() - .filter(|arg| !is_empty_block(arg)) + .filter(|arg| !is_empty_block(arg) && !is_expr_default(cx, arg)) .filter_map(|arg| arg.span.get_source_text(cx)) .collect(); @@ -114,10 +116,10 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp expr, &call_snippet, &arg_snippets, - &arg_snippets_without_empty_blocks, + &arg_snippets_without_redundant_exprs, ); - if arg_snippets_without_empty_blocks.is_empty() { + if arg_snippets_without_redundant_exprs.is_empty() { db.multipart_suggestion( format!("use {singular}unit literal{plural} instead"), args_to_recover @@ -127,7 +129,7 @@ fn lint_unit_args(cx: &LateContext<'_>, expr: &Expr<'_>, args_to_recover: &[&Exp applicability, ); } else { - let plural = arg_snippets_without_empty_blocks.len() > 1; + let plural = arg_snippets_without_redundant_exprs.len() > 1; let empty_or_s = if plural { "s" } else { "" }; let it_or_them = if plural { "them" } else { "it" }; db.span_suggestion( diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 3a31917657103..f3cd22de5229b 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -3471,3 +3471,16 @@ pub fn desugar_await<'tcx>(expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { None } } + +/// Checks if the given expression is the `default` method belonging to the `Default` trait. +pub fn is_expr_default<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + if let ExprKind::Call(fn_expr, []) = &expr.kind + && let ExprKind::Path(qpath) = &fn_expr.kind + && let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id) + { + // right hand side of assignment is `Default::default` + cx.tcx.is_diagnostic_item(sym::default_fn, def_id) + } else { + false + } +} diff --git a/tests/ui/unit_arg_empty_blocks.fixed b/tests/ui/unit_arg_fixable.fixed similarity index 87% rename from tests/ui/unit_arg_empty_blocks.fixed rename to tests/ui/unit_arg_fixable.fixed index b045a33608d7c..7a25b93e5aee3 100644 --- a/tests/ui/unit_arg_empty_blocks.fixed +++ b/tests/ui/unit_arg_fixable.fixed @@ -32,3 +32,9 @@ fn taking_three_units(a: (), b: (), c: ()) {} fn main() { bad(); } + +fn issue14857() { + let fn_take_unit = |_: ()| {}; + fn_take_unit(()); + //~^ unit_arg +} diff --git a/tests/ui/unit_arg_empty_blocks.rs b/tests/ui/unit_arg_fixable.rs similarity index 84% rename from tests/ui/unit_arg_empty_blocks.rs rename to tests/ui/unit_arg_fixable.rs index ab305913f3f69..65152abbe7b82 100644 --- a/tests/ui/unit_arg_empty_blocks.rs +++ b/tests/ui/unit_arg_fixable.rs @@ -29,3 +29,9 @@ fn taking_three_units(a: (), b: (), c: ()) {} fn main() { bad(); } + +fn issue14857() { + let fn_take_unit = |_: ()| {}; + fn_take_unit(Default::default()); + //~^ unit_arg +} diff --git a/tests/ui/unit_arg_empty_blocks.stderr b/tests/ui/unit_arg_fixable.stderr similarity index 69% rename from tests/ui/unit_arg_empty_blocks.stderr rename to tests/ui/unit_arg_fixable.stderr index 2c686d58ecc1a..79afe67e57247 100644 --- a/tests/ui/unit_arg_empty_blocks.stderr +++ b/tests/ui/unit_arg_fixable.stderr @@ -1,5 +1,5 @@ error: passing a unit value to a function - --> tests/ui/unit_arg_empty_blocks.rs:16:5 + --> tests/ui/unit_arg_fixable.rs:16:5 | LL | foo({}); | ^^^^--^ @@ -10,7 +10,7 @@ LL | foo({}); = help: to override `-D warnings` add `#[allow(clippy::unit_arg)]` error: passing a unit value to a function - --> tests/ui/unit_arg_empty_blocks.rs:18:5 + --> tests/ui/unit_arg_fixable.rs:18:5 | LL | foo3({}, 2, 2); | ^^^^^--^^^^^^^ @@ -18,7 +18,7 @@ LL | foo3({}, 2, 2); | help: use a unit literal instead: `()` error: passing unit values to a function - --> tests/ui/unit_arg_empty_blocks.rs:20:5 + --> tests/ui/unit_arg_fixable.rs:20:5 | LL | taking_two_units({}, foo(0)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -30,7 +30,7 @@ LL ~ taking_two_units((), ()); | error: passing unit values to a function - --> tests/ui/unit_arg_empty_blocks.rs:22:5 + --> tests/ui/unit_arg_fixable.rs:22:5 | LL | taking_three_units({}, foo(0), foo(1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -42,5 +42,13 @@ LL + foo(1); LL ~ taking_three_units((), (), ()); | -error: aborting due to 4 previous errors +error: passing a unit value to a function + --> tests/ui/unit_arg_fixable.rs:35:5 + | +LL | fn_take_unit(Default::default()); + | ^^^^^^^^^^^^^------------------^ + | | + | help: use a unit literal instead: `()` + +error: aborting due to 5 previous errors From ebf39a54786ffb10a8ff5aec5504e4fbd29db19a Mon Sep 17 00:00:00 2001 From: yanglsh Date: Sat, 24 May 2025 21:10:46 +0800 Subject: [PATCH 04/50] fix: `unit_arg` wrongly handled macros --- clippy_lints/src/unit_types/unit_arg.rs | 54 ++++++++++++++++++------- clippy_utils/src/lib.rs | 3 +- tests/ui/unit_arg.rs | 24 +++++++++++ tests/ui/unit_arg.stderr | 23 ++++++++++- tests/ui/unit_arg_fixable.fixed | 30 ++++++++++++++ tests/ui/unit_arg_fixable.rs | 27 +++++++++++++ tests/ui/unit_arg_fixable.stderr | 46 ++++++++++++++++++++- 7 files changed, 188 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/unit_types/unit_arg.rs b/clippy_lints/src/unit_types/unit_arg.rs index e09b362800c0e..74b3331414c3c 100644 --- a/clippy_lints/src/unit_types/unit_arg.rs +++ b/clippy_lints/src/unit_types/unit_arg.rs @@ -1,9 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::{SourceText, SpanRangeExt, indent_of, reindent_multiline}; +use clippy_utils::source::{SpanRangeExt, indent_of, reindent_multiline}; +use clippy_utils::sugg::Sugg; use clippy_utils::{is_expr_default, is_from_proc_macro}; use rustc_errors::Applicability; use rustc_hir::{Block, Expr, ExprKind, MatchSource, Node, StmtKind}; use rustc_lint::LateContext; +use rustc_span::SyntaxContext; use super::{UNIT_ARG, utils}; @@ -100,14 +102,16 @@ fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_ let arg_snippets: Vec<_> = args_to_recover .iter() - .filter_map(|arg| arg.span.get_source_text(cx)) + // If the argument is from an expansion and is a `Default::default()`, we skip it + .filter(|arg| !arg.span.from_expansion() || !is_expr_default_nested(cx, arg)) + .filter_map(|arg| get_expr_snippet(cx, arg)) .collect(); // If the argument is an empty block or `Default::default()`, we can replace it with `()`. let arg_snippets_without_redundant_exprs: Vec<_> = args_to_recover .iter() - .filter(|arg| !is_empty_block(arg) && !is_expr_default(cx, arg)) - .filter_map(|arg| arg.span.get_source_text(cx)) + .filter(|arg| !is_expr_default_nested(cx, arg) && (arg.span.from_expansion() || !is_empty_block(arg))) + .filter_map(|arg| get_expr_snippet(cx, arg)) .collect(); if let Some(call_snippet) = expr.span.get_source_text(cx) { @@ -119,13 +123,17 @@ fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_ &arg_snippets_without_redundant_exprs, ); - if arg_snippets_without_redundant_exprs.is_empty() { + if arg_snippets_without_redundant_exprs.is_empty() + && let suggestions = args_to_recover + .iter() + .filter(|arg| !arg.span.from_expansion() || !is_expr_default_nested(cx, arg)) + .map(|arg| (arg.span.parent_callsite().unwrap_or(arg.span), "()".to_string())) + .collect::>() + && !suggestions.is_empty() + { db.multipart_suggestion( format!("use {singular}unit literal{plural} instead"), - args_to_recover - .iter() - .map(|arg| (arg.span, "()".to_string())) - .collect::>(), + suggestions, applicability, ); } else { @@ -146,6 +154,22 @@ fn lint_unit_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, args_to_ ); } +fn is_expr_default_nested<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { + is_expr_default(cx, expr) + || matches!(expr.kind, ExprKind::Block(block, _) + if block.expr.is_some() && is_expr_default_nested(cx, block.expr.unwrap())) +} + +fn get_expr_snippet<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option> { + let mut app = Applicability::MachineApplicable; + let snip = Sugg::hir_with_context(cx, expr, SyntaxContext::root(), "..", &mut app); + if app != Applicability::MachineApplicable { + return None; + } + + Some(snip) +} + fn is_empty_block(expr: &Expr<'_>) -> bool { matches!( expr.kind, @@ -164,17 +188,17 @@ fn fmt_stmts_and_call( cx: &LateContext<'_>, call_expr: &Expr<'_>, call_snippet: &str, - args_snippets: &[SourceText], - non_empty_block_args_snippets: &[SourceText], + args_snippets: &[Sugg<'_>], + non_empty_block_args_snippets: &[Sugg<'_>], ) -> String { let call_expr_indent = indent_of(cx, call_expr.span).unwrap_or(0); - let call_snippet_with_replacements = args_snippets - .iter() - .fold(call_snippet.to_owned(), |acc, arg| acc.replacen(arg.as_ref(), "()", 1)); + let call_snippet_with_replacements = args_snippets.iter().fold(call_snippet.to_owned(), |acc, arg| { + acc.replacen(&arg.to_string(), "()", 1) + }); let mut stmts_and_call = non_empty_block_args_snippets .iter() - .map(|it| it.as_ref().to_owned()) + .map(ToString::to_string) .collect::>(); stmts_and_call.push(call_snippet_with_replacements); stmts_and_call = stmts_and_call diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index f3cd22de5229b..4e588adf43ebd 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -3472,13 +3472,12 @@ pub fn desugar_await<'tcx>(expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { } } -/// Checks if the given expression is the `default` method belonging to the `Default` trait. +/// Checks if the given expression is a call to `Default::default()`. pub fn is_expr_default<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { if let ExprKind::Call(fn_expr, []) = &expr.kind && let ExprKind::Path(qpath) = &fn_expr.kind && let Res::Def(_, def_id) = cx.qpath_res(qpath, fn_expr.hir_id) { - // right hand side of assignment is `Default::default` cx.tcx.is_diagnostic_item(sym::default_fn, def_id) } else { false diff --git a/tests/ui/unit_arg.rs b/tests/ui/unit_arg.rs index 22a6a26dab626..4208efad6774d 100644 --- a/tests/ui/unit_arg.rs +++ b/tests/ui/unit_arg.rs @@ -151,3 +151,27 @@ fn main() { bad(); ok(); } + +fn issue14857() { + let fn_take_unit = |_: ()| {}; + fn some_other_fn(_: &i32) {} + + macro_rules! mac { + (def) => { + Default::default() + }; + (func $f:expr) => { + $f() + }; + (nonempty_block $e:expr) => {{ + some_other_fn(&$e); + $e + }}; + } + fn_take_unit(mac!(def)); + //~^ unit_arg + fn_take_unit(mac!(func Default::default)); + //~^ unit_arg + fn_take_unit(mac!(nonempty_block Default::default())); + //~^ unit_arg +} diff --git a/tests/ui/unit_arg.stderr b/tests/ui/unit_arg.stderr index 6c333d9792d4d..0dcfb02b9fa08 100644 --- a/tests/ui/unit_arg.stderr +++ b/tests/ui/unit_arg.stderr @@ -199,5 +199,26 @@ LL ~ foo(1); LL + Some(()) | -error: aborting due to 10 previous errors +error: passing a unit value to a function + --> tests/ui/unit_arg.rs:171:5 + | +LL | fn_take_unit(mac!(def)); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + +error: passing a unit value to a function + --> tests/ui/unit_arg.rs:173:5 + | +LL | fn_take_unit(mac!(func Default::default)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + +error: passing a unit value to a function + --> tests/ui/unit_arg.rs:175:5 + | +LL | fn_take_unit(mac!(nonempty_block Default::default())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + +error: aborting due to 13 previous errors diff --git a/tests/ui/unit_arg_fixable.fixed b/tests/ui/unit_arg_fixable.fixed index 7a25b93e5aee3..3ef6a953c729b 100644 --- a/tests/ui/unit_arg_fixable.fixed +++ b/tests/ui/unit_arg_fixable.fixed @@ -37,4 +37,34 @@ fn issue14857() { let fn_take_unit = |_: ()| {}; fn_take_unit(()); //~^ unit_arg + + fn some_other_fn(_: &i32) {} + + macro_rules! another_mac { + () => { + some_other_fn(&Default::default()) + }; + ($e:expr) => { + some_other_fn(&$e) + }; + } + + another_mac!(); + fn_take_unit(()); + //~^ unit_arg + another_mac!(1); + fn_take_unit(()); + //~^ unit_arg + + macro_rules! mac { + (nondef $e:expr) => { + $e + }; + (empty_block) => {{}}; + } + fn_take_unit(mac!(nondef ())); + //~^ unit_arg + mac!(empty_block); + fn_take_unit(()); + //~^ unit_arg } diff --git a/tests/ui/unit_arg_fixable.rs b/tests/ui/unit_arg_fixable.rs index 65152abbe7b82..097d51e9481c5 100644 --- a/tests/ui/unit_arg_fixable.rs +++ b/tests/ui/unit_arg_fixable.rs @@ -34,4 +34,31 @@ fn issue14857() { let fn_take_unit = |_: ()| {}; fn_take_unit(Default::default()); //~^ unit_arg + + fn some_other_fn(_: &i32) {} + + macro_rules! another_mac { + () => { + some_other_fn(&Default::default()) + }; + ($e:expr) => { + some_other_fn(&$e) + }; + } + + fn_take_unit(another_mac!()); + //~^ unit_arg + fn_take_unit(another_mac!(1)); + //~^ unit_arg + + macro_rules! mac { + (nondef $e:expr) => { + $e + }; + (empty_block) => {{}}; + } + fn_take_unit(mac!(nondef Default::default())); + //~^ unit_arg + fn_take_unit(mac!(empty_block)); + //~^ unit_arg } diff --git a/tests/ui/unit_arg_fixable.stderr b/tests/ui/unit_arg_fixable.stderr index 79afe67e57247..94063f02a3f15 100644 --- a/tests/ui/unit_arg_fixable.stderr +++ b/tests/ui/unit_arg_fixable.stderr @@ -50,5 +50,49 @@ LL | fn_take_unit(Default::default()); | | | help: use a unit literal instead: `()` -error: aborting due to 5 previous errors +error: passing a unit value to a function + --> tests/ui/unit_arg_fixable.rs:49:5 + | +LL | fn_take_unit(another_mac!()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move the expression in front of the call and replace it with the unit literal `()` + | +LL ~ another_mac!(); +LL ~ fn_take_unit(()); + | + +error: passing a unit value to a function + --> tests/ui/unit_arg_fixable.rs:51:5 + | +LL | fn_take_unit(another_mac!(1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move the expression in front of the call and replace it with the unit literal `()` + | +LL ~ another_mac!(1); +LL ~ fn_take_unit(()); + | + +error: passing a unit value to a function + --> tests/ui/unit_arg_fixable.rs:60:5 + | +LL | fn_take_unit(mac!(nondef Default::default())); + | ^^^^^^^^^^^^^^^^^^^^^^^^^------------------^^ + | | + | help: use a unit literal instead: `()` + +error: passing a unit value to a function + --> tests/ui/unit_arg_fixable.rs:62:5 + | +LL | fn_take_unit(mac!(empty_block)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: move the expression in front of the call and replace it with the unit literal `()` + | +LL ~ mac!(empty_block); +LL ~ fn_take_unit(()); + | + +error: aborting due to 9 previous errors From d82e51b6404acda3db23639d3f03b62458c3516a Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 19 May 2025 00:05:13 +0200 Subject: [PATCH 05/50] Test that all interned symbols are referenced in Clippy sources --- clippy_utils/src/sym.rs | 1 - tests/symbols-used.rs | 81 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 tests/symbols-used.rs diff --git a/clippy_utils/src/sym.rs b/clippy_utils/src/sym.rs index f417530be3672..9a75b946001f9 100644 --- a/clippy_utils/src/sym.rs +++ b/clippy_utils/src/sym.rs @@ -132,7 +132,6 @@ generate! { enum_glob_use, enumerate, err, - error, exp, expect_err, expn_data, diff --git a/tests/symbols-used.rs b/tests/symbols-used.rs new file mode 100644 index 0000000000000..bc0456711fb2b --- /dev/null +++ b/tests/symbols-used.rs @@ -0,0 +1,81 @@ +// This test checks that all symbols defined in Clippy's `sym.rs` file +// are used in Clippy. Otherwise, it will fail with a list of symbols +// which are unused. +// +// This test is a no-op if run as part of the compiler test suite +// and will always succeed. + +use std::collections::HashSet; +use std::ffi::OsStr; +use std::fs; + +use regex::Regex; +use walkdir::{DirEntry, WalkDir}; + +const SYM_FILE: &str = "clippy_utils/src/sym.rs"; + +type Result = std::result::Result; +type AnyError = Box; + +#[test] +#[allow(clippy::case_sensitive_file_extension_comparisons)] +fn all_symbols_are_used() -> Result<()> { + if option_env!("RUSTC_TEST_SUITE").is_some() { + return Ok(()); + } + + // Load all symbols defined in `SYM_FILE`. + let content = fs::read_to_string(SYM_FILE)?; + let content = content + .split_once("generate! {") + .ok_or("cannot find symbols start")? + .1 + .split_once("\n}\n") + .ok_or("cannot find symbols end")? + .0; + let mut interned: HashSet = Regex::new(r"(?m)^ (\w+)") + .unwrap() + .captures_iter(content) + .map(|m| m[1].to_owned()) + .collect(); + + // Remove symbols used as `sym::*`. + let used_re = Regex::new(r"\bsym::(\w+)\b").unwrap(); + let rs_ext = OsStr::new("rs"); + for dir in ["clippy_lints", "clippy_lints_internal", "clippy_utils", "src"] { + for file in WalkDir::new(dir) + .into_iter() + .flatten() + .map(DirEntry::into_path) + .filter(|p| p.extension() == Some(rs_ext)) + { + for cap in used_re.captures_iter(&fs::read_to_string(file)?) { + interned.remove(&cap[1]); + } + } + } + + // Remove symbols used as part of paths. + let paths_re = Regex::new(r"path!\(([\w:]+)\)").unwrap(); + for path in [ + "clippy_utils/src/paths.rs", + "clippy_lints_internal/src/internal_paths.rs", + ] { + for cap in paths_re.captures_iter(&fs::read_to_string(path)?) { + for sym in cap[1].split("::") { + interned.remove(sym); + } + } + } + + let mut extra = interned.iter().collect::>(); + if !extra.is_empty() { + extra.sort_unstable(); + eprintln!("Unused symbols defined in {SYM_FILE}:"); + for sym in extra { + eprintln!(" - {sym}"); + } + Err(format!("extra symbols found — remove them {SYM_FILE}"))?; + } + Ok(()) +} From f6e5fa20bf30477d582746dc5189fbabf8509c15 Mon Sep 17 00:00:00 2001 From: yukang Date: Mon, 26 May 2025 14:11:02 +0800 Subject: [PATCH 06/50] remove eq_unspanned from TokenStream --- clippy_utils/src/ast_utils/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_utils/src/ast_utils/mod.rs b/clippy_utils/src/ast_utils/mod.rs index 8996b694ed8f7..01e916fb5353b 100644 --- a/clippy_utils/src/ast_utils/mod.rs +++ b/clippy_utils/src/ast_utils/mod.rs @@ -960,5 +960,7 @@ pub fn eq_attr_args(l: &AttrArgs, r: &AttrArgs) -> bool { } pub fn eq_delim_args(l: &DelimArgs, r: &DelimArgs) -> bool { - l.delim == r.delim && l.tokens.eq_unspanned(&r.tokens) + l.delim == r.delim + && l.tokens.len() == r.tokens.len() + && l.tokens.iter().zip(r.tokens.iter()).all(|(a, b)| a.eq_unspanned(b)) } From 8e581e31a2191392461210f3f7a168910da5bf49 Mon Sep 17 00:00:00 2001 From: Dan Johnson Date: Thu, 15 May 2025 01:08:47 -0700 Subject: [PATCH 07/50] add lint infallible_try_from Looks for `TryFrom` implementations with uninhabited error types and suggests to implement `From` instead. --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/infallible_try_from.rs | 76 +++++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + tests/ui/crashes/ice-rust-107877.rs | 1 + tests/ui/infallible_try_from.rs | 33 +++++++++++ tests/ui/infallible_try_from.stderr | 23 ++++++++ 7 files changed, 137 insertions(+) create mode 100644 clippy_lints/src/infallible_try_from.rs create mode 100644 tests/ui/infallible_try_from.rs create mode 100644 tests/ui/infallible_try_from.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 28147dfbea3e0..9c06562e18e7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5862,6 +5862,7 @@ Released 2018-09-13 [`ineffective_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_open_options [`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string [`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match +[`infallible_try_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_try_from [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter [`infinite_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_loop [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index bb825c7655f87..12dd407711ca9 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -211,6 +211,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::indexing_slicing::INDEXING_SLICING_INFO, crate::indexing_slicing::OUT_OF_BOUNDS_INDEXING_INFO, crate::ineffective_open_options::INEFFECTIVE_OPEN_OPTIONS_INFO, + crate::infallible_try_from::INFALLIBLE_TRY_FROM_INFO, crate::infinite_iter::INFINITE_ITER_INFO, crate::infinite_iter::MAYBE_INFINITE_ITER_INFO, crate::inherent_impl::MULTIPLE_INHERENT_IMPL_INFO, diff --git a/clippy_lints/src/infallible_try_from.rs b/clippy_lints/src/infallible_try_from.rs new file mode 100644 index 0000000000000..b54c289fa7e13 --- /dev/null +++ b/clippy_lints/src/infallible_try_from.rs @@ -0,0 +1,76 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::sym; +use rustc_errors::MultiSpan; +use rustc_hir::{AssocItemKind, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::declare_lint_pass; + +declare_clippy_lint! { + /// ### What it does + /// + /// Finds manual impls of `TryFrom` with infallible error types. + /// + /// ### Why is this bad? + /// + /// Infalliable conversions should be implemented via `From` with the blanket conversion. + /// + /// ### Example + /// ```no_run + /// use std::convert::Infallible; + /// struct MyStruct(i16); + /// impl TryFrom for MyStruct { + /// type Error = Infallible; + /// fn try_from(other: i16) -> Result { + /// Ok(Self(other.into())) + /// } + /// } + /// ``` + /// Use instead: + /// ```no_run + /// struct MyStruct(i16); + /// impl From for MyStruct { + /// fn from(other: i16) -> Self { + /// Self(other) + /// } + /// } + /// ``` + #[clippy::version = "1.88.0"] + pub INFALLIBLE_TRY_FROM, + suspicious, + "TryFrom with infallible Error type" +} +declare_lint_pass!(InfallibleTryFrom => [INFALLIBLE_TRY_FROM]); + +impl<'tcx> LateLintPass<'tcx> for InfallibleTryFrom { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + let ItemKind::Impl(imp) = item.kind else { return }; + let Some(r#trait) = imp.of_trait else { return }; + let Some(trait_def_id) = r#trait.trait_def_id() else { + return; + }; + if !cx.tcx.is_diagnostic_item(sym::TryFrom, trait_def_id) { + return; + } + for ii in imp.items { + if ii.kind == AssocItemKind::Type { + let ii = cx.tcx.hir_impl_item(ii.id); + if ii.ident.name != sym::Error { + continue; + } + let ii_ty = ii.expect_type(); + let ii_ty_span = ii_ty.span; + let ii_ty = clippy_utils::ty::ty_from_hir_ty(cx, ii_ty); + if !ii_ty.is_inhabited_from(cx.tcx, ii.owner_id.to_def_id(), cx.typing_env()) { + let mut span = MultiSpan::from_span(cx.tcx.def_span(item.owner_id.to_def_id())); + span.push_span_label(ii_ty_span, "infallible error type"); + span_lint( + cx, + INFALLIBLE_TRY_FROM, + span, + "infallible TryFrom impl; consider implementing From, instead", + ); + } + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 006145cc623c3..0f60c62ed094b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -169,6 +169,7 @@ mod inconsistent_struct_constructor; mod index_refutable_slice; mod indexing_slicing; mod ineffective_open_options; +mod infallible_try_from; mod infinite_iter; mod inherent_impl; mod inherent_to_string; @@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(single_option_map::SingleOptionMap)); store.register_late_pass(move |_| Box::new(redundant_test_prefix::RedundantTestPrefix)); store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf))); + store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/crashes/ice-rust-107877.rs b/tests/ui/crashes/ice-rust-107877.rs index 55fe418bed123..dccb0cf8ac152 100644 --- a/tests/ui/crashes/ice-rust-107877.rs +++ b/tests/ui/crashes/ice-rust-107877.rs @@ -4,6 +4,7 @@ struct Foo; +#[allow(clippy::infallible_try_from)] impl<'a> std::convert::TryFrom<&'a String> for Foo { type Error = std::convert::Infallible; diff --git a/tests/ui/infallible_try_from.rs b/tests/ui/infallible_try_from.rs new file mode 100644 index 0000000000000..6a1f12f824f55 --- /dev/null +++ b/tests/ui/infallible_try_from.rs @@ -0,0 +1,33 @@ +#![feature(never_type)] +#![warn(clippy::infallible_try_from)] + +use std::convert::Infallible; + +struct MyStruct(i32); + +impl TryFrom for MyStruct { + //~^ infallible_try_from + type Error = !; + fn try_from(other: i8) -> Result { + Ok(Self(other.into())) + } +} + +impl TryFrom for MyStruct { + //~^ infallible_try_from + type Error = Infallible; + fn try_from(other: i16) -> Result { + Ok(Self(other.into())) + } +} + +impl TryFrom for MyStruct { + type Error = i64; + fn try_from(other: i64) -> Result { + Ok(Self(i32::try_from(other).map_err(|_| other)?)) + } +} + +fn main() { + // test code goes here +} diff --git a/tests/ui/infallible_try_from.stderr b/tests/ui/infallible_try_from.stderr new file mode 100644 index 0000000000000..705b1188489c2 --- /dev/null +++ b/tests/ui/infallible_try_from.stderr @@ -0,0 +1,23 @@ +error: infallible TryFrom impl; consider implementing From, instead + --> tests/ui/infallible_try_from.rs:8:1 + | +LL | impl TryFrom for MyStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | +LL | type Error = !; + | - infallible error type + | + = note: `-D clippy::infallible-try-from` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::infallible_try_from)]` + +error: infallible TryFrom impl; consider implementing From, instead + --> tests/ui/infallible_try_from.rs:16:1 + | +LL | impl TryFrom for MyStruct { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | +LL | type Error = Infallible; + | ---------- infallible error type + +error: aborting due to 2 previous errors + From e177dd0b439ec460112cc4693694cbd5d275ce0e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 27 May 2025 02:32:22 +1000 Subject: [PATCH 08/50] Reduce `P` to a typedef of `Box`. Keep the `P` constructor function for now, to minimize immediate churn. All the `into_inner` calls are removed, which is nice. --- clippy_lints/src/unnested_or_patterns.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/unnested_or_patterns.rs b/clippy_lints/src/unnested_or_patterns.rs index 9ad184450de43..b839b6f56728c 100644 --- a/clippy_lints/src/unnested_or_patterns.rs +++ b/clippy_lints/src/unnested_or_patterns.rs @@ -426,7 +426,7 @@ fn drain_matching( // Check if we should extract, but only if `idx >= start`. if idx > start && predicate(&alternatives[i].kind) { let pat = alternatives.remove(i); - tail_or.push(extract(pat.into_inner().kind)); + tail_or.push(extract(pat.kind)); } else { i += 1; } From 1c070ca1c84b056314ca883d8f47badf2d4c8bd9 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Wed, 28 May 2025 21:04:22 +0800 Subject: [PATCH 09/50] fix: `branches_sharing_code` suggests wrongly when dealing with macros --- clippy_lints/src/copies.rs | 51 ++++++++++++++++++- .../branches_sharing_code/shared_at_bottom.rs | 37 ++++++++++++++ .../shared_at_bottom.stderr | 17 ++++++- .../ui/branches_sharing_code/shared_at_top.rs | 31 +++++++++++ .../shared_at_top.stderr | 17 ++++++- .../shared_at_top_and_bottom.rs | 39 ++++++++++++++ .../shared_at_top_and_bottom.stderr | 28 +++++++++- 7 files changed, 215 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index 2467fc95fd055..5ef726638a560 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -425,7 +425,9 @@ fn scan_block_for_eq<'tcx>( modifies_any_local(cx, stmt, &cond_locals) || !eq_stmts(stmt, blocks, |b| b.stmts.get(i), &mut eq, &mut moved_locals) }) - .map_or(block.stmts.len(), |(i, _)| i); + .map_or(block.stmts.len(), |(i, stmt)| { + adjust_by_closest_callsite(i, stmt, block.stmts[..i].iter().enumerate().rev()) + }); if local_needs_ordered_drop { return BlockEq { @@ -467,7 +469,9 @@ fn scan_block_for_eq<'tcx>( .is_none_or(|s| hash != hash_stmt(cx, s)) }) }) - .map_or(block.stmts.len() - start_end_eq, |(i, _)| i); + .map_or(block.stmts.len() - start_end_eq, |(i, stmt)| { + adjust_by_closest_callsite(i, stmt, (0..i).rev().zip(block.stmts[(block.stmts.len() - i)..].iter())) + }); let moved_locals_at_start = moved_locals.len(); let mut i = end_search_start; @@ -522,6 +526,49 @@ fn scan_block_for_eq<'tcx>( } } +/// Adjusts the index for which the statements begin to differ to the closest macro callsite. This +/// avoids giving suggestions that requires splitting a macro call in half, when only a part of the +/// macro expansion is equal. +/// +/// For example, for the following macro: +/// ```rust,ignore +/// macro_rules! foo { +/// ($x:expr) => { +/// let y = 42; +/// $x; +/// }; +/// } +/// ``` +/// If the macro is called like this: +/// ```rust,ignore +/// if false { +/// let z = 42; +/// foo!(println!("Hello")); +/// } else { +/// let z = 42; +/// foo!(println!("World")); +/// } +/// ``` +/// Although the expanded `let y = 42;` is equal, the macro call should not be included in the +/// suggestion. +fn adjust_by_closest_callsite<'tcx>( + i: usize, + stmt: &'tcx Stmt<'tcx>, + mut iter: impl Iterator)>, +) -> usize { + let Some((_, first)) = iter.next() else { + return 0; + }; + + // If it is already at the boundary of a macro call, then just return. + if first.span.source_callsite() != stmt.span.source_callsite() { + return i; + } + + iter.find(|(_, stmt)| stmt.span.source_callsite() != first.span.source_callsite()) + .map_or(0, |(i, _)| i + 1) +} + fn check_for_warn_of_moved_symbol(cx: &LateContext<'_>, symbols: &[(HirId, Symbol)], if_expr: &Expr<'_>) -> bool { get_enclosing_block(cx, if_expr.hir_id).is_some_and(|block| { let ignore_span = block.span.shrink_to_lo().to(if_expr.span); diff --git a/tests/ui/branches_sharing_code/shared_at_bottom.rs b/tests/ui/branches_sharing_code/shared_at_bottom.rs index 06472a4f5d572..922d30443fcc2 100644 --- a/tests/ui/branches_sharing_code/shared_at_bottom.rs +++ b/tests/ui/branches_sharing_code/shared_at_bottom.rs @@ -239,3 +239,40 @@ fn fp_if_let_issue7054() { } fn main() {} + +mod issue14873 { + fn foo() -> i32 { + todo!() + } + + macro_rules! qux { + ($a:ident, $b:ident, $condition:expr) => { + if $condition { + "." + } else { + "" + }; + $a = foo(); + $b = foo(); + }; + } + + fn share_on_bottom() { + let mut a = 0; + let mut b = 0; + if false { + qux!(a, b, a == b); + } else { + qux!(a, b, a != b); + }; + + if false { + qux!(a, b, a == b); + let y = 1; + } else { + qux!(a, b, a != b); + let y = 1; + //~^ branches_sharing_code + } + } +} diff --git a/tests/ui/branches_sharing_code/shared_at_bottom.stderr b/tests/ui/branches_sharing_code/shared_at_bottom.stderr index 648a99c65ed22..f437db8b73313 100644 --- a/tests/ui/branches_sharing_code/shared_at_bottom.stderr +++ b/tests/ui/branches_sharing_code/shared_at_bottom.stderr @@ -157,5 +157,20 @@ LL ~ if x == 17 { b = 1; a = 0x99; } else { } LL + a = 0x99; | -error: aborting due to 9 previous errors +error: all if blocks contain the same code at the end + --> tests/ui/branches_sharing_code/shared_at_bottom.rs:274:9 + | +LL | / let y = 1; +LL | | +LL | | } + | |_________^ + | + = warning: some moved values might need to be renamed to avoid wrong references +help: consider moving these statements after the if + | +LL ~ } +LL + let y = 1; + | + +error: aborting due to 10 previous errors diff --git a/tests/ui/branches_sharing_code/shared_at_top.rs b/tests/ui/branches_sharing_code/shared_at_top.rs index 694c67d4c85b3..dcd77679fc691 100644 --- a/tests/ui/branches_sharing_code/shared_at_top.rs +++ b/tests/ui/branches_sharing_code/shared_at_top.rs @@ -124,3 +124,34 @@ fn pf_local_with_inferred_type_issue7053() { } fn main() {} + +mod issue14873 { + fn foo() -> i32 { + todo!() + } + + macro_rules! qux { + ($a:ident, $b:ident, $condition:expr) => { + let $a: i32 = foo(); + let $b: i32 = foo(); + if $condition { "." } else { "" } + }; + } + + fn share_on_top() { + if false { + qux!(a, b, a == b); + } else { + qux!(a, b, a != b); + }; + + if false { + //~^ branches_sharing_code + let x = 1; + qux!(a, b, a == b); + } else { + let x = 1; + qux!(a, b, a != b); + } + } +} diff --git a/tests/ui/branches_sharing_code/shared_at_top.stderr b/tests/ui/branches_sharing_code/shared_at_top.stderr index d28e9c7af2967..30efb98b3f5f6 100644 --- a/tests/ui/branches_sharing_code/shared_at_top.stderr +++ b/tests/ui/branches_sharing_code/shared_at_top.stderr @@ -125,5 +125,20 @@ note: the lint level is defined here LL | #![deny(clippy::branches_sharing_code, clippy::if_same_then_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: all if blocks contain the same code at the start + --> tests/ui/branches_sharing_code/shared_at_top.rs:148:9 + | +LL | / if false { +LL | | +LL | | let x = 1; + | |______________________^ + | + = warning: some moved values might need to be renamed to avoid wrong references +help: consider moving these statements before the if + | +LL ~ let x = 1; +LL + if false { + | + +error: aborting due to 8 previous errors diff --git a/tests/ui/branches_sharing_code/shared_at_top_and_bottom.rs b/tests/ui/branches_sharing_code/shared_at_top_and_bottom.rs index 75334f70f1f62..e848f0601e32d 100644 --- a/tests/ui/branches_sharing_code/shared_at_top_and_bottom.rs +++ b/tests/ui/branches_sharing_code/shared_at_top_and_bottom.rs @@ -128,3 +128,42 @@ fn added_note_for_expression_use() -> u32 { } fn main() {} + +mod issue14873 { + fn foo() -> i32 { + todo!() + } + + macro_rules! qux { + ($a:ident, $b:ident, $condition:expr) => { + let mut $a: i32 = foo(); + let mut $b: i32 = foo(); + if $condition { + "." + } else { + "" + }; + $a = foo(); + $b = foo(); + }; + } + + fn share_on_top_and_bottom() { + if false { + qux!(a, b, a == b); + } else { + qux!(a, b, a != b); + }; + + if false { + //~^ branches_sharing_code + let x = 1; + qux!(a, b, a == b); + let y = 1; + } else { + let x = 1; + qux!(a, b, a != b); + let y = 1; + } + } +} diff --git a/tests/ui/branches_sharing_code/shared_at_top_and_bottom.stderr b/tests/ui/branches_sharing_code/shared_at_top_and_bottom.stderr index 2200ab450890c..40f3453edb9ae 100644 --- a/tests/ui/branches_sharing_code/shared_at_top_and_bottom.stderr +++ b/tests/ui/branches_sharing_code/shared_at_top_and_bottom.stderr @@ -159,5 +159,31 @@ LL ~ } LL + x * 4 | -error: aborting due to 5 previous errors +error: all if blocks contain the same code at both the start and the end + --> tests/ui/branches_sharing_code/shared_at_top_and_bottom.rs:158:9 + | +LL | / if false { +LL | | +LL | | let x = 1; + | |______________________^ + | +note: this code is shared at the end + --> tests/ui/branches_sharing_code/shared_at_top_and_bottom.rs:166:9 + | +LL | / let y = 1; +LL | | } + | |_________^ + = warning: some moved values might need to be renamed to avoid wrong references +help: consider moving these statements before the if + | +LL ~ let x = 1; +LL + if false { + | +help: consider moving these statements after the if + | +LL ~ } +LL + let y = 1; + | + +error: aborting due to 6 previous errors From 4f3d529c88350be9c9bccdd366474d18169e13b6 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Wed, 28 May 2025 15:17:32 +0000 Subject: [PATCH 10/50] Remove incremental test, disable incremental for cargo tests --- tests/compile-test.rs | 8 ++++---- tests/ui/to_string_in_format_args_incremental.fixed | 11 ----------- tests/ui/to_string_in_format_args_incremental.rs | 11 ----------- tests/ui/to_string_in_format_args_incremental.stderr | 11 ----------- 4 files changed, 4 insertions(+), 37 deletions(-) delete mode 100644 tests/ui/to_string_in_format_args_incremental.fixed delete mode 100644 tests/ui/to_string_in_format_args_incremental.rs delete mode 100644 tests/ui/to_string_in_format_args_incremental.stderr diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 78b27e2f61399..99a01257a7b69 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -273,10 +273,10 @@ fn run_ui_cargo(cx: &TestContext) { config.program.input_file_flag = CommandBuilder::cargo().input_file_flag; config.program.out_dir_flag = CommandBuilder::cargo().out_dir_flag; config.program.args = vec!["clippy".into(), "--color".into(), "never".into(), "--quiet".into()]; - config - .program - .envs - .push(("RUSTFLAGS".into(), Some("-Dwarnings".into()))); + config.program.envs.extend([ + ("RUSTFLAGS".into(), Some("-Dwarnings".into())), + ("CARGO_INCREMENTAL".into(), Some("0".into())), + ]); // We need to do this while we still have a rustc in the `program` field. config.fill_host_and_target().unwrap(); config.program.program.set_file_name(if cfg!(windows) { diff --git a/tests/ui/to_string_in_format_args_incremental.fixed b/tests/ui/to_string_in_format_args_incremental.fixed deleted file mode 100644 index 98b028c15fd0a..0000000000000 --- a/tests/ui/to_string_in_format_args_incremental.fixed +++ /dev/null @@ -1,11 +0,0 @@ -//@compile-flags: -C incremental=target/debug/test/incr - -#![allow(clippy::uninlined_format_args)] - -// see https://github.com/rust-lang/rust-clippy/issues/10969 - -fn main() { - let s = "Hello, world!"; - println!("{}", s); - //~^ to_string_in_format_args -} diff --git a/tests/ui/to_string_in_format_args_incremental.rs b/tests/ui/to_string_in_format_args_incremental.rs deleted file mode 100644 index a5ebe4b032750..0000000000000 --- a/tests/ui/to_string_in_format_args_incremental.rs +++ /dev/null @@ -1,11 +0,0 @@ -//@compile-flags: -C incremental=target/debug/test/incr - -#![allow(clippy::uninlined_format_args)] - -// see https://github.com/rust-lang/rust-clippy/issues/10969 - -fn main() { - let s = "Hello, world!"; - println!("{}", s.to_string()); - //~^ to_string_in_format_args -} diff --git a/tests/ui/to_string_in_format_args_incremental.stderr b/tests/ui/to_string_in_format_args_incremental.stderr deleted file mode 100644 index 62178cc0cfbff..0000000000000 --- a/tests/ui/to_string_in_format_args_incremental.stderr +++ /dev/null @@ -1,11 +0,0 @@ -error: `to_string` applied to a type that implements `Display` in `println!` args - --> tests/ui/to_string_in_format_args_incremental.rs:9:21 - | -LL | println!("{}", s.to_string()); - | ^^^^^^^^^^^^ help: remove this - | - = note: `-D clippy::to-string-in-format-args` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::to_string_in_format_args)]` - -error: aborting due to 1 previous error - From 3c8bfd144b7b740f7cde10b14ea86fc1dbc7ce63 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Wed, 28 May 2025 20:18:22 +0200 Subject: [PATCH 11/50] `zombie_processes`: do not complain about early early returns If an early return is found under the HIR local declaration node but before the child process is created, it should not trigger the lint because the child process has not yet been created. --- clippy_lints/src/zombie_processes.rs | 26 +++++++++++--------------- tests/ui/zombie_processes.rs | 10 ++++++++++ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/zombie_processes.rs b/clippy_lints/src/zombie_processes.rs index 09f1084fe7004..6ab94a5221092 100644 --- a/clippy_lints/src/zombie_processes.rs +++ b/clippy_lints/src/zombie_processes.rs @@ -5,7 +5,7 @@ use rustc_ast::Mutability; use rustc_ast::visit::visit_opt; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; -use rustc_hir::intravisit::{Visitor, walk_block, walk_expr, walk_local}; +use rustc_hir::intravisit::{Visitor, walk_block, walk_expr}; use rustc_hir::{Expr, ExprKind, HirId, LetStmt, Node, PatKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; @@ -69,8 +69,9 @@ impl<'tcx> LateLintPass<'tcx> for ZombieProcesses { let mut vis = WaitFinder { cx, local_id, + create_id: expr.hir_id, body_id: cx.tcx.hir_enclosing_body_owner(expr.hir_id), - state: VisitorState::WalkUpToLocal, + state: VisitorState::WalkUpToCreate, early_return: None, missing_wait_branch: None, }; @@ -131,6 +132,7 @@ struct MaybeWait(Span); struct WaitFinder<'a, 'tcx> { cx: &'a LateContext<'tcx>, local_id: HirId, + create_id: HirId, body_id: LocalDefId, state: VisitorState, early_return: Option, @@ -141,8 +143,8 @@ struct WaitFinder<'a, 'tcx> { #[derive(PartialEq)] enum VisitorState { - WalkUpToLocal, - LocalFound, + WalkUpToCreate, + CreateFound, } #[derive(Copy, Clone)] @@ -155,19 +157,13 @@ impl<'tcx> Visitor<'tcx> for WaitFinder<'_, 'tcx> { type NestedFilter = nested_filter::OnlyBodies; type Result = ControlFlow; - fn visit_local(&mut self, l: &'tcx LetStmt<'tcx>) -> Self::Result { - if self.state == VisitorState::WalkUpToLocal - && let PatKind::Binding(_, pat_id, ..) = l.pat.kind - && self.local_id == pat_id - { - self.state = VisitorState::LocalFound; + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> Self::Result { + if ex.hir_id == self.create_id { + self.state = VisitorState::CreateFound; + return Continue(()); } - walk_local(self, l) - } - - fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) -> Self::Result { - if self.state != VisitorState::LocalFound { + if self.state != VisitorState::CreateFound { return walk_expr(self, ex); } diff --git a/tests/ui/zombie_processes.rs b/tests/ui/zombie_processes.rs index 395f9dd2defb5..e81b5fd4f3e24 100644 --- a/tests/ui/zombie_processes.rs +++ b/tests/ui/zombie_processes.rs @@ -198,3 +198,13 @@ mod issue14677 { child.wait().unwrap(); } } + +fn issue14911() -> std::io::Result { + let (mut recv, send) = std::io::pipe()?; + let mut command = Command::new("ls") + .stdout(send.try_clone()?) + .spawn() + .expect("Could not spawn new process..."); + command.wait()?; + Ok("".into()) +} From 459280c978b6b4ee7132f628ca480c77fb8cd532 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 28 May 2025 15:18:48 +1000 Subject: [PATCH 12/50] Reorder fields in `hir::ItemKind` variants. Specifically `TyAlias`, `Enum`, `Struct`, `Union`. So the fields match the textual order in the source code. The interesting part of the change is in `compiler/rustc_hir/src/hir.rs`. The rest is extremely mechanical refactoring. --- clippy_lints/src/arbitrary_source_item_ordering.rs | 4 ++-- clippy_lints/src/empty_with_brackets.rs | 2 +- clippy_lints/src/enum_clike.rs | 2 +- clippy_lints/src/excessive_bools.rs | 2 +- clippy_lints/src/exhaustive_items.rs | 2 +- clippy_lints/src/functions/result.rs | 2 +- clippy_lints/src/item_name_repetitions.rs | 4 ++-- clippy_lints/src/large_const_arrays.rs | 2 +- clippy_lints/src/large_enum_variant.rs | 2 +- clippy_lints/src/manual_non_exhaustive.rs | 4 ++-- clippy_lints/src/missing_fields_in_debug.rs | 2 +- clippy_lints/src/non_std_lazy_statics.rs | 2 +- clippy_lints/src/pub_underscore_fields.rs | 2 +- clippy_lints/src/trailing_empty_array.rs | 2 +- clippy_lints/src/types/mod.rs | 2 +- clippy_lints/src/upper_case_acronyms.rs | 2 +- clippy_utils/src/check_proc_macro.rs | 2 +- clippy_utils/src/lib.rs | 2 +- 18 files changed, 21 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index 5c1c85d39184d..59a0c7c88684d 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -272,7 +272,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { return; } match &item.kind { - ItemKind::Enum(_, enum_def, _generics) if self.enable_ordering_for_enum => { + ItemKind::Enum(_, _generics, enum_def) if self.enable_ordering_for_enum => { let mut cur_v: Option<&Variant<'_>> = None; for variant in enum_def.variants { if variant.span.in_external_macro(cx.sess().source_map()) { @@ -288,7 +288,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { cur_v = Some(variant); } }, - ItemKind::Struct(_, VariantData::Struct { fields, .. }, _generics) if self.enable_ordering_for_struct => { + ItemKind::Struct(_, _generics, VariantData::Struct { fields, .. }) if self.enable_ordering_for_struct => { let mut cur_f: Option<&FieldDef<'_>> = None; for field in *fields { if field.span.in_external_macro(cx.sess().source_map()) { diff --git a/clippy_lints/src/empty_with_brackets.rs b/clippy_lints/src/empty_with_brackets.rs index 8c12364883c7e..4414aebbf9a39 100644 --- a/clippy_lints/src/empty_with_brackets.rs +++ b/clippy_lints/src/empty_with_brackets.rs @@ -92,7 +92,7 @@ impl_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VA impl LateLintPass<'_> for EmptyWithBrackets { fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if let ItemKind::Struct(ident, var_data, _) = &item.kind + if let ItemKind::Struct(ident, _, var_data) = &item.kind && !item.span.from_expansion() && has_brackets(var_data) && let span_after_ident = item.span.with_lo(ident.span.hi()) diff --git a/clippy_lints/src/enum_clike.rs b/clippy_lints/src/enum_clike.rs index ec81294624efa..098571a535121 100644 --- a/clippy_lints/src/enum_clike.rs +++ b/clippy_lints/src/enum_clike.rs @@ -38,7 +38,7 @@ impl<'tcx> LateLintPass<'tcx> for UnportableVariant { if cx.tcx.data_layout.pointer_size.bits() != 64 { return; } - if let ItemKind::Enum(_, def, _) = &item.kind { + if let ItemKind::Enum(_, _, def) = &item.kind { for var in def.variants { if let Some(anon_const) = &var.disr_expr { let def_id = cx.tcx.hir_body_owner_def_id(anon_const.body); diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 38d115b878c71..686dc5c3c4fc4 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -127,7 +127,7 @@ fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>, sp: Span, max: u64) { impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { - if let ItemKind::Struct(_, variant_data, _) = &item.kind + if let ItemKind::Struct(_, _, variant_data) = &item.kind && variant_data.fields().len() as u64 > self.max_struct_bools && has_n_bools( variant_data.fields().iter().map(|field| field.ty), diff --git a/clippy_lints/src/exhaustive_items.rs b/clippy_lints/src/exhaustive_items.rs index 5a74e97c97c5e..1fb0e4d24d062 100644 --- a/clippy_lints/src/exhaustive_items.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -76,7 +76,7 @@ impl LateLintPass<'_> for ExhaustiveItems { "exported enums should not be exhaustive", [].as_slice(), ), - ItemKind::Struct(_, v, ..) => ( + ItemKind::Struct(_, _, v) => ( EXHAUSTIVE_STRUCTS, "exported structs should not be exhaustive", v.fields(), diff --git a/clippy_lints/src/functions/result.rs b/clippy_lints/src/functions/result.rs index 00ce4cfcc5266..bb98ae8261119 100644 --- a/clippy_lints/src/functions/result.rs +++ b/clippy_lints/src/functions/result.rs @@ -103,7 +103,7 @@ fn check_result_large_err<'tcx>(cx: &LateContext<'tcx>, err_ty: Ty<'tcx>, hir_ty .did() .as_local() && let hir::Node::Item(item) = cx.tcx.hir_node_by_def_id(local_def_id) - && let hir::ItemKind::Enum(_, ref def, _) = item.kind + && let hir::ItemKind::Enum(_, _, ref def) = item.kind { let variants_size = AdtVariantInfo::new(cx, *adt, subst); if let Some((first_variant, variants)) = variants_size.split_first() diff --git a/clippy_lints/src/item_name_repetitions.rs b/clippy_lints/src/item_name_repetitions.rs index 3d4dcd0207023..9c91cf6808512 100644 --- a/clippy_lints/src/item_name_repetitions.rs +++ b/clippy_lints/src/item_name_repetitions.rs @@ -535,10 +535,10 @@ impl LateLintPass<'_> for ItemNameRepetitions { if span_is_local(item.span) { match item.kind { - ItemKind::Enum(_, def, _) => { + ItemKind::Enum(_, _, def) => { self.check_variants(cx, item, &def); }, - ItemKind::Struct(_, VariantData::Struct { fields, .. }, _) => { + ItemKind::Struct(_, _, VariantData::Struct { fields, .. }) => { self.check_fields(cx, item, fields); }, _ => (), diff --git a/clippy_lints/src/large_const_arrays.rs b/clippy_lints/src/large_const_arrays.rs index 394005e991295..cee8ca1261e53 100644 --- a/clippy_lints/src/large_const_arrays.rs +++ b/clippy_lints/src/large_const_arrays.rs @@ -48,7 +48,7 @@ impl_lint_pass!(LargeConstArrays => [LARGE_CONST_ARRAYS]); impl<'tcx> LateLintPass<'tcx> for LargeConstArrays { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Const(ident, _, generics, _) = &item.kind + if let ItemKind::Const(ident, generics, _, _) = &item.kind // Since static items may not have generics, skip generic const items. // FIXME(generic_const_items): I don't think checking `generics.hwcp` suffices as it // doesn't account for empty where-clauses that only consist of keyword `where` IINM. diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index d08efa0ec9cc0..e85d779b48806 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -73,7 +73,7 @@ impl_lint_pass!(LargeEnumVariant => [LARGE_ENUM_VARIANT]); impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) { - if let ItemKind::Enum(ident, ref def, _) = item.kind + if let ItemKind::Enum(ident, _, ref def) = item.kind && let ty = cx.tcx.type_of(item.owner_id).instantiate_identity() && let ty::Adt(adt, subst) = ty.kind() && adt.variants().len() > 1 diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs index 067b92cd46eed..3562b1ff5ccec 100644 --- a/clippy_lints/src/manual_non_exhaustive.rs +++ b/clippy_lints/src/manual_non_exhaustive.rs @@ -87,7 +87,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualNonExhaustive { } match item.kind { - ItemKind::Enum(_, def, _) if def.variants.len() > 1 => { + ItemKind::Enum(_, _, def) if def.variants.len() > 1 => { let iter = def.variants.iter().filter_map(|v| { (matches!(v.data, VariantData::Unit(_, _)) && is_doc_hidden(cx.tcx.hir_attrs(v.hir_id))) .then_some((v.def_id, v.span)) @@ -98,7 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualNonExhaustive { self.potential_enums.push((item.owner_id.def_id, id, item.span, span)); } }, - ItemKind::Struct(_, variant_data, _) => { + ItemKind::Struct(_, _, variant_data) => { let fields = variant_data.fields(); let private_fields = fields .iter() diff --git a/clippy_lints/src/missing_fields_in_debug.rs b/clippy_lints/src/missing_fields_in_debug.rs index be7dd74fd62b9..d4d33029dbdb1 100644 --- a/clippy_lints/src/missing_fields_in_debug.rs +++ b/clippy_lints/src/missing_fields_in_debug.rs @@ -225,7 +225,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug { && let typeck_results = cx.tcx.typeck_body(*body_id) && should_lint(cx, typeck_results, block) // we intentionally only lint structs, see lint description - && let ItemKind::Struct(_, data, _) = &self_item.kind + && let ItemKind::Struct(_, _, data) = &self_item.kind { check_struct(cx, typeck_results, block, self_ty, item, data); } diff --git a/clippy_lints/src/non_std_lazy_statics.rs b/clippy_lints/src/non_std_lazy_statics.rs index f66b9519317be..abee3c44c5a35 100644 --- a/clippy_lints/src/non_std_lazy_statics.rs +++ b/clippy_lints/src/non_std_lazy_statics.rs @@ -187,7 +187,7 @@ struct LazyInfo { impl LazyInfo { fn from_item(cx: &LateContext<'_>, item: &Item<'_>) -> Option { // Check if item is a `once_cell:sync::Lazy` static. - if let ItemKind::Static(_, ty, _, body_id) = item.kind + if let ItemKind::Static(_, _, ty, body_id) = item.kind && let Some(path_def_id) = path_def_id(cx, ty) && let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = ty.kind && paths::ONCE_CELL_SYNC_LAZY.matches(cx, path_def_id) diff --git a/clippy_lints/src/pub_underscore_fields.rs b/clippy_lints/src/pub_underscore_fields.rs index e4a9bf7a84812..66c59cb70d361 100644 --- a/clippy_lints/src/pub_underscore_fields.rs +++ b/clippy_lints/src/pub_underscore_fields.rs @@ -58,7 +58,7 @@ impl PubUnderscoreFields { impl<'tcx> LateLintPass<'tcx> for PubUnderscoreFields { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { // This lint only pertains to structs. - let ItemKind::Struct(_, variant_data, _) = &item.kind else { + let ItemKind::Struct(_, _, variant_data) = &item.kind else { return; }; diff --git a/clippy_lints/src/trailing_empty_array.rs b/clippy_lints/src/trailing_empty_array.rs index 20bf3a0bff1c3..75a82770af04e 100644 --- a/clippy_lints/src/trailing_empty_array.rs +++ b/clippy_lints/src/trailing_empty_array.rs @@ -57,7 +57,7 @@ impl<'tcx> LateLintPass<'tcx> for TrailingEmptyArray { } fn is_struct_with_trailing_zero_sized_array<'tcx>(cx: &LateContext<'tcx>, item: &Item<'tcx>) -> bool { - if let ItemKind::Struct(_, data, _) = &item.kind + if let ItemKind::Struct(_, _, data) = &item.kind && let Some(last_field) = data.fields().last() && let field_ty = cx.tcx.normalize_erasing_regions( cx.typing_env(), diff --git a/clippy_lints/src/types/mod.rs b/clippy_lints/src/types/mod.rs index c1c7cc5165652..515be5adeed0b 100644 --- a/clippy_lints/src/types/mod.rs +++ b/clippy_lints/src/types/mod.rs @@ -447,7 +447,7 @@ impl<'tcx> LateLintPass<'tcx> for Types { let is_exported = cx.effective_visibilities.is_exported(item.owner_id.def_id); match item.kind { - ItemKind::Static(_, ty, _, _) | ItemKind::Const(_, ty, _, _) => self.check_ty( + ItemKind::Static(_, _, ty, _) | ItemKind::Const(_, _, ty, _) => self.check_ty( cx, ty, CheckTyContext { diff --git a/clippy_lints/src/upper_case_acronyms.rs b/clippy_lints/src/upper_case_acronyms.rs index 8922478e71830..02281b9e9223e 100644 --- a/clippy_lints/src/upper_case_acronyms.rs +++ b/clippy_lints/src/upper_case_acronyms.rs @@ -134,7 +134,7 @@ impl LateLintPass<'_> for UpperCaseAcronyms { ItemKind::TyAlias(ident, ..) | ItemKind::Struct(ident, ..) | ItemKind::Trait(_, _, ident, ..) => { check_ident(cx, &ident, it.hir_id(), self.upper_case_acronyms_aggressive); }, - ItemKind::Enum(ident, ref enumdef, _) => { + ItemKind::Enum(ident, _, ref enumdef) => { check_ident(cx, &ident, it.hir_id(), self.upper_case_acronyms_aggressive); // check enum variants separately because again we only want to lint on private enums and // the fn check_variant does not know about the vis of the enum of its variants diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 004c840c3310b..407e92d88fb0a 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -249,7 +249,7 @@ fn item_search_pat(item: &Item<'_>) -> (Pat, Pat) { ItemKind::ForeignMod { .. } => (Pat::Str("extern"), Pat::Str("}")), ItemKind::TyAlias(..) => (Pat::Str("type"), Pat::Str(";")), ItemKind::Enum(..) => (Pat::Str("enum"), Pat::Str("}")), - ItemKind::Struct(_, VariantData::Struct { .. }, _) => (Pat::Str("struct"), Pat::Str("}")), + ItemKind::Struct(_, _, VariantData::Struct { .. }) => (Pat::Str("struct"), Pat::Str("}")), ItemKind::Struct(..) => (Pat::Str("struct"), Pat::Str(";")), ItemKind::Union(..) => (Pat::Str("union"), Pat::Str("}")), ItemKind::Trait(_, Safety::Unsafe, ..) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8716ee48c88fd..2020f3d6b5bd6 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -2362,7 +2362,7 @@ fn with_test_item_names(tcx: TyCtxt<'_>, module: LocalModDefId, f: impl FnOnce(& for id in tcx.hir_module_free_items(module) { if matches!(tcx.def_kind(id.owner_id), DefKind::Const) && let item = tcx.hir_item(id) - && let ItemKind::Const(ident, ty, _generics, _body) = item.kind + && let ItemKind::Const(ident, _generics, ty, _body) = item.kind && let TyKind::Path(QPath::Resolved(_, path)) = ty.kind // We could also check for the type name `test::TestDescAndFn` && let Res::Def(DefKind::Struct, _) = path.res From e64dd8ed96c7593da92c3d3cf3603a1799064c39 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Tue, 13 May 2025 07:56:04 +0200 Subject: [PATCH 13/50] new restriction lint: pointer_format --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/format_args.rs | 112 +++++++++++++++++++++++++++-- tests/ui/pointer_format.rs | 66 +++++++++++++++++ tests/ui/pointer_format.stderr | 47 ++++++++++++ 5 files changed, 223 insertions(+), 4 deletions(-) create mode 100644 tests/ui/pointer_format.rs create mode 100644 tests/ui/pointer_format.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a98217f625a0..97a8fdedc4dfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6163,6 +6163,7 @@ Released 2018-09-13 [`pathbuf_init_then_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#pathbuf_init_then_push [`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch [`permissions_set_readonly_false`]: https://rust-lang.github.io/rust-clippy/master/index.html#permissions_set_readonly_false +[`pointer_format`]: https://rust-lang.github.io/rust-clippy/master/index.html#pointer_format [`pointers_in_nomem_asm_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#pointers_in_nomem_asm_block [`positional_named_format_parameters`]: https://rust-lang.github.io/rust-clippy/master/index.html#positional_named_format_parameters [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fcb851dfebce..4e6ee06d03577 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -166,6 +166,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO, crate::format::USELESS_FORMAT_INFO, crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO, + crate::format_args::POINTER_FORMAT_INFO, crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO, crate::format_args::UNINLINED_FORMAT_ARGS_INFO, crate::format_args::UNNECESSARY_DEBUG_FORMATTING_INFO, diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index a26e736c7ae35..ab6ae91a00f85 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -1,6 +1,8 @@ +use std::collections::hash_map::Entry; + use arrayvec::ArrayVec; use clippy_config::Conf; -use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::macros::{ FormatArgsStorage, FormatParamUsage, MacroCall, find_format_arg_expr, format_arg_removal_span, format_placeholder_format_span, is_assert_macro, is_format_macro, is_panic, matching_root_macro_call, @@ -22,10 +24,12 @@ use rustc_errors::SuggestionStyle::{CompletelyHidden, ShowCode}; use rustc_hir::{Expr, ExprKind, LangItem}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty::adjustment::{Adjust, Adjustment}; -use rustc_middle::ty::{List, Ty, TyCtxt}; +use rustc_middle::ty::{self, GenericArg, List, TraitRef, Ty, TyCtxt, Upcast}; use rustc_session::impl_lint_pass; use rustc_span::edition::Edition::Edition2021; use rustc_span::{Span, Symbol, sym}; +use rustc_trait_selection::infer::TyCtxtInferExt; +use rustc_trait_selection::traits::{Obligation, ObligationCause, Selection, SelectionContext}; declare_clippy_lint! { /// ### What it does @@ -194,12 +198,41 @@ declare_clippy_lint! { "use of a format specifier that has no effect" } +declare_clippy_lint! { + /// ### What it does + /// Detects [pointer format] as well as `Debug` formatting of raw pointers or function pointers + /// or any types that have a derived `Debug` impl that recursively contains them. + /// + /// ### Why restrict this? + /// The addresses are only useful in very specific contexts, and certain projects may want to keep addresses of + /// certain data structures or functions from prying hacker eyes as an additional line of security. + /// + /// ### Known problems + /// The lint currently only looks through derived `Debug` implementations. Checking whether a manual + /// implementation prints an address is left as an exercise to the next lint implementer. + /// + /// ### Example + /// ```no_run + /// let foo = &0_u32; + /// fn bar() {} + /// println!("{:p}", foo); + /// let _ = format!("{:?}", &(bar as fn())); + /// ``` + /// + /// [pointer format]: https://doc.rust-lang.org/std/fmt/index.html#formatting-traits + #[clippy::version = "1.88.0"] + pub POINTER_FORMAT, + restriction, + "formatting a pointer" +} + impl_lint_pass!(FormatArgs<'_> => [ FORMAT_IN_FORMAT_ARGS, TO_STRING_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, UNNECESSARY_DEBUG_FORMATTING, UNUSED_FORMAT_SPECS, + POINTER_FORMAT, ]); #[allow(clippy::struct_field_names)] @@ -208,6 +241,8 @@ pub struct FormatArgs<'tcx> { msrv: Msrv, ignore_mixed: bool, ty_msrv_map: FxHashMap, Option>, + has_derived_debug: FxHashMap, bool>, + has_pointer_format: FxHashMap, bool>, } impl<'tcx> FormatArgs<'tcx> { @@ -218,6 +253,8 @@ impl<'tcx> FormatArgs<'tcx> { msrv: conf.msrv, ignore_mixed: conf.allow_mixed_uninlined_format_args, ty_msrv_map, + has_derived_debug: FxHashMap::default(), + has_pointer_format: FxHashMap::default(), } } } @@ -228,7 +265,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> { && is_format_macro(cx, macro_call.def_id) && let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn) { - let linter = FormatArgsExpr { + let mut linter = FormatArgsExpr { cx, expr, macro_call: ¯o_call, @@ -236,6 +273,8 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> { ignore_mixed: self.ignore_mixed, msrv: &self.msrv, ty_msrv_map: &self.ty_msrv_map, + has_derived_debug: &mut self.has_derived_debug, + has_pointer_format: &mut self.has_pointer_format, }; linter.check_templates(); @@ -255,10 +294,12 @@ struct FormatArgsExpr<'a, 'tcx> { ignore_mixed: bool, msrv: &'a Msrv, ty_msrv_map: &'a FxHashMap, Option>, + has_derived_debug: &'a mut FxHashMap, bool>, + has_pointer_format: &'a mut FxHashMap, bool>, } impl<'tcx> FormatArgsExpr<'_, 'tcx> { - fn check_templates(&self) { + fn check_templates(&mut self) { for piece in &self.format_args.template { if let FormatArgsPiece::Placeholder(placeholder) = piece && let Ok(index) = placeholder.argument.index @@ -279,6 +320,17 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> { if placeholder.format_trait == FormatTrait::Debug { let name = self.cx.tcx.item_name(self.macro_call.def_id); self.check_unnecessary_debug_formatting(name, arg_expr); + if let Some(span) = placeholder.span + && self.has_pointer_debug(self.cx.typeck_results().expr_ty(arg_expr), 0) + { + span_lint(self.cx, POINTER_FORMAT, span, "pointer formatting detected"); + } + } + + if placeholder.format_trait == FormatTrait::Pointer + && let Some(span) = placeholder.span + { + span_lint(self.cx, POINTER_FORMAT, span, "pointer formatting detected"); } } } @@ -559,6 +611,58 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> { false } + + fn has_pointer_debug(&mut self, ty: Ty<'tcx>, depth: usize) -> bool { + let cx = self.cx; + let tcx = cx.tcx; + if !tcx.recursion_limit().value_within_limit(depth) { + return false; + } + let depth = depth + 1; + let typing_env = cx.typing_env(); + let ty = tcx.normalize_erasing_regions(typing_env, ty); + match ty.kind() { + ty::RawPtr(..) | ty::FnPtr(..) | ty::FnDef(..) => true, + ty::Ref(_, t, _) | ty::Slice(t) | ty::Array(t, _) => self.has_pointer_debug(*t, depth), + ty::Tuple(ts) => ts.iter().any(|t| self.has_pointer_debug(t, depth)), + ty::Adt(adt, args) => { + match self.has_pointer_format.entry(ty) { + Entry::Occupied(o) => return *o.get(), + Entry::Vacant(v) => v.insert(false), + }; + let derived_debug = if let Some(&known) = self.has_derived_debug.get(&ty) { + known + } else { + let Some(trait_id) = tcx.get_diagnostic_item(sym::Debug) else { + return false; + }; + let (infcx, param_env) = tcx.infer_ctxt().build_with_typing_env(typing_env); + let trait_ref = TraitRef::new(tcx, trait_id, [GenericArg::from(ty)]); + let obligation = Obligation { + cause: ObligationCause::dummy(), + param_env, + recursion_depth: 0, + predicate: trait_ref.upcast(tcx), + }; + let selection = SelectionContext::new(&infcx).select(&obligation); + let derived = if let Ok(Some(Selection::UserDefined(data))) = selection { + tcx.has_attr(data.impl_def_id, sym::automatically_derived) + } else { + false + }; + self.has_derived_debug.insert(ty, derived); + derived + }; + let pointer_debug = derived_debug + && adt.all_fields().any(|f| { + self.has_pointer_debug(tcx.normalize_erasing_regions(typing_env, f.ty(tcx, args)), depth) + }); + self.has_pointer_format.insert(ty, pointer_debug); + pointer_debug + }, + _ => false, + } + } } fn make_ty_msrv_map(tcx: TyCtxt<'_>) -> FxHashMap, Option> { diff --git a/tests/ui/pointer_format.rs b/tests/ui/pointer_format.rs new file mode 100644 index 0000000000000..0621f966ad111 --- /dev/null +++ b/tests/ui/pointer_format.rs @@ -0,0 +1,66 @@ +#![warn(clippy::pointer_format)] + +use core::fmt::Debug; +use core::marker::PhantomData; + +#[derive(Debug)] +struct ContainsPointerDeep { + w: WithPointer, +} + +struct ManualDebug { + ptr: *const u8, +} + +#[derive(Debug)] +struct WithPointer { + len: usize, + ptr: *const u8, +} + +impl std::fmt::Debug for ManualDebug { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("ManualDebug") + } +} + +trait Foo { + type Assoc: Foo + Debug; +} + +#[derive(Debug)] +struct S(&'static S, PhantomData); + +#[allow(unused)] +fn unbounded(s: &S) { + format!("{s:?}"); +} + +fn main() { + let m = &(main as fn()); + let g = &0; + let o = &format!("{m:p}"); + //~^ pointer_format + let _ = format!("{m:?}"); + //~^ pointer_format + println!("{g:p}"); + //~^ pointer_format + panic!("{o:p}"); + //~^ pointer_format + let answer = 42; + let x = &raw const answer; + let arr = [0u8; 8]; + let with_ptr = WithPointer { len: 8, ptr: &arr as _ }; + let _ = format!("{x:?}"); + //~^ pointer_format + print!("{with_ptr:?}"); + //~^ pointer_format + let container = ContainsPointerDeep { w: with_ptr }; + print!("{container:?}"); + //~^ pointer_format + + let no_pointer = "foo"; + println!("{no_pointer:?}"); + let manual_debug = ManualDebug { ptr: &arr as _ }; + println!("{manual_debug:?}"); +} diff --git a/tests/ui/pointer_format.stderr b/tests/ui/pointer_format.stderr new file mode 100644 index 0000000000000..21ba39b8f8cc9 --- /dev/null +++ b/tests/ui/pointer_format.stderr @@ -0,0 +1,47 @@ +error: pointer formatting detected + --> tests/ui/pointer_format.rs:42:23 + | +LL | let o = &format!("{m:p}"); + | ^^^^^ + | + = note: `-D clippy::pointer-format` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::pointer_format)]` + +error: pointer formatting detected + --> tests/ui/pointer_format.rs:44:22 + | +LL | let _ = format!("{m:?}"); + | ^^^^^ + +error: pointer formatting detected + --> tests/ui/pointer_format.rs:46:15 + | +LL | println!("{g:p}"); + | ^^^^^ + +error: pointer formatting detected + --> tests/ui/pointer_format.rs:48:13 + | +LL | panic!("{o:p}"); + | ^^^^^ + +error: pointer formatting detected + --> tests/ui/pointer_format.rs:54:22 + | +LL | let _ = format!("{x:?}"); + | ^^^^^ + +error: pointer formatting detected + --> tests/ui/pointer_format.rs:56:13 + | +LL | print!("{with_ptr:?}"); + | ^^^^^^^^^^^^ + +error: pointer formatting detected + --> tests/ui/pointer_format.rs:59:13 + | +LL | print!("{container:?}"); + | ^^^^^^^^^^^^^ + +error: aborting due to 7 previous errors + From 9f86a576e30d85912602f67b605ff3a33a50401f Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Sat, 31 May 2025 14:22:15 +0200 Subject: [PATCH 14/50] Merge commit '57cbadd68ac473bc50453f6b1320a02b68115f12' --- Cargo.toml | 2 +- book/src/development/trait_checking.md | 4 +- book/src/lint_configuration.md | 2 +- clippy_config/src/conf.rs | 2 +- clippy_dev/src/fmt.rs | 133 +++++-------- clippy_dev/src/main.rs | 2 +- clippy_dev/src/rename_lint.rs | 20 +- clippy_dev/src/update_lints.rs | 7 +- clippy_dev/src/utils.rs | 140 ++++++++++---- clippy_lints/src/dbg_macro.rs | 122 +++++++----- clippy_lints/src/dereference.rs | 19 +- clippy_lints/src/doc/lazy_continuation.rs | 88 ++++----- clippy_lints/src/doc/markdown.rs | 56 ++++-- clippy_lints/src/doc/mod.rs | 27 +-- clippy_lints/src/loops/manual_find.rs | 7 + clippy_lints/src/loops/manual_flatten.rs | 5 +- clippy_lints/src/loops/while_let_loop.rs | 103 ++++++---- .../src/methods/manual_is_variant_and.rs | 70 ++++++- clippy_lints/src/methods/mod.rs | 3 +- clippy_lints/src/methods/or_fun_call.rs | 3 +- .../src/methods/unnecessary_to_owned.rs | 2 +- clippy_lints/src/mut_reference.rs | 32 ++-- clippy_lints/src/needless_for_each.rs | 32 +++- .../src/operators/assign_op_pattern.rs | 14 +- clippy_lints/src/operators/mod.rs | 2 +- .../src/operators/modulo_arithmetic.rs | 45 ++--- clippy_lints/src/panic_unimplemented.rs | 1 - clippy_lints/src/pass_by_ref_or_value.rs | 22 +-- clippy_lints/src/returns.rs | 4 +- clippy_lints/src/unit_return_expecting_ord.rs | 126 +++++++----- clippy_utils/README.md | 2 +- clippy_utils/src/ast_utils/mod.rs | 4 +- clippy_utils/src/lib.rs | 14 +- clippy_utils/src/qualify_min_const_fn.rs | 3 +- clippy_utils/src/sugg.rs | 2 +- lintcheck/src/input.rs | 21 +- lintcheck/src/main.rs | 20 +- lintcheck/src/output.rs | 6 +- rust-toolchain.toml | 2 +- rustfmt.toml | 6 +- tests/headers.rs | 34 ---- tests/ui/assign_ops.fixed | 31 +++ tests/ui/assign_ops.rs | 31 +++ tests/ui/assign_ops.stderr | 24 +-- tests/ui/dbg_macro/dbg_macro.fixed | 16 ++ tests/ui/dbg_macro/dbg_macro.rs | 16 ++ tests/ui/dbg_macro/dbg_macro.stderr | 14 +- tests/ui/doc/doc_markdown-issue_13097.fixed | 36 +++- tests/ui/doc/doc_markdown-issue_13097.rs | 36 +++- tests/ui/doc/doc_markdown-issue_13097.stderr | 74 +++++++- tests/ui/explicit_deref_methods.fixed | 6 +- tests/ui/explicit_deref_methods.rs | 2 - tests/ui/explicit_deref_methods.stderr | 22 +-- tests/ui/manual_find_fixable.fixed | 10 + tests/ui/manual_find_fixable.rs | 22 +++ tests/ui/manual_find_fixable.stderr | 24 ++- tests/ui/manual_flatten.rs | 7 + tests/ui/manual_flatten.stderr | 4 +- tests/ui/manual_is_variant_and.fixed | 62 ++++++ tests/ui/manual_is_variant_and.rs | 62 ++++++ tests/ui/manual_is_variant_and.stderr | 60 +++++- tests/ui/needless_borrow.fixed | 15 +- tests/ui/needless_borrow.rs | 15 +- tests/ui/needless_borrow.stderr | 32 ++-- tests/ui/needless_for_each_fixable.fixed | 15 ++ tests/ui/needless_for_each_fixable.rs | 15 ++ tests/ui/needless_for_each_fixable.stderr | 14 +- tests/ui/needless_return.fixed | 65 +++++++ tests/ui/needless_return.rs | 65 +++++++ tests/ui/needless_return.stderr | 98 +++++++++- .../non_expressive_names_error_recovery.fixed | 0 .../non_expressive_names_error_recovery.rs | 0 ...non_expressive_names_error_recovery.stderr | 2 +- tests/ui/question_mark.fixed | 10 + tests/ui/question_mark.rs | 10 + tests/ui/trivially_copy_pass_by_ref.fixed | 179 ++++++++++++++++++ tests/ui/trivially_copy_pass_by_ref.rs | 60 +++--- tests/ui/trivially_copy_pass_by_ref.stderr | 80 ++++---- tests/ui/while_let_loop.rs | 86 +++++++++ tests/ui/while_let_loop.stderr | 122 +++++++++++- triagebot.toml | 1 + util/gh-pages/index_template.html | 2 +- util/gh-pages/script.js | 12 ++ 83 files changed, 1995 insertions(+), 671 deletions(-) delete mode 100644 tests/headers.rs rename tests/ui/{skip_rustfmt => }/non_expressive_names_error_recovery.fixed (100%) rename tests/ui/{skip_rustfmt => }/non_expressive_names_error_recovery.rs (100%) rename tests/ui/{skip_rustfmt => }/non_expressive_names_error_recovery.stderr (81%) create mode 100644 tests/ui/trivially_copy_pass_by_ref.fixed diff --git a/Cargo.toml b/Cargo.toml index f69e5bee4bb2b..3a76c61489e24 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,7 +42,7 @@ walkdir = "2.3" filetime = "0.2.9" itertools = "0.12" pulldown-cmark = { version = "0.11", default-features = false, features = ["html"] } -askama = { version = "0.13", default-features = false, features = ["alloc", "config", "derive"] } +askama = { version = "0.14", default-features = false, features = ["alloc", "config", "derive"] } # UI test dependencies if_chain = "1.0" diff --git a/book/src/development/trait_checking.md b/book/src/development/trait_checking.md index cc4eb966f596a..6d01496eebe07 100644 --- a/book/src/development/trait_checking.md +++ b/book/src/development/trait_checking.md @@ -17,7 +17,7 @@ providing the `LateContext` (`cx`), our expression at hand, and the symbol of the trait in question: ```rust -use clippy_utils::is_trait_method; +use clippy_utils::ty::implements_trait; use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_span::symbol::sym; @@ -25,7 +25,7 @@ use rustc_span::symbol::sym; impl LateLintPass<'_> for CheckIteratorTraitLint { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { let implements_iterator = cx.tcx.get_diagnostic_item(sym::Iterator).map_or(false, |id| { - implements_trait(cx, cx.typeck_results().expr_ty(arg), id, &[]) + implements_trait(cx, cx.typeck_results().expr_ty(expr), id, &[]) }); if implements_iterator { // [...] diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 9809e32de8a44..7c850b4b023a7 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -1026,7 +1026,7 @@ The order of associated items in traits. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference. -**Default Value:** `target_pointer_width * 2` +**Default Value:** `target_pointer_width` --- **Affected lints:** diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 4ce8d001c2f00..87158cec42b24 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -828,7 +828,7 @@ define_Conf! { trait_assoc_item_kinds_order: SourceItemOrderingTraitAssocItemKinds = DEFAULT_TRAIT_ASSOC_ITEM_KINDS_ORDER.into(), /// The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by /// reference. - #[default_text = "target_pointer_width * 2"] + #[default_text = "target_pointer_width"] #[lints(trivially_copy_pass_by_ref)] trivial_copy_size_limit: Option = None, /// The maximum complexity a type can have diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 13d6b1285dcdb..c1b6b37070696 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -1,5 +1,6 @@ use crate::utils::{ - ClippyInfo, ErrAction, FileUpdater, UpdateMode, UpdateStatus, panic_action, run_with_args_split, run_with_output, + ErrAction, FileUpdater, UpdateMode, UpdateStatus, expect_action, run_with_output, split_args_for_threads, + walk_dir_no_dot_or_target, }; use itertools::Itertools; use rustc_lexer::{TokenKind, tokenize}; @@ -9,7 +10,6 @@ use std::io::{self, Read}; use std::ops::ControlFlow; use std::path::PathBuf; use std::process::{self, Command, Stdio}; -use walkdir::WalkDir; pub enum Error { Io(io::Error), @@ -260,7 +260,7 @@ fn fmt_syms(update_mode: UpdateMode) { ); } -fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { +fn run_rustfmt(update_mode: UpdateMode) { let mut rustfmt_path = String::from_utf8(run_with_output( "rustup which rustfmt", Command::new("rustup").args(["which", "rustfmt"]), @@ -268,42 +268,19 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { .expect("invalid rustfmt path"); rustfmt_path.truncate(rustfmt_path.trim_end().len()); - let mut cargo_path = String::from_utf8(run_with_output( - "rustup which cargo", - Command::new("rustup").args(["which", "cargo"]), - )) - .expect("invalid cargo path"); - cargo_path.truncate(cargo_path.trim_end().len()); - - // Start all format jobs first before waiting on the results. - let mut children = Vec::with_capacity(16); - for &path in &[ - ".", - "clippy_config", - "clippy_dev", - "clippy_lints", - "clippy_lints_internal", - "clippy_utils", - "rustc_tools_util", - "lintcheck", - ] { - let mut cmd = Command::new(&cargo_path); - cmd.current_dir(clippy.path.join(path)) - .args(["fmt"]) - .env("RUSTFMT", &rustfmt_path) - .stdout(Stdio::null()) - .stdin(Stdio::null()) - .stderr(Stdio::piped()); - if update_mode.is_check() { - cmd.arg("--check"); - } - match cmd.spawn() { - Ok(x) => children.push(("cargo fmt", x)), - Err(ref e) => panic_action(&e, ErrAction::Run, "cargo fmt".as_ref()), - } - } + let args: Vec<_> = walk_dir_no_dot_or_target() + .filter_map(|e| { + let e = expect_action(e, ErrAction::Read, "."); + e.path() + .as_os_str() + .as_encoded_bytes() + .ends_with(b".rs") + .then(|| e.into_path().into_os_string()) + }) + .collect(); - run_with_args_split( + let mut children: Vec<_> = split_args_for_threads( + 32, || { let mut cmd = Command::new(&rustfmt_path); if update_mode.is_check() { @@ -312,66 +289,44 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) { cmd.stdout(Stdio::null()) .stdin(Stdio::null()) .stderr(Stdio::piped()) - .args(["--config", "show_parse_errors=false"]); + .args(["--unstable-features", "--skip-children"]); cmd }, - |cmd| match cmd.spawn() { - Ok(x) => children.push(("rustfmt", x)), - Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()), - }, - WalkDir::new("tests") - .into_iter() - .filter_entry(|p| p.path().file_name().is_none_or(|x| x != "skip_rustfmt")) - .filter_map(|e| { - let e = e.expect("error reading `tests`"); - e.path() - .as_os_str() - .as_encoded_bytes() - .ends_with(b".rs") - .then(|| e.into_path().into_os_string()) - }), - ); + args.iter(), + ) + .map(|mut cmd| expect_action(cmd.spawn(), ErrAction::Run, "rustfmt")) + .collect(); - for (name, child) in &mut children { - match child.wait() { - Ok(status) => match (update_mode, status.exit_ok()) { - (UpdateMode::Check | UpdateMode::Change, Ok(())) => {}, - (UpdateMode::Check, Err(_)) => { - let mut s = String::new(); - if let Some(mut stderr) = child.stderr.take() - && stderr.read_to_string(&mut s).is_ok() - { - eprintln!("{s}"); - } - eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update."); - process::exit(1); - }, - (UpdateMode::Change, Err(e)) => { - let mut s = String::new(); - if let Some(mut stderr) = child.stderr.take() - && stderr.read_to_string(&mut s).is_ok() - { - eprintln!("{s}"); - } - panic_action(&e, ErrAction::Run, name.as_ref()); - }, + for child in &mut children { + let status = expect_action(child.wait(), ErrAction::Run, "rustfmt"); + match (update_mode, status.exit_ok()) { + (UpdateMode::Check | UpdateMode::Change, Ok(())) => {}, + (UpdateMode::Check, Err(_)) => { + let mut s = String::new(); + if let Some(mut stderr) = child.stderr.take() + && stderr.read_to_string(&mut s).is_ok() + { + eprintln!("{s}"); + } + eprintln!("Formatting check failed!\nRun `cargo dev fmt` to update."); + process::exit(1); + }, + (UpdateMode::Change, e) => { + let mut s = String::new(); + if let Some(mut stderr) = child.stderr.take() + && stderr.read_to_string(&mut s).is_ok() + { + eprintln!("{s}"); + } + expect_action(e, ErrAction::Run, "rustfmt"); }, - Err(ref e) => panic_action(e, ErrAction::Run, name.as_ref()), } } } // the "main" function of cargo dev fmt -pub fn run(clippy: &ClippyInfo, update_mode: UpdateMode) { - if clippy.has_intellij_hook { - eprintln!( - "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\ - Not formatting because that would format the local repo as well!\n\ - Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`." - ); - return; - } - run_rustfmt(clippy, update_mode); +pub fn run(update_mode: UpdateMode) { + run_rustfmt(update_mode); fmt_syms(update_mode); if let Err(e) = fmt_conf(update_mode.is_check()) { e.display(); diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index ebcd8611d78cd..26aa269fb6388 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -26,7 +26,7 @@ fn main() { allow_staged, allow_no_vcs, } => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs), - DevCommand::Fmt { check } => fmt::run(&clippy, utils::UpdateMode::from_check(check)), + DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)), DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)), DevCommand::NewLint { pass, diff --git a/clippy_dev/src/rename_lint.rs b/clippy_dev/src/rename_lint.rs index be8b27c7a9e9f..d62597428e210 100644 --- a/clippy_dev/src/rename_lint.rs +++ b/clippy_dev/src/rename_lint.rs @@ -1,13 +1,12 @@ use crate::update_lints::{RenamedLint, find_lint_decls, generate_lint_files, read_deprecated_lints}; use crate::utils::{ - FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, delete_file_if_exists, - try_rename_dir, try_rename_file, + ErrAction, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, Version, delete_dir_if_exists, + delete_file_if_exists, expect_action, try_rename_dir, try_rename_file, walk_dir_no_dot_or_target, }; use rustc_lexer::TokenKind; use std::ffi::OsString; use std::fs; use std::path::Path; -use walkdir::WalkDir; /// Runs the `rename_lint` command. /// @@ -133,17 +132,10 @@ pub fn rename(clippy_version: Version, old_name: &str, new_name: &str, uplift: b } let mut update_fn = file_update_fn(old_name, new_name, mod_edit); - for file in WalkDir::new(".").into_iter().filter_entry(|e| { - // Skip traversing some of the larger directories. - e.path() - .as_os_str() - .as_encoded_bytes() - .get(2..) - .is_none_or(|x| x != "target".as_bytes() && x != ".git".as_bytes()) - }) { - let file = file.expect("error reading clippy directory"); - if file.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { - updater.update_file(file.path(), &mut update_fn); + for e in walk_dir_no_dot_or_target() { + let e = expect_action(e, ErrAction::Read, "."); + if e.path().as_os_str().as_encoded_bytes().ends_with(b".rs") { + updater.update_file(e.path(), &mut update_fn); } } generate_lint_files(UpdateMode::Change, &lints, &deprecated_lints, &renamed_lints); diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 25ba2c7204904..320462a2c9688 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -1,5 +1,5 @@ use crate::utils::{ - ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, panic_action, update_text_region_fn, + ErrAction, File, FileUpdater, RustSearcher, Token, UpdateMode, UpdateStatus, expect_action, update_text_region_fn, }; use itertools::Itertools; use std::collections::HashSet; @@ -201,10 +201,7 @@ pub fn find_lint_decls() -> Vec { /// Reads the source files from the given root directory fn read_src_with_module(src_root: &Path) -> impl use<'_> + Iterator { WalkDir::new(src_root).into_iter().filter_map(move |e| { - let e = match e { - Ok(e) => e, - Err(ref e) => panic_action(e, ErrAction::Read, src_root), - }; + let e = expect_action(e, ErrAction::Read, src_root); let path = e.path().as_os_str().as_encoded_bytes(); if let Some(path) = path.strip_suffix(b".rs") && let Some(path) = path.get("clippy_lints/src/".len()..) diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs index 255e36afe69c2..c4808b7048b03 100644 --- a/clippy_dev/src/utils.rs +++ b/clippy_dev/src/utils.rs @@ -1,14 +1,16 @@ use core::fmt::{self, Display}; +use core::num::NonZero; use core::ops::Range; use core::slice; use core::str::FromStr; use rustc_lexer::{self as lexer, FrontmatterAllowed}; -use std::env; use std::ffi::OsStr; use std::fs::{self, OpenOptions}; use std::io::{self, Read as _, Seek as _, SeekFrom, Write}; use std::path::{Path, PathBuf}; use std::process::{self, Command, ExitStatus, Stdio}; +use std::{env, thread}; +use walkdir::WalkDir; #[cfg(not(windows))] static CARGO_CLIPPY_EXE: &str = "cargo-clippy"; @@ -45,6 +47,14 @@ pub fn panic_action(err: &impl Display, action: ErrAction, path: &Path) -> ! { panic!("error {} `{}`: {}", action.as_str(), path.display(), *err) } +#[track_caller] +pub fn expect_action(res: Result, action: ErrAction, path: impl AsRef) -> T { + match res { + Ok(x) => x, + Err(ref e) => panic_action(e, action, path.as_ref()), + } +} + /// Wrapper around `std::fs::File` which panics with a path on failure. pub struct File<'a> { pub inner: fs::File, @@ -55,9 +65,9 @@ impl<'a> File<'a> { #[track_caller] pub fn open(path: &'a (impl AsRef + ?Sized), options: &mut OpenOptions) -> Self { let path = path.as_ref(); - match options.open(path) { - Ok(inner) => Self { inner, path }, - Err(e) => panic_action(&e, ErrAction::Open, path), + Self { + inner: expect_action(options.open(path), ErrAction::Open, path), + path, } } @@ -84,10 +94,7 @@ impl<'a> File<'a> { /// Read the entire contents of a file to the given buffer. #[track_caller] pub fn read_append_to_string<'dst>(&mut self, dst: &'dst mut String) -> &'dst mut String { - match self.inner.read_to_string(dst) { - Ok(_) => {}, - Err(e) => panic_action(&e, ErrAction::Read, self.path), - } + expect_action(self.inner.read_to_string(dst), ErrAction::Read, self.path); dst } @@ -107,9 +114,7 @@ impl<'a> File<'a> { }, Err(e) => Err(e), }; - if let Err(e) = res { - panic_action(&e, ErrAction::Write, self.path); - } + expect_action(res, ErrAction::Write, self.path); } } @@ -660,47 +665,91 @@ pub fn try_rename_dir(old_name: &Path, new_name: &Path) -> bool { } pub fn write_file(path: &Path, contents: &str) { - fs::write(path, contents).unwrap_or_else(|e| panic_action(&e, ErrAction::Write, path)); + expect_action(fs::write(path, contents), ErrAction::Write, path); } #[must_use] pub fn run_with_output(path: &(impl AsRef + ?Sized), cmd: &mut Command) -> Vec { fn f(path: &Path, cmd: &mut Command) -> Vec { - match cmd - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::inherit()) - .output() - { - Ok(x) => match x.status.exit_ok() { - Ok(()) => x.stdout, - Err(ref e) => panic_action(e, ErrAction::Run, path), - }, - Err(ref e) => panic_action(e, ErrAction::Run, path), - } + let output = expect_action( + cmd.stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .output(), + ErrAction::Run, + path, + ); + expect_action(output.status.exit_ok(), ErrAction::Run, path); + output.stdout } f(path.as_ref(), cmd) } -pub fn run_with_args_split( - mut make_cmd: impl FnMut() -> Command, - mut run_cmd: impl FnMut(&mut Command), - args: impl Iterator>, -) { - let mut cmd = make_cmd(); - let mut len = 0; - for arg in args { - len += arg.as_ref().len(); - cmd.arg(arg); - // Very conservative limit - if len > 10000 { - run_cmd(&mut cmd); - cmd = make_cmd(); - len = 0; +/// Splits an argument list across multiple `Command` invocations. +/// +/// The argument list will be split into a number of batches based on +/// `thread::available_parallelism`, with `min_batch_size` setting a lower bound on the size of each +/// batch. +/// +/// If the size of the arguments would exceed the system limit additional batches will be created. +pub fn split_args_for_threads( + min_batch_size: usize, + make_cmd: impl FnMut() -> Command, + args: impl ExactSizeIterator>, +) -> impl Iterator { + struct Iter { + make_cmd: F, + args: I, + min_batch_size: usize, + batch_size: usize, + thread_count: usize, + } + impl Iterator for Iter + where + F: FnMut() -> Command, + I: ExactSizeIterator>, + { + type Item = Command; + fn next(&mut self) -> Option { + if self.thread_count > 1 { + self.thread_count -= 1; + } + let mut cmd = (self.make_cmd)(); + let mut cmd_len = 0usize; + for arg in self.args.by_ref().take(self.batch_size) { + cmd.arg(arg.as_ref()); + // `+ 8` to account for the `argv` pointer on unix. + // Windows is complicated since the arguments are first converted to UTF-16ish, + // but this needs to account for the space between arguments and whatever additional + // is needed to escape within an argument. + cmd_len += arg.as_ref().len() + 8; + cmd_len += 8; + + // Windows has a command length limit of 32767. For unix systems this is more + // complicated since the limit includes environment variables and room needs to be + // left to edit them once the program starts, but the total size comes from + // `getconf ARG_MAX`. + // + // For simplicity we use 30000 here under a few assumptions. + // * Individual arguments aren't super long (the final argument is still added) + // * `ARG_MAX` is set to a reasonable amount. Basically every system will be configured way above + // what windows supports, but POSIX only requires `4096`. + if cmd_len > 30000 { + self.batch_size = self.args.len().div_ceil(self.thread_count).max(self.min_batch_size); + break; + } + } + (cmd_len != 0).then_some(cmd) } } - if len != 0 { - run_cmd(&mut cmd); + let thread_count = thread::available_parallelism().map_or(1, NonZero::get); + let batch_size = args.len().div_ceil(thread_count).max(min_batch_size); + Iter { + make_cmd, + args, + min_batch_size, + batch_size, + thread_count, } } @@ -720,3 +769,12 @@ pub fn delete_dir_if_exists(path: &Path) { Err(ref e) => panic_action(e, ErrAction::Delete, path), } } + +/// Walks all items excluding top-level dot files/directories and any target directories. +pub fn walk_dir_no_dot_or_target() -> impl Iterator> { + WalkDir::new(".").into_iter().filter_entry(|e| { + e.path() + .file_name() + .is_none_or(|x| x != "target" && x.as_encoded_bytes().first().copied() != Some(b'.')) + }) +} diff --git a/clippy_lints/src/dbg_macro.rs b/clippy_lints/src/dbg_macro.rs index 06376c57119d5..152516baf7342 100644 --- a/clippy_lints/src/dbg_macro.rs +++ b/clippy_lints/src/dbg_macro.rs @@ -5,7 +5,7 @@ use clippy_utils::macros::{MacroCall, macro_backtrace}; use clippy_utils::source::snippet_with_applicability; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, Node}; +use rustc_hir::{Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; use rustc_span::{Span, SyntaxContext, sym}; @@ -60,6 +60,8 @@ impl LateLintPass<'_> for DbgMacro { if cur_syntax_ctxt != self.prev_ctxt && let Some(macro_call) = first_dbg_macro_in_expansion(cx, expr.span) && !macro_call.span.in_external_macro(cx.sess().source_map()) && + // avoids exprs generated by the desugaring of coroutines + !is_coroutine_desugar(expr) && self.checked_dbg_call_site.insert(macro_call.span) && // allows `dbg!` in test code if allow-dbg-in-test is set to true in clippy.toml !(self.allow_dbg_in_tests && is_in_test(cx.tcx, expr.hir_id)) @@ -73,50 +75,51 @@ impl LateLintPass<'_> for DbgMacro { "the `dbg!` macro is intended as a debugging tool", |diag| { let mut applicability = Applicability::MachineApplicable; - - let (sugg_span, suggestion) = match expr.peel_drop_temps().kind { - // dbg!() - ExprKind::Block(..) => { - // If the `dbg!` macro is a "free" statement and not contained within other expressions, - // remove the whole statement. - if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id) - && let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span) - { - (macro_call.span.to(semi_span), String::new()) - } else { - (macro_call.span, String::from("()")) - } - }, - // dbg!(1) - ExprKind::Match(val, ..) => ( - macro_call.span, - snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) - .to_string(), - ), - // dbg!(2, 3) - ExprKind::Tup( - [ - Expr { - kind: ExprKind::Match(first, ..), - .. - }, - .., - Expr { - kind: ExprKind::Match(last, ..), - .. - }, - ], - ) => { - let snippet = snippet_with_applicability( - cx, - first.span.source_callsite().to(last.span.source_callsite()), - "..", - &mut applicability, - ); - (macro_call.span, format!("({snippet})")) - }, - _ => unreachable!(), - }; + let (sugg_span, suggestion) = + match is_async_move_desugar(expr).unwrap_or(expr).peel_drop_temps().kind { + // dbg!() + ExprKind::Block(..) => { + // If the `dbg!` macro is a "free" statement and not contained within other expressions, + // remove the whole statement. + if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id) + && let Some(semi_span) = + cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span) + { + (macro_call.span.to(semi_span), String::new()) + } else { + (macro_call.span, String::from("()")) + } + }, + // dbg!(1) + ExprKind::Match(val, ..) => ( + macro_call.span, + snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) + .to_string(), + ), + // dbg!(2, 3) + ExprKind::Tup( + [ + Expr { + kind: ExprKind::Match(first, ..), + .. + }, + .., + Expr { + kind: ExprKind::Match(last, ..), + .. + }, + ], + ) => { + let snippet = snippet_with_applicability( + cx, + first.span.source_callsite().to(last.span.source_callsite()), + "..", + &mut applicability, + ); + (macro_call.span, format!("({snippet})")) + }, + _ => unreachable!(), + }; diag.span_suggestion( sugg_span, @@ -134,6 +137,35 @@ impl LateLintPass<'_> for DbgMacro { } } +fn is_coroutine_desugar(expr: &Expr<'_>) -> bool { + matches!( + expr.kind, + ExprKind::Closure(Closure { + kind: ClosureKind::Coroutine(CoroutineKind::Desugared(..)) | ClosureKind::CoroutineClosure(..), + .. + }) + ) +} + +fn is_async_move_desugar<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> { + if let ExprKind::Block(block, _) = expr.kind + && let [ + Stmt { + kind: + StmtKind::Let(LetStmt { + source: LocalSource::AsyncFn, + .. + }), + .. + }, + ] = block.stmts + { + return block.expr; + } + + None +} + fn first_dbg_macro_in_expansion(cx: &LateContext<'_>, span: Span) -> Option { macro_backtrace(span).find(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id)) } diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index a22a2ee66d258..cde9528cd8789 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -305,7 +305,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> { RefOp::Method { mutbl, is_ufcs } if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) // Allow explicit deref in method chains. e.g. `foo.deref().bar()` - && (is_ufcs || !in_postfix_position(cx, expr)) => + && (is_ufcs || !is_in_method_chain(cx, expr)) => { let ty_changed_count = usize::from(!deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr))); self.state = Some(( @@ -728,7 +728,13 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool { } } -fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool { +fn is_in_method_chain<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool { + if let ExprKind::MethodCall(_, recv, _, _) = e.kind + && matches!(recv.kind, ExprKind::MethodCall(..)) + { + return true; + } + if let Some(parent) = get_parent_expr(cx, e) && parent.span.eq_ctxt(e.span) { @@ -986,6 +992,15 @@ fn report<'tcx>( ); }, State::DerefedBorrow(state) => { + // Do not suggest removing a non-mandatory `&` in `&*rawptr` in an `unsafe` context, + // as this may make rustc trigger its `dangerous_implicit_autorefs` lint. + if let ExprKind::AddrOf(BorrowKind::Ref, _, subexpr) = data.first_expr.kind + && let ExprKind::Unary(UnOp::Deref, subsubexpr) = subexpr.kind + && cx.typeck_results().expr_ty_adjusted(subsubexpr).is_raw_ptr() + { + return; + } + let mut app = Applicability::MachineApplicable; let (snip, snip_is_macro) = snippet_with_context(cx, expr.span, data.first_expr.span.ctxt(), "..", &mut app); diff --git a/clippy_lints/src/doc/lazy_continuation.rs b/clippy_lints/src/doc/lazy_continuation.rs index 8aeb835fe3934..cb9d68224dbb8 100644 --- a/clippy_lints/src/doc/lazy_continuation.rs +++ b/clippy_lints/src/doc/lazy_continuation.rs @@ -2,11 +2,10 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use itertools::Itertools; use rustc_errors::Applicability; use rustc_lint::LateContext; -use rustc_span::{BytePos, Span}; -use std::cmp::Ordering; +use rustc_span::BytePos; use std::ops::Range; -use super::{DOC_LAZY_CONTINUATION, DOC_OVERINDENTED_LIST_ITEMS}; +use super::{DOC_LAZY_CONTINUATION, DOC_OVERINDENTED_LIST_ITEMS, Fragments}; fn map_container_to_text(c: &super::Container) -> &'static str { match c { @@ -19,29 +18,27 @@ fn map_container_to_text(c: &super::Container) -> &'static str { pub(super) fn check( cx: &LateContext<'_>, doc: &str, - range: Range, - mut span: Span, + cooked_range: Range, + fragments: &Fragments<'_>, containers: &[super::Container], ) { - if doc[range.clone()].contains('\t') { - // We don't do tab stops correctly. - return; - } - - // Blockquote - let ccount = doc[range.clone()].chars().filter(|c| *c == '>').count(); + // Get blockquotes + let ccount = doc[cooked_range.clone()].chars().filter(|c| *c == '>').count(); let blockquote_level = containers .iter() .filter(|c| matches!(c, super::Container::Blockquote)) .count(); - if ccount < blockquote_level { + + if ccount < blockquote_level + && let Some(mut span) = fragments.span(cx, cooked_range.clone()) + { span_lint_and_then( cx, DOC_LAZY_CONTINUATION, span, "doc quote line without `>` marker", |diag| { - let mut doc_start_range = &doc[range]; + let mut doc_start_range = &doc[cooked_range]; let mut suggested = String::new(); for c in containers { let text = map_container_to_text(c); @@ -78,7 +75,7 @@ pub(super) fn check( } // List - let leading_spaces = doc[range].chars().filter(|c| *c == ' ').count(); + let leading_spaces = doc[cooked_range.clone()].chars().filter(|c| *c == ' ').count(); let list_indentation = containers .iter() .map(|c| { @@ -89,36 +86,41 @@ pub(super) fn check( } }) .sum(); - match leading_spaces.cmp(&list_indentation) { - Ordering::Less => span_lint_and_then( - cx, - DOC_LAZY_CONTINUATION, - span, - "doc list item without indentation", - |diag| { - // simpler suggestion style for indentation - let indent = list_indentation - leading_spaces; - diag.span_suggestion_verbose( - span.shrink_to_hi(), - "indent this line", - std::iter::repeat_n(" ", indent).join(""), - Applicability::MaybeIncorrect, - ); - diag.help("if this is supposed to be its own paragraph, add a blank line"); - }, - ), - Ordering::Greater => { - let sugg = std::iter::repeat_n(" ", list_indentation).join(""); - span_lint_and_sugg( + + if leading_spaces != list_indentation + && let Some(span) = fragments.span(cx, cooked_range.clone()) + { + if leading_spaces < list_indentation { + span_lint_and_then( cx, - DOC_OVERINDENTED_LIST_ITEMS, + DOC_LAZY_CONTINUATION, span, - "doc list item overindented", - format!("try using `{sugg}` ({list_indentation} spaces)"), - sugg, - Applicability::MaybeIncorrect, + "doc list item without indentation", + |diag| { + // simpler suggestion style for indentation + let indent = list_indentation - leading_spaces; + diag.span_suggestion_verbose( + span.shrink_to_hi(), + "indent this line", + std::iter::repeat_n(" ", indent).join(""), + Applicability::MaybeIncorrect, + ); + diag.help("if this is supposed to be its own paragraph, add a blank line"); + }, ); - }, - Ordering::Equal => {}, + + return; + } + + let sugg = std::iter::repeat_n(" ", list_indentation).join(""); + span_lint_and_sugg( + cx, + DOC_OVERINDENTED_LIST_ITEMS, + span, + "doc list item overindented", + format!("try using `{sugg}` ({list_indentation} spaces)"), + sugg, + Applicability::MaybeIncorrect, + ); } } diff --git a/clippy_lints/src/doc/markdown.rs b/clippy_lints/src/doc/markdown.rs index 7a1c7c675d2ec..69c3b9150c300 100644 --- a/clippy_lints/src/doc/markdown.rs +++ b/clippy_lints/src/doc/markdown.rs @@ -6,13 +6,15 @@ use rustc_lint::LateContext; use rustc_span::{BytePos, Pos, Span}; use url::Url; -use crate::doc::DOC_MARKDOWN; +use crate::doc::{DOC_MARKDOWN, Fragments}; +use std::ops::Range; pub fn check( cx: &LateContext<'_>, valid_idents: &FxHashSet, text: &str, - span: Span, + fragments: &Fragments<'_>, + fragment_range: Range, code_level: isize, blockquote_level: isize, ) { @@ -64,20 +66,31 @@ pub fn check( close_parens += 1; } + // We'll use this offset to calculate the span to lint. + let fragment_offset = word.as_ptr() as usize - text.as_ptr() as usize; + // Adjust for the current word - let offset = word.as_ptr() as usize - text.as_ptr() as usize; - let span = Span::new( - span.lo() + BytePos::from_usize(offset), - span.lo() + BytePos::from_usize(offset + word.len()), - span.ctxt(), - span.parent(), + check_word( + cx, + word, + fragments, + &fragment_range, + fragment_offset, + code_level, + blockquote_level, ); - - check_word(cx, word, span, code_level, blockquote_level); } } -fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, blockquote_level: isize) { +fn check_word( + cx: &LateContext<'_>, + word: &str, + fragments: &Fragments<'_>, + range: &Range, + fragment_offset: usize, + code_level: isize, + blockquote_level: isize, +) { /// Checks if a string is upper-camel-case, i.e., starts with an uppercase and /// contains at least two uppercase letters (`Clippy` is ok) and one lower-case /// letter (`NASA` is ok). @@ -117,6 +130,16 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b // try to get around the fact that `foo::bar` parses as a valid URL && !url.cannot_be_a_base() { + let Some(fragment_span) = fragments.span(cx, range.clone()) else { + return; + }; + let span = Span::new( + fragment_span.lo() + BytePos::from_usize(fragment_offset), + fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()), + fragment_span.ctxt(), + fragment_span.parent(), + ); + span_lint_and_sugg( cx, DOC_MARKDOWN, @@ -137,6 +160,17 @@ fn check_word(cx: &LateContext<'_>, word: &str, span: Span, code_level: isize, b } if has_underscore(word) || word.contains("::") || is_camel_case(word) || word.ends_with("()") { + let Some(fragment_span) = fragments.span(cx, range.clone()) else { + return; + }; + + let span = Span::new( + fragment_span.lo() + BytePos::from_usize(fragment_offset), + fragment_span.lo() + BytePos::from_usize(fragment_offset + word.len()), + fragment_span.ctxt(), + fragment_span.parent(), + ); + span_lint_and_then( cx, DOC_MARKDOWN, diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 87da380e95401..c46dd09d60c56 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -3,7 +3,6 @@ use clippy_config::Conf; use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_then}; -use clippy_utils::source::snippet_opt; use clippy_utils::{is_entrypoint_fn, is_trait_impl_item}; use pulldown_cmark::Event::{ Code, DisplayMath, End, FootnoteReference, HardBreak, Html, InlineHtml, InlineMath, Rule, SoftBreak, Start, @@ -730,7 +729,10 @@ struct Fragments<'a> { } impl Fragments<'_> { - fn span(self, cx: &LateContext<'_>, range: Range) -> Option { + /// get the span for the markdown range. Note that this function is not cheap, use it with + /// caution. + #[must_use] + fn span(&self, cx: &LateContext<'_>, range: Range) -> Option { source_span_for_markdown_range(cx.tcx, self.doc, &range, self.fragments) } } @@ -1068,9 +1070,7 @@ fn check_doc<'a, Events: Iterator, Range, Range (), SoftBreak | HardBreak => { if !containers.is_empty() - && let Some((next_event, next_range)) = events.peek() - && let Some(next_span) = fragments.span(cx, next_range.clone()) - && let Some(span) = fragments.span(cx, range.clone()) && !in_footnote_definition + // Tabs aren't handled correctly vvvv + && !doc[range.clone()].contains('\t') + && let Some((next_event, next_range)) = events.peek() && !matches!(next_event, End(_)) { lazy_continuation::check( cx, doc, range.end..next_range.start, - Span::new(span.hi(), next_span.lo(), span.ctxt(), span.parent()), + &fragments, &containers[..], ); } - if let Some(span) = fragments.span(cx, range.clone()) + + if event == HardBreak + && !doc[range.clone()].trim().starts_with('\\') + && let Some(span) = fragments.span(cx, range.clone()) && !span.from_expansion() - && let Some(snippet) = snippet_opt(cx, span) - && !snippet.trim().starts_with('\\') - && event == HardBreak { + { collected_breaks.push(span); } }, diff --git a/clippy_lints/src/loops/manual_find.rs b/clippy_lints/src/loops/manual_find.rs index 35737f3eafe23..f99989ec6ba4b 100644 --- a/clippy_lints/src/loops/manual_find.rs +++ b/clippy_lints/src/loops/manual_find.rs @@ -83,6 +83,13 @@ pub(super) fn check<'tcx>( )[..], ); } + + // If the return type requires adjustments, we need to add a `.map` after the iterator + let inner_ret_adjust = cx.typeck_results().expr_adjustments(inner_ret); + if !inner_ret_adjust.is_empty() { + snippet.push_str(".map(|v| v as _)"); + } + // Extends to `last_stmt` to include semicolon in case of `return None;` let lint_span = span.to(last_stmt.span).to(last_ret.span); span_lint_and_then( diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index 9b6f97b9a2eb0..81f14b7b2b01d 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/clippy_lints/src/loops/manual_flatten.rs @@ -3,7 +3,7 @@ use super::utils::make_iterator_snippet; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::visitors::is_local_used; -use clippy_utils::{higher, path_to_local_id, peel_blocks_with_stmt}; +use clippy_utils::{higher, is_refutable, path_to_local_id, peel_blocks_with_stmt}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Expr, Pat, PatKind}; @@ -28,7 +28,7 @@ pub(super) fn check<'tcx>( && let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind && path_to_local_id(let_expr, pat_hir_id) // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result` - && let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind + && let PatKind::TupleStruct(ref qpath, [inner_pat], _) = let_pat.kind && let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id) && let Some(variant_id) = cx.tcx.opt_parent(ctor_id) && let some_ctor = cx.tcx.lang_items().option_some_variant() == Some(variant_id) @@ -37,6 +37,7 @@ pub(super) fn check<'tcx>( // Ensure expr in `if let` is not used afterwards && !is_local_used(cx, if_then, pat_hir_id) && msrv.meets(cx, msrvs::ITER_FLATTEN) + && !is_refutable(cx, inner_pat) { let if_let_type = if some_ctor { "Some" } else { "Ok" }; // Prepare the error message diff --git a/clippy_lints/src/loops/while_let_loop.rs b/clippy_lints/src/loops/while_let_loop.rs index bd04827a1f0ea..845edb9cae158 100644 --- a/clippy_lints/src/loops/while_let_loop.rs +++ b/clippy_lints/src/loops/while_let_loop.rs @@ -1,68 +1,65 @@ use super::WHILE_LET_LOOP; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher; -use clippy_utils::source::snippet_with_applicability; +use clippy_utils::source::{snippet, snippet_indent, snippet_opt}; use clippy_utils::ty::needs_ordered_drop; use clippy_utils::visitors::any_temporaries_need_ordered_drop; +use clippy_utils::{higher, peel_blocks}; +use rustc_ast::BindingMode; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, StmtKind}; +use rustc_hir::{Block, Expr, ExprKind, LetStmt, MatchSource, Pat, PatKind, Path, QPath, StmtKind, Ty}; use rustc_lint::LateContext; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, loop_block: &'tcx Block<'_>) { - let (init, has_trailing_exprs) = match (loop_block.stmts, loop_block.expr) { - ([stmt, stmts @ ..], expr) => { - if let StmtKind::Let(&LetStmt { + let (init, let_info) = match (loop_block.stmts, loop_block.expr) { + ([stmt, ..], _) => match stmt.kind { + StmtKind::Let(LetStmt { init: Some(e), els: None, + pat, + ty, .. - }) - | StmtKind::Semi(e) - | StmtKind::Expr(e) = stmt.kind - { - (e, !stmts.is_empty() || expr.is_some()) - } else { - return; - } + }) => (*e, Some((*pat, *ty))), + StmtKind::Semi(e) | StmtKind::Expr(e) => (e, None), + _ => return, }, - ([], Some(e)) => (e, false), + ([], Some(e)) => (e, None), _ => return, }; + let has_trailing_exprs = loop_block.stmts.len() + usize::from(loop_block.expr.is_some()) > 1; if let Some(if_let) = higher::IfLet::hir(cx, init) && let Some(else_expr) = if_let.if_else && is_simple_break_expr(else_expr) { - could_be_while_let(cx, expr, if_let.let_pat, if_let.let_expr, has_trailing_exprs); + could_be_while_let( + cx, + expr, + if_let.let_pat, + if_let.let_expr, + has_trailing_exprs, + let_info, + if_let.if_then, + ); } else if let ExprKind::Match(scrutinee, [arm1, arm2], MatchSource::Normal) = init.kind && arm1.guard.is_none() && arm2.guard.is_none() && is_simple_break_expr(arm2.body) { - could_be_while_let(cx, expr, arm1.pat, scrutinee, has_trailing_exprs); + could_be_while_let(cx, expr, arm1.pat, scrutinee, has_trailing_exprs, let_info, arm1.body); } } -/// Returns `true` if expr contains a single break expression without a label or eub-expression. +/// Returns `true` if expr contains a single break expression without a label or sub-expression, +/// possibly embedded in blocks. fn is_simple_break_expr(e: &Expr<'_>) -> bool { - matches!(peel_blocks(e).kind, ExprKind::Break(dest, None) if dest.label.is_none()) -} - -/// Removes any blocks containing only a single expression. -fn peel_blocks<'tcx>(e: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> { if let ExprKind::Block(b, _) = e.kind { match (b.stmts, b.expr) { - ([s], None) => { - if let StmtKind::Expr(e) | StmtKind::Semi(e) = s.kind { - peel_blocks(e) - } else { - e - } - }, - ([], Some(e)) => peel_blocks(e), - _ => e, + ([s], None) => matches!(s.kind, StmtKind::Expr(e) | StmtKind::Semi(e) if is_simple_break_expr(e)), + ([], Some(e)) => is_simple_break_expr(e), + _ => false, } } else { - e + matches!(e.kind, ExprKind::Break(dest, None) if dest.label.is_none()) } } @@ -72,6 +69,8 @@ fn could_be_while_let<'tcx>( let_pat: &'tcx Pat<'_>, let_expr: &'tcx Expr<'_>, has_trailing_exprs: bool, + let_info: Option<(&Pat<'_>, Option<&Ty<'_>>)>, + inner_expr: &Expr<'_>, ) { if has_trailing_exprs && (needs_ordered_drop(cx, cx.typeck_results().expr_ty(let_expr)) @@ -86,7 +85,24 @@ fn could_be_while_let<'tcx>( // 1) it was ugly with big bodies; // 2) it was not indented properly; // 3) it wasn’t very smart (see #675). - let mut applicability = Applicability::HasPlaceholders; + let inner_content = if let Some((pat, ty)) = let_info + // Prevent trivial reassignments such as `let x = x;` or `let _ = …;`, but + // keep them if the type has been explicitly specified. + && (!is_trivial_assignment(pat, peel_blocks(inner_expr)) || ty.is_some()) + && let Some(pat_str) = snippet_opt(cx, pat.span) + && let Some(init_str) = snippet_opt(cx, peel_blocks(inner_expr).span) + { + let ty_str = ty + .map(|ty| format!(": {}", snippet(cx, ty.span, "_"))) + .unwrap_or_default(); + format!( + "\n{indent} let {pat_str}{ty_str} = {init_str};\n{indent} ..\n{indent}", + indent = snippet_indent(cx, expr.span).unwrap_or_default(), + ) + } else { + " .. ".into() + }; + span_lint_and_sugg( cx, WHILE_LET_LOOP, @@ -94,10 +110,21 @@ fn could_be_while_let<'tcx>( "this loop could be written as a `while let` loop", "try", format!( - "while let {} = {} {{ .. }}", - snippet_with_applicability(cx, let_pat.span, "..", &mut applicability), - snippet_with_applicability(cx, let_expr.span, "..", &mut applicability), + "while let {} = {} {{{inner_content}}}", + snippet(cx, let_pat.span, ".."), + snippet(cx, let_expr.span, ".."), ), - applicability, + Applicability::HasPlaceholders, ); } + +fn is_trivial_assignment(pat: &Pat<'_>, init: &Expr<'_>) -> bool { + match (pat.kind, init.kind) { + (PatKind::Wild, _) => true, + ( + PatKind::Binding(BindingMode::NONE, _, pat_ident, None), + ExprKind::Path(QPath::Resolved(None, Path { segments: [init], .. })), + ) => pat_ident.name == init.ident.name, + _ => false, + } +} diff --git a/clippy_lints/src/methods/manual_is_variant_and.rs b/clippy_lints/src/methods/manual_is_variant_and.rs index 40aad03960c43..4a61c223d2c1e 100644 --- a/clippy_lints/src/methods/manual_is_variant_and.rs +++ b/clippy_lints/src/methods/manual_is_variant_and.rs @@ -1,18 +1,22 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::get_parent_expr; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::source::snippet; +use clippy_utils::source::{snippet, snippet_opt}; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; +use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::LateContext; -use rustc_span::{Span, sym}; +use rustc_middle::ty; +use rustc_span::{BytePos, Span, sym}; use super::MANUAL_IS_VARIANT_AND; -pub(super) fn check<'tcx>( +pub(super) fn check( cx: &LateContext<'_>, - expr: &'tcx rustc_hir::Expr<'_>, - map_recv: &'tcx rustc_hir::Expr<'_>, - map_arg: &'tcx rustc_hir::Expr<'_>, + expr: &Expr<'_>, + map_recv: &Expr<'_>, + map_arg: &Expr<'_>, map_span: Span, msrv: Msrv, ) { @@ -57,3 +61,57 @@ pub(super) fn check<'tcx>( Applicability::MachineApplicable, ); } + +fn emit_lint(cx: &LateContext<'_>, op: BinOpKind, parent: &Expr<'_>, method_span: Span, is_option: bool) { + if let Some(before_map_snippet) = snippet_opt(cx, parent.span.with_hi(method_span.lo())) + && let Some(after_map_snippet) = snippet_opt(cx, method_span.with_lo(method_span.lo() + BytePos(3))) + { + span_lint_and_sugg( + cx, + MANUAL_IS_VARIANT_AND, + parent.span, + format!( + "called `.map() {}= {}()`", + if op == BinOpKind::Eq { '=' } else { '!' }, + if is_option { "Some" } else { "Ok" }, + ), + "use", + if is_option && op == BinOpKind::Ne { + format!("{before_map_snippet}is_none_or{after_map_snippet}",) + } else { + format!( + "{}{before_map_snippet}{}{after_map_snippet}", + if op == BinOpKind::Eq { "" } else { "!" }, + if is_option { "is_some_and" } else { "is_ok_and" }, + ) + }, + Applicability::MachineApplicable, + ); + } +} + +pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) { + if let Some(parent_expr) = get_parent_expr(cx, expr) + && let ExprKind::Binary(op, left, right) = parent_expr.kind + && matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) + && op.span.eq_ctxt(expr.span) + { + // Check `left` and `right` expression in any order, and for `Option` and `Result` + for (expr1, expr2) in [(left, right), (right, left)] { + for item in [sym::Option, sym::Result] { + if let ExprKind::Call(call, ..) = expr1.kind + && let ExprKind::Path(QPath::Resolved(_, path)) = call.kind + && let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res + && let ty = cx.typeck_results().expr_ty(expr1) + && let ty::Adt(adt, args) = ty.kind() + && cx.tcx.is_diagnostic_item(item, adt.did()) + && args.type_at(0).is_bool() + && let ExprKind::MethodCall(_, recv, _, span) = expr2.kind + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), item) + { + return emit_lint(cx, op.node, parent_expr, span, item == sym::Option); + } + } + } + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d2d59f0013c0c..bc1592069850a 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4439,7 +4439,7 @@ declare_clippy_lint! { /// Checks for usage of `iter().any()` on slices when it can be replaced with `contains()` and suggests doing so. /// /// ### Why is this bad? - /// `contains()` is more concise and idiomatic, sometimes more fast. + /// `contains()` is more concise and idiomatic, while also being faster in some cases. /// /// ### Example /// ```no_run @@ -5203,6 +5203,7 @@ impl Methods { unused_enumerate_index::check(cx, expr, recv, m_arg); map_clone::check(cx, expr, recv, m_arg, self.msrv); map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span); + manual_is_variant_and::check_map(cx, expr); match method_call(recv) { Some((map_name @ (sym::iter | sym::into_iter), recv2, _, _, _)) => { iter_kv_map::check(cx, map_name, expr, recv2, m_arg, self.msrv); diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 38cb4d51ca0f4..7bdd999bbbadd 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -11,8 +11,7 @@ use clippy_utils::{ use rustc_errors::Applicability; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::Span; -use rustc_span::Symbol; +use rustc_span::{Span, Symbol}; use {rustc_ast as ast, rustc_hir as hir}; use super::{OR_FUN_CALL, UNWRAP_OR_DEFAULT}; diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index 768bbebccd4af..fdccf1fb33db7 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -44,7 +44,7 @@ pub fn check<'tcx>( return; } // At this point, we know the call is of a `to_owned`-like function. The functions - // `check_addr_of_expr` and `check_call_arg` determine whether the call is unnecessary + // `check_addr_of_expr` and `check_into_iter_call_arg` determine whether the call is unnecessary // based on its context, that is, whether it is a referent in an `AddrOf` expression, an // argument in a `into_iter` call, or an argument in the call of some other function. if check_addr_of_expr(cx, expr, method_name, method_def_id, receiver) { diff --git a/clippy_lints/src/mut_reference.rs b/clippy_lints/src/mut_reference.rs index 2fd1049f42e10..2f1ab3d2652ae 100644 --- a/clippy_lints/src/mut_reference.rs +++ b/clippy_lints/src/mut_reference.rs @@ -79,25 +79,19 @@ fn check_arguments<'tcx>( name: &str, fn_kind: &str, ) { - match type_definition.kind() { - ty::FnDef(..) | ty::FnPtr(..) => { - let parameters = type_definition.fn_sig(cx.tcx).skip_binder().inputs(); - for (argument, parameter) in iter::zip(arguments, parameters) { - match parameter.kind() { - ty::Ref(_, _, Mutability::Not) | ty::RawPtr(_, Mutability::Not) => { - if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) = argument.kind { - span_lint( - cx, - UNNECESSARY_MUT_PASSED, - argument.span, - format!("the {fn_kind} `{name}` doesn't need a mutable reference"), - ); - } - }, - _ => (), - } + if let ty::FnDef(..) | ty::FnPtr(..) = type_definition.kind() { + let parameters = type_definition.fn_sig(cx.tcx).skip_binder().inputs(); + for (argument, parameter) in iter::zip(arguments, parameters) { + if let ty::Ref(_, _, Mutability::Not) | ty::RawPtr(_, Mutability::Not) = parameter.kind() + && let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, _) = argument.kind + { + span_lint( + cx, + UNNECESSARY_MUT_PASSED, + argument.span, + format!("the {fn_kind} `{name}` doesn't need a mutable reference"), + ); } - }, - _ => (), + } } } diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index 7dd96f1f037fd..6a7c8436bad4f 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -74,7 +74,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { && let body = cx.tcx.hir_body(body) // Skip the lint if the body is not safe, so as not to suggest `for … in … unsafe {}` // and suggesting `for … in … { unsafe { } }` is a little ugly. - && let ExprKind::Block(Block { rules: BlockCheckMode::DefaultBlock, .. }, ..) = body.value.kind + && !matches!(body.value.kind, ExprKind::Block(Block { rules: BlockCheckMode::UnsafeBlock(_), .. }, ..)) { let mut ret_collector = RetCollector::default(); ret_collector.visit_expr(body.value); @@ -99,11 +99,21 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { ) }; + let body_param_sugg = snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability); + let for_each_rev_sugg = snippet_with_applicability(cx, for_each_recv.span, "..", &mut applicability); + let body_value_sugg = snippet_with_applicability(cx, body.value.span, "..", &mut applicability); + let sugg = format!( "for {} in {} {}", - snippet_with_applicability(cx, body.params[0].pat.span, "..", &mut applicability), - snippet_with_applicability(cx, for_each_recv.span, "..", &mut applicability), - snippet_with_applicability(cx, body.value.span, "..", &mut applicability), + body_param_sugg, + for_each_rev_sugg, + match body.value.kind { + ExprKind::Block(block, _) if is_let_desugar(block) => { + format!("{{ {body_value_sugg} }}") + }, + ExprKind::Block(_, _) => body_value_sugg.to_string(), + _ => format!("{{ {body_value_sugg}; }}"), + } ); span_lint_and_then(cx, NEEDLESS_FOR_EACH, stmt.span, "needless use of `for_each`", |diag| { @@ -116,6 +126,20 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { } } +/// Check if the block is a desugared `_ = expr` statement. +fn is_let_desugar(block: &Block<'_>) -> bool { + matches!( + block, + Block { + stmts: [Stmt { + kind: StmtKind::Let(_), + .. + },], + .. + } + ) +} + /// This type plays two roles. /// 1. Collect spans of `return` in the closure body. /// 2. Detect use of `return` in `Loop` in the closure body. diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs index 4be42267b14b3..9c6141d822263 100644 --- a/clippy_lints/src/operators/assign_op_pattern.rs +++ b/clippy_lints/src/operators/assign_op_pattern.rs @@ -1,8 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::Msrv; +use clippy_utils::qualify_min_const_fn::is_stable_const_fn; use clippy_utils::source::SpanRangeExt; use clippy_utils::ty::implements_trait; use clippy_utils::visitors::for_each_expr_without_closures; -use clippy_utils::{binop_traits, eq_expr_value, trait_ref_of_method}; +use clippy_utils::{binop_traits, eq_expr_value, is_in_const_context, trait_ref_of_method}; use core::ops::ControlFlow; use rustc_errors::Applicability; use rustc_hir as hir; @@ -19,6 +21,7 @@ pub(super) fn check<'tcx>( expr: &'tcx hir::Expr<'_>, assignee: &'tcx hir::Expr<'_>, e: &'tcx hir::Expr<'_>, + msrv: Msrv, ) { if let hir::ExprKind::Binary(op, l, r) = &e.kind { let lint = |assignee: &hir::Expr<'_>, rhs: &hir::Expr<'_>| { @@ -40,6 +43,15 @@ pub(super) fn check<'tcx>( return; } } + + // Skip if the trait is not stable in const contexts + if is_in_const_context(cx) + && let Some(binop_id) = cx.tcx.associated_item_def_ids(trait_id).first() + && !is_stable_const_fn(cx, *binop_id, msrv) + { + return; + } + span_lint_and_then( cx, ASSIGN_OP_PATTERN, diff --git a/clippy_lints/src/operators/mod.rs b/clippy_lints/src/operators/mod.rs index d32c062cf56a7..2f4e8e9958868 100644 --- a/clippy_lints/src/operators/mod.rs +++ b/clippy_lints/src/operators/mod.rs @@ -919,7 +919,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators { modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false); }, ExprKind::Assign(lhs, rhs, _) => { - assign_op_pattern::check(cx, e, lhs, rhs); + assign_op_pattern::check(cx, e, lhs, rhs, self.msrv); self_assignment::check(cx, e, lhs, rhs); }, ExprKind::Unary(op, arg) => { diff --git a/clippy_lints/src/operators/modulo_arithmetic.rs b/clippy_lints/src/operators/modulo_arithmetic.rs index 691d7b904eff5..b79461663d7bf 100644 --- a/clippy_lints/src/operators/modulo_arithmetic.rs +++ b/clippy_lints/src/operators/modulo_arithmetic.rs @@ -34,14 +34,10 @@ pub(super) fn check<'tcx>( } fn used_in_comparison_with_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - let Node::Expr(parent_expr) = cx.tcx.parent_hir_node(expr.hir_id) else { - return false; - }; - let ExprKind::Binary(op, lhs, rhs) = parent_expr.kind else { - return false; - }; - - if op.node == BinOpKind::Eq || op.node == BinOpKind::Ne { + if let Node::Expr(parent_expr) = cx.tcx.parent_hir_node(expr.hir_id) + && let ExprKind::Binary(op, lhs, rhs) = parent_expr.kind + && let BinOpKind::Eq | BinOpKind::Ne = op.node + { let ecx = ConstEvalCtxt::new(cx); matches!(ecx.eval(lhs), Some(Constant::Int(0))) || matches!(ecx.eval(rhs), Some(Constant::Int(0))) } else { @@ -56,35 +52,28 @@ struct OperandInfo { } fn analyze_operand(operand: &Expr<'_>, cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { - match ConstEvalCtxt::new(cx).eval(operand) { - Some(Constant::Int(v)) => match *cx.typeck_results().expr_ty(expr).kind() { + match ConstEvalCtxt::new(cx).eval(operand)? { + Constant::Int(v) => match *cx.typeck_results().expr_ty(expr).kind() { ty::Int(ity) => { let value = sext(cx.tcx, v, ity); - return Some(OperandInfo { + Some(OperandInfo { string_representation: Some(value.to_string()), is_negative: value < 0, is_integral: true, - }); - }, - ty::Uint(_) => { - return Some(OperandInfo { - string_representation: None, - is_negative: false, - is_integral: true, - }); + }) }, - _ => {}, + ty::Uint(_) => Some(OperandInfo { + string_representation: None, + is_negative: false, + is_integral: true, + }), + _ => None, }, // FIXME(f16_f128): add when casting is available on all platforms - Some(Constant::F32(f)) => { - return Some(floating_point_operand_info(&f)); - }, - Some(Constant::F64(f)) => { - return Some(floating_point_operand_info(&f)); - }, - _ => {}, + Constant::F32(f) => Some(floating_point_operand_info(&f)), + Constant::F64(f) => Some(floating_point_operand_info(&f)), + _ => None, } - None } fn floating_point_operand_info>(f: &T) -> OperandInfo { diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index 8962f36db1e69..449d3da763941 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -152,7 +152,6 @@ impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { expr.span, "`panic_any` should not be present in production code", ); - return; } } } diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs index 5d30b66def2c8..dadf49b64e517 100644 --- a/clippy_lints/src/pass_by_ref_or_value.rs +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -1,5 +1,3 @@ -use std::{cmp, iter}; - use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; @@ -20,6 +18,7 @@ use rustc_middle::ty::{self, RegionKind, TyCtxt}; use rustc_session::impl_lint_pass; use rustc_span::def_id::LocalDefId; use rustc_span::{Span, sym}; +use std::iter; declare_clippy_lint! { /// ### What it does @@ -33,10 +32,8 @@ declare_clippy_lint! { /// registers. /// /// ### Known problems - /// This lint is target register size dependent, it is - /// limited to 32-bit to try and reduce portability problems between 32 and - /// 64-bit, but if you are compiling for 8 or 16-bit targets then the limit - /// will be different. + /// This lint is target dependent, some cases will lint on 64-bit targets but + /// not 32-bit or lower targets. /// /// The configuration option `trivial_copy_size_limit` can be set to override /// this limit for a project. @@ -112,16 +109,9 @@ pub struct PassByRefOrValue { impl PassByRefOrValue { pub fn new(tcx: TyCtxt<'_>, conf: &'static Conf) -> Self { - let ref_min_size = conf.trivial_copy_size_limit.unwrap_or_else(|| { - let bit_width = u64::from(tcx.sess.target.pointer_width); - // Cap the calculated bit width at 32-bits to reduce - // portability problems between 32 and 64-bit targets - let bit_width = cmp::min(bit_width, 32); - #[expect(clippy::integer_division)] - let byte_width = bit_width / 8; - // Use a limit of 2 times the register byte width - byte_width * 2 - }); + let ref_min_size = conf + .trivial_copy_size_limit + .unwrap_or_else(|| u64::from(tcx.sess.target.pointer_width / 8)); Self { ref_min_size, diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 6bc5af268ff8a..e0c93153a77a8 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -435,7 +435,9 @@ fn check_final_expr<'tcx>( ExprKind::If(_, then, else_clause_opt) => { check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone()); if let Some(else_clause) = else_clause_opt { - check_block_return(cx, &else_clause.kind, peeled_drop_expr.span, semi_spans); + // The `RetReplacement` won't be used there as `else_clause` will be either a block or + // a `if` expression. + check_final_expr(cx, else_clause, semi_spans, RetReplacement::Empty, match_ty_opt); } }, // a match expr, check all arms diff --git a/clippy_lints/src/unit_return_expecting_ord.rs b/clippy_lints/src/unit_return_expecting_ord.rs index 67ceac92dbc06..39f4130afcf36 100644 --- a/clippy_lints/src/unit_return_expecting_ord.rs +++ b/clippy_lints/src/unit_return_expecting_ord.rs @@ -5,7 +5,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_middle::ty::{ClauseKind, GenericPredicates, ProjectionPredicate, TraitPredicate}; use rustc_session::declare_lint_pass; -use rustc_span::{BytePos, Span, sym}; +use rustc_span::{BytePos, Span, Symbol, sym}; declare_clippy_lint! { /// ### What it does @@ -36,21 +36,26 @@ declare_clippy_lint! { declare_lint_pass!(UnitReturnExpectingOrd => [UNIT_RETURN_EXPECTING_ORD]); -fn get_trait_predicates_for_trait_id<'tcx>( +// For each +fn get_trait_predicates_for_trait_ids<'tcx>( cx: &LateContext<'tcx>, generics: GenericPredicates<'tcx>, - trait_id: Option, -) -> Vec> { - let mut preds = Vec::new(); + trait_ids: &[Option], // At least 2 ids +) -> [Vec>; 3] { + debug_assert!(trait_ids.len() >= 2); + let mut preds = [Vec::new(), Vec::new(), Vec::new()]; for (pred, _) in generics.predicates { - if let ClauseKind::Trait(poly_trait_pred) = pred.kind().skip_binder() - && let trait_pred = cx + if let ClauseKind::Trait(poly_trait_pred) = pred.kind().skip_binder() { + let trait_pred = cx .tcx - .instantiate_bound_regions_with_erased(pred.kind().rebind(poly_trait_pred)) - && let Some(trait_def_id) = trait_id - && trait_def_id == trait_pred.trait_ref.def_id - { - preds.push(trait_pred); + .instantiate_bound_regions_with_erased(pred.kind().rebind(poly_trait_pred)); + for (i, tid) in trait_ids.iter().enumerate() { + if let Some(tid) = tid + && *tid == trait_pred.trait_ref.def_id + { + preds[i].push(trait_pred); + } + } } } preds @@ -74,15 +79,24 @@ fn get_projection_pred<'tcx>( }) } -fn get_args_to_check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Vec<(usize, String)> { +fn get_args_to_check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + args_len: usize, + fn_mut_trait: DefId, + ord_trait: Option, + partial_ord_trait: Option, +) -> Vec<(usize, Symbol)> { let mut args_to_check = Vec::new(); if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) { let fn_sig = cx.tcx.fn_sig(def_id).instantiate_identity(); let generics = cx.tcx.predicates_of(def_id); - let fn_mut_preds = get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().fn_mut_trait()); - let ord_preds = get_trait_predicates_for_trait_id(cx, generics, cx.tcx.get_diagnostic_item(sym::Ord)); - let partial_ord_preds = - get_trait_predicates_for_trait_id(cx, generics, cx.tcx.lang_items().partial_ord_trait()); + let [fn_mut_preds, ord_preds, partial_ord_preds] = + get_trait_predicates_for_trait_ids(cx, generics, &[Some(fn_mut_trait), ord_trait, partial_ord_trait]); + if fn_mut_preds.is_empty() { + return vec![]; + } + // Trying to call instantiate_bound_regions_with_erased on fn_sig.inputs() gives the following error // The trait `rustc::ty::TypeFoldable<'_>` is not implemented for // `&[rustc_middle::ty::Ty<'_>]` @@ -102,12 +116,18 @@ fn get_args_to_check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Ve .iter() .any(|ord| Some(ord.self_ty()) == return_ty_pred.term.as_type()) { - args_to_check.push((i, "Ord".to_string())); + args_to_check.push((i, sym::Ord)); + if args_to_check.len() == args_len - 1 { + break; + } } else if partial_ord_preds .iter() .any(|pord| pord.self_ty() == return_ty_pred.term.expect_type()) { - args_to_check.push((i, "PartialOrd".to_string())); + args_to_check.push((i, sym::PartialOrd)); + if args_to_check.len() == args_len - 1 { + break; + } } } } @@ -142,38 +162,50 @@ fn check_arg<'tcx>(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Option<(Spa impl<'tcx> LateLintPass<'tcx> for UnitReturnExpectingOrd { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if let ExprKind::MethodCall(_, receiver, args, _) = expr.kind { - let arg_indices = get_args_to_check(cx, expr); + if let ExprKind::MethodCall(_, receiver, args, _) = expr.kind + && args.iter().any(|arg| { + matches!( + arg.peel_blocks().peel_borrows().peel_drop_temps().kind, + ExprKind::Path(_) | ExprKind::Closure(_) + ) + }) + && let Some(fn_mut_trait) = cx.tcx.lang_items().fn_mut_trait() + { + let ord_trait = cx.tcx.get_diagnostic_item(sym::Ord); + let partial_ord_trait = cx.tcx.lang_items().partial_ord_trait(); + if (ord_trait, partial_ord_trait) == (None, None) { + return; + } + let args = std::iter::once(receiver).chain(args.iter()).collect::>(); + let arg_indices = get_args_to_check(cx, expr, args.len(), fn_mut_trait, ord_trait, partial_ord_trait); for (i, trait_name) in arg_indices { - if i < args.len() { - match check_arg(cx, args[i]) { - Some((span, None)) => { - span_lint( - cx, - UNIT_RETURN_EXPECTING_ORD, - span, - format!( - "this closure returns \ + match check_arg(cx, args[i]) { + Some((span, None)) => { + span_lint( + cx, + UNIT_RETURN_EXPECTING_ORD, + span, + format!( + "this closure returns \ the unit type which also implements {trait_name}" - ), - ); - }, - Some((span, Some(last_semi))) => { - span_lint_and_help( - cx, - UNIT_RETURN_EXPECTING_ORD, - span, - format!( - "this closure returns \ + ), + ); + }, + Some((span, Some(last_semi))) => { + span_lint_and_help( + cx, + UNIT_RETURN_EXPECTING_ORD, + span, + format!( + "this closure returns \ the unit type which also implements {trait_name}" - ), - Some(last_semi), - "probably caused by this trailing semicolon", - ); - }, - None => {}, - } + ), + Some(last_semi), + "probably caused by this trailing semicolon", + ); + }, + None => {}, } } } diff --git a/clippy_utils/README.md b/clippy_utils/README.md index c9083f654c4cb..efbacbd72dba5 100644 --- a/clippy_utils/README.md +++ b/clippy_utils/README.md @@ -8,7 +8,7 @@ This crate is only guaranteed to build with this `nightly` toolchain: ``` -nightly-2025-05-21 +nightly-2025-05-31 ``` diff --git a/clippy_utils/src/ast_utils/mod.rs b/clippy_utils/src/ast_utils/mod.rs index a0503a699e691..6c186ab4a6f8f 100644 --- a/clippy_utils/src/ast_utils/mod.rs +++ b/clippy_utils/src/ast_utils/mod.rs @@ -437,10 +437,10 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { && both(lt.as_ref(), rt.as_ref(), |l, r| eq_ty(l, r)) }, (Enum(li, lg, le), Enum(ri, rg, re)) => { - eq_id(*li, *ri) && eq_generics(lg, rg) && over(&le.variants, &re.variants, eq_variant) + eq_id(*li, *ri) && eq_generics(lg, rg) && over(&le.variants, &re.variants, eq_variant) }, (Struct(li, lg, lv), Struct(ri, rg, rv)) | (Union(li, lg, lv), Union(ri, rg, rv)) => { - eq_id(*li, *ri) && eq_generics(lg, rg) && eq_variant_data(lv, rv) + eq_id(*li, *ri) && eq_generics(lg, rg) && eq_variant_data(lv, rv) }, ( Trait(box ast::Trait { diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8716ee48c88fd..55469e8ebc90c 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1565,10 +1565,10 @@ pub fn is_ctor_or_promotable_const_function(cx: &LateContext<'_>, expr: &Expr<'_ /// Returns `true` if a pattern is refutable. // TODO: should be implemented using rustc/mir_build/thir machinery pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { - fn is_enum_variant(cx: &LateContext<'_>, qpath: &QPath<'_>, id: HirId) -> bool { - matches!( + fn is_qpath_refutable(cx: &LateContext<'_>, qpath: &QPath<'_>, id: HirId) -> bool { + !matches!( cx.qpath_res(qpath, id), - Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), _) + Res::Def(DefKind::Struct, ..) | Res::Def(DefKind::Ctor(def::CtorOf::Struct, _), _) ) } @@ -1585,16 +1585,18 @@ pub fn is_refutable(cx: &LateContext<'_>, pat: &Pat<'_>) -> bool { kind: PatExprKind::Path(qpath), hir_id, .. - }) => is_enum_variant(cx, qpath, *hir_id), + }) => is_qpath_refutable(cx, qpath, *hir_id), PatKind::Or(pats) => { // TODO: should be the honest check, that pats is exhaustive set are_refutable(cx, pats) }, PatKind::Tuple(pats, _) => are_refutable(cx, pats), PatKind::Struct(ref qpath, fields, _) => { - is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| field.pat)) + is_qpath_refutable(cx, qpath, pat.hir_id) || are_refutable(cx, fields.iter().map(|field| field.pat)) + }, + PatKind::TupleStruct(ref qpath, pats, _) => { + is_qpath_refutable(cx, qpath, pat.hir_id) || are_refutable(cx, pats) }, - PatKind::TupleStruct(ref qpath, pats, _) => is_enum_variant(cx, qpath, pat.hir_id) || are_refutable(cx, pats), PatKind::Slice(head, middle, tail) => { match &cx.typeck_results().node_type(pat.hir_id).kind() { rustc_ty::Slice(..) => { diff --git a/clippy_utils/src/qualify_min_const_fn.rs b/clippy_utils/src/qualify_min_const_fn.rs index bb04520c6b7d8..5b4ec12cbece6 100644 --- a/clippy_utils/src/qualify_min_const_fn.rs +++ b/clippy_utils/src/qualify_min_const_fn.rs @@ -393,7 +393,8 @@ fn check_terminator<'tcx>( } } -fn is_stable_const_fn(cx: &LateContext<'_>, def_id: DefId, msrv: Msrv) -> bool { +/// Checks if the given `def_id` is a stable const fn, in respect to the given MSRV. +pub fn is_stable_const_fn(cx: &LateContext<'_>, def_id: DefId, msrv: Msrv) -> bool { cx.tcx.is_const_fn(def_id) && cx .tcx diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index f4dfc8f4b5a7f..6974e6512e2ca 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -940,7 +940,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> { // note: unable to trigger `Subslice` kind in tests ProjectionKind::Subslice | // Doesn't have surface syntax. Only occurs in patterns. - ProjectionKind::OpaqueCast => (), + ProjectionKind::OpaqueCast | // Only occurs in closure captures. ProjectionKind::UnwrapUnsafeBinder => (), ProjectionKind::Deref => { diff --git a/lintcheck/src/input.rs b/lintcheck/src/input.rs index 83eb0a577d6b9..408a2e087af28 100644 --- a/lintcheck/src/input.rs +++ b/lintcheck/src/input.rs @@ -8,7 +8,7 @@ use std::time::Duration; use serde::Deserialize; use walkdir::{DirEntry, WalkDir}; -use crate::{Crate, LINTCHECK_DOWNLOADS, LINTCHECK_SOURCES}; +use crate::{Crate, lintcheck_sources, target_dir}; const DEFAULT_DOCS_LINK: &str = "https://docs.rs/{krate}/{version}/src/{krate_}/{file}.html#{line}"; const DEFAULT_GITHUB_LINK: &str = "{url}/blob/{hash}/src/{file}#L{line}"; @@ -201,8 +201,10 @@ impl CrateWithSource { let file_link = &self.file_link; match &self.source { CrateSource::CratesIo { version } => { - let extract_dir = PathBuf::from(LINTCHECK_SOURCES); - let krate_download_dir = PathBuf::from(LINTCHECK_DOWNLOADS); + let extract_dir = PathBuf::from(lintcheck_sources()); + // Keep constant downloads path to avoid repeating work and + // filling up disk space unnecessarily. + let krate_download_dir = PathBuf::from("target/lintcheck/downloads/"); // url to download the crate from crates.io let url = format!("https://crates.io/api/v1/crates/{name}/{version}/download"); @@ -211,7 +213,7 @@ impl CrateWithSource { let krate_file_path = krate_download_dir.join(format!("{name}-{version}.crate.tar.gz")); // don't download/extract if we already have done so - if !krate_file_path.is_file() { + if !krate_file_path.is_file() || !extract_dir.join(format!("{name}-{version}")).exists() { // create a file path to download and write the crate data into let mut krate_dest = fs::File::create(&krate_file_path).unwrap(); let mut krate_req = get(&url).unwrap().into_reader(); @@ -236,7 +238,7 @@ impl CrateWithSource { }, CrateSource::Git { url, commit } => { let repo_path = { - let mut repo_path = PathBuf::from(LINTCHECK_SOURCES); + let mut repo_path = PathBuf::from(lintcheck_sources()); // add a -git suffix in case we have the same crate from crates.io and a git repo repo_path.push(format!("{name}-git")); repo_path @@ -286,7 +288,7 @@ impl CrateWithSource { // copy path into the dest_crate_root but skip directories that contain a CACHEDIR.TAG file. // The target/ directory contains a CACHEDIR.TAG file so it is the most commonly skipped directory // as a result of this filter. - let dest_crate_root = PathBuf::from(LINTCHECK_SOURCES).join(name); + let dest_crate_root = PathBuf::from(lintcheck_sources()).join(name); if dest_crate_root.exists() { println!("Deleting existing directory at `{}`", dest_crate_root.display()); fs::remove_dir_all(&dest_crate_root).unwrap(); @@ -326,15 +328,16 @@ impl CrateWithSource { /// /// This function panics if creating one of the dirs fails. fn create_dirs(krate_download_dir: &Path, extract_dir: &Path) { - fs::create_dir("target/lintcheck/").unwrap_or_else(|err| { + fs::create_dir(format!("{}/lintcheck/", target_dir())).unwrap_or_else(|err| { assert_eq!( err.kind(), ErrorKind::AlreadyExists, "cannot create lintcheck target dir" ); }); - fs::create_dir(krate_download_dir).unwrap_or_else(|err| { - assert_eq!(err.kind(), ErrorKind::AlreadyExists, "cannot create crate download dir"); + fs::create_dir_all(krate_download_dir).unwrap_or_else(|err| { + // We are allowed to reuse download dirs + assert_ne!(err.kind(), ErrorKind::AlreadyExists); }); fs::create_dir(extract_dir).unwrap_or_else(|err| { assert_eq!( diff --git a/lintcheck/src/main.rs b/lintcheck/src/main.rs index d4bf6cd48a152..8418383143280 100644 --- a/lintcheck/src/main.rs +++ b/lintcheck/src/main.rs @@ -43,8 +43,14 @@ use input::read_crates; use output::{ClippyCheckOutput, ClippyWarning, RustcIce}; use rayon::prelude::*; -const LINTCHECK_DOWNLOADS: &str = "target/lintcheck/downloads"; -const LINTCHECK_SOURCES: &str = "target/lintcheck/sources"; +#[must_use] +pub fn target_dir() -> String { + env::var("CARGO_TARGET_DIR").unwrap_or("target".to_owned()) +} + +fn lintcheck_sources() -> String { + format!("{}/lintcheck/sources", target_dir()) +} /// Represents the actual source code of a crate that we ran "cargo clippy" on #[derive(Debug)] @@ -307,7 +313,8 @@ fn main() { fn lintcheck(config: LintcheckConfig) { let clippy_ver = build_clippy(config.perf); let clippy_driver_path = fs::canonicalize(format!( - "target/{}/clippy-driver{EXE_SUFFIX}", + "{}/{}/clippy-driver{EXE_SUFFIX}", + target_dir(), if config.perf { "release" } else { "debug" } )) .unwrap(); @@ -315,7 +322,8 @@ fn lintcheck(config: LintcheckConfig) { // assert that clippy is found assert!( clippy_driver_path.is_file(), - "target/{}/clippy-driver binary not found! {}", + "{}/{}/clippy-driver binary not found! {}", + target_dir(), if config.perf { "release" } else { "debug" }, clippy_driver_path.display() ); @@ -386,7 +394,7 @@ fn lintcheck(config: LintcheckConfig) { .unwrap(); let server = config.recursive.then(|| { - let _: io::Result<()> = fs::remove_dir_all("target/lintcheck/shared_target_dir/recursive"); + let _: io::Result<()> = fs::remove_dir_all(format!("{}/lintcheck/shared_target_dir/recursive", target_dir())); LintcheckServer::spawn(recursive_options) }); @@ -488,7 +496,7 @@ fn clippy_project_root() -> &'static Path { #[must_use] fn shared_target_dir(qualifier: &str) -> PathBuf { clippy_project_root() - .join("target/lintcheck/shared_target_dir") + .join(format!("{}/lintcheck/shared_target_dir", target_dir())) .join(qualifier) } diff --git a/lintcheck/src/output.rs b/lintcheck/src/output.rs index dcc1ec339ef90..d7fe0915121db 100644 --- a/lintcheck/src/output.rs +++ b/lintcheck/src/output.rs @@ -162,9 +162,9 @@ pub fn summarize_and_print_changes( fn gather_stats(warnings: &[ClippyWarning]) -> (String, HashMap<&String, usize>) { // count lint type occurrences let mut counter: HashMap<&String, usize> = HashMap::new(); - warnings - .iter() - .for_each(|wrn| *counter.entry(&wrn.name).or_insert(0) += 1); + for wrn in warnings { + *counter.entry(&wrn.name).or_insert(0) += 1; + } // collect into a tupled list for sorting let mut stats: Vec<(&&String, &usize)> = counter.iter().collect(); diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 0b9ebe554f5b4..b6817d9a14639 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,6 +1,6 @@ [toolchain] # begin autogenerated nightly -channel = "nightly-2025-05-21" +channel = "nightly-2025-05-31" # end autogenerated nightly components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] profile = "minimal" diff --git a/rustfmt.toml b/rustfmt.toml index 0dc6adce7bfce..0ed58a2dfc19f 100644 --- a/rustfmt.toml +++ b/rustfmt.toml @@ -6,4 +6,8 @@ edition = "2024" error_on_line_overflow = true imports_granularity = "Module" style_edition = "2024" -ignore = ["tests/ui/crashes/ice-10912.rs"] +ignore = [ + "tests/ui/crashes/ice-9405.rs", + "tests/ui/crashes/ice-10912.rs", + "tests/ui/non_expressive_names_error_recovery.rs", +] diff --git a/tests/headers.rs b/tests/headers.rs deleted file mode 100644 index d1f986ef52632..0000000000000 --- a/tests/headers.rs +++ /dev/null @@ -1,34 +0,0 @@ -use regex::Regex; -use std::fs; -use walkdir::WalkDir; - -#[test] -fn old_test_headers() { - let old_headers = Regex::new( - r"^//( ?\[\w+\])? ?((check|build|run|ignore|aux|only|needs|rustc|unset|no|normalize|run|compile)-|edition|incremental|revisions).*", - ) - .unwrap(); - let mut failed = false; - - for entry in WalkDir::new("tests") { - let entry = entry.unwrap(); - let is_hidden_file = entry - .file_name() - .to_str() - .expect("non-UTF-8 file name") - .starts_with('.'); - if is_hidden_file || !entry.file_type().is_file() { - continue; - } - - let file = fs::read_to_string(entry.path()).unwrap_or_else(|err| panic!("{}: {err}", entry.path().display())); - - if let Some(header) = old_headers.find(&file) { - println!("Found header `{}` in {}", header.as_str(), entry.path().display()); - - failed = true; - } - } - - assert!(!failed, "use `//@foo` style test headers instead"); -} diff --git a/tests/ui/assign_ops.fixed b/tests/ui/assign_ops.fixed index 429c20f95e919..3bc6885d7c3e6 100644 --- a/tests/ui/assign_ops.fixed +++ b/tests/ui/assign_ops.fixed @@ -1,5 +1,6 @@ #![allow(clippy::useless_vec)] #![warn(clippy::assign_op_pattern)] +#![feature(const_trait_impl, const_ops)] use core::num::Wrapping; use std::ops::{Mul, MulAssign}; @@ -73,3 +74,33 @@ impl MulAssign for Wrap { *self = *self * rhs } } + +mod issue14871 { + + use std::ops::{Add, AddAssign}; + + pub trait Number: Copy + Add + AddAssign { + const ZERO: Self; + const ONE: Self; + } + + #[const_trait] + pub trait NumberConstants { + fn constant(value: usize) -> Self; + } + + impl const NumberConstants for T + where + T: Number + ~const core::ops::Add, + { + fn constant(value: usize) -> Self { + let mut res = Self::ZERO; + let mut count = 0; + while count < value { + res = res + Self::ONE; + count += 1; + } + res + } + } +} diff --git a/tests/ui/assign_ops.rs b/tests/ui/assign_ops.rs index 480ff07f150ea..f1f8f9daff95e 100644 --- a/tests/ui/assign_ops.rs +++ b/tests/ui/assign_ops.rs @@ -1,5 +1,6 @@ #![allow(clippy::useless_vec)] #![warn(clippy::assign_op_pattern)] +#![feature(const_trait_impl, const_ops)] use core::num::Wrapping; use std::ops::{Mul, MulAssign}; @@ -73,3 +74,33 @@ impl MulAssign for Wrap { *self = *self * rhs } } + +mod issue14871 { + + use std::ops::{Add, AddAssign}; + + pub trait Number: Copy + Add + AddAssign { + const ZERO: Self; + const ONE: Self; + } + + #[const_trait] + pub trait NumberConstants { + fn constant(value: usize) -> Self; + } + + impl const NumberConstants for T + where + T: Number + ~const core::ops::Add, + { + fn constant(value: usize) -> Self { + let mut res = Self::ZERO; + let mut count = 0; + while count < value { + res = res + Self::ONE; + count += 1; + } + res + } + } +} diff --git a/tests/ui/assign_ops.stderr b/tests/ui/assign_ops.stderr index 881a333fbe4b9..c5e698b3ee118 100644 --- a/tests/ui/assign_ops.stderr +++ b/tests/ui/assign_ops.stderr @@ -1,5 +1,5 @@ error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:9:5 + --> tests/ui/assign_ops.rs:10:5 | LL | a = a + 1; | ^^^^^^^^^ help: replace it with: `a += 1` @@ -8,67 +8,67 @@ LL | a = a + 1; = help: to override `-D warnings` add `#[allow(clippy::assign_op_pattern)]` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:11:5 + --> tests/ui/assign_ops.rs:12:5 | LL | a = 1 + a; | ^^^^^^^^^ help: replace it with: `a += 1` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:13:5 + --> tests/ui/assign_ops.rs:14:5 | LL | a = a - 1; | ^^^^^^^^^ help: replace it with: `a -= 1` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:15:5 + --> tests/ui/assign_ops.rs:16:5 | LL | a = a * 99; | ^^^^^^^^^^ help: replace it with: `a *= 99` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:17:5 + --> tests/ui/assign_ops.rs:18:5 | LL | a = 42 * a; | ^^^^^^^^^^ help: replace it with: `a *= 42` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:19:5 + --> tests/ui/assign_ops.rs:20:5 | LL | a = a / 2; | ^^^^^^^^^ help: replace it with: `a /= 2` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:21:5 + --> tests/ui/assign_ops.rs:22:5 | LL | a = a % 5; | ^^^^^^^^^ help: replace it with: `a %= 5` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:23:5 + --> tests/ui/assign_ops.rs:24:5 | LL | a = a & 1; | ^^^^^^^^^ help: replace it with: `a &= 1` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:30:5 + --> tests/ui/assign_ops.rs:31:5 | LL | s = s + "bla"; | ^^^^^^^^^^^^^ help: replace it with: `s += "bla"` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:35:5 + --> tests/ui/assign_ops.rs:36:5 | LL | a = a + Wrapping(1u32); | ^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a += Wrapping(1u32)` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:38:5 + --> tests/ui/assign_ops.rs:39:5 | LL | v[0] = v[0] + v[1]; | ^^^^^^^^^^^^^^^^^^ help: replace it with: `v[0] += v[1]` error: manual implementation of an assign operation - --> tests/ui/assign_ops.rs:51:5 + --> tests/ui/assign_ops.rs:52:5 | LL | buf = buf + cows.clone(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `buf += cows.clone()` diff --git a/tests/ui/dbg_macro/dbg_macro.fixed b/tests/ui/dbg_macro/dbg_macro.fixed index 3b9dee81898ab..5993c2faf0ddd 100644 --- a/tests/ui/dbg_macro/dbg_macro.fixed +++ b/tests/ui/dbg_macro/dbg_macro.fixed @@ -123,3 +123,19 @@ mod issue12131 { //~^ dbg_macro } } + +mod issue14914 { + use std::future::Future; + + fn takes_async_fn(_f: F) + where + F: FnOnce(i32) -> Fut, + Fut: Future, + { + } + + fn should_not_panic() { + takes_async_fn(async |val| val); + //~^ dbg_macro + } +} diff --git a/tests/ui/dbg_macro/dbg_macro.rs b/tests/ui/dbg_macro/dbg_macro.rs index 1dbbc6fe98456..58d7e106e238d 100644 --- a/tests/ui/dbg_macro/dbg_macro.rs +++ b/tests/ui/dbg_macro/dbg_macro.rs @@ -123,3 +123,19 @@ mod issue12131 { //~^ dbg_macro } } + +mod issue14914 { + use std::future::Future; + + fn takes_async_fn(_f: F) + where + F: FnOnce(i32) -> Fut, + Fut: Future, + { + } + + fn should_not_panic() { + takes_async_fn(async |val| dbg!(val)); + //~^ dbg_macro + } +} diff --git a/tests/ui/dbg_macro/dbg_macro.stderr b/tests/ui/dbg_macro/dbg_macro.stderr index f1412023cc897..5a65b38a85c4c 100644 --- a/tests/ui/dbg_macro/dbg_macro.stderr +++ b/tests/ui/dbg_macro/dbg_macro.stderr @@ -230,5 +230,17 @@ LL - print!("{}", dbg!(s)); LL + print!("{}", s); | -error: aborting due to 19 previous errors +error: the `dbg!` macro is intended as a debugging tool + --> tests/ui/dbg_macro/dbg_macro.rs:138:36 + | +LL | takes_async_fn(async |val| dbg!(val)); + | ^^^^^^^^^ + | +help: remove the invocation before committing it to a version control system + | +LL - takes_async_fn(async |val| dbg!(val)); +LL + takes_async_fn(async |val| val); + | + +error: aborting due to 20 previous errors diff --git a/tests/ui/doc/doc_markdown-issue_13097.fixed b/tests/ui/doc/doc_markdown-issue_13097.fixed index fb0f40b34a4b8..e0136584f3deb 100644 --- a/tests/ui/doc/doc_markdown-issue_13097.fixed +++ b/tests/ui/doc/doc_markdown-issue_13097.fixed @@ -1,11 +1,37 @@ -// This test checks that words starting with capital letters and ending with "ified" don't -// trigger the lint. - #![deny(clippy::doc_markdown)] +#![allow(clippy::doc_lazy_continuation)] + +mod issue13097 { + // This test checks that words starting with capital letters and ending with "ified" don't + // trigger the lint. + pub enum OutputFormat { + /// `HumaNified` + //~^ ERROR: item in documentation is missing backticks + Plain, + // Should not warn! + /// JSONified console output + Json, + } +} +#[rustfmt::skip] pub enum OutputFormat { - /// `HumaNified` - //~^ ERROR: item in documentation is missing backticks + /** + * `HumaNified` + //~^ ERROR: item in documentation is missing backticks + * Before \u{08888} `HumaNified` \{u08888} After + //~^ ERROR: item in documentation is missing backticks + * meow meow \[`meow_meow`\] meow meow? + //~^ ERROR: item in documentation is missing backticks + * \u{08888} `meow_meow` \[meow meow] meow? + //~^ ERROR: item in documentation is missing backticks + * Above + * \u{08888} + * \[hi\]() `HumaNified` \[example]() + //~^ ERROR: item in documentation is missing backticks + * \u{08888} + * Below + */ Plain, // Should not warn! /// JSONified console output diff --git a/tests/ui/doc/doc_markdown-issue_13097.rs b/tests/ui/doc/doc_markdown-issue_13097.rs index 8c1e1a3cd6c24..2e89fe6c56b4c 100644 --- a/tests/ui/doc/doc_markdown-issue_13097.rs +++ b/tests/ui/doc/doc_markdown-issue_13097.rs @@ -1,11 +1,37 @@ -// This test checks that words starting with capital letters and ending with "ified" don't -// trigger the lint. - #![deny(clippy::doc_markdown)] +#![allow(clippy::doc_lazy_continuation)] + +mod issue13097 { + // This test checks that words starting with capital letters and ending with "ified" don't + // trigger the lint. + pub enum OutputFormat { + /// HumaNified + //~^ ERROR: item in documentation is missing backticks + Plain, + // Should not warn! + /// JSONified console output + Json, + } +} +#[rustfmt::skip] pub enum OutputFormat { - /// HumaNified - //~^ ERROR: item in documentation is missing backticks + /** + * HumaNified + //~^ ERROR: item in documentation is missing backticks + * Before \u{08888} HumaNified \{u08888} After + //~^ ERROR: item in documentation is missing backticks + * meow meow \[meow_meow\] meow meow? + //~^ ERROR: item in documentation is missing backticks + * \u{08888} meow_meow \[meow meow] meow? + //~^ ERROR: item in documentation is missing backticks + * Above + * \u{08888} + * \[hi\]() HumaNified \[example]() + //~^ ERROR: item in documentation is missing backticks + * \u{08888} + * Below + */ Plain, // Should not warn! /// JSONified console output diff --git a/tests/ui/doc/doc_markdown-issue_13097.stderr b/tests/ui/doc/doc_markdown-issue_13097.stderr index 65b8f2ed80b6c..cea788301d4dc 100644 --- a/tests/ui/doc/doc_markdown-issue_13097.stderr +++ b/tests/ui/doc/doc_markdown-issue_13097.stderr @@ -1,19 +1,79 @@ error: item in documentation is missing backticks - --> tests/ui/doc/doc_markdown-issue_13097.rs:7:9 + --> tests/ui/doc/doc_markdown-issue_13097.rs:8:13 | -LL | /// HumaNified - | ^^^^^^^^^^ +LL | /// HumaNified + | ^^^^^^^^^^ | note: the lint level is defined here - --> tests/ui/doc/doc_markdown-issue_13097.rs:4:9 + --> tests/ui/doc/doc_markdown-issue_13097.rs:1:9 | LL | #![deny(clippy::doc_markdown)] | ^^^^^^^^^^^^^^^^^^^^ help: try | -LL - /// HumaNified -LL + /// `HumaNified` +LL - /// HumaNified +LL + /// `HumaNified` | -error: aborting due to 1 previous error +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:20:8 + | +LL | * HumaNified + | ^^^^^^^^^^ + | +help: try + | +LL - * HumaNified +LL + * `HumaNified` + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:22:25 + | +LL | * Before \u{08888} HumaNified \{u08888} After + | ^^^^^^^^^^ + | +help: try + | +LL - * Before \u{08888} HumaNified \{u08888} After +LL + * Before \u{08888} `HumaNified` \{u08888} After + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:24:20 + | +LL | * meow meow \[meow_meow\] meow meow? + | ^^^^^^^^^ + | +help: try + | +LL - * meow meow \[meow_meow\] meow meow? +LL + * meow meow \[`meow_meow`\] meow meow? + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:26:18 + | +LL | * \u{08888} meow_meow \[meow meow] meow? + | ^^^^^^^^^ + | +help: try + | +LL - * \u{08888} meow_meow \[meow meow] meow? +LL + * \u{08888} `meow_meow` \[meow meow] meow? + | + +error: item in documentation is missing backticks + --> tests/ui/doc/doc_markdown-issue_13097.rs:30:38 + | +LL | * \[hi\]() HumaNified \[example]() + | ^^^^^^^^^^ + | +help: try + | +LL - * \[hi\]() HumaNified \[example]() +LL + * \[hi\]() `HumaNified` \[example]() + | + +error: aborting due to 6 previous errors diff --git a/tests/ui/explicit_deref_methods.fixed b/tests/ui/explicit_deref_methods.fixed index 619329a6ade0c..52c4d1b1f301a 100644 --- a/tests/ui/explicit_deref_methods.fixed +++ b/tests/ui/explicit_deref_methods.fixed @@ -81,12 +81,10 @@ fn main() { let b: String = concat(just_return(a)); //~^ explicit_deref_methods - let b: &str = &**a; - //~^ explicit_deref_methods + let b: &str = a.deref().deref(); let opt_a = Some(a.clone()); - let b = &*opt_a.unwrap(); - //~^ explicit_deref_methods + let b = opt_a.unwrap().deref(); Aaa::deref(&Aaa); Aaa::deref_mut(&mut Aaa); diff --git a/tests/ui/explicit_deref_methods.rs b/tests/ui/explicit_deref_methods.rs index 9f2d513283c9b..706d6cb2b79a6 100644 --- a/tests/ui/explicit_deref_methods.rs +++ b/tests/ui/explicit_deref_methods.rs @@ -82,11 +82,9 @@ fn main() { //~^ explicit_deref_methods let b: &str = a.deref().deref(); - //~^ explicit_deref_methods let opt_a = Some(a.clone()); let b = opt_a.unwrap().deref(); - //~^ explicit_deref_methods Aaa::deref(&Aaa); Aaa::deref_mut(&mut Aaa); diff --git a/tests/ui/explicit_deref_methods.stderr b/tests/ui/explicit_deref_methods.stderr index a81e2f60317b3..5036884366cfc 100644 --- a/tests/ui/explicit_deref_methods.stderr +++ b/tests/ui/explicit_deref_methods.stderr @@ -56,40 +56,28 @@ LL | let b: String = concat(just_return(a).deref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `just_return(a)` error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:84:19 - | -LL | let b: &str = a.deref().deref(); - | ^^^^^^^^^^^^^^^^^ help: try: `&**a` - -error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:88:13 - | -LL | let b = opt_a.unwrap().deref(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*opt_a.unwrap()` - -error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:123:31 + --> tests/ui/explicit_deref_methods.rs:121:31 | LL | let b: &str = expr_deref!(a.deref()); | ^^^^^^^^^ help: try: `&*a` error: explicit `deref` method call - --> tests/ui/explicit_deref_methods.rs:141:14 + --> tests/ui/explicit_deref_methods.rs:139:14 | LL | let _ = &Deref::deref(&"foo"); | ^^^^^^^^^^^^^^^^^^^^ help: try: `*&"foo"` error: explicit `deref_mut` method call - --> tests/ui/explicit_deref_methods.rs:143:14 + --> tests/ui/explicit_deref_methods.rs:141:14 | LL | let _ = &DerefMut::deref_mut(&mut x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut **&mut x` error: explicit `deref_mut` method call - --> tests/ui/explicit_deref_methods.rs:144:14 + --> tests/ui/explicit_deref_methods.rs:142:14 | LL | let _ = &DerefMut::deref_mut((&mut &mut x).deref_mut()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut ***(&mut &mut x)` -error: aborting due to 15 previous errors +error: aborting due to 13 previous errors diff --git a/tests/ui/manual_find_fixable.fixed b/tests/ui/manual_find_fixable.fixed index 5e6849a4dfb0f..01b3ebacbebcb 100644 --- a/tests/ui/manual_find_fixable.fixed +++ b/tests/ui/manual_find_fixable.fixed @@ -179,3 +179,13 @@ fn two_bindings(v: Vec<(u8, u8)>) -> Option { } fn main() {} + +mod issue14826 { + fn adjust_fixable(needle: &str) -> Option<&'static str> { + ["foo", "bar"].iter().find(|&candidate| candidate.eq_ignore_ascii_case(needle)).map(|v| v as _) + } + + fn adjust_unfixable(needle: &str) -> Option<*const str> { + ["foo", "bar"].iter().find(|&&candidate| candidate.eq_ignore_ascii_case(needle)).copied().map(|v| v as _) + } +} diff --git a/tests/ui/manual_find_fixable.rs b/tests/ui/manual_find_fixable.rs index 08a7dd2c6eeef..ce62a4beba1c4 100644 --- a/tests/ui/manual_find_fixable.rs +++ b/tests/ui/manual_find_fixable.rs @@ -251,3 +251,25 @@ fn two_bindings(v: Vec<(u8, u8)>) -> Option { } fn main() {} + +mod issue14826 { + fn adjust_fixable(needle: &str) -> Option<&'static str> { + for candidate in &["foo", "bar"] { + //~^ manual_find + if candidate.eq_ignore_ascii_case(needle) { + return Some(candidate); + } + } + None + } + + fn adjust_unfixable(needle: &str) -> Option<*const str> { + for &candidate in &["foo", "bar"] { + //~^ manual_find + if candidate.eq_ignore_ascii_case(needle) { + return Some(candidate); + } + } + None + } +} diff --git a/tests/ui/manual_find_fixable.stderr b/tests/ui/manual_find_fixable.stderr index afa453c5a876a..020635d90bb5c 100644 --- a/tests/ui/manual_find_fixable.stderr +++ b/tests/ui/manual_find_fixable.stderr @@ -139,5 +139,27 @@ LL | | return Some(x); LL | | None | |____________^ help: replace with an iterator: `arr.into_iter().find(|&x| x < 1)` -error: aborting due to 12 previous errors +error: manual implementation of `Iterator::find` + --> tests/ui/manual_find_fixable.rs:257:9 + | +LL | / for candidate in &["foo", "bar"] { +LL | | +LL | | if candidate.eq_ignore_ascii_case(needle) { +LL | | return Some(candidate); +... | +LL | | None + | |____________^ help: replace with an iterator: `["foo", "bar"].iter().find(|&candidate| candidate.eq_ignore_ascii_case(needle)).map(|v| v as _)` + +error: manual implementation of `Iterator::find` + --> tests/ui/manual_find_fixable.rs:267:9 + | +LL | / for &candidate in &["foo", "bar"] { +LL | | +LL | | if candidate.eq_ignore_ascii_case(needle) { +LL | | return Some(candidate); +... | +LL | | None + | |____________^ help: replace with an iterator: `["foo", "bar"].iter().find(|&&candidate| candidate.eq_ignore_ascii_case(needle)).copied().map(|v| v as _)` + +error: aborting due to 14 previous errors diff --git a/tests/ui/manual_flatten.rs b/tests/ui/manual_flatten.rs index 97f35c36e24cc..f1a0053ef384d 100644 --- a/tests/ui/manual_flatten.rs +++ b/tests/ui/manual_flatten.rs @@ -123,6 +123,13 @@ fn main() { println!("{}", n); } + // Using nested `Some` pattern should not trigger the lint + for n in vec![Some((1, Some(2)))] { + if let Some((_, Some(n))) = n { + println!("{}", n); + } + } + run_unformatted_tests(); } diff --git a/tests/ui/manual_flatten.stderr b/tests/ui/manual_flatten.stderr index 5ab25658017eb..9a846fe17f329 100644 --- a/tests/ui/manual_flatten.stderr +++ b/tests/ui/manual_flatten.stderr @@ -178,7 +178,7 @@ LL | | } | |_________^ error: unnecessary `if let` since only the `Some` variant of the iterator element is used - --> tests/ui/manual_flatten.rs:132:5 + --> tests/ui/manual_flatten.rs:139:5 | LL | / for n in vec![ LL | | @@ -189,7 +189,7 @@ LL | | } | |_____^ | help: remove the `if let` statement in the for loop and then... - --> tests/ui/manual_flatten.rs:139:9 + --> tests/ui/manual_flatten.rs:146:9 | LL | / if let Some(n) = n { LL | | println!("{:?}", n); diff --git a/tests/ui/manual_is_variant_and.fixed b/tests/ui/manual_is_variant_and.fixed index c9c184561dd69..18a72188ab593 100644 --- a/tests/ui/manual_is_variant_and.fixed +++ b/tests/ui/manual_is_variant_and.fixed @@ -4,6 +4,44 @@ #[macro_use] extern crate option_helpers; +struct Foo(T); + +impl Foo { + fn map bool>(self, mut f: F) -> Option { + Some(f(self.0)) + } +} + +fn foo() -> Option { + Some(true) +} + +macro_rules! some_true { + () => { + Some(true) + }; +} +macro_rules! some_false { + () => { + Some(false) + }; +} + +macro_rules! mac { + (some $e:expr) => { + Some($e) + }; + (some_map $e:expr) => { + Some($e).map(|x| x % 2 == 0) + }; + (map $e:expr) => { + $e.map(|x| x % 2 == 0) + }; + (eq $a:expr, $b:expr) => { + $a == $b + }; +} + #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -21,6 +59,15 @@ fn option_methods() { let _ = opt .is_some_and(|x| x > 1); + let _ = Some(2).is_some_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = Some(2).is_none_or(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = Some(2).is_some_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = Some(2).is_none_or(|x| x % 2 == 0); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -28,6 +75,14 @@ fn option_methods() { let _ = opt2.is_some_and(char::is_alphanumeric); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint + + // Should not lint. + let _ = Foo::(0).map(|x| x % 2 == 0) == Some(true); + let _ = Some(2).map(|x| x % 2 == 0) != foo(); + let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); + let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); + let _ = mac!(some_map 2) == Some(true); + let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -41,6 +96,13 @@ fn result_methods() { }); let _ = res.is_ok_and(|x| x > 1); + let _ = Ok::(2).is_ok_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = !Ok::(2).is_ok_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + let _ = !Ok::(2).is_ok_and(|x| x % 2 == 0); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/tests/ui/manual_is_variant_and.rs b/tests/ui/manual_is_variant_and.rs index 52c7b56804ce2..a92f7c0436959 100644 --- a/tests/ui/manual_is_variant_and.rs +++ b/tests/ui/manual_is_variant_and.rs @@ -4,6 +4,44 @@ #[macro_use] extern crate option_helpers; +struct Foo(T); + +impl Foo { + fn map bool>(self, mut f: F) -> Option { + Some(f(self.0)) + } +} + +fn foo() -> Option { + Some(true) +} + +macro_rules! some_true { + () => { + Some(true) + }; +} +macro_rules! some_false { + () => { + Some(false) + }; +} + +macro_rules! mac { + (some $e:expr) => { + Some($e) + }; + (some_map $e:expr) => { + Some($e).map(|x| x % 2 == 0) + }; + (map $e:expr) => { + $e.map(|x| x % 2 == 0) + }; + (eq $a:expr, $b:expr) => { + $a == $b + }; +} + #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -27,6 +65,15 @@ fn option_methods() { //~^ manual_is_variant_and .unwrap_or_default(); + let _ = Some(2).map(|x| x % 2 == 0) == Some(true); + //~^ manual_is_variant_and + let _ = Some(2).map(|x| x % 2 == 0) != Some(true); + //~^ manual_is_variant_and + let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); + //~^ manual_is_variant_and + let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -34,6 +81,14 @@ fn option_methods() { let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint + + // Should not lint. + let _ = Foo::(0).map(|x| x % 2 == 0) == Some(true); + let _ = Some(2).map(|x| x % 2 == 0) != foo(); + let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); + let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); + let _ = mac!(some_map 2) == Some(true); + let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -50,6 +105,13 @@ fn result_methods() { //~^ manual_is_variant_and .unwrap_or_default(); + let _ = Ok::(2).map(|x| x % 2 == 0) == Ok(true); + //~^ manual_is_variant_and + let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); + //~^ manual_is_variant_and + let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); + //~^ manual_is_variant_and + // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/tests/ui/manual_is_variant_and.stderr b/tests/ui/manual_is_variant_and.stderr index a4fa500580d0a..1fb437a8bc744 100644 --- a/tests/ui/manual_is_variant_and.stderr +++ b/tests/ui/manual_is_variant_and.stderr @@ -1,5 +1,5 @@ error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:13:17 + --> tests/ui/manual_is_variant_and.rs:51:17 | LL | let _ = opt.map(|x| x > 1) | _________________^ @@ -11,7 +11,7 @@ LL | | .unwrap_or_default(); = help: to override `-D warnings` add `#[allow(clippy::manual_is_variant_and)]` error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:18:17 + --> tests/ui/manual_is_variant_and.rs:56:17 | LL | let _ = opt.map(|x| { | _________________^ @@ -30,13 +30,13 @@ LL ~ }); | error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:23:17 + --> tests/ui/manual_is_variant_and.rs:61:17 | LL | let _ = opt.map(|x| x > 1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)` error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:26:10 + --> tests/ui/manual_is_variant_and.rs:64:10 | LL | .map(|x| x > 1) | __________^ @@ -44,14 +44,38 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_some_and(|x| x > 1)` +error: called `.map() == Some()` + --> tests/ui/manual_is_variant_and.rs:68:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) == Some(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` + +error: called `.map() != Some()` + --> tests/ui/manual_is_variant_and.rs:70:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) != Some(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` + +error: called `.map() == Some()` + --> tests/ui/manual_is_variant_and.rs:72:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` + +error: called `.map() != Some()` + --> tests/ui/manual_is_variant_and.rs:74:13 + | +LL | let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` + error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:34:18 + --> tests/ui/manual_is_variant_and.rs:81:18 | LL | let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)` error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:44:17 + --> tests/ui/manual_is_variant_and.rs:99:17 | LL | let _ = res.map(|x| { | _________________^ @@ -70,7 +94,7 @@ LL ~ }); | error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:49:17 + --> tests/ui/manual_is_variant_and.rs:104:17 | LL | let _ = res.map(|x| x > 1) | _________________^ @@ -78,11 +102,29 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_ok_and(|x| x > 1)` +error: called `.map() == Ok()` + --> tests/ui/manual_is_variant_and.rs:108:13 + | +LL | let _ = Ok::(2).map(|x| x % 2 == 0) == Ok(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Ok::(2).is_ok_and(|x| x % 2 == 0)` + +error: called `.map() != Ok()` + --> tests/ui/manual_is_variant_and.rs:110:13 + | +LL | let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::(2).is_ok_and(|x| x % 2 == 0)` + +error: called `.map() != Ok()` + --> tests/ui/manual_is_variant_and.rs:112:13 + | +LL | let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::(2).is_ok_and(|x| x % 2 == 0)` + error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:57:18 + --> tests/ui/manual_is_variant_and.rs:119:18 | LL | let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)` -error: aborting due to 8 previous errors +error: aborting due to 15 previous errors diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index d7d344452c5d8..54cad2e393fd4 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -107,9 +107,6 @@ fn main() { let x = (1, 2); let _ = x.0; //~^ needless_borrow - let x = &x as *const (i32, i32); - let _ = unsafe { (*x).0 }; - //~^ needless_borrow // Issue #8367 trait Foo { @@ -289,3 +286,15 @@ fn issue_12268() { // compiler } + +fn issue_14743(slice: &[T]) { + let _ = slice.len(); + //~^ needless_borrow + + let slice = slice as *const [T]; + let _ = unsafe { (&*slice).len() }; + + // Check that rustc would actually warn if Clippy had suggested removing the reference + #[expect(dangerous_implicit_autorefs)] + let _ = unsafe { (*slice).len() }; +} diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 1f05b90b47285..b698c6bfc9695 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -107,9 +107,6 @@ fn main() { let x = (1, 2); let _ = (&x).0; //~^ needless_borrow - let x = &x as *const (i32, i32); - let _ = unsafe { (&*x).0 }; - //~^ needless_borrow // Issue #8367 trait Foo { @@ -289,3 +286,15 @@ fn issue_12268() { // compiler } + +fn issue_14743(slice: &[T]) { + let _ = (&slice).len(); + //~^ needless_borrow + + let slice = slice as *const [T]; + let _ = unsafe { (&*slice).len() }; + + // Check that rustc would actually warn if Clippy had suggested removing the reference + #[expect(dangerous_implicit_autorefs)] + let _ = unsafe { (*slice).len() }; +} diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index b036b1e47d187..172d36bd73a0a 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -103,71 +103,71 @@ error: this expression borrows a value the compiler would automatically borrow LL | let _ = (&x).0; | ^^^^ help: change this to: `x` -error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:111:22 - | -LL | let _ = unsafe { (&*x).0 }; - | ^^^^^ help: change this to: `(*x)` - error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:122:5 + --> tests/ui/needless_borrow.rs:119:5 | LL | (&&()).foo(); | ^^^^^^ help: change this to: `(&())` error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:132:5 + --> tests/ui/needless_borrow.rs:129:5 | LL | (&&5).foo(); | ^^^^^ help: change this to: `(&5)` error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:159:23 + --> tests/ui/needless_borrow.rs:156:23 | LL | let x: (&str,) = (&"",); | ^^^ help: change this to: `""` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:202:13 + --> tests/ui/needless_borrow.rs:199:13 | LL | (&self.f)() | ^^^^^^^^^ help: change this to: `(self.f)` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:212:13 + --> tests/ui/needless_borrow.rs:209:13 | LL | (&mut self.f)() | ^^^^^^^^^^^^^ help: change this to: `(self.f)` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:250:22 + --> tests/ui/needless_borrow.rs:247:22 | LL | let _ = &mut (&mut { x.u }).x; | ^^^^^^^^^^^^^^ help: change this to: `{ x.u }` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:258:22 + --> tests/ui/needless_borrow.rs:255:22 | LL | let _ = &mut (&mut { x.u }).x; | ^^^^^^^^^^^^^^ help: change this to: `{ x.u }` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:263:22 + --> tests/ui/needless_borrow.rs:260:22 | LL | let _ = &mut (&mut x.u).x; | ^^^^^^^^^^ help: change this to: `x.u` error: this expression borrows a value the compiler would automatically borrow - --> tests/ui/needless_borrow.rs:265:22 + --> tests/ui/needless_borrow.rs:262:22 | LL | let _ = &mut (&mut { x.u }).x; | ^^^^^^^^^^^^^^ help: change this to: `{ x.u }` error: this expression creates a reference which is immediately dereferenced by the compiler - --> tests/ui/needless_borrow.rs:287:23 + --> tests/ui/needless_borrow.rs:284:23 | LL | option.unwrap_or((&x.0,)); | ^^^^ help: change this to: `x.0` +error: this expression creates a reference which is immediately dereferenced by the compiler + --> tests/ui/needless_borrow.rs:291:13 + | +LL | let _ = (&slice).len(); + | ^^^^^^^^ help: change this to: `slice` + error: aborting due to 28 previous errors diff --git a/tests/ui/needless_for_each_fixable.fixed b/tests/ui/needless_for_each_fixable.fixed index fa23e18318f19..a73aff556399e 100644 --- a/tests/ui/needless_for_each_fixable.fixed +++ b/tests/ui/needless_for_each_fixable.fixed @@ -128,3 +128,18 @@ fn should_not_lint() { } fn main() {} + +mod issue14734 { + fn let_desugar(rows: &[u8]) { + let mut v = vec![]; + for x in rows.iter() { _ = v.push(x) } + //~^ needless_for_each + } + + fn do_something(_: &u8, _: u8) {} + + fn single_expr(rows: &[u8]) { + for x in rows.iter() { do_something(x, 1u8); } + //~^ needless_for_each + } +} diff --git a/tests/ui/needless_for_each_fixable.rs b/tests/ui/needless_for_each_fixable.rs index 2c7e68a6f5128..d92f055d3f45b 100644 --- a/tests/ui/needless_for_each_fixable.rs +++ b/tests/ui/needless_for_each_fixable.rs @@ -128,3 +128,18 @@ fn should_not_lint() { } fn main() {} + +mod issue14734 { + fn let_desugar(rows: &[u8]) { + let mut v = vec![]; + rows.iter().for_each(|x| _ = v.push(x)); + //~^ needless_for_each + } + + fn do_something(_: &u8, _: u8) {} + + fn single_expr(rows: &[u8]) { + rows.iter().for_each(|x| do_something(x, 1u8)); + //~^ needless_for_each + } +} diff --git a/tests/ui/needless_for_each_fixable.stderr b/tests/ui/needless_for_each_fixable.stderr index 013a3fa3e36d5..f801445609769 100644 --- a/tests/ui/needless_for_each_fixable.stderr +++ b/tests/ui/needless_for_each_fixable.stderr @@ -136,5 +136,17 @@ LL + acc += elem; LL + } | -error: aborting due to 8 previous errors +error: needless use of `for_each` + --> tests/ui/needless_for_each_fixable.rs:135:9 + | +LL | rows.iter().for_each(|x| _ = v.push(x)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in rows.iter() { _ = v.push(x) }` + +error: needless use of `for_each` + --> tests/ui/needless_for_each_fixable.rs:142:9 + | +LL | rows.iter().for_each(|x| do_something(x, 1u8)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for x in rows.iter() { do_something(x, 1u8); }` + +error: aborting due to 10 previous errors diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index 17d3862cd8668..d571b97f51946 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -452,3 +452,68 @@ pub unsafe fn issue_12157() -> *const i32 { (unsafe { todo() } as *const i32) //~^ needless_return } + +mod else_ifs { + fn test1(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if a < 10 { + 2 + //~^ needless_return + } else { + 3 + //~^ needless_return + } + } + + fn test2(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if a < 10 { + 2 + } else { + 3 + //~^ needless_return + } + } + + fn test3(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if a < 10 { + 2 + } else { + 3 + //~^ needless_return + } + } + + #[allow(clippy::match_single_binding, clippy::redundant_pattern)] + fn test4(a: i32) -> u32 { + if a == 0 { + 1 + //~^ needless_return + } else if if if a > 0x1_1 { + return 2; + } else { + return 5; + } { + true + } else { + true + } { + 0xDEADC0DE + } else if match a { + b @ _ => { + return 1; + }, + } { + 0xDEADBEEF + } else { + 1 + } + } +} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 1c6e7ffa1ee8c..2e4348ea338c0 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -461,3 +461,68 @@ pub unsafe fn issue_12157() -> *const i32 { return unsafe { todo() } as *const i32; //~^ needless_return } + +mod else_ifs { + fn test1(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if a < 10 { + return 2; + //~^ needless_return + } else { + return 3; + //~^ needless_return + } + } + + fn test2(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if a < 10 { + 2 + } else { + return 3; + //~^ needless_return + } + } + + fn test3(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if a < 10 { + 2 + } else { + return 3; + //~^ needless_return + } + } + + #[allow(clippy::match_single_binding, clippy::redundant_pattern)] + fn test4(a: i32) -> u32 { + if a == 0 { + return 1; + //~^ needless_return + } else if if if a > 0x1_1 { + return 2; + } else { + return 5; + } { + true + } else { + true + } { + 0xDEADC0DE + } else if match a { + b @ _ => { + return 1; + }, + } { + 0xDEADBEEF + } else { + 1 + } + } +} diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index 26dd265379be0..206bd8ee5af11 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -685,5 +685,101 @@ LL - return unsafe { todo() } as *const i32; LL + (unsafe { todo() } as *const i32) | -error: aborting due to 55 previous errors +error: unneeded `return` statement + --> tests/ui/needless_return.rs:468:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:471:13 + | +LL | return 2; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 2; +LL + 2 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:474:13 + | +LL | return 3; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 3; +LL + 3 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:481:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:486:13 + | +LL | return 3; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 3; +LL + 3 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:493:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:498:13 + | +LL | return 3; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 3; +LL + 3 + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:506:13 + | +LL | return 1; + | ^^^^^^^^ + | +help: remove `return` + | +LL - return 1; +LL + 1 + | + +error: aborting due to 63 previous errors diff --git a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed b/tests/ui/non_expressive_names_error_recovery.fixed similarity index 100% rename from tests/ui/skip_rustfmt/non_expressive_names_error_recovery.fixed rename to tests/ui/non_expressive_names_error_recovery.fixed diff --git a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs b/tests/ui/non_expressive_names_error_recovery.rs similarity index 100% rename from tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs rename to tests/ui/non_expressive_names_error_recovery.rs diff --git a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr b/tests/ui/non_expressive_names_error_recovery.stderr similarity index 81% rename from tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr rename to tests/ui/non_expressive_names_error_recovery.stderr index 4998b9bd2cc40..28d9a42a9a1ed 100644 --- a/tests/ui/skip_rustfmt/non_expressive_names_error_recovery.stderr +++ b/tests/ui/non_expressive_names_error_recovery.stderr @@ -1,5 +1,5 @@ error: expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `)` - --> tests/ui/skip_rustfmt/non_expressive_names_error_recovery.rs:6:19 + --> tests/ui/non_expressive_names_error_recovery.rs:6:19 | LL | fn aa(a: Aa>) -> Option { //~^^^ question_mark Some(format!("{a}")) } + +fn const_in_pattern(x: Option<(i32, i32)>) -> Option<()> { + const N: i32 = 0; + + let Some((x, N)) = x else { + return None; + }; + + None +} diff --git a/tests/ui/question_mark.rs b/tests/ui/question_mark.rs index 64b51b849ede0..99d0122a98faf 100644 --- a/tests/ui/question_mark.rs +++ b/tests/ui/question_mark.rs @@ -539,3 +539,13 @@ fn issue_14615(a: MutexGuard>) -> Option { //~^^^ question_mark Some(format!("{a}")) } + +fn const_in_pattern(x: Option<(i32, i32)>) -> Option<()> { + const N: i32 = 0; + + let Some((x, N)) = x else { + return None; + }; + + None +} diff --git a/tests/ui/trivially_copy_pass_by_ref.fixed b/tests/ui/trivially_copy_pass_by_ref.fixed new file mode 100644 index 0000000000000..af7d82130f01c --- /dev/null +++ b/tests/ui/trivially_copy_pass_by_ref.fixed @@ -0,0 +1,179 @@ +//@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)" +//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)" +#![deny(clippy::trivially_copy_pass_by_ref)] +#![allow( + clippy::disallowed_names, + clippy::extra_unused_lifetimes, + clippy::needless_lifetimes, + clippy::needless_pass_by_ref_mut, + clippy::redundant_field_names, + clippy::uninlined_format_args +)] + +#[derive(Copy, Clone)] +struct Foo(u32); + +#[derive(Copy, Clone)] +struct Bar([u8; 24]); + +#[derive(Copy, Clone)] +pub struct Color { + pub r: u8, + pub g: u8, + pub b: u8, + pub a: u8, +} + +struct FooRef<'a> { + foo: &'a Foo, +} + +type Baz = u32; + +fn good(a: &mut u32, b: u32, c: &Bar) {} + +fn good_return_implicit_lt_ref(foo: &Foo) -> &u32 { + &foo.0 +} + +#[allow(clippy::needless_lifetimes)] +fn good_return_explicit_lt_ref<'a>(foo: &'a Foo) -> &'a u32 { + &foo.0 +} + +fn good_return_implicit_lt_struct(foo: &Foo) -> FooRef { + FooRef { foo } +} + +#[allow(clippy::needless_lifetimes)] +fn good_return_explicit_lt_struct<'a>(foo: &'a Foo) -> FooRef<'a> { + FooRef { foo } +} + +fn bad(x: u32, y: Foo, z: Baz) {} +//~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by +//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by +//~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + +impl Foo { + fn good(self, a: &mut u32, b: u32, c: &Bar) {} + + fn good2(&mut self) {} + + fn bad(self, x: u32, y: Foo, z: Baz) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + + fn bad2(x: u32, y: Foo, z: Baz) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + + fn bad_issue7518(self, other: Self) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if +} + +impl AsRef for Foo { + fn as_ref(&self) -> &u32 { + &self.0 + } +} + +impl Bar { + fn good(&self, a: &mut u32, b: u32, c: &Bar) {} + + fn bad2(x: u32, y: Foo, z: Baz) {} + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if + //~| ERROR: this argument (4 byte) is passed by reference, but would be more efficient if +} + +trait MyTrait { + fn trait_method(&self, foo: Foo); + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if +} + +pub trait MyTrait2 { + fn trait_method2(&self, color: &Color); +} + +trait MyTrait3 { + #[expect(clippy::trivially_copy_pass_by_ref)] + fn trait_method(&self, foo: &Foo); +} + +// Trait impls should not warn +impl MyTrait3 for Foo { + fn trait_method(&self, foo: &Foo) { + unimplemented!() + } +} + +mod issue3992 { + pub trait A { + #[allow(clippy::trivially_copy_pass_by_ref)] + fn a(b: &u16) {} + } + + #[allow(clippy::trivially_copy_pass_by_ref)] + pub fn c(d: &u16) {} +} + +mod issue5876 { + // Don't lint here as it is always inlined + #[inline(always)] + fn foo_always(x: &i32) { + println!("{}", x); + } + + #[inline(never)] + fn foo_never(x: i32) { + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + println!("{}", x); + } + + #[inline] + fn foo(x: i32) { + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + println!("{}", x); + } +} + +fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> { + Some(x) +} + +fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> { + Some(x) +} + +fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 { + if true { x } else { y } +} + +async fn async_implicit(x: &u32) -> &u32 { + x +} + +async fn async_explicit<'a>(x: &'a u32) -> &'a u32 { + x +} + +fn unrelated_lifetimes<'a, 'b>(_x: u32, y: &'b u32) -> &'b u32 { + //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by + y +} + +fn return_ptr(x: &u32) -> *const u32 { + x +} + +fn return_field_ptr(x: &(u32, u32)) -> *const u32 { + &x.0 +} + +fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 { + core::ptr::addr_of!(x.0) +} diff --git a/tests/ui/trivially_copy_pass_by_ref.rs b/tests/ui/trivially_copy_pass_by_ref.rs index 37bc6f89a20af..00e11a1ea2808 100644 --- a/tests/ui/trivially_copy_pass_by_ref.rs +++ b/tests/ui/trivially_copy_pass_by_ref.rs @@ -1,14 +1,15 @@ //@normalize-stderr-test: "\(\d+ byte\)" -> "(N byte)" -//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: 8 byte)" +//@normalize-stderr-test: "\(limit: \d+ byte\)" -> "(limit: N byte)" #![deny(clippy::trivially_copy_pass_by_ref)] #![allow( clippy::disallowed_names, + clippy::extra_unused_lifetimes, clippy::needless_lifetimes, + clippy::needless_pass_by_ref_mut, clippy::redundant_field_names, - clippy::uninlined_format_args, - clippy::needless_pass_by_ref_mut + clippy::uninlined_format_args )] -//@no-rustfix + #[derive(Copy, Clone)] struct Foo(u32); @@ -90,21 +91,26 @@ impl Bar { } trait MyTrait { - fn trait_method(&self, _foo: &Foo); + fn trait_method(&self, foo: &Foo); //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if } pub trait MyTrait2 { - fn trait_method2(&self, _color: &Color); + fn trait_method2(&self, color: &Color); +} + +trait MyTrait3 { + #[expect(clippy::trivially_copy_pass_by_ref)] + fn trait_method(&self, foo: &Foo); } -impl MyTrait for Foo { - fn trait_method(&self, _foo: &Foo) { +// Trait impls should not warn +impl MyTrait3 for Foo { + fn trait_method(&self, foo: &Foo) { unimplemented!() } } -#[allow(unused_variables)] mod issue3992 { pub trait A { #[allow(clippy::trivially_copy_pass_by_ref)] @@ -135,57 +141,39 @@ mod issue5876 { } } -fn _ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> { +fn ref_to_opt_ref_implicit(x: &u32) -> Option<&u32> { Some(x) } -#[allow(clippy::needless_lifetimes)] -fn _ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> { +fn ref_to_opt_ref_explicit<'a>(x: &'a u32) -> Option<&'a u32> { Some(x) } -fn _with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 { +fn with_constraint<'a, 'b: 'a>(x: &'b u32, y: &'a u32) -> &'a u32 { if true { x } else { y } } -async fn _async_implicit(x: &u32) -> &u32 { +async fn async_implicit(x: &u32) -> &u32 { x } -#[allow(clippy::needless_lifetimes)] -async fn _async_explicit<'a>(x: &'a u32) -> &'a u32 { +async fn async_explicit<'a>(x: &'a u32) -> &'a u32 { x } -fn _unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { +fn unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { //~^ ERROR: this argument (4 byte) is passed by reference, but would be more efficient if passed by y } -fn _return_ptr(x: &u32) -> *const u32 { +fn return_ptr(x: &u32) -> *const u32 { x } -fn _return_field_ptr(x: &(u32, u32)) -> *const u32 { +fn return_field_ptr(x: &(u32, u32)) -> *const u32 { &x.0 } -fn _return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 { +fn return_field_ptr_addr_of(x: &(u32, u32)) -> *const u32 { core::ptr::addr_of!(x.0) } - -fn main() { - let (mut foo, bar) = (Foo(0), Bar([0; 24])); - let (mut a, b, c, x, y, z) = (0, 0, Bar([0; 24]), 0, Foo(0), 0); - good(&mut a, b, &c); - good_return_implicit_lt_ref(&y); - good_return_explicit_lt_ref(&y); - bad(&x, &y, &z); - foo.good(&mut a, b, &c); - foo.good2(); - foo.bad(&x, &y, &z); - Foo::bad2(&x, &y, &z); - bar.good(&mut a, b, &c); - Bar::bad2(&x, &y, &z); - foo.as_ref(); -} diff --git a/tests/ui/trivially_copy_pass_by_ref.stderr b/tests/ui/trivially_copy_pass_by_ref.stderr index e813fecf653ac..f101ac5ccd680 100644 --- a/tests/ui/trivially_copy_pass_by_ref.stderr +++ b/tests/ui/trivially_copy_pass_by_ref.stderr @@ -1,5 +1,5 @@ -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:52:11 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:53:11 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` @@ -10,107 +10,107 @@ note: the lint level is defined here LL | #![deny(clippy::trivially_copy_pass_by_ref)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:52:20 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:53:20 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:52:29 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:53:29 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:12 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:12 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^^ help: consider passing by value instead: `self` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:22 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:22 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:31 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:31 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:62:40 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:63:40 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:68:16 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:69:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:68:25 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:69:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:68:34 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:69:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:73:35 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:74:35 | LL | fn bad_issue7518(self, other: &Self) {} | ^^^^^ help: consider passing by value instead: `Self` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:86:16 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:87:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:86:25 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:87:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:86:34 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:87:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:93:34 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:94:33 | -LL | fn trait_method(&self, _foo: &Foo); - | ^^^^ help: consider passing by value instead: `Foo` +LL | fn trait_method(&self, foo: &Foo); + | ^^^^ help: consider passing by value instead: `Foo` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:126:21 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:132:21 | LL | fn foo_never(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:132:15 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:138:15 | LL | fn foo(x: &i32) { | ^^^^ help: consider passing by value instead: `i32` -error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte) - --> tests/ui/trivially_copy_pass_by_ref.rs:160:37 +error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) + --> tests/ui/trivially_copy_pass_by_ref.rs:164:36 | -LL | fn _unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { - | ^^^^^^^ help: consider passing by value instead: `u32` +LL | fn unrelated_lifetimes<'a, 'b>(_x: &'a u32, y: &'b u32) -> &'b u32 { + | ^^^^^^^ help: consider passing by value instead: `u32` error: aborting due to 18 previous errors diff --git a/tests/ui/while_let_loop.rs b/tests/ui/while_let_loop.rs index d591ab984cfaf..95062c9f46c7d 100644 --- a/tests/ui/while_let_loop.rs +++ b/tests/ui/while_let_loop.rs @@ -154,3 +154,89 @@ fn issue_5715(mut m: core::cell::RefCell>) { m = core::cell::RefCell::new(Some(x + 1)); } } + +mod issue_362 { + pub fn merge_sorted(xs: Vec, ys: Vec) -> Vec + where + T: PartialOrd, + { + let total_len = xs.len() + ys.len(); + let mut res = Vec::with_capacity(total_len); + let mut ix = xs.into_iter().peekable(); + let mut iy = ys.into_iter().peekable(); + loop { + //~^ while_let_loop + let lt = match (ix.peek(), iy.peek()) { + (Some(x), Some(y)) => x < y, + _ => break, + }; + res.push(if lt { &mut ix } else { &mut iy }.next().unwrap()); + } + res.extend(ix); + res.extend(iy); + res + } +} + +fn let_assign() { + loop { + //~^ while_let_loop + let x = if let Some(y) = Some(3) { + y + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x: u32 = if let Some(y) = Some(3) { + y + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x = if let Some(x) = Some(3) { + x + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x: u32 = if let Some(x) = Some(3) { + x + } else { + break; + }; + if x == 3 { + break; + } + } + + loop { + //~^ while_let_loop + let x = if let Some(x) = Some(2) { + let t = 1; + t + x + } else { + break; + }; + if x == 3 { + break; + } + } +} diff --git a/tests/ui/while_let_loop.stderr b/tests/ui/while_let_loop.stderr index bd482857e675a..ed42628a53e7f 100644 --- a/tests/ui/while_let_loop.stderr +++ b/tests/ui/while_let_loop.stderr @@ -57,7 +57,125 @@ LL | | let (e, l) = match "".split_whitespace().next() { ... | LL | | let _ = (e, l); LL | | } - | |_____^ help: try: `while let Some(word) = "".split_whitespace().next() { .. }` + | |_____^ + | +help: try + | +LL ~ while let Some(word) = "".split_whitespace().next() { +LL + let (e, l) = (word.is_empty(), word.len()); +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:167:9 + | +LL | / loop { +LL | | +LL | | let lt = match (ix.peek(), iy.peek()) { +LL | | (Some(x), Some(y)) => x < y, +... | +LL | | res.push(if lt { &mut ix } else { &mut iy }.next().unwrap()); +LL | | } + | |_________^ + | +help: try + | +LL ~ while let (Some(x), Some(y)) = (ix.peek(), iy.peek()) { +LL + let lt = x < y; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:182:5 + | +LL | / loop { +LL | | +LL | | let x = if let Some(y) = Some(3) { +LL | | y +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(y) = Some(3) { +LL + let x = y; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:194:5 + | +LL | / loop { +LL | | +LL | | let x: u32 = if let Some(y) = Some(3) { +LL | | y +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(y) = Some(3) { +LL + let x: u32 = y; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:206:5 + | +LL | / loop { +LL | | +LL | | let x = if let Some(x) = Some(3) { +LL | | x +... | +LL | | } + | |_____^ help: try: `while let Some(x) = Some(3) { .. }` + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:218:5 + | +LL | / loop { +LL | | +LL | | let x: u32 = if let Some(x) = Some(3) { +LL | | x +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(x) = Some(3) { +LL + let x: u32 = x; +LL + .. +LL + } + | + +error: this loop could be written as a `while let` loop + --> tests/ui/while_let_loop.rs:230:5 + | +LL | / loop { +LL | | +LL | | let x = if let Some(x) = Some(2) { +LL | | let t = 1; +... | +LL | | } + | |_____^ + | +help: try + | +LL ~ while let Some(x) = Some(2) { +LL + let x = { +LL + let t = 1; +LL + t + x +LL + }; +LL + .. +LL + } + | -error: aborting due to 5 previous errors +error: aborting due to 11 previous errors diff --git a/triagebot.toml b/triagebot.toml index 389f22c6a2ca3..16557a4bebb83 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -45,6 +45,7 @@ contributing_url = "https://github.com/rust-lang/rust-clippy/blob/master/CONTRIB users_on_vacation = [ "matthiaskrgr", "Manishearth", + "blyxyas", ] [assign.owners] diff --git a/util/gh-pages/index_template.html b/util/gh-pages/index_template.html index 19dc1ec0b0cc6..865b9523c39e7 100644 --- a/util/gh-pages/index_template.html +++ b/util/gh-pages/index_template.html @@ -50,7 +50,7 @@
{# #}
{# #} -

Clippy Lints

{# #} +

Clippy Lints

{# #}
{# #}