Skip to content

Commit

Permalink
Auto merge of #5656 - ebroto:len_zero_ranges, r=matthiaskrgr
Browse files Browse the repository at this point in the history
len_zero: skip ranges if feature `range_is_empty` is not enabled

If the feature is not enabled, calling `is_empty()` on a range is ambiguous. Moreover, the two possible resolutions are unstable methods, one inherent to the range and the other being part of the `ExactSizeIterator` trait.

Since `len_zero` only checks for existing `is_empty()` inherent methods, we only take into account the `range_is_empty` feature.

Related: rust-lang/rust#48111 (comment)

changelog: len_zero: avoid linting ranges without #![feature(range_is_empty)]

Fixes: #3807
  • Loading branch information
bors committed May 28, 2020
2 parents 7ea7cd1 + 64a05f5 commit ebfbe2b
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 1 deletion.
17 changes: 16 additions & 1 deletion clippy_lints/src/len_zero.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::utils::{get_item_name, snippet_with_applicability, span_lint, span_lint_and_sugg, walk_ptrs_ty};
use crate::utils::{get_item_name, higher, 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;
Expand Down Expand Up @@ -259,6 +259,17 @@ 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(cx, 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 {
Expand All @@ -284,6 +295,10 @@ fn has_is_empty(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
})
}

if should_skip_range(cx, expr) {
return false;
}

let ty = &walk_ptrs_ty(cx.tables.expr_ty(expr));
match ty.kind {
ty::Dynamic(ref tt, ..) => {
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/len_zero.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,11 @@ 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;
}
}
8 changes: 8 additions & 0 deletions tests/ui/len_zero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,11 @@ 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;
}
}
14 changes: 14 additions & 0 deletions tests/ui/len_zero_ranges.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// run-rustfix

#![feature(range_is_empty)]
#![warn(clippy::len_zero)]
#![allow(unused)]

mod issue_3807 {
// With the feature enabled, `is_empty` should be suggested
fn suggestion_is_fine() {
let _ = (0..42).is_empty();
}
}

fn main() {}
14 changes: 14 additions & 0 deletions tests/ui/len_zero_ranges.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// run-rustfix

#![feature(range_is_empty)]
#![warn(clippy::len_zero)]
#![allow(unused)]

mod issue_3807 {
// With the feature enabled, `is_empty` should be suggested
fn suggestion_is_fine() {
let _ = (0..42).len() == 0;
}
}

fn main() {}
10 changes: 10 additions & 0 deletions tests/ui/len_zero_ranges.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: length comparison to zero
--> $DIR/len_zero_ranges.rs:10: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

0 comments on commit ebfbe2b

Please sign in to comment.