From adcaa1b86ddbf51659d5f7e4e7471427682649ba Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 2 Apr 2020 18:22:18 -0700 Subject: [PATCH 01/28] Downgrade let_unit_value to pedantic --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/types.rs | 2 +- src/lintlist/mod.rs | 2 +- tests/ui/doc_unsafe.rs | 1 - tests/ui/redundant_pattern_matching.fixed | 2 +- tests/ui/redundant_pattern_matching.rs | 2 +- tests/ui/uninit.rs | 1 - tests/ui/uninit.stderr | 4 ++-- 8 files changed, 7 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..84b89311fa0b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1125,6 +1125,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::CAST_PRECISION_LOSS), LintId::of(&types::CAST_SIGN_LOSS), LintId::of(&types::INVALID_UPCAST_COMPARISONS), + LintId::of(&types::LET_UNIT_VALUE), LintId::of(&types::LINKEDLIST), LintId::of(&types::OPTION_OPTION), LintId::of(&unicode::NON_ASCII_LITERAL), @@ -1376,7 +1377,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::FN_TO_NUMERIC_CAST), LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&types::IMPLICIT_HASHER), - LintId::of(&types::LET_UNIT_VALUE), LintId::of(&types::REDUNDANT_ALLOCATION), LintId::of(&types::TYPE_COMPLEXITY), LintId::of(&types::UNIT_ARG), @@ -1489,7 +1489,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&types::FN_TO_NUMERIC_CAST), LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION), LintId::of(&types::IMPLICIT_HASHER), - LintId::of(&types::LET_UNIT_VALUE), LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME), LintId::of(&write::PRINTLN_EMPTY_STRING), LintId::of(&write::PRINT_LITERAL), diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 8c151f2277c1..171852d41e36 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -593,7 +593,7 @@ declare_clippy_lint! { /// }; /// ``` pub LET_UNIT_VALUE, - style, + pedantic, "creating a `let` binding to a value of unit type, which usually can't be used afterwards" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8a6d0af5f8a7..79e337f55307 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -999,7 +999,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "let_unit_value", - group: "style", + group: "pedantic", desc: "creating a `let` binding to a value of unit type, which usually can\'t be used afterwards", deprecation: None, module: "types", diff --git a/tests/ui/doc_unsafe.rs b/tests/ui/doc_unsafe.rs index c44f3c62a98e..484aa72d59a2 100644 --- a/tests/ui/doc_unsafe.rs +++ b/tests/ui/doc_unsafe.rs @@ -88,7 +88,6 @@ very_unsafe!(); // we don't lint code from external macros undocd_unsafe!(); -#[allow(clippy::let_unit_value)] fn main() { unsafe { you_dont_see_me(); diff --git a/tests/ui/redundant_pattern_matching.fixed b/tests/ui/redundant_pattern_matching.fixed index 776c9444566b..538fa1ed9cb0 100644 --- a/tests/ui/redundant_pattern_matching.fixed +++ b/tests/ui/redundant_pattern_matching.fixed @@ -2,7 +2,7 @@ #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::unit_arg, clippy::let_unit_value, unused_must_use)] +#![allow(clippy::unit_arg, unused_must_use)] fn main() { Ok::(42).is_ok(); diff --git a/tests/ui/redundant_pattern_matching.rs b/tests/ui/redundant_pattern_matching.rs index 2b2d5b1c1ec6..34d2cd62e54e 100644 --- a/tests/ui/redundant_pattern_matching.rs +++ b/tests/ui/redundant_pattern_matching.rs @@ -2,7 +2,7 @@ #![warn(clippy::all)] #![warn(clippy::redundant_pattern_matching)] -#![allow(clippy::unit_arg, clippy::let_unit_value, unused_must_use)] +#![allow(clippy::unit_arg, unused_must_use)] fn main() { if let Ok(_) = Ok::(42) {} diff --git a/tests/ui/uninit.rs b/tests/ui/uninit.rs index a4424c490e70..f42b884e0f0e 100644 --- a/tests/ui/uninit.rs +++ b/tests/ui/uninit.rs @@ -2,7 +2,6 @@ use std::mem::MaybeUninit; -#[allow(clippy::let_unit_value)] fn main() { let _: usize = unsafe { MaybeUninit::uninit().assume_init() }; diff --git a/tests/ui/uninit.stderr b/tests/ui/uninit.stderr index f4c45354aefe..a37233ecddae 100644 --- a/tests/ui/uninit.stderr +++ b/tests/ui/uninit.stderr @@ -1,5 +1,5 @@ error: this call for this type may be undefined behavior - --> $DIR/uninit.rs:7:29 + --> $DIR/uninit.rs:6:29 | LL | let _: usize = unsafe { MaybeUninit::uninit().assume_init() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | let _: usize = unsafe { MaybeUninit::uninit().assume_init() }; = note: `#[deny(clippy::uninit_assumed_init)]` on by default error: this call for this type may be undefined behavior - --> $DIR/uninit.rs:10:31 + --> $DIR/uninit.rs:9:31 | LL | let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 94154cad20d4687461fcbb4901a1252576329d13 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 2 Apr 2020 18:46:01 -0700 Subject: [PATCH 02/28] Downgrade trivially_copy_pass_by_ref to pedantic --- clippy_lints/src/lib.rs | 3 +- .../src/trivially_copy_pass_by_ref.rs | 2 +- src/lintlist/mod.rs | 2 +- tests/ui-toml/toml_trivially_copy/test.rs | 1 + tests/ui-toml/toml_trivially_copy/test.stderr | 10 ++++-- tests/ui/clone_on_copy_mut.rs | 1 - tests/ui/debug_assert_with_mut_call.rs | 2 +- tests/ui/eta.fixed | 3 +- tests/ui/eta.rs | 3 +- tests/ui/eta.stderr | 24 ++++++------- tests/ui/extra_unused_lifetimes.rs | 3 +- tests/ui/extra_unused_lifetimes.stderr | 8 ++--- tests/ui/float_arithmetic.rs | 3 +- tests/ui/float_arithmetic.stderr | 34 +++++++++--------- tests/ui/infinite_iter.rs | 1 - tests/ui/infinite_iter.stderr | 32 ++++++++--------- tests/ui/infinite_loop.rs | 2 -- tests/ui/infinite_loop.stderr | 22 ++++++------ tests/ui/integer_arithmetic.rs | 3 +- tests/ui/integer_arithmetic.stderr | 34 +++++++++--------- tests/ui/mut_from_ref.rs | 2 +- tests/ui/mut_reference.rs | 2 +- tests/ui/needless_borrow.fixed | 1 - tests/ui/needless_borrow.rs | 1 - tests/ui/needless_borrow.stderr | 8 ++--- tests/ui/needless_lifetimes.rs | 2 +- tests/ui/new_ret_no_self.rs | 2 +- tests/ui/trivially_copy_pass_by_ref.rs | 1 + tests/ui/trivially_copy_pass_by_ref.stderr | 36 ++++++++++--------- tests/ui/useless_asref.fixed | 1 - tests/ui/useless_asref.rs | 1 - tests/ui/useless_asref.stderr | 22 ++++++------ tests/ui/wrong_self_convention.rs | 2 +- 33 files changed, 135 insertions(+), 139 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..ece9efded461 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1119,6 +1119,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&shadow::SHADOW_UNRELATED), LintId::of(&strings::STRING_ADD_ASSIGN), LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS), + LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&types::CAST_LOSSLESS), LintId::of(&types::CAST_POSSIBLE_TRUNCATION), LintId::of(&types::CAST_POSSIBLE_WRAP), @@ -1365,7 +1366,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(&transmute::WRONG_TRANSMUTE), LintId::of(&transmuting_null::TRANSMUTING_NULL), - LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&try_err::TRY_ERR), LintId::of(&types::ABSURD_EXTREME_COMPARISONS), LintId::of(&types::BORROWED_BOX), @@ -1660,7 +1660,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&mutex_atomic::MUTEX_ATOMIC), LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), - LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&types::BOX_VEC), LintId::of(&types::REDUNDANT_ALLOCATION), LintId::of(&vec::USELESS_VEC), diff --git a/clippy_lints/src/trivially_copy_pass_by_ref.rs b/clippy_lints/src/trivially_copy_pass_by_ref.rs index 52b07fb34017..2c101220c5d6 100644 --- a/clippy_lints/src/trivially_copy_pass_by_ref.rs +++ b/clippy_lints/src/trivially_copy_pass_by_ref.rs @@ -49,7 +49,7 @@ declare_clippy_lint! { /// fn foo(v: u32) {} /// ``` pub TRIVIALLY_COPY_PASS_BY_REF, - perf, + pedantic, "functions taking small copyable arguments by reference" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8a6d0af5f8a7..62e7fe42110f 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2161,7 +2161,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "trivially_copy_pass_by_ref", - group: "perf", + group: "pedantic", desc: "functions taking small copyable arguments by reference", deprecation: None, module: "trivially_copy_pass_by_ref", diff --git a/tests/ui-toml/toml_trivially_copy/test.rs b/tests/ui-toml/toml_trivially_copy/test.rs index 6dcbae040750..19019a254163 100644 --- a/tests/ui-toml/toml_trivially_copy/test.rs +++ b/tests/ui-toml/toml_trivially_copy/test.rs @@ -1,6 +1,7 @@ // normalize-stderr-test "\(\d+ byte\)" -> "(N byte)" // normalize-stderr-test "\(limit: \d+ byte\)" -> "(limit: N byte)" +#![deny(clippy::trivially_copy_pass_by_ref)] #![allow(clippy::many_single_char_names)] #[derive(Copy, Clone)] diff --git a/tests/ui-toml/toml_trivially_copy/test.stderr b/tests/ui-toml/toml_trivially_copy/test.stderr index d2b55eff16db..912761a8f009 100644 --- a/tests/ui-toml/toml_trivially_copy/test.stderr +++ b/tests/ui-toml/toml_trivially_copy/test.stderr @@ -1,13 +1,17 @@ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/test.rs:14:11 + --> $DIR/test.rs:15:11 | LL | fn bad(x: &u16, y: &Foo) {} | ^^^^ help: consider passing by value instead: `u16` | - = note: `-D clippy::trivially-copy-pass-by-ref` implied by `-D warnings` +note: the lint level is defined here + --> $DIR/test.rs:4:9 + | +LL | #![deny(clippy::trivially_copy_pass_by_ref)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/test.rs:14:20 + --> $DIR/test.rs:15:20 | LL | fn bad(x: &u16, y: &Foo) {} | ^^^^ help: consider passing by value instead: `Foo` diff --git a/tests/ui/clone_on_copy_mut.rs b/tests/ui/clone_on_copy_mut.rs index 3cbbcb7c0830..5bfa256623b6 100644 --- a/tests/ui/clone_on_copy_mut.rs +++ b/tests/ui/clone_on_copy_mut.rs @@ -5,7 +5,6 @@ pub fn dec_read_dec(i: &mut i32) -> i32 { ret } -#[allow(clippy::trivially_copy_pass_by_ref)] pub fn minus_1(i: &i32) -> i32 { dec_read_dec(&mut i.clone()) } diff --git a/tests/ui/debug_assert_with_mut_call.rs b/tests/ui/debug_assert_with_mut_call.rs index 3db7e0164fa4..b061fff6b9e9 100644 --- a/tests/ui/debug_assert_with_mut_call.rs +++ b/tests/ui/debug_assert_with_mut_call.rs @@ -2,7 +2,7 @@ #![feature(custom_inner_attributes)] #![rustfmt::skip] #![warn(clippy::debug_assert_with_mut_call)] -#![allow(clippy::trivially_copy_pass_by_ref, clippy::cognitive_complexity, clippy::redundant_closure_call)] +#![allow(clippy::cognitive_complexity, clippy::redundant_closure_call)] struct S; diff --git a/tests/ui/eta.fixed b/tests/ui/eta.fixed index 5d62a6d9b01e..1b34c2f74eba 100644 --- a/tests/ui/eta.fixed +++ b/tests/ui/eta.fixed @@ -6,8 +6,7 @@ clippy::redundant_closure_call, clippy::many_single_char_names, clippy::needless_pass_by_value, - clippy::option_map_unit_fn, - clippy::trivially_copy_pass_by_ref + clippy::option_map_unit_fn )] #![warn( clippy::redundant_closure, diff --git a/tests/ui/eta.rs b/tests/ui/eta.rs index a9c4b209960c..4f050bd8479a 100644 --- a/tests/ui/eta.rs +++ b/tests/ui/eta.rs @@ -6,8 +6,7 @@ clippy::redundant_closure_call, clippy::many_single_char_names, clippy::needless_pass_by_value, - clippy::option_map_unit_fn, - clippy::trivially_copy_pass_by_ref + clippy::option_map_unit_fn )] #![warn( clippy::redundant_closure, diff --git a/tests/ui/eta.stderr b/tests/ui/eta.stderr index d19d21eec0db..c4713ca8083d 100644 --- a/tests/ui/eta.stderr +++ b/tests/ui/eta.stderr @@ -1,5 +1,5 @@ error: redundant closure found - --> $DIR/eta.rs:21:27 + --> $DIR/eta.rs:20:27 | LL | let a = Some(1u8).map(|a| foo(a)); | ^^^^^^^^^^ help: remove closure as shown: `foo` @@ -7,13 +7,13 @@ LL | let a = Some(1u8).map(|a| foo(a)); = note: `-D clippy::redundant-closure` implied by `-D warnings` error: redundant closure found - --> $DIR/eta.rs:22:10 + --> $DIR/eta.rs:21:10 | LL | meta(|a| foo(a)); | ^^^^^^^^^^ help: remove closure as shown: `foo` error: this expression borrows a reference that is immediately dereferenced by the compiler - --> $DIR/eta.rs:25:21 + --> $DIR/eta.rs:24:21 | LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted | ^^^ help: change this to: `&2` @@ -21,13 +21,13 @@ LL | all(&[1, 2, 3], &&2, |x, y| below(x, y)); //is adjusted = note: `-D clippy::needless-borrow` implied by `-D warnings` error: redundant closure found - --> $DIR/eta.rs:32:27 + --> $DIR/eta.rs:31:27 | LL | let e = Some(1u8).map(|a| generic(a)); | ^^^^^^^^^^^^^^ help: remove closure as shown: `generic` error: redundant closure found - --> $DIR/eta.rs:75:51 + --> $DIR/eta.rs:74:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); | ^^^^^^^^^^^ help: remove closure as shown: `TestStruct::foo` @@ -35,43 +35,43 @@ LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.foo()); = note: `-D clippy::redundant-closure-for-method-calls` implied by `-D warnings` error: redundant closure found - --> $DIR/eta.rs:77:51 + --> $DIR/eta.rs:76:51 | LL | let e = Some(TestStruct { some_ref: &i }).map(|a| a.trait_foo()); | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `TestTrait::trait_foo` error: redundant closure found - --> $DIR/eta.rs:80:42 + --> $DIR/eta.rs:79:42 | LL | let e = Some(&mut vec![1, 2, 3]).map(|v| v.clear()); | ^^^^^^^^^^^^^ help: remove closure as shown: `std::vec::Vec::clear` error: redundant closure found - --> $DIR/eta.rs:85:29 + --> $DIR/eta.rs:84:29 | LL | let e = Some("str").map(|s| s.to_string()); | ^^^^^^^^^^^^^^^^^ help: remove closure as shown: `std::string::ToString::to_string` error: redundant closure found - --> $DIR/eta.rs:87:27 + --> $DIR/eta.rs:86:27 | LL | let e = Some('a').map(|s| s.to_uppercase()); | ^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_uppercase` error: redundant closure found - --> $DIR/eta.rs:90:65 + --> $DIR/eta.rs:89:65 | LL | let e: std::vec::Vec = vec!['a', 'b', 'c'].iter().map(|c| c.to_ascii_uppercase()).collect(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `char::to_ascii_uppercase` error: redundant closure found - --> $DIR/eta.rs:173:27 + --> $DIR/eta.rs:172:27 | LL | let a = Some(1u8).map(|a| foo_ptr(a)); | ^^^^^^^^^^^^^^ help: remove closure as shown: `foo_ptr` error: redundant closure found - --> $DIR/eta.rs:178:27 + --> $DIR/eta.rs:177:27 | LL | let a = Some(1u8).map(|a| closure(a)); | ^^^^^^^^^^^^^^ help: remove closure as shown: `closure` diff --git a/tests/ui/extra_unused_lifetimes.rs b/tests/ui/extra_unused_lifetimes.rs index ba95fd63bf9a..26df71ddcb0f 100644 --- a/tests/ui/extra_unused_lifetimes.rs +++ b/tests/ui/extra_unused_lifetimes.rs @@ -2,8 +2,7 @@ unused, dead_code, clippy::needless_lifetimes, - clippy::needless_pass_by_value, - clippy::trivially_copy_pass_by_ref + clippy::needless_pass_by_value )] #![warn(clippy::extra_unused_lifetimes)] diff --git a/tests/ui/extra_unused_lifetimes.stderr b/tests/ui/extra_unused_lifetimes.stderr index ebdb8e749520..e997951346f7 100644 --- a/tests/ui/extra_unused_lifetimes.stderr +++ b/tests/ui/extra_unused_lifetimes.stderr @@ -1,5 +1,5 @@ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:14:14 + --> $DIR/extra_unused_lifetimes.rs:13:14 | LL | fn unused_lt<'a>(x: u8) {} | ^^ @@ -7,19 +7,19 @@ LL | fn unused_lt<'a>(x: u8) {} = note: `-D clippy::extra-unused-lifetimes` implied by `-D warnings` error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:16:25 + --> $DIR/extra_unused_lifetimes.rs:15:25 | LL | fn unused_lt_transitive<'a, 'b: 'a>(x: &'b u8) { | ^^ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:41:10 + --> $DIR/extra_unused_lifetimes.rs:40:10 | LL | fn x<'a>(&self) {} | ^^ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:67:22 + --> $DIR/extra_unused_lifetimes.rs:66:22 | LL | fn unused_lt<'a>(x: u8) {} | ^^ diff --git a/tests/ui/float_arithmetic.rs b/tests/ui/float_arithmetic.rs index 5ad320c62095..60fa7569eb9d 100644 --- a/tests/ui/float_arithmetic.rs +++ b/tests/ui/float_arithmetic.rs @@ -5,8 +5,7 @@ clippy::shadow_unrelated, clippy::no_effect, clippy::unnecessary_operation, - clippy::op_ref, - clippy::trivially_copy_pass_by_ref + clippy::op_ref )] #[rustfmt::skip] diff --git a/tests/ui/float_arithmetic.stderr b/tests/ui/float_arithmetic.stderr index 809392529fd9..1ceffb35beed 100644 --- a/tests/ui/float_arithmetic.stderr +++ b/tests/ui/float_arithmetic.stderr @@ -1,5 +1,5 @@ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:16:5 + --> $DIR/float_arithmetic.rs:15:5 | LL | f * 2.0; | ^^^^^^^ @@ -7,97 +7,97 @@ LL | f * 2.0; = note: `-D clippy::float-arithmetic` implied by `-D warnings` error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:18:5 + --> $DIR/float_arithmetic.rs:17:5 | LL | 1.0 + f; | ^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:19:5 + --> $DIR/float_arithmetic.rs:18:5 | LL | f * 2.0; | ^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:20:5 + --> $DIR/float_arithmetic.rs:19:5 | LL | f / 2.0; | ^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:21:5 + --> $DIR/float_arithmetic.rs:20:5 | LL | f - 2.0 * 4.2; | ^^^^^^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:22:5 + --> $DIR/float_arithmetic.rs:21:5 | LL | -f; | ^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:24:5 + --> $DIR/float_arithmetic.rs:23:5 | LL | f += 1.0; | ^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:25:5 + --> $DIR/float_arithmetic.rs:24:5 | LL | f -= 1.0; | ^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:26:5 + --> $DIR/float_arithmetic.rs:25:5 | LL | f *= 2.0; | ^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:27:5 + --> $DIR/float_arithmetic.rs:26:5 | LL | f /= 2.0; | ^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:33:5 + --> $DIR/float_arithmetic.rs:32:5 | LL | 3.1_f32 + &1.2_f32; | ^^^^^^^^^^^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:34:5 + --> $DIR/float_arithmetic.rs:33:5 | LL | &3.4_f32 + 1.5_f32; | ^^^^^^^^^^^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:35:5 + --> $DIR/float_arithmetic.rs:34:5 | LL | &3.5_f32 + &1.3_f32; | ^^^^^^^^^^^^^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:40:5 + --> $DIR/float_arithmetic.rs:39:5 | LL | a + f | ^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:44:5 + --> $DIR/float_arithmetic.rs:43:5 | LL | f1 + f2 | ^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:48:5 + --> $DIR/float_arithmetic.rs:47:5 | LL | f1 + f2 | ^^^^^^^ error: floating-point arithmetic detected - --> $DIR/float_arithmetic.rs:52:5 + --> $DIR/float_arithmetic.rs:51:5 | LL | (&f1 + &f2) | ^^^^^^^^^^^ diff --git a/tests/ui/infinite_iter.rs b/tests/ui/infinite_iter.rs index c324eb957776..1fe688977659 100644 --- a/tests/ui/infinite_iter.rs +++ b/tests/ui/infinite_iter.rs @@ -1,5 +1,4 @@ use std::iter::repeat; -#[allow(clippy::trivially_copy_pass_by_ref)] fn square_is_lower_64(x: &u32) -> bool { x * x < 64 } diff --git a/tests/ui/infinite_iter.stderr b/tests/ui/infinite_iter.stderr index 4750316d3f4e..5f5e7ac9f253 100644 --- a/tests/ui/infinite_iter.stderr +++ b/tests/ui/infinite_iter.stderr @@ -1,29 +1,29 @@ error: infinite iteration detected - --> $DIR/infinite_iter.rs:10:5 + --> $DIR/infinite_iter.rs:9:5 | LL | repeat(0_u8).collect::>(); // infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/infinite_iter.rs:8:8 + --> $DIR/infinite_iter.rs:7:8 | LL | #[deny(clippy::infinite_iter)] | ^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> $DIR/infinite_iter.rs:11:5 + --> $DIR/infinite_iter.rs:10:5 | LL | (0..8_u32).take_while(square_is_lower_64).cycle().count(); // infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> $DIR/infinite_iter.rs:12:5 + --> $DIR/infinite_iter.rs:11:5 | LL | (0..8_u64).chain(0..).max(); // infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> $DIR/infinite_iter.rs:17:5 + --> $DIR/infinite_iter.rs:16:5 | LL | / (0..8_u32) LL | | .rev() @@ -33,37 +33,37 @@ LL | | .for_each(|x| println!("{}", x)); // infinite iter | |________________________________________^ error: infinite iteration detected - --> $DIR/infinite_iter.rs:23:5 + --> $DIR/infinite_iter.rs:22:5 | LL | (0_usize..).flat_map(|x| 0..x).product::(); // infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> $DIR/infinite_iter.rs:24:5 + --> $DIR/infinite_iter.rs:23:5 | LL | (0_u64..).filter(|x| x % 2 == 0).last(); // infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:31:5 + --> $DIR/infinite_iter.rs:30:5 | LL | (0..).zip((0..).take_while(square_is_lower_64)).count(); // maybe infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | note: the lint level is defined here - --> $DIR/infinite_iter.rs:29:8 + --> $DIR/infinite_iter.rs:28:8 | LL | #[deny(clippy::maybe_infinite_iter)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:32:5 + --> $DIR/infinite_iter.rs:31:5 | LL | repeat(42).take_while(|x| *x == 42).chain(0..42).max(); // maybe infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:33:5 + --> $DIR/infinite_iter.rs:32:5 | LL | / (1..) LL | | .scan(0, |state, x| { @@ -74,31 +74,31 @@ LL | | .min(); // maybe infinite iter | |______________^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:39:5 + --> $DIR/infinite_iter.rs:38:5 | LL | (0..).find(|x| *x == 24); // maybe infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:40:5 + --> $DIR/infinite_iter.rs:39:5 | LL | (0..).position(|x| x == 24); // maybe infinite iter | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:41:5 + --> $DIR/infinite_iter.rs:40:5 | LL | (0..).any(|x| x == 24); // maybe infinite iter | ^^^^^^^^^^^^^^^^^^^^^^ error: possible infinite iteration detected - --> $DIR/infinite_iter.rs:42:5 + --> $DIR/infinite_iter.rs:41:5 | LL | (0..).all(|x| x == 24); // maybe infinite iter | ^^^^^^^^^^^^^^^^^^^^^^ error: infinite iteration detected - --> $DIR/infinite_iter.rs:65:31 + --> $DIR/infinite_iter.rs:64:31 | LL | let _: HashSet = (0..).collect(); // Infinite iter | ^^^^^^^^^^^^^^^ diff --git a/tests/ui/infinite_loop.rs b/tests/ui/infinite_loop.rs index 09f47adc46e6..72591f12baf8 100644 --- a/tests/ui/infinite_loop.rs +++ b/tests/ui/infinite_loop.rs @@ -1,5 +1,3 @@ -#![allow(clippy::trivially_copy_pass_by_ref)] - fn fn_val(i: i32) -> i32 { unimplemented!() } diff --git a/tests/ui/infinite_loop.stderr b/tests/ui/infinite_loop.stderr index 2736753c14b6..1fcb29eff18e 100644 --- a/tests/ui/infinite_loop.stderr +++ b/tests/ui/infinite_loop.stderr @@ -1,5 +1,5 @@ error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:23:11 + --> $DIR/infinite_loop.rs:21:11 | LL | while y < 10 { | ^^^^^^ @@ -8,7 +8,7 @@ LL | while y < 10 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:28:11 + --> $DIR/infinite_loop.rs:26:11 | LL | while y < 10 && x < 3 { | ^^^^^^^^^^^^^^^ @@ -16,7 +16,7 @@ LL | while y < 10 && x < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:35:11 + --> $DIR/infinite_loop.rs:33:11 | LL | while !cond { | ^^^^^ @@ -24,7 +24,7 @@ LL | while !cond { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:79:11 + --> $DIR/infinite_loop.rs:77:11 | LL | while i < 3 { | ^^^^^ @@ -32,7 +32,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:84:11 + --> $DIR/infinite_loop.rs:82:11 | LL | while i < 3 && j > 0 { | ^^^^^^^^^^^^^^ @@ -40,7 +40,7 @@ LL | while i < 3 && j > 0 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:88:11 + --> $DIR/infinite_loop.rs:86:11 | LL | while i < 3 { | ^^^^^ @@ -48,7 +48,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:103:11 + --> $DIR/infinite_loop.rs:101:11 | LL | while i < 3 { | ^^^^^ @@ -56,7 +56,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:108:11 + --> $DIR/infinite_loop.rs:106:11 | LL | while i < 3 { | ^^^^^ @@ -64,7 +64,7 @@ LL | while i < 3 { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:174:15 + --> $DIR/infinite_loop.rs:172:15 | LL | while self.count < n { | ^^^^^^^^^^^^^^ @@ -72,7 +72,7 @@ LL | while self.count < n { = note: this may lead to an infinite or to a never running loop error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:182:11 + --> $DIR/infinite_loop.rs:180:11 | LL | while y < 10 { | ^^^^^^ @@ -82,7 +82,7 @@ LL | while y < 10 { = help: rewrite it as `if cond { loop { } }` error: variables in the condition are not mutated in the loop body - --> $DIR/infinite_loop.rs:189:11 + --> $DIR/infinite_loop.rs:187:11 | LL | while y < 10 { | ^^^^^^ diff --git a/tests/ui/integer_arithmetic.rs b/tests/ui/integer_arithmetic.rs index 31a07e7c35b0..2fe32c6ace87 100644 --- a/tests/ui/integer_arithmetic.rs +++ b/tests/ui/integer_arithmetic.rs @@ -5,8 +5,7 @@ clippy::shadow_unrelated, clippy::no_effect, clippy::unnecessary_operation, - clippy::op_ref, - clippy::trivially_copy_pass_by_ref + clippy::op_ref )] #[rustfmt::skip] diff --git a/tests/ui/integer_arithmetic.stderr b/tests/ui/integer_arithmetic.stderr index 0b8d0b767bf8..64c44d7ecc7b 100644 --- a/tests/ui/integer_arithmetic.stderr +++ b/tests/ui/integer_arithmetic.stderr @@ -1,5 +1,5 @@ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:15:5 + --> $DIR/integer_arithmetic.rs:14:5 | LL | 1 + i; | ^^^^^ @@ -7,98 +7,98 @@ LL | 1 + i; = note: `-D clippy::integer-arithmetic` implied by `-D warnings` error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:16:5 + --> $DIR/integer_arithmetic.rs:15:5 | LL | i * 2; | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:17:5 + --> $DIR/integer_arithmetic.rs:16:5 | LL | / 1 % LL | | i / 2; // no error, this is part of the expression in the preceding line | |_________^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:19:5 + --> $DIR/integer_arithmetic.rs:18:5 | LL | i - 2 + 2 - i; | ^^^^^^^^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:20:5 + --> $DIR/integer_arithmetic.rs:19:5 | LL | -i; | ^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:32:5 + --> $DIR/integer_arithmetic.rs:31:5 | LL | i += 1; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:33:5 + --> $DIR/integer_arithmetic.rs:32:5 | LL | i -= 1; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:34:5 + --> $DIR/integer_arithmetic.rs:33:5 | LL | i *= 2; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:35:5 + --> $DIR/integer_arithmetic.rs:34:5 | LL | i /= 2; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:36:5 + --> $DIR/integer_arithmetic.rs:35:5 | LL | i %= 2; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:82:5 + --> $DIR/integer_arithmetic.rs:81:5 | LL | 3 + &1; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:83:5 + --> $DIR/integer_arithmetic.rs:82:5 | LL | &3 + 1; | ^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:84:5 + --> $DIR/integer_arithmetic.rs:83:5 | LL | &3 + &1; | ^^^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:89:5 + --> $DIR/integer_arithmetic.rs:88:5 | LL | a + x | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:93:5 + --> $DIR/integer_arithmetic.rs:92:5 | LL | x + y | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:97:5 + --> $DIR/integer_arithmetic.rs:96:5 | LL | x + y | ^^^^^ error: integer arithmetic detected - --> $DIR/integer_arithmetic.rs:101:5 + --> $DIR/integer_arithmetic.rs:100:5 | LL | (&x + &y) | ^^^^^^^^^ diff --git a/tests/ui/mut_from_ref.rs b/tests/ui/mut_from_ref.rs index 8f9ed7ed6374..a9a04c8f56b9 100644 --- a/tests/ui/mut_from_ref.rs +++ b/tests/ui/mut_from_ref.rs @@ -1,4 +1,4 @@ -#![allow(unused, clippy::trivially_copy_pass_by_ref)] +#![allow(unused)] #![warn(clippy::mut_from_ref)] struct Foo; diff --git a/tests/ui/mut_reference.rs b/tests/ui/mut_reference.rs index c4379e0ea1c4..73906121c402 100644 --- a/tests/ui/mut_reference.rs +++ b/tests/ui/mut_reference.rs @@ -1,4 +1,4 @@ -#![allow(unused_variables, clippy::trivially_copy_pass_by_ref)] +#![allow(unused_variables)] fn takes_an_immutable_reference(a: &i32) {} fn takes_a_mutable_reference(a: &mut i32) {} diff --git a/tests/ui/needless_borrow.fixed b/tests/ui/needless_borrow.fixed index 50f9b7c7ba63..5ae4a0e79b99 100644 --- a/tests/ui/needless_borrow.fixed +++ b/tests/ui/needless_borrow.fixed @@ -2,7 +2,6 @@ #![allow(clippy::needless_borrowed_reference)] -#[allow(clippy::trivially_copy_pass_by_ref)] fn x(y: &i32) -> i32 { *y } diff --git a/tests/ui/needless_borrow.rs b/tests/ui/needless_borrow.rs index 8677b957e4c3..1e281316c8a3 100644 --- a/tests/ui/needless_borrow.rs +++ b/tests/ui/needless_borrow.rs @@ -2,7 +2,6 @@ #![allow(clippy::needless_borrowed_reference)] -#[allow(clippy::trivially_copy_pass_by_ref)] fn x(y: &i32) -> i32 { *y } diff --git a/tests/ui/needless_borrow.stderr b/tests/ui/needless_borrow.stderr index 49df9cd072b3..0bfeda7914db 100644 --- a/tests/ui/needless_borrow.stderr +++ b/tests/ui/needless_borrow.stderr @@ -1,5 +1,5 @@ error: this expression borrows a reference that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:15:15 + --> $DIR/needless_borrow.rs:14:15 | LL | let c = x(&&a); | ^^^ help: change this to: `&a` @@ -7,19 +7,19 @@ LL | let c = x(&&a); = note: `-D clippy::needless-borrow` implied by `-D warnings` error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:22:17 + --> $DIR/needless_borrow.rs:21:17 | LL | if let Some(ref cake) = Some(&5) {} | ^^^^^^^^ help: change this to: `cake` error: this expression borrows a reference that is immediately dereferenced by the compiler - --> $DIR/needless_borrow.rs:29:15 + --> $DIR/needless_borrow.rs:28:15 | LL | 46 => &&a, | ^^^ help: change this to: `&a` error: this pattern creates a reference to a reference - --> $DIR/needless_borrow.rs:52:31 + --> $DIR/needless_borrow.rs:51:31 | LL | let _ = v.iter().filter(|&ref a| a.is_empty()); | ^^^^^ help: change this to: `a` diff --git a/tests/ui/needless_lifetimes.rs b/tests/ui/needless_lifetimes.rs index f3fdd48633f8..913cd004f19f 100644 --- a/tests/ui/needless_lifetimes.rs +++ b/tests/ui/needless_lifetimes.rs @@ -1,5 +1,5 @@ #![warn(clippy::needless_lifetimes)] -#![allow(dead_code, clippy::needless_pass_by_value, clippy::trivially_copy_pass_by_ref)] +#![allow(dead_code, clippy::needless_pass_by_value)] fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {} diff --git a/tests/ui/new_ret_no_self.rs b/tests/ui/new_ret_no_self.rs index a31f046c0841..35aaecc9ac42 100644 --- a/tests/ui/new_ret_no_self.rs +++ b/tests/ui/new_ret_no_self.rs @@ -1,5 +1,5 @@ #![warn(clippy::new_ret_no_self)] -#![allow(dead_code, clippy::trivially_copy_pass_by_ref)] +#![allow(dead_code)] fn main() {} diff --git a/tests/ui/trivially_copy_pass_by_ref.rs b/tests/ui/trivially_copy_pass_by_ref.rs index bd23aa99ceb0..316426f1cf18 100644 --- a/tests/ui/trivially_copy_pass_by_ref.rs +++ b/tests/ui/trivially_copy_pass_by_ref.rs @@ -1,6 +1,7 @@ // normalize-stderr-test "\(\d+ byte\)" -> "(N byte)" // normalize-stderr-test "\(limit: \d+ byte\)" -> "(limit: N byte)" +#![deny(clippy::trivially_copy_pass_by_ref)] #![allow( clippy::many_single_char_names, clippy::blacklisted_name, diff --git a/tests/ui/trivially_copy_pass_by_ref.stderr b/tests/ui/trivially_copy_pass_by_ref.stderr index 1addc3d7195d..be0914e4a794 100644 --- a/tests/ui/trivially_copy_pass_by_ref.stderr +++ b/tests/ui/trivially_copy_pass_by_ref.stderr @@ -1,91 +1,95 @@ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:50:11 + --> $DIR/trivially_copy_pass_by_ref.rs:51:11 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` | - = note: `-D clippy::trivially-copy-pass-by-ref` implied by `-D warnings` +note: the lint level is defined here + --> $DIR/trivially_copy_pass_by_ref.rs:4:9 + | +LL | #![deny(clippy::trivially_copy_pass_by_ref)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:50:20 + --> $DIR/trivially_copy_pass_by_ref.rs:51:20 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:50:29 + --> $DIR/trivially_copy_pass_by_ref.rs:51:29 | LL | fn bad(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:57:12 + --> $DIR/trivially_copy_pass_by_ref.rs:58:12 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^^ help: consider passing by value instead: `self` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:57:22 + --> $DIR/trivially_copy_pass_by_ref.rs:58:22 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:57:31 + --> $DIR/trivially_copy_pass_by_ref.rs:58:31 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:57:40 + --> $DIR/trivially_copy_pass_by_ref.rs:58:40 | LL | fn bad(&self, x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:59:16 + --> $DIR/trivially_copy_pass_by_ref.rs:60:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:59:25 + --> $DIR/trivially_copy_pass_by_ref.rs:60:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:59:34 + --> $DIR/trivially_copy_pass_by_ref.rs:60:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:71:16 + --> $DIR/trivially_copy_pass_by_ref.rs:72:16 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `u32` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:71:25 + --> $DIR/trivially_copy_pass_by_ref.rs:72:25 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:71:34 + --> $DIR/trivially_copy_pass_by_ref.rs:72:34 | LL | fn bad2(x: &u32, y: &Foo, z: &Baz) {} | ^^^^ help: consider passing by value instead: `Baz` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:75:34 + --> $DIR/trivially_copy_pass_by_ref.rs:76:34 | LL | fn trait_method(&self, _foo: &Foo); | ^^^^ help: consider passing by value instead: `Foo` error: this argument (N byte) is passed by reference, but would be more efficient if passed by value (limit: N byte) - --> $DIR/trivially_copy_pass_by_ref.rs:79:37 + --> $DIR/trivially_copy_pass_by_ref.rs:80:37 | LL | fn trait_method2(&self, _color: &Color); | ^^^^^^ help: consider passing by value instead: `Color` diff --git a/tests/ui/useless_asref.fixed b/tests/ui/useless_asref.fixed index c6fce5df210d..e356f13d087b 100644 --- a/tests/ui/useless_asref.fixed +++ b/tests/ui/useless_asref.fixed @@ -1,7 +1,6 @@ // run-rustfix #![deny(clippy::useless_asref)] -#![allow(clippy::trivially_copy_pass_by_ref)] use std::fmt::Debug; diff --git a/tests/ui/useless_asref.rs b/tests/ui/useless_asref.rs index 1d23760bd148..2a80291f5d83 100644 --- a/tests/ui/useless_asref.rs +++ b/tests/ui/useless_asref.rs @@ -1,7 +1,6 @@ // run-rustfix #![deny(clippy::useless_asref)] -#![allow(clippy::trivially_copy_pass_by_ref)] use std::fmt::Debug; diff --git a/tests/ui/useless_asref.stderr b/tests/ui/useless_asref.stderr index b21c67bb3645..5876b54aca8f 100644 --- a/tests/ui/useless_asref.stderr +++ b/tests/ui/useless_asref.stderr @@ -1,5 +1,5 @@ error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:44:18 + --> $DIR/useless_asref.rs:43:18 | LL | foo_rstr(rstr.as_ref()); | ^^^^^^^^^^^^^ help: try this: `rstr` @@ -11,61 +11,61 @@ LL | #![deny(clippy::useless_asref)] | ^^^^^^^^^^^^^^^^^^^^^ error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:46:20 + --> $DIR/useless_asref.rs:45:20 | LL | foo_rslice(rslice.as_ref()); | ^^^^^^^^^^^^^^^ help: try this: `rslice` error: this call to `as_mut` does nothing - --> $DIR/useless_asref.rs:50:21 + --> $DIR/useless_asref.rs:49:21 | LL | foo_mrslice(mrslice.as_mut()); | ^^^^^^^^^^^^^^^^ help: try this: `mrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:52:20 + --> $DIR/useless_asref.rs:51:20 | LL | foo_rslice(mrslice.as_ref()); | ^^^^^^^^^^^^^^^^ help: try this: `mrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:59:20 + --> $DIR/useless_asref.rs:58:20 | LL | foo_rslice(rrrrrslice.as_ref()); | ^^^^^^^^^^^^^^^^^^^ help: try this: `rrrrrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:61:18 + --> $DIR/useless_asref.rs:60:18 | LL | foo_rstr(rrrrrstr.as_ref()); | ^^^^^^^^^^^^^^^^^ help: try this: `rrrrrstr` error: this call to `as_mut` does nothing - --> $DIR/useless_asref.rs:66:21 + --> $DIR/useless_asref.rs:65:21 | LL | foo_mrslice(mrrrrrslice.as_mut()); | ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:68:20 + --> $DIR/useless_asref.rs:67:20 | LL | foo_rslice(mrrrrrslice.as_ref()); | ^^^^^^^^^^^^^^^^^^^^ help: try this: `mrrrrrslice` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:72:16 + --> $DIR/useless_asref.rs:71:16 | LL | foo_rrrrmr((&&&&MoreRef).as_ref()); | ^^^^^^^^^^^^^^^^^^^^^^ help: try this: `(&&&&MoreRef)` error: this call to `as_mut` does nothing - --> $DIR/useless_asref.rs:122:13 + --> $DIR/useless_asref.rs:121:13 | LL | foo_mrt(mrt.as_mut()); | ^^^^^^^^^^^^ help: try this: `mrt` error: this call to `as_ref` does nothing - --> $DIR/useless_asref.rs:124:12 + --> $DIR/useless_asref.rs:123:12 | LL | foo_rt(mrt.as_ref()); | ^^^^^^^^^^^^ help: try this: `mrt` diff --git a/tests/ui/wrong_self_convention.rs b/tests/ui/wrong_self_convention.rs index 7567fa7158cb..99652ca4470c 100644 --- a/tests/ui/wrong_self_convention.rs +++ b/tests/ui/wrong_self_convention.rs @@ -1,6 +1,6 @@ #![warn(clippy::wrong_self_convention)] #![warn(clippy::wrong_pub_self_convention)] -#![allow(dead_code, clippy::trivially_copy_pass_by_ref)] +#![allow(dead_code)] fn main() {} From e26ae7a0ff54cbb229790011df1031df68d258bb Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 2 Apr 2020 20:00:12 -0700 Subject: [PATCH 03/28] Downgrade inefficient_to_string to pedantic --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/methods/mod.rs | 2 +- src/lintlist/mod.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..7648225f9c65 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1105,6 +1105,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::FILTER_MAP), LintId::of(&methods::FILTER_MAP_NEXT), 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), @@ -1259,7 +1260,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::EXPECT_FUN_CALL), LintId::of(&methods::FILTER_NEXT), LintId::of(&methods::FLAT_MAP_IDENTITY), - LintId::of(&methods::INEFFICIENT_TO_STRING), LintId::of(&methods::INTO_ITER_ON_REF), LintId::of(&methods::ITERATOR_STEP_BY_ZERO), LintId::of(&methods::ITER_CLONED_COLLECT), @@ -1652,7 +1652,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&loops::MANUAL_MEMCPY), LintId::of(&loops::NEEDLESS_COLLECT), LintId::of(&methods::EXPECT_FUN_CALL), - LintId::of(&methods::INEFFICIENT_TO_STRING), LintId::of(&methods::ITER_NTH), LintId::of(&methods::OR_FUN_CALL), LintId::of(&methods::SINGLE_CHAR_PATTERN), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 527508af8a31..9064d2a41b5b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -698,7 +698,7 @@ declare_clippy_lint! { /// ["foo", "bar"].iter().map(|&s| s.to_string()); /// ``` pub INEFFICIENT_TO_STRING, - perf, + pedantic, "using `to_string` on `&&T` where `T: ToString`" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8a6d0af5f8a7..4ea974ec6e83 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -789,7 +789,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "inefficient_to_string", - group: "perf", + group: "pedantic", desc: "using `to_string` on `&&T` where `T: ToString`", deprecation: None, module: "methods", From ffb2e4123458e50202c02e63b9e044583c0fe79a Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 2 Apr 2020 18:20:23 +0200 Subject: [PATCH 04/28] Clean up update_lints --- clippy_dev/src/lib.rs | 19 +++++++++---------- clippy_dev/src/update_lints.rs | 10 +++++----- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 83f60f15906f..b01112539bef 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -85,7 +85,7 @@ impl Lint { /// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`. #[must_use] -pub fn gen_lint_group_list(lints: Vec) -> Vec { +pub fn gen_lint_group_list(lints: &[Lint]) -> Vec { lints .into_iter() .filter_map(|l| { @@ -101,14 +101,14 @@ pub fn gen_lint_group_list(lints: Vec) -> Vec { /// Generates the `pub mod module_name` list in `clippy_lints/src/lib.rs`. #[must_use] -pub fn gen_modules_list(lints: Vec) -> Vec { +pub fn gen_modules_list(lints: &[Lint]) -> Vec { lints .into_iter() .filter_map(|l| { if l.is_internal() || l.deprecation.is_some() { None } else { - Some(l.module) + Some(l.module.clone()) } }) .unique() @@ -119,11 +119,10 @@ pub fn gen_modules_list(lints: Vec) -> Vec { /// Generates the list of lint links at the bottom of the README #[must_use] -pub fn gen_changelog_lint_list(lints: Vec) -> Vec { - let mut lint_list_sorted: Vec = lints; - lint_list_sorted.sort_by_key(|l| l.name.clone()); - lint_list_sorted +pub fn gen_changelog_lint_list(lints: &[Lint]) -> Vec { + lints .iter() + .sorted_by_key(|l| l.name.clone()) .filter_map(|l| { if l.is_internal() { None @@ -475,7 +474,7 @@ fn test_gen_changelog_lint_list() { format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()), format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string()), ]; - assert_eq!(expected, gen_changelog_lint_list(lints)); + assert_eq!(expected, gen_changelog_lint_list(&lints)); } #[test] @@ -525,7 +524,7 @@ fn test_gen_modules_list() { "pub mod another_module;".to_string(), "pub mod module_name;".to_string(), ]; - assert_eq!(expected, gen_modules_list(lints)); + assert_eq!(expected, gen_modules_list(&lints)); } #[test] @@ -541,5 +540,5 @@ fn test_gen_lint_group_list() { " LintId::of(&module_name::INTERNAL),".to_string(), " LintId::of(&module_name::SHOULD_ASSERT_EQ),".to_string(), ]; - assert_eq!(expected, gen_lint_group_list(lints)); + assert_eq!(expected, gen_lint_group_list(&lints)); } diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index d30d6f97a2f7..d709e4892fe2 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -61,7 +61,7 @@ pub fn run(update_mode: UpdateMode) { "", false, update_mode == UpdateMode::Change, - || gen_changelog_lint_list(lint_list.clone()), + || gen_changelog_lint_list(&lint_list), ) .changed; @@ -91,7 +91,7 @@ pub fn run(update_mode: UpdateMode) { "end lints modules", false, update_mode == UpdateMode::Change, - || gen_modules_list(lint_list.clone()), + || gen_modules_list(&lint_list), ) .changed; @@ -110,9 +110,9 @@ pub fn run(update_mode: UpdateMode) { .filter(|l| { l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf" }) - .collect(); + .collect::>(); - gen_lint_group_list(all_group_lints) + gen_lint_group_list(&all_group_lints) }, ) .changed; @@ -125,7 +125,7 @@ pub fn run(update_mode: UpdateMode) { r#"\]\);"#, false, update_mode == UpdateMode::Change, - || gen_lint_group_list(lints.clone()), + || gen_lint_group_list(&lints), ) .changed; } From da679825e0d3b1dda22044b3ec9ec1612a4e26f0 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 2 Apr 2020 18:31:32 +0200 Subject: [PATCH 05/28] Get rid of Lint::is_internal method --- clippy_dev/src/lib.rs | 27 +++++---------------------- clippy_dev/src/update_lints.rs | 4 ++-- 2 files changed, 7 insertions(+), 24 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index b01112539bef..4531d9b39c0e 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -63,7 +63,7 @@ impl Lint { /// Returns all non-deprecated lints and non-internal lints pub fn usable_lints(lints: impl Iterator) -> impl Iterator { - lints.filter(|l| l.deprecation.is_none() && !l.is_internal()) + lints.filter(|l| l.deprecation.is_none() && !l.group.starts_with("internal")) } /// Returns all internal lints (not `internal_warn` lints) @@ -76,11 +76,6 @@ impl Lint { pub fn by_lint_group(lints: impl Iterator) -> HashMap> { lints.map(|lint| (lint.group.to_string(), lint)).into_group_map() } - - #[must_use] - pub fn is_internal(&self) -> bool { - self.group.starts_with("internal") - } } /// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`. @@ -103,14 +98,8 @@ pub fn gen_lint_group_list(lints: &[Lint]) -> Vec { #[must_use] pub fn gen_modules_list(lints: &[Lint]) -> Vec { lints - .into_iter() - .filter_map(|l| { - if l.is_internal() || l.deprecation.is_some() { - None - } else { - Some(l.module.clone()) - } - }) + .iter() + .map(|l| &l.module) .unique() .map(|module| format!("pub mod {};", module)) .sorted() @@ -124,7 +113,7 @@ pub fn gen_changelog_lint_list(lints: &[Lint]) -> Vec { .iter() .sorted_by_key(|l| l.name.clone()) .filter_map(|l| { - if l.is_internal() { + if l.group.starts_with("internal") { None } else { Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name)) @@ -158,13 +147,7 @@ pub fn gen_register_lint_list(lints: &[Lint]) -> Vec { let post = " ]);".to_string(); let mut inner = lints .iter() - .filter_map(|l| { - if !l.is_internal() && l.deprecation.is_none() { - Some(format!(" &{}::{},", l.module, l.name.to_uppercase())) - } else { - None - } - }) + .map(|l| format!(" &{}::{},", l.module, l.name.to_uppercase())) .sorted() .collect::>(); inner.insert(0, pre); diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index d709e4892fe2..be565700b012 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -81,7 +81,7 @@ pub fn run(update_mode: UpdateMode) { "end register lints", false, update_mode == UpdateMode::Change, - || gen_register_lint_list(&lint_list), + || gen_register_lint_list(&usable_lints), ) .changed; @@ -91,7 +91,7 @@ pub fn run(update_mode: UpdateMode) { "end lints modules", false, update_mode == UpdateMode::Change, - || gen_modules_list(&lint_list), + || gen_modules_list(&usable_lints), ) .changed; From 98c30fea8c8bac46f79a70e0ef0d7be0c76ae4e1 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 2 Apr 2020 22:04:54 +0200 Subject: [PATCH 06/28] Build lint lists once and the reuse them to update files --- clippy_dev/src/lib.rs | 45 ++++++++++++++++++++-------------- clippy_dev/src/update_lints.rs | 38 +++++++++++++--------------- 2 files changed, 43 insertions(+), 40 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 4531d9b39c0e..bec415aae94c 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -62,13 +62,25 @@ impl Lint { } /// Returns all non-deprecated lints and non-internal lints - pub fn usable_lints(lints: impl Iterator) -> impl Iterator { - lints.filter(|l| l.deprecation.is_none() && !l.group.starts_with("internal")) + #[must_use] + pub fn usable_lints(lints: &[Self]) -> Vec { + lints + .iter() + .filter(|l| l.deprecation.is_none() && !l.group.starts_with("internal")) + .cloned() + .collect() } /// Returns all internal lints (not `internal_warn` lints) - pub fn internal_lints(lints: impl Iterator) -> impl Iterator { - lints.filter(|l| l.group == "internal") + #[must_use] + pub fn internal_lints(lints: &[Self]) -> Vec { + lints.iter().filter(|l| l.group == "internal").cloned().collect() + } + + /// Returns all deprecated lints + #[must_use] + pub fn deprecated_lints(lints: &[Self]) -> Vec { + lints.iter().filter(|l| l.deprecation.is_some()).cloned().collect() } /// Returns the lints in a `HashMap`, grouped by the different lint groups @@ -80,9 +92,8 @@ impl Lint { /// Generates the Vec items for `register_lint_group` calls in `clippy_lints/src/lib.rs`. #[must_use] -pub fn gen_lint_group_list(lints: &[Lint]) -> Vec { +pub fn gen_lint_group_list<'a>(lints: impl Iterator) -> Vec { lints - .into_iter() .filter_map(|l| { if l.deprecation.is_some() { None @@ -96,9 +107,8 @@ pub fn gen_lint_group_list(lints: &[Lint]) -> Vec { /// Generates the `pub mod module_name` list in `clippy_lints/src/lib.rs`. #[must_use] -pub fn gen_modules_list(lints: &[Lint]) -> Vec { +pub fn gen_modules_list<'a>(lints: impl Iterator) -> Vec { lints - .iter() .map(|l| &l.module) .unique() .map(|module| format!("pub mod {};", module)) @@ -108,9 +118,8 @@ pub fn gen_modules_list(lints: &[Lint]) -> Vec { /// Generates the list of lint links at the bottom of the README #[must_use] -pub fn gen_changelog_lint_list(lints: &[Lint]) -> Vec { +pub fn gen_changelog_lint_list<'a>(lints: impl Iterator) -> Vec { lints - .iter() .sorted_by_key(|l| l.name.clone()) .filter_map(|l| { if l.group.starts_with("internal") { @@ -124,9 +133,8 @@ pub fn gen_changelog_lint_list(lints: &[Lint]) -> Vec { /// Generates the `register_removed` code in `./clippy_lints/src/lib.rs`. #[must_use] -pub fn gen_deprecated(lints: &[Lint]) -> Vec { +pub fn gen_deprecated<'a>(lints: impl Iterator) -> Vec { lints - .iter() .filter_map(|l| { l.clone().deprecation.map(|depr_text| { vec![ @@ -142,11 +150,10 @@ pub fn gen_deprecated(lints: &[Lint]) -> Vec { } #[must_use] -pub fn gen_register_lint_list(lints: &[Lint]) -> Vec { +pub fn gen_register_lint_list<'a>(lints: impl Iterator) -> Vec { let pre = " store.register_lints(&[".to_string(); let post = " ]);".to_string(); let mut inner = lints - .iter() .map(|l| format!(" &{}::{},", l.module, l.name.to_uppercase())) .sorted() .collect::>(); @@ -421,7 +428,7 @@ fn test_usable_lints() { None, "module_name", )]; - assert_eq!(expected, Lint::usable_lints(lints.into_iter()).collect::>()); + assert_eq!(expected, Lint::usable_lints(&lints)); } #[test] @@ -457,7 +464,7 @@ fn test_gen_changelog_lint_list() { format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()), format!("[`should_assert_eq2`]: {}#should_assert_eq2", DOCS_LINK.to_string()), ]; - assert_eq!(expected, gen_changelog_lint_list(&lints)); + assert_eq!(expected, gen_changelog_lint_list(lints.iter())); } #[test] @@ -492,7 +499,7 @@ fn test_gen_deprecated() { .into_iter() .map(String::from) .collect(); - assert_eq!(expected, gen_deprecated(&lints)); + assert_eq!(expected, gen_deprecated(lints.iter())); } #[test] @@ -507,7 +514,7 @@ fn test_gen_modules_list() { "pub mod another_module;".to_string(), "pub mod module_name;".to_string(), ]; - assert_eq!(expected, gen_modules_list(&lints)); + assert_eq!(expected, gen_modules_list(lints.iter())); } #[test] @@ -523,5 +530,5 @@ fn test_gen_lint_group_list() { " LintId::of(&module_name::INTERNAL),".to_string(), " LintId::of(&module_name::SHOULD_ASSERT_EQ),".to_string(), ]; - assert_eq!(expected, gen_lint_group_list(&lints)); + assert_eq!(expected, gen_lint_group_list(lints.iter())); } diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index be565700b012..a9a709299426 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -14,14 +14,14 @@ pub enum UpdateMode { pub fn run(update_mode: UpdateMode) { let lint_list: Vec = gather_all().collect(); - let internal_lints = Lint::internal_lints(lint_list.clone().into_iter()); - - let usable_lints: Vec = Lint::usable_lints(lint_list.clone().into_iter()).collect(); - let usable_lint_count = round_to_fifty(usable_lints.len()); - + let internal_lints = Lint::internal_lints(&lint_list); + let deprecated_lints = Lint::deprecated_lints(&lint_list); + let usable_lints = Lint::usable_lints(&lint_list); let mut sorted_usable_lints = usable_lints.clone(); sorted_usable_lints.sort_by_key(|lint| lint.name.clone()); + let usable_lint_count = round_to_fifty(usable_lints.len()); + let mut file_change = replace_region_in_file( Path::new("src/lintlist/mod.rs"), "begin lint list", @@ -61,7 +61,7 @@ pub fn run(update_mode: UpdateMode) { "", false, update_mode == UpdateMode::Change, - || gen_changelog_lint_list(&lint_list), + || gen_changelog_lint_list(usable_lints.iter().chain(deprecated_lints.iter())), ) .changed; @@ -71,7 +71,7 @@ pub fn run(update_mode: UpdateMode) { "end deprecated lints", false, update_mode == UpdateMode::Change, - || gen_deprecated(&lint_list), + || gen_deprecated(deprecated_lints.iter()), ) .changed; @@ -81,7 +81,7 @@ pub fn run(update_mode: UpdateMode) { "end register lints", false, update_mode == UpdateMode::Change, - || gen_register_lint_list(&usable_lints), + || gen_register_lint_list(usable_lints.iter().chain(internal_lints.iter())), ) .changed; @@ -91,7 +91,7 @@ pub fn run(update_mode: UpdateMode) { "end lints modules", false, update_mode == UpdateMode::Change, - || gen_modules_list(&usable_lints), + || gen_modules_list(usable_lints.iter()), ) .changed; @@ -104,15 +104,11 @@ pub fn run(update_mode: UpdateMode) { update_mode == UpdateMode::Change, || { // clippy::all should only include the following lint groups: - let all_group_lints = usable_lints - .clone() - .into_iter() - .filter(|l| { - l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf" - }) - .collect::>(); - - gen_lint_group_list(&all_group_lints) + let all_group_lints = usable_lints.iter().filter(|l| { + l.group == "correctness" || l.group == "style" || l.group == "complexity" || l.group == "perf" + }); + + gen_lint_group_list(all_group_lints) }, ) .changed; @@ -125,7 +121,7 @@ pub fn run(update_mode: UpdateMode) { r#"\]\);"#, false, update_mode == UpdateMode::Change, - || gen_lint_group_list(&lints), + || gen_lint_group_list(lints.iter()), ) .changed; } @@ -140,8 +136,8 @@ pub fn run(update_mode: UpdateMode) { } pub fn print_lints() { - let lint_list = gather_all(); - let usable_lints: Vec = Lint::usable_lints(lint_list).collect(); + let lint_list: Vec = gather_all().collect(); + let usable_lints = Lint::usable_lints(&lint_list); let usable_lint_count = usable_lints.len(); let grouped_by_lint_group = Lint::by_lint_group(usable_lints.into_iter()); From a186d9fafd7424feab7b5bde044219a0e8e58c9f Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 2 Apr 2020 22:07:33 +0200 Subject: [PATCH 07/28] Don't filter lints in code generation functions --- clippy_dev/src/lib.rs | 52 ++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index bec415aae94c..10056e8f03e0 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -94,13 +94,7 @@ impl Lint { #[must_use] pub fn gen_lint_group_list<'a>(lints: impl Iterator) -> Vec { lints - .filter_map(|l| { - if l.deprecation.is_some() { - None - } else { - Some(format!(" LintId::of(&{}::{}),", l.module, l.name.to_uppercase())) - } - }) + .map(|l| format!(" LintId::of(&{}::{}),", l.module, l.name.to_uppercase())) .sorted() .collect::>() } @@ -120,14 +114,8 @@ pub fn gen_modules_list<'a>(lints: impl Iterator) -> Vec(lints: impl Iterator) -> Vec { lints - .sorted_by_key(|l| l.name.clone()) - .filter_map(|l| { - if l.group.starts_with("internal") { - None - } else { - Some(format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name)) - } - }) + .sorted_by_key(|l| &l.name) + .map(|l| format!("[`{}`]: {}#{}", l.name, DOCS_LINK, l.name)) .collect() } @@ -135,17 +123,19 @@ pub fn gen_changelog_lint_list<'a>(lints: impl Iterator) -> Vec #[must_use] pub fn gen_deprecated<'a>(lints: impl Iterator) -> Vec { lints - .filter_map(|l| { - l.clone().deprecation.map(|depr_text| { - vec![ - " store.register_removed(".to_string(), - format!(" \"clippy::{}\",", l.name), - format!(" \"{}\",", depr_text), - " );".to_string(), - ] - }) + .flat_map(|l| { + l.deprecation + .clone() + .map(|depr_text| { + vec![ + " store.register_removed(".to_string(), + format!(" \"clippy::{}\",", l.name), + format!(" \"{}\",", depr_text), + " );".to_string(), + ] + }) + .expect("only deprecated lints should be passed") }) - .flatten() .collect::>() } @@ -458,7 +448,6 @@ fn test_gen_changelog_lint_list() { let lints = vec![ Lint::new("should_assert_eq", "group1", "abc", None, "module_name"), Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"), - Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"), ]; let expected = vec![ format!("[`should_assert_eq`]: {}#should_assert_eq", DOCS_LINK.to_string()), @@ -484,7 +473,6 @@ fn test_gen_deprecated() { Some("will be removed"), "module_name", ), - Lint::new("should_assert_eq2", "group2", "abc", None, "module_name"), ]; let expected: Vec = vec![ " store.register_removed(", @@ -502,13 +490,18 @@ fn test_gen_deprecated() { assert_eq!(expected, gen_deprecated(lints.iter())); } +#[test] +#[should_panic] +fn test_gen_deprecated_fail() { + let lints = vec![Lint::new("should_assert_eq2", "group2", "abc", None, "module_name")]; + let _ = gen_deprecated(lints.iter()); +} + #[test] fn test_gen_modules_list() { let lints = vec![ Lint::new("should_assert_eq", "group1", "abc", None, "module_name"), - Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"), Lint::new("incorrect_stuff", "group3", "abc", None, "another_module"), - Lint::new("incorrect_internal", "internal_style", "abc", None, "module_name"), ]; let expected = vec![ "pub mod another_module;".to_string(), @@ -522,7 +515,6 @@ fn test_gen_lint_group_list() { let lints = vec![ Lint::new("abc", "group1", "abc", None, "module_name"), Lint::new("should_assert_eq", "group1", "abc", None, "module_name"), - Lint::new("should_assert_eq2", "group2", "abc", Some("abc"), "deprecated"), Lint::new("internal", "internal_style", "abc", None, "module_name"), ]; let expected = vec![ From d89bb50f72e1db538d6ae18023c0edb746b119f9 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 2 Apr 2020 22:08:10 +0200 Subject: [PATCH 08/28] Make lint modules private --- clippy_dev/src/lib.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 10056e8f03e0..1f8510f43a61 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -105,7 +105,7 @@ pub fn gen_modules_list<'a>(lints: impl Iterator) -> Vec>() } @@ -503,10 +503,7 @@ fn test_gen_modules_list() { Lint::new("should_assert_eq", "group1", "abc", None, "module_name"), Lint::new("incorrect_stuff", "group3", "abc", None, "another_module"), ]; - let expected = vec![ - "pub mod another_module;".to_string(), - "pub mod module_name;".to_string(), - ]; + let expected = vec!["mod another_module;".to_string(), "mod module_name;".to_string()]; assert_eq!(expected, gen_modules_list(lints.iter())); } From 045722a17ea05cb11ae933578ee01a3d5d831352 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Thu, 2 Apr 2020 22:08:25 +0200 Subject: [PATCH 09/28] Run update_lints --- clippy_lints/src/lib.rs | 308 ++++++++++++++++++++-------------------- 1 file changed, 157 insertions(+), 151 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..b24110fc1844 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -170,157 +170,157 @@ mod consts; mod utils; // begin lints modules, do not remove this comment, it’s used in `update_lints` -pub mod approx_const; -pub mod arithmetic; -pub mod as_conversions; -pub mod assertions_on_constants; -pub mod assign_ops; -pub mod atomic_ordering; -pub mod attrs; -pub mod bit_mask; -pub mod blacklisted_name; -pub mod block_in_if_condition; -pub mod booleans; -pub mod bytecount; -pub mod cargo_common_metadata; -pub mod checked_conversions; -pub mod cognitive_complexity; -pub mod collapsible_if; -pub mod comparison_chain; -pub mod copies; -pub mod copy_iterator; -pub mod dbg_macro; -pub mod default_trait_access; -pub mod derive; -pub mod doc; -pub mod double_comparison; -pub mod double_parens; -pub mod drop_bounds; -pub mod drop_forget_ref; -pub mod duration_subsec; -pub mod else_if_without_else; -pub mod empty_enum; -pub mod entry; -pub mod enum_clike; -pub mod enum_variants; -pub mod eq_op; -pub mod erasing_op; -pub mod escape; -pub mod eta_reduction; -pub mod eval_order_dependence; -pub mod excessive_bools; -pub mod exit; -pub mod explicit_write; -pub mod fallible_impl_from; -pub mod float_literal; -pub mod floating_point_arithmetic; -pub mod format; -pub mod formatting; -pub mod functions; -pub mod get_last_with_len; -pub mod identity_conversion; -pub mod identity_op; -pub mod if_let_some_result; -pub mod if_not_else; -pub mod implicit_return; -pub mod indexing_slicing; -pub mod infinite_iter; -pub mod inherent_impl; -pub mod inherent_to_string; -pub mod inline_fn_without_body; -pub mod int_plus_one; -pub mod integer_division; -pub mod items_after_statements; -pub mod large_enum_variant; -pub mod large_stack_arrays; -pub mod len_zero; -pub mod let_if_seq; -pub mod let_underscore; -pub mod lifetimes; -pub mod literal_representation; -pub mod loops; -pub mod macro_use; -pub mod main_recursion; -pub mod map_clone; -pub mod map_unit_fn; -pub mod matches; -pub mod mem_discriminant; -pub mod mem_forget; -pub mod mem_replace; -pub mod methods; -pub mod minmax; -pub mod misc; -pub mod misc_early; -pub mod missing_const_for_fn; -pub mod missing_doc; -pub mod missing_inline; -pub mod modulo_arithmetic; -pub mod multiple_crate_versions; -pub mod mut_key; -pub mod mut_mut; -pub mod mut_reference; -pub mod mutable_debug_assertion; -pub mod mutex_atomic; -pub mod needless_bool; -pub mod needless_borrow; -pub mod needless_borrowed_ref; -pub mod needless_continue; -pub mod needless_pass_by_value; -pub mod needless_update; -pub mod neg_cmp_op_on_partial_ord; -pub mod neg_multiply; -pub mod new_without_default; -pub mod no_effect; -pub mod non_copy_const; -pub mod non_expressive_names; -pub mod open_options; -pub mod option_env_unwrap; -pub mod overflow_check_conditional; -pub mod panic_unimplemented; -pub mod partialeq_ne_impl; -pub mod path_buf_push_overwrite; -pub mod precedence; -pub mod ptr; -pub mod ptr_offset_with_cast; -pub mod question_mark; -pub mod ranges; -pub mod redundant_clone; -pub mod redundant_field_names; -pub mod redundant_pattern_matching; -pub mod redundant_pub_crate; -pub mod redundant_static_lifetimes; -pub mod reference; -pub mod regex; -pub mod returns; -pub mod serde_api; -pub mod shadow; -pub mod single_component_path_imports; -pub mod slow_vector_initialization; -pub mod strings; -pub mod suspicious_trait_impl; -pub mod swap; -pub mod tabs_in_doc_comments; -pub mod temporary_assignment; -pub mod to_digit_is_some; -pub mod trait_bounds; -pub mod transmute; -pub mod transmuting_null; -pub mod trivially_copy_pass_by_ref; -pub mod try_err; -pub mod types; -pub mod unicode; -pub mod unnamed_address; -pub mod unsafe_removed_from_name; -pub mod unused_io_amount; -pub mod unused_self; -pub mod unwrap; -pub mod use_self; -pub mod vec; -pub mod verbose_file_reads; -pub mod wildcard_dependencies; -pub mod wildcard_imports; -pub mod write; -pub mod zero_div_zero; +mod approx_const; +mod arithmetic; +mod as_conversions; +mod assertions_on_constants; +mod assign_ops; +mod atomic_ordering; +mod attrs; +mod bit_mask; +mod blacklisted_name; +mod block_in_if_condition; +mod booleans; +mod bytecount; +mod cargo_common_metadata; +mod checked_conversions; +mod cognitive_complexity; +mod collapsible_if; +mod comparison_chain; +mod copies; +mod copy_iterator; +mod dbg_macro; +mod default_trait_access; +mod derive; +mod doc; +mod double_comparison; +mod double_parens; +mod drop_bounds; +mod drop_forget_ref; +mod duration_subsec; +mod else_if_without_else; +mod empty_enum; +mod entry; +mod enum_clike; +mod enum_variants; +mod eq_op; +mod erasing_op; +mod escape; +mod eta_reduction; +mod eval_order_dependence; +mod excessive_bools; +mod exit; +mod explicit_write; +mod fallible_impl_from; +mod float_literal; +mod floating_point_arithmetic; +mod format; +mod formatting; +mod functions; +mod get_last_with_len; +mod identity_conversion; +mod identity_op; +mod if_let_some_result; +mod if_not_else; +mod implicit_return; +mod indexing_slicing; +mod infinite_iter; +mod inherent_impl; +mod inherent_to_string; +mod inline_fn_without_body; +mod int_plus_one; +mod integer_division; +mod items_after_statements; +mod large_enum_variant; +mod large_stack_arrays; +mod len_zero; +mod let_if_seq; +mod let_underscore; +mod lifetimes; +mod literal_representation; +mod loops; +mod macro_use; +mod main_recursion; +mod map_clone; +mod map_unit_fn; +mod matches; +mod mem_discriminant; +mod mem_forget; +mod mem_replace; +mod methods; +mod minmax; +mod misc; +mod misc_early; +mod missing_const_for_fn; +mod missing_doc; +mod missing_inline; +mod modulo_arithmetic; +mod multiple_crate_versions; +mod mut_key; +mod mut_mut; +mod mut_reference; +mod mutable_debug_assertion; +mod mutex_atomic; +mod needless_bool; +mod needless_borrow; +mod needless_borrowed_ref; +mod needless_continue; +mod needless_pass_by_value; +mod needless_update; +mod neg_cmp_op_on_partial_ord; +mod neg_multiply; +mod new_without_default; +mod no_effect; +mod non_copy_const; +mod non_expressive_names; +mod open_options; +mod option_env_unwrap; +mod overflow_check_conditional; +mod panic_unimplemented; +mod partialeq_ne_impl; +mod path_buf_push_overwrite; +mod precedence; +mod ptr; +mod ptr_offset_with_cast; +mod question_mark; +mod ranges; +mod redundant_clone; +mod redundant_field_names; +mod redundant_pattern_matching; +mod redundant_pub_crate; +mod redundant_static_lifetimes; +mod reference; +mod regex; +mod returns; +mod serde_api; +mod shadow; +mod single_component_path_imports; +mod slow_vector_initialization; +mod strings; +mod suspicious_trait_impl; +mod swap; +mod tabs_in_doc_comments; +mod temporary_assignment; +mod to_digit_is_some; +mod trait_bounds; +mod transmute; +mod transmuting_null; +mod trivially_copy_pass_by_ref; +mod try_err; +mod types; +mod unicode; +mod unnamed_address; +mod unsafe_removed_from_name; +mod unused_io_amount; +mod unused_self; +mod unwrap; +mod use_self; +mod vec; +mod verbose_file_reads; +mod wildcard_dependencies; +mod wildcard_imports; +mod write; +mod zero_div_zero; // end lints modules, do not remove this comment, it’s used in `update_lints` pub use crate::utils::conf::Conf; @@ -828,6 +828,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &unwrap::PANICKING_UNWRAP, &unwrap::UNNECESSARY_UNWRAP, &use_self::USE_SELF, + &utils::internal_lints::CLIPPY_LINTS_INTERNAL, + &utils::internal_lints::COMPILER_LINT_FUNCTIONS, + &utils::internal_lints::DEFAULT_LINT, + &utils::internal_lints::LINT_WITHOUT_LINT_PASS, + &utils::internal_lints::OUTER_EXPN_EXPN_DATA, + &utils::internal_lints::PRODUCE_ICE, &vec::USELESS_VEC, &verbose_file_reads::VERBOSE_FILE_READS, &wildcard_dependencies::WILDCARD_DEPENDENCIES, From 30503a91d2cd19239d0aed802cd740ec6f2a7e06 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 3 Apr 2020 21:31:47 +0200 Subject: [PATCH 10/28] Move matches test in matches module --- clippy_lints/src/matches.rs | 37 ++++++++++++++++++++++++++++++++ tests/matches.rs | 42 ------------------------------------- 2 files changed, 37 insertions(+), 42 deletions(-) delete mode 100644 tests/matches.rs diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 20b793f95ded..4298e62b8037 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -1197,3 +1197,40 @@ where None } + +#[test] +fn test_overlapping() { + use rustc_span::source_map::DUMMY_SP; + + let sp = |s, e| SpannedRange { + span: DUMMY_SP, + node: (s, e), + }; + + assert_eq!(None, overlapping::(&[])); + assert_eq!(None, overlapping(&[sp(1, Bound::Included(4))])); + assert_eq!( + None, + overlapping(&[sp(1, Bound::Included(4)), sp(5, Bound::Included(6))]) + ); + assert_eq!( + None, + overlapping(&[ + sp(1, Bound::Included(4)), + sp(5, Bound::Included(6)), + sp(10, Bound::Included(11)) + ],) + ); + assert_eq!( + Some((&sp(1, Bound::Included(4)), &sp(3, Bound::Included(6)))), + overlapping(&[sp(1, Bound::Included(4)), sp(3, Bound::Included(6))]) + ); + assert_eq!( + Some((&sp(5, Bound::Included(6)), &sp(6, Bound::Included(11)))), + overlapping(&[ + sp(1, Bound::Included(4)), + sp(5, Bound::Included(6)), + sp(6, Bound::Included(11)) + ],) + ); +} diff --git a/tests/matches.rs b/tests/matches.rs deleted file mode 100644 index 6691c074caf9..000000000000 --- a/tests/matches.rs +++ /dev/null @@ -1,42 +0,0 @@ -#![feature(rustc_private)] - -extern crate rustc_span; -use std::collections::Bound; - -#[test] -fn test_overlapping() { - use clippy_lints::matches::overlapping; - use rustc_span::source_map::DUMMY_SP; - - let sp = |s, e| clippy_lints::matches::SpannedRange { - span: DUMMY_SP, - node: (s, e), - }; - - assert_eq!(None, overlapping::(&[])); - assert_eq!(None, overlapping(&[sp(1, Bound::Included(4))])); - assert_eq!( - None, - overlapping(&[sp(1, Bound::Included(4)), sp(5, Bound::Included(6))]) - ); - assert_eq!( - None, - overlapping(&[ - sp(1, Bound::Included(4)), - sp(5, Bound::Included(6)), - sp(10, Bound::Included(11)) - ],) - ); - assert_eq!( - Some((&sp(1, Bound::Included(4)), &sp(3, Bound::Included(6)))), - overlapping(&[sp(1, Bound::Included(4)), sp(3, Bound::Included(6))]) - ); - assert_eq!( - Some((&sp(5, Bound::Included(6)), &sp(6, Bound::Included(11)))), - overlapping(&[ - sp(1, Bound::Included(4)), - sp(5, Bound::Included(6)), - sp(6, Bound::Included(11)) - ],) - ); -} From 91d8a804d34b44a414b02ea5eba5305573748fff Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Fri, 3 Apr 2020 23:59:52 -0700 Subject: [PATCH 11/28] result_map_or_into_option: add lint to catch manually adpating Result -> Option Result has an `ok()` method that adapts a Result into an Option. It's possible to get around this adapter by writing Result.map_or(None, Some). This lint is implemented as a new variant of the existing [`option_map_none` lint](https://github.com/rust-lang/rust-clippy/pull/2128) --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 + clippy_lints/src/methods/mod.rs | 105 ++++++++++++++++------ src/lintlist/mod.rs | 7 ++ tests/ui/result_map_or_into_option.fixed | 16 ++++ tests/ui/result_map_or_into_option.rs | 14 +++ tests/ui/result_map_or_into_option.stderr | 10 +++ 7 files changed, 131 insertions(+), 25 deletions(-) create mode 100644 tests/ui/result_map_or_into_option.fixed create mode 100644 tests/ui/result_map_or_into_option.rs create mode 100644 tests/ui/result_map_or_into_option.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 894aab21fb37..b7ac3cace204 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1448,6 +1448,7 @@ Released 2018-09-13 [`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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..83dcb350e185 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -666,6 +666,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &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, @@ -1273,6 +1274,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::OPTION_AS_REF_DEREF), LintId::of(&methods::OPTION_MAP_OR_NONE), LintId::of(&methods::OR_FUN_CALL), + LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION), LintId::of(&methods::SEARCH_IS_SOME), LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT), LintId::of(&methods::SINGLE_CHAR_PATTERN), @@ -1453,6 +1455,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::NEW_RET_NO_SELF), LintId::of(&methods::OK_EXPECT), LintId::of(&methods::OPTION_MAP_OR_NONE), + LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION), LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT), LintId::of(&methods::STRING_EXTEND_CHARS), LintId::of(&methods::UNNECESSARY_FOLD), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 527508af8a31..e8d642ed71e0 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -330,6 +330,24 @@ declare_clippy_lint! { "using `Option.map_or(None, f)`, which is more succinctly expressed as `and_then(f)`" } +declare_clippy_lint! { + /// **What it does:** Checks for usage of `_.map_or(None, Some)`. + /// + /// **Why is this bad?** Readability, this can be written more concisely as + /// `_.ok()`. + /// + /// **Known problems:** + /// + /// **Example:** + /// ```rust + /// # let opt = Some(1); + /// # let r = opt.map_or(None, Some); + /// ``` + pub RESULT_MAP_OR_INTO_OPTION, + style, + "using `Result.map_or(None, Some)`, which is more succinctly expressed as `ok()`" +} + declare_clippy_lint! { /// **What it does:** Checks for usage of `_.and_then(|x| Some(y))`. /// @@ -1248,6 +1266,7 @@ declare_lint_pass!(Methods => [ OPTION_MAP_UNWRAP_OR, OPTION_MAP_UNWRAP_OR_ELSE, RESULT_MAP_UNWRAP_OR_ELSE, + RESULT_MAP_OR_INTO_OPTION, OPTION_MAP_OR_NONE, OPTION_AND_THEN_SOME, OR_FUN_CALL, @@ -2517,37 +2536,73 @@ fn lint_map_unwrap_or_else<'a, 'tcx>( } } -/// lint use of `_.map_or(None, _)` for `Option`s +/// lint use of `_.map_or(None, _)` for `Option`s and `Result`s fn lint_map_or_none<'a, 'tcx>( cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>, map_or_args: &'tcx [hir::Expr<'_>], ) { - if match_type(cx, cx.tables.expr_ty(&map_or_args[0]), &paths::OPTION) { - // check if the first non-self argument to map_or() is None - let map_or_arg_is_none = if let hir::ExprKind::Path(ref qpath) = map_or_args[1].kind { - match_qpath(qpath, &paths::OPTION_NONE) - } else { - false - }; + let is_option = match_type(cx, cx.tables.expr_ty(&map_or_args[0]), &paths::OPTION); + let is_result = match_type(cx, cx.tables.expr_ty(&map_or_args[0]), &paths::RESULT); + + // There are two variants of this `map_or` lint: + // (1) using `map_or` as an adapter from `Result` to `Option` + // (2) using `map_or` as a combinator instead of `and_then` + // + // (For this lint) we don't care if any other type calls `map_or` + if !is_option && !is_result { + return; + } - if map_or_arg_is_none { - // lint message - let msg = "called `map_or(None, f)` on an `Option` value. This can be done more directly by calling \ - `and_then(f)` instead"; - let map_or_self_snippet = snippet(cx, map_or_args[0].span, ".."); - let map_or_func_snippet = snippet(cx, map_or_args[2].span, ".."); - let hint = format!("{0}.and_then({1})", map_or_self_snippet, map_or_func_snippet); - span_lint_and_sugg( - cx, - OPTION_MAP_OR_NONE, - expr.span, - msg, - "try using `and_then` instead", - hint, - Applicability::MachineApplicable, - ); - } + let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = map_or_args[1].kind { + match_qpath(qpath, &paths::OPTION_NONE) + } else { + false + }; + + // This is really only needed if `is_result` holds. Computing it here + // makes `mess`'s assignment a bit easier, so just compute it here. + let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_or_args[2].kind { + match_qpath(qpath, &paths::OPTION_SOME) + } else { + false + }; + + let mess = if is_option && default_arg_is_none { + let self_snippet = snippet(cx, map_or_args[0].span, ".."); + let func_snippet = snippet(cx, map_or_args[2].span, ".."); + let msg = "called `map_or(None, f)` on an `Option` value. This can be done more directly by calling \ + `and_then(f)` instead"; + Some(( + OPTION_MAP_OR_NONE, + msg, + "try using `and_then` instead", + format!("{0}.and_then({1})", self_snippet, func_snippet), + )) + } else if is_result && f_arg_is_some { + let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \ + `ok()` instead"; + let self_snippet = snippet(cx, map_or_args[0].span, ".."); + Some(( + RESULT_MAP_OR_INTO_OPTION, + msg, + "try using `ok` instead", + format!("{0}.ok()", self_snippet), + )) + } else { + None + }; + + if let Some((lint, msg, instead, hint)) = mess { + span_lint_and_sugg( + cx, + lint, + expr.span, + msg, + instead, + hint, + Applicability::MachineApplicable, + ); } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8a6d0af5f8a7..01d1d1a06723 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1823,6 +1823,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "methods", }, + Lint { + name: "result_map_or_into_option", + group: "style", + desc: "using `Result.map_or(None, Some)`, which is more succinctly expressed as `ok()`", + deprecation: None, + module: "methods", + }, Lint { name: "result_map_unit_fn", group: "complexity", diff --git a/tests/ui/result_map_or_into_option.fixed b/tests/ui/result_map_or_into_option.fixed new file mode 100644 index 000000000000..948eb6a3aca1 --- /dev/null +++ b/tests/ui/result_map_or_into_option.fixed @@ -0,0 +1,16 @@ +// run-rustfix + +#![warn(clippy::result_map_or_into_option)] + +fn main() { + let opt: Result = Ok(1); + let _ = opt.ok(); + + let rewrap = |s: u32| -> Option { + Some(s) + }; + + // A non-Some `f` arg should not emit the lint + let opt: Result = Ok(1); + let _ = opt.map_or(None, rewrap); +} diff --git a/tests/ui/result_map_or_into_option.rs b/tests/ui/result_map_or_into_option.rs new file mode 100644 index 000000000000..d097c19e44b9 --- /dev/null +++ b/tests/ui/result_map_or_into_option.rs @@ -0,0 +1,14 @@ +// run-rustfix + +#![warn(clippy::result_map_or_into_option)] + +fn main() { + let opt: Result = Ok(1); + let _ = opt.map_or(None, Some); + + let rewrap = |s: u32| -> Option { Some(s) }; + + // A non-Some `f` arg should not emit the lint + let opt: Result = Ok(1); + let _ = opt.map_or(None, rewrap); +} diff --git a/tests/ui/result_map_or_into_option.stderr b/tests/ui/result_map_or_into_option.stderr new file mode 100644 index 000000000000..febf32147d13 --- /dev/null +++ b/tests/ui/result_map_or_into_option.stderr @@ -0,0 +1,10 @@ +error: called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling `ok()` instead + --> $DIR/result_map_or_into_option.rs:7:13 + | +LL | let _ = opt.map_or(None, Some); + | ^^^^^^^^^^^^^^^^^^^^^^ help: try using `ok` instead: `opt.ok()` + | + = note: `-D clippy::result-map-or-into-option` implied by `-D warnings` + +error: aborting due to previous error + From 2481d3c74a0b89767d9b78db55d967f083bda0d7 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 4 Apr 2020 17:01:42 +0200 Subject: [PATCH 12/28] Rename rustc -> rustc_middle in doc links --- CONTRIBUTING.md | 2 +- doc/adding_lints.md | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1f9be44b73d..466478c81509 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -70,7 +70,7 @@ an AST expression). `match_def_path()` in Clippy's `utils` module can also be us [`T-AST`]: https://github.com/rust-lang/rust-clippy/labels/T-AST [`T-middle`]: https://github.com/rust-lang/rust-clippy/labels/T-middle [`E-medium`]: https://github.com/rust-lang/rust-clippy/labels/E-medium -[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty +[`ty`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty [nodes in the AST docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/ [deep-nesting]: https://github.com/rust-lang/rust-clippy/blob/557f6848bd5b7183f55c1e1522a326e9e1df6030/clippy_lints/src/mem_forget.rs#L29-L43 [if_chain]: https://docs.rs/if_chain/*/if_chain diff --git a/doc/adding_lints.md b/doc/adding_lints.md index 1d78a27820a6..a66d4e66add2 100644 --- a/doc/adding_lints.md +++ b/doc/adding_lints.md @@ -463,11 +463,11 @@ don't hesitate to ask on [Discord] or in the issue/PR. [utils]: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/utils/mod.rs [if_chain]: https://docs.rs/if_chain/*/if_chain/ [from_expansion]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html#method.from_expansion -[in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/fn.in_external_macro.html +[in_external_macro]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/lint/fn.in_external_macro.html [span]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_span/struct.Span.html [applicability]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/enum.Applicability.html [rustc-dev-guide]: https://rustc-dev-guide.rust-lang.org/ -[nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ +[nightly_docs]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ [ast]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/index.html -[ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/ty/sty/index.html +[ty]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/sty/index.html [Discord]: https://discord.gg/rust-lang From fac5a41ca5ba3adb55ba76936c505c99b3f7c7c7 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Sat, 4 Apr 2020 17:02:05 +0200 Subject: [PATCH 13/28] Update CONTRIBUTING.md --- CONTRIBUTING.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 466478c81509..12b066b83fce 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,8 @@ High level approach: ### Finding something to fix/improve -All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @llogiq, @mcarton or @oli-obk. +All issues on Clippy are mentored, if you want help with a bug just ask +@Manishearth, @flip1995, @phansch or @yaahc. Some issues are easier than others. The [`good first issue`] label can be used to find the easy issues. If you want to work on an issue, please leave a comment so that we can assign it to you! @@ -78,8 +79,7 @@ an AST expression). `match_def_path()` in Clippy's `utils` module can also be us ## Writing code -Have a look at the [docs for writing lints][adding_lints] for more details. [Llogiq's blog post on lints] -is also a nice primer to lint-writing, though it does get into advanced stuff and may be a bit outdated. +Have a look at the [docs for writing lints][adding_lints] for more details. If you want to add a new lint or change existing ones apart from bugfixing, it's also a good idea to give the [stability guarantees][rfc_stability] and @@ -87,7 +87,6 @@ also a good idea to give the [stability guarantees][rfc_stability] and quick read. [adding_lints]: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md -[Llogiq's blog post on lints]: https://llogiq.github.io/2015/06/04/workflows.html [clippy_rfc]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md [rfc_stability]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#stability-guarantees [rfc_lint_cats]: https://github.com/rust-lang/rfcs/blob/master/text/2476-clippy-uno.md#lint-audit-and-categories From be34bc46ed7699c598e4525b07e04067e19e49aa Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Apr 2020 12:47:16 -0700 Subject: [PATCH 14/28] Downgrade unreadable_literal to pedantic --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/literal_representation.rs | 2 +- src/lintlist/mod.rs | 2 +- tests/ui/approx_const.rs | 2 +- tests/ui/inconsistent_digit_grouping.fixed | 1 + tests/ui/inconsistent_digit_grouping.rs | 1 + tests/ui/inconsistent_digit_grouping.stderr | 26 ++++++++++++--------- 7 files changed, 21 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..2e0a77068acd 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1098,6 +1098,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&items_after_statements::ITEMS_AFTER_STATEMENTS), LintId::of(&large_stack_arrays::LARGE_STACK_ARRAYS), LintId::of(&literal_representation::LARGE_DIGIT_GROUPS), + LintId::of(&literal_representation::UNREADABLE_LITERAL), LintId::of(&loops::EXPLICIT_INTO_ITER_LOOP), LintId::of(&loops::EXPLICIT_ITER_LOOP), LintId::of(¯o_use::MACRO_USE_IMPORTS), @@ -1219,7 +1220,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&lifetimes::NEEDLESS_LIFETIMES), LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING), LintId::of(&literal_representation::MISTYPED_LITERAL_SUFFIXES), - LintId::of(&literal_representation::UNREADABLE_LITERAL), LintId::of(&loops::EMPTY_LOOP), LintId::of(&loops::EXPLICIT_COUNTER_LOOP), LintId::of(&loops::FOR_KV_MAP), @@ -1428,7 +1428,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&len_zero::LEN_ZERO), LintId::of(&let_if_seq::USELESS_LET_IF_SEQ), LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING), - LintId::of(&literal_representation::UNREADABLE_LITERAL), LintId::of(&loops::EMPTY_LOOP), LintId::of(&loops::FOR_KV_MAP), LintId::of(&loops::NEEDLESS_RANGE_LOOP), diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 1b0b111bcfed..0a6ffc1130a3 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -27,7 +27,7 @@ declare_clippy_lint! { /// let x: u64 = 61864918973511; /// ``` pub UNREADABLE_LITERAL, - style, + pedantic, "long integer literal without underscores" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8a6d0af5f8a7..e24169bd3506 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2294,7 +2294,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "unreadable_literal", - group: "style", + group: "pedantic", desc: "long integer literal without underscores", deprecation: None, module: "literal_representation", diff --git a/tests/ui/approx_const.rs b/tests/ui/approx_const.rs index e1c150fdefd4..fb57a0becbb2 100644 --- a/tests/ui/approx_const.rs +++ b/tests/ui/approx_const.rs @@ -1,5 +1,5 @@ #[warn(clippy::approx_constant)] -#[allow(unused, clippy::shadow_unrelated, clippy::similar_names, clippy::unreadable_literal)] +#[allow(unused, clippy::shadow_unrelated, clippy::similar_names)] fn main() { let my_e = 2.7182; let almost_e = 2.718; diff --git a/tests/ui/inconsistent_digit_grouping.fixed b/tests/ui/inconsistent_digit_grouping.fixed index f10673adfb2d..ae4d1806af49 100644 --- a/tests/ui/inconsistent_digit_grouping.fixed +++ b/tests/ui/inconsistent_digit_grouping.fixed @@ -1,5 +1,6 @@ // run-rustfix #[warn(clippy::inconsistent_digit_grouping)] +#[deny(clippy::unreadable_literal)] #[allow(unused_variables, clippy::excessive_precision)] fn main() { macro_rules! mac1 { diff --git a/tests/ui/inconsistent_digit_grouping.rs b/tests/ui/inconsistent_digit_grouping.rs index b97df0865ee8..a1ac21746f64 100644 --- a/tests/ui/inconsistent_digit_grouping.rs +++ b/tests/ui/inconsistent_digit_grouping.rs @@ -1,5 +1,6 @@ // run-rustfix #[warn(clippy::inconsistent_digit_grouping)] +#[deny(clippy::unreadable_literal)] #[allow(unused_variables, clippy::excessive_precision)] fn main() { macro_rules! mac1 { diff --git a/tests/ui/inconsistent_digit_grouping.stderr b/tests/ui/inconsistent_digit_grouping.stderr index 37211efcab5f..b8ac91554620 100644 --- a/tests/ui/inconsistent_digit_grouping.stderr +++ b/tests/ui/inconsistent_digit_grouping.stderr @@ -1,5 +1,5 @@ error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:25:16 + --> $DIR/inconsistent_digit_grouping.rs:26:16 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^ help: consider: `123_456` @@ -7,57 +7,61 @@ LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f = note: `-D clippy::inconsistent-digit-grouping` implied by `-D warnings` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:25:26 + --> $DIR/inconsistent_digit_grouping.rs:26:26 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^^^ help: consider: `12_345_678` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:25:38 + --> $DIR/inconsistent_digit_grouping.rs:26:38 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^ help: consider: `1_234_567` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:25:48 + --> $DIR/inconsistent_digit_grouping.rs:26:48 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^^^^^^^ help: consider: `1_234.567_8_f32` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:25:64 + --> $DIR/inconsistent_digit_grouping.rs:26:64 | LL | let bad = (1_23_456, 1_234_5678, 1234_567, 1_234.5678_f32, 1.234_5678_f32); | ^^^^^^^^^^^^^^ help: consider: `1.234_567_8_f32` error: long literal lacking separators - --> $DIR/inconsistent_digit_grouping.rs:28:13 + --> $DIR/inconsistent_digit_grouping.rs:29:13 | LL | let _ = 0x100000; | ^^^^^^^^ help: consider: `0x0010_0000` | - = note: `-D clippy::unreadable-literal` implied by `-D warnings` +note: the lint level is defined here + --> $DIR/inconsistent_digit_grouping.rs:3:8 + | +LL | #[deny(clippy::unreadable_literal)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: long literal lacking separators - --> $DIR/inconsistent_digit_grouping.rs:29:13 + --> $DIR/inconsistent_digit_grouping.rs:30:13 | LL | let _ = 0x1000000; | ^^^^^^^^^ help: consider: `0x0100_0000` error: long literal lacking separators - --> $DIR/inconsistent_digit_grouping.rs:30:13 + --> $DIR/inconsistent_digit_grouping.rs:31:13 | LL | let _ = 0x10000000; | ^^^^^^^^^^ help: consider: `0x1000_0000` error: long literal lacking separators - --> $DIR/inconsistent_digit_grouping.rs:31:13 + --> $DIR/inconsistent_digit_grouping.rs:32:13 | LL | let _ = 0x100000000_u64; | ^^^^^^^^^^^^^^^ help: consider: `0x0001_0000_0000_u64` error: digits grouped inconsistently by underscores - --> $DIR/inconsistent_digit_grouping.rs:34:18 + --> $DIR/inconsistent_digit_grouping.rs:35:18 | LL | let _: f32 = 1_23_456.; | ^^^^^^^^^ help: consider: `123_456.` From 560c8c9c701c0f84cdc725e00562c560b48dd0c2 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Apr 2020 12:58:18 -0700 Subject: [PATCH 15/28] Downgrade new_ret_no_self to pedantic --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/methods/mod.rs | 2 +- src/lintlist/mod.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index dfc2a26b06b2..f6942f18476c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1106,6 +1106,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::FILTER_MAP_NEXT), LintId::of(&methods::FIND_MAP), LintId::of(&methods::MAP_FLATTEN), + LintId::of(&methods::NEW_RET_NO_SELF), LintId::of(&methods::OPTION_MAP_UNWRAP_OR), LintId::of(&methods::OPTION_MAP_UNWRAP_OR_ELSE), LintId::of(&methods::RESULT_MAP_UNWRAP_OR_ELSE), @@ -1267,7 +1268,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::ITER_NTH_ZERO), LintId::of(&methods::ITER_SKIP_NEXT), 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), @@ -1450,7 +1450,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::ITER_NTH_ZERO), LintId::of(&methods::ITER_SKIP_NEXT), LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC), - LintId::of(&methods::NEW_RET_NO_SELF), LintId::of(&methods::OK_EXPECT), LintId::of(&methods::OPTION_MAP_OR_NONE), LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 527508af8a31..7d7a375ddac3 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -721,7 +721,7 @@ declare_clippy_lint! { /// } /// ``` pub NEW_RET_NO_SELF, - style, + pedantic, "not returning `Self` in a `new` method" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 8a6d0af5f8a7..0a565f4466fe 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1447,7 +1447,7 @@ pub static ref ALL_LINTS: Vec = vec![ }, Lint { name: "new_ret_no_self", - group: "style", + group: "pedantic", desc: "not returning `Self` in a `new` method", deprecation: None, module: "methods", From 91759a7582a292c3f85204b1d61e7185c8543959 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 13:42:24 -0700 Subject: [PATCH 16/28] result_map_or_into_option: explicitly note absence of known problems --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index e8d642ed71e0..37bb3710fabc 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -336,7 +336,7 @@ declare_clippy_lint! { /// **Why is this bad?** Readability, this can be written more concisely as /// `_.ok()`. /// - /// **Known problems:** + /// **Known problems:** None. /// /// **Example:** /// ```rust From 3a29aedf8db3af19ee0f64dee6f00489812e6cb0 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 13:44:11 -0700 Subject: [PATCH 17/28] result_map_or_into_option: add good and bad examples --- clippy_lints/src/methods/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 37bb3710fabc..7c818232a553 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -340,8 +340,13 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust - /// # let opt = Some(1); - /// # let r = opt.map_or(None, Some); + /// # Bad + /// let r: Result = Ok(1); + /// assert_eq!(Some(1), r.map_or(None, Some)); + /// + /// # Good + /// let r: Result = Ok(1); + /// assert_eq!(Some(1), r.ok()); /// ``` pub RESULT_MAP_OR_INTO_OPTION, style, From d0738bd673fe8e2b42d26b6d116f197f4aecea82 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 13:53:08 -0700 Subject: [PATCH 18/28] result_map_or_into_option: destructure lint tuple or return early --- clippy_lints/src/methods/mod.rs | 74 +++++++++++++----------- tests/ui/result_map_or_into_option.fixed | 4 +- 2 files changed, 41 insertions(+), 37 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7c818232a553..62fcd801bc30 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2573,42 +2573,48 @@ fn lint_map_or_none<'a, 'tcx>( false }; - let mess = if is_option && default_arg_is_none { - let self_snippet = snippet(cx, map_or_args[0].span, ".."); - let func_snippet = snippet(cx, map_or_args[2].span, ".."); - let msg = "called `map_or(None, f)` on an `Option` value. This can be done more directly by calling \ - `and_then(f)` instead"; - Some(( - OPTION_MAP_OR_NONE, - msg, - "try using `and_then` instead", - format!("{0}.and_then({1})", self_snippet, func_snippet), - )) - } else if is_result && f_arg_is_some { - let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \ - `ok()` instead"; - let self_snippet = snippet(cx, map_or_args[0].span, ".."); - Some(( - RESULT_MAP_OR_INTO_OPTION, - msg, - "try using `ok` instead", - format!("{0}.ok()", self_snippet), - )) - } else { - None + let (lint, msg, instead, hint) = { + if !default_arg_is_none { + // nothing to lint! + return; + } + + if is_option { + let self_snippet = snippet(cx, map_or_args[0].span, ".."); + let func_snippet = snippet(cx, map_or_args[2].span, ".."); + let msg = "called `map_or(None, f)` on an `Option` value. This can be done more directly by calling \ + `and_then(f)` instead"; + ( + OPTION_MAP_OR_NONE, + msg, + "try using `and_then` instead", + format!("{0}.and_then({1})", self_snippet, func_snippet), + ) + } else if f_arg_is_some { + let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \ + `ok()` instead"; + let self_snippet = snippet(cx, map_or_args[0].span, ".."); + ( + RESULT_MAP_OR_INTO_OPTION, + msg, + "try using `ok` instead", + format!("{0}.ok()", self_snippet), + ) + } else { + // nothing to lint! + return; + } }; - if let Some((lint, msg, instead, hint)) = mess { - span_lint_and_sugg( - cx, - lint, - expr.span, - msg, - instead, - hint, - Applicability::MachineApplicable, - ); - } + span_lint_and_sugg( + cx, + lint, + expr.span, + msg, + instead, + hint, + Applicability::MachineApplicable, + ); } /// Lint use of `_.and_then(|x| Some(y))` for `Option`s diff --git a/tests/ui/result_map_or_into_option.fixed b/tests/ui/result_map_or_into_option.fixed index 948eb6a3aca1..07daf19fa250 100644 --- a/tests/ui/result_map_or_into_option.fixed +++ b/tests/ui/result_map_or_into_option.fixed @@ -6,9 +6,7 @@ fn main() { let opt: Result = Ok(1); let _ = opt.ok(); - let rewrap = |s: u32| -> Option { - Some(s) - }; + let rewrap = |s: u32| -> Option { Some(s) }; // A non-Some `f` arg should not emit the lint let opt: Result = Ok(1); From fb276dc3fa1d644ce6de1849ff3b78d3e54f5d9e Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 14:02:45 -0700 Subject: [PATCH 19/28] result_map_or_into_option: add `opt.map_or(None, |_| Some(y))` test --- tests/ui/result_map_or_into_option.fixed | 5 +++++ tests/ui/result_map_or_into_option.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/ui/result_map_or_into_option.fixed b/tests/ui/result_map_or_into_option.fixed index 07daf19fa250..331531b5165f 100644 --- a/tests/ui/result_map_or_into_option.fixed +++ b/tests/ui/result_map_or_into_option.fixed @@ -11,4 +11,9 @@ fn main() { // A non-Some `f` arg should not emit the lint let opt: Result = Ok(1); let _ = opt.map_or(None, rewrap); + + // A non-Some `f` closure where the argument is not used as the + // return should not emit the lint + let opt: Result = Ok(1); + opt.map_or(None, |_x| Some(1)); } diff --git a/tests/ui/result_map_or_into_option.rs b/tests/ui/result_map_or_into_option.rs index d097c19e44b9..3058480e2ad3 100644 --- a/tests/ui/result_map_or_into_option.rs +++ b/tests/ui/result_map_or_into_option.rs @@ -11,4 +11,9 @@ fn main() { // A non-Some `f` arg should not emit the lint let opt: Result = Ok(1); let _ = opt.map_or(None, rewrap); + + // A non-Some `f` closure where the argument is not used as the + // return should not emit the lint + let opt: Result = Ok(1); + opt.map_or(None, |_x| Some(1)); } From acc3bc1ba27129f6fc5bda6d943046e9e2ad2d45 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 14:24:22 -0700 Subject: [PATCH 20/28] result_map_or_into_option: move arg checks into tuple assignment --- clippy_lints/src/methods/mod.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 62fcd801bc30..d57cce47ff9b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2559,26 +2559,27 @@ fn lint_map_or_none<'a, 'tcx>( return; } - let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = map_or_args[1].kind { - match_qpath(qpath, &paths::OPTION_NONE) - } else { - false - }; - // This is really only needed if `is_result` holds. Computing it here - // makes `mess`'s assignment a bit easier, so just compute it here. - let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_or_args[2].kind { - match_qpath(qpath, &paths::OPTION_SOME) - } else { - false - }; let (lint, msg, instead, hint) = { + + let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = map_or_args[1].kind { + match_qpath(qpath, &paths::OPTION_NONE) + } else { + return; + }; + if !default_arg_is_none { // nothing to lint! return; } + let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_or_args[2].kind { + match_qpath(qpath, &paths::OPTION_SOME) + } else { + false + }; + if is_option { let self_snippet = snippet(cx, map_or_args[0].span, ".."); let func_snippet = snippet(cx, map_or_args[2].span, ".."); From 2533f56a0eaa4ebbeb607b6596d12e303f97e008 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 14:33:43 -0700 Subject: [PATCH 21/28] result_map_or_into_option: fix `cargo dev fmt --check` errors --- clippy_lints/src/methods/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d57cce47ff9b..9ff669515cf8 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2559,10 +2559,7 @@ fn lint_map_or_none<'a, 'tcx>( return; } - - let (lint, msg, instead, hint) = { - let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = map_or_args[1].kind { match_qpath(qpath, &paths::OPTION_NONE) } else { From fecdb72b14031617f1245db279ddffb6840e1dac Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 14:39:24 -0700 Subject: [PATCH 22/28] CONTRIBUTING.md: fix broken triage link --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b1f9be44b73d..614caf9cd27d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -223,7 +223,7 @@ You can find the Clippy bors queue [here][homu_queue]. If you have @bors permissions, you can find an overview of the available commands [here][homu_instructions]. -[triage]: https://forge.rust-lang.org/triage-procedure.html +[triage]: https://forge.rust-lang.org/release/triage-procedure.html [l-crash]: https://github.com/rust-lang/rust-clippy/labels/L-crash%20%3Aboom%3A [l-bug]: https://github.com/rust-lang/rust-clippy/labels/L-bug%20%3Abeetle%3A [homu]: https://github.com/rust-lang/homu From 325d0b69d2dffccecf48c5d7975ccd100141e1f5 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 15:02:38 -0700 Subject: [PATCH 23/28] result_map_or_into: fix dogfood_clippy error => {h,l}int --- clippy_lints/src/methods/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 9ff669515cf8..7f4964da72e7 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -2559,7 +2559,7 @@ fn lint_map_or_none<'a, 'tcx>( return; } - let (lint, msg, instead, hint) = { + let (lint_name, msg, instead, hint) = { let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = map_or_args[1].kind { match_qpath(qpath, &paths::OPTION_NONE) } else { @@ -2606,7 +2606,7 @@ fn lint_map_or_none<'a, 'tcx>( span_lint_and_sugg( cx, - lint, + lint_name, expr.span, msg, instead, From 5d54fbb7914cc1ea5b3bd8cdfd6f24dbacd8f649 Mon Sep 17 00:00:00 2001 From: Nick Torres Date: Sat, 4 Apr 2020 17:20:23 -0700 Subject: [PATCH 24/28] result_map_or_into_option: fix syntax error in example --- clippy_lints/src/methods/mod.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 7f4964da72e7..98d9426bd8a5 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -339,13 +339,16 @@ declare_clippy_lint! { /// **Known problems:** None. /// /// **Example:** + /// + /// Bad: /// ```rust - /// # Bad - /// let r: Result = Ok(1); + /// # let r: Result = Ok(1); /// assert_eq!(Some(1), r.map_or(None, Some)); + /// ``` /// - /// # Good - /// let r: Result = Ok(1); + /// Good: + /// ```rust + /// # let r: Result = Ok(1); /// assert_eq!(Some(1), r.ok()); /// ``` pub RESULT_MAP_OR_INTO_OPTION, From 9c9af1d88525de5649375e08abf16437426349b8 Mon Sep 17 00:00:00 2001 From: Jacek Pospychala Date: Sun, 5 Apr 2020 20:40:51 +0200 Subject: [PATCH 25/28] Include OpAssign in suspicious_op_assign_impl --- clippy_lints/src/suspicious_trait_impl.rs | 10 ++++++---- tests/ui/suspicious_arithmetic_impl.rs | 21 ++++++++++++++++++++- tests/ui/suspicious_arithmetic_impl.stderr | 8 +++++++- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index 89b57ed1a8de..f1e223d9a48c 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -54,7 +54,7 @@ declare_lint_pass!(SuspiciousImpl => [SUSPICIOUS_ARITHMETIC_IMPL, SUSPICIOUS_OP_ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SuspiciousImpl { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr<'_>) { - if let hir::ExprKind::Binary(binop, _, _) = expr.kind { + if let hir::ExprKind::Binary(binop, _, _) | hir::ExprKind::AssignOp(binop, ..) = expr.kind { match binop.node { hir::BinOpKind::Eq | hir::BinOpKind::Lt @@ -65,14 +65,15 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SuspiciousImpl { _ => {}, } // Check if the binary expression is part of another bi/unary expression - // as a child node + // or operator assignment as a child node let mut parent_expr = cx.tcx.hir().get_parent_node(expr.hir_id); while parent_expr != hir::CRATE_HIR_ID { if let hir::Node::Expr(e) = cx.tcx.hir().get(parent_expr) { match e.kind { hir::ExprKind::Binary(..) | hir::ExprKind::Unary(hir::UnOp::UnNot, _) - | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) => return, + | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) + | hir::ExprKind::AssignOp(..) => return, _ => {}, } } @@ -191,7 +192,8 @@ impl<'a, 'tcx> Visitor<'tcx> for BinaryExprVisitor { match expr.kind { hir::ExprKind::Binary(..) | hir::ExprKind::Unary(hir::UnOp::UnNot, _) - | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) => self.in_binary_expr = true, + | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) + | hir::ExprKind::AssignOp(..) => self.in_binary_expr = true, _ => {}, } diff --git a/tests/ui/suspicious_arithmetic_impl.rs b/tests/ui/suspicious_arithmetic_impl.rs index 6ee924d3b2ed..1f5b98118870 100644 --- a/tests/ui/suspicious_arithmetic_impl.rs +++ b/tests/ui/suspicious_arithmetic_impl.rs @@ -1,5 +1,5 @@ #![warn(clippy::suspicious_arithmetic_impl)] -use std::ops::{Add, AddAssign, Div, Mul, Sub}; +use std::ops::{Add, AddAssign, BitOrAssign, Div, DivAssign, Mul, MulAssign, Sub}; #[derive(Copy, Clone)] struct Foo(u32); @@ -18,6 +18,25 @@ impl AddAssign for Foo { } } +impl BitOrAssign for Foo { + fn bitor_assign(&mut self, other: Foo) { + let idx = other.0; + self.0 |= 1 << idx; // OK: BinOpKind::Shl part of AssignOp as child node + } +} + +impl MulAssign for Foo { + fn mul_assign(&mut self, other: Foo) { + self.0 /= other.0; + } +} + +impl DivAssign for Foo { + fn div_assign(&mut self, other: Foo) { + self.0 /= other.0; // OK: BinOpKind::Div == DivAssign + } +} + impl Mul for Foo { type Output = Foo; diff --git a/tests/ui/suspicious_arithmetic_impl.stderr b/tests/ui/suspicious_arithmetic_impl.stderr index e8a6efc4c4d2..7e42d72c30b2 100644 --- a/tests/ui/suspicious_arithmetic_impl.stderr +++ b/tests/ui/suspicious_arithmetic_impl.stderr @@ -14,5 +14,11 @@ LL | *self = *self - other; | = note: `#[deny(clippy::suspicious_op_assign_impl)]` on by default -error: aborting due to 2 previous errors +error: Suspicious use of binary operator in `MulAssign` impl + --> $DIR/suspicious_arithmetic_impl.rs:30:16 + | +LL | self.0 /= other.0; + | ^^ + +error: aborting due to 3 previous errors From 4f14826e091e08f1ee4ff06d3ca6d5015045cfb5 Mon Sep 17 00:00:00 2001 From: xiongmao86 Date: Mon, 6 Apr 2020 21:48:38 +0800 Subject: [PATCH 26/28] Lint on opt.as_ref().map(|x| &**x). --- clippy_lints/src/loops.rs | 4 +-- clippy_lints/src/methods/mod.rs | 49 ++++++++++++++++++++--------- tests/ui/option_as_ref_deref.fixed | 3 ++ tests/ui/option_as_ref_deref.rs | 3 ++ tests/ui/option_as_ref_deref.stderr | 14 ++++++++- 5 files changed, 55 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 43217b6cc64e..7cc21e4bdd23 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -654,7 +654,7 @@ fn combine_branches(b1: NeverLoopResult, b2: NeverLoopResult) -> NeverLoopResult fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult { let stmts = block.stmts.iter().map(stmt_to_expr); - let expr = once(block.expr.as_ref().map(|p| &**p)); + let expr = once(block.expr.as_deref()); let mut iter = stmts.chain(expr).filter_map(|e| e); never_loop_expr_seq(&mut iter, main_loop_id) } @@ -662,7 +662,7 @@ fn never_loop_block(block: &Block<'_>, main_loop_id: HirId) -> NeverLoopResult { fn stmt_to_expr<'tcx>(stmt: &Stmt<'tcx>) -> Option<&'tcx Expr<'tcx>> { match stmt.kind { StmtKind::Semi(ref e, ..) | StmtKind::Expr(ref e, ..) => Some(e), - StmtKind::Local(ref local) => local.init.as_ref().map(|p| &**p), + StmtKind::Local(ref local) => local.init.as_deref(), _ => None, } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 527508af8a31..6f85d1d69596 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3159,6 +3159,8 @@ fn lint_option_as_ref_deref<'a, 'tcx>( map_args: &[hir::Expr<'_>], is_mut: bool, ) { + let same_mutability = |m| (is_mut && m == &hir::Mutability::Mut) || (!is_mut && m == &hir::Mutability::Not); + let option_ty = cx.tables.expr_ty(&as_ref_args[0]); if !match_type(cx, option_ty, &paths::OPTION) { return; @@ -3181,23 +3183,40 @@ fn lint_option_as_ref_deref<'a, 'tcx>( hir::ExprKind::Closure(_, _, body_id, _, _) => { let closure_body = cx.tcx.hir().body(body_id); let closure_expr = remove_blocks(&closure_body.value); - if_chain! { - if let hir::ExprKind::MethodCall(_, _, args) = &closure_expr.kind; - if args.len() == 1; - if let hir::ExprKind::Path(qpath) = &args[0].kind; - if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id); - if closure_body.params[0].pat.hir_id == local_id; - let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::>(); - if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj; - then { - let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap(); - deref_aliases.iter().any(|path| match_def_path(cx, method_did, path)) - } else { - false - } + + match &closure_expr.kind { + hir::ExprKind::MethodCall(_, _, args) => { + if_chain! { + if args.len() == 1; + if let hir::ExprKind::Path(qpath) = &args[0].kind; + if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, args[0].hir_id); + if closure_body.params[0].pat.hir_id == local_id; + let adj = cx.tables.expr_adjustments(&args[0]).iter().map(|x| &x.kind).collect::>(); + if let [ty::adjustment::Adjust::Deref(None), ty::adjustment::Adjust::Borrow(_)] = *adj; + then { + let method_did = cx.tables.type_dependent_def_id(closure_expr.hir_id).unwrap(); + deref_aliases.iter().any(|path| match_def_path(cx, method_did, path)) + } else { + false + } + } + }, + hir::ExprKind::AddrOf(hir::BorrowKind::Ref, m, ref inner) if same_mutability(m) => { + if_chain! { + if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner1) = inner.kind; + if let hir::ExprKind::Unary(hir::UnOp::UnDeref, ref inner2) = inner1.kind; + if let hir::ExprKind::Path(ref qpath) = inner2.kind; + if let hir::def::Res::Local(local_id) = cx.tables.qpath_res(qpath, inner2.hir_id); + then { + closure_body.params[0].pat.hir_id == local_id + } else { + false + } + } + }, + _ => false, } }, - _ => false, }; diff --git a/tests/ui/option_as_ref_deref.fixed b/tests/ui/option_as_ref_deref.fixed index 973e5b308a2c..076692e64451 100644 --- a/tests/ui/option_as_ref_deref.fixed +++ b/tests/ui/option_as_ref_deref.fixed @@ -35,4 +35,7 @@ fn main() { let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted + + let _ = opt.as_deref(); + let _ = opt.as_deref_mut(); } diff --git a/tests/ui/option_as_ref_deref.rs b/tests/ui/option_as_ref_deref.rs index baad85ab9083..3bf5f715f833 100644 --- a/tests/ui/option_as_ref_deref.rs +++ b/tests/ui/option_as_ref_deref.rs @@ -38,4 +38,7 @@ fn main() { let _ = Some(1_usize).as_ref().map(|x| vc[*x].as_str()); // should not be linted let _: Option<&str> = Some(&String::new()).as_ref().map(|x| x.as_str()); // should not be linted + + let _ = opt.as_ref().map(|x| &**x); + let _ = opt.as_mut().map(|x| &mut **x); } diff --git a/tests/ui/option_as_ref_deref.stderr b/tests/ui/option_as_ref_deref.stderr index 09a0fa058e62..6c2bf8517060 100644 --- a/tests/ui/option_as_ref_deref.stderr +++ b/tests/ui/option_as_ref_deref.stderr @@ -88,5 +88,17 @@ error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases LL | let _ = opt.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.clone().as_deref_mut()` -error: aborting due to 14 previous errors +error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead + --> $DIR/option_as_ref_deref.rs:42:13 + | +LL | let _ = opt.as_ref().map(|x| &**x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()` + +error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead + --> $DIR/option_as_ref_deref.rs:43:13 + | +LL | let _ = opt.as_mut().map(|x| &mut **x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()` + +error: aborting due to 16 previous errors From d7056f8ffba7ad87622b7fdcc429f33abc5c62a7 Mon Sep 17 00:00:00 2001 From: xiongmao86 Date: Tue, 7 Apr 2020 21:25:07 +0800 Subject: [PATCH 27/28] Refine lint message. --- clippy_lints/src/methods/mod.rs | 8 ++++---- tests/ui/option_as_ref_deref.stderr | 32 ++++++++++++++--------------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6f85d1d69596..0f331aad0682 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -3222,17 +3222,17 @@ fn lint_option_as_ref_deref<'a, 'tcx>( if is_deref { let current_method = if is_mut { - ".as_mut().map(DerefMut::deref_mut)" + format!(".as_mut().map({})", snippet(cx, map_args[1].span, "..")) } else { - ".as_ref().map(Deref::deref)" + format!(".as_ref().map({})", snippet(cx, map_args[1].span, "..")) }; let method_hint = if is_mut { "as_deref_mut" } else { "as_deref" }; let hint = format!("{}.{}()", snippet(cx, as_ref_args[0].span, ".."), method_hint); let suggestion = format!("try using {} instead", method_hint); let msg = format!( - "called `{0}` (or with one of deref aliases) on an Option value. \ - This can be done more directly by calling `{1}` instead", + "called `{0}` on an Option value. This can be done more directly \ + by calling `{1}` instead", current_method, hint ); span_lint_and_sugg( diff --git a/tests/ui/option_as_ref_deref.stderr b/tests/ui/option_as_ref_deref.stderr index 6c2bf8517060..a106582a6332 100644 --- a/tests/ui/option_as_ref_deref.stderr +++ b/tests/ui/option_as_ref_deref.stderr @@ -1,4 +1,4 @@ -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead +error: called `.as_ref().map(Deref::deref)` on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead --> $DIR/option_as_ref_deref.rs:13:13 | LL | let _ = opt.clone().as_ref().map(Deref::deref).map(str::len); @@ -6,7 +6,7 @@ LL | let _ = opt.clone().as_ref().map(Deref::deref).map(str::len); | = note: `-D clippy::option-as-ref-deref` implied by `-D warnings` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead +error: called `.as_ref().map(Deref::deref)` on an Option value. This can be done more directly by calling `opt.clone().as_deref()` instead --> $DIR/option_as_ref_deref.rs:16:13 | LL | let _ = opt.clone() @@ -16,85 +16,85 @@ LL | | Deref::deref LL | | ) | |_________^ help: try using as_deref instead: `opt.clone().as_deref()` -error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead +error: called `.as_mut().map(DerefMut::deref_mut)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead --> $DIR/option_as_ref_deref.rs:22:13 | LL | let _ = opt.as_mut().map(DerefMut::deref_mut); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead +error: called `.as_ref().map(String::as_str)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead --> $DIR/option_as_ref_deref.rs:24:13 | LL | let _ = opt.as_ref().map(String::as_str); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead +error: called `.as_ref().map(|x| x.as_str())` on an Option value. This can be done more directly by calling `opt.as_deref()` instead --> $DIR/option_as_ref_deref.rs:25:13 | LL | let _ = opt.as_ref().map(|x| x.as_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()` -error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead +error: called `.as_mut().map(String::as_mut_str)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead --> $DIR/option_as_ref_deref.rs:26:13 | LL | let _ = opt.as_mut().map(String::as_mut_str); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()` -error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead +error: called `.as_mut().map(|x| x.as_mut_str())` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead --> $DIR/option_as_ref_deref.rs:27:13 | LL | let _ = opt.as_mut().map(|x| x.as_mut_str()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.as_deref_mut()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(CString::new(vec![]).unwrap()).as_deref()` instead +error: called `.as_ref().map(CString::as_c_str)` on an Option value. This can be done more directly by calling `Some(CString::new(vec![]).unwrap()).as_deref()` instead --> $DIR/option_as_ref_deref.rs:28:13 | LL | let _ = Some(CString::new(vec![]).unwrap()).as_ref().map(CString::as_c_str); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(CString::new(vec![]).unwrap()).as_deref()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(OsString::new()).as_deref()` instead +error: called `.as_ref().map(OsString::as_os_str)` on an Option value. This can be done more directly by calling `Some(OsString::new()).as_deref()` instead --> $DIR/option_as_ref_deref.rs:29:13 | LL | let _ = Some(OsString::new()).as_ref().map(OsString::as_os_str); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(OsString::new()).as_deref()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(PathBuf::new()).as_deref()` instead +error: called `.as_ref().map(PathBuf::as_path)` on an Option value. This can be done more directly by calling `Some(PathBuf::new()).as_deref()` instead --> $DIR/option_as_ref_deref.rs:30:13 | LL | let _ = Some(PathBuf::new()).as_ref().map(PathBuf::as_path); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(PathBuf::new()).as_deref()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref()` instead +error: called `.as_ref().map(Vec::as_slice)` on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref()` instead --> $DIR/option_as_ref_deref.rs:31:13 | LL | let _ = Some(Vec::<()>::new()).as_ref().map(Vec::as_slice); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `Some(Vec::<()>::new()).as_deref()` -error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref_mut()` instead +error: called `.as_mut().map(Vec::as_mut_slice)` on an Option value. This can be done more directly by calling `Some(Vec::<()>::new()).as_deref_mut()` instead --> $DIR/option_as_ref_deref.rs:32:13 | LL | let _ = Some(Vec::<()>::new()).as_mut().map(Vec::as_mut_slice); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `Some(Vec::<()>::new()).as_deref_mut()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead +error: called `.as_ref().map(|x| x.deref())` on an Option value. This can be done more directly by calling `opt.as_deref()` instead --> $DIR/option_as_ref_deref.rs:34:13 | LL | let _ = opt.as_ref().map(|x| x.deref()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()` -error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.clone().as_deref_mut()` instead +error: called `.as_mut().map(|x| x.deref_mut())` on an Option value. This can be done more directly by calling `opt.clone().as_deref_mut()` instead --> $DIR/option_as_ref_deref.rs:35:13 | LL | let _ = opt.clone().as_mut().map(|x| x.deref_mut()).map(|x| x.len()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref_mut instead: `opt.clone().as_deref_mut()` -error: called `.as_ref().map(Deref::deref)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref()` instead +error: called `.as_ref().map(|x| &**x)` on an Option value. This can be done more directly by calling `opt.as_deref()` instead --> $DIR/option_as_ref_deref.rs:42:13 | LL | let _ = opt.as_ref().map(|x| &**x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using as_deref instead: `opt.as_deref()` -error: called `.as_mut().map(DerefMut::deref_mut)` (or with one of deref aliases) on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead +error: called `.as_mut().map(|x| &mut **x)` on an Option value. This can be done more directly by calling `opt.as_deref_mut()` instead --> $DIR/option_as_ref_deref.rs:43:13 | LL | let _ = opt.as_mut().map(|x| &mut **x); From 381f9cb34e499501f73b99f086f3ef85ab15d401 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 8 Apr 2020 16:00:03 +0200 Subject: [PATCH 28/28] Run fmt and update test --- tests/ui/extra_unused_lifetimes.rs | 7 +------ tests/ui/extra_unused_lifetimes.stderr | 8 ++++---- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/ui/extra_unused_lifetimes.rs b/tests/ui/extra_unused_lifetimes.rs index 26df71ddcb0f..ddbf4e98c51a 100644 --- a/tests/ui/extra_unused_lifetimes.rs +++ b/tests/ui/extra_unused_lifetimes.rs @@ -1,9 +1,4 @@ -#![allow( - unused, - dead_code, - clippy::needless_lifetimes, - clippy::needless_pass_by_value -)] +#![allow(unused, dead_code, clippy::needless_lifetimes, clippy::needless_pass_by_value)] #![warn(clippy::extra_unused_lifetimes)] fn empty() {} diff --git a/tests/ui/extra_unused_lifetimes.stderr b/tests/ui/extra_unused_lifetimes.stderr index e997951346f7..16bbb1c037d8 100644 --- a/tests/ui/extra_unused_lifetimes.stderr +++ b/tests/ui/extra_unused_lifetimes.stderr @@ -1,5 +1,5 @@ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:13:14 + --> $DIR/extra_unused_lifetimes.rs:8:14 | LL | fn unused_lt<'a>(x: u8) {} | ^^ @@ -7,19 +7,19 @@ LL | fn unused_lt<'a>(x: u8) {} = note: `-D clippy::extra-unused-lifetimes` implied by `-D warnings` error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:15:25 + --> $DIR/extra_unused_lifetimes.rs:10:25 | LL | fn unused_lt_transitive<'a, 'b: 'a>(x: &'b u8) { | ^^ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:40:10 + --> $DIR/extra_unused_lifetimes.rs:35:10 | LL | fn x<'a>(&self) {} | ^^ error: this lifetime isn't used in the function definition - --> $DIR/extra_unused_lifetimes.rs:66:22 + --> $DIR/extra_unused_lifetimes.rs:61:22 | LL | fn unused_lt<'a>(x: u8) {} | ^^