]> git.lizzy.rs Git - rust.git/commitdiff
Merge commit 'e214ea82ad0a751563acf67e1cd9279cf302db3a' into clippyup
authorflip1995 <hello@philkrones.com>
Sun, 17 May 2020 15:36:26 +0000 (17:36 +0200)
committerflip1995 <hello@philkrones.com>
Sun, 17 May 2020 15:36:26 +0000 (17:36 +0200)
91 files changed:
CHANGELOG.md
clippy_lints/src/block_in_if_condition.rs [deleted file]
clippy_lints/src/blocks_in_if_conditions.rs [new file with mode: 0644]
clippy_lints/src/bytecount.rs
clippy_lints/src/comparison_chain.rs
clippy_lints/src/eq_op.rs
clippy_lints/src/identity_conversion.rs [deleted file]
clippy_lints/src/identity_op.rs
clippy_lints/src/let_if_seq.rs
clippy_lints/src/lib.rs
clippy_lints/src/loops.rs
clippy_lints/src/map_clone.rs
clippy_lints/src/matches.rs
clippy_lints/src/methods/bind_instead_of_map.rs [new file with mode: 0644]
clippy_lints/src/methods/mod.rs
clippy_lints/src/methods/option_map_unwrap_or.rs
clippy_lints/src/needless_pass_by_value.rs
clippy_lints/src/ranges.rs
clippy_lints/src/returns.rs
clippy_lints/src/types.rs
clippy_lints/src/unwrap.rs
clippy_lints/src/useless_conversion.rs [new file with mode: 0644]
clippy_lints/src/utils/diagnostics.rs
clippy_lints/src/utils/usage.rs
src/driver.rs
src/lintlist/mod.rs
tests/ui/bind_instead_of_map.fixed [new file with mode: 0644]
tests/ui/bind_instead_of_map.rs [new file with mode: 0644]
tests/ui/bind_instead_of_map.stderr [new file with mode: 0644]
tests/ui/bind_instead_of_map_multipart.rs [new file with mode: 0644]
tests/ui/bind_instead_of_map_multipart.stderr [new file with mode: 0644]
tests/ui/block_in_if_condition.fixed [deleted file]
tests/ui/block_in_if_condition.rs [deleted file]
tests/ui/block_in_if_condition.stderr [deleted file]
tests/ui/block_in_if_condition_closure.rs [deleted file]
tests/ui/block_in_if_condition_closure.stderr [deleted file]
tests/ui/blocks_in_if_conditions.fixed [new file with mode: 0644]
tests/ui/blocks_in_if_conditions.rs [new file with mode: 0644]
tests/ui/blocks_in_if_conditions.stderr [new file with mode: 0644]
tests/ui/blocks_in_if_conditions_closure.rs [new file with mode: 0644]
tests/ui/blocks_in_if_conditions_closure.stderr [new file with mode: 0644]
tests/ui/comparison_chain.rs
tests/ui/crashes/ice-5579.rs [new file with mode: 0644]
tests/ui/expect.rs
tests/ui/expect.stderr
tests/ui/for_loop_fixable.fixed
tests/ui/for_loop_fixable.rs
tests/ui/for_loop_fixable.stderr
tests/ui/for_loop_over_option_result.rs [deleted file]
tests/ui/for_loop_over_option_result.stderr [deleted file]
tests/ui/for_loop_unfixable.rs
tests/ui/for_loop_unfixable.stderr
tests/ui/for_loops_over_fallibles.rs [new file with mode: 0644]
tests/ui/for_loops_over_fallibles.stderr [new file with mode: 0644]
tests/ui/identity_conversion.fixed [deleted file]
tests/ui/identity_conversion.rs [deleted file]
tests/ui/identity_conversion.stderr [deleted file]
tests/ui/identity_op.rs
tests/ui/identity_op.stderr
tests/ui/implicit_saturating_sub.fixed
tests/ui/implicit_saturating_sub.rs
tests/ui/manual_memcpy.rs
tests/ui/map_unwrap_or.rs [new file with mode: 0644]
tests/ui/map_unwrap_or.stderr [new file with mode: 0644]
tests/ui/option_and_then_some.fixed [deleted file]
tests/ui/option_and_then_some.rs [deleted file]
tests/ui/option_and_then_some.stderr [deleted file]
tests/ui/option_map_or_none.fixed
tests/ui/option_map_or_none.rs
tests/ui/option_map_unwrap_or.rs [deleted file]
tests/ui/option_map_unwrap_or.stderr [deleted file]
tests/ui/result_map_unwrap_or_else.rs [deleted file]
tests/ui/result_map_unwrap_or_else.stderr [deleted file]
tests/ui/reversed_empty_ranges_fixable.fixed [new file with mode: 0644]
tests/ui/reversed_empty_ranges_fixable.rs [new file with mode: 0644]
tests/ui/reversed_empty_ranges_fixable.stderr [new file with mode: 0644]
tests/ui/reversed_empty_ranges_loops_fixable.fixed [new file with mode: 0644]
tests/ui/reversed_empty_ranges_loops_fixable.rs [new file with mode: 0644]
tests/ui/reversed_empty_ranges_loops_fixable.stderr [new file with mode: 0644]
tests/ui/reversed_empty_ranges_loops_unfixable.rs [new file with mode: 0644]
tests/ui/reversed_empty_ranges_loops_unfixable.stderr [new file with mode: 0644]
tests/ui/reversed_empty_ranges_unfixable.rs [new file with mode: 0644]
tests/ui/reversed_empty_ranges_unfixable.stderr [new file with mode: 0644]
tests/ui/unused_unit.fixed
tests/ui/unused_unit.rs
tests/ui/unused_unit.stderr
tests/ui/unwrap.rs
tests/ui/unwrap.stderr
tests/ui/useless_conversion.fixed [new file with mode: 0644]
tests/ui/useless_conversion.rs [new file with mode: 0644]
tests/ui/useless_conversion.stderr [new file with mode: 0644]

