diff --git a/Cargo.lock b/Cargo.lock index 03a4d71f67c3a..0be6de0a2871b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -621,7 +621,7 @@ checksum = "a1d728cc89cf3aee9ff92b05e62b19ee65a02b5702cff7d5a377e32c6ae29d8d" [[package]] name = "clippy" -version = "0.1.93" +version = "0.1.94" dependencies = [ "anstream", "askama", @@ -648,7 +648,7 @@ dependencies = [ [[package]] name = "clippy_config" -version = "0.1.93" +version = "0.1.94" dependencies = [ "clippy_utils", "itertools", @@ -671,7 +671,7 @@ dependencies = [ [[package]] name = "clippy_lints" -version = "0.1.93" +version = "0.1.94" dependencies = [ "arrayvec", "cargo_metadata 0.18.1", @@ -703,7 +703,7 @@ dependencies = [ [[package]] name = "clippy_utils" -version = "0.1.93" +version = "0.1.94" dependencies = [ "arrayvec", "itertools", @@ -1107,7 +1107,7 @@ dependencies = [ [[package]] name = "declare_clippy_lint" -version = "0.1.93" +version = "0.1.94" [[package]] name = "derive-where" diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 78b81b5b74d69..6f666caf306f0 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -6,7 +6,83 @@ document. ## Unreleased / Beta / In Rust Nightly -[e9b7045...master](https://github.com/rust-lang/rust-clippy/compare/e9b7045...master) +[d9fb15c...master](https://github.com/rust-lang/rust-clippy/compare/d9fb15c...master) + +## Rust 1.92 + +Current stable, released 2025-12-11 + +[View all 124 merged pull requests](https://github.com/rust-lang/rust-clippy/pulls?q=merged%3A2025-09-05T18%3A24%3A03Z..2025-10-16T14%3A13%3A43Z+base%3Amaster) + +### New Lints + +* Added [`unnecessary_option_map_or_else`] to `suspicious` + [#14662](https://github.com/rust-lang/rust-clippy/pull/14662) +* Added [`replace_box`] to `perf` + [#14953](https://github.com/rust-lang/rust-clippy/pull/14953) +* Added [`volatile_composites`] to `nursery` + [#15686](https://github.com/rust-lang/rust-clippy/pull/15686) +* Added [`self_only_used_in_recursion`] to `pedantic` + [#14787](https://github.com/rust-lang/rust-clippy/pull/14787) +* Added [`redundant_iter_cloned`] to `perf` + [#15277](https://github.com/rust-lang/rust-clippy/pull/15277) + +### Moves and Deprecations + +* Renamed [`unchecked_duration_subtraction`] to [`unchecked_time_subtraction`] + [#13800](https://github.com/rust-lang/rust-clippy/pull/13800) + +### Enhancements + +* [`mutex_atomic`] and [`mutex_integer`] overhauled to only lint definitions, not uses; added suggestions + and better help messages + [#15632](https://github.com/rust-lang/rust-clippy/pull/15632) +* [`manual_rotate`] now recognizes non-const rotation amounts + [#15402](https://github.com/rust-lang/rust-clippy/pull/15402) +* [`multiple_inherent_impl`] added `inherent-impl-lint-scope` config option (`module`, `file`, + or `crate`) + [#15843](https://github.com/rust-lang/rust-clippy/pull/15843) +* [`use_self`] now checks structs and enums + [#15566](https://github.com/rust-lang/rust-clippy/pull/15566) +* [`while_let_loop`] extended to lint on `loop { let else }` + [#15701](https://github.com/rust-lang/rust-clippy/pull/15701) +* [`mut_mut`] overhauled with structured suggestions and improved documentation + [#15417](https://github.com/rust-lang/rust-clippy/pull/15417) +* [`nonstandard_macro_braces`] now suggests trailing semicolon when needed + [#15593](https://github.com/rust-lang/rust-clippy/pull/15593) +* [`ptr_offset_with_cast`] now respects MSRV when suggesting fix, and lints more cases + [#15613](https://github.com/rust-lang/rust-clippy/pull/15613) +* [`cast_sign_loss`] and [`cast_possible_wrap`] added suggestions using `cast_{un,}signed()` methods + (MSRV 1.87+) + [#15384](https://github.com/rust-lang/rust-clippy/pull/15384) +* [`unchecked_time_subtraction`] extended to include `Duration - Duration` operations + [#13800](https://github.com/rust-lang/rust-clippy/pull/13800) +* [`filter_next`] now suggests replacing `filter().next_back()` with `rfind()` for + `DoubleEndedIterator` + [#15748](https://github.com/rust-lang/rust-clippy/pull/15748) + +### False Positive Fixes + +* [`unnecessary_safety_comment`] fixed FPs with comments above attributes + [#15678](https://github.com/rust-lang/rust-clippy/pull/15678) +* [`manual_unwrap_or`] fixed FP edge case + [#15812](https://github.com/rust-lang/rust-clippy/pull/15812) +* [`needless_continue`] fixed FP when match type is not unit or never + [#15547](https://github.com/rust-lang/rust-clippy/pull/15547) +* [`if_then_some_else_none`] fixed FP when return exists in block expr + [#15783](https://github.com/rust-lang/rust-clippy/pull/15783) +* [`new_without_default`] fixed to copy `#[cfg]` onto `impl Default` and fixed FP on private type + with trait impl + [#15720](https://github.com/rust-lang/rust-clippy/pull/15720) + [#15782](https://github.com/rust-lang/rust-clippy/pull/15782) +* [`question_mark`] fixed FP on variables used after + [#15644](https://github.com/rust-lang/rust-clippy/pull/15644) +* [`needless_return`] fixed FP with `cfg`d code after `return` + [#15669](https://github.com/rust-lang/rust-clippy/pull/15669) +* [`useless_attribute`] fixed FP on `deprecated_in_future` + [#15645](https://github.com/rust-lang/rust-clippy/pull/15645) +* [`double_parens`] fixed FP when macros are involved + [#15420](https://github.com/rust-lang/rust-clippy/pull/15420) ## Rust 1.91 @@ -6280,6 +6356,7 @@ Released 2018-09-13 [`cyclomatic_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cyclomatic_complexity [`dbg_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#dbg_macro [`debug_assert_with_mut_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#debug_assert_with_mut_call +[`decimal_bitwise_operands`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_bitwise_operands [`decimal_literal_representation`]: https://rust-lang.github.io/rust-clippy/master/index.html#decimal_literal_representation [`declare_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#declare_interior_mutable_const [`default_constructed_unit_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#default_constructed_unit_structs @@ -6541,6 +6618,7 @@ Released 2018-09-13 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one [`manual_ignore_case_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ignore_case_cmp +[`manual_ilog2`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ilog2 [`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check @@ -6686,6 +6764,7 @@ Released 2018-09-13 [`needless_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return [`needless_return_with_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return_with_question_mark [`needless_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_splitn +[`needless_type_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_type_cast [`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update [`neg_cmp_op_on_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_cmp_op_on_partial_ord [`neg_multiply`]: https://rust-lang.github.io/rust-clippy/master/index.html#neg_multiply @@ -6765,6 +6844,7 @@ Released 2018-09-13 [`ptr_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [`ptr_cast_constness`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness [`ptr_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_eq +[`ptr_offset_by_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_by_literal [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names [`pub_underscore_fields`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_underscore_fields @@ -7081,6 +7161,7 @@ Released 2018-09-13 [`allow-expect-in-consts`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-consts [`allow-expect-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-expect-in-tests [`allow-indexing-slicing-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-indexing-slicing-in-tests +[`allow-large-stack-frames-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-large-stack-frames-in-tests [`allow-mixed-uninlined-format-args`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-mixed-uninlined-format-args [`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings [`allow-panic-in-tests`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-panic-in-tests diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index fee885d8fa7e9..67078adea2b47 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.1.93" +version = "0.1.94" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/src/tools/clippy/README.md b/src/tools/clippy/README.md index 20a5e997e629b..78498c73ae786 100644 --- a/src/tools/clippy/README.md +++ b/src/tools/clippy/README.md @@ -4,7 +4,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the lint level by category. diff --git a/src/tools/clippy/book/src/README.md b/src/tools/clippy/book/src/README.md index 5d2c3972b060a..c5b264c9f703c 100644 --- a/src/tools/clippy/book/src/README.md +++ b/src/tools/clippy/book/src/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are over 750 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are over 800 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) Lints are divided into categories, each with a default [lint level](https://doc.rust-lang.org/rustc/lints/levels.html). You can choose how diff --git a/src/tools/clippy/book/src/lint_configuration.md b/src/tools/clippy/book/src/lint_configuration.md index 2e185fb3a086e..a1c079898594f 100644 --- a/src/tools/clippy/book/src/lint_configuration.md +++ b/src/tools/clippy/book/src/lint_configuration.md @@ -111,6 +111,16 @@ Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]` * [`indexing_slicing`](https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing) +## `allow-large-stack-frames-in-tests` +Whether functions inside `#[cfg(test)]` modules or test functions should be checked. + +**Default Value:** `true` + +--- +**Affected lints:** +* [`large_stack_frames`](https://rust-lang.github.io/rust-clippy/master/index.html#large_stack_frames) + + ## `allow-mixed-uninlined-format-args` Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)` diff --git a/src/tools/clippy/clippy_config/Cargo.toml b/src/tools/clippy/clippy_config/Cargo.toml index 3f6b26d3334ed..a65fe7bcbda58 100644 --- a/src/tools/clippy/clippy_config/Cargo.toml +++ b/src/tools/clippy/clippy_config/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_config" -version = "0.1.93" +version = "0.1.94" edition = "2024" publish = false diff --git a/src/tools/clippy/clippy_config/src/conf.rs b/src/tools/clippy/clippy_config/src/conf.rs index 2e9cf8e91f7d0..e1d7c1d88eb9a 100644 --- a/src/tools/clippy/clippy_config/src/conf.rs +++ b/src/tools/clippy/clippy_config/src/conf.rs @@ -373,6 +373,9 @@ define_Conf! { /// Whether `indexing_slicing` should be allowed in test functions or `#[cfg(test)]` #[lints(indexing_slicing)] allow_indexing_slicing_in_tests: bool = false, + /// Whether functions inside `#[cfg(test)]` modules or test functions should be checked. + #[lints(large_stack_frames)] + allow_large_stack_frames_in_tests: bool = true, /// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)` #[lints(uninlined_format_args)] allow_mixed_uninlined_format_args: bool = true, diff --git a/src/tools/clippy/clippy_lints/Cargo.toml b/src/tools/clippy/clippy_lints/Cargo.toml index bc97746a1cbae..7a78ef32bf3c2 100644 --- a/src/tools/clippy/clippy_lints/Cargo.toml +++ b/src/tools/clippy/clippy_lints/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_lints" -version = "0.1.93" +version = "0.1.94" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/src/tools/clippy/clippy_lints/src/casts/mod.rs b/src/tools/clippy/clippy_lints/src/casts/mod.rs index 47cc1da0a6e9f..494d6180d3cb1 100644 --- a/src/tools/clippy/clippy_lints/src/casts/mod.rs +++ b/src/tools/clippy/clippy_lints/src/casts/mod.rs @@ -19,6 +19,7 @@ mod fn_to_numeric_cast; mod fn_to_numeric_cast_any; mod fn_to_numeric_cast_with_truncation; mod manual_dangling_ptr; +mod needless_type_cast; mod ptr_as_ptr; mod ptr_cast_constness; mod ref_as_ptr; @@ -813,6 +814,32 @@ declare_clippy_lint! { "casting a primitive method pointer to any integer type" } +declare_clippy_lint! { + /// ### What it does + /// Checks for bindings (constants, statics, or let bindings) that are defined + /// with one numeric type but are consistently cast to a different type in all usages. + /// + /// ### Why is this bad? + /// If a binding is always cast to a different type when used, it would be clearer + /// and more efficient to define it with the target type from the start. + /// + /// ### Example + /// ```no_run + /// const SIZE: u16 = 15; + /// let arr: [u8; SIZE as usize] = [0; SIZE as usize]; + /// ``` + /// + /// Use instead: + /// ```no_run + /// const SIZE: usize = 15; + /// let arr: [u8; SIZE] = [0; SIZE]; + /// ``` + #[clippy::version = "1.93.0"] + pub NEEDLESS_TYPE_CAST, + pedantic, + "binding defined with one type but always cast to another" +} + pub struct Casts { msrv: Msrv, } @@ -851,6 +878,7 @@ impl_lint_pass!(Casts => [ AS_POINTER_UNDERSCORE, MANUAL_DANGLING_PTR, CONFUSING_METHOD_TO_NUMERIC_CAST, + NEEDLESS_TYPE_CAST, ]); impl<'tcx> LateLintPass<'tcx> for Casts { @@ -920,4 +948,8 @@ impl<'tcx> LateLintPass<'tcx> for Casts { cast_slice_different_sizes::check(cx, expr, self.msrv); ptr_cast_constness::check_null_ptr_cast_method(cx, expr); } + + fn check_body(&mut self, cx: &LateContext<'tcx>, body: &rustc_hir::Body<'tcx>) { + needless_type_cast::check(cx, body); + } } diff --git a/src/tools/clippy/clippy_lints/src/casts/needless_type_cast.rs b/src/tools/clippy/clippy_lints/src/casts/needless_type_cast.rs new file mode 100644 index 0000000000000..ca6aa0f87bbf3 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/casts/needless_type_cast.rs @@ -0,0 +1,289 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::visitors::{Descend, for_each_expr, for_each_expr_without_closures}; +use core::ops::ControlFlow; +use rustc_data_structures::fx::FxHashMap; +use rustc_errors::Applicability; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::{BlockCheckMode, Body, Expr, ExprKind, HirId, LetStmt, PatKind, StmtKind, UnsafeSource}; +use rustc_lint::LateContext; +use rustc_middle::ty::{Ty, TypeVisitableExt}; +use rustc_span::Span; + +use super::NEEDLESS_TYPE_CAST; + +struct BindingInfo<'a> { + source_ty: Ty<'a>, + ty_span: Span, +} + +struct UsageInfo<'a> { + cast_to: Option>, + in_generic_context: bool, +} + +pub(super) fn check<'a>(cx: &LateContext<'a>, body: &Body<'a>) { + let mut bindings: FxHashMap> = FxHashMap::default(); + + for_each_expr_without_closures(body.value, |expr| { + match expr.kind { + ExprKind::Block(block, _) => { + for stmt in block.stmts { + if let StmtKind::Let(let_stmt) = stmt.kind { + collect_binding_from_local(cx, let_stmt, &mut bindings); + } + } + }, + ExprKind::Let(let_expr) => { + collect_binding_from_let(cx, let_expr, &mut bindings); + }, + _ => {}, + } + ControlFlow::<()>::Continue(()) + }); + + #[allow(rustc::potential_query_instability)] + let mut binding_vec: Vec<_> = bindings.into_iter().collect(); + binding_vec.sort_by_key(|(_, info)| info.ty_span.lo()); + + for (hir_id, binding_info) in binding_vec { + check_binding_usages(cx, body, hir_id, &binding_info); + } +} + +fn collect_binding_from_let<'a>( + cx: &LateContext<'a>, + let_expr: &rustc_hir::LetExpr<'a>, + bindings: &mut FxHashMap>, +) { + if let_expr.ty.is_none() + || let_expr.span.from_expansion() + || has_generic_return_type(cx, let_expr.init) + || contains_unsafe(let_expr.init) + { + return; + } + + if let PatKind::Binding(_, hir_id, _, _) = let_expr.pat.kind + && let Some(ty_hir) = let_expr.ty + { + let ty = cx.typeck_results().pat_ty(let_expr.pat); + if ty.is_numeric() { + bindings.insert( + hir_id, + BindingInfo { + source_ty: ty, + ty_span: ty_hir.span, + }, + ); + } + } +} + +fn collect_binding_from_local<'a>( + cx: &LateContext<'a>, + let_stmt: &LetStmt<'a>, + bindings: &mut FxHashMap>, +) { + if let_stmt.ty.is_none() + || let_stmt.span.from_expansion() + || let_stmt + .init + .is_some_and(|init| has_generic_return_type(cx, init) || contains_unsafe(init)) + { + return; + } + + if let PatKind::Binding(_, hir_id, _, _) = let_stmt.pat.kind + && let Some(ty_hir) = let_stmt.ty + { + let ty = cx.typeck_results().pat_ty(let_stmt.pat); + if ty.is_numeric() { + bindings.insert( + hir_id, + BindingInfo { + source_ty: ty, + ty_span: ty_hir.span, + }, + ); + } + } +} + +fn contains_unsafe(expr: &Expr<'_>) -> bool { + for_each_expr_without_closures(expr, |e| { + if let ExprKind::Block(block, _) = e.kind + && let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules + { + return ControlFlow::Break(()); + } + ControlFlow::Continue(()) + }) + .is_some() +} + +fn has_generic_return_type(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + match &expr.kind { + ExprKind::Block(block, _) => { + if let Some(tail_expr) = block.expr { + return has_generic_return_type(cx, tail_expr); + } + false + }, + ExprKind::If(_, then_block, else_expr) => { + has_generic_return_type(cx, then_block) || else_expr.is_some_and(|e| has_generic_return_type(cx, e)) + }, + ExprKind::Match(_, arms, _) => arms.iter().any(|arm| has_generic_return_type(cx, arm.body)), + ExprKind::Loop(block, label, ..) => for_each_expr_without_closures(*block, |e| { + match e.kind { + ExprKind::Loop(..) => { + // Unlabeled breaks inside nested loops target the inner loop, not ours + return ControlFlow::Continue(Descend::No); + }, + ExprKind::Break(dest, Some(break_expr)) => { + let targets_this_loop = + dest.label.is_none() || dest.label.map(|l| l.ident) == label.map(|l| l.ident); + if targets_this_loop && has_generic_return_type(cx, break_expr) { + return ControlFlow::Break(()); + } + }, + _ => {}, + } + ControlFlow::Continue(Descend::Yes) + }) + .is_some(), + ExprKind::MethodCall(..) => { + if let Some(def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) { + let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); + let ret_ty = sig.output().skip_binder(); + return ret_ty.has_param(); + } + false + }, + ExprKind::Call(callee, _) => { + if let ExprKind::Path(qpath) = &callee.kind { + let res = cx.qpath_res(qpath, callee.hir_id); + if let Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) = res { + let sig = cx.tcx.fn_sig(def_id).instantiate_identity(); + let ret_ty = sig.output().skip_binder(); + return ret_ty.has_param(); + } + } + false + }, + _ => false, + } +} + +fn is_generic_res(cx: &LateContext<'_>, res: Res) -> bool { + let has_type_params = |def_id| { + cx.tcx + .generics_of(def_id) + .own_params + .iter() + .any(|p| p.kind.is_ty_or_const()) + }; + match res { + Res::Def(DefKind::Fn | DefKind::AssocFn, def_id) => has_type_params(def_id), + // Ctor → Variant → ADT: constructor's parent is variant, variant's parent is the ADT + Res::Def(DefKind::Ctor(..), def_id) => has_type_params(cx.tcx.parent(cx.tcx.parent(def_id))), + _ => false, + } +} + +fn is_cast_in_generic_context<'a>(cx: &LateContext<'a>, cast_expr: &Expr<'a>) -> bool { + let mut current_id = cast_expr.hir_id; + + loop { + let parent_id = cx.tcx.parent_hir_id(current_id); + if parent_id == current_id { + return false; + } + + let parent = cx.tcx.hir_node(parent_id); + + match parent { + rustc_hir::Node::Expr(parent_expr) => { + match &parent_expr.kind { + ExprKind::Closure(_) => return false, + ExprKind::Call(callee, _) => { + if let ExprKind::Path(qpath) = &callee.kind { + let res = cx.qpath_res(qpath, callee.hir_id); + if is_generic_res(cx, res) { + return true; + } + } + }, + ExprKind::MethodCall(..) => { + if let Some(def_id) = cx.typeck_results().type_dependent_def_id(parent_expr.hir_id) + && cx + .tcx + .generics_of(def_id) + .own_params + .iter() + .any(|p| p.kind.is_ty_or_const()) + { + return true; + } + }, + _ => {}, + } + current_id = parent_id; + }, + _ => return false, + } + } +} + +fn check_binding_usages<'a>(cx: &LateContext<'a>, body: &Body<'a>, hir_id: HirId, binding_info: &BindingInfo<'a>) { + let mut usages = Vec::new(); + + for_each_expr(cx, body.value, |expr| { + if let ExprKind::Path(ref qpath) = expr.kind + && !expr.span.from_expansion() + && let Res::Local(id) = cx.qpath_res(qpath, expr.hir_id) + && id == hir_id + { + let parent_id = cx.tcx.parent_hir_id(expr.hir_id); + let parent = cx.tcx.hir_node(parent_id); + + let usage = if let rustc_hir::Node::Expr(parent_expr) = parent + && let ExprKind::Cast(..) = parent_expr.kind + && !parent_expr.span.from_expansion() + { + UsageInfo { + cast_to: Some(cx.typeck_results().expr_ty(parent_expr)), + in_generic_context: is_cast_in_generic_context(cx, parent_expr), + } + } else { + UsageInfo { + cast_to: None, + in_generic_context: false, + } + }; + usages.push(usage); + } + ControlFlow::<()>::Continue(()) + }); + + let Some(first_target) = usages + .first() + .and_then(|u| u.cast_to) + .filter(|&t| t != binding_info.source_ty) + .filter(|&t| usages.iter().all(|u| u.cast_to == Some(t) && !u.in_generic_context)) + else { + return; + }; + + span_lint_and_sugg( + cx, + NEEDLESS_TYPE_CAST, + binding_info.ty_span, + format!( + "this binding is defined as `{}` but is always cast to `{}`", + binding_info.source_ty, first_target + ), + "consider defining it as", + first_target.to_string(), + Applicability::MaybeIncorrect, + ); +} diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index 4a350dca2993b..87d75234ebc01 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/clippy_lints/src/declared_lints.rs @@ -70,6 +70,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::casts::FN_TO_NUMERIC_CAST_ANY_INFO, crate::casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION_INFO, crate::casts::MANUAL_DANGLING_PTR_INFO, + crate::casts::NEEDLESS_TYPE_CAST_INFO, crate::casts::PTR_AS_PTR_INFO, crate::casts::PTR_CAST_CONSTNESS_INFO, crate::casts::REF_AS_PTR_INFO, @@ -245,8 +246,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::large_stack_arrays::LARGE_STACK_ARRAYS_INFO, crate::large_stack_frames::LARGE_STACK_FRAMES_INFO, crate::legacy_numeric_constants::LEGACY_NUMERIC_CONSTANTS_INFO, + crate::len_without_is_empty::LEN_WITHOUT_IS_EMPTY_INFO, crate::len_zero::COMPARISON_TO_EMPTY_INFO, - crate::len_zero::LEN_WITHOUT_IS_EMPTY_INFO, crate::len_zero::LEN_ZERO_INFO, crate::let_if_seq::USELESS_LET_IF_SEQ_INFO, crate::let_underscore::LET_UNDERSCORE_FUTURE_INFO, @@ -300,6 +301,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::manual_float_methods::MANUAL_IS_INFINITE_INFO, crate::manual_hash_one::MANUAL_HASH_ONE_INFO, crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO, + crate::manual_ilog2::MANUAL_ILOG2_INFO, crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO, crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO, crate::manual_let_else::MANUAL_LET_ELSE_INFO, @@ -444,6 +446,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::methods::OR_THEN_UNWRAP_INFO, crate::methods::PATH_BUF_PUSH_OVERWRITE_INFO, crate::methods::PATH_ENDS_WITH_EXT_INFO, + crate::methods::PTR_OFFSET_BY_LITERAL_INFO, crate::methods::PTR_OFFSET_WITH_CAST_INFO, crate::methods::RANGE_ZIP_WITH_LEN_INFO, crate::methods::READONLY_WRITE_LOCK_INFO, @@ -578,6 +581,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::operators::ASSIGN_OP_PATTERN_INFO, crate::operators::BAD_BIT_MASK_INFO, crate::operators::CMP_OWNED_INFO, + crate::operators::DECIMAL_BITWISE_OPERANDS_INFO, crate::operators::DOUBLE_COMPARISONS_INFO, crate::operators::DURATION_SUBSEC_INFO, crate::operators::EQ_OP_INFO, diff --git a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs index f010d17917f9e..6e62e983d2f31 100644 --- a/src/tools/clippy/clippy_lints/src/deprecated_lints.rs +++ b/src/tools/clippy/clippy_lints/src/deprecated_lints.rs @@ -34,7 +34,7 @@ declare_with_version! { DEPRECATED(DEPRECATED_VERSION) = [ #[clippy::version = "pre 1.29.0"] ("clippy::should_assert_eq", "`assert!(a == b)` can now print the values the same way `assert_eq!(a, b) can"), #[clippy::version = "1.91.0"] - ("clippy::string_to_string", "`clippy:implicit_clone` covers those cases"), + ("clippy::string_to_string", "`clippy::implicit_clone` covers those cases"), #[clippy::version = "pre 1.29.0"] ("clippy::unsafe_vector_initialization", "the suggested alternative could be substantially slower"), #[clippy::version = "pre 1.29.0"] diff --git a/src/tools/clippy/clippy_lints/src/entry.rs b/src/tools/clippy/clippy_lints/src/entry.rs index a5ec6777b4344..bdfe2e49e66e1 100644 --- a/src/tools/clippy/clippy_lints/src/entry.rs +++ b/src/tools/clippy/clippy_lints/src/entry.rs @@ -3,11 +3,12 @@ use clippy_utils::source::{reindent_multiline, snippet_indent, snippet_with_appl use clippy_utils::ty::is_copy; use clippy_utils::visitors::for_each_expr; use clippy_utils::{ - SpanlessEq, can_move_expr_to_closure_no_visit, higher, is_expr_final_block_expr, is_expr_used_or_unified, - peel_hir_expr_while, + SpanlessEq, can_move_expr_to_closure_no_visit, desugar_await, higher, is_expr_final_block_expr, + is_expr_used_or_unified, paths, peel_hir_expr_while, }; use core::fmt::{self, Write}; use rustc_errors::Applicability; +use rustc_hir::def_id::DefId; use rustc_hir::hir_id::HirIdSet; use rustc_hir::intravisit::{Visitor, walk_body, walk_expr}; use rustc_hir::{Block, Expr, ExprKind, HirId, Pat, Stmt, StmtKind, UnOp}; @@ -382,6 +383,8 @@ struct InsertSearcher<'cx, 'tcx> { loops: Vec, /// Local variables created in the expression. These don't need to be captured. locals: HirIdSet, + /// Whether the map is a non-async-aware `MutexGuard`. + map_is_mutex_guard: bool, } impl<'tcx> InsertSearcher<'_, 'tcx> { /// Visit the expression as a branch in control flow. Multiple insert calls can be used, but @@ -524,15 +527,22 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> { ExprKind::If(cond_expr, then_expr, Some(else_expr)) => { self.is_single_insert = false; self.visit_non_tail_expr(cond_expr); - // Each branch may contain it's own insert expression. + // Each branch may contain its own insert expression. let mut is_map_used = self.visit_cond_arm(then_expr); is_map_used |= self.visit_cond_arm(else_expr); self.is_map_used = is_map_used; }, ExprKind::Match(scrutinee_expr, arms, _) => { + // If the map is a non-async-aware `MutexGuard` and + // `.await` expression appears alongside map insertion in the same `then` or `else` block, + // we cannot suggest using `entry()` because it would hold the lock across the await point, + // triggering `await_holding_lock` and risking deadlock. + if self.map_is_mutex_guard && desugar_await(expr).is_some() { + self.can_use_entry = false; + } self.is_single_insert = false; self.visit_non_tail_expr(scrutinee_expr); - // Each branch may contain it's own insert expression. + // Each branch may contain its own insert expression. let mut is_map_used = self.is_map_used; for arm in arms { self.visit_pat(arm.pat); @@ -725,16 +735,32 @@ fn find_insert_calls<'tcx>( edits: Vec::new(), loops: Vec::new(), locals: HirIdSet::default(), + map_is_mutex_guard: false, }; + // Check if the map is a non-async-aware `MutexGuard` + if let rustc_middle::ty::Adt(adt, _) = cx.typeck_results().expr_ty(contains_expr.map).kind() + && is_mutex_guard(cx, adt.did()) + { + s.map_is_mutex_guard = true; + } + s.visit_expr(expr); - let allow_insert_closure = s.allow_insert_closure; - let is_single_insert = s.is_single_insert; + if !s.can_use_entry { + return None; + } + let is_key_used_and_no_copy = s.is_key_used && !is_copy(cx, cx.typeck_results().expr_ty(contains_expr.key)); - let edits = s.edits; - s.can_use_entry.then_some(InsertSearchResults { - edits, - allow_insert_closure, - is_single_insert, + Some(InsertSearchResults { + edits: s.edits, + allow_insert_closure: s.allow_insert_closure, + is_single_insert: s.is_single_insert, is_key_used_and_no_copy, }) } + +fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool { + match cx.tcx.get_diagnostic_name(def_id) { + Some(name) => matches!(name, sym::MutexGuard | sym::RwLockReadGuard | sym::RwLockWriteGuard), + None => paths::PARKING_LOT_GUARDS.iter().any(|guard| guard.matches(cx, def_id)), + } +} diff --git a/src/tools/clippy/clippy_lints/src/large_stack_frames.rs b/src/tools/clippy/clippy_lints/src/large_stack_frames.rs index 5ed948c02bbcc..6b0080d04c447 100644 --- a/src/tools/clippy/clippy_lints/src/large_stack_frames.rs +++ b/src/tools/clippy/clippy_lints/src/large_stack_frames.rs @@ -2,15 +2,16 @@ use std::{fmt, ops}; use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::fn_has_unsatisfiable_preds; -use clippy_utils::source::SpanRangeExt; +use clippy_utils::source::{HasSession, SpanRangeExt}; +use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, is_in_test}; +use rustc_errors::Diag; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::FnKind; use rustc_hir::{Body, FnDecl}; use rustc_lexer::is_ident; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; -use rustc_span::Span; +use rustc_span::{Span, SyntaxContext}; declare_clippy_lint! { /// ### What it does @@ -83,12 +84,14 @@ declare_clippy_lint! { pub struct LargeStackFrames { maximum_allowed_size: u64, + allow_large_stack_frames_in_tests: bool, } impl LargeStackFrames { pub fn new(conf: &'static Conf) -> Self { Self { maximum_allowed_size: conf.stack_size_threshold, + allow_large_stack_frames_in_tests: conf.allow_large_stack_frames_in_tests, } } } @@ -152,67 +155,122 @@ impl<'tcx> LateLintPass<'tcx> for LargeStackFrames { let mir = cx.tcx.optimized_mir(def_id); let typing_env = mir.typing_env(cx.tcx); - let sizes_of_locals = || { - mir.local_decls.iter().filter_map(|local| { + let sizes_of_locals = mir + .local_decls + .iter() + .filter_map(|local| { let layout = cx.tcx.layout_of(typing_env.as_query_input(local.ty)).ok()?; Some((local, layout.size.bytes())) }) - }; + .collect::>(); - let frame_size = sizes_of_locals().fold(Space::Used(0), |sum, (_, size)| sum + size); + let frame_size = sizes_of_locals + .iter() + .fold(Space::Used(0), |sum, (_, size)| sum + *size); let limit = self.maximum_allowed_size; if frame_size.exceeds_limit(limit) { // Point at just the function name if possible, because lints that span // the entire body and don't have to are less legible. - let fn_span = match fn_kind { - FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span, - FnKind::Closure => entire_fn_span, + let (fn_span, fn_name) = match fn_kind { + FnKind::ItemFn(ident, _, _) => (ident.span, format!("function `{}`", ident.name)), + FnKind::Method(ident, _) => (ident.span, format!("method `{}`", ident.name)), + FnKind::Closure => (entire_fn_span, "closure".to_string()), }; + // Don't lint inside tests if configured to not do so. + if self.allow_large_stack_frames_in_tests && is_in_test(cx.tcx, cx.tcx.local_def_id_to_hir_id(local_def_id)) + { + return; + } + + let explain_lint = |diag: &mut Diag<'_, ()>, ctxt: SyntaxContext| { + // Point out the largest individual contribution to this size, because + // it is the most likely to be unintentionally large. + if let Some((local, size)) = sizes_of_locals.iter().max_by_key(|&(_, size)| size) + && let local_span = local.source_info.span + && local_span.ctxt() == ctxt + { + let size = Space::Used(*size); // pluralizes for us + let ty = local.ty; + + // TODO: Is there a cleaner, robust way to ask this question? + // The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data", + // and that doesn't get us the true name in scope rather than the span text either. + if let Some(name) = local_span.get_source_text(cx) + && is_ident(&name) + { + // If the local is an ordinary named variable, + // print its name rather than relying solely on the span. + diag.span_label( + local_span, + format!("`{name}` is the largest part, at {size} for type `{ty}`"), + ); + } else { + diag.span_label( + local_span, + format!("this is the largest part, at {size} for type `{ty}`"), + ); + } + } + + // Explain why we are linting this and not other functions. + diag.note(format!( + "{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}" + )); + + // Explain why the user should care, briefly. + diag.note_once( + "allocating large amounts of stack space can overflow the stack \ + and cause the program to abort", + ); + }; + + if fn_span.from_expansion() { + // Don't lint on the main function generated by `--test` target + if cx.sess().is_test_crate() && is_entrypoint_fn(cx, local_def_id.to_def_id()) { + return; + } + + let is_from_external_macro = fn_span.in_external_macro(cx.sess().source_map()); + span_lint_and_then( + cx, + LARGE_STACK_FRAMES, + fn_span.source_callsite(), + format!( + "{} generated by this macro may allocate a lot of stack space", + if is_from_external_macro { + cx.tcx.def_descr(local_def_id.into()) + } else { + fn_name.as_str() + } + ), + |diag| { + if is_from_external_macro { + return; + } + + diag.span_label( + fn_span, + format!( + "this {} has a stack frame size of {frame_size}", + cx.tcx.def_descr(local_def_id.into()) + ), + ); + + explain_lint(diag, fn_span.ctxt()); + }, + ); + return; + } + span_lint_and_then( cx, LARGE_STACK_FRAMES, fn_span, format!("this function may allocate {frame_size} on the stack"), |diag| { - // Point out the largest individual contribution to this size, because - // it is the most likely to be unintentionally large. - if let Some((local, size)) = sizes_of_locals().max_by_key(|&(_, size)| size) { - let local_span: Span = local.source_info.span; - let size = Space::Used(size); // pluralizes for us - let ty = local.ty; - - // TODO: Is there a cleaner, robust way to ask this question? - // The obvious `LocalDecl::is_user_variable()` panics on "unwrapping cross-crate data", - // and that doesn't get us the true name in scope rather than the span text either. - if let Some(name) = local_span.get_source_text(cx) - && is_ident(&name) - { - // If the local is an ordinary named variable, - // print its name rather than relying solely on the span. - diag.span_label( - local_span, - format!("`{name}` is the largest part, at {size} for type `{ty}`"), - ); - } else { - diag.span_label( - local_span, - format!("this is the largest part, at {size} for type `{ty}`"), - ); - } - } - - // Explain why we are linting this and not other functions. - diag.note(format!( - "{frame_size} is larger than Clippy's configured `stack-size-threshold` of {limit}" - )); - - // Explain why the user should care, briefly. - diag.note_once( - "allocating large amounts of stack space can overflow the stack \ - and cause the program to abort", - ); + explain_lint(diag, SyntaxContext::root()); }, ); } diff --git a/src/tools/clippy/clippy_lints/src/len_without_is_empty.rs b/src/tools/clippy/clippy_lints/src/len_without_is_empty.rs new file mode 100644 index 0000000000000..1d219d7c3b74d --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/len_without_is_empty.rs @@ -0,0 +1,342 @@ +use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +use clippy_utils::res::MaybeDef; +use clippy_utils::{fulfill_or_allowed, get_parent_as_impl, sym}; +use rustc_hir::def::Res; +use rustc_hir::def_id::{DefId, DefIdSet}; +use rustc_hir::{ + FnRetTy, GenericArg, GenericBound, HirId, ImplItem, ImplItemKind, ImplicitSelfKind, Item, ItemKind, Mutability, + Node, OpaqueTyOrigin, PathSegment, PrimTy, QPath, TraitItemId, TyKind, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, FnSig, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::symbol::kw; +use rustc_span::{Ident, Span, Symbol}; +use rustc_trait_selection::traits::supertrait_def_ids; + +declare_clippy_lint! { + /// ### What it does + /// Checks for items that implement `.len()` but not + /// `.is_empty()`. + /// + /// ### Why is this bad? + /// It is good custom to have both methods, because for + /// some data structures, asking about the length will be a costly operation, + /// whereas `.is_empty()` can usually answer in constant time. Also it used to + /// lead to false positives on the [`len_zero`](#len_zero) lint – currently that + /// lint will ignore such entities. + /// + /// ### Example + /// ```ignore + /// impl X { + /// pub fn len(&self) -> usize { + /// .. + /// } + /// } + /// ``` + #[clippy::version = "pre 1.29.0"] + pub LEN_WITHOUT_IS_EMPTY, + style, + "traits or impls with a public `len` method but no corresponding `is_empty` method" +} + +declare_lint_pass!(LenWithoutIsEmpty => [LEN_WITHOUT_IS_EMPTY]); + +impl<'tcx> LateLintPass<'tcx> for LenWithoutIsEmpty { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { + if let ItemKind::Trait(_, _, _, ident, _, _, trait_items) = item.kind + && !item.span.from_expansion() + { + check_trait_items(cx, item, ident, trait_items); + } + } + + fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { + if item.ident.name == sym::len + && let ImplItemKind::Fn(sig, _) = &item.kind + && sig.decl.implicit_self.has_implicit_self() + && sig.decl.inputs.len() == 1 + && cx.effective_visibilities.is_exported(item.owner_id.def_id) + && matches!(sig.decl.output, FnRetTy::Return(_)) + && let Some(imp) = get_parent_as_impl(cx.tcx, item.hir_id()) + && imp.of_trait.is_none() + && let TyKind::Path(ty_path) = &imp.self_ty.kind + && let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id() + && let Some(local_id) = ty_id.as_local() + && let ty_hir_id = cx.tcx.local_def_id_to_hir_id(local_id) + && let Some(output) = LenOutput::new(cx, cx.tcx.fn_sig(item.owner_id).instantiate_identity().skip_binder()) + { + let (name, kind) = match cx.tcx.hir_node(ty_hir_id) { + Node::ForeignItem(x) => (x.ident.name, "extern type"), + Node::Item(x) => match x.kind { + ItemKind::Struct(ident, ..) => (ident.name, "struct"), + ItemKind::Enum(ident, ..) => (ident.name, "enum"), + ItemKind::Union(ident, ..) => (ident.name, "union"), + _ => (x.kind.ident().unwrap().name, "type"), + }, + _ => return, + }; + check_for_is_empty( + cx, + sig.span, + sig.decl.implicit_self, + output, + ty_id, + name, + kind, + item.hir_id(), + ty_hir_id, + ); + } + } +} + +fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, ident: Ident, trait_items: &[TraitItemId]) { + fn is_named_self(cx: &LateContext<'_>, item: TraitItemId, name: Symbol) -> bool { + cx.tcx.item_name(item.owner_id) == name + && matches!( + cx.tcx.fn_arg_idents(item.owner_id), + [Some(Ident { + name: kw::SelfLower, + .. + })], + ) + } + + // fill the set with current and super traits + fn fill_trait_set(traitt: DefId, set: &mut DefIdSet, cx: &LateContext<'_>) { + if set.insert(traitt) { + for supertrait in supertrait_def_ids(cx.tcx, traitt) { + fill_trait_set(supertrait, set, cx); + } + } + } + + if cx.effective_visibilities.is_exported(visited_trait.owner_id.def_id) + && trait_items.iter().any(|&i| is_named_self(cx, i, sym::len)) + { + let mut current_and_super_traits = DefIdSet::default(); + fill_trait_set(visited_trait.owner_id.to_def_id(), &mut current_and_super_traits, cx); + let is_empty_method_found = current_and_super_traits + .items() + .flat_map(|&i| cx.tcx.associated_items(i).filter_by_name_unhygienic(sym::is_empty)) + .any(|i| i.is_method() && cx.tcx.fn_sig(i.def_id).skip_binder().inputs().skip_binder().len() == 1); + + if !is_empty_method_found { + span_lint( + cx, + LEN_WITHOUT_IS_EMPTY, + visited_trait.span, + format!( + "trait `{}` has a `len` method but no (possibly inherited) `is_empty` method", + ident.name + ), + ); + } + } +} + +fn extract_future_output<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx PathSegment<'tcx>> { + if let ty::Alias(_, alias_ty) = ty.kind() + && let Some(Node::OpaqueTy(opaque)) = cx.tcx.hir_get_if_local(alias_ty.def_id) + && let OpaqueTyOrigin::AsyncFn { .. } = opaque.origin + && let [GenericBound::Trait(trait_ref)] = &opaque.bounds + && let Some(segment) = trait_ref.trait_ref.path.segments.last() + && let Some(generic_args) = segment.args + && let [constraint] = generic_args.constraints + && let Some(ty) = constraint.ty() + && let TyKind::Path(QPath::Resolved(_, path)) = ty.kind + && let [segment] = path.segments + { + return Some(segment); + } + + None +} + +fn is_first_generic_integral<'tcx>(segment: &'tcx PathSegment<'tcx>) -> bool { + if let Some(generic_args) = segment.args + && let [GenericArg::Type(ty), ..] = &generic_args.args + && let TyKind::Path(QPath::Resolved(_, path)) = ty.kind + && let [segment, ..] = &path.segments + && matches!(segment.res, Res::PrimTy(PrimTy::Uint(_) | PrimTy::Int(_))) + { + true + } else { + false + } +} + +#[derive(Debug, Clone, Copy)] +enum LenOutput { + Integral, + Option(DefId), + Result(DefId), +} + +impl LenOutput { + fn new<'tcx>(cx: &LateContext<'tcx>, sig: FnSig<'tcx>) -> Option { + if let Some(segment) = extract_future_output(cx, sig.output()) { + let res = segment.res; + + if matches!(res, Res::PrimTy(PrimTy::Uint(_) | PrimTy::Int(_))) { + return Some(Self::Integral); + } + + if let Res::Def(_, def_id) = res + && let Some(res) = match cx.tcx.get_diagnostic_name(def_id) { + Some(sym::Option) => Some(Self::Option(def_id)), + Some(sym::Result) => Some(Self::Result(def_id)), + _ => None, + } + && is_first_generic_integral(segment) + { + return Some(res); + } + + return None; + } + + match *sig.output().kind() { + ty::Int(_) | ty::Uint(_) => Some(Self::Integral), + ty::Adt(adt, subs) => match cx.tcx.get_diagnostic_name(adt.did()) { + Some(sym::Option) => subs.type_at(0).is_integral().then(|| Self::Option(adt.did())), + Some(sym::Result) => subs.type_at(0).is_integral().then(|| Self::Result(adt.did())), + _ => None, + }, + _ => None, + } + } + + fn matches_is_empty_output<'tcx>(self, cx: &LateContext<'tcx>, is_empty_output: Ty<'tcx>) -> bool { + if let Some(segment) = extract_future_output(cx, is_empty_output) { + return match (self, segment.res) { + (_, Res::PrimTy(PrimTy::Bool)) => true, + (Self::Option(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Option, def_id) => true, + (Self::Result(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Result, def_id) => true, + _ => false, + }; + } + + match (self, is_empty_output.kind()) { + (_, &ty::Bool) => true, + (Self::Option(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(), + (Self::Result(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(), + _ => false, + } + } +} + +/// The expected signature of `is_empty`, based on that of `len` +fn expected_is_empty_sig(len_output: LenOutput, len_self_kind: ImplicitSelfKind) -> String { + let self_ref = match len_self_kind { + ImplicitSelfKind::RefImm => "&", + ImplicitSelfKind::RefMut => "&(mut) ", + _ => "", + }; + match len_output { + LenOutput::Integral => format!("expected signature: `({self_ref}self) -> bool`"), + LenOutput::Option(_) => { + format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Option") + }, + LenOutput::Result(..) => { + format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Result") + }, + } +} + +/// Checks if the given signature matches the expectations for `is_empty` +fn check_is_empty_sig<'tcx>( + cx: &LateContext<'tcx>, + is_empty_sig: FnSig<'tcx>, + len_self_kind: ImplicitSelfKind, + len_output: LenOutput, +) -> bool { + if let [is_empty_self_arg, is_empty_output] = &**is_empty_sig.inputs_and_output + && len_output.matches_is_empty_output(cx, *is_empty_output) + { + match (is_empty_self_arg.kind(), len_self_kind) { + // if `len` takes `&self`, `is_empty` should do so as well + (ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::RefImm) + // if `len` takes `&mut self`, `is_empty` may take that _or_ `&self` (#16190) + | (ty::Ref(_, _, Mutability::Mut | Mutability::Not), ImplicitSelfKind::RefMut) => true, + // if len takes `self`, `is_empty` should do so as well + // XXX: we might want to relax this to allow `&self` and `&mut self` + (_, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut) if !is_empty_self_arg.is_ref() => true, + _ => false, + } + } else { + false + } +} + +/// Checks if the given type has an `is_empty` method with the appropriate signature. +#[expect(clippy::too_many_arguments)] +fn check_for_is_empty( + cx: &LateContext<'_>, + len_span: Span, + len_self_kind: ImplicitSelfKind, + len_output: LenOutput, + impl_ty: DefId, + item_name: Symbol, + item_kind: &str, + len_method_hir_id: HirId, + ty_decl_hir_id: HirId, +) { + // Implementor may be a type alias, in which case we need to get the `DefId` of the aliased type to + // find the correct inherent impls. + let impl_ty = if let Some(adt) = cx.tcx.type_of(impl_ty).skip_binder().ty_adt_def() { + adt.did() + } else { + return; + }; + + let is_empty = cx + .tcx + .inherent_impls(impl_ty) + .iter() + .flat_map(|&id| cx.tcx.associated_items(id).filter_by_name_unhygienic(sym::is_empty)) + .find(|item| item.is_fn()); + + let (msg, is_empty_span, is_empty_expected_sig) = match is_empty { + None => ( + format!("{item_kind} `{item_name}` has a public `len` method, but no `is_empty` method"), + None, + None, + ), + Some(is_empty) if !cx.effective_visibilities.is_exported(is_empty.def_id.expect_local()) => ( + format!("{item_kind} `{item_name}` has a public `len` method, but a private `is_empty` method"), + Some(cx.tcx.def_span(is_empty.def_id)), + None, + ), + Some(is_empty) + if !(is_empty.is_method() + && check_is_empty_sig( + cx, + cx.tcx.fn_sig(is_empty.def_id).instantiate_identity().skip_binder(), + len_self_kind, + len_output, + )) => + { + ( + format!( + "{item_kind} `{item_name}` has a public `len` method, but the `is_empty` method has an unexpected signature", + ), + Some(cx.tcx.def_span(is_empty.def_id)), + Some(expected_is_empty_sig(len_output, len_self_kind)), + ) + }, + Some(_) => return, + }; + + if !fulfill_or_allowed(cx, LEN_WITHOUT_IS_EMPTY, [len_method_hir_id, ty_decl_hir_id]) { + span_lint_and_then(cx, LEN_WITHOUT_IS_EMPTY, len_span, msg, |db| { + if let Some(span) = is_empty_span { + db.span_note(span, "`is_empty` defined here"); + } + if let Some(expected_sig) = is_empty_expected_sig { + db.note(expected_sig); + } + }); + } +} diff --git a/src/tools/clippy/clippy_lints/src/len_zero.rs b/src/tools/clippy/clippy_lints/src/len_zero.rs index 877bd34a732bc..2e576da38b894 100644 --- a/src/tools/clippy/clippy_lints/src/len_zero.rs +++ b/src/tools/clippy/clippy_lints/src/len_zero.rs @@ -1,27 +1,20 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::msrvs::Msrv; use clippy_utils::res::{MaybeDef, MaybeTypeckRes}; use clippy_utils::source::{SpanRangeExt, snippet_with_context}; use clippy_utils::sugg::{Sugg, has_enclosing_paren}; use clippy_utils::ty::implements_trait; -use clippy_utils::{fulfill_or_allowed, get_parent_as_impl, parent_item_name, peel_ref_operators, sym}; +use clippy_utils::{parent_item_name, peel_ref_operators, sym}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::def::Res; -use rustc_hir::def_id::{DefId, DefIdSet}; -use rustc_hir::{ - BinOpKind, Expr, ExprKind, FnRetTy, GenericArg, GenericBound, HirId, ImplItem, ImplItemKind, ImplicitSelfKind, - Item, ItemKind, Mutability, Node, OpaqueTyOrigin, PatExprKind, PatKind, PathSegment, PrimTy, QPath, RustcVersion, - StabilityLevel, StableSince, TraitItemId, TyKind, -}; +use rustc_hir::def_id::DefId; +use rustc_hir::{BinOpKind, Expr, ExprKind, PatExprKind, PatKind, RustcVersion, StabilityLevel, StableSince}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{self, FnSig, Ty}; +use rustc_middle::ty::{self, Ty}; use rustc_session::impl_lint_pass; use rustc_span::source_map::Spanned; -use rustc_span::symbol::kw; -use rustc_span::{Ident, Span, Symbol}; -use rustc_trait_selection::traits::supertrait_def_ids; +use rustc_span::{Span, Symbol}; declare_clippy_lint! { /// ### What it does @@ -58,32 +51,6 @@ declare_clippy_lint! { "checking `.len() == 0` or `.len() > 0` (or similar) when `.is_empty()` could be used instead" } -declare_clippy_lint! { - /// ### What it does - /// Checks for items that implement `.len()` but not - /// `.is_empty()`. - /// - /// ### Why is this bad? - /// It is good custom to have both methods, because for - /// some data structures, asking about the length will be a costly operation, - /// whereas `.is_empty()` can usually answer in constant time. Also it used to - /// lead to false positives on the [`len_zero`](#len_zero) lint – currently that - /// lint will ignore such entities. - /// - /// ### Example - /// ```ignore - /// impl X { - /// pub fn len(&self) -> usize { - /// .. - /// } - /// } - /// ``` - #[clippy::version = "pre 1.29.0"] - pub LEN_WITHOUT_IS_EMPTY, - style, - "traits or impls with a public `len` method but no corresponding `is_empty` method" -} - declare_clippy_lint! { /// ### What it does /// Checks for comparing to an empty slice such as `""` or `[]`, @@ -126,7 +93,7 @@ pub struct LenZero { msrv: Msrv, } -impl_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]); +impl_lint_pass!(LenZero => [LEN_ZERO, COMPARISON_TO_EMPTY]); impl LenZero { pub fn new(conf: &'static Conf) -> Self { @@ -135,54 +102,6 @@ impl LenZero { } impl<'tcx> LateLintPass<'tcx> for LenZero { - fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { - if let ItemKind::Trait(_, _, _, ident, _, _, trait_items) = item.kind - && !item.span.from_expansion() - { - check_trait_items(cx, item, ident, trait_items); - } - } - - fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) { - if item.ident.name == sym::len - && let ImplItemKind::Fn(sig, _) = &item.kind - && sig.decl.implicit_self.has_implicit_self() - && sig.decl.inputs.len() == 1 - && cx.effective_visibilities.is_exported(item.owner_id.def_id) - && matches!(sig.decl.output, FnRetTy::Return(_)) - && let Some(imp) = get_parent_as_impl(cx.tcx, item.hir_id()) - && imp.of_trait.is_none() - && let TyKind::Path(ty_path) = &imp.self_ty.kind - && let Some(ty_id) = cx.qpath_res(ty_path, imp.self_ty.hir_id).opt_def_id() - && let Some(local_id) = ty_id.as_local() - && let ty_hir_id = cx.tcx.local_def_id_to_hir_id(local_id) - && let Some(output) = - parse_len_output(cx, cx.tcx.fn_sig(item.owner_id).instantiate_identity().skip_binder()) - { - let (name, kind) = match cx.tcx.hir_node(ty_hir_id) { - Node::ForeignItem(x) => (x.ident.name, "extern type"), - Node::Item(x) => match x.kind { - ItemKind::Struct(ident, ..) => (ident.name, "struct"), - ItemKind::Enum(ident, ..) => (ident.name, "enum"), - ItemKind::Union(ident, ..) => (ident.name, "union"), - _ => (x.kind.ident().unwrap().name, "type"), - }, - _ => return, - }; - check_for_is_empty( - cx, - sig.span, - sig.decl.implicit_self, - output, - ty_id, - name, - kind, - item.hir_id(), - ty_hir_id, - ); - } - } - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let ExprKind::Let(lt) = expr.kind && match lt.pat.kind { @@ -356,256 +275,6 @@ fn span_without_enclosing_paren(cx: &LateContext<'_>, span: Span) -> Span { } } -fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, ident: Ident, trait_items: &[TraitItemId]) { - fn is_named_self(cx: &LateContext<'_>, item: TraitItemId, name: Symbol) -> bool { - cx.tcx.item_name(item.owner_id) == name - && matches!( - cx.tcx.fn_arg_idents(item.owner_id), - [Some(Ident { - name: kw::SelfLower, - .. - })], - ) - } - - // fill the set with current and super traits - fn fill_trait_set(traitt: DefId, set: &mut DefIdSet, cx: &LateContext<'_>) { - if set.insert(traitt) { - for supertrait in supertrait_def_ids(cx.tcx, traitt) { - fill_trait_set(supertrait, set, cx); - } - } - } - - if cx.effective_visibilities.is_exported(visited_trait.owner_id.def_id) - && trait_items.iter().any(|&i| is_named_self(cx, i, sym::len)) - { - let mut current_and_super_traits = DefIdSet::default(); - fill_trait_set(visited_trait.owner_id.to_def_id(), &mut current_and_super_traits, cx); - let is_empty_method_found = current_and_super_traits - .items() - .flat_map(|&i| cx.tcx.associated_items(i).filter_by_name_unhygienic(sym::is_empty)) - .any(|i| i.is_method() && cx.tcx.fn_sig(i.def_id).skip_binder().inputs().skip_binder().len() == 1); - - if !is_empty_method_found { - span_lint( - cx, - LEN_WITHOUT_IS_EMPTY, - visited_trait.span, - format!( - "trait `{}` has a `len` method but no (possibly inherited) `is_empty` method", - ident.name - ), - ); - } - } -} - -#[derive(Debug, Clone, Copy)] -enum LenOutput { - Integral, - Option(DefId), - Result(DefId), -} - -fn extract_future_output<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<&'tcx PathSegment<'tcx>> { - if let ty::Alias(_, alias_ty) = ty.kind() - && let Some(Node::OpaqueTy(opaque)) = cx.tcx.hir_get_if_local(alias_ty.def_id) - && let OpaqueTyOrigin::AsyncFn { .. } = opaque.origin - && let [GenericBound::Trait(trait_ref)] = &opaque.bounds - && let Some(segment) = trait_ref.trait_ref.path.segments.last() - && let Some(generic_args) = segment.args - && let [constraint] = generic_args.constraints - && let Some(ty) = constraint.ty() - && let TyKind::Path(QPath::Resolved(_, path)) = ty.kind - && let [segment] = path.segments - { - return Some(segment); - } - - None -} - -fn is_first_generic_integral<'tcx>(segment: &'tcx PathSegment<'tcx>) -> bool { - if let Some(generic_args) = segment.args - && let [GenericArg::Type(ty), ..] = &generic_args.args - && let TyKind::Path(QPath::Resolved(_, path)) = ty.kind - && let [segment, ..] = &path.segments - && matches!(segment.res, Res::PrimTy(PrimTy::Uint(_) | PrimTy::Int(_))) - { - true - } else { - false - } -} - -fn parse_len_output<'tcx>(cx: &LateContext<'tcx>, sig: FnSig<'tcx>) -> Option { - if let Some(segment) = extract_future_output(cx, sig.output()) { - let res = segment.res; - - if matches!(res, Res::PrimTy(PrimTy::Uint(_) | PrimTy::Int(_))) { - return Some(LenOutput::Integral); - } - - if let Res::Def(_, def_id) = res - && let Some(res) = match cx.tcx.get_diagnostic_name(def_id) { - Some(sym::Option) => Some(LenOutput::Option(def_id)), - Some(sym::Result) => Some(LenOutput::Result(def_id)), - _ => None, - } - && is_first_generic_integral(segment) - { - return Some(res); - } - - return None; - } - - match *sig.output().kind() { - ty::Int(_) | ty::Uint(_) => Some(LenOutput::Integral), - ty::Adt(adt, subs) => match cx.tcx.get_diagnostic_name(adt.did()) { - Some(sym::Option) => subs.type_at(0).is_integral().then(|| LenOutput::Option(adt.did())), - Some(sym::Result) => subs.type_at(0).is_integral().then(|| LenOutput::Result(adt.did())), - _ => None, - }, - _ => None, - } -} - -impl LenOutput { - fn matches_is_empty_output<'tcx>(self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { - if let Some(segment) = extract_future_output(cx, ty) { - return match (self, segment.res) { - (_, Res::PrimTy(PrimTy::Bool)) => true, - (Self::Option(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Option, def_id) => true, - (Self::Result(_), Res::Def(_, def_id)) if cx.tcx.is_diagnostic_item(sym::Result, def_id) => true, - _ => false, - }; - } - - match (self, ty.kind()) { - (_, &ty::Bool) => true, - (Self::Option(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(), - (Self::Result(id), &ty::Adt(adt, subs)) if id == adt.did() => subs.type_at(0).is_bool(), - _ => false, - } - } - - fn expected_sig(self, self_kind: ImplicitSelfKind) -> String { - let self_ref = match self_kind { - ImplicitSelfKind::RefImm => "&", - ImplicitSelfKind::RefMut => "&mut ", - _ => "", - }; - match self { - Self::Integral => format!("expected signature: `({self_ref}self) -> bool`"), - Self::Option(_) => { - format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Option") - }, - Self::Result(..) => { - format!("expected signature: `({self_ref}self) -> bool` or `({self_ref}self) -> Result") - }, - } - } -} - -/// Checks if the given signature matches the expectations for `is_empty` -fn check_is_empty_sig<'tcx>( - cx: &LateContext<'tcx>, - sig: FnSig<'tcx>, - self_kind: ImplicitSelfKind, - len_output: LenOutput, -) -> bool { - match &**sig.inputs_and_output { - [arg, res] if len_output.matches_is_empty_output(cx, *res) => { - matches!( - (arg.kind(), self_kind), - (ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::RefImm) - | (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::RefMut) - ) || (!arg.is_ref() && matches!(self_kind, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut)) - }, - _ => false, - } -} - -/// Checks if the given type has an `is_empty` method with the appropriate signature. -#[expect(clippy::too_many_arguments)] -fn check_for_is_empty( - cx: &LateContext<'_>, - span: Span, - self_kind: ImplicitSelfKind, - output: LenOutput, - impl_ty: DefId, - item_name: Symbol, - item_kind: &str, - len_method_hir_id: HirId, - ty_decl_hir_id: HirId, -) { - // Implementor may be a type alias, in which case we need to get the `DefId` of the aliased type to - // find the correct inherent impls. - let impl_ty = if let Some(adt) = cx.tcx.type_of(impl_ty).skip_binder().ty_adt_def() { - adt.did() - } else { - return; - }; - - let is_empty = cx - .tcx - .inherent_impls(impl_ty) - .iter() - .flat_map(|&id| cx.tcx.associated_items(id).filter_by_name_unhygienic(sym::is_empty)) - .find(|item| item.is_fn()); - - let (msg, is_empty_span, self_kind) = match is_empty { - None => ( - format!( - "{item_kind} `{}` has a public `len` method, but no `is_empty` method", - item_name.as_str(), - ), - None, - None, - ), - Some(is_empty) if !cx.effective_visibilities.is_exported(is_empty.def_id.expect_local()) => ( - format!( - "{item_kind} `{}` has a public `len` method, but a private `is_empty` method", - item_name.as_str(), - ), - Some(cx.tcx.def_span(is_empty.def_id)), - None, - ), - Some(is_empty) - if !(is_empty.is_method() - && check_is_empty_sig( - cx, - cx.tcx.fn_sig(is_empty.def_id).instantiate_identity().skip_binder(), - self_kind, - output, - )) => - { - ( - format!( - "{item_kind} `{}` has a public `len` method, but the `is_empty` method has an unexpected signature", - item_name.as_str(), - ), - Some(cx.tcx.def_span(is_empty.def_id)), - Some(self_kind), - ) - }, - Some(_) => return, - }; - - if !fulfill_or_allowed(cx, LEN_WITHOUT_IS_EMPTY, [len_method_hir_id, ty_decl_hir_id]) { - span_lint_and_then(cx, LEN_WITHOUT_IS_EMPTY, span, msg, |db| { - if let Some(span) = is_empty_span { - db.span_note(span, "`is_empty` defined here"); - } - if let Some(self_kind) = self_kind { - db.note(output.expected_sig(self_kind)); - } - }); - } -} - fn is_empty_string(expr: &Expr<'_>) -> bool { if let ExprKind::Lit(lit) = expr.kind && let LitKind::Str(lit, _) = lit.node diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index bc62d9b8450c7..40487fe48f222 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -182,6 +182,7 @@ mod large_include_file; mod large_stack_arrays; mod large_stack_frames; mod legacy_numeric_constants; +mod len_without_is_empty; mod len_zero; mod let_if_seq; mod let_underscore; @@ -201,6 +202,7 @@ mod manual_clamp; mod manual_float_methods; mod manual_hash_one; mod manual_ignore_case_cmp; +mod manual_ilog2; mod manual_is_ascii_check; mod manual_is_power_of_two; mod manual_let_else; @@ -538,6 +540,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::new(unnecessary_mut_passed::UnnecessaryMutPassed)), Box::new(|_| Box::>::default()), Box::new(move |_| Box::new(len_zero::LenZero::new(conf))), + Box::new(|_| Box::new(len_without_is_empty::LenWithoutIsEmpty)), Box::new(move |_| Box::new(attrs::Attributes::new(conf))), Box::new(|_| Box::new(blocks_in_conditions::BlocksInConditions)), Box::new(|_| Box::new(unicode::Unicode)), @@ -848,6 +851,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(|_| Box::new(toplevel_ref_arg::ToplevelRefArg)), Box::new(|_| Box::new(volatile_composites::VolatileComposites)), Box::new(|_| Box::::default()), + Box::new(move |_| Box::new(manual_ilog2::ManualIlog2::new(conf))), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs b/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs index 2545f81f1afad..6c95c7b54c500 100644 --- a/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/src/tools/clippy/clippy_lints/src/loops/while_let_on_iterator.rs @@ -43,26 +43,26 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { }; // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be - // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used + // passed by reference. TODO: If the struct can be partially moved from and the struct isn't used // afterwards a mutable borrow of a field isn't necessary. - let by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut) + let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability); + let iterator_by_ref = if cx.typeck_results().expr_ty(iter_expr).ref_mutability() == Some(Mutability::Mut) || !iter_expr_struct.can_move || !iter_expr_struct.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr_struct, expr) { - ".by_ref()" + make_iterator_snippet(cx, iter_expr, &iterator) } else { - "" + iterator.into_owned() }; - let iterator = snippet_with_applicability(cx, iter_expr.span, "_", &mut applicability); span_lint_and_sugg( cx, WHILE_LET_ON_ITERATOR, expr.span.with_hi(let_expr.span.hi()), "this loop could be written as a `for` loop", "try", - format!("{loop_label}for {loop_var} in {iterator}{by_ref}"), + format!("{loop_label}for {loop_var} in {iterator_by_ref}"), applicability, ); } @@ -355,3 +355,22 @@ fn needs_mutable_borrow(cx: &LateContext<'_>, iter_expr: &IterExpr, loop_expr: & .is_break() } } + +/// Constructs the transformed iterator expression for the suggestion. +/// Returns `iterator.by_ref()` unless the last deref adjustment targets an unsized type, +/// in which case it applies all derefs (e.g., `&mut **iterator` or `&mut ***iterator`). +fn make_iterator_snippet<'tcx>(cx: &LateContext<'tcx>, iter_expr: &Expr<'tcx>, iterator: &str) -> String { + if let Some((n, adjust)) = cx + .typeck_results() + .expr_adjustments(iter_expr) + .iter() + .take_while(|x| matches!(x.kind, Adjust::Deref(_))) + .enumerate() + .last() + && !adjust.target.is_sized(cx.tcx, cx.typing_env()) + { + format!("&mut {:* Self { + Self { msrv: conf.msrv } + } +} + +impl_lint_pass!(ManualIlog2 => [MANUAL_ILOG2]); + +impl LateLintPass<'_> for ManualIlog2 { + fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + if expr.span.in_external_macro(cx.sess().source_map()) { + return; + } + + match expr.kind { + // `BIT_WIDTH - 1 - n.leading_zeros()` + ExprKind::Binary(op, left, right) + if left.span.eq_ctxt(right.span) + && op.node == BinOpKind::Sub + && let ExprKind::Lit(lit) = left.kind + && let LitKind::Int(Pu128(val), _) = lit.node + && let ExprKind::MethodCall(leading_zeros, recv, [], _) = right.kind + && leading_zeros.ident.name == sym::leading_zeros + && let ty = cx.typeck_results().expr_ty(recv) + && let Some(bit_width) = match ty.kind() { + ty::Uint(uint_ty) => uint_ty.bit_width(), + ty::Int(_) => { + // On non-positive integers, `ilog2` would panic, which might be a sign that the author does + // in fact want to calculate something different, so stay on the safer side and don't + // suggest anything. + return; + }, + _ => return, + } + && val == u128::from(bit_width) - 1 + && self.msrv.meets(cx, msrvs::ILOG2) + && !is_from_proc_macro(cx, expr) => + { + emit(cx, recv, expr); + }, + + // `n.ilog(2)` + ExprKind::MethodCall(ilog, recv, [two], _) + if expr.span.eq_ctxt(two.span) + && ilog.ident.name == sym::ilog + && let ExprKind::Lit(lit) = two.kind + && let LitKind::Int(Pu128(2), _) = lit.node + && cx.typeck_results().expr_ty_adjusted(recv).is_integral() + /* no need to check MSRV here, as `ilog` and `ilog2` were introduced simultaneously */ + && !is_from_proc_macro(cx, expr) => + { + emit(cx, recv, expr); + }, + + _ => {}, + } + } +} + +fn emit(cx: &LateContext<'_>, recv: &Expr<'_>, full_expr: &Expr<'_>) { + let mut app = Applicability::MachineApplicable; + let recv = snippet_with_applicability(cx, recv.span, "_", &mut app); + span_lint_and_sugg( + cx, + MANUAL_ILOG2, + full_expr.span, + "manually reimplementing `ilog2`", + "try", + format!("{recv}.ilog2()"), + app, + ); +} diff --git a/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs b/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs index b5f631e8fea33..89411115f730d 100644 --- a/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs +++ b/src/tools/clippy/clippy_lints/src/matches/match_like_matches.rs @@ -1,7 +1,8 @@ //! Lint a `match` or `if let .. { .. } else { .. }` expr that could be replaced by `matches!` use super::REDUNDANT_PATTERN_MATCHING; -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::higher::has_let_expr; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{is_lint_allowed, is_wild, span_contains_comment}; use rustc_ast::LitKind; @@ -43,18 +44,23 @@ pub(crate) fn check_if_let<'tcx>( { ex_new = ex_inner; } - span_lint_and_sugg( + span_lint_and_then( cx, MATCH_LIKE_MATCHES_MACRO, expr.span, - "if let .. else expression looks like `matches!` macro", - "try", - format!( - "{}matches!({}, {pat})", - if b0 { "" } else { "!" }, - snippet_with_applicability(cx, ex_new.span, "..", &mut applicability), - ), - applicability, + "`if let .. else` expression looks like `matches!` macro", + |diag| { + diag.span_suggestion_verbose( + expr.span, + "use `matches!` directly", + format!( + "{}matches!({}, {pat})", + if b0 { "" } else { "!" }, + snippet_with_applicability(cx, ex_new.span, "..", &mut applicability), + ), + applicability, + ); + }, ); } } @@ -87,7 +93,10 @@ pub(super) fn check_match<'tcx>( // ```rs // matches!(e, Either::Left $(if $guard)|+) // ``` - middle_arms.is_empty() + // + // But if the guard _is_ present, it may not be an `if-let` guard, as `matches!` doesn't + // support these (currently?) + (middle_arms.is_empty() && first_arm.guard.is_none_or(|g| !has_let_expr(g))) // - (added in #6216) There are middle arms // @@ -169,18 +178,23 @@ pub(super) fn check_match<'tcx>( { ex_new = ex_inner; } - span_lint_and_sugg( + span_lint_and_then( cx, MATCH_LIKE_MATCHES_MACRO, e.span, "match expression looks like `matches!` macro", - "try", - format!( - "{}matches!({}, {pat_and_guard})", - if b0 { "" } else { "!" }, - snippet_with_applicability(cx, ex_new.span, "..", &mut applicability), - ), - applicability, + |diag| { + diag.span_suggestion_verbose( + e.span, + "use `matches!` directly", + format!( + "{}matches!({}, {pat_and_guard})", + if b0 { "" } else { "!" }, + snippet_with_applicability(cx, ex_new.span, "..", &mut applicability), + ), + applicability, + ); + }, ); true } else { diff --git a/src/tools/clippy/clippy_lints/src/methods/manual_saturating_arithmetic.rs b/src/tools/clippy/clippy_lints/src/methods/manual_saturating_arithmetic.rs index 2196ce92b0ab5..a3dd967bd77a9 100644 --- a/src/tools/clippy/clippy_lints/src/methods/manual_saturating_arithmetic.rs +++ b/src/tools/clippy/clippy_lints/src/methods/manual_saturating_arithmetic.rs @@ -4,18 +4,20 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::sym; use rustc_ast::ast; use rustc_errors::Applicability; -use rustc_hir as hir; use rustc_hir::def::Res; +use rustc_hir::{self as hir, Expr}; use rustc_lint::LateContext; +use rustc_middle::ty::Ty; use rustc_middle::ty::layout::LayoutOf; +use rustc_span::Symbol; -pub fn check( +pub fn check_unwrap_or( cx: &LateContext<'_>, - expr: &hir::Expr<'_>, - arith_lhs: &hir::Expr<'_>, - arith_rhs: &hir::Expr<'_>, - unwrap_arg: &hir::Expr<'_>, - arith: &str, + expr: &Expr<'_>, + arith_lhs: &Expr<'_>, + arith_rhs: &Expr<'_>, + unwrap_arg: &Expr<'_>, + arith: Symbol, ) { let ty = cx.typeck_results().expr_ty(arith_lhs); if !ty.is_integral() { @@ -26,35 +28,75 @@ pub fn check( return; }; - if ty.is_signed() { - use self::MinMax::{Max, Min}; - use self::Sign::{Neg, Pos}; + let Some(checked_arith) = CheckedArith::new(arith) else { + return; + }; + + check(cx, expr, arith_lhs, arith_rhs, ty, mm, checked_arith); +} + +pub(super) fn check_sub_unwrap_or_default( + cx: &LateContext<'_>, + expr: &Expr<'_>, + arith_lhs: &Expr<'_>, + arith_rhs: &Expr<'_>, +) { + let ty = cx.typeck_results().expr_ty(arith_lhs); + if !ty.is_integral() { + return; + } + + let mm = if ty.is_signed() { + return; // iN::default() is 0, which is neither MIN nor MAX + } else { + MinMax::Min // uN::default() is 0, which is also the MIN + }; + + let checked_arith = CheckedArith::Sub; + check(cx, expr, arith_lhs, arith_rhs, ty, mm, checked_arith); +} + +fn check( + cx: &LateContext<'_>, + expr: &Expr<'_>, + arith_lhs: &Expr<'_>, + arith_rhs: &Expr<'_>, + ty: Ty<'_>, + mm: MinMax, + checked_arith: CheckedArith, +) { + use self::MinMax::{Max, Min}; + use self::Sign::{Neg, Pos}; + use CheckedArith::{Add, Mul, Sub}; + + if ty.is_signed() { let Some(sign) = lit_sign(arith_rhs) else { return; }; - match (arith, sign, mm) { - ("add", Pos, Max) | ("add", Neg, Min) | ("sub", Neg, Max) | ("sub", Pos, Min) => (), + match (checked_arith, sign, mm) { + (Add, Pos, Max) | (Add, Neg, Min) | (Sub, Neg, Max) | (Sub, Pos, Min) => (), // "mul" is omitted because lhs can be negative. _ => return, } } else { - match (mm, arith) { - (MinMax::Max, "add" | "mul") | (MinMax::Min, "sub") => (), + match (mm, checked_arith) { + (Max, Add | Mul) | (Min, Sub) => (), _ => return, } } let mut applicability = Applicability::MachineApplicable; + let saturating_arith = checked_arith.as_saturating(); span_lint_and_sugg( cx, super::MANUAL_SATURATING_ARITHMETIC, expr.span, "manual saturating arithmetic", - format!("consider using `saturating_{arith}`"), + format!("consider using `{saturating_arith}`"), format!( - "{}.saturating_{arith}({})", + "{}.{saturating_arith}({})", snippet_with_applicability(cx, arith_lhs.span, "..", &mut applicability), snippet_with_applicability(cx, arith_rhs.span, "..", &mut applicability), ), @@ -62,13 +104,40 @@ pub fn check( ); } +#[derive(Clone, Copy)] +enum CheckedArith { + Add, + Sub, + Mul, +} + +impl CheckedArith { + fn new(sym: Symbol) -> Option { + let res = match sym { + sym::checked_add => Self::Add, + sym::checked_sub => Self::Sub, + sym::checked_mul => Self::Mul, + _ => return None, + }; + Some(res) + } + + fn as_saturating(self) -> &'static str { + match self { + Self::Add => "saturating_add", + Self::Sub => "saturating_sub", + Self::Mul => "saturating_mul", + } + } +} + #[derive(PartialEq, Eq)] enum MinMax { Min, Max, } -fn is_min_or_max(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option { +fn is_min_or_max(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { // `T::max_value()` `T::min_value()` inherent methods if let hir::ExprKind::Call(func, []) = &expr.kind && let hir::ExprKind::Path(hir::QPath::TypeRelative(_, segment)) = &func.kind @@ -106,7 +175,7 @@ fn is_min_or_max(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option { (0, if bits == 128 { !0 } else { (1 << bits) - 1 }) }; - let check_lit = |expr: &hir::Expr<'_>, check_min: bool| { + let check_lit = |expr: &Expr<'_>, check_min: bool| { if let hir::ExprKind::Lit(lit) = &expr.kind && let ast::LitKind::Int(value, _) = lit.node { @@ -141,7 +210,7 @@ enum Sign { Neg, } -fn lit_sign(expr: &hir::Expr<'_>) -> Option { +fn lit_sign(expr: &Expr<'_>) -> Option { if let hir::ExprKind::Unary(hir::UnOp::Neg, inner) = &expr.kind { if let hir::ExprKind::Lit(..) = &inner.kind { return Some(Sign::Neg); diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index c22b0a548e3df..48842c8739c02 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -94,6 +94,7 @@ mod or_fun_call; mod or_then_unwrap; mod path_buf_push_overwrite; mod path_ends_with_ext; +mod ptr_offset_by_literal; mod ptr_offset_with_cast; mod range_zip_with_len; mod read_line_without_trim; @@ -1728,6 +1729,40 @@ declare_clippy_lint! { "Check for offset calculations on raw pointers to zero-sized types" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of the `offset` pointer method with an integer + /// literal. + /// + /// ### Why is this bad? + /// The `add` and `sub` methods more accurately express the intent. + /// + /// ### Example + /// ```no_run + /// let vec = vec![b'a', b'b', b'c']; + /// let ptr = vec.as_ptr(); + /// + /// unsafe { + /// ptr.offset(-8); + /// } + /// ``` + /// + /// Could be written: + /// + /// ```no_run + /// let vec = vec![b'a', b'b', b'c']; + /// let ptr = vec.as_ptr(); + /// + /// unsafe { + /// ptr.sub(8); + /// } + /// ``` + #[clippy::version = "1.92.0"] + pub PTR_OFFSET_BY_LITERAL, + pedantic, + "unneeded pointer offset" +} + declare_clippy_lint! { /// ### What it does /// Checks for usage of the `offset` pointer method with a `usize` casted to an @@ -4635,7 +4670,7 @@ declare_clippy_lint! { /// let x = vec![String::new()]; /// let _ = x.iter().map(|x| x.len()); /// ``` - #[clippy::version = "1.90.0"] + #[clippy::version = "1.92.0"] pub REDUNDANT_ITER_CLONED, perf, "detects redundant calls to `Iterator::cloned`" @@ -4659,7 +4694,7 @@ declare_clippy_lint! { /// let x: Option = Some(4); /// let y = x.unwrap_or_else(|| 2 * k); /// ``` - #[clippy::version = "1.88.0"] + #[clippy::version = "1.92.0"] pub UNNECESSARY_OPTION_MAP_OR_ELSE, suspicious, "making no use of the \"map closure\" when calling `.map_or_else(|| 2 * k, |n| n)`" @@ -4803,6 +4838,7 @@ impl_lint_pass!(Methods => [ UNINIT_ASSUMED_INIT, MANUAL_SATURATING_ARITHMETIC, ZST_OFFSET, + PTR_OFFSET_BY_LITERAL, PTR_OFFSET_WITH_CAST, FILETYPE_IS_FILE, OPTION_AS_REF_DEREF, @@ -5426,6 +5462,7 @@ impl Methods { zst_offset::check(cx, expr, recv); ptr_offset_with_cast::check(cx, name, expr, recv, arg, self.msrv); + ptr_offset_by_literal::check(cx, expr, self.msrv); }, (sym::ok_or_else, [arg]) => { unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"); @@ -5567,14 +5604,7 @@ impl Methods { (sym::unwrap_or, [u_arg]) => { match method_call(recv) { Some((arith @ (sym::checked_add | sym::checked_sub | sym::checked_mul), lhs, [rhs], _, _)) => { - manual_saturating_arithmetic::check( - cx, - expr, - lhs, - rhs, - u_arg, - &arith.as_str()[const { "checked_".len() }..], - ); + manual_saturating_arithmetic::check_unwrap_or(cx, expr, lhs, rhs, u_arg, arith); }, Some((sym::map, m_recv, [m_arg], span, _)) => { option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span, self.msrv); @@ -5595,6 +5625,9 @@ impl Methods { }, (sym::unwrap_or_default, []) => { match method_call(recv) { + Some((sym::checked_sub, lhs, [rhs], _, _)) => { + manual_saturating_arithmetic::check_sub_unwrap_or_default(cx, expr, lhs, rhs); + }, Some((sym::map, m_recv, [arg], span, _)) => { manual_is_variant_and::check(cx, expr, m_recv, arg, span, self.msrv); }, diff --git a/src/tools/clippy/clippy_lints/src/methods/ptr_offset_by_literal.rs b/src/tools/clippy/clippy_lints/src/methods/ptr_offset_by_literal.rs new file mode 100644 index 0000000000000..b5d2add65cf19 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/methods/ptr_offset_by_literal.rs @@ -0,0 +1,138 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::msrvs::{self, Msrv}; +use clippy_utils::source::SpanRangeExt; +use clippy_utils::sym; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Lit, UnOp}; +use rustc_lint::LateContext; +use std::cmp::Ordering; +use std::fmt; + +use super::PTR_OFFSET_BY_LITERAL; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Msrv) { + // `pointer::add` and `pointer::wrapping_add` are only stable since 1.26.0. These functions + // became const-stable in 1.61.0, the same version that `pointer::offset` became const-stable. + if !msrv.meets(cx, msrvs::POINTER_ADD_SUB_METHODS) { + return; + } + + let ExprKind::MethodCall(method_name, recv, [arg_expr], _) = expr.kind else { + return; + }; + + let method = match method_name.ident.name { + sym::offset => Method::Offset, + sym::wrapping_offset => Method::WrappingOffset, + _ => return, + }; + + if !cx.typeck_results().expr_ty_adjusted(recv).is_raw_ptr() { + return; + } + + // Check if the argument to the method call is a (negated) literal. + let Some((literal, literal_text)) = expr_as_literal(cx, arg_expr) else { + return; + }; + + match method.suggestion(literal) { + None => { + let msg = format!("use of `{method}` with zero"); + span_lint_and_then(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg, |diag| { + diag.span_suggestion( + expr.span.with_lo(recv.span.hi()), + format!("remove the call to `{method}`"), + String::new(), + Applicability::MachineApplicable, + ); + }); + }, + Some(method_suggestion) => { + let msg = format!("use of `{method}` with a literal"); + span_lint_and_then(cx, PTR_OFFSET_BY_LITERAL, expr.span, msg, |diag| { + diag.multipart_suggestion( + format!("use `{method_suggestion}` instead"), + vec![ + (method_name.ident.span, method_suggestion.to_string()), + (arg_expr.span, literal_text), + ], + Applicability::MachineApplicable, + ); + }); + }, + } +} + +fn get_literal_bits<'tcx>(expr: &'tcx Expr<'tcx>) -> Option { + match expr.kind { + ExprKind::Lit(Lit { + node: LitKind::Int(packed_u128, _), + .. + }) => Some(packed_u128.get()), + _ => None, + } +} + +// If the given expression is a (negated) literal, return its value. +fn expr_as_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<(i128, String)> { + if let Some(literal_bits) = get_literal_bits(expr) { + // The value must fit in a isize, so we can't have overflow here. + return Some((literal_bits.cast_signed(), format_isize_literal(cx, expr)?)); + } + + if let ExprKind::Unary(UnOp::Neg, inner) = expr.kind + && let Some(literal_bits) = get_literal_bits(inner) + { + return Some((-(literal_bits.cast_signed()), format_isize_literal(cx, inner)?)); + } + + None +} + +fn format_isize_literal<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option { + let text = expr.span.get_source_text(cx)?; + let text = peel_parens_str(&text); + Some(text.trim_end_matches("isize").trim_end_matches('_').to_string()) +} + +fn peel_parens_str(snippet: &str) -> &str { + let mut s = snippet.trim(); + while let Some(next) = s.strip_prefix("(").and_then(|suf| suf.strip_suffix(")")) { + s = next.trim(); + } + s +} + +#[derive(Copy, Clone)] +enum Method { + Offset, + WrappingOffset, +} + +impl Method { + fn suggestion(self, literal: i128) -> Option<&'static str> { + match Ord::cmp(&literal, &0) { + Ordering::Greater => match self { + Method::Offset => Some("add"), + Method::WrappingOffset => Some("wrapping_add"), + }, + // `ptr.offset(0)` is equivalent to `ptr`, so no adjustment is needed + Ordering::Equal => None, + Ordering::Less => match self { + Method::Offset => Some("sub"), + Method::WrappingOffset => Some("wrapping_sub"), + }, + } + } +} + +impl fmt::Display for Method { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Offset => write!(f, "offset"), + Self::WrappingOffset => write!(f, "wrapping_offset"), + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/missing_asserts_for_indexing.rs b/src/tools/clippy/clippy_lints/src/missing_asserts_for_indexing.rs index 808adb7e71cee..87ee164a1760e 100644 --- a/src/tools/clippy/clippy_lints/src/missing_asserts_for_indexing.rs +++ b/src/tools/clippy/clippy_lints/src/missing_asserts_for_indexing.rs @@ -5,7 +5,7 @@ use clippy_utils::comparisons::{Rel, normalize_comparison}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::{If, Range}; use clippy_utils::macros::{find_assert_eq_args, first_node_macro_backtrace, root_macro_call}; -use clippy_utils::source::snippet; +use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::visitors::for_each_expr_without_closures; use clippy_utils::{eq_expr_value, hash_expr}; use rustc_ast::{BinOpKind, LitKind, RangeLimits}; @@ -67,16 +67,13 @@ declare_clippy_lint! { } declare_lint_pass!(MissingAssertsForIndexing => [MISSING_ASSERTS_FOR_INDEXING]); -fn report_lint(cx: &LateContext<'_>, full_span: Span, msg: &'static str, indexes: &[Span], f: F) +fn report_lint(cx: &LateContext<'_>, index_spans: Vec, msg: &'static str, f: F) where F: FnOnce(&mut Diag<'_, ()>), { - span_lint_and_then(cx, MISSING_ASSERTS_FOR_INDEXING, full_span, msg, |diag| { + span_lint_and_then(cx, MISSING_ASSERTS_FOR_INDEXING, index_spans, msg, |diag| { f(diag); - for span in indexes { - diag.span_note(*span, "slice indexed here"); - } - diag.note("asserting the length before indexing will elide bounds checks"); + diag.note_once("asserting the length before indexing will elide bounds checks"); }); } @@ -213,15 +210,6 @@ impl<'hir> IndexEntry<'hir> { | IndexEntry::IndexWithoutAssert { slice, .. } => slice, } } - - pub fn index_spans(&self) -> Option<&[Span]> { - match self { - IndexEntry::StrayAssert { .. } => None, - IndexEntry::AssertWithIndex { indexes, .. } | IndexEntry::IndexWithoutAssert { indexes, .. } => { - Some(indexes) - }, - } - } } /// Extracts the upper index of a slice indexing expression. @@ -354,63 +342,47 @@ fn check_assert<'hir>(cx: &LateContext<'_>, expr: &'hir Expr<'hir>, map: &mut Un /// Inspects indexes and reports lints. /// /// Called at the end of this lint after all indexing and `assert!` expressions have been collected. -fn report_indexes(cx: &LateContext<'_>, map: &UnindexMap>>) { - for bucket in map.values() { +fn report_indexes(cx: &LateContext<'_>, map: UnindexMap>>) { + for bucket in map.into_values() { for entry in bucket { - let Some(full_span) = entry - .index_spans() - .and_then(|spans| spans.first().zip(spans.last())) - .map(|(low, &high)| low.to(high)) - else { - continue; - }; - - match *entry { + match entry { IndexEntry::AssertWithIndex { highest_index, is_first_highest, asserted_len, - ref indexes, + indexes, comparison, assert_span, slice, macro_call, } if indexes.len() > 1 && !is_first_highest => { + let mut app = Applicability::MachineApplicable; + let slice_str = snippet_with_applicability(cx, slice.span, "_", &mut app); // if we have found an `assert!`, let's also check that it's actually right // and if it covers the highest index and if not, suggest the correct length let sugg = match comparison { // `v.len() < 5` and `v.len() <= 5` does nothing in terms of bounds checks. // The user probably meant `v.len() > 5` - LengthComparison::LengthLessThanInt | LengthComparison::LengthLessThanOrEqualInt => Some( - format!("assert!({}.len() > {highest_index})", snippet(cx, slice.span, "..")), - ), + LengthComparison::LengthLessThanInt | LengthComparison::LengthLessThanOrEqualInt => { + Some(format!("assert!({slice_str}.len() > {highest_index})",)) + }, // `5 < v.len()` == `v.len() > 5` - LengthComparison::IntLessThanLength if asserted_len < highest_index => Some(format!( - "assert!({}.len() > {highest_index})", - snippet(cx, slice.span, "..") - )), + LengthComparison::IntLessThanLength if asserted_len < highest_index => { + Some(format!("assert!({slice_str}.len() > {highest_index})",)) + }, // `5 <= v.len() == `v.len() >= 5` - LengthComparison::IntLessThanOrEqualLength if asserted_len <= highest_index => Some(format!( - "assert!({}.len() > {highest_index})", - snippet(cx, slice.span, "..") - )), + LengthComparison::IntLessThanOrEqualLength if asserted_len <= highest_index => { + Some(format!("assert!({slice_str}.len() > {highest_index})",)) + }, // `highest_index` here is rather a length, so we need to add 1 to it LengthComparison::LengthEqualInt if asserted_len < highest_index + 1 => match macro_call { - sym::assert_eq_macro => Some(format!( - "assert_eq!({}.len(), {})", - snippet(cx, slice.span, ".."), - highest_index + 1 - )), - sym::debug_assert_eq_macro => Some(format!( - "debug_assert_eq!({}.len(), {})", - snippet(cx, slice.span, ".."), - highest_index + 1 - )), - _ => Some(format!( - "assert!({}.len() == {})", - snippet(cx, slice.span, ".."), - highest_index + 1 - )), + sym::assert_eq_macro => { + Some(format!("assert_eq!({slice_str}.len(), {})", highest_index + 1)) + }, + sym::debug_assert_eq_macro => { + Some(format!("debug_assert_eq!({slice_str}.len(), {})", highest_index + 1)) + }, + _ => Some(format!("assert!({slice_str}.len() == {})", highest_index + 1)), }, _ => None, }; @@ -418,22 +390,21 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnindexMap if let Some(sugg) = sugg { report_lint( cx, - full_span, - "indexing into a slice multiple times with an `assert` that does not cover the highest index", indexes, + "indexing into a slice multiple times with an `assert` that does not cover the highest index", |diag| { - diag.span_suggestion( + diag.span_suggestion_verbose( assert_span, "provide the highest index that is indexed with", sugg, - Applicability::MachineApplicable, + app, ); }, ); } }, IndexEntry::IndexWithoutAssert { - ref indexes, + indexes, highest_index, is_first_highest, slice, @@ -442,9 +413,8 @@ fn report_indexes(cx: &LateContext<'_>, map: &UnindexMap // adding an `assert!` that covers the highest index report_lint( cx, - full_span, - "indexing into a slice multiple times without an `assert`", indexes, + "indexing into a slice multiple times without an `assert`", |diag| { diag.help(format!( "consider asserting the length before indexing: `assert!({}.len() > {highest_index});`", @@ -469,6 +439,6 @@ impl LateLintPass<'_> for MissingAssertsForIndexing { ControlFlow::::Continue(()) }); - report_indexes(cx, &map); + report_indexes(cx, map); } } diff --git a/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs b/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs index 3a8a4dd0c7137..8970970f4b475 100644 --- a/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs +++ b/src/tools/clippy/clippy_lints/src/nonstandard_macro_braces.rs @@ -115,35 +115,41 @@ impl EarlyLintPass for MacroBraces { } fn is_offending_macro(cx: &EarlyContext<'_>, span: Span, mac_braces: &MacroBraces) -> Option { - let unnested_or_local = || { - !span.ctxt().outer_expn_data().call_site.from_expansion() + let unnested_or_local = |span: Span| { + !span.from_expansion() || span .macro_backtrace() .last() .is_some_and(|e| e.macro_def_id.is_some_and(DefId::is_local)) }; - let callsite_span = span.ctxt().outer_expn_data().call_site; - if let ExpnKind::Macro(MacroKind::Bang, mac_name) = span.ctxt().outer_expn_data().kind + + let mut ctxt = span.ctxt(); + while !ctxt.is_root() { + let expn_data = ctxt.outer_expn_data(); + if let ExpnKind::Macro(MacroKind::Bang, mac_name) = expn_data.kind && let name = mac_name.as_str() && let Some(&braces) = mac_braces.macro_braces.get(name) - && let Some(snip) = callsite_span.get_source_text(cx) + && let Some(snip) = expn_data.call_site.get_source_text(cx) // we must check only invocation sites // https://github.com/rust-lang/rust-clippy/issues/7422 && let Some(macro_args_str) = snip.strip_prefix(name).and_then(|snip| snip.strip_prefix('!')) && let Some(old_open_brace @ ('{' | '(' | '[')) = macro_args_str.trim_start().chars().next() && old_open_brace != braces.0 - && unnested_or_local() - && !mac_braces.done.contains(&callsite_span) - { - Some(MacroInfo { - callsite_span, - callsite_snippet: snip, - old_open_brace, - braces, - }) - } else { - None + && unnested_or_local(expn_data.call_site) + && !mac_braces.done.contains(&expn_data.call_site) + { + return Some(MacroInfo { + callsite_span: expn_data.call_site, + callsite_snippet: snip, + old_open_brace, + braces, + }); + } + + ctxt = expn_data.call_site.ctxt(); } + + None } fn emit_help(cx: &EarlyContext<'_>, snip: &str, (open, close): (char, char), span: Span, add_semi: bool) { diff --git a/src/tools/clippy/clippy_lints/src/operators/decimal_bitwise_operands.rs b/src/tools/clippy/clippy_lints/src/operators/decimal_bitwise_operands.rs new file mode 100644 index 0000000000000..8511f2151342d --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/operators/decimal_bitwise_operands.rs @@ -0,0 +1,76 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::numeric_literal; +use clippy_utils::numeric_literal::NumericLiteral; +use clippy_utils::source::SpanRangeExt; +use rustc_ast::LitKind; +use rustc_data_structures::packed::Pu128; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::Span; + +use super::DECIMAL_BITWISE_OPERANDS; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, op: BinOpKind, left: &'tcx Expr<'_>, right: &'tcx Expr<'_>) { + if !matches!(op, BinOpKind::BitAnd | BinOpKind::BitOr | BinOpKind::BitXor) { + return; + } + + for expr in [left, right] { + check_expr(cx, expr); + } +} + +fn check_expr(cx: &LateContext<'_>, expr: &Expr<'_>) { + match &expr.kind { + ExprKind::Block(block, _) => { + if let Some(block_expr) = block.expr { + check_expr(cx, block_expr); + } + }, + ExprKind::Cast(cast_expr, _) => { + check_expr(cx, cast_expr); + }, + ExprKind::Unary(_, unary_expr) => { + check_expr(cx, unary_expr); + }, + ExprKind::AddrOf(_, _, addr_of_expr) => { + check_expr(cx, addr_of_expr); + }, + ExprKind::Lit(lit) => { + if let LitKind::Int(Pu128(val), _) = lit.node + && !is_single_digit(val) + && !is_power_of_twoish(val) + && let Some(src) = lit.span.get_source_text(cx) + && let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node) + && num_lit.is_decimal() + { + emit_lint(cx, lit.span, num_lit.suffix, val); + } + }, + _ => (), + } +} + +fn is_power_of_twoish(val: u128) -> bool { + val.is_power_of_two() || val.wrapping_add(1).is_power_of_two() +} + +fn is_single_digit(val: u128) -> bool { + val <= 9 +} + +fn emit_lint(cx: &LateContext<'_>, span: Span, suffix: Option<&str>, val: u128) { + span_lint_and_help( + cx, + DECIMAL_BITWISE_OPERANDS, + span, + "using decimal literal for bitwise operation", + None, + format!( + "use binary ({}), hex ({}), or octal ({}) notation for better readability", + numeric_literal::format(&format!("{val:#b}"), suffix, false), + numeric_literal::format(&format!("{val:#x}"), suffix, false), + numeric_literal::format(&format!("{val:#o}"), suffix, false), + ), + ); +} diff --git a/src/tools/clippy/clippy_lints/src/operators/mod.rs b/src/tools/clippy/clippy_lints/src/operators/mod.rs index 8db2cc1d3f578..53b8e9e5d5ae7 100644 --- a/src/tools/clippy/clippy_lints/src/operators/mod.rs +++ b/src/tools/clippy/clippy_lints/src/operators/mod.rs @@ -3,6 +3,7 @@ mod assign_op_pattern; mod bit_mask; mod cmp_owned; mod const_comparisons; +mod decimal_bitwise_operands; mod double_comparison; mod duration_subsec; mod eq_op; @@ -935,6 +936,28 @@ declare_clippy_lint! { "use of disallowed default division and remainder operations" } +declare_clippy_lint! { + /// ### What it does + /// Checks for decimal literals used as bit masks in bitwise operations. + /// + /// ### Why is this bad? + /// Using decimal literals for bit masks can make the code less readable and obscure the intended bit pattern. + /// Binary, hexadecimal, or octal literals make the bit pattern more explicit and easier to understand at a glance. + /// + /// ### Example + /// ```rust,no_run + /// let a = 14 & 6; // Bit pattern is not immediately clear + /// ``` + /// Use instead: + /// ```rust,no_run + /// let a = 0b1110 & 0b0110; + /// ``` + #[clippy::version = "1.93.0"] + pub DECIMAL_BITWISE_OPERANDS, + pedantic, + "use binary, hex, or octal literals for bitwise operations" +} + pub struct Operators { arithmetic_context: numeric_arithmetic::Context, verbose_bit_mask_threshold: u64, @@ -984,6 +1007,7 @@ impl_lint_pass!(Operators => [ MANUAL_IS_MULTIPLE_OF, MANUAL_DIV_CEIL, INVALID_UPCAST_COMPARISONS, + DECIMAL_BITWISE_OPERANDS ]); impl<'tcx> LateLintPass<'tcx> for Operators { @@ -1003,6 +1027,7 @@ impl<'tcx> LateLintPass<'tcx> for Operators { needless_bitwise_bool::check(cx, e, op.node, lhs, rhs); manual_midpoint::check(cx, e, op.node, lhs, rhs, self.msrv); manual_is_multiple_of::check(cx, e, op.node, lhs, rhs, self.msrv); + decimal_bitwise_operands::check(cx, op.node, lhs, rhs); } self.arithmetic_context.check_binary(cx, e, op.node, lhs, rhs); bit_mask::check(cx, e, op.node, lhs, rhs); @@ -1028,6 +1053,9 @@ impl<'tcx> LateLintPass<'tcx> for Operators { }, ExprKind::AssignOp(op, lhs, rhs) => { let bin_op = op.node.into(); + if !e.span.from_expansion() { + decimal_bitwise_operands::check(cx, bin_op, lhs, rhs); + } self.arithmetic_context.check_binary(cx, e, bin_op, lhs, rhs); misrefactored_assign_op::check(cx, e, bin_op, lhs, rhs); modulo_arithmetic::check(cx, e, bin_op, lhs, rhs, false); diff --git a/src/tools/clippy/clippy_lints/src/transmute/mod.rs b/src/tools/clippy/clippy_lints/src/transmute/mod.rs index d643f7aea497f..435cd7bcba4e3 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/mod.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/mod.rs @@ -18,8 +18,11 @@ mod wrong_transmute; use clippy_config::Conf; use clippy_utils::is_in_const_context; use clippy_utils::msrvs::Msrv; +use clippy_utils::sugg::Sugg; +use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Ty}; use rustc_session::impl_lint_pass; use rustc_span::symbol::sym; @@ -490,6 +493,32 @@ impl Transmute { pub fn new(conf: &'static Conf) -> Self { Self { msrv: conf.msrv } } + + /// When transmuting, a struct containing a single field works like the field. + /// This function extracts the field type and the expression to get the field. + fn extract_struct_field<'tcx>( + cx: &LateContext<'tcx>, + e: &'tcx Expr<'_>, + outer_type: Ty<'tcx>, + outer: &'tcx Expr<'tcx>, + ) -> (Ty<'tcx>, Sugg<'tcx>) { + let mut applicability = Applicability::MachineApplicable; + let outer_sugg = Sugg::hir_with_context(cx, outer, e.span.ctxt(), "..", &mut applicability); + if let ty::Adt(struct_def, struct_args) = *outer_type.kind() + && struct_def.is_struct() + && let mut fields = struct_def.all_fields() + && let Some(first) = fields.next() + && fields.next().is_none() + && first.vis.is_accessible_from(cx.tcx.parent_module(outer.hir_id), cx.tcx) + { + ( + first.ty(cx.tcx, struct_args), + Sugg::NonParen(format!("{}.{}", outer_sugg.maybe_paren(), first.name).into()), + ) + } else { + (outer_type, outer_sugg) + } + } } impl<'tcx> LateLintPass<'tcx> for Transmute { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { @@ -516,14 +545,17 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { return; } + // A struct having a single pointer can be treated like a pointer. + let (from_field_ty, from_field_expr) = Self::extract_struct_field(cx, e, from_ty, arg); + let linted = wrong_transmute::check(cx, e, from_ty, to_ty) | crosspointer_transmute::check(cx, e, from_ty, to_ty) | transmuting_null::check(cx, e, arg, to_ty) | transmute_null_to_fn::check(cx, e, arg, to_ty) - | transmute_ptr_to_ref::check(cx, e, from_ty, to_ty, arg, path, self.msrv) + | transmute_ptr_to_ref::check(cx, e, from_field_ty, to_ty, from_field_expr.clone(), path, self.msrv) | missing_transmute_annotations::check(cx, path, arg, from_ty, to_ty, e.hir_id) | transmute_ref_to_ref::check(cx, e, from_ty, to_ty, arg, const_context) - | transmute_ptr_to_ptr::check(cx, e, from_ty, to_ty, arg, self.msrv) + | transmute_ptr_to_ptr::check(cx, e, from_field_ty, to_ty, from_field_expr, self.msrv) | transmute_int_to_bool::check(cx, e, from_ty, to_ty, arg) | transmute_int_to_non_zero::check(cx, e, from_ty, to_ty, arg) | (unsound_collection_transmute::check(cx, e, from_ty, to_ty) diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs index 91fce5d5bd68a..036b16e3dc3d6 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ptr.rs @@ -14,11 +14,9 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - arg: &'tcx Expr<'_>, + arg: sugg::Sugg<'_>, msrv: Msrv, ) -> bool { - let mut applicability = Applicability::MachineApplicable; - let arg_sugg = sugg::Sugg::hir_with_context(cx, arg, e.span.ctxt(), "..", &mut applicability); match (from_ty.kind(), to_ty.kind()) { (ty::RawPtr(from_pointee_ty, from_mutbl), ty::RawPtr(to_pointee_ty, to_mutbl)) => { span_lint_and_then( @@ -34,7 +32,7 @@ pub(super) fn check<'tcx>( diag.span_suggestion_verbose( e.span, "use `pointer::cast` instead", - format!("{}.cast::<{to_pointee_ty}>()", arg_sugg.maybe_paren()), + format!("{}.cast::<{to_pointee_ty}>()", arg.maybe_paren()), Applicability::MaybeIncorrect, ); } else if from_pointee_ty == to_pointee_ty @@ -49,14 +47,14 @@ pub(super) fn check<'tcx>( diag.span_suggestion_verbose( e.span, format!("use `pointer::{method}` instead"), - format!("{}.{method}()", arg_sugg.maybe_paren()), + format!("{}.{method}()", arg.maybe_paren()), Applicability::MaybeIncorrect, ); } else { diag.span_suggestion_verbose( e.span, "use an `as` cast instead", - arg_sugg.as_ty(to_ty), + arg.as_ty(to_ty), Applicability::MaybeIncorrect, ); } diff --git a/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs b/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs index e67ab6a73d269..ba107eed6b140 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/transmute_ptr_to_ref.rs @@ -15,7 +15,7 @@ pub(super) fn check<'tcx>( e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>, - arg: &'tcx Expr<'_>, + arg: sugg::Sugg<'_>, path: &'tcx Path<'_>, msrv: Msrv, ) -> bool { @@ -27,7 +27,6 @@ pub(super) fn check<'tcx>( e.span, format!("transmute from a pointer type (`{from_ty}`) to a reference type (`{to_ty}`)"), |diag| { - let arg = sugg::Sugg::hir(cx, arg, ".."); let (deref, cast) = match mutbl { Mutability::Mut => ("&mut *", "*mut"), Mutability::Not => ("&*", "*const"), diff --git a/src/tools/clippy/clippy_lints/src/tuple_array_conversions.rs b/src/tools/clippy/clippy_lints/src/tuple_array_conversions.rs index 5d0945bece553..98bf9f9fea586 100644 --- a/src/tools/clippy/clippy_lints/src/tuple_array_conversions.rs +++ b/src/tools/clippy/clippy_lints/src/tuple_array_conversions.rs @@ -1,9 +1,9 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::res::MaybeResPath; -use clippy_utils::visitors::for_each_local_use_after_expr; +use clippy_utils::visitors::local_used_once; +use clippy_utils::{get_enclosing_block, is_from_proc_macro}; use itertools::Itertools; use rustc_ast::LitKind; use rustc_hir::{Expr, ExprKind, Node, PatKind}; @@ -11,7 +11,6 @@ use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::ty::{self, Ty}; use rustc_session::impl_lint_pass; use std::iter::once; -use std::ops::ControlFlow; declare_clippy_lint! { /// ### What it does @@ -86,7 +85,7 @@ fn check_array<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: & ExprKind::Path(_) => Some(elements.iter().collect()), _ => None, }) - && all_bindings_are_for_conv(cx, &[ty], expr, elements, &locals, ToType::Array) + && all_bindings_are_for_conv(cx, &[ty], elements, &locals, ToType::Array) && !is_from_proc_macro(cx, expr) { span_lint_and_help( @@ -123,7 +122,7 @@ fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: & ExprKind::Path(_) => Some(elements.iter().collect()), _ => None, }) - && all_bindings_are_for_conv(cx, tys, expr, elements, &locals, ToType::Tuple) + && all_bindings_are_for_conv(cx, tys, elements, &locals, ToType::Tuple) && !is_from_proc_macro(cx, expr) { span_lint_and_help( @@ -148,7 +147,6 @@ fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: & fn all_bindings_are_for_conv<'tcx>( cx: &LateContext<'tcx>, final_tys: &[Ty<'tcx>], - expr: &Expr<'_>, elements: &[Expr<'_>], locals: &[&Expr<'_>], kind: ToType, @@ -166,13 +164,30 @@ fn all_bindings_are_for_conv<'tcx>( _ => None, }) .all_equal() - // Fix #11124, very convenient utils function! ❤️ - && locals - .iter() - .all(|&l| for_each_local_use_after_expr(cx, l, expr.hir_id, |_| ControlFlow::Break::<()>(())).is_continue()) + && locals.iter().zip(local_parents.iter()).all(|(&l, &parent)| { + if let Node::LetStmt(_) = parent { + return true; + } + + let Some(b) = get_enclosing_block(cx, l) else { + return true; + }; + local_used_once(cx, b, l).is_some() + }) && local_parents.first().is_some_and(|node| { let Some(ty) = match node { - Node::Pat(pat) => Some(pat.hir_id), + Node::Pat(pat) + if let PatKind::Tuple(pats, _) | PatKind::Slice(pats, None, []) = &pat.kind + && pats.iter().zip(locals.iter()).all(|(p, l)| { + if let PatKind::Binding(_, id, _, _) = p.kind { + id == *l + } else { + true + } + }) => + { + Some(pat.hir_id) + }, Node::LetStmt(l) => Some(l.hir_id), _ => None, } @@ -186,7 +201,9 @@ fn all_bindings_are_for_conv<'tcx>( tys.len() == elements.len() && tys.iter().chain(final_tys.iter().copied()).all_equal() }, (ToType::Tuple, ty::Array(ty, len)) => { - let Some(len) = len.try_to_target_usize(cx.tcx) else { return false }; + let Some(len) = len.try_to_target_usize(cx.tcx) else { + return false; + }; len as usize == elements.len() && final_tys.iter().chain(once(ty)).all_equal() }, _ => false, diff --git a/src/tools/clippy/clippy_lints/src/unwrap.rs b/src/tools/clippy/clippy_lints/src/unwrap.rs index 8ed3df8731b3f..a95f2b681add0 100644 --- a/src/tools/clippy/clippy_lints/src/unwrap.rs +++ b/src/tools/clippy/clippy_lints/src/unwrap.rs @@ -180,14 +180,19 @@ impl Local { field_indices, .. } => { + let field_projections = place + .projections + .iter() + .filter(|proj| matches!(proj.kind, ProjectionKind::Field(_, _))) + .collect::>(); is_potentially_local_place(*local_id, place) // If there were projections other than field projections, err on the side of caution and say that they // _might_ be mutating something. // // The reason we use `<=` and not `==` is that a mutation of `struct` or `struct.field1` should count as // mutation of the child fields such as `struct.field1.field2` - && place.projections.len() <= field_indices.len() - && iter::zip(&place.projections, field_indices.iter().copied().rev()).all(|(proj, field_idx)| { + && field_projections.len() <= field_indices.len() + && iter::zip(&field_projections, field_indices.iter().copied().rev()).all(|(proj, field_idx)| { match proj.kind { ProjectionKind::Field(f_idx, _) => f_idx == field_idx, // If this is a projection we don't expect, it _might_ be mutating something diff --git a/src/tools/clippy/clippy_lints/src/useless_conversion.rs b/src/tools/clippy/clippy_lints/src/useless_conversion.rs index 0cf5b9431a342..c06313d1a4c44 100644 --- a/src/tools/clippy/clippy_lints/src/useless_conversion.rs +++ b/src/tools/clippy/clippy_lints/src/useless_conversion.rs @@ -354,7 +354,10 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { return; } - let sugg = snippet(cx, recv.span, "").into_owned(); + let mut applicability = Applicability::MachineApplicable; + let sugg = snippet_with_context(cx, recv.span, e.span.ctxt(), "", &mut applicability) + .0 + .into_owned(); span_lint_and_sugg( cx, USELESS_CONVERSION, diff --git a/src/tools/clippy/clippy_lints/src/zero_repeat_side_effects.rs b/src/tools/clippy/clippy_lints/src/zero_repeat_side_effects.rs index a8351690068df..95085161c09c9 100644 --- a/src/tools/clippy/clippy_lints/src/zero_repeat_side_effects.rs +++ b/src/tools/clippy/clippy_lints/src/zero_repeat_side_effects.rs @@ -4,10 +4,11 @@ use clippy_utils::source::{snippet, snippet_indent}; use rustc_ast::LitKind; use rustc_data_structures::packed::Pu128; use rustc_errors::Applicability; -use rustc_hir::{ConstArgKind, ExprKind, Node}; +use rustc_hir::{ConstArgKind, Expr, ExprKind, LetStmt, LocalSource, Node}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::IsSuggestable; +use rustc_middle::ty::{IsSuggestable, Ty}; use rustc_session::declare_lint_pass; +use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -44,7 +45,7 @@ declare_clippy_lint! { declare_lint_pass!(ZeroRepeatSideEffects => [ZERO_REPEAT_SIDE_EFFECTS]); impl LateLintPass<'_> for ZeroRepeatSideEffects { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &rustc_hir::Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { if let Some(args) = VecArgs::hir(cx, expr) && let VecArgs::Repeat(inner_expr, len) = args && let ExprKind::Lit(l) = len.kind @@ -69,7 +70,7 @@ impl LateLintPass<'_> for ZeroRepeatSideEffects { } } -fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: &'_ rustc_hir::Expr<'_>, is_vec: bool) { +fn inner_check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, inner_expr: &'_ Expr<'_>, is_vec: bool) { // check if expr is a call or has a call inside it if inner_expr.can_have_side_effects() { let parent_hir_node = cx.tcx.parent_hir_node(expr.hir_id); @@ -81,19 +82,22 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: let vec = if is_vec { "vec!" } else { "" }; let (span, sugg) = match parent_hir_node { - Node::LetStmt(l) => ( - l.span, - format!( - "{inner_expr};\n{indent}let {var_name}: {return_type} = {vec}[];", - var_name = snippet(cx, l.pat.span.source_callsite(), "..") - ), - ), + Node::LetStmt(l) + if matches!(l.source, LocalSource::AssignDesugar) + && let mut parent_iter = cx.tcx.hir_parent_iter(l.hir_id) + && let Some((_, Node::Stmt(_))) = parent_iter.next() + && let Some((_, Node::Block(_))) = parent_iter.next() + && let Some((_, Node::Expr(x))) = parent_iter.next() => + { + ( + x.span, + assign_expr_suggestion(cx, x, l.pat.span, &inner_expr, return_type, vec), + ) + }, + Node::LetStmt(l) => (l.span, let_stmt_suggestion(cx, l, &inner_expr, return_type, vec)), Node::Expr(x) if let ExprKind::Assign(l, _, _) = x.kind => ( x.span, - format!( - "{inner_expr};\n{indent}{var_name} = {vec}[] as {return_type}", - var_name = snippet(cx, l.span.source_callsite(), "..") - ), + assign_expr_suggestion(cx, x, l.span, &inner_expr, return_type, vec), ), // NOTE: don't use the stmt span to avoid touching the trailing semicolon Node::Stmt(_) => (expr.span, format!("{inner_expr};\n{indent}{vec}[] as {return_type}")), @@ -131,3 +135,41 @@ fn inner_check(cx: &LateContext<'_>, expr: &'_ rustc_hir::Expr<'_>, inner_expr: ); } } + +fn let_stmt_suggestion( + cx: &LateContext<'_>, + let_stmt: &LetStmt<'_>, + inner_expr: &str, + return_type: Ty<'_>, + vec_str: &str, +) -> String { + let indent = snippet_indent(cx, let_stmt.span).unwrap_or_default(); + format!( + "{inner_expr};\n{}let {var_name}: {return_type} = {vec_str}[];", + indent, + var_name = snippet(cx, let_stmt.pat.span.source_callsite(), "..") + ) +} + +fn assign_expr_suggestion( + cx: &LateContext<'_>, + outer_expr: &Expr<'_>, + assign_expr_span: Span, + inner_expr: &str, + return_type: Ty<'_>, + vec_str: &str, +) -> String { + let mut parent_hir_node = cx.tcx.parent_hir_node(outer_expr.hir_id); + if let Node::Stmt(stmt) = parent_hir_node { + parent_hir_node = cx.tcx.parent_hir_node(stmt.hir_id); + } + let needs_curly = !matches!(parent_hir_node, Node::Block(_)); + + let indent = snippet_indent(cx, outer_expr.span).unwrap_or_default(); + let var_name = snippet(cx, assign_expr_span.source_callsite(), ".."); + if needs_curly { + format!("{{\n {indent}{inner_expr};\n {indent}{var_name} = {vec_str}[] as {return_type}\n{indent}}}",) + } else { + format!("{inner_expr};\n{indent}{var_name} = {vec_str}[] as {return_type}") + } +} diff --git a/src/tools/clippy/clippy_utils/Cargo.toml b/src/tools/clippy/clippy_utils/Cargo.toml index a1e1b763dccb1..503d581d6c7f9 100644 --- a/src/tools/clippy/clippy_utils/Cargo.toml +++ b/src/tools/clippy/clippy_utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_utils" -version = "0.1.93" +version = "0.1.94" edition = "2024" description = "Helpful tools for writing lints, provided as they are used in Clippy" repository = "https://github.com/rust-lang/rust-clippy" @@ -16,6 +16,10 @@ itertools = "0.12" rustc_apfloat = "0.2.0" serde = { version = "1.0", features = ["derive"] } +[lints.rust.unexpected_cfgs] +level = "warn" +check-cfg = ['cfg(bootstrap)'] + [package.metadata.rust-analyzer] # This crate uses #[feature(rustc_private)] rustc_private = true diff --git a/src/tools/clippy/clippy_utils/README.md b/src/tools/clippy/clippy_utils/README.md index 4b1a10a3d9cfa..dc8695fef9f56 100644 --- a/src/tools/clippy/clippy_utils/README.md +++ b/src/tools/clippy/clippy_utils/README.md @@ -8,7 +8,7 @@ This crate is only guaranteed to build with this `nightly` toolchain: ``` -nightly-2025-11-28 +nightly-2025-12-11 ``` diff --git a/src/tools/clippy/clippy_utils/src/msrvs.rs b/src/tools/clippy/clippy_utils/src/msrvs.rs index 86d17a8231d53..4a7fa3472cae4 100644 --- a/src/tools/clippy/clippy_utils/src/msrvs.rs +++ b/src/tools/clippy/clippy_utils/src/msrvs.rs @@ -40,6 +40,7 @@ msrv_aliases! { 1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE } 1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN } 1,68,0 { PATH_MAIN_SEPARATOR_STR } + 1,67,0 { ILOG2 } 1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS } 1,63,0 { CLONE_INTO, CONST_SLICE_FROM_REF } 1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CONST_EXTERN_C_FN } diff --git a/src/tools/clippy/clippy_utils/src/sym.rs b/src/tools/clippy/clippy_utils/src/sym.rs index 1d1537dd0e91f..00f4a9c7e5862 100644 --- a/src/tools/clippy/clippy_utils/src/sym.rs +++ b/src/tools/clippy/clippy_utils/src/sym.rs @@ -180,6 +180,7 @@ generate! { has_significant_drop, hidden_glob_reexports, hygiene, + ilog, insert, insert_str, inspect, @@ -207,6 +208,7 @@ generate! { join, kw, lazy_static, + leading_zeros, lint_vec, ln, lock, diff --git a/src/tools/clippy/declare_clippy_lint/Cargo.toml b/src/tools/clippy/declare_clippy_lint/Cargo.toml index b73a7c7bb4d98..ee6d6cdbc34eb 100644 --- a/src/tools/clippy/declare_clippy_lint/Cargo.toml +++ b/src/tools/clippy/declare_clippy_lint/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "declare_clippy_lint" -version = "0.1.93" +version = "0.1.94" edition = "2024" repository = "https://github.com/rust-lang/rust-clippy" license = "MIT OR Apache-2.0" diff --git a/src/tools/clippy/rust-toolchain.toml b/src/tools/clippy/rust-toolchain.toml index 5157b79832a39..1384f4078ebe4 100644 --- a/src/tools/clippy/rust-toolchain.toml +++ b/src/tools/clippy/rust-toolchain.toml @@ -1,6 +1,6 @@ [toolchain] # begin autogenerated nightly -channel = "nightly-2025-11-28" +channel = "nightly-2025-12-11" # end autogenerated nightly components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] profile = "minimal" diff --git a/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/clippy.toml b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/clippy.toml new file mode 100644 index 0000000000000..b6fd0e8a0483e --- /dev/null +++ b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/clippy.toml @@ -0,0 +1 @@ +stack-size-threshold = 0 diff --git a/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs new file mode 100644 index 0000000000000..1f6265590f936 --- /dev/null +++ b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs @@ -0,0 +1,42 @@ +//@ignore-target: i686 +//@normalize-stderr-test: "\b10000(08|16|32)\b" -> "100$$PTR" +//@normalize-stderr-test: "\b2500(060|120)\b" -> "250$$PTR" + +#![warn(clippy::large_stack_frames)] + +extern crate serde; +use serde::{Deserialize, Serialize}; + +struct ArrayDefault([u8; N]); + +macro_rules! mac { + ($name:ident) => { + fn foo() { + let $name = 1; + println!("macro_name called"); + } + + fn bar() { + let $name = ArrayDefault([0; 1000]); + } + }; +} + +mac!(something); +//~^ large_stack_frames +//~| large_stack_frames + +#[derive(Deserialize, Serialize)] +//~^ large_stack_frames +//~| large_stack_frames +//~| large_stack_frames +//~| large_stack_frames +//~| large_stack_frames +//~| large_stack_frames +//~| large_stack_frames +//~| large_stack_frames +struct S { + a: [u128; 31], +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.stderr b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.stderr new file mode 100644 index 0000000000000..bc222f6b10395 --- /dev/null +++ b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.stderr @@ -0,0 +1,89 @@ +error: function `foo` generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:25:1 + | +LL | fn foo() { + | --- this function has a stack frame size of 20 bytes +... +LL | mac!(something); + | ^^^^^^^^^^^^^^^ + | + = note: 20 bytes is larger than Clippy's configured `stack-size-threshold` of 0 + = note: allocating large amounts of stack space can overflow the stack and cause the program to abort + = note: `-D clippy::large-stack-frames` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::large_stack_frames)]` + +error: function `bar` generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:25:1 + | +LL | fn bar() { + | --- this function has a stack frame size of 2000 bytes +LL | let $name = ArrayDefault([0; 1000]); + | --------- this is the largest part, at 1000 bytes for type `[u8; 1000]` +... +LL | mac!(something); + | ^^^^^^^^^^^^^^^ + | + = note: 2000 bytes is larger than Clippy's configured `stack-size-threshold` of 0 + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:10 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^^^ + | + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error: method generated by this macro may allocate a lot of stack space + --> tests/ui-toml/large_stack_frames_for_macros/large_stack_frames.rs:29:23 + | +LL | #[derive(Deserialize, Serialize)] + | ^^^^^^^^^ + +error: aborting due to 10 previous errors + diff --git a/src/tools/clippy/tests/ui-toml/large_stack_frames_for_special_targets/clippy.toml b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_special_targets/clippy.toml new file mode 100644 index 0000000000000..02f3bc02dc4b7 --- /dev/null +++ b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_special_targets/clippy.toml @@ -0,0 +1,2 @@ +stack-size-threshold = 0 +allow-large-stack-frames-in-tests = false diff --git a/src/tools/clippy/tests/ui-toml/large_stack_frames_for_special_targets/large_stack_frames.rs b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_special_targets/large_stack_frames.rs new file mode 100644 index 0000000000000..cc01232ca4089 --- /dev/null +++ b/src/tools/clippy/tests/ui-toml/large_stack_frames_for_special_targets/large_stack_frames.rs @@ -0,0 +1,13 @@ +// This test checks if `clippy::large_stack_frames` is working correctly when encountering functions +// generated by special compiling targets like `--test`. +//@compile-flags: --test +//@check-pass + +#![warn(clippy::large_stack_frames)] + +#[cfg(test)] +#[expect(clippy::large_stack_frames)] +mod test { + #[test] + fn main_test() {} +} diff --git a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed index 419e62f92f469..3683e826aa925 100644 --- a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed +++ b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.fixed @@ -1,6 +1,7 @@ //@aux-build:proc_macro_derive.rs #![warn(clippy::nonstandard_macro_braces)] +#![allow(clippy::println_empty_string)] extern crate proc_macro_derive; extern crate quote; @@ -75,3 +76,16 @@ fn issue9913() { [0]; // separate statement, not indexing into the result of println. //~^^ nonstandard_macro_braces } + +fn issue15594() { + println!(); + println!(""); + println!(); + //~^ nonstandard_macro_braces + println!(""); + //~^ nonstandard_macro_braces + println!(); + //~^ nonstandard_macro_braces + println!(""); + //~^ nonstandard_macro_braces +} diff --git a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs index b0bbced4ea3c2..c1779dceff172 100644 --- a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs +++ b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs @@ -1,6 +1,7 @@ //@aux-build:proc_macro_derive.rs #![warn(clippy::nonstandard_macro_braces)] +#![allow(clippy::println_empty_string)] extern crate proc_macro_derive; extern crate quote; @@ -75,3 +76,16 @@ fn issue9913() { [0]; // separate statement, not indexing into the result of println. //~^^ nonstandard_macro_braces } + +fn issue15594() { + println!(); + println!(""); + println![]; + //~^ nonstandard_macro_braces + println![""]; + //~^ nonstandard_macro_braces + println! {}; + //~^ nonstandard_macro_braces + println! {""}; + //~^ nonstandard_macro_braces +} diff --git a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr index 87325f05c9bcd..2488f7fa01e5b 100644 --- a/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr +++ b/src/tools/clippy/tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.stderr @@ -1,5 +1,5 @@ error: use of irregular braces for `vec!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:44:13 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:45:13 | LL | let _ = vec! {1, 2, 3}; | ^^^^^^^^^^^^^^ help: consider writing: `vec![1, 2, 3]` @@ -8,31 +8,31 @@ LL | let _ = vec! {1, 2, 3}; = help: to override `-D warnings` add `#[allow(clippy::nonstandard_macro_braces)]` error: use of irregular braces for `format!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:46:13 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:47:13 | LL | let _ = format!["ugh {} stop being such a good compiler", "hello"]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `format!("ugh {} stop being such a good compiler", "hello")` error: use of irregular braces for `matches!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:48:13 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:49:13 | LL | let _ = matches!{{}, ()}; | ^^^^^^^^^^^^^^^^ help: consider writing: `matches!({}, ())` error: use of irregular braces for `quote!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:50:13 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:51:13 | LL | let _ = quote!(let x = 1;); | ^^^^^^^^^^^^^^^^^^ help: consider writing: `quote!{let x = 1;}` error: use of irregular braces for `quote::quote!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:52:13 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:53:13 | LL | let _ = quote::quote!(match match match); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `quote::quote!{match match match}` error: use of irregular braces for `vec!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:18:9 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:19:9 | LL | vec!{0, 0, 0} | ^^^^^^^^^^^^^ help: consider writing: `vec![0, 0, 0]` @@ -43,22 +43,46 @@ LL | let _ = test!(); // trigger when macro def is inside our own crate = note: this error originates in the macro `test` (in Nightly builds, run with -Z macro-backtrace for more info) error: use of irregular braces for `type_pos!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:62:12 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:63:12 | LL | let _: type_pos!(usize) = vec![]; | ^^^^^^^^^^^^^^^^ help: consider writing: `type_pos![usize]` error: use of irregular braces for `eprint!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:65:5 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:66:5 | LL | eprint!("test if user config overrides defaults"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `eprint!["test if user config overrides defaults"]` error: use of irregular braces for `println!` macro - --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:74:5 + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:75:5 | LL | println! {"hello world"} | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `println!("hello world");` -error: aborting due to 9 previous errors +error: use of irregular braces for `println!` macro + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:83:5 + | +LL | println![]; + | ^^^^^^^^^^ help: consider writing: `println!()` + +error: use of irregular braces for `println!` macro + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:85:5 + | +LL | println![""]; + | ^^^^^^^^^^^^ help: consider writing: `println!("")` + +error: use of irregular braces for `println!` macro + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:87:5 + | +LL | println! {}; + | ^^^^^^^^^^^ help: consider writing: `println!()` + +error: use of irregular braces for `println!` macro + --> tests/ui-toml/nonstandard_macro_braces/conf_nonstandard_macro_braces.rs:89:5 + | +LL | println! {""}; + | ^^^^^^^^^^^^^ help: consider writing: `println!("")` + +error: aborting due to 13 previous errors diff --git a/src/tools/clippy/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/src/tools/clippy/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index d5040f4a39bfd..e208bd510657e 100644 --- a/src/tools/clippy/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/src/tools/clippy/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -9,6 +9,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect allow-expect-in-consts allow-expect-in-tests allow-indexing-slicing-in-tests + allow-large-stack-frames-in-tests allow-mixed-uninlined-format-args allow-one-hash-in-raw-strings allow-panic-in-tests @@ -107,6 +108,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect allow-expect-in-consts allow-expect-in-tests allow-indexing-slicing-in-tests + allow-large-stack-frames-in-tests allow-mixed-uninlined-format-args allow-one-hash-in-raw-strings allow-panic-in-tests @@ -205,6 +207,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni allow-expect-in-consts allow-expect-in-tests allow-indexing-slicing-in-tests + allow-large-stack-frames-in-tests allow-mixed-uninlined-format-args allow-one-hash-in-raw-strings allow-panic-in-tests diff --git a/src/tools/clippy/tests/ui/borrow_as_ptr.fixed b/src/tools/clippy/tests/ui/borrow_as_ptr.fixed index bfe826508f36e..a4689a7840cea 100644 --- a/src/tools/clippy/tests/ui/borrow_as_ptr.fixed +++ b/src/tools/clippy/tests/ui/borrow_as_ptr.fixed @@ -1,6 +1,6 @@ //@aux-build:proc_macros.rs #![warn(clippy::borrow_as_ptr)] -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::ptr_offset_by_literal)] extern crate proc_macros; diff --git a/src/tools/clippy/tests/ui/borrow_as_ptr.rs b/src/tools/clippy/tests/ui/borrow_as_ptr.rs index ce248f157c6ef..d7468f37a3a49 100644 --- a/src/tools/clippy/tests/ui/borrow_as_ptr.rs +++ b/src/tools/clippy/tests/ui/borrow_as_ptr.rs @@ -1,6 +1,6 @@ //@aux-build:proc_macros.rs #![warn(clippy::borrow_as_ptr)] -#![allow(clippy::useless_vec)] +#![allow(clippy::useless_vec, clippy::ptr_offset_by_literal)] extern crate proc_macros; diff --git a/src/tools/clippy/tests/ui/checked_unwrap/simple_conditionals.rs b/src/tools/clippy/tests/ui/checked_unwrap/simple_conditionals.rs index c6476a7507a1d..bab20b091d389 100644 --- a/src/tools/clippy/tests/ui/checked_unwrap/simple_conditionals.rs +++ b/src/tools/clippy/tests/ui/checked_unwrap/simple_conditionals.rs @@ -472,3 +472,22 @@ fn issue15321() { //~^ unnecessary_unwrap } } + +mod issue16188 { + struct Foo { + value: Option, + } + + impl Foo { + pub fn bar(&mut self) { + let print_value = |v: i32| { + println!("{}", v); + }; + + if self.value.is_none() { + self.value = Some(10); + print_value(self.value.unwrap()); + } + } + } +} diff --git a/src/tools/clippy/tests/ui/crashes/ice-4579.rs b/src/tools/clippy/tests/ui/crashes/ice-4579.rs index 14c8113e315b7..a2592e74c66d1 100644 --- a/src/tools/clippy/tests/ui/crashes/ice-4579.rs +++ b/src/tools/clippy/tests/ui/crashes/ice-4579.rs @@ -1,6 +1,6 @@ //@ check-pass -#![allow(clippy::single_match)] +#![allow(clippy::single_match, clippy::ptr_offset_by_literal)] use std::ptr; diff --git a/src/tools/clippy/tests/ui/decimal_bitwise_operands.rs b/src/tools/clippy/tests/ui/decimal_bitwise_operands.rs new file mode 100644 index 0000000000000..f1c053bb4358a --- /dev/null +++ b/src/tools/clippy/tests/ui/decimal_bitwise_operands.rs @@ -0,0 +1,133 @@ +#![allow( + clippy::erasing_op, + clippy::no_effect, + clippy::unnecessary_operation, + clippy::unnecessary_cast, + clippy::op_ref +)] +#![warn(clippy::decimal_bitwise_operands)] + +macro_rules! bitwise_op { + ($x:expr, $y:expr) => { + $x & $y; + }; +} + +pub const SOME_CONST: i32 = 12345; + +fn main() { + let mut x = 0; + // BAD: Bitwise operation, decimal literal, one literal + x & 9_8765_4321; //~ decimal_bitwise_operands + x & 100_i32; //~ decimal_bitwise_operands + x | (/* comment */99); //~ decimal_bitwise_operands + x ^ (99); //~ decimal_bitwise_operands + x &= 99; //~ decimal_bitwise_operands + x |= { 99 }; //~ decimal_bitwise_operands + x |= { { 99 } }; //~ decimal_bitwise_operands + x |= { + 0b1000; + 99 //~ decimal_bitwise_operands + }; + x ^= (99); //~ decimal_bitwise_operands + + // BAD: Bitwise operation, decimal literal, two literals + 0b1010 & 99; //~ decimal_bitwise_operands + 0b1010 | (99); //~ decimal_bitwise_operands + 0b1010 ^ (/* comment */99); //~ decimal_bitwise_operands + 99 & 0b1010; //~ decimal_bitwise_operands + (99) | 0b1010; //~ decimal_bitwise_operands + (/* comment */99) ^ 0b1010; //~ decimal_bitwise_operands + 0xD | { 99 }; //~ decimal_bitwise_operands + 88 & 99; + //~^ decimal_bitwise_operands + //~| decimal_bitwise_operands + 37 & 38 & 39; + //~^ decimal_bitwise_operands + //~| decimal_bitwise_operands + //~| decimal_bitwise_operands + + // GOOD: Bitwise operation, binary/hex/octal literal, one literal + x & 0b1010; + x | 0b1010; + x ^ 0b1010; + x &= 0b1010; + x |= 0b1010; + x ^= 0b1010; + x & 0xD; + x & 0o77; + x | 0o123; + x ^ 0o377; + x &= 0o777; + x |= 0o7; + x ^= 0o70; + + // GOOD: Bitwise operation, binary/hex/octal literal, two literals + 0b1010 & 0b1101; + 0xD ^ 0xF; + 0o377 ^ 0o77; + 0b1101 ^ 0xFF; + + // GOOD: Numeric operation, any literal + x += 99; + x -= 0b1010; + x *= 0xD; + 99 + 99; + 0b1010 - 0b1101; + 0xD * 0xD; + + // BAD: Unary, cast and reference, decimal literal + x & !100; //~ decimal_bitwise_operands + x & -100; //~ decimal_bitwise_operands + x & (100 as i32); //~ decimal_bitwise_operands + x & &100; //~ decimal_bitwise_operands + + // GOOD: Unary, cast and reference, non-decimal literal + x & !0b1101; + x & -0xD; + x & (0o333 as i32); + x & &0b1010; + + // GOOD: Bitwise operation, variables only + let y = 0; + x & y; + x &= y; + x + y; + x += y; + + // GOOD: Macro expansion (should be ignored) + bitwise_op!(x, 123); + bitwise_op!(0b1010, 123); + + // GOOD: Using const (should be ignored) + x & SOME_CONST; + x |= SOME_CONST; + + // GOOD: Parenthesized binary/hex literal (should not trigger lint) + x & (0b1111); + x |= (0b1010); + x ^ (/* comment */0b1100); + (0xFF) & x; + + // GOOD: Power of two and power of two minus one + x & 16; // 2^4 + x | (31); // 2^5 - 1 + x ^ 0x40; // 2^6 (hex) + x ^= 7; // 2^3 - 1 + + // GOOD: Bitwise operation, single digit decimal literal + 5 & 9; + x ^ 6; + x ^= 7; + + // GOOD: More complex expressions + (x + 1) & 0xFF; + (x * 2) | (y & 0xF); + (x ^ y) & 0b11110000; + x | (1 << 9); + + // GOOD: Special cases + x & 0; // All bits off + x | !0; // All bits on + x ^ 1; // Toggle LSB +} diff --git a/src/tools/clippy/tests/ui/decimal_bitwise_operands.stderr b/src/tools/clippy/tests/ui/decimal_bitwise_operands.stderr new file mode 100644 index 0000000000000..1b2b7bb71b69c --- /dev/null +++ b/src/tools/clippy/tests/ui/decimal_bitwise_operands.stderr @@ -0,0 +1,204 @@ +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:21:9 + | +LL | x & 9_8765_4321; + | ^^^^^^^^^^^ + | + = help: use binary (0b11_1010_1101_1110_0110_1000_1011_0001), hex (0x3ade_68b1), or octal (0o7_267_464_261) notation for better readability + = note: `-D clippy::decimal-bitwise-operands` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::decimal_bitwise_operands)]` + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:22:9 + | +LL | x & 100_i32; + | ^^^^^^^ + | + = help: use binary (0b110_0100_i32), hex (0x0064_i32), or octal (0o144_i32) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:23:23 + | +LL | x | (/* comment */99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:24:10 + | +LL | x ^ (99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:25:10 + | +LL | x &= 99; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:26:12 + | +LL | x |= { 99 }; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:27:14 + | +LL | x |= { { 99 } }; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:30:9 + | +LL | 99 + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:32:11 + | +LL | x ^= (99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:35:14 + | +LL | 0b1010 & 99; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:36:15 + | +LL | 0b1010 | (99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:37:28 + | +LL | 0b1010 ^ (/* comment */99); + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:38:5 + | +LL | 99 & 0b1010; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:39:6 + | +LL | (99) | 0b1010; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:40:19 + | +LL | (/* comment */99) ^ 0b1010; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:41:13 + | +LL | 0xD | { 99 }; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:42:5 + | +LL | 88 & 99; + | ^^ + | + = help: use binary (0b101_1000), hex (0x0058), or octal (0o130) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:42:10 + | +LL | 88 & 99; + | ^^ + | + = help: use binary (0b110_0011), hex (0x0063), or octal (0o143) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:45:15 + | +LL | 37 & 38 & 39; + | ^^ + | + = help: use binary (0b10_0111), hex (0x0027), or octal (0o47) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:45:5 + | +LL | 37 & 38 & 39; + | ^^ + | + = help: use binary (0b10_0101), hex (0x0025), or octal (0o45) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:45:10 + | +LL | 37 & 38 & 39; + | ^^ + | + = help: use binary (0b10_0110), hex (0x0026), or octal (0o46) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:80:10 + | +LL | x & !100; + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:81:10 + | +LL | x & -100; + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:82:10 + | +LL | x & (100 as i32); + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: using decimal literal for bitwise operation + --> tests/ui/decimal_bitwise_operands.rs:83:10 + | +LL | x & &100; + | ^^^ + | + = help: use binary (0b110_0100), hex (0x0064), or octal (0o144) notation for better readability + +error: aborting due to 25 previous errors + diff --git a/src/tools/clippy/tests/ui/deprecated.stderr b/src/tools/clippy/tests/ui/deprecated.stderr index cd225da611c48..fa70371b926fe 100644 --- a/src/tools/clippy/tests/ui/deprecated.stderr +++ b/src/tools/clippy/tests/ui/deprecated.stderr @@ -61,7 +61,7 @@ error: lint `clippy::should_assert_eq` has been removed: `assert!(a == b)` can n LL | #![warn(clippy::should_assert_eq)] | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: lint `clippy::string_to_string` has been removed: `clippy:implicit_clone` covers those cases +error: lint `clippy::string_to_string` has been removed: `clippy::implicit_clone` covers those cases --> tests/ui/deprecated.rs:15:9 | LL | #![warn(clippy::string_to_string)] diff --git a/src/tools/clippy/tests/ui/entry.fixed b/src/tools/clippy/tests/ui/entry.fixed index f2df9f0204ea6..1e36ca4f1f096 100644 --- a/src/tools/clippy/tests/ui/entry.fixed +++ b/src/tools/clippy/tests/ui/entry.fixed @@ -248,4 +248,28 @@ mod issue14449 { } } +// Don't suggest when it would cause `MutexGuard` to be held across an await point. +mod issue_16173 { + use std::collections::HashMap; + use std::sync::Mutex; + + async fn f() {} + + async fn foo() { + let mu_map = Mutex::new(HashMap::new()); + if !mu_map.lock().unwrap().contains_key(&0) { + f().await; + mu_map.lock().unwrap().insert(0, 0); + } + + if mu_map.lock().unwrap().contains_key(&1) { + todo!(); + } else { + mu_map.lock().unwrap().insert(1, 42); + todo!(); + f().await; + } + } +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/entry.rs b/src/tools/clippy/tests/ui/entry.rs index 166eea417ac23..b3da0ef3ffd69 100644 --- a/src/tools/clippy/tests/ui/entry.rs +++ b/src/tools/clippy/tests/ui/entry.rs @@ -254,4 +254,28 @@ mod issue14449 { } } +// Don't suggest when it would cause `MutexGuard` to be held across an await point. +mod issue_16173 { + use std::collections::HashMap; + use std::sync::Mutex; + + async fn f() {} + + async fn foo() { + let mu_map = Mutex::new(HashMap::new()); + if !mu_map.lock().unwrap().contains_key(&0) { + f().await; + mu_map.lock().unwrap().insert(0, 0); + } + + if mu_map.lock().unwrap().contains_key(&1) { + todo!(); + } else { + mu_map.lock().unwrap().insert(1, 42); + todo!(); + f().await; + } + } +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/len_without_is_empty.rs b/src/tools/clippy/tests/ui/len_without_is_empty.rs index 011833072d762..509348628dd67 100644 --- a/src/tools/clippy/tests/ui/len_without_is_empty.rs +++ b/src/tools/clippy/tests/ui/len_without_is_empty.rs @@ -473,4 +473,16 @@ impl Alias2 { } } +// Issue #16190 +pub struct RefMutLenButRefIsEmpty; +impl RefMutLenButRefIsEmpty { + pub fn len(&mut self) -> usize { + todo!() + } + + pub fn is_empty(&self) -> bool { + todo!() + } +} + fn main() {} diff --git a/src/tools/clippy/tests/ui/manual_ilog2.fixed b/src/tools/clippy/tests/ui/manual_ilog2.fixed new file mode 100644 index 0000000000000..a0f6d9392c303 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_ilog2.fixed @@ -0,0 +1,32 @@ +//@aux-build:proc_macros.rs +#![warn(clippy::manual_ilog2)] +#![allow(clippy::unnecessary_operation)] + +use proc_macros::{external, with_span}; + +fn foo(a: u32, b: u64) { + a.ilog2(); //~ manual_ilog2 + a.ilog2(); //~ manual_ilog2 + + b.ilog2(); //~ manual_ilog2 + 64 - b.leading_zeros(); // No lint because manual ilog2 is `BIT_WIDTH - 1 - x.leading_zeros()` + + // don't lint when macros are involved + macro_rules! two { + () => { + 2 + }; + }; + + macro_rules! thirty_one { + () => { + 31 + }; + }; + + a.ilog(two!()); + thirty_one!() - a.leading_zeros(); + + external!($a.ilog(2)); + with_span!(span; a.ilog(2)); +} diff --git a/src/tools/clippy/tests/ui/manual_ilog2.rs b/src/tools/clippy/tests/ui/manual_ilog2.rs new file mode 100644 index 0000000000000..bd4b5d9d3c0d7 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_ilog2.rs @@ -0,0 +1,32 @@ +//@aux-build:proc_macros.rs +#![warn(clippy::manual_ilog2)] +#![allow(clippy::unnecessary_operation)] + +use proc_macros::{external, with_span}; + +fn foo(a: u32, b: u64) { + 31 - a.leading_zeros(); //~ manual_ilog2 + a.ilog(2); //~ manual_ilog2 + + 63 - b.leading_zeros(); //~ manual_ilog2 + 64 - b.leading_zeros(); // No lint because manual ilog2 is `BIT_WIDTH - 1 - x.leading_zeros()` + + // don't lint when macros are involved + macro_rules! two { + () => { + 2 + }; + }; + + macro_rules! thirty_one { + () => { + 31 + }; + }; + + a.ilog(two!()); + thirty_one!() - a.leading_zeros(); + + external!($a.ilog(2)); + with_span!(span; a.ilog(2)); +} diff --git a/src/tools/clippy/tests/ui/manual_ilog2.stderr b/src/tools/clippy/tests/ui/manual_ilog2.stderr new file mode 100644 index 0000000000000..7c9694f353301 --- /dev/null +++ b/src/tools/clippy/tests/ui/manual_ilog2.stderr @@ -0,0 +1,23 @@ +error: manually reimplementing `ilog2` + --> tests/ui/manual_ilog2.rs:8:5 + | +LL | 31 - a.leading_zeros(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `a.ilog2()` + | + = note: `-D clippy::manual-ilog2` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_ilog2)]` + +error: manually reimplementing `ilog2` + --> tests/ui/manual_ilog2.rs:9:5 + | +LL | a.ilog(2); + | ^^^^^^^^^ help: try: `a.ilog2()` + +error: manually reimplementing `ilog2` + --> tests/ui/manual_ilog2.rs:11:5 + | +LL | 63 - b.leading_zeros(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `b.ilog2()` + +error: aborting due to 3 previous errors + diff --git a/src/tools/clippy/tests/ui/manual_saturating_arithmetic.fixed b/src/tools/clippy/tests/ui/manual_saturating_arithmetic.fixed index 304be05f6c4cf..8dd142b2c79ac 100644 --- a/src/tools/clippy/tests/ui/manual_saturating_arithmetic.fixed +++ b/src/tools/clippy/tests/ui/manual_saturating_arithmetic.fixed @@ -58,3 +58,13 @@ fn main() { let _ = 1i8.checked_sub(1).unwrap_or(127); // ok let _ = 1i8.checked_sub(-1).unwrap_or(-128); // ok } + +fn issue15655() { + let _ = 5u32.saturating_sub(1u32); //~ manual_saturating_arithmetic + let _ = 5u32.checked_add(1u32).unwrap_or_default(); // ok + let _ = 5u32.checked_mul(1u32).unwrap_or_default(); // ok + + let _ = 5i32.checked_sub(1i32).unwrap_or_default(); // ok + let _ = 5i32.checked_add(1i32).unwrap_or_default(); // ok + let _ = 5i32.checked_mul(1i32).unwrap_or_default(); // ok +} diff --git a/src/tools/clippy/tests/ui/manual_saturating_arithmetic.rs b/src/tools/clippy/tests/ui/manual_saturating_arithmetic.rs index c2b570e974ac8..9cc8bc42410da 100644 --- a/src/tools/clippy/tests/ui/manual_saturating_arithmetic.rs +++ b/src/tools/clippy/tests/ui/manual_saturating_arithmetic.rs @@ -73,3 +73,13 @@ fn main() { let _ = 1i8.checked_sub(1).unwrap_or(127); // ok let _ = 1i8.checked_sub(-1).unwrap_or(-128); // ok } + +fn issue15655() { + let _ = 5u32.checked_sub(1u32).unwrap_or_default(); //~ manual_saturating_arithmetic + let _ = 5u32.checked_add(1u32).unwrap_or_default(); // ok + let _ = 5u32.checked_mul(1u32).unwrap_or_default(); // ok + + let _ = 5i32.checked_sub(1i32).unwrap_or_default(); // ok + let _ = 5i32.checked_add(1i32).unwrap_or_default(); // ok + let _ = 5i32.checked_mul(1i32).unwrap_or_default(); // ok +} diff --git a/src/tools/clippy/tests/ui/manual_saturating_arithmetic.stderr b/src/tools/clippy/tests/ui/manual_saturating_arithmetic.stderr index 2f006a3ae1702..aec0f0a164198 100644 --- a/src/tools/clippy/tests/ui/manual_saturating_arithmetic.stderr +++ b/src/tools/clippy/tests/ui/manual_saturating_arithmetic.stderr @@ -165,5 +165,11 @@ LL | | .checked_sub(-1) LL | | .unwrap_or(170_141_183_460_469_231_731_687_303_715_884_105_727); | |_______________________________________________________________________^ help: consider using `saturating_sub`: `1i128.saturating_sub(-1)` -error: aborting due to 24 previous errors +error: manual saturating arithmetic + --> tests/ui/manual_saturating_arithmetic.rs:78:13 + | +LL | let _ = 5u32.checked_sub(1u32).unwrap_or_default(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `saturating_sub`: `5u32.saturating_sub(1u32)` + +error: aborting due to 25 previous errors diff --git a/src/tools/clippy/tests/ui/match_like_matches_macro.fixed b/src/tools/clippy/tests/ui/match_like_matches_macro.fixed index a1c95e8a94f14..dad59c1ce6e4d 100644 --- a/src/tools/clippy/tests/ui/match_like_matches_macro.fixed +++ b/src/tools/clippy/tests/ui/match_like_matches_macro.fixed @@ -223,3 +223,10 @@ fn msrv_1_42() { let _y = matches!(Some(5), Some(0)); //~^^^^ match_like_matches_macro } + +#[expect(clippy::option_option)] +fn issue15841(opt: Option>>, value: i32) { + // Lint: no if-let _in the guard_ + let _ = matches!(opt, Some(first) if (if let Some(second) = first { true } else { todo!() })); + //~^^^^ match_like_matches_macro +} diff --git a/src/tools/clippy/tests/ui/match_like_matches_macro.rs b/src/tools/clippy/tests/ui/match_like_matches_macro.rs index eb419ba5bf8de..94bc6433e5cb7 100644 --- a/src/tools/clippy/tests/ui/match_like_matches_macro.rs +++ b/src/tools/clippy/tests/ui/match_like_matches_macro.rs @@ -267,3 +267,13 @@ fn msrv_1_42() { }; //~^^^^ match_like_matches_macro } + +#[expect(clippy::option_option)] +fn issue15841(opt: Option>>, value: i32) { + // Lint: no if-let _in the guard_ + let _ = match opt { + Some(first) if (if let Some(second) = first { true } else { todo!() }) => true, + _ => false, + }; + //~^^^^ match_like_matches_macro +} diff --git a/src/tools/clippy/tests/ui/match_like_matches_macro.stderr b/src/tools/clippy/tests/ui/match_like_matches_macro.stderr index ae277ce4dca68..a8e352461dbb6 100644 --- a/src/tools/clippy/tests/ui/match_like_matches_macro.stderr +++ b/src/tools/clippy/tests/ui/match_like_matches_macro.stderr @@ -6,10 +6,18 @@ LL | let _y = match x { LL | | Some(0) => true, LL | | _ => false, LL | | }; - | |_____^ help: try: `matches!(x, Some(0))` + | |_____^ | = note: `-D clippy::match-like-matches-macro` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::match_like_matches_macro)]` +help: use `matches!` directly + | +LL - let _y = match x { +LL - Some(0) => true, +LL - _ => false, +LL - }; +LL + let _y = matches!(x, Some(0)); + | error: redundant pattern matching, consider using `is_some()` --> tests/ui/match_like_matches_macro.rs:20:14 @@ -42,13 +50,28 @@ LL | let _zz = match x { LL | | Some(r) if r == 0 => false, LL | | _ => true, LL | | }; - | |_____^ help: try: `!matches!(x, Some(r) if r == 0)` + | |_____^ + | +help: use `matches!` directly + | +LL - let _zz = match x { +LL - Some(r) if r == 0 => false, +LL - _ => true, +LL - }; +LL + let _zz = !matches!(x, Some(r) if r == 0); + | -error: if let .. else expression looks like `matches!` macro +error: `if let .. else` expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:41:16 | LL | let _zzz = if let Some(5) = x { true } else { false }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(x, Some(5))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `matches!` directly + | +LL - let _zzz = if let Some(5) = x { true } else { false }; +LL + let _zzz = matches!(x, Some(5)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:66:20 @@ -59,7 +82,17 @@ LL | | E::A(_) => true, LL | | E::B(_) => true, LL | | _ => false, LL | | }; - | |_________^ help: try: `matches!(x, E::A(_) | E::B(_))` + | |_________^ + | +help: use `matches!` directly + | +LL - let _ans = match x { +LL - E::A(_) => true, +LL - E::B(_) => true, +LL - _ => false, +LL - }; +LL + let _ans = matches!(x, E::A(_) | E::B(_)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:77:20 @@ -71,7 +104,19 @@ LL | | true ... | LL | | _ => false, LL | | }; - | |_________^ help: try: `matches!(x, E::A(_) | E::B(_))` + | |_________^ + | +help: use `matches!` directly + | +LL - let _ans = match x { +LL - E::A(_) => { +LL - true +LL - } +LL - E::B(_) => true, +LL - _ => false, +LL - }; +LL + let _ans = matches!(x, E::A(_) | E::B(_)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:88:20 @@ -82,7 +127,17 @@ LL | | E::B(_) => false, LL | | E::C => false, LL | | _ => true, LL | | }; - | |_________^ help: try: `!matches!(x, E::B(_) | E::C)` + | |_________^ + | +help: use `matches!` directly + | +LL - let _ans = match x { +LL - E::B(_) => false, +LL - E::C => false, +LL - _ => true, +LL - }; +LL + let _ans = !matches!(x, E::B(_) | E::C); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:149:18 @@ -92,7 +147,16 @@ LL | let _z = match &z { LL | | Some(3) => true, LL | | _ => false, LL | | }; - | |_________^ help: try: `matches!(z, Some(3))` + | |_________^ + | +help: use `matches!` directly + | +LL - let _z = match &z { +LL - Some(3) => true, +LL - _ => false, +LL - }; +LL + let _z = matches!(z, Some(3)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:159:18 @@ -102,7 +166,16 @@ LL | let _z = match &z { LL | | Some(3) => true, LL | | _ => false, LL | | }; - | |_________^ help: try: `matches!(&z, Some(3))` + | |_________^ + | +help: use `matches!` directly + | +LL - let _z = match &z { +LL - Some(3) => true, +LL - _ => false, +LL - }; +LL + let _z = matches!(&z, Some(3)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:177:21 @@ -112,7 +185,16 @@ LL | let _ = match &z { LL | | AnEnum::X => true, LL | | _ => false, LL | | }; - | |_____________^ help: try: `matches!(&z, AnEnum::X)` + | |_____________^ + | +help: use `matches!` directly + | +LL - let _ = match &z { +LL - AnEnum::X => true, +LL - _ => false, +LL - }; +LL + let _ = matches!(&z, AnEnum::X); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:192:20 @@ -122,7 +204,16 @@ LL | let _res = match &val { LL | | &Some(ref _a) => true, LL | | _ => false, LL | | }; - | |_________^ help: try: `matches!(&val, &Some(ref _a))` + | |_________^ + | +help: use `matches!` directly + | +LL - let _res = match &val { +LL - &Some(ref _a) => true, +LL - _ => false, +LL - }; +LL + let _res = matches!(&val, &Some(ref _a)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:205:20 @@ -132,7 +223,16 @@ LL | let _res = match &val { LL | | &Some(ref _a) => true, LL | | _ => false, LL | | }; - | |_________^ help: try: `matches!(&val, &Some(ref _a))` + | |_________^ + | +help: use `matches!` directly + | +LL - let _res = match &val { +LL - &Some(ref _a) => true, +LL - _ => false, +LL - }; +LL + let _res = matches!(&val, &Some(ref _a)); + | error: match expression looks like `matches!` macro --> tests/ui/match_like_matches_macro.rs:264:14 @@ -142,7 +242,35 @@ LL | let _y = match Some(5) { LL | | Some(0) => true, LL | | _ => false, LL | | }; - | |_____^ help: try: `matches!(Some(5), Some(0))` + | |_____^ + | +help: use `matches!` directly + | +LL - let _y = match Some(5) { +LL - Some(0) => true, +LL - _ => false, +LL - }; +LL + let _y = matches!(Some(5), Some(0)); + | + +error: match expression looks like `matches!` macro + --> tests/ui/match_like_matches_macro.rs:274:13 + | +LL | let _ = match opt { + | _____________^ +LL | | Some(first) if (if let Some(second) = first { true } else { todo!() }) => true, +LL | | _ => false, +LL | | }; + | |_____^ + | +help: use `matches!` directly + | +LL - let _ = match opt { +LL - Some(first) if (if let Some(second) = first { true } else { todo!() }) => true, +LL - _ => false, +LL - }; +LL + let _ = matches!(opt, Some(first) if (if let Some(second) = first { true } else { todo!() })); + | -error: aborting due to 14 previous errors +error: aborting due to 15 previous errors diff --git a/src/tools/clippy/tests/ui/match_like_matches_macro_if_let_guard.rs b/src/tools/clippy/tests/ui/match_like_matches_macro_if_let_guard.rs new file mode 100644 index 0000000000000..b596d36072e53 --- /dev/null +++ b/src/tools/clippy/tests/ui/match_like_matches_macro_if_let_guard.rs @@ -0,0 +1,51 @@ +//@check-pass +#![warn(clippy::match_like_matches_macro)] +#![feature(if_let_guard)] + +#[expect(clippy::option_option)] +fn issue15841(opt: Option>>, value: i32) { + let _ = match opt { + Some(first) + if let Some(second) = first + && let Some(third) = second + && third == value => + { + true + }, + _ => false, + }; + + // if-let is the second if + let _ = match opt { + Some(first) + if first.is_some() + && let Some(second) = first => + { + true + }, + _ => false, + }; + + // if-let is the third if + let _ = match opt { + Some(first) + if first.is_some() + && first.is_none() + && let Some(second) = first => + { + true + }, + _ => false, + }; + + // don't get confused by `or`s + let _ = match opt { + Some(first) + if (first.is_some() || first.is_none()) + && let Some(second) = first => + { + true + }, + _ => false, + }; +} diff --git a/src/tools/clippy/tests/ui/missing_asserts_for_indexing.stderr b/src/tools/clippy/tests/ui/missing_asserts_for_indexing.stderr index b686eda7530a0..d5b5455343e43 100644 --- a/src/tools/clippy/tests/ui/missing_asserts_for_indexing.stderr +++ b/src/tools/clippy/tests/ui/missing_asserts_for_indexing.stderr @@ -1,407 +1,191 @@ error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:30:5 | -LL | assert!(v.len() < 5); - | -------------------- help: provide the highest index that is indexed with: `assert!(v.len() > 4)` LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ ^^^^ ^^^^ ^^^^ ^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:30:5 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:30:12 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:30:19 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:30:26 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:30:33 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ = note: asserting the length before indexing will elide bounds checks = note: `-D clippy::missing-asserts-for-indexing` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::missing_asserts_for_indexing)]` - -error: indexing into a slice multiple times with an `assert` that does not cover the highest index - --> tests/ui/missing_asserts_for_indexing.rs:36:5 +help: provide the highest index that is indexed with | -LL | assert!(v.len() <= 5); - | --------------------- help: provide the highest index that is indexed with: `assert!(v.len() > 4)` -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL - assert!(v.len() < 5); +LL + assert!(v.len() > 4); | -note: slice indexed here + +error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:36:5 | LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:36:12 + | ^^^^ ^^^^ ^^^^ ^^^^ ^^^^ | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:36:19 +help: provide the highest index that is indexed with | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:36:26 +LL - assert!(v.len() <= 5); +LL + assert!(v.len() > 4); | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:36:33 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:42:5 | -LL | assert!(v.len() > 3); - | -------------------- help: provide the highest index that is indexed with: `assert!(v.len() > 4)` LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:42:5 + | ^^^^ ^^^^ ^^^^ ^^^^ ^^^^ | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:42:12 +help: provide the highest index that is indexed with | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:42:19 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:42:26 +LL - assert!(v.len() > 3); +LL + assert!(v.len() > 4); | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:42:33 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:48:5 | -LL | assert!(v.len() >= 4); - | --------------------- help: provide the highest index that is indexed with: `assert!(v.len() > 4)` -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:48:5 - | LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:48:12 + | ^^^^ ^^^^ ^^^^ ^^^^ ^^^^ | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:48:19 +help: provide the highest index that is indexed with | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:48:26 +LL - assert!(v.len() >= 4); +LL + assert!(v.len() > 4); | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:48:33 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:66:13 | -LL | assert!(v.len() >= 3); - | --------------------- help: provide the highest index that is indexed with: `assert!(v.len() > 3)` -LL | let _ = v[0]; - | _____________^ -... | -LL | | let _ = v[1..4]; - | |___________________^ - | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:66:13 - | LL | let _ = v[0]; | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:69:13 - | +... LL | let _ = v[1..4]; | ^^^^^^^ - = note: asserting the length before indexing will elide bounds checks - -error: indexing into a slice multiple times with an `assert` that does not cover the highest index - --> tests/ui/missing_asserts_for_indexing.rs:81:13 | -LL | assert!(v.len() >= 4); - | --------------------- help: provide the highest index that is indexed with: `assert!(v.len() > 4)` -LL | let _ = v[0]; - | _____________^ -... | -LL | | let _ = v[1..=4]; - | |____________________^ +help: provide the highest index that is indexed with | -note: slice indexed here +LL - assert!(v.len() >= 3); +LL + assert!(v.len() > 3); + | + +error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:81:13 | LL | let _ = v[0]; | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:84:13 - | +... LL | let _ = v[1..=4]; | ^^^^^^^^ - = note: asserting the length before indexing will elide bounds checks + | +help: provide the highest index that is indexed with + | +LL - assert!(v.len() >= 4); +LL + assert!(v.len() > 4); + | error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:97:13 | -LL | assert!(v1.len() >= 12); - | ----------------------- help: provide the highest index that is indexed with: `assert!(v1.len() > 12)` -LL | assert!(v2.len() >= 15); LL | let _ = v1[0] + v1[12]; - | ^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:97:13 +help: provide the highest index that is indexed with | -LL | let _ = v1[0] + v1[12]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:97:21 +LL - assert!(v1.len() >= 12); +LL + assert!(v1.len() > 12); | -LL | let _ = v1[0] + v1[12]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:100:13 | -LL | assert!(v2.len() >= 15); - | ----------------------- help: provide the highest index that is indexed with: `assert!(v2.len() > 15)` -... LL | let _ = v2[5] + v2[15]; - | ^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:100:13 +help: provide the highest index that is indexed with | -LL | let _ = v2[5] + v2[15]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:100:21 +LL - assert!(v2.len() >= 15); +LL + assert!(v2.len() > 15); | -LL | let _ = v2[5] + v2[15]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:106:13 | -LL | assert!(v1.len() >= 12); - | ----------------------- help: provide the highest index that is indexed with: `assert!(v1.len() > 12)` -LL | assert!(v2.len() > 15); LL | let _ = v1[0] + v1[12]; - | ^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:106:13 +help: provide the highest index that is indexed with | -LL | let _ = v1[0] + v1[12]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:106:21 +LL - assert!(v1.len() >= 12); +LL + assert!(v1.len() > 12); | -LL | let _ = v1[0] + v1[12]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:131:13 | -LL | assert!(v1.len() == 2); - | ---------------------- help: provide the highest index that is indexed with: `assert!(v1.len() == 3)` -... LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^ ^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:131:13 +help: provide the highest index that is indexed with | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:131:21 - | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:131:29 +LL - assert!(v1.len() == 2); +LL + assert!(v1.len() == 3); | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:136:13 | -LL | assert!(2 == v3.len()); - | ---------------------- help: provide the highest index that is indexed with: `assert!(v3.len() == 3)` -... LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^ ^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:136:13 +help: provide the highest index that is indexed with | -LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:136:21 - | -LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:136:29 +LL - assert!(2 == v3.len()); +LL + assert!(v3.len() == 3); | -LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:158:13 | -LL | assert_eq!(v1.len(), 2); - | ----------------------- help: provide the highest index that is indexed with: `assert_eq!(v1.len(), 3)` -... LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^ ^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:158:13 +help: provide the highest index that is indexed with | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:158:21 - | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:158:29 +LL - assert_eq!(v1.len(), 2); +LL + assert_eq!(v1.len(), 3); | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:163:13 | -LL | assert_eq!(2, v3.len()); - | ----------------------- help: provide the highest index that is indexed with: `assert_eq!(v3.len(), 3)` -... LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^ ^^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:163:13 +help: provide the highest index that is indexed with | -LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:163:21 - | -LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:163:29 +LL - assert_eq!(2, v3.len()); +LL + assert_eq!(v3.len(), 3); | -LL | let _ = v3[0] + v3[1] + v3[2]; - | ^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:172:17 | -LL | assert_eq!(v.len(), 2); - | ---------------------- help: provide the highest index that is indexed with: `assert_eq!(v.len(), 3)` LL | let _ = v[0] + v[1] + v[2]; - | ^^^^^^^^^^^^^^^^^^ + | ^^^^ ^^^^ ^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:172:17 +help: provide the highest index that is indexed with | -LL | let _ = v[0] + v[1] + v[2]; - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:172:24 +LL - assert_eq!(v.len(), 2); +LL + assert_eq!(v.len(), 3); | -LL | let _ = v[0] + v[1] + v[2]; - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:172:31 - | -LL | let _ = v[0] + v[1] + v[2]; - | ^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times with an `assert` that does not cover the highest index --> tests/ui/missing_asserts_for_indexing.rs:178:17 | -LL | debug_assert_eq!(v.len(), 2); - | ---------------------------- help: provide the highest index that is indexed with: `debug_assert_eq!(v.len(), 3)` LL | let _ = v[0] + v[1] + v[2]; - | ^^^^^^^^^^^^^^^^^^ + | ^^^^ ^^^^ ^^^^ | -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:178:17 +help: provide the highest index that is indexed with | -LL | let _ = v[0] + v[1] + v[2]; - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:178:24 +LL - debug_assert_eq!(v.len(), 2); +LL + debug_assert_eq!(v.len(), 3); | -LL | let _ = v[0] + v[1] + v[2]; - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing.rs:178:31 - | -LL | let _ = v[0] + v[1] + v[2]; - | ^^^^ - = note: asserting the length before indexing will elide bounds checks error: aborting due to 15 previous errors diff --git a/src/tools/clippy/tests/ui/missing_asserts_for_indexing_unfixable.stderr b/src/tools/clippy/tests/ui/missing_asserts_for_indexing_unfixable.stderr index a17ad02321386..2929646494a41 100644 --- a/src/tools/clippy/tests/ui/missing_asserts_for_indexing_unfixable.stderr +++ b/src/tools/clippy/tests/ui/missing_asserts_for_indexing_unfixable.stderr @@ -2,34 +2,9 @@ error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:5:5 | LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^ ^^^^ ^^^^ ^^^^ ^^^^ | = help: consider asserting the length before indexing: `assert!(v.len() > 4);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:5:5 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:5:12 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:5:19 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:5:26 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:5:33 - | -LL | v[0] + v[1] + v[2] + v[3] + v[4] - | ^^^^ = note: asserting the length before indexing will elide bounds checks = note: `-D clippy::missing-asserts-for-indexing` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::missing_asserts_for_indexing)]` @@ -37,191 +12,82 @@ LL | v[0] + v[1] + v[2] + v[3] + v[4] error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:10:13 | -LL | let _ = v[0]; - | _____________^ -... | -LL | | let _ = v[1..4]; - | |___________________^ - | - = help: consider asserting the length before indexing: `assert!(v.len() > 3);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:10:13 - | LL | let _ = v[0]; | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:13:13 - | +... LL | let _ = v[1..4]; | ^^^^^^^ - = note: asserting the length before indexing will elide bounds checks + | + = help: consider asserting the length before indexing: `assert!(v.len() > 3);` error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:17:13 | -LL | let a = v[0]; - | _____________^ -LL | | -LL | | -LL | | let b = v[1]; -LL | | let c = v[2]; - | |________________^ - | - = help: consider asserting the length before indexing: `assert!(v.len() > 2);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:17:13 - | LL | let a = v[0]; | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:20:13 - | +... LL | let b = v[1]; | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:21:13 - | LL | let c = v[2]; | ^^^^ - = note: asserting the length before indexing will elide bounds checks + | + = help: consider asserting the length before indexing: `assert!(v.len() > 2);` error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:26:13 | LL | let _ = v1[0] + v1[12]; - | ^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^^ | = help: consider asserting the length before indexing: `assert!(v1.len() > 12);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:26:13 - | -LL | let _ = v1[0] + v1[12]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:26:21 - | -LL | let _ = v1[0] + v1[12]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:28:13 | LL | let _ = v2[5] + v2[15]; - | ^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^^ | = help: consider asserting the length before indexing: `assert!(v2.len() > 15);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:28:13 - | -LL | let _ = v2[5] + v2[15]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:28:21 - | -LL | let _ = v2[5] + v2[15]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:35:13 | LL | let _ = v2[5] + v2[15]; - | ^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^^ | = help: consider asserting the length before indexing: `assert!(v2.len() > 15);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:35:13 - | -LL | let _ = v2[5] + v2[15]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:35:21 - | -LL | let _ = v2[5] + v2[15]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:45:13 | LL | let _ = f.v[0] + f.v[1]; - | ^^^^^^^^^^^^^^^ + | ^^^^^^ ^^^^^^ | = help: consider asserting the length before indexing: `assert!(f.v.len() > 1);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:45:13 - | -LL | let _ = f.v[0] + f.v[1]; - | ^^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:45:22 - | -LL | let _ = f.v[0] + f.v[1]; - | ^^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:59:13 | LL | let _ = x[0] + x[1]; - | ^^^^^^^^^^^ + | ^^^^ ^^^^ | = help: consider asserting the length before indexing: `assert!(x.len() > 1);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:59:13 - | -LL | let _ = x[0] + x[1]; - | ^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:59:20 - | -LL | let _ = x[0] + x[1]; - | ^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:77:13 | LL | let _ = v1[1] + v1[2]; - | ^^^^^^^^^^^^^ + | ^^^^^ ^^^^^ | = help: consider asserting the length before indexing: `assert!(v1.len() > 2);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:77:13 - | -LL | let _ = v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:77:21 - | -LL | let _ = v1[1] + v1[2]; - | ^^^^^ - = note: asserting the length before indexing will elide bounds checks error: indexing into a slice multiple times without an `assert` --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:13 | LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^ ^^^^^ ^^^^^ | = help: consider asserting the length before indexing: `assert!(v1.len() > 2);` -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:13 - | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:21 - | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ -note: slice indexed here - --> tests/ui/missing_asserts_for_indexing_unfixable.rs:85:29 - | -LL | let _ = v1[0] + v1[1] + v1[2]; - | ^^^^^ - = note: asserting the length before indexing will elide bounds checks error: aborting due to 10 previous errors diff --git a/src/tools/clippy/tests/ui/needless_type_cast.fixed b/src/tools/clippy/tests/ui/needless_type_cast.fixed new file mode 100644 index 0000000000000..32c348d3ca3aa --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_type_cast.fixed @@ -0,0 +1,182 @@ +#![warn(clippy::needless_type_cast)] +#![allow(clippy::no_effect, clippy::unnecessary_cast, unused)] + +fn takes_i32(x: i32) -> i32 { + x +} + +fn generic(x: T) -> T { + x +} + +fn main() { + let a: i32 = 10; + //~^ needless_type_cast + let _ = a as i32 + 5; + let _ = a as i32 * 2; + + let b: u16 = 20; + let _ = b; + let _ = b as u32; + + let c: u8 = 5; + let _ = c as u16; + let _ = c as u32; + + let d: i32 = 100; + let _ = d + 1; + + let e = 42u8; + let _ = e as i64; + let _ = e as i64 + 10; + + let f: usize = 1; + //~^ needless_type_cast + let _ = f as usize; +} + +fn test_function_call() { + let a: i32 = 10; + //~^ needless_type_cast + let _ = takes_i32(a as i32); + let _ = takes_i32(a as i32); +} + +fn test_generic_call() { + let a: u8 = 10; + let _ = generic(a as i32); + let _ = generic(a as i32); +} + +fn test_method_on_cast() { + let a: i32 = 10; + //~^ needless_type_cast + let _ = (a as i32).checked_add(5); + let _ = (a as i32).saturating_mul(2); +} + +fn test_iterator_sum() { + let a: i32 = 10; + //~^ needless_type_cast + let arr = [a as i32, a as i32]; + let _: i32 = arr.iter().copied().sum(); +} + +fn test_closure() { + let a: i32 = 10; + //~^ needless_type_cast + let _: i32 = [1i32, 2].iter().map(|x| x + a as i32).sum(); +} + +fn test_struct_field() { + struct S { + x: i32, + y: i32, + } + + let a: i32 = 10; + //~^ needless_type_cast + let _ = S { + x: a as i32, + y: a as i32, + }; +} + +fn test_option() { + let a: u8 = 10; + let _: Option = Some(a as i32); + let _: Option = Some(a as i32); +} + +fn test_mixed_context() { + let a: u8 = 10; + let _ = takes_i32(a as i32); + let _ = generic(a as i32); +} + +fn test_nested_block() { + if true { + let a: i32 = 10; + //~^ needless_type_cast + let _ = a as i32 + 1; + let _ = a as i32 * 2; + } +} + +fn test_match_expr() { + let a: i32 = 10; + //~^ needless_type_cast + let _ = match 1 { + 1 => a as i32, + _ => a as i32, + }; +} + +fn test_return_expr() -> i32 { + let a: i32 = 10; + //~^ needless_type_cast + a as i32 +} + +fn test_closure_always_cast() { + let a: i32 = 10; + //~^ needless_type_cast + let _ = [1, 2].iter().map(|_| a as i32).sum::(); + let _ = a as i32; +} + +fn test_closure_mixed_usage() { + let a: u8 = 10; + let _ = [1, 2].iter().map(|_| a as i32).sum::(); + let _ = a + 1; +} + +fn test_nested_generic_call() { + let a: u8 = 10; + let _ = generic(takes_i32(a as i32)); + let _ = generic(takes_i32(a as i32)); +} + +fn test_generic_initializer() { + // Should not lint: changing type would affect what generic() returns + let a: u8 = generic(10u8); + let _ = a as i32; + let _ = a as i32; +} + +fn test_unsafe_transmute() { + // Should not lint: initializer contains unsafe block + #[allow(clippy::useless_transmute)] + let x: u32 = unsafe { std::mem::transmute(0u32) }; + let _ = x as u64; +} + +fn test_if_with_generic() { + // Should not lint: one branch has generic return type + let x: u8 = if true { generic(1) } else { 2 }; + let _ = x as i32; +} + +fn test_match_with_generic() { + // Should not lint: one branch has generic return type + let x: u8 = match 1 { + 1 => generic(1), + _ => 2, + }; + let _ = x as i32; +} + +fn test_default() { + // Should not lint: Default::default() has generic return type + let x: u8 = Default::default(); + let _ = x as i32; +} + +fn test_loop_with_generic() { + // Should not lint: loop break has generic return type + #[allow(clippy::never_loop)] + let x: u8 = loop { + break generic(1); + }; + let _ = x as i32; +} diff --git a/src/tools/clippy/tests/ui/needless_type_cast.rs b/src/tools/clippy/tests/ui/needless_type_cast.rs new file mode 100644 index 0000000000000..e28f620e035f6 --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_type_cast.rs @@ -0,0 +1,182 @@ +#![warn(clippy::needless_type_cast)] +#![allow(clippy::no_effect, clippy::unnecessary_cast, unused)] + +fn takes_i32(x: i32) -> i32 { + x +} + +fn generic(x: T) -> T { + x +} + +fn main() { + let a: u8 = 10; + //~^ needless_type_cast + let _ = a as i32 + 5; + let _ = a as i32 * 2; + + let b: u16 = 20; + let _ = b; + let _ = b as u32; + + let c: u8 = 5; + let _ = c as u16; + let _ = c as u32; + + let d: i32 = 100; + let _ = d + 1; + + let e = 42u8; + let _ = e as i64; + let _ = e as i64 + 10; + + let f: u8 = 1; + //~^ needless_type_cast + let _ = f as usize; +} + +fn test_function_call() { + let a: u8 = 10; + //~^ needless_type_cast + let _ = takes_i32(a as i32); + let _ = takes_i32(a as i32); +} + +fn test_generic_call() { + let a: u8 = 10; + let _ = generic(a as i32); + let _ = generic(a as i32); +} + +fn test_method_on_cast() { + let a: u8 = 10; + //~^ needless_type_cast + let _ = (a as i32).checked_add(5); + let _ = (a as i32).saturating_mul(2); +} + +fn test_iterator_sum() { + let a: u8 = 10; + //~^ needless_type_cast + let arr = [a as i32, a as i32]; + let _: i32 = arr.iter().copied().sum(); +} + +fn test_closure() { + let a: u8 = 10; + //~^ needless_type_cast + let _: i32 = [1i32, 2].iter().map(|x| x + a as i32).sum(); +} + +fn test_struct_field() { + struct S { + x: i32, + y: i32, + } + + let a: u8 = 10; + //~^ needless_type_cast + let _ = S { + x: a as i32, + y: a as i32, + }; +} + +fn test_option() { + let a: u8 = 10; + let _: Option = Some(a as i32); + let _: Option = Some(a as i32); +} + +fn test_mixed_context() { + let a: u8 = 10; + let _ = takes_i32(a as i32); + let _ = generic(a as i32); +} + +fn test_nested_block() { + if true { + let a: u8 = 10; + //~^ needless_type_cast + let _ = a as i32 + 1; + let _ = a as i32 * 2; + } +} + +fn test_match_expr() { + let a: u8 = 10; + //~^ needless_type_cast + let _ = match 1 { + 1 => a as i32, + _ => a as i32, + }; +} + +fn test_return_expr() -> i32 { + let a: u8 = 10; + //~^ needless_type_cast + a as i32 +} + +fn test_closure_always_cast() { + let a: u8 = 10; + //~^ needless_type_cast + let _ = [1, 2].iter().map(|_| a as i32).sum::(); + let _ = a as i32; +} + +fn test_closure_mixed_usage() { + let a: u8 = 10; + let _ = [1, 2].iter().map(|_| a as i32).sum::(); + let _ = a + 1; +} + +fn test_nested_generic_call() { + let a: u8 = 10; + let _ = generic(takes_i32(a as i32)); + let _ = generic(takes_i32(a as i32)); +} + +fn test_generic_initializer() { + // Should not lint: changing type would affect what generic() returns + let a: u8 = generic(10u8); + let _ = a as i32; + let _ = a as i32; +} + +fn test_unsafe_transmute() { + // Should not lint: initializer contains unsafe block + #[allow(clippy::useless_transmute)] + let x: u32 = unsafe { std::mem::transmute(0u32) }; + let _ = x as u64; +} + +fn test_if_with_generic() { + // Should not lint: one branch has generic return type + let x: u8 = if true { generic(1) } else { 2 }; + let _ = x as i32; +} + +fn test_match_with_generic() { + // Should not lint: one branch has generic return type + let x: u8 = match 1 { + 1 => generic(1), + _ => 2, + }; + let _ = x as i32; +} + +fn test_default() { + // Should not lint: Default::default() has generic return type + let x: u8 = Default::default(); + let _ = x as i32; +} + +fn test_loop_with_generic() { + // Should not lint: loop break has generic return type + #[allow(clippy::never_loop)] + let x: u8 = loop { + break generic(1); + }; + let _ = x as i32; +} diff --git a/src/tools/clippy/tests/ui/needless_type_cast.stderr b/src/tools/clippy/tests/ui/needless_type_cast.stderr new file mode 100644 index 0000000000000..3ee9df1043e74 --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_type_cast.stderr @@ -0,0 +1,71 @@ +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:13:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + | + = note: `-D clippy::needless-type-cast` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::needless_type_cast)]` + +error: this binding is defined as `u8` but is always cast to `usize` + --> tests/ui/needless_type_cast.rs:33:12 + | +LL | let f: u8 = 1; + | ^^ help: consider defining it as: `usize` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:39:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:52:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:59:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:66:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:77:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:99:16 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:107:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:116:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: this binding is defined as `u8` but is always cast to `i32` + --> tests/ui/needless_type_cast.rs:122:12 + | +LL | let a: u8 = 10; + | ^^ help: consider defining it as: `i32` + +error: aborting due to 11 previous errors + diff --git a/src/tools/clippy/tests/ui/ptr_offset_by_literal.fixed b/src/tools/clippy/tests/ui/ptr_offset_by_literal.fixed new file mode 100644 index 0000000000000..bd9e41def938d --- /dev/null +++ b/src/tools/clippy/tests/ui/ptr_offset_by_literal.fixed @@ -0,0 +1,50 @@ +#![warn(clippy::ptr_offset_by_literal)] +#![allow(clippy::inconsistent_digit_grouping)] + +fn main() { + let arr = [b'a', b'b', b'c']; + let ptr = arr.as_ptr(); + + let var = 32; + const CONST: isize = 42; + + unsafe { + let _ = ptr; + //~^ ptr_offset_by_literal + let _ = ptr; + //~^ ptr_offset_by_literal + + let _ = ptr.add(5); + //~^ ptr_offset_by_literal + let _ = ptr.sub(5); + //~^ ptr_offset_by_literal + + let _ = ptr.offset(var); + let _ = ptr.offset(CONST); + + let _ = ptr.wrapping_add(5); + //~^ ptr_offset_by_literal + let _ = ptr.wrapping_sub(5); + //~^ ptr_offset_by_literal + + let _ = ptr.sub(5); + //~^ ptr_offset_by_literal + let _ = ptr.wrapping_sub(5); + //~^ ptr_offset_by_literal + + // isize::MAX and isize::MIN on 32-bit systems. + let _ = ptr.add(2_147_483_647); + //~^ ptr_offset_by_literal + let _ = ptr.sub(2_147_483_648); + //~^ ptr_offset_by_literal + + let _ = ptr.add(5_0); + //~^ ptr_offset_by_literal + let _ = ptr.sub(5_0); + //~^ ptr_offset_by_literal + + macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; } + offs!(ptr, 6); + offs!(ptr, var); + } +} diff --git a/src/tools/clippy/tests/ui/ptr_offset_by_literal.rs b/src/tools/clippy/tests/ui/ptr_offset_by_literal.rs new file mode 100644 index 0000000000000..b8e3f9b26c68f --- /dev/null +++ b/src/tools/clippy/tests/ui/ptr_offset_by_literal.rs @@ -0,0 +1,50 @@ +#![warn(clippy::ptr_offset_by_literal)] +#![allow(clippy::inconsistent_digit_grouping)] + +fn main() { + let arr = [b'a', b'b', b'c']; + let ptr = arr.as_ptr(); + + let var = 32; + const CONST: isize = 42; + + unsafe { + let _ = ptr.offset(0); + //~^ ptr_offset_by_literal + let _ = ptr.offset(-0); + //~^ ptr_offset_by_literal + + let _ = ptr.offset(5); + //~^ ptr_offset_by_literal + let _ = ptr.offset(-5); + //~^ ptr_offset_by_literal + + let _ = ptr.offset(var); + let _ = ptr.offset(CONST); + + let _ = ptr.wrapping_offset(5isize); + //~^ ptr_offset_by_literal + let _ = ptr.wrapping_offset(-5isize); + //~^ ptr_offset_by_literal + + let _ = ptr.offset(-(5)); + //~^ ptr_offset_by_literal + let _ = ptr.wrapping_offset(-(5)); + //~^ ptr_offset_by_literal + + // isize::MAX and isize::MIN on 32-bit systems. + let _ = ptr.offset(2_147_483_647isize); + //~^ ptr_offset_by_literal + let _ = ptr.offset(-2_147_483_648isize); + //~^ ptr_offset_by_literal + + let _ = ptr.offset(5_0__isize); + //~^ ptr_offset_by_literal + let _ = ptr.offset(-5_0__isize); + //~^ ptr_offset_by_literal + + macro_rules! offs { { $e:expr, $offs:expr } => { $e.offset($offs) }; } + offs!(ptr, 6); + offs!(ptr, var); + } +} diff --git a/src/tools/clippy/tests/ui/ptr_offset_by_literal.stderr b/src/tools/clippy/tests/ui/ptr_offset_by_literal.stderr new file mode 100644 index 0000000000000..f85fef87d55f2 --- /dev/null +++ b/src/tools/clippy/tests/ui/ptr_offset_by_literal.stderr @@ -0,0 +1,141 @@ +error: use of `offset` with zero + --> tests/ui/ptr_offset_by_literal.rs:12:17 + | +LL | let _ = ptr.offset(0); + | ^^^---------- + | | + | help: remove the call to `offset` + | + = note: `-D clippy::ptr-offset-by-literal` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::ptr_offset_by_literal)]` + +error: use of `offset` with zero + --> tests/ui/ptr_offset_by_literal.rs:14:17 + | +LL | let _ = ptr.offset(-0); + | ^^^----------- + | | + | help: remove the call to `offset` + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:17:17 + | +LL | let _ = ptr.offset(5); + | ^^^^^^^^^^^^^ + | +help: use `add` instead + | +LL - let _ = ptr.offset(5); +LL + let _ = ptr.add(5); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:19:17 + | +LL | let _ = ptr.offset(-5); + | ^^^^^^^^^^^^^^ + | +help: use `sub` instead + | +LL - let _ = ptr.offset(-5); +LL + let _ = ptr.sub(5); + | + +error: use of `wrapping_offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:25:17 + | +LL | let _ = ptr.wrapping_offset(5isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `wrapping_add` instead + | +LL - let _ = ptr.wrapping_offset(5isize); +LL + let _ = ptr.wrapping_add(5); + | + +error: use of `wrapping_offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:27:17 + | +LL | let _ = ptr.wrapping_offset(-5isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `wrapping_sub` instead + | +LL - let _ = ptr.wrapping_offset(-5isize); +LL + let _ = ptr.wrapping_sub(5); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:30:17 + | +LL | let _ = ptr.offset(-(5)); + | ^^^^^^^^^^^^^^^^ + | +help: use `sub` instead + | +LL - let _ = ptr.offset(-(5)); +LL + let _ = ptr.sub(5); + | + +error: use of `wrapping_offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:32:17 + | +LL | let _ = ptr.wrapping_offset(-(5)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `wrapping_sub` instead + | +LL - let _ = ptr.wrapping_offset(-(5)); +LL + let _ = ptr.wrapping_sub(5); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:36:17 + | +LL | let _ = ptr.offset(2_147_483_647isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `add` instead + | +LL - let _ = ptr.offset(2_147_483_647isize); +LL + let _ = ptr.add(2_147_483_647); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:38:17 + | +LL | let _ = ptr.offset(-2_147_483_648isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `sub` instead + | +LL - let _ = ptr.offset(-2_147_483_648isize); +LL + let _ = ptr.sub(2_147_483_648); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:41:17 + | +LL | let _ = ptr.offset(5_0__isize); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `add` instead + | +LL - let _ = ptr.offset(5_0__isize); +LL + let _ = ptr.add(5_0); + | + +error: use of `offset` with a literal + --> tests/ui/ptr_offset_by_literal.rs:43:17 + | +LL | let _ = ptr.offset(-5_0__isize); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `sub` instead + | +LL - let _ = ptr.offset(-5_0__isize); +LL + let _ = ptr.sub(5_0); + | + +error: aborting due to 12 previous errors + diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.fixed b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.fixed index 476e7e35a1f61..caba277db754d 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.fixed +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.fixed @@ -24,6 +24,13 @@ struct GenericParam { t: T, } +#[derive(Clone, Copy)] +struct PtrNamed { + ptr: *const u32, +} +#[derive(Clone, Copy)] +struct Ptr(*const u32); + fn transmute_ptr_to_ptr() { let ptr = &1u32 as *const u32; let mut_ptr = &mut 1u32 as *mut u32; @@ -68,6 +75,18 @@ fn transmute_ptr_to_ptr() { let _: &GenericParam<&LifetimeParam<'static>> = unsafe { transmute(&GenericParam { t: &lp }) }; } +fn issue1966() { + let ptr = &1u32 as *const u32; + unsafe { + let _: *const f32 = Ptr(ptr).0.cast::(); + //~^ transmute_ptr_to_ptr + let _: *const f32 = PtrNamed { ptr }.ptr.cast::(); + //~^ transmute_ptr_to_ptr + let _: *mut u32 = Ptr(ptr).0.cast_mut(); + //~^ transmute_ptr_to_ptr + } +} + fn lifetime_to_static(v: *mut &()) -> *const &'static () { unsafe { v as *const &() } //~^ transmute_ptr_to_ptr @@ -81,11 +100,15 @@ const _: &() = { unsafe { transmute::<&'static Zst, &'static ()>(zst) } }; +#[derive(Clone, Copy)] +struct Ptr8(*const u8); #[clippy::msrv = "1.37"] fn msrv_1_37(ptr: *const u8) { unsafe { let _: *const i8 = ptr as *const i8; //~^ transmute_ptr_to_ptr + let _: *const i8 = Ptr8(ptr).0 as *const i8; + //~^ transmute_ptr_to_ptr } } diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs index 7356668bcab5a..b3c2baf29c361 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs @@ -24,6 +24,13 @@ struct GenericParam { t: T, } +#[derive(Clone, Copy)] +struct PtrNamed { + ptr: *const u32, +} +#[derive(Clone, Copy)] +struct Ptr(*const u32); + fn transmute_ptr_to_ptr() { let ptr = &1u32 as *const u32; let mut_ptr = &mut 1u32 as *mut u32; @@ -68,6 +75,18 @@ fn transmute_ptr_to_ptr() { let _: &GenericParam<&LifetimeParam<'static>> = unsafe { transmute(&GenericParam { t: &lp }) }; } +fn issue1966() { + let ptr = &1u32 as *const u32; + unsafe { + let _: *const f32 = transmute(Ptr(ptr)); + //~^ transmute_ptr_to_ptr + let _: *const f32 = transmute(PtrNamed { ptr }); + //~^ transmute_ptr_to_ptr + let _: *mut u32 = transmute(Ptr(ptr)); + //~^ transmute_ptr_to_ptr + } +} + fn lifetime_to_static(v: *mut &()) -> *const &'static () { unsafe { transmute(v) } //~^ transmute_ptr_to_ptr @@ -81,11 +100,15 @@ const _: &() = { unsafe { transmute::<&'static Zst, &'static ()>(zst) } }; +#[derive(Clone, Copy)] +struct Ptr8(*const u8); #[clippy::msrv = "1.37"] fn msrv_1_37(ptr: *const u8) { unsafe { let _: *const i8 = transmute(ptr); //~^ transmute_ptr_to_ptr + let _: *const i8 = transmute(Ptr8(ptr)); + //~^ transmute_ptr_to_ptr } } diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.stderr b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.stderr index c8db4fe214fdb..ba9e6df6c2d7c 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.stderr +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.stderr @@ -1,5 +1,5 @@ error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:32:29 + --> tests/ui/transmute_ptr_to_ptr.rs:39:29 | LL | let _: *const f32 = transmute(ptr); | ^^^^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL + let _: *const f32 = ptr.cast::(); | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:35:27 + --> tests/ui/transmute_ptr_to_ptr.rs:42:27 | LL | let _: *mut f32 = transmute(mut_ptr); | ^^^^^^^^^^^^^^^^^^ @@ -25,37 +25,37 @@ LL + let _: *mut f32 = mut_ptr.cast::(); | error: transmute from a reference to a reference - --> tests/ui/transmute_ptr_to_ptr.rs:39:23 + --> tests/ui/transmute_ptr_to_ptr.rs:46:23 | LL | let _: &f32 = transmute(&1u32); | ^^^^^^^^^^^^^^^^ help: try: `&*(&1u32 as *const u32 as *const f32)` error: transmute from a reference to a reference - --> tests/ui/transmute_ptr_to_ptr.rs:42:23 + --> tests/ui/transmute_ptr_to_ptr.rs:49:23 | LL | let _: &f32 = transmute(&1f64); | ^^^^^^^^^^^^^^^^ help: try: `&*(&1f64 as *const f64 as *const f32)` error: transmute from a reference to a reference - --> tests/ui/transmute_ptr_to_ptr.rs:47:27 + --> tests/ui/transmute_ptr_to_ptr.rs:54:27 | LL | let _: &mut f32 = transmute(&mut 1u32); | ^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(&mut 1u32 as *mut u32 as *mut f32)` error: transmute from a reference to a reference - --> tests/ui/transmute_ptr_to_ptr.rs:50:37 + --> tests/ui/transmute_ptr_to_ptr.rs:57:37 | LL | let _: &GenericParam = transmute(&GenericParam { t: 1u32 }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&GenericParam { t: 1u32 } as *const GenericParam as *const GenericParam)` error: transmute from a reference to a reference - --> tests/ui/transmute_ptr_to_ptr.rs:54:27 + --> tests/ui/transmute_ptr_to_ptr.rs:61:27 | LL | let u8_ref: &u8 = transmute(u64_ref); | ^^^^^^^^^^^^^^^^^^ help: try: `&*(u64_ref as *const u64 as *const u8)` error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:57:29 + --> tests/ui/transmute_ptr_to_ptr.rs:64:29 | LL | let _: *const u32 = transmute(mut_ptr); | ^^^^^^^^^^^^^^^^^^ @@ -67,7 +67,7 @@ LL + let _: *const u32 = mut_ptr.cast_const(); | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:60:27 + --> tests/ui/transmute_ptr_to_ptr.rs:67:27 | LL | let _: *mut u32 = transmute(ptr); | ^^^^^^^^^^^^^^ @@ -79,7 +79,43 @@ LL + let _: *mut u32 = ptr.cast_mut(); | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:72:14 + --> tests/ui/transmute_ptr_to_ptr.rs:81:29 + | +LL | let _: *const f32 = transmute(Ptr(ptr)); + | ^^^^^^^^^^^^^^^^^^^ + | +help: use `pointer::cast` instead + | +LL - let _: *const f32 = transmute(Ptr(ptr)); +LL + let _: *const f32 = Ptr(ptr).0.cast::(); + | + +error: transmute from a pointer to a pointer + --> tests/ui/transmute_ptr_to_ptr.rs:83:29 + | +LL | let _: *const f32 = transmute(PtrNamed { ptr }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: use `pointer::cast` instead + | +LL - let _: *const f32 = transmute(PtrNamed { ptr }); +LL + let _: *const f32 = PtrNamed { ptr }.ptr.cast::(); + | + +error: transmute from a pointer to a pointer + --> tests/ui/transmute_ptr_to_ptr.rs:85:27 + | +LL | let _: *mut u32 = transmute(Ptr(ptr)); + | ^^^^^^^^^^^^^^^^^^^ + | +help: use `pointer::cast_mut` instead + | +LL - let _: *mut u32 = transmute(Ptr(ptr)); +LL + let _: *mut u32 = Ptr(ptr).0.cast_mut(); + | + +error: transmute from a pointer to a pointer + --> tests/ui/transmute_ptr_to_ptr.rs:91:14 | LL | unsafe { transmute(v) } | ^^^^^^^^^^^^ @@ -91,7 +127,7 @@ LL + unsafe { v as *const &() } | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:87:28 + --> tests/ui/transmute_ptr_to_ptr.rs:108:28 | LL | let _: *const i8 = transmute(ptr); | ^^^^^^^^^^^^^^ @@ -103,7 +139,19 @@ LL + let _: *const i8 = ptr as *const i8; | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:95:28 + --> tests/ui/transmute_ptr_to_ptr.rs:110:28 + | +LL | let _: *const i8 = transmute(Ptr8(ptr)); + | ^^^^^^^^^^^^^^^^^^^^ + | +help: use an `as` cast instead + | +LL - let _: *const i8 = transmute(Ptr8(ptr)); +LL + let _: *const i8 = Ptr8(ptr).0 as *const i8; + | + +error: transmute from a pointer to a pointer + --> tests/ui/transmute_ptr_to_ptr.rs:118:28 | LL | let _: *const i8 = transmute(ptr); | ^^^^^^^^^^^^^^ @@ -115,7 +163,7 @@ LL + let _: *const i8 = ptr.cast::(); | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:103:26 + --> tests/ui/transmute_ptr_to_ptr.rs:126:26 | LL | let _: *mut u8 = transmute(ptr); | ^^^^^^^^^^^^^^ @@ -127,7 +175,7 @@ LL + let _: *mut u8 = ptr as *mut u8; | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:105:28 + --> tests/ui/transmute_ptr_to_ptr.rs:128:28 | LL | let _: *const u8 = transmute(mut_ptr); | ^^^^^^^^^^^^^^^^^^ @@ -139,7 +187,7 @@ LL + let _: *const u8 = mut_ptr as *const u8; | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:113:26 + --> tests/ui/transmute_ptr_to_ptr.rs:136:26 | LL | let _: *mut u8 = transmute(ptr); | ^^^^^^^^^^^^^^ @@ -151,7 +199,7 @@ LL + let _: *mut u8 = ptr.cast_mut(); | error: transmute from a pointer to a pointer - --> tests/ui/transmute_ptr_to_ptr.rs:115:28 + --> tests/ui/transmute_ptr_to_ptr.rs:138:28 | LL | let _: *const u8 = transmute(mut_ptr); | ^^^^^^^^^^^^^^^^^^ @@ -162,5 +210,5 @@ LL - let _: *const u8 = transmute(mut_ptr); LL + let _: *const u8 = mut_ptr.cast_const(); | -error: aborting due to 16 previous errors +error: aborting due to 20 previous errors diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ref.fixed b/src/tools/clippy/tests/ui/transmute_ptr_to_ref.fixed index c130575df9607..8de47031a4009 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ref.fixed +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ref.fixed @@ -55,6 +55,52 @@ fn issue1231() { //~^ transmute_ptr_to_ref } +#[derive(Clone, Copy)] +struct PtrRefNamed<'a> { + ptr: *const &'a u32, +} +#[derive(Clone, Copy)] +struct PtrRef<'a>(*const &'a u32); +#[derive(Clone, Copy)] +struct PtrSliceRef<'a>(*const [&'a str]); +#[derive(Clone, Copy)] +struct PtrSlice(*const [i32]); +#[derive(Clone, Copy)] +struct Ptr(*const u32); +impl std::ops::Add for Ptr { + type Output = Self; + fn add(self, _: Self) -> Self { + self + } +} +mod ptr_mod { + #[derive(Clone, Copy)] + pub struct Ptr(*const u32); +} +fn issue1966(u: PtrSlice, v: PtrSliceRef, w: Ptr, x: PtrRefNamed, y: PtrRef, z: ptr_mod::Ptr) { + unsafe { + let _: &i32 = &*(w.0 as *const i32); + //~^ transmute_ptr_to_ref + let _: &u32 = &*w.0; + //~^ transmute_ptr_to_ref + let _: &&u32 = &*x.ptr.cast::<&u32>(); + //~^ transmute_ptr_to_ref + // The field is not accessible. The program should not generate code + // that accesses the field. + let _: &u32 = std::mem::transmute(z); + let _ = &*w.0.cast::(); + //~^ transmute_ptr_to_ref + let _: &[&str] = &*(v.0 as *const [&str]); + //~^ transmute_ptr_to_ref + let _ = &*(u.0 as *const [i32]); + //~^ transmute_ptr_to_ref + let _: &&u32 = &*y.0.cast::<&u32>(); + //~^ transmute_ptr_to_ref + let _: &u32 = &*(w + w).0; + //~^ transmute_ptr_to_ref + } +} + fn issue8924<'a, 'b, 'c>(x: *const &'a u32, y: *const &'b u32) -> &'c &'b u32 { unsafe { match 0 { @@ -89,7 +135,7 @@ fn meets_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 { } #[clippy::msrv = "1.37"] -fn under_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 { +fn under_msrv<'a, 'b, 'c>(x: *const &'a u32, y: PtrRef) -> &'c &'b u32 { unsafe { let a = 0u32; let a = &a as *const u32; @@ -97,10 +143,16 @@ fn under_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 { //~^ transmute_ptr_to_ref let _: &u32 = &*(a as *const u32); //~^ transmute_ptr_to_ref + let _ = &*(Ptr(a).0 as *const u32); + //~^ transmute_ptr_to_ref match 0 { 0 => &*(x as *const () as *const &u32), //~^ transmute_ptr_to_ref - _ => &*(x as *const () as *const &'b u32), + 1 => &*(x as *const () as *const &'b u32), + //~^ transmute_ptr_to_ref + 2 => &*(y.0 as *const () as *const &u32), + //~^ transmute_ptr_to_ref + _ => &*(y.0 as *const () as *const &'b u32), //~^ transmute_ptr_to_ref } } diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ref.rs b/src/tools/clippy/tests/ui/transmute_ptr_to_ref.rs index f79d54234a2c9..52fe669de9355 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ref.rs +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ref.rs @@ -55,6 +55,52 @@ fn issue1231() { //~^ transmute_ptr_to_ref } +#[derive(Clone, Copy)] +struct PtrRefNamed<'a> { + ptr: *const &'a u32, +} +#[derive(Clone, Copy)] +struct PtrRef<'a>(*const &'a u32); +#[derive(Clone, Copy)] +struct PtrSliceRef<'a>(*const [&'a str]); +#[derive(Clone, Copy)] +struct PtrSlice(*const [i32]); +#[derive(Clone, Copy)] +struct Ptr(*const u32); +impl std::ops::Add for Ptr { + type Output = Self; + fn add(self, _: Self) -> Self { + self + } +} +mod ptr_mod { + #[derive(Clone, Copy)] + pub struct Ptr(*const u32); +} +fn issue1966(u: PtrSlice, v: PtrSliceRef, w: Ptr, x: PtrRefNamed, y: PtrRef, z: ptr_mod::Ptr) { + unsafe { + let _: &i32 = std::mem::transmute(w); + //~^ transmute_ptr_to_ref + let _: &u32 = std::mem::transmute(w); + //~^ transmute_ptr_to_ref + let _: &&u32 = core::mem::transmute(x); + //~^ transmute_ptr_to_ref + // The field is not accessible. The program should not generate code + // that accesses the field. + let _: &u32 = std::mem::transmute(z); + let _ = std::mem::transmute::<_, &u32>(w); + //~^ transmute_ptr_to_ref + let _: &[&str] = core::mem::transmute(v); + //~^ transmute_ptr_to_ref + let _ = std::mem::transmute::<_, &[i32]>(u); + //~^ transmute_ptr_to_ref + let _: &&u32 = std::mem::transmute(y); + //~^ transmute_ptr_to_ref + let _: &u32 = std::mem::transmute(w + w); + //~^ transmute_ptr_to_ref + } +} + fn issue8924<'a, 'b, 'c>(x: *const &'a u32, y: *const &'b u32) -> &'c &'b u32 { unsafe { match 0 { @@ -89,7 +135,7 @@ fn meets_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 { } #[clippy::msrv = "1.37"] -fn under_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 { +fn under_msrv<'a, 'b, 'c>(x: *const &'a u32, y: PtrRef) -> &'c &'b u32 { unsafe { let a = 0u32; let a = &a as *const u32; @@ -97,10 +143,16 @@ fn under_msrv<'a, 'b, 'c>(x: *const &'a u32) -> &'c &'b u32 { //~^ transmute_ptr_to_ref let _: &u32 = std::mem::transmute::<_, &u32>(a); //~^ transmute_ptr_to_ref + let _ = std::mem::transmute::<_, &u32>(Ptr(a)); + //~^ transmute_ptr_to_ref match 0 { 0 => std::mem::transmute(x), //~^ transmute_ptr_to_ref - _ => std::mem::transmute::<_, &&'b u32>(x), + 1 => std::mem::transmute::<_, &&'b u32>(x), + //~^ transmute_ptr_to_ref + 2 => std::mem::transmute(y), + //~^ transmute_ptr_to_ref + _ => std::mem::transmute::<_, &&'b u32>(y), //~^ transmute_ptr_to_ref } } diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ref.stderr b/src/tools/clippy/tests/ui/transmute_ptr_to_ref.stderr index 3f404d295fef0..c0f0ca916761d 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ref.stderr +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ref.stderr @@ -61,125 +61,191 @@ error: transmute from a pointer type (`*const i32`) to a reference type (`&u8`) LL | unsafe { std::mem::transmute::<_, Bar>(raw) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(raw as *const u8)` +error: transmute from a pointer type (`*const u32`) to a reference type (`&i32`) + --> tests/ui/transmute_ptr_to_ref.rs:82:23 + | +LL | let _: &i32 = std::mem::transmute(w); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(w.0 as *const i32)` + +error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:84:23 + | +LL | let _: &u32 = std::mem::transmute(w); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*w.0` + +error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:86:24 + | +LL | let _: &&u32 = core::mem::transmute(x); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*x.ptr.cast::<&u32>()` + +error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:91:17 + | +LL | let _ = std::mem::transmute::<_, &u32>(w); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*w.0.cast::()` + +error: transmute from a pointer type (`*const [&str]`) to a reference type (`&[&str]`) + --> tests/ui/transmute_ptr_to_ref.rs:93:26 + | +LL | let _: &[&str] = core::mem::transmute(v); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(v.0 as *const [&str])` + +error: transmute from a pointer type (`*const [i32]`) to a reference type (`&[i32]`) + --> tests/ui/transmute_ptr_to_ref.rs:95:17 + | +LL | let _ = std::mem::transmute::<_, &[i32]>(u); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(u.0 as *const [i32])` + error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:61:18 + --> tests/ui/transmute_ptr_to_ref.rs:97:24 + | +LL | let _: &&u32 = std::mem::transmute(y); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*y.0.cast::<&u32>()` + +error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:99:23 + | +LL | let _: &u32 = std::mem::transmute(w + w); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(w + w).0` + +error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:107:18 | LL | 0 => std::mem::transmute(x), | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*x.cast::<&u32>()` error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:63:18 + --> tests/ui/transmute_ptr_to_ref.rs:109:18 | LL | 1 => std::mem::transmute(y), | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*y.cast::<&u32>()` error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:65:18 + --> tests/ui/transmute_ptr_to_ref.rs:111:18 | LL | 2 => std::mem::transmute::<_, &&'b u32>(x), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*x.cast::<&'b u32>()` error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:67:18 + --> tests/ui/transmute_ptr_to_ref.rs:113:18 | LL | _ => std::mem::transmute::<_, &&'b u32>(y), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*y.cast::<&'b u32>()` error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:78:23 + --> tests/ui/transmute_ptr_to_ref.rs:124:23 | LL | let _: &u32 = std::mem::transmute(a); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*a` error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:80:23 + --> tests/ui/transmute_ptr_to_ref.rs:126:23 | LL | let _: &u32 = std::mem::transmute::<_, &u32>(a); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*a.cast::()` error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:83:18 + --> tests/ui/transmute_ptr_to_ref.rs:129:18 | LL | 0 => std::mem::transmute(x), | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*x.cast::<&u32>()` error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:85:18 + --> tests/ui/transmute_ptr_to_ref.rs:131:18 | LL | _ => std::mem::transmute::<_, &&'b u32>(x), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*x.cast::<&'b u32>()` error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:96:23 + --> tests/ui/transmute_ptr_to_ref.rs:142:23 | LL | let _: &u32 = std::mem::transmute(a); | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*a` error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:98:23 + --> tests/ui/transmute_ptr_to_ref.rs:144:23 | LL | let _: &u32 = std::mem::transmute::<_, &u32>(a); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(a as *const u32)` +error: transmute from a pointer type (`*const u32`) to a reference type (`&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:146:17 + | +LL | let _ = std::mem::transmute::<_, &u32>(Ptr(a)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(Ptr(a).0 as *const u32)` + error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:101:18 + --> tests/ui/transmute_ptr_to_ref.rs:149:18 | LL | 0 => std::mem::transmute(x), | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(x as *const () as *const &u32)` error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) - --> tests/ui/transmute_ptr_to_ref.rs:103:18 + --> tests/ui/transmute_ptr_to_ref.rs:151:18 | -LL | _ => std::mem::transmute::<_, &&'b u32>(x), +LL | 1 => std::mem::transmute::<_, &&'b u32>(x), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(x as *const () as *const &'b u32)` +error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:153:18 + | +LL | 2 => std::mem::transmute(y), + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(y.0 as *const () as *const &u32)` + +error: transmute from a pointer type (`*const &u32`) to a reference type (`&&u32`) + --> tests/ui/transmute_ptr_to_ref.rs:155:18 + | +LL | _ => std::mem::transmute::<_, &&'b u32>(y), + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(y.0 as *const () as *const &'b u32)` + error: transmute from a pointer type (`*const [i32]`) to a reference type (`&[u32]`) - --> tests/ui/transmute_ptr_to_ref.rs:113:17 + --> tests/ui/transmute_ptr_to_ref.rs:165:17 | LL | let _ = core::mem::transmute::<_, &[u32]>(ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(ptr as *const [u32])` error: transmute from a pointer type (`*const [i32]`) to a reference type (`&[u32]`) - --> tests/ui/transmute_ptr_to_ref.rs:115:25 + --> tests/ui/transmute_ptr_to_ref.rs:167:25 | LL | let _: &[u32] = core::mem::transmute(ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(ptr as *const [u32])` error: transmute from a pointer type (`*const [&str]`) to a reference type (`&[&[u8]]`) - --> tests/ui/transmute_ptr_to_ref.rs:119:17 + --> tests/ui/transmute_ptr_to_ref.rs:171:17 | LL | let _ = core::mem::transmute::<_, &[&[u8]]>(a_s_ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(a_s_ptr as *const [&[u8]])` error: transmute from a pointer type (`*const [&str]`) to a reference type (`&[&[u8]]`) - --> tests/ui/transmute_ptr_to_ref.rs:121:27 + --> tests/ui/transmute_ptr_to_ref.rs:173:27 | LL | let _: &[&[u8]] = core::mem::transmute(a_s_ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(a_s_ptr as *const [&[u8]])` error: transmute from a pointer type (`*const [i32]`) to a reference type (`&[i32]`) - --> tests/ui/transmute_ptr_to_ref.rs:125:17 + --> tests/ui/transmute_ptr_to_ref.rs:177:17 | LL | let _ = core::mem::transmute::<_, &[i32]>(ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(ptr as *const [i32])` error: transmute from a pointer type (`*const [i32]`) to a reference type (`&[i32]`) - --> tests/ui/transmute_ptr_to_ref.rs:127:25 + --> tests/ui/transmute_ptr_to_ref.rs:179:25 | LL | let _: &[i32] = core::mem::transmute(ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*ptr` error: transmute from a pointer type (`*const [&str]`) to a reference type (`&[&str]`) - --> tests/ui/transmute_ptr_to_ref.rs:131:17 + --> tests/ui/transmute_ptr_to_ref.rs:183:17 | LL | let _ = core::mem::transmute::<_, &[&str]>(a_s_ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(a_s_ptr as *const [&str])` error: transmute from a pointer type (`*const [&str]`) to a reference type (`&[&str]`) - --> tests/ui/transmute_ptr_to_ref.rs:133:26 + --> tests/ui/transmute_ptr_to_ref.rs:185:26 | LL | let _: &[&str] = core::mem::transmute(a_s_ptr); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(a_s_ptr as *const [&str])` -error: aborting due to 30 previous errors +error: aborting due to 41 previous errors diff --git a/src/tools/clippy/tests/ui/tuple_array_conversions.rs b/src/tools/clippy/tests/ui/tuple_array_conversions.rs index 772c41df090e5..17e6a252b2669 100644 --- a/src/tools/clippy/tests/ui/tuple_array_conversions.rs +++ b/src/tools/clippy/tests/ui/tuple_array_conversions.rs @@ -116,3 +116,26 @@ fn msrv_juust_right() { let x = &[1, 2]; let x = (x[0], x[1]); } + +fn issue16192() { + fn do_something(tuple: (u32, u32)) {} + fn produce_array() -> [u32; 2] { + [1, 2] + } + + let [a, b] = produce_array(); + for tuple in [(a, b), (b, a)] { + do_something(tuple); + } + + let [a, b] = produce_array(); + let x = b; + do_something((a, b)); + + let [a, b] = produce_array(); + do_something((b, a)); + + let [a, b] = produce_array(); + do_something((a, b)); + //~^ tuple_array_conversions +} diff --git a/src/tools/clippy/tests/ui/tuple_array_conversions.stderr b/src/tools/clippy/tests/ui/tuple_array_conversions.stderr index 6dafb8d285d4d..4c15769b74875 100644 --- a/src/tools/clippy/tests/ui/tuple_array_conversions.stderr +++ b/src/tools/clippy/tests/ui/tuple_array_conversions.stderr @@ -80,5 +80,13 @@ LL | let x = [x.0, x.1]; | = help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed -error: aborting due to 10 previous errors +error: it looks like you're trying to convert an array to a tuple + --> tests/ui/tuple_array_conversions.rs:139:18 + | +LL | do_something((a, b)); + | ^^^^^^ + | + = help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed + +error: aborting due to 11 previous errors diff --git a/src/tools/clippy/tests/ui/useless_conversion.fixed b/src/tools/clippy/tests/ui/useless_conversion.fixed index 9de7d2c67149d..adf5e58d9a1ab 100644 --- a/src/tools/clippy/tests/ui/useless_conversion.fixed +++ b/src/tools/clippy/tests/ui/useless_conversion.fixed @@ -442,3 +442,14 @@ fn issue14739() { let _ = R.map(|_x| 0); //~^ useless_conversion } + +fn issue16165() { + macro_rules! mac { + (iter $e:expr) => { + $e.iter() + }; + } + + for _ in mac!(iter [1, 2]) {} + //~^ useless_conversion +} diff --git a/src/tools/clippy/tests/ui/useless_conversion.rs b/src/tools/clippy/tests/ui/useless_conversion.rs index 38cd1175aa485..d95fe49e2e2be 100644 --- a/src/tools/clippy/tests/ui/useless_conversion.rs +++ b/src/tools/clippy/tests/ui/useless_conversion.rs @@ -442,3 +442,14 @@ fn issue14739() { let _ = R.into_iter().map(|_x| 0); //~^ useless_conversion } + +fn issue16165() { + macro_rules! mac { + (iter $e:expr) => { + $e.iter() + }; + } + + for _ in mac!(iter [1, 2]).into_iter() {} + //~^ useless_conversion +} diff --git a/src/tools/clippy/tests/ui/useless_conversion.stderr b/src/tools/clippy/tests/ui/useless_conversion.stderr index 3bfaf1411c2c6..052c664f6f2e1 100644 --- a/src/tools/clippy/tests/ui/useless_conversion.stderr +++ b/src/tools/clippy/tests/ui/useless_conversion.stderr @@ -389,5 +389,11 @@ error: useless conversion to the same type: `std::ops::Range` LL | let _ = R.into_iter().map(|_x| 0); | ^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `R` -error: aborting due to 43 previous errors +error: useless conversion to the same type: `std::slice::Iter<'_, i32>` + --> tests/ui/useless_conversion.rs:453:14 + | +LL | for _ in mac!(iter [1, 2]).into_iter() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `mac!(iter [1, 2])` + +error: aborting due to 44 previous errors diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.fixed b/src/tools/clippy/tests/ui/while_let_on_iterator.fixed index f9ccefab5898d..0d3b446af43fa 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.fixed +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.fixed @@ -492,6 +492,110 @@ fn issue13123() { } } +fn issue16089() { + trait CertainTrait: Iterator { + fn iter_over_self(&mut self) { + let mut a = 0; + for r in &mut *self { + //~^ while_let_on_iterator + a = r; + } + self.use_after_iter() + } + + fn use_after_iter(&mut self) {} + } +} + +fn issue16089_sized_trait_not_reborrowed() { + trait CertainTrait: Iterator + Sized { + fn iter_over_self(&mut self) { + let mut a = 0; + // Check that the suggestion is just "self", since the trait is sized. + for r in self.by_ref() { + //~^ while_let_on_iterator + a = r; + } + self.use_after_iter() + } + + fn use_after_iter(&mut self) {} + } +} + +fn issue16089_nested_derefs() { + struct S(T); + impl core::ops::Deref for S { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl core::ops::DerefMut for S { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + fn f(mut x: S>>) { + for _ in &mut ***x {} + //~^ while_let_on_iterator + } +} + +fn issue16089_nested_derefs_last_not_sized() { + struct WithSize(T); + impl core::ops::Deref for WithSize { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl core::ops::DerefMut for WithSize { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + // The suggestion must use `&mut **x`. Using `x.by_ref()` doesn't work in this + // case, since the last type adjustment for `x` in the expression `x.next()` is + // to dereference a `?Sized` trait. + fn f(mut x: WithSize<&mut dyn Iterator>) { + for _ in &mut **x {} + //~^ while_let_on_iterator + } +} + +fn issue16089_nested_derefs_last_sized() { + struct NoSize(T); + impl core::ops::Deref for NoSize { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl core::ops::DerefMut for NoSize { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + struct SizedIter {} + + impl Iterator for SizedIter { + type Item = u32; + fn next(&mut self) -> Option { + Some(0) + } + } + + // We want the suggestion to be `x.by_ref()`. It works in this case since the last type + // adjustment for `x` in the expression `x.next()` is to dereference a Sized type. + fn f(mut x: NoSize>) { + for _ in x.by_ref() {} + //~^ while_let_on_iterator + } +} + fn main() { let mut it = 0..20; for _ in it { diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.rs b/src/tools/clippy/tests/ui/while_let_on_iterator.rs index f957f2e5a5238..e1d9e9081e452 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.rs +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.rs @@ -492,6 +492,110 @@ fn issue13123() { } } +fn issue16089() { + trait CertainTrait: Iterator { + fn iter_over_self(&mut self) { + let mut a = 0; + while let Some(r) = self.next() { + //~^ while_let_on_iterator + a = r; + } + self.use_after_iter() + } + + fn use_after_iter(&mut self) {} + } +} + +fn issue16089_sized_trait_not_reborrowed() { + trait CertainTrait: Iterator + Sized { + fn iter_over_self(&mut self) { + let mut a = 0; + // Check that the suggestion is just "self", since the trait is sized. + while let Some(r) = self.next() { + //~^ while_let_on_iterator + a = r; + } + self.use_after_iter() + } + + fn use_after_iter(&mut self) {} + } +} + +fn issue16089_nested_derefs() { + struct S(T); + impl core::ops::Deref for S { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl core::ops::DerefMut for S { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + fn f(mut x: S>>) { + while let Some(_) = x.next() {} + //~^ while_let_on_iterator + } +} + +fn issue16089_nested_derefs_last_not_sized() { + struct WithSize(T); + impl core::ops::Deref for WithSize { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl core::ops::DerefMut for WithSize { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + // The suggestion must use `&mut **x`. Using `x.by_ref()` doesn't work in this + // case, since the last type adjustment for `x` in the expression `x.next()` is + // to dereference a `?Sized` trait. + fn f(mut x: WithSize<&mut dyn Iterator>) { + while let Some(_) = x.next() {} + //~^ while_let_on_iterator + } +} + +fn issue16089_nested_derefs_last_sized() { + struct NoSize(T); + impl core::ops::Deref for NoSize { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl core::ops::DerefMut for NoSize { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + struct SizedIter {} + + impl Iterator for SizedIter { + type Item = u32; + fn next(&mut self) -> Option { + Some(0) + } + } + + // We want the suggestion to be `x.by_ref()`. It works in this case since the last type + // adjustment for `x` in the expression `x.next()` is to dereference a Sized type. + fn f(mut x: NoSize>) { + while let Some(_) = x.next() {} + //~^ while_let_on_iterator + } +} + fn main() { let mut it = 0..20; while let Some(..) = it.next() { diff --git a/src/tools/clippy/tests/ui/while_let_on_iterator.stderr b/src/tools/clippy/tests/ui/while_let_on_iterator.stderr index 50f20227b90f9..cd43d3c17800e 100644 --- a/src/tools/clippy/tests/ui/while_let_on_iterator.stderr +++ b/src/tools/clippy/tests/ui/while_let_on_iterator.stderr @@ -164,10 +164,40 @@ LL | 'label: while let Some(n) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'label: for n in it` error: this loop could be written as a `for` loop - --> tests/ui/while_let_on_iterator.rs:497:5 + --> tests/ui/while_let_on_iterator.rs:499:13 + | +LL | while let Some(r) = self.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for r in &mut *self` + +error: this loop could be written as a `for` loop + --> tests/ui/while_let_on_iterator.rs:515:13 + | +LL | while let Some(r) = self.next() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for r in self.by_ref()` + +error: this loop could be written as a `for` loop + --> tests/ui/while_let_on_iterator.rs:541:9 + | +LL | while let Some(_) = x.next() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in &mut ***x` + +error: this loop could be written as a `for` loop + --> tests/ui/while_let_on_iterator.rs:563:9 + | +LL | while let Some(_) = x.next() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in &mut **x` + +error: this loop could be written as a `for` loop + --> tests/ui/while_let_on_iterator.rs:594:9 + | +LL | while let Some(_) = x.next() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in x.by_ref()` + +error: this loop could be written as a `for` loop + --> tests/ui/while_let_on_iterator.rs:601:5 | LL | while let Some(..) = it.next() { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `for _ in it` -error: aborting due to 28 previous errors +error: aborting due to 33 previous errors diff --git a/src/tools/clippy/tests/ui/zero_offset.rs b/src/tools/clippy/tests/ui/zero_offset.rs index bedb09536c538..5a9c3ac9248fa 100644 --- a/src/tools/clippy/tests/ui/zero_offset.rs +++ b/src/tools/clippy/tests/ui/zero_offset.rs @@ -1,4 +1,4 @@ -#[allow(clippy::borrow_as_ptr)] +#[allow(clippy::borrow_as_ptr, clippy::ptr_offset_by_literal)] fn main() { unsafe { let m = &mut () as *mut (); diff --git a/src/tools/clippy/tests/ui/zero_repeat_side_effects.fixed b/src/tools/clippy/tests/ui/zero_repeat_side_effects.fixed index b5fca36f3f089..4bdff6fd0f6c5 100644 --- a/src/tools/clippy/tests/ui/zero_repeat_side_effects.fixed +++ b/src/tools/clippy/tests/ui/zero_repeat_side_effects.fixed @@ -1,6 +1,11 @@ #![warn(clippy::zero_repeat_side_effects)] -#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)] -#![allow(clippy::no_effect)] // only fires _after_ the fix +#![allow( + clippy::unnecessary_operation, + clippy::useless_vec, + clippy::needless_late_init, + clippy::single_match, + clippy::no_effect // only fires _after_ the fix +)] fn f() -> i32 { println!("side effect"); @@ -119,3 +124,26 @@ fn issue_14681() { }); //~^ zero_repeat_side_effects } + +fn issue_15824() { + fn f() {} + + match 0 { + 0 => { + f(); + _ = [] as [(); 0] + }, + //~^ zero_repeat_side_effects + _ => {}, + } + + let mut a = [(); 0]; + match 0 { + 0 => { + f(); + a = [] as [(); 0] + }, + //~^ zero_repeat_side_effects + _ => {}, + } +} diff --git a/src/tools/clippy/tests/ui/zero_repeat_side_effects.rs b/src/tools/clippy/tests/ui/zero_repeat_side_effects.rs index ea043d21638cc..a1454d724c870 100644 --- a/src/tools/clippy/tests/ui/zero_repeat_side_effects.rs +++ b/src/tools/clippy/tests/ui/zero_repeat_side_effects.rs @@ -1,6 +1,11 @@ #![warn(clippy::zero_repeat_side_effects)] -#![expect(clippy::unnecessary_operation, clippy::useless_vec, clippy::needless_late_init)] -#![allow(clippy::no_effect)] // only fires _after_ the fix +#![allow( + clippy::unnecessary_operation, + clippy::useless_vec, + clippy::needless_late_init, + clippy::single_match, + clippy::no_effect // only fires _after_ the fix +)] fn f() -> i32 { println!("side effect"); @@ -102,3 +107,20 @@ fn issue_14681() { foo(&[Some(Some(S::new())); 0]); //~^ zero_repeat_side_effects } + +fn issue_15824() { + fn f() {} + + match 0 { + 0 => _ = [f(); 0], + //~^ zero_repeat_side_effects + _ => {}, + } + + let mut a = [(); 0]; + match 0 { + 0 => a = [f(); 0], + //~^ zero_repeat_side_effects + _ => {}, + } +} diff --git a/src/tools/clippy/tests/ui/zero_repeat_side_effects.stderr b/src/tools/clippy/tests/ui/zero_repeat_side_effects.stderr index 49e850d035343..f376a1501b001 100644 --- a/src/tools/clippy/tests/ui/zero_repeat_side_effects.stderr +++ b/src/tools/clippy/tests/ui/zero_repeat_side_effects.stderr @@ -1,5 +1,5 @@ error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:17:5 + --> tests/ui/zero_repeat_side_effects.rs:22:5 | LL | let a = [f(); 0]; | ^^^^^^^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL + let a: [i32; 0] = []; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:20:5 + --> tests/ui/zero_repeat_side_effects.rs:25:5 | LL | b = [f(); 0]; | ^^^^^^^^^^^^ @@ -25,7 +25,7 @@ LL ~ b = [] as [i32; 0]; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:25:5 + --> tests/ui/zero_repeat_side_effects.rs:30:5 | LL | let c = vec![f(); 0]; | ^^^^^^^^^^^^^^^^^^^^^ @@ -37,7 +37,7 @@ LL + let c: std::vec::Vec = vec![]; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:28:5 + --> tests/ui/zero_repeat_side_effects.rs:33:5 | LL | d = vec![f(); 0]; | ^^^^^^^^^^^^^^^^ @@ -49,7 +49,7 @@ LL ~ d = vec![] as std::vec::Vec; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:32:5 + --> tests/ui/zero_repeat_side_effects.rs:37:5 | LL | let e = [println!("side effect"); 0]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -61,7 +61,7 @@ LL + let e: [(); 0] = []; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:36:5 + --> tests/ui/zero_repeat_side_effects.rs:41:5 | LL | let g = [{ f() }; 0]; | ^^^^^^^^^^^^^^^^^^^^^ @@ -73,7 +73,7 @@ LL + let g: [i32; 0] = []; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:40:10 + --> tests/ui/zero_repeat_side_effects.rs:45:10 | LL | drop(vec![f(); 0]); | ^^^^^^^^^^^^ @@ -87,7 +87,7 @@ LL ~ }); | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:44:5 + --> tests/ui/zero_repeat_side_effects.rs:49:5 | LL | vec![f(); 0]; | ^^^^^^^^^^^^ @@ -99,7 +99,7 @@ LL ~ vec![] as std::vec::Vec; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:46:5 + --> tests/ui/zero_repeat_side_effects.rs:51:5 | LL | [f(); 0]; | ^^^^^^^^ @@ -111,7 +111,7 @@ LL ~ [] as [i32; 0]; | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:100:10 + --> tests/ui/zero_repeat_side_effects.rs:105:10 | LL | foo(&[Some(f()); 0]); | ^^^^^^^^^^^^^^ @@ -125,7 +125,7 @@ LL ~ }); | error: expression with side effects as the initial value in a zero-sized array initializer - --> tests/ui/zero_repeat_side_effects.rs:102:10 + --> tests/ui/zero_repeat_side_effects.rs:107:10 | LL | foo(&[Some(Some(S::new())); 0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -138,5 +138,33 @@ LL + [] as [std::option::Option>; 0] LL ~ }); | -error: aborting due to 11 previous errors +error: expression with side effects as the initial value in a zero-sized array initializer + --> tests/ui/zero_repeat_side_effects.rs:115:14 + | +LL | 0 => _ = [f(); 0], + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL ~ 0 => { +LL + f(); +LL + _ = [] as [(); 0] +LL ~ }, + | + +error: expression with side effects as the initial value in a zero-sized array initializer + --> tests/ui/zero_repeat_side_effects.rs:122:14 + | +LL | 0 => a = [f(); 0], + | ^^^^^^^^^^^^ + | +help: consider performing the side effect separately + | +LL ~ 0 => { +LL + f(); +LL + a = [] as [(); 0] +LL ~ }, + | + +error: aborting due to 13 previous errors diff --git a/src/tools/clippy/triagebot.toml b/src/tools/clippy/triagebot.toml index 3bf62b6b3bbaf..09dec7675e7e5 100644 --- a/src/tools/clippy/triagebot.toml +++ b/src/tools/clippy/triagebot.toml @@ -22,9 +22,6 @@ allow-unauthenticated = [ [mentions."clippy_lints/src/doc"] cc = ["@notriddle"] -# Prevents mentions in commits to avoid users being spammed -[no-mentions] - # Have rustbot inform users about the *No Merge Policy* [no-merges] exclude_titles = ["Rustup"] # exclude syncs from rust-lang/rust @@ -65,6 +62,7 @@ users_on_vacation = [ "Manishearth", "Alexendoo", "y21", + "blyxyas", ] [assign.owners] @@ -77,7 +75,6 @@ users_on_vacation = [ "@Alexendoo", "@dswij", "@Jarcho", - "@blyxyas", "@y21", "@samueltardieu", ] diff --git a/src/tools/clippy/util/gh-pages/style.css b/src/tools/clippy/util/gh-pages/style.css index 242e2227ed949..ce478a3e18d0b 100644 --- a/src/tools/clippy/util/gh-pages/style.css +++ b/src/tools/clippy/util/gh-pages/style.css @@ -637,14 +637,14 @@ pre, hr { display: flex; } -ul.dropdown-menu li.checkbox > button { +#menu-filters ul.dropdown-menu li.checkbox > button { border: 0; width: 100%; background: var(--theme-popup-bg); color: var(--fg); } -ul.dropdown-menu li.checkbox > button:hover { +#menu-filters ul.dropdown-menu li.checkbox > button:hover { background: var(--theme-hover); box-shadow: none; }