From 4ab8cdd5b9f715938f5d4f1c4c6a87b4ea400ce5 Mon Sep 17 00:00:00 2001 From: Jules Bertholet Date: Tue, 19 Dec 2023 22:52:04 -0500 Subject: [PATCH 01/13] Hide foreign `#[doc(hidden)]` paths in import suggestions --- tests/ui/crashes/ice-6252.stderr | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ui/crashes/ice-6252.stderr b/tests/ui/crashes/ice-6252.stderr index f929bec9583..f6d0976091c 100644 --- a/tests/ui/crashes/ice-6252.stderr +++ b/tests/ui/crashes/ice-6252.stderr @@ -8,8 +8,6 @@ help: consider importing one of these items | LL + use core::marker::PhantomData; | -LL + use serde::__private::PhantomData; - | LL + use std::marker::PhantomData; | From 99b4330f5cdcf258a8146950d33426c6bb442abc Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 21 Dec 2023 01:52:10 +0000 Subject: [PATCH 02/13] Remove movability from TyKind::Coroutine --- clippy_lints/src/doc/missing_headers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/doc/missing_headers.rs b/clippy_lints/src/doc/missing_headers.rs index 4cbfa97a8a3..26f120cb33f 100644 --- a/clippy_lints/src/doc/missing_headers.rs +++ b/clippy_lints/src/doc/missing_headers.rs @@ -72,7 +72,7 @@ pub fn check( && let body = cx.tcx.hir().body(body_id) && let ret_ty = typeck.expr_ty(body.value) && implements_trait(cx, ret_ty, future, &[]) - && let ty::Coroutine(_, subs, _) = ret_ty.kind() + && let ty::Coroutine(_, subs) = ret_ty.kind() && is_type_diagnostic_item(cx, subs.as_coroutine().return_ty(), sym::Result) { span_lint( From 15b1edb2092efef167b375ac4a88429a90714210 Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Thu, 28 Dec 2023 19:33:07 +0100 Subject: [PATCH 03/13] Merge commit 'ac4c2094a6030530661bee3876e0228ddfeb6b8b' into clippy-subtree-sync --- CHANGELOG.md | 68 ++++- Cargo.toml | 2 +- book/src/lint_configuration.md | 2 +- clippy_config/Cargo.toml | 2 +- clippy_config/src/conf.rs | 5 +- clippy_config/src/msrvs.rs | 7 +- clippy_lints/Cargo.toml | 2 +- clippy_lints/src/assertions_on_constants.rs | 15 +- clippy_lints/src/async_yields_async.rs | 4 +- clippy_lints/src/declared_lints.rs | 5 + .../src/default_constructed_unit_structs.rs | 2 +- clippy_lints/src/format_push_string.rs | 2 +- clippy_lints/src/implied_bounds_in_impls.rs | 2 +- clippy_lints/src/indexing_slicing.rs | 18 +- clippy_lints/src/ineffective_open_options.rs | 2 +- clippy_lints/src/iter_without_into_iter.rs | 4 +- clippy_lints/src/len_zero.rs | 4 +- clippy_lints/src/lib.rs | 3 + clippy_lints/src/loops/infinite_loop.rs | 2 +- clippy_lints/src/loops/manual_flatten.rs | 2 +- .../src/loops/while_let_on_iterator.rs | 2 +- clippy_lints/src/manual_async_fn.rs | 24 +- clippy_lints/src/manual_hash_one.rs | 2 +- clippy_lints/src/manual_let_else.rs | 2 +- clippy_lints/src/matches/collapsible_match.rs | 4 +- clippy_lints/src/matches/manual_utils.rs | 4 +- clippy_lints/src/matches/mod.rs | 22 +- .../src/matches/redundant_pattern_match.rs | 130 ++++++--- clippy_lints/src/methods/filter_map.rs | 89 +++++-- clippy_lints/src/methods/iter_filter.rs | 87 ++++++ clippy_lints/src/methods/mod.rs | 101 ++++++- .../src/methods/unnecessary_to_owned.rs | 56 ++++ clippy_lints/src/needless_bool.rs | 38 ++- clippy_lints/src/option_if_let_else.rs | 1 + clippy_lints/src/question_mark.rs | 30 ++- clippy_lints/src/redundant_async_block.rs | 16 +- clippy_lints/src/redundant_closure_call.rs | 5 +- clippy_lints/src/transmute/eager_transmute.rs | 74 ++++++ clippy_lints/src/transmute/mod.rs | 61 ++++- clippy_lints/src/unconditional_recursion.rs | 134 ++++++++++ .../src/undocumented_unsafe_blocks.rs | 4 +- clippy_lints/src/uninhabited_references.rs | 2 +- clippy_lints/src/utils/author.rs | 1 + clippy_utils/Cargo.toml | 2 +- clippy_utils/src/ast_utils.rs | 1 + clippy_utils/src/attrs.rs | 11 +- clippy_utils/src/higher.rs | 20 +- clippy_utils/src/visitors.rs | 5 +- declare_clippy_lint/Cargo.toml | 2 +- rust-toolchain | 2 +- .../ui-internal/disallow_struct_span_lint.rs | 4 +- .../disallow_struct_span_lint.stderr | 8 +- tests/ui-toml/suppress_lint_in_const/test.rs | 3 +- .../suppress_lint_in_const/test.stderr | 18 +- tests/ui/assertions_on_constants.rs | 7 + tests/ui/assertions_on_constants.stderr | 10 +- tests/ui/author/loop.rs | 6 +- tests/ui/bool_comparison.fixed | 9 + tests/ui/bool_comparison.rs | 9 + tests/ui/bool_comparison.stderr | 20 +- .../branches_sharing_code/shared_at_bottom.rs | 8 +- .../shared_at_bottom.stderr | 18 +- tests/ui/collapsible_if.fixed | 3 +- tests/ui/collapsible_if.rs | 3 +- tests/ui/collapsible_if.stderr | 18 +- tests/ui/crashes/ice-11939.rs | 14 + tests/ui/crashes/ice-5497.rs | 2 + tests/ui/crashes/ice-5497.stderr | 2 +- tests/ui/doc/doc-fixable.fixed | 2 +- tests/ui/doc/doc-fixable.rs | 2 +- tests/ui/eager_transmute.fixed | 72 +++++ tests/ui/eager_transmute.rs | 72 +++++ tests/ui/eager_transmute.stderr | 92 +++++++ tests/ui/get_unwrap.fixed | 3 +- tests/ui/get_unwrap.rs | 3 +- tests/ui/get_unwrap.stderr | 62 ++--- tests/ui/if_then_some_else_none.rs | 1 + tests/ui/if_then_some_else_none.stderr | 10 +- tests/ui/indexing_slicing_index.rs | 3 + tests/ui/indexing_slicing_index.stderr | 35 ++- tests/ui/infinite_loops.stderr | 30 +-- tests/ui/iter_filter_is_ok.fixed | 26 ++ tests/ui/iter_filter_is_ok.rs | 26 ++ tests/ui/iter_filter_is_ok.stderr | 23 ++ tests/ui/iter_filter_is_some.fixed | 27 ++ tests/ui/iter_filter_is_some.rs | 27 ++ tests/ui/iter_filter_is_some.stderr | 23 ++ tests/ui/manual_let_else_question_mark.fixed | 21 ++ tests/ui/manual_let_else_question_mark.rs | 23 ++ tests/ui/manual_let_else_question_mark.stderr | 10 +- tests/ui/needless_if.fixed | 1 + tests/ui/needless_if.rs | 1 + tests/ui/needless_if.stderr | 14 +- tests/ui/nonminimal_bool.rs | 7 +- tests/ui/nonminimal_bool.stderr | 26 +- tests/ui/option_filter_map.fixed | 6 + tests/ui/option_filter_map.rs | 8 + tests/ui/option_filter_map.stderr | 16 +- tests/ui/redundant_async_block.fixed | 8 +- tests/ui/redundant_async_block.rs | 8 +- ...dundant_pattern_matching_if_let_true.fixed | 38 +++ .../redundant_pattern_matching_if_let_true.rs | 38 +++ ...undant_pattern_matching_if_let_true.stderr | 47 ++++ .../redundant_pattern_matching_ipaddr.fixed | 6 + tests/ui/redundant_pattern_matching_ipaddr.rs | 6 + .../redundant_pattern_matching_ipaddr.stderr | 44 +-- .../ui/redundant_pattern_matching_poll.fixed | 6 + tests/ui/redundant_pattern_matching_poll.rs | 6 + .../ui/redundant_pattern_matching_poll.stderr | 44 +-- tests/ui/result_filter_map.fixed | 27 ++ tests/ui/result_filter_map.rs | 35 +++ tests/ui/result_filter_map.stderr | 41 +++ tests/ui/unconditional_recursion.rs | 163 ++++++++++++ tests/ui/unconditional_recursion.stderr | 251 ++++++++++++++++++ tests/ui/unnecessary_to_owned_on_split.fixed | 21 ++ tests/ui/unnecessary_to_owned_on_split.rs | 21 ++ tests/ui/unnecessary_to_owned_on_split.stderr | 53 ++++ util/gh-pages/index.html | 70 ++++- 118 files changed, 2531 insertions(+), 321 deletions(-) create mode 100644 clippy_lints/src/methods/iter_filter.rs create mode 100644 clippy_lints/src/transmute/eager_transmute.rs create mode 100644 clippy_lints/src/unconditional_recursion.rs create mode 100644 tests/ui/crashes/ice-11939.rs create mode 100644 tests/ui/eager_transmute.fixed create mode 100644 tests/ui/eager_transmute.rs create mode 100644 tests/ui/eager_transmute.stderr create mode 100644 tests/ui/iter_filter_is_ok.fixed create mode 100644 tests/ui/iter_filter_is_ok.rs create mode 100644 tests/ui/iter_filter_is_ok.stderr create mode 100644 tests/ui/iter_filter_is_some.fixed create mode 100644 tests/ui/iter_filter_is_some.rs create mode 100644 tests/ui/iter_filter_is_some.stderr create mode 100644 tests/ui/redundant_pattern_matching_if_let_true.fixed create mode 100644 tests/ui/redundant_pattern_matching_if_let_true.rs create mode 100644 tests/ui/redundant_pattern_matching_if_let_true.stderr create mode 100644 tests/ui/result_filter_map.fixed create mode 100644 tests/ui/result_filter_map.rs create mode 100644 tests/ui/result_filter_map.stderr create mode 100644 tests/ui/unconditional_recursion.rs create mode 100644 tests/ui/unconditional_recursion.stderr create mode 100644 tests/ui/unnecessary_to_owned_on_split.fixed create mode 100644 tests/ui/unnecessary_to_owned_on_split.rs create mode 100644 tests/ui/unnecessary_to_owned_on_split.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 70aff7f60e0..f82421a687b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,68 @@ document. ## Unreleased / Beta / In Rust Nightly -[7671c283...master](https://github.com/rust-lang/rust-clippy/compare/7671c283...master) +[09ac14c9...master](https://github.com/rust-lang/rust-clippy/compare/09ac14c9...master) + +## Rust 1.75 + +Current stable, released 2023-12-28 + +[View all 69 merged pull requests](https://github.com/rust-lang/rust-clippy/pulls?q=merged%3A2023-09-25T11%3A47%3A47Z..2023-11-02T16%3A41%3A59Z+base%3Amaster) + +### New Lints + +* [`unused_enumerate_index`] + [#10404](https://github.com/rust-lang/rust-clippy/pull/10404) +* [`unnecessary_fallible_conversions`] + [#11669](https://github.com/rust-lang/rust-clippy/pull/11669) +* [`waker_clone_wake`] + [#11698](https://github.com/rust-lang/rust-clippy/pull/11698) +* [`struct_field_names`] + [#11496](https://github.com/rust-lang/rust-clippy/pull/11496) +* [`into_iter_without_iter`] + [#11587](https://github.com/rust-lang/rust-clippy/pull/11587) +* [`iter_without_into_iter`] + [#11527](https://github.com/rust-lang/rust-clippy/pull/11527) +* [`manual_hash_one`] + [#11556](https://github.com/rust-lang/rust-clippy/pull/11556) + + +### Moves and Deprecations + +* Moved [`read_zero_byte_vec`] to `nursery` (Now allow-by-default) + [#11727](https://github.com/rust-lang/rust-clippy/pull/11727) +* Moved [`missing_enforced_import_renames`] to `style` (Now warn-by-default) + [#11539](https://github.com/rust-lang/rust-clippy/pull/11539) +* Moved [`needless_raw_string_hashes`] to `pedantic` (Now allow-by-default) + [#11415](https://github.com/rust-lang/rust-clippy/pull/11415) +* Moved [`needless_pass_by_ref_mut`] to `nursery` (Now allow-by-default) + [#11596](https://github.com/rust-lang/rust-clippy/pull/11596) + +### Enhancements + +* [`declare_interior_mutable_const`] and [`borrow_interior_mutable_const`]: Now check the + [`ignore-interior-mutability`] config value + [#11678](https://github.com/rust-lang/rust-clippy/pull/11678) + +### Suggestion Fixes/Improvements + +* [`items_after_test_module`]: The suggestion is now machine-applicable + [#11611](https://github.com/rust-lang/rust-clippy/pull/11611) + +### ICE Fixes + +* [`redundant_locals`]: No longer crashes if variables are rebound above macros + [#11623](https://github.com/rust-lang/rust-clippy/pull/11623) +* [`implicit_hasher`]: No longer lints inside macros, which could cause ICEs + [#11593](https://github.com/rust-lang/rust-clippy/pull/11593) + +### Documentation Improvements + +* `cargo clippy --help` now uses colors for readability :tada: ## Rust 1.74 -Current stable, released 2023-11-16 +Released 2023-11-16 [View all 94 merged pull requests](https://github.com/rust-lang/rust-clippy/pulls?q=merged%3A2023-08-11T15%3A29%3A18Z..2023-09-25T08%3A48%3A22Z+base%3Amaster) @@ -51,7 +108,7 @@ Current stable, released 2023-11-16 ### Enhancements * [`undocumented_unsafe_blocks`]: The config values [`accept-comment-above-statement`] and - [`accept-comment-above-attributes`] to `true` by default + [`accept-comment-above-attributes`] are now `true` by default [#11170](https://github.com/rust-lang/rust-clippy/pull/11170) * [`explicit_iter_loop`]: Added [`enforce-iter-loop-reborrow`] to disable reborrow linting by default [#11418](https://github.com/rust-lang/rust-clippy/pull/11418) @@ -5044,6 +5101,7 @@ Released 2018-09-13 [`duplicate_mod`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_mod [`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument [`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec +[`eager_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#eager_transmute [`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else [`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop [`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum @@ -5177,6 +5235,8 @@ Released 2018-09-13 [`items_after_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_test_module [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count +[`iter_filter_is_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_filter_is_ok +[`iter_filter_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_filter_is_some [`iter_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_kv_map [`iter_next_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_loop [`iter_next_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_next_slice @@ -5470,6 +5530,7 @@ Released 2018-09-13 [`reserve_after_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#reserve_after_initialization [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs [`result_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_expect_used +[`result_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_filter_map [`result_large_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err [`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn @@ -5582,6 +5643,7 @@ Released 2018-09-13 [`type_id_on_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_id_on_box [`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds [`unchecked_duration_subtraction`]: https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_subtraction +[`unconditional_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#unconditional_recursion [`undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks [`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc diff --git a/Cargo.toml b/Cargo.toml index f6084a46272..eda20531e40 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy" -version = "0.1.76" +version = "0.1.77" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 2bb89321cef..7c9a8eb1bfb 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -212,7 +212,7 @@ default configuration of Clippy. By default, any configuration will replace the * `doc-valid-idents = ["ClipPy"]` would replace the default list with `["ClipPy"]`. * `doc-valid-idents = ["ClipPy", ".."]` would append `ClipPy` to the default list. -**Default Value:** `["KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", "ECMAScript", "GPLv2", "GPLv3", "GitHub", "GitLab", "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", "WebAssembly", "NaN", "NaNs", "OAuth", "GraphQL", "OCaml", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenDNS", "WebGL", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase"]` +**Default Value:** `["KiB", "MiB", "GiB", "TiB", "PiB", "EiB", "DirectX", "ECMAScript", "GPLv2", "GPLv3", "GitHub", "GitLab", "IPv4", "IPv6", "ClojureScript", "CoffeeScript", "JavaScript", "PureScript", "TypeScript", "WebAssembly", "NaN", "NaNs", "OAuth", "GraphQL", "OCaml", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenDNS", "WebGL", "WebGL2", "WebGPU", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", "TeX", "LaTeX", "BibTeX", "BibLaTeX", "MinGW", "CamelCase"]` --- **Affected lints:** diff --git a/clippy_config/Cargo.toml b/clippy_config/Cargo.toml index 20f31320127..74b8e5eaa1c 100644 --- a/clippy_config/Cargo.toml +++ b/clippy_config/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_config" -version = "0.1.76" +version = "0.1.77" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 3cf4377002a..a4f368397ce 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -27,7 +27,7 @@ "OAuth", "GraphQL", "OCaml", "OpenGL", "OpenMP", "OpenSSH", "OpenSSL", "OpenStreetMap", "OpenDNS", - "WebGL", + "WebGL", "WebGL2", "WebGPU", "TensorFlow", "TrueType", "iOS", "macOS", "FreeBSD", @@ -640,7 +640,8 @@ fn read_inner(sess: &Session, path: &io::Result<(Option, Vec)>) } }, Err(error) => { - sess.dcx().err(format!("error finding Clippy's configuration file: {error}")); + sess.dcx() + .err(format!("error finding Clippy's configuration file: {error}")); }, } diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index 13e61e5a032..471ad73e207 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -41,6 +41,7 @@ macro_rules! msrv_aliases { 1,35,0 { OPTION_COPIED, RANGE_CONTAINS } 1,34,0 { TRY_FROM } 1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES } + 1,29,0 { ITER_FLATTEN } 1,28,0 { FROM_BOOL } 1,27,0 { ITERATOR_TRY_FOLD } 1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN } @@ -106,7 +107,8 @@ fn parse_attr(sess: &Session, attrs: &[Attribute]) -> Option { if let Some(msrv_attr) = msrv_attrs.next() { if let Some(duplicate) = msrv_attrs.last() { - sess.dcx().struct_span_err(duplicate.span, "`clippy::msrv` is defined multiple times") + sess.dcx() + .struct_span_err(duplicate.span, "`clippy::msrv` is defined multiple times") .span_note(msrv_attr.span, "first definition found here") .emit(); } @@ -116,7 +118,8 @@ fn parse_attr(sess: &Session, attrs: &[Attribute]) -> Option { return Some(version); } - sess.dcx().span_err(msrv_attr.span, format!("`{msrv}` is not a valid Rust version")); + sess.dcx() + .span_err(msrv_attr.span, format!("`{msrv}` is not a valid Rust version")); } else { sess.dcx().span_err(msrv_attr.span, "bad clippy attribute"); } diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index ad8b7ded46b..8cba35f3d87 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_lints" -version = "0.1.76" +version = "0.1.77" description = "A bunch of helpful lints to avoid common pitfalls in Rust" repository = "https://github.com/rust-lang/rust-clippy" readme = "README.md" diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index a15ec199a28..1e327f7a6df 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -1,7 +1,7 @@ -use clippy_utils::consts::{constant, Constant}; +use clippy_utils::consts::{constant_with_source, Constant, ConstantSource}; use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn}; -use rustc_hir::Expr; +use rustc_hir::{Expr, Item, ItemKind, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use rustc_span::sym; @@ -42,9 +42,18 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else { return; }; - let Some(Constant::Bool(val)) = constant(cx, cx.typeck_results(), condition) else { + let Some((Constant::Bool(val), source)) = constant_with_source(cx, cx.typeck_results(), condition) else { return; }; + if let ConstantSource::Constant = source + && let Some(node) = cx.tcx.hir().find_parent(e.hir_id) + && let Node::Item(Item { + kind: ItemKind::Const(..), + .. + }) = node + { + return; + } if val { span_lint_and_help( cx, diff --git a/clippy_lints/src/async_yields_async.rs b/clippy_lints/src/async_yields_async.rs index c965341d3fd..bb08ac7508b 100644 --- a/clippy_lints/src/async_yields_async.rs +++ b/clippy_lints/src/async_yields_async.rs @@ -2,9 +2,7 @@ use clippy_utils::source::snippet; use clippy_utils::ty::implements_trait; use rustc_errors::Applicability; -use rustc_hir::{ - Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, QPath, -}; +use rustc_hir::{Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 1220eb89013..eae9dfac064 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -369,6 +369,8 @@ crate::methods::ITERATOR_STEP_BY_ZERO_INFO, crate::methods::ITER_CLONED_COLLECT_INFO, crate::methods::ITER_COUNT_INFO, + crate::methods::ITER_FILTER_IS_OK_INFO, + crate::methods::ITER_FILTER_IS_SOME_INFO, crate::methods::ITER_KV_MAP_INFO, crate::methods::ITER_NEXT_SLICE_INFO, crate::methods::ITER_NTH_INFO, @@ -419,6 +421,7 @@ crate::methods::READ_LINE_WITHOUT_TRIM_INFO, crate::methods::REDUNDANT_AS_STR_INFO, crate::methods::REPEAT_ONCE_INFO, + crate::methods::RESULT_FILTER_MAP_INFO, crate::methods::RESULT_MAP_OR_INTO_OPTION_INFO, crate::methods::SEARCH_IS_SOME_INFO, crate::methods::SEEK_FROM_CURRENT_INFO, @@ -650,6 +653,7 @@ crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO, crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO, crate::transmute::CROSSPOINTER_TRANSMUTE_INFO, + crate::transmute::EAGER_TRANSMUTE_INFO, crate::transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS_INFO, crate::transmute::TRANSMUTE_BYTES_TO_STR_INFO, crate::transmute::TRANSMUTE_FLOAT_TO_INT_INFO, @@ -676,6 +680,7 @@ crate::types::REDUNDANT_ALLOCATION_INFO, crate::types::TYPE_COMPLEXITY_INFO, crate::types::VEC_BOX_INFO, + crate::unconditional_recursion::UNCONDITIONAL_RECURSION_INFO, crate::undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS_INFO, crate::undocumented_unsafe_blocks::UNNECESSARY_SAFETY_COMMENT_INFO, crate::unicode::INVISIBLE_CHARACTERS_INFO, diff --git a/clippy_lints/src/default_constructed_unit_structs.rs b/clippy_lints/src/default_constructed_unit_structs.rs index 9ce5acfbcc2..71e1a25c2bc 100644 --- a/clippy_lints/src/default_constructed_unit_structs.rs +++ b/clippy_lints/src/default_constructed_unit_structs.rs @@ -42,7 +42,7 @@ #[clippy::version = "1.71.0"] pub DEFAULT_CONSTRUCTED_UNIT_STRUCTS, complexity, - "unit structs can be contructed without calling `default`" + "unit structs can be constructed without calling `default`" } declare_lint_pass!(DefaultConstructedUnitStructs => [DEFAULT_CONSTRUCTED_UNIT_STRUCTS]); diff --git a/clippy_lints/src/format_push_string.rs b/clippy_lints/src/format_push_string.rs index 3901dd984f9..2b08437d827 100644 --- a/clippy_lints/src/format_push_string.rs +++ b/clippy_lints/src/format_push_string.rs @@ -57,7 +57,7 @@ fn is_format(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { Some(higher::IfLetOrMatch::Match(_, arms, MatchSource::Normal)) => { arms.iter().any(|arm| is_format(cx, arm.body)) }, - Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else)) => { + Some(higher::IfLetOrMatch::IfLet(_, _, then, r#else, _)) => { is_format(cx, then) || r#else.is_some_and(|e| is_format(cx, e)) }, _ => false, diff --git a/clippy_lints/src/implied_bounds_in_impls.rs b/clippy_lints/src/implied_bounds_in_impls.rs index 0f5a9ea5d84..fab8ffedb94 100644 --- a/clippy_lints/src/implied_bounds_in_impls.rs +++ b/clippy_lints/src/implied_bounds_in_impls.rs @@ -158,7 +158,7 @@ fn try_resolve_type<'tcx>( /// This function tries to, for all generic type parameters in a supertrait predicate `trait ...: /// GenericTrait`, check if the substituted type in the implied-by bound matches with what's -/// subtituted in the implied bound. +/// substituted in the implied bound. /// /// Consider this example. /// ```rust,ignore diff --git a/clippy_lints/src/indexing_slicing.rs b/clippy_lints/src/indexing_slicing.rs index 0ae03d101ab..391db0b0df7 100644 --- a/clippy_lints/src/indexing_slicing.rs +++ b/clippy_lints/src/indexing_slicing.rs @@ -170,7 +170,23 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { return; } // Index is a constant uint. - if constant(cx, cx.typeck_results(), index).is_some() { + if let Some(constant) = constant(cx, cx.typeck_results(), index) { + // only `usize` index is legal in rust array index + // leave other type to rustc + if let Constant::Int(off) = constant + && let ty::Uint(utype) = cx.typeck_results().expr_ty(index).kind() + && *utype == ty::UintTy::Usize + && let ty::Array(_, s) = ty.kind() + && let Some(size) = s.try_eval_target_usize(cx.tcx, cx.param_env) + { + // get constant offset and check whether it is in bounds + let off = usize::try_from(off).unwrap(); + let size = usize::try_from(size).unwrap(); + + if off >= size { + span_lint(cx, OUT_OF_BOUNDS_INDEXING, expr.span, "index is out of bounds"); + } + } // Let rustc's `const_err` lint handle constant `usize` indexing on arrays. return; } diff --git a/clippy_lints/src/ineffective_open_options.rs b/clippy_lints/src/ineffective_open_options.rs index 955f90d4262..af5b1f95739 100644 --- a/clippy_lints/src/ineffective_open_options.rs +++ b/clippy_lints/src/ineffective_open_options.rs @@ -16,7 +16,7 @@ /// /// ### Why is this bad? /// `.append(true)` already enables `write(true)`, making this one - /// superflous. + /// superfluous. /// /// ### Example /// ```no_run diff --git a/clippy_lints/src/iter_without_into_iter.rs b/clippy_lints/src/iter_without_into_iter.rs index 3a5756482a1..c9dc48668f2 100644 --- a/clippy_lints/src/iter_without_into_iter.rs +++ b/clippy_lints/src/iter_without_into_iter.rs @@ -49,7 +49,7 @@ /// } /// } /// ``` - #[clippy::version = "1.74.0"] + #[clippy::version = "1.75.0"] pub ITER_WITHOUT_INTO_ITER, pedantic, "implementing `iter(_mut)` without an associated `IntoIterator for (&|&mut) Type` impl" @@ -101,7 +101,7 @@ /// } /// } /// ``` - #[clippy::version = "1.74.0"] + #[clippy::version = "1.75.0"] pub INTO_ITER_WITHOUT_ITER, pedantic, "implementing `IntoIterator for (&|&mut) Type` without an inherent `iter(_mut)` method" diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 8c032b17023..c09d3d1862b 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -8,8 +8,8 @@ use rustc_hir::def_id::{DefId, DefIdSet}; use rustc_hir::{ AssocItemKind, BinOpKind, Expr, ExprKind, FnRetTy, GenericArg, GenericBound, ImplItem, ImplItemKind, - ImplicitSelfKind, Item, ItemKind, Mutability, Node, PatKind, PathSegment, PrimTy, QPath, TraitItemRef, - TyKind, TypeBindingKind, OpaqueTyOrigin, + ImplicitSelfKind, Item, ItemKind, Mutability, Node, OpaqueTyOrigin, PatKind, PathSegment, PrimTy, QPath, + TraitItemRef, TyKind, TypeBindingKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, AssocKind, FnSig, Ty}; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 7758d6a58e6..755a4ff525d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -22,6 +22,7 @@ // FIXME: switch to something more ergonomic here, once available. // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate pulldown_cmark; +extern crate rustc_abi; extern crate rustc_arena; extern crate rustc_ast; extern crate rustc_ast_pretty; @@ -327,6 +328,7 @@ mod transmute; mod tuple_array_conversions; mod types; +mod unconditional_recursion; mod undocumented_unsafe_blocks; mod unicode; mod uninhabited_references; @@ -1078,6 +1080,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(repeat_vec_with_capacity::RepeatVecWithCapacity)); store.register_late_pass(|_| Box::new(uninhabited_references::UninhabitedReferences)); store.register_late_pass(|_| Box::new(ineffective_open_options::IneffectiveOpenOptions)); + store.register_late_pass(|_| Box::new(unconditional_recursion::UnconditionalRecursion)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/loops/infinite_loop.rs b/clippy_lints/src/loops/infinite_loop.rs index 9b88dd76e5c..5e099f1e76f 100644 --- a/clippy_lints/src/loops/infinite_loop.rs +++ b/clippy_lints/src/loops/infinite_loop.rs @@ -49,7 +49,7 @@ pub(super) fn check<'tcx>( if let FnRetTy::DefaultReturn(ret_span) = parent_fn_ret { diag.span_suggestion( ret_span, - "if this is intentional, consider specifing `!` as function return", + "if this is intentional, consider specifying `!` as function return", " -> !", Applicability::MaybeIncorrect, ); diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index a726b116956..36de9021f49 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/clippy_lints/src/loops/manual_flatten.rs @@ -20,7 +20,7 @@ pub(super) fn check<'tcx>( span: Span, ) { let inner_expr = peel_blocks_with_stmt(body); - if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None }) + if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else: None, .. }) = higher::IfLet::hir(cx, inner_expr) // Ensure match_expr in `if let` statement is the same as the pat from the for-loop && let PatKind::Binding(_, pat_hir_id, _, _) = pat.kind diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 21b9efba54c..d070ee74985 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -14,7 +14,7 @@ use rustc_span::Symbol; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr) + if let Some(higher::WhileLet { if_then, let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) // check for `Some(..)` pattern && let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind && is_res_lang_ctor(cx, cx.qpath_res(pat_path, let_pat.hir_id), LangItem::OptionSome) diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs index 9ba1d3afcbe..4cd5f3b81e5 100644 --- a/clippy_lints/src/manual_async_fn.rs +++ b/clippy_lints/src/manual_async_fn.rs @@ -3,9 +3,9 @@ use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{ - Block, Body, Closure, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, FnDecl, FnRetTy, - GenericArg, GenericBound, ImplItem, Item, ItemKind, LifetimeName, Node, Term, TraitRef, Ty, TyKind, - TypeBindingKind, ClosureKind, + Block, Body, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, FnDecl, + FnRetTy, GenericArg, GenericBound, ImplItem, Item, ItemKind, LifetimeName, Node, Term, TraitRef, Ty, TyKind, + TypeBindingKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -172,23 +172,13 @@ fn captures_all_lifetimes(inputs: &[Ty<'_>], output_lifetimes: &[LifetimeName]) .all(|in_lt| output_lifetimes.iter().any(|out_lt| in_lt == out_lt)) } -fn desugared_async_block<'tcx>( - cx: &LateContext<'tcx>, - block: &'tcx Block<'tcx>, -) -> Option<&'tcx Body<'tcx>> { +fn desugared_async_block<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) -> Option<&'tcx Body<'tcx>> { if let Some(Expr { - kind: - ExprKind::Closure(&Closure { - kind: - ClosureKind::Coroutine(CoroutineKind::Desugared( - CoroutineDesugaring::Async, - CoroutineSource::Block, - )), - body, - .. - }), + kind: ExprKind::Closure(&Closure { kind, body, .. }), .. }) = block.expr + && let ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Block)) = + kind { return Some(cx.tcx.hir().body(body)); } diff --git a/clippy_lints/src/manual_hash_one.rs b/clippy_lints/src/manual_hash_one.rs index 252b3a83a18..73687fbbe54 100644 --- a/clippy_lints/src/manual_hash_one.rs +++ b/clippy_lints/src/manual_hash_one.rs @@ -40,7 +40,7 @@ /// /// let hash = s.hash_one(&value); /// ``` - #[clippy::version = "1.74.0"] + #[clippy::version = "1.75.0"] pub MANUAL_HASH_ONE, complexity, "manual implementations of `BuildHasher::hash_one`" diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 92dc4d57ab1..fdf8fa4e277 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -61,7 +61,7 @@ pub(crate) fn check_manual_let_else(&mut self, cx: &LateContext<'tcx>, stmt: &'t && let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init) { match if_let_or_match { - IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else) => { + IfLetOrMatch::IfLet(if_let_expr, let_pat, if_then, if_else, ..) => { if let Some(ident_map) = expr_simple_identity_map(local.pat, let_pat, if_then) && let Some(if_else) = if_else && is_never_expr(cx, if_else).is_some() diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index 48fc5746b3c..91e6ca7fa8b 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -41,7 +41,7 @@ fn check_arm<'tcx>( let inner_expr = peel_blocks_with_stmt(outer_then_body); if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) && let Some((inner_scrutinee, inner_then_pat, inner_else_body)) = match inner { - IfLetOrMatch::IfLet(scrutinee, pat, _, els) => Some((scrutinee, pat, els)), + IfLetOrMatch::IfLet(scrutinee, pat, _, els, _) => Some((scrutinee, pat, els)), IfLetOrMatch::Match(scrutinee, arms, ..) => if arms.len() == 2 && arms.iter().all(|a| a.guard.is_none()) // if there are more than two arms, collapsing would be non-trivial // one of the arms must be "wild-like" @@ -75,7 +75,7 @@ fn check_arm<'tcx>( ) // ...or anywhere in the inner expression && match inner { - IfLetOrMatch::IfLet(_, _, body, els) => { + IfLetOrMatch::IfLet(_, _, body, els, _) => { !is_local_used(cx, body, binding_id) && els.map_or(true, |e| !is_local_used(cx, e, binding_id)) }, IfLetOrMatch::Match(_, arms, ..) => !arms.iter().any(|arm| is_local_used(cx, arm, binding_id)), diff --git a/clippy_lints/src/matches/manual_utils.rs b/clippy_lints/src/matches/manual_utils.rs index 0627e458dfe..152aba99ce9 100644 --- a/clippy_lints/src/matches/manual_utils.rs +++ b/clippy_lints/src/matches/manual_utils.rs @@ -63,9 +63,7 @@ pub(super) fn check_with<'tcx, F>( return None; } - let Some(some_expr) = get_some_expr_fn(cx, some_pat, some_expr, expr_ctxt) else { - return None; - }; + let some_expr = get_some_expr_fn(cx, some_pat, some_expr, expr_ctxt)?; // These two lints will go back and forth with each other. if cx.typeck_results().expr_ty(some_expr.expr) == cx.tcx.types.unit diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 4c7568f39b4..50494f4819f 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -469,15 +469,15 @@ declare_clippy_lint! { /// ### What it does /// Lint for redundant pattern matching over `Result`, `Option`, - /// `std::task::Poll` or `std::net::IpAddr` + /// `std::task::Poll`, `std::net::IpAddr` or `bool`s /// /// ### Why is this bad? /// It's more concise and clear to just use the proper - /// utility function + /// utility function or using the condition directly /// /// ### Known problems - /// This will change the drop order for the matched type. Both `if let` and - /// `while let` will drop the value at the end of the block, both `if` and `while` will drop the + /// For suggestions involving bindings in patterns, this will change the drop order for the matched type. + /// Both `if let` and `while let` will drop the value at the end of the block, both `if` and `while` will drop the /// value before entering the block. For most types this change will not matter, but for a few /// types this will not be an acceptable change (e.g. locks). See the /// [reference](https://doc.rust-lang.org/reference/destructors.html#drop-scopes) for more about @@ -499,6 +499,10 @@ /// Ok(_) => true, /// Err(_) => false, /// }; + /// + /// let cond = true; + /// if let true = cond {} + /// matches!(cond, true); /// ``` /// /// The more idiomatic use would be: @@ -515,6 +519,10 @@ /// if IpAddr::V4(Ipv4Addr::LOCALHOST).is_ipv4() {} /// if IpAddr::V6(Ipv6Addr::LOCALHOST).is_ipv6() {} /// Ok::(42).is_ok(); + /// + /// let cond = true; + /// if cond {} + /// cond; /// ``` #[clippy::version = "1.31.0"] pub REDUNDANT_PATTERN_MATCHING, @@ -1019,8 +1027,11 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let from_expansion = expr.span.from_expansion(); if let ExprKind::Match(ex, arms, source) = expr.kind { - if is_direct_expn_of(expr.span, "matches").is_some() { + if is_direct_expn_of(expr.span, "matches").is_some() + && let [arm, _] = arms + { redundant_pattern_match::check_match(cx, expr, ex, arms); + redundant_pattern_match::check_matches_true(cx, expr, arm, ex); } if source == MatchSource::Normal && !is_span_match(cx, expr.span) { @@ -1104,6 +1115,7 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if_let.let_pat, if_let.let_expr, if_let.if_else.is_some(), + if_let.let_span, ); needless_match::check_if_let(cx, expr, &if_let); } diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 2582f7edcf6..a4acdfb1db4 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -1,7 +1,7 @@ use super::REDUNDANT_PATTERN_MATCHING; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, walk_span_to_context}; -use clippy_utils::sugg::Sugg; +use clippy_utils::sugg::{make_unop, Sugg}; use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop}; use clippy_utils::visitors::{any_temporaries_need_ordered_drop, for_each_expr}; use clippy_utils::{higher, is_expn_of, is_trait_method}; @@ -12,13 +12,20 @@ use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_middle::ty::{self, GenericArgKind, Ty}; -use rustc_span::{sym, Symbol}; +use rustc_span::{sym, Span, Symbol}; use std::fmt::Write; use std::ops::ControlFlow; pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if let Some(higher::WhileLet { let_pat, let_expr, .. }) = higher::WhileLet::hir(expr) { - find_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false); + if let Some(higher::WhileLet { + let_pat, + let_expr, + let_span, + .. + }) = higher::WhileLet::hir(expr) + { + find_method_sugg_for_if_let(cx, expr, let_pat, let_expr, "while", false); + find_if_let_true(cx, let_pat, let_expr, let_span); } } @@ -28,8 +35,73 @@ pub(super) fn check_if_let<'tcx>( pat: &'tcx Pat<'_>, scrutinee: &'tcx Expr<'_>, has_else: bool, + let_span: Span, ) { - find_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else); + find_if_let_true(cx, pat, scrutinee, let_span); + find_method_sugg_for_if_let(cx, expr, pat, scrutinee, "if", has_else); +} + +/// Looks for: +/// * `matches!(expr, true)` +pub fn check_matches_true<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'_>, + arm: &'tcx Arm<'_>, + scrutinee: &'tcx Expr<'_>, +) { + find_match_true( + cx, + arm.pat, + scrutinee, + expr.span.source_callsite(), + "using `matches!` to pattern match a bool", + ); +} + +/// Looks for any of: +/// * `if let true = ...` +/// * `if let false = ...` +/// * `while let true = ...` +fn find_if_let_true<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, scrutinee: &'tcx Expr<'_>, let_span: Span) { + find_match_true(cx, pat, scrutinee, let_span, "using `if let` to pattern match a bool"); +} + +/// Common logic between `find_if_let_true` and `check_matches_true` +fn find_match_true<'tcx>( + cx: &LateContext<'tcx>, + pat: &'tcx Pat<'_>, + scrutinee: &'tcx Expr<'_>, + span: Span, + message: &str, +) { + if let PatKind::Lit(lit) = pat.kind + && let ExprKind::Lit(lit) = lit.kind + && let LitKind::Bool(pat_is_true) = lit.node + { + let mut applicability = Applicability::MachineApplicable; + + let mut sugg = Sugg::hir_with_context( + cx, + scrutinee, + scrutinee.span.source_callsite().ctxt(), + "..", + &mut applicability, + ); + + if !pat_is_true { + sugg = make_unop("!", sugg); + } + + span_lint_and_sugg( + cx, + REDUNDANT_PATTERN_MATCHING, + span, + message, + "consider using the condition directly", + sugg.to_string(), + applicability, + ); + } } // Extract the generic arguments out of a type @@ -56,9 +128,7 @@ fn find_method_and_type<'tcx>( if is_wildcard || is_rest { let res = cx.typeck_results().qpath_res(qpath, check_pat.hir_id); - let Some(id) = res.opt_def_id().map(|ctor_id| cx.tcx.parent(ctor_id)) else { - return None; - }; + let id = res.opt_def_id().map(|ctor_id| cx.tcx.parent(ctor_id))?; let lang_items = cx.tcx.lang_items(); if Some(id) == lang_items.result_ok_variant() { Some(("is_ok()", try_get_generic_ty(op_ty, 0).unwrap_or(op_ty))) @@ -100,7 +170,7 @@ fn find_method_and_type<'tcx>( } } -fn find_sugg_for_if_let<'tcx>( +fn find_method_sugg_for_if_let<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, let_pat: &Pat<'_>, @@ -341,31 +411,25 @@ fn get_good_method<'tcx>( path_left: &QPath<'_>, ) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> { if let Some(name) = get_ident(path_left) { - return match name.as_str() { - "Ok" => { - find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultOk), "is_ok()", "is_err()") - }, - "Err" => { - find_good_method_for_matches_macro(cx, arms, path_left, Item::Lang(ResultErr), "is_err()", "is_ok()") - }, - "Some" => find_good_method_for_matches_macro( - cx, - arms, - path_left, - Item::Lang(OptionSome), - "is_some()", - "is_none()", - ), - "None" => find_good_method_for_matches_macro( - cx, - arms, - path_left, - Item::Lang(OptionNone), - "is_none()", - "is_some()", - ), - _ => None, + let (expected_item_left, should_be_left, should_be_right) = match name.as_str() { + "Ok" => (Item::Lang(ResultOk), "is_ok()", "is_err()"), + "Err" => (Item::Lang(ResultErr), "is_err()", "is_ok()"), + "Some" => (Item::Lang(OptionSome), "is_some()", "is_none()"), + "None" => (Item::Lang(OptionNone), "is_none()", "is_some()"), + "Ready" => (Item::Lang(PollReady), "is_ready()", "is_pending()"), + "Pending" => (Item::Lang(PollPending), "is_pending()", "is_ready()"), + "V4" => (Item::Diag(sym::IpAddr, sym!(V4)), "is_ipv4()", "is_ipv6()"), + "V6" => (Item::Diag(sym::IpAddr, sym!(V6)), "is_ipv6()", "is_ipv4()"), + _ => return None, }; + return find_good_method_for_matches_macro( + cx, + arms, + path_left, + expected_item_left, + should_be_left, + should_be_right, + ); } None } diff --git a/clippy_lints/src/methods/filter_map.rs b/clippy_lints/src/methods/filter_map.rs index 844ab40cab1..7cfd3d346b6 100644 --- a/clippy_lints/src/methods/filter_map.rs +++ b/clippy_lints/src/methods/filter_map.rs @@ -14,7 +14,7 @@ use rustc_span::Span; use std::borrow::Cow; -use super::{MANUAL_FILTER_MAP, MANUAL_FIND_MAP, OPTION_FILTER_MAP}; +use super::{MANUAL_FILTER_MAP, MANUAL_FIND_MAP, OPTION_FILTER_MAP, RESULT_FILTER_MAP}; fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> bool { match &expr.kind { @@ -22,6 +22,7 @@ fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> hir::ExprKind::Path(QPath::Resolved(_, segments)) => { segments.segments.last().unwrap().ident.name == method_name }, + hir::ExprKind::MethodCall(segment, _, _, _) => segment.ident.name == method_name, hir::ExprKind::Closure(&hir::Closure { body, .. }) => { let body = cx.tcx.hir().body(body); let closure_expr = peel_blocks(body.value); @@ -46,6 +47,9 @@ fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> fn is_option_filter_map(cx: &LateContext<'_>, filter_arg: &hir::Expr<'_>, map_arg: &hir::Expr<'_>) -> bool { is_method(cx, map_arg, sym::unwrap) && is_method(cx, filter_arg, sym!(is_some)) } +fn is_ok_filter_map(cx: &LateContext<'_>, filter_arg: &hir::Expr<'_>, map_arg: &hir::Expr<'_>) -> bool { + is_method(cx, map_arg, sym::unwrap) && is_method(cx, filter_arg, sym!(is_ok)) +} #[derive(Debug, Copy, Clone)] enum OffendingFilterExpr<'tcx> { @@ -186,7 +190,7 @@ pub fn check_map_call( match higher::IfLetOrMatch::parse(cx, map_body.value) { // For `if let` we want to check that the variant matching arm references the local created by // its pattern - Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_))) + Some(higher::IfLetOrMatch::IfLet(sc, pat, then, Some(else_), ..)) if let Some((ident, span)) = expr_uses_local(pat, then) => { (sc, else_, ident, span) @@ -273,6 +277,18 @@ fn is_filter_some_map_unwrap( (iterator || option) && is_option_filter_map(cx, filter_arg, map_arg) } +/// is `filter(|x| x.is_ok()).map(|x| x.unwrap())` +fn is_filter_ok_map_unwrap( + cx: &LateContext<'_>, + expr: &hir::Expr<'_>, + filter_arg: &hir::Expr<'_>, + map_arg: &hir::Expr<'_>, +) -> bool { + // result has no filter, so we only check for iterators + let iterator = is_trait_method(cx, expr, sym::Iterator); + iterator && is_ok_filter_map(cx, filter_arg, map_arg) +} + /// lint use of `filter().map()` or `find().map()` for `Iterators` #[allow(clippy::too_many_arguments)] pub(super) fn check( @@ -300,30 +316,21 @@ pub(super) fn check( return; } - if is_trait_method(cx, map_recv, sym::Iterator) + if is_filter_ok_map_unwrap(cx, expr, filter_arg, map_arg) { + span_lint_and_sugg( + cx, + RESULT_FILTER_MAP, + filter_span.with_hi(expr.span.hi()), + "`filter` for `Ok` followed by `unwrap`", + "consider using `flatten` instead", + reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, map_span)).into_owned(), + Applicability::MachineApplicable, + ); - // filter(|x| ...is_some())... - && let ExprKind::Closure(&Closure { body: filter_body_id, .. }) = filter_arg.kind - && let filter_body = cx.tcx.hir().body(filter_body_id) - && let [filter_param] = filter_body.params - // optional ref pattern: `filter(|&x| ..)` - && let (filter_pat, is_filter_param_ref) = if let PatKind::Ref(ref_pat, _) = filter_param.pat.kind { - (ref_pat, true) - } else { - (filter_param.pat, false) - } + return; + } - && let PatKind::Binding(_, filter_param_id, _, None) = filter_pat.kind - && let Some(mut offending_expr) = OffendingFilterExpr::hir(cx, filter_body.value, filter_param_id) - - && let ExprKind::Closure(&Closure { body: map_body_id, .. }) = map_arg.kind - && let map_body = cx.tcx.hir().body(map_body_id) - && let [map_param] = map_body.params - && let PatKind::Binding(_, map_param_id, map_param_ident, None) = map_param.pat.kind - - && let Some(check_result) = - offending_expr.check_map_call(cx, map_body, map_param_id, filter_param_id, is_filter_param_ref) - { + if let Some((map_param_ident, check_result)) = is_find_or_filter(cx, map_recv, filter_arg, map_arg) { let span = filter_span.with_hi(expr.span.hi()); let (filter_name, lint) = if is_find { ("find", MANUAL_FIND_MAP) @@ -395,6 +402,40 @@ pub(super) fn check( } } +fn is_find_or_filter<'a>( + cx: &LateContext<'a>, + map_recv: &hir::Expr<'_>, + filter_arg: &hir::Expr<'_>, + map_arg: &hir::Expr<'_>, +) -> Option<(Ident, CheckResult<'a>)> { + if is_trait_method(cx, map_recv, sym::Iterator) + // filter(|x| ...is_some())... + && let ExprKind::Closure(&Closure { body: filter_body_id, .. }) = filter_arg.kind + && let filter_body = cx.tcx.hir().body(filter_body_id) + && let [filter_param] = filter_body.params + // optional ref pattern: `filter(|&x| ..)` + && let (filter_pat, is_filter_param_ref) = if let PatKind::Ref(ref_pat, _) = filter_param.pat.kind { + (ref_pat, true) + } else { + (filter_param.pat, false) + } + + && let PatKind::Binding(_, filter_param_id, _, None) = filter_pat.kind + && let Some(mut offending_expr) = OffendingFilterExpr::hir(cx, filter_body.value, filter_param_id) + + && let ExprKind::Closure(&Closure { body: map_body_id, .. }) = map_arg.kind + && let map_body = cx.tcx.hir().body(map_body_id) + && let [map_param] = map_body.params + && let PatKind::Binding(_, map_param_id, map_param_ident, None) = map_param.pat.kind + + && let Some(check_result) = + offending_expr.check_map_call(cx, map_body, map_param_id, filter_param_id, is_filter_param_ref) + { + return Some((map_param_ident, check_result)); + } + None +} + fn acceptable_methods(method: &PathSegment<'_>) -> bool { let methods: [Symbol; 8] = [ sym::clone, diff --git a/clippy_lints/src/methods/iter_filter.rs b/clippy_lints/src/methods/iter_filter.rs new file mode 100644 index 00000000000..ade8e3155fa --- /dev/null +++ b/clippy_lints/src/methods/iter_filter.rs @@ -0,0 +1,87 @@ +use rustc_lint::{LateContext, LintContext}; + +use super::{ITER_FILTER_IS_OK, ITER_FILTER_IS_SOME}; + +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::{indent_of, reindent_multiline}; +use clippy_utils::{is_trait_method, peel_blocks, span_contains_comment}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::QPath; +use rustc_span::symbol::{sym, Symbol}; +use rustc_span::Span; +use std::borrow::Cow; + +fn is_method(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_name: Symbol) -> bool { + match &expr.kind { + hir::ExprKind::Path(QPath::TypeRelative(_, mname)) => mname.ident.name == method_name, + hir::ExprKind::Path(QPath::Resolved(_, segments)) => { + segments.segments.last().unwrap().ident.name == method_name + }, + hir::ExprKind::MethodCall(segment, _, _, _) => segment.ident.name == method_name, + hir::ExprKind::Closure(&hir::Closure { body, .. }) => { + let body = cx.tcx.hir().body(body); + let closure_expr = peel_blocks(body.value); + let arg_id = body.params[0].pat.hir_id; + match closure_expr.kind { + hir::ExprKind::MethodCall(hir::PathSegment { ident, .. }, receiver, ..) => { + if ident.name == method_name + && let hir::ExprKind::Path(path) = &receiver.kind + && let Res::Local(ref local) = cx.qpath_res(path, receiver.hir_id) + { + return arg_id == *local; + } + false + }, + _ => false, + } + }, + _ => false, + } +} + +fn parent_is_map(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + if let hir::Node::Expr(parent_expr) = cx.tcx.hir().get_parent(expr.hir_id) { + is_method(cx, parent_expr, rustc_span::sym::map) + } else { + false + } +} + +#[allow(clippy::too_many_arguments)] +pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir::Expr<'_>, filter_span: Span) { + let is_iterator = is_trait_method(cx, expr, sym::Iterator); + let parent_is_not_map = !parent_is_map(cx, expr); + + if is_iterator + && parent_is_not_map + && is_method(cx, filter_arg, sym!(is_some)) + && !span_contains_comment(cx.sess().source_map(), filter_span.with_hi(expr.span.hi())) + { + span_lint_and_sugg( + cx, + ITER_FILTER_IS_SOME, + filter_span.with_hi(expr.span.hi()), + "`filter` for `is_some` on iterator over `Option`", + "consider using `flatten` instead", + reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, filter_span)).into_owned(), + Applicability::HasPlaceholders, + ); + } + if is_iterator + && parent_is_not_map + && is_method(cx, filter_arg, sym!(is_ok)) + && !span_contains_comment(cx.sess().source_map(), filter_span.with_hi(expr.span.hi())) + { + span_lint_and_sugg( + cx, + ITER_FILTER_IS_OK, + filter_span.with_hi(expr.span.hi()), + "`filter` for `is_ok` on iterator over `Result`s", + "consider using `flatten` instead", + reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, filter_span)).into_owned(), + Applicability::HasPlaceholders, + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index b4f60ffadd7..25b1ea526e2 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -38,6 +38,7 @@ mod is_digit_ascii_radix; mod iter_cloned_collect; mod iter_count; +mod iter_filter; mod iter_kv_map; mod iter_next_slice; mod iter_nth; @@ -1175,7 +1176,8 @@ declare_clippy_lint! { /// ### What it does - /// Checks for indirect collection of populated `Option` + /// Checks for iterators of `Option`s using `.filter(Option::is_some).map(Option::unwrap)` that may + /// be replaced with a `.flatten()` call. /// /// ### Why is this bad? /// `Option` is like a collection of 0-1 things, so `flatten` @@ -3752,6 +3754,81 @@ "using `Option.map_or(Err(_), Ok)`, which is more succinctly expressed as `Option.ok_or(_)`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for iterators of `Result`s using `.filter(Result::is_ok).map(Result::unwrap)` that may + /// be replaced with a `.flatten()` call. + /// + /// ### Why is this bad? + /// `Result` implements `IntoIterator`. This means that `Result` can be flattened + /// automatically without suspicious-looking `unwrap` calls. + /// + /// ### Example + /// ```no_run + /// let _ = std::iter::empty::>().filter(Result::is_ok).map(Result::unwrap); + /// ``` + /// Use instead: + /// ```no_run + /// let _ = std::iter::empty::>().flatten(); + /// ``` + #[clippy::version = "1.76.0"] + pub RESULT_FILTER_MAP, + complexity, + "filtering `Result` for `Ok` then force-unwrapping, which can be one type-safe operation" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `.filter(Option::is_some)` that may be replaced with a `.flatten()` call. + /// This lint will require additional changes to the follow-up calls as it appects the type. + /// + /// ### Why is this bad? + /// This pattern is often followed by manual unwrapping of the `Option`. The simplification + /// results in more readable and succint code without the need for manual unwrapping. + /// + /// ### Example + /// ```no_run + /// // example code where clippy issues a warning + /// vec![Some(1)].into_iter().filter(Option::is_some); + /// + /// ``` + /// Use instead: + /// ```no_run + /// // example code which does not raise clippy warning + /// vec![Some(1)].into_iter().flatten(); + /// ``` + #[clippy::version = "1.76.0"] + pub ITER_FILTER_IS_SOME, + pedantic, + "filtering an iterator over `Option`s for `Some` can be achieved with `flatten`" +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `.filter(Result::is_ok)` that may be replaced with a `.flatten()` call. + /// This lint will require additional changes to the follow-up calls as it appects the type. + /// + /// ### Why is this bad? + /// This pattern is often followed by manual unwrapping of `Result`. The simplification + /// results in more readable and succint code without the need for manual unwrapping. + /// + /// ### Example + /// ```no_run + /// // example code where clippy issues a warning + /// vec![Ok::(1)].into_iter().filter(Result::is_ok); + /// + /// ``` + /// Use instead: + /// ```no_run + /// // example code which does not raise clippy warning + /// vec![Ok::(1)].into_iter().flatten(); + /// ``` + #[clippy::version = "1.76.0"] + pub ITER_FILTER_IS_OK, + pedantic, + "filtering an iterator over `Result`s for `Ok` can be achieved with `flatten`" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3903,6 +3980,9 @@ pub fn new( UNNECESSARY_FALLIBLE_CONVERSIONS, JOIN_ABSOLUTE_PATHS, OPTION_MAP_OR_ERR_OK, + RESULT_FILTER_MAP, + ITER_FILTER_IS_SOME, + ITER_FILTER_IS_OK, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4232,7 +4312,24 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { string_extend_chars::check(cx, expr, recv, arg); extend_with_drain::check(cx, expr, recv, arg); }, - (name @ ("filter" | "find"), [arg]) => { + ("filter", [arg]) => { + if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { + // if `arg` has side-effect, the semantic will change + iter_overeager_cloned::check( + cx, + expr, + recv, + recv2, + iter_overeager_cloned::Op::FixClosure(name, arg), + false, + ); + } + if self.msrv.meets(msrvs::ITER_FLATTEN) { + // use the sourcemap to get the span of the closure + iter_filter::check(cx, expr, arg, span); + } + }, + ("find", [arg]) => { if let Some(("cloned", recv2, [], _span2, _)) = method_call(recv) { // if `arg` has side-effect, the semantic will change iter_overeager_cloned::check( diff --git a/clippy_lints/src/methods/unnecessary_to_owned.rs b/clippy_lints/src/methods/unnecessary_to_owned.rs index c4775b6bd04..637368e9361 100644 --- a/clippy_lints/src/methods/unnecessary_to_owned.rs +++ b/clippy_lints/src/methods/unnecessary_to_owned.rs @@ -7,6 +7,7 @@ use clippy_utils::visitors::find_all_ret_expressions; use clippy_utils::{fn_def_id, get_parent_expr, is_diag_item_method, is_diag_trait_item, return_ty}; use rustc_errors::Applicability; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::{BorrowKind, Expr, ExprKind, ItemKind, Node}; use rustc_hir_typeck::{FnCtxt, Inherited}; @@ -37,6 +38,9 @@ pub fn check<'tcx>( if is_cloned_or_copied(cx, method_name, method_def_id) { unnecessary_iter_cloned::check(cx, expr, method_name, receiver); } else if is_to_owned_like(cx, expr, method_name, method_def_id) { + if check_split_call_arg(cx, expr, method_name, receiver) { + return; + } // At this point, we know the call is of a `to_owned`-like function. The functions // `check_addr_of_expr` and `check_call_arg` determine whether the call is unnecessary // based on its context, that is, whether it is a referent in an `AddrOf` expression, an @@ -233,6 +237,58 @@ fn check_into_iter_call_arg( false } +/// Checks whether `expr` is an argument in an `into_iter` call and, if so, determines whether its +/// call of a `to_owned`-like function is unnecessary. +fn check_split_call_arg(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, receiver: &Expr<'_>) -> bool { + if let Some(parent) = get_parent_expr(cx, expr) + && let Some((fn_name, argument_expr)) = get_fn_name_and_arg(cx, parent) + && fn_name.as_str() == "split" + && let Some(receiver_snippet) = snippet_opt(cx, receiver.span) + && let Some(arg_snippet) = snippet_opt(cx, argument_expr.span) + { + // The next suggestion may be incorrect because the removal of the `to_owned`-like + // function could cause the iterator to hold a reference to a resource that is used + // mutably. See https://github.com/rust-lang/rust-clippy/issues/8148. + span_lint_and_sugg( + cx, + UNNECESSARY_TO_OWNED, + parent.span, + &format!("unnecessary use of `{method_name}`"), + "use", + format!("{receiver_snippet}.split({arg_snippet})"), + Applicability::MaybeIncorrect, + ); + return true; + } + false +} + +fn get_fn_name_and_arg<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(Symbol, Expr<'tcx>)> { + match &expr.kind { + ExprKind::MethodCall(path, _, [arg_expr], ..) => Some((path.ident.name, *arg_expr)), + ExprKind::Call( + Expr { + kind: ExprKind::Path(qpath), + hir_id: path_hir_id, + .. + }, + [arg_expr], + ) => { + // Only return Fn-like DefIds, not the DefIds of statics/consts/etc that contain or + // deref to fn pointers, dyn Fn, impl Fn - #8850 + if let Res::Def(DefKind::Fn | DefKind::Ctor(..) | DefKind::AssocFn, def_id) = + cx.typeck_results().qpath_res(qpath, *path_hir_id) + && let Some(fn_name) = cx.tcx.opt_item_name(def_id) + { + Some((fn_name, *arg_expr)) + } else { + None + } + }, + _ => None, + } +} + /// Checks whether `expr` is an argument in a function call and, if so, determines whether its call /// of a `to_owned`-like function is unnecessary. fn check_other_call_arg<'tcx>( diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 218ca5e80f3..bf27adb45af 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -340,6 +340,12 @@ fn check_comparison<'a, 'tcx>( } if l_ty.is_bool() && r_ty.is_bool() { let mut applicability = Applicability::MachineApplicable; + // Eliminate parentheses in `e` by using the lo pos of lhs and hi pos of rhs, + // calling `source_callsite` make sure macros are handled correctly, see issue #9907 + let binop_span = left_side + .span + .source_callsite() + .with_hi(right_side.span.source_callsite().hi()); if op.node == BinOpKind::Eq { let expression_info = one_side_is_unary_not(left_side, right_side); @@ -347,13 +353,23 @@ fn check_comparison<'a, 'tcx>( span_lint_and_sugg( cx, BOOL_COMPARISON, - e.span, + binop_span, "this comparison might be written more concisely", "try simplifying it as shown", format!( "{} != {}", - snippet_with_applicability(cx, expression_info.left_span, "..", &mut applicability), - snippet_with_applicability(cx, expression_info.right_span, "..", &mut applicability) + snippet_with_applicability( + cx, + expression_info.left_span.source_callsite(), + "..", + &mut applicability + ), + snippet_with_applicability( + cx, + expression_info.right_span.source_callsite(), + "..", + &mut applicability + ) ), applicability, ); @@ -362,16 +378,16 @@ fn check_comparison<'a, 'tcx>( match (fetch_bool_expr(left_side), fetch_bool_expr(right_side)) { (Some(true), None) => left_true.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, right_side, applicability, m, h); + suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h); }), (None, Some(true)) => right_true.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, left_side, applicability, m, h); + suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h); }), (Some(false), None) => left_false.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, right_side, applicability, m, h); + suggest_bool_comparison(cx, binop_span, right_side, applicability, m, h); }), (None, Some(false)) => right_false.map_or((), |(h, m)| { - suggest_bool_comparison(cx, e, left_side, applicability, m, h); + suggest_bool_comparison(cx, binop_span, left_side, applicability, m, h); }), (None, None) => no_literal.map_or((), |(h, m)| { let left_side = Sugg::hir_with_applicability(cx, left_side, "..", &mut applicability); @@ -379,7 +395,7 @@ fn check_comparison<'a, 'tcx>( span_lint_and_sugg( cx, BOOL_COMPARISON, - e.span, + binop_span, m, "try simplifying it as shown", h(left_side, right_side).to_string(), @@ -394,17 +410,17 @@ fn check_comparison<'a, 'tcx>( fn suggest_bool_comparison<'a, 'tcx>( cx: &LateContext<'tcx>, - e: &'tcx Expr<'_>, + span: Span, expr: &Expr<'_>, mut app: Applicability, message: &str, conv_hint: impl FnOnce(Sugg<'a>) -> Sugg<'a>, ) { - let hint = Sugg::hir_with_context(cx, expr, e.span.ctxt(), "..", &mut app); + let hint = Sugg::hir_with_context(cx, expr, span.ctxt(), "..", &mut app); span_lint_and_sugg( cx, BOOL_COMPARISON, - e.span, + span, message, "try simplifying it as shown", conv_hint(hint).to_string(), diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 89e4e3c740d..556c493d36c 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -238,6 +238,7 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> let_expr, if_then, if_else: Some(if_else), + .. }) = higher::IfLet::hir(cx, expr) && !cx.typeck_results().expr_ty(expr).is_unit() && !is_else_clause(cx.tcx, expr) diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index fc5835408a9..509d9483e1d 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -8,7 +8,7 @@ use clippy_utils::{ eq_expr_value, get_parent_node, higher, in_constant, is_else_clause, is_lint_allowed, is_path_lang_item, is_res_lang_ctor, pat_and_expr_can_be_question_mark, path_to_local, path_to_local_id, peel_blocks, - peel_blocks_with_stmt, + peel_blocks_with_stmt, span_contains_comment, }; use rustc_errors::Applicability; use rustc_hir::def::Res; @@ -96,6 +96,24 @@ enum IfBlockType<'hir> { ), } +fn find_let_else_ret_expression<'hir>(block: &'hir Block<'hir>) -> Option<&'hir Expr<'hir>> { + if let Block { + stmts: &[], + expr: Some(els), + .. + } = block + { + Some(els) + } else if let [stmt] = block.stmts + && let StmtKind::Semi(expr) = stmt.kind + && let ExprKind::Ret(..) = expr.kind + { + Some(expr) + } else { + None + } +} + fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) { if let StmtKind::Local(Local { pat, @@ -103,12 +121,9 @@ fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) { els: Some(els), .. }) = stmt.kind - && let Block { - stmts: &[], - expr: Some(els), - .. - } = els - && let Some(inner_pat) = pat_and_expr_can_be_question_mark(cx, pat, els) + && let Some(ret) = find_let_else_ret_expression(els) + && let Some(inner_pat) = pat_and_expr_can_be_question_mark(cx, pat, ret) + && !span_contains_comment(cx.tcx.sess.source_map(), els.span) { let mut applicability = Applicability::MaybeIncorrect; let init_expr_str = snippet_with_applicability(cx, init_expr.span, "..", &mut applicability); @@ -256,6 +271,7 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(&self, cx: &LateContext<'tcx> let_expr, if_then, if_else, + .. }) = higher::IfLet::hir(cx, expr) && !is_else_clause(cx.tcx, expr) && let PatKind::TupleStruct(ref path1, [field], ddpos) = let_pat.kind diff --git a/clippy_lints/src/redundant_async_block.rs b/clippy_lints/src/redundant_async_block.rs index b50141f048c..0ed957f1f2f 100644 --- a/clippy_lints/src/redundant_async_block.rs +++ b/clippy_lints/src/redundant_async_block.rs @@ -3,9 +3,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::peel_blocks; use clippy_utils::source::{snippet, walk_span_to_context}; +use clippy_utils::ty::implements_trait; use clippy_utils::visitors::for_each_expr; use rustc_errors::Applicability; -use rustc_hir::{Closure, ClosureKind, CoroutineKind, CoroutineSource, CoroutineDesugaring, Expr, ExprKind, MatchSource}; +use rustc_hir::{ + Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Expr, ExprKind, MatchSource, +}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty::UpvarCapture; @@ -49,6 +52,9 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let Some(expr) = desugar_await(peel_blocks(body_expr)) && // The await prefix must not come from a macro as its content could change in the future. expr.span.eq_ctxt(body_expr.span) && + // The await prefix must implement Future, as implementing IntoFuture is not enough. + let Some(future_trait) = cx.tcx.lang_items().future_trait() && + implements_trait(cx, cx.typeck_results().expr_ty(expr), future_trait, &[]) && // An async block does not have immediate side-effects from a `.await` point-of-view. (!expr.can_have_side_effects() || desugar_async_block(cx, expr).is_some()) && let Some(shortened_span) = walk_span_to_context(expr.span, span.ctxt()) @@ -71,7 +77,13 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { fn desugar_async_block<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { if let ExprKind::Closure(Closure { body, def_id, kind, .. }) = expr.kind && let body = cx.tcx.hir().body(*body) - && matches!(kind, ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Block))) + && matches!( + kind, + ClosureKind::Coroutine(CoroutineKind::Desugared( + CoroutineDesugaring::Async, + CoroutineSource::Block + )) + ) { cx.typeck_results() .closure_min_captures diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index 16c929edb92..cde08dfcc74 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -5,7 +5,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::intravisit::{Visitor as HirVisitor, Visitor}; -use rustc_hir::{intravisit as hir_visit, CoroutineKind, CoroutineSource, CoroutineDesugaring, Node, ClosureKind}; +use rustc_hir::{intravisit as hir_visit, ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, Node}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_middle::lint::in_external_macro; @@ -66,7 +66,8 @@ fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { fn is_async_closure(body: &hir::Body<'_>) -> bool { if let hir::ExprKind::Closure(innermost_closure_generated_by_desugar) = body.value.kind // checks whether it is `async || whatever_expression` - && let ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Closure)) = innermost_closure_generated_by_desugar.kind + && let ClosureKind::Coroutine(CoroutineKind::Desugared(CoroutineDesugaring::Async, CoroutineSource::Closure)) + = innermost_closure_generated_by_desugar.kind { true } else { diff --git a/clippy_lints/src/transmute/eager_transmute.rs b/clippy_lints/src/transmute/eager_transmute.rs new file mode 100644 index 00000000000..01a23c515f5 --- /dev/null +++ b/clippy_lints/src/transmute/eager_transmute.rs @@ -0,0 +1,74 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::is_normalizable; +use clippy_utils::{path_to_local, path_to_local_id}; +use rustc_abi::WrappingRange; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind, Node}; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; + +use super::EAGER_TRANSMUTE; + +fn peel_parent_unsafe_blocks<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> { + for (_, parent) in cx.tcx.hir().parent_iter(expr.hir_id) { + match parent { + Node::Block(_) => {}, + Node::Expr(e) if let ExprKind::Block(..) = e.kind => {}, + Node::Expr(e) => return Some(e), + _ => break, + } + } + None +} + +fn range_fully_contained(from: WrappingRange, to: WrappingRange) -> bool { + to.contains(from.start) && to.contains(from.end) +} + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + transmutable: &'tcx Expr<'tcx>, + from_ty: Ty<'tcx>, + to_ty: Ty<'tcx>, +) -> bool { + if let Some(then_some_call) = peel_parent_unsafe_blocks(cx, expr) + && let ExprKind::MethodCall(path, receiver, [arg], _) = then_some_call.kind + && cx.typeck_results().expr_ty(receiver).is_bool() + && path.ident.name == sym!(then_some) + && let ExprKind::Binary(_, lhs, rhs) = receiver.kind + && let Some(local_id) = path_to_local(transmutable) + && (path_to_local_id(lhs, local_id) || path_to_local_id(rhs, local_id)) + && is_normalizable(cx, cx.param_env, from_ty) + && is_normalizable(cx, cx.param_env, to_ty) + // we only want to lint if the target type has a niche that is larger than the one of the source type + // e.g. `u8` to `NonZeroU8` should lint, but `NonZeroU8` to `u8` should not + && let Ok(from_layout) = cx.tcx.layout_of(cx.param_env.and(from_ty)) + && let Ok(to_layout) = cx.tcx.layout_of(cx.param_env.and(to_ty)) + && match (from_layout.largest_niche, to_layout.largest_niche) { + (Some(from_niche), Some(to_niche)) => !range_fully_contained(from_niche.valid_range, to_niche.valid_range), + (None, Some(_)) => true, + (_, None) => false, + } + { + span_lint_and_then( + cx, + EAGER_TRANSMUTE, + expr.span, + "this transmute is always evaluated eagerly, even if the condition is false", + |diag| { + diag.multipart_suggestion( + "consider using `bool::then` to only transmute if the condition holds", + vec![ + (path.ident.span, "then".into()), + (arg.span.shrink_to_lo(), "|| ".into()), + ], + Applicability::MaybeIncorrect, + ); + }, + ); + true + } else { + false + } +} diff --git a/clippy_lints/src/transmute/mod.rs b/clippy_lints/src/transmute/mod.rs index 95a92afea66..06de7a11031 100644 --- a/clippy_lints/src/transmute/mod.rs +++ b/clippy_lints/src/transmute/mod.rs @@ -1,4 +1,5 @@ mod crosspointer_transmute; +mod eager_transmute; mod transmute_float_to_int; mod transmute_int_to_bool; mod transmute_int_to_char; @@ -463,6 +464,62 @@ "transmute results in a null function pointer, which is undefined behavior" } +declare_clippy_lint! { + /// ### What it does + /// Checks for integer validity checks, followed by a transmute that is (incorrectly) evaluated + /// eagerly (e.g. using `bool::then_some`). + /// + /// ### Why is this bad? + /// Eager evaluation means that the `transmute` call is executed regardless of whether the condition is true or false. + /// This can introduce unsoundness and other subtle bugs. + /// + /// ### Example + /// Consider the following function which is meant to convert an unsigned integer to its enum equivalent via transmute. + /// + /// ```no_run + /// #[repr(u8)] + /// enum Opcode { + /// Add = 0, + /// Sub = 1, + /// Mul = 2, + /// Div = 3 + /// } + /// + /// fn int_to_opcode(op: u8) -> Option { + /// (op < 4).then_some(unsafe { std::mem::transmute(op) }) + /// } + /// ``` + /// This may appear fine at first given that it checks that the `u8` is within the validity range of the enum, + /// *however* the transmute is evaluated eagerly, meaning that it executes even if `op >= 4`! + /// + /// This makes the function unsound, because it is possible for the caller to cause undefined behavior + /// (creating an enum with an invalid bitpattern) entirely in safe code only by passing an incorrect value, + /// which is normally only a bug that is possible in unsafe code. + /// + /// One possible way in which this can go wrong practically is that the compiler sees it as: + /// ```rust,ignore (illustrative) + /// let temp: Foo = unsafe { std::mem::transmute(op) }; + /// (0 < 4).then_some(temp) + /// ``` + /// and optimizes away the `(0 < 4)` check based on the assumption that since a `Foo` was created from `op` with the validity range `0..3`, + /// it is **impossible** for this condition to be false. + /// + /// In short, it is possible for this function to be optimized in a way that makes it [never return `None`](https://godbolt.org/z/ocrcenevq), + /// even if passed the value `4`. + /// + /// This can be avoided by instead using lazy evaluation. For the example above, this should be written: + /// ```rust,ignore (illustrative) + /// fn int_to_opcode(op: u8) -> Option { + /// (op < 4).then(|| unsafe { std::mem::transmute(op) }) + /// ^^^^ ^^ `bool::then` only executes the closure if the condition is true! + /// } + /// ``` + #[clippy::version = "1.76.0"] + pub EAGER_TRANSMUTE, + correctness, + "eager evaluation of `transmute`" +} + pub struct Transmute { msrv: Msrv, } @@ -484,6 +541,7 @@ pub struct Transmute { TRANSMUTE_UNDEFINED_REPR, TRANSMUTING_NULL, TRANSMUTE_NULL_TO_FN, + EAGER_TRANSMUTE, ]); impl Transmute { #[must_use] @@ -530,7 +588,8 @@ fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { | transmute_float_to_int::check(cx, e, from_ty, to_ty, arg, const_context) | transmute_num_to_bytes::check(cx, e, from_ty, to_ty, arg, const_context) | (unsound_collection_transmute::check(cx, e, from_ty, to_ty) - || transmute_undefined_repr::check(cx, e, from_ty, to_ty)); + || transmute_undefined_repr::check(cx, e, from_ty, to_ty)) + | (eager_transmute::check(cx, e, arg, from_ty, to_ty)); if !linted { transmutes_expressible_as_ptr_casts::check(cx, e, from_ty, from_ty_adjusted, to_ty, arg); diff --git a/clippy_lints/src/unconditional_recursion.rs b/clippy_lints/src/unconditional_recursion.rs new file mode 100644 index 00000000000..b1fa30aa068 --- /dev/null +++ b/clippy_lints/src/unconditional_recursion.rs @@ -0,0 +1,134 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::{get_trait_def_id, path_res}; +use rustc_ast::BinOpKind; +use rustc_hir::def::Res; +use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, Expr, ExprKind, FnDecl, Item, ItemKind, Node}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::{sym, Span}; + +declare_clippy_lint! { + /// ### What it does + /// Checks that there isn't an infinite recursion in `PartialEq` trait + /// implementation. + /// + /// ### Why is this bad? + /// This is a hard to find infinite recursion which will crashing any code + /// using it. + /// + /// ### Example + /// ```no_run + /// enum Foo { + /// A, + /// B, + /// } + /// + /// impl PartialEq for Foo { + /// fn eq(&self, other: &Self) -> bool { + /// self == other // bad! + /// } + /// } + /// ``` + /// Use instead: + /// + /// In such cases, either use `#[derive(PartialEq)]` or don't implement it. + #[clippy::version = "1.76.0"] + pub UNCONDITIONAL_RECURSION, + suspicious, + "detect unconditional recursion in some traits implementation" +} + +declare_lint_pass!(UnconditionalRecursion => [UNCONDITIONAL_RECURSION]); + +fn get_ty_def_id(ty: Ty<'_>) -> Option { + match ty.peel_refs().kind() { + ty::Adt(adt, _) => Some(adt.did()), + ty::Foreign(def_id) => Some(*def_id), + _ => None, + } +} + +fn is_local(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + matches!(path_res(cx, expr), Res::Local(_)) +} + +impl<'tcx> LateLintPass<'tcx> for UnconditionalRecursion { + #[allow(clippy::unnecessary_def_path)] + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: FnKind<'tcx>, + _decl: &'tcx FnDecl<'tcx>, + body: &'tcx Body<'tcx>, + method_span: Span, + def_id: LocalDefId, + ) { + // If the function is a method... + if let FnKind::Method(name, _) = kind + // That has two arguments. + && let [self_arg, other_arg] = cx + .tcx + .instantiate_bound_regions_with_erased(cx.tcx.fn_sig(def_id).skip_binder()) + .inputs() + && let Some(self_arg) = get_ty_def_id(*self_arg) + && let Some(other_arg) = get_ty_def_id(*other_arg) + // The two arguments are of the same type. + && self_arg == other_arg + && let hir_id = cx.tcx.local_def_id_to_hir_id(def_id) + && let Some(( + _, + Node::Item(Item { + kind: ItemKind::Impl(impl_), + owner_id, + .. + }), + )) = cx.tcx.hir().parent_iter(hir_id).next() + // We exclude `impl` blocks generated from rustc's proc macros. + && !cx.tcx.has_attr(*owner_id, sym::automatically_derived) + // It is a implementation of a trait. + && let Some(trait_) = impl_.of_trait + && let Some(trait_def_id) = trait_.trait_def_id() + // The trait is `PartialEq`. + && Some(trait_def_id) == get_trait_def_id(cx, &["core", "cmp", "PartialEq"]) + { + let to_check_op = if name.name == sym::eq { + BinOpKind::Eq + } else { + BinOpKind::Ne + }; + let expr = body.value.peel_blocks(); + let is_bad = match expr.kind { + ExprKind::Binary(op, left, right) if op.node == to_check_op => { + is_local(cx, left) && is_local(cx, right) + }, + ExprKind::MethodCall(segment, receiver, &[arg], _) if segment.ident.name == name.name => { + if is_local(cx, receiver) + && is_local(cx, &arg) + && let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && let Some(trait_id) = cx.tcx.trait_of_item(fn_id) + && trait_id == trait_def_id + { + true + } else { + false + } + }, + _ => false, + }; + if is_bad { + span_lint_and_then( + cx, + UNCONDITIONAL_RECURSION, + method_span, + "function cannot return without recursing", + |diag| { + diag.span_note(expr.span, "recursive call site"); + }, + ); + } + } + } +} diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 7a6549a7c54..e5bc3b5a25f 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -680,9 +680,7 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos }) .filter(|(_, text)| !text.is_empty()); - let Some((line_start, line)) = lines.next() else { - return None; - }; + let (line_start, line) = lines.next()?; // Check for a sequence of line comments. if line.starts_with("//") { let (mut line, mut line_start) = (line, line_start); diff --git a/clippy_lints/src/uninhabited_references.rs b/clippy_lints/src/uninhabited_references.rs index d41576cadad..903593ecfd7 100644 --- a/clippy_lints/src/uninhabited_references.rs +++ b/clippy_lints/src/uninhabited_references.rs @@ -32,7 +32,7 @@ /// ``` #[clippy::version = "1.76.0"] pub UNINHABITED_REFERENCES, - suspicious, + nursery, "reference to uninhabited type" } diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index df715b12dea..8817e46b3c8 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -351,6 +351,7 @@ fn expr(&self, expr: &Binding<&hir::Expr<'_>>) { let_pat, let_expr, if_then, + .. }) = higher::WhileLet::hir(expr.value) { bind!(self, let_pat, let_expr, if_then); diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 5d23326cec8..b8869eedf52 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "clippy_utils" -version = "0.1.76" +version = "0.1.77" edition = "2021" publish = false diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 7fe76b946a4..adc35bd82ae 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -134,6 +134,7 @@ pub fn eq_struct_rest(l: &StructRest, r: &StructRest) -> bool { } } +#[allow(clippy::too_many_lines)] // Just a big match statement pub fn eq_expr(l: &Expr, r: &Expr) -> bool { use ExprKind::*; if !over(&l.attrs, &r.attrs, eq_attr) { diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 46c96fad884..db80e07ca1c 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -76,12 +76,14 @@ pub fn get_attr<'a>( }) .map_or_else( || { - sess.dcx().span_err(attr_segments[1].ident.span, "usage of unknown attribute"); + sess.dcx() + .span_err(attr_segments[1].ident.span, "usage of unknown attribute"); false }, |deprecation_status| { - let mut diag = - sess.dcx().struct_span_err(attr_segments[1].ident.span, "usage of deprecated attribute"); + let mut diag = sess + .dcx() + .struct_span_err(attr_segments[1].ident.span, "usage of deprecated attribute"); match *deprecation_status { DeprecationStatus::Deprecated => { diag.emit(); @@ -132,7 +134,8 @@ pub fn get_unique_attr<'a>( let mut unique_attr: Option<&ast::Attribute> = None; for attr in get_attr(sess, attrs, name) { if let Some(duplicate) = unique_attr { - sess.dcx().struct_span_err(attr.span, format!("`{name}` is defined multiple times")) + sess.dcx() + .struct_span_err(attr.span, format!("`{name}` is defined multiple times")) .span_note(duplicate.span, "first definition found here") .emit(); } else { diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index 3135a033648..ba682813dad 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -91,6 +91,9 @@ pub struct IfLet<'hir> { pub if_then: &'hir Expr<'hir>, /// `if let` else expression pub if_else: Option<&'hir Expr<'hir>>, + /// `if let PAT = EXPR` + /// ^^^^^^^^^^^^^^ + pub let_span: Span, } impl<'hir> IfLet<'hir> { @@ -99,9 +102,10 @@ pub fn hir(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option { if let ExprKind::If( Expr { kind: - ExprKind::Let(hir::Let { + ExprKind::Let(&hir::Let { pat: let_pat, init: let_expr, + span: let_span, .. }), .. @@ -129,6 +133,7 @@ pub fn hir(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option { let_expr, if_then, if_else, + let_span, }); } None @@ -146,6 +151,9 @@ pub enum IfLetOrMatch<'hir> { &'hir Pat<'hir>, &'hir Expr<'hir>, Option<&'hir Expr<'hir>>, + /// `if let PAT = EXPR` + /// ^^^^^^^^^^^^^^ + Span, ), } @@ -160,7 +168,8 @@ pub fn parse(cx: &LateContext<'_>, expr: &Expr<'hir>) -> Option { let_pat, if_then, if_else, - }| { Self::IfLet(let_expr, let_pat, if_then, if_else) }, + let_span, + }| { Self::IfLet(let_expr, let_pat, if_then, if_else, let_span) }, ), } } @@ -353,6 +362,9 @@ pub struct WhileLet<'hir> { pub let_expr: &'hir Expr<'hir>, /// `while let` loop body pub if_then: &'hir Expr<'hir>, + /// `while let PAT = EXPR` + /// ^^^^^^^^^^^^^^ + pub let_span: Span, } impl<'hir> WhileLet<'hir> { @@ -367,9 +379,10 @@ pub const fn hir(expr: &Expr<'hir>) -> Option { ExprKind::If( Expr { kind: - ExprKind::Let(hir::Let { + ExprKind::Let(&hir::Let { pat: let_pat, init: let_expr, + span: let_span, .. }), .. @@ -390,6 +403,7 @@ pub const fn hir(expr: &Expr<'hir>) -> Option { let_pat, let_expr, if_then, + let_span, }); } None diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs index d752fe7d97e..ebc38e531fe 100644 --- a/clippy_utils/src/visitors.rs +++ b/clippy_utils/src/visitors.rs @@ -316,10 +316,7 @@ struct V<'a, 'tcx> { is_const: bool, } impl<'tcx> Visitor<'tcx> for V<'_, 'tcx> { - type NestedFilter = nested_filter::OnlyBodies; - fn nested_visit_map(&mut self) -> Self::Map { - self.cx.tcx.hir() - } + type NestedFilter = rustc_hir::intravisit::nested_filter::None; fn visit_expr(&mut self, e: &'tcx Expr<'_>) { if !self.is_const { diff --git a/declare_clippy_lint/Cargo.toml b/declare_clippy_lint/Cargo.toml index af123e107d5..5aaafb41721 100644 --- a/declare_clippy_lint/Cargo.toml +++ b/declare_clippy_lint/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "declare_clippy_lint" -version = "0.1.76" +version = "0.1.77" edition = "2021" publish = false diff --git a/rust-toolchain b/rust-toolchain index d575da6dece..5a2526fd267 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-12-16" +channel = "nightly-2023-12-28" components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"] diff --git a/tests/ui-internal/disallow_struct_span_lint.rs b/tests/ui-internal/disallow_struct_span_lint.rs index 3155c0235ff..c81d54918cb 100644 --- a/tests/ui-internal/disallow_struct_span_lint.rs +++ b/tests/ui-internal/disallow_struct_span_lint.rs @@ -11,7 +11,7 @@ use rustc_middle::ty::TyCtxt; pub fn a(cx: impl LintContext, lint: &'static Lint, span: impl Into, msg: impl Into) { - cx.struct_span_lint(lint, span, msg, |b| b); + cx.struct_span_lint(lint, span, msg, |_| {}); } pub fn b( @@ -21,7 +21,7 @@ pub fn b( span: impl Into, msg: impl Into, ) { - tcx.struct_span_lint_hir(lint, hir_id, span, msg, |b| b); + tcx.struct_span_lint_hir(lint, hir_id, span, msg, |_| {}); } fn main() {} diff --git a/tests/ui-internal/disallow_struct_span_lint.stderr b/tests/ui-internal/disallow_struct_span_lint.stderr index 76c487fb135..7d424124f2b 100644 --- a/tests/ui-internal/disallow_struct_span_lint.stderr +++ b/tests/ui-internal/disallow_struct_span_lint.stderr @@ -1,8 +1,8 @@ error: use of a disallowed method `rustc_lint::context::LintContext::struct_span_lint` --> $DIR/disallow_struct_span_lint.rs:14:5 | -LL | cx.struct_span_lint(lint, span, msg, |b| b); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | cx.struct_span_lint(lint, span, msg, |_| {}); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::disallowed-methods` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::disallowed_methods)]` @@ -10,8 +10,8 @@ LL | cx.struct_span_lint(lint, span, msg, |b| b); error: use of a disallowed method `rustc_middle::ty::context::TyCtxt::struct_span_lint_hir` --> $DIR/disallow_struct_span_lint.rs:24:5 | -LL | tcx.struct_span_lint_hir(lint, hir_id, span, msg, |b| b); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | tcx.struct_span_lint_hir(lint, hir_id, span, msg, |_| {}); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors diff --git a/tests/ui-toml/suppress_lint_in_const/test.rs b/tests/ui-toml/suppress_lint_in_const/test.rs index 17c1b03d88c..3edb3a10b76 100644 --- a/tests/ui-toml/suppress_lint_in_const/test.rs +++ b/tests/ui-toml/suppress_lint_in_const/test.rs @@ -7,7 +7,8 @@ unconditional_panic, clippy::no_effect, clippy::unnecessary_operation, - clippy::useless_vec + clippy::useless_vec, + clippy::out_of_bounds_indexing )] const ARR: [i32; 2] = [1, 2]; diff --git a/tests/ui-toml/suppress_lint_in_const/test.stderr b/tests/ui-toml/suppress_lint_in_const/test.stderr index f8ace799593..84e7eff4557 100644 --- a/tests/ui-toml/suppress_lint_in_const/test.stderr +++ b/tests/ui-toml/suppress_lint_in_const/test.stderr @@ -1,17 +1,17 @@ error[E0080]: evaluation of `main::{constant#3}` failed - --> $DIR/test.rs:37:14 + --> $DIR/test.rs:38:14 | LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true. | ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4 note: erroneous constant encountered - --> $DIR/test.rs:37:5 + --> $DIR/test.rs:38:5 | LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true. | ^^^^^^^^^^^^^^^^^^^^^^ error: indexing may panic - --> $DIR/test.rs:28:5 + --> $DIR/test.rs:29:5 | LL | x[index]; | ^^^^^^^^ @@ -21,7 +21,7 @@ LL | x[index]; = help: to override `-D warnings` add `#[allow(clippy::indexing_slicing)]` error: indexing may panic - --> $DIR/test.rs:46:5 + --> $DIR/test.rs:47:5 | LL | v[0]; | ^^^^ @@ -29,7 +29,7 @@ LL | v[0]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> $DIR/test.rs:47:5 + --> $DIR/test.rs:48:5 | LL | v[10]; | ^^^^^ @@ -37,7 +37,7 @@ LL | v[10]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> $DIR/test.rs:48:5 + --> $DIR/test.rs:49:5 | LL | v[1 << 3]; | ^^^^^^^^^ @@ -45,7 +45,7 @@ LL | v[1 << 3]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> $DIR/test.rs:54:5 + --> $DIR/test.rs:55:5 | LL | v[N]; | ^^^^ @@ -53,7 +53,7 @@ LL | v[N]; = help: consider using `.get(n)` or `.get_mut(n)` instead error: indexing may panic - --> $DIR/test.rs:55:5 + --> $DIR/test.rs:56:5 | LL | v[M]; | ^^^^ @@ -61,7 +61,7 @@ LL | v[M]; = help: consider using `.get(n)` or `.get_mut(n)` instead error[E0080]: evaluation of constant value failed - --> $DIR/test.rs:15:24 + --> $DIR/test.rs:16:24 | LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts. | ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4 diff --git a/tests/ui/assertions_on_constants.rs b/tests/ui/assertions_on_constants.rs index 10809a6d247..1309ae45d0a 100644 --- a/tests/ui/assertions_on_constants.rs +++ b/tests/ui/assertions_on_constants.rs @@ -45,4 +45,11 @@ fn main() { const CFG_FLAG: &bool = &cfg!(feature = "hey"); assert!(!CFG_FLAG); + + const _: () = assert!(true); + //~^ ERROR: `assert!(true)` will be optimized out by the compiler + + // Don't lint if the value is dependent on a defined constant: + const N: usize = 1024; + const _: () = assert!(N.is_power_of_two()); } diff --git a/tests/ui/assertions_on_constants.stderr b/tests/ui/assertions_on_constants.stderr index 780d1fe1c8a..099be4ed355 100644 --- a/tests/ui/assertions_on_constants.stderr +++ b/tests/ui/assertions_on_constants.stderr @@ -72,5 +72,13 @@ LL | debug_assert!(true); | = help: remove it -error: aborting due to 9 previous errors +error: `assert!(true)` will be optimized out by the compiler + --> $DIR/assertions_on_constants.rs:49:19 + | +LL | const _: () = assert!(true); + | ^^^^^^^^^^^^^ + | + = help: remove it + +error: aborting due to 10 previous errors diff --git a/tests/ui/author/loop.rs b/tests/ui/author/loop.rs index d6de21631e2..ff5b6100117 100644 --- a/tests/ui/author/loop.rs +++ b/tests/ui/author/loop.rs @@ -1,5 +1,9 @@ #![feature(stmt_expr_attributes)] -#![allow(clippy::never_loop, clippy::while_immutable_condition)] +#![allow( + clippy::never_loop, + clippy::while_immutable_condition, + clippy::redundant_pattern_matching +)] fn main() { #[clippy::author] diff --git a/tests/ui/bool_comparison.fixed b/tests/ui/bool_comparison.fixed index e3f2ca72d1c..02f1d09b833 100644 --- a/tests/ui/bool_comparison.fixed +++ b/tests/ui/bool_comparison.fixed @@ -165,3 +165,12 @@ fn issue3973() { if is_debug == m!(func) {} if m!(func) == is_debug {} } + +#[allow(clippy::unnecessary_cast)] +fn issue9907() { + let _ = (1 >= 2) as usize; + let _ = (!m!(func)) as usize; + // This is not part of the issue, but an unexpected found when fixing the issue, + // the provided span was inside of macro rather than the macro callsite. + let _ = ((1 < 2) != m!(func)) as usize; +} diff --git a/tests/ui/bool_comparison.rs b/tests/ui/bool_comparison.rs index d1bc20d6831..5ef696d855e 100644 --- a/tests/ui/bool_comparison.rs +++ b/tests/ui/bool_comparison.rs @@ -165,3 +165,12 @@ fn issue3973() { if is_debug == m!(func) {} if m!(func) == is_debug {} } + +#[allow(clippy::unnecessary_cast)] +fn issue9907() { + let _ = ((1 < 2) == false) as usize; + let _ = (false == m!(func)) as usize; + // This is not part of the issue, but an unexpected found when fixing the issue, + // the provided span was inside of macro rather than the macro callsite. + let _ = ((1 < 2) == !m!(func)) as usize; +} diff --git a/tests/ui/bool_comparison.stderr b/tests/ui/bool_comparison.stderr index 4560df6d4cd..6907dc0523f 100644 --- a/tests/ui/bool_comparison.stderr +++ b/tests/ui/bool_comparison.stderr @@ -133,5 +133,23 @@ error: equality checks against true are unnecessary LL | if m!(func) == true {} | ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `m!(func)` -error: aborting due to 22 previous errors +error: equality checks against false can be replaced by a negation + --> $DIR/bool_comparison.rs:171:14 + | +LL | let _ = ((1 < 2) == false) as usize; + | ^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `1 >= 2` + +error: equality checks against false can be replaced by a negation + --> $DIR/bool_comparison.rs:172:14 + | +LL | let _ = (false == m!(func)) as usize; + | ^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `!m!(func)` + +error: this comparison might be written more concisely + --> $DIR/bool_comparison.rs:175:14 + | +LL | let _ = ((1 < 2) == !m!(func)) as usize; + | ^^^^^^^^^^^^^^^^^^^^ help: try simplifying it as shown: `(1 < 2) != m!(func)` + +error: aborting due to 25 previous errors diff --git a/tests/ui/branches_sharing_code/shared_at_bottom.rs b/tests/ui/branches_sharing_code/shared_at_bottom.rs index d102efa7a58..549908b8770 100644 --- a/tests/ui/branches_sharing_code/shared_at_bottom.rs +++ b/tests/ui/branches_sharing_code/shared_at_bottom.rs @@ -1,6 +1,10 @@ #![deny(clippy::if_same_then_else, clippy::branches_sharing_code)] -#![allow(dead_code)] -#![allow(clippy::equatable_if_let, clippy::uninlined_format_args)] +#![allow( + clippy::equatable_if_let, + clippy::uninlined_format_args, + clippy::redundant_pattern_matching, + dead_code +)] //@no-rustfix // This tests the branches_sharing_code lint at the end of blocks diff --git a/tests/ui/branches_sharing_code/shared_at_bottom.stderr b/tests/ui/branches_sharing_code/shared_at_bottom.stderr index d00717befc1..8223df0fe7b 100644 --- a/tests/ui/branches_sharing_code/shared_at_bottom.stderr +++ b/tests/ui/branches_sharing_code/shared_at_bottom.stderr @@ -1,5 +1,5 @@ error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:31:5 + --> $DIR/shared_at_bottom.rs:35:5 | LL | / let result = false; LL | | @@ -26,7 +26,7 @@ LL ~ result; | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:51:5 + --> $DIR/shared_at_bottom.rs:55:5 | LL | / println!("Same end of block"); LL | | @@ -40,7 +40,7 @@ LL + println!("Same end of block"); | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:69:5 + --> $DIR/shared_at_bottom.rs:73:5 | LL | / println!( LL | | @@ -61,7 +61,7 @@ LL + ); | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:82:9 + --> $DIR/shared_at_bottom.rs:86:9 | LL | / println!("Hello World"); LL | | @@ -75,7 +75,7 @@ LL + println!("Hello World"); | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:99:5 + --> $DIR/shared_at_bottom.rs:103:5 | LL | / let later_used_value = "A string value"; LL | | @@ -94,7 +94,7 @@ LL + println!("{}", later_used_value); | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:113:5 + --> $DIR/shared_at_bottom.rs:117:5 | LL | / let simple_examples = "I now identify as a &str :)"; LL | | @@ -112,7 +112,7 @@ LL + println!("This is the new simple_example: {}", simple_examples); | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:179:5 + --> $DIR/shared_at_bottom.rs:183:5 | LL | / x << 2 LL | | @@ -128,7 +128,7 @@ LL ~ x << 2; | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:188:5 + --> $DIR/shared_at_bottom.rs:192:5 | LL | / x * 4 LL | | @@ -144,7 +144,7 @@ LL + x * 4 | error: all if blocks contain the same code at the end - --> $DIR/shared_at_bottom.rs:202:44 + --> $DIR/shared_at_bottom.rs:206:44 | LL | if x == 17 { b = 1; a = 0x99; } else { a = 0x99; } | ^^^^^^^^^^^ diff --git a/tests/ui/collapsible_if.fixed b/tests/ui/collapsible_if.fixed index fff6bfcc753..44b0b6e7391 100644 --- a/tests/ui/collapsible_if.fixed +++ b/tests/ui/collapsible_if.fixed @@ -3,7 +3,8 @@ clippy::equatable_if_let, clippy::needless_if, clippy::nonminimal_bool, - clippy::eq_op + clippy::eq_op, + clippy::redundant_pattern_matching )] #[rustfmt::skip] diff --git a/tests/ui/collapsible_if.rs b/tests/ui/collapsible_if.rs index 70bfea231ae..563a273dcdd 100644 --- a/tests/ui/collapsible_if.rs +++ b/tests/ui/collapsible_if.rs @@ -3,7 +3,8 @@ clippy::equatable_if_let, clippy::needless_if, clippy::nonminimal_bool, - clippy::eq_op + clippy::eq_op, + clippy::redundant_pattern_matching )] #[rustfmt::skip] diff --git a/tests/ui/collapsible_if.stderr b/tests/ui/collapsible_if.stderr index e8a36bf48f1..16df3e433db 100644 --- a/tests/ui/collapsible_if.stderr +++ b/tests/ui/collapsible_if.stderr @@ -1,5 +1,5 @@ error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:14:5 + --> $DIR/collapsible_if.rs:15:5 | LL | / if x == "hello" { LL | | if y == "world" { @@ -18,7 +18,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:20:5 + --> $DIR/collapsible_if.rs:21:5 | LL | / if x == "hello" || x == "world" { LL | | if y == "world" || y == "hello" { @@ -35,7 +35,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:26:5 + --> $DIR/collapsible_if.rs:27:5 | LL | / if x == "hello" && x == "world" { LL | | if y == "world" || y == "hello" { @@ -52,7 +52,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:32:5 + --> $DIR/collapsible_if.rs:33:5 | LL | / if x == "hello" || x == "world" { LL | | if y == "world" && y == "hello" { @@ -69,7 +69,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:38:5 + --> $DIR/collapsible_if.rs:39:5 | LL | / if x == "hello" && x == "world" { LL | | if y == "world" && y == "hello" { @@ -86,7 +86,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:44:5 + --> $DIR/collapsible_if.rs:45:5 | LL | / if 42 == 1337 { LL | | if 'a' != 'A' { @@ -103,7 +103,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:100:5 + --> $DIR/collapsible_if.rs:101:5 | LL | / if x == "hello" { LL | | if y == "world" { // Collapsible @@ -120,7 +120,7 @@ LL + } | error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:159:5 + --> $DIR/collapsible_if.rs:160:5 | LL | / if matches!(true, true) { LL | | if matches!(true, true) {} @@ -128,7 +128,7 @@ LL | | } | |_____^ help: collapse nested if block: `if matches!(true, true) && matches!(true, true) {}` error: this `if` statement can be collapsed - --> $DIR/collapsible_if.rs:164:5 + --> $DIR/collapsible_if.rs:165:5 | LL | / if matches!(true, true) && truth() { LL | | if matches!(true, true) {} diff --git a/tests/ui/crashes/ice-11939.rs b/tests/ui/crashes/ice-11939.rs new file mode 100644 index 00000000000..5e7193b6826 --- /dev/null +++ b/tests/ui/crashes/ice-11939.rs @@ -0,0 +1,14 @@ +#![allow(clippy::unit_arg, clippy::no_effect)] + +const fn v(_: ()) {} + +fn main() { + if true { + v({ + [0; 1 + 1]; + }); + Some(()) + } else { + None + }; +} diff --git a/tests/ui/crashes/ice-5497.rs b/tests/ui/crashes/ice-5497.rs index f77f691c192..fe8d640c470 100644 --- a/tests/ui/crashes/ice-5497.rs +++ b/tests/ui/crashes/ice-5497.rs @@ -1,4 +1,6 @@ // reduced from rustc issue-69020-assoc-const-arith-overflow.rs +#![allow(clippy::out_of_bounds_indexing)] + pub fn main() {} pub trait Foo { diff --git a/tests/ui/crashes/ice-5497.stderr b/tests/ui/crashes/ice-5497.stderr index ee69f3379b6..3efaf05827e 100644 --- a/tests/ui/crashes/ice-5497.stderr +++ b/tests/ui/crashes/ice-5497.stderr @@ -1,5 +1,5 @@ error: this operation will panic at runtime - --> $DIR/ice-5497.rs:9:22 + --> $DIR/ice-5497.rs:11:22 | LL | const OOB: i32 = [1][1] + T::OOB; | ^^^^^^ index out of bounds: the length is 1 but the index is 1 diff --git a/tests/ui/doc/doc-fixable.fixed b/tests/ui/doc/doc-fixable.fixed index 708ac666675..bff46e55722 100644 --- a/tests/ui/doc/doc-fixable.fixed +++ b/tests/ui/doc/doc-fixable.fixed @@ -65,7 +65,7 @@ fn test_units() { /// OAuth GraphQL /// OCaml /// OpenGL OpenMP OpenSSH OpenSSL OpenStreetMap OpenDNS -/// WebGL +/// WebGL WebGL2 WebGPU /// TensorFlow /// TrueType /// iOS macOS FreeBSD diff --git a/tests/ui/doc/doc-fixable.rs b/tests/ui/doc/doc-fixable.rs index 040d6352c52..4e162a97dee 100644 --- a/tests/ui/doc/doc-fixable.rs +++ b/tests/ui/doc/doc-fixable.rs @@ -65,7 +65,7 @@ fn test_units() { /// OAuth GraphQL /// OCaml /// OpenGL OpenMP OpenSSH OpenSSL OpenStreetMap OpenDNS -/// WebGL +/// WebGL WebGL2 WebGPU /// TensorFlow /// TrueType /// iOS macOS FreeBSD diff --git a/tests/ui/eager_transmute.fixed b/tests/ui/eager_transmute.fixed new file mode 100644 index 00000000000..e06aa2cc9fd --- /dev/null +++ b/tests/ui/eager_transmute.fixed @@ -0,0 +1,72 @@ +#![feature(rustc_attrs)] +#![warn(clippy::eager_transmute)] +#![allow(clippy::transmute_int_to_non_zero)] + +use std::num::NonZeroU8; + +#[repr(u8)] +enum Opcode { + Add = 0, + Sub = 1, + Mul = 2, + Div = 3, +} + +fn int_to_opcode(op: u8) -> Option { + (op < 4).then(|| unsafe { std::mem::transmute(op) }) +} + +fn f(op: u8, unrelated: u8) { + true.then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + (unrelated < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + (op < 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) }); + (op > 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) }); + (op == 0).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) }); +} + +unsafe fn f2(op: u8) { + (op < 4).then(|| std::mem::transmute::<_, Opcode>(op)); +} + +#[rustc_layout_scalar_valid_range_end(254)] +struct NonMaxU8(u8); +#[rustc_layout_scalar_valid_range_end(254)] +#[rustc_layout_scalar_valid_range_start(1)] +struct NonZeroNonMaxU8(u8); + +macro_rules! impls { + ($($t:ty),*) => { + $( + impl PartialEq for $t { + fn eq(&self, other: &u8) -> bool { + self.0 == *other + } + } + impl PartialOrd for $t { + fn partial_cmp(&self, other: &u8) -> Option { + self.0.partial_cmp(other) + } + } + )* + }; +} +impls!(NonMaxU8, NonZeroNonMaxU8); + +fn niche_tests(v1: u8, v2: NonZeroU8, v3: NonZeroNonMaxU8) { + // u8 -> NonZeroU8, do lint + let _: Option = (v1 > 0).then(|| unsafe { std::mem::transmute(v1) }); + + // NonZeroU8 -> u8, don't lint, target type has no niche and therefore a higher validity range + let _: Option = (v2 > NonZeroU8::new(1).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); + + // NonZeroU8 -> NonMaxU8, do lint, different niche + let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then(|| unsafe { std::mem::transmute(v2) }); + + // NonZeroNonMaxU8 -> NonMaxU8, don't lint, target type has more validity + let _: Option = (v3 < 255).then_some(unsafe { std::mem::transmute(v2) }); + + // NonZeroU8 -> NonZeroNonMaxU8, do lint, target type has less validity + let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then(|| unsafe { std::mem::transmute(v2) }); +} + +fn main() {} diff --git a/tests/ui/eager_transmute.rs b/tests/ui/eager_transmute.rs new file mode 100644 index 00000000000..89ccdf583f3 --- /dev/null +++ b/tests/ui/eager_transmute.rs @@ -0,0 +1,72 @@ +#![feature(rustc_attrs)] +#![warn(clippy::eager_transmute)] +#![allow(clippy::transmute_int_to_non_zero)] + +use std::num::NonZeroU8; + +#[repr(u8)] +enum Opcode { + Add = 0, + Sub = 1, + Mul = 2, + Div = 3, +} + +fn int_to_opcode(op: u8) -> Option { + (op < 4).then_some(unsafe { std::mem::transmute(op) }) +} + +fn f(op: u8, unrelated: u8) { + true.then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + (unrelated < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + (op < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + (op > 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + (op == 0).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); +} + +unsafe fn f2(op: u8) { + (op < 4).then_some(std::mem::transmute::<_, Opcode>(op)); +} + +#[rustc_layout_scalar_valid_range_end(254)] +struct NonMaxU8(u8); +#[rustc_layout_scalar_valid_range_end(254)] +#[rustc_layout_scalar_valid_range_start(1)] +struct NonZeroNonMaxU8(u8); + +macro_rules! impls { + ($($t:ty),*) => { + $( + impl PartialEq for $t { + fn eq(&self, other: &u8) -> bool { + self.0 == *other + } + } + impl PartialOrd for $t { + fn partial_cmp(&self, other: &u8) -> Option { + self.0.partial_cmp(other) + } + } + )* + }; +} +impls!(NonMaxU8, NonZeroNonMaxU8); + +fn niche_tests(v1: u8, v2: NonZeroU8, v3: NonZeroNonMaxU8) { + // u8 -> NonZeroU8, do lint + let _: Option = (v1 > 0).then_some(unsafe { std::mem::transmute(v1) }); + + // NonZeroU8 -> u8, don't lint, target type has no niche and therefore a higher validity range + let _: Option = (v2 > NonZeroU8::new(1).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); + + // NonZeroU8 -> NonMaxU8, do lint, different niche + let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); + + // NonZeroNonMaxU8 -> NonMaxU8, don't lint, target type has more validity + let _: Option = (v3 < 255).then_some(unsafe { std::mem::transmute(v2) }); + + // NonZeroU8 -> NonZeroNonMaxU8, do lint, target type has less validity + let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); +} + +fn main() {} diff --git a/tests/ui/eager_transmute.stderr b/tests/ui/eager_transmute.stderr new file mode 100644 index 00000000000..5eb163c5fcb --- /dev/null +++ b/tests/ui/eager_transmute.stderr @@ -0,0 +1,92 @@ +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:16:33 + | +LL | (op < 4).then_some(unsafe { std::mem::transmute(op) }) + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::eager-transmute` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::eager_transmute)]` +help: consider using `bool::then` to only transmute if the condition holds + | +LL | (op < 4).then(|| unsafe { std::mem::transmute(op) }) + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:22:33 + | +LL | (op < 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | (op < 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:23:33 + | +LL | (op > 4).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | (op > 4).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:24:34 + | +LL | (op == 0).then_some(unsafe { std::mem::transmute::<_, Opcode>(op) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | (op == 0).then(|| unsafe { std::mem::transmute::<_, Opcode>(op) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:28:24 + | +LL | (op < 4).then_some(std::mem::transmute::<_, Opcode>(op)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | (op < 4).then(|| std::mem::transmute::<_, Opcode>(op)); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:57:60 + | +LL | let _: Option = (v1 > 0).then_some(unsafe { std::mem::transmute(v1) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (v1 > 0).then(|| unsafe { std::mem::transmute(v1) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:63:86 + | +LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then(|| unsafe { std::mem::transmute(v2) }); + | ~~~~ ++ + +error: this transmute is always evaluated eagerly, even if the condition is false + --> $DIR/eager_transmute.rs:69:93 + | +LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then_some(unsafe { std::mem::transmute(v2) }); + | ^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `bool::then` to only transmute if the condition holds + | +LL | let _: Option = (v2 < NonZeroU8::new(255).unwrap()).then(|| unsafe { std::mem::transmute(v2) }); + | ~~~~ ++ + +error: aborting due to 8 previous errors + diff --git a/tests/ui/get_unwrap.fixed b/tests/ui/get_unwrap.fixed index d5a4309db59..62beb195939 100644 --- a/tests/ui/get_unwrap.fixed +++ b/tests/ui/get_unwrap.fixed @@ -2,7 +2,8 @@ unused_mut, clippy::from_iter_instead_of_collect, clippy::get_first, - clippy::useless_vec + clippy::useless_vec, + clippy::out_of_bounds_indexing )] #![warn(clippy::unwrap_used)] #![deny(clippy::get_unwrap)] diff --git a/tests/ui/get_unwrap.rs b/tests/ui/get_unwrap.rs index 5a9ad204f70..1e09ff5c67e 100644 --- a/tests/ui/get_unwrap.rs +++ b/tests/ui/get_unwrap.rs @@ -2,7 +2,8 @@ unused_mut, clippy::from_iter_instead_of_collect, clippy::get_first, - clippy::useless_vec + clippy::useless_vec, + clippy::out_of_bounds_indexing )] #![warn(clippy::unwrap_used)] #![deny(clippy::get_unwrap)] diff --git a/tests/ui/get_unwrap.stderr b/tests/ui/get_unwrap.stderr index 384860ea116..700f3cfec4e 100644 --- a/tests/ui/get_unwrap.stderr +++ b/tests/ui/get_unwrap.stderr @@ -1,17 +1,17 @@ error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:36:17 + --> $DIR/get_unwrap.rs:37:17 | LL | let _ = boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&boxed_slice[1]` | note: the lint level is defined here - --> $DIR/get_unwrap.rs:8:9 + --> $DIR/get_unwrap.rs:9:9 | LL | #![deny(clippy::get_unwrap)] | ^^^^^^^^^^^^^^^^^^ error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:36:17 + --> $DIR/get_unwrap.rs:37:17 | LL | let _ = boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -22,13 +22,13 @@ LL | let _ = boxed_slice.get(1).unwrap(); = help: to override `-D warnings` add `#[allow(clippy::unwrap_used)]` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:37:17 + --> $DIR/get_unwrap.rs:38:17 | LL | let _ = some_slice.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_slice[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:37:17 + --> $DIR/get_unwrap.rs:38:17 | LL | let _ = some_slice.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -37,13 +37,13 @@ LL | let _ = some_slice.get(0).unwrap(); = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:38:17 + --> $DIR/get_unwrap.rs:39:17 | LL | let _ = some_vec.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_vec[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:38:17 + --> $DIR/get_unwrap.rs:39:17 | LL | let _ = some_vec.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^ @@ -52,13 +52,13 @@ LL | let _ = some_vec.get(0).unwrap(); = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a VecDeque. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:39:17 + --> $DIR/get_unwrap.rs:40:17 | LL | let _ = some_vecdeque.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_vecdeque[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:39:17 + --> $DIR/get_unwrap.rs:40:17 | LL | let _ = some_vecdeque.get(0).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -67,13 +67,13 @@ LL | let _ = some_vecdeque.get(0).unwrap(); = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a HashMap. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:40:17 + --> $DIR/get_unwrap.rs:41:17 | LL | let _ = some_hashmap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_hashmap[&1]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:40:17 + --> $DIR/get_unwrap.rs:41:17 | LL | let _ = some_hashmap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -82,13 +82,13 @@ LL | let _ = some_hashmap.get(&1).unwrap(); = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a BTreeMap. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:41:17 + --> $DIR/get_unwrap.rs:42:17 | LL | let _ = some_btreemap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&some_btreemap[&1]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:41:17 + --> $DIR/get_unwrap.rs:42:17 | LL | let _ = some_btreemap.get(&1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -97,13 +97,13 @@ LL | let _ = some_btreemap.get(&1).unwrap(); = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:45:21 + --> $DIR/get_unwrap.rs:46:21 | LL | let _: u8 = *boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `boxed_slice[1]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:45:22 + --> $DIR/get_unwrap.rs:46:22 | LL | let _: u8 = *boxed_slice.get(1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -112,13 +112,13 @@ LL | let _: u8 = *boxed_slice.get(1).unwrap(); = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:50:9 + --> $DIR/get_unwrap.rs:51:9 | LL | *boxed_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `boxed_slice[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:50:10 + --> $DIR/get_unwrap.rs:51:10 | LL | *boxed_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -127,13 +127,13 @@ LL | *boxed_slice.get_mut(0).unwrap() = 1; = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:51:9 + --> $DIR/get_unwrap.rs:52:9 | LL | *some_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_slice[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:51:10 + --> $DIR/get_unwrap.rs:52:10 | LL | *some_slice.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -142,13 +142,13 @@ LL | *some_slice.get_mut(0).unwrap() = 1; = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:52:9 + --> $DIR/get_unwrap.rs:53:9 | LL | *some_vec.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:52:10 + --> $DIR/get_unwrap.rs:53:10 | LL | *some_vec.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -157,13 +157,13 @@ LL | *some_vec.get_mut(0).unwrap() = 1; = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a VecDeque. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:53:9 + --> $DIR/get_unwrap.rs:54:9 | LL | *some_vecdeque.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vecdeque[0]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:53:10 + --> $DIR/get_unwrap.rs:54:10 | LL | *some_vecdeque.get_mut(0).unwrap() = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -172,13 +172,13 @@ LL | *some_vecdeque.get_mut(0).unwrap() = 1; = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a Vec. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:65:17 + --> $DIR/get_unwrap.rs:66:17 | LL | let _ = some_vec.get(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[0..1]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:65:17 + --> $DIR/get_unwrap.rs:66:17 | LL | let _ = some_vec.get(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -187,13 +187,13 @@ LL | let _ = some_vec.get(0..1).unwrap().to_vec(); = help: consider using `expect()` to provide a better panic message error: called `.get_mut().unwrap()` on a Vec. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:66:17 + --> $DIR/get_unwrap.rs:67:17 | LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `some_vec[0..1]` error: used `unwrap()` on an `Option` value - --> $DIR/get_unwrap.rs:66:17 + --> $DIR/get_unwrap.rs:67:17 | LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -202,25 +202,25 @@ LL | let _ = some_vec.get_mut(0..1).unwrap().to_vec(); = help: consider using `expect()` to provide a better panic message error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:76:24 + --> $DIR/get_unwrap.rs:77:24 | LL | let _x: &i32 = f.get(1 + 2).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `&f[1 + 2]` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:79:18 + --> $DIR/get_unwrap.rs:80:18 | LL | let _x = f.get(1 + 2).unwrap().to_string(); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `f[1 + 2]` error: called `.get().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:82:18 + --> $DIR/get_unwrap.rs:83:18 | LL | let _x = f.get(1 + 2).unwrap().abs(); | ^^^^^^^^^^^^^^^^^^^^^ help: try: `f[1 + 2]` error: called `.get_mut().unwrap()` on a slice. Using `[]` is more clear and more concise - --> $DIR/get_unwrap.rs:99:33 + --> $DIR/get_unwrap.rs:100:33 | LL | let b = rest.get_mut(linidx(j, k) - linidx(i, k) - 1).unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut rest[linidx(j, k) - linidx(i, k) - 1]` diff --git a/tests/ui/if_then_some_else_none.rs b/tests/ui/if_then_some_else_none.rs index 77abd663e0a..abc92459148 100644 --- a/tests/ui/if_then_some_else_none.rs +++ b/tests/ui/if_then_some_else_none.rs @@ -1,4 +1,5 @@ #![warn(clippy::if_then_some_else_none)] +#![allow(clippy::redundant_pattern_matching)] fn main() { // Should issue an error. diff --git a/tests/ui/if_then_some_else_none.stderr b/tests/ui/if_then_some_else_none.stderr index 5c97b06da15..9b3d65cc803 100644 --- a/tests/ui/if_then_some_else_none.stderr +++ b/tests/ui/if_then_some_else_none.stderr @@ -1,5 +1,5 @@ error: this could be simplified with `bool::then` - --> $DIR/if_then_some_else_none.rs:5:13 + --> $DIR/if_then_some_else_none.rs:6:13 | LL | let _ = if foo() { | _____________^ @@ -16,7 +16,7 @@ LL | | }; = help: to override `-D warnings` add `#[allow(clippy::if_then_some_else_none)]` error: this could be simplified with `bool::then` - --> $DIR/if_then_some_else_none.rs:14:13 + --> $DIR/if_then_some_else_none.rs:15:13 | LL | let _ = if matches!(true, true) { | _____________^ @@ -31,7 +31,7 @@ LL | | }; = help: consider using `bool::then` like: `matches!(true, true).then(|| { /* snippet */ matches!(true, false) })` error: this could be simplified with `bool::then_some` - --> $DIR/if_then_some_else_none.rs:24:28 + --> $DIR/if_then_some_else_none.rs:25:28 | LL | let _ = x.and_then(|o| if o < 32 { Some(o) } else { None }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -39,7 +39,7 @@ LL | let _ = x.and_then(|o| if o < 32 { Some(o) } else { None }); = help: consider using `bool::then_some` like: `(o < 32).then_some(o)` error: this could be simplified with `bool::then_some` - --> $DIR/if_then_some_else_none.rs:29:13 + --> $DIR/if_then_some_else_none.rs:30:13 | LL | let _ = if !x { Some(0) } else { None }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -47,7 +47,7 @@ LL | let _ = if !x { Some(0) } else { None }; = help: consider using `bool::then_some` like: `(!x).then_some(0)` error: this could be simplified with `bool::then` - --> $DIR/if_then_some_else_none.rs:85:13 + --> $DIR/if_then_some_else_none.rs:86:13 | LL | let _ = if foo() { | _____________^ diff --git a/tests/ui/indexing_slicing_index.rs b/tests/ui/indexing_slicing_index.rs index f0da5dfc60b..1ac0bb11014 100644 --- a/tests/ui/indexing_slicing_index.rs +++ b/tests/ui/indexing_slicing_index.rs @@ -74,4 +74,7 @@ fn main() { //~^ ERROR: indexing may panic v[M]; //~^ ERROR: indexing may panic + + let slice = &x; + let _ = x[4]; } diff --git a/tests/ui/indexing_slicing_index.stderr b/tests/ui/indexing_slicing_index.stderr index 1c34875d2b8..6d64fa1e6cf 100644 --- a/tests/ui/indexing_slicing_index.stderr +++ b/tests/ui/indexing_slicing_index.stderr @@ -38,6 +38,21 @@ LL | x[index]; | = help: consider using `.get(n)` or `.get_mut(n)` instead +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:32:5 + | +LL | x[4]; + | ^^^^ + | + = note: `-D clippy::out-of-bounds-indexing` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::out_of_bounds_indexing)]` + +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:34:5 + | +LL | x[1 << 3]; + | ^^^^^^^^^ + error: indexing may panic --> $DIR/indexing_slicing_index.rs:45:14 | @@ -56,6 +71,12 @@ LL | const { &ARR[idx4()] }; = help: consider using `.get(n)` or `.get_mut(n)` instead = note: the suggestion might not be applicable in constant blocks +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:55:5 + | +LL | y[4]; + | ^^^^ + error: indexing may panic --> $DIR/indexing_slicing_index.rs:58:5 | @@ -80,6 +101,12 @@ LL | v[1 << 3]; | = help: consider using `.get(n)` or `.get_mut(n)` instead +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:70:5 + | +LL | x[N]; + | ^^^^ + error: indexing may panic --> $DIR/indexing_slicing_index.rs:73:5 | @@ -96,12 +123,18 @@ LL | v[M]; | = help: consider using `.get(n)` or `.get_mut(n)` instead +error: index is out of bounds + --> $DIR/indexing_slicing_index.rs:79:13 + | +LL | let _ = x[4]; + | ^^^^ + error[E0080]: evaluation of constant value failed --> $DIR/indexing_slicing_index.rs:16:24 | LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts. | ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4 -error: aborting due to 12 previous errors +error: aborting due to 17 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/tests/ui/infinite_loops.stderr b/tests/ui/infinite_loops.stderr index f58b3cebbc3..771fbfa44ee 100644 --- a/tests/ui/infinite_loops.stderr +++ b/tests/ui/infinite_loops.stderr @@ -9,7 +9,7 @@ LL | | } | = note: `-D clippy::infinite-loop` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::infinite_loop)]` -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn no_break() -> ! { | ++++ @@ -26,7 +26,7 @@ LL | | do_something(); LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn all_inf() -> ! { | ++++ @@ -43,7 +43,7 @@ LL | | } LL | | } | |_________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn all_inf() -> ! { | ++++ @@ -57,7 +57,7 @@ LL | | do_something(); LL | | } | |_____________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn all_inf() -> ! { | ++++ @@ -84,7 +84,7 @@ LL | | do_something(); LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn no_break_never_ret_noise() -> ! { | ++++ @@ -101,7 +101,7 @@ LL | | } LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn break_inner_but_not_outer_1(cond: bool) -> ! { | ++++ @@ -118,7 +118,7 @@ LL | | } LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn break_inner_but_not_outer_2(cond: bool) -> ! { | ++++ @@ -132,7 +132,7 @@ LL | | do_something(); LL | | } | |_________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn break_outer_but_not_inner() -> ! { | ++++ @@ -149,7 +149,7 @@ LL | | } LL | | } | |_________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn break_wrong_loop(cond: bool) -> ! { | ++++ @@ -166,7 +166,7 @@ LL | | } LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn match_like() -> ! { | ++++ @@ -180,7 +180,7 @@ LL | | let _x = matches!(result, Ok(v) if v != 0).then_some(0); LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn match_like() -> ! { | ++++ @@ -197,7 +197,7 @@ LL | | }); LL | | } | |_____^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn match_like() -> ! { | ++++ @@ -211,7 +211,7 @@ LL | | do_something(); LL | | } | |_________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn problematic_trait_method() -> ! { | ++++ @@ -225,7 +225,7 @@ LL | | do_something(); LL | | } | |_________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | fn could_be_problematic() -> ! { | ++++ @@ -239,7 +239,7 @@ LL | | do_something(); LL | | } | |_________^ | -help: if this is intentional, consider specifing `!` as function return +help: if this is intentional, consider specifying `!` as function return | LL | let _loop_forever = || -> ! { | ++++ diff --git a/tests/ui/iter_filter_is_ok.fixed b/tests/ui/iter_filter_is_ok.fixed new file mode 100644 index 00000000000..a5ca41528af --- /dev/null +++ b/tests/ui/iter_filter_is_ok.fixed @@ -0,0 +1,26 @@ +#![warn(clippy::iter_filter_is_ok)] + +fn main() { + let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().flatten(); + //~^ HELP: consider using `flatten` instead + let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().flatten(); + //~^ HELP: consider using `flatten` instead + + #[rustfmt::skip] + let _ = vec![Ok(1), Err(2)].into_iter().flatten(); + //~^ HELP: consider using `flatten` instead + + // Don't lint below + let mut counter = 0; + let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| { + counter += 1; + o.is_ok() + }); + let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| { + // Roses are red, + // Violets are blue, + // `Err` is not an `Option`, + // and this doesn't ryme + o.is_ok() + }); +} diff --git a/tests/ui/iter_filter_is_ok.rs b/tests/ui/iter_filter_is_ok.rs new file mode 100644 index 00000000000..e4e73f5ada1 --- /dev/null +++ b/tests/ui/iter_filter_is_ok.rs @@ -0,0 +1,26 @@ +#![warn(clippy::iter_filter_is_ok)] + +fn main() { + let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().filter(Result::is_ok); + //~^ HELP: consider using `flatten` instead + let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().filter(|a| a.is_ok()); + //~^ HELP: consider using `flatten` instead + + #[rustfmt::skip] + let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| { o.is_ok() }); + //~^ HELP: consider using `flatten` instead + + // Don't lint below + let mut counter = 0; + let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| { + counter += 1; + o.is_ok() + }); + let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| { + // Roses are red, + // Violets are blue, + // `Err` is not an `Option`, + // and this doesn't ryme + o.is_ok() + }); +} diff --git a/tests/ui/iter_filter_is_ok.stderr b/tests/ui/iter_filter_is_ok.stderr new file mode 100644 index 00000000000..f3acbe38d8a --- /dev/null +++ b/tests/ui/iter_filter_is_ok.stderr @@ -0,0 +1,23 @@ +error: `filter` for `is_ok` on iterator over `Result`s + --> $DIR/iter_filter_is_ok.rs:4:52 + | +LL | let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().filter(Result::is_ok); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` + | + = note: `-D clippy::iter-filter-is-ok` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::iter_filter_is_ok)]` + +error: `filter` for `is_ok` on iterator over `Result`s + --> $DIR/iter_filter_is_ok.rs:6:52 + | +LL | let _ = vec![Ok(1), Err(2), Ok(3)].into_iter().filter(|a| a.is_ok()); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` + +error: `filter` for `is_ok` on iterator over `Result`s + --> $DIR/iter_filter_is_ok.rs:10:45 + | +LL | let _ = vec![Ok(1), Err(2)].into_iter().filter(|o| { o.is_ok() }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/iter_filter_is_some.fixed b/tests/ui/iter_filter_is_some.fixed new file mode 100644 index 00000000000..c3fa93f0ab2 --- /dev/null +++ b/tests/ui/iter_filter_is_some.fixed @@ -0,0 +1,27 @@ +#![warn(clippy::iter_filter_is_some)] + +fn main() { + let _ = vec![Some(1)].into_iter().flatten(); + //~^ HELP: consider using `flatten` instead + let _ = vec![Some(1)].into_iter().flatten(); + //~^ HELP: consider using `flatten` instead + + #[rustfmt::skip] + let _ = vec![Some(1)].into_iter().flatten(); + //~^ HELP: consider using `flatten` instead + + // Don't lint below + let mut counter = 0; + let _ = vec![Some(1)].into_iter().filter(|o| { + counter += 1; + o.is_some() + }); + + let _ = vec![Some(1)].into_iter().filter(|o| { + // Roses are red, + // Violets are blue, + // `Err` is not an `Option`, + // and this doesn't ryme + o.is_some() + }); +} diff --git a/tests/ui/iter_filter_is_some.rs b/tests/ui/iter_filter_is_some.rs new file mode 100644 index 00000000000..b023776abe4 --- /dev/null +++ b/tests/ui/iter_filter_is_some.rs @@ -0,0 +1,27 @@ +#![warn(clippy::iter_filter_is_some)] + +fn main() { + let _ = vec![Some(1)].into_iter().filter(Option::is_some); + //~^ HELP: consider using `flatten` instead + let _ = vec![Some(1)].into_iter().filter(|o| o.is_some()); + //~^ HELP: consider using `flatten` instead + + #[rustfmt::skip] + let _ = vec![Some(1)].into_iter().filter(|o| { o.is_some() }); + //~^ HELP: consider using `flatten` instead + + // Don't lint below + let mut counter = 0; + let _ = vec![Some(1)].into_iter().filter(|o| { + counter += 1; + o.is_some() + }); + + let _ = vec![Some(1)].into_iter().filter(|o| { + // Roses are red, + // Violets are blue, + // `Err` is not an `Option`, + // and this doesn't ryme + o.is_some() + }); +} diff --git a/tests/ui/iter_filter_is_some.stderr b/tests/ui/iter_filter_is_some.stderr new file mode 100644 index 00000000000..1f2b10036fe --- /dev/null +++ b/tests/ui/iter_filter_is_some.stderr @@ -0,0 +1,23 @@ +error: `filter` for `is_some` on iterator over `Option` + --> $DIR/iter_filter_is_some.rs:4:39 + | +LL | let _ = vec![Some(1)].into_iter().filter(Option::is_some); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` + | + = note: `-D clippy::iter-filter-is-some` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::iter_filter_is_some)]` + +error: `filter` for `is_some` on iterator over `Option` + --> $DIR/iter_filter_is_some.rs:6:39 + | +LL | let _ = vec![Some(1)].into_iter().filter(|o| o.is_some()); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` + +error: `filter` for `is_some` on iterator over `Option` + --> $DIR/iter_filter_is_some.rs:10:39 + | +LL | let _ = vec![Some(1)].into_iter().filter(|o| { o.is_some() }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` + +error: aborting due to 3 previous errors + diff --git a/tests/ui/manual_let_else_question_mark.fixed b/tests/ui/manual_let_else_question_mark.fixed index b555186cc22..6b29ce75985 100644 --- a/tests/ui/manual_let_else_question_mark.fixed +++ b/tests/ui/manual_let_else_question_mark.fixed @@ -60,3 +60,24 @@ fn foo() -> Option<()> { Some(()) } + +// lint not just `return None`, but also `return None;` (note the semicolon) +fn issue11993(y: Option) -> Option { + let x = y?; + + // don't lint: more than one statement in the else body + let Some(x) = y else { + todo!(); + return None; + }; + + let Some(x) = y else { + // Roses are red, + // violets are blue, + // please keep this comment, + // it's art, you know? + return None; + }; + + None +} diff --git a/tests/ui/manual_let_else_question_mark.rs b/tests/ui/manual_let_else_question_mark.rs index 5852c7094a4..e92c4c1375e 100644 --- a/tests/ui/manual_let_else_question_mark.rs +++ b/tests/ui/manual_let_else_question_mark.rs @@ -65,3 +65,26 @@ fn foo() -> Option<()> { Some(()) } + +// lint not just `return None`, but also `return None;` (note the semicolon) +fn issue11993(y: Option) -> Option { + let Some(x) = y else { + return None; + }; + + // don't lint: more than one statement in the else body + let Some(x) = y else { + todo!(); + return None; + }; + + let Some(x) = y else { + // Roses are red, + // violets are blue, + // please keep this comment, + // it's art, you know? + return None; + }; + + None +} diff --git a/tests/ui/manual_let_else_question_mark.stderr b/tests/ui/manual_let_else_question_mark.stderr index bf0b1bbf0dd..dec6947697a 100644 --- a/tests/ui/manual_let_else_question_mark.stderr +++ b/tests/ui/manual_let_else_question_mark.stderr @@ -53,5 +53,13 @@ error: this could be rewritten as `let...else` LL | let v = if let Some(v_some) = g() { v_some } else { return None }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider writing: `let Some(v) = g() else { return None };` -error: aborting due to 6 previous errors +error: this `let...else` may be rewritten with the `?` operator + --> $DIR/manual_let_else_question_mark.rs:71:5 + | +LL | / let Some(x) = y else { +LL | | return None; +LL | | }; + | |______^ help: replace it with: `let x = y?;` + +error: aborting due to 7 previous errors diff --git a/tests/ui/needless_if.fixed b/tests/ui/needless_if.fixed index 1086ae2c984..79e33a7218b 100644 --- a/tests/ui/needless_if.fixed +++ b/tests/ui/needless_if.fixed @@ -10,6 +10,7 @@ clippy::nonminimal_bool, clippy::short_circuit_statement, clippy::unnecessary_operation, + clippy::redundant_pattern_matching, unused )] #![warn(clippy::needless_if)] diff --git a/tests/ui/needless_if.rs b/tests/ui/needless_if.rs index 131cceaf712..2c135fb22bf 100644 --- a/tests/ui/needless_if.rs +++ b/tests/ui/needless_if.rs @@ -10,6 +10,7 @@ clippy::nonminimal_bool, clippy::short_circuit_statement, clippy::unnecessary_operation, + clippy::redundant_pattern_matching, unused )] #![warn(clippy::needless_if)] diff --git a/tests/ui/needless_if.stderr b/tests/ui/needless_if.stderr index c3e83c0f1f5..9a911b4dbac 100644 --- a/tests/ui/needless_if.stderr +++ b/tests/ui/needless_if.stderr @@ -1,5 +1,5 @@ error: this `if` branch is empty - --> $DIR/needless_if.rs:26:5 + --> $DIR/needless_if.rs:27:5 | LL | if (true) {} | ^^^^^^^^^^^^ help: you can remove it @@ -8,13 +8,13 @@ LL | if (true) {} = help: to override `-D warnings` add `#[allow(clippy::needless_if)]` error: this `if` branch is empty - --> $DIR/needless_if.rs:28:5 + --> $DIR/needless_if.rs:29:5 | LL | if maybe_side_effect() {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `maybe_side_effect();` error: this `if` branch is empty - --> $DIR/needless_if.rs:33:5 + --> $DIR/needless_if.rs:34:5 | LL | / if { LL | | return; @@ -29,7 +29,7 @@ LL + }); | error: this `if` branch is empty - --> $DIR/needless_if.rs:49:5 + --> $DIR/needless_if.rs:50:5 | LL | / if { LL | | if let true = true @@ -54,19 +54,19 @@ LL + } && true); | error: this `if` branch is empty - --> $DIR/needless_if.rs:93:5 + --> $DIR/needless_if.rs:94:5 | LL | if { maybe_side_effect() } {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `({ maybe_side_effect() });` error: this `if` branch is empty - --> $DIR/needless_if.rs:95:5 + --> $DIR/needless_if.rs:96:5 | LL | if { maybe_side_effect() } && true {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: you can remove it: `({ maybe_side_effect() } && true);` error: this `if` branch is empty - --> $DIR/needless_if.rs:99:5 + --> $DIR/needless_if.rs:100:5 | LL | if true {} | ^^^^^^^^^^ help: you can remove it: `true;` diff --git a/tests/ui/nonminimal_bool.rs b/tests/ui/nonminimal_bool.rs index da7876e772e..4d48ef14d31 100644 --- a/tests/ui/nonminimal_bool.rs +++ b/tests/ui/nonminimal_bool.rs @@ -1,6 +1,11 @@ //@no-rustfix: overlapping suggestions #![feature(lint_reasons)] -#![allow(unused, clippy::diverging_sub_expression, clippy::needless_if)] +#![allow( + unused, + clippy::diverging_sub_expression, + clippy::needless_if, + clippy::redundant_pattern_matching +)] #![warn(clippy::nonminimal_bool)] #![allow(clippy::useless_vec)] diff --git a/tests/ui/nonminimal_bool.stderr b/tests/ui/nonminimal_bool.stderr index deae389dbef..fd1568d94e3 100644 --- a/tests/ui/nonminimal_bool.stderr +++ b/tests/ui/nonminimal_bool.stderr @@ -1,5 +1,5 @@ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:13:13 + --> $DIR/nonminimal_bool.rs:18:13 | LL | let _ = !true; | ^^^^^ help: try: `false` @@ -8,43 +8,43 @@ LL | let _ = !true; = help: to override `-D warnings` add `#[allow(clippy::nonminimal_bool)]` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:16:13 + --> $DIR/nonminimal_bool.rs:21:13 | LL | let _ = !false; | ^^^^^^ help: try: `true` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:18:13 + --> $DIR/nonminimal_bool.rs:23:13 | LL | let _ = !!a; | ^^^ help: try: `a` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:20:13 + --> $DIR/nonminimal_bool.rs:25:13 | LL | let _ = false || a; | ^^^^^^^^^^ help: try: `a` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:25:13 + --> $DIR/nonminimal_bool.rs:30:13 | LL | let _ = !(!a && b); | ^^^^^^^^^^ help: try: `a || !b` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:27:13 + --> $DIR/nonminimal_bool.rs:32:13 | LL | let _ = !(!a || b); | ^^^^^^^^^^ help: try: `a && !b` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:29:13 + --> $DIR/nonminimal_bool.rs:34:13 | LL | let _ = !a && !(b && c); | ^^^^^^^^^^^^^^^ help: try: `!(a || b && c)` error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:38:13 + --> $DIR/nonminimal_bool.rs:43:13 | LL | let _ = a == b && c == 5 && a == b; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -57,7 +57,7 @@ LL | let _ = a == b && c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:40:13 + --> $DIR/nonminimal_bool.rs:45:13 | LL | let _ = a == b || c == 5 || a == b; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -70,7 +70,7 @@ LL | let _ = a == b || c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:42:13 + --> $DIR/nonminimal_bool.rs:47:13 | LL | let _ = a == b && c == 5 && b == a; | ^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -83,7 +83,7 @@ LL | let _ = a == b && c == 5; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:44:13 + --> $DIR/nonminimal_bool.rs:49:13 | LL | let _ = a != b || !(a != b || c == d); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -96,7 +96,7 @@ LL | let _ = a != b || c != d; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:46:13 + --> $DIR/nonminimal_bool.rs:51:13 | LL | let _ = a != b && !(a != b && c == d); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -109,7 +109,7 @@ LL | let _ = a != b && c != d; | ~~~~~~~~~~~~~~~~ error: this boolean expression can be simplified - --> $DIR/nonminimal_bool.rs:77:8 + --> $DIR/nonminimal_bool.rs:82:8 | LL | if matches!(true, true) && true { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `matches!(true, true)` diff --git a/tests/ui/option_filter_map.fixed b/tests/ui/option_filter_map.fixed index ee004c0e194..b1c3cfa48a4 100644 --- a/tests/ui/option_filter_map.fixed +++ b/tests/ui/option_filter_map.fixed @@ -3,12 +3,18 @@ fn main() { let _ = Some(Some(1)).flatten(); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = Some(Some(1)).flatten(); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = Some(1).map(odds_out).flatten(); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = Some(1).map(odds_out).flatten(); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = vec![Some(1)].into_iter().flatten(); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = vec![Some(1)].into_iter().flatten(); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = vec![1] .into_iter() .map(odds_out) diff --git a/tests/ui/option_filter_map.rs b/tests/ui/option_filter_map.rs index eae2fa176a8..2550b9cd2b3 100644 --- a/tests/ui/option_filter_map.rs +++ b/tests/ui/option_filter_map.rs @@ -3,21 +3,29 @@ fn main() { let _ = Some(Some(1)).filter(Option::is_some).map(Option::unwrap); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = Some(Some(1)).filter(|o| o.is_some()).map(|o| o.unwrap()); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = Some(1).map(odds_out).filter(Option::is_some).map(Option::unwrap); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = Some(1).map(odds_out).filter(|o| o.is_some()).map(|o| o.unwrap()); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = vec![Some(1)].into_iter().filter(Option::is_some).map(Option::unwrap); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = vec![Some(1)].into_iter().filter(|o| o.is_some()).map(|o| o.unwrap()); + //~^ ERROR: `filter` for `Some` followed by `unwrap` let _ = vec![1] .into_iter() .map(odds_out) .filter(Option::is_some) + //~^ ERROR: `filter` for `Some` followed by `unwrap` .map(Option::unwrap); let _ = vec![1] .into_iter() .map(odds_out) .filter(|o| o.is_some()) + //~^ ERROR: `filter` for `Some` followed by `unwrap` .map(|o| o.unwrap()); } diff --git a/tests/ui/option_filter_map.stderr b/tests/ui/option_filter_map.stderr index 148f9d02f5e..6a0fc10822b 100644 --- a/tests/ui/option_filter_map.stderr +++ b/tests/ui/option_filter_map.stderr @@ -8,48 +8,50 @@ LL | let _ = Some(Some(1)).filter(Option::is_some).map(Option::unwrap); = help: to override `-D warnings` add `#[allow(clippy::option_filter_map)]` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:6:27 + --> $DIR/option_filter_map.rs:7:27 | LL | let _ = Some(Some(1)).filter(|o| o.is_some()).map(|o| o.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:7:35 + --> $DIR/option_filter_map.rs:9:35 | LL | let _ = Some(1).map(odds_out).filter(Option::is_some).map(Option::unwrap); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:8:35 + --> $DIR/option_filter_map.rs:11:35 | LL | let _ = Some(1).map(odds_out).filter(|o| o.is_some()).map(|o| o.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:10:39 + --> $DIR/option_filter_map.rs:14:39 | LL | let _ = vec![Some(1)].into_iter().filter(Option::is_some).map(Option::unwrap); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:11:39 + --> $DIR/option_filter_map.rs:16:39 | LL | let _ = vec![Some(1)].into_iter().filter(|o| o.is_some()).map(|o| o.unwrap()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `flatten` instead: `flatten()` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:15:10 + --> $DIR/option_filter_map.rs:21:10 | LL | .filter(Option::is_some) | __________^ +LL | | LL | | .map(Option::unwrap); | |____________________________^ help: consider using `flatten` instead: `flatten()` error: `filter` for `Some` followed by `unwrap` - --> $DIR/option_filter_map.rs:20:10 + --> $DIR/option_filter_map.rs:27:10 | LL | .filter(|o| o.is_some()) | __________^ +LL | | LL | | .map(|o| o.unwrap()); | |____________________________^ help: consider using `flatten` instead: `flatten()` diff --git a/tests/ui/redundant_async_block.fixed b/tests/ui/redundant_async_block.fixed index d492ea1be75..a1875c1c06e 100644 --- a/tests/ui/redundant_async_block.fixed +++ b/tests/ui/redundant_async_block.fixed @@ -1,7 +1,7 @@ #![allow(unused, clippy::manual_async_fn)] #![warn(clippy::redundant_async_block)] -use std::future::Future; +use std::future::{Future, IntoFuture}; async fn func1(n: usize) -> usize { n + 1 @@ -189,3 +189,9 @@ fn await_from_macro_deep() -> impl Future { // or return different things depending on its argument async { mac!(async { 42 }) } } + +// Issue 11959 +fn from_into_future(a: impl IntoFuture) -> impl Future { + // Do not lint: `a` is not equivalent to this expression + async { a.await } +} diff --git a/tests/ui/redundant_async_block.rs b/tests/ui/redundant_async_block.rs index dd96e141006..bb43403a043 100644 --- a/tests/ui/redundant_async_block.rs +++ b/tests/ui/redundant_async_block.rs @@ -1,7 +1,7 @@ #![allow(unused, clippy::manual_async_fn)] #![warn(clippy::redundant_async_block)] -use std::future::Future; +use std::future::{Future, IntoFuture}; async fn func1(n: usize) -> usize { n + 1 @@ -189,3 +189,9 @@ macro_rules! mac { // or return different things depending on its argument async { mac!(async { 42 }) } } + +// Issue 11959 +fn from_into_future(a: impl IntoFuture) -> impl Future { + // Do not lint: `a` is not equivalent to this expression + async { a.await } +} diff --git a/tests/ui/redundant_pattern_matching_if_let_true.fixed b/tests/ui/redundant_pattern_matching_if_let_true.fixed new file mode 100644 index 00000000000..6d910678934 --- /dev/null +++ b/tests/ui/redundant_pattern_matching_if_let_true.fixed @@ -0,0 +1,38 @@ +#![warn(clippy::redundant_pattern_matching)] +#![allow(clippy::needless_if, clippy::no_effect, clippy::nonminimal_bool)] + +macro_rules! condition { + () => { + true + }; +} + +macro_rules! lettrue { + (if) => { + if let true = true {} + }; + (while) => { + while let true = true {} + }; +} + +fn main() { + let mut k = 5; + + if k > 1 {} + if !(k > 5) {} + if k > 1 {} + if let (true, true) = (k > 1, k > 2) {} + while k > 1 { + k += 1; + } + while condition!() { + k += 1; + } + + k > 5; + !(k > 5); + // Whole loop is from a macro expansion, don't lint: + lettrue!(if); + lettrue!(while); +} diff --git a/tests/ui/redundant_pattern_matching_if_let_true.rs b/tests/ui/redundant_pattern_matching_if_let_true.rs new file mode 100644 index 00000000000..a82e673982a --- /dev/null +++ b/tests/ui/redundant_pattern_matching_if_let_true.rs @@ -0,0 +1,38 @@ +#![warn(clippy::redundant_pattern_matching)] +#![allow(clippy::needless_if, clippy::no_effect, clippy::nonminimal_bool)] + +macro_rules! condition { + () => { + true + }; +} + +macro_rules! lettrue { + (if) => { + if let true = true {} + }; + (while) => { + while let true = true {} + }; +} + +fn main() { + let mut k = 5; + + if let true = k > 1 {} + if let false = k > 5 {} + if let (true) = k > 1 {} + if let (true, true) = (k > 1, k > 2) {} + while let true = k > 1 { + k += 1; + } + while let true = condition!() { + k += 1; + } + + matches!(k > 5, true); + matches!(k > 5, false); + // Whole loop is from a macro expansion, don't lint: + lettrue!(if); + lettrue!(while); +} diff --git a/tests/ui/redundant_pattern_matching_if_let_true.stderr b/tests/ui/redundant_pattern_matching_if_let_true.stderr new file mode 100644 index 00000000000..211a332d79a --- /dev/null +++ b/tests/ui/redundant_pattern_matching_if_let_true.stderr @@ -0,0 +1,47 @@ +error: using `if let` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:22:8 + | +LL | if let true = k > 1 {} + | ^^^^^^^^^^^^^^^^ help: consider using the condition directly: `k > 1` + | + = note: `-D clippy::redundant-pattern-matching` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::redundant_pattern_matching)]` + +error: using `if let` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:23:8 + | +LL | if let false = k > 5 {} + | ^^^^^^^^^^^^^^^^^ help: consider using the condition directly: `!(k > 5)` + +error: using `if let` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:24:8 + | +LL | if let (true) = k > 1 {} + | ^^^^^^^^^^^^^^^^^^ help: consider using the condition directly: `k > 1` + +error: using `if let` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:26:11 + | +LL | while let true = k > 1 { + | ^^^^^^^^^^^^^^^^ help: consider using the condition directly: `k > 1` + +error: using `if let` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:29:11 + | +LL | while let true = condition!() { + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using the condition directly: `condition!()` + +error: using `matches!` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:33:5 + | +LL | matches!(k > 5, true); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using the condition directly: `k > 5` + +error: using `matches!` to pattern match a bool + --> $DIR/redundant_pattern_matching_if_let_true.rs:34:5 + | +LL | matches!(k > 5, false); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using the condition directly: `!(k > 5)` + +error: aborting due to 7 previous errors + diff --git a/tests/ui/redundant_pattern_matching_ipaddr.fixed b/tests/ui/redundant_pattern_matching_ipaddr.fixed index 70dd9fc250f..429d33118a5 100644 --- a/tests/ui/redundant_pattern_matching_ipaddr.fixed +++ b/tests/ui/redundant_pattern_matching_ipaddr.fixed @@ -18,6 +18,12 @@ fn main() { if V6(Ipv6Addr::LOCALHOST).is_ipv6() {} + // Issue 6459 + if V4(Ipv4Addr::LOCALHOST).is_ipv4() {} + + // Issue 6459 + if V6(Ipv6Addr::LOCALHOST).is_ipv6() {} + while V4(Ipv4Addr::LOCALHOST).is_ipv4() {} while V6(Ipv6Addr::LOCALHOST).is_ipv6() {} diff --git a/tests/ui/redundant_pattern_matching_ipaddr.rs b/tests/ui/redundant_pattern_matching_ipaddr.rs index 6e2a2f7b6d2..e7136b72c20 100644 --- a/tests/ui/redundant_pattern_matching_ipaddr.rs +++ b/tests/ui/redundant_pattern_matching_ipaddr.rs @@ -18,6 +18,12 @@ fn main() { if let V6(_) = V6(Ipv6Addr::LOCALHOST) {} + // Issue 6459 + if matches!(V4(Ipv4Addr::LOCALHOST), V4(_)) {} + + // Issue 6459 + if matches!(V6(Ipv6Addr::LOCALHOST), V6(_)) {} + while let V4(_) = V4(Ipv4Addr::LOCALHOST) {} while let V6(_) = V6(Ipv6Addr::LOCALHOST) {} diff --git a/tests/ui/redundant_pattern_matching_ipaddr.stderr b/tests/ui/redundant_pattern_matching_ipaddr.stderr index d36129a2bee..54cefa5e82b 100644 --- a/tests/ui/redundant_pattern_matching_ipaddr.stderr +++ b/tests/ui/redundant_pattern_matching_ipaddr.stderr @@ -20,19 +20,31 @@ LL | if let V6(_) = V6(Ipv6Addr::LOCALHOST) {} | -------^^^^^-------------------------- help: try: `if V6(Ipv6Addr::LOCALHOST).is_ipv6()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:21:15 + --> $DIR/redundant_pattern_matching_ipaddr.rs:22:8 + | +LL | if matches!(V4(Ipv4Addr::LOCALHOST), V4(_)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `V4(Ipv4Addr::LOCALHOST).is_ipv4()` + +error: redundant pattern matching, consider using `is_ipv6()` + --> $DIR/redundant_pattern_matching_ipaddr.rs:25:8 + | +LL | if matches!(V6(Ipv6Addr::LOCALHOST), V6(_)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `V6(Ipv6Addr::LOCALHOST).is_ipv6()` + +error: redundant pattern matching, consider using `is_ipv4()` + --> $DIR/redundant_pattern_matching_ipaddr.rs:27:15 | LL | while let V4(_) = V4(Ipv4Addr::LOCALHOST) {} | ----------^^^^^-------------------------- help: try: `while V4(Ipv4Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:23:15 + --> $DIR/redundant_pattern_matching_ipaddr.rs:29:15 | LL | while let V6(_) = V6(Ipv6Addr::LOCALHOST) {} | ----------^^^^^-------------------------- help: try: `while V6(Ipv6Addr::LOCALHOST).is_ipv6()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:33:5 + --> $DIR/redundant_pattern_matching_ipaddr.rs:39:5 | LL | / match V4(Ipv4Addr::LOCALHOST) { LL | | V4(_) => true, @@ -41,7 +53,7 @@ LL | | }; | |_____^ help: try: `V4(Ipv4Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:38:5 + --> $DIR/redundant_pattern_matching_ipaddr.rs:44:5 | LL | / match V4(Ipv4Addr::LOCALHOST) { LL | | V4(_) => false, @@ -50,7 +62,7 @@ LL | | }; | |_____^ help: try: `V4(Ipv4Addr::LOCALHOST).is_ipv6()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:43:5 + --> $DIR/redundant_pattern_matching_ipaddr.rs:49:5 | LL | / match V6(Ipv6Addr::LOCALHOST) { LL | | V4(_) => false, @@ -59,7 +71,7 @@ LL | | }; | |_____^ help: try: `V6(Ipv6Addr::LOCALHOST).is_ipv6()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:48:5 + --> $DIR/redundant_pattern_matching_ipaddr.rs:54:5 | LL | / match V6(Ipv6Addr::LOCALHOST) { LL | | V4(_) => true, @@ -68,49 +80,49 @@ LL | | }; | |_____^ help: try: `V6(Ipv6Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:53:20 + --> $DIR/redundant_pattern_matching_ipaddr.rs:59:20 | LL | let _ = if let V4(_) = V4(Ipv4Addr::LOCALHOST) { | -------^^^^^-------------------------- help: try: `if V4(Ipv4Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:61:20 + --> $DIR/redundant_pattern_matching_ipaddr.rs:67:20 | LL | let _ = if let V4(_) = gen_ipaddr() { | -------^^^^^--------------- help: try: `if gen_ipaddr().is_ipv4()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:63:19 + --> $DIR/redundant_pattern_matching_ipaddr.rs:69:19 | LL | } else if let V6(_) = gen_ipaddr() { | -------^^^^^--------------- help: try: `if gen_ipaddr().is_ipv6()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:75:12 + --> $DIR/redundant_pattern_matching_ipaddr.rs:81:12 | LL | if let V4(_) = V4(Ipv4Addr::LOCALHOST) {} | -------^^^^^-------------------------- help: try: `if V4(Ipv4Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:77:12 + --> $DIR/redundant_pattern_matching_ipaddr.rs:83:12 | LL | if let V6(_) = V6(Ipv6Addr::LOCALHOST) {} | -------^^^^^-------------------------- help: try: `if V6(Ipv6Addr::LOCALHOST).is_ipv6()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:79:15 + --> $DIR/redundant_pattern_matching_ipaddr.rs:85:15 | LL | while let V4(_) = V4(Ipv4Addr::LOCALHOST) {} | ----------^^^^^-------------------------- help: try: `while V4(Ipv4Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:81:15 + --> $DIR/redundant_pattern_matching_ipaddr.rs:87:15 | LL | while let V6(_) = V6(Ipv6Addr::LOCALHOST) {} | ----------^^^^^-------------------------- help: try: `while V6(Ipv6Addr::LOCALHOST).is_ipv6()` error: redundant pattern matching, consider using `is_ipv4()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:83:5 + --> $DIR/redundant_pattern_matching_ipaddr.rs:89:5 | LL | / match V4(Ipv4Addr::LOCALHOST) { LL | | V4(_) => true, @@ -119,7 +131,7 @@ LL | | }; | |_____^ help: try: `V4(Ipv4Addr::LOCALHOST).is_ipv4()` error: redundant pattern matching, consider using `is_ipv6()` - --> $DIR/redundant_pattern_matching_ipaddr.rs:88:5 + --> $DIR/redundant_pattern_matching_ipaddr.rs:94:5 | LL | / match V6(Ipv6Addr::LOCALHOST) { LL | | V4(_) => false, @@ -127,5 +139,5 @@ LL | | V6(_) => true, LL | | }; | |_____^ help: try: `V6(Ipv6Addr::LOCALHOST).is_ipv6()` -error: aborting due to 18 previous errors +error: aborting due to 20 previous errors diff --git a/tests/ui/redundant_pattern_matching_poll.fixed b/tests/ui/redundant_pattern_matching_poll.fixed index 718c2f8ea3d..08d83f87e4c 100644 --- a/tests/ui/redundant_pattern_matching_poll.fixed +++ b/tests/ui/redundant_pattern_matching_poll.fixed @@ -22,6 +22,12 @@ fn main() { bar(); } + // Issue 6459 + if Ready(42).is_ready() {} + + // Issue 6459 + if Pending::<()>.is_pending() {} + while Ready(42).is_ready() {} while Ready(42).is_pending() {} diff --git a/tests/ui/redundant_pattern_matching_poll.rs b/tests/ui/redundant_pattern_matching_poll.rs index daa4761aff5..7bc2b3be4d3 100644 --- a/tests/ui/redundant_pattern_matching_poll.rs +++ b/tests/ui/redundant_pattern_matching_poll.rs @@ -22,6 +22,12 @@ fn main() { bar(); } + // Issue 6459 + if matches!(Ready(42), Ready(_)) {} + + // Issue 6459 + if matches!(Pending::<()>, Pending) {} + while let Ready(_) = Ready(42) {} while let Pending = Ready(42) {} diff --git a/tests/ui/redundant_pattern_matching_poll.stderr b/tests/ui/redundant_pattern_matching_poll.stderr index c010c3c4437..de64331b4e8 100644 --- a/tests/ui/redundant_pattern_matching_poll.stderr +++ b/tests/ui/redundant_pattern_matching_poll.stderr @@ -20,25 +20,37 @@ LL | if let Ready(_) = Ready(42) { | -------^^^^^^^^------------ help: try: `if Ready(42).is_ready()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:25:15 + --> $DIR/redundant_pattern_matching_poll.rs:26:8 + | +LL | if matches!(Ready(42), Ready(_)) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ready(42).is_ready()` + +error: redundant pattern matching, consider using `is_pending()` + --> $DIR/redundant_pattern_matching_poll.rs:29:8 + | +LL | if matches!(Pending::<()>, Pending) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Pending::<()>.is_pending()` + +error: redundant pattern matching, consider using `is_ready()` + --> $DIR/redundant_pattern_matching_poll.rs:31:15 | LL | while let Ready(_) = Ready(42) {} | ----------^^^^^^^^------------ help: try: `while Ready(42).is_ready()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:27:15 + --> $DIR/redundant_pattern_matching_poll.rs:33:15 | LL | while let Pending = Ready(42) {} | ----------^^^^^^^------------ help: try: `while Ready(42).is_pending()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:29:15 + --> $DIR/redundant_pattern_matching_poll.rs:35:15 | LL | while let Pending = Pending::<()> {} | ----------^^^^^^^---------------- help: try: `while Pending::<()>.is_pending()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:35:5 + --> $DIR/redundant_pattern_matching_poll.rs:41:5 | LL | / match Ready(42) { LL | | Ready(_) => true, @@ -47,7 +59,7 @@ LL | | }; | |_____^ help: try: `Ready(42).is_ready()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:40:5 + --> $DIR/redundant_pattern_matching_poll.rs:46:5 | LL | / match Pending::<()> { LL | | Ready(_) => false, @@ -56,7 +68,7 @@ LL | | }; | |_____^ help: try: `Pending::<()>.is_pending()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:45:13 + --> $DIR/redundant_pattern_matching_poll.rs:51:13 | LL | let _ = match Pending::<()> { | _____________^ @@ -66,49 +78,49 @@ LL | | }; | |_____^ help: try: `Pending::<()>.is_pending()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:51:20 + --> $DIR/redundant_pattern_matching_poll.rs:57:20 | LL | let _ = if let Ready(_) = poll { true } else { false }; | -------^^^^^^^^------- help: try: `if poll.is_ready()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:55:20 + --> $DIR/redundant_pattern_matching_poll.rs:61:20 | LL | let _ = if let Ready(_) = gen_poll() { | -------^^^^^^^^------------- help: try: `if gen_poll().is_ready()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:57:19 + --> $DIR/redundant_pattern_matching_poll.rs:63:19 | LL | } else if let Pending = gen_poll() { | -------^^^^^^^------------- help: try: `if gen_poll().is_pending()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:73:12 + --> $DIR/redundant_pattern_matching_poll.rs:79:12 | LL | if let Ready(_) = Ready(42) {} | -------^^^^^^^^------------ help: try: `if Ready(42).is_ready()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:75:12 + --> $DIR/redundant_pattern_matching_poll.rs:81:12 | LL | if let Pending = Pending::<()> {} | -------^^^^^^^---------------- help: try: `if Pending::<()>.is_pending()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:77:15 + --> $DIR/redundant_pattern_matching_poll.rs:83:15 | LL | while let Ready(_) = Ready(42) {} | ----------^^^^^^^^------------ help: try: `while Ready(42).is_ready()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:79:15 + --> $DIR/redundant_pattern_matching_poll.rs:85:15 | LL | while let Pending = Pending::<()> {} | ----------^^^^^^^---------------- help: try: `while Pending::<()>.is_pending()` error: redundant pattern matching, consider using `is_ready()` - --> $DIR/redundant_pattern_matching_poll.rs:81:5 + --> $DIR/redundant_pattern_matching_poll.rs:87:5 | LL | / match Ready(42) { LL | | Ready(_) => true, @@ -117,7 +129,7 @@ LL | | }; | |_____^ help: try: `Ready(42).is_ready()` error: redundant pattern matching, consider using `is_pending()` - --> $DIR/redundant_pattern_matching_poll.rs:86:5 + --> $DIR/redundant_pattern_matching_poll.rs:92:5 | LL | / match Pending::<()> { LL | | Ready(_) => false, @@ -125,5 +137,5 @@ LL | | Pending => true, LL | | }; | |_____^ help: try: `Pending::<()>.is_pending()` -error: aborting due to 18 previous errors +error: aborting due to 20 previous errors diff --git a/tests/ui/result_filter_map.fixed b/tests/ui/result_filter_map.fixed new file mode 100644 index 00000000000..e8943dbc72d --- /dev/null +++ b/tests/ui/result_filter_map.fixed @@ -0,0 +1,27 @@ +#![warn(clippy::result_filter_map)] +#![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)] + +fn odds_out(x: i32) -> Result { + if x % 2 == 0 { Ok(x) } else { Err(()) } +} + +fn main() { + // Unlike the Option version, Result does not have a filter operation => we will check for iterators + // only + let _ = vec![Ok(1) as Result] + .into_iter() + .flatten(); + + let _ = vec![Ok(1) as Result] + .into_iter() + .flatten(); + + let _ = vec![1] + .into_iter() + .map(odds_out) + .flatten(); + let _ = vec![1] + .into_iter() + .map(odds_out) + .flatten(); +} diff --git a/tests/ui/result_filter_map.rs b/tests/ui/result_filter_map.rs new file mode 100644 index 00000000000..bfe47ffcf38 --- /dev/null +++ b/tests/ui/result_filter_map.rs @@ -0,0 +1,35 @@ +#![warn(clippy::result_filter_map)] +#![allow(clippy::map_flatten, clippy::unnecessary_map_on_constructor)] + +fn odds_out(x: i32) -> Result { + if x % 2 == 0 { Ok(x) } else { Err(()) } +} + +fn main() { + // Unlike the Option version, Result does not have a filter operation => we will check for iterators + // only + let _ = vec![Ok(1) as Result] + .into_iter() + .filter(Result::is_ok) + //~^ ERROR: `filter` for `Ok` followed by `unwrap` + .map(Result::unwrap); + + let _ = vec![Ok(1) as Result] + .into_iter() + .filter(|o| o.is_ok()) + //~^ ERROR: `filter` for `Ok` followed by `unwrap` + .map(|o| o.unwrap()); + + let _ = vec![1] + .into_iter() + .map(odds_out) + .filter(Result::is_ok) + //~^ ERROR: `filter` for `Ok` followed by `unwrap` + .map(Result::unwrap); + let _ = vec![1] + .into_iter() + .map(odds_out) + .filter(|o| o.is_ok()) + //~^ ERROR: `filter` for `Ok` followed by `unwrap` + .map(|o| o.unwrap()); +} diff --git a/tests/ui/result_filter_map.stderr b/tests/ui/result_filter_map.stderr new file mode 100644 index 00000000000..4687794949d --- /dev/null +++ b/tests/ui/result_filter_map.stderr @@ -0,0 +1,41 @@ +error: `filter` for `Ok` followed by `unwrap` + --> $DIR/result_filter_map.rs:13:10 + | +LL | .filter(Result::is_ok) + | __________^ +LL | | +LL | | .map(Result::unwrap); + | |____________________________^ help: consider using `flatten` instead: `flatten()` + | + = note: `-D clippy::result-filter-map` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::result_filter_map)]` + +error: `filter` for `Ok` followed by `unwrap` + --> $DIR/result_filter_map.rs:19:10 + | +LL | .filter(|o| o.is_ok()) + | __________^ +LL | | +LL | | .map(|o| o.unwrap()); + | |____________________________^ help: consider using `flatten` instead: `flatten()` + +error: `filter` for `Ok` followed by `unwrap` + --> $DIR/result_filter_map.rs:26:10 + | +LL | .filter(Result::is_ok) + | __________^ +LL | | +LL | | .map(Result::unwrap); + | |____________________________^ help: consider using `flatten` instead: `flatten()` + +error: `filter` for `Ok` followed by `unwrap` + --> $DIR/result_filter_map.rs:32:10 + | +LL | .filter(|o| o.is_ok()) + | __________^ +LL | | +LL | | .map(|o| o.unwrap()); + | |____________________________^ help: consider using `flatten` instead: `flatten()` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/unconditional_recursion.rs b/tests/ui/unconditional_recursion.rs new file mode 100644 index 00000000000..1169118de83 --- /dev/null +++ b/tests/ui/unconditional_recursion.rs @@ -0,0 +1,163 @@ +//@no-rustfix + +#![warn(clippy::unconditional_recursion)] +#![allow(clippy::partialeq_ne_impl)] + +enum Foo { + A, + B, +} + +impl PartialEq for Foo { + fn ne(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + self != other + } + fn eq(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + self == other + } +} + +enum Foo2 { + A, + B, +} + +impl PartialEq for Foo2 { + fn ne(&self, other: &Self) -> bool { + self != &Foo2::B // no error here + } + fn eq(&self, other: &Self) -> bool { + self == &Foo2::B // no error here + } +} + +enum Foo3 { + A, + B, +} + +impl PartialEq for Foo3 { + fn ne(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + self.ne(other) + } + fn eq(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + self.eq(other) + } +} + +enum Foo4 { + A, + B, +} + +impl PartialEq for Foo4 { + fn ne(&self, other: &Self) -> bool { + self.eq(other) // no error + } + fn eq(&self, other: &Self) -> bool { + self.ne(other) // no error + } +} + +enum Foo5 { + A, + B, +} + +impl Foo5 { + fn a(&self) -> bool { + true + } +} + +impl PartialEq for Foo5 { + fn ne(&self, other: &Self) -> bool { + self.a() // no error + } + fn eq(&self, other: &Self) -> bool { + self.a() // no error + } +} + +struct S; + +// Check the order doesn't matter. +impl PartialEq for S { + fn ne(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + other != self + } + fn eq(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + other == self + } +} + +struct S2; + +// Check that if the same element is compared, it's also triggering the lint. +impl PartialEq for S2 { + fn ne(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + other != other + } + fn eq(&self, other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + other == other + } +} + +struct S3; + +impl PartialEq for S3 { + fn ne(&self, _other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + self != self + } + fn eq(&self, _other: &Self) -> bool { + //~^ ERROR: function cannot return without recursing + self == self + } +} + +// There should be no warning here! +#[derive(PartialEq)] +enum E { + A, + B, +} + +#[derive(PartialEq)] +struct Bar(T); + +struct S4; + +impl PartialEq for S4 { + fn eq(&self, other: &Self) -> bool { + // No warning here. + Bar(self) == Bar(other) + } +} + +macro_rules! impl_partial_eq { + ($ty:ident) => { + impl PartialEq for $ty { + fn eq(&self, other: &Self) -> bool { + self == other + } + } + }; +} + +struct S5; + +impl_partial_eq!(S5); +//~^ ERROR: function cannot return without recursing + +fn main() { + // test code goes here +} diff --git a/tests/ui/unconditional_recursion.stderr b/tests/ui/unconditional_recursion.stderr new file mode 100644 index 00000000000..1fb01c00f19 --- /dev/null +++ b/tests/ui/unconditional_recursion.stderr @@ -0,0 +1,251 @@ +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:42:5 + | +LL | fn ne(&self, other: &Self) -> bool { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | +LL | self.ne(other) + | -------------- recursive call site + | + = help: a `loop` may express intention better if this is on purpose + = note: `-D unconditional-recursion` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(unconditional_recursion)]` + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:46:5 + | +LL | fn eq(&self, other: &Self) -> bool { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing +LL | +LL | self.eq(other) + | -------------- recursive call site + | + = help: a `loop` may express intention better if this is on purpose + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:12:5 + | +LL | / fn ne(&self, other: &Self) -> bool { +LL | | +LL | | self != other +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:14:9 + | +LL | self != other + | ^^^^^^^^^^^^^ + = note: `-D clippy::unconditional-recursion` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unconditional_recursion)]` + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:16:5 + | +LL | / fn eq(&self, other: &Self) -> bool { +LL | | +LL | | self == other +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:18:9 + | +LL | self == other + | ^^^^^^^^^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:42:5 + | +LL | / fn ne(&self, other: &Self) -> bool { +LL | | +LL | | self.ne(other) +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:44:9 + | +LL | self.ne(other) + | ^^^^^^^^^^^^^^ + +error: parameter is only used in recursion + --> $DIR/unconditional_recursion.rs:42:18 + | +LL | fn ne(&self, other: &Self) -> bool { + | ^^^^^ help: if this is intentional, prefix it with an underscore: `_other` + | +note: parameter used here + --> $DIR/unconditional_recursion.rs:44:17 + | +LL | self.ne(other) + | ^^^^^ + = note: `-D clippy::only-used-in-recursion` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::only_used_in_recursion)]` + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:46:5 + | +LL | / fn eq(&self, other: &Self) -> bool { +LL | | +LL | | self.eq(other) +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:48:9 + | +LL | self.eq(other) + | ^^^^^^^^^^^^^^ + +error: parameter is only used in recursion + --> $DIR/unconditional_recursion.rs:46:18 + | +LL | fn eq(&self, other: &Self) -> bool { + | ^^^^^ help: if this is intentional, prefix it with an underscore: `_other` + | +note: parameter used here + --> $DIR/unconditional_recursion.rs:48:17 + | +LL | self.eq(other) + | ^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:90:5 + | +LL | / fn ne(&self, other: &Self) -> bool { +LL | | +LL | | other != self +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:92:9 + | +LL | other != self + | ^^^^^^^^^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:94:5 + | +LL | / fn eq(&self, other: &Self) -> bool { +LL | | +LL | | other == self +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:96:9 + | +LL | other == self + | ^^^^^^^^^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:104:5 + | +LL | / fn ne(&self, other: &Self) -> bool { +LL | | +LL | | other != other +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:106:9 + | +LL | other != other + | ^^^^^^^^^^^^^^ + +error: equal expressions as operands to `!=` + --> $DIR/unconditional_recursion.rs:106:9 + | +LL | other != other + | ^^^^^^^^^^^^^^ + | + = note: `#[deny(clippy::eq_op)]` on by default + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:108:5 + | +LL | / fn eq(&self, other: &Self) -> bool { +LL | | +LL | | other == other +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:110:9 + | +LL | other == other + | ^^^^^^^^^^^^^^ + +error: equal expressions as operands to `==` + --> $DIR/unconditional_recursion.rs:110:9 + | +LL | other == other + | ^^^^^^^^^^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:117:5 + | +LL | / fn ne(&self, _other: &Self) -> bool { +LL | | +LL | | self != self +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:119:9 + | +LL | self != self + | ^^^^^^^^^^^^ + +error: equal expressions as operands to `!=` + --> $DIR/unconditional_recursion.rs:119:9 + | +LL | self != self + | ^^^^^^^^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:121:5 + | +LL | / fn eq(&self, _other: &Self) -> bool { +LL | | +LL | | self == self +LL | | } + | |_____^ + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:123:9 + | +LL | self == self + | ^^^^^^^^^^^^ + +error: equal expressions as operands to `==` + --> $DIR/unconditional_recursion.rs:123:9 + | +LL | self == self + | ^^^^^^^^^^^^ + +error: function cannot return without recursing + --> $DIR/unconditional_recursion.rs:149:13 + | +LL | / fn eq(&self, other: &Self) -> bool { +LL | | self == other +LL | | } + | |_____________^ +... +LL | impl_partial_eq!(S5); + | -------------------- in this macro invocation + | +note: recursive call site + --> $DIR/unconditional_recursion.rs:150:17 + | +LL | self == other + | ^^^^^^^^^^^^^ +... +LL | impl_partial_eq!(S5); + | -------------------- in this macro invocation + = note: this error originates in the macro `impl_partial_eq` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 19 previous errors + diff --git a/tests/ui/unnecessary_to_owned_on_split.fixed b/tests/ui/unnecessary_to_owned_on_split.fixed new file mode 100644 index 00000000000..53dc3c43e2f --- /dev/null +++ b/tests/ui/unnecessary_to_owned_on_split.fixed @@ -0,0 +1,21 @@ +#[allow(clippy::single_char_pattern)] + +fn main() { + let _ = "a".split('a').next().unwrap(); + //~^ ERROR: unnecessary use of `to_string` + let _ = "a".split("a").next().unwrap(); + //~^ ERROR: unnecessary use of `to_string` + let _ = "a".split('a').next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` + let _ = "a".split("a").next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` + + let _ = [1].split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_vec` + let _ = [1].split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_vec` + let _ = [1].split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` + let _ = [1].split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` +} diff --git a/tests/ui/unnecessary_to_owned_on_split.rs b/tests/ui/unnecessary_to_owned_on_split.rs new file mode 100644 index 00000000000..62400e7eee1 --- /dev/null +++ b/tests/ui/unnecessary_to_owned_on_split.rs @@ -0,0 +1,21 @@ +#[allow(clippy::single_char_pattern)] + +fn main() { + let _ = "a".to_string().split('a').next().unwrap(); + //~^ ERROR: unnecessary use of `to_string` + let _ = "a".to_string().split("a").next().unwrap(); + //~^ ERROR: unnecessary use of `to_string` + let _ = "a".to_owned().split('a').next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` + let _ = "a".to_owned().split("a").next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` + + let _ = [1].to_vec().split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_vec` + let _ = [1].to_vec().split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_vec` + let _ = [1].to_owned().split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` + let _ = [1].to_owned().split(|x| *x == 2).next().unwrap(); + //~^ ERROR: unnecessary use of `to_owned` +} diff --git a/tests/ui/unnecessary_to_owned_on_split.stderr b/tests/ui/unnecessary_to_owned_on_split.stderr new file mode 100644 index 00000000000..cfb3766d15e --- /dev/null +++ b/tests/ui/unnecessary_to_owned_on_split.stderr @@ -0,0 +1,53 @@ +error: unnecessary use of `to_string` + --> $DIR/unnecessary_to_owned_on_split.rs:4:13 + | +LL | let _ = "a".to_string().split('a').next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split('a')` + | + = note: `-D clippy::unnecessary-to-owned` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_to_owned)]` + +error: unnecessary use of `to_string` + --> $DIR/unnecessary_to_owned_on_split.rs:6:13 + | +LL | let _ = "a".to_string().split("a").next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split("a")` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned_on_split.rs:8:13 + | +LL | let _ = "a".to_owned().split('a').next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split('a')` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned_on_split.rs:10:13 + | +LL | let _ = "a".to_owned().split("a").next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `"a".split("a")` + +error: unnecessary use of `to_vec` + --> $DIR/unnecessary_to_owned_on_split.rs:13:13 + | +LL | let _ = [1].to_vec().split(|x| *x == 2).next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)` + +error: unnecessary use of `to_vec` + --> $DIR/unnecessary_to_owned_on_split.rs:15:13 + | +LL | let _ = [1].to_vec().split(|x| *x == 2).next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned_on_split.rs:17:13 + | +LL | let _ = [1].to_owned().split(|x| *x == 2).next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)` + +error: unnecessary use of `to_owned` + --> $DIR/unnecessary_to_owned_on_split.rs:19:13 + | +LL | let _ = [1].to_owned().split(|x| *x == 2).next().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `[1].split(|x| *x == 2)` + +error: aborting due to 8 previous errors + diff --git a/util/gh-pages/index.html b/util/gh-pages/index.html index 99e211654d1..a5274eeb349 100644 --- a/util/gh-pages/index.html +++ b/util/gh-pages/index.html @@ -566,8 +566,74 @@ Otherwise, have a great day =^.^= - - Fork me on GitHub + + + From de06ce886ea74cc60d7c883024009a5ee1c19eaa Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Wed, 29 Nov 2023 15:28:46 -0500 Subject: [PATCH 04/13] Address unused tuple struct fields in clippy --- .../src/methods/iter_overeager_cloned.rs | 8 ++++---- clippy_lints/src/methods/mod.rs | 8 ++++---- clippy_lints/src/question_mark.rs | 5 ++--- .../src/transmute/transmute_undefined_repr.rs | 16 ++++++++-------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/methods/iter_overeager_cloned.rs b/clippy_lints/src/methods/iter_overeager_cloned.rs index eac6df0545f..b2fe129cd95 100644 --- a/clippy_lints/src/methods/iter_overeager_cloned.rs +++ b/clippy_lints/src/methods/iter_overeager_cloned.rs @@ -22,7 +22,7 @@ pub(super) enum Op<'a> { // rm `.cloned()` // e.g. `map` `for_each` `all` `any` - NeedlessMove(&'a str, &'a Expr<'a>), + NeedlessMove(&'a Expr<'a>), // later `.cloned()` // and add `&` to the parameter of closure parameter @@ -59,7 +59,7 @@ pub(super) fn check<'tcx>( return; } - if let Op::NeedlessMove(_, expr) = op { + if let Op::NeedlessMove(expr) = op { let rustc_hir::ExprKind::Closure(closure) = expr.kind else { return; }; @@ -104,7 +104,7 @@ pub(super) fn check<'tcx>( } let (lint, msg, trailing_clone) = match op { - Op::RmCloned | Op::NeedlessMove(_, _) => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""), + Op::RmCloned | Op::NeedlessMove(_) => (REDUNDANT_CLONE, "unneeded cloning of iterator items", ""), Op::LaterCloned | Op::FixClosure(_, _) => ( ITER_OVEREAGER_CLONED, "unnecessarily eager cloning of iterator items", @@ -133,7 +133,7 @@ pub(super) fn check<'tcx>( diag.span_suggestion(replace_span, "try", snip, Applicability::MachineApplicable); } }, - Op::NeedlessMove(_, _) => { + Op::NeedlessMove(_) => { let method_span = expr.span.with_lo(cloned_call.span.hi()); if let Some(snip) = snippet_opt(cx, method_span) { let replace_span = expr.span.with_lo(cloned_recv.span.hi()); diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 25b1ea526e2..c1e126137df 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -4186,7 +4186,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr, recv, recv2, - iter_overeager_cloned::Op::NeedlessMove(name, arg), + iter_overeager_cloned::Op::NeedlessMove(arg), false, ); } @@ -4204,7 +4204,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr, recv, recv2, - iter_overeager_cloned::Op::NeedlessMove(name, arg), + iter_overeager_cloned::Op::NeedlessMove(arg), false, ), Some(("chars", recv, _, _, _)) @@ -4379,7 +4379,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr, recv, recv2, - iter_overeager_cloned::Op::NeedlessMove(name, arg), + iter_overeager_cloned::Op::NeedlessMove(arg), false, ), _ => {}, @@ -4433,7 +4433,7 @@ fn check_methods<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { expr, recv, recv2, - iter_overeager_cloned::Op::NeedlessMove(name, m_arg), + iter_overeager_cloned::Op::NeedlessMove(m_arg), false, ), _ => {}, diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 509d9483e1d..9469888a4d4 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -80,7 +80,6 @@ enum IfBlockType<'hir> { Ty<'hir>, Symbol, &'hir Expr<'hir>, - Option<&'hir Expr<'hir>>, ), /// An `if let Xxx(a) = b { c } else { d }` expression. /// @@ -143,7 +142,7 @@ fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) { fn is_early_return(smbl: Symbol, cx: &LateContext<'_>, if_block: &IfBlockType<'_>) -> bool { match *if_block { - IfBlockType::IfIs(caller, caller_ty, call_sym, if_then, _) => { + IfBlockType::IfIs(caller, caller_ty, call_sym, if_then) => { // If the block could be identified as `if x.is_none()/is_err()`, // we then only need to check the if_then return to see if it is none/err. is_type_diagnostic_item(cx, caller_ty, smbl) @@ -235,7 +234,7 @@ fn check_is_none_or_err_and_early_return<'tcx>(&self, cx: &LateContext<'tcx>, ex && !is_else_clause(cx.tcx, expr) && let ExprKind::MethodCall(segment, caller, ..) = &cond.kind && let caller_ty = cx.typeck_results().expr_ty(caller) - && let if_block = IfBlockType::IfIs(caller, caller_ty, segment.ident.name, then, r#else) + && let if_block = IfBlockType::IfIs(caller, caller_ty, segment.ident.name, then) && (is_early_return(sym::Option, cx, &if_block) || is_early_return(sym::Result, cx, &if_block)) { let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_lints/src/transmute/transmute_undefined_repr.rs b/clippy_lints/src/transmute/transmute_undefined_repr.rs index a65bc0ce458..db378cfd755 100644 --- a/clippy_lints/src/transmute/transmute_undefined_repr.rs +++ b/clippy_lints/src/transmute/transmute_undefined_repr.rs @@ -26,18 +26,18 @@ pub(super) fn check<'tcx>( // `Repr(C)` <-> unordered type. // If the first field of the `Repr(C)` type matches then the transmute is ok - (ReducedTy::OrderedFields(_, Some(from_sub_ty)), ReducedTy::UnorderedFields(to_sub_ty)) - | (ReducedTy::UnorderedFields(from_sub_ty), ReducedTy::OrderedFields(_, Some(to_sub_ty))) => { + (ReducedTy::OrderedFields(Some(from_sub_ty)), ReducedTy::UnorderedFields(to_sub_ty)) + | (ReducedTy::UnorderedFields(from_sub_ty), ReducedTy::OrderedFields(Some(to_sub_ty))) => { from_ty = from_sub_ty; to_ty = to_sub_ty; continue; }, - (ReducedTy::OrderedFields(_, Some(from_sub_ty)), ReducedTy::Other(to_sub_ty)) if reduced_tys.to_fat_ptr => { + (ReducedTy::OrderedFields(Some(from_sub_ty)), ReducedTy::Other(to_sub_ty)) if reduced_tys.to_fat_ptr => { from_ty = from_sub_ty; to_ty = to_sub_ty; continue; }, - (ReducedTy::Other(from_sub_ty), ReducedTy::OrderedFields(_, Some(to_sub_ty))) + (ReducedTy::Other(from_sub_ty), ReducedTy::OrderedFields(Some(to_sub_ty))) if reduced_tys.from_fat_ptr => { from_ty = from_sub_ty; @@ -235,8 +235,8 @@ enum ReducedTy<'tcx> { TypeErasure { raw_ptr_only: bool }, /// The type is a struct containing either zero non-zero sized fields, or multiple non-zero /// sized fields with a defined order. - /// The second value is the first non-zero sized type. - OrderedFields(Ty<'tcx>, Option>), + /// The value is the first non-zero sized type. + OrderedFields(Option>), /// The type is a struct containing multiple non-zero sized fields with no defined order. UnorderedFields(Ty<'tcx>), /// Any other type. @@ -259,7 +259,7 @@ fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> ty::Tuple(args) => { let mut iter = args.iter(); let Some(sized_ty) = iter.find(|&ty| !is_zero_sized_ty(cx, ty)) else { - return ReducedTy::OrderedFields(ty, None); + return ReducedTy::OrderedFields(None); }; if iter.all(|ty| is_zero_sized_ty(cx, ty)) { ty = sized_ty; @@ -281,7 +281,7 @@ fn reduce_ty<'tcx>(cx: &LateContext<'tcx>, mut ty: Ty<'tcx>) -> ReducedTy<'tcx> continue; } if def.repr().inhibit_struct_field_reordering_opt() { - ReducedTy::OrderedFields(ty, Some(sized_ty)) + ReducedTy::OrderedFields(Some(sized_ty)) } else { ReducedTy::UnorderedFields(ty) } From 04ebf3c8c9aef6c021a60d73f0f1a798a81b7171 Mon Sep 17 00:00:00 2001 From: Jake Goulding Date: Wed, 29 Nov 2023 15:29:11 -0500 Subject: [PATCH 05/13] Remove #[allow(unused_tuple_struct_fields)] from Clippy tests --- .../auxiliary/helper.rs | 1 - tests/ui/format.fixed | 1 - tests/ui/format.rs | 1 - tests/ui/format.stderr | 30 +++++------ tests/ui/from_iter_instead_of_collect.fixed | 2 +- tests/ui/from_iter_instead_of_collect.rs | 2 +- tests/ui/must_use_candidates.fixed | 1 - tests/ui/must_use_candidates.rs | 1 - tests/ui/must_use_candidates.stderr | 10 ++-- tests/ui/numbered_fields.fixed | 1 - tests/ui/numbered_fields.rs | 1 - tests/ui/numbered_fields.stderr | 4 +- tests/ui/option_if_let_else.fixed | 1 - tests/ui/option_if_let_else.rs | 1 - tests/ui/option_if_let_else.stderr | 50 +++++++++---------- tests/ui/unreadable_literal.fixed | 1 - tests/ui/unreadable_literal.rs | 1 - tests/ui/unreadable_literal.stderr | 20 ++++---- 18 files changed, 59 insertions(+), 70 deletions(-) diff --git a/tests/ui/borrow_interior_mutable_const/auxiliary/helper.rs b/tests/ui/borrow_interior_mutable_const/auxiliary/helper.rs index b03c21262c3..96e037d4fcd 100644 --- a/tests/ui/borrow_interior_mutable_const/auxiliary/helper.rs +++ b/tests/ui/borrow_interior_mutable_const/auxiliary/helper.rs @@ -2,7 +2,6 @@ // As the most common case is the `http` crate, it replicates `http::HeaderName`'s structure. #![allow(clippy::declare_interior_mutable_const)] -#![allow(unused_tuple_struct_fields)] use std::sync::atomic::AtomicUsize; diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index 36679a9c883..2b32fdeae2b 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -1,6 +1,5 @@ #![warn(clippy::useless_format)] #![allow( - unused_tuple_struct_fields, clippy::print_literal, clippy::redundant_clone, clippy::to_string_in_format_args, diff --git a/tests/ui/format.rs b/tests/ui/format.rs index b0920daf088..bad192067e9 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -1,6 +1,5 @@ #![warn(clippy::useless_format)] #![allow( - unused_tuple_struct_fields, clippy::print_literal, clippy::redundant_clone, clippy::to_string_in_format_args, diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index d4630a8f1da..e02fdb1e415 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -1,5 +1,5 @@ error: useless use of `format!` - --> $DIR/format.rs:20:5 + --> $DIR/format.rs:19:5 | LL | format!("foo"); | ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"foo".to_string()` @@ -8,19 +8,19 @@ LL | format!("foo"); = help: to override `-D warnings` add `#[allow(clippy::useless_format)]` error: useless use of `format!` - --> $DIR/format.rs:21:5 + --> $DIR/format.rs:20:5 | LL | format!("{{}}"); | ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"{}".to_string()` error: useless use of `format!` - --> $DIR/format.rs:22:5 + --> $DIR/format.rs:21:5 | LL | format!("{{}} abc {{}}"); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"{} abc {}".to_string()` error: useless use of `format!` - --> $DIR/format.rs:23:5 + --> $DIR/format.rs:22:5 | LL | / format!( LL | | r##"foo {{}} @@ -35,67 +35,67 @@ LL ~ " bar"##.to_string(); | error: useless use of `format!` - --> $DIR/format.rs:28:13 + --> $DIR/format.rs:27:13 | LL | let _ = format!(""); | ^^^^^^^^^^^ help: consider using `String::new()`: `String::new()` error: useless use of `format!` - --> $DIR/format.rs:30:5 + --> $DIR/format.rs:29:5 | LL | format!("{}", "foo"); | ^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"foo".to_string()` error: useless use of `format!` - --> $DIR/format.rs:38:5 + --> $DIR/format.rs:37:5 | LL | format!("{}", arg); | ^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `arg.to_string()` error: useless use of `format!` - --> $DIR/format.rs:68:5 + --> $DIR/format.rs:67:5 | LL | format!("{}", 42.to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `42.to_string()` error: useless use of `format!` - --> $DIR/format.rs:70:5 + --> $DIR/format.rs:69:5 | LL | format!("{}", x.display().to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.display().to_string()` error: useless use of `format!` - --> $DIR/format.rs:74:18 + --> $DIR/format.rs:73:18 | LL | let _ = Some(format!("{}", a + "bar")); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `a + "bar"` error: useless use of `format!` - --> $DIR/format.rs:78:22 + --> $DIR/format.rs:77:22 | LL | let _s: String = format!("{}", &*v.join("\n")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `(&*v.join("\n")).to_string()` error: useless use of `format!` - --> $DIR/format.rs:84:13 + --> $DIR/format.rs:83:13 | LL | let _ = format!("{x}"); | ^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()` error: useless use of `format!` - --> $DIR/format.rs:86:13 + --> $DIR/format.rs:85:13 | LL | let _ = format!("{y}", y = x); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `x.to_string()` error: useless use of `format!` - --> $DIR/format.rs:90:13 + --> $DIR/format.rs:89:13 | LL | let _ = format!("{abc}"); | ^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `abc.to_string()` error: useless use of `format!` - --> $DIR/format.rs:92:13 + --> $DIR/format.rs:91:13 | LL | let _ = format!("{xx}"); | ^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `xx.to_string()` diff --git a/tests/ui/from_iter_instead_of_collect.fixed b/tests/ui/from_iter_instead_of_collect.fixed index 82c8e1d8abd..c250162dfb8 100644 --- a/tests/ui/from_iter_instead_of_collect.fixed +++ b/tests/ui/from_iter_instead_of_collect.fixed @@ -1,5 +1,5 @@ #![warn(clippy::from_iter_instead_of_collect)] -#![allow(unused_imports, unused_tuple_struct_fields)] +#![allow(unused_imports)] #![allow(clippy::useless_vec)] use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque}; diff --git a/tests/ui/from_iter_instead_of_collect.rs b/tests/ui/from_iter_instead_of_collect.rs index 2aed6b14be1..8adbb841c8b 100644 --- a/tests/ui/from_iter_instead_of_collect.rs +++ b/tests/ui/from_iter_instead_of_collect.rs @@ -1,5 +1,5 @@ #![warn(clippy::from_iter_instead_of_collect)] -#![allow(unused_imports, unused_tuple_struct_fields)] +#![allow(unused_imports)] #![allow(clippy::useless_vec)] use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque}; diff --git a/tests/ui/must_use_candidates.fixed b/tests/ui/must_use_candidates.fixed index 3ed705b2906..db20ba29f3d 100644 --- a/tests/ui/must_use_candidates.fixed +++ b/tests/ui/must_use_candidates.fixed @@ -1,7 +1,6 @@ #![feature(never_type)] #![allow( unused_mut, - unused_tuple_struct_fields, clippy::redundant_allocation, clippy::needless_pass_by_ref_mut )] diff --git a/tests/ui/must_use_candidates.rs b/tests/ui/must_use_candidates.rs index ab8efea0ac7..d7e56130245 100644 --- a/tests/ui/must_use_candidates.rs +++ b/tests/ui/must_use_candidates.rs @@ -1,7 +1,6 @@ #![feature(never_type)] #![allow( unused_mut, - unused_tuple_struct_fields, clippy::redundant_allocation, clippy::needless_pass_by_ref_mut )] diff --git a/tests/ui/must_use_candidates.stderr b/tests/ui/must_use_candidates.stderr index 581399f3e48..39446bf6cd9 100644 --- a/tests/ui/must_use_candidates.stderr +++ b/tests/ui/must_use_candidates.stderr @@ -1,5 +1,5 @@ error: this function could have a `#[must_use]` attribute - --> $DIR/must_use_candidates.rs:16:1 + --> $DIR/must_use_candidates.rs:15:1 | LL | pub fn pure(i: u8) -> u8 { | ^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn pure(i: u8) -> u8` @@ -8,25 +8,25 @@ LL | pub fn pure(i: u8) -> u8 { = help: to override `-D warnings` add `#[allow(clippy::must_use_candidate)]` error: this method could have a `#[must_use]` attribute - --> $DIR/must_use_candidates.rs:21:5 + --> $DIR/must_use_candidates.rs:20:5 | LL | pub fn inherent_pure(&self) -> u8 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn inherent_pure(&self) -> u8` error: this function could have a `#[must_use]` attribute - --> $DIR/must_use_candidates.rs:52:1 + --> $DIR/must_use_candidates.rs:51:1 | LL | pub fn with_marker(_d: std::marker::PhantomData<&mut u32>) -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn with_marker(_d: std::marker::PhantomData<&mut u32>) -> bool` error: this function could have a `#[must_use]` attribute - --> $DIR/must_use_candidates.rs:64:1 + --> $DIR/must_use_candidates.rs:63:1 | LL | pub fn rcd(_x: Rc) -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn rcd(_x: Rc) -> bool` error: this function could have a `#[must_use]` attribute - --> $DIR/must_use_candidates.rs:72:1 + --> $DIR/must_use_candidates.rs:71:1 | LL | pub fn arcd(_x: Arc) -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: add the attribute: `#[must_use] pub fn arcd(_x: Arc) -> bool` diff --git a/tests/ui/numbered_fields.fixed b/tests/ui/numbered_fields.fixed index 7f0a6f8e544..dc88081ba0a 100644 --- a/tests/ui/numbered_fields.fixed +++ b/tests/ui/numbered_fields.fixed @@ -1,5 +1,4 @@ #![warn(clippy::init_numbered_fields)] -#![allow(unused_tuple_struct_fields)] #[derive(Default)] struct TupleStruct(u32, u32, u8); diff --git a/tests/ui/numbered_fields.rs b/tests/ui/numbered_fields.rs index 38f3b36ec4d..e8fa652e3c1 100644 --- a/tests/ui/numbered_fields.rs +++ b/tests/ui/numbered_fields.rs @@ -1,5 +1,4 @@ #![warn(clippy::init_numbered_fields)] -#![allow(unused_tuple_struct_fields)] #[derive(Default)] struct TupleStruct(u32, u32, u8); diff --git a/tests/ui/numbered_fields.stderr b/tests/ui/numbered_fields.stderr index d52a0cf15a8..76f5e082f32 100644 --- a/tests/ui/numbered_fields.stderr +++ b/tests/ui/numbered_fields.stderr @@ -1,5 +1,5 @@ error: used a field initializer for a tuple struct - --> $DIR/numbered_fields.rs:18:13 + --> $DIR/numbered_fields.rs:17:13 | LL | let _ = TupleStruct { | _____________^ @@ -13,7 +13,7 @@ LL | | }; = help: to override `-D warnings` add `#[allow(clippy::init_numbered_fields)]` error: used a field initializer for a tuple struct - --> $DIR/numbered_fields.rs:25:13 + --> $DIR/numbered_fields.rs:24:13 | LL | let _ = TupleStruct { | _____________^ diff --git a/tests/ui/option_if_let_else.fixed b/tests/ui/option_if_let_else.fixed index 363520112ef..d443334bb05 100644 --- a/tests/ui/option_if_let_else.fixed +++ b/tests/ui/option_if_let_else.fixed @@ -1,6 +1,5 @@ #![warn(clippy::option_if_let_else)] #![allow( - unused_tuple_struct_fields, clippy::ref_option_ref, clippy::equatable_if_let, clippy::let_unit_value, diff --git a/tests/ui/option_if_let_else.rs b/tests/ui/option_if_let_else.rs index aaa87a0db54..317c35bf842 100644 --- a/tests/ui/option_if_let_else.rs +++ b/tests/ui/option_if_let_else.rs @@ -1,6 +1,5 @@ #![warn(clippy::option_if_let_else)] #![allow( - unused_tuple_struct_fields, clippy::ref_option_ref, clippy::equatable_if_let, clippy::let_unit_value, diff --git a/tests/ui/option_if_let_else.stderr b/tests/ui/option_if_let_else.stderr index 55a8360ffd0..e053d356ff2 100644 --- a/tests/ui/option_if_let_else.stderr +++ b/tests/ui/option_if_let_else.stderr @@ -1,5 +1,5 @@ error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:11:5 + --> $DIR/option_if_let_else.rs:10:5 | LL | / if let Some(x) = string { LL | | (true, x) @@ -12,19 +12,19 @@ LL | | } = help: to override `-D warnings` add `#[allow(clippy::option_if_let_else)]` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:29:13 + --> $DIR/option_if_let_else.rs:28:13 | LL | let _ = if let Some(s) = *string { s.len() } else { 0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `string.map_or(0, |s| s.len())` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:30:13 + --> $DIR/option_if_let_else.rs:29:13 | LL | let _ = if let Some(s) = &num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:31:13 + --> $DIR/option_if_let_else.rs:30:13 | LL | let _ = if let Some(s) = &mut num { | _____________^ @@ -44,13 +44,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:37:13 + --> $DIR/option_if_let_else.rs:36:13 | LL | let _ = if let Some(ref s) = num { s } else { &0 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `num.as_ref().map_or(&0, |s| s)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:38:13 + --> $DIR/option_if_let_else.rs:37:13 | LL | let _ = if let Some(mut s) = num { | _____________^ @@ -70,7 +70,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:44:13 + --> $DIR/option_if_let_else.rs:43:13 | LL | let _ = if let Some(ref mut s) = num { | _____________^ @@ -90,7 +90,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:53:5 + --> $DIR/option_if_let_else.rs:52:5 | LL | / if let Some(x) = arg { LL | | let y = x * x; @@ -109,7 +109,7 @@ LL + }) | error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:66:13 + --> $DIR/option_if_let_else.rs:65:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -121,7 +121,7 @@ LL | | }; | |_____^ help: try: `arg.map_or_else(side_effect, |x| x)` error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:75:13 + --> $DIR/option_if_let_else.rs:74:13 | LL | let _ = if let Some(x) = arg { | _____________^ @@ -144,7 +144,7 @@ LL ~ }, |x| x * x * x * x); | error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:108:13 + --> $DIR/option_if_let_else.rs:107:13 | LL | / if let Some(idx) = s.find('.') { LL | | vec![s[..idx].to_string(), s[idx..].to_string()] @@ -154,7 +154,7 @@ LL | | } | |_____________^ help: try: `s.find('.').map_or_else(|| vec![s.to_string()], |idx| vec![s[..idx].to_string(), s[idx..].to_string()])` error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:119:5 + --> $DIR/option_if_let_else.rs:118:5 | LL | / if let Ok(binding) = variable { LL | | println!("Ok {binding}"); @@ -177,13 +177,13 @@ LL + }) | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:143:13 + --> $DIR/option_if_let_else.rs:142:13 | LL | let _ = if let Some(x) = optional { x + 2 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `optional.map_or(5, |x| x + 2)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:153:13 + --> $DIR/option_if_let_else.rs:152:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -205,13 +205,13 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:181:13 + --> $DIR/option_if_let_else.rs:180:13 | LL | let _ = if let Some(x) = Some(0) { s.len() + x } else { s.len() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(0).map_or(s.len(), |x| s.len() + x)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:185:13 + --> $DIR/option_if_let_else.rs:184:13 | LL | let _ = if let Some(x) = Some(0) { | _____________^ @@ -231,7 +231,7 @@ LL ~ }); | error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:224:13 + --> $DIR/option_if_let_else.rs:223:13 | LL | let _ = match s { | _____________^ @@ -241,7 +241,7 @@ LL | | }; | |_____^ help: try: `s.map_or(1, |string| string.len())` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:228:13 + --> $DIR/option_if_let_else.rs:227:13 | LL | let _ = match Some(10) { | _____________^ @@ -251,7 +251,7 @@ LL | | }; | |_____^ help: try: `Some(10).map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:234:13 + --> $DIR/option_if_let_else.rs:233:13 | LL | let _ = match res { | _____________^ @@ -261,7 +261,7 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:238:13 + --> $DIR/option_if_let_else.rs:237:13 | LL | let _ = match res { | _____________^ @@ -271,13 +271,13 @@ LL | | }; | |_____^ help: try: `res.map_or(1, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:242:13 + --> $DIR/option_if_let_else.rs:241:13 | LL | let _ = if let Ok(a) = res { a + 1 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `res.map_or(5, |a| a + 1)` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:259:17 + --> $DIR/option_if_let_else.rs:258:17 | LL | let _ = match initial { | _________________^ @@ -287,7 +287,7 @@ LL | | }; | |_________^ help: try: `initial.as_ref().map_or(42, |value| do_something(value))` error: use Option::map_or instead of an if let/else - --> $DIR/option_if_let_else.rs:266:17 + --> $DIR/option_if_let_else.rs:265:17 | LL | let _ = match initial { | _________________^ @@ -297,7 +297,7 @@ LL | | }; | |_________^ help: try: `initial.as_mut().map_or(42, |value| do_something2(value))` error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:289:24 + --> $DIR/option_if_let_else.rs:288:24 | LL | let mut _hashmap = if let Some(hm) = &opt { | ________________________^ @@ -308,7 +308,7 @@ LL | | }; | |_____^ help: try: `opt.as_ref().map_or_else(HashMap::new, |hm| hm.clone())` error: use Option::map_or_else instead of an if let/else - --> $DIR/option_if_let_else.rs:295:19 + --> $DIR/option_if_let_else.rs:294:19 | LL | let mut _hm = if let Some(hm) = &opt { hm.clone() } else { new_map!() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `opt.as_ref().map_or_else(|| new_map!(), |hm| hm.clone())` diff --git a/tests/ui/unreadable_literal.fixed b/tests/ui/unreadable_literal.fixed index 6d8c719ee07..fb9c2672db8 100644 --- a/tests/ui/unreadable_literal.fixed +++ b/tests/ui/unreadable_literal.fixed @@ -1,5 +1,4 @@ #![warn(clippy::unreadable_literal)] -#![allow(unused_tuple_struct_fields)] struct Foo(u64); diff --git a/tests/ui/unreadable_literal.rs b/tests/ui/unreadable_literal.rs index 42ca773c351..0a24fa85254 100644 --- a/tests/ui/unreadable_literal.rs +++ b/tests/ui/unreadable_literal.rs @@ -1,5 +1,4 @@ #![warn(clippy::unreadable_literal)] -#![allow(unused_tuple_struct_fields)] struct Foo(u64); diff --git a/tests/ui/unreadable_literal.stderr b/tests/ui/unreadable_literal.stderr index d7a3377ec37..37f91acf82b 100644 --- a/tests/ui/unreadable_literal.stderr +++ b/tests/ui/unreadable_literal.stderr @@ -1,5 +1,5 @@ error: long literal lacking separators - --> $DIR/unreadable_literal.rs:32:17 + --> $DIR/unreadable_literal.rs:31:17 | LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^ help: consider: `0b11_0110_i64` @@ -8,55 +8,55 @@ LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); = help: to override `-D warnings` add `#[allow(clippy::unreadable_literal)]` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:32:31 + --> $DIR/unreadable_literal.rs:31:31 | LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^^^^^ help: consider: `0x1234_5678_usize` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:32:49 + --> $DIR/unreadable_literal.rs:31:49 | LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^ help: consider: `123_456_f32` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:32:61 + --> $DIR/unreadable_literal.rs:31:61 | LL | let _bad = (0b110110_i64, 0x12345678_usize, 123456_f32, 1.234567_f32); | ^^^^^^^^^^^^ help: consider: `1.234_567_f32` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:34:20 + --> $DIR/unreadable_literal.rs:33:20 | LL | let _bad_sci = 1.123456e1; | ^^^^^^^^^^ help: consider: `1.123_456e1` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:36:18 + --> $DIR/unreadable_literal.rs:35:18 | LL | let _fail1 = 0xabcdef; | ^^^^^^^^ help: consider: `0x00ab_cdef` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:37:23 + --> $DIR/unreadable_literal.rs:36:23 | LL | let _fail2: u32 = 0xBAFEBAFE; | ^^^^^^^^^^ help: consider: `0xBAFE_BAFE` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:38:18 + --> $DIR/unreadable_literal.rs:37:18 | LL | let _fail3 = 0xabcdeff; | ^^^^^^^^^ help: consider: `0x0abc_deff` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:39:24 + --> $DIR/unreadable_literal.rs:38:24 | LL | let _fail4: i128 = 0xabcabcabcabcabcabc; | ^^^^^^^^^^^^^^^^^^^^ help: consider: `0x00ab_cabc_abca_bcab_cabc` error: long literal lacking separators - --> $DIR/unreadable_literal.rs:40:18 + --> $DIR/unreadable_literal.rs:39:18 | LL | let _fail5 = 1.100300400; | ^^^^^^^^^^^ help: consider: `1.100_300_400` From ca83a98e66a1a6f71c09d92ab25fa385ca8353b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 20 Dec 2023 19:13:24 +0000 Subject: [PATCH 06/13] Track `HirId` instead of `Span` in `ObligationCauseCode::SizedArgumentType` This gets us more accurate suggestions. --- tests/ui/crashes/ice-6251.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ui/crashes/ice-6251.stderr b/tests/ui/crashes/ice-6251.stderr index 11081dc8087..0196c9923db 100644 --- a/tests/ui/crashes/ice-6251.stderr +++ b/tests/ui/crashes/ice-6251.stderr @@ -6,7 +6,7 @@ LL | fn bug() -> impl Iterator { | = help: the trait `std::marker::Sized` is not implemented for `[u8]` = help: unsized fn params are gated as an unstable feature -help: function arguments must have a statically known size, borrowed types always have a known size +help: function arguments must have a statically known size, borrowed slices always have a known size | LL | fn bug() -> impl Iterator { | + From 0bac101c21d3b410b819fdac09d88adb71bfd8e5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 5 Jan 2024 10:02:40 +1100 Subject: [PATCH 07/13] Rename `EmitterWriter` as `HumanEmitter`. For consistency with other `Emitter` impls, such as `JsonEmitter`, `SilentEmitter`, `SharedEmitter`, etc. --- clippy_lints/src/doc/needless_doctest_main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/doc/needless_doctest_main.rs b/clippy_lints/src/doc/needless_doctest_main.rs index c639813a3f9..a744b69ecb4 100644 --- a/clippy_lints/src/doc/needless_doctest_main.rs +++ b/clippy_lints/src/doc/needless_doctest_main.rs @@ -5,7 +5,7 @@ use clippy_utils::diagnostics::span_lint; use rustc_ast::{CoroutineKind, Fn, FnRetTy, Item, ItemKind}; use rustc_data_structures::sync::Lrc; -use rustc_errors::emitter::EmitterWriter; +use rustc_errors::emitter::HumanEmitter; use rustc_errors::DiagCtxt; use rustc_lint::LateContext; use rustc_parse::maybe_new_parser_from_source_str; @@ -44,7 +44,7 @@ fn check_code_sample(code: String, edition: Edition, ignore: bool) -> (bool, Vec let fallback_bundle = rustc_errors::fallback_fluent_bundle(rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false); - let emitter = EmitterWriter::new(Box::new(io::sink()), fallback_bundle); + let emitter = HumanEmitter::new(Box::new(io::sink()), fallback_bundle); let dcx = DiagCtxt::with_emitter(Box::new(emitter)).disable_warnings(); #[expect(clippy::arc_with_non_send_sync)] // `Lrc` is expected by with_dcx let sm = Lrc::new(SourceMap::new(FilePathMapping::empty())); From f73e37d00ce29fbf81f62103ce06d2d3787c3842 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 4 Jan 2024 10:29:47 +0000 Subject: [PATCH 08/13] Update clippy for hir::Guard removal --- clippy_lints/src/entry.rs | 4 +- clippy_lints/src/manual_clamp.rs | 4 +- clippy_lints/src/matches/collapsible_match.rs | 8 +-- .../src/matches/match_like_matches.rs | 23 ++------- clippy_lints/src/matches/needless_match.rs | 17 ++----- clippy_lints/src/matches/redundant_guards.rs | 50 ++++++++----------- .../src/matches/redundant_pattern_match.rs | 20 ++++---- .../src/mixed_read_write_in_expression.rs | 4 +- clippy_lints/src/utils/author.rs | 10 +--- clippy_utils/src/hir_utils.rs | 24 ++------- clippy_utils/src/lib.rs | 4 +- 11 files changed, 56 insertions(+), 112 deletions(-) diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index ce0a1dfdc61..0b4bc375df0 100644 --- a/clippy_lints/src/entry.rs +++ b/clippy_lints/src/entry.rs @@ -8,7 +8,7 @@ use rustc_errors::Applicability; use rustc_hir::hir_id::HirIdSet; use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{Block, Expr, ExprKind, Guard, HirId, Let, Pat, Stmt, StmtKind, UnOp}; +use rustc_hir::{Block, Expr, ExprKind, HirId, Pat, Stmt, StmtKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; use rustc_span::{Span, SyntaxContext, DUMMY_SP}; @@ -465,7 +465,7 @@ fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { let mut is_map_used = self.is_map_used; for arm in arms { self.visit_pat(arm.pat); - if let Some(Guard::If(guard) | Guard::IfLet(&Let { init: guard, .. })) = arm.guard { + if let Some(guard) = arm.guard { self.visit_non_tail_expr(guard); } is_map_used |= self.visit_cond_arm(arm.body); diff --git a/clippy_lints/src/manual_clamp.rs b/clippy_lints/src/manual_clamp.rs index 385fe387a31..0da309f9531 100644 --- a/clippy_lints/src/manual_clamp.rs +++ b/clippy_lints/src/manual_clamp.rs @@ -11,7 +11,7 @@ use itertools::Itertools; use rustc_errors::{Applicability, Diagnostic}; use rustc_hir::def::Res; -use rustc_hir::{Arm, BinOpKind, Block, Expr, ExprKind, Guard, HirId, PatKind, PathSegment, PrimTy, QPath, StmtKind}; +use rustc_hir::{Arm, BinOpKind, Block, Expr, ExprKind, HirId, PatKind, PathSegment, PrimTy, QPath, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::Ty; use rustc_session::impl_lint_pass; @@ -394,7 +394,7 @@ fn is_match_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Opt // Find possible min/max branches let minmax_values = |a: &'tcx Arm<'tcx>| { if let PatKind::Binding(_, var_hir_id, _, None) = &a.pat.kind - && let Some(Guard::If(e)) = a.guard + && let Some(e) = a.guard { Some((e, var_hir_id, a.body)) } else { diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index 91e6ca7fa8b..5fef5930fab 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -7,7 +7,7 @@ }; use rustc_errors::MultiSpan; use rustc_hir::LangItem::OptionNone; -use rustc_hir::{Arm, Expr, Guard, HirId, Let, Pat, PatKind}; +use rustc_hir::{Arm, Expr, HirId, Pat, PatKind}; use rustc_lint::LateContext; use rustc_span::Span; @@ -16,7 +16,7 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'_>]) { if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) { for arm in arms { - check_arm(cx, true, arm.pat, arm.body, arm.guard.as_ref(), Some(els_arm.body)); + check_arm(cx, true, arm.pat, arm.body, arm.guard, Some(els_arm.body)); } } } @@ -35,7 +35,7 @@ fn check_arm<'tcx>( outer_is_match: bool, outer_pat: &'tcx Pat<'tcx>, outer_then_body: &'tcx Expr<'tcx>, - outer_guard: Option<&'tcx Guard<'tcx>>, + outer_guard: Option<&'tcx Expr<'tcx>>, outer_else_body: Option<&'tcx Expr<'tcx>>, ) { let inner_expr = peel_blocks_with_stmt(outer_then_body); @@ -71,7 +71,7 @@ fn check_arm<'tcx>( // the binding must not be used in the if guard && outer_guard.map_or( true, - |(Guard::If(e) | Guard::IfLet(Let { init: e, .. }))| !is_local_used(cx, *e, binding_id) + |e| !is_local_used(cx, e, binding_id) ) // ...or anywhere in the inner expression && match inner { diff --git a/clippy_lints/src/matches/match_like_matches.rs b/clippy_lints/src/matches/match_like_matches.rs index 56123326fe4..b062e81cefd 100644 --- a/clippy_lints/src/matches/match_like_matches.rs +++ b/clippy_lints/src/matches/match_like_matches.rs @@ -4,7 +4,7 @@ use clippy_utils::{is_lint_allowed, is_wild, span_contains_comment}; use rustc_ast::{Attribute, LitKind}; use rustc_errors::Applicability; -use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Guard, Pat, PatKind, QPath}; +use rustc_hir::{Arm, BorrowKind, Expr, ExprKind, Pat, PatKind, QPath}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::ty; use rustc_span::source_map::Spanned; @@ -41,14 +41,8 @@ pub(super) fn check_match<'tcx>( find_matches_sugg( cx, scrutinee, - arms.iter().map(|arm| { - ( - cx.tcx.hir().attrs(arm.hir_id), - Some(arm.pat), - arm.body, - arm.guard.as_ref(), - ) - }), + arms.iter() + .map(|arm| (cx.tcx.hir().attrs(arm.hir_id), Some(arm.pat), arm.body, arm.guard)), e, false, ) @@ -67,14 +61,7 @@ fn find_matches_sugg<'a, 'b, I>( I: Clone + DoubleEndedIterator + ExactSizeIterator - + Iterator< - Item = ( - &'a [Attribute], - Option<&'a Pat<'b>>, - &'a Expr<'b>, - Option<&'a Guard<'b>>, - ), - >, + + Iterator>, &'a Expr<'b>, Option<&'a Expr<'b>>)>, { if !span_contains_comment(cx.sess().source_map(), expr.span) && iter.len() >= 2 @@ -115,7 +102,7 @@ fn find_matches_sugg<'a, 'b, I>( }) .join(" | ") }; - let pat_and_guard = if let Some(Guard::If(g)) = first_guard { + let pat_and_guard = if let Some(g) = first_guard { format!( "{pat} if {}", snippet_with_applicability(cx, g.span, "..", &mut applicability) diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index 44dc29c36a6..cc482f15a91 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -8,7 +8,7 @@ }; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionNone; -use rustc_hir::{Arm, BindingAnnotation, ByRef, Expr, ExprKind, Guard, ItemKind, Node, Pat, PatKind, Path, QPath}; +use rustc_hir::{Arm, BindingAnnotation, ByRef, Expr, ExprKind, ItemKind, Node, Pat, PatKind, Path, QPath}; use rustc_lint::LateContext; use rustc_span::sym; @@ -66,18 +66,9 @@ fn check_all_arms(cx: &LateContext<'_>, match_expr: &Expr<'_>, arms: &[Arm<'_>]) let arm_expr = peel_blocks_with_stmt(arm.body); if let Some(guard_expr) = &arm.guard { - match guard_expr { - // gives up if `pat if expr` can have side effects - Guard::If(if_cond) => { - if if_cond.can_have_side_effects() { - return false; - } - }, - // gives up `pat if let ...` arm - Guard::IfLet(_) => { - return false; - }, - }; + if guard_expr.can_have_side_effects() { + return false; + } } if let PatKind::Wild = arm.pat.kind { diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index f57b22374c8..dfaaeb14ca3 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -5,7 +5,7 @@ use rustc_ast::{BorrowKind, LitKind}; use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; -use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, Guard, MatchSource, Node, Pat, PatKind}; +use rustc_hir::{Arm, BinOpKind, Expr, ExprKind, MatchSource, Node, Pat, PatKind}; use rustc_lint::LateContext; use rustc_span::symbol::Ident; use rustc_span::{Span, Symbol}; @@ -21,20 +21,19 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { }; // `Some(x) if matches!(x, y)` - if let Guard::If(if_expr) = guard - && let ExprKind::Match( - scrutinee, - [ - arm, - Arm { - pat: Pat { - kind: PatKind::Wild, .. - }, - .. + if let ExprKind::Match( + scrutinee, + [ + arm, + Arm { + pat: Pat { + kind: PatKind::Wild, .. }, - ], - MatchSource::Normal, - ) = if_expr.kind + .. + }, + ], + MatchSource::Normal, + ) = guard.kind && let Some(binding) = get_pat_binding(cx, scrutinee, outer_arm) { let pat_span = match (arm.pat.kind, binding.byref_ident) { @@ -45,14 +44,14 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { emit_redundant_guards( cx, outer_arm, - if_expr.span, + guard.span, snippet(cx, pat_span, ""), &binding, arm.guard, ); } // `Some(x) if let Some(2) = x` - else if let Guard::IfLet(let_expr) = guard + else if let ExprKind::Let(let_expr) = guard.kind && let Some(binding) = get_pat_binding(cx, let_expr.init, outer_arm) { let pat_span = match (let_expr.pat.kind, binding.byref_ident) { @@ -71,8 +70,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { } // `Some(x) if x == Some(2)` // `Some(x) if Some(2) == x` - else if let Guard::If(if_expr) = guard - && let ExprKind::Binary(bin_op, local, pat) = if_expr.kind + else if let ExprKind::Binary(bin_op, local, pat) = guard.kind && matches!(bin_op.node, BinOpKind::Eq) // Ensure they have the same type. If they don't, we'd need deref coercion which isn't // possible (currently) in a pattern. In some cases, you can use something like @@ -96,16 +94,15 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, arms: &'tcx [Arm<'tcx>]) { emit_redundant_guards( cx, outer_arm, - if_expr.span, + guard.span, snippet(cx, pat_span, ""), &binding, None, ); - } else if let Guard::If(if_expr) = guard - && let ExprKind::MethodCall(path, recv, args, ..) = if_expr.kind + } else if let ExprKind::MethodCall(path, recv, args, ..) = guard.kind && let Some(binding) = get_pat_binding(cx, recv, outer_arm) { - check_method_calls(cx, outer_arm, path.ident.name, recv, args, if_expr, &binding); + check_method_calls(cx, outer_arm, path.ident.name, recv, args, guard, &binding); } } } @@ -216,7 +213,7 @@ fn emit_redundant_guards<'tcx>( guard_span: Span, binding_replacement: Cow<'static, str>, pat_binding: &PatBindingInfo, - inner_guard: Option>, + inner_guard: Option<&Expr<'_>>, ) { span_lint_and_then( cx, @@ -242,12 +239,7 @@ fn emit_redundant_guards<'tcx>( ( guard_span.source_callsite().with_lo(outer_arm.pat.span.hi()), inner_guard.map_or_else(String::new, |guard| { - let (prefix, span) = match guard { - Guard::If(e) => ("if", e.span), - Guard::IfLet(l) => ("if let", l.span), - }; - - format!(" {prefix} {}", snippet(cx, span, "")) + format!(" if {}", snippet(cx, guard.span, "")) }), ), ], diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index a4acdfb1db4..b5870d94d99 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -9,7 +9,7 @@ use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk}; -use rustc_hir::{Arm, Expr, ExprKind, Guard, Node, Pat, PatKind, QPath, UnOp}; +use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp}; use rustc_lint::LateContext; use rustc_middle::ty::{self, GenericArgKind, Ty}; use rustc_span::{sym, Span, Symbol}; @@ -277,8 +277,6 @@ pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, op let mut sugg = format!("{}.{good_method}", snippet(cx, result_expr.span, "_")); if let Some(guard) = maybe_guard { - let Guard::If(guard) = *guard else { return }; // `...is_none() && let ...` is a syntax error - // wow, the HIR for match guards in `PAT if let PAT = expr && expr => ...` is annoying! // `guard` here is `Guard::If` with the let expression somewhere deep in the tree of exprs, // counter to the intuition that it should be `Guard::IfLet`, so we need another check @@ -319,7 +317,7 @@ fn found_good_method<'tcx>( cx: &LateContext<'_>, arms: &'tcx [Arm<'tcx>], node: (&PatKind<'_>, &PatKind<'_>), -) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> { +) -> Option<(&'static str, Option<&'tcx Expr<'tcx>>)> { match node { ( PatKind::TupleStruct(ref path_left, patterns_left, _), @@ -409,7 +407,7 @@ fn get_good_method<'tcx>( cx: &LateContext<'_>, arms: &'tcx [Arm<'tcx>], path_left: &QPath<'_>, -) -> Option<(&'static str, Option<&'tcx Guard<'tcx>>)> { +) -> Option<(&'static str, Option<&'tcx Expr<'tcx>>)> { if let Some(name) = get_ident(path_left) { let (expected_item_left, should_be_left, should_be_right) = match name.as_str() { "Ok" => (Item::Lang(ResultOk), "is_ok()", "is_err()"), @@ -478,7 +476,7 @@ fn find_good_method_for_match<'a, 'tcx>( expected_item_right: Item, should_be_left: &'a str, should_be_right: &'a str, -) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> { +) -> Option<(&'a str, Option<&'tcx Expr<'tcx>>)> { let first_pat = arms[0].pat; let second_pat = arms[1].pat; @@ -496,8 +494,8 @@ fn find_good_method_for_match<'a, 'tcx>( match body_node_pair { (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) { - (LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())), - (LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())), + (LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard)), + (LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard)), _ => None, }, _ => None, @@ -511,7 +509,7 @@ fn find_good_method_for_matches_macro<'a, 'tcx>( expected_item_left: Item, should_be_left: &'a str, should_be_right: &'a str, -) -> Option<(&'a str, Option<&'tcx Guard<'tcx>>)> { +) -> Option<(&'a str, Option<&'tcx Expr<'tcx>>)> { let first_pat = arms[0].pat; let body_node_pair = if is_pat_variant(cx, first_pat, path_left, expected_item_left) { @@ -522,8 +520,8 @@ fn find_good_method_for_matches_macro<'a, 'tcx>( match body_node_pair { (ExprKind::Lit(lit_left), ExprKind::Lit(lit_right)) => match (&lit_left.node, &lit_right.node) { - (LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard.as_ref())), - (LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard.as_ref())), + (LitKind::Bool(true), LitKind::Bool(false)) => Some((should_be_left, arms[0].guard)), + (LitKind::Bool(false), LitKind::Bool(true)) => Some((should_be_right, arms[1].guard)), _ => None, }, _ => None, diff --git a/clippy_lints/src/mixed_read_write_in_expression.rs b/clippy_lints/src/mixed_read_write_in_expression.rs index 3ff40081c47..195ce17629a 100644 --- a/clippy_lints/src/mixed_read_write_in_expression.rs +++ b/clippy_lints/src/mixed_read_write_in_expression.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_note}; use clippy_utils::{get_parent_expr, path_to_local, path_to_local_id}; use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Guard, HirId, Local, Node, Stmt, StmtKind}; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind, HirId, Local, Node, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::declare_lint_pass; @@ -119,7 +119,7 @@ fn maybe_walk_expr(&mut self, e: &'tcx Expr<'_>) { ExprKind::Match(e, arms, _) => { self.visit_expr(e); for arm in arms { - if let Some(Guard::If(if_expr)) = arm.guard { + if let Some(if_expr) = arm.guard { self.visit_expr(if_expr); } // make sure top level arm expressions aren't linted diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 8817e46b3c8..8d38b87e1d7 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -318,17 +318,11 @@ fn arm(&self, arm: &Binding<&hir::Arm<'_>>) { self.pat(field!(arm.pat)); match arm.value.guard { None => chain!(self, "{arm}.guard.is_none()"), - Some(hir::Guard::If(expr)) => { + Some(expr) => { bind!(self, expr); - chain!(self, "let Some(Guard::If({expr})) = {arm}.guard"); + chain!(self, "let Some({expr}) = {arm}.guard"); self.expr(expr); }, - Some(hir::Guard::IfLet(let_expr)) => { - bind!(self, let_expr); - chain!(self, "let Some(Guard::IfLet({let_expr}) = {arm}.guard"); - self.pat(field!(let_expr.pat)); - self.expr(field!(let_expr.init)); - }, } self.expr(field!(arm.body)); } diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index e610ed93050..a23105691bf 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -8,7 +8,7 @@ use rustc_hir::MatchSource::TryDesugar; use rustc_hir::{ ArrayLen, BinOpKind, BindingAnnotation, Block, BodyId, Closure, Expr, ExprField, ExprKind, FnRetTy, GenericArg, - GenericArgs, Guard, HirId, HirIdMap, InlineAsmOperand, Let, Lifetime, LifetimeName, Pat, PatField, PatKind, Path, + GenericArgs, HirId, HirIdMap, InlineAsmOperand, Let, Lifetime, LifetimeName, Pat, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, }; use rustc_lexer::{tokenize, TokenKind}; @@ -320,7 +320,7 @@ pub fn eq_expr(&mut self, left: &Expr<'_>, right: &Expr<'_>) -> bool { && self.eq_expr(le, re) && over(la, ra, |l, r| { self.eq_pat(l.pat, r.pat) - && both(&l.guard, &r.guard, |l, r| self.eq_guard(l, r)) + && both(&l.guard, &r.guard, |l, r| self.eq_expr(l, r)) && self.eq_expr(l.body, r.body) }) }, @@ -410,16 +410,6 @@ fn eq_expr_field(&mut self, left: &ExprField<'_>, right: &ExprField<'_>) -> bool left.ident.name == right.ident.name && self.eq_expr(left.expr, right.expr) } - fn eq_guard(&mut self, left: &Guard<'_>, right: &Guard<'_>) -> bool { - match (left, right) { - (Guard::If(l), Guard::If(r)) => self.eq_expr(l, r), - (Guard::IfLet(l), Guard::IfLet(r)) => { - self.eq_pat(l.pat, r.pat) && both(&l.ty, &r.ty, |l, r| self.eq_ty(l, r)) && self.eq_expr(l.init, r.init) - }, - _ => false, - } - } - fn eq_generic_arg(&mut self, left: &GenericArg<'_>, right: &GenericArg<'_>) -> bool { match (left, right) { (GenericArg::Const(l), GenericArg::Const(r)) => self.eq_body(l.value.body, r.value.body), @@ -876,7 +866,7 @@ pub fn hash_expr(&mut self, e: &Expr<'_>) { for arm in arms { self.hash_pat(arm.pat); if let Some(ref e) = arm.guard { - self.hash_guard(e); + self.hash_expr(e); } self.hash_expr(arm.body); } @@ -1056,14 +1046,6 @@ pub fn hash_stmt(&mut self, b: &Stmt<'_>) { } } - pub fn hash_guard(&mut self, g: &Guard<'_>) { - match g { - Guard::If(expr) | Guard::IfLet(Let { init: expr, .. }) => { - self.hash_expr(expr); - }, - } - } - pub fn hash_lifetime(&mut self, lifetime: &Lifetime) { lifetime.ident.name.hash(&mut self.s); std::mem::discriminant(&lifetime.res).hash(&mut self.s); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 70a3c6f82c1..cdf8528f48a 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -3164,7 +3164,7 @@ fn visit_expr(&mut self, e: &'tcx Expr<'_>) { self.is_never = false; if let Some(guard) = arm.guard { let in_final_expr = mem::replace(&mut self.in_final_expr, false); - self.visit_expr(guard.body()); + self.visit_expr(guard); self.in_final_expr = in_final_expr; // The compiler doesn't consider diverging guards as causing the arm to diverge. self.is_never = false; @@ -3223,7 +3223,7 @@ fn visit_local(&mut self, l: &'tcx Local<'_>) { fn visit_arm(&mut self, arm: &Arm<'tcx>) { if let Some(guard) = arm.guard { let in_final_expr = mem::replace(&mut self.in_final_expr, false); - self.visit_expr(guard.body()); + self.visit_expr(guard); self.in_final_expr = in_final_expr; } self.visit_expr(arm.body); From e10a05dff3f278f89dc4bce7fb9cbafc40767d49 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 2 Jan 2024 23:32:40 +0300 Subject: [PATCH 09/13] rustc_span: Optimize syntax context comparisons Including comparisons with root context --- clippy_lints/src/casts/unnecessary_cast.rs | 2 +- clippy_lints/src/implicit_hasher.rs | 2 +- clippy_lints/src/implicit_return.rs | 2 +- clippy_lints/src/methods/option_map_unwrap_or.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs index 849920bb76d..3761ba81f52 100644 --- a/clippy_lints/src/casts/unnecessary_cast.rs +++ b/clippy_lints/src/casts/unnecessary_cast.rs @@ -145,7 +145,7 @@ pub(super) fn check<'tcx>( if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { if let Some(id) = path_to_local(cast_expr) && let Some(span) = cx.tcx.hir().opt_span(id) - && span.ctxt() != cast_expr.span.ctxt() + && !span.eq_ctxt(cast_expr.span) { // Binding context is different than the identifiers context. // Weird macro wizardry could be involved here. diff --git a/clippy_lints/src/implicit_hasher.rs b/clippy_lints/src/implicit_hasher.rs index 43eb6a9b838..788fe828727 100644 --- a/clippy_lints/src/implicit_hasher.rs +++ b/clippy_lints/src/implicit_hasher.rs @@ -118,7 +118,7 @@ fn suggestion( vis.visit_ty(impl_.self_ty); for target in &vis.found { - if item.span.ctxt() != target.span().ctxt() { + if !item.span.eq_ctxt(target.span()) { return; } diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index d68c5c4bac6..5288efd8df8 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -225,7 +225,7 @@ fn check_fn( _: LocalDefId, ) { if (!matches!(kind, FnKind::Closure) && matches!(decl.output, FnRetTy::DefaultReturn(_))) - || span.ctxt() != body.value.span.ctxt() + || !span.eq_ctxt(body.value.span) || in_external_macro(cx.sess(), span) { return; diff --git a/clippy_lints/src/methods/option_map_unwrap_or.rs b/clippy_lints/src/methods/option_map_unwrap_or.rs index 63e64a5b35d..47c9438c588 100644 --- a/clippy_lints/src/methods/option_map_unwrap_or.rs +++ b/clippy_lints/src/methods/option_map_unwrap_or.rs @@ -67,7 +67,7 @@ pub(super) fn check<'tcx>( } } - if unwrap_arg.span.ctxt() != map_span.ctxt() { + if !unwrap_arg.span.eq_ctxt(map_span) { return; } From 122520d80cde154036ed9c0b9a0aacf423b121ed Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 3 Jan 2024 12:17:35 +1100 Subject: [PATCH 10/13] Make `DiagnosticBuilder::emit` consuming. This works for most of its call sites. This is nice, because `emit` very much makes sense as a consuming operation -- indeed, `DiagnosticBuilderState` exists to ensure no diagnostic is emitted twice, but it uses runtime checks. For the small number of call sites where a consuming emit doesn't work, the commit adds `DiagnosticBuilder::emit_without_consuming`. (This will be removed in subsequent commits.) Likewise, `emit_unless` becomes consuming. And `delay_as_bug` becomes consuming, while `delay_as_bug_without_consuming` is added (which will also be removed in subsequent commits.) All this requires significant changes to `DiagnosticBuilder`'s chaining methods. Currently `DiagnosticBuilder` method chaining uses a non-consuming `&mut self -> &mut Self` style, which allows chaining to be used when the chain ends in `emit()`, like so: ``` struct_err(msg).span(span).emit(); ``` But it doesn't work when producing a `DiagnosticBuilder` value, requiring this: ``` let mut err = self.struct_err(msg); err.span(span); err ``` This style of chaining won't work with consuming `emit` though. For that, we need to use to a `self -> Self` style. That also would allow `DiagnosticBuilder` production to be chained, e.g.: ``` self.struct_err(msg).span(span) ``` However, removing the `&mut self -> &mut Self` style would require that individual modifications of a `DiagnosticBuilder` go from this: ``` err.span(span); ``` to this: ``` err = err.span(span); ``` There are *many* such places. I have a high tolerance for tedious refactorings, but even I gave up after a long time trying to convert them all. Instead, this commit has it both ways: the existing `&mut self -> Self` chaining methods are kept, and new `self -> Self` chaining methods are added, all of which have a `_mv` suffix (short for "move"). Changes to the existing `forward!` macro lets this happen with very little additional boilerplate code. I chose to add the suffix to the new chaining methods rather than the existing ones, because the number of changes required is much smaller that way. This doubled chainging is a bit clumsy, but I think it is worthwhile because it allows a *lot* of good things to subsequently happen. In this commit, there are many `mut` qualifiers removed in places where diagnostics are emitted without being modified. In subsequent commits: - chaining can be used more, making the code more concise; - more use of chaining also permits the removal of redundant diagnostic APIs like `struct_err_with_code`, which can be replaced easily with `struct_err` + `code_mv`; - `emit_without_diagnostic` can be removed, which simplifies a lot of machinery, removing the need for `DiagnosticBuilderState`. --- clippy_config/src/msrvs.rs | 2 +- clippy_utils/src/attrs.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index 471ad73e207..76f3663f04f 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -109,7 +109,7 @@ fn parse_attr(sess: &Session, attrs: &[Attribute]) -> Option { if let Some(duplicate) = msrv_attrs.last() { sess.dcx() .struct_span_err(duplicate.span, "`clippy::msrv` is defined multiple times") - .span_note(msrv_attr.span, "first definition found here") + .span_note_mv(msrv_attr.span, "first definition found here") .emit(); } diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index db80e07ca1c..19d38903ade 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -136,7 +136,7 @@ pub fn get_unique_attr<'a>( if let Some(duplicate) = unique_attr { sess.dcx() .struct_span_err(attr.span, format!("`{name}` is defined multiple times")) - .span_note(duplicate.span, "first definition found here") + .span_note_mv(duplicate.span, "first definition found here") .emit(); } else { unique_attr = Some(attr); From 89f511e4dd4cde330c85f7b596d3f68d381a582e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 12 Nov 2023 18:57:24 +0000 Subject: [PATCH 11/13] Rustdoc and Clippy stop misusing Key for Ty -> (adt) DefId --- clippy_lints/src/copies.rs | 3 +-- clippy_lints/src/methods/drain_collect.rs | 7 +++---- clippy_lints/src/methods/redundant_as_str.rs | 7 +------ clippy_lints/src/mut_key.rs | 3 +-- clippy_lints/src/non_copy_const.rs | 3 +-- clippy_lints/src/transmute/transmute_int_to_non_zero.rs | 5 ++--- 6 files changed, 9 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/copies.rs b/clippy_lints/src/copies.rs index d91af76f5e0..bd07c19a2d8 100644 --- a/clippy_lints/src/copies.rs +++ b/clippy_lints/src/copies.rs @@ -12,7 +12,6 @@ use rustc_hir::def_id::DefIdSet; use rustc_hir::{intravisit, BinOpKind, Block, Expr, ExprKind, HirId, HirIdSet, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::query::Key; use rustc_session::impl_lint_pass; use rustc_span::hygiene::walk_chain; use rustc_span::source_map::SourceMap; @@ -574,7 +573,7 @@ fn method_caller_is_mutable(cx: &LateContext<'_>, caller_expr: &Expr<'_>, ignore let caller_ty = cx.typeck_results().expr_ty(caller_expr); // Check if given type has inner mutability and was not set to ignored by the configuration let is_inner_mut_ty = is_interior_mut_ty(cx, caller_ty) - && !matches!(caller_ty.ty_adt_id(), Some(adt_id) if ignored_ty_ids.contains(&adt_id)); + && !matches!(caller_ty.ty_adt_def(), Some(adt) if ignored_ty_ids.contains(&adt.did())); is_inner_mut_ty || caller_ty.is_mutable_ptr() diff --git a/clippy_lints/src/methods/drain_collect.rs b/clippy_lints/src/methods/drain_collect.rs index 6a82d8f756a..3a8ca37610a 100644 --- a/clippy_lints/src/methods/drain_collect.rs +++ b/clippy_lints/src/methods/drain_collect.rs @@ -6,7 +6,6 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, LangItem, Path, QPath}; use rustc_lint::LateContext; -use rustc_middle::query::Key; use rustc_middle::ty; use rustc_middle::ty::Ty; use rustc_span::{sym, Symbol}; @@ -18,10 +17,10 @@ /// `vec![1,2].drain(..).collect::>()` /// ^^^^^^^^^ ^^^^^^^^^^ false fn types_match_diagnostic_item(cx: &LateContext<'_>, expr: Ty<'_>, recv: Ty<'_>, sym: Symbol) -> bool { - if let Some(expr_adt_did) = expr.ty_adt_id() - && let Some(recv_adt_did) = recv.ty_adt_id() + if let Some(expr_adt) = expr.ty_adt_def() + && let Some(recv_adt) = recv.ty_adt_def() { - cx.tcx.is_diagnostic_item(sym, expr_adt_did) && cx.tcx.is_diagnostic_item(sym, recv_adt_did) + cx.tcx.is_diagnostic_item(sym, expr_adt.did()) && cx.tcx.is_diagnostic_item(sym, recv_adt.did()) } else { false } diff --git a/clippy_lints/src/methods/redundant_as_str.rs b/clippy_lints/src/methods/redundant_as_str.rs index 98cd6afc2b7..2a2feedd2b4 100644 --- a/clippy_lints/src/methods/redundant_as_str.rs +++ b/clippy_lints/src/methods/redundant_as_str.rs @@ -4,7 +4,6 @@ use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; -use rustc_middle::query::Key; use rustc_span::Span; pub(super) fn check( @@ -14,11 +13,7 @@ pub(super) fn check( as_str_span: Span, other_method_span: Span, ) { - if cx - .tcx - .lang_items() - .string() - .is_some_and(|id| Some(id) == cx.typeck_results().expr_ty(recv).ty_adt_id()) + if cx.typeck_results().expr_ty(recv).ty_adt_def().is_some_and(|adt| Some(adt.did()) == cx.tcx.lang_items().string()) { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( diff --git a/clippy_lints/src/mut_key.rs b/clippy_lints/src/mut_key.rs index 04d2ced6abf..c32025fcbb6 100644 --- a/clippy_lints/src/mut_key.rs +++ b/clippy_lints/src/mut_key.rs @@ -4,7 +4,6 @@ use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::query::Key; use rustc_middle::ty::{Adt, Ty}; use rustc_session::impl_lint_pass; use rustc_span::def_id::LocalDefId; @@ -166,7 +165,7 @@ fn check_ty_<'tcx>(&self, cx: &LateContext<'tcx>, span: Span, ty: Ty<'tcx>) { // Determines if a type contains interior mutability which would affect its implementation of // [`Hash`] or [`Ord`]. if is_interior_mut_ty(cx, subst_ty) - && !matches!(subst_ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id)) + && !matches!(subst_ty.ty_adt_def(), Some(adt) if self.ignore_mut_def_ids.contains(&adt.did())) { span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type"); } diff --git a/clippy_lints/src/non_copy_const.rs b/clippy_lints/src/non_copy_const.rs index 4013cb34561..f8365deebd4 100644 --- a/clippy_lints/src/non_copy_const.rs +++ b/clippy_lints/src/non_copy_const.rs @@ -15,7 +15,6 @@ }; use rustc_lint::{LateContext, LateLintPass, Lint}; use rustc_middle::mir::interpret::{ErrorHandled, EvalToValTreeResult, GlobalId}; -use rustc_middle::query::Key; use rustc_middle::ty::adjustment::Adjust; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_session::impl_lint_pass; @@ -188,7 +187,7 @@ pub fn new(ignore_interior_mutability: Vec) -> Self { } fn is_ty_ignored(&self, ty: Ty<'_>) -> bool { - matches!(ty.ty_adt_id(), Some(adt_id) if self.ignore_mut_def_ids.contains(&adt_id)) + matches!(ty.ty_adt_def(), Some(adt) if self.ignore_mut_def_ids.contains(&adt.did())) } fn is_unfrozen<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool { diff --git a/clippy_lints/src/transmute/transmute_int_to_non_zero.rs b/clippy_lints/src/transmute/transmute_int_to_non_zero.rs index c0d0d2b93dc..5df645491ff 100644 --- a/clippy_lints/src/transmute/transmute_int_to_non_zero.rs +++ b/clippy_lints/src/transmute/transmute_int_to_non_zero.rs @@ -4,7 +4,6 @@ use rustc_errors::Applicability; use rustc_hir::Expr; use rustc_lint::LateContext; -use rustc_middle::query::Key; use rustc_middle::ty::{self, Ty}; use rustc_span::symbol::sym; @@ -17,10 +16,10 @@ pub(super) fn check<'tcx>( to_ty: Ty<'tcx>, arg: &'tcx Expr<'_>, ) -> bool { - let (ty::Int(_) | ty::Uint(_), Some(to_ty_id)) = (&from_ty.kind(), to_ty.ty_adt_id()) else { + let (ty::Int(_) | ty::Uint(_), Some(to_ty_adt)) = (&from_ty.kind(), to_ty.ty_adt_def()) else { return false; }; - let Some(to_type_sym) = cx.tcx.get_diagnostic_name(to_ty_id) else { + let Some(to_type_sym) = cx.tcx.get_diagnostic_name(to_ty_adt.did()) else { return false; }; From beeaee97859f34c7a618565e6ff8539b52ac3912 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 9 Jan 2024 09:08:49 +1100 Subject: [PATCH 12/13] Rename consuming chaining methods on `DiagnosticBuilder`. In #119606 I added them and used a `_mv` suffix, but that wasn't great. A `with_` prefix has three different existing uses. - Constructors, e.g. `Vec::with_capacity`. - Wrappers that provide an environment to execute some code, e.g. `with_session_globals`. - Consuming chaining methods, e.g. `Span::with_{lo,hi,ctxt}`. The third case is exactly what we want, so this commit changes `DiagnosticBuilder::foo_mv` to `DiagnosticBuilder::with_foo`. Thanks to @compiler-errors for the suggestion. --- clippy_config/src/msrvs.rs | 2 +- clippy_utils/src/attrs.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_config/src/msrvs.rs b/clippy_config/src/msrvs.rs index 76f3663f04f..dae9f09ec00 100644 --- a/clippy_config/src/msrvs.rs +++ b/clippy_config/src/msrvs.rs @@ -109,7 +109,7 @@ fn parse_attr(sess: &Session, attrs: &[Attribute]) -> Option { if let Some(duplicate) = msrv_attrs.last() { sess.dcx() .struct_span_err(duplicate.span, "`clippy::msrv` is defined multiple times") - .span_note_mv(msrv_attr.span, "first definition found here") + .with_span_note(msrv_attr.span, "first definition found here") .emit(); } diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index 19d38903ade..ad8619f0d3d 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -136,7 +136,7 @@ pub fn get_unique_attr<'a>( if let Some(duplicate) = unique_attr { sess.dcx() .struct_span_err(attr.span, format!("`{name}` is defined multiple times")) - .span_note_mv(duplicate.span, "first definition found here") + .with_span_note(duplicate.span, "first definition found here") .emit(); } else { unique_attr = Some(attr); From 7d8b6e451ec1e6cfeba78755a75ed0c6240be6b1 Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Thu, 11 Jan 2024 17:20:07 +0100 Subject: [PATCH 13/13] Bump nightly version -> 2024-01-11 --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index 5a2526fd267..2589d46e7da 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,3 +1,3 @@ [toolchain] -channel = "nightly-2023-12-28" +channel = "nightly-2024-01-11" components = ["cargo", "llvm-tools", "rust-src", "rust-std", "rustc", "rustc-dev", "rustfmt"]