index 8457d6ad05cf9d876fe7739d364189234bb9980d..583c32ca9e032ed67f6cf333bcbcc8c107936d58 100644 (file)
@@ -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/block_in_if_condition.rs
deleted file mode 100644 (file)
index 9e533ea..0000000
+++ /dev/null
@@ -1,148 +0,0 @@
-use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg};
-use rustc_errors::Applicability;
-use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
-use rustc_hir::{BlockCheckMode, Expr, ExprKind};
-use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::hir::map::Map;
-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.
-    ///
-    /// **Why is this bad?** It isn't really Rust style, same as using parentheses
-    /// to contain expressions.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    /// ```rust
-    /// 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 }) {}
-    /// ```
-    pub BLOCK_IN_IF_CONDITION_STMT,
-    style,
-    "complex blocks in conditions, e.g., `if { let x = true; x } ...`"
-}
-
-declare_lint_pass!(BlockInIfCondition => [BLOCK_IN_IF_CONDITION_EXPR, BLOCK_IN_IF_CONDITION_STMT]);
-
-struct ExVisitor<'a, 'tcx> {
-    found_block: Option<&'tcx Expr<'tcx>>,
-    cx: &'a LateContext<'a, 'tcx>,
-}
-
-impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
-    type Map = Map<'tcx>;
-
-    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
-        if let ExprKind::Closure(_, _, eid, _, _) = expr.kind {
-            let body = self.cx.tcx.hir().body(eid);
-            let ex = &body.value;
-            if matches!(ex.kind, ExprKind::Block(_, _)) && !body.value.span.from_expansion() {
-                self.found_block = Some(ex);
-                return;
-            }
-        }
-        walk_expr(self, expr);
-    }
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
-        NestedVisitorMap::None
-    }
-}
-
-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`";
-
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BlockInIfCondition {
-    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
-        if in_external_macro(cx.sess(), expr.span) {
-            return;
-        }
-        if let Some((cond, _, _)) = higher::if_block(&expr) {
-            if let ExprKind::Block(block, _) = &cond.kind {
-                if block.rules == BlockCheckMode::DefaultBlock {
-                    if block.stmts.is_empty() {
-                        if let Some(ex) = &block.expr {
-                            // don't dig into the expression here, just suggest that they remove
-                            // the block
-                            if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) {
-                                return;
-                            }
-                            let mut applicability = Applicability::MachineApplicable;
-                            span_lint_and_sugg(
-                                cx,
-                                BLOCK_IN_IF_CONDITION_EXPR,
-                                cond.span,
-                                BRACED_EXPR_MESSAGE,
-                                "try",
-                                format!(
-                                    "{}",
-                                    snippet_block_with_applicability(
-                                        cx,
-                                        ex.span,
-                                        "..",
-                                        Some(expr.span),
-                                        &mut applicability
-                                    )
-                                ),
-                                applicability,
-                            );
-                        }
-                    } else {
-                        let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
-                        if span.from_expansion() || differing_macro_contexts(expr.span, span) {
-                            return;
-                        }
-                        // move block higher
-                        let mut applicability = Applicability::MachineApplicable;
-                        span_lint_and_sugg(
-                            cx,
-                            BLOCK_IN_IF_CONDITION_STMT,
-                            expr.span.with_hi(cond.span.hi()),
-                            COMPLEX_BLOCK_MESSAGE,
-                            "try",
-                            format!(
-                                "let res = {}; if res",
-                                snippet_block_with_applicability(
-                                    cx,
-                                    block.span,
-                                    "..",
-                                    Some(expr.span),
-                                    &mut applicability
-                                ),
-                            ),
-                            applicability,
-                        );
-                    }
-                }
-            } else {
-                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);
-                }
-            }
-        }
-    }
-}
diff --git a/clippy_lints/src/blocks_in_if_conditions.rs b/clippy_lints/src/blocks_in_if_conditions.rs
new file mode 100644 (file)
index 0000000..8fa9b05
--- /dev/null
@@ -0,0 +1,145 @@
+use crate::utils::{differing_macro_contexts, higher, snippet_block_with_applicability, span_lint, span_lint_and_sugg};
+use rustc_errors::Applicability;
+use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
+use rustc_hir::{BlockCheckMode, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::hir::map::Map;
+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 containing an
+    /// expression, statements or conditions that use closures with blocks.
+    ///
+    /// **Why is this bad?** Style, using blocks in the condition makes it hard to read.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Examples:**
+    /// ```rust
+    /// // Bad
+    /// if { true } { /* ... */ }
+    ///
+    /// // Good
+    /// if true { /* ... */ }
+    /// ```
+    ///
+    /// // or
+    ///
+    /// ```rust
+    /// # fn somefunc() -> bool { true };
+    ///
+    /// // Bad
+    /// if { let x = somefunc(); x } { /* ... */ }
+    ///
+    /// // Good
+    /// let res = { let x = somefunc(); x };
+    /// if res { /* ... */ }
+    /// ```
+    pub BLOCKS_IN_IF_CONDITIONS,
+    style,
+    "useless or complex blocks that can be eliminated in conditions"
+}
+
+declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]);
+
+struct ExVisitor<'a, 'tcx> {
+    found_block: Option<&'tcx Expr<'tcx>>,
+    cx: &'a LateContext<'a, 'tcx>,
+}
+
+impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> {
+    type Map = Map<'tcx>;
+
+    fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
+        if let ExprKind::Closure(_, _, eid, _, _) = expr.kind {
+            let body = self.cx.tcx.hir().body(eid);
+            let ex = &body.value;
+            if matches!(ex.kind, ExprKind::Block(_, _)) && !body.value.span.from_expansion() {
+                self.found_block = Some(ex);
+                return;
+            }
+        }
+        walk_expr(self, expr);
+    }
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+        NestedVisitorMap::None
+    }
+}
+
+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`";
+
+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;
+        }
+        if let Some((cond, _, _)) = higher::if_block(&expr) {
+            if let ExprKind::Block(block, _) = &cond.kind {
+                if block.rules == BlockCheckMode::DefaultBlock {
+                    if block.stmts.is_empty() {
+                        if let Some(ex) = &block.expr {
+                            // don't dig into the expression here, just suggest that they remove
+                            // the block
+                            if expr.span.from_expansion() || differing_macro_contexts(expr.span, ex.span) {
+                                return;
+                            }
+                            let mut applicability = Applicability::MachineApplicable;
+                            span_lint_and_sugg(
+                                cx,
+                                BLOCKS_IN_IF_CONDITIONS,
+                                cond.span,
+                                BRACED_EXPR_MESSAGE,
+                                "try",
+                                format!(
+                                    "{}",
+                                    snippet_block_with_applicability(
+                                        cx,
+                                        ex.span,
+                                        "..",
+                                        Some(expr.span),
+                                        &mut applicability
+                                    )
+                                ),
+                                applicability,
+                            );
+                        }
+                    } else {
+                        let span = block.expr.as_ref().map_or_else(|| block.stmts[0].span, |e| e.span);
+                        if span.from_expansion() || differing_macro_contexts(expr.span, span) {
+                            return;
+                        }
+                        // move block higher
+                        let mut applicability = Applicability::MachineApplicable;
+                        span_lint_and_sugg(
+                            cx,
+                            BLOCKS_IN_IF_CONDITIONS,
+                            expr.span.with_hi(cond.span.hi()),
+                            COMPLEX_BLOCK_MESSAGE,
+                            "try",
+                            format!(
+                                "let res = {}; if res",
+                                snippet_block_with_applicability(
+                                    cx,
+                                    block.span,
+                                    "..",
+                                    Some(expr.span),
+                                    &mut applicability
+                                ),
+                            ),
+                            applicability,
+                        );
+                    }
+                }
+            } else {
+                let mut visitor = ExVisitor { found_block: None, cx };
+                walk_expr(&mut visitor, cond);
+                if let Some(block) = visitor.found_block {
+                    span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE);
+                }
+            }
+        }
+    }
+}
index 278d043732f4942631731dda65e2f96873ee47e6..90c00ad098ffe53a5a6ecc6d0e6c776b99b58483 100644 (file)
@@ -3,7 +3,7 @@
     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};
index 96df3ffe3ce6727650e8da1c95448c046aab2f74..93e29edcaa58fead1be38d9718804bd9d4892f84 100644 (file)
@@ -81,12 +81,23 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
 
                 // 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, &[]));
index 098d47bdd40cb10aba75123be872c7e28aeae5af..4e1c1f131405f39011b78660d6551e24b521718f 100644 (file)
@@ -115,7 +115,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
                                     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_conversion.rs b/clippy_lints/src/identity_conversion.rs
deleted file mode 100644 (file)
index 33a9478..0000000
+++ /dev/null
@@ -1,124 +0,0 @@
-use crate::utils::{
-    match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_sugg,
-};
-use rustc_errors::Applicability;
-use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
-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.
-    ///
-    /// **Why is this bad?** Redundant code.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    /// ```rust
-    /// // format!() returns a `String`
-    /// let s: String = format!("hello").into();
-    /// ```
-    pub IDENTITY_CONVERSION,
-    complexity,
-    "using always-identical `Into`/`From`/`IntoIter` conversions"
-}
-
-#[derive(Default)]
-pub struct IdentityConversion {
-    try_desugar_arm: Vec<HirId>,
-}
-
-impl_lint_pass!(IdentityConversion => [IDENTITY_CONVERSION]);
-
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityConversion {
-    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
-        if e.span.from_expansion() {
-            return;
-        }
-
-        if Some(&e.hir_id) == self.try_desugar_arm.last() {
-            return;
-        }
-
-        match e.kind {
-            ExprKind::Match(_, ref arms, MatchSource::TryDesugar) => {
-                let e = match arms[0].body.kind {
-                    ExprKind::Ret(Some(ref e)) | ExprKind::Break(_, Some(ref e)) => e,
-                    _ => return,
-                };
-                if let ExprKind::Call(_, ref args) = e.kind {
-                    self.try_desugar_arm.push(args[0].hir_id);
-                }
-            },
-
-            ExprKind::MethodCall(ref name, .., ref args) => {
-                if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" {
-                    let a = cx.tables.expr_ty(e);
-                    let b = cx.tables.expr_ty(&args[0]);
-                    if same_tys(cx, a, b) {
-                        let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();
-
-                        span_lint_and_sugg(
-                            cx,
-                            IDENTITY_CONVERSION,
-                            e.span,
-                            "identical conversion",
-                            "consider removing `.into()`",
-                            sugg,
-                            Applicability::MachineApplicable, // snippet
-                        );
-                    }
-                }
-                if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" {
-                    let a = cx.tables.expr_ty(e);
-                    let b = cx.tables.expr_ty(&args[0]);
-                    if same_tys(cx, a, b) {
-                        let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
-                        span_lint_and_sugg(
-                            cx,
-                            IDENTITY_CONVERSION,
-                            e.span,
-                            "identical conversion",
-                            "consider removing `.into_iter()`",
-                            sugg,
-                            Applicability::MachineApplicable, // snippet
-                        );
-                    }
-                }
-            },
-
-            ExprKind::Call(ref path, ref args) => {
-                if let ExprKind::Path(ref qpath) = path.kind {
-                    if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id() {
-                        if match_def_path(cx, def_id, &paths::FROM_FROM) {
-                            let a = cx.tables.expr_ty(e);
-                            let b = cx.tables.expr_ty(&args[0]);
-                            if same_tys(cx, a, b) {
-                                let sugg = snippet(cx, args[0].span.source_callsite(), "<expr>").into_owned();
-                                let sugg_msg =
-                                    format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
-                                span_lint_and_sugg(
-                                    cx,
-                                    IDENTITY_CONVERSION,
-                                    e.span,
-                                    "identical conversion",
-                                    &sugg_msg,
-                                    sugg,
-                                    Applicability::MachineApplicable, // snippet
-                                );
-                            }
-                        }
-                    }
-                }
-            },
-
-            _ => {},
-        }
-    }
-
-    fn check_expr_post(&mut self, _: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
-        if Some(&e.hir_id) == self.try_desugar_arm.last() {
-            self.try_desugar_arm.pop();
-        }
-    }
-}
index 088e4ab1921fb8a31f0d38397521dbb1cbeb2486..78e07d25f67c573cf122a36039ac487312149dd8 100644 (file)
@@ -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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
         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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
     }
 }
 
+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) {
index 398a3103a037194ab5c1ffe557412244405d9cea..d7bf8a1476817c28892be2a3033f120fb2585301 100644 (file)
@@ -50,7 +50,7 @@
     /// };
     /// ```
     pub USELESS_LET_IF_SEQ,
-    style,
+    nursery,
     "unidiomatic `let mut` declaration followed by initialization in `if`"
 }
 
index 51b5401da7d00e65ce96ccac97d70d34ebcd5f69..e0787ac0887e4fde54a7c8e77a6bfa80b42bb8a2 100644 (file)
@@ -180,7 +180,7 @@ macro_rules! declare_clippy_lint {
 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 @@ macro_rules! declare_clippy_lint {
 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 @@ macro_rules! declare_clippy_lint {
 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(&regex::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.
index 2bbf4dba614b41970f846d8175d995a06fca8ba6..84e8a010738cfb9cf9f3307667aadefac106a668 100644 (file)
@@ -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 @@
     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};
 }
 
 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`.
     /// **Known problems:** None.
     ///
     /// **Example:**
-    /// ```ignore
-    /// for x in option {
-    ///     ..
+    /// ```rust
+    /// # let opt = Some(1);
+    ///
+    /// // Bad
+    /// for x in opt {
+    ///     // ..
     /// }
-    /// ```
     ///
-    /// This should be
-    /// ```ignore
-    /// if let Some(x) = option {
-    ///     ..
+    /// // Good
+    /// if let Some(x) = opt {
+    ///     // ..
     /// }
     /// ```
-    pub FOR_LOOP_OVER_OPTION,
-    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`.
+    /// // or
     ///
-    /// **Known problems:** None.
+    /// ```rust
+    /// # let res: Result<i32, std::io::Error> = Ok(1);
     ///
-    /// **Example:**
-    /// ```ignore
-    /// for x in result {
-    ///     ..
+    /// // Bad
+    /// for x in &res {
+    ///     // ..
     /// }
-    /// ```
     ///
-    /// This should be
-    /// ```ignore
-    /// if let Ok(x) = result {
-    ///     ..
+    /// // Good
+    /// if let Ok(x) = res {
+    ///     // ..
     /// }
     /// ```
-    pub FOR_LOOP_OVER_RESULT,
+    pub FOR_LOOPS_OVER_FALLIBLES,
     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! {
     "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()`.
     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!("({}, <item>)", 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, "<item>".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)),
index 0163b3f8dbc8e58898bef2762a244583c2dd57ee..d5adf6b0f0dcbb33271eb97d872c288df2315846 100644 (file)
@@ -9,8 +9,8 @@
 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
index 8f86535ef1e0f25aeea9de1792099c8541815bc0..bbf14374a1f7f0c741fd2336e20900be5e386cf1 100644 (file)
@@ -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 (file)
index 0000000..32e8663
--- /dev/null
@@ -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<Self::Map> {
+            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<F> {
+        in_stmt: bool,
+        failed: bool,
+        cb: F,
+    }
+
+    struct WithStmtGuarg<'a, F> {
+        val: &'a mut RetFinder<F>,
+        prev_in_stmt: bool,
+    }
+
+    impl<F> RetFinder<F> {
+        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<F> std::ops::Deref for WithStmtGuarg<'_, F> {
+        type Target = RetFinder<F>;
+
+        fn deref(&self) -> &Self::Target {
+            self.val
+        }
+    }
+
+    impl<F> std::ops::DerefMut for WithStmtGuarg<'_, F> {
+        fn deref_mut(&mut self) -> &mut Self::Target {
+            self.val
+        }
+    }
+
+    impl<F> 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<F> {
+        type Map = Map<'hir>;
+
+        fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> {
+            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
+    }
+}
index 3676dc5b09d21d7fdddcda2a79e4fd5677f9d780..626427c15ecf52b2e47c7c3f77ccada601afbc7b 100644 (file)
@@ -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::fmt;
 use std::iter;
 
+use bind_instead_of_map::BindInsteadOfMap;
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_errors::Applicability;
 };
 
 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
     ///
     /// **Known problems:** None.
     ///
