Skip to content

Commit

Permalink
Rollup merge of #5752 - chrisduerr:pedantic_ranges, r=flip1995
Browse files Browse the repository at this point in the history
Move range_minus_one to pedantic

This moves the range_minus_one lint to the pedantic category, so there
will not be any warnings emitted by default. This should work around
problems where the suggestion is impossible to resolve due to the range
consumer only accepting a specific range implementation, rather than the
`RangeBounds` trait (see #3307).

While it is possible to work around this by extracting the boundary into
a variable, I don't think clippy should encourage people to disable or
work around lints, but instead the lints should be fixable. So hopefully
this will help until a proper implementation checks what the range is
used for.

*Please keep the line below*
changelog: move [`range_minus_one`] to pedantic
  • Loading branch information
flip1995 authored Jul 13, 2020
2 parents 75d43aa + afa4148 commit 019e281
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 14 deletions.
3 changes: 1 addition & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
LintId::of(&non_expressive_names::SIMILAR_NAMES),
LintId::of(&option_if_let_else::OPTION_IF_LET_ELSE),
LintId::of(&ranges::RANGE_MINUS_ONE),
LintId::of(&ranges::RANGE_PLUS_ONE),
LintId::of(&shadow::SHADOW_UNRELATED),
LintId::of(&strings::STRING_ADD_ASSIGN),
Expand Down Expand Up @@ -1383,7 +1384,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&ptr::PTR_ARG),
LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
LintId::of(&question_mark::QUESTION_MARK),
LintId::of(&ranges::RANGE_MINUS_ONE),
LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
LintId::of(&ranges::REVERSED_EMPTY_RANGES),
LintId::of(&redundant_clone::REDUNDANT_CLONE),
Expand Down Expand Up @@ -1599,7 +1599,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&partialeq_ne_impl::PARTIALEQ_NE_IMPL),
LintId::of(&precedence::PRECEDENCE),
LintId::of(&ptr_offset_with_cast::PTR_OFFSET_WITH_CAST),
LintId::of(&ranges::RANGE_MINUS_ONE),
LintId::of(&ranges::RANGE_ZIP_WITH_LEN),
LintId::of(&reference::DEREF_ADDROF),
LintId::of(&reference::REF_IN_DEREF),
Expand Down
12 changes: 10 additions & 2 deletions clippy_lints/src/ranges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ declare_clippy_lint! {
/// exclusive ranges, because they essentially add an extra branch that
/// LLVM may fail to hoist out of the loop.
///
/// This will cause a warning that cannot be fixed if the consumer of the
/// range only accepts a specific range type, instead of the generic
/// `RangeBounds` trait
/// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)).
///
/// **Example:**
/// ```rust,ignore
/// for x..(y+1) { .. }
Expand All @@ -72,7 +77,10 @@ declare_clippy_lint! {
/// **Why is this bad?** The code is more readable with an exclusive range
/// like `x..y`.
///
/// **Known problems:** None.
/// **Known problems:** This will cause a warning that cannot be fixed if
/// the consumer of the range only accepts a specific range type, instead of
/// the generic `RangeBounds` trait
/// ([#3307](https://github.com/rust-lang/rust-clippy/issues/3307)).
///
/// **Example:**
/// ```rust,ignore
Expand All @@ -83,7 +91,7 @@ declare_clippy_lint! {
/// for x..y { .. }
/// ```
pub RANGE_MINUS_ONE,
complexity,
pedantic,
"`x..=(y-1)` reads better as `x..y`"
}

Expand Down
2 changes: 1 addition & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
},
Lint {
name: "range_minus_one",
group: "complexity",
group: "pedantic",
desc: "`x..=(y-1)` reads better as `x..y`",
deprecation: None,
module: "ranges",
Expand Down
1 change: 1 addition & 0 deletions tests/ui/range_plus_minus_one.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn f() -> usize {
}

#[warn(clippy::range_plus_one)]
#[warn(clippy::range_minus_one)]
fn main() {
for _ in 0..2 {}
for _ in 0..=2 {}
Expand Down
1 change: 1 addition & 0 deletions tests/ui/range_plus_minus_one.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn f() -> usize {
}

#[warn(clippy::range_plus_one)]
#[warn(clippy::range_minus_one)]
fn main() {
for _ in 0..2 {}
for _ in 0..=2 {}
Expand Down
18 changes: 9 additions & 9 deletions tests/ui/range_plus_minus_one.stderr
Original file line number Diff line number Diff line change
@@ -1,57 +1,57 @@
error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:14:14
--> $DIR/range_plus_minus_one.rs:15:14
|
LL | for _ in 0..3 + 1 {}
| ^^^^^^^^ help: use: `0..=3`
|
= note: `-D clippy::range-plus-one` implied by `-D warnings`

error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:17:14
--> $DIR/range_plus_minus_one.rs:18:14
|
LL | for _ in 0..1 + 5 {}
| ^^^^^^^^ help: use: `0..=5`

error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:20:14
--> $DIR/range_plus_minus_one.rs:21:14
|
LL | for _ in 1..1 + 1 {}
| ^^^^^^^^ help: use: `1..=1`

error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:26:14
--> $DIR/range_plus_minus_one.rs:27:14
|
LL | for _ in 0..(1 + f()) {}
| ^^^^^^^^^^^^ help: use: `0..=f()`

error: an exclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:30:13
--> $DIR/range_plus_minus_one.rs:31:13
|
LL | let _ = ..=11 - 1;
| ^^^^^^^^^ help: use: `..11`
|
= note: `-D clippy::range-minus-one` implied by `-D warnings`

error: an exclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:31:13
--> $DIR/range_plus_minus_one.rs:32:13
|
LL | let _ = ..=(11 - 1);
| ^^^^^^^^^^^ help: use: `..11`

error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:32:13
--> $DIR/range_plus_minus_one.rs:33:13
|
LL | let _ = (1..11 + 1);
| ^^^^^^^^^^^ help: use: `(1..=11)`

error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:33:13
--> $DIR/range_plus_minus_one.rs:34:13
|
LL | let _ = (f() + 1)..(f() + 1);
| ^^^^^^^^^^^^^^^^^^^^ help: use: `((f() + 1)..=f())`

error: an inclusive range would be more readable
--> $DIR/range_plus_minus_one.rs:37:14
--> $DIR/range_plus_minus_one.rs:38:14
|
LL | for _ in 1..ONE + ONE {}
| ^^^^^^^^^^^^ help: use: `1..=ONE`
Expand Down

0 comments on commit 019e281

Please sign in to comment.