diff options
| author | bors <bors@rust-lang.org> | 2022-03-14 16:24:12 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2022-03-14 16:24:12 +0000 |
| commit | 285fa7ecd05dcbfdaf2faaf20400f5f92b39b3c6 (patch) | |
| tree | a8b13585116a678e07763e47514130459f5e4347 | |
| parent | bce19cf7f19ee5729defaccc86b068cc3c206c9c (diff) | |
| parent | 706fa54456f42086a943992e03354c75e1754307 (diff) | |
| download | rust-285fa7ecd05dcbfdaf2faaf20400f5f92b39b3c6.tar.gz rust-285fa7ecd05dcbfdaf2faaf20400f5f92b39b3c6.zip | |
Auto merge of #94929 - flip1995:clippyup, r=Manishearth
Update Clippy r? `@Manishearth` A few days delayed, because I recovered from a cold last week and couldn't get myself to do the sync, sorry. :upside_down_face:
92 files changed, 3351 insertions, 332 deletions
diff --git a/Cargo.lock b/Cargo.lock index 5d9edc5157d..dce5357b12d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -624,10 +624,10 @@ dependencies = [ "futures 0.3.19", "if_chain", "itertools", - "num_cpus", "parking_lot", "quote", "regex", + "rustc-semver", "rustc-workspace-hack", "rustc_tools_util 0.2.0", "semver", diff --git a/src/tools/clippy/.github/workflows/clippy.yml b/src/tools/clippy/.github/workflows/clippy.yml index 116ae031bb7..cd83bc9642b 100644 --- a/src/tools/clippy/.github/workflows/clippy.yml +++ b/src/tools/clippy/.github/workflows/clippy.yml @@ -74,10 +74,3 @@ jobs: run: bash .github/driver.sh env: OS: ${{ runner.os }} - - - name: Test cargo dev new lint - run: | - cargo dev new_lint --name new_early_pass --pass early - cargo dev new_lint --name new_late_pass --pass late - cargo check - git reset --hard HEAD diff --git a/src/tools/clippy/.github/workflows/clippy_bors.yml b/src/tools/clippy/.github/workflows/clippy_bors.yml index 989667037c1..f571485e6d3 100644 --- a/src/tools/clippy/.github/workflows/clippy_bors.yml +++ b/src/tools/clippy/.github/workflows/clippy_bors.yml @@ -143,13 +143,6 @@ jobs: env: OS: ${{ runner.os }} - - name: Test cargo dev new lint - run: | - cargo dev new_lint --name new_early_pass --pass early - cargo dev new_lint --name new_late_pass --pass late - cargo check - git reset --hard HEAD - integration_build: needs: changelog runs-on: ubuntu-latest diff --git a/src/tools/clippy/.github/workflows/clippy_dev.yml b/src/tools/clippy/.github/workflows/clippy_dev.yml index fe8bce00fa8..5dfab1d2ebc 100644 --- a/src/tools/clippy/.github/workflows/clippy_dev.yml +++ b/src/tools/clippy/.github/workflows/clippy_dev.yml @@ -36,6 +36,13 @@ jobs: - name: Test fmt run: cargo dev fmt --check + - name: Test cargo dev new lint + run: | + cargo dev new_lint --name new_early_pass --pass early + cargo dev new_lint --name new_late_pass --pass late + cargo check + git reset --hard HEAD + # These jobs doesn't actually test anything, but they're only used to tell # bors the build completed, as there is no practical way to detect when a # workflow is successful listening to webhooks only. diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 35983b7fb50..2bc393d6042 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -3042,6 +3042,7 @@ Released 2018-09-13 <!-- lint disable no-unused-definitions --> <!-- begin autogenerated links to lint list --> [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons +[`allow_attributes_without_reason`]: https://rust-lang.github.io/rust-clippy/master/index.html#allow_attributes_without_reason [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant [`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions @@ -3076,6 +3077,7 @@ Released 2018-09-13 [`cast_ptr_alignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment [`cast_ref_to_mut`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_ref_to_mut [`cast_sign_loss`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_sign_loss +[`cast_slice_different_sizes`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_slice_different_sizes [`char_lit_as_u8`]: https://rust-lang.github.io/rust-clippy/master/index.html#char_lit_as_u8 [`chars_last_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_last_cmp [`chars_next_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#chars_next_cmp @@ -3225,6 +3227,7 @@ Released 2018-09-13 [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero [`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next +[`iter_with_drain`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_with_drain [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero [`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits [`large_const_arrays`]: https://rust-lang.github.io/rust-clippy/master/index.html#large_const_arrays @@ -3297,6 +3300,7 @@ Released 2018-09-13 [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items [`missing_panics_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc [`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc +[`missing_spin_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_spin_loop [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes [`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals [`mod_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#mod_module_files @@ -3327,6 +3331,7 @@ Released 2018-09-13 [`needless_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_for_each [`needless_late_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_late_init [`needless_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes +[`needless_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_match [`needless_option_as_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_as_deref [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark @@ -3351,6 +3356,7 @@ Released 2018-09-13 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref [`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect +[`only_used_in_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#only_used_in_recursion [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap @@ -3498,6 +3504,7 @@ Released 2018-09-13 [`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord [`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map +[`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map [`unnecessary_fold`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fold [`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations [`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed diff --git a/src/tools/clippy/Cargo.toml b/src/tools/clippy/Cargo.toml index d4ca9480bec..123af23881b 100644 --- a/src/tools/clippy/Cargo.toml +++ b/src/tools/clippy/Cargo.toml @@ -50,7 +50,7 @@ syn = { version = "1.0", features = ["full"] } futures = "0.3" parking_lot = "0.11.2" tokio = { version = "1", features = ["io-util"] } -num_cpus = "1.13" +rustc-semver = "1.1" [build-dependencies] rustc_tools_util = { version = "0.2", path = "rustc_tools_util" } diff --git a/src/tools/clippy/clippy_lints/src/attrs.rs b/src/tools/clippy/clippy_lints/src/attrs.rs index a58d12ddd6b..d0b03c0a9c2 100644 --- a/src/tools/clippy/clippy_lints/src/attrs.rs +++ b/src/tools/clippy/clippy_lints/src/attrs.rs @@ -255,7 +255,38 @@ declare_clippy_lint! { "usage of `cfg(operating_system)` instead of `cfg(target_os = \"operating_system\")`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for attributes that allow lints without a reason. + /// + /// (This requires the `lint_reasons` feature) + /// + /// ### Why is this bad? + /// Allowing a lint should always have a reason. This reason should be documented to + /// ensure that others understand the reasoning + /// + /// ### Example + /// Bad: + /// ```rust + /// #![feature(lint_reasons)] + /// + /// #![allow(clippy::some_lint)] + /// ``` + /// + /// Good: + /// ```rust + /// #![feature(lint_reasons)] + /// + /// #![allow(clippy::some_lint, reason = "False positive rust-lang/rust-clippy#1002020")] + /// ``` + #[clippy::version = "1.61.0"] + pub ALLOW_ATTRIBUTES_WITHOUT_REASON, + restriction, + "ensures that all `allow` and `expect` attributes have a reason" +} + declare_lint_pass!(Attributes => [ + ALLOW_ATTRIBUTES_WITHOUT_REASON, INLINE_ALWAYS, DEPRECATED_SEMVER, USELESS_ATTRIBUTE, @@ -269,6 +300,9 @@ impl<'tcx> LateLintPass<'tcx> for Attributes { if is_lint_level(ident.name) { check_clippy_lint_names(cx, ident.name, items); } + if matches!(ident.name, sym::allow | sym::expect) { + check_lint_reason(cx, ident.name, items, attr); + } if items.is_empty() || !attr.has_name(sym::deprecated) { return; } @@ -404,6 +438,30 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, name: Symbol, items: &[NestedMe } } +fn check_lint_reason(cx: &LateContext<'_>, name: Symbol, items: &[NestedMetaItem], attr: &'_ Attribute) { + // Check for the feature + if !cx.tcx.sess.features_untracked().lint_reasons { + return; + } + + // Check if the reason is present + if let Some(item) = items.last().and_then(NestedMetaItem::meta_item) + && let MetaItemKind::NameValue(_) = &item.kind + && item.path == sym::reason + { + return; + } + + span_lint_and_help( + cx, + ALLOW_ATTRIBUTES_WITHOUT_REASON, + attr.span, + &format!("`{}` attribute without specifying a reason", name.as_str()), + None, + "try adding a reason at the end with `, reason = \"..\"`", + ); +} + fn is_relevant_item(cx: &LateContext<'_>, item: &Item<'_>) -> bool { if let ItemKind::Fn(_, _, eid) = item.kind { is_relevant_expr(cx, cx.tcx.typeck_body(eid), &cx.tcx.hir().body(eid).value) @@ -659,5 +717,5 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) { } fn is_lint_level(symbol: Symbol) -> bool { - matches!(symbol, sym::allow | sym::warn | sym::deny | sym::forbid) + matches!(symbol, sym::allow | sym::expect | sym::warn | sym::deny | sym::forbid) } diff --git a/src/tools/clippy/clippy_lints/src/casts/cast_slice_different_sizes.rs b/src/tools/clippy/clippy_lints/src/casts/cast_slice_different_sizes.rs new file mode 100644 index 00000000000..3608c1654d5 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/casts/cast_slice_different_sizes.rs @@ -0,0 +1,117 @@ +use clippy_utils::{diagnostics::span_lint_and_then, meets_msrv, msrvs, source::snippet_opt}; +use if_chain::if_chain; +use rustc_ast::Mutability; +use rustc_hir::{Expr, ExprKind, Node}; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, layout::LayoutOf, Ty, TypeAndMut}; +use rustc_semver::RustcVersion; + +use super::CAST_SLICE_DIFFERENT_SIZES; + +fn is_child_of_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + let map = cx.tcx.hir(); + if_chain! { + if let Some(parent_id) = map.find_parent_node(expr.hir_id); + if let Some(parent) = map.find(parent_id); + then { + let expr = match parent { + Node::Block(block) => { + if let Some(parent_expr) = block.expr { + parent_expr + } else { + return false; + } + }, + Node::Expr(expr) => expr, + _ => return false, + }; + + matches!(expr.kind, ExprKind::Cast(..)) + } else { + false + } + } +} + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Option<RustcVersion>) { + // suggestion is invalid if `ptr::slice_from_raw_parts` does not exist + if !meets_msrv(msrv.as_ref(), &msrvs::PTR_SLICE_RAW_PARTS) { + return; + } + + // if this cast is the child of another cast expression then don't emit something for it, the full + // chain will be analyzed + if is_child_of_cast(cx, expr) { + return; + } + + if let Some((from_slice_ty, to_slice_ty)) = expr_cast_chain_tys(cx, expr) { + if let (Ok(from_layout), Ok(to_layout)) = (cx.layout_of(from_slice_ty.ty), cx.layout_of(to_slice_ty.ty)) { + let from_size = from_layout.size.bytes(); + let to_size = to_layout.size.bytes(); + if from_size != to_size && from_size != 0 && to_size != 0 { + span_lint_and_then( + cx, + CAST_SLICE_DIFFERENT_SIZES, + expr.span, + &format!( + "casting between raw pointers to `[{}]` (element size {}) and `[{}]` (element size {}) does not adjust the count", + from_slice_ty, from_size, to_slice_ty, to_size, + ), + |diag| { + let cast_expr = match expr.kind { + ExprKind::Cast(cast_expr, ..) => cast_expr, + _ => unreachable!("expr should be a cast as checked by expr_cast_chain_tys"), + }; + let ptr_snippet = snippet_opt(cx, cast_expr.span).unwrap(); + + let (mutbl_fn_str, mutbl_ptr_str) = match to_slice_ty.mutbl { + Mutability::Mut => ("_mut", "mut"), + Mutability::Not => ("", "const"), + }; + let sugg = format!( + "core::ptr::slice_from_raw_parts{mutbl_fn_str}({ptr_snippet} as *{mutbl_ptr_str} {to_slice_ty}, ..)" + ); + + diag.span_suggestion( + expr.span, + &format!("replace with `ptr::slice_from_raw_parts{mutbl_fn_str}`"), + sugg, + rustc_errors::Applicability::HasPlaceholders, + ); + }, + ); + } + } + } +} + +/// Returns the type T of the pointed to *const [T] or *mut [T] and the mutability of the slice if +/// the type is one of those slices +fn get_raw_slice_ty_mut(ty: Ty<'_>) -> Option<TypeAndMut<'_>> { + match ty.kind() { + ty::RawPtr(TypeAndMut { ty: slice_ty, mutbl }) => match slice_ty.kind() { + ty::Slice(ty) => Some(TypeAndMut { ty: *ty, mutbl: *mutbl }), + _ => None, + }, + _ => None, + } +} + +/// Returns the pair (original ptr T, final ptr U) if the expression is composed of casts +/// Returns None if the expr is not a Cast +fn expr_cast_chain_tys<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<(TypeAndMut<'tcx>, TypeAndMut<'tcx>)> { + if let ExprKind::Cast(cast_expr, _cast_to_hir_ty) = expr.peel_blocks().kind { + let cast_to = cx.typeck_results().expr_ty(expr); + let to_slice_ty = get_raw_slice_ty_mut(cast_to)?; + if let Some((inner_from_ty, _inner_to_ty)) = expr_cast_chain_tys(cx, cast_expr) { + Some((inner_from_ty, to_slice_ty)) + } else { + let cast_from = cx.typeck_results().expr_ty(cast_expr); + let from_slice_ty = get_raw_slice_ty_mut(cast_from)?; + Some((from_slice_ty, to_slice_ty)) + } + } else { + None + } +} diff --git a/src/tools/clippy/clippy_lints/src/casts/mod.rs b/src/tools/clippy/clippy_lints/src/casts/mod.rs index f2077c569c0..6a0eabd089d 100644 --- a/src/tools/clippy/clippy_lints/src/casts/mod.rs +++ b/src/tools/clippy/clippy_lints/src/casts/mod.rs @@ -5,6 +5,7 @@ mod cast_precision_loss; mod cast_ptr_alignment; mod cast_ref_to_mut; mod cast_sign_loss; +mod cast_slice_different_sizes; mod char_lit_as_u8; mod fn_to_numeric_cast; mod fn_to_numeric_cast_any; @@ -409,6 +410,50 @@ declare_clippy_lint! { "casts from an enum type to an integral type which will truncate the value" } +declare_clippy_lint! { + /// Checks for `as` casts between raw pointers to slices with differently sized elements. + /// + /// ### Why is this bad? + /// The produced raw pointer to a slice does not update its length metadata. The produced + /// pointer will point to a different number of bytes than the original pointer because the + /// length metadata of a raw slice pointer is in elements rather than bytes. + /// Producing a slice reference from the raw pointer will either create a slice with + /// less data (which can be surprising) or create a slice with more data and cause Undefined Behavior. + /// + /// ### Example + /// // Missing data + /// ```rust + /// let a = [1_i32, 2, 3, 4]; + /// let p = &a as *const [i32] as *const [u8]; + /// unsafe { + /// println!("{:?}", &*p); + /// } + /// ``` + /// // Undefined Behavior (note: also potential alignment issues) + /// ```rust + /// let a = [1_u8, 2, 3, 4]; + /// let p = &a as *const [u8] as *const [u32]; + /// unsafe { + /// println!("{:?}", &*p); + /// } + /// ``` + /// Instead use `ptr::slice_from_raw_parts` to construct a slice from a data pointer and the correct length + /// ```rust + /// let a = [1_i32, 2, 3, 4]; + /// let old_ptr = &a as *const [i32]; + /// // The data pointer is cast to a pointer to the target `u8` not `[u8]` + /// // The length comes from the known length of 4 i32s times the 4 bytes per i32 + /// let new_ptr = core::ptr::slice_from_raw_parts(old_ptr as *const u8, 16); + /// unsafe { + /// println!("{:?}", &*new_ptr); + /// } + /// ``` + #[clippy::version = "1.60.0"] + pub CAST_SLICE_DIFFERENT_SIZES, + correctness, + "casting using `as` between raw pointers to slices of types with different sizes" +} + pub struct Casts { msrv: Option<RustcVersion>, } @@ -428,6 +473,7 @@ impl_lint_pass!(Casts => [ CAST_LOSSLESS, CAST_REF_TO_MUT, CAST_PTR_ALIGNMENT, + CAST_SLICE_DIFFERENT_SIZES, UNNECESSARY_CAST, FN_TO_NUMERIC_CAST_ANY, FN_TO_NUMERIC_CAST, @@ -478,6 +524,8 @@ impl<'tcx> LateLintPass<'tcx> for Casts { cast_ref_to_mut::check(cx, expr); cast_ptr_alignment::check(cx, expr); char_lit_as_u8::check(cx, expr); + ptr_as_ptr::check(cx, expr, &self.msrv); + cast_slice_different_sizes::check(cx, expr, &self.msrv); } extract_msrv_attr!(LateContext); diff --git a/src/tools/clippy/clippy_lints/src/dbg_macro.rs b/src/tools/clippy/clippy_lints/src/dbg_macro.rs index df1a4128af3..a0e5d302633 100644 --- a/src/tools/clippy/clippy_lints/src/dbg_macro.rs +++ b/src/tools/clippy/clippy_lints/src/dbg_macro.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_in_test_function; use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::source::snippet_with_applicability; use rustc_errors::Applicability; @@ -35,6 +36,10 @@ impl LateLintPass<'_> for DbgMacro { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return }; if cx.tcx.is_diagnostic_item(sym::dbg_macro, macro_call.def_id) { + // we make an exception for test code + if is_in_test_function(cx.tcx, expr.hir_id) { + return; + } let mut applicability = Applicability::MachineApplicable; let suggestion = match expr.peel_drop_temps().kind { // dbg!() diff --git a/src/tools/clippy/clippy_lints/src/lib.register_all.rs b/src/tools/clippy/clippy_lints/src/lib.register_all.rs index abfb4603537..23bca5a0eab 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_all.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_all.rs @@ -25,6 +25,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(booleans::NONMINIMAL_BOOL), LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(casts::CAST_REF_TO_MUT), + LintId::of(casts::CAST_SLICE_DIFFERENT_SIZES), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::FN_TO_NUMERIC_CAST), LintId::of(casts::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), @@ -109,6 +110,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(loops::ITER_NEXT_LOOP), LintId::of(loops::MANUAL_FLATTEN), LintId::of(loops::MANUAL_MEMCPY), + LintId::of(loops::MISSING_SPIN_LOOP), LintId::of(loops::MUT_RANGE_BOUND), LintId::of(loops::NEEDLESS_COLLECT), LintId::of(loops::NEEDLESS_RANGE_LOOP), @@ -136,6 +138,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(matches::MATCH_OVERLAPPING_ARM), LintId::of(matches::MATCH_REF_PATS), LintId::of(matches::MATCH_SINGLE_BINDING), + LintId::of(matches::NEEDLESS_MATCH), LintId::of(matches::REDUNDANT_PATTERN_MATCHING), LintId::of(matches::SINGLE_MATCH), LintId::of(matches::WILDCARD_IN_OR_PATTERNS), @@ -163,6 +166,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::ITER_NTH_ZERO), LintId::of(methods::ITER_OVEREAGER_CLONED), LintId::of(methods::ITER_SKIP_NEXT), + LintId::of(methods::ITER_WITH_DRAIN), LintId::of(methods::MANUAL_FILTER_MAP), LintId::of(methods::MANUAL_FIND_MAP), LintId::of(methods::MANUAL_SATURATING_ARITHMETIC), @@ -189,6 +193,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::SUSPICIOUS_SPLITN), LintId::of(methods::UNINIT_ASSUMED_INIT), LintId::of(methods::UNNECESSARY_FILTER_MAP), + LintId::of(methods::UNNECESSARY_FIND_MAP), LintId::of(methods::UNNECESSARY_FOLD), LintId::of(methods::UNNECESSARY_LAZY_EVALUATIONS), LintId::of(methods::UNNECESSARY_TO_OWNED), @@ -231,6 +236,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS), LintId::of(octal_escapes::OCTAL_ESCAPES), + LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs b/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs index bd5ff613447..68d6c6ce5f7 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_complexity.rs @@ -30,6 +30,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_SINGLE_BINDING), + LintId::of(matches::NEEDLESS_MATCH), LintId::of(matches::WILDCARD_IN_OR_PATTERNS), LintId::of(methods::BIND_INSTEAD_OF_MAP), LintId::of(methods::CLONE_ON_COPY), @@ -49,6 +50,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(methods::SEARCH_IS_SOME), LintId::of(methods::SKIP_WHILE_NEXT), LintId::of(methods::UNNECESSARY_FILTER_MAP), + LintId::of(methods::UNNECESSARY_FIND_MAP), LintId::of(methods::USELESS_ASREF), LintId::of(misc::SHORT_CIRCUIT_STATEMENT), LintId::of(misc_early::UNNEEDED_WILDCARD_PATTERN), @@ -63,6 +65,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD), LintId::of(no_effect::NO_EFFECT), LintId::of(no_effect::UNNECESSARY_OPERATION), + LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL), LintId::of(precedence::PRECEDENCE), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs b/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs index d7bf91eb692..df63f84463d 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_correctness.rs @@ -13,6 +13,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(bit_mask::INEFFECTIVE_BIT_MASK), LintId::of(booleans::LOGIC_BUG), LintId::of(casts::CAST_REF_TO_MUT), + LintId::of(casts::CAST_SLICE_DIFFERENT_SIZES), LintId::of(copies::IFS_SAME_COND), LintId::of(copies::IF_SAME_THEN_ELSE), LintId::of(derive::DERIVE_HASH_XOR_EQ), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_internal.rs b/src/tools/clippy/clippy_lints/src/lib.register_internal.rs index 7d4c7d2adb5..4778f4fdfa7 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_internal.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_internal.rs @@ -14,6 +14,7 @@ store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![ LintId::of(utils::internal_lints::LINT_WITHOUT_LINT_PASS), LintId::of(utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM), LintId::of(utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE), + LintId::of(utils::internal_lints::MISSING_MSRV_ATTR_IMPL), LintId::of(utils::internal_lints::OUTER_EXPN_EXPN_DATA), LintId::of(utils::internal_lints::PRODUCE_ICE), LintId::of(utils::internal_lints::UNNECESSARY_SYMBOL_STR), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs index 686482671b4..1a45763a869 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs @@ -26,6 +26,8 @@ store.register_lints(&[ #[cfg(feature = "internal")] utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE, #[cfg(feature = "internal")] + utils::internal_lints::MISSING_MSRV_ATTR_IMPL, + #[cfg(feature = "internal")] utils::internal_lints::OUTER_EXPN_EXPN_DATA, #[cfg(feature = "internal")] utils::internal_lints::PRODUCE_ICE, @@ -42,6 +44,7 @@ store.register_lints(&[ assign_ops::ASSIGN_OP_PATTERN, assign_ops::MISREFACTORED_ASSIGN_OP, async_yields_async::ASYNC_YIELDS_ASYNC, + attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON, attrs::BLANKET_CLIPPY_RESTRICTION_LINTS, attrs::DEPRECATED_CFG_ATTR, attrs::DEPRECATED_SEMVER, @@ -75,6 +78,7 @@ store.register_lints(&[ casts::CAST_PTR_ALIGNMENT, casts::CAST_REF_TO_MUT, casts::CAST_SIGN_LOSS, + casts::CAST_SLICE_DIFFERENT_SIZES, casts::CHAR_LIT_AS_U8, casts::FN_TO_NUMERIC_CAST, casts::FN_TO_NUMERIC_CAST_ANY, @@ -219,6 +223,7 @@ store.register_lints(&[ loops::ITER_NEXT_LOOP, loops::MANUAL_FLATTEN, loops::MANUAL_MEMCPY, + loops::MISSING_SPIN_LOOP, loops::MUT_RANGE_BOUND, loops::NEEDLESS_COLLECT, loops::NEEDLESS_RANGE_LOOP, @@ -255,6 +260,7 @@ store.register_lints(&[ matches::MATCH_SINGLE_BINDING, matches::MATCH_WILDCARD_FOR_SINGLE_VARIANTS, matches::MATCH_WILD_ERR_ARM, + matches::NEEDLESS_MATCH, matches::REDUNDANT_PATTERN_MATCHING, matches::REST_PAT_IN_FULLY_BOUND_STRUCTS, matches::SINGLE_MATCH, @@ -296,6 +302,7 @@ store.register_lints(&[ methods::ITER_NTH_ZERO, methods::ITER_OVEREAGER_CLONED, methods::ITER_SKIP_NEXT, + methods::ITER_WITH_DRAIN, methods::MANUAL_FILTER_MAP, methods::MANUAL_FIND_MAP, methods::MANUAL_SATURATING_ARITHMETIC, @@ -323,6 +330,7 @@ store.register_lints(&[ methods::SUSPICIOUS_SPLITN, methods::UNINIT_ASSUMED_INIT, methods::UNNECESSARY_FILTER_MAP, + methods::UNNECESSARY_FIND_MAP, methods::UNNECESSARY_FOLD, methods::UNNECESSARY_LAZY_EVALUATIONS, methods::UNNECESSARY_TO_OWNED, @@ -392,6 +400,7 @@ store.register_lints(&[ non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY, nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES, octal_escapes::OCTAL_ESCAPES, + only_used_in_recursion::ONLY_USED_IN_RECURSION, open_options::NONSENSICAL_OPEN_OPTIONS, option_env_unwrap::OPTION_ENV_UNWRAP, option_if_let_else::OPTION_IF_LET_ELSE, diff --git a/src/tools/clippy/clippy_lints/src/lib.register_perf.rs b/src/tools/clippy/clippy_lints/src/lib.register_perf.rs index c44ef124bfa..6e9c0ee33a1 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_perf.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_perf.rs @@ -10,11 +10,13 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ LintId::of(large_const_arrays::LARGE_CONST_ARRAYS), LintId::of(large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(loops::MANUAL_MEMCPY), + LintId::of(loops::MISSING_SPIN_LOOP), LintId::of(loops::NEEDLESS_COLLECT), LintId::of(methods::EXPECT_FUN_CALL), LintId::of(methods::EXTEND_WITH_DRAIN), LintId::of(methods::ITER_NTH), LintId::of(methods::ITER_OVEREAGER_CLONED), + LintId::of(methods::ITER_WITH_DRAIN), LintId::of(methods::MANUAL_STR_REPEAT), LintId::of(methods::OR_FUN_CALL), LintId::of(methods::SINGLE_CHAR_PATTERN), diff --git a/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs b/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs index f89f35b885c..4e30fc38197 100644 --- a/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs +++ b/src/tools/clippy/clippy_lints/src/lib.register_restriction.rs @@ -8,6 +8,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve LintId::of(as_conversions::AS_CONVERSIONS), LintId::of(asm_syntax::INLINE_ASM_X86_ATT_SYNTAX), LintId::of(asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX), + LintId::of(attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON), LintId::of(casts::FN_TO_NUMERIC_CAST_ANY), LintId::of(create_dir::CREATE_DIR), LintId::of(dbg_macro::DBG_MACRO), diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 230e2b2ccdf..504235d0d1e 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -318,6 +318,7 @@ mod non_octal_unix_permissions; mod non_send_fields_in_send_ty; mod nonstandard_macro_braces; mod octal_escapes; +mod only_used_in_recursion; mod open_options; mod option_env_unwrap; mod option_if_let_else; @@ -505,6 +506,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(utils::internal_lints::LintWithoutLintPass::default())); store.register_late_pass(|| Box::new(utils::internal_lints::MatchTypeOnDiagItem)); store.register_late_pass(|| Box::new(utils::internal_lints::OuterExpnDataPass)); + store.register_late_pass(|| Box::new(utils::internal_lints::MsrvAttrImpl)); } store.register_late_pass(|| Box::new(utils::author::Author)); @@ -856,6 +858,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(borrow_as_ptr::BorrowAsPtr::new(msrv))); store.register_late_pass(move || Box::new(manual_bits::ManualBits::new(msrv))); store.register_late_pass(|| Box::new(default_union_representation::DefaultUnionRepresentation)); + store.register_late_pass(|| Box::new(only_used_in_recursion::OnlyUsedInRecursion)); store.register_late_pass(|| Box::new(dbg_macro::DbgMacro)); let cargo_ignore_publish = conf.cargo_ignore_publish; store.register_late_pass(move || { diff --git a/src/tools/clippy/clippy_lints/src/loops/missing_spin_loop.rs b/src/tools/clippy/clippy_lints/src/loops/missing_spin_loop.rs new file mode 100644 index 00000000000..0696afa3922 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/loops/missing_spin_loop.rs @@ -0,0 +1,56 @@ +use super::MISSING_SPIN_LOOP; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_no_std_crate; +use rustc_errors::Applicability; +use rustc_hir::{Block, Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_middle::ty; +use rustc_span::sym; + +fn unpack_cond<'tcx>(cond: &'tcx Expr<'tcx>) -> &'tcx Expr<'tcx> { + match &cond.kind { + ExprKind::Block( + Block { + stmts: [], + expr: Some(e), + .. + }, + _, + ) + | ExprKind::Unary(_, e) => unpack_cond(e), + ExprKind::Binary(_, l, r) => { + let l = unpack_cond(l); + if let ExprKind::MethodCall(..) = l.kind { + l + } else { + unpack_cond(r) + } + }, + _ => cond, + } +} + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, body: &'tcx Expr<'_>) { + if_chain! { + if let ExprKind::Block(Block { stmts: [], expr: None, ..}, _) = body.kind; + if let ExprKind::MethodCall(method, [callee, ..], _) = unpack_cond(cond).kind; + if [sym::load, sym::compare_exchange, sym::compare_exchange_weak].contains(&method.ident.name); + if let ty::Adt(def, _substs) = cx.typeck_results().expr_ty(callee).kind(); + if cx.tcx.is_diagnostic_item(sym::AtomicBool, def.did()); + then { + span_lint_and_sugg( + cx, + MISSING_SPIN_LOOP, + body.span, + "busy-waiting loop should at least have a spin loop hint", + "try this", + (if is_no_std_crate(cx) { + "{ core::hint::spin_loop() }" + } else { + "{ std::hint::spin_loop() }" + }).into(), + Applicability::MachineApplicable + ); + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/loops/mod.rs b/src/tools/clippy/clippy_lints/src/loops/mod.rs index 5bc32acf56e..f029067d367 100644 --- a/src/tools/clippy/clippy_lints/src/loops/mod.rs +++ b/src/tools/clippy/clippy_lints/src/loops/mod.rs @@ -7,6 +7,7 @@ mod for_loops_over_fallibles; mod iter_next_loop; mod manual_flatten; mod manual_memcpy; +mod missing_spin_loop; mod mut_range_bound; mod needless_collect; mod needless_range_loop; @@ -560,6 +561,42 @@ declare_clippy_lint! { "for loops over `Option`s or `Result`s with a single expression can be simplified" } +declare_clippy_lint! { + /// ### What it does + /// Check for empty spin loops + /// + /// ### Why is this bad? + /// The loop body should have something like `thread::park()` or at least + /// `std::hint::spin_loop()` to avoid needlessly burning cycles and conserve + /// energy. Perhaps even better use an actual lock, if possible. + /// + /// ### Known problems + /// This lint doesn't currently trigger on `while let` or + /// `loop { match .. { .. } }` loops, which would be considered idiomatic in + /// combination with e.g. `AtomicBool::compare_exchange_weak`. + /// + /// ### Example + /// + /// ```ignore + /// use core::sync::atomic::{AtomicBool, Ordering}; + /// let b = AtomicBool::new(true); + /// // give a ref to `b` to another thread,wait for it to become false + /// while b.load(Ordering::Acquire) {}; + /// ``` + /// Use instead: + /// ```rust,no_run + ///# use core::sync::atomic::{AtomicBool, Ordering}; + ///# let b = AtomicBool::new(true); + /// while b.load(Ordering::Acquire) { + /// std::hint::spin_loop() + /// } + /// ``` + #[clippy::version = "1.59.0"] + pub MISSING_SPIN_LOOP, + perf, + "An empty busy waiting loop" +} + declare_lint_pass!(Loops => [ MANUAL_MEMCPY, MANUAL_FLATTEN, @@ -579,6 +616,7 @@ declare_lint_pass!(Loops => [ WHILE_IMMUTABLE_CONDITION, SAME_ITEM_PUSH, SINGLE_ELEMENT_LOOP, + MISSING_SPIN_LOOP, ]); impl<'tcx> LateLintPass<'tcx> for Loops { @@ -628,6 +666,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops { if let Some(higher::While { condition, body }) = higher::While::hir(expr) { while_immutable_condition::check(cx, condition, body); + missing_spin_loop::check(cx, condition, body); } needless_collect::check(expr, cx); diff --git a/src/tools/clippy/clippy_lints/src/matches/mod.rs b/src/tools/clippy/clippy_lints/src/matches/mod.rs index 92179eb6f0e..ff85623acf4 100644 --- a/src/tools/clippy/clippy_lints/src/matches/mod.rs +++ b/src/tools/clippy/clippy_lints/src/matches/mod.rs @@ -16,6 +16,7 @@ mod match_same_arms; mod match_single_binding; mod match_wild_enum; mod match_wild_err_arm; +mod needless_match; mod overlapping_arms; mod redundant_pattern_match; mod rest_pat_in_fully_bound_struct; @@ -566,6 +567,49 @@ declare_clippy_lint! { "`match` with identical arm bodies" } +declare_clippy_lint! { + /// ### What it does + /// Checks for unnecessary `match` or match-like `if let` returns for `Option` and `Result` + /// when function signatures are the same. + /// + /// ### Why is this bad? + /// This `match` block does nothing and might not be what the coder intended. + /// + /// ### Example + /// ```rust,ignore + /// fn foo() -> Result<(), i32> { + /// match result { + /// Ok(val) => Ok(val), + /// Err(err) => Err(err), + /// } + /// } + /// + /// fn bar() -> Option<i32> { + /// if let Some(val) = option { + /// Some(val) + /// } else { + /// None + /// } + /// } + /// ``` + /// + /// Could be replaced as + /// + /// ```rust,ignore + /// fn foo() -> Result<(), i32> { + /// result + /// } + /// + /// fn bar() -> Option<i32> { + /// option + /// } + /// ``` + #[clippy::version = "1.61.0"] + pub NEEDLESS_MATCH, + complexity, + "`match` or match-like `if let` that are unnecessary" +} + #[derive(Default)] pub struct Matches { msrv: Option<RustcVersion>, @@ -599,6 +643,7 @@ impl_lint_pass!(Matches => [ REDUNDANT_PATTERN_MATCHING, MATCH_LIKE_MATCHES_MACRO, MATCH_SAME_ARMS, + NEEDLESS_MATCH, ]); impl<'tcx> LateLintPass<'tcx> for Matches { @@ -622,6 +667,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { overlapping_arms::check(cx, ex, arms); match_wild_enum::check(cx, ex, arms); match_as_ref::check(cx, ex, arms, expr); + needless_match::check_match(cx, ex, arms); if self.infallible_destructuring_match_linted { self.infallible_destructuring_match_linted = false; @@ -640,6 +686,7 @@ impl<'tcx> LateLintPass<'tcx> for Matches { match_like_matches::check(cx, expr); } redundant_pattern_match::check(cx, expr); + needless_match::check(cx, expr); } } diff --git a/src/tools/clippy/clippy_lints/src/matches/needless_match.rs b/src/tools/clippy/clippy_lints/src/matches/needless_match.rs new file mode 100644 index 00000000000..76131d307d7 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/matches/needless_match.rs @@ -0,0 +1,197 @@ +use super::NEEDLESS_MATCH; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{eq_expr_value, get_parent_expr, higher, is_else_clause, is_lang_ctor, peel_blocks_with_stmt}; +use rustc_errors::Applicability; +use rustc_hir::LangItem::OptionNone; +use rustc_hir::{Arm, BindingAnnotation, Expr, ExprKind, Pat, PatKind, Path, PathSegment, QPath, UnOp}; +use rustc_lint::LateContext; +use rustc_span::sym; + +pub(crate) fn check_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) { + // This is for avoiding collision with `match_single_binding`. + if arms.len() < 2 { + return; + } + + for arm in arms { + if let PatKind::Wild = arm.pat.kind { + let ret_expr = strip_return(arm.body); + if !eq_expr_value(cx, ex, ret_expr) { + return; + } + } else if !pat_same_as_expr(arm.pat, arm.body) { + return; + } + } + + if let Some(match_expr) = get_parent_expr(cx, ex) { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + NEEDLESS_MATCH, + match_expr.span, + "this match expression is unnecessary", + "replace it with", + snippet_with_applicability(cx, ex.span, "..", &mut applicability).to_string(), + applicability, + ); + } +} + +/// Check for nop `if let` expression that assembled as unnecessary match +/// +/// ```rust,ignore +/// if let Some(a) = option { +/// Some(a) +/// } else { +/// None +/// } +/// ``` +/// OR +/// ```rust,ignore +/// if let SomeEnum::A = some_enum { +/// SomeEnum::A +/// } else if let SomeEnum::B = some_enum { +/// SomeEnum::B +/// } else { +/// some_enum +/// } +/// ``` +pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>) { + if_chain! { + if let Some(ref if_let) = higher::IfLet::hir(cx, ex); + if !is_else_clause(cx.tcx, ex); + if check_if_let(cx, if_let); + then { + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + NEEDLESS_MATCH, + ex.span, + "this if-let expression is unnecessary", + "replace it with", + snippet_with_applicability(cx, if_let.let_expr.span, "..", &mut applicability).to_string(), + applicability, + ); + } + } +} + +fn check_if_let(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool { + if let Some(if_else) = if_let.if_else { + if !pat_same_as_expr(if_let.let_pat, peel_blocks_with_stmt(if_let.if_then)) { + return false; + } + + // Recurrsively check for each `else if let` phrase, + if let Some(ref nested_if_let) = higher::IfLet::hir(cx, if_else) { + return check_if_let(cx, nested_if_let); + } + + if matches!(if_else.kind, ExprKind::Block(..)) { + let else_expr = peel_blocks_with_stmt(if_else); + let ret = strip_return(else_expr); + let let_expr_ty = cx.typeck_results().expr_ty(if_let.let_expr); + if is_type_diagnostic_item(cx, let_expr_ty, sym::Option) { + if let ExprKind::Path(ref qpath) = ret.kind { + return is_lang_ctor(cx, qpath, OptionNone) || eq_expr_value(cx, if_let.let_expr, ret); + } + } else { + return eq_expr_value(cx, if_let.let_expr, ret); + } + return true; + } + } + false +} + +/// Strip `return` keyword if the expression type is `ExprKind::Ret`. +fn strip_return<'hir>(expr: &'hir Expr<'hir>) -> &'hir Expr<'hir> { + if let ExprKind::Ret(Some(ret)) = expr.kind { + ret + } else { + expr + } +} + +fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool { + let expr = strip_return(expr); + match (&pat.kind, &expr.kind) { + // Example: `Some(val) => Some(val)` + ( + PatKind::TupleStruct(QPath::Resolved(_, path), [first_pat, ..], _), + ExprKind::Call(call_expr, [first_param, ..]), + ) => { + if let ExprKind::Path(QPath::Resolved(_, call_path)) = call_expr.kind { + if has_identical_segments(path.segments, call_path.segments) + && has_same_non_ref_symbol(first_pat, first_param) + { + return true; + } + } + }, + // Example: `val => val`, or `ref val => *val` + (PatKind::Binding(annot, _, pat_ident, _), _) => { + let new_expr = if let ( + BindingAnnotation::Ref | BindingAnnotation::RefMut, + ExprKind::Unary(UnOp::Deref, operand_expr), + ) = (annot, &expr.kind) + { + operand_expr + } else { + expr + }; + + if let ExprKind::Path(QPath::Resolved( + _, + Path { + segments: [first_seg, ..], + .. + }, + )) = new_expr.kind + { + return pat_ident.name == first_seg.ident.name; + } + }, + // Example: `Custom::TypeA => Custom::TypeB`, or `None => None` + (PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => { + return has_identical_segments(p_path.segments, e_path.segments); + }, + // Example: `5 => 5` + (PatKind::Lit(pat_lit_expr), ExprKind::Lit(expr_spanned)) => { + if let ExprKind::Lit(pat_spanned) = &pat_lit_expr.kind { + return pat_spanned.node == expr_spanned.node; + } + }, + _ => {}, + } + + false +} + +fn has_identical_segments(left_segs: &[PathSegment<'_>], right_segs: &[PathSegment<'_>]) -> bool { + if left_segs.len() != right_segs.len() { + return false; + } + for i in 0..left_segs.len() { + if left_segs[i].ident.name != right_segs[i].ident.name { + return false; + } + } + true +} + +fn has_same_non_ref_symbol(pat: &Pat<'_>, expr: &Expr<'_>) -> bool { + if_chain! { + if let PatKind::Binding(annot, _, pat_ident, _) = pat.kind; + if !matches!(annot, BindingAnnotation::Ref | BindingAnnotation::RefMut); + if let ExprKind::Path(QPath::Resolved(_, Path {segments: [first_seg, ..], .. })) = expr.kind; + then { + return pat_ident.name == first_seg.ident.name; + } + } + + false +} diff --git a/src/tools/clippy/clippy_lints/src/methods/iter_with_drain.rs b/src/tools/clippy/clippy_lints/src/methods/iter_with_drain.rs new file mode 100644 index 00000000000..958c3773087 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/methods/iter_with_drain.rs @@ -0,0 +1,72 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_integer_const; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{ + higher::{self, Range}, + SpanlessEq, +}; +use rustc_ast::ast::RangeLimits; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::LateContext; +use rustc_span::symbol::{sym, Symbol}; +use rustc_span::Span; + +use super::ITER_WITH_DRAIN; + +const DRAIN_TYPES: &[Symbol] = &[sym::Vec, sym::VecDeque]; + +pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span, arg: &Expr<'_>) { + let ty = cx.typeck_results().expr_ty(recv).peel_refs(); + if let Some(drained_type) = DRAIN_TYPES.iter().find(|&&sym| is_type_diagnostic_item(cx, ty, sym)) { + // Refuse to emit `into_iter` suggestion on draining struct fields due + // to the strong possibility of processing unmovable field. + if let ExprKind::Field(..) = recv.kind { + return; + } + + if let Some(range) = higher::Range::hir(arg) { + let left_full = match range { + Range { start: Some(start), .. } if is_integer_const(cx, start, 0) => true, + Range { start: None, .. } => true, + _ => false, + }; + let full = left_full + && match range { + Range { + end: Some(end), + limits: RangeLimits::HalfOpen, + .. + } => { + // `x.drain(..x.len())` call + if_chain! { + if let ExprKind::MethodCall(len_path, len_args, _) = end.kind; + if len_path.ident.name == sym::len && len_args.len() == 1; + if let ExprKind::Path(QPath::Resolved(_, drain_path)) = recv.kind; + if let ExprKind::Path(QPath::Resolved(_, len_path)) = len_args[0].kind; + if SpanlessEq::new(cx).eq_path(drain_path, len_path); + then { true } + else { false } + } + }, + Range { + end: None, + limits: RangeLimits::HalfOpen, + .. + } => true, + _ => false, + }; + if full { + span_lint_and_sugg( + cx, + ITER_WITH_DRAIN, + span.with_hi(expr.span.hi()), + &format!("`drain(..)` used on a `{}`", drained_type), + "try this", + "into_iter()".to_string(), + Applicability::MaybeIncorrect, + ); + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 3021a40fae1..5edd22cd14c 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -32,6 +32,7 @@ mod iter_nth; mod iter_nth_zero; mod iter_overeager_cloned; mod iter_skip_next; +mod iter_with_drain; mod iterator_step_by_zero; mod manual_saturating_arithmetic; mod manual_str_repeat; @@ -1120,6 +1121,31 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for use of `.drain(..)` on `Vec` and `VecDeque` for iteration. + /// + /// ### Why is this bad? + /// `.into_iter()` is simpler with better performance. + /// + /// ### Example + /// ```rust + /// # use std::collections::HashSet; + /// let mut foo = vec![0, 1, 2, 3]; + /// let bar: HashSet<usize> = foo.drain(..).collect(); + /// ``` + /// Use instead: + /// ```rust + /// # use std::collections::HashSet; + /// let foo = vec![0, 1, 2, 3]; + /// let bar: HashSet<usize> = foo.into_iter().collect(); + /// ``` + #[clippy::version = "1.61.0"] + pub ITER_WITH_DRAIN, + perf, + "replace `.drain(..)` with `.into_iter()`" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for use of `.get().unwrap()` (or /// `.get_mut().unwrap`) on a standard library type which implements `Index` /// @@ -1309,7 +1335,7 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for `filter_map` calls which could be replaced by `filter` or `map`. + /// Checks for `filter_map` calls that could be replaced by `filter` or `map`. /// More specifically it checks if the closure provided is only performing one of the /// filter or map operations and suggests the appropriate option. /// @@ -1339,6 +1365,36 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does + /// Checks for `find_map` calls that could be replaced by `find` or `map`. More + /// specifically it checks if the closure provided is only performing one of the + /// find or map operations and suggests the appropriate option. + /// + /// ### Why is this bad? + /// Complexity. The intent is also clearer if only a single + /// operation is being performed. + /// + /// ### Example + /// ```rust + /// let _ = (0..3).find_map(|x| if x > 2 { Some(x) } else { None }); + /// + /// // As there is no transformation of the argument this could be written as: + /// let _ = (0..3).find(|&x| x > 2); + /// ``` + /// + /// ```rust + /// let _ = (0..4).find_map(|x| Some(x + 1)); + /// + /// // As there is no conditional check on the argument this could be written as: + /// let _ = (0..4).map(|x| x + 1).next(); + /// ``` + #[clippy::version = "1.61.0"] + pub UNNECESSARY_FIND_MAP, + complexity, + "using `find_map` when a more succinct alternative exists" +} + +declare_clippy_lint! { + /// ### What it does /// Checks for `into_iter` calls on references which should be replaced by `iter` /// or `iter_mut`. /// @@ -2017,9 +2073,11 @@ impl_lint_pass!(Methods => [ GET_UNWRAP, STRING_EXTEND_CHARS, ITER_CLONED_COLLECT, + ITER_WITH_DRAIN, USELESS_ASREF, UNNECESSARY_FOLD, UNNECESSARY_FILTER_MAP, + UNNECESSARY_FIND_MAP, INTO_ITER_ON_REF, SUSPICIOUS_MAP, UNINIT_ASSUMED_INIT, @@ -2296,6 +2354,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio Some(("map", [_, arg], _)) => suspicious_map::check(cx, expr, recv, arg), _ => {}, }, + ("drain", [arg]) => { + iter_with_drain::check(cx, expr, recv, span, arg); + }, ("expect", [_]) => match method_call(recv) { Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv), _ => expect_used::check(cx, expr, recv), @@ -2305,9 +2366,12 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio extend_with_drain::check(cx, expr, recv, arg); }, ("filter_map", [arg]) => { - unnecessary_filter_map::check(cx, expr, arg); + unnecessary_filter_map::check(cx, expr, arg, name); filter_map_identity::check(cx, expr, arg, span); }, + ("find_map", [arg]) => { + unnecessary_filter_map::check(cx, expr, arg, name); + }, ("flat_map", [arg]) => { flat_map_identity::check(cx, expr, arg, span); flat_map_option::check(cx, expr, arg, span); diff --git a/src/tools/clippy/clippy_lints/src/methods/option_map_or_none.rs b/src/tools/clippy/clippy_lints/src/methods/option_map_or_none.rs index bdf8cea1207..2a5ab6e625c 100644 --- a/src/tools/clippy/clippy_lints/src/methods/option_map_or_none.rs +++ b/src/tools/clippy/clippy_lints/src/methods/option_map_or_none.rs @@ -14,10 +14,7 @@ use super::RESULT_MAP_OR_INTO_OPTION; // The expression inside a closure may or may not have surrounding braces // which causes problems when generating a suggestion. -fn reduce_unit_expression<'a>( - cx: &LateContext<'_>, - expr: &'a hir::Expr<'_>, -) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> { +fn reduce_unit_expression<'a>(expr: &'a hir::Expr<'_>) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> { match expr.kind { hir::ExprKind::Call(func, arg_char) => Some((func, arg_char)), hir::ExprKind::Block(block, _) => { @@ -25,7 +22,7 @@ fn reduce_unit_expression<'a>( (&[], Some(inner_expr)) => { // If block only contains an expression, // reduce `|x| { x + 1 }` to `|x| x + 1` - reduce_unit_expression(cx, inner_expr) + reduce_unit_expression(inner_expr) }, _ => None, } @@ -77,7 +74,7 @@ pub(super) fn check<'tcx>( if let hir::ExprKind::Closure(_, _, id, span, _) = map_arg.kind; let arg_snippet = snippet(cx, span, ".."); let body = cx.tcx.hir().body(id); - if let Some((func, [arg_char])) = reduce_unit_expression(cx, &body.value); + if let Some((func, [arg_char])) = reduce_unit_expression(&body.value); if let Some(id) = path_def_id(cx, func).and_then(|ctor_id| cx.tcx.parent(ctor_id)); if Some(id) == cx.tcx.lang_items().option_some_variant(); then { diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs index a307e33875e..2fda254ca98 100644 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -1,4 +1,6 @@ +use super::utils::clone_or_copy_needed; use clippy_utils::diagnostics::span_lint; +use clippy_utils::ty::is_copy; use clippy_utils::usage::mutated_variables; use clippy_utils::{is_lang_ctor, is_trait_method, path_to_local_id}; use rustc_hir as hir; @@ -9,8 +11,9 @@ use rustc_middle::ty; use rustc_span::sym; use super::UNNECESSARY_FILTER_MAP; +use super::UNNECESSARY_FIND_MAP; -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>) { +pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) { if !is_trait_method(cx, expr, sym::Iterator) { return; } @@ -20,6 +23,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< let arg_id = body.params[0].pat.hir_id; let mutates_arg = mutated_variables(&body.value, cx).map_or(true, |used_mutably| used_mutably.contains(&arg_id)); + let (clone_or_copy_needed, _) = clone_or_copy_needed(cx, body.params[0].pat, &body.value); let (mut found_mapping, mut found_filtering) = check_expression(cx, arg_id, &body.value); @@ -28,13 +32,15 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< found_mapping |= return_visitor.found_mapping; found_filtering |= return_visitor.found_filtering; + let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); let sugg = if !found_filtering { - "map" - } else if !found_mapping && !mutates_arg { - let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); + if name == "filter_map" { "map" } else { "map(..).next()" } + } else if !found_mapping && !mutates_arg && (!clone_or_copy_needed || is_copy(cx, in_ty)) { match cx.typeck_results().expr_ty(&body.value).kind() { - ty::Adt(adt, subst) if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) && in_ty == subst.type_at(0) => { - "filter" + ty::Adt(adt, subst) + if cx.tcx.is_diagnostic_item(sym::Option, adt.did()) && in_ty == subst.type_at(0) => + { + if name == "filter_map" { "filter" } else { "find" } }, _ => return, } @@ -43,9 +49,13 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< }; span_lint( cx, - UNNECESSARY_FILTER_MAP, + if name == "filter_map" { + UNNECESSARY_FILTER_MAP + } else { + UNNECESSARY_FIND_MAP + }, expr.span, - &format!("this `.filter_map` can be written more simply using `.{}`", sugg), + &format!("this `.{}` can be written more simply using `.{}`", name, sugg), ); } } diff --git a/src/tools/clippy/clippy_lints/src/methods/unnecessary_iter_cloned.rs b/src/tools/clippy/clippy_lints/src/methods/unnecessary_iter_cloned.rs index 65e94c5f44a..7a39557ad57 100644 --- a/src/tools/clippy/clippy_lints/src/methods/unnecessary_iter_cloned.rs +++ b/src/tools/clippy/clippy_lints/src/methods/unnecessary_iter_cloned.rs @@ -1,14 +1,12 @@ +use super::utils::clone_or_copy_needed; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::ForLoop; use clippy_utils::source::snippet_opt; use clippy_utils::ty::{get_associated_type, get_iterator_item_ty, implements_trait}; -use clippy_utils::{fn_def_id, get_parent_expr, path_to_local_id, usage}; +use clippy_utils::{fn_def_id, get_parent_expr}; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{def_id::DefId, BorrowKind, Expr, ExprKind, HirId, LangItem, Mutability, Pat}; +use rustc_hir::{def_id::DefId, Expr, ExprKind, LangItem}; use rustc_lint::LateContext; -use rustc_middle::hir::nested_filter; -use rustc_middle::ty; use rustc_span::{sym, Symbol}; use super::UNNECESSARY_TO_OWNED; @@ -100,89 +98,6 @@ pub fn check_for_loop_iter( false } -/// The core logic of `check_for_loop_iter` above, this function wraps a use of -/// `CloneOrCopyVisitor`. -fn clone_or_copy_needed<'tcx>( - cx: &LateContext<'tcx>, - pat: &Pat<'tcx>, - body: &'tcx Expr<'tcx>, -) -> (bool, Vec<&'tcx Expr<'tcx>>) { - let mut visitor = CloneOrCopyVisitor { - cx, - binding_hir_ids: pat_bindings(pat), - clone_or_copy_needed: false, - addr_of_exprs: Vec::new(), - }; - visitor.visit_expr(body); - (visitor.clone_or_copy_needed, visitor.addr_of_exprs) -} - -/// Returns a vector of all `HirId`s bound by the pattern. -fn pat_bindings(pat: &Pat<'_>) -> Vec<HirId> { - let mut collector = usage::ParamBindingIdCollector { - binding_hir_ids: Vec::new(), - }; - collector.visit_pat(pat); - collector.binding_hir_ids -} - -/// `clone_or_copy_needed` will be false when `CloneOrCopyVisitor` is done visiting if the only -/// operations performed on `binding_hir_ids` are: -/// * to take non-mutable references to them -/// * to use them as non-mutable `&self` in method calls -/// If any of `binding_hir_ids` is used in any other way, then `clone_or_copy_needed` will be true -/// when `CloneOrCopyVisitor` is done visiting. -struct CloneOrCopyVisitor<'cx, 'tcx> { - cx: &'cx LateContext<'tcx>, - binding_hir_ids: Vec<HirId>, - clone_or_copy_needed: bool, - addr_of_exprs: Vec<&'tcx Expr<'tcx>>, -} - -impl<'cx, 'tcx> Visitor<'tcx> for CloneOrCopyVisitor<'cx, 'tcx> { - type NestedFilter = nested_filter::OnlyBodies; - - fn nested_visit_map(&mut self) -> Self::Map { - self.cx.tcx.hir() - } - - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - walk_expr(self, expr); - if self.is_binding(expr) { - if let Some(parent) = get_parent_expr(self.cx, expr) { - match parent.kind { - ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) => { - self.addr_of_exprs.push(parent); - return; - }, - ExprKind::MethodCall(_, args, _) => { - if_chain! { - if args.iter().skip(1).all(|arg| !self.is_binding(arg)); - if let Some(method_def_id) = self.cx.typeck_results().type_dependent_def_id(parent.hir_id); - let method_ty = self.cx.tcx.type_of(method_def_id); - let self_ty = method_ty.fn_sig(self.cx.tcx).input(0).skip_binder(); - if matches!(self_ty.kind(), ty::Ref(_, _, Mutability::Not)); - then { - return; - } - } - }, - _ => {}, - } - } - self.clone_or_copy_needed = true; - } - } -} - -impl<'cx, 'tcx> CloneOrCopyVisitor<'cx, 'tcx> { - fn is_binding(&self, expr: &Expr<'tcx>) -> bool { - self.binding_hir_ids - .iter() - .any(|hir_id| path_to_local_id(expr, *hir_id)) - } -} - /// Returns true if the named method is `IntoIterator::into_iter`. pub fn is_into_iter(cx: &LateContext<'_>, callee_def_id: DefId) -> bool { cx.tcx.lang_items().require(LangItem::IntoIterIntoIter) == Ok(callee_def_id) diff --git a/src/tools/clippy/clippy_lints/src/methods/utils.rs b/src/tools/clippy/clippy_lints/src/methods/utils.rs index 63c3273bd68..3015531e843 100644 --- a/src/tools/clippy/clippy_lints/src/methods/utils.rs +++ b/src/tools/clippy/clippy_lints/src/methods/utils.rs @@ -1,10 +1,14 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{get_parent_expr, path_to_local_id, usage}; use if_chain::if_chain; use rustc_ast::ast; use rustc_errors::Applicability; use rustc_hir as hir; +use rustc_hir::intravisit::{walk_expr, Visitor}; +use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, Mutability, Pat}; use rustc_lint::LateContext; +use rustc_middle::hir::nested_filter; use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::sym; @@ -79,3 +83,86 @@ pub(super) fn get_hint_if_single_char_arg( } } } + +/// The core logic of `check_for_loop_iter` in `unnecessary_iter_cloned.rs`, this function wraps a +/// use of `CloneOrCopyVisitor`. +pub(super) fn clone_or_copy_needed<'tcx>( + cx: &LateContext<'tcx>, + pat: &Pat<'tcx>, + body: &'tcx Expr<'tcx>, +) -> (bool, Vec<&'tcx Expr<'tcx>>) { + let mut visitor = CloneOrCopyVisitor { + cx, + binding_hir_ids: pat_bindings(pat), + clone_or_copy_needed: false, + addr_of_exprs: Vec::new(), + }; + visitor.visit_expr(body); + (visitor.clone_or_copy_needed, visitor.addr_of_exprs) +} + +/// Returns a vector of all `HirId`s bound by the pattern. +fn pat_bindings(pat: &Pat<'_>) -> Vec<HirId> { + let mut collector = usage::ParamBindingIdCollector { + binding_hir_ids: Vec::new(), + }; + collector.visit_pat(pat); + collector.binding_hir_ids +} + +/// `clone_or_copy_needed` will be false when `CloneOrCopyVisitor` is done visiting if the only +/// operations performed on `binding_hir_ids` are: +/// * to take non-mutable references to them +/// * to use them as non-mutable `&self` in method calls +/// If any of `binding_hir_ids` is used in any other way, then `clone_or_copy_needed` will be true +/// when `CloneOrCopyVisitor` is done visiting. +struct CloneOrCopyVisitor<'cx, 'tcx> { + cx: &'cx LateContext<'tcx>, + binding_hir_ids: Vec<HirId>, + clone_or_copy_needed: bool, + addr_of_exprs: Vec<&'tcx Expr<'tcx>>, +} + +impl<'cx, 'tcx> Visitor<'tcx> for CloneOrCopyVisitor<'cx, 'tcx> { + type NestedFilter = nested_filter::OnlyBodies; + + fn nested_visit_map(&mut self) -> Self::Map { + self.cx.tcx.hir() + } + + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + walk_expr(self, expr); + if self.is_binding(expr) { + if let Some(parent) = get_parent_expr(self.cx, expr) { + match parent.kind { + ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, _) => { + self.addr_of_exprs.push(parent); + return; + }, + ExprKind::MethodCall(_, args, _) => { + if_chain! { + if args.iter().skip(1).all(|arg| !self.is_binding(arg)); + if let Some(method_def_id) = self.cx.typeck_results().type_dependent_def_id(parent.hir_id); + let method_ty = self.cx.tcx.type_of(method_def_id); + let self_ty = method_ty.fn_sig(self.cx.tcx).input(0).skip_binder(); + if matches!(self_ty.kind(), ty::Ref(_, _, Mutability::Not)); + then { + return; + } + } + }, + _ => {}, + } + } + self.clone_or_copy_needed = true; + } + } +} + +impl<'cx, 'tcx> CloneOrCopyVisitor<'cx, 'tcx> { + fn is_binding(&self, expr: &Expr<'tcx>) -> bool { + self.binding_hir_ids + .iter() + .any(|hir_id| path_to_local_id(expr, *hir_id)) + } +} diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index a57dc2b2798..5eb7b0f0521 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -199,7 +199,12 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue { let sugg = |diag: &mut Diagnostic| { if let ty::Adt(def, ..) = ty.kind() { if let Some(span) = cx.tcx.hir().span_if_local(def.did()) { - if can_type_implement_copy(cx.tcx, cx.param_env, ty, traits::ObligationCause::dummy_with_span(span)).is_ok() { + if can_type_implement_copy( + cx.tcx, + cx.param_env, + ty, + traits::ObligationCause::dummy_with_span(span), + ).is_ok() { diag.span_help(span, "consider marking this type as `Copy`"); } } diff --git a/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs b/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs new file mode 100644 index 00000000000..b828d9334ee --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs @@ -0,0 +1,668 @@ +use std::collections::VecDeque; + +use clippy_utils::diagnostics::span_lint_and_sugg; +use itertools::{izip, Itertools}; +use rustc_ast::{walk_list, Label, Mutability}; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_errors::Applicability; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::DefId; +use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; +use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; +use rustc_hir::{ + Arm, Block, Body, Expr, ExprKind, Guard, HirId, ImplicitSelfKind, Let, Local, Pat, PatKind, Path, PathSegment, + QPath, Stmt, StmtKind, TyKind, UnOp, +}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_middle::ty::{Ty, TyCtxt, TypeckResults}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::symbol::kw; +use rustc_span::symbol::Ident; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for arguments that are only used in recursion with no side-effects. + /// + /// ### Why is this bad? + /// It could contain a useless calculation and can make function simpler. + /// + /// The arguments can be involved in calculations and assignments but as long as + /// the calculations have no side-effects (function calls or mutating dereference) + /// and the assigned variables are also only in recursion, it is useless. + /// + /// ### Known problems + /// In some cases, this would not catch all useless arguments. + /// + /// ```rust + /// fn foo(a: usize, b: usize) -> usize { + /// let f = |x| x + 1; + /// + /// if a == 0 { + /// 1 + /// } else { + /// foo(a - 1, f(b)) + /// } + /// } + /// ``` + /// + /// For example, the argument `b` is only used in recursion, but the lint would not catch it. + /// + /// List of some examples that can not be caught: + /// - binary operation of non-primitive types + /// - closure usage + /// - some `break` relative operations + /// - struct pattern binding + /// + /// Also, when you recurse the function name with path segments, it is not possible to detect. + /// + /// ### Example + /// ```rust + /// fn f(a: usize, b: usize) -> usize { + /// if a == 0 { + /// 1 + /// } else { + /// f(a - 1, b + 1) + /// } + /// } + /// # fn main() { + /// # print!("{}", f(1, 1)); + /// # } + /// ``` + /// Use instead: + /// ```rust + /// fn f(a: usize) -> usize { + /// if a == 0 { + /// 1 + /// } else { + /// f(a - 1) + /// } + /// } + /// # fn main() { + /// # print!("{}", f(1)); + /// # } + /// ``` + #[clippy::version = "1.60.0"] + pub ONLY_USED_IN_RECURSION, + complexity, + "arguments that is only used in recursion can be removed" +} +declare_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]); + +impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion { + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + decl: &'tcx rustc_hir::FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + _: Span, + id: HirId, + ) { + if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind { + let def_id = id.owner.to_def_id(); + let data = cx.tcx.def_path(def_id).data; + + if data.len() > 1 { + match data.get(data.len() - 2) { + Some(DisambiguatedDefPathData { + data: DefPathData::Impl, + disambiguator, + }) if *disambiguator != 0 => return, + _ => {}, + } + } + + let has_self = !matches!(decl.implicit_self, ImplicitSelfKind::None); + + let ty_res = cx.typeck_results(); + let param_span = body + .params + .iter() + .flat_map(|param| { + let mut v = Vec::new(); + param.pat.each_binding(|_, hir_id, span, ident| { + v.push((hir_id, span, ident)); + }); + v + }) + .skip(if has_self { 1 } else { 0 }) + .filter(|(_, _, ident)| !ident.name.as_str().starts_with('_')) + .collect_vec(); + + let params = body.params.iter().map(|param| param.pat).collect(); + + let mut visitor = SideEffectVisit { + graph: FxHashMap::default(), + has_side_effect: FxHashSet::default(), + ret_vars: Vec::new(), + contains_side_effect: false, + break_vars: FxHashMap::default(), + params, + fn_ident: ident, + fn_def_id: def_id, + is_method: matches!(kind, FnKind::Method(..)), + has_self, + ty_res, + ty_ctx: cx.tcx, + }; + + visitor.visit_expr(&body.value); + let vars = std::mem::take(&mut visitor.ret_vars); + // this would set the return variables to side effect + visitor.add_side_effect(vars); + + let mut queue = visitor.has_side_effect.iter().copied().collect::<VecDeque<_>>(); + + // a simple BFS to check all the variables that have side effect + while let Some(id) = queue.pop_front() { + if let Some(next) = visitor.graph.get(&id) { + for i in next { + if !visitor.has_side_effect.contains(i) { + visitor.has_side_effect.insert(*i); + queue.push_back(*i); + } + } + } + } + + for (id, span, ident) in param_span { + // if the variable is not used in recursion, it would be marked as unused + if !visitor.has_side_effect.contains(&id) { + let mut queue = VecDeque::new(); + let mut visited = FxHashSet::default(); + + queue.push_back(id); + + // a simple BFS to check the graph can reach to itself + // if it can't, it means the variable is never used in recursion + while let Some(id) = queue.pop_front() { + if let Some(next) = visitor.graph.get(&id) { + for i in next { + if !visited.contains(i) { + visited.insert(id); + queue.push_back(*i); + } + } + } + } + + if visited.contains(&id) { + span_lint_and_sugg( + cx, + ONLY_USED_IN_RECURSION, + span, + "parameter is only used in recursion", + "if this is intentional, prefix with an underscore", + format!("_{}", ident.name.as_str()), + Applicability::MaybeIncorrect, + ); + } + } + } + } + } +} + +pub fn is_primitive(ty: Ty<'_>) -> bool { + match ty.kind() { + ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => true, + ty::Ref(_, t, _) => is_primitive(*t), + _ => false, + } +} + +pub fn is_array(ty: Ty<'_>) -> bool { + match ty.kind() { + ty::Array(..) | ty::Slice(..) => true, + ty::Ref(_, t, _) => is_array(*t), + _ => false, + } +} + +/// This builds the graph of side effect. +/// The edge `a -> b` means if `a` has side effect, `b` will have side effect. +/// +/// There are some exmaple in following code: +/// ```rust, ignore +/// let b = 1; +/// let a = b; // a -> b +/// let (c, d) = (a, b); // c -> b, d -> b +/// +/// let e = if a == 0 { // e -> a +/// c // e -> c +/// } else { +/// d // e -> d +/// }; +/// ``` +pub struct SideEffectVisit<'tcx> { + graph: FxHashMap<HirId, FxHashSet<HirId>>, + has_side_effect: FxHashSet<HirId>, + // bool for if the variable was dereferenced from mutable reference + ret_vars: Vec<(HirId, bool)>, + contains_side_effect: bool, + // break label + break_vars: FxHashMap<Ident, Vec<(HirId, bool)>>, + params: Vec<&'tcx Pat<'tcx>>, + fn_ident: Ident, + fn_def_id: DefId, + is_method: bool, + has_self: bool, + ty_res: &'tcx TypeckResults<'tcx>, + ty_ctx: TyCtxt<'tcx>, +} + +impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> { + fn visit_block(&mut self, b: &'tcx Block<'tcx>) { + b.stmts.iter().for_each(|stmt| { + self.visit_stmt(stmt); + self.ret_vars.clear(); + }); + walk_list!(self, visit_expr, b.expr); + } + + fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) { + match s.kind { + StmtKind::Local(Local { + pat, init: Some(init), .. + }) => { + self.visit_pat_expr(pat, init, false); + self.ret_vars.clear(); + }, + StmtKind::Item(i) => { + let item = self.ty_ctx.hir().item(i); + self.visit_item(item); + self.ret_vars.clear(); + }, + StmtKind::Expr(e) | StmtKind::Semi(e) => { + self.visit_expr(e); + self.ret_vars.clear(); + }, + StmtKind::Local(_) => {}, + } + } + + fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { + match ex.kind { + ExprKind::Array(exprs) | ExprKind::Tup(exprs) => { + self.ret_vars = exprs + .iter() + .flat_map(|expr| { + self.visit_expr(expr); + std::mem::take(&mut self.ret_vars) + }) + .collect(); + }, + ExprKind::Call(callee, args) => self.visit_fn(callee, args), + ExprKind::MethodCall(path, args, _) => self.visit_method_call(path, args), + ExprKind::Binary(_, lhs, rhs) => { + self.visit_bin_op(lhs, rhs); + }, + ExprKind::Unary(op, expr) => self.visit_un_op(op, expr), + ExprKind::Let(Let { pat, init, .. }) => self.visit_pat_expr(pat, init, false), + ExprKind::If(bind, then_expr, else_expr) => { + self.visit_if(bind, then_expr, else_expr); + }, + ExprKind::Match(expr, arms, _) => self.visit_match(expr, arms), + // since analysing the closure is not easy, just set all variables in it to side-effect + ExprKind::Closure(_, _, body_id, _, _) => { + let body = self.ty_ctx.hir().body(body_id); + self.visit_body(body); + let vars = std::mem::take(&mut self.ret_vars); + self.add_side_effect(vars); + }, + ExprKind::Loop(block, label, _, _) | ExprKind::Block(block, label) => { + self.visit_block_label(block, label); + }, + ExprKind::Assign(bind, expr, _) => { + self.visit_assign(bind, expr); + }, + ExprKind::AssignOp(_, bind, expr) => { + self.visit_assign(bind, expr); + self.visit_bin_op(bind, expr); + }, + ExprKind::Field(expr, _) => { + self.visit_expr(expr); + if matches!(self.ty_res.expr_ty(expr).kind(), ty::Ref(_, _, Mutability::Mut)) { + self.ret_vars.iter_mut().for_each(|(_, b)| *b = true); + } + }, + ExprKind::Index(expr, index) => { + self.visit_expr(expr); + let mut vars = std::mem::take(&mut self.ret_vars); + self.visit_expr(index); + self.ret_vars.append(&mut vars); + + if !is_array(self.ty_res.expr_ty(expr)) { + self.add_side_effect(self.ret_vars.clone()); + } else if matches!(self.ty_res.expr_ty(expr).kind(), ty::Ref(_, _, Mutability::Mut)) { + self.ret_vars.iter_mut().for_each(|(_, b)| *b = true); + } + }, + ExprKind::Break(dest, Some(expr)) => { + self.visit_expr(expr); + if let Some(label) = dest.label { + self.break_vars + .entry(label.ident) + .or_insert(Vec::new()) + .append(&mut self.ret_vars); + } + self.contains_side_effect = true; + }, + ExprKind::Ret(Some(expr)) => { + self.visit_expr(expr); + let vars = std::mem::take(&mut self.ret_vars); + self.add_side_effect(vars); + self.contains_side_effect = true; + }, + ExprKind::Break(_, None) | ExprKind::Continue(_) | ExprKind::Ret(None) => { + self.contains_side_effect = true; + }, + ExprKind::Struct(_, exprs, expr) => { + let mut ret_vars = exprs + .iter() + .flat_map(|field| { + self.visit_expr(field.expr); + std::mem::take(&mut self.ret_vars) + }) + .collect(); + + walk_list!(self, visit_expr, expr); + self.ret_vars.append(&mut ret_vars); + }, + _ => walk_expr(self, ex), + } + } + + fn visit_path(&mut self, path: &'tcx Path<'tcx>, _id: HirId) { + if let Res::Local(id) = path.res { + self.ret_vars.push((id, false)); + } + } +} + +impl<'tcx> SideEffectVisit<'tcx> { + fn visit_assign(&mut self, lhs: &'tcx Expr<'tcx>, rhs: &'tcx Expr<'tcx>) { + // Just support array and tuple unwrapping for now. + // + // ex) `(a, b) = (c, d);` + // The graph would look like this: + // a -> c + // b -> d + // + // This would minimize the connection of the side-effect graph. + match (&lhs.kind, &rhs.kind) { + (ExprKind::Array(lhs), ExprKind::Array(rhs)) | (ExprKind::Tup(lhs), ExprKind::Tup(rhs)) => { + // if not, it is a compile error + debug_assert!(lhs.len() == rhs.len()); + izip!(*lhs, *rhs).for_each(|(lhs, rhs)| self.visit_assign(lhs, rhs)); + }, + // in other assigns, we have to connect all each other + // because they can be connected somehow + _ => { + self.visit_expr(lhs); + let lhs_vars = std::mem::take(&mut self.ret_vars); + self.visit_expr(rhs); + let rhs_vars = std::mem::take(&mut self.ret_vars); + self.connect_assign(&lhs_vars, &rhs_vars, false); + }, + } + } + + fn visit_block_label(&mut self, block: &'tcx Block<'tcx>, label: Option<Label>) { + self.visit_block(block); + let _ = label.and_then(|label| { + self.break_vars + .remove(&label.ident) + .map(|mut break_vars| self.ret_vars.append(&mut break_vars)) + }); + } + + fn visit_bin_op(&mut self, lhs: &'tcx Expr<'tcx>, rhs: &'tcx Expr<'tcx>) { + self.visit_expr(lhs); + let mut ret_vars = std::mem::take(&mut self.ret_vars); + self.visit_expr(rhs); + self.ret_vars.append(&mut ret_vars); + + // the binary operation between non primitive values are overloaded operators + // so they can have side-effects + if !is_primitive(self.ty_res.expr_ty(lhs)) || !is_primitive(self.ty_res.expr_ty(rhs)) { + self.ret_vars.iter().for_each(|id| { + self.has_side_effect.insert(id.0); + }); + self.contains_side_effect = true; + } + } + + fn visit_un_op(&mut self, op: UnOp, expr: &'tcx Expr<'tcx>) { + self.visit_expr(expr); + let ty = self.ty_res.expr_ty(expr); + // dereferencing a reference has no side-effect + if !is_primitive(ty) && !matches!((op, ty.kind()), (UnOp::Deref, ty::Ref(..))) { + self.add_side_effect(self.ret_vars.clone()); + } + + if matches!((op, ty.kind()), (UnOp::Deref, ty::Ref(_, _, Mutability::Mut))) { + self.ret_vars.iter_mut().for_each(|(_, b)| *b = true); + } + } + + fn visit_pat_expr(&mut self, pat: &'tcx Pat<'tcx>, expr: &'tcx Expr<'tcx>, connect_self: bool) { + match (&pat.kind, &expr.kind) { + (PatKind::Tuple(pats, _), ExprKind::Tup(exprs)) => { + self.ret_vars = izip!(*pats, *exprs) + .flat_map(|(pat, expr)| { + self.visit_pat_expr(pat, expr, connect_self); + std::mem::take(&mut self.ret_vars) + }) + .collect(); + }, + (PatKind::Slice(front_exprs, _, back_exprs), ExprKind::Array(exprs)) => { + let mut vars = izip!(*front_exprs, *exprs) + .flat_map(|(pat, expr)| { + self.visit_pat_expr(pat, expr, connect_self); + std::mem::take(&mut self.ret_vars) + }) + .collect(); + self.ret_vars = izip!(back_exprs.iter().rev(), exprs.iter().rev()) + .flat_map(|(pat, expr)| { + self.visit_pat_expr(pat, expr, connect_self); + std::mem::take(&mut self.ret_vars) + }) + .collect(); + self.ret_vars.append(&mut vars); + }, + _ => { + let mut lhs_vars = Vec::new(); + pat.each_binding(|_, id, _, _| lhs_vars.push((id, false))); + self.visit_expr(expr); + let rhs_vars = std::mem::take(&mut self.ret_vars); + self.connect_assign(&lhs_vars, &rhs_vars, connect_self); + self.ret_vars = rhs_vars; + }, + } + } + + fn visit_fn(&mut self, callee: &'tcx Expr<'tcx>, args: &'tcx [Expr<'tcx>]) { + self.visit_expr(callee); + let mut ret_vars = std::mem::take(&mut self.ret_vars); + self.add_side_effect(ret_vars.clone()); + + let mut is_recursive = false; + + if_chain! { + if !self.has_self; + if let ExprKind::Path(QPath::Resolved(_, path)) = callee.kind; + if let Res::Def(DefKind::Fn, def_id) = path.res; + if self.fn_def_id == def_id; + then { + is_recursive = true; + } + } + + if_chain! { + if !self.has_self && self.is_method; + if let ExprKind::Path(QPath::TypeRelative(ty, segment)) = callee.kind; + if segment.ident == self.fn_ident; + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind; + if let Res::SelfTy{ .. } = path.res; + then { + is_recursive = true; + } + } + + if is_recursive { + izip!(self.params.clone(), args).for_each(|(pat, expr)| { + self.visit_pat_expr(pat, expr, true); + self.ret_vars.clear(); + }); + } else { + // This would set arguments used in closure that does not have side-effect. + // Closure itself can be detected whether there is a side-effect, but the + // value of variable that is holding closure can change. + // So, we just check the variables. + self.ret_vars = args + .iter() + .flat_map(|expr| { + self.visit_expr(expr); + std::mem::take(&mut self.ret_vars) + }) + .collect_vec() + .into_iter() + .map(|id| { + self.has_side_effect.insert(id.0); + id + }) + .collect(); + self.contains_side_effect = true; + } + + self.ret_vars.append(&mut ret_vars); + } + + fn visit_method_call(&mut self, path: &'tcx PathSegment<'tcx>, args: &'tcx [Expr<'tcx>]) { + if_chain! { + if self.is_method; + if path.ident == self.fn_ident; + if let ExprKind::Path(QPath::Resolved(_, path)) = args.first().unwrap().kind; + if let Res::Local(..) = path.res; + let ident = path.segments.last().unwrap().ident; + if ident.name == kw::SelfLower; + then { + izip!(self.params.clone(), args.iter()) + .for_each(|(pat, expr)| { + self.visit_pat_expr(pat, expr, true); + self.ret_vars.clear(); + }); + } else { + self.ret_vars = args + .iter() + .flat_map(|expr| { + self.visit_expr(expr); + std::mem::take(&mut self.ret_vars) + }) + .collect_vec() + .into_iter() + .map(|a| { + self.has_side_effect.insert(a.0); + a + }) + .collect(); + self.contains_side_effect = true; + } + } + } + + fn visit_if(&mut self, bind: &'tcx Expr<'tcx>, then_expr: &'tcx Expr<'tcx>, else_expr: Option<&'tcx Expr<'tcx>>) { + let contains_side_effect = self.contains_side_effect; + self.contains_side_effect = false; + self.visit_expr(bind); + let mut vars = std::mem::take(&mut self.ret_vars); + self.visit_expr(then_expr); + let mut then_vars = std::mem::take(&mut self.ret_vars); + walk_list!(self, visit_expr, else_expr); + if self.contains_side_effect { + self.add_side_effect(vars.clone()); + } + self.contains_side_effect |= contains_side_effect; + self.ret_vars.append(&mut vars); + self.ret_vars.append(&mut then_vars); + } + + fn visit_match(&mut self, expr: &'tcx Expr<'tcx>, arms: &'tcx [Arm<'tcx>]) { + self.visit_expr(expr); + let mut expr_vars = std::mem::take(&mut self.ret_vars); + self.ret_vars = arms + .iter() + .flat_map(|arm| { + let contains_side_effect = self.contains_side_effect; + self.contains_side_effect = false; + // this would visit `expr` multiple times + // but couldn't think of a better way + self.visit_pat_expr(arm.pat, expr, false); + let mut vars = std::mem::take(&mut self.ret_vars); + let _ = arm.guard.as_ref().map(|guard| { + self.visit_expr(match guard { + Guard::If(expr) | Guard::IfLet(_, expr) => expr, + }); + vars.append(&mut self.ret_vars); + }); + self.visit_expr(arm.body); + if self.contains_side_effect { + self.add_side_effect(vars.clone()); + self.add_side_effect(expr_vars.clone()); + } + self.contains_side_effect |= contains_side_effect; + vars.append(&mut self.ret_vars); + vars + }) + .collect(); + self.ret_vars.append(&mut expr_vars); + } + + fn connect_assign(&mut self, lhs: &[(HirId, bool)], rhs: &[(HirId, bool)], connect_self: bool) { + // if mutable dereference is on assignment it can have side-effect + // (this can lead to parameter mutable dereference and change the original value) + // too hard to detect whether this value is from parameter, so this would all + // check mutable dereference assignment to side effect + lhs.iter().filter(|(_, b)| *b).for_each(|(id, _)| { + self.has_side_effect.insert(*id); + self.contains_side_effect = true; + }); + + // there is no connection + if lhs.is_empty() || rhs.is_empty() { + return; + } + + // by connected rhs in cycle, the connections would decrease + // from `n * m` to `n + m` + // where `n` and `m` are length of `lhs` and `rhs`. + + // unwrap is possible since rhs is not empty + let rhs_first = rhs.first().unwrap(); + for (id, _) in lhs.iter() { + if connect_self || *id != rhs_first.0 { + self.graph + .entry(*id) + .or_insert_with(FxHashSet::default) + .insert(rhs_first.0); + } + } + + let rhs = rhs.iter(); + izip!(rhs.clone().cycle().skip(1), rhs).for_each(|(from, to)| { + if connect_self || from.0 != to.0 { + self.graph.entry(from.0).or_insert_with(FxHashSet::default).insert(to.0); + } + }); + } + + fn add_side_effect(&mut self, v: Vec<(HirId, bool)>) { + for (id, _) in v { + self.has_side_effect.insert(id); + self.contains_side_effect = true; + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/question_mark.rs b/src/tools/clippy/clippy_lints/src/question_mark.rs index 6f634ded5fe..899568a933f 100644 --- a/src/tools/clippy/clippy_lints/src/question_mark.rs +++ b/src/tools/clippy/clippy_lints/src/question_mark.rs @@ -123,7 +123,7 @@ impl QuestionMark { } fn result_check_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>, nested_expr: &Expr<'_>) -> bool { - Self::is_result(cx, expr) && Self::expression_returns_unmodified_err(cx, nested_expr, expr) + Self::is_result(cx, expr) && Self::expression_returns_unmodified_err(nested_expr, expr) } fn option_check_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>, nested_expr: &Expr<'_>) -> bool { @@ -156,9 +156,9 @@ impl QuestionMark { } } - fn expression_returns_unmodified_err(cx: &LateContext<'_>, expr: &Expr<'_>, cond_expr: &Expr<'_>) -> bool { + fn expression_returns_unmodified_err(expr: &Expr<'_>, cond_expr: &Expr<'_>) -> bool { match peel_blocks_with_stmt(expr).kind { - ExprKind::Ret(Some(ret_expr)) => Self::expression_returns_unmodified_err(cx, ret_expr, cond_expr), + ExprKind::Ret(Some(ret_expr)) => Self::expression_returns_unmodified_err(ret_expr, cond_expr), ExprKind::Path(_) => path_to_local(expr).is_some() && path_to_local(expr) == path_to_local(cond_expr), _ => false, } diff --git a/src/tools/clippy/clippy_lints/src/redundant_clone.rs b/src/tools/clippy/clippy_lints/src/redundant_clone.rs index 1f134be2cbc..1507c75ff61 100644 --- a/src/tools/clippy/clippy_lints/src/redundant_clone.rs +++ b/src/tools/clippy/clippy_lints/src/redundant_clone.rs @@ -3,7 +3,7 @@ use clippy_utils::source::snippet_opt; use clippy_utils::ty::{has_drop, is_copy, is_type_diagnostic_item, walk_ptrs_ty_depth}; use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths}; use if_chain::if_chain; -use rustc_data_structures::{fx::FxHashMap, transitive_relation::TransitiveRelation}; +use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{def_id, Body, FnDecl, HirId}; @@ -512,7 +512,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeStorageLive { /// For example, `b = &a; c = &a;` will make `b` and (transitively) `c` /// possible borrowers of `a`. struct PossibleBorrowerVisitor<'a, 'tcx> { - possible_borrower: TransitiveRelation<mir::Local>, + possible_borrower: TransitiveRelation, body: &'a mir::Body<'tcx>, cx: &'a LateContext<'tcx>, possible_origin: FxHashMap<mir::Local, HybridBitSet<mir::Local>>, @@ -543,18 +543,10 @@ impl<'a, 'tcx> PossibleBorrowerVisitor<'a, 'tcx> { continue; } - let borrowers = self.possible_borrower.reachable_from(row); + let mut borrowers = self.possible_borrower.reachable_from(row, self.body.local_decls.len()); + borrowers.remove(mir::Local::from_usize(0)); if !borrowers.is_empty() { - let mut bs = HybridBitSet::new_empty(self.body.local_decls.len()); - for c in borrowers { - if c != mir::Local::from_usize(0) { - bs.insert(c); - } - } - - if !bs.is_empty() { - map.insert(row, bs); - } + map.insert(row, borrowers); } } @@ -644,7 +636,7 @@ impl<'a, 'tcx> mir::visit::Visitor<'tcx> for PossibleBorrowerVisitor<'a, 'tcx> { /// For exampel, `_1 = &mut _2` generate _1: {_2,...} /// Known Problems: not sure all borrowed are tracked struct PossibleOriginVisitor<'a, 'tcx> { - possible_origin: TransitiveRelation<mir::Local>, + possible_origin: TransitiveRelation, body: &'a mir::Body<'tcx>, } @@ -663,18 +655,10 @@ impl<'a, 'tcx> PossibleOriginVisitor<'a, 'tcx> { continue; } - let borrowers = self.possible_origin.reachable_from(row); + let mut borrowers = self.possible_origin.reachable_from(row, self.body.local_decls.len()); + borrowers.remove(mir::Local::from_usize(0)); if !borrowers.is_empty() { - let mut bs = HybridBitSet::new_empty(self.body.local_decls.len()); - for c in borrowers { - if c != mir::Local::from_usize(0) { - bs.insert(c); - } - } - - if !bs.is_empty() { - map.insert(row, bs); - } + map.insert(row, borrowers); } } map @@ -766,3 +750,28 @@ impl PossibleBorrowerMap<'_, '_> { self.maybe_live.contains(local) } } + +#[derive(Default)] +struct TransitiveRelation { + relations: FxHashMap<mir::Local, Vec<mir::Local>>, +} +impl TransitiveRelation { + fn add(&mut self, a: mir::Local, b: mir::Local) { + self.relations.entry(a).or_default().push(b); + } + + fn reachable_from(&self, a: mir::Local, domain_size: usize) -> HybridBitSet<mir::Local> { + let mut seen = HybridBitSet::new_empty(domain_size); + let mut stack = vec![a]; + while let Some(u) = stack.pop() { + if let Some(edges) = self.relations.get(&u) { + for &v in edges { + if seen.insert(v) { + stack.push(v); + } + } + } + } + seen + } +} diff --git a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs index c3d984fb5ae..940a8428f77 100644 --- a/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs +++ b/src/tools/clippy/clippy_lints/src/suspicious_operation_groupings.rs @@ -399,9 +399,9 @@ fn if_statment_binops(kind: &ExprKind) -> Option<Vec<BinaryOp<'_>>> { fn append_opt_vecs<A>(target_opt: Option<Vec<A>>, source_opt: Option<Vec<A>>) -> Option<Vec<A>> { match (target_opt, source_opt) { - (Some(mut target), Some(mut source)) => { + (Some(mut target), Some(source)) => { target.reserve(source.len()); - for op in source.drain(..) { + for op in source { target.push(op); } Some(target) @@ -436,9 +436,9 @@ fn chained_binops_helper<'expr>(left_outer: &'expr Expr, right_outer: &'expr Exp chained_binops_helper(left_left, left_right), chained_binops_helper(right_left, right_right), ) { - (Some(mut left_ops), Some(mut right_ops)) => { + (Some(mut left_ops), Some(right_ops)) => { left_ops.reserve(right_ops.len()); - for op in right_ops.drain(..) { + for op in right_ops { left_ops.push(op); } Some(left_ops) diff --git a/src/tools/clippy/clippy_lints/src/transmute/mod.rs b/src/tools/clippy/clippy_lints/src/transmute/mod.rs index 1da3b765904..23cb9d40dfd 100644 --- a/src/tools/clippy/clippy_lints/src/transmute/mod.rs +++ b/src/tools/clippy/clippy_lints/src/transmute/mod.rs @@ -364,6 +364,10 @@ declare_clippy_lint! { /// ### Why is this bad? /// The results of such a transmute are not defined. /// + /// ### Known problems + /// This lint has had multiple problems in the past and was moved to `nursery`. See issue + /// [#8496](https://github.com/rust-lang/rust-clippy/issues/8496) for more details. + /// /// ### Example /// ```rust /// struct Foo<T>(u32, T); diff --git a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs index e6c260ed96a..d371cafb16b 100644 --- a/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs +++ b/src/tools/clippy/clippy_lints/src/unnecessary_sort_by.rs @@ -67,59 +67,51 @@ struct SortByKeyDetection { /// Detect if the two expressions are mirrored (identical, except one /// contains a and the other replaces it with b) -fn mirrored_exprs( - cx: &LateContext<'_>, - a_expr: &Expr<'_>, - a_ident: &Ident, - b_expr: &Expr<'_>, - b_ident: &Ident, -) -> bool { +fn mirrored_exprs(a_expr: &Expr<'_>, a_ident: &Ident, b_expr: &Expr<'_>, b_ident: &Ident) -> bool { match (&a_expr.kind, &b_expr.kind) { // Two boxes with mirrored contents (ExprKind::Box(left_expr), ExprKind::Box(right_expr)) => { - mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) + mirrored_exprs(left_expr, a_ident, right_expr, b_ident) }, // Two arrays with mirrored contents (ExprKind::Array(left_exprs), ExprKind::Array(right_exprs)) => { - iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident)) }, // The two exprs are function calls. // Check to see that the function itself and its arguments are mirrored (ExprKind::Call(left_expr, left_args), ExprKind::Call(right_expr, right_args)) => { - mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) - && iter::zip(*left_args, *right_args) - .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + mirrored_exprs(left_expr, a_ident, right_expr, b_ident) + && iter::zip(*left_args, *right_args).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident)) }, // The two exprs are method calls. // Check to see that the function is the same and the arguments are mirrored // This is enough because the receiver of the method is listed in the arguments (ExprKind::MethodCall(left_segment, left_args, _), ExprKind::MethodCall(right_segment, right_args, _)) => { left_segment.ident == right_segment.ident - && iter::zip(*left_args, *right_args) - .all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + && iter::zip(*left_args, *right_args).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident)) }, // Two tuples with mirrored contents (ExprKind::Tup(left_exprs), ExprKind::Tup(right_exprs)) => { - iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(cx, left, a_ident, right, b_ident)) + iter::zip(*left_exprs, *right_exprs).all(|(left, right)| mirrored_exprs(left, a_ident, right, b_ident)) }, // Two binary ops, which are the same operation and which have mirrored arguments (ExprKind::Binary(left_op, left_left, left_right), ExprKind::Binary(right_op, right_left, right_right)) => { left_op.node == right_op.node - && mirrored_exprs(cx, left_left, a_ident, right_left, b_ident) - && mirrored_exprs(cx, left_right, a_ident, right_right, b_ident) + && mirrored_exprs(left_left, a_ident, right_left, b_ident) + && mirrored_exprs(left_right, a_ident, right_right, b_ident) }, // Two unary ops, which are the same operation and which have the same argument (ExprKind::Unary(left_op, left_expr), ExprKind::Unary(right_op, right_expr)) => { - left_op == right_op && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident) + left_op == right_op && mirrored_exprs(left_expr, a_ident, right_expr, b_ident) }, // The two exprs are literals of some kind (ExprKind::Lit(left_lit), ExprKind::Lit(right_lit)) => left_lit.node == right_lit.node, - (ExprKind::Cast(left, _), ExprKind::Cast(right, _)) => mirrored_exprs(cx, left, a_ident, right, b_ident), + (ExprKind::Cast(left, _), ExprKind::Cast(right, _)) => mirrored_exprs(left, a_ident, right, b_ident), (ExprKind::DropTemps(left_block), ExprKind::DropTemps(right_block)) => { - mirrored_exprs(cx, left_block, a_ident, right_block, b_ident) + mirrored_exprs(left_block, a_ident, right_block, b_ident) }, (ExprKind::Field(left_expr, left_ident), ExprKind::Field(right_expr, right_ident)) => { - left_ident.name == right_ident.name && mirrored_exprs(cx, left_expr, a_ident, right_expr, right_ident) + left_ident.name == right_ident.name && mirrored_exprs(left_expr, a_ident, right_expr, right_ident) }, // Two paths: either one is a and the other is b, or they're identical to each other ( @@ -151,11 +143,9 @@ fn mirrored_exprs( ( ExprKind::AddrOf(left_kind, Mutability::Not, left_expr), ExprKind::AddrOf(right_kind, Mutability::Not, right_expr), - ) => left_kind == right_kind && mirrored_exprs(cx, left_expr, a_ident, right_expr, b_ident), - (_, ExprKind::AddrOf(_, Mutability::Not, right_expr)) => { - mirrored_exprs(cx, a_expr, a_ident, right_expr, b_ident) - }, - (ExprKind::AddrOf(_, Mutability::Not, left_expr), _) => mirrored_exprs(cx, left_expr, a_ident, b_expr, b_ident), + ) => left_kind == right_kind && mirrored_exprs(left_expr, a_ident, right_expr, b_ident), + (_, ExprKind::AddrOf(_, Mutability::Not, right_expr)) => mirrored_exprs(a_expr, a_ident, right_expr, b_ident), + (ExprKind::AddrOf(_, Mutability::Not, left_expr), _) => mirrored_exprs(left_expr, a_ident, b_expr, b_ident), _ => false, } } @@ -176,14 +166,13 @@ fn detect_lint(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<LintTrigger> { if method_path.ident.name == sym::cmp; then { let (closure_body, closure_arg, reverse) = if mirrored_exprs( - cx, left_expr, left_ident, right_expr, right_ident ) { (Sugg::hir(cx, left_expr, "..").to_string(), left_ident.name.to_string(), false) - } else if mirrored_exprs(cx, left_expr, right_ident, right_expr, left_ident) { + } else if mirrored_exprs(left_expr, right_ident, right_expr, left_ident) { (Sugg::hir(cx, left_expr, "..").to_string(), right_ident.name.to_string(), true) } else { return None; @@ -239,7 +228,7 @@ impl LateLintPass<'_> for UnnecessarySortBy { if trigger.unstable { "_unstable" } else { "" }, trigger.closure_arg, if trigger.reverse { - format!("Reverse({})", trigger.closure_body) + format!("std::cmp::Reverse({})", trigger.closure_body) } else { trigger.closure_body.to_string() }, diff --git a/src/tools/clippy/clippy_lints/src/use_self.rs b/src/tools/clippy/clippy_lints/src/use_self.rs index 80164c59ba7..09d671e1118 100644 --- a/src/tools/clippy/clippy_lints/src/use_self.rs +++ b/src/tools/clippy/clippy_lints/src/use_self.rs @@ -9,7 +9,8 @@ use rustc_hir::{ def::{CtorOf, DefKind, Res}, def_id::LocalDefId, intravisit::{walk_inf, walk_ty, Visitor}, - Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Path, QPath, TyKind, + Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Pat, PatKind, Path, QPath, + TyKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_semver::RustcVersion; @@ -252,6 +253,22 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf { } } + fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { + if_chain! { + if !pat.span.from_expansion(); + if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS); + if let Some(&StackItem::Check { impl_id, .. }) = self.stack.last(); + if let PatKind::Path(QPath::Resolved(_, path)) = pat.kind; + if !matches!(path.res, Res::SelfTy { .. } | Res::Def(DefKind::TyParam, _)); + if cx.typeck_results().pat_ty(pat) == cx.tcx.type_of(impl_id); + if let [first, ..] = path.segments; + if let Some(hir_id) = first.hir_id; + then { + span_lint(cx, cx.tcx.hir().span(hir_id)); + } + } + } + extract_msrv_attr!(LateContext); } diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs index 4433d5f5bf1..b3b241392fe 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints.rs @@ -25,7 +25,7 @@ use rustc_hir::{ use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; use rustc_middle::hir::nested_filter; use rustc_middle::mir::interpret::ConstValue; -use rustc_middle::ty; +use rustc_middle::ty::{self, subst::GenericArgKind}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Spanned; @@ -337,6 +337,15 @@ declare_clippy_lint! { "found clippy lint without `clippy::version` attribute" } +declare_clippy_lint! { + /// ### What it does + /// Check that the `extract_msrv_attr!` macro is used, when a lint has a MSRV. + /// + pub MISSING_MSRV_ATTR_IMPL, + internal, + "checking if all necessary steps were taken when adding a MSRV to a lint" +} + declare_lint_pass!(ClippyLintsInternal => [CLIPPY_LINTS_INTERNAL]); impl EarlyLintPass for ClippyLintsInternal { @@ -1314,3 +1323,46 @@ fn if_chain_local_span(cx: &LateContext<'_>, local: &Local<'_>, if_chain_span: S span.parent(), ) } + +declare_lint_pass!(MsrvAttrImpl => [MISSING_MSRV_ATTR_IMPL]); + +impl LateLintPass<'_> for MsrvAttrImpl { + fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { + if_chain! { + if let hir::ItemKind::Impl(hir::Impl { + of_trait: Some(lint_pass_trait_ref), + self_ty, + items, + .. + }) = &item.kind; + if let Some(lint_pass_trait_def_id) = lint_pass_trait_ref.trait_def_id(); + let is_late_pass = match_def_path(cx, lint_pass_trait_def_id, &paths::LATE_LINT_PASS); + if is_late_pass || match_def_path(cx, lint_pass_trait_def_id, &paths::EARLY_LINT_PASS); + let self_ty = hir_ty_to_ty(cx.tcx, self_ty); + if let ty::Adt(self_ty_def, _) = self_ty.kind(); + if self_ty_def.is_struct(); + if self_ty_def.all_fields().any(|f| { + cx.tcx + .type_of(f.did) + .walk() + .filter(|t| matches!(t.unpack(), GenericArgKind::Type(_))) + .any(|t| match_type(cx, t.expect_ty(), &paths::RUSTC_VERSION)) + }); + if !items.iter().any(|item| item.ident.name == sym!(enter_lint_attrs)); + then { + let context = if is_late_pass { "LateContext" } else { "EarlyContext" }; + let lint_pass = if is_late_pass { "LateLintPass" } else { "EarlyLintPass" }; + let span = cx.sess().source_map().span_through_char(item.span, '{'); + span_lint_and_sugg( + cx, + MISSING_MSRV_ATTR_IMPL, + span, + &format!("`extract_msrv_attr!` macro missing from `{lint_pass}` implementation"), + &format!("add `extract_msrv_attr!({context})` to the `{lint_pass}` implementation"), + format!("{}\n extract_msrv_attr!({context});", snippet(cx, span, "..")), + Applicability::MachineApplicable, + ); + } + } + } +} diff --git a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs index 56633490eaa..a617422bbeb 100644 --- a/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs +++ b/src/tools/clippy/clippy_lints/src/utils/internal_lints/metadata_collector.rs @@ -473,7 +473,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { /// ``` fn check_expr(&mut self, cx: &LateContext<'hir>, expr: &'hir hir::Expr<'_>) { if let Some(args) = match_lint_emission(cx, expr) { - let mut emission_info = extract_emission_info(cx, args); + let emission_info = extract_emission_info(cx, args); if emission_info.is_empty() { // See: // - src/misc.rs:734:9 @@ -483,7 +483,7 @@ impl<'hir> LateLintPass<'hir> for MetadataCollector { return; } - for (lint_name, applicability, is_multi_part) in emission_info.drain(..) { + for (lint_name, applicability, is_multi_part) in emission_info { let app_info = self.applicability_info.entry(lint_name).or_default(); app_info.applicability = applicability; app_info.is_multi_part_suggestion = is_multi_part; @@ -693,7 +693,7 @@ fn extract_emission_info<'hir>( } lints - .drain(..) + .into_iter() .map(|lint_name| (lint_name, applicability, multi_part)) .collect() } diff --git a/src/tools/clippy/clippy_lints/src/write.rs b/src/tools/clippy/clippy_lints/src/write.rs index a328ddda5ae..532bd810a2e 100644 --- a/src/tools/clippy/clippy_lints/src/write.rs +++ b/src/tools/clippy/clippy_lints/src/write.rs @@ -7,7 +7,7 @@ use clippy_utils::source::{snippet_opt, snippet_with_applicability}; use rustc_ast::ast::{Expr, ExprKind, Impl, Item, ItemKind, MacCall, Path, StrLit, StrStyle}; use rustc_ast::token::{self, LitKind}; use rustc_ast::tokenstream::TokenStream; -use rustc_errors::Applicability; +use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_lexer::unescape::{self, EscapeError}; use rustc_lint::{EarlyContext, EarlyLintPass, LintContext}; use rustc_parse::parser; @@ -534,7 +534,7 @@ impl Write { match parser .parse_expr() .map(rustc_ast::ptr::P::into_inner) - .map_err(|e| e.cancel()) + .map_err(DiagnosticBuilder::cancel) { // write!(e, ...) Ok(p) if parser.eat(&token::Comma) => Some(p), @@ -563,7 +563,7 @@ impl Write { } let comma_span = parser.prev_token.span; - let token_expr = if let Ok(expr) = parser.parse_expr().map_err(|err| err.cancel()) { + let token_expr = if let Ok(expr) = parser.parse_expr().map_err(DiagnosticBuilder::cancel) { expr } else { return (Some(fmtstr), None); diff --git a/src/tools/clippy/clippy_utils/src/ast_utils.rs b/src/tools/clippy/clippy_utils/src/ast_utils.rs index 3a47845ec82..3fce4987679 100644 --- a/src/tools/clippy/clippy_utils/src/ast_utils.rs +++ b/src/tools/clippy/clippy_utils/src/ast_utils.rs @@ -279,8 +279,22 @@ pub fn eq_item_kind(l: &ItemKind, r: &ItemKind) -> bool { (ForeignMod(l), ForeignMod(r)) => { both(&l.abi, &r.abi, eq_str_lit) && over(&l.items, &r.items, |l, r| eq_item(l, r, eq_foreign_item_kind)) }, - (TyAlias(box ast::TyAlias { defaultness: ld, generics: lg, bounds: lb, ty: lt, .. }), - TyAlias(box ast::TyAlias { defaultness: rd, generics: rg, bounds: rb, ty: rt, .. })) => { + ( + TyAlias(box ast::TyAlias { + defaultness: ld, + generics: lg, + bounds: lb, + ty: lt, + .. + }), + TyAlias(box ast::TyAlias { + defaultness: rd, + generics: rg, + bounds: rb, + ty: rt, + .. + }), + ) => { eq_defaultness(*ld, *rd) && eq_generics(lg, rg) && over(lb, rb, eq_generic_bound) @@ -370,8 +384,22 @@ pub fn eq_foreign_item_kind(l: &ForeignItemKind, r: &ForeignItemKind) -> bool { ) => { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) && eq_generics(lg, rg) && both(lb, rb, |l, r| eq_block(l, r)) }, - (TyAlias(box ast::TyAlias { defaultness: ld, generics: lg, bounds: lb, ty: lt, .. }), - TyAlias(box ast::TyAlias { defaultness: rd, generics: rg, bounds: rb, ty: rt, .. })) => { + ( + TyAlias(box ast::TyAlias { + defaultness: ld, + generics: lg, + bounds: lb, + ty: lt, + .. + }), + TyAlias(box ast::TyAlias { + defaultness: rd, + generics: rg, + bounds: rb, + ty: rt, + .. + }), + ) => { eq_defaultness(*ld, *rd) && eq_generics(lg, rg) && over(lb, rb, eq_generic_bound) @@ -402,8 +430,22 @@ pub fn eq_assoc_item_kind(l: &AssocItemKind, r: &AssocItemKind) -> bool { ) => { eq_defaultness(*ld, *rd) && eq_fn_sig(lf, rf) && eq_generics(lg, rg) && both(lb, rb, |l, r| eq_block(l, r)) }, - (TyAlias(box ast::TyAlias { defaultness: ld, generics: lg, bounds: lb, ty: lt, .. }), - TyAlias(box ast::TyAlias { defaultness: rd, generics: rg, bounds: rb, ty: rt, .. })) => { + ( + TyAlias(box ast::TyAlias { + defaultness: ld, + generics: lg, + bounds: lb, + ty: lt, + .. + }), + TyAlias(box ast::TyAlias { + defaultness: rd, + generics: rg, + bounds: rb, + ty: rt, + .. + }), + ) => { eq_defaultness(*ld, *rd) && eq_generics(lg, rg) && over(lb, rb, eq_generic_bound) diff --git a/src/tools/clippy/clippy_utils/src/consts.rs b/src/tools/clippy/clippy_utils/src/consts.rs index 42b9e692d3f..1d6f7acab13 100644 --- a/src/tools/clippy/clippy_utils/src/consts.rs +++ b/src/tools/clippy/clippy_utils/src/consts.rs @@ -593,7 +593,8 @@ pub fn miri_to_const(result: ty::Const<'_>) -> Option<Constant> { ty::ConstKind::Value(ConstValue::Slice { data, start, end }) => match result.ty().kind() { ty::Ref(_, tam, _) => match tam.kind() { ty::Str => String::from_utf8( - data.inner().inspect_with_uninit_and_ptr_outside_interpreter(start..end) + data.inner() + .inspect_with_uninit_and_ptr_outside_interpreter(start..end) .to_owned(), ) .ok() @@ -605,7 +606,8 @@ pub fn miri_to_const(result: ty::Const<'_>) -> Option<Constant> { ty::ConstKind::Value(ConstValue::ByRef { alloc, offset: _ }) => match result.ty().kind() { ty::Array(sub_type, len) => match sub_type.kind() { ty::Float(FloatTy::F32) => match miri_to_const(*len) { - Some(Constant::Int(len)) => alloc.inner() + Some(Constant::Int(len)) => alloc + .inner() .inspect_with_uninit_and_ptr_outside_interpreter(0..(4 * len as usize)) .to_owned() .chunks(4) @@ -619,7 +621,8 @@ pub fn miri_to_const(result: ty::Const<'_>) -> Option<Constant> { _ => None, }, ty::Float(FloatTy::F64) => match miri_to_const(*len) { - Some(Constant::Int(len)) => alloc.inner() + Some(Constant::Int(len)) => alloc + .inner() .inspect_with_uninit_and_ptr_outside_interpreter(0..(8 * len as usize)) .to_owned() .chunks(8) diff --git a/src/tools/clippy/clippy_utils/src/hir_utils.rs b/src/tools/clippy/clippy_utils/src/hir_utils.rs index 9654895060f..00594f4d42a 100644 --- a/src/tools/clippy/clippy_utils/src/hir_utils.rs +++ b/src/tools/clippy/clippy_utils/src/hir_utils.rs @@ -74,6 +74,10 @@ impl<'a, 'tcx> SpanlessEq<'a, 'tcx> { self.inter_expr().eq_expr(left, right) } + pub fn eq_path(&mut self, left: &Path<'_>, right: &Path<'_>) -> bool { + self.inter_expr().eq_path(left, right) + } + pub fn eq_path_segment(&mut self, left: &PathSegment<'_>, right: &PathSegment<'_>) -> bool { self.inter_expr().eq_path_segment(left, right) } @@ -362,7 +366,7 @@ impl HirEqInterExpr<'_, '_, '_> { } } - fn eq_path(&mut self, left: &Path<'_>, right: &Path<'_>) -> bool { + pub fn eq_path(&mut self, left: &Path<'_>, right: &Path<'_>) -> bool { match (left.res, right.res) { (Res::Local(l), Res::Local(r)) => l == r || self.locals.get(&l) == Some(&r), (Res::Local(_), _) | (_, Res::Local(_)) => false, diff --git a/src/tools/clippy/clippy_utils/src/msrvs.rs b/src/tools/clippy/clippy_utils/src/msrvs.rs index a5b409ad96b..fce93153d96 100644 --- a/src/tools/clippy/clippy_utils/src/msrvs.rs +++ b/src/tools/clippy/clippy_utils/src/msrvs.rs @@ -20,7 +20,7 @@ msrv_aliases! { 1,46,0 { CONST_IF_MATCH } 1,45,0 { STR_STRIP_PREFIX } 1,43,0 { LOG2_10, LOG10_2 } - 1,42,0 { MATCHES_MACRO, SLICE_PATTERNS } + 1,42,0 { MATCHES_MACRO, SLICE_PATTERNS, PTR_SLICE_RAW_PARTS } 1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE } 1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF } 1,38,0 { POINTER_CAST } diff --git a/src/tools/clippy/clippy_utils/src/paths.rs b/src/tools/clippy/clippy_utils/src/paths.rs index 2778e30388e..6f56f8d5136 100644 --- a/src/tools/clippy/clippy_utils/src/paths.rs +++ b/src/tools/clippy/clippy_utils/src/paths.rs @@ -32,6 +32,8 @@ pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"]; pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; #[cfg(feature = "internal")] pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"]; +#[cfg(feature = "internal")] +pub const EARLY_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "EarlyLintPass"]; pub const EXIT: [&str; 3] = ["std", "process", "exit"]; pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"]; pub const F64_EPSILON: [&str; 4] = ["core", "f64", "<impl f64>", "EPSILON"]; @@ -67,6 +69,8 @@ pub const KW_MODULE: [&str; 3] = ["rustc_span", "symbol", "kw"]; #[cfg(feature = "internal")] pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"]; #[cfg(feature = "internal")] +pub const LATE_LINT_PASS: [&str; 3] = ["rustc_lint", "passes", "LateLintPass"]; +#[cfg(feature = "internal")] pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"]; pub const MUTEX_GUARD: [&str; 4] = ["std", "sync", "mutex", "MutexGuard"]; pub const OPEN_OPTIONS: [&str; 3] = ["std", "fs", "OpenOptions"]; @@ -126,6 +130,8 @@ pub const REGEX_SET_NEW: [&str; 5] = ["regex", "re_set", "unicode", "RegexSet", pub const RESULT: [&str; 3] = ["core", "result", "Result"]; pub const RESULT_ERR: [&str; 4] = ["core", "result", "Result", "Err"]; pub const RESULT_OK: [&str; 4] = ["core", "result", "Result", "Ok"]; +#[cfg(feature = "internal")] +pub const RUSTC_VERSION: [&str; 2] = ["rustc_semver", "RustcVersion"]; pub const RWLOCK_READ_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockReadGuard"]; pub const RWLOCK_WRITE_GUARD: [&str; 4] = ["std", "sync", "rwlock", "RwLockWriteGuard"]; pub const SERDE_DESERIALIZE: [&str; 3] = ["serde", "de", "Deserialize"]; diff --git a/src/tools/clippy/clippy_utils/src/ty.rs b/src/tools/clippy/clippy_utils/src/ty.rs index 3645a9a5228..e3fc76f4e1a 100644 --- a/src/tools/clippy/clippy_utils/src/ty.rs +++ b/src/tools/clippy/clippy_utils/src/ty.rs @@ -294,7 +294,11 @@ pub fn is_type_diagnostic_item(cx: &LateContext<'_>, ty: Ty<'_>, diag_item: Symb /// Returns `false` if the `LangItem` is not defined. pub fn is_type_lang_item(cx: &LateContext<'_>, ty: Ty<'_>, lang_item: hir::LangItem) -> bool { match ty.kind() { - ty::Adt(adt, _) => cx.tcx.lang_items().require(lang_item).map_or(false, |li| li == adt.did()), + ty::Adt(adt, _) => cx + .tcx + .lang_items() + .require(lang_item) + .map_or(false, |li| li == adt.did()), _ => false, } } diff --git a/src/tools/clippy/rust-toolchain b/src/tools/clippy/rust-toolchain index 4d2c5761991..9d5da4ed68f 100644 --- a/src/tools/clippy/rust-toolchain +++ b/src/tools/clippy/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2022-02-24" +channel = "nightly-2022-03-14" components = ["cargo", "llvm-tools-preview", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/src/tools/clippy/rustfmt.toml b/src/tools/clippy/rustfmt.toml index 4b415a31b27..10d39762043 100644 --- a/src/tools/clippy/rustfmt.toml +++ b/src/tools/clippy/rustfmt.toml @@ -2,6 +2,6 @@ max_width = 120 comment_width = 100 match_block_trailing_comma = true wrap_comments = true -edition = "2018" +edition = "2021" error_on_line_overflow = true version = "Two" diff --git a/src/tools/clippy/tests/compile-test.rs b/src/tools/clippy/tests/compile-test.rs index 6bc74bc1e9a..c9710e3db8e 100644 --- a/src/tools/clippy/tests/compile-test.rs +++ b/src/tools/clippy/tests/compile-test.rs @@ -34,6 +34,7 @@ static TEST_DEPENDENCIES: &[&str] = &[ "syn", "tokio", "parking_lot", + "rustc_semver", ]; // Test dependencies may need an `extern crate` here to ensure that they show up @@ -53,6 +54,8 @@ extern crate parking_lot; #[allow(unused_extern_crates)] extern crate quote; #[allow(unused_extern_crates)] +extern crate rustc_semver; +#[allow(unused_extern_crates)] extern crate syn; #[allow(unused_extern_crates)] extern crate tokio; @@ -165,7 +168,11 @@ fn run_ui() { let _threads = VarGuard::set( "RUST_TEST_THREADS", // if RUST_TEST_THREADS is set, adhere to it, otherwise override it - env::var("RUST_TEST_THREADS").unwrap_or_else(|_| num_cpus::get().to_string()), + env::var("RUST_TEST_THREADS").unwrap_or_else(|_| { + std::thread::available_parallelism() + .map_or(1, std::num::NonZeroUsize::get) + .to_string() + }), ); compiletest::run_tests(&config); } diff --git a/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.fixed b/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.fixed new file mode 100644 index 00000000000..900a8fffd40 --- /dev/null +++ b/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.fixed @@ -0,0 +1,40 @@ +// run-rustfix + +#![deny(clippy::internal)] +#![allow(clippy::missing_clippy_version_attribute)] +#![feature(rustc_private)] + +extern crate rustc_ast; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +#[macro_use] +extern crate rustc_session; +use clippy_utils::extract_msrv_attr; +use rustc_hir::Expr; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; +use rustc_semver::RustcVersion; + +declare_lint! { + pub TEST_LINT, + Warn, + "" +} + +struct Pass { + msrv: Option<RustcVersion>, +} + +impl_lint_pass!(Pass => [TEST_LINT]); + +impl LateLintPass<'_> for Pass { + extract_msrv_attr!(LateContext); + fn check_expr(&mut self, _: &LateContext<'_>, _: &Expr<'_>) {} +} + +impl EarlyLintPass for Pass { + extract_msrv_attr!(EarlyContext); + fn check_expr(&mut self, _: &EarlyContext<'_>, _: &rustc_ast::Expr) {} +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.rs b/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.rs new file mode 100644 index 00000000000..4bc8164db67 --- /dev/null +++ b/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.rs @@ -0,0 +1,38 @@ +// run-rustfix + +#![deny(clippy::internal)] +#![allow(clippy::missing_clippy_version_attribute)] +#![feature(rustc_private)] + +extern crate rustc_ast; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +#[macro_use] +extern crate rustc_session; +use clippy_utils::extract_msrv_attr; +use rustc_hir::Expr; +use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass}; +use rustc_semver::RustcVersion; + +declare_lint! { + pub TEST_LINT, + Warn, + "" +} + +struct Pass { + msrv: Option<RustcVersion>, +} + +impl_lint_pass!(Pass => [TEST_LINT]); + +impl LateLintPass<'_> for Pass { + fn check_expr(&mut self, _: &LateContext<'_>, _: &Expr<'_>) {} +} + +impl EarlyLintPass for Pass { + fn check_expr(&mut self, _: &EarlyContext<'_>, _: &rustc_ast::Expr) {} +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.stderr b/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.stderr new file mode 100644 index 00000000000..ddc06f0be1b --- /dev/null +++ b/src/tools/clippy/tests/ui-internal/invalid_msrv_attr_impl.stderr @@ -0,0 +1,32 @@ +error: `extract_msrv_attr!` macro missing from `LateLintPass` implementation + --> $DIR/invalid_msrv_attr_impl.rs:30:1 + | +LL | impl LateLintPass<'_> for Pass { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/invalid_msrv_attr_impl.rs:3:9 + | +LL | #![deny(clippy::internal)] + | ^^^^^^^^^^^^^^^^ + = note: `#[deny(clippy::missing_msrv_attr_impl)]` implied by `#[deny(clippy::internal)]` +help: add `extract_msrv_attr!(LateContext)` to the `LateLintPass` implementation + | +LL + impl LateLintPass<'_> for Pass { +LL + extract_msrv_attr!(LateContext); + | + +error: `extract_msrv_attr!` macro missing from `EarlyLintPass` implementation + --> $DIR/invalid_msrv_attr_impl.rs:34:1 + | +LL | impl EarlyLintPass for Pass { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: add `extract_msrv_attr!(EarlyContext)` to the `EarlyLintPass` implementation + | +LL + impl EarlyLintPass for Pass { +LL + extract_msrv_attr!(EarlyContext); + | + +error: aborting due to 2 previous errors + diff --git a/src/tools/clippy/tests/ui/allow_attributes_without_reason.rs b/src/tools/clippy/tests/ui/allow_attributes_without_reason.rs new file mode 100644 index 00000000000..1a0d4e88657 --- /dev/null +++ b/src/tools/clippy/tests/ui/allow_attributes_without_reason.rs @@ -0,0 +1,14 @@ +#![feature(lint_reasons)] +#![deny(clippy::allow_attributes_without_reason)] + +// These should trigger the lint +#[allow(dead_code)] +#[allow(dead_code, deprecated)] +// These should be fine +#[allow(dead_code, reason = "This should be allowed")] +#[warn(dyn_drop, reason = "Warnings can also have reasons")] +#[warn(deref_nullptr)] +#[deny(deref_nullptr)] +#[forbid(deref_nullptr)] + +fn main() {} diff --git a/src/tools/clippy/tests/ui/allow_attributes_without_reason.stderr b/src/tools/clippy/tests/ui/allow_attributes_without_reason.stderr new file mode 100644 index 00000000000..cd040a144aa --- /dev/null +++ b/src/tools/clippy/tests/ui/allow_attributes_without_reason.stderr @@ -0,0 +1,23 @@ +error: `allow` attribute without specifying a reason + --> $DIR/allow_attributes_without_reason.rs:5:1 + | +LL | #[allow(dead_code)] + | ^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/allow_attributes_without_reason.rs:2:9 + | +LL | #![deny(clippy::allow_attributes_without_reason)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: try adding a reason at the end with `, reason = ".."` + +error: `allow` attribute without specifying a reason + --> $DIR/allow_attributes_without_reason.rs:6:1 + | +LL | #[allow(dead_code, deprecated)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try adding a reason at the end with `, reason = ".."` + +error: aborting due to 2 previous errors + diff --git a/src/tools/clippy/tests/ui/cast_slice_different_sizes.rs b/src/tools/clippy/tests/ui/cast_slice_different_sizes.rs new file mode 100644 index 00000000000..cfe1cca2eba --- /dev/null +++ b/src/tools/clippy/tests/ui/cast_slice_different_sizes.rs @@ -0,0 +1,39 @@ +fn main() { + let x: [i32; 3] = [1_i32, 2, 3]; + let r_x = &x; + // Check casting through multiple bindings + // Because it's separate, it does not check the cast back to something of the same size + let a = r_x as *const [i32]; + let b = a as *const [u8]; + let c = b as *const [u32]; + + // loses data + let loss = r_x as *const [i32] as *const [u8]; + + // Cast back to same size but different type loses no data, just type conversion + // This is weird code but there's no reason for this lint specifically to fire *twice* on it + let restore = r_x as *const [i32] as *const [u8] as *const [u32]; + + // Check casting through blocks is detected + let loss_block_1 = { r_x as *const [i32] } as *const [u8]; + let loss_block_2 = { + let _ = (); + r_x as *const [i32] + } as *const [u8]; + + // Check that resores of the same size are detected through blocks + let restore_block_1 = { r_x as *const [i32] } as *const [u8] as *const [u32]; + let restore_block_2 = { ({ r_x as *const [i32] }) as *const [u8] } as *const [u32]; + let restore_block_3 = { + let _ = (); + ({ + let _ = (); + r_x as *const [i32] + }) as *const [u8] + } as *const [u32]; + + // Check that the result of a long chain of casts is detected + let long_chain_loss = r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const [u8]; + let long_chain_restore = + r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const [u8] as *const [u32]; +} diff --git a/src/tools/clippy/tests/ui/cast_slice_different_sizes.stderr b/src/tools/clippy/tests/ui/cast_slice_different_sizes.stderr new file mode 100644 index 00000000000..a37cec7cb3b --- /dev/null +++ b/src/tools/clippy/tests/ui/cast_slice_different_sizes.stderr @@ -0,0 +1,52 @@ +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:7:13 + | +LL | let b = a as *const [u8]; + | ^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(a as *const u8, ..)` + | + = note: `#[deny(clippy::cast_slice_different_sizes)]` on by default + +error: casting between raw pointers to `[u8]` (element size 1) and `[u32]` (element size 4) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:8:13 + | +LL | let c = b as *const [u32]; + | ^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(b as *const u32, ..)` + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:11:16 + | +LL | let loss = r_x as *const [i32] as *const [u8]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(r_x as *const [i32] as *const u8, ..)` + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:18:24 + | +LL | let loss_block_1 = { r_x as *const [i32] } as *const [u8]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts({ r_x as *const [i32] } as *const u8, ..)` + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:19:24 + | +LL | let loss_block_2 = { + | ________________________^ +LL | | let _ = (); +LL | | r_x as *const [i32] +LL | | } as *const [u8]; + | |____________________^ + | +help: replace with `ptr::slice_from_raw_parts` + | +LL ~ let loss_block_2 = core::ptr::slice_from_raw_parts({ +LL + let _ = (); +LL + r_x as *const [i32] +LL ~ } as *const u8, ..); + | + +error: casting between raw pointers to `[i32]` (element size 4) and `[u8]` (element size 1) does not adjust the count + --> $DIR/cast_slice_different_sizes.rs:36:27 + | +LL | let long_chain_loss = r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const [u8]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `ptr::slice_from_raw_parts`: `core::ptr::slice_from_raw_parts(r_x as *const [i32] as *const [u32] as *const [u16] as *const [i8] as *const u8, ..)` + +error: aborting due to 6 previous errors + diff --git a/src/tools/clippy/tests/ui/crate_level_checks/no_std_main_recursion.rs b/src/tools/clippy/tests/ui/crate_level_checks/no_std_main_recursion.rs index be4348e2bb7..4a5c597dda5 100644 --- a/src/tools/clippy/tests/ui/crate_level_checks/no_std_main_recursion.rs +++ b/src/tools/clippy/tests/ui/crate_level_checks/no_std_main_recursion.rs @@ -12,12 +12,12 @@ static N: AtomicUsize = AtomicUsize::new(0); #[warn(clippy::main_recursion)] #[start] -fn main(argc: isize, argv: *const *const u8) -> isize { +fn main(_argc: isize, _argv: *const *const u8) -> isize { let x = N.load(Ordering::Relaxed); N.store(x + 1, Ordering::Relaxed); if x < 3 { - main(argc, argv); + main(_argc, _argv); } 0 diff --git a/src/tools/clippy/tests/ui/dbg_macro.rs b/src/tools/clippy/tests/ui/dbg_macro.rs index 9b03c9b4783..baf01174b67 100644 --- a/src/tools/clippy/tests/ui/dbg_macro.rs +++ b/src/tools/clippy/tests/ui/dbg_macro.rs @@ -1,3 +1,4 @@ +// compile-flags: --test #![warn(clippy::dbg_macro)] fn foo(n: u32) -> u32 { @@ -40,3 +41,8 @@ mod issue7274 { dbg!(2); }); } + +#[test] +pub fn issue8481() { + dbg!(1); +} diff --git a/src/tools/clippy/tests/ui/dbg_macro.stderr b/src/tools/clippy/tests/ui/dbg_macro.stderr index 8ee1b328720..a3e7a7162e5 100644 --- a/src/tools/clippy/tests/ui/dbg_macro.stderr +++ b/src/tools/clippy/tests/ui/dbg_macro.stderr @@ -1,5 +1,5 @@ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:4:22 + --> $DIR/dbg_macro.rs:5:22 | LL | if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n } | ^^^^^^^^^^^^^^^^^^^^^^ @@ -11,7 +11,7 @@ LL | if let Some(n) = n.checked_sub(4) { n } else { n } | ~~~~~~~~~~~~~~~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:8:8 + --> $DIR/dbg_macro.rs:9:8 | LL | if dbg!(n <= 1) { | ^^^^^^^^^^^^ @@ -22,7 +22,7 @@ LL | if n <= 1 { | ~~~~~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:9:9 + --> $DIR/dbg_macro.rs:10:9 | LL | dbg!(1) | ^^^^^^^ @@ -33,7 +33,7 @@ LL | 1 | error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:11:9 + --> $DIR/dbg_macro.rs:12:9 | LL | dbg!(n * factorial(n - 1)) | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -44,7 +44,7 @@ LL | n * factorial(n - 1) | error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:16:5 + --> $DIR/dbg_macro.rs:17:5 | LL | dbg!(42); | ^^^^^^^^ @@ -55,7 +55,7 @@ LL | 42; | ~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:17:5 + --> $DIR/dbg_macro.rs:18:5 | LL | dbg!(dbg!(dbg!(42))); | ^^^^^^^^^^^^^^^^^^^^ @@ -66,7 +66,7 @@ LL | dbg!(dbg!(42)); | ~~~~~~~~~~~~~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:18:14 + --> $DIR/dbg_macro.rs:19:14 | LL | foo(3) + dbg!(factorial(4)); | ^^^^^^^^^^^^^^^^^^ @@ -77,7 +77,7 @@ LL | foo(3) + factorial(4); | ~~~~~~~~~~~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:19:5 + --> $DIR/dbg_macro.rs:20:5 | LL | dbg!(1, 2, dbg!(3, 4)); | ^^^^^^^^^^^^^^^^^^^^^^ @@ -88,7 +88,7 @@ LL | (1, 2, dbg!(3, 4)); | ~~~~~~~~~~~~~~~~~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:20:5 + --> $DIR/dbg_macro.rs:21:5 | LL | dbg!(1, 2, 3, 4, 5); | ^^^^^^^^^^^^^^^^^^^ @@ -99,7 +99,7 @@ LL | (1, 2, 3, 4, 5); | ~~~~~~~~~~~~~~~ error: `dbg!` macro is intended as a debugging tool - --> $DIR/dbg_macro.rs:40:9 + --> $DIR/dbg_macro.rs:41:9 | LL | dbg!(2); | ^^^^^^^ diff --git a/src/tools/clippy/tests/ui/extend_with_drain.fixed b/src/tools/clippy/tests/ui/extend_with_drain.fixed index e863870e7d6..71ebad24c16 100644 --- a/src/tools/clippy/tests/ui/extend_with_drain.fixed +++ b/src/tools/clippy/tests/ui/extend_with_drain.fixed @@ -1,11 +1,11 @@ // run-rustfix #![warn(clippy::extend_with_drain)] +#![allow(clippy::iter_with_drain)] use std::collections::BinaryHeap; fn main() { //gets linted let mut vec1 = vec![0u8; 1024]; let mut vec2: std::vec::Vec<u8> = Vec::new(); - vec2.append(&mut vec1); let mut vec3 = vec![0u8; 1024]; @@ -17,7 +17,7 @@ fn main() { vec11.append(&mut return_vector()); - //won't get linted it dosen't move the entire content of a vec into another + //won't get linted it doesn't move the entire content of a vec into another let mut test1 = vec![0u8, 10]; let mut test2: std::vec::Vec<u8> = Vec::new(); diff --git a/src/tools/clippy/tests/ui/extend_with_drain.rs b/src/tools/clippy/tests/ui/extend_with_drain.rs index dcb36b5951c..e9f011abb0e 100644 --- a/src/tools/clippy/tests/ui/extend_with_drain.rs +++ b/src/tools/clippy/tests/ui/extend_with_drain.rs @@ -1,11 +1,11 @@ // run-rustfix #![warn(clippy::extend_with_drain)] +#![allow(clippy::iter_with_drain)] use std::collections::BinaryHeap; fn main() { //gets linted let mut vec1 = vec![0u8; 1024]; let mut vec2: std::vec::Vec<u8> = Vec::new(); - vec2.extend(vec1.drain(..)); let mut vec3 = vec![0u8; 1024]; @@ -17,7 +17,7 @@ fn main() { vec11.extend(return_vector().drain(..)); - //won't get linted it dosen't move the entire content of a vec into another + //won't get linted it doesn't move the entire content of a vec into another let mut test1 = vec![0u8, 10]; let mut test2: std::vec::Vec<u8> = Vec::new(); diff --git a/src/tools/clippy/tests/ui/iter_with_drain.fixed b/src/tools/clippy/tests/ui/iter_with_drain.fixed new file mode 100644 index 00000000000..aea4dba9dd5 --- /dev/null +++ b/src/tools/clippy/tests/ui/iter_with_drain.fixed @@ -0,0 +1,56 @@ +// run-rustfix +// will emits unused mut warnings after fixing +#![allow(unused_mut)] +// will emits needless collect warnings after fixing +#![allow(clippy::needless_collect)] +#![warn(clippy::iter_with_drain)] +use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque}; + +fn full() { + let mut a = vec!["aaa".to_string(), "bbb".to_string()]; + let mut a: BinaryHeap<_> = a.into_iter().collect(); + let mut a: HashSet<_> = a.drain().collect(); + let mut a: VecDeque<_> = a.drain().collect(); + let mut a: Vec<_> = a.into_iter().collect(); + let mut a: HashMap<_, _> = a.into_iter().map(|x| (x.clone(), x)).collect(); + let _: Vec<(String, String)> = a.drain().collect(); +} + +fn closed() { + let mut a = vec!["aaa".to_string(), "bbb".to_string()]; + let mut a: BinaryHeap<_> = a.into_iter().collect(); + let mut a: HashSet<_> = a.drain().collect(); + let mut a: VecDeque<_> = a.drain().collect(); + let mut a: Vec<_> = a.into_iter().collect(); + let mut a: HashMap<_, _> = a.into_iter().map(|x| (x.clone(), x)).collect(); + let _: Vec<(String, String)> = a.drain().collect(); +} + +fn should_not_help() { + let mut a = vec!["aaa".to_string(), "bbb".to_string()]; + let mut a: BinaryHeap<_> = a.drain(1..).collect(); + let mut a: HashSet<_> = a.drain().collect(); + let mut a: VecDeque<_> = a.drain().collect(); + let mut a: Vec<_> = a.drain(..a.len() - 1).collect(); + let mut a: HashMap<_, _> = a.drain(1..a.len() - 1).map(|x| (x.clone(), x)).collect(); + let _: Vec<(String, String)> = a.drain().collect(); + + let mut b = vec!["aaa".to_string(), "bbb".to_string()]; + let _: Vec<_> = b.drain(0..a.len()).collect(); +} + +#[derive(Default)] +struct Bomb { + fire: Vec<u8>, +} + +fn should_not_help_0(bomb: &mut Bomb) { + let _: Vec<u8> = bomb.fire.drain(..).collect(); +} + +fn main() { + full(); + closed(); + should_not_help(); + should_not_help_0(&mut Bomb::default()); +} diff --git a/src/tools/clippy/tests/ui/iter_with_drain.rs b/src/tools/clippy/tests/ui/iter_with_drain.rs new file mode 100644 index 00000000000..271878cffb4 --- /dev/null +++ b/src/tools/clippy/tests/ui/iter_with_drain.rs @@ -0,0 +1,56 @@ +// run-rustfix +// will emits unused mut warnings after fixing +#![allow(unused_mut)] +// will emits needless collect warnings after fixing +#![allow(clippy::needless_collect)] +#![warn(clippy::iter_with_drain)] +use std::collections::{BinaryHeap, HashMap, HashSet, VecDeque}; + +fn full() { + let mut a = vec!["aaa".to_string(), "bbb".to_string()]; + let mut a: BinaryHeap<_> = a.drain(..).collect(); + let mut a: HashSet<_> = a.drain().collect(); + let mut a: VecDeque<_> = a.drain().collect(); + let mut a: Vec<_> = a.drain(..).collect(); + let mut a: HashMap<_, _> = a.drain(..).map(|x| (x.clone(), x)).collect(); + let _: Vec<(String, String)> = a.drain().collect(); +} + +fn closed() { + let mut a = vec!["aaa".to_string(), "bbb".to_string()]; + let mut a: BinaryHeap<_> = a.drain(0..).collect(); + let mut a: HashSet<_> = a.drain().collect(); + let mut a: VecDeque<_> = a.drain().collect(); + let mut a: Vec<_> = a.drain(..a.len()).collect(); + let mut a: HashMap<_, _> = a.drain(0..a.len()).map(|x| (x.clone(), x)).collect(); + let _: Vec<(String, String)> = a.drain().collect(); +} + +fn should_not_help() { + let mut a = vec!["aaa".to_string(), "bbb".to_string()]; + let mut a: BinaryHeap<_> = a.drain(1..).collect(); + let mut a: HashSet<_> = a.drain().collect(); + let mut a: VecDeque<_> = a.drain().collect(); + let mut a: Vec<_> = a.drain(..a.len() - 1).collect(); + let mut a: HashMap<_, _> = a.drain(1..a.len() - 1).map(|x| (x.clone(), x)).collect(); + let _: Vec<(String, String)> = a.drain().collect(); + + let mut b = vec!["aaa".to_string(), "bbb".to_string()]; + let _: Vec<_> = b.drain(0..a.len()).collect(); +} + +#[derive(Default)] +struct Bomb { + fire: Vec<u8>, +} + +fn should_not_help_0(bomb: &mut Bomb) { + let _: Vec<u8> = bomb.fire.drain(..).collect(); +} + +fn main() { + full(); + closed(); + should_not_help(); + should_not_help_0(&mut Bomb::default()); +} diff --git a/src/tools/clippy/tests/ui/iter_with_drain.stderr b/src/tools/clippy/tests/ui/iter_with_drain.stderr new file mode 100644 index 00000000000..aa394439fa6 --- /dev/null +++ b/src/tools/clippy/tests/ui/iter_with_drain.stderr @@ -0,0 +1,40 @@ +error: `drain(..)` used on a `Vec` + --> $DIR/iter_with_drain.rs:11:34 + | +LL | let mut a: BinaryHeap<_> = a.drain(..).collect(); + | ^^^^^^^^^ help: try this: `into_iter()` + | + = note: `-D clippy::iter-with-drain` implied by `-D warnings` + +error: `drain(..)` used on a `VecDeque` + --> $DIR/iter_with_drain.rs:14:27 + | +LL | let mut a: Vec<_> = a.drain(..).collect(); + | ^^^^^^^^^ help: try this: `into_iter()` + +error: `drain(..)` used on a `Vec` + --> $DIR/iter_with_drain.rs:15:34 + | +LL | let mut a: HashMap<_, _> = a.drain(..).map(|x| (x.clone(), x)).collect(); + | ^^^^^^^^^ help: try this: `into_iter()` + +error: `drain(..)` used on a `Vec` + --> $DIR/iter_with_drain.rs:21:34 + | +LL | let mut a: BinaryHeap<_> = a.drain(0..).collect(); + | ^^^^^^^^^^ help: try this: `into_iter()` + +error: `drain(..)` used on a `VecDeque` + --> $DIR/iter_with_drain.rs:24:27 + | +LL | let mut a: Vec<_> = a.drain(..a.len()).collect(); + | ^^^^^^^^^^^^^^^^ help: try this: `into_iter()` + +error: `drain(..)` used on a `Vec` + --> $DIR/iter_with_drain.rs:25:34 + | +LL | let mut a: HashMap<_, _> = a.drain(0..a.len()).map(|x| (x.clone(), x)).collect(); + | ^^^^^^^^^^^^^^^^^ help: try this: `into_iter()` + +error: aborting due to 6 previous errors + diff --git a/src/tools/clippy/tests/ui/manual_map_option.fixed b/src/tools/clippy/tests/ui/manual_map_option.fixed index 294d79abc04..fc6a7abca0e 100644 --- a/src/tools/clippy/tests/ui/manual_map_option.fixed +++ b/src/tools/clippy/tests/ui/manual_map_option.fixed @@ -148,6 +148,7 @@ fn main() { // #7077 let s = &String::new(); + #[allow(clippy::needless_match)] let _: Option<&str> = match Some(s) { Some(s) => Some(s), None => None, diff --git a/src/tools/clippy/tests/ui/manual_map_option.rs b/src/tools/clippy/tests/ui/manual_map_option.rs index d11bf5ecb82..16508270f64 100644 --- a/src/tools/clippy/tests/ui/manual_map_option.rs +++ b/src/tools/clippy/tests/ui/manual_map_option.rs @@ -214,6 +214,7 @@ fn main() { // #7077 let s = &String::new(); + #[allow(clippy::needless_match)] let _: Option<&str> = match Some(s) { Some(s) => Some(s), None => None, diff --git a/src/tools/clippy/tests/ui/missing_spin_loop.fixed b/src/tools/clippy/tests/ui/missing_spin_loop.fixed new file mode 100644 index 00000000000..aa89e04d26e --- /dev/null +++ b/src/tools/clippy/tests/ui/missing_spin_loop.fixed @@ -0,0 +1,28 @@ +// run-rustfix +#![warn(clippy::missing_spin_loop)] +#![allow(clippy::bool_comparison)] +#![allow(unused_braces)] + +use core::sync::atomic::{AtomicBool, Ordering}; + +fn main() { + let b = AtomicBool::new(true); + // Those should lint + while b.load(Ordering::Acquire) { std::hint::spin_loop() } + + while !b.load(Ordering::SeqCst) { std::hint::spin_loop() } + + while b.load(Ordering::Acquire) == false { std::hint::spin_loop() } + + while { true == b.load(Ordering::Acquire) } { std::hint::spin_loop() } + + while b.compare_exchange(true, false, Ordering::Acquire, Ordering::Relaxed) != Ok(true) { std::hint::spin_loop() } + + while Ok(false) != b.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) { std::hint::spin_loop() } + + // This is OK, as the body is not empty + while b.load(Ordering::Acquire) { + std::hint::spin_loop() + } + // TODO: also match on loop+match or while let +} diff --git a/src/tools/clippy/tests/ui/missing_spin_loop.rs b/src/tools/clippy/tests/ui/missing_spin_loop.rs new file mode 100644 index 00000000000..88745e47732 --- /dev/null +++ b/src/tools/clippy/tests/ui/missing_spin_loop.rs @@ -0,0 +1,28 @@ +// run-rustfix +#![warn(clippy::missing_spin_loop)] +#![allow(clippy::bool_comparison)] +#![allow(unused_braces)] + +use core::sync::atomic::{AtomicBool, Ordering}; + +fn main() { + let b = AtomicBool::new(true); + // Those should lint + while b.load(Ordering::Acquire) {} + + while !b.load(Ordering::SeqCst) {} + + while b.load(Ordering::Acquire) == false {} + + while { true == b.load(Ordering::Acquire) } {} + + while b.compare_exchange(true, false, Ordering::Acquire, Ordering::Relaxed) != Ok(true) {} + + while Ok(false) != b.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) {} + + // This is OK, as the body is not empty + while b.load(Ordering::Acquire) { + std::hint::spin_loop() + } + // TODO: also match on loop+match or while let +} diff --git a/src/tools/clippy/tests/ui/missing_spin_loop.stderr b/src/tools/clippy/tests/ui/missing_spin_loop.stderr new file mode 100644 index 00000000000..485da00dc64 --- /dev/null +++ b/src/tools/clippy/tests/ui/missing_spin_loop.stderr @@ -0,0 +1,40 @@ +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop.rs:11:37 + | +LL | while b.load(Ordering::Acquire) {} + | ^^ help: try this: `{ std::hint::spin_loop() }` + | + = note: `-D clippy::missing-spin-loop` implied by `-D warnings` + +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop.rs:13:37 + | +LL | while !b.load(Ordering::SeqCst) {} + | ^^ help: try this: `{ std::hint::spin_loop() }` + +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop.rs:15:46 + | +LL | while b.load(Ordering::Acquire) == false {} + | ^^ help: try this: `{ std::hint::spin_loop() }` + +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop.rs:17:49 + | +LL | while { true == b.load(Ordering::Acquire) } {} + | ^^ help: try this: `{ std::hint::spin_loop() }` + +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop.rs:19:93 + | +LL | while b.compare_exchange(true, false, Ordering::Acquire, Ordering::Relaxed) != Ok(true) {} + | ^^ help: try this: `{ std::hint::spin_loop() }` + +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop.rs:21:94 + | +LL | while Ok(false) != b.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed) {} + | ^^ help: try this: `{ std::hint::spin_loop() }` + +error: aborting due to 6 previous errors + diff --git a/src/tools/clippy/tests/ui/missing_spin_loop_no_std.fixed b/src/tools/clippy/tests/ui/missing_spin_loop_no_std.fixed new file mode 100644 index 00000000000..bb4b4795516 --- /dev/null +++ b/src/tools/clippy/tests/ui/missing_spin_loop_no_std.fixed @@ -0,0 +1,23 @@ +// run-rustfix +#![warn(clippy::missing_spin_loop)] +#![feature(lang_items, start, libc)] +#![no_std] + +use core::sync::atomic::{AtomicBool, Ordering}; + +#[start] +fn main(_argc: isize, _argv: *const *const u8) -> isize { + // This should trigger the lint + let b = AtomicBool::new(true); + // This should lint with `core::hint::spin_loop()` + while b.load(Ordering::Acquire) { core::hint::spin_loop() } + 0 +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} + +#[lang = "eh_personality"] +extern "C" fn eh_personality() {} diff --git a/src/tools/clippy/tests/ui/missing_spin_loop_no_std.rs b/src/tools/clippy/tests/ui/missing_spin_loop_no_std.rs new file mode 100644 index 00000000000..a19bc72baf8 --- /dev/null +++ b/src/tools/clippy/tests/ui/missing_spin_loop_no_std.rs @@ -0,0 +1,23 @@ +// run-rustfix +#![warn(clippy::missing_spin_loop)] +#![feature(lang_items, start, libc)] +#![no_std] + +use core::sync::atomic::{AtomicBool, Ordering}; + +#[start] +fn main(_argc: isize, _argv: *const *const u8) -> isize { + // This should trigger the lint + let b = AtomicBool::new(true); + // This should lint with `core::hint::spin_loop()` + while b.load(Ordering::Acquire) {} + 0 +} + +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} + +#[lang = "eh_personality"] +extern "C" fn eh_personality() {} diff --git a/src/tools/clippy/tests/ui/missing_spin_loop_no_std.stderr b/src/tools/clippy/tests/ui/missing_spin_loop_no_std.stderr new file mode 100644 index 00000000000..2b3b6873c3c --- /dev/null +++ b/src/tools/clippy/tests/ui/missing_spin_loop_no_std.stderr @@ -0,0 +1,10 @@ +error: busy-waiting loop should at least have a spin loop hint + --> $DIR/missing_spin_loop_no_std.rs:13:37 + | +LL | while b.load(Ordering::Acquire) {} + | ^^ help: try this: `{ core::hint::spin_loop() }` + | + = note: `-D clippy::missing-spin-loop` implied by `-D warnings` + +error: aborting due to previous error + diff --git a/src/tools/clippy/tests/ui/needless_match.fixed b/src/tools/clippy/tests/ui/needless_match.fixed new file mode 100644 index 00000000000..ece18ad737f --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_match.fixed @@ -0,0 +1,83 @@ +// run-rustfix +#![warn(clippy::needless_match)] +#![allow(clippy::manual_map)] +#![allow(dead_code)] + +#[derive(Clone, Copy)] +enum Choice { + A, + B, + C, + D, +} + +#[allow(unused_mut)] +fn useless_match() { + let mut i = 10; + let _: i32 = i; + let _: i32 = i; + let mut _i_mut = i; + + let s = "test"; + let _: &str = s; +} + +fn custom_type_match(se: Choice) { + let _: Choice = se; + // Don't trigger + let _: Choice = match se { + Choice::A => Choice::A, + Choice::B => Choice::B, + _ => Choice::C, + }; + // Mingled, don't trigger + let _: Choice = match se { + Choice::A => Choice::B, + Choice::B => Choice::C, + Choice::C => Choice::D, + Choice::D => Choice::A, + }; +} + +fn option_match(x: Option<i32>) { + let _: Option<i32> = x; + // Don't trigger, this is the case for manual_map_option + let _: Option<i32> = match x { + Some(a) => Some(-a), + None => None, + }; +} + +fn func_ret_err<T>(err: T) -> Result<i32, T> { + Err(err) +} + +fn result_match() { + let _: Result<i32, i32> = Ok(1); + let _: Result<i32, i32> = func_ret_err(0_i32); +} + +fn if_let_option() -> Option<i32> { + Some(1) +} + +fn if_let_result(x: Result<(), i32>) { + let _: Result<(), i32> = x; + let _: Result<(), i32> = x; + // Input type mismatch, don't trigger + let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x }; +} + +fn if_let_custom_enum(x: Choice) { + let _: Choice = x; + // Don't trigger + let _: Choice = if let Choice::A = x { + Choice::A + } else if true { + Choice::B + } else { + x + }; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/needless_match.rs b/src/tools/clippy/tests/ui/needless_match.rs new file mode 100644 index 00000000000..36649de35a6 --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_match.rs @@ -0,0 +1,122 @@ +// run-rustfix +#![warn(clippy::needless_match)] +#![allow(clippy::manual_map)] +#![allow(dead_code)] + +#[derive(Clone, Copy)] +enum Choice { + A, + B, + C, + D, +} + +#[allow(unused_mut)] +fn useless_match() { + let mut i = 10; + let _: i32 = match i { + 0 => 0, + 1 => 1, + 2 => 2, + _ => i, + }; + let _: i32 = match i { + 0 => 0, + 1 => 1, + ref i => *i, + }; + let mut _i_mut = match i { + 0 => 0, + 1 => 1, + ref mut i => *i, + }; + + let s = "test"; + let _: &str = match s { + "a" => "a", + "b" => "b", + s => s, + }; +} + +fn custom_type_match(se: Choice) { + let _: Choice = match se { + Choice::A => Choice::A, + Choice::B => Choice::B, + Choice::C => Choice::C, + Choice::D => Choice::D, + }; + // Don't trigger + let _: Choice = match se { + Choice::A => Choice::A, + Choice::B => Choice::B, + _ => Choice::C, + }; + // Mingled, don't trigger + let _: Choice = match se { + Choice::A => Choice::B, + Choice::B => Choice::C, + Choice::C => Choice::D, + Choice::D => Choice::A, + }; +} + +fn option_match(x: Option<i32>) { + let _: Option<i32> = match x { + Some(a) => Some(a), + None => None, + }; + // Don't trigger, this is the case for manual_map_option + let _: Option<i32> = match x { + Some(a) => Some(-a), + None => None, + }; +} + +fn func_ret_err<T>(err: T) -> Result<i32, T> { + Err(err) +} + +fn result_match() { + let _: Result<i32, i32> = match Ok(1) { + Ok(a) => Ok(a), + Err(err) => Err(err), + }; + let _: Result<i32, i32> = match func_ret_err(0_i32) { + Err(err) => Err(err), + Ok(a) => Ok(a), + }; +} + +fn if_let_option() -> Option<i32> { + if let Some(a) = Some(1) { Some(a) } else { None } +} + +fn if_let_result(x: Result<(), i32>) { + let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x }; + let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x }; + // Input type mismatch, don't trigger + let _: Result<(), i32> = if let Err(e) = Ok(1) { Err(e) } else { x }; +} + +fn if_let_custom_enum(x: Choice) { + let _: Choice = if let Choice::A = x { + Choice::A + } else if let Choice::B = x { + Choice::B + } else if let Choice::C = x { + Choice::C + } else { + x + }; + // Don't trigger + let _: Choice = if let Choice::A = x { + Choice::A + } else if true { + Choice::B + } else { + x + }; +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/needless_match.stderr b/src/tools/clippy/tests/ui/needless_match.stderr new file mode 100644 index 00000000000..ad1525406ad --- /dev/null +++ b/src/tools/clippy/tests/ui/needless_match.stderr @@ -0,0 +1,122 @@ +error: this match expression is unnecessary + --> $DIR/needless_match.rs:17:18 + | +LL | let _: i32 = match i { + | __________________^ +LL | | 0 => 0, +LL | | 1 => 1, +LL | | 2 => 2, +LL | | _ => i, +LL | | }; + | |_____^ help: replace it with: `i` + | + = note: `-D clippy::needless-match` implied by `-D warnings` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:23:18 + | +LL | let _: i32 = match i { + | __________________^ +LL | | 0 => 0, +LL | | 1 => 1, +LL | | ref i => *i, +LL | | }; + | |_____^ help: replace it with: `i` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:28:22 + | +LL | let mut _i_mut = match i { + | ______________________^ +LL | | 0 => 0, +LL | | 1 => 1, +LL | | ref mut i => *i, +LL | | }; + | |_____^ help: replace it with: `i` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:35:19 + | +LL | let _: &str = match s { + | ___________________^ +LL | | "a" => "a", +LL | | "b" => "b", +LL | | s => s, +LL | | }; + | |_____^ help: replace it with: `s` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:43:21 + | +LL | let _: Choice = match se { + | _____________________^ +LL | | Choice::A => Choice::A, +LL | | Choice::B => Choice::B, +LL | | Choice::C => Choice::C, +LL | | Choice::D => Choice::D, +LL | | }; + | |_____^ help: replace it with: `se` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:65:26 + | +LL | let _: Option<i32> = match x { + | __________________________^ +LL | | Some(a) => Some(a), +LL | | None => None, +LL | | }; + | |_____^ help: replace it with: `x` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:81:31 + | +LL | let _: Result<i32, i32> = match Ok(1) { + | _______________________________^ +LL | | Ok(a) => Ok(a), +LL | | Err(err) => Err(err), +LL | | }; + | |_____^ help: replace it with: `Ok(1)` + +error: this match expression is unnecessary + --> $DIR/needless_match.rs:85:31 + | +LL | let _: Result<i32, i32> = match func_ret_err(0_i32) { + | _______________________________^ +LL | | Err(err) => Err(err), +LL | | Ok(a) => Ok(a), +LL | | }; + | |_____^ help: replace it with: `func_ret_err(0_i32)` + +error: this if-let expression is unnecessary + --> $DIR/needless_match.rs:92:5 + | +LL | if let Some(a) = Some(1) { Some(a) } else { None } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(1)` + +error: this if-let expression is unnecessary + --> $DIR/needless_match.rs:96:30 + | +LL | let _: Result<(), i32> = if let Err(e) = x { Err(e) } else { x }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x` + +error: this if-let expression is unnecessary + --> $DIR/needless_match.rs:97:30 + | +LL | let _: Result<(), i32> = if let Ok(val) = x { Ok(val) } else { x }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x` + +error: this if-let expression is unnecessary + --> $DIR/needless_match.rs:103:21 + | +LL | let _: Choice = if let Choice::A = x { + | _____________________^ +LL | | Choice::A +LL | | } else if let Choice::B = x { +LL | | Choice::B +... | +LL | | x +LL | | }; + | |_____^ help: replace it with: `x` + +error: aborting due to 12 previous errors + diff --git a/src/tools/clippy/tests/ui/only_used_in_recursion.rs b/src/tools/clippy/tests/ui/only_used_in_recursion.rs new file mode 100644 index 00000000000..5768434f988 --- /dev/null +++ b/src/tools/clippy/tests/ui/only_used_in_recursion.rs @@ -0,0 +1,122 @@ +#![warn(clippy::only_used_in_recursion)] + +fn simple(a: usize, b: usize) -> usize { + if a == 0 { 1 } else { simple(a - 1, b) } +} + +fn with_calc(a: usize, b: isize) -> usize { + if a == 0 { 1 } else { with_calc(a - 1, -b + 1) } +} + +fn tuple((a, b): (usize, usize)) -> usize { + if a == 0 { 1 } else { tuple((a - 1, b + 1)) } +} + +fn let_tuple(a: usize, b: usize) -> usize { + let (c, d) = (a, b); + if c == 0 { 1 } else { let_tuple(c - 1, d + 1) } +} + +fn array([a, b]: [usize; 2]) -> usize { + if a == 0 { 1 } else { array([a - 1, b + 1]) } +} + +fn index(a: usize, mut b: &[usize], c: usize) -> usize { + if a == 0 { 1 } else { index(a - 1, b, c + b[0]) } +} + +fn break_(a: usize, mut b: usize, mut c: usize) -> usize { + let c = loop { + b += 1; + c += 1; + if c == 10 { + break b; + } + }; + + if a == 0 { 1 } else { break_(a - 1, c, c) } +} + +// this has a side effect +fn mut_ref(a: usize, b: &mut usize) -> usize { + *b = 1; + if a == 0 { 1 } else { mut_ref(a - 1, b) } +} + +fn mut_ref2(a: usize, b: &mut usize) -> usize { + let mut c = *b; + if a == 0 { 1 } else { mut_ref2(a - 1, &mut c) } +} + +fn not_primitive(a: usize, b: String) -> usize { + if a == 0 { 1 } else { not_primitive(a - 1, b) } +} + +// this doesn't have a side effect, +// but `String` is not primitive. +fn not_primitive_op(a: usize, b: String, c: &str) -> usize { + if a == 1 { 1 } else { not_primitive_op(a, b + c, c) } +} + +struct A; + +impl A { + fn method(a: usize, b: usize) -> usize { + if a == 0 { 1 } else { A::method(a - 1, b - 1) } + } + + fn method2(&self, a: usize, b: usize) -> usize { + if a == 0 { 1 } else { self.method2(a - 1, b + 1) } + } +} + +trait B { + fn hello(a: usize, b: usize) -> usize; + + fn hello2(&self, a: usize, b: usize) -> usize; +} + +impl B for A { + fn hello(a: usize, b: usize) -> usize { + if a == 0 { 1 } else { A::hello(a - 1, b + 1) } + } + + fn hello2(&self, a: usize, b: usize) -> usize { + if a == 0 { 1 } else { self.hello2(a - 1, b + 1) } + } +} + +trait C { + fn hello(a: usize, b: usize) -> usize { + if a == 0 { 1 } else { Self::hello(a - 1, b + 1) } + } + + fn hello2(&self, a: usize, b: usize) -> usize { + if a == 0 { 1 } else { self.hello2(a - 1, b + 1) } + } +} + +fn ignore(a: usize, _: usize) -> usize { + if a == 1 { 1 } else { ignore(a - 1, 0) } +} + +fn ignore2(a: usize, _b: usize) -> usize { + if a == 1 { 1 } else { ignore2(a - 1, _b) } +} + +fn f1(a: u32) -> u32 { + a +} + +fn f2(a: u32) -> u32 { + f1(a) +} + +fn inner_fn(a: u32) -> u32 { + fn inner_fn(a: u32) -> u32 { + a + } + inner_fn(a) +} + +fn main() {} diff --git a/src/tools/clippy/tests/ui/only_used_in_recursion.stderr b/src/tools/clippy/tests/ui/only_used_in_recursion.stderr new file mode 100644 index 00000000000..6fe9361bf5f --- /dev/null +++ b/src/tools/clippy/tests/ui/only_used_in_recursion.stderr @@ -0,0 +1,82 @@ +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:3:21 + | +LL | fn simple(a: usize, b: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + | + = note: `-D clippy::only-used-in-recursion` implied by `-D warnings` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:7:24 + | +LL | fn with_calc(a: usize, b: isize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:11:14 + | +LL | fn tuple((a, b): (usize, usize)) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:15:24 + | +LL | fn let_tuple(a: usize, b: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:20:14 + | +LL | fn array([a, b]: [usize; 2]) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:24:20 + | +LL | fn index(a: usize, mut b: &[usize], c: usize) -> usize { + | ^^^^^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:24:37 + | +LL | fn index(a: usize, mut b: &[usize], c: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_c` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:28:21 + | +LL | fn break_(a: usize, mut b: usize, mut c: usize) -> usize { + | ^^^^^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:46:23 + | +LL | fn mut_ref2(a: usize, b: &mut usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:51:28 + | +LL | fn not_primitive(a: usize, b: String) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:68:33 + | +LL | fn method2(&self, a: usize, b: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:90:24 + | +LL | fn hello(a: usize, b: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: parameter is only used in recursion + --> $DIR/only_used_in_recursion.rs:94:32 + | +LL | fn hello2(&self, a: usize, b: usize) -> usize { + | ^ help: if this is intentional, prefix with an underscore: `_b` + +error: aborting due to 13 previous errors + diff --git a/src/tools/clippy/tests/ui/range_plus_minus_one.fixed b/src/tools/clippy/tests/ui/range_plus_minus_one.fixed index 19b253b0fe2..40d7791df28 100644 --- a/src/tools/clippy/tests/ui/range_plus_minus_one.fixed +++ b/src/tools/clippy/tests/ui/range_plus_minus_one.fixed @@ -1,7 +1,7 @@ // run-rustfix #![allow(unused_parens)] - +#![allow(clippy::iter_with_drain)] fn f() -> usize { 42 } diff --git a/src/tools/clippy/tests/ui/range_plus_minus_one.rs b/src/tools/clippy/tests/ui/range_plus_minus_one.rs index 7d034117547..a8ddd9b5f75 100644 --- a/src/tools/clippy/tests/ui/range_plus_minus_one.rs +++ b/src/tools/clippy/tests/ui/range_plus_minus_one.rs @@ -1,7 +1,7 @@ // run-rustfix #![allow(unused_parens)] - +#![allow(clippy::iter_with_drain)] fn f() -> usize { 42 } diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed index 9ae0ed0b13f..b425cdd6cbf 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -25,8 +25,8 @@ fn main() { let slice_ptr = &[0, 1, 2, 3] as *const [i32]; // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast - let _ptr_to_unsized_transmute = unsafe { slice_ptr as *const [u16] }; - let _ptr_to_unsized = slice_ptr as *const [u16]; + let _ptr_to_unsized_transmute = unsafe { slice_ptr as *const [u32] }; + let _ptr_to_unsized = slice_ptr as *const [u32]; // TODO: We could try testing vtable casts here too, but maybe // we should wait until std::raw::TraitObject is stabilized? diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 985cf9a075d..8fd57c59652 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -25,8 +25,8 @@ fn main() { let slice_ptr = &[0, 1, 2, 3] as *const [i32]; // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast - let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u16]>(slice_ptr) }; - let _ptr_to_unsized = slice_ptr as *const [u16]; + let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u32]>(slice_ptr) }; + let _ptr_to_unsized = slice_ptr as *const [u32]; // TODO: We could try testing vtable casts here too, but maybe // we should wait until std::raw::TraitObject is stabilized? diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr index e8496a325d6..d9b64a0ed7b 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -17,8 +17,8 @@ LL | let _ptr_i8_transmute = unsafe { transmute::<*const i32, *const i8>(ptr error: transmute from a pointer to a pointer --> $DIR/transmutes_expressible_as_ptr_casts.rs:28:46 | -LL | let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u16]>(slice_ptr) }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u16]` +LL | let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u32]>(slice_ptr) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u32]` error: transmute from `*const i32` to `usize` which could be expressed as a pointer cast instead --> $DIR/transmutes_expressible_as_ptr_casts.rs:34:50 diff --git a/src/tools/clippy/tests/ui/unnecessary_filter_map.rs b/src/tools/clippy/tests/ui/unnecessary_filter_map.rs index c58181f518d..8e01c2674f1 100644 --- a/src/tools/clippy/tests/ui/unnecessary_filter_map.rs +++ b/src/tools/clippy/tests/ui/unnecessary_filter_map.rs @@ -1,3 +1,5 @@ +#![allow(dead_code)] + fn main() { let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None }); let _ = (0..4).filter_map(|x| { @@ -19,3 +21,130 @@ fn main() { fn filter_map_none_changes_item_type() -> impl Iterator<Item = bool> { "".chars().filter_map(|_| None) } + +// https://github.com/rust-lang/rust-clippy/issues/4433#issue-483920107 +mod comment_483920107 { + enum Severity { + Warning, + Other, + } + + struct ServerError; + + impl ServerError { + fn severity(&self) -> Severity { + Severity::Warning + } + } + + struct S { + warnings: Vec<ServerError>, + } + + impl S { + fn foo(&mut self, server_errors: Vec<ServerError>) { + #[allow(unused_variables)] + let errors: Vec<ServerError> = server_errors + .into_iter() + .filter_map(|se| match se.severity() { + Severity::Warning => { + self.warnings.push(se); + None + }, + _ => Some(se), + }) + .collect(); + } + } +} + +// https://github.com/rust-lang/rust-clippy/issues/4433#issuecomment-611006622 +mod comment_611006622 { + struct PendingRequest { + reply_to: u8, + token: u8, + expires: u8, + group_id: u8, + } + + enum Value { + Null, + } + + struct Node; + + impl Node { + fn send_response(&self, _reply_to: u8, _token: u8, _value: Value) -> &Self { + self + } + fn on_error_warn(&self) -> &Self { + self + } + } + + struct S { + pending_requests: Vec<PendingRequest>, + } + + impl S { + fn foo(&mut self, node: Node, now: u8, group_id: u8) { + // "drain_filter" + self.pending_requests = self + .pending_requests + .drain(..) + .filter_map(|pending| { + if pending.expires <= now { + return None; // Expired, remove + } + + if pending.group_id == group_id { + // Matched - reuse strings and remove + node.send_response(pending.reply_to, pending.token, Value::Null) + .on_error_warn(); + None + } else { + // Keep waiting + Some(pending) + } + }) + .collect(); + } + } +} + +// https://github.com/rust-lang/rust-clippy/issues/4433#issuecomment-621925270 +// This extrapolation doesn't reproduce the false positive. Additional context seems necessary. +mod comment_621925270 { + struct Signature(u8); + + fn foo(sig_packets: impl Iterator<Item = Result<Signature, ()>>) -> impl Iterator<Item = u8> { + sig_packets.filter_map(|res| match res { + Ok(Signature(sig_packet)) => Some(sig_packet), + _ => None, + }) + } +} + +// https://github.com/rust-lang/rust-clippy/issues/4433#issuecomment-1052978898 +mod comment_1052978898 { + #![allow(clippy::redundant_closure)] + + pub struct S(u8); + + impl S { + pub fn consume(self) { + println!("yum"); + } + } + + pub fn filter_owned() -> impl Iterator<Item = S> { + (0..10).map(|i| S(i)).filter_map(|s| { + if s.0 & 1 == 0 { + s.consume(); + None + } else { + Some(s) + } + }) + } +} diff --git a/src/tools/clippy/tests/ui/unnecessary_filter_map.stderr b/src/tools/clippy/tests/ui/unnecessary_filter_map.stderr index 041829c3c78..5585b10ab90 100644 --- a/src/tools/clippy/tests/ui/unnecessary_filter_map.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_filter_map.stderr @@ -1,5 +1,5 @@ error: this `.filter_map` can be written more simply using `.filter` - --> $DIR/unnecessary_filter_map.rs:2:13 + --> $DIR/unnecessary_filter_map.rs:4:13 | LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | let _ = (0..4).filter_map(|x| if x > 1 { Some(x) } else { None }); = note: `-D clippy::unnecessary-filter-map` implied by `-D warnings` error: this `.filter_map` can be written more simply using `.filter` - --> $DIR/unnecessary_filter_map.rs:3:13 + --> $DIR/unnecessary_filter_map.rs:5:13 | LL | let _ = (0..4).filter_map(|x| { | _____________^ @@ -19,7 +19,7 @@ LL | | }); | |______^ error: this `.filter_map` can be written more simply using `.filter` - --> $DIR/unnecessary_filter_map.rs:9:13 + --> $DIR/unnecessary_filter_map.rs:11:13 | LL | let _ = (0..4).filter_map(|x| match x { | _____________^ @@ -29,7 +29,7 @@ LL | | }); | |______^ error: this `.filter_map` can be written more simply using `.map` - --> $DIR/unnecessary_filter_map.rs:14:13 + --> $DIR/unnecessary_filter_map.rs:16:13 | LL | let _ = (0..4).filter_map(|x| Some(x + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/tools/clippy/tests/ui/unnecessary_find_map.rs b/src/tools/clippy/tests/ui/unnecessary_find_map.rs new file mode 100644 index 00000000000..a52390861b4 --- /dev/null +++ b/src/tools/clippy/tests/ui/unnecessary_find_map.rs @@ -0,0 +1,23 @@ +#![allow(dead_code)] + +fn main() { + let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None }); + let _ = (0..4).find_map(|x| { + if x > 1 { + return Some(x); + }; + None + }); + let _ = (0..4).find_map(|x| match x { + 0 | 1 => None, + _ => Some(x), + }); + + let _ = (0..4).find_map(|x| Some(x + 1)); + + let _ = (0..4).find_map(i32::checked_abs); +} + +fn find_map_none_changes_item_type() -> Option<bool> { + "".chars().find_map(|_| None) +} diff --git a/src/tools/clippy/tests/ui/unnecessary_find_map.stderr b/src/tools/clippy/tests/ui/unnecessary_find_map.stderr new file mode 100644 index 00000000000..fb33c122fe3 --- /dev/null +++ b/src/tools/clippy/tests/ui/unnecessary_find_map.stderr @@ -0,0 +1,38 @@ +error: this `.find_map` can be written more simply using `.find` + --> $DIR/unnecessary_find_map.rs:4:13 + | +LL | let _ = (0..4).find_map(|x| if x > 1 { Some(x) } else { None }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unnecessary-find-map` implied by `-D warnings` + +error: this `.find_map` can be written more simply using `.find` + --> $DIR/unnecessary_find_map.rs:5:13 + | +LL | let _ = (0..4).find_map(|x| { + | _____________^ +LL | | if x > 1 { +LL | | return Some(x); +LL | | }; +LL | | None +LL | | }); + | |______^ + +error: this `.find_map` can be written more simply using `.find` + --> $DIR/unnecessary_find_map.rs:11:13 + | +LL | let _ = (0..4).find_map(|x| match x { + | _____________^ +LL | | 0 | 1 => None, +LL | | _ => Some(x), +LL | | }); + | |______^ + +error: this `.find_map` can be written more simply using `.map(..).next()` + --> $DIR/unnecessary_find_map.rs:16:13 + | +LL | let _ = (0..4).find_map(|x| Some(x + 1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors + diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed b/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed index d806d620b17..21e2da474a8 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.fixed @@ -3,7 +3,6 @@ #![allow(clippy::stable_sort_primitive)] use std::cell::Ref; -use std::cmp::Reverse; fn unnecessary_sort_by() { fn id(x: isize) -> isize { @@ -18,8 +17,8 @@ fn unnecessary_sort_by() { vec.sort_unstable_by_key(|a| id(-a)); // Reverse examples vec.sort_by(|a, b| b.cmp(a)); // not linted to avoid suggesting `Reverse(b)` which would borrow - vec.sort_by_key(|b| Reverse((b + 5).abs())); - vec.sort_unstable_by_key(|b| Reverse(id(-b))); + vec.sort_by_key(|b| std::cmp::Reverse((b + 5).abs())); + vec.sort_unstable_by_key(|b| std::cmp::Reverse(id(-b))); // Negative examples (shouldn't be changed) let c = &7; vec.sort_by(|a, b| (b - a).cmp(&(a - b))); @@ -76,7 +75,6 @@ mod issue_5754 { // The closure parameter is not dereferenced anymore, so non-Copy types can be linted mod issue_6001 { - use super::*; struct Test(String); impl Test { @@ -93,8 +91,8 @@ mod issue_6001 { args.sort_by_key(|a| a.name()); args.sort_unstable_by_key(|a| a.name()); // Reverse - args.sort_by_key(|b| Reverse(b.name())); - args.sort_unstable_by_key(|b| Reverse(b.name())); + args.sort_by_key(|b| std::cmp::Reverse(b.name())); + args.sort_unstable_by_key(|b| std::cmp::Reverse(b.name())); } } diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.rs b/src/tools/clippy/tests/ui/unnecessary_sort_by.rs index 6ee9c3af455..3365bf6e119 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.rs +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.rs @@ -3,7 +3,6 @@ #![allow(clippy::stable_sort_primitive)] use std::cell::Ref; -use std::cmp::Reverse; fn unnecessary_sort_by() { fn id(x: isize) -> isize { @@ -76,7 +75,6 @@ mod issue_5754 { // The closure parameter is not dereferenced anymore, so non-Copy types can be linted mod issue_6001 { - use super::*; struct Test(String); impl Test { diff --git a/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr b/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr index ca9641e8803..89da5e7ea8b 100644 --- a/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr +++ b/src/tools/clippy/tests/ui/unnecessary_sort_by.stderr @@ -1,5 +1,5 @@ error: use Vec::sort here instead - --> $DIR/unnecessary_sort_by.rs:15:5 + --> $DIR/unnecessary_sort_by.rs:14:5 | LL | vec.sort_by(|a, b| a.cmp(b)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort()` @@ -7,70 +7,70 @@ LL | vec.sort_by(|a, b| a.cmp(b)); = note: `-D clippy::unnecessary-sort-by` implied by `-D warnings` error: use Vec::sort here instead - --> $DIR/unnecessary_sort_by.rs:16:5 + --> $DIR/unnecessary_sort_by.rs:15:5 | LL | vec.sort_unstable_by(|a, b| a.cmp(b)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable()` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:17:5 + --> $DIR/unnecessary_sort_by.rs:16:5 | LL | vec.sort_by(|a, b| (a + 5).abs().cmp(&(b + 5).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (a + 5).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:18:5 + --> $DIR/unnecessary_sort_by.rs:17:5 | LL | vec.sort_unstable_by(|a, b| id(-a).cmp(&id(-b))); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| id(-a))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:21:5 + --> $DIR/unnecessary_sort_by.rs:20:5 | LL | vec.sort_by(|a, b| (b + 5).abs().cmp(&(a + 5).abs())); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| Reverse((b + 5).abs()))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|b| std::cmp::Reverse((b + 5).abs()))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:22:5 + --> $DIR/unnecessary_sort_by.rs:21:5 | LL | vec.sort_unstable_by(|a, b| id(-b).cmp(&id(-a))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|b| Reverse(id(-b)))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|b| std::cmp::Reverse(id(-b)))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:32:5 + --> $DIR/unnecessary_sort_by.rs:31:5 | LL | vec.sort_by(|a, b| (***a).abs().cmp(&(***b).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_by_key(|a| (***a).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:33:5 + --> $DIR/unnecessary_sort_by.rs:32:5 | LL | vec.sort_unstable_by(|a, b| (***a).abs().cmp(&(***b).abs())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `vec.sort_unstable_by_key(|a| (***a).abs())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:93:9 + --> $DIR/unnecessary_sort_by.rs:91:9 | LL | args.sort_by(|a, b| a.name().cmp(&b.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|a| a.name())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:94:9 + --> $DIR/unnecessary_sort_by.rs:92:9 | LL | args.sort_unstable_by(|a, b| a.name().cmp(&b.name())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|a| a.name())` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:96:9 + --> $DIR/unnecessary_sort_by.rs:94:9 | LL | args.sort_by(|a, b| b.name().cmp(&a.name())); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| Reverse(b.name()))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_by_key(|b| std::cmp::Reverse(b.name()))` error: use Vec::sort_by_key here instead - --> $DIR/unnecessary_sort_by.rs:97:9 + --> $DIR/unnecessary_sort_by.rs:95:9 | LL | args.sort_unstable_by(|a, b| b.name().cmp(&a.name())); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|b| Reverse(b.name()))` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `args.sort_unstable_by_key(|b| std::cmp::Reverse(b.name()))` error: aborting due to 12 previous errors diff --git a/src/tools/clippy/tests/ui/use_self.fixed b/src/tools/clippy/tests/ui/use_self.fixed index 4e33e343ce0..9d216f56ae6 100644 --- a/src/tools/clippy/tests/ui/use_self.fixed +++ b/src/tools/clippy/tests/ui/use_self.fixed @@ -2,7 +2,7 @@ // aux-build:proc_macro_derive.rs #![warn(clippy::use_self)] -#![allow(dead_code)] +#![allow(dead_code, unreachable_code)] #![allow( clippy::should_implement_trait, clippy::upper_case_acronyms, @@ -519,3 +519,26 @@ mod self_is_ty_param { } } } + +mod use_self_in_pat { + enum Foo { + Bar, + Baz, + } + + impl Foo { + fn do_stuff(self) { + match self { + Self::Bar => unimplemented!(), + Self::Baz => unimplemented!(), + } + match Some(1) { + Some(_) => unimplemented!(), + None => unimplemented!(), + } + if let Self::Bar = self { + unimplemented!() + } + } + } +} diff --git a/src/tools/clippy/tests/ui/use_self.rs b/src/tools/clippy/tests/ui/use_self.rs index 7b621ff9bca..5f604fe25e4 100644 --- a/src/tools/clippy/tests/ui/use_self.rs +++ b/src/tools/clippy/tests/ui/use_self.rs @@ -2,7 +2,7 @@ // aux-build:proc_macro_derive.rs #![warn(clippy::use_self)] -#![allow(dead_code)] +#![allow(dead_code, unreachable_code)] #![allow( clippy::should_implement_trait, clippy::upper_case_acronyms, @@ -519,3 +519,26 @@ mod self_is_ty_param { } } } + +mod use_self_in_pat { + enum Foo { + Bar, + Baz, + } + + impl Foo { + fn do_stuff(self) { + match self { + Foo::Bar => unimplemented!(), + Foo::Baz => unimplemented!(), + } + match Some(1) { + Some(_) => unimplemented!(), + None => unimplemented!(), + } + if let Foo::Bar = self { + unimplemented!() + } + } + } +} diff --git a/src/tools/clippy/tests/ui/use_self.stderr b/src/tools/clippy/tests/ui/use_self.stderr index ecb78b3f972..34d98618253 100644 --- a/src/tools/clippy/tests/ui/use_self.stderr +++ b/src/tools/clippy/tests/ui/use_self.stderr @@ -168,5 +168,23 @@ error: unnecessary structure name repetition LL | S2::new() | ^^ help: use the applicable keyword: `Self` -error: aborting due to 28 previous errors +error: unnecessary structure name repetition + --> $DIR/use_self.rs:532:17 + | +LL | Foo::Bar => unimplemented!(), + | ^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:533:17 + | +LL | Foo::Baz => unimplemented!(), + | ^^^ help: use the applicable keyword: `Self` + +error: unnecessary structure name repetition + --> $DIR/use_self.rs:539:20 + | +LL | if let Foo::Bar = self { + | ^^^ help: use the applicable keyword: `Self` + +error: aborting due to 31 previous errors diff --git a/src/tools/clippy/util/gh-pages/index.html b/src/tools/clippy/util/gh-pages/index.html index 83a200ca3c4..97c974003c6 100644 --- a/src/tools/clippy/util/gh-pages/index.html +++ b/src/tools/clippy/util/gh-pages/index.html @@ -25,7 +25,56 @@ Otherwise, have a great day =^.^= blockquote { font-size: 1em; } [ng\:cloak], [ng-cloak], [data-ng-cloak], [x-ng-cloak], .ng-cloak, .x-ng-cloak { display: none !important; } - .form-inline .checkbox { margin-right: 0.6em } + .dropdown-menu { + color: var(--fg); + background: var(--theme-popup-bg); + border: 1px solid var(--theme-popup-border); + } + + .dropdown-menu .divider { + background-color: var(--theme-popup-border); + } + + .dropdown-menu .checkbox { + display: block; + white-space: nowrap; + margin: 0; + } + .dropdown-menu .checkbox label { + padding: 3px 20px; + width: 100%; + } + + .dropdown-menu .checkbox input { + position: relative; + margin: 0 0.5rem 0; + padding: 0; + } + + .dropdown-menu .checkbox:hover { + background-color: var(--theme-hover); + } + + div.panel div.panel-body button.dropdown-toggle { + background: var(--searchbar-bg); + color: var(--searchbar-fg); + border-color: var(--theme-popup-border); + } + + div.panel div.panel-body button.dropdown-toggle:hover { + box-shadow: 0 0 3px var(--searchbar-shadow-color); + } + + div.panel div.panel-body .open button.dropdown-toggle { + background: var(--searchbar-bg); + color: var(--searchbar-fg); + border-color: var(--theme-popup-border); + filter: brightness(90%); + } + + .dropdown-toggle .badge { + background-color: #777; + } .panel-heading { cursor: pointer; } @@ -38,6 +87,16 @@ Otherwise, have a great day =^.^= .panel .panel-title-name .anchor { display: none; } .panel:hover .panel-title-name .anchor { display: inline;} + .search-control { + margin-top: 15px; + } + + @media (min-width: 992px) { + .search-control { + margin-top: 0; + } + } + .label { padding-top: 0.3em; padding-bottom: 0.3em; @@ -143,13 +202,17 @@ Otherwise, have a great day =^.^= --inline-code-bg: #191f26; } + .theme-dropdown { + position: absolute; + margin: 0.7em; + z-index: 10; + } + /* Applying the mdBook theme */ .theme-icon { - position: absolute; text-align: center; width: 2em; height: 2em; - margin: 0.7em; line-height: 2em; border: solid 1px var(--icons); border-radius: 5px; @@ -160,23 +223,28 @@ Otherwise, have a great day =^.^= background: var(--theme-hover); } .theme-choice { - position: absolute; - margin-top: calc(2em + 0.7em); - margin-left: 0.7em; + display: none; list-style: none; border: 1px solid var(--theme-popup-border); border-radius: 5px; color: var(--fg); background: var(--theme-popup-bg); padding: 0 0; + overflow: hidden; } + + .theme-dropdown.open .theme-choice { + display: block; + } + .theme-choice > li { padding: 5px 10px; font-size: 0.8em; user-select: none; cursor: pointer; } - .theme-choice > li:hover { + + .theme-choice>li:hover { background: var(--theme-hover); } @@ -240,17 +308,15 @@ Otherwise, have a great day =^.^= </style> </head> -<body> - <div id="theme-icon" class="theme-icon">🖌</div> - <ul id="theme-menu" class="theme-choice" style="display: none;"> - <li id="light">Light</li> - <li id="rust">Rust</li> - <li id="coal">Coal</li> - <li id="navy">Navy</li> - <li id="ayu">Ayu</li> - </ul> - - <div class="container" ng-app="clippy" ng-controller="lintList"> +<body ng-app="clippy" ng-controller="lintList"> + <div theme-dropdown class="theme-dropdown"> + <div id="theme-icon" class="theme-icon">🖌</div> + <ul id="theme-menu" class="theme-choice"> + <li id="{{id}}" ng-repeat="(id, name) in themes" ng-click="selectTheme(id)">{{name}}</li> + </ul> + </div> + + <div class="container"> <div class="page-header"> <h1>Clippy Lints</h1> </div> @@ -271,38 +337,62 @@ Otherwise, have a great day =^.^= </div> <div class="panel panel-default" ng-show="data"> - <div class="panel-body row filter-panel"> - <div class="col-md-6 form-inline"> - <div class="form-group form-group-lg"> - <p class="h4"> - Lint levels - <a href="https://doc.rust-lang.org/rustc/lints/levels.html">(?)</a> - </p> - <div class="checkbox" ng-repeat="(level, enabled) in levels"> - <label class="text-capitalize"> - <input type="checkbox" ng-model="levels[level]" /> - {{level}} - </label> - </div> + <div class="panel-body row"> + <div class="col-12 col-md-4"> + <div class="btn-group" filter-dropdown> + <button type="button" class="btn btn-default dropdown-toggle"> + Lint levels <span class="badge">{{selectedValuesCount(levels)}}</span> <span class="caret"></span> + </button> + <ul class="dropdown-menu"> + <li class="checkbox"> + <label ng-click="toggleLevels(true)"> + <input type="checkbox" class="invisible" /> + All + </label> + </li> + <li class="checkbox"> + <label ng-click="toggleLevels(false)"> + <input type="checkbox" class="invisible" /> + None + </label> + </li> + <li role="separator" class="divider"></li> + <li class="checkbox" ng-repeat="(level, enabled) in levels"> + <label class="text-capitalize"> + <input type="checkbox" ng-model="levels[level]" /> + {{level}} + </label> + </li> + </ul> </div> - </div> - <div class="col-md-6 form-inline"> - <div class="form-group form-group-lg"> - <p class="h4"> - Lint groups - <a href="https://github.com/rust-lang/rust-clippy/#clippy">(?)</a> - </p> - <div class="checkbox" ng-repeat="(group, enabled) in groups"> - <label class="text-capitalize"> - <input type="checkbox" ng-model="groups[group]" /> - {{group}} - </label> - </div> + <div class="btn-group" filter-dropdown> + <button type="button" class="btn btn-default dropdown-toggle"> + Lint groups <span class="badge">{{selectedValuesCount(groups)}}</span> <span class="caret"></span> + </button> + <ul class="dropdown-menu"> + <li class="checkbox"> + <label ng-click="toggleGroups(true)"> + <input type="checkbox" class="invisible" /> + All + </label> + </li> + <li class="checkbox"> + <label ng-click="toggleGroups(false)"> + <input type="checkbox" class="invisible" /> + None + </label> + </li> + <li role="separator" class="divider"></li> + <li class="checkbox" ng-repeat="(group, enabled) in groups"> + <label class="text-capitalize"> + <input type="checkbox" ng-model="groups[group]" /> + {{group}} + </label> + </li> + </ul> </div> </div> - </div> - <div class="panel-body row"> - <div class="col-md-12 form-horizontal"> + <div class="col-12 col-md-8 search-control"> <div class="input-group"> <label class="input-group-addon" id="filter-label" for="filter-input">Filter:</label> <input type="text" class="form-control" placeholder="Keywords or search string" id="filter-input" ng-model="search" ng-model-options="{debounce: 50}"/> @@ -336,7 +426,7 @@ Otherwise, have a great day =^.^= </h2> </header> - <div class="list-group lint-docs" ng-class="{collapse: true, in: open[lint.id]}"> + <div class="list-group lint-docs" ng-if="open[lint.id]" ng-class="{collapse: true, in: open[lint.id]}"> <div class="list-group-item lint-doc-md" ng-bind-html="lint.docs | markdown"></div> <div class="lint-additional-info-container"> <!-- Applicability --> @@ -365,7 +455,7 @@ Otherwise, have a great day =^.^= </div> <a href="https://github.com/rust-lang/rust-clippy"> - <img style="position: absolute; top: 0; right: 0; border: 0;" src="https://s3.amazonaws.com/github/ribbons/forkme_right_darkblue_121621.png" alt="Fork me on Github"/> + <img style="position: absolute; top: 0; right: 0; border: 0; clip-path: polygon(0% 0%, 100% 0%, 100% 100%);" src="https://s3.amazonaws.com/github/ribbons/forkme_right_darkblue_121621.png" alt="Fork me on Github"/> </a> <script src="https://cdnjs.cloudflare.com/ajax/libs/markdown-it/12.3.2/markdown-it.min.js"></script> @@ -429,6 +519,46 @@ Otherwise, have a great day =^.^= ); }; }) + .directive('themeDropdown', function ($document) { + return { + restrict: 'A', + link: function ($scope, $element, $attr) { + $element.bind('click', function () { + $element.toggleClass('open'); + $element.addClass('open-recent'); + }); + + $document.bind('click', function () { + if (!$element.hasClass('open-recent')) { + $element.removeClass('open'); + } + $element.removeClass('open-recent'); + }) + } + } + }) + .directive('filterDropdown', function ($document) { + return { + restrict: 'A', + link: function ($scope, $element, $attr) { + $element.bind('click', function (event) { + if (event.target.closest('button')) { + $element.toggleClass('open'); + } else { + $element.addClass('open'); + } + $element.addClass('open-recent'); + }); + + $document.bind('click', function () { + if (!$element.hasClass('open-recent')) { + $element.removeClass('open'); + } + $element.removeClass('open-recent'); + }) + } + } + }) .directive('onFinishRender', function ($timeout) { return { restrict: 'A', @@ -462,6 +592,38 @@ Otherwise, have a great day =^.^= suspicious: true, }; $scope.groups = GROUPS_FILTER_DEFAULT; + const THEMES_DEFAULT = { + light: "Light", + rust: "Rust", + coal: "Coal", + navy: "Navy", + ayu: "Ayu" + }; + $scope.themes = THEMES_DEFAULT; + + $scope.selectTheme = function (theme) { + setTheme(theme, true); + } + + $scope.toggleLevels = function (value) { + const levels = $scope.levels; + for (const key in levels) { + if (levels.hasOwnProperty(key)) { + levels[key] = value; + } + } + }; + $scope.toggleGroups = function (value) { + const groups = $scope.groups; + for (const key in groups) { + if (groups.hasOwnProperty(key)) { + groups[key] = value; + } + } + }; + $scope.selectedValuesCount = function (obj) { + return Object.values(obj).filter(x => x).length; + } $scope.byGroups = function (lint) { return $scope.groups[lint.group]; }; @@ -558,28 +720,6 @@ Otherwise, have a great day =^.^= } } - function setupListeners() { - let themeIcon = document.getElementById("theme-icon"); - let themeMenu = document.getElementById("theme-menu"); - themeIcon.addEventListener("click", function(e) { - if (themeMenu.style.display == "none") { - themeMenu.style.display = "block"; - } else { - themeMenu.style.display = "none"; - } - }); - - let children = themeMenu.children; - for (let index = 0; index < children.length; index++) { - let child = children[index]; - child.addEventListener("click", function(e) { - setTheme(child.id, true); - }); - } - } - - setupListeners(); - function setTheme(theme, store) { let enableHighlight = false; let enableNight = false; |