From 2d10babb71f6658c39279425ec5cda11a28d3af4 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 1 Feb 2020 01:02:31 +0300 Subject: [PATCH 1/5] Stabilize fn-like proc macros in expression, pattern and statement positions --- mini-macro/src/lib.rs | 2 +- tests/ui/auxiliary/proc_macro_derive.rs | 2 +- tests/ui/crashes/ice-3741.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/mini-macro/src/lib.rs b/mini-macro/src/lib.rs index 92b6f701555..ba946563ec5 100644 --- a/mini-macro/src/lib.rs +++ b/mini-macro/src/lib.rs @@ -1,4 +1,4 @@ -#![feature(proc_macro_quote, proc_macro_hygiene)] +#![feature(proc_macro_quote)] #![deny(rust_2018_idioms)] // FIXME: Remove this attribute once the weird failure is gone. #![allow(unused_extern_crates)] diff --git a/tests/ui/auxiliary/proc_macro_derive.rs b/tests/ui/auxiliary/proc_macro_derive.rs index 21bb5b01e02..05ffb55f620 100644 --- a/tests/ui/auxiliary/proc_macro_derive.rs +++ b/tests/ui/auxiliary/proc_macro_derive.rs @@ -1,7 +1,7 @@ // no-prefer-dynamic #![crate_type = "proc-macro"] -#![feature(repr128, proc_macro_hygiene, proc_macro_quote)] +#![feature(repr128, proc_macro_quote)] extern crate proc_macro; diff --git a/tests/ui/crashes/ice-3741.rs b/tests/ui/crashes/ice-3741.rs index 74b9c9c86c8..a548415da62 100644 --- a/tests/ui/crashes/ice-3741.rs +++ b/tests/ui/crashes/ice-3741.rs @@ -1,7 +1,6 @@ // aux-build:proc_macro_crash.rs // run-pass -#![feature(proc_macro_hygiene)] #![warn(clippy::suspicious_else_formatting)] extern crate proc_macro_crash; From f1d3086492db10deaa3268952792e93ad09ecec0 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sun, 17 May 2020 17:36:26 +0200 Subject: [PATCH 2/5] Merge commit 'e214ea82ad0a751563acf67e1cd9279cf302db3a' into clippyup --- CHANGELOG.md | 44 +-- ...ondition.rs => blocks_in_if_conditions.rs} | 55 ++- clippy_lints/src/bytecount.rs | 2 +- clippy_lints/src/comparison_chain.rs | 17 +- clippy_lints/src/eq_op.rs | 2 +- clippy_lints/src/identity_op.rs | 22 +- clippy_lints/src/let_if_seq.rs | 2 +- clippy_lints/src/lib.rs | 84 ++--- clippy_lints/src/loops.rs | 184 ++-------- clippy_lints/src/map_clone.rs | 2 +- clippy_lints/src/matches.rs | 2 +- .../src/methods/bind_instead_of_map.rs | 309 ++++++++++++++++ clippy_lints/src/methods/mod.rs | 331 ++++++------------ .../src/methods/option_map_unwrap_or.rs | 6 +- clippy_lints/src/needless_pass_by_value.rs | 2 +- clippy_lints/src/ranges.rs | 112 +++++- clippy_lints/src/returns.rs | 64 ++-- clippy_lints/src/types.rs | 4 +- clippy_lints/src/unwrap.rs | 12 +- ...ty_conversion.rs => useless_conversion.rs} | 30 +- clippy_lints/src/utils/diagnostics.rs | 28 +- clippy_lints/src/utils/usage.rs | 4 +- src/driver.rs | 228 ++++++------ src/lintlist/mod.rs | 130 +++---- ...n_some.fixed => bind_instead_of_map.fixed} | 4 +- ...nd_then_some.rs => bind_instead_of_map.rs} | 2 +- ...some.stderr => bind_instead_of_map.stderr} | 18 +- tests/ui/bind_instead_of_map_multipart.rs | 61 ++++ tests/ui/bind_instead_of_map_multipart.stderr | 73 ++++ ...on.fixed => blocks_in_if_conditions.fixed} | 7 +- ...ondition.rs => blocks_in_if_conditions.rs} | 7 +- ....stderr => blocks_in_if_conditions.stderr} | 10 +- ....rs => blocks_in_if_conditions_closure.rs} | 5 +- ...=> blocks_in_if_conditions_closure.stderr} | 6 +- tests/ui/comparison_chain.rs | 66 ++++ tests/ui/crashes/ice-5579.rs | 17 + tests/ui/expect.rs | 2 +- tests/ui/expect.stderr | 3 +- tests/ui/for_loop_fixable.fixed | 54 --- tests/ui/for_loop_fixable.rs | 54 --- tests/ui/for_loop_fixable.stderr | 88 +---- tests/ui/for_loop_unfixable.rs | 18 - tests/ui/for_loop_unfixable.stderr | 11 +- ..._result.rs => for_loops_over_fallibles.rs} | 10 +- ...stderr => for_loops_over_fallibles.stderr} | 19 +- tests/ui/identity_op.rs | 5 + tests/ui/identity_op.stderr | 20 +- tests/ui/implicit_saturating_sub.fixed | 4 +- tests/ui/implicit_saturating_sub.rs | 4 +- tests/ui/manual_memcpy.rs | 2 +- ...tion_map_unwrap_or.rs => map_unwrap_or.rs} | 23 +- ..._unwrap_or.stderr => map_unwrap_or.stderr} | 49 ++- tests/ui/option_map_or_none.fixed | 2 +- tests/ui/option_map_or_none.rs | 2 +- tests/ui/result_map_unwrap_or_else.rs | 23 -- tests/ui/result_map_unwrap_or_else.stderr | 27 -- tests/ui/reversed_empty_ranges_fixable.fixed | 24 ++ tests/ui/reversed_empty_ranges_fixable.rs | 24 ++ tests/ui/reversed_empty_ranges_fixable.stderr | 47 +++ .../reversed_empty_ranges_loops_fixable.fixed | 57 +++ .../ui/reversed_empty_ranges_loops_fixable.rs | 57 +++ ...reversed_empty_ranges_loops_fixable.stderr | 69 ++++ .../reversed_empty_ranges_loops_unfixable.rs | 11 + ...versed_empty_ranges_loops_unfixable.stderr | 16 + tests/ui/reversed_empty_ranges_unfixable.rs | 15 + .../ui/reversed_empty_ranges_unfixable.stderr | 34 ++ tests/ui/unused_unit.fixed | 21 +- tests/ui/unused_unit.rs | 18 +- tests/ui/unused_unit.stderr | 76 +++- tests/ui/unwrap.rs | 2 +- tests/ui/unwrap.stderr | 3 +- ...version.fixed => useless_conversion.fixed} | 4 +- ...ty_conversion.rs => useless_conversion.rs} | 4 +- ...rsion.stderr => useless_conversion.stderr} | 46 +-- 74 files changed, 1784 insertions(+), 1116 deletions(-) rename clippy_lints/src/{block_in_if_condition.rs => blocks_in_if_conditions.rs} (78%) create mode 100644 clippy_lints/src/methods/bind_instead_of_map.rs rename clippy_lints/src/{identity_conversion.rs => useless_conversion.rs} (84%) rename tests/ui/{option_and_then_some.fixed => bind_instead_of_map.fixed} (90%) rename tests/ui/{option_and_then_some.rs => bind_instead_of_map.rs} (94%) rename tests/ui/{option_and_then_some.stderr => bind_instead_of_map.stderr} (50%) create mode 100644 tests/ui/bind_instead_of_map_multipart.rs create mode 100644 tests/ui/bind_instead_of_map_multipart.stderr rename tests/ui/{block_in_if_condition.fixed => blocks_in_if_conditions.fixed} (88%) rename tests/ui/{block_in_if_condition.rs => blocks_in_if_conditions.rs} (88%) rename tests/ui/{block_in_if_condition.stderr => blocks_in_if_conditions.stderr} (71%) rename tests/ui/{block_in_if_condition_closure.rs => blocks_in_if_conditions_closure.rs} (85%) rename tests/ui/{block_in_if_condition_closure.stderr => blocks_in_if_conditions_closure.stderr} (78%) create mode 100644 tests/ui/crashes/ice-5579.rs rename tests/ui/{for_loop_over_option_result.rs => for_loops_over_fallibles.rs} (81%) rename tests/ui/{for_loop_over_option_result.stderr => for_loops_over_fallibles.stderr} (81%) rename tests/ui/{option_map_unwrap_or.rs => map_unwrap_or.rs} (75%) rename tests/ui/{option_map_unwrap_or.stderr => map_unwrap_or.stderr} (70%) delete mode 100644 tests/ui/result_map_unwrap_or_else.rs delete mode 100644 tests/ui/result_map_unwrap_or_else.stderr create mode 100644 tests/ui/reversed_empty_ranges_fixable.fixed create mode 100644 tests/ui/reversed_empty_ranges_fixable.rs create mode 100644 tests/ui/reversed_empty_ranges_fixable.stderr create mode 100644 tests/ui/reversed_empty_ranges_loops_fixable.fixed create mode 100644 tests/ui/reversed_empty_ranges_loops_fixable.rs create mode 100644 tests/ui/reversed_empty_ranges_loops_fixable.stderr create mode 100644 tests/ui/reversed_empty_ranges_loops_unfixable.rs create mode 100644 tests/ui/reversed_empty_ranges_loops_unfixable.stderr create mode 100644 tests/ui/reversed_empty_ranges_unfixable.rs create mode 100644 tests/ui/reversed_empty_ranges_unfixable.stderr rename tests/ui/{identity_conversion.fixed => useless_conversion.fixed} (93%) rename tests/ui/{identity_conversion.rs => useless_conversion.rs} (94%) rename tests/ui/{identity_conversion.stderr => useless_conversion.stderr} (67%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8457d6ad05c..583c32ca9e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -198,7 +198,7 @@ Released 2020-03-12 ### Suggestion Improvements -* [`option_map_unwrap_or`] [#4634](https://github.com/rust-lang/rust-clippy/pull/4634) +* `option_map_unwrap_or` [#4634](https://github.com/rust-lang/rust-clippy/pull/4634) * [`wildcard_enum_match_arm`] [#4934](https://github.com/rust-lang/rust-clippy/pull/4934) * [`cognitive_complexity`] [#4935](https://github.com/rust-lang/rust-clippy/pull/4935) * [`decimal_literal_representation`] [#4956](https://github.com/rust-lang/rust-clippy/pull/4956) @@ -282,8 +282,8 @@ Released 2019-12-19 * [`panic`] [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) * [`unreachable`] [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) * [`todo`] [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) - * [`option_expect_used`] [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) - * [`result_expect_used`] [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) + * `option_expect_used` [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) + * `result_expect_used` [#4657](https://github.com/rust-lang/rust-clippy/pull/4657) * Move `redundant_clone` to perf group [#4509](https://github.com/rust-lang/rust-clippy/pull/4509) * Move `manual_mul_add` to nursery group [#4736](https://github.com/rust-lang/rust-clippy/pull/4736) * Expand `unit_cmp` to also work with `assert_eq!`, `debug_assert_eq!`, `assert_ne!` and `debug_assert_ne!` [#4613](https://github.com/rust-lang/rust-clippy/pull/4613) @@ -315,7 +315,7 @@ Released 2019-11-07 * [`missing_safety_doc`] [#4535](https://github.com/rust-lang/rust-clippy/pull/4535) * [`mem_replace_with_uninit`] [#4511](https://github.com/rust-lang/rust-clippy/pull/4511) * [`suspicious_map`] [#4394](https://github.com/rust-lang/rust-clippy/pull/4394) - * [`option_and_then_some`] [#4386](https://github.com/rust-lang/rust-clippy/pull/4386) + * `option_and_then_some` [#4386](https://github.com/rust-lang/rust-clippy/pull/4386) * [`manual_saturating_arithmetic`] [#4498](https://github.com/rust-lang/rust-clippy/pull/4498) * Deprecate `unused_collect` lint. This is fully covered by rustc's `#[must_use]` on `collect` [#4348](https://github.com/rust-lang/rust-clippy/pull/4348) * Move `type_repetition_in_bounds` to pedantic group [#4403](https://github.com/rust-lang/rust-clippy/pull/4403) @@ -395,7 +395,7 @@ Released 2019-08-15 * Fix false positive in [`useless_attribute`] [#4107](https://github.com/rust-lang/rust-clippy/pull/4107) * Fix incorrect suggestion for [`float_cmp`] [#4214](https://github.com/rust-lang/rust-clippy/pull/4214) * Add suggestions for [`print_with_newline`] and [`write_with_newline`] [#4136](https://github.com/rust-lang/rust-clippy/pull/4136) -* Improve suggestions for [`option_map_unwrap_or_else`] and [`result_map_unwrap_or_else`] [#4164](https://github.com/rust-lang/rust-clippy/pull/4164) +* Improve suggestions for `option_map_unwrap_or_else` and `result_map_unwrap_or_else` [#4164](https://github.com/rust-lang/rust-clippy/pull/4164) * Improve suggestions for [`non_ascii_literal`] [#4119](https://github.com/rust-lang/rust-clippy/pull/4119) * Improve diagnostics for [`let_and_return`] [#4137](https://github.com/rust-lang/rust-clippy/pull/4137) * Improve diagnostics for [`trivially_copy_pass_by_ref`] [#4071](https://github.com/rust-lang/rust-clippy/pull/4071) @@ -448,7 +448,7 @@ Released 2019-05-20 * Fix false positive in [`needless_range_loop`] pertaining to structs without a `.iter()` * Fix false positive in [`bool_comparison`] pertaining to non-bool types * Fix false positive in [`redundant_closure`] pertaining to differences in borrows -* Fix false positive in [`option_map_unwrap_or`] on non-copy types +* Fix false positive in `option_map_unwrap_or` on non-copy types * Fix false positives in [`missing_const_for_fn`] pertaining to macros and trait method impls * Fix false positive in [`needless_pass_by_value`] pertaining to procedural macros * Fix false positive in [`needless_continue`] pertaining to loop labels @@ -794,7 +794,7 @@ Released 2018-09-13 ## 0.0.169 * Rustup to *rustc 1.23.0-nightly (3b82e4c74 2017-11-05)* -* New lints: [`just_underscores_and_digits`], [`result_map_unwrap_or_else`], [`transmute_bytes_to_str`] +* New lints: [`just_underscores_and_digits`], `result_map_unwrap_or_else`, [`transmute_bytes_to_str`] ## 0.0.168 * Rustup to *rustc 1.23.0-nightly (f0fe716db 2017-10-30)* @@ -805,7 +805,7 @@ Released 2018-09-13 ## 0.0.166 * Rustup to *rustc 1.22.0-nightly (b7960878b 2017-10-18)* -* New lints: [`explicit_write`], [`identity_conversion`], [`implicit_hasher`], [`invalid_ref`], [`option_map_or_none`], +* New lints: [`explicit_write`], `identity_conversion`, [`implicit_hasher`], [`invalid_ref`], [`option_map_or_none`], [`range_minus_one`], [`range_plus_one`], [`transmute_int_to_bool`], [`transmute_int_to_char`], [`transmute_int_to_float`] @@ -1068,7 +1068,7 @@ Released 2018-09-13 ## 0.0.93 — 2016-10-03 * Rustup to *rustc 1.14.0-nightly (144af3e97 2016-10-02)* -* [`option_map_unwrap_or`] and [`option_map_unwrap_or_else`] are now +* `option_map_unwrap_or` and `option_map_unwrap_or_else` are now allowed by default. * New lint: [`explicit_into_iter_loop`] @@ -1087,8 +1087,8 @@ Released 2018-09-13 ## 0.0.88 — 2016-09-04 * Rustup to *rustc 1.13.0-nightly (70598e04f 2016-09-03)* * The following lints are not new but were only usable through the `clippy` - lint groups: [`filter_next`], [`for_loop_over_option`], - [`for_loop_over_result`] and [`match_overlapping_arm`]. You should now be + lint groups: [`filter_next`], `for_loop_over_option`, + `for_loop_over_result` and [`match_overlapping_arm`]. You should now be able to `#[allow/deny]` them individually and they are available directly through `cargo clippy`. @@ -1273,9 +1273,9 @@ Released 2018-09-13 [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops [`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask +[`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map [`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name -[`block_in_if_condition_expr`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_expr -[`block_in_if_condition_stmt`]: https://rust-lang.github.io/rust-clippy/master/index.html#block_in_if_condition_stmt +[`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions [`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison [`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const [`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box @@ -1338,6 +1338,7 @@ Released 2018-09-13 [`excessive_precision`]: https://rust-lang.github.io/rust-clippy/master/index.html#excessive_precision [`exit`]: https://rust-lang.github.io/rust-clippy/master/index.html#exit [`expect_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_fun_call +[`expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#expect_used [`expl_impl_clone_on_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#expl_impl_clone_on_copy [`explicit_counter_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_counter_loop [`explicit_deref_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_deref_methods @@ -1361,14 +1362,12 @@ Released 2018-09-13 [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast [`fn_to_numeric_cast_with_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation [`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map -[`for_loop_over_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loop_over_option -[`for_loop_over_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loop_over_result +[`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles [`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy [`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref [`future_not_send`]: https://rust-lang.github.io/rust-clippy/master/index.html#future_not_send [`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len [`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap -[`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion [`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op [`if_let_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_mutex [`if_let_redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_let_redundant_pattern_matching @@ -1431,6 +1430,7 @@ Released 2018-09-13 [`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone [`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry [`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten +[`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or [`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref [`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool [`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items @@ -1494,16 +1494,11 @@ Released 2018-09-13 [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref [`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref -[`option_and_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_and_then_some [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap -[`option_expect_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_expect_used [`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none [`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn -[`option_map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or -[`option_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unwrap_or_else [`option_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option -[`option_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_unwrap_used [`or_fun_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call [`out_of_bounds_indexing`]: https://rust-lang.github.io/rust-clippy/master/index.html#out_of_bounds_indexing [`overflow_check_conditional`]: https://rust-lang.github.io/rust-clippy/master/index.html#overflow_check_conditional @@ -1540,12 +1535,9 @@ Released 2018-09-13 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts [`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_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 -[`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else -[`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used -[`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop +[`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse @@ -1625,11 +1617,13 @@ Released 2018-09-13 [`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label [`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit +[`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref [`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute +[`useless_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion [`useless_format`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_format [`useless_let_if_seq`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_let_if_seq [`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute diff --git a/clippy_lints/src/block_in_if_condition.rs b/clippy_lints/src/blocks_in_if_conditions.rs similarity index 78% rename from clippy_lints/src/block_in_if_condition.rs rename to clippy_lints/src/blocks_in_if_conditions.rs index 9e533eaa32c..8fa9b05ca32 100644 --- a/clippy_lints/src/block_in_if_condition.rs +++ b/clippy_lints/src/blocks_in_if_conditions.rs @@ -8,43 +8,40 @@ use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { - /// **What it does:** Checks for `if` conditions that use blocks to contain an - /// expression. + /// **What it does:** Checks for `if` conditions that use blocks containing an + /// expression, statements or conditions that use closures with blocks. /// - /// **Why is this bad?** It isn't really Rust style, same as using parentheses - /// to contain expressions. + /// **Why is this bad?** Style, using blocks in the condition makes it hard to read. /// /// **Known problems:** None. /// - /// **Example:** + /// **Examples:** /// ```rust + /// // Bad /// if { true } { /* ... */ } + /// + /// // Good + /// if true { /* ... */ } /// ``` - pub BLOCK_IN_IF_CONDITION_EXPR, - style, - "braces that can be eliminated in conditions, e.g., `if { true } ...`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for `if` conditions that use blocks containing - /// statements, or conditions that use closures with blocks. /// - /// **Why is this bad?** Using blocks in the condition makes it hard to read. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```rust,ignore - /// if { let x = somefunc(); x } {} /// // or - /// if somefunc(|x| { x == 47 }) {} + /// + /// ```rust + /// # fn somefunc() -> bool { true }; + /// + /// // Bad + /// if { let x = somefunc(); x } { /* ... */ } + /// + /// // Good + /// let res = { let x = somefunc(); x }; + /// if res { /* ... */ } /// ``` - pub BLOCK_IN_IF_CONDITION_STMT, + pub BLOCKS_IN_IF_CONDITIONS, style, - "complex blocks in conditions, e.g., `if { let x = true; x } ...`" + "useless or complex blocks that can be eliminated in conditions" } -declare_lint_pass!(BlockInIfCondition => [BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT]); +declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]); struct ExVisitor<'a, 'tcx> { found_block: Option<&'tcx Expr<'tcx>>, @@ -72,9 +69,9 @@ impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> { const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition"; const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \ - instead, move the block or closure higher and bind it with a `let`"; + instead, move the block or closure higher and bind it with a `let`"; -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlocksInIfConditions { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) { if in_external_macro(cx.sess(), expr.span) { return; @@ -92,7 +89,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( cx, - BLOCK_IN_IF_CONDITION_EXPR, + BLOCKS_IN_IF_CONDITIONS, cond.span, BRACED_EXPR_MESSAGE, "try", @@ -118,7 +115,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { let mut applicability = Applicability::MachineApplicable; span_lint_and_sugg( cx, - BLOCK_IN_IF_CONDITION_STMT, + BLOCKS_IN_IF_CONDITIONS, expr.span.with_hi(cond.span.hi()), COMPLEX_BLOCK_MESSAGE, "try", @@ -140,7 +137,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition { let mut visitor = ExVisitor { found_block: None, cx }; walk_expr(&mut visitor, cond); if let Some(block) = visitor.found_block { - span_lint(cx, BLOCK_IN_IF_CONDITION_STMT, block.span, COMPLEX_BLOCK_MESSAGE); + span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE); } } } diff --git a/clippy_lints/src/bytecount.rs b/clippy_lints/src/bytecount.rs index 278d043732f..90c00ad098f 100644 --- a/clippy_lints/src/bytecount.rs +++ b/clippy_lints/src/bytecount.rs @@ -3,7 +3,7 @@ use crate::utils::{ span_lint_and_sugg, walk_ptrs_ty, }; use if_chain::if_chain; -use rustc_ast::ast::{UintTy}; +use rustc_ast::ast::UintTy; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs index 96df3ffe3ce..93e29edcaa5 100644 --- a/clippy_lints/src/comparison_chain.rs +++ b/clippy_lints/src/comparison_chain.rs @@ -81,12 +81,23 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain { // Check that both sets of operands are equal let mut spanless_eq = SpanlessEq::new(cx); - if (!spanless_eq.eq_expr(lhs1, lhs2) || !spanless_eq.eq_expr(rhs1, rhs2)) - && (!spanless_eq.eq_expr(lhs1, rhs2) || !spanless_eq.eq_expr(rhs1, lhs2)) - { + let same_fixed_operands = spanless_eq.eq_expr(lhs1, lhs2) && spanless_eq.eq_expr(rhs1, rhs2); + let same_transposed_operands = spanless_eq.eq_expr(lhs1, rhs2) && spanless_eq.eq_expr(rhs1, lhs2); + + if !same_fixed_operands && !same_transposed_operands { return; } + // Check that if the operation is the same, either it's not `==` or the operands are transposed + if kind1.node == kind2.node { + if kind1.node == BinOpKind::Eq { + return; + } + if !same_transposed_operands { + return; + } + } + // Check that the type being compared implements `core::cmp::Ord` let ty = cx.tables.expr_ty(lhs1); let is_ord = get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[])); diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index 098d47bdd40..4e1c1f13140 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -115,7 +115,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp { let rsnip = snippet(cx, r.span, "...").to_string(); multispan_sugg( diag, - "use the values directly".to_string(), + "use the values directly", vec![(left.span, lsnip), (right.span, rsnip)], ); }, diff --git a/clippy_lints/src/identity_op.rs b/clippy_lints/src/identity_op.rs index 088e4ab1921..78e07d25f67 100644 --- a/clippy_lints/src/identity_op.rs +++ b/clippy_lints/src/identity_op.rs @@ -1,4 +1,5 @@ -use rustc_hir::{BinOpKind, Expr, ExprKind}; +use if_chain::if_chain; +use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -32,7 +33,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityOp { if e.span.from_expansion() { return; } - if let ExprKind::Binary(ref cmp, ref left, ref right) = e.kind { + if let ExprKind::Binary(cmp, ref left, ref right) = e.kind { + if is_allowed(cx, cmp, left, right) { + return; + } match cmp.node { BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => { check(cx, left, 0, e.span, right.span); @@ -54,6 +58,20 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityOp { } } +fn is_allowed(cx: &LateContext<'_, '_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool { + // `1 << 0` is a common pattern in bit manipulation code + if_chain! { + if let BinOpKind::Shl = cmp.node; + if let Some(Constant::Int(0)) = constant_simple(cx, cx.tables, right); + if let Some(Constant::Int(1)) = constant_simple(cx, cx.tables, left); + then { + return true; + } + } + + false +} + #[allow(clippy::cast_possible_wrap)] fn check(cx: &LateContext<'_, '_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) { if let Some(Constant::Int(v)) = constant_simple(cx, cx.tables, e) { diff --git a/clippy_lints/src/let_if_seq.rs b/clippy_lints/src/let_if_seq.rs index 398a3103a03..d7bf8a14768 100644 --- a/clippy_lints/src/let_if_seq.rs +++ b/clippy_lints/src/let_if_seq.rs @@ -50,7 +50,7 @@ declare_clippy_lint! { /// }; /// ``` pub USELESS_LET_IF_SEQ, - style, + nursery, "unidiomatic `let mut` declaration followed by initialization in `if`" } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 51b5401da7d..e0787ac0887 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -180,7 +180,7 @@ mod attrs; mod await_holding_lock; mod bit_mask; mod blacklisted_name; -mod block_in_if_condition; +mod blocks_in_if_conditions; mod booleans; mod bytecount; mod cargo_common_metadata; @@ -221,7 +221,6 @@ mod formatting; mod functions; mod future_not_send; mod get_last_with_len; -mod identity_conversion; mod identity_op; mod if_let_mutex; mod if_let_some_result; @@ -324,6 +323,7 @@ mod unused_io_amount; mod unused_self; mod unwrap; mod use_self; +mod useless_conversion; mod vec; mod verbose_file_reads; mod wildcard_dependencies; @@ -507,8 +507,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &bit_mask::INEFFECTIVE_BIT_MASK, &bit_mask::VERBOSE_BIT_MASK, &blacklisted_name::BLACKLISTED_NAME, - &block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR, - &block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT, + &blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS, &booleans::LOGIC_BUG, &booleans::NONMINIMAL_BOOL, &bytecount::NAIVE_BYTECOUNT, @@ -578,7 +577,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &functions::TOO_MANY_LINES, &future_not_send::FUTURE_NOT_SEND, &get_last_with_len::GET_LAST_WITH_LEN, - &identity_conversion::IDENTITY_CONVERSION, &identity_op::IDENTITY_OP, &if_let_mutex::IF_LET_MUTEX, &if_let_some_result::IF_LET_SOME_RESULT, @@ -616,15 +614,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &loops::EXPLICIT_INTO_ITER_LOOP, &loops::EXPLICIT_ITER_LOOP, &loops::FOR_KV_MAP, - &loops::FOR_LOOP_OVER_OPTION, - &loops::FOR_LOOP_OVER_RESULT, + &loops::FOR_LOOPS_OVER_FALLIBLES, &loops::ITER_NEXT_LOOP, &loops::MANUAL_MEMCPY, &loops::MUT_RANGE_BOUND, &loops::NEEDLESS_COLLECT, &loops::NEEDLESS_RANGE_LOOP, &loops::NEVER_LOOP, - &loops::REVERSE_RANGE_LOOP, &loops::WHILE_IMMUTABLE_CONDITION, &loops::WHILE_LET_LOOP, &loops::WHILE_LET_ON_ITERATOR, @@ -653,12 +649,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &mem_replace::MEM_REPLACE_OPTION_WITH_NONE, &mem_replace::MEM_REPLACE_WITH_DEFAULT, &mem_replace::MEM_REPLACE_WITH_UNINIT, + &methods::BIND_INSTEAD_OF_MAP, &methods::CHARS_LAST_CMP, &methods::CHARS_NEXT_CMP, &methods::CLONE_DOUBLE_REF, &methods::CLONE_ON_COPY, &methods::CLONE_ON_REF_PTR, &methods::EXPECT_FUN_CALL, + &methods::EXPECT_USED, &methods::FILETYPE_IS_FILE, &methods::FILTER_MAP, &methods::FILTER_MAP_NEXT, @@ -675,20 +673,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &methods::ITER_SKIP_NEXT, &methods::MANUAL_SATURATING_ARITHMETIC, &methods::MAP_FLATTEN, + &methods::MAP_UNWRAP_OR, &methods::NEW_RET_NO_SELF, &methods::OK_EXPECT, - &methods::OPTION_AND_THEN_SOME, &methods::OPTION_AS_REF_DEREF, - &methods::OPTION_EXPECT_USED, &methods::OPTION_MAP_OR_NONE, - &methods::OPTION_MAP_UNWRAP_OR, - &methods::OPTION_MAP_UNWRAP_OR_ELSE, - &methods::OPTION_UNWRAP_USED, &methods::OR_FUN_CALL, - &methods::RESULT_EXPECT_USED, &methods::RESULT_MAP_OR_INTO_OPTION, - &methods::RESULT_MAP_UNWRAP_OR_ELSE, - &methods::RESULT_UNWRAP_USED, &methods::SEARCH_IS_SOME, &methods::SHOULD_IMPLEMENT_TRAIT, &methods::SINGLE_CHAR_PATTERN, @@ -699,6 +690,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &methods::UNINIT_ASSUMED_INIT, &methods::UNNECESSARY_FILTER_MAP, &methods::UNNECESSARY_FOLD, + &methods::UNWRAP_USED, &methods::USELESS_ASREF, &methods::WRONG_PUB_SELF_CONVENTION, &methods::WRONG_SELF_CONVENTION, @@ -770,6 +762,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &ranges::RANGE_MINUS_ONE, &ranges::RANGE_PLUS_ONE, &ranges::RANGE_ZIP_WITH_LEN, + &ranges::REVERSED_EMPTY_RANGES, &redundant_clone::REDUNDANT_CLONE, &redundant_field_names::REDUNDANT_FIELD_NAMES, &redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING, @@ -849,6 +842,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unwrap::PANICKING_UNWRAP, &unwrap::UNNECESSARY_UNWRAP, &use_self::USE_SELF, + &useless_conversion::USELESS_CONVERSION, &utils::internal_lints::CLIPPY_LINTS_INTERNAL, &utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS, &utils::internal_lints::COMPILER_LINT_FUNCTIONS, @@ -900,7 +894,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box mut_reference::UnnecessaryMutPassed); store.register_late_pass(|| box len_zero::LenZero); store.register_late_pass(|| box attrs::Attributes); - store.register_late_pass(|| box block_in_if_condition::BlockInIfCondition); + store.register_late_pass(|| box blocks_in_if_conditions::BlocksInIfConditions); store.register_late_pass(|| box unicode::Unicode); store.register_late_pass(|| box strings::StringAdd); store.register_late_pass(|| box implicit_return::ImplicitReturn); @@ -986,7 +980,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box bytecount::ByteCount); store.register_late_pass(|| box infinite_iter::InfiniteIter); store.register_late_pass(|| box inline_fn_without_body::InlineFnWithoutBody); - store.register_late_pass(|| box identity_conversion::IdentityConversion::default()); + store.register_late_pass(|| box useless_conversion::UselessConversion::default()); store.register_late_pass(|| box types::ImplicitHasher); store.register_late_pass(|| box fallible_impl_from::FallibleImplFrom); store.register_late_pass(|| box types::UnitArg); @@ -1090,12 +1084,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM), LintId::of(&mem_forget::MEM_FORGET), LintId::of(&methods::CLONE_ON_REF_PTR), + LintId::of(&methods::EXPECT_USED), LintId::of(&methods::FILETYPE_IS_FILE), LintId::of(&methods::GET_UNWRAP), - LintId::of(&methods::OPTION_EXPECT_USED), - LintId::of(&methods::OPTION_UNWRAP_USED), - LintId::of(&methods::RESULT_EXPECT_USED), - LintId::of(&methods::RESULT_UNWRAP_USED), + LintId::of(&methods::UNWRAP_USED), LintId::of(&methods::WRONG_PUB_SELF_CONVENTION), LintId::of(&misc::FLOAT_CMP_CONST), LintId::of(&misc_early::UNNEEDED_FIELD_PATTERN), @@ -1153,9 +1145,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::FIND_MAP), LintId::of(&methods::INEFFICIENT_TO_STRING), LintId::of(&methods::MAP_FLATTEN), - LintId::of(&methods::OPTION_MAP_UNWRAP_OR), - LintId::of(&methods::OPTION_MAP_UNWRAP_OR_ELSE), - LintId::of(&methods::RESULT_MAP_UNWRAP_OR_ELSE), + LintId::of(&methods::MAP_UNWRAP_OR), LintId::of(&misc::USED_UNDERSCORE_BINDING), LintId::of(&misc_early::UNSEPARATED_LITERAL_SUFFIX), LintId::of(&mut_mut::MUT_MUT), @@ -1209,8 +1199,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK), LintId::of(&bit_mask::VERBOSE_BIT_MASK), LintId::of(&blacklisted_name::BLACKLISTED_NAME), - LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR), - LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT), + LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(&booleans::LOGIC_BUG), LintId::of(&booleans::NONMINIMAL_BOOL), LintId::of(&bytecount::NAIVE_BYTECOUNT), @@ -1252,7 +1241,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&functions::NOT_UNSAFE_PTR_ARG_DEREF), LintId::of(&functions::TOO_MANY_ARGUMENTS), LintId::of(&get_last_with_len::GET_LAST_WITH_LEN), - LintId::of(&identity_conversion::IDENTITY_CONVERSION), LintId::of(&identity_op::IDENTITY_OP), LintId::of(&if_let_mutex::IF_LET_MUTEX), LintId::of(&if_let_some_result::IF_LET_SOME_RESULT), @@ -1266,7 +1254,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(&len_zero::LEN_ZERO), - LintId::of(&let_if_seq::USELESS_LET_IF_SEQ), LintId::of(&let_underscore::LET_UNDERSCORE_LOCK), LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES), LintId::of(&lifetimes::NEEDLESS_LIFETIMES), @@ -1275,15 +1262,13 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::EMPTY_LOOP), LintId::of(&loops::EXPLICIT_COUNTER_LOOP), LintId::of(&loops::FOR_KV_MAP), - LintId::of(&loops::FOR_LOOP_OVER_OPTION), - LintId::of(&loops::FOR_LOOP_OVER_RESULT), + LintId::of(&loops::FOR_LOOPS_OVER_FALLIBLES), LintId::of(&loops::ITER_NEXT_LOOP), LintId::of(&loops::MANUAL_MEMCPY), LintId::of(&loops::MUT_RANGE_BOUND), LintId::of(&loops::NEEDLESS_COLLECT), LintId::of(&loops::NEEDLESS_RANGE_LOOP), LintId::of(&loops::NEVER_LOOP), - LintId::of(&loops::REVERSE_RANGE_LOOP), LintId::of(&loops::WHILE_IMMUTABLE_CONDITION), LintId::of(&loops::WHILE_LET_LOOP), LintId::of(&loops::WHILE_LET_ON_ITERATOR), @@ -1305,6 +1290,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&mem_replace::MEM_REPLACE_OPTION_WITH_NONE), LintId::of(&mem_replace::MEM_REPLACE_WITH_DEFAULT), LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT), + LintId::of(&methods::BIND_INSTEAD_OF_MAP), LintId::of(&methods::CHARS_LAST_CMP), LintId::of(&methods::CHARS_NEXT_CMP), LintId::of(&methods::CLONE_DOUBLE_REF), @@ -1321,7 +1307,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC), LintId::of(&methods::NEW_RET_NO_SELF), LintId::of(&methods::OK_EXPECT), - LintId::of(&methods::OPTION_AND_THEN_SOME), LintId::of(&methods::OPTION_AS_REF_DEREF), LintId::of(&methods::OPTION_MAP_OR_NONE), LintId::of(&methods::OR_FUN_CALL), @@ -1384,6 +1369,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&question_mark::QUESTION_MARK), LintId::of(&ranges::RANGE_MINUS_ONE), LintId::of(&ranges::RANGE_ZIP_WITH_LEN), + LintId::of(&ranges::REVERSED_EMPTY_RANGES), LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES), LintId::of(&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING), @@ -1440,6 +1426,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unwrap::PANICKING_UNWRAP), LintId::of(&unwrap::UNNECESSARY_UNWRAP), + LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&vec::USELESS_VEC), LintId::of(&write::PRINTLN_EMPTY_STRING), LintId::of(&write::PRINT_LITERAL), @@ -1456,8 +1443,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS), LintId::of(&bit_mask::VERBOSE_BIT_MASK), LintId::of(&blacklisted_name::BLACKLISTED_NAME), - LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR), - LintId::of(&block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT), + LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS), LintId::of(&collapsible_if::COLLAPSIBLE_IF), LintId::of(&comparison_chain::COMPARISON_CHAIN), LintId::of(&doc::MISSING_SAFETY_DOC), @@ -1476,7 +1462,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&inherent_to_string::INHERENT_TO_STRING), LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY), LintId::of(&len_zero::LEN_ZERO), - LintId::of(&let_if_seq::USELESS_LET_IF_SEQ), LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING), LintId::of(&loops::EMPTY_LOOP), LintId::of(&loops::FOR_KV_MAP), @@ -1561,7 +1546,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&format::USELESS_FORMAT), LintId::of(&functions::TOO_MANY_ARGUMENTS), LintId::of(&get_last_with_len::GET_LAST_WITH_LEN), - LintId::of(&identity_conversion::IDENTITY_CONVERSION), LintId::of(&identity_op::IDENTITY_OP), LintId::of(&int_plus_one::INT_PLUS_ONE), LintId::of(&lifetimes::EXTRA_UNUSED_LIFETIMES), @@ -1574,10 +1558,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&matches::MATCH_AS_REF), LintId::of(&matches::MATCH_SINGLE_BINDING), LintId::of(&matches::WILDCARD_IN_OR_PATTERNS), + LintId::of(&methods::BIND_INSTEAD_OF_MAP), LintId::of(&methods::CLONE_ON_COPY), LintId::of(&methods::FILTER_NEXT), LintId::of(&methods::FLAT_MAP_IDENTITY), - LintId::of(&methods::OPTION_AND_THEN_SOME), LintId::of(&methods::OPTION_AS_REF_DEREF), LintId::of(&methods::SEARCH_IS_SOME), LintId::of(&methods::SKIP_WHILE_NEXT), @@ -1620,6 +1604,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::UNNECESSARY_CAST), LintId::of(&types::VEC_BOX), LintId::of(&unwrap::UNNECESSARY_UNWRAP), + LintId::of(&useless_conversion::USELESS_CONVERSION), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), ]); @@ -1652,11 +1637,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY), LintId::of(&let_underscore::LET_UNDERSCORE_LOCK), LintId::of(&literal_representation::MISTYPED_LITERAL_SUFFIXES), - LintId::of(&loops::FOR_LOOP_OVER_OPTION), - LintId::of(&loops::FOR_LOOP_OVER_RESULT), + LintId::of(&loops::FOR_LOOPS_OVER_FALLIBLES), LintId::of(&loops::ITER_NEXT_LOOP), LintId::of(&loops::NEVER_LOOP), - LintId::of(&loops::REVERSE_RANGE_LOOP), LintId::of(&loops::WHILE_IMMUTABLE_CONDITION), LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM), LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT), @@ -1675,6 +1658,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(&ptr::MUT_FROM_REF), + LintId::of(&ranges::REVERSED_EMPTY_RANGES), LintId::of(®ex::INVALID_REGEX), LintId::of(&serde_api::SERDE_API_MISUSE), LintId::of(&suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL), @@ -1728,6 +1712,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&floating_point_arithmetic::IMPRECISE_FLOPS), LintId::of(&floating_point_arithmetic::SUBOPTIMAL_FLOPS), LintId::of(&future_not_send::FUTURE_NOT_SEND), + LintId::of(&let_if_seq::USELESS_LET_IF_SEQ), LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(&mutex_atomic::MUTEX_INTEGER), @@ -1785,6 +1770,10 @@ fn register_removed_non_tool_lints(store: &mut rustc_lint::LintStore) { "unsafe_vector_initialization", "the replacement suggested by this lint had substantially different behavior", ); + store.register_removed( + "reverse_range_loop", + "this lint is now included in reversed_empty_ranges", + ); } /// Register renamed lints. @@ -1795,6 +1784,19 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) { ls.register_renamed("clippy::new_without_default_derive", "clippy::new_without_default"); ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity"); ls.register_renamed("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes"); + ls.register_renamed("clippy::option_and_then_some", "clippy::bind_instead_of_map"); + ls.register_renamed("clippy::block_in_if_condition_expr", "clippy::blocks_in_if_conditions"); + ls.register_renamed("clippy::block_in_if_condition_stmt", "clippy::blocks_in_if_conditions"); + ls.register_renamed("clippy::option_map_unwrap_or", "clippy::map_unwrap_or"); + ls.register_renamed("clippy::option_map_unwrap_or_else", "clippy::map_unwrap_or"); + ls.register_renamed("clippy::result_map_unwrap_or_else", "clippy::map_unwrap_or"); + ls.register_renamed("clippy::option_unwrap_used", "clippy::unwrap_used"); + ls.register_renamed("clippy::result_unwrap_used", "clippy::unwrap_used"); + ls.register_renamed("clippy::option_expect_used", "clippy::expect_used"); + ls.register_renamed("clippy::result_expect_used", "clippy::expect_used"); + ls.register_renamed("clippy::for_loop_over_option", "clippy::for_loops_over_fallibles"); + ls.register_renamed("clippy::for_loop_over_result", "clippy::for_loops_over_fallibles"); + ls.register_renamed("clippy::identity_conversion", "clippy::useless_conversion"); } // only exists to let the dogfood integration test works. diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 2bbf4dba614..84e8a010738 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -1,4 +1,4 @@ -use crate::consts::{constant, Constant}; +use crate::consts::constant; use crate::reexport::Name; use crate::utils::paths; use crate::utils::usage::{is_unused, mutated_variables}; @@ -8,7 +8,7 @@ use crate::utils::{ multispan_sugg, snippet, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, SpanlessEq, }; -use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg}; +use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sugg}; use if_chain::if_chain; use rustc_ast::ast; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; @@ -168,7 +168,7 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for `for` loops over `Option` values. + /// **What it does:** Checks for `for` loops over `Option` or `Result` values. /// /// **Why is this bad?** Readability. This is more clearly expressed as an `if /// let`. @@ -176,47 +176,38 @@ declare_clippy_lint! { /// **Known problems:** None. /// /// **Example:** - /// ```ignore - /// for x in option { - /// .. + /// ```rust + /// # let opt = Some(1); + /// + /// // Bad + /// for x in opt { + /// // .. + /// } + /// + /// // Good + /// if let Some(x) = opt { + /// // .. /// } /// ``` /// - /// This should be - /// ```ignore - /// if let Some(x) = option { - /// .. + /// // or + /// + /// ```rust + /// # let res: Result = Ok(1); + /// + /// // Bad + /// for x in &res { + /// // .. + /// } + /// + /// // Good + /// if let Ok(x) = res { + /// // .. /// } /// ``` - pub FOR_LOOP_OVER_OPTION, + pub FOR_LOOPS_OVER_FALLIBLES, correctness, - "for-looping over an `Option`, which is more clearly expressed as an `if let`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for `for` loops over `Result` values. - /// - /// **Why is this bad?** Readability. This is more clearly expressed as an `if - /// let`. - /// - /// **Known problems:** None. - /// - /// **Example:** - /// ```ignore - /// for x in result { - /// .. - /// } - /// ``` - /// - /// This should be - /// ```ignore - /// if let Ok(x) = result { - /// .. - /// } - /// ``` - pub FOR_LOOP_OVER_RESULT, - correctness, - "for-looping over a `Result`, which is more clearly expressed as an `if let`" + "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`" } declare_clippy_lint! { @@ -270,30 +261,6 @@ declare_clippy_lint! { "collecting an iterator when collect is not needed" } -declare_clippy_lint! { - /// **What it does:** Checks for loops over ranges `x..y` where both `x` and `y` - /// are constant and `x` is greater or equal to `y`, unless the range is - /// reversed or has a negative `.step_by(_)`. - /// - /// **Why is it bad?** Such loops will either be skipped or loop until - /// wrap-around (in debug code, this may `panic!()`). Both options are probably - /// not intended. - /// - /// **Known problems:** The lint cannot catch loops over dynamically defined - /// ranges. Doing this would require simulating all possible inputs and code - /// paths through the program, which would be complex and error-prone. - /// - /// **Example:** - /// ```ignore - /// for x in 5..10 - 5 { - /// .. - /// } // oops, stray `-` - /// ``` - pub REVERSE_RANGE_LOOP, - correctness, - "iteration over an empty range, such as `10..0` or `5..5`" -} - declare_clippy_lint! { /// **What it does:** Checks `for` loops over slices with an explicit counter /// and suggests the use of `.enumerate()`. @@ -459,11 +426,9 @@ declare_lint_pass!(Loops => [ EXPLICIT_ITER_LOOP, EXPLICIT_INTO_ITER_LOOP, ITER_NEXT_LOOP, - FOR_LOOP_OVER_RESULT, - FOR_LOOP_OVER_OPTION, + FOR_LOOPS_OVER_FALLIBLES, WHILE_LET_LOOP, NEEDLESS_COLLECT, - REVERSE_RANGE_LOOP, EXPLICIT_COUNTER_LOOP, EMPTY_LOOP, WHILE_LET_ON_ITERATOR, @@ -761,7 +726,6 @@ fn check_for_loop<'a, 'tcx>( expr: &'tcx Expr<'_>, ) { check_for_loop_range(cx, pat, arg, body, expr); - check_for_loop_reverse_range(cx, arg, expr); check_for_loop_arg(cx, pat, arg, expr); check_for_loop_explicit_counter(cx, pat, arg, body, expr); check_for_loop_over_map_kv(cx, pat, arg, body, expr); @@ -1170,7 +1134,7 @@ fn check_for_loop_range<'a, 'tcx>( |diag| { multispan_sugg( diag, - "consider using an iterator".to_string(), + "consider using an iterator", vec![ (pat.span, format!("({}, )", ident.name)), ( @@ -1199,7 +1163,7 @@ fn check_for_loop_range<'a, 'tcx>( |diag| { multispan_sugg( diag, - "consider using an iterator".to_string(), + "consider using an iterator", vec![(pat.span, "".to_string()), (arg.span, repl)], ); }, @@ -1248,78 +1212,6 @@ fn is_end_eq_array_len<'tcx>( false } -fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) { - // if this for loop is iterating over a two-sided range... - if let Some(higher::Range { - start: Some(start), - end: Some(end), - limits, - }) = higher::range(cx, arg) - { - // ...and both sides are compile-time constant integers... - if let Some((start_idx, _)) = constant(cx, cx.tables, start) { - if let Some((end_idx, _)) = constant(cx, cx.tables, end) { - // ...and the start index is greater than the end index, - // this loop will never run. This is often confusing for developers - // who think that this will iterate from the larger value to the - // smaller value. - let ty = cx.tables.expr_ty(start); - let (sup, eq) = match (start_idx, end_idx) { - (Constant::Int(start_idx), Constant::Int(end_idx)) => ( - match ty.kind { - ty::Int(ity) => sext(cx.tcx, start_idx, ity) > sext(cx.tcx, end_idx, ity), - ty::Uint(_) => start_idx > end_idx, - _ => false, - }, - start_idx == end_idx, - ), - _ => (false, false), - }; - - if sup { - let start_snippet = snippet(cx, start.span, "_"); - let end_snippet = snippet(cx, end.span, "_"); - let dots = if limits == ast::RangeLimits::Closed { - "..=" - } else { - ".." - }; - - span_lint_and_then( - cx, - REVERSE_RANGE_LOOP, - expr.span, - "this range is empty so this for loop will never run", - |diag| { - diag.span_suggestion( - arg.span, - "consider using the following if you are attempting to iterate over this \ - range in reverse", - format!( - "({end}{dots}{start}).rev()", - end = end_snippet, - dots = dots, - start = start_snippet - ), - Applicability::MaybeIncorrect, - ); - }, - ); - } else if eq && limits != ast::RangeLimits::Closed { - // if they are equal, it's also problematic - this loop - // will never run. - span_lint( - cx, - REVERSE_RANGE_LOOP, - expr.span, - "this range is empty so this for loop will never run", - ); - } - } - } - } -} - fn lint_iter_method(cx: &LateContext<'_, '_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) { let mut applicability = Applicability::MachineApplicable; let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability); @@ -1381,7 +1273,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>, e ITER_NEXT_LOOP, expr.span, "you are iterating over `Iterator::next()` which is an Option; this will compile but is \ - probably not what you want", + probably not what you want", ); next_loop_linted = true; } @@ -1398,11 +1290,11 @@ fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>) { if is_type_diagnostic_item(cx, ty, sym!(option_type)) { span_lint_and_help( cx, - FOR_LOOP_OVER_OPTION, + FOR_LOOPS_OVER_FALLIBLES, arg.span, &format!( "for loop over `{0}`, which is an `Option`. This is more readably written as an \ - `if let` statement.", + `if let` statement.", snippet(cx, arg.span, "_") ), None, @@ -1415,11 +1307,11 @@ fn check_arg_type(cx: &LateContext<'_, '_>, pat: &Pat<'_>, arg: &Expr<'_>) { } else if is_type_diagnostic_item(cx, ty, sym!(result_type)) { span_lint_and_help( cx, - FOR_LOOP_OVER_RESULT, + FOR_LOOPS_OVER_FALLIBLES, arg.span, &format!( "for loop over `{0}`, which is a `Result`. This is more readably written as an \ - `if let` statement.", + `if let` statement.", snippet(cx, arg.span, "_") ), None, @@ -1570,7 +1462,7 @@ fn check_for_loop_over_map_kv<'a, 'tcx>( let map = sugg::Sugg::hir(cx, arg, "map"); multispan_sugg( diag, - "use the corresponding method".into(), + "use the corresponding method", vec![ (pat_span, snippet(cx, new_pat_span, kind).into_owned()), (arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)), diff --git a/clippy_lints/src/map_clone.rs b/clippy_lints/src/map_clone.rs index 0163b3f8dbc..d5adf6b0f0d 100644 --- a/clippy_lints/src/map_clone.rs +++ b/clippy_lints/src/map_clone.rs @@ -9,8 +9,8 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::mir::Mutability; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; use rustc_span::symbol::Ident; +use rustc_span::Span; declare_clippy_lint! { /// **What it does:** Checks for usage of `iterator.map(|x| x.clone())` and suggests diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 8f86535ef1e..bbf14374a1f 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -820,7 +820,7 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_> span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |diag| { if !expr.span.from_expansion() { - multispan_sugg(diag, msg.to_owned(), suggs); + multispan_sugg(diag, msg, suggs); } }); } diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs new file mode 100644 index 00000000000..32e86637569 --- /dev/null +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -0,0 +1,309 @@ +use super::{contains_return, BIND_INSTEAD_OF_MAP}; +use crate::utils::{ + in_macro, match_qpath, match_type, method_calls, multispan_sugg_with_applicability, paths, remove_blocks, snippet, + snippet_with_macro_callsite, span_lint_and_sugg, span_lint_and_then, +}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::intravisit::{self, Visitor}; +use rustc_lint::LateContext; +use rustc_middle::hir::map::Map; +use rustc_span::Span; + +pub(crate) struct OptionAndThenSome; +impl BindInsteadOfMap for OptionAndThenSome { + const TYPE_NAME: &'static str = "Option"; + const TYPE_QPATH: &'static [&'static str] = &paths::OPTION; + + const BAD_METHOD_NAME: &'static str = "and_then"; + const BAD_VARIANT_NAME: &'static str = "Some"; + const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::OPTION_SOME; + + const GOOD_METHOD_NAME: &'static str = "map"; +} + +pub(crate) struct ResultAndThenOk; +impl BindInsteadOfMap for ResultAndThenOk { + const TYPE_NAME: &'static str = "Result"; + const TYPE_QPATH: &'static [&'static str] = &paths::RESULT; + + const BAD_METHOD_NAME: &'static str = "and_then"; + const BAD_VARIANT_NAME: &'static str = "Ok"; + const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::RESULT_OK; + + const GOOD_METHOD_NAME: &'static str = "map"; +} + +pub(crate) struct ResultOrElseErrInfo; +impl BindInsteadOfMap for ResultOrElseErrInfo { + const TYPE_NAME: &'static str = "Result"; + const TYPE_QPATH: &'static [&'static str] = &paths::RESULT; + + const BAD_METHOD_NAME: &'static str = "or_else"; + const BAD_VARIANT_NAME: &'static str = "Err"; + const BAD_VARIANT_QPATH: &'static [&'static str] = &paths::RESULT_ERR; + + const GOOD_METHOD_NAME: &'static str = "map_err"; +} + +pub(crate) trait BindInsteadOfMap { + const TYPE_NAME: &'static str; + const TYPE_QPATH: &'static [&'static str]; + + const BAD_METHOD_NAME: &'static str; + const BAD_VARIANT_NAME: &'static str; + const BAD_VARIANT_QPATH: &'static [&'static str]; + + const GOOD_METHOD_NAME: &'static str; + + fn no_op_msg() -> String { + format!( + "using `{}.{}({})`, which is a no-op", + Self::TYPE_NAME, + Self::BAD_METHOD_NAME, + Self::BAD_VARIANT_NAME + ) + } + + fn lint_msg() -> String { + format!( + "using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`", + Self::TYPE_NAME, + Self::BAD_METHOD_NAME, + Self::BAD_VARIANT_NAME, + Self::GOOD_METHOD_NAME + ) + } + + fn lint_closure_autofixable( + cx: &LateContext<'_, '_>, + expr: &hir::Expr<'_>, + args: &[hir::Expr<'_>], + closure_expr: &hir::Expr<'_>, + closure_args_span: Span, + ) -> bool { + if_chain! { + if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.kind; + if let hir::ExprKind::Path(ref qpath) = some_expr.kind; + if match_qpath(qpath, Self::BAD_VARIANT_QPATH); + if some_args.len() == 1; + then { + let inner_expr = &some_args[0]; + + if contains_return(inner_expr) { + return false; + } + + let some_inner_snip = if inner_expr.span.from_expansion() { + snippet_with_macro_callsite(cx, inner_expr.span, "_") + } else { + snippet(cx, inner_expr.span, "_") + }; + + let closure_args_snip = snippet(cx, closure_args_span, ".."); + let option_snip = snippet(cx, args[0].span, ".."); + let note = format!("{}.{}({} {})", option_snip, Self::GOOD_METHOD_NAME, closure_args_snip, some_inner_snip); + span_lint_and_sugg( + cx, + BIND_INSTEAD_OF_MAP, + expr.span, + Self::lint_msg().as_ref(), + "try this", + note, + Applicability::MachineApplicable, + ); + true + } else { + false + } + } + } + + fn lint_closure(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) { + let mut suggs = Vec::new(); + let can_sugg = find_all_ret_expressions(cx, closure_expr, |ret_expr| { + if_chain! { + if !in_macro(ret_expr.span); + if let hir::ExprKind::Call(ref func_path, ref args) = ret_expr.kind; + if let hir::ExprKind::Path(ref qpath) = func_path.kind; + if match_qpath(qpath, Self::BAD_VARIANT_QPATH); + if args.len() == 1; + if !contains_return(&args[0]); + then { + suggs.push((ret_expr.span, args[0].span.source_callsite())); + true + } else { + false + } + } + }); + + if can_sugg { + span_lint_and_then(cx, BIND_INSTEAD_OF_MAP, expr.span, Self::lint_msg().as_ref(), |diag| { + multispan_sugg_with_applicability( + diag, + "try this", + Applicability::MachineApplicable, + std::iter::once((*method_calls(expr, 1).2.get(0).unwrap(), Self::GOOD_METHOD_NAME.into())).chain( + suggs + .into_iter() + .map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())), + ), + ) + }); + } + } + + /// Lint use of `_.and_then(|x| Some(y))` for `Option`s + fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) { + if !match_type(cx, cx.tables.expr_ty(&args[0]), Self::TYPE_QPATH) { + return; + } + + match args[1].kind { + hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => { + let closure_body = cx.tcx.hir().body(body_id); + let closure_expr = remove_blocks(&closure_body.value); + + if !Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) { + Self::lint_closure(cx, expr, closure_expr); + } + }, + // `_.and_then(Some)` case, which is no-op. + hir::ExprKind::Path(ref qpath) if match_qpath(qpath, Self::BAD_VARIANT_QPATH) => { + span_lint_and_sugg( + cx, + BIND_INSTEAD_OF_MAP, + expr.span, + Self::no_op_msg().as_ref(), + "use the expression directly", + snippet(cx, args[0].span, "..").into(), + Applicability::MachineApplicable, + ); + }, + _ => {}, + } + } +} + +/// returns `true` if expr contains match expr desugared from try +fn contains_try(expr: &hir::Expr<'_>) -> bool { + struct TryFinder { + found: bool, + } + + impl<'hir> intravisit::Visitor<'hir> for TryFinder { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { + intravisit::NestedVisitorMap::None + } + + fn visit_expr(&mut self, expr: &'hir hir::Expr<'hir>) { + if self.found { + return; + } + match expr.kind { + hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar) => self.found = true, + _ => intravisit::walk_expr(self, expr), + } + } + } + + let mut visitor = TryFinder { found: false }; + visitor.visit_expr(expr); + visitor.found +} + +fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_, '_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool +where + F: FnMut(&'hir hir::Expr<'hir>) -> bool, +{ + struct RetFinder { + in_stmt: bool, + failed: bool, + cb: F, + } + + struct WithStmtGuarg<'a, F> { + val: &'a mut RetFinder, + prev_in_stmt: bool, + } + + impl RetFinder { + fn inside_stmt(&mut self, in_stmt: bool) -> WithStmtGuarg<'_, F> { + let prev_in_stmt = std::mem::replace(&mut self.in_stmt, in_stmt); + WithStmtGuarg { + val: self, + prev_in_stmt, + } + } + } + + impl std::ops::Deref for WithStmtGuarg<'_, F> { + type Target = RetFinder; + + fn deref(&self) -> &Self::Target { + self.val + } + } + + impl std::ops::DerefMut for WithStmtGuarg<'_, F> { + fn deref_mut(&mut self) -> &mut Self::Target { + self.val + } + } + + impl Drop for WithStmtGuarg<'_, F> { + fn drop(&mut self) { + self.val.in_stmt = self.prev_in_stmt; + } + } + + impl<'hir, F: FnMut(&'hir hir::Expr<'hir>) -> bool> intravisit::Visitor<'hir> for RetFinder { + type Map = Map<'hir>; + + fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap { + intravisit::NestedVisitorMap::None + } + + fn visit_stmt(&mut self, stmt: &'hir hir::Stmt<'_>) { + intravisit::walk_stmt(&mut *self.inside_stmt(true), stmt) + } + + fn visit_expr(&mut self, expr: &'hir hir::Expr<'_>) { + if self.failed { + return; + } + if self.in_stmt { + match expr.kind { + hir::ExprKind::Ret(Some(expr)) => self.inside_stmt(false).visit_expr(expr), + _ => intravisit::walk_expr(self, expr), + } + } else { + match expr.kind { + hir::ExprKind::Match(cond, arms, _) => { + self.inside_stmt(true).visit_expr(cond); + for arm in arms { + self.visit_expr(arm.body); + } + }, + hir::ExprKind::Block(..) => intravisit::walk_expr(self, expr), + hir::ExprKind::Ret(Some(expr)) => self.visit_expr(expr), + _ => self.failed |= !(self.cb)(expr), + } + } + } + } + + !contains_try(expr) && { + let mut ret_finder = RetFinder { + in_stmt: false, + failed: false, + cb: callback, + }; + ret_finder.visit_expr(expr); + !ret_finder.failed + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3676dc5b09d..626427c15ec 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,3 +1,4 @@ +mod bind_instead_of_map; mod inefficient_to_string; mod manual_saturating_arithmetic; mod option_map_unwrap_or; @@ -7,6 +8,7 @@ use std::borrow::Cow; use std::fmt; use std::iter; +use bind_instead_of_map::BindInsteadOfMap; use if_chain::if_chain; use rustc_ast::ast; use rustc_errors::Applicability; @@ -33,40 +35,15 @@ use crate::utils::{ }; declare_clippy_lint! { - /// **What it does:** Checks for `.unwrap()` calls on `Option`s. + /// **What it does:** Checks for `.unwrap()` calls on `Option`s and on `Result`s. /// - /// **Why is this bad?** Usually it is better to handle the `None` case, or to - /// at least call `.expect(_)` with a more helpful message. Still, for a lot of + /// **Why is this bad?** It is better to handle the `None` or `Err` case, + /// or at least call `.expect(_)` with a more helpful message. Still, for a lot of /// quick-and-dirty code, `unwrap` is a good choice, which is why this lint is /// `Allow` by default. /// - /// **Known problems:** None. - /// - /// **Example:** - /// - /// Using unwrap on an `Option`: - /// - /// ```rust - /// let opt = Some(1); - /// opt.unwrap(); - /// ``` - /// - /// Better: - /// - /// ```rust - /// let opt = Some(1); - /// opt.expect("more helpful message"); - /// ``` - pub OPTION_UNWRAP_USED, - restriction, - "using `Option.unwrap()`, which should at least get a better message using `expect()`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for `.unwrap()` calls on `Result`s. - /// - /// **Why is this bad?** `result.unwrap()` will let the thread panic on `Err` - /// values. Normally, you want to implement more sophisticated error handling, + /// `result.unwrap()` will let the thread panic on `Err` values. + /// Normally, you want to implement more sophisticated error handling, /// and propagate errors upwards with `?` operator. /// /// Even if you want to panic on errors, not all `Error`s implement good @@ -75,81 +52,73 @@ declare_clippy_lint! { /// /// **Known problems:** None. /// - /// **Example:** - /// Using unwrap on an `Result`: - /// + /// **Examples:** /// ```rust - /// let res: Result = Ok(1); - /// res.unwrap(); + /// # let opt = Some(1); + /// + /// // Bad + /// opt.unwrap(); + /// + /// // Good + /// opt.expect("more helpful message"); /// ``` /// - /// Better: + /// // or /// /// ```rust - /// let res: Result = Ok(1); + /// # let res: Result = Ok(1); + /// + /// // Bad + /// res.unwrap(); + /// + /// // Good /// res.expect("more helpful message"); /// ``` - pub RESULT_UNWRAP_USED, + pub UNWRAP_USED, restriction, - "using `Result.unwrap()`, which might be better handled" + "using `.unwrap()` on `Result` or `Option`, which should at least get a better message using `expect()`" } declare_clippy_lint! { - /// **What it does:** Checks for `.expect()` calls on `Option`s. + /// **What it does:** Checks for `.expect()` calls on `Option`s and `Result`s. /// - /// **Why is this bad?** Usually it is better to handle the `None` case. Still, - /// for a lot of quick-and-dirty code, `expect` is a good choice, which is why - /// this lint is `Allow` by default. + /// **Why is this bad?** Usually it is better to handle the `None` or `Err` case. + /// Still, for a lot of quick-and-dirty code, `expect` is a good choice, which is why + /// this lint is `Allow` by default. /// - /// **Known problems:** None. - /// - /// **Example:** - /// - /// Using expect on an `Option`: - /// - /// ```rust - /// let opt = Some(1); - /// opt.expect("one"); - /// ``` - /// - /// Better: - /// - /// ```rust,ignore - /// let opt = Some(1); - /// opt?; - /// ``` - pub OPTION_EXPECT_USED, - restriction, - "using `Option.expect()`, which might be better handled" -} - -declare_clippy_lint! { - /// **What it does:** Checks for `.expect()` calls on `Result`s. - /// - /// **Why is this bad?** `result.expect()` will let the thread panic on `Err` + /// `result.expect()` will let the thread panic on `Err` /// values. Normally, you want to implement more sophisticated error handling, /// and propagate errors upwards with `?` operator. /// /// **Known problems:** None. /// - /// **Example:** - /// Using expect on an `Result`: + /// **Examples:** + /// ```rust,ignore + /// # let opt = Some(1); /// - /// ```rust - /// let res: Result = Ok(1); - /// res.expect("one"); + /// // Bad + /// opt.expect("one"); + /// + /// // Good + /// let opt = Some(1); + /// opt?; /// ``` /// - /// Better: + /// // or /// /// ```rust - /// let res: Result = Ok(1); + /// # let res: Result = Ok(1); + /// + /// // Bad + /// res.expect("one"); + /// + /// // Good /// res?; /// # Ok::<(), ()>(()) /// ``` - pub RESULT_EXPECT_USED, + pub EXPECT_USED, restriction, - "using `Result.expect()`, which might be better handled" + "using `.expect()` on `Result` or `Option`, which might be better handled" } declare_clippy_lint! { @@ -257,59 +226,40 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for usage of `_.map(_).unwrap_or(_)`. + /// **What it does:** Checks for usage of `option.map(_).unwrap_or(_)` or `option.map(_).unwrap_or_else(_)` or + /// `result.map(_).unwrap_or_else(_)`. /// - /// **Why is this bad?** Readability, this can be written more concisely as - /// `_.map_or(_, _)`. + /// **Why is this bad?** Readability, these can be written more concisely (resp.) as + /// `option.map_or(_, _)`, `option.map_or_else(_, _)` and `result.map_or_else(_, _)`. /// /// **Known problems:** The order of the arguments is not in execution order /// - /// **Example:** + /// **Examples:** /// ```rust /// # let x = Some(1); + /// + /// // Bad /// x.map(|a| a + 1).unwrap_or(0); + /// + /// // Good + /// x.map_or(0, |a| a + 1); /// ``` - pub OPTION_MAP_UNWRAP_OR, - pedantic, - "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for usage of `_.map(_).unwrap_or_else(_)`. /// - /// **Why is this bad?** Readability, this can be written more concisely as - /// `_.map_or_else(_, _)`. + /// // or /// - /// **Known problems:** The order of the arguments is not in execution order. - /// - /// **Example:** - /// ```rust - /// # let x = Some(1); - /// # fn some_function() -> usize { 1 } - /// x.map(|a| a + 1).unwrap_or_else(some_function); - /// ``` - pub OPTION_MAP_UNWRAP_OR_ELSE, - pedantic, - "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`" -} - -declare_clippy_lint! { - /// **What it does:** Checks for usage of `result.map(_).unwrap_or_else(_)`. - /// - /// **Why is this bad?** Readability, this can be written more concisely as - /// `result.map_or_else(_, _)`. - /// - /// **Known problems:** None. - /// - /// **Example:** /// ```rust /// # let x: Result = Ok(1); /// # fn some_function(foo: ()) -> usize { 1 } + /// + /// // Bad /// x.map(|a| a + 1).unwrap_or_else(some_function); + /// + /// // Good + /// x.map_or_else(some_function, |a| a + 1); /// ``` - pub RESULT_MAP_UNWRAP_OR_ELSE, + pub MAP_UNWRAP_OR, pedantic, - "using `Result.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `.map_or_else(g, f)`" + "using `.map(f).unwrap_or(a)` or `.map(f).unwrap_or_else(func)`, which are more succinctly expressed as `map_or(a, f)` or `map_or_else(a, f)`" } declare_clippy_lint! { @@ -358,27 +308,34 @@ declare_clippy_lint! { } declare_clippy_lint! { - /// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`. + /// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`, `_.and_then(|x| Ok(y))` or + /// `_.or_else(|x| Err(y))`. /// /// **Why is this bad?** Readability, this can be written more concisely as - /// `_.map(|x| y)`. + /// `_.map(|x| y)` or `_.map_err(|x| y)`. /// /// **Known problems:** None /// /// **Example:** /// /// ```rust - /// let x = Some("foo"); - /// let _ = x.and_then(|s| Some(s.len())); + /// # fn opt() -> Option<&'static str> { Some("42") } + /// # fn res() -> Result<&'static str, &'static str> { Ok("42") } + /// let _ = opt().and_then(|s| Some(s.len())); + /// let _ = res().and_then(|s| if s.len() == 42 { Ok(10) } else { Ok(20) }); + /// let _ = res().or_else(|s| if s.len() == 42 { Err(10) } else { Err(20) }); /// ``` /// /// The correct use would be: /// /// ```rust - /// let x = Some("foo"); - /// let _ = x.map(|s| s.len()); + /// # fn opt() -> Option<&'static str> { Some("42") } + /// # fn res() -> Result<&'static str, &'static str> { Ok("42") } + /// let _ = opt().map(|s| s.len()); + /// let _ = res().map(|s| if s.len() == 42 { 10 } else { 20 }); + /// let _ = res().map_err(|s| if s.len() == 42 { 10 } else { 20 }); /// ``` - pub OPTION_AND_THEN_SOME, + pub BIND_INSTEAD_OF_MAP, complexity, "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`" } @@ -1286,20 +1243,16 @@ declare_clippy_lint! { } declare_lint_pass!(Methods => [ - OPTION_UNWRAP_USED, - RESULT_UNWRAP_USED, - OPTION_EXPECT_USED, - RESULT_EXPECT_USED, + UNWRAP_USED, + EXPECT_USED, SHOULD_IMPLEMENT_TRAIT, WRONG_SELF_CONVENTION, WRONG_PUB_SELF_CONVENTION, OK_EXPECT, - OPTION_MAP_UNWRAP_OR, - OPTION_MAP_UNWRAP_OR_ELSE, - RESULT_MAP_UNWRAP_OR_ELSE, + MAP_UNWRAP_OR, RESULT_MAP_OR_INTO_OPTION, OPTION_MAP_OR_NONE, - OPTION_AND_THEN_SOME, + BIND_INSTEAD_OF_MAP, OR_FUN_CALL, EXPECT_FUN_CALL, CHARS_NEXT_CMP, @@ -1358,7 +1311,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { ["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0], method_spans[1]), ["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]), ["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]), - ["and_then", ..] => lint_option_and_then_some(cx, expr, arg_lists[0]), + ["and_then", ..] => { + bind_instead_of_map::OptionAndThenSome::lint(cx, expr, arg_lists[0]); + bind_instead_of_map::ResultAndThenOk::lint(cx, expr, arg_lists[0]); + }, + ["or_else", ..] => { + bind_instead_of_map::ResultOrElseErrInfo::lint(cx, expr, arg_lists[0]); + }, ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]), ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]), ["map", "filter"] => lint_filter_map(cx, expr, arg_lists[1], arg_lists[0]), @@ -1503,9 +1462,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods { cx, lint, first_arg.pat.span, - &format!( - "methods called `{}` usually take {}; consider choosing a less \ - ambiguous name", + &format!("methods called `{}` usually take {}; consider choosing a less ambiguous name", conv, &self_kinds .iter() @@ -1678,7 +1635,7 @@ fn lint_or_fun_call<'a, 'tcx>( let self_ty = cx.tables.expr_ty(self_expr); if let Some(&(_, fn_has_arguments, poss, suffix)) = - know_types.iter().find(|&&i| match_type(cx, self_ty, i.0)); + know_types.iter().find(|&&i| match_type(cx, self_ty, i.0)); if poss.contains(&name); @@ -1931,7 +1888,7 @@ fn lint_clone_on_copy(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, arg: &hir: CLONE_DOUBLE_REF, expr.span, "using `clone` on a double-reference; \ - this will copy the reference instead of cloning the inner type", + this will copy the reference instead of cloning the inner type", |diag| { if let Some(snip) = sugg::Sugg::hir_opt(cx, arg) { let mut ty = innermost; @@ -2121,7 +2078,7 @@ fn lint_iter_cloned_collect<'a, 'tcx>( ITER_CLONED_COLLECT, to_replace, "called `iter().cloned().collect()` on a slice to create a `Vec`. Calling `to_vec()` is both faster and \ - more readable", + more readable", "try", ".to_vec()".to_string(), Applicability::MachineApplicable, @@ -2420,9 +2377,9 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, unwrap_args: &[hi let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(&unwrap_args[0])); let mess = if is_type_diagnostic_item(cx, obj_ty, sym!(option_type)) { - Some((OPTION_UNWRAP_USED, "an Option", "None")) + Some((UNWRAP_USED, "an Option", "None")) } else if is_type_diagnostic_item(cx, obj_ty, sym!(result_type)) { - Some((RESULT_UNWRAP_USED, "a Result", "Err")) + Some((UNWRAP_USED, "a Result", "Err")) } else { None }; @@ -2436,7 +2393,7 @@ fn lint_unwrap(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, unwrap_args: &[hi None, &format!( "if you don't want to handle the `{}` case gracefully, consider \ - using `expect()` to provide a better panic message", + using `expect()` to provide a better panic message", none_value, ), ); @@ -2448,9 +2405,9 @@ fn lint_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, expect_args: &[hi let obj_ty = walk_ptrs_ty(cx.tables.expr_ty(&expect_args[0])); let mess = if is_type_diagnostic_item(cx, obj_ty, sym!(option_type)) { - Some((OPTION_EXPECT_USED, "an Option", "None")) + Some((EXPECT_USED, "an Option", "None")) } else if is_type_diagnostic_item(cx, obj_ty, sym!(result_type)) { - Some((RESULT_EXPECT_USED, "a Result", "Err")) + Some((EXPECT_USED, "a Result", "Err")) } else { None }; @@ -2494,7 +2451,7 @@ fn lint_map_flatten<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr< // lint if caller of `.map().flatten()` is an Iterator if match_trait_method(cx, expr, &paths::ITERATOR) { let msg = "called `map(..).flatten()` on an `Iterator`. \ - This is more succinctly expressed by calling `.flat_map(..)`"; + This is more succinctly expressed by calling `.flat_map(..)`"; let self_snippet = snippet(cx, map_args[0].span, ".."); let func_snippet = snippet(cx, map_args[1].span, ".."); let hint = format!("{0}.flat_map({1})", self_snippet, func_snippet); @@ -2555,10 +2512,10 @@ fn lint_map_unwrap_or_else<'a, 'tcx>( // lint message let msg = if is_option { "called `map(f).unwrap_or_else(g)` on an `Option` value. This can be done more directly by calling \ - `map_or_else(g, f)` instead" + `map_or_else(g, f)` instead" } else { "called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling \ - `.map_or_else(g, f)` instead" + `.map_or_else(g, f)` instead" }; // get snippets for args to map() and unwrap_or_else() let map_snippet = snippet(cx, map_args[1].span, ".."); @@ -2570,11 +2527,7 @@ fn lint_map_unwrap_or_else<'a, 'tcx>( if same_span && !multiline { span_lint_and_note( cx, - if is_option { - OPTION_MAP_UNWRAP_OR_ELSE - } else { - RESULT_MAP_UNWRAP_OR_ELSE - }, + MAP_UNWRAP_OR, expr.span, msg, None, @@ -2584,16 +2537,7 @@ fn lint_map_unwrap_or_else<'a, 'tcx>( ), ); } else if same_span && multiline { - span_lint( - cx, - if is_option { - OPTION_MAP_UNWRAP_OR_ELSE - } else { - RESULT_MAP_UNWRAP_OR_ELSE - }, - expr.span, - msg, - ); + span_lint(cx, MAP_UNWRAP_OR, expr.span, msg); }; } } @@ -2672,73 +2616,6 @@ fn lint_map_or_none<'a, 'tcx>( ); } -/// Lint use of `_.and_then(|x| Some(y))` for `Option`s -fn lint_option_and_then_some(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) { - const LINT_MSG: &str = "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`"; - const NO_OP_MSG: &str = "using `Option.and_then(Some)`, which is a no-op"; - - let ty = cx.tables.expr_ty(&args[0]); - if !is_type_diagnostic_item(cx, ty, sym!(option_type)) { - return; - } - - match args[1].kind { - hir::ExprKind::Closure(_, _, body_id, closure_args_span, _) => { - let closure_body = cx.tcx.hir().body(body_id); - let closure_expr = remove_blocks(&closure_body.value); - if_chain! { - if let hir::ExprKind::Call(ref some_expr, ref some_args) = closure_expr.kind; - if let hir::ExprKind::Path(ref qpath) = some_expr.kind; - if match_qpath(qpath, &paths::OPTION_SOME); - if some_args.len() == 1; - then { - let inner_expr = &some_args[0]; - - if contains_return(inner_expr) { - return; - } - - let some_inner_snip = if inner_expr.span.from_expansion() { - snippet_with_macro_callsite(cx, inner_expr.span, "_") - } else { - snippet(cx, inner_expr.span, "_") - }; - - let closure_args_snip = snippet(cx, closure_args_span, ".."); - let option_snip = snippet(cx, args[0].span, ".."); - let note = format!("{}.map({} {})", option_snip, closure_args_snip, some_inner_snip); - span_lint_and_sugg( - cx, - OPTION_AND_THEN_SOME, - expr.span, - LINT_MSG, - "try this", - note, - Applicability::MachineApplicable, - ); - } - } - }, - // `_.and_then(Some)` case, which is no-op. - hir::ExprKind::Path(ref qpath) => { - if match_qpath(qpath, &paths::OPTION_SOME) { - let option_snip = snippet(cx, args[0].span, ".."); - let note = format!("{}", option_snip); - span_lint_and_sugg( - cx, - OPTION_AND_THEN_SOME, - expr.span, - NO_OP_MSG, - "use the expression directly", - note, - Applicability::MachineApplicable, - ); - } - }, - _ => {}, - } -} - /// lint use of `filter().next()` for `Iterators` fn lint_filter_next<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, diff --git a/clippy_lints/src/methods/option_map_unwrap_or.rs b/clippy_lints/src/methods/option_map_unwrap_or.rs index bf9dd3c9369..20c60ef3318 100644 --- a/clippy_lints/src/methods/option_map_unwrap_or.rs +++ b/clippy_lints/src/methods/option_map_unwrap_or.rs @@ -9,7 +9,7 @@ use rustc_middle::hir::map::Map; use rustc_span::source_map::Span; use rustc_span::symbol::Symbol; -use super::OPTION_MAP_UNWRAP_OR; +use super::MAP_UNWRAP_OR; /// lint use of `map().unwrap_or()` for `Option`s pub(super) fn lint<'a, 'tcx>( @@ -62,11 +62,11 @@ pub(super) fn lint<'a, 'tcx>( }; let msg = &format!( "called `map(f).unwrap_or({})` on an `Option` value. \ - This can be done more directly by calling `{}` instead", + This can be done more directly by calling `{}` instead", arg, suggest ); - span_lint_and_then(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, |diag| { + span_lint_and_then(cx, MAP_UNWRAP_OR, expr.span, msg, |diag| { let map_arg_span = map_args[1].span; let mut suggestion = vec![ diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index a21818701da..ed48ab54897 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -293,7 +293,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue { ); spans.sort_by_key(|&(span, _)| span); } - multispan_sugg(diag, "consider taking a reference instead".to_string(), spans); + multispan_sugg(diag, "consider taking a reference instead", spans); }; span_lint_and_then( diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index d7ce2e66d69..83c6faac041 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -1,14 +1,17 @@ +use crate::consts::{constant, Constant}; use if_chain::if_chain; use rustc_ast::ast::RangeLimits; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Spanned; +use std::cmp::Ordering; use crate::utils::sugg::Sugg; +use crate::utils::{get_parent_expr, is_integer_const, snippet, snippet_opt, span_lint, span_lint_and_then}; use crate::utils::{higher, SpanlessEq}; -use crate::utils::{is_integer_const, snippet, snippet_opt, span_lint, span_lint_and_then}; declare_clippy_lint! { /// **What it does:** Checks for zipping a collection with the range of @@ -84,10 +87,44 @@ declare_clippy_lint! { "`x..=(y-1)` reads better as `x..y`" } +declare_clippy_lint! { + /// **What it does:** Checks for range expressions `x..y` where both `x` and `y` + /// are constant and `x` is greater or equal to `y`. + /// + /// **Why is this bad?** Empty ranges yield no values so iterating them is a no-op. + /// Moreover, trying to use a reversed range to index a slice will panic at run-time. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,no_run + /// fn main() { + /// (10..=0).for_each(|x| println!("{}", x)); + /// + /// let arr = [1, 2, 3, 4, 5]; + /// let sub = &arr[3..1]; + /// } + /// ``` + /// Use instead: + /// ```rust + /// fn main() { + /// (0..=10).rev().for_each(|x| println!("{}", x)); + /// + /// let arr = [1, 2, 3, 4, 5]; + /// let sub = &arr[1..3]; + /// } + /// ``` + pub REVERSED_EMPTY_RANGES, + correctness, + "reversing the limits of range expressions, resulting in empty ranges" +} + declare_lint_pass!(Ranges => [ RANGE_ZIP_WITH_LEN, RANGE_PLUS_ONE, - RANGE_MINUS_ONE + RANGE_MINUS_ONE, + REVERSED_EMPTY_RANGES, ]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges { @@ -124,6 +161,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Ranges { check_exclusive_range_plus_one(cx, expr); check_inclusive_range_minus_one(cx, expr); + check_reversed_empty_range(cx, expr); } } @@ -202,6 +240,76 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { } } +fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { + fn inside_indexing_expr(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { + matches!( + get_parent_expr(cx, expr), + Some(Expr { + kind: ExprKind::Index(..), + .. + }) + ) + } + + fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool { + match limits { + RangeLimits::HalfOpen => ordering != Ordering::Less, + RangeLimits::Closed => ordering == Ordering::Greater, + } + } + + if_chain! { + if let Some(higher::Range { start: Some(start), end: Some(end), limits }) = higher::range(cx, expr); + let ty = cx.tables.expr_ty(start); + if let ty::Int(_) | ty::Uint(_) = ty.kind; + if let Some((start_idx, _)) = constant(cx, cx.tables, start); + if let Some((end_idx, _)) = constant(cx, cx.tables, end); + if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx); + if is_empty_range(limits, ordering); + then { + if inside_indexing_expr(cx, expr) { + let (reason, outcome) = if ordering == Ordering::Equal { + ("empty", "always yield an empty slice") + } else { + ("reversed", "panic at run-time") + }; + + span_lint( + cx, + REVERSED_EMPTY_RANGES, + expr.span, + &format!("this range is {} and using it to index a slice will {}", reason, outcome), + ); + } else { + span_lint_and_then( + cx, + REVERSED_EMPTY_RANGES, + expr.span, + "this range is empty so it will yield no values", + |diag| { + if ordering != Ordering::Equal { + let start_snippet = snippet(cx, start.span, "_"); + let end_snippet = snippet(cx, end.span, "_"); + let dots = match limits { + RangeLimits::HalfOpen => "..", + RangeLimits::Closed => "..=" + }; + + diag.span_suggestion( + expr.span, + "consider using the following if you are attempting to iterate over this \ + range in reverse", + format!("({}{}{}).rev()", end_snippet, dots, start_snippet), + Applicability::MaybeIncorrect, + ); + } + }, + ); + } + } + } +} + fn y_plus_one<'t>(cx: &LateContext<'_, '_>, expr: &'t Expr<'_>) -> Option<&'t Expr<'t>> { match expr.kind { ExprKind::Binary( diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 5c9117d5b81..35464f629c3 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -248,28 +248,7 @@ impl EarlyLintPass for Return { if let ast::TyKind::Tup(ref vals) = ty.kind; if vals.is_empty() && !ty.span.from_expansion() && get_def(span) == get_def(ty.span); then { - let (rspan, appl) = if let Ok(fn_source) = - cx.sess().source_map() - .span_to_snippet(span.with_hi(ty.span.hi())) { - if let Some(rpos) = fn_source.rfind("->") { - #[allow(clippy::cast_possible_truncation)] - (ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)), - Applicability::MachineApplicable) - } else { - (ty.span, Applicability::MaybeIncorrect) - } - } else { - (ty.span, Applicability::MaybeIncorrect) - }; - span_lint_and_sugg( - cx, - UNUSED_UNIT, - rspan, - "unneeded unit return type", - "remove the `-> ()`", - String::new(), - appl, - ); + lint_unneeded_unit_return(cx, ty, span); } } } @@ -313,6 +292,22 @@ impl EarlyLintPass for Return { _ => (), } } + + fn check_poly_trait_ref(&mut self, cx: &EarlyContext<'_>, poly: &ast::PolyTraitRef, _: &ast::TraitBoundModifier) { + let segments = &poly.trait_ref.path.segments; + + if_chain! { + if segments.len() == 1; + if ["Fn", "FnMut", "FnOnce"].contains(&&*segments[0].ident.name.as_str()); + if let Some(args) = &segments[0].args; + if let ast::GenericArgs::Parenthesized(generic_args) = &**args; + if let ast::FnRetTy::Ty(ty) = &generic_args.output; + if ty.kind.is_unit(); + then { + lint_unneeded_unit_return(cx, ty, generic_args.span); + } + } + } } fn attr_is_cfg(attr: &ast::Attribute) -> bool { @@ -337,3 +332,28 @@ fn is_unit_expr(expr: &ast::Expr) -> bool { false } } + +fn lint_unneeded_unit_return(cx: &EarlyContext<'_>, ty: &ast::Ty, span: Span) { + let (ret_span, appl) = if let Ok(fn_source) = cx.sess().source_map().span_to_snippet(span.with_hi(ty.span.hi())) { + if let Some(rpos) = fn_source.rfind("->") { + #[allow(clippy::cast_possible_truncation)] + ( + ty.span.with_lo(BytePos(span.lo().0 + rpos as u32)), + Applicability::MachineApplicable, + ) + } else { + (ty.span, Applicability::MaybeIncorrect) + } + } else { + (ty.span, Applicability::MaybeIncorrect) + }; + span_lint_and_sugg( + cx, + UNUSED_UNIT, + ret_span, + "unneeded unit return type", + "remove the `-> ()`", + String::new(), + appl, + ); +} diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 6d49f50d550..6ed9ff22e46 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -2206,7 +2206,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher { multispan_sugg( diag, - "consider adding a type parameter".to_string(), + "consider adding a type parameter", vec![ ( generics_suggestion_span, @@ -2230,7 +2230,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher { ); if !vis.suggestions.is_empty() { - multispan_sugg(diag, "...and use generic constructor".into(), vis.suggestions); + multispan_sugg(diag, "...and use generic constructor", vis.suggestions); } } diff --git a/clippy_lints/src/unwrap.rs b/clippy_lints/src/unwrap.rs index f3844c7d3b6..8b971e7064b 100644 --- a/clippy_lints/src/unwrap.rs +++ b/clippy_lints/src/unwrap.rs @@ -8,6 +8,7 @@ use rustc_hir::{BinOpKind, Body, Expr, ExprKind, FnDecl, HirId, Path, QPath, UnO use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; +use rustc_middle::ty::Ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Span; @@ -90,6 +91,14 @@ fn collect_unwrap_info<'a, 'tcx>( branch: &'tcx Expr<'_>, invert: bool, ) -> Vec> { + fn is_relevant_option_call(cx: &LateContext<'_, '_>, ty: Ty<'_>, method_name: &str) -> bool { + is_type_diagnostic_item(cx, ty, sym!(option_type)) && ["is_some", "is_none"].contains(&method_name) + } + + fn is_relevant_result_call(cx: &LateContext<'_, '_>, ty: Ty<'_>, method_name: &str) -> bool { + is_type_diagnostic_item(cx, ty, sym!(result_type)) && ["is_ok", "is_err"].contains(&method_name) + } + if let ExprKind::Binary(op, left, right) = &expr.kind { match (invert, op.node) { (false, BinOpKind::And) | (false, BinOpKind::BitAnd) | (true, BinOpKind::Or) | (true, BinOpKind::BitOr) => { @@ -106,9 +115,8 @@ fn collect_unwrap_info<'a, 'tcx>( if let ExprKind::MethodCall(method_name, _, args) = &expr.kind; if let ExprKind::Path(QPath::Resolved(None, path)) = &args[0].kind; let ty = cx.tables.expr_ty(&args[0]); - if is_type_diagnostic_item(cx, ty, sym!(option_type)) || is_type_diagnostic_item(cx, ty, sym!(result_type)); let name = method_name.ident.as_str(); - if ["is_some", "is_none", "is_ok", "is_err"].contains(&&*name); + if is_relevant_option_call(cx, ty, &name) || is_relevant_result_call(cx, ty, &name); then { assert!(args.len() == 1); let unwrappable = match name.as_ref() { diff --git a/clippy_lints/src/identity_conversion.rs b/clippy_lints/src/useless_conversion.rs similarity index 84% rename from clippy_lints/src/identity_conversion.rs rename to clippy_lints/src/useless_conversion.rs index 33a9478f058..95921518986 100644 --- a/clippy_lints/src/identity_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -7,30 +7,36 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; declare_clippy_lint! { - /// **What it does:** Checks for always-identical `Into`/`From`/`IntoIter` conversions. + /// **What it does:** Checks for `Into`/`From`/`IntoIter` calls that useless converts + /// to the same type as caller. /// /// **Why is this bad?** Redundant code. /// /// **Known problems:** None. /// /// **Example:** + /// /// ```rust + /// // Bad /// // format!() returns a `String` /// let s: String = format!("hello").into(); + /// + /// // Good + /// let s: String = format!("hello"); /// ``` - pub IDENTITY_CONVERSION, + pub USELESS_CONVERSION, complexity, - "using always-identical `Into`/`From`/`IntoIter` conversions" + "calls to `Into`/`From`/`IntoIter` that performs useless conversions to the same type" } #[derive(Default)] -pub struct IdentityConversion { +pub struct UselessConversion { try_desugar_arm: Vec, } -impl_lint_pass!(IdentityConversion => [IDENTITY_CONVERSION]); +impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessConversion { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) { if e.span.from_expansion() { return; @@ -60,9 +66,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { span_lint_and_sugg( cx, - IDENTITY_CONVERSION, + USELESS_CONVERSION, e.span, - "identical conversion", + "useless conversion", "consider removing `.into()`", sugg, Applicability::MachineApplicable, // snippet @@ -76,9 +82,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { let sugg = snippet(cx, args[0].span, "").into_owned(); span_lint_and_sugg( cx, - IDENTITY_CONVERSION, + USELESS_CONVERSION, e.span, - "identical conversion", + "useless conversion", "consider removing `.into_iter()`", sugg, Applicability::MachineApplicable, // snippet @@ -99,9 +105,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion { format!("consider removing `{}()`", snippet(cx, path.span, "From::from")); span_lint_and_sugg( cx, - IDENTITY_CONVERSION, + USELESS_CONVERSION, e.span, - "identical conversion", + "useless conversion", &sugg_msg, sugg, Applicability::MachineApplicable, // snippet diff --git a/clippy_lints/src/utils/diagnostics.rs b/clippy_lints/src/utils/diagnostics.rs index 24a1bdf1883..f6d87c8532e 100644 --- a/clippy_lints/src/utils/diagnostics.rs +++ b/clippy_lints/src/utils/diagnostics.rs @@ -1,6 +1,6 @@ //! Clippy wrappers around rustc's diagnostic functions. -use rustc_errors::{Applicability, CodeSuggestion, DiagnosticBuilder, Substitution, SubstitutionPart, SuggestionStyle}; +use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir::HirId; use rustc_lint::{LateContext, Lint, LintContext}; use rustc_span::source_map::{MultiSpan, Span}; @@ -198,20 +198,20 @@ pub fn span_lint_and_sugg<'a, T: LintContext>( /// appear once per /// replacement. In human-readable format though, it only appears once before /// the whole suggestion. -pub fn multispan_sugg(diag: &mut DiagnosticBuilder<'_>, help_msg: String, sugg: I) +pub fn multispan_sugg(diag: &mut DiagnosticBuilder<'_>, help_msg: &str, sugg: I) where I: IntoIterator, { - let sugg = CodeSuggestion { - substitutions: vec![Substitution { - parts: sugg - .into_iter() - .map(|(span, snippet)| SubstitutionPart { snippet, span }) - .collect(), - }], - msg: help_msg, - style: SuggestionStyle::ShowCode, - applicability: Applicability::Unspecified, - }; - diag.suggestions.push(sugg); + multispan_sugg_with_applicability(diag, help_msg, Applicability::Unspecified, sugg) +} + +pub fn multispan_sugg_with_applicability( + diag: &mut DiagnosticBuilder<'_>, + help_msg: &str, + applicability: Applicability, + sugg: I, +) where + I: IntoIterator, +{ + diag.multipart_suggestion(help_msg, sugg.into_iter().collect(), applicability); } diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index e8535677987..904d948ad29 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -77,8 +77,8 @@ impl<'tcx> Delegate<'tcx> for MutVarsDelegate { } pub struct UsedVisitor { - pub var: Symbol, // var to look for - pub used: bool, // has the var been used otherwise? + pub var: Symbol, // var to look for + pub used: bool, // has the var been used otherwise? } impl<'tcx> Visitor<'tcx> for UsedVisitor { diff --git a/src/driver.rs b/src/driver.rs index 1ce0300f239..d3a7e24937f 100644 --- a/src/driver.rs +++ b/src/driver.rs @@ -295,121 +295,119 @@ fn toolchain_path(home: Option, toolchain: Option) -> Option = env::args().collect(); + exit(rustc_driver::catch_with_exit_code(move || { + let mut orig_args: Vec = env::args().collect(); - if orig_args.iter().any(|a| a == "--version" || a == "-V") { - let version_info = rustc_tools_util::get_version_info!(); - println!("{}", version_info); - exit(0); + if orig_args.iter().any(|a| a == "--version" || a == "-V") { + let version_info = rustc_tools_util::get_version_info!(); + println!("{}", version_info); + exit(0); + } + + // Get the sysroot, looking from most specific to this invocation to the least: + // - command line + // - runtime environment + // - SYSROOT + // - RUSTUP_HOME, MULTIRUST_HOME, RUSTUP_TOOLCHAIN, MULTIRUST_TOOLCHAIN + // - sysroot from rustc in the path + // - compile-time environment + // - SYSROOT + // - RUSTUP_HOME, MULTIRUST_HOME, RUSTUP_TOOLCHAIN, MULTIRUST_TOOLCHAIN + let sys_root_arg = arg_value(&orig_args, "--sysroot", |_| true); + let have_sys_root_arg = sys_root_arg.is_some(); + let sys_root = sys_root_arg + .map(PathBuf::from) + .or_else(|| std::env::var("SYSROOT").ok().map(PathBuf::from)) + .or_else(|| { + let home = std::env::var("RUSTUP_HOME") + .or_else(|_| std::env::var("MULTIRUST_HOME")) + .ok(); + let toolchain = std::env::var("RUSTUP_TOOLCHAIN") + .or_else(|_| std::env::var("MULTIRUST_TOOLCHAIN")) + .ok(); + toolchain_path(home, toolchain) + }) + .or_else(|| { + Command::new("rustc") + .arg("--print") + .arg("sysroot") + .output() + .ok() + .and_then(|out| String::from_utf8(out.stdout).ok()) + .map(|s| PathBuf::from(s.trim())) + }) + .or_else(|| option_env!("SYSROOT").map(PathBuf::from)) + .or_else(|| { + let home = option_env!("RUSTUP_HOME") + .or(option_env!("MULTIRUST_HOME")) + .map(ToString::to_string); + let toolchain = option_env!("RUSTUP_TOOLCHAIN") + .or(option_env!("MULTIRUST_TOOLCHAIN")) + .map(ToString::to_string); + toolchain_path(home, toolchain) + }) + .map(|pb| pb.to_string_lossy().to_string()) + .expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust"); + + // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument. + // We're invoking the compiler programmatically, so we ignore this/ + let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref()); + + if wrapper_mode { + // we still want to be able to invoke it normally though + orig_args.remove(1); + } + + if !wrapper_mode && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) { + display_help(); + exit(0); + } + + let should_describe_lints = || { + let args: Vec<_> = env::args().collect(); + args.windows(2).any(|args| { + args[1] == "help" + && match args[0].as_str() { + "-W" | "-A" | "-D" | "-F" => true, + _ => false, + } + }) + }; + + if !wrapper_mode && should_describe_lints() { + describe_lints(); + exit(0); + } + + // this conditional check for the --sysroot flag is there so users can call + // `clippy_driver` directly + // without having to pass --sysroot or anything + let mut args: Vec = orig_args.clone(); + if !have_sys_root_arg { + args.extend(vec!["--sysroot".into(), sys_root]); + }; + + // this check ensures that dependencies are built but not linted and the final + // crate is linted but not built + let clippy_enabled = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true") + || arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_none(); + + if clippy_enabled { + args.extend(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()]); + if let Ok(extra_args) = env::var("CLIPPY_ARGS") { + args.extend(extra_args.split("__CLIPPY_HACKERY__").filter_map(|s| { + if s.is_empty() { + None + } else { + Some(s.to_string()) + } + })); } - - // Get the sysroot, looking from most specific to this invocation to the least: - // - command line - // - runtime environment - // - SYSROOT - // - RUSTUP_HOME, MULTIRUST_HOME, RUSTUP_TOOLCHAIN, MULTIRUST_TOOLCHAIN - // - sysroot from rustc in the path - // - compile-time environment - // - SYSROOT - // - RUSTUP_HOME, MULTIRUST_HOME, RUSTUP_TOOLCHAIN, MULTIRUST_TOOLCHAIN - let sys_root_arg = arg_value(&orig_args, "--sysroot", |_| true); - let have_sys_root_arg = sys_root_arg.is_some(); - let sys_root = sys_root_arg - .map(PathBuf::from) - .or_else(|| std::env::var("SYSROOT").ok().map(PathBuf::from)) - .or_else(|| { - let home = std::env::var("RUSTUP_HOME") - .or_else(|_| std::env::var("MULTIRUST_HOME")) - .ok(); - let toolchain = std::env::var("RUSTUP_TOOLCHAIN") - .or_else(|_| std::env::var("MULTIRUST_TOOLCHAIN")) - .ok(); - toolchain_path(home, toolchain) - }) - .or_else(|| { - Command::new("rustc") - .arg("--print") - .arg("sysroot") - .output() - .ok() - .and_then(|out| String::from_utf8(out.stdout).ok()) - .map(|s| PathBuf::from(s.trim())) - }) - .or_else(|| option_env!("SYSROOT").map(PathBuf::from)) - .or_else(|| { - let home = option_env!("RUSTUP_HOME") - .or(option_env!("MULTIRUST_HOME")) - .map(ToString::to_string); - let toolchain = option_env!("RUSTUP_TOOLCHAIN") - .or(option_env!("MULTIRUST_TOOLCHAIN")) - .map(ToString::to_string); - toolchain_path(home, toolchain) - }) - .map(|pb| pb.to_string_lossy().to_string()) - .expect("need to specify SYSROOT env var during clippy compilation, or use rustup or multirust"); - - // Setting RUSTC_WRAPPER causes Cargo to pass 'rustc' as the first argument. - // We're invoking the compiler programmatically, so we ignore this/ - let wrapper_mode = orig_args.get(1).map(Path::new).and_then(Path::file_stem) == Some("rustc".as_ref()); - - if wrapper_mode { - // we still want to be able to invoke it normally though - orig_args.remove(1); - } - - if !wrapper_mode && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) { - display_help(); - exit(0); - } - - let should_describe_lints = || { - let args: Vec<_> = env::args().collect(); - args.windows(2).any(|args| { - args[1] == "help" - && match args[0].as_str() { - "-W" | "-A" | "-D" | "-F" => true, - _ => false, - } - }) - }; - - if !wrapper_mode && should_describe_lints() { - describe_lints(); - exit(0); - } - - // this conditional check for the --sysroot flag is there so users can call - // `clippy_driver` directly - // without having to pass --sysroot or anything - let mut args: Vec = orig_args.clone(); - if !have_sys_root_arg { - args.extend(vec!["--sysroot".into(), sys_root]); - }; - - // this check ensures that dependencies are built but not linted and the final - // crate is linted but not built - let clippy_enabled = env::var("CLIPPY_TESTS").map_or(false, |val| val == "true") - || arg_value(&orig_args, "--cap-lints", |val| val == "allow").is_none(); - - if clippy_enabled { - args.extend(vec!["--cfg".into(), r#"feature="cargo-clippy""#.into()]); - if let Ok(extra_args) = env::var("CLIPPY_ARGS") { - args.extend(extra_args.split("__CLIPPY_HACKERY__").filter_map(|s| { - if s.is_empty() { - None - } else { - Some(s.to_string()) - } - })); - } - } - let mut clippy = ClippyCallbacks; - let mut default = DefaultCallbacks; - let callbacks: &mut (dyn rustc_driver::Callbacks + Send) = - if clippy_enabled { &mut clippy } else { &mut default }; - rustc_driver::run_compiler(&args, callbacks, None, None) - }) - ) + } + let mut clippy = ClippyCallbacks; + let mut default = DefaultCallbacks; + let callbacks: &mut (dyn rustc_driver::Callbacks + Send) = + if clippy_enabled { &mut clippy } else { &mut default }; + rustc_driver::run_compiler(&args, callbacks, None, None) + })) } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 51d1cb2216a..9457a64f9c6 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -66,6 +66,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "bit_mask", }, + Lint { + name: "bind_instead_of_map", + group: "complexity", + desc: "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`", + deprecation: None, + module: "methods", + }, Lint { name: "blacklisted_name", group: "style", @@ -74,18 +81,11 @@ pub static ref ALL_LINTS: Vec = vec![ module: "blacklisted_name", }, Lint { - name: "block_in_if_condition_expr", + name: "blocks_in_if_conditions", group: "style", - desc: "braces that can be eliminated in conditions, e.g., `if { true } ...`", + desc: "useless or complex blocks that can be eliminated in conditions", deprecation: None, - module: "block_in_if_condition", - }, - Lint { - name: "block_in_if_condition_stmt", - group: "style", - desc: "complex blocks in conditions, e.g., `if { let x = true; x } ...`", - deprecation: None, - module: "block_in_if_condition", + module: "blocks_in_if_conditions", }, Lint { name: "bool_comparison", @@ -521,6 +521,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "methods", }, + Lint { + name: "expect_used", + group: "restriction", + desc: "using `.expect()` on `Result` or `Option`, which might be better handled", + deprecation: None, + module: "methods", + }, Lint { name: "expl_impl_clone_on_copy", group: "pedantic", @@ -676,16 +683,9 @@ pub static ref ALL_LINTS: Vec = vec![ module: "loops", }, Lint { - name: "for_loop_over_option", + name: "for_loops_over_fallibles", group: "correctness", - desc: "for-looping over an `Option`, which is more clearly expressed as an `if let`", - deprecation: None, - module: "loops", - }, - Lint { - name: "for_loop_over_result", - group: "correctness", - desc: "for-looping over a `Result`, which is more clearly expressed as an `if let`", + desc: "for-looping over an `Option` or a `Result`, which is more clearly expressed as an `if let`", deprecation: None, module: "loops", }, @@ -724,13 +724,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "methods", }, - Lint { - name: "identity_conversion", - group: "complexity", - desc: "using always-identical `Into`/`From`/`IntoIter` conversions", - deprecation: None, - module: "identity_conversion", - }, Lint { name: "identity_op", group: "complexity", @@ -1144,6 +1137,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "methods", }, + Lint { + name: "map_unwrap_or", + group: "pedantic", + desc: "using `.map(f).unwrap_or(a)` or `.map(f).unwrap_or_else(func)`, which are more succinctly expressed as `map_or(a, f)` or `map_or_else(a, f)`", + deprecation: None, + module: "methods", + }, Lint { name: "match_as_ref", group: "complexity", @@ -1578,13 +1578,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "eq_op", }, - Lint { - name: "option_and_then_some", - group: "complexity", - desc: "using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)`", - deprecation: None, - module: "methods", - }, Lint { name: "option_as_ref_deref", group: "complexity", @@ -1599,13 +1592,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "option_env_unwrap", }, - Lint { - name: "option_expect_used", - group: "restriction", - desc: "using `Option.expect()`, which might be better handled", - deprecation: None, - module: "methods", - }, Lint { name: "option_map_or_none", group: "style", @@ -1620,20 +1606,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "map_unit_fn", }, - Lint { - name: "option_map_unwrap_or", - group: "pedantic", - desc: "using `Option.map(f).unwrap_or(a)`, which is more succinctly expressed as `map_or(a, f)`", - deprecation: None, - module: "methods", - }, - Lint { - name: "option_map_unwrap_or_else", - group: "pedantic", - desc: "using `Option.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `map_or_else(g, f)`", - deprecation: None, - module: "methods", - }, Lint { name: "option_option", group: "pedantic", @@ -1641,13 +1613,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "types", }, - Lint { - name: "option_unwrap_used", - group: "restriction", - desc: "using `Option.unwrap()`, which should at least get a better message using `expect()`", - deprecation: None, - module: "methods", - }, Lint { name: "or_fun_call", group: "perf", @@ -1886,13 +1851,6 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "matches", }, - Lint { - name: "result_expect_used", - group: "restriction", - desc: "using `Result.expect()`, which might be better handled", - deprecation: None, - module: "methods", - }, Lint { name: "result_map_or_into_option", group: "style", @@ -1908,25 +1866,11 @@ pub static ref ALL_LINTS: Vec = vec![ module: "map_unit_fn", }, Lint { - name: "result_map_unwrap_or_else", - group: "pedantic", - desc: "using `Result.map(f).unwrap_or_else(g)`, which is more succinctly expressed as `.map_or_else(g, f)`", - deprecation: None, - module: "methods", - }, - Lint { - name: "result_unwrap_used", - group: "restriction", - desc: "using `Result.unwrap()`, which might be better handled", - deprecation: None, - module: "methods", - }, - Lint { - name: "reverse_range_loop", + name: "reversed_empty_ranges", group: "correctness", - desc: "iteration over an empty range, such as `10..0` or `5..5`", + desc: "reversing the limits of range expressions, resulting in empty ranges", deprecation: None, - module: "loops", + module: "ranges", }, Lint { name: "same_functions_in_if_condition", @@ -2425,6 +2369,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "returns", }, + Lint { + name: "unwrap_used", + group: "restriction", + desc: "using `.unwrap()` on `Result` or `Option`, which should at least get a better message using `expect()`", + deprecation: None, + module: "methods", + }, Lint { name: "use_debug", group: "restriction", @@ -2460,6 +2411,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "attrs", }, + Lint { + name: "useless_conversion", + group: "complexity", + desc: "calls to `Into`/`From`/`IntoIter` that performs useless conversions to the same type", + deprecation: None, + module: "useless_conversion", + }, Lint { name: "useless_format", group: "complexity", @@ -2469,7 +2427,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "useless_let_if_seq", - group: "style", + group: "nursery", desc: "unidiomatic `let mut` declaration followed by initialization in `if`", deprecation: None, module: "let_if_seq", diff --git a/tests/ui/option_and_then_some.fixed b/tests/ui/bind_instead_of_map.fixed similarity index 90% rename from tests/ui/option_and_then_some.fixed rename to tests/ui/bind_instead_of_map.fixed index 035bc1e5465..5815550d7a6 100644 --- a/tests/ui/option_and_then_some.fixed +++ b/tests/ui/bind_instead_of_map.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![deny(clippy::option_and_then_some)] +#![deny(clippy::bind_instead_of_map)] // need a main anyway, use it get rid of unused warnings too pub fn main() { @@ -12,7 +12,7 @@ pub fn main() { // Different type let x: Result = Ok(1); - let _ = x.and_then(Ok); + let _ = x; } pub fn foo() -> Option { diff --git a/tests/ui/option_and_then_some.rs b/tests/ui/bind_instead_of_map.rs similarity index 94% rename from tests/ui/option_and_then_some.rs rename to tests/ui/bind_instead_of_map.rs index d49da7813c6..623b100a4ce 100644 --- a/tests/ui/option_and_then_some.rs +++ b/tests/ui/bind_instead_of_map.rs @@ -1,5 +1,5 @@ // run-rustfix -#![deny(clippy::option_and_then_some)] +#![deny(clippy::bind_instead_of_map)] // need a main anyway, use it get rid of unused warnings too pub fn main() { diff --git a/tests/ui/option_and_then_some.stderr b/tests/ui/bind_instead_of_map.stderr similarity index 50% rename from tests/ui/option_and_then_some.stderr rename to tests/ui/bind_instead_of_map.stderr index 47825491765..24c6b7f9ef3 100644 --- a/tests/ui/option_and_then_some.stderr +++ b/tests/ui/bind_instead_of_map.stderr @@ -1,20 +1,26 @@ error: using `Option.and_then(Some)`, which is a no-op - --> $DIR/option_and_then_some.rs:8:13 + --> $DIR/bind_instead_of_map.rs:8:13 | LL | let _ = x.and_then(Some); | ^^^^^^^^^^^^^^^^ help: use the expression directly: `x` | note: the lint level is defined here - --> $DIR/option_and_then_some.rs:2:9 + --> $DIR/bind_instead_of_map.rs:2:9 | -LL | #![deny(clippy::option_and_then_some)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(clippy::bind_instead_of_map)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` - --> $DIR/option_and_then_some.rs:9:13 + --> $DIR/bind_instead_of_map.rs:9:13 | LL | let _ = x.and_then(|o| Some(o + 1)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.map(|o| o + 1)` -error: aborting due to 2 previous errors +error: using `Result.and_then(Ok)`, which is a no-op + --> $DIR/bind_instead_of_map.rs:15:13 + | +LL | let _ = x.and_then(Ok); + | ^^^^^^^^^^^^^^ help: use the expression directly: `x` + +error: aborting due to 3 previous errors diff --git a/tests/ui/bind_instead_of_map_multipart.rs b/tests/ui/bind_instead_of_map_multipart.rs new file mode 100644 index 00000000000..91d9d11e3c1 --- /dev/null +++ b/tests/ui/bind_instead_of_map_multipart.rs @@ -0,0 +1,61 @@ +#![deny(clippy::bind_instead_of_map)] +#![allow(clippy::blocks_in_if_conditions)] + +pub fn main() { + let _ = Some("42").and_then(|s| if s.len() < 42 { Some(0) } else { Some(s.len()) }); + let _ = Some("42").and_then(|s| if s.len() < 42 { None } else { Some(s.len()) }); + + let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Ok(0) } else { Ok(s.len()) }); + let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Err(()) } else { Ok(s.len()) }); + + let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Err(s.len() + 20) } else { Err(s.len()) }); + let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Ok(()) } else { Err(s.len()) }); + + hard_example(); + macro_example(); +} + +fn hard_example() { + Some("42").and_then(|s| { + if { + if s == "43" { + return Some(43); + } + s == "42" + } { + return Some(45); + } + match s.len() { + 10 => Some(2), + 20 => { + if foo() { + return { + if foo() { + return Some(20); + } + println!("foo"); + Some(3) + }; + } + Some(20) + }, + 40 => Some(30), + _ => Some(1), + } + }); +} + +fn foo() -> bool { + true +} + +macro_rules! m { + () => { + Some(10) + }; +} + +fn macro_example() { + let _ = Some("").and_then(|s| if s.len() == 20 { m!() } else { Some(20) }); + let _ = Some("").and_then(|s| if s.len() == 20 { Some(m!()) } else { Some(Some(20)) }); +} diff --git a/tests/ui/bind_instead_of_map_multipart.stderr b/tests/ui/bind_instead_of_map_multipart.stderr new file mode 100644 index 00000000000..50ce2f4051e --- /dev/null +++ b/tests/ui/bind_instead_of_map_multipart.stderr @@ -0,0 +1,73 @@ +error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` + --> $DIR/bind_instead_of_map_multipart.rs:5:13 + | +LL | let _ = Some("42").and_then(|s| if s.len() < 42 { Some(0) } else { Some(s.len()) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/bind_instead_of_map_multipart.rs:1:9 + | +LL | #![deny(clippy::bind_instead_of_map)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: try this + | +LL | let _ = Some("42").map(|s| if s.len() < 42 { 0 } else { s.len() }); + | ^^^ ^ ^^^^^^^ + +error: using `Result.and_then(|x| Ok(y))`, which is more succinctly expressed as `map(|x| y)` + --> $DIR/bind_instead_of_map_multipart.rs:8:13 + | +LL | let _ = Ok::<_, ()>("42").and_then(|s| if s.len() < 42 { Ok(0) } else { Ok(s.len()) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try this + | +LL | let _ = Ok::<_, ()>("42").map(|s| if s.len() < 42 { 0 } else { s.len() }); + | ^^^ ^ ^^^^^^^ + +error: using `Result.or_else(|x| Err(y))`, which is more succinctly expressed as `map_err(|x| y)` + --> $DIR/bind_instead_of_map_multipart.rs:11:13 + | +LL | let _ = Err::<(), _>("42").or_else(|s| if s.len() < 42 { Err(s.len() + 20) } else { Err(s.len()) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try this + | +LL | let _ = Err::<(), _>("42").map_err(|s| if s.len() < 42 { s.len() + 20 } else { s.len() }); + | ^^^^^^^ ^^^^^^^^^^^^ ^^^^^^^ + +error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` + --> $DIR/bind_instead_of_map_multipart.rs:19:5 + | +LL | / Some("42").and_then(|s| { +LL | | if { +LL | | if s == "43" { +LL | | return Some(43); +... | +LL | | } +LL | | }); + | |______^ + | +help: try this + | +LL | Some("42").map(|s| { +LL | if { +LL | if s == "43" { +LL | return 43; +LL | } +LL | s == "42" + ... + +error: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` + --> $DIR/bind_instead_of_map_multipart.rs:60:13 + | +LL | let _ = Some("").and_then(|s| if s.len() == 20 { Some(m!()) } else { Some(Some(20)) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: try this + | +LL | let _ = Some("").map(|s| if s.len() == 20 { m!() } else { Some(20) }); + | ^^^ ^^^^ ^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/block_in_if_condition.fixed b/tests/ui/blocks_in_if_conditions.fixed similarity index 88% rename from tests/ui/block_in_if_condition.fixed rename to tests/ui/blocks_in_if_conditions.fixed index 955801e40f9..14562c4d32c 100644 --- a/tests/ui/block_in_if_condition.fixed +++ b/tests/ui/blocks_in_if_conditions.fixed @@ -1,6 +1,5 @@ // run-rustfix -#![warn(clippy::block_in_if_condition_expr)] -#![warn(clippy::block_in_if_condition_stmt)] +#![warn(clippy::blocks_in_if_conditions)] #![allow(unused, clippy::let_and_return)] #![warn(clippy::nonminimal_bool)] @@ -64,10 +63,10 @@ fn block_in_assert() { let opt = Some(42); assert!(opt .as_ref() - .and_then(|val| { + .map(|val| { let mut v = val * 2; v -= 1; - Some(v * 3) + v * 3 }) .is_some()); } diff --git a/tests/ui/block_in_if_condition.rs b/tests/ui/blocks_in_if_conditions.rs similarity index 88% rename from tests/ui/block_in_if_condition.rs rename to tests/ui/blocks_in_if_conditions.rs index a6ea01d5fc5..bda87650f6d 100644 --- a/tests/ui/block_in_if_condition.rs +++ b/tests/ui/blocks_in_if_conditions.rs @@ -1,6 +1,5 @@ // run-rustfix -#![warn(clippy::block_in_if_condition_expr)] -#![warn(clippy::block_in_if_condition_stmt)] +#![warn(clippy::blocks_in_if_conditions)] #![allow(unused, clippy::let_and_return)] #![warn(clippy::nonminimal_bool)] @@ -64,10 +63,10 @@ fn block_in_assert() { let opt = Some(42); assert!(opt .as_ref() - .and_then(|val| { + .map(|val| { let mut v = val * 2; v -= 1; - Some(v * 3) + v * 3 }) .is_some()); } diff --git a/tests/ui/block_in_if_condition.stderr b/tests/ui/blocks_in_if_conditions.stderr similarity index 71% rename from tests/ui/block_in_if_condition.stderr rename to tests/ui/blocks_in_if_conditions.stderr index b0a0a276c89..9bdddc8e152 100644 --- a/tests/ui/block_in_if_condition.stderr +++ b/tests/ui/blocks_in_if_conditions.stderr @@ -1,5 +1,5 @@ error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` - --> $DIR/block_in_if_condition.rs:27:5 + --> $DIR/blocks_in_if_conditions.rs:26:5 | LL | / if { LL | | let x = 3; @@ -7,7 +7,7 @@ LL | | x == 3 LL | | } { | |_____^ | - = note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings` + = note: `-D clippy::blocks-in-if-conditions` implied by `-D warnings` help: try | LL | let res = { @@ -17,15 +17,13 @@ LL | }; if res { | error: omit braces around single expression condition - --> $DIR/block_in_if_condition.rs:38:8 + --> $DIR/blocks_in_if_conditions.rs:37:8 | LL | if { true } { | ^^^^^^^^ help: try: `true` - | - = note: `-D clippy::block-in-if-condition-expr` implied by `-D warnings` error: this boolean expression can be simplified - --> $DIR/block_in_if_condition.rs:47:8 + --> $DIR/blocks_in_if_conditions.rs:46:8 | LL | if true && x == 3 { | ^^^^^^^^^^^^^^ help: try: `x == 3` diff --git a/tests/ui/block_in_if_condition_closure.rs b/tests/ui/blocks_in_if_conditions_closure.rs similarity index 85% rename from tests/ui/block_in_if_condition_closure.rs rename to tests/ui/blocks_in_if_conditions_closure.rs index bac3eda5e7f..acbabfa20d7 100644 --- a/tests/ui/block_in_if_condition_closure.rs +++ b/tests/ui/blocks_in_if_conditions_closure.rs @@ -1,5 +1,4 @@ -#![warn(clippy::block_in_if_condition_expr)] -#![warn(clippy::block_in_if_condition_stmt)] +#![warn(clippy::blocks_in_if_conditions)] #![allow(unused, clippy::let_and_return)] fn predicate bool, T>(pfn: F, val: T) -> bool { @@ -10,7 +9,7 @@ fn pred_test() { let v = 3; let sky = "blue"; // This is a sneaky case, where the block isn't directly in the condition, - // but is actually nside a closure that the condition is using. + // but is actually inside a closure that the condition is using. // The same principle applies -- add some extra expressions to make sure // linter isn't confused by them. if v == 3 diff --git a/tests/ui/block_in_if_condition_closure.stderr b/tests/ui/blocks_in_if_conditions_closure.stderr similarity index 78% rename from tests/ui/block_in_if_condition_closure.stderr rename to tests/ui/blocks_in_if_conditions_closure.stderr index 86cd24fe763..941d604dd5f 100644 --- a/tests/ui/block_in_if_condition_closure.stderr +++ b/tests/ui/blocks_in_if_conditions_closure.stderr @@ -1,5 +1,5 @@ error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` - --> $DIR/block_in_if_condition_closure.rs:19:17 + --> $DIR/blocks_in_if_conditions_closure.rs:18:17 | LL | |x| { | _________________^ @@ -8,10 +8,10 @@ LL | | x == target LL | | }, | |_____________^ | - = note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings` + = note: `-D clippy::blocks-in-if-conditions` implied by `-D warnings` error: in an `if` condition, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let` - --> $DIR/block_in_if_condition_closure.rs:28:13 + --> $DIR/blocks_in_if_conditions_closure.rs:27:13 | LL | |x| { | _____________^ diff --git a/tests/ui/comparison_chain.rs b/tests/ui/comparison_chain.rs index 9c2128469de..3b03f8c7dfe 100644 --- a/tests/ui/comparison_chain.rs +++ b/tests/ui/comparison_chain.rs @@ -137,4 +137,70 @@ fn h(x: T, y: T, z: T) { } } +// The following uses should be ignored +mod issue_5212 { + use super::{a, b, c}; + fn foo() -> u8 { + 21 + } + + fn same_operation_equals() { + // operands are fixed + + if foo() == 42 { + a() + } else if foo() == 42 { + b() + } + + if foo() == 42 { + a() + } else if foo() == 42 { + b() + } else { + c() + } + + // operands are transposed + + if foo() == 42 { + a() + } else if 42 == foo() { + b() + } + } + + fn same_operation_not_equals() { + // operands are fixed + + if foo() > 42 { + a() + } else if foo() > 42 { + b() + } + + if foo() > 42 { + a() + } else if foo() > 42 { + b() + } else { + c() + } + + if foo() < 42 { + a() + } else if foo() < 42 { + b() + } + + if foo() < 42 { + a() + } else if foo() < 42 { + b() + } else { + c() + } + } +} + fn main() {} diff --git a/tests/ui/crashes/ice-5579.rs b/tests/ui/crashes/ice-5579.rs new file mode 100644 index 00000000000..e1842c73f0e --- /dev/null +++ b/tests/ui/crashes/ice-5579.rs @@ -0,0 +1,17 @@ +trait IsErr { + fn is_err(&self, err: &str) -> bool; +} + +impl IsErr for Option { + fn is_err(&self, _err: &str) -> bool { + true + } +} + +fn main() { + let t = Some(1); + + if t.is_err("") { + t.unwrap(); + } +} diff --git a/tests/ui/expect.rs b/tests/ui/expect.rs index 0bd4252c49a..1073acf6f0c 100644 --- a/tests/ui/expect.rs +++ b/tests/ui/expect.rs @@ -1,4 +1,4 @@ -#![warn(clippy::option_expect_used, clippy::result_expect_used)] +#![warn(clippy::expect_used)] fn expect_option() { let opt = Some(0); diff --git a/tests/ui/expect.stderr b/tests/ui/expect.stderr index adf9f4f1921..9d3fc7df15c 100644 --- a/tests/ui/expect.stderr +++ b/tests/ui/expect.stderr @@ -4,7 +4,7 @@ error: used `expect()` on `an Option` value LL | let _ = opt.expect(""); | ^^^^^^^^^^^^^^ | - = note: `-D clippy::option-expect-used` implied by `-D warnings` + = note: `-D clippy::expect-used` implied by `-D warnings` = help: if this value is an `None`, it will panic error: used `expect()` on `a Result` value @@ -13,7 +13,6 @@ error: used `expect()` on `a Result` value LL | let _ = res.expect(""); | ^^^^^^^^^^^^^^ | - = note: `-D clippy::result-expect-used` implied by `-D warnings` = help: if this value is an `Err`, it will panic error: aborting due to 2 previous errors diff --git a/tests/ui/for_loop_fixable.fixed b/tests/ui/for_loop_fixable.fixed index 5fc84ada9ef..249a88a0b39 100644 --- a/tests/ui/for_loop_fixable.fixed +++ b/tests/ui/for_loop_fixable.fixed @@ -21,7 +21,6 @@ impl Unrelated { clippy::explicit_iter_loop, clippy::explicit_into_iter_loop, clippy::iter_next_loop, - clippy::reverse_range_loop, clippy::for_kv_map )] #[allow( @@ -32,61 +31,8 @@ impl Unrelated { )] #[allow(clippy::many_single_char_names, unused_variables)] fn main() { - const MAX_LEN: usize = 42; let mut vec = vec![1, 2, 3, 4]; - for i in (0..10).rev() { - println!("{}", i); - } - - for i in (0..=10).rev() { - println!("{}", i); - } - - for i in (0..MAX_LEN).rev() { - println!("{}", i); - } - - for i in 5..=5 { - // not an error, this is the range with only one element “5” - println!("{}", i); - } - - for i in 0..10 { - // not an error, the start index is less than the end index - println!("{}", i); - } - - for i in -10..0 { - // not an error - println!("{}", i); - } - - for i in (10..0).map(|x| x * 2) { - // not an error, it can't be known what arbitrary methods do to a range - println!("{}", i); - } - - // testing that the empty range lint folds constants - for i in (5 + 4..10).rev() { - println!("{}", i); - } - - for i in ((3 - 1)..(5 + 2)).rev() { - println!("{}", i); - } - - for i in (2 * 2)..(2 * 3) { - // no error, 4..6 is fine - println!("{}", i); - } - - let x = 42; - for i in x..10 { - // no error, not constant-foldable - println!("{}", i); - } - // See #601 for i in 0..10 { // no error, id_col does not exist outside the loop diff --git a/tests/ui/for_loop_fixable.rs b/tests/ui/for_loop_fixable.rs index 4165b0dc004..306d85a6351 100644 --- a/tests/ui/for_loop_fixable.rs +++ b/tests/ui/for_loop_fixable.rs @@ -21,7 +21,6 @@ impl Unrelated { clippy::explicit_iter_loop, clippy::explicit_into_iter_loop, clippy::iter_next_loop, - clippy::reverse_range_loop, clippy::for_kv_map )] #[allow( @@ -32,61 +31,8 @@ impl Unrelated { )] #[allow(clippy::many_single_char_names, unused_variables)] fn main() { - const MAX_LEN: usize = 42; let mut vec = vec![1, 2, 3, 4]; - for i in 10..0 { - println!("{}", i); - } - - for i in 10..=0 { - println!("{}", i); - } - - for i in MAX_LEN..0 { - println!("{}", i); - } - - for i in 5..=5 { - // not an error, this is the range with only one element “5” - println!("{}", i); - } - - for i in 0..10 { - // not an error, the start index is less than the end index - println!("{}", i); - } - - for i in -10..0 { - // not an error - println!("{}", i); - } - - for i in (10..0).map(|x| x * 2) { - // not an error, it can't be known what arbitrary methods do to a range - println!("{}", i); - } - - // testing that the empty range lint folds constants - for i in 10..5 + 4 { - println!("{}", i); - } - - for i in (5 + 2)..(3 - 1) { - println!("{}", i); - } - - for i in (2 * 2)..(2 * 3) { - // no error, 4..6 is fine - println!("{}", i); - } - - let x = 42; - for i in x..10 { - // no error, not constant-foldable - println!("{}", i); - } - // See #601 for i in 0..10 { // no error, id_col does not exist outside the loop diff --git a/tests/ui/for_loop_fixable.stderr b/tests/ui/for_loop_fixable.stderr index cffb4b9f0a9..ddfe66d675f 100644 --- a/tests/ui/for_loop_fixable.stderr +++ b/tests/ui/for_loop_fixable.stderr @@ -1,61 +1,5 @@ -error: this range is empty so this for loop will never run - --> $DIR/for_loop_fixable.rs:38:14 - | -LL | for i in 10..0 { - | ^^^^^ - | - = note: `-D clippy::reverse-range-loop` implied by `-D warnings` -help: consider using the following if you are attempting to iterate over this range in reverse - | -LL | for i in (0..10).rev() { - | ^^^^^^^^^^^^^ - -error: this range is empty so this for loop will never run - --> $DIR/for_loop_fixable.rs:42:14 - | -LL | for i in 10..=0 { - | ^^^^^^ - | -help: consider using the following if you are attempting to iterate over this range in reverse - | -LL | for i in (0..=10).rev() { - | ^^^^^^^^^^^^^^ - -error: this range is empty so this for loop will never run - --> $DIR/for_loop_fixable.rs:46:14 - | -LL | for i in MAX_LEN..0 { - | ^^^^^^^^^^ - | -help: consider using the following if you are attempting to iterate over this range in reverse - | -LL | for i in (0..MAX_LEN).rev() { - | ^^^^^^^^^^^^^^^^^^ - -error: this range is empty so this for loop will never run - --> $DIR/for_loop_fixable.rs:71:14 - | -LL | for i in 10..5 + 4 { - | ^^^^^^^^^ - | -help: consider using the following if you are attempting to iterate over this range in reverse - | -LL | for i in (5 + 4..10).rev() { - | ^^^^^^^^^^^^^^^^^ - -error: this range is empty so this for loop will never run - --> $DIR/for_loop_fixable.rs:75:14 - | -LL | for i in (5 + 2)..(3 - 1) { - | ^^^^^^^^^^^^^^^^ - | -help: consider using the following if you are attempting to iterate over this range in reverse - | -LL | for i in ((3 - 1)..(5 + 2)).rev() { - | ^^^^^^^^^^^^^^^^^^^^^^^^ - error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:97:15 + --> $DIR/for_loop_fixable.rs:43:15 | LL | for _v in vec.iter() {} | ^^^^^^^^^^ help: to write this more concisely, try: `&vec` @@ -63,13 +7,13 @@ LL | for _v in vec.iter() {} = note: `-D clippy::explicit-iter-loop` implied by `-D warnings` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:99:15 + --> $DIR/for_loop_fixable.rs:45:15 | LL | for _v in vec.iter_mut() {} | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec` error: it is more concise to loop over containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:102:15 + --> $DIR/for_loop_fixable.rs:48:15 | LL | for _v in out_vec.into_iter() {} | ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec` @@ -77,76 +21,76 @@ LL | for _v in out_vec.into_iter() {} = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:107:15 + --> $DIR/for_loop_fixable.rs:53:15 | LL | for _v in [1, 2, 3].iter() {} | ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:111:15 + --> $DIR/for_loop_fixable.rs:57:15 | LL | for _v in [0; 32].iter() {} | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:116:15 + --> $DIR/for_loop_fixable.rs:62:15 | LL | for _v in ll.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&ll` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:119:15 + --> $DIR/for_loop_fixable.rs:65:15 | LL | for _v in vd.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&vd` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:122:15 + --> $DIR/for_loop_fixable.rs:68:15 | LL | for _v in bh.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bh` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:125:15 + --> $DIR/for_loop_fixable.rs:71:15 | LL | for _v in hm.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&hm` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:128:15 + --> $DIR/for_loop_fixable.rs:74:15 | LL | for _v in bt.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bt` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:131:15 + --> $DIR/for_loop_fixable.rs:77:15 | LL | for _v in hs.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&hs` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:134:15 + --> $DIR/for_loop_fixable.rs:80:15 | LL | for _v in bs.iter() {} | ^^^^^^^^^ help: to write this more concisely, try: `&bs` error: it is more concise to loop over containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:309:18 + --> $DIR/for_loop_fixable.rs:255:18 | LL | for i in iterator.into_iter() { | ^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `iterator` error: it is more concise to loop over references to containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:329:18 + --> $DIR/for_loop_fixable.rs:275:18 | LL | for _ in t.into_iter() {} | ^^^^^^^^^^^^^ help: to write this more concisely, try: `&t` error: it is more concise to loop over containers instead of using explicit iteration methods - --> $DIR/for_loop_fixable.rs:331:18 + --> $DIR/for_loop_fixable.rs:277:18 | LL | for _ in r.into_iter() {} | ^^^^^^^^^^^^^ help: to write this more concisely, try: `r` -error: aborting due to 20 previous errors +error: aborting due to 15 previous errors diff --git a/tests/ui/for_loop_unfixable.rs b/tests/ui/for_loop_unfixable.rs index 179b255e08c..e73536052f0 100644 --- a/tests/ui/for_loop_unfixable.rs +++ b/tests/ui/for_loop_unfixable.rs @@ -5,7 +5,6 @@ clippy::explicit_iter_loop, clippy::explicit_into_iter_loop, clippy::iter_next_loop, - clippy::reverse_range_loop, clippy::for_kv_map )] #[allow( @@ -16,25 +15,8 @@ unused, dead_code )] -#[allow(clippy::many_single_char_names, unused_variables)] fn main() { - for i in 5..5 { - println!("{}", i); - } - let vec = vec![1, 2, 3, 4]; for _v in vec.iter().next() {} - - for i in (5 + 2)..(8 - 1) { - println!("{}", i); - } - - const ZERO: usize = 0; - - for i in ZERO..vec.len() { - if f(&vec[i], &vec[i]) { - panic!("at the disco"); - } - } } diff --git a/tests/ui/for_loop_unfixable.stderr b/tests/ui/for_loop_unfixable.stderr index 1da8e0f3588..1c9287b6acb 100644 --- a/tests/ui/for_loop_unfixable.stderr +++ b/tests/ui/for_loop_unfixable.stderr @@ -1,9 +1,10 @@ -error[E0425]: cannot find function `f` in this scope - --> $DIR/for_loop_unfixable.rs:36:12 +error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want + --> $DIR/for_loop_unfixable.rs:21:15 | -LL | if f(&vec[i], &vec[i]) { - | ^ help: a local variable with a similar name exists: `i` +LL | for _v in vec.iter().next() {} + | ^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::iter-next-loop` implied by `-D warnings` error: aborting due to previous error -For more information about this error, try `rustc --explain E0425`. diff --git a/tests/ui/for_loop_over_option_result.rs b/tests/ui/for_loops_over_fallibles.rs similarity index 81% rename from tests/ui/for_loop_over_option_result.rs rename to tests/ui/for_loops_over_fallibles.rs index 6b207b26b6b..1b9dde87cd5 100644 --- a/tests/ui/for_loop_over_option_result.rs +++ b/tests/ui/for_loops_over_fallibles.rs @@ -1,18 +1,16 @@ -#![warn(clippy::for_loop_over_option, clippy::for_loop_over_result)] +#![warn(clippy::for_loops_over_fallibles)] -/// Tests for_loop_over_result and for_loop_over_option - -fn for_loop_over_option_and_result() { +fn for_loops_over_fallibles() { let option = Some(1); let result = option.ok_or("x not found"); let v = vec![0, 1, 2]; - // check FOR_LOOP_OVER_OPTION lint + // check over an `Option` for x in option { println!("{}", x); } - // check FOR_LOOP_OVER_RESULT lint + // check over a `Result` for x in result { println!("{}", x); } diff --git a/tests/ui/for_loop_over_option_result.stderr b/tests/ui/for_loops_over_fallibles.stderr similarity index 81% rename from tests/ui/for_loop_over_option_result.stderr rename to tests/ui/for_loops_over_fallibles.stderr index 194a0bfec5b..bef228d4b93 100644 --- a/tests/ui/for_loop_over_option_result.stderr +++ b/tests/ui/for_loops_over_fallibles.stderr @@ -1,23 +1,22 @@ error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement. - --> $DIR/for_loop_over_option_result.rs:11:14 + --> $DIR/for_loops_over_fallibles.rs:9:14 | LL | for x in option { | ^^^^^^ | - = note: `-D clippy::for-loop-over-option` implied by `-D warnings` + = note: `-D clippy::for-loops-over-fallibles` implied by `-D warnings` = help: consider replacing `for x in option` with `if let Some(x) = option` error: for loop over `result`, which is a `Result`. This is more readably written as an `if let` statement. - --> $DIR/for_loop_over_option_result.rs:16:14 + --> $DIR/for_loops_over_fallibles.rs:14:14 | LL | for x in result { | ^^^^^^ | - = note: `-D clippy::for-loop-over-result` implied by `-D warnings` = help: consider replacing `for x in result` with `if let Ok(x) = result` error: for loop over `option.ok_or("x not found")`, which is a `Result`. This is more readably written as an `if let` statement. - --> $DIR/for_loop_over_option_result.rs:20:14 + --> $DIR/for_loops_over_fallibles.rs:18:14 | LL | for x in option.ok_or("x not found") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -25,7 +24,7 @@ LL | for x in option.ok_or("x not found") { = help: consider replacing `for x in option.ok_or("x not found")` with `if let Ok(x) = option.ok_or("x not found")` error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want - --> $DIR/for_loop_over_option_result.rs:26:14 + --> $DIR/for_loops_over_fallibles.rs:24:14 | LL | for x in v.iter().next() { | ^^^^^^^^^^^^^^^ @@ -33,7 +32,7 @@ LL | for x in v.iter().next() { = note: `#[deny(clippy::iter_next_loop)]` on by default error: for loop over `v.iter().next().and(Some(0))`, which is an `Option`. This is more readably written as an `if let` statement. - --> $DIR/for_loop_over_option_result.rs:31:14 + --> $DIR/for_loops_over_fallibles.rs:29:14 | LL | for x in v.iter().next().and(Some(0)) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -41,7 +40,7 @@ LL | for x in v.iter().next().and(Some(0)) { = help: consider replacing `for x in v.iter().next().and(Some(0))` with `if let Some(x) = v.iter().next().and(Some(0))` error: for loop over `v.iter().next().ok_or("x not found")`, which is a `Result`. This is more readably written as an `if let` statement. - --> $DIR/for_loop_over_option_result.rs:35:14 + --> $DIR/for_loops_over_fallibles.rs:33:14 | LL | for x in v.iter().next().ok_or("x not found") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -49,7 +48,7 @@ LL | for x in v.iter().next().ok_or("x not found") { = help: consider replacing `for x in v.iter().next().ok_or("x not found")` with `if let Ok(x) = v.iter().next().ok_or("x not found")` error: this loop never actually loops - --> $DIR/for_loop_over_option_result.rs:47:5 + --> $DIR/for_loops_over_fallibles.rs:45:5 | LL | / while let Some(x) = option { LL | | println!("{}", x); @@ -60,7 +59,7 @@ LL | | } = note: `#[deny(clippy::never_loop)]` on by default error: this loop never actually loops - --> $DIR/for_loop_over_option_result.rs:53:5 + --> $DIR/for_loops_over_fallibles.rs:51:5 | LL | / while let Ok(x) = result { LL | | println!("{}", x); diff --git a/tests/ui/identity_op.rs b/tests/ui/identity_op.rs index ae2815d345a..ceaacaaf6bd 100644 --- a/tests/ui/identity_op.rs +++ b/tests/ui/identity_op.rs @@ -33,4 +33,9 @@ fn main() { let u: u8 = 0; u & 255; + + 1 << 0; // no error, this case is allowed, see issue 3430 + 42 << 0; + 1 >> 0; + 42 >> 0; } diff --git a/tests/ui/identity_op.stderr b/tests/ui/identity_op.stderr index 4742877706a..d8d44a74f9a 100644 --- a/tests/ui/identity_op.stderr +++ b/tests/ui/identity_op.stderr @@ -48,5 +48,23 @@ error: the operation is ineffective. Consider reducing it to `u` LL | u & 255; | ^^^^^^^ -error: aborting due to 8 previous errors +error: the operation is ineffective. Consider reducing it to `42` + --> $DIR/identity_op.rs:38:5 + | +LL | 42 << 0; + | ^^^^^^^ + +error: the operation is ineffective. Consider reducing it to `1` + --> $DIR/identity_op.rs:39:5 + | +LL | 1 >> 0; + | ^^^^^^ + +error: the operation is ineffective. Consider reducing it to `42` + --> $DIR/identity_op.rs:40:5 + | +LL | 42 >> 0; + | ^^^^^^^ + +error: aborting due to 11 previous errors diff --git a/tests/ui/implicit_saturating_sub.fixed b/tests/ui/implicit_saturating_sub.fixed index e0b5b31a00c..859765d08a7 100644 --- a/tests/ui/implicit_saturating_sub.fixed +++ b/tests/ui/implicit_saturating_sub.fixed @@ -29,8 +29,8 @@ fn main() { // Lint u_16 = u_16.saturating_sub(1); - let mut end_32: u32 = 7000; - let mut start_32: u32 = 7010; + let mut end_32: u32 = 7010; + let mut start_32: u32 = 7000; let mut u_32: u32 = end_32 - start_32; diff --git a/tests/ui/implicit_saturating_sub.rs b/tests/ui/implicit_saturating_sub.rs index 39d81608922..24cb216e79b 100644 --- a/tests/ui/implicit_saturating_sub.rs +++ b/tests/ui/implicit_saturating_sub.rs @@ -35,8 +35,8 @@ fn main() { u_16 -= 1; } - let mut end_32: u32 = 7000; - let mut start_32: u32 = 7010; + let mut end_32: u32 = 7010; + let mut start_32: u32 = 7000; let mut u_32: u32 = end_32 - start_32; diff --git a/tests/ui/manual_memcpy.rs b/tests/ui/manual_memcpy.rs index 9c24d6d4db1..0083f94798f 100644 --- a/tests/ui/manual_memcpy.rs +++ b/tests/ui/manual_memcpy.rs @@ -104,7 +104,7 @@ pub fn manual_copy(src: &[i32], dst: &mut [i32], dst2: &mut [i32]) { dst[i - 0] = src[i]; } - #[allow(clippy::reverse_range_loop)] + #[allow(clippy::reversed_empty_ranges)] for i in 0..0 { dst[i] = src[i]; } diff --git a/tests/ui/option_map_unwrap_or.rs b/tests/ui/map_unwrap_or.rs similarity index 75% rename from tests/ui/option_map_unwrap_or.rs rename to tests/ui/map_unwrap_or.rs index 0364d83663a..585944032e7 100644 --- a/tests/ui/option_map_unwrap_or.rs +++ b/tests/ui/map_unwrap_or.rs @@ -1,21 +1,18 @@ // FIXME: Add "run-rustfix" once it's supported for multipart suggestions // aux-build:option_helpers.rs -#![warn(clippy::option_map_unwrap_or, clippy::option_map_unwrap_or_else)] +#![warn(clippy::map_unwrap_or)] #[macro_use] extern crate option_helpers; use std::collections::HashMap; -/// Checks implementation of the following lints: -/// * `OPTION_MAP_UNWRAP_OR` -/// * `OPTION_MAP_UNWRAP_OR_ELSE` #[rustfmt::skip] fn option_methods() { let opt = Some(1); - // Check `OPTION_MAP_UNWRAP_OR`. + // Check for `option.map(_).unwrap_or(_)` use. // Single line case. let _ = opt.map(|x| x + 1) // Should lint even though this call is on a separate line. @@ -49,7 +46,7 @@ fn option_methods() { let id: String = "identifier".to_string(); let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); - // Check OPTION_MAP_UNWRAP_OR_ELSE + // Check for `option.map(_).unwrap_or_else(_)` use. // single line case let _ = opt.map(|x| x + 1) // Should lint even though this call is on a separate line. @@ -83,6 +80,20 @@ fn option_methods() { } } +fn result_methods() { + let res: Result = Ok(1); + + // Check for `result.map(_).unwrap_or_else(_)` use. + // single line case + let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); // should lint even though this call is on a separate line + // multi line cases + let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); + let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); + // macro case + let _ = opt_map!(res, |x| x + 1).unwrap_or_else(|e| 0); // should not lint +} + fn main() { option_methods(); + result_methods(); } diff --git a/tests/ui/option_map_unwrap_or.stderr b/tests/ui/map_unwrap_or.stderr similarity index 70% rename from tests/ui/option_map_unwrap_or.stderr rename to tests/ui/map_unwrap_or.stderr index f05f2893de2..b62080a073f 100644 --- a/tests/ui/option_map_unwrap_or.stderr +++ b/tests/ui/map_unwrap_or.stderr @@ -1,5 +1,5 @@ error: called `map(f).unwrap_or(a)` on an `Option` value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/option_map_unwrap_or.rs:20:13 + --> $DIR/map_unwrap_or.rs:17:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -7,14 +7,14 @@ LL | | // Should lint even though this call is on a separate line. LL | | .unwrap_or(0); | |_____________________^ | - = note: `-D clippy::option-map-unwrap-or` implied by `-D warnings` + = note: `-D clippy::map-unwrap-or` implied by `-D warnings` help: use `map_or(a, f)` instead | LL | let _ = opt.map_or(0, |x| x + 1); | ^^^^^^ ^^ -- error: called `map(f).unwrap_or(a)` on an `Option` value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/option_map_unwrap_or.rs:24:13 + --> $DIR/map_unwrap_or.rs:21:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -32,7 +32,7 @@ LL | ); | error: called `map(f).unwrap_or(a)` on an `Option` value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/option_map_unwrap_or.rs:28:13 + --> $DIR/map_unwrap_or.rs:25:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -49,7 +49,7 @@ LL | }, |x| x + 1); | error: called `map(f).unwrap_or(None)` on an `Option` value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/option_map_unwrap_or.rs:33:13 + --> $DIR/map_unwrap_or.rs:30:13 | LL | let _ = opt.map(|x| Some(x + 1)).unwrap_or(None); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -60,7 +60,7 @@ LL | let _ = opt.and_then(|x| Some(x + 1)); | ^^^^^^^^ -- error: called `map(f).unwrap_or(None)` on an `Option` value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/option_map_unwrap_or.rs:35:13 + --> $DIR/map_unwrap_or.rs:32:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -78,7 +78,7 @@ LL | ); | error: called `map(f).unwrap_or(None)` on an `Option` value. This can be done more directly by calling `and_then(f)` instead - --> $DIR/option_map_unwrap_or.rs:39:13 + --> $DIR/map_unwrap_or.rs:36:13 | LL | let _ = opt | _____________^ @@ -92,7 +92,7 @@ LL | .and_then(|x| Some(x + 1)); | ^^^^^^^^ -- error: called `map(f).unwrap_or(a)` on an `Option` value. This can be done more directly by calling `map_or(a, f)` instead - --> $DIR/option_map_unwrap_or.rs:50:13 + --> $DIR/map_unwrap_or.rs:47:13 | LL | let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -103,7 +103,7 @@ LL | let _ = Some("prefix").map_or(id, |p| format!("{}.", p)); | ^^^^^^ ^^^ -- error: called `map(f).unwrap_or_else(g)` on an `Option` value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/option_map_unwrap_or.rs:54:13 + --> $DIR/map_unwrap_or.rs:51:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -111,11 +111,10 @@ LL | | // Should lint even though this call is on a separate line. LL | | .unwrap_or_else(|| 0); | |_____________________________^ | - = note: `-D clippy::option-map-unwrap-or-else` implied by `-D warnings` = note: replace `map(|x| x + 1).unwrap_or_else(|| 0)` with `map_or_else(|| 0, |x| x + 1)` error: called `map(f).unwrap_or_else(g)` on an `Option` value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/option_map_unwrap_or.rs:58:13 + --> $DIR/map_unwrap_or.rs:55:13 | LL | let _ = opt.map(|x| { | _____________^ @@ -125,7 +124,7 @@ LL | | ).unwrap_or_else(|| 0); | |__________________________^ error: called `map(f).unwrap_or_else(g)` on an `Option` value. This can be done more directly by calling `map_or_else(g, f)` instead - --> $DIR/option_map_unwrap_or.rs:62:13 + --> $DIR/map_unwrap_or.rs:59:13 | LL | let _ = opt.map(|x| x + 1) | _____________^ @@ -134,5 +133,29 @@ LL | | 0 LL | | ); | |_________^ -error: aborting due to 10 previous errors +error: called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling `.map_or_else(g, f)` instead + --> $DIR/map_unwrap_or.rs:88:13 + | +LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); // should lint even though this call is on a separate line + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)` + +error: called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling `.map_or_else(g, f)` instead + --> $DIR/map_unwrap_or.rs:90:13 + | +LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)` + +error: called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling `.map_or_else(g, f)` instead + --> $DIR/map_unwrap_or.rs:91:13 + | +LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)` + +error: aborting due to 13 previous errors diff --git a/tests/ui/option_map_or_none.fixed b/tests/ui/option_map_or_none.fixed index decbae4e5af..d80c3c7c1b7 100644 --- a/tests/ui/option_map_or_none.fixed +++ b/tests/ui/option_map_or_none.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::option_and_then_some)] +#![allow(clippy::bind_instead_of_map)] fn main() { let opt = Some(1); diff --git a/tests/ui/option_map_or_none.rs b/tests/ui/option_map_or_none.rs index 0f1d2218d5d..629842419e5 100644 --- a/tests/ui/option_map_or_none.rs +++ b/tests/ui/option_map_or_none.rs @@ -1,6 +1,6 @@ // run-rustfix -#![allow(clippy::option_and_then_some)] +#![allow(clippy::bind_instead_of_map)] fn main() { let opt = Some(1); diff --git a/tests/ui/result_map_unwrap_or_else.rs b/tests/ui/result_map_unwrap_or_else.rs deleted file mode 100644 index 40751bfebe6..00000000000 --- a/tests/ui/result_map_unwrap_or_else.rs +++ /dev/null @@ -1,23 +0,0 @@ -// aux-build:option_helpers.rs - -//! Checks implementation of `RESULT_MAP_UNWRAP_OR_ELSE` - -#![warn(clippy::result_map_unwrap_or_else)] - -#[macro_use] -extern crate option_helpers; - -fn result_methods() { - let res: Result = Ok(1); - - // Check RESULT_MAP_UNWRAP_OR_ELSE - // single line case - let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); // should lint even though this call is on a separate line - // multi line cases - let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); - let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); - // macro case - let _ = opt_map!(res, |x| x + 1).unwrap_or_else(|e| 0); // should not lint -} - -fn main() {} diff --git a/tests/ui/result_map_unwrap_or_else.stderr b/tests/ui/result_map_unwrap_or_else.stderr deleted file mode 100644 index ec7bc8f1241..00000000000 --- a/tests/ui/result_map_unwrap_or_else.stderr +++ /dev/null @@ -1,27 +0,0 @@ -error: called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling `.map_or_else(g, f)` instead - --> $DIR/result_map_unwrap_or_else.rs:15:13 - | -LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); // should lint even though this call is on a separate line - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::result-map-unwrap-or-else` implied by `-D warnings` - = note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)` - -error: called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling `.map_or_else(g, f)` instead - --> $DIR/result_map_unwrap_or_else.rs:17:13 - | -LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)` - -error: called `map(f).unwrap_or_else(g)` on a `Result` value. This can be done more directly by calling `.map_or_else(g, f)` instead - --> $DIR/result_map_unwrap_or_else.rs:18:13 - | -LL | let _ = res.map(|x| x + 1).unwrap_or_else(|e| 0); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: replace `map(|x| x + 1).unwrap_or_else(|e| 0)` with `map_or_else(|e| 0, |x| x + 1)` - -error: aborting due to 3 previous errors - diff --git a/tests/ui/reversed_empty_ranges_fixable.fixed b/tests/ui/reversed_empty_ranges_fixable.fixed new file mode 100644 index 00000000000..ee2cbc3cf54 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_fixable.fixed @@ -0,0 +1,24 @@ +// run-rustfix +#![warn(clippy::reversed_empty_ranges)] + +const ANSWER: i32 = 42; + +fn main() { + (21..=42).rev().for_each(|x| println!("{}", x)); + let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::>(); + + for _ in (-42..=-21).rev() {} + for _ in (21u32..42u32).rev() {} + + // These should be ignored as they are not empty ranges: + + (21..=42).for_each(|x| println!("{}", x)); + (21..42).for_each(|x| println!("{}", x)); + + let arr = [1, 2, 3, 4, 5]; + let _ = &arr[1..=3]; + let _ = &arr[1..3]; + + for _ in 21..=42 {} + for _ in 21..42 {} +} diff --git a/tests/ui/reversed_empty_ranges_fixable.rs b/tests/ui/reversed_empty_ranges_fixable.rs new file mode 100644 index 00000000000..6ed5ca6daa0 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_fixable.rs @@ -0,0 +1,24 @@ +// run-rustfix +#![warn(clippy::reversed_empty_ranges)] + +const ANSWER: i32 = 42; + +fn main() { + (42..=21).for_each(|x| println!("{}", x)); + let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::>(); + + for _ in -21..=-42 {} + for _ in 42u32..21u32 {} + + // These should be ignored as they are not empty ranges: + + (21..=42).for_each(|x| println!("{}", x)); + (21..42).for_each(|x| println!("{}", x)); + + let arr = [1, 2, 3, 4, 5]; + let _ = &arr[1..=3]; + let _ = &arr[1..3]; + + for _ in 21..=42 {} + for _ in 21..42 {} +} diff --git a/tests/ui/reversed_empty_ranges_fixable.stderr b/tests/ui/reversed_empty_ranges_fixable.stderr new file mode 100644 index 00000000000..97933b8ff85 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_fixable.stderr @@ -0,0 +1,47 @@ +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_fixable.rs:7:5 + | +LL | (42..=21).for_each(|x| println!("{}", x)); + | ^^^^^^^^^ + | + = note: `-D clippy::reversed-empty-ranges` implied by `-D warnings` +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | (21..=42).rev().for_each(|x| println!("{}", x)); + | ^^^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_fixable.rs:8:13 + | +LL | let _ = (ANSWER..21).filter(|x| x % 2 == 0).take(10).collect::>(); + | ^^^^^^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | let _ = (21..ANSWER).rev().filter(|x| x % 2 == 0).take(10).collect::>(); + | ^^^^^^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_fixable.rs:10:14 + | +LL | for _ in -21..=-42 {} + | ^^^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for _ in (-42..=-21).rev() {} + | ^^^^^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_fixable.rs:11:14 + | +LL | for _ in 42u32..21u32 {} + | ^^^^^^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for _ in (21u32..42u32).rev() {} + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors + diff --git a/tests/ui/reversed_empty_ranges_loops_fixable.fixed b/tests/ui/reversed_empty_ranges_loops_fixable.fixed new file mode 100644 index 00000000000..f1503ed6d12 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_loops_fixable.fixed @@ -0,0 +1,57 @@ +// run-rustfix +#![warn(clippy::reversed_empty_ranges)] + +fn main() { + const MAX_LEN: usize = 42; + + for i in (0..10).rev() { + println!("{}", i); + } + + for i in (0..=10).rev() { + println!("{}", i); + } + + for i in (0..MAX_LEN).rev() { + println!("{}", i); + } + + for i in 5..=5 { + // not an error, this is the range with only one element “5” + println!("{}", i); + } + + for i in 0..10 { + // not an error, the start index is less than the end index + println!("{}", i); + } + + for i in -10..0 { + // not an error + println!("{}", i); + } + + for i in (0..10).rev().map(|x| x * 2) { + println!("{}", i); + } + + // testing that the empty range lint folds constants + for i in (5 + 4..10).rev() { + println!("{}", i); + } + + for i in ((3 - 1)..(5 + 2)).rev() { + println!("{}", i); + } + + for i in (2 * 2)..(2 * 3) { + // no error, 4..6 is fine + println!("{}", i); + } + + let x = 42; + for i in x..10 { + // no error, not constant-foldable + println!("{}", i); + } +} diff --git a/tests/ui/reversed_empty_ranges_loops_fixable.rs b/tests/ui/reversed_empty_ranges_loops_fixable.rs new file mode 100644 index 00000000000..a733788dc22 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_loops_fixable.rs @@ -0,0 +1,57 @@ +// run-rustfix +#![warn(clippy::reversed_empty_ranges)] + +fn main() { + const MAX_LEN: usize = 42; + + for i in 10..0 { + println!("{}", i); + } + + for i in 10..=0 { + println!("{}", i); + } + + for i in MAX_LEN..0 { + println!("{}", i); + } + + for i in 5..=5 { + // not an error, this is the range with only one element “5” + println!("{}", i); + } + + for i in 0..10 { + // not an error, the start index is less than the end index + println!("{}", i); + } + + for i in -10..0 { + // not an error + println!("{}", i); + } + + for i in (10..0).map(|x| x * 2) { + println!("{}", i); + } + + // testing that the empty range lint folds constants + for i in 10..5 + 4 { + println!("{}", i); + } + + for i in (5 + 2)..(3 - 1) { + println!("{}", i); + } + + for i in (2 * 2)..(2 * 3) { + // no error, 4..6 is fine + println!("{}", i); + } + + let x = 42; + for i in x..10 { + // no error, not constant-foldable + println!("{}", i); + } +} diff --git a/tests/ui/reversed_empty_ranges_loops_fixable.stderr b/tests/ui/reversed_empty_ranges_loops_fixable.stderr new file mode 100644 index 00000000000..e89e040a0ff --- /dev/null +++ b/tests/ui/reversed_empty_ranges_loops_fixable.stderr @@ -0,0 +1,69 @@ +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_fixable.rs:7:14 + | +LL | for i in 10..0 { + | ^^^^^ + | + = note: `-D clippy::reversed-empty-ranges` implied by `-D warnings` +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for i in (0..10).rev() { + | ^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_fixable.rs:11:14 + | +LL | for i in 10..=0 { + | ^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for i in (0..=10).rev() { + | ^^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_fixable.rs:15:14 + | +LL | for i in MAX_LEN..0 { + | ^^^^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for i in (0..MAX_LEN).rev() { + | ^^^^^^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_fixable.rs:34:14 + | +LL | for i in (10..0).map(|x| x * 2) { + | ^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for i in (0..10).rev().map(|x| x * 2) { + | ^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_fixable.rs:39:14 + | +LL | for i in 10..5 + 4 { + | ^^^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for i in (5 + 4..10).rev() { + | ^^^^^^^^^^^^^^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_fixable.rs:43:14 + | +LL | for i in (5 + 2)..(3 - 1) { + | ^^^^^^^^^^^^^^^^ + | +help: consider using the following if you are attempting to iterate over this range in reverse + | +LL | for i in ((3 - 1)..(5 + 2)).rev() { + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + diff --git a/tests/ui/reversed_empty_ranges_loops_unfixable.rs b/tests/ui/reversed_empty_ranges_loops_unfixable.rs new file mode 100644 index 00000000000..c4c57224416 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_loops_unfixable.rs @@ -0,0 +1,11 @@ +#![warn(clippy::reversed_empty_ranges)] + +fn main() { + for i in 5..5 { + println!("{}", i); + } + + for i in (5 + 2)..(8 - 1) { + println!("{}", i); + } +} diff --git a/tests/ui/reversed_empty_ranges_loops_unfixable.stderr b/tests/ui/reversed_empty_ranges_loops_unfixable.stderr new file mode 100644 index 00000000000..30095d20cfd --- /dev/null +++ b/tests/ui/reversed_empty_ranges_loops_unfixable.stderr @@ -0,0 +1,16 @@ +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_unfixable.rs:4:14 + | +LL | for i in 5..5 { + | ^^^^ + | + = note: `-D clippy::reversed-empty-ranges` implied by `-D warnings` + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_loops_unfixable.rs:8:14 + | +LL | for i in (5 + 2)..(8 - 1) { + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + diff --git a/tests/ui/reversed_empty_ranges_unfixable.rs b/tests/ui/reversed_empty_ranges_unfixable.rs new file mode 100644 index 00000000000..c9ca4c47668 --- /dev/null +++ b/tests/ui/reversed_empty_ranges_unfixable.rs @@ -0,0 +1,15 @@ +#![warn(clippy::reversed_empty_ranges)] + +const ANSWER: i32 = 42; +const SOME_NUM: usize = 3; + +fn main() { + let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21); + + let arr = [1, 2, 3, 4, 5]; + let _ = &arr[3usize..=1usize]; + let _ = &arr[SOME_NUM..1]; + let _ = &arr[3..3]; + + for _ in ANSWER..ANSWER {} +} diff --git a/tests/ui/reversed_empty_ranges_unfixable.stderr b/tests/ui/reversed_empty_ranges_unfixable.stderr new file mode 100644 index 00000000000..12e5483ecdf --- /dev/null +++ b/tests/ui/reversed_empty_ranges_unfixable.stderr @@ -0,0 +1,34 @@ +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_unfixable.rs:7:13 + | +LL | let _ = (42 + 10..42 + 10).map(|x| x / 2).find(|&x| x == 21); + | ^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::reversed-empty-ranges` implied by `-D warnings` + +error: this range is reversed and using it to index a slice will panic at run-time + --> $DIR/reversed_empty_ranges_unfixable.rs:10:18 + | +LL | let _ = &arr[3usize..=1usize]; + | ^^^^^^^^^^^^^^^ + +error: this range is reversed and using it to index a slice will panic at run-time + --> $DIR/reversed_empty_ranges_unfixable.rs:11:18 + | +LL | let _ = &arr[SOME_NUM..1]; + | ^^^^^^^^^^^ + +error: this range is empty and using it to index a slice will always yield an empty slice + --> $DIR/reversed_empty_ranges_unfixable.rs:12:18 + | +LL | let _ = &arr[3..3]; + | ^^^^ + +error: this range is empty so it will yield no values + --> $DIR/reversed_empty_ranges_unfixable.rs:14:14 + | +LL | for _ in ANSWER..ANSWER {} + | ^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors + diff --git a/tests/ui/unused_unit.fixed b/tests/ui/unused_unit.fixed index 3f63624720f..07f2791786d 100644 --- a/tests/ui/unused_unit.fixed +++ b/tests/ui/unused_unit.fixed @@ -14,11 +14,10 @@ struct Unitter; impl Unitter { - // try to disorient the lint with multiple unit returns and newlines #[allow(clippy::no_effect)] - pub fn get_unit (), G>(&self, f: F, _g: G) - where G: Fn() -> () { - let _y: &dyn Fn() -> () = &f; + pub fn get_unit(&self, f: F, _g: G) + where G: Fn() { + let _y: &dyn Fn() = &f; (); // this should not lint, as it's not in return type position } } @@ -30,6 +29,20 @@ impl Into<()> for Unitter { } } +trait Trait { + fn redundant(&self, _f: F, _g: G, _h: H) + where + G: FnMut() , + H: Fn() ; +} + +impl Trait for Unitter { + fn redundant(&self, _f: F, _g: G, _h: H) + where + G: FnMut() , + H: Fn() {} +} + fn return_unit() { } #[allow(clippy::needless_return)] diff --git a/tests/ui/unused_unit.rs b/tests/ui/unused_unit.rs index 8fc072ebd69..e2c6afb020f 100644 --- a/tests/ui/unused_unit.rs +++ b/tests/ui/unused_unit.rs @@ -14,10 +14,8 @@ struct Unitter; impl Unitter { - // try to disorient the lint with multiple unit returns and newlines #[allow(clippy::no_effect)] - pub fn get_unit (), G>(&self, f: F, _g: G) -> - () + pub fn get_unit (), G>(&self, f: F, _g: G) -> () where G: Fn() -> () { let _y: &dyn Fn() -> () = &f; (); // this should not lint, as it's not in return type position @@ -31,6 +29,20 @@ impl Into<()> for Unitter { } } +trait Trait { + fn redundant (), G, H>(&self, _f: F, _g: G, _h: H) + where + G: FnMut() -> (), + H: Fn() -> (); +} + +impl Trait for Unitter { + fn redundant (), G, H>(&self, _f: F, _g: G, _h: H) + where + G: FnMut() -> (), + H: Fn() -> () {} +} + fn return_unit() -> () { () } #[allow(clippy::needless_return)] diff --git a/tests/ui/unused_unit.stderr b/tests/ui/unused_unit.stderr index a013d2b3495..81e6738e6bf 100644 --- a/tests/ui/unused_unit.stderr +++ b/tests/ui/unused_unit.stderr @@ -1,10 +1,8 @@ error: unneeded unit return type - --> $DIR/unused_unit.rs:19:59 + --> $DIR/unused_unit.rs:18:29 | -LL | pub fn get_unit (), G>(&self, f: F, _g: G) -> - | ___________________________________________________________^ -LL | | () - | |__________^ help: remove the `-> ()` +LL | pub fn get_unit (), G>(&self, f: F, _g: G) -> () + | ^^^^^ help: remove the `-> ()` | note: the lint level is defined here --> $DIR/unused_unit.rs:12:9 @@ -13,40 +11,94 @@ LL | #![deny(clippy::unused_unit)] | ^^^^^^^^^^^^^^^^^^^ error: unneeded unit return type - --> $DIR/unused_unit.rs:29:19 + --> $DIR/unused_unit.rs:19:19 + | +LL | where G: Fn() -> () { + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:18:59 + | +LL | pub fn get_unit (), G>(&self, f: F, _g: G) -> () + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:20:27 + | +LL | let _y: &dyn Fn() -> () = &f; + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:27:19 | LL | fn into(self) -> () { | ^^^^^ help: remove the `-> ()` error: unneeded unit expression - --> $DIR/unused_unit.rs:30:9 + --> $DIR/unused_unit.rs:28:9 | LL | () | ^^ help: remove the final `()` error: unneeded unit return type - --> $DIR/unused_unit.rs:34:18 + --> $DIR/unused_unit.rs:33:30 + | +LL | fn redundant (), G, H>(&self, _f: F, _g: G, _h: H) + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:35:20 + | +LL | G: FnMut() -> (), + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:36:17 + | +LL | H: Fn() -> (); + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:40:30 + | +LL | fn redundant (), G, H>(&self, _f: F, _g: G, _h: H) + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:42:20 + | +LL | G: FnMut() -> (), + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:43:17 + | +LL | H: Fn() -> () {} + | ^^^^^ help: remove the `-> ()` + +error: unneeded unit return type + --> $DIR/unused_unit.rs:46:18 | LL | fn return_unit() -> () { () } | ^^^^^ help: remove the `-> ()` error: unneeded unit expression - --> $DIR/unused_unit.rs:34:26 + --> $DIR/unused_unit.rs:46:26 | LL | fn return_unit() -> () { () } | ^^ help: remove the final `()` error: unneeded `()` - --> $DIR/unused_unit.rs:44:14 + --> $DIR/unused_unit.rs:56:14 | LL | break(); | ^^ help: remove the `()` error: unneeded `()` - --> $DIR/unused_unit.rs:46:11 + --> $DIR/unused_unit.rs:58:11 | LL | return(); | ^^ help: remove the `()` -error: aborting due to 7 previous errors +error: aborting due to 16 previous errors diff --git a/tests/ui/unwrap.rs b/tests/ui/unwrap.rs index fcd1fcd14d4..a4a3cd1d379 100644 --- a/tests/ui/unwrap.rs +++ b/tests/ui/unwrap.rs @@ -1,4 +1,4 @@ -#![warn(clippy::option_unwrap_used, clippy::result_unwrap_used)] +#![warn(clippy::unwrap_used)] fn unwrap_option() { let opt = Some(0); diff --git a/tests/ui/unwrap.stderr b/tests/ui/unwrap.stderr index b90ce68fa97..4f0858005f6 100644 --- a/tests/ui/unwrap.stderr +++ b/tests/ui/unwrap.stderr @@ -4,7 +4,7 @@ error: used `unwrap()` on `an Option` value LL | let _ = opt.unwrap(); | ^^^^^^^^^^^^ | - = note: `-D clippy::option-unwrap-used` implied by `-D warnings` + = note: `-D clippy::unwrap-used` implied by `-D warnings` = help: if you don't want to handle the `None` case gracefully, consider using `expect()` to provide a better panic message error: used `unwrap()` on `a Result` value @@ -13,7 +13,6 @@ error: used `unwrap()` on `a Result` value LL | let _ = res.unwrap(); | ^^^^^^^^^^^^ | - = note: `-D clippy::result-unwrap-used` implied by `-D warnings` = help: if you don't want to handle the `Err` case gracefully, consider using `expect()` to provide a better panic message error: aborting due to 2 previous errors diff --git a/tests/ui/identity_conversion.fixed b/tests/ui/useless_conversion.fixed similarity index 93% rename from tests/ui/identity_conversion.fixed rename to tests/ui/useless_conversion.fixed index dd3fc56e98b..fdd4bc581f3 100644 --- a/tests/ui/identity_conversion.fixed +++ b/tests/ui/useless_conversion.fixed @@ -1,6 +1,6 @@ // run-rustfix -#![deny(clippy::identity_conversion)] +#![deny(clippy::useless_conversion)] fn test_generic(val: T) -> T { let _ = val; @@ -41,7 +41,7 @@ fn main() { let _: String = "foo".into(); let _: String = From::from("foo"); let _ = String::from("foo"); - #[allow(clippy::identity_conversion)] + #[allow(clippy::useless_conversion)] { let _: String = "foo".into(); let _ = String::from("foo"); diff --git a/tests/ui/identity_conversion.rs b/tests/ui/useless_conversion.rs similarity index 94% rename from tests/ui/identity_conversion.rs rename to tests/ui/useless_conversion.rs index 875ed7db373..4cae745e7c0 100644 --- a/tests/ui/identity_conversion.rs +++ b/tests/ui/useless_conversion.rs @@ -1,6 +1,6 @@ // run-rustfix -#![deny(clippy::identity_conversion)] +#![deny(clippy::useless_conversion)] fn test_generic(val: T) -> T { let _ = T::from(val); @@ -41,7 +41,7 @@ fn main() { let _: String = "foo".into(); let _: String = From::from("foo"); let _ = String::from("foo"); - #[allow(clippy::identity_conversion)] + #[allow(clippy::useless_conversion)] { let _: String = "foo".into(); let _ = String::from("foo"); diff --git a/tests/ui/identity_conversion.stderr b/tests/ui/useless_conversion.stderr similarity index 67% rename from tests/ui/identity_conversion.stderr rename to tests/ui/useless_conversion.stderr index 57626b23795..7df3507edfd 100644 --- a/tests/ui/identity_conversion.stderr +++ b/tests/ui/useless_conversion.stderr @@ -1,65 +1,65 @@ -error: identical conversion - --> $DIR/identity_conversion.rs:6:13 +error: useless conversion + --> $DIR/useless_conversion.rs:6:13 | LL | let _ = T::from(val); | ^^^^^^^^^^^^ help: consider removing `T::from()`: `val` | note: the lint level is defined here - --> $DIR/identity_conversion.rs:3:9 + --> $DIR/useless_conversion.rs:3:9 | -LL | #![deny(clippy::identity_conversion)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | #![deny(clippy::useless_conversion)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: identical conversion - --> $DIR/identity_conversion.rs:7:5 +error: useless conversion + --> $DIR/useless_conversion.rs:7:5 | LL | val.into() | ^^^^^^^^^^ help: consider removing `.into()`: `val` -error: identical conversion - --> $DIR/identity_conversion.rs:19:22 +error: useless conversion + --> $DIR/useless_conversion.rs:19:22 | LL | let _: i32 = 0i32.into(); | ^^^^^^^^^^^ help: consider removing `.into()`: `0i32` -error: identical conversion - --> $DIR/identity_conversion.rs:51:21 +error: useless conversion + --> $DIR/useless_conversion.rs:51:21 | LL | let _: String = "foo".to_string().into(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()` -error: identical conversion - --> $DIR/identity_conversion.rs:52:21 +error: useless conversion + --> $DIR/useless_conversion.rs:52:21 | LL | let _: String = From::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `From::from()`: `"foo".to_string()` -error: identical conversion - --> $DIR/identity_conversion.rs:53:13 +error: useless conversion + --> $DIR/useless_conversion.rs:53:13 | LL | let _ = String::from("foo".to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()` -error: identical conversion - --> $DIR/identity_conversion.rs:54:13 +error: useless conversion + --> $DIR/useless_conversion.rs:54:13 | LL | let _ = String::from(format!("A: {:04}", 123)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `format!("A: {:04}", 123)` -error: identical conversion - --> $DIR/identity_conversion.rs:55:13 +error: useless conversion + --> $DIR/useless_conversion.rs:55:13 | LL | let _ = "".lines().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()` -error: identical conversion - --> $DIR/identity_conversion.rs:56:13 +error: useless conversion + --> $DIR/useless_conversion.rs:56:13 | LL | let _ = vec![1, 2, 3].into_iter().into_iter(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `vec![1, 2, 3].into_iter()` -error: identical conversion - --> $DIR/identity_conversion.rs:57:21 +error: useless conversion + --> $DIR/useless_conversion.rs:57:21 | LL | let _: String = format!("Hello {}", "world").into(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")` From d25b25610b1ced7dda9c61c3ee7f6281be247bd7 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Wed, 6 May 2020 15:03:53 +0100 Subject: [PATCH 3/5] Handle InlineAsm in clippy --- clippy_lints/src/loops.rs | 18 ++++++++-- clippy_lints/src/utils/author.rs | 4 +++ clippy_lints/src/utils/hir_utils.rs | 55 +++++++++++++++++++++++++++-- clippy_lints/src/utils/inspector.rs | 27 +++++++++++++- clippy_lints/src/utils/sugg.rs | 2 ++ clippy_lints/src/write.rs | 5 +-- 6 files changed, 104 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 84e8a010738..38a5829b3f7 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -16,8 +16,8 @@ use rustc_errors::Applicability; use rustc_hir::def::{DefKind, Res}; use rustc_hir::intravisit::{walk_block, walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor}; use rustc_hir::{ - def_id, BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, LoopSource, - MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind, + def_id, BinOpKind, BindingAnnotation, Block, BorrowKind, Expr, ExprKind, GenericArg, HirId, InlineAsmOperand, + LoopSource, MatchSource, Mutability, Node, Pat, PatKind, QPath, Stmt, StmtKind, }; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -693,6 +693,20 @@ fn never_loop_expr(expr: &Expr<'_>, main_loop_id: HirId) -> NeverLoopResult { NeverLoopResult::AlwaysBreak } }, + ExprKind::InlineAsm(ref asm) => asm + .operands + .iter() + .map(|o| match o { + InlineAsmOperand::In { expr, .. } + | InlineAsmOperand::InOut { expr, .. } + | InlineAsmOperand::Const { expr } + | InlineAsmOperand::Sym { expr } => never_loop_expr(expr, main_loop_id), + InlineAsmOperand::Out { expr, .. } => never_loop_expr_all(&mut expr.iter(), main_loop_id), + InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => { + never_loop_expr_all(&mut once(in_expr).chain(out_expr.iter()), main_loop_id) + }, + }) + .fold(NeverLoopResult::Otherwise, combine_both), ExprKind::Struct(_, _, None) | ExprKind::Yield(_, _) | ExprKind::Closure(_, _, _, _, _) diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 74601008dca..bbcf396eef7 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -469,6 +469,10 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor { println!("Ret(None) = {};", current); } }, + ExprKind::InlineAsm(_) => { + println!("InlineAsm(_) = {};", current); + println!(" // unimplemented: `ExprKind::InlineAsm` is not further destructured at the moment"); + }, ExprKind::LlvmInlineAsm(_) => { println!("LlvmInlineAsm(_) = {};", current); println!(" // unimplemented: `ExprKind::LlvmInlineAsm` is not further destructured at the moment"); diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index bd7da57c665..92c27e79452 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -1,10 +1,11 @@ use crate::consts::{constant_context, constant_simple}; use crate::utils::differing_macro_contexts; +use rustc_ast::ast::InlineAsmTemplatePiece; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::{ BinOpKind, Block, BlockCheckMode, BodyId, BorrowKind, CaptureBy, Expr, ExprKind, Field, FnRetTy, GenericArg, - GenericArgs, Guard, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path, PathSegment, QPath, Stmt, StmtKind, Ty, - TyKind, TypeBinding, + GenericArgs, Guard, InlineAsmOperand, Lifetime, LifetimeName, ParamName, Pat, PatKind, Path, PathSegment, QPath, + Stmt, StmtKind, Ty, TyKind, TypeBinding, }; use rustc_lint::LateContext; use rustc_middle::ich::StableHashingContextProvider; @@ -474,6 +475,56 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_expr(a); self.hash_expr(i); }, + ExprKind::InlineAsm(ref asm) => { + for piece in asm.template { + match piece { + InlineAsmTemplatePiece::String(s) => s.hash(&mut self.s), + InlineAsmTemplatePiece::Placeholder { + operand_idx, + modifier, + span: _, + } => { + operand_idx.hash(&mut self.s); + modifier.hash(&mut self.s); + }, + } + } + asm.options.hash(&mut self.s); + for op in asm.operands { + match op { + InlineAsmOperand::In { reg, expr } => { + reg.hash(&mut self.s); + self.hash_expr(expr); + }, + InlineAsmOperand::Out { reg, late, expr } => { + reg.hash(&mut self.s); + late.hash(&mut self.s); + if let Some(expr) = expr { + self.hash_expr(expr); + } + }, + InlineAsmOperand::InOut { reg, late, expr } => { + reg.hash(&mut self.s); + late.hash(&mut self.s); + self.hash_expr(expr); + }, + InlineAsmOperand::SplitInOut { + reg, + late, + in_expr, + out_expr, + } => { + reg.hash(&mut self.s); + late.hash(&mut self.s); + self.hash_expr(in_expr); + if let Some(out_expr) = out_expr { + self.hash_expr(out_expr); + } + }, + InlineAsmOperand::Const { expr } | InlineAsmOperand::Sym { expr } => self.hash_expr(expr), + } + } + }, ExprKind::LlvmInlineAsm(..) | ExprKind::Err => {}, ExprKind::Lit(ref l) => { l.node.hash(&mut self.s); diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index 7e8c61ba24a..748c11fac64 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -1,7 +1,7 @@ //! checks for attributes use crate::utils::get_attr; -use rustc_ast::ast::Attribute; +use rustc_ast::ast::{Attribute, InlineAsmTemplatePiece}; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::Session; @@ -282,6 +282,31 @@ fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, indent: usize) { print_expr(cx, e, indent + 1); } }, + hir::ExprKind::InlineAsm(ref asm) => { + println!("{}InlineAsm", ind); + println!("{}template: {}", ind, InlineAsmTemplatePiece::to_string(asm.template)); + println!("{}options: {:?}", ind, asm.options); + println!("{}operands:", ind); + for op in asm.operands { + match op { + hir::InlineAsmOperand::In { expr, .. } => print_expr(cx, expr, indent + 1), + hir::InlineAsmOperand::Out { expr, .. } => { + if let Some(expr) = expr { + print_expr(cx, expr, indent + 1); + } + }, + hir::InlineAsmOperand::InOut { expr, .. } => print_expr(cx, expr, indent + 1), + hir::InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => { + print_expr(cx, in_expr, indent + 1); + if let Some(out_expr) = out_expr { + print_expr(cx, out_expr, indent + 1); + } + }, + hir::InlineAsmOperand::Const { expr } => print_expr(cx, expr, indent + 1), + hir::InlineAsmOperand::Sym { expr } => print_expr(cx, expr, indent + 1), + } + } + }, hir::ExprKind::LlvmInlineAsm(ref asm) => { let inputs = &asm.inputs_exprs; let outputs = &asm.outputs_exprs; diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index a8fe637d3d9..4ebe2e2852f 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -108,6 +108,7 @@ impl<'a> Sugg<'a> { | hir::ExprKind::Call(..) | hir::ExprKind::Field(..) | hir::ExprKind::Index(..) + | hir::ExprKind::InlineAsm(..) | hir::ExprKind::LlvmInlineAsm(..) | hir::ExprKind::Lit(..) | hir::ExprKind::Loop(..) @@ -150,6 +151,7 @@ impl<'a> Sugg<'a> { | ast::ExprKind::Field(..) | ast::ExprKind::ForLoop(..) | ast::ExprKind::Index(..) + | ast::ExprKind::InlineAsm(..) | ast::ExprKind::LlvmInlineAsm(..) | ast::ExprKind::Lit(..) | ast::ExprKind::Loop(..) diff --git a/clippy_lints/src/write.rs b/clippy_lints/src/write.rs index 26bf463bd29..dfa6223f1b9 100644 --- a/clippy_lints/src/write.rs +++ b/clippy_lints/src/write.rs @@ -353,7 +353,8 @@ impl Write { is_write: bool, ) -> (Option, Option) { use fmt_macros::{ - AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, Parser, Piece, + AlignUnknown, ArgumentImplicitlyIs, ArgumentIs, ArgumentNamed, CountImplied, FormatSpec, ParseMode, Parser, + Piece, }; let tts = tts.clone(); @@ -376,7 +377,7 @@ impl Write { }; let tmp = fmtstr.symbol.as_str(); let mut args = vec![]; - let mut fmt_parser = Parser::new(&tmp, None, Vec::new(), false); + let mut fmt_parser = Parser::new(&tmp, None, None, false, ParseMode::Format); while let Some(piece) = fmt_parser.next() { if !fmt_parser.errors.is_empty() { return (None, expr); From da9b138ec7645d483112c6b20e91ab595326c41d Mon Sep 17 00:00:00 2001 From: flip1995 Date: Tue, 19 May 2020 16:12:03 +0200 Subject: [PATCH 4/5] Update test after const_ptr functions are must_use now --- tests/ui/ptr_offset_with_cast.fixed | 12 ++++++------ tests/ui/ptr_offset_with_cast.rs | 12 ++++++------ tests/ui/ptr_offset_with_cast.stderr | 12 ++++++------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/ui/ptr_offset_with_cast.fixed b/tests/ui/ptr_offset_with_cast.fixed index ebdd6c4003d..718e391e8bf 100644 --- a/tests/ui/ptr_offset_with_cast.fixed +++ b/tests/ui/ptr_offset_with_cast.fixed @@ -9,12 +9,12 @@ fn main() { let offset_isize = 1_isize; unsafe { - ptr.add(offset_usize); - ptr.offset(offset_isize as isize); - ptr.offset(offset_u8 as isize); + let _ = ptr.add(offset_usize); + let _ = ptr.offset(offset_isize as isize); + let _ = ptr.offset(offset_u8 as isize); - ptr.wrapping_add(offset_usize); - ptr.wrapping_offset(offset_isize as isize); - ptr.wrapping_offset(offset_u8 as isize); + let _ = ptr.wrapping_add(offset_usize); + let _ = ptr.wrapping_offset(offset_isize as isize); + let _ = ptr.wrapping_offset(offset_u8 as isize); } } diff --git a/tests/ui/ptr_offset_with_cast.rs b/tests/ui/ptr_offset_with_cast.rs index 3416c4b727a..f613742c741 100644 --- a/tests/ui/ptr_offset_with_cast.rs +++ b/tests/ui/ptr_offset_with_cast.rs @@ -9,12 +9,12 @@ fn main() { let offset_isize = 1_isize; unsafe { - ptr.offset(offset_usize as isize); - ptr.offset(offset_isize as isize); - ptr.offset(offset_u8 as isize); + let _ = ptr.offset(offset_usize as isize); + let _ = ptr.offset(offset_isize as isize); + let _ = ptr.offset(offset_u8 as isize); - ptr.wrapping_offset(offset_usize as isize); - ptr.wrapping_offset(offset_isize as isize); - ptr.wrapping_offset(offset_u8 as isize); + let _ = ptr.wrapping_offset(offset_usize as isize); + let _ = ptr.wrapping_offset(offset_isize as isize); + let _ = ptr.wrapping_offset(offset_u8 as isize); } } diff --git a/tests/ui/ptr_offset_with_cast.stderr b/tests/ui/ptr_offset_with_cast.stderr index b5c7a03e277..fd45224ca06 100644 --- a/tests/ui/ptr_offset_with_cast.stderr +++ b/tests/ui/ptr_offset_with_cast.stderr @@ -1,16 +1,16 @@ error: use of `offset` with a `usize` casted to an `isize` - --> $DIR/ptr_offset_with_cast.rs:12:9 + --> $DIR/ptr_offset_with_cast.rs:12:17 | -LL | ptr.offset(offset_usize as isize); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(offset_usize)` +LL | let _ = ptr.offset(offset_usize as isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(offset_usize)` | = note: `-D clippy::ptr-offset-with-cast` implied by `-D warnings` error: use of `wrapping_offset` with a `usize` casted to an `isize` - --> $DIR/ptr_offset_with_cast.rs:16:9 + --> $DIR/ptr_offset_with_cast.rs:16:17 | -LL | ptr.wrapping_offset(offset_usize as isize); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(offset_usize)` +LL | let _ = ptr.wrapping_offset(offset_usize as isize); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(offset_usize)` error: aborting due to 2 previous errors From f28f1f15da825bcf5cf78413f464dfea0bc553e5 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 20 May 2020 13:32:53 +0200 Subject: [PATCH 5/5] Fix dogfood fallout --- clippy_lints/src/utils/inspector.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/utils/inspector.rs b/clippy_lints/src/utils/inspector.rs index 748c11fac64..9b672b9ec22 100644 --- a/clippy_lints/src/utils/inspector.rs +++ b/clippy_lints/src/utils/inspector.rs @@ -289,21 +289,21 @@ fn print_expr(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, indent: usize) { println!("{}operands:", ind); for op in asm.operands { match op { - hir::InlineAsmOperand::In { expr, .. } => print_expr(cx, expr, indent + 1), + hir::InlineAsmOperand::In { expr, .. } + | hir::InlineAsmOperand::InOut { expr, .. } + | hir::InlineAsmOperand::Const { expr } + | hir::InlineAsmOperand::Sym { expr } => print_expr(cx, expr, indent + 1), hir::InlineAsmOperand::Out { expr, .. } => { if let Some(expr) = expr { print_expr(cx, expr, indent + 1); } }, - hir::InlineAsmOperand::InOut { expr, .. } => print_expr(cx, expr, indent + 1), hir::InlineAsmOperand::SplitInOut { in_expr, out_expr, .. } => { print_expr(cx, in_expr, indent + 1); if let Some(out_expr) = out_expr { print_expr(cx, out_expr, indent + 1); } }, - hir::InlineAsmOperand::Const { expr } => print_expr(cx, expr, indent + 1), - hir::InlineAsmOperand::Sym { expr } => print_expr(cx, expr, indent + 1), } } },