-    /// **Example:**
-    /// Using unwrap on an `Result`:
-    ///
+    /// **Examples:**
     /// ```rust
-    /// let res: Result<usize, ()> = Ok(1);
-    /// res.unwrap();
+    /// # let opt = Some(1);
+    ///
+    /// // Bad
+    /// opt.unwrap();
+    ///
+    /// // Good
+    /// opt.expect("more helpful message");
     /// ```
     ///
-    /// Better:
+    /// // or
     ///
     /// ```rust
-    /// let res: Result<usize, ()> = Ok(1);
+    /// # let res: Result<usize, ()> = 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.
+    /// `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.
     ///
-    /// **Example:**
+    /// **Known problems:** None.
     ///
-    /// Using expect on an `Option`:
+    /// **Examples:**
+    /// ```rust,ignore
+    /// # let opt = Some(1);
     ///
-    /// ```rust
-    /// let opt = Some(1);
+    /// // Bad
     /// opt.expect("one");
-    /// ```
     ///
-    /// Better:
-    ///
-    /// ```rust,ignore
+    /// // Good
     /// 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`
-    /// 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`:
+    /// // or
     ///
     /// ```rust
-    /// let res: Result<usize, ()> = Ok(1);
-    /// res.expect("one");
-    /// ```
+    /// # let res: Result<usize, ()> = Ok(1);
     ///
-    /// Better:
+    /// // Bad
+    /// res.expect("one");
     ///
-    /// ```rust
-    /// let res: Result<usize, ()> = Ok(1);
+    /// // 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! {
 }
 
 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);
-    /// x.map(|a| a + 1).unwrap_or(0);
-    /// ```
-    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(_, _)`.
     ///
-    /// **Known problems:** The order of the arguments is not in execution order.
+    /// // Bad
+    /// x.map(|a| a + 1).unwrap_or(0);
     ///
-    /// **Example:**
-    /// ```rust
-    /// # let x = Some(1);
-    /// # fn some_function() -> usize { 1 }
-    /// x.map(|a| a + 1).unwrap_or_else(some_function);
+    /// // Good
+    /// x.map_or(0, |a| a + 1);
     /// ```
-    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(_, _)`.
+    /// // or
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
     /// ```rust
     /// # let x: Result<usize, ()> = 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! {
 }
 
 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)`"
 }
 }
 
 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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>)
             ["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 @@ fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx hir::
                             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 check_general_case<'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>,
index bf9dd3c9369298ad06f81d97acdaef7f90ad6971..20c60ef33189dc45f41ffa9f421b369a2648af00 100644 (file)
@@ -9,7 +9,7 @@
 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![
index a21818701dacc2fd059bb7387a3dc3f572adc4ea..ed48ab548978c67811c7056c949e38007ca41743 100644 (file)
@@ -293,7 +293,7 @@ fn check_fn(
                             );
                             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(
index d7ce2e66d69fbb3af1fa666169e3f5c8f0b79443..83c6faac04149f344ea4916f50e05d437f7411e4 100644 (file)
@@ -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
     "`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 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
 
         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(
index 5c9117d5b81cd6c62de819caa00c87e626fe8b77..35464f629c3646e1e3031eb82844e676c1e92eea 100644 (file)
@@ -248,28 +248,7 @@ fn check_fn(&mut self, cx: &EarlyContext<'_>, kind: FnKind<'_>, span: Span, _: a
             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 @@ fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
             _ => (),
         }
     }
+
+    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,
+    );
+}
index 6d49f50d550e858d0a0f87bb92fa3e275f093e95..6ed9ff22e466482bd7d473cea7f40992e3d3ad02 100644 (file)
@@ -2206,7 +2206,7 @@ fn suggestion<'a, 'tcx>(
 
             multispan_sugg(
                 diag,
-                "consider adding a type parameter".to_string(),
+                "consider adding a type parameter",
                 vec![
                     (
                         generics_suggestion_span,
@@ -2230,7 +2230,7 @@ fn suggestion<'a, 'tcx>(
             );
 
             if !vis.suggestions.is_empty() {
-                multispan_sugg(diag, "...and use generic constructor".into(), vis.suggestions);
+                multispan_sugg(diag, "...and use generic constructor", vis.suggestions);
             }
         }
 
index f3844c7d3b68f4f7e37449a3640c879a2691990e..8b971e7064b52d9137298169e778a57b23ea7f7e 100644 (file)
@@ -8,6 +8,7 @@
 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<UnwrapInfo<'tcx>> {
+    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/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
new file mode 100644 (file)
index 0000000..9592151
--- /dev/null
@@ -0,0 +1,130 @@
+use crate::utils::{
+    match_def_path, match_trait_method, paths, same_tys, snippet, snippet_with_macro_callsite, span_lint_and_sugg,
+};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, HirId, MatchSource};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+
+declare_clippy_lint! {
+    /// **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 USELESS_CONVERSION,
+    complexity,
+    "calls to `Into`/`From`/`IntoIter` that performs useless conversions to the same type"
+}
+
+#[derive(Default)]
+pub struct UselessConversion {
+    try_desugar_arm: Vec<HirId>,
+}
+
+impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
+
+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;
+        }
+
+        if Some(&e.hir_id) == self.try_desugar_arm.last() {
+            return;
+        }
+
+        match e.kind {
+            ExprKind::Match(_, ref arms, MatchSource::TryDesugar) => {
+                let e = match arms[0].body.kind {
+                    ExprKind::Ret(Some(ref e)) | ExprKind::Break(_, Some(ref e)) => e,
+                    _ => return,
+                };
+                if let ExprKind::Call(_, ref args) = e.kind {
+                    self.try_desugar_arm.push(args[0].hir_id);
+                }
+            },
+
+            ExprKind::MethodCall(ref name, .., ref args) => {
+                if match_trait_method(cx, e, &paths::INTO) && &*name.ident.as_str() == "into" {
+                    let a = cx.tables.expr_ty(e);
+                    let b = cx.tables.expr_ty(&args[0]);
+                    if same_tys(cx, a, b) {
+                        let sugg = snippet_with_macro_callsite(cx, args[0].span, "<expr>").to_string();
+
+                        span_lint_and_sugg(
+                            cx,
+                            USELESS_CONVERSION,
+                            e.span,
+                            "useless conversion",
+                            "consider removing `.into()`",
+                            sugg,
+                            Applicability::MachineApplicable, // snippet
+                        );
+                    }
+                }
+                if match_trait_method(cx, e, &paths::INTO_ITERATOR) && &*name.ident.as_str() == "into_iter" {
+                    let a = cx.tables.expr_ty(e);
+                    let b = cx.tables.expr_ty(&args[0]);
+                    if same_tys(cx, a, b) {
+                        let sugg = snippet(cx, args[0].span, "<expr>").into_owned();
+                        span_lint_and_sugg(
+                            cx,
+                            USELESS_CONVERSION,
+                            e.span,
+                            "useless conversion",
+                            "consider removing `.into_iter()`",
+                            sugg,
+                            Applicability::MachineApplicable, // snippet
+                        );
+                    }
+                }
+            },
+
+            ExprKind::Call(ref path, ref args) => {
+                if let ExprKind::Path(ref qpath) = path.kind {
+                    if let Some(def_id) = cx.tables.qpath_res(qpath, path.hir_id).opt_def_id() {
+                        if match_def_path(cx, def_id, &paths::FROM_FROM) {
+                            let a = cx.tables.expr_ty(e);
+                            let b = cx.tables.expr_ty(&args[0]);
+                            if same_tys(cx, a, b) {
+                                let sugg = snippet(cx, args[0].span.source_callsite(), "<expr>").into_owned();
+                                let sugg_msg =
+                                    format!("consider removing `{}()`", snippet(cx, path.span, "From::from"));
+                                span_lint_and_sugg(
+                                    cx,
+                                    USELESS_CONVERSION,
+                                    e.span,
+                                    "useless conversion",
+                                    &sugg_msg,
+                                    sugg,
+                                    Applicability::MachineApplicable, // snippet
+                                );
+                            }
+                        }
+                    }
+                }
+            },
+
+            _ => {},
+        }
+    }
+
+    fn check_expr_post(&mut self, _: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
+        if Some(&e.hir_id) == self.try_desugar_arm.last() {
+            self.try_desugar_arm.pop();
+        }
+    }
+}
index 24a1bdf1883f6b86edbe53f755d8a26821ca4265..f6d87c8532e430357b22a54b825c580fe1f0dc31 100644 (file)
@@ -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<I>(diag: &mut DiagnosticBuilder<'_>, help_msg: String, sugg: I)
+pub fn multispan_sugg<I>(diag: &mut DiagnosticBuilder<'_>, help_msg: &str, sugg: I)
 where
     I: IntoIterator<Item = (Span, String)>,
 {
-    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<I>(
+    diag: &mut DiagnosticBuilder<'_>,
+    help_msg: &str,
+    applicability: Applicability,
+    sugg: I,
+) where
+    I: IntoIterator<Item = (Span, String)>,
+{
+    diag.multipart_suggestion(help_msg, sugg.into_iter().collect(), applicability);
 }
index e85356779877cbfdb62abae5e5cb71929264a706..904d948ad29ed17244faf871f2c8a75d474db8ad 100644 (file)
@@ -77,8 +77,8 @@ fn mutate(&mut self, cmt: &Place<'tcx>) {
 }
 
 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 {
index 1ce0300f23904c6a9dc80004cfd3bf3959725472..d3a7e24937f95e865ff81cd7ae31ec6d994a8026 100644 (file)
@@ -295,121 +295,119 @@ fn toolchain_path(home: Option<String>, toolchain: Option<String>) -> Option<Pat
 pub fn main() {
     rustc_driver::init_rustc_env_logger();
     lazy_static::initialize(&ICE_HOOK);
-    exit(
-        rustc_driver::catch_with_exit_code(move || {
-            let mut orig_args: Vec<String> = 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);
-            }
+    exit(rustc_driver::catch_with_exit_code(move || {
+        let mut orig_args: Vec<String> = env::args().collect();
 
-            // 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 orig_args.iter().any(|a| a == "--version" || a == "-V") {
+            let version_info = rustc_tools_util::get_version_info!();
+            println!("{}", version_info);
+            exit(0);
+        }
 
-            if !wrapper_mode && (orig_args.iter().any(|a| a == "--help" || a == "-h") || orig_args.len() == 1) {
-                display_help();
-                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");
 
-            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);
-            }
+        // 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,
+                    }
+            })
+        };
 
-            // 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<String> = 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())
-                        }
-                    }));
-                }
+        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<String> = 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)
+    }))
 }
