From 3b1e5d6ff79f93e0215c6fb4c802167834ea4e15 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Tue, 25 Aug 2020 12:05:02 -0700 Subject: [PATCH] Re-enable len_zero for ranges now that `is_empty` is stable on them --- clippy_lints/src/len_zero.rs | 17 +---------------- tests/ui/len_zero.fixed | 8 -------- tests/ui/len_zero.rs | 8 -------- tests/ui/len_zero_ranges.fixed | 10 ++++++---- tests/ui/len_zero_ranges.rs | 10 ++++++---- tests/ui/len_zero_ranges.stderr | 10 ++++++++-- 6 files changed, 21 insertions(+), 42 deletions(-) diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index e5daa30f8ca..b691d363d2f 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -1,4 +1,4 @@ -use crate::utils::{get_item_name, higher, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty}; +use crate::utils::{get_item_name, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty}; use rustc_ast::ast::LitKind; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; @@ -260,17 +260,6 @@ fn check_len( /// Checks if this type has an `is_empty` method. fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - /// Special case ranges until `range_is_empty` is stabilized. See issue 3807. - fn should_skip_range(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - higher::range(expr).map_or(false, |_| { - !cx.tcx - .features() - .declared_lib_features - .iter() - .any(|(name, _)| name.as_str() == "range_is_empty") - }) - } - /// Gets an `AssocItem` and return true if it matches `is_empty(self)`. fn is_is_empty(cx: &LateContext<'_>, item: &ty::AssocItem) -> bool { if let ty::AssocKind::Fn = item.kind { @@ -296,10 +285,6 @@ fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { }) } - if should_skip_range(cx, expr) { - return false; - } - let ty = &walk_ptrs_ty(cx.typeck_results().expr_ty(expr)); match ty.kind { ty::Dynamic(ref tt, ..) => tt.principal().map_or(false, |principal| { diff --git a/tests/ui/len_zero.fixed b/tests/ui/len_zero.fixed index d81676a3d9f..1f3b8ac99b1 100644 --- a/tests/ui/len_zero.fixed +++ b/tests/ui/len_zero.fixed @@ -141,11 +141,3 @@ fn main() { fn test_slice(b: &[u8]) { if !b.is_empty() {} } - -mod issue_3807 { - // Avoid suggesting changes to ranges if the user did not enable `range_is_empty`. - // See https://github.com/rust-lang/rust/issues/48111#issuecomment-445132965 - fn no_suggestion() { - let _ = (0..42).len() == 0; - } -} diff --git a/tests/ui/len_zero.rs b/tests/ui/len_zero.rs index ecdba921a5d..dc21de0001b 100644 --- a/tests/ui/len_zero.rs +++ b/tests/ui/len_zero.rs @@ -141,11 +141,3 @@ fn main() { fn test_slice(b: &[u8]) { if b.len() != 0 {} } - -mod issue_3807 { - // Avoid suggesting changes to ranges if the user did not enable `range_is_empty`. - // See https://github.com/rust-lang/rust/issues/48111#issuecomment-445132965 - fn no_suggestion() { - let _ = (0..42).len() == 0; - } -} diff --git a/tests/ui/len_zero_ranges.fixed b/tests/ui/len_zero_ranges.fixed index eee3db9b7d4..79781766242 100644 --- a/tests/ui/len_zero_ranges.fixed +++ b/tests/ui/len_zero_ranges.fixed @@ -1,15 +1,17 @@ // run-rustfix -#![feature(range_is_empty)] #![warn(clippy::len_zero)] #![allow(unused)] -#![allow(stable_features)] // TODO: https://github.com/rust-lang/rust-clippy/issues/5956 +// Now that `Range(Inclusive)::is_empty` is stable (1.47), we can always suggest this mod issue_3807 { - // With the feature enabled, `is_empty` should be suggested - fn suggestion_is_fine() { + fn suggestion_is_fine_range() { let _ = (0..42).is_empty(); } + + fn suggestion_is_fine_range_inclusive() { + let _ = (0_u8..=42).is_empty(); + } } fn main() {} diff --git a/tests/ui/len_zero_ranges.rs b/tests/ui/len_zero_ranges.rs index be2e0f38fd1..a0eb51cc976 100644 --- a/tests/ui/len_zero_ranges.rs +++ b/tests/ui/len_zero_ranges.rs @@ -1,15 +1,17 @@ // run-rustfix -#![feature(range_is_empty)] #![warn(clippy::len_zero)] #![allow(unused)] -#![allow(stable_features)] // TODO: https://github.com/rust-lang/rust-clippy/issues/5956 +// Now that `Range(Inclusive)::is_empty` is stable (1.47), we can always suggest this mod issue_3807 { - // With the feature enabled, `is_empty` should be suggested - fn suggestion_is_fine() { + fn suggestion_is_fine_range() { let _ = (0..42).len() == 0; } + + fn suggestion_is_fine_range_inclusive() { + let _ = (0_u8..=42).len() == 0; + } } fn main() {} diff --git a/tests/ui/len_zero_ranges.stderr b/tests/ui/len_zero_ranges.stderr index acb85f7100a..d0defb5a79e 100644 --- a/tests/ui/len_zero_ranges.stderr +++ b/tests/ui/len_zero_ranges.stderr @@ -1,10 +1,16 @@ error: length comparison to zero - --> $DIR/len_zero_ranges.rs:11:17 + --> $DIR/len_zero_ranges.rs:9:17 | LL | let _ = (0..42).len() == 0; | ^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(0..42).is_empty()` | = note: `-D clippy::len-zero` implied by `-D warnings` -error: aborting due to previous error +error: length comparison to zero + --> $DIR/len_zero_ranges.rs:13:17 + | +LL | let _ = (0_u8..=42).len() == 0; + | ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(0_u8..=42).is_empty()` + +error: aborting due to 2 previous errors