index 51d1cb2216a954c55e5907abc8c5885af1ea1a10..9457a64f9c6b96e48048d6118c9f731f0a4056a8 100644 (file)
         module: "bit_mask",
     },
     Lint {
-        name: "blacklisted_name",
-        group: "style",
-        desc: "usage of a blacklisted/placeholder name",
+        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: "blacklisted_name",
+        module: "methods",
     },
     Lint {
-        name: "block_in_if_condition_expr",
+        name: "blacklisted_name",
         group: "style",
-        desc: "braces that can be eliminated in conditions, e.g., `if { true } ...`",
+        desc: "usage of a blacklisted/placeholder name",
         deprecation: None,
-        module: "block_in_if_condition",
+        module: "blacklisted_name",
     },
     Lint {
-        name: "block_in_if_condition_stmt",
+        name: "blocks_in_if_conditions",
         group: "style",
-        desc: "complex blocks in conditions, e.g., `if { let x = true; x } ...`",
+        desc: "useless or complex blocks that can be eliminated in conditions",
         deprecation: None,
-        module: "block_in_if_condition",
+        module: "blocks_in_if_conditions",
     },
     Lint {
         name: "bool_comparison",
         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",
         module: "loops",
     },
     Lint {
-        name: "for_loop_over_option",
-        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",
+        name: "for_loops_over_fallibles",
         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",
     },
         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",
         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",
         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",
         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",
         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",
         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",
         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",
         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",
         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",
         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",
     },
     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/bind_instead_of_map.fixed b/tests/ui/bind_instead_of_map.fixed
new file mode 100644 (file)
index 0000000..5815550
--- /dev/null
@@ -0,0 +1,25 @@
+// run-rustfix
+#![deny(clippy::bind_instead_of_map)]
+
+// need a main anyway, use it get rid of unused warnings too
+pub fn main() {
+    let x = Some(5);
+    // the easiest cases
+    let _ = x;
+    let _ = x.map(|o| o + 1);
+    // and an easy counter-example
+    let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
+
+    // Different type
+    let x: Result<u32, &str> = Ok(1);
+    let _ = x;
+}
+
+pub fn foo() -> Option<String> {
+    let x = Some(String::from("hello"));
+    Some("hello".to_owned()).and_then(|s| Some(format!("{}{}", s, x?)))
+}
+
+pub fn example2(x: bool) -> Option<&'static str> {
+    Some("a").and_then(|s| Some(if x { s } else { return None }))
+}
diff --git a/tests/ui/bind_instead_of_map.rs b/tests/ui/bind_instead_of_map.rs
new file mode 100644 (file)
index 0000000..623b100
--- /dev/null
@@ -0,0 +1,25 @@
+// run-rustfix
+#![deny(clippy::bind_instead_of_map)]
+
+// need a main anyway, use it get rid of unused warnings too
+pub fn main() {
+    let x = Some(5);
+    // the easiest cases
+    let _ = x.and_then(Some);
+    let _ = x.and_then(|o| Some(o + 1));
+    // and an easy counter-example
+    let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
+
+    // Different type
+    let x: Result<u32, &str> = Ok(1);
+    let _ = x.and_then(Ok);
+}
+
+pub fn foo() -> Option<String> {
+    let x = Some(String::from("hello"));
+    Some("hello".to_owned()).and_then(|s| Some(format!("{}{}", s, x?)))
+}
+
+pub fn example2(x: bool) -> Option<&'static str> {
+    Some("a").and_then(|s| Some(if x { s } else { return None }))
+}
diff --git a/tests/ui/bind_instead_of_map.stderr b/tests/ui/bind_instead_of_map.stderr
new file mode 100644 (file)
index 0000000..24c6b7f
--- /dev/null
@@ -0,0 +1,26 @@
+error: using `Option.and_then(Some)`, which is a no-op
+  --> $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/bind_instead_of_map.rs:2:9
+   |
+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/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: 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 (file)
index 0000000..91d9d11
--- /dev/null
@@ -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 (file)
index 0000000..50ce2f4
--- /dev/null
@@ -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/block_in_if_condition.fixed
deleted file mode 100644 (file)
index 955801e..0000000
+++ /dev/null
@@ -1,75 +0,0 @@
-// run-rustfix
-#![warn(clippy::block_in_if_condition_expr)]
-#![warn(clippy::block_in_if_condition_stmt)]
-#![allow(unused, clippy::let_and_return)]
-#![warn(clippy::nonminimal_bool)]
-
-macro_rules! blocky {
-    () => {{
-        true
-    }};
-}
-
-macro_rules! blocky_too {
-    () => {{
-        let r = true;
-        r
-    }};
-}
-
-fn macro_if() {
-    if blocky!() {}
-
-    if blocky_too!() {}
-}
-
-fn condition_has_block() -> i32 {
-    let res = {
-        let x = 3;
-        x == 3
-    }; if res {
-        6
-    } else {
-        10
-    }
-}
-
-fn condition_has_block_with_single_expression() -> i32 {
-    if true {
-        6
-    } else {
-        10
-    }
-}
-
-fn condition_is_normal() -> i32 {
-    let x = 3;
-    if x == 3 {
-        6
-    } else {
-        10
-    }
-}
-
-fn condition_is_unsafe_block() {
-    let a: i32 = 1;
-
-    // this should not warn because the condition is an unsafe block
-    if unsafe { 1u32 == std::mem::transmute(a) } {
-        println!("1u32 == a");
-    }
-}
-
-fn block_in_assert() {
-    let opt = Some(42);
-    assert!(opt
-        .as_ref()
-        .and_then(|val| {
-            let mut v = val * 2;
-            v -= 1;
-            Some(v * 3)
-        })
-        .is_some());
-}
-
-fn main() {}
diff --git a/tests/ui/block_in_if_condition.rs b/tests/ui/block_in_if_condition.rs
deleted file mode 100644 (file)
index a6ea01d..0000000
+++ /dev/null
@@ -1,75 +0,0 @@
-// run-rustfix
-#![warn(clippy::block_in_if_condition_expr)]
-#![warn(clippy::block_in_if_condition_stmt)]
-#![allow(unused, clippy::let_and_return)]
-#![warn(clippy::nonminimal_bool)]
-
-macro_rules! blocky {
-    () => {{
-        true
-    }};
-}
-
-macro_rules! blocky_too {
-    () => {{
-        let r = true;
-        r
-    }};
-}
-
-fn macro_if() {
-    if blocky!() {}
-
-    if blocky_too!() {}
-}
-
-fn condition_has_block() -> i32 {
-    if {
-        let x = 3;
-        x == 3
-    } {
-        6
-    } else {
-        10
-    }
-}
-
-fn condition_has_block_with_single_expression() -> i32 {
-    if { true } {
-        6
-    } else {
-        10
-    }
-}
-
-fn condition_is_normal() -> i32 {
-    let x = 3;
-    if true && x == 3 {
-        6
-    } else {
-        10
-    }
-}
-
-fn condition_is_unsafe_block() {
-    let a: i32 = 1;
-
-    // this should not warn because the condition is an unsafe block
-    if unsafe { 1u32 == std::mem::transmute(a) } {
-        println!("1u32 == a");
-    }
-}
-
-fn block_in_assert() {
-    let opt = Some(42);
-    assert!(opt
-        .as_ref()
-        .and_then(|val| {
-            let mut v = val * 2;
-            v -= 1;
-            Some(v * 3)
-        })
-        .is_some());
-}
-
-fn main() {}
diff --git a/tests/ui/block_in_if_condition.stderr b/tests/ui/block_in_if_condition.stderr
deleted file mode 100644 (file)
index b0a0a27..0000000
+++ /dev/null
@@ -1,36 +0,0 @@
-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
-   |
-LL | /     if {
-LL | |         let x = 3;
-LL | |         x == 3
-LL | |     } {
-   | |_____^
-   |
-   = note: `-D clippy::block-in-if-condition-stmt` implied by `-D warnings`
-help: try
-   |
-LL |     let res = {
-LL |         let x = 3;
-LL |         x == 3
-LL |     }; if res {
-   |
-
-error: omit braces around single expression condition
-  --> $DIR/block_in_if_condition.rs:38: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
-   |
-LL |     if true && x == 3 {
-   |        ^^^^^^^^^^^^^^ help: try: `x == 3`
-   |
-   = note: `-D clippy::nonminimal-bool` implied by `-D warnings`
-
-error: aborting due to 3 previous errors
-
diff --git a/tests/ui/block_in_if_condition_closure.rs b/tests/ui/block_in_if_condition_closure.rs
deleted file mode 100644 (file)
index bac3eda..0000000
+++ /dev/null
@@ -1,48 +0,0 @@
-#![warn(clippy::block_in_if_condition_expr)]
-#![warn(clippy::block_in_if_condition_stmt)]
-#![allow(unused, clippy::let_and_return)]
-
-fn predicate<F: FnOnce(T) -> bool, T>(pfn: F, val: T) -> bool {
-    pfn(val)
-}
-
-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.
-    // The same principle applies -- add some extra expressions to make sure
-    // linter isn't confused by them.
-    if v == 3
-        && sky == "blue"
-        && predicate(
-            |x| {
-                let target = 3;
-                x == target
-            },
-            v,
-        )
-    {}
-
-    if predicate(
-        |x| {
-            let target = 3;
-            x == target
-        },
-        v,
-    ) {}
-}
-
-fn closure_without_block() {
-    if predicate(|x| x == 3, 6) {}
-}
-
-fn macro_in_closure() {
-    let option = Some(true);
-
-    if option.unwrap_or_else(|| unimplemented!()) {
-        unimplemented!()
-    }
-}
-
-fn main() {}
diff --git a/tests/ui/block_in_if_condition_closure.stderr b/tests/ui/block_in_if_condition_closure.stderr
deleted file mode 100644 (file)
index 86cd24f..0000000
+++ /dev/null
@@ -1,24 +0,0 @@
-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
-   |
-LL |               |x| {
-   |  _________________^
-LL | |                 let target = 3;
-LL | |                 x == target
-LL | |             },
-   | |_____________^
-   |
-   = note: `-D clippy::block-in-if-condition-stmt` 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
-   |
-LL |           |x| {
-   |  _____________^
-LL | |             let target = 3;
-LL | |             x == target
-LL | |         },
-   | |_________^
-
-error: aborting due to 2 previous errors
-
diff --git a/tests/ui/blocks_in_if_conditions.fixed b/tests/ui/blocks_in_if_conditions.fixed
new file mode 100644 (file)
index 0000000..14562c4
--- /dev/null
@@ -0,0 +1,74 @@
+// run-rustfix
+#![warn(clippy::blocks_in_if_conditions)]
+#![allow(unused, clippy::let_and_return)]
+#![warn(clippy::nonminimal_bool)]
+
+macro_rules! blocky {
+    () => {{
+        true
+    }};
+}
+
+macro_rules! blocky_too {
+    () => {{
+        let r = true;
+        r
+    }};
+}
+
+fn macro_if() {
+    if blocky!() {}
+
+    if blocky_too!() {}
+}
+
+fn condition_has_block() -> i32 {
+    let res = {
+        let x = 3;
+        x == 3
+    }; if res {
+        6
+    } else {
+        10
+    }
+}
+
+fn condition_has_block_with_single_expression() -> i32 {
+    if true {
+        6
+    } else {
+        10
+    }
+}
+
+fn condition_is_normal() -> i32 {
+    let x = 3;
+    if x == 3 {
+        6
+    } else {
+        10
+    }
+}
+
+fn condition_is_unsafe_block() {
+    let a: i32 = 1;
+
+    // this should not warn because the condition is an unsafe block
+    if unsafe { 1u32 == std::mem::transmute(a) } {
+        println!("1u32 == a");
+    }
+}
+
+fn block_in_assert() {
+    let opt = Some(42);
+    assert!(opt
+        .as_ref()
+        .map(|val| {
+            let mut v = val * 2;
+            v -= 1;
+            v * 3
+        })
+        .is_some());
+}
+
+fn main() {}
diff --git a/tests/ui/blocks_in_if_conditions.rs b/tests/ui/blocks_in_if_conditions.rs
new file mode 100644 (file)
index 0000000..bda8765
--- /dev/null
@@ -0,0 +1,74 @@
+// run-rustfix
+#![warn(clippy::blocks_in_if_conditions)]
+#![allow(unused, clippy::let_and_return)]
+#![warn(clippy::nonminimal_bool)]
+
+macro_rules! blocky {
+    () => {{
+        true
+    }};
+}
+
+macro_rules! blocky_too {
+    () => {{
+        let r = true;
+        r
+    }};
+}
+
+fn macro_if() {
+    if blocky!() {}
+
+    if blocky_too!() {}
+}
+
+fn condition_has_block() -> i32 {
+    if {
+        let x = 3;
+        x == 3
+    } {
+        6
+    } else {
+        10
+    }
+}
+
+fn condition_has_block_with_single_expression() -> i32 {
+    if { true } {
+        6
+    } else {
+        10
+    }
+}
+
+fn condition_is_normal() -> i32 {
+    let x = 3;
+    if true && x == 3 {
+        6
+    } else {
+        10
+    }
+}
+
+fn condition_is_unsafe_block() {
+    let a: i32 = 1;
+
+    // this should not warn because the condition is an unsafe block
+    if unsafe { 1u32 == std::mem::transmute(a) } {
+        println!("1u32 == a");
+    }
+}
+
+fn block_in_assert() {
+    let opt = Some(42);
+    assert!(opt
+        .as_ref()
+        .map(|val| {
+            let mut v = val * 2;
+            v -= 1;
+            v * 3
+        })
+        .is_some());
+}
+
+fn main() {}
diff --git a/tests/ui/blocks_in_if_conditions.stderr b/tests/ui/blocks_in_if_conditions.stderr
new file mode 100644 (file)
index 0000000..9bdddc8
--- /dev/null
@@ -0,0 +1,34 @@
+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/blocks_in_if_conditions.rs:26:5
+   |
+LL | /     if {
+LL | |         let x = 3;
+LL | |         x == 3
+LL | |     } {
+   | |_____^
+   |
+   = note: `-D clippy::blocks-in-if-conditions` implied by `-D warnings`
+help: try
+   |
+LL |     let res = {
+LL |         let x = 3;
+LL |         x == 3
+LL |     }; if res {
+   |
+
+error: omit braces around single expression condition
+  --> $DIR/blocks_in_if_conditions.rs:37:8
+   |
+LL |     if { true } {
+   |        ^^^^^^^^ help: try: `true`
+
+error: this boolean expression can be simplified
+  --> $DIR/blocks_in_if_conditions.rs:46:8
+   |
+LL |     if true && x == 3 {
+   |        ^^^^^^^^^^^^^^ help: try: `x == 3`
+   |
+   = note: `-D clippy::nonminimal-bool` implied by `-D warnings`
+
+error: aborting due to 3 previous errors
+
diff --git a/tests/ui/blocks_in_if_conditions_closure.rs b/tests/ui/blocks_in_if_conditions_closure.rs
new file mode 100644 (file)
index 0000000..acbabfa
--- /dev/null
@@ -0,0 +1,47 @@
+#![warn(clippy::blocks_in_if_conditions)]
+#![allow(unused, clippy::let_and_return)]
+
+fn predicate<F: FnOnce(T) -> bool, T>(pfn: F, val: T) -> bool {
+    pfn(val)
+}
+
+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 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
+        && sky == "blue"
+        && predicate(
+            |x| {
+                let target = 3;
+                x == target
+            },
+            v,
+        )
+    {}
+
+    if predicate(
+        |x| {
+            let target = 3;
+            x == target
+        },
+        v,
+    ) {}
+}
+
+fn closure_without_block() {
+    if predicate(|x| x == 3, 6) {}
+}
+
+fn macro_in_closure() {
+    let option = Some(true);
+
+    if option.unwrap_or_else(|| unimplemented!()) {
+        unimplemented!()
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/blocks_in_if_conditions_closure.stderr b/tests/ui/blocks_in_if_conditions_closure.stderr
new file mode 100644 (file)
index 0000000..941d604
--- /dev/null
@@ -0,0 +1,24 @@
+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/blocks_in_if_conditions_closure.rs:18:17
+   |
+LL |               |x| {
+   |  _________________^
+LL | |                 let target = 3;
+LL | |                 x == target
+LL | |             },
+   | |_____________^
+   |
+   = 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/blocks_in_if_conditions_closure.rs:27:13
+   |
+LL |           |x| {
+   |  _____________^
+LL | |             let target = 3;
+LL | |             x == target
+LL | |         },
+   | |_________^
+
+error: aborting due to 2 previous errors
+
index 9c2128469de9ded7afeaf9571882f4e3be171383..3b03f8c7dfe7c71f6642646d2543a719ce3cbb4f 100644 (file)
@@ -137,4 +137,70 @@ fn h<T: Ord>(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 (file)
index 0000000..e1842c7
--- /dev/null
@@ -0,0 +1,17 @@
+trait IsErr {
+    fn is_err(&self, err: &str) -> bool;
+}
+
+impl<T> IsErr for Option<T> {
+    fn is_err(&self, _err: &str) -> bool {
+        true
+    }
+}
+
+fn main() {
+    let t = Some(1);
+
+    if t.is_err("") {
+        t.unwrap();
+    }
+}
index 0bd4252c49aa5e9e77f675f37df8f82aa0ae1251..1073acf6f0cd66b51c33588bfa7296e1a9364a57 100644 (file)
@@ -1,4 +1,4 @@
-#![warn(clippy::option_expect_used, clippy::result_expect_used)]
+#![warn(clippy::expect_used)]
 
 fn expect_option() {
     let opt = Some(0);
index adf9f4f192191d652593cdf04dad5eac2de4ca51..9d3fc7df15cc7a9de6f73efca2de52e3f4a93a3c 100644 (file)
@@ -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
index 5fc84ada9efdd4368f1f6806c4c74dcffabdb917..249a88a0b3982cdccb57fa64418d5831d54c9c33 100644 (file)
@@ -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
index 4165b0dc004942cae3f3fc0ef23b75ed7107d14f..306d85a6351e107f6a7e6bbb0b8c70d9843a4a93 100644 (file)
@@ -21,7 +21,6 @@ fn iter(&self) -> std::slice::Iter<u8> {
     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 @@ fn iter(&self) -> std::slice::Iter<u8> {
 )]
 #[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
index cffb4b9f0a9c0bf1eecca85f10488dff5588246f..ddfe66d675f91efbc8f070116c570ef9eda5a496 100644 (file)
@@ -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_over_option_result.rs b/tests/ui/for_loop_over_option_result.rs
deleted file mode 100644 (file)
index 6b207b2..0000000
+++ /dev/null
@@ -1,59 +0,0 @@
-#![warn(clippy::for_loop_over_option, clippy::for_loop_over_result)]
-
-/// Tests for_loop_over_result and for_loop_over_option
-
-fn for_loop_over_option_and_result() {
-    let option = Some(1);
-    let result = option.ok_or("x not found");
-    let v = vec![0, 1, 2];
-
-    // check FOR_LOOP_OVER_OPTION lint
-    for x in option {
-        println!("{}", x);
-    }
-
-    // check FOR_LOOP_OVER_RESULT lint
-    for x in result {
-        println!("{}", x);
-    }
-
-    for x in option.ok_or("x not found") {
-        println!("{}", x);
-    }
-
-    // make sure LOOP_OVER_NEXT lint takes clippy::precedence when next() is the last call
-    // in the chain
-    for x in v.iter().next() {
-        println!("{}", x);
-    }
-
-    // make sure we lint when next() is not the last call in the chain
-    for x in v.iter().next().and(Some(0)) {
-        println!("{}", x);
-    }
-
-    for x in v.iter().next().ok_or("x not found") {
-        println!("{}", x);
-    }
-
-    // check for false positives
-
-    // for loop false positive
-    for x in v {
-        println!("{}", x);
-    }
-
-    // while let false positive for Option
-    while let Some(x) = option {
-        println!("{}", x);
-        break;
-    }
-
-    // while let false positive for Result
-    while let Ok(x) = result {
-        println!("{}", x);
-        break;
-    }
-}
-
-fn main() {}
diff --git a/tests/ui/for_loop_over_option_result.stderr b/tests/ui/for_loop_over_option_result.stderr
deleted file mode 100644 (file)
index 194a0bf..0000000
+++ /dev/null
@@ -1,72 +0,0 @@
-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
-   |
-LL |     for x in option {
-   |              ^^^^^^
-   |
-   = note: `-D clippy::for-loop-over-option` 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
-   |
-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
-   |
-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
-   |
-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
-   |
-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
-   |
-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
-   |
-LL | /     while let Some(x) = option {
-LL | |         println!("{}", x);
-LL | |         break;
-LL | |     }
-   | |_____^
-   |
-   = note: `#[deny(clippy::never_loop)]` on by default
-
-error: this loop never actually loops
-  --> $DIR/for_loop_over_option_result.rs:53:5
-   |
-LL | /     while let Ok(x) = result {
-LL | |         println!("{}", x);
-LL | |         break;
-LL | |     }
-   | |_____^
-
-error: aborting due to 8 previous errors
-
index 179b255e08ca77962bdb5a096333dd4d854b068c..e73536052f0f5bf597dd9154b4209984d87c54a7 100644 (file)
@@ -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(
     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");
-        }
-    }
 }
index 1da8e0f3588d79fd3defd9429ff4bcd91055b61d..1c9287b6acbb328d5b2335a797acb4a1c903a478 100644 (file)
@@ -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_loops_over_fallibles.rs b/tests/ui/for_loops_over_fallibles.rs
new file mode 100644 (file)
index 0000000..1b9dde8
--- /dev/null
@@ -0,0 +1,57 @@
+#![warn(clippy::for_loops_over_fallibles)]
+
+fn for_loops_over_fallibles() {
+    let option = Some(1);
+    let result = option.ok_or("x not found");
+    let v = vec![0, 1, 2];
+
+    // check over an `Option`
+    for x in option {
+        println!("{}", x);
+    }
+
+    // check over a `Result`
+    for x in result {
+        println!("{}", x);
+    }
+
+    for x in option.ok_or("x not found") {
+        println!("{}", x);
+    }
+
+    // make sure LOOP_OVER_NEXT lint takes clippy::precedence when next() is the last call
+    // in the chain
+    for x in v.iter().next() {
+        println!("{}", x);
+    }
+
+    // make sure we lint when next() is not the last call in the chain
+    for x in v.iter().next().and(Some(0)) {
+        println!("{}", x);
+    }
+
+    for x in v.iter().next().ok_or("x not found") {
+        println!("{}", x);
+    }
+
+    // check for false positives
+
+    // for loop false positive
+    for x in v {
+        println!("{}", x);
+    }
+
+    // while let false positive for Option
+    while let Some(x) = option {
+        println!("{}", x);
+        break;
+    }
+
+    // while let false positive for Result
+    while let Ok(x) = result {
+        println!("{}", x);
+        break;
+    }
+}
+
+fn main() {}
diff --git a/tests/ui/for_loops_over_fallibles.stderr b/tests/ui/for_loops_over_fallibles.stderr
new file mode 100644 (file)
index 0000000..bef228d
--- /dev/null
@@ -0,0 +1,71 @@
+error: for loop over `option`, which is an `Option`. This is more readably written as an `if let` statement.
+  --> $DIR/for_loops_over_fallibles.rs:9:14
+   |
+LL |     for x in option {
+   |              ^^^^^^
+   |
+   = 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_loops_over_fallibles.rs:14:14
+   |
+LL |     for x in result {
+   |              ^^^^^^
+   |
+   = 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_loops_over_fallibles.rs:18:14
+   |
+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_loops_over_fallibles.rs:24:14
+   |
+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_loops_over_fallibles.rs:29:14
+   |
+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_loops_over_fallibles.rs:33:14
+   |
+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_loops_over_fallibles.rs:45:5
+   |
+LL | /     while let Some(x) = option {
+LL | |         println!("{}", x);
+LL | |         break;
+LL | |     }
+   | |_____^
+   |
+   = note: `#[deny(clippy::never_loop)]` on by default
+
+error: this loop never actually loops
+  --> $DIR/for_loops_over_fallibles.rs:51:5
+   |
+LL | /     while let Ok(x) = result {
+LL | |         println!("{}", x);
+LL | |         break;
+LL | |     }
+   | |_____^
+
+error: aborting due to 8 previous errors
+
diff --git a/tests/ui/identity_conversion.fixed b/tests/ui/identity_conversion.fixed
deleted file mode 100644 (file)
index dd3fc56..0000000
+++ /dev/null
@@ -1,58 +0,0 @@
-// run-rustfix
-
-#![deny(clippy::identity_conversion)]
-
-fn test_generic<T: Copy>(val: T) -> T {
-    let _ = val;
-    val
-}
-
-fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
-    // ok
-    let _: i32 = val.into();
-    let _: U = val.into();
-    let _ = U::from(val);
-}
-
-fn test_questionmark() -> Result<(), ()> {
-    {
-        let _: i32 = 0i32;
-        Ok(Ok(()))
-    }??;
-    Ok(())
-}
-
-fn test_issue_3913() -> Result<(), std::io::Error> {
-    use std::fs;
-    use std::path::Path;
-
-    let path = Path::new(".");
-    for _ in fs::read_dir(path)? {}
-
-    Ok(())
-}
-
-fn main() {
-    test_generic(10i32);
-    test_generic2::<i32, i32>(10i32);
-    test_questionmark().unwrap();
-    test_issue_3913().unwrap();
-
-    let _: String = "foo".into();
-    let _: String = From::from("foo");
-    let _ = String::from("foo");
-    #[allow(clippy::identity_conversion)]
-    {
-        let _: String = "foo".into();
-        let _ = String::from("foo");
-        let _ = "".lines().into_iter();
-    }
-
-    let _: String = "foo".to_string();
-    let _: String = "foo".to_string();
-    let _ = "foo".to_string();
-    let _ = format!("A: {:04}", 123);
-    let _ = "".lines();
-    let _ = vec![1, 2, 3].into_iter();
-    let _: String = format!("Hello {}", "world");
-}
diff --git a/tests/ui/identity_conversion.rs b/tests/ui/identity_conversion.rs
deleted file mode 100644 (file)
index 875ed7d..0000000
+++ /dev/null
@@ -1,58 +0,0 @@
-// run-rustfix
-
-#![deny(clippy::identity_conversion)]
-
-fn test_generic<T: Copy>(val: T) -> T {
-    let _ = T::from(val);
-    val.into()
-}
-
-fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
-    // ok
-    let _: i32 = val.into();
-    let _: U = val.into();
-    let _ = U::from(val);
-}
-
-fn test_questionmark() -> Result<(), ()> {
-    {
-        let _: i32 = 0i32.into();
-        Ok(Ok(()))
-    }??;
-    Ok(())
-}
-
-fn test_issue_3913() -> Result<(), std::io::Error> {
-    use std::fs;
-    use std::path::Path;
-
-    let path = Path::new(".");
-    for _ in fs::read_dir(path)? {}
-
-    Ok(())
-}
-
-fn main() {
-    test_generic(10i32);
-    test_generic2::<i32, i32>(10i32);
-    test_questionmark().unwrap();
-    test_issue_3913().unwrap();
-
-    let _: String = "foo".into();
-    let _: String = From::from("foo");
-    let _ = String::from("foo");
-    #[allow(clippy::identity_conversion)]
-    {
-        let _: String = "foo".into();
-        let _ = String::from("foo");
-        let _ = "".lines().into_iter();
-    }
-
-    let _: String = "foo".to_string().into();
-    let _: String = From::from("foo".to_string());
-    let _ = String::from("foo".to_string());
-    let _ = String::from(format!("A: {:04}", 123));
-    let _ = "".lines().into_iter();
-    let _ = vec![1, 2, 3].into_iter().into_iter();
-    let _: String = format!("Hello {}", "world").into();
-}
diff --git a/tests/ui/identity_conversion.stderr b/tests/ui/identity_conversion.stderr
deleted file mode 100644 (file)
index 57626b2..0000000
+++ /dev/null
@@ -1,68 +0,0 @@
-error: identical conversion
-  --> $DIR/identity_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
-   |
-LL | #![deny(clippy::identity_conversion)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-error: identical conversion
-  --> $DIR/identity_conversion.rs:7:5
-   |
-LL |     val.into()
-   |     ^^^^^^^^^^ help: consider removing `.into()`: `val`
-
-error: identical conversion
-  --> $DIR/identity_conversion.rs:19:22
-   |
-LL |         let _: i32 = 0i32.into();
-   |                      ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
-
-error: identical conversion
-  --> $DIR/identity_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
-   |
-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
-   |
-LL |     let _ = String::from("foo".to_string());
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
-
-error: identical conversion
-  --> $DIR/identity_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
-   |
-LL |     let _ = "".lines().into_iter();
-   |             ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
-
-error: identical conversion
-  --> $DIR/identity_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
-   |
-LL |     let _: String = format!("Hello {}", "world").into();
-   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")`
-
-error: aborting due to 10 previous errors
-
index ae2815d345a94dd5233f3177ceb707bd5e47497a..ceaacaaf6bd706618ecac1133af2b63d9d261595 100644 (file)
@@ -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;
 }
index 4742877706acd6d360768c41dfbe4f0b8011d8c7..d8d44a74f9ab0fb05c1b19f7566eb2be6ad3e7be 100644 (file)
@@ -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
 
index e0b5b31a00c7c4aef6e064173d51449f98e34196..859765d08a70bbb83d313b53f608cd23d07aff4b 100644 (file)
@@ -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;
 
index 39d81608922975e74fbb105eb8432ae159b1eff5..24cb216e79bf3715d00ffcd57f4e2aef9d421bf2 100644 (file)
@@ -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;
 
index 9c24d6d4db1f257de14382cdd58ad2d58c11ff21..0083f94798fe4b2c650998d439f05cda4e7b0d0c 100644 (file)
@@ -104,7 +104,7 @@ fn index(&self, _: usize) -> &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/map_unwrap_or.rs b/tests/ui/map_unwrap_or.rs
new file mode 100644 (file)
index 0000000..5859440
--- /dev/null
@@ -0,0 +1,99 @@
+// FIXME: Add "run-rustfix" once it's supported for multipart suggestions
+// aux-build:option_helpers.rs
+
+#![warn(clippy::map_unwrap_or)]
+
+#[macro_use]
+extern crate option_helpers;
+
+use std::collections::HashMap;
+
+#[rustfmt::skip]
+fn option_methods() {
+    let opt = Some(1);
+
+    // 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.
+        .unwrap_or(0);
+    // Multi-line cases.
+    let _ = opt.map(|x| {
+        x + 1
+    }
+    ).unwrap_or(0);
+    let _ = opt.map(|x| x + 1)
+        .unwrap_or({
+            0
+        });
+    // Single line `map(f).unwrap_or(None)` case.
+    let _ = opt.map(|x| Some(x + 1)).unwrap_or(None);
+    // Multi-line `map(f).unwrap_or(None)` cases.
+    let _ = opt.map(|x| {
+        Some(x + 1)
+    }
+    ).unwrap_or(None);
+    let _ = opt
+        .map(|x| Some(x + 1))
+        .unwrap_or(None);
+    // macro case
+    let _ = opt_map!(opt, |x| x + 1).unwrap_or(0); // should not lint
+
+    // Should not lint if not copyable
+    let id: String = "identifier".to_string();
+    let _ = Some("prefix").map(|p| format!("{}.{}", p, id)).unwrap_or(id);
+    // ...but DO lint if the `unwrap_or` argument is not used in the `map`
+    let id: String = "identifier".to_string();
+    let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
+
+    // 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.
+        .unwrap_or_else(|| 0);
+    // Multi-line cases.
+    let _ = opt.map(|x| {
+        x + 1
+    }
+    ).unwrap_or_else(|| 0);
+    let _ = opt.map(|x| x + 1)
+        .unwrap_or_else(||
+            0
+        );
+    // Macro case.
+    // Should not lint.
+    let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0);
+
+    // Issue #4144
+    {
+        let mut frequencies = HashMap::new();
+        let word = "foo";
+
+        frequencies
+            .get_mut(word)
+            .map(|count| {
+                *count += 1;
+            })
+            .unwrap_or_else(|| {
+                frequencies.insert(word.to_owned(), 1);
+            });
+    }
+}
+
+fn result_methods() {
+    let res: Result<i32, ()> = 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/map_unwrap_or.stderr b/tests/ui/map_unwrap_or.stderr
new file mode 100644 (file)
index 0000000..b62080a
--- /dev/null
@@ -0,0 +1,161 @@
+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/map_unwrap_or.rs:17:13
+   |
+LL |       let _ = opt.map(|x| x + 1)
+   |  _____________^
+LL | |         // Should lint even though this call is on a separate line.
+LL | |         .unwrap_or(0);
+   | |_____________________^
+   |
+   = 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/map_unwrap_or.rs:21:13
+   |
+LL |       let _ = opt.map(|x| {
+   |  _____________^
+LL | |         x + 1
+LL | |     }
+LL | |     ).unwrap_or(0);
+   | |__________________^
+   |
+help: use `map_or(a, f)` instead
+   |
+LL |     let _ = opt.map_or(0, |x| {
+LL |         x + 1
+LL |     }
+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/map_unwrap_or.rs:25:13
+   |
+LL |       let _ = opt.map(|x| x + 1)
+   |  _____________^
+LL | |         .unwrap_or({
+LL | |             0
+LL | |         });
+   | |__________^
+   |
+help: use `map_or(a, f)` instead
+   |
+LL |     let _ = opt.map_or({
+LL |             0
+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/map_unwrap_or.rs:30:13
+   |
+LL |     let _ = opt.map(|x| Some(x + 1)).unwrap_or(None);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `and_then(f)` instead
+   |
+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/map_unwrap_or.rs:32:13
+   |
+LL |       let _ = opt.map(|x| {
+   |  _____________^
+LL | |         Some(x + 1)
+LL | |     }
+LL | |     ).unwrap_or(None);
+   | |_____________________^
+   |
+help: use `and_then(f)` instead
+   |
+LL |     let _ = opt.and_then(|x| {
+LL |         Some(x + 1)
+LL |     }
+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/map_unwrap_or.rs:36:13
+   |
+LL |       let _ = opt
+   |  _____________^
+LL | |         .map(|x| Some(x + 1))
+LL | |         .unwrap_or(None);
+   | |________________________^
+   |
+help: use `and_then(f)` instead
+   |
+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/map_unwrap_or.rs:47:13
+   |
+LL |     let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: use `map_or(a, f)` instead
+   |
+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/map_unwrap_or.rs:51:13
+   |
+LL |       let _ = opt.map(|x| x + 1)
+   |  _____________^
+LL | |         // Should lint even though this call is on a separate line.
+LL | |         .unwrap_or_else(|| 0);
+   | |_____________________________^
+   |
+   = 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/map_unwrap_or.rs:55:13
+   |
+LL |       let _ = opt.map(|x| {
+   |  _____________^
+LL | |         x + 1
+LL | |     }
+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/map_unwrap_or.rs:59:13
+   |
+LL |       let _ = opt.map(|x| x + 1)
+   |  _____________^
+LL | |         .unwrap_or_else(||
+LL | |             0
+LL | |         );
+   | |_________^
+
+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_and_then_some.fixed b/tests/ui/option_and_then_some.fixed
deleted file mode 100644 (file)
index 035bc1e..0000000
+++ /dev/null
@@ -1,25 +0,0 @@
-// run-rustfix
-#![deny(clippy::option_and_then_some)]
-
-// need a main anyway, use it get rid of unused warnings too
-pub fn main() {
-    let x = Some(5);
-    // the easiest cases
-    let _ = x;
-    let _ = x.map(|o| o + 1);
-    // and an easy counter-example
-    let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
-
-    // Different type
-    let x: Result<u32, &str> = Ok(1);
-    let _ = x.and_then(Ok);
-}
-
-pub fn foo() -> Option<String> {
-    let x = Some(String::from("hello"));
-    Some("hello".to_owned()).and_then(|s| Some(format!("{}{}", s, x?)))
-}
-
-pub fn example2(x: bool) -> Option<&'static str> {
-    Some("a").and_then(|s| Some(if x { s } else { return None }))
-}
diff --git a/tests/ui/option_and_then_some.rs b/tests/ui/option_and_then_some.rs
deleted file mode 100644 (file)
index d49da78..0000000
+++ /dev/null
@@ -1,25 +0,0 @@
-// run-rustfix
-#![deny(clippy::option_and_then_some)]
-
-// need a main anyway, use it get rid of unused warnings too
-pub fn main() {
-    let x = Some(5);
-    // the easiest cases
-    let _ = x.and_then(Some);
-    let _ = x.and_then(|o| Some(o + 1));
-    // and an easy counter-example
-    let _ = x.and_then(|o| if o < 32 { Some(o) } else { None });
-
-    // Different type
-    let x: Result<u32, &str> = Ok(1);
-    let _ = x.and_then(Ok);
-}
-
-pub fn foo() -> Option<String> {
-    let x = Some(String::from("hello"));
-    Some("hello".to_owned()).and_then(|s| Some(format!("{}{}", s, x?)))
-}
-
-pub fn example2(x: bool) -> Option<&'static str> {
-    Some("a").and_then(|s| Some(if x { s } else { return None }))
-}
diff --git a/tests/ui/option_and_then_some.stderr b/tests/ui/option_and_then_some.stderr
deleted file mode 100644 (file)
index 4782549..0000000
+++ /dev/null
@@ -1,20 +0,0 @@
-error: using `Option.and_then(Some)`, which is a no-op
-  --> $DIR/option_and_then_some.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
-   |
-LL | #![deny(clippy::option_and_then_some)]
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-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
-   |
-LL |     let _ = x.and_then(|o| Some(o + 1));
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `x.map(|o| o + 1)`
-
-error: aborting due to 2 previous errors
-
index decbae4e5af9aabb7285ba7519845f48071af8f6..d80c3c7c1b7225664a91e95254005e748d919c9a 100644 (file)
@@ -1,6 +1,6 @@
 // run-rustfix
 
-#![allow(clippy::option_and_then_some)]
+#![allow(clippy::bind_instead_of_map)]
 
 fn main() {
     let opt = Some(1);
index 0f1d2218d5d3889efd3af6f482e6e149bee4381f..629842419e546d1af1e57ffe1c8062797a46f04d 100644 (file)
@@ -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_unwrap_or.rs b/tests/ui/option_map_unwrap_or.rs
deleted file mode 100644 (file)
index 0364d83..0000000
+++ /dev/null
@@ -1,88 +0,0 @@
-// 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)]
-
-#[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`.
-    // Single line case.
-    let _ = opt.map(|x| x + 1)
-        // Should lint even though this call is on a separate line.
-        .unwrap_or(0);
-    // Multi-line cases.
-    let _ = opt.map(|x| {
-        x + 1
-    }
-    ).unwrap_or(0);
-    let _ = opt.map(|x| x + 1)
-        .unwrap_or({
-            0
-        });
-    // Single line `map(f).unwrap_or(None)` case.
-    let _ = opt.map(|x| Some(x + 1)).unwrap_or(None);
-    // Multi-line `map(f).unwrap_or(None)` cases.
-    let _ = opt.map(|x| {
-        Some(x + 1)
-    }
-    ).unwrap_or(None);
-    let _ = opt
-        .map(|x| Some(x + 1))
-        .unwrap_or(None);
-    // macro case
-    let _ = opt_map!(opt, |x| x + 1).unwrap_or(0); // should not lint
-
-    // Should not lint if not copyable
-    let id: String = "identifier".to_string();
-    let _ = Some("prefix").map(|p| format!("{}.{}", p, id)).unwrap_or(id);
-    // ...but DO lint if the `unwrap_or` argument is not used in the `map`
-    let id: String = "identifier".to_string();
-    let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
-
-    // Check OPTION_MAP_UNWRAP_OR_ELSE
-    // single line case
-    let _ = opt.map(|x| x + 1)
-        // Should lint even though this call is on a separate line.
-        .unwrap_or_else(|| 0);
-    // Multi-line cases.
-    let _ = opt.map(|x| {
-        x + 1
-    }
-    ).unwrap_or_else(|| 0);
-    let _ = opt.map(|x| x + 1)
-        .unwrap_or_else(||
-            0
-        );
-    // Macro case.
-    // Should not lint.
-    let _ = opt_map!(opt, |x| x + 1).unwrap_or_else(|| 0);
-
-    // Issue #4144
-    {
-        let mut frequencies = HashMap::new();
-        let word = "foo";
-
-        frequencies
-            .get_mut(word)
-            .map(|count| {
-                *count += 1;
-            })
-            .unwrap_or_else(|| {
-                frequencies.insert(word.to_owned(), 1);
-            });
-    }
-}
-
-fn main() {
-    option_methods();
-}
diff --git a/tests/ui/option_map_unwrap_or.stderr b/tests/ui/option_map_unwrap_or.stderr
deleted file mode 100644 (file)
index f05f289..0000000
+++ /dev/null
@@ -1,138 +0,0 @@
-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
-   |
-LL |       let _ = opt.map(|x| x + 1)
-   |  _____________^
-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`
-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
-   |
-LL |       let _ = opt.map(|x| {
-   |  _____________^
-LL | |         x + 1
-LL | |     }
-LL | |     ).unwrap_or(0);
-   | |__________________^
-   |
-help: use `map_or(a, f)` instead
-   |
-LL |     let _ = opt.map_or(0, |x| {
-LL |         x + 1
-LL |     }
-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
-   |
-LL |       let _ = opt.map(|x| x + 1)
-   |  _____________^
-LL | |         .unwrap_or({
-LL | |             0
-LL | |         });
-   | |__________^
-   |
-help: use `map_or(a, f)` instead
-   |
-LL |     let _ = opt.map_or({
-LL |             0
-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
-   |
-LL |     let _ = opt.map(|x| Some(x + 1)).unwrap_or(None);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: use `and_then(f)` instead
-   |
-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
-   |
-LL |       let _ = opt.map(|x| {
-   |  _____________^
-LL | |         Some(x + 1)
-LL | |     }
-LL | |     ).unwrap_or(None);
-   | |_____________________^
-   |
-help: use `and_then(f)` instead
-   |
-LL |     let _ = opt.and_then(|x| {
-LL |         Some(x + 1)
-LL |     }
-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
-   |
-LL |       let _ = opt
-   |  _____________^
-LL | |         .map(|x| Some(x + 1))
-LL | |         .unwrap_or(None);
-   | |________________________^
-   |
-help: use `and_then(f)` instead
-   |
-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
-   |
-LL |     let _ = Some("prefix").map(|p| format!("{}.", p)).unwrap_or(id);
-   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-help: use `map_or(a, f)` instead
-   |
-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
-   |
-LL |       let _ = opt.map(|x| x + 1)
-   |  _____________^
-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
-   |
-LL |       let _ = opt.map(|x| {
-   |  _____________^
-LL | |         x + 1
-LL | |     }
-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
-   |
-LL |       let _ = opt.map(|x| x + 1)
-   |  _____________^
-LL | |         .unwrap_or_else(||
-LL | |             0
-LL | |         );
-   | |_________^
-
-error: aborting due to 10 previous errors
-
diff --git a/tests/ui/result_map_unwrap_or_else.rs b/tests/ui/result_map_unwrap_or_else.rs
deleted file mode 100644 (file)
index 40751bf..0000000
+++ /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<i32, ()> = 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 (file)
index ec7bc8f..0000000
+++ /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 (file)
index 0000000..ee2cbc3
--- /dev/null
@@ -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::<Vec<_>>();
+
+    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 (file)
index 0000000..6ed5ca6
--- /dev/null
@@ -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::<Vec<_>>();
+
+    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 (file)
index 0000000..97933b8
--- /dev/null
@@ -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::<Vec<_>>();
+   |             ^^^^^^^^^^^^
+   |
+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::<Vec<_>>();
+   |             ^^^^^^^^^^^^^^^^^^
+
+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 (file)
index 0000000..f1503ed
--- /dev/null
@@ -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 (file)
index 0000000..a733788
--- /dev/null
@@ -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 (file)
index 0000000..e89e040
--- /dev/null
@@ -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 (file)
index 0000000..c4c5722
--- /dev/null
@@ -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 (file)
index 0000000..30095d2
--- /dev/null
@@ -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 (file)
index 0000000..c9ca4c4
--- /dev/null
@@ -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 (file)
index 0000000..12e5483
--- /dev/null
@@ -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
+
index 3f63624720f7553276aea8c6f721bed2c573066e..07f2791786d7f48711378386377019c698b33af4 100644 (file)
 
 struct Unitter;
 impl Unitter {
-    // try to disorient the lint with multiple unit returns and newlines
     #[allow(clippy::no_effect)]
-    pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) 
-    where G: Fn() -> () {
-        let _y: &dyn Fn() -> () = &f;
+    pub fn get_unit<F: Fn() , 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
     }
 }
@@ -30,6 +29,20 @@ impl Into<()> for Unitter {
     }
 }
 
+trait Trait {
+    fn redundant<F: FnOnce() , G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() ,
+        H: Fn() ;
+}
+
+impl Trait for Unitter {
+    fn redundant<F: FnOnce() , G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() ,
+        H: Fn()  {}
+}
+
 fn return_unit()  {  }
 
 #[allow(clippy::needless_return)]
index 8fc072ebd69f848ffc41dd09306febf07e459a96..e2c6afb020f5887dc175b3f8a832fb8293173637 100644 (file)
 
 struct Unitter;
 impl Unitter {
-    // try to disorient the lint with multiple unit returns and newlines
     #[allow(clippy::no_effect)]
-    pub fn get_unit<F: Fn() -> (), G>(&self, f: F, _g: G) ->
-        ()
+    pub fn get_unit<F: Fn() -> (), 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 @@ fn into(self) -> () {
     }
 }
 
+trait Trait {
+    fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() -> (),
+        H: Fn() -> ();
+}
+
+impl Trait for Unitter {
+    fn redundant<F: FnOnce() -> (), G, H>(&self, _f: F, _g: G, _h: H)
+    where
+        G: FnMut() -> (),
+        H: Fn() -> () {}
+}
+
 fn return_unit() -> () { () }
 
 #[allow(clippy::needless_return)]
index a013d2b3495ba40a4d824e042e71ce6f3d7e873e..81e6738e6bf67b8cdec71e21fc20b5027cfe19b4 100644 (file)
@@ -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<F: Fn() -> (), G>(&self, f: F, _g: G) ->
-   |  ___________________________________________________________^
-LL | |         ()
-   | |__________^ help: remove the `-> ()`
+LL |     pub fn get_unit<F: Fn() -> (), 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<F: Fn() -> (), 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<F: FnOnce() -> (), 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<F: FnOnce() -> (), 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
 
index fcd1fcd14d48f67590a89b0c33848af7d96f5dd9..a4a3cd1d37977d3dcb16181db4ad8c20a54ffaab 100644 (file)
@@ -1,4 +1,4 @@
-#![warn(clippy::option_unwrap_used, clippy::result_unwrap_used)]
+#![warn(clippy::unwrap_used)]
 
 fn unwrap_option() {
     let opt = Some(0);
index b90ce68fa97ac76c064465d579c2b85e315cb9af..4f0858005f6e7f140cd0c69bcea3c9111e82b673 100644 (file)
@@ -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/useless_conversion.fixed b/tests/ui/useless_conversion.fixed
new file mode 100644 (file)
index 0000000..fdd4bc5
--- /dev/null
@@ -0,0 +1,58 @@
+// run-rustfix
+
+#![deny(clippy::useless_conversion)]
+
+fn test_generic<T: Copy>(val: T) -> T {
+    let _ = val;
+    val
+}
+
+fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
+    // ok
+    let _: i32 = val.into();
+    let _: U = val.into();
+    let _ = U::from(val);
+}
+
+fn test_questionmark() -> Result<(), ()> {
+    {
+        let _: i32 = 0i32;
+        Ok(Ok(()))
+    }??;
+    Ok(())
+}
+
+fn test_issue_3913() -> Result<(), std::io::Error> {
+    use std::fs;
+    use std::path::Path;
+
+    let path = Path::new(".");
+    for _ in fs::read_dir(path)? {}
+
+    Ok(())
+}
+
+fn main() {
+    test_generic(10i32);
+    test_generic2::<i32, i32>(10i32);
+    test_questionmark().unwrap();
+    test_issue_3913().unwrap();
+
+    let _: String = "foo".into();
+    let _: String = From::from("foo");
+    let _ = String::from("foo");
+    #[allow(clippy::useless_conversion)]
+    {
+        let _: String = "foo".into();
+        let _ = String::from("foo");
+        let _ = "".lines().into_iter();
+    }
+
+    let _: String = "foo".to_string();
+    let _: String = "foo".to_string();
+    let _ = "foo".to_string();
+    let _ = format!("A: {:04}", 123);
+    let _ = "".lines();
+    let _ = vec![1, 2, 3].into_iter();
+    let _: String = format!("Hello {}", "world");
+}
diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs
new file mode 100644 (file)
index 0000000..4cae745
--- /dev/null
@@ -0,0 +1,58 @@
+// run-rustfix
+
+#![deny(clippy::useless_conversion)]
+
+fn test_generic<T: Copy>(val: T) -> T {
+    let _ = T::from(val);
+    val.into()
+}
+
+fn test_generic2<T: Copy + Into<i32> + Into<U>, U: From<T>>(val: T) {
+    // ok
+    let _: i32 = val.into();
+    let _: U = val.into();
+    let _ = U::from(val);
+}
+
+fn test_questionmark() -> Result<(), ()> {
+    {
+        let _: i32 = 0i32.into();
+        Ok(Ok(()))
+    }??;
+    Ok(())
+}
+
+fn test_issue_3913() -> Result<(), std::io::Error> {
+    use std::fs;
+    use std::path::Path;
+
+    let path = Path::new(".");
+    for _ in fs::read_dir(path)? {}
+
+    Ok(())
+}
+
+fn main() {
+    test_generic(10i32);
+    test_generic2::<i32, i32>(10i32);
+    test_questionmark().unwrap();
+    test_issue_3913().unwrap();
+
+    let _: String = "foo".into();
+    let _: String = From::from("foo");
+    let _ = String::from("foo");
+    #[allow(clippy::useless_conversion)]
+    {
+        let _: String = "foo".into();
+        let _ = String::from("foo");
+        let _ = "".lines().into_iter();
+    }
+
+    let _: String = "foo".to_string().into();
+    let _: String = From::from("foo".to_string());
+    let _ = String::from("foo".to_string());
+    let _ = String::from(format!("A: {:04}", 123));
+    let _ = "".lines().into_iter();
+    let _ = vec![1, 2, 3].into_iter().into_iter();
+    let _: String = format!("Hello {}", "world").into();
+}
diff --git a/tests/ui/useless_conversion.stderr b/tests/ui/useless_conversion.stderr
new file mode 100644 (file)
index 0000000..7df3507
--- /dev/null
@@ -0,0 +1,68 @@
+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/useless_conversion.rs:3:9
+   |
+LL | #![deny(clippy::useless_conversion)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: useless conversion
+  --> $DIR/useless_conversion.rs:7:5
+   |
+LL |     val.into()
+   |     ^^^^^^^^^^ help: consider removing `.into()`: `val`
+
+error: useless conversion
+  --> $DIR/useless_conversion.rs:19:22
+   |
+LL |         let _: i32 = 0i32.into();
+   |                      ^^^^^^^^^^^ help: consider removing `.into()`: `0i32`
+
+error: useless conversion
+  --> $DIR/useless_conversion.rs:51:21
+   |
+LL |     let _: String = "foo".to_string().into();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `"foo".to_string()`
+
+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: useless conversion
+  --> $DIR/useless_conversion.rs:53:13
+   |
+LL |     let _ = String::from("foo".to_string());
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `String::from()`: `"foo".to_string()`
+
+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: useless conversion
+  --> $DIR/useless_conversion.rs:55:13
+   |
+LL |     let _ = "".lines().into_iter();
+   |             ^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into_iter()`: `"".lines()`
+
+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: useless conversion
+  --> $DIR/useless_conversion.rs:57:21
+   |
+LL |     let _: String = format!("Hello {}", "world").into();
+   |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `.into()`: `format!("Hello {}", "world")`
+
+error: aborting due to 10 previous errors
+