Merge some lints together
This PR merges following lints:
- `block_in_if_condition_expr` and `block_in_if_condition_stmt` → `blocks_in_if_conditions`
- `option_map_unwrap_or`, `option_map_unwrap_or_else` and `result_map_unwrap_or_else` → `map_unwrap`
- `option_unwrap_used` and `result_unwrap_used` → `unwrap_used`
- `option_expect_used` and `result_expect_used` → `expect_used`
- `wrong_pub_self_convention` into `wrong_self_convention`
- `for_loop_over_option` and `for_loop_over_result` → `for_loops_over_fallibles`
Lints that have already been merged since the issue was created:
- [x] `new_without_default` and `new_without_default_derive` → `new_without_default`
Need more discussion:
- `string_add` and `string_add_assign`: do we agree to merge them or not? Is there something more to do? → **not merge finally**
- `identity_op` and `modulo_one` → `useless_arithmetic`: seems outdated, since `modulo_arithmetic` has been created.
fixes#1078
changelog: Merging some lints together:
- `block_in_if_condition_expr` and `block_in_if_condition_stmt` → `blocks_in_if_conditions`
- `option_map_unwrap_or`, `option_map_unwrap_or_else` and `result_map_unwrap_or_else` → `map_unwrap_or`
- `option_unwrap_used` and `result_unwrap_used` → `unwrap_used`
- `option_expect_used` and `result_expect_used` → `expect_used`
- `for_loop_over_option` and `for_loop_over_result` → `for_loops_over_fallibles`
identity_op: allow `1 << 0`
I went for accepting `1 << 0` verbatim instead of something more general as it seems to be what everyone in the issue thread needed.
changelog: identity_op: allow `1 << 0` as it's a common pattern in bit manipulation code.
Fixes#3430
Downgrade useless_let_if_seq to nursery
I feel that this lint has the wrong balance of incorrect suggestions for a default-enabled lint.
The immediate code I faced was something like:
```rust
fn main() {
let mut good = do1();
if !do2() {
good = false;
}
if good {
println!("good");
}
}
fn do1() -> bool { println!("1"); false }
fn do2() -> bool { println!("2"); false }
```
On this code Clippy calls it unidiomatic and suggests the following diff, which has different behavior in a way that I don't necessarily want.
```diff
- let mut good = do1();
- if !do2() {
- good = false;
- }
+ let good = if !do2() {
+ false
+ } else {
+ do1()
+ };
```
On exploring issues filed about this lint, I have found that other users have also struggled with inappropriate suggestions (https://github.com/rust-lang/rust-clippy/issues/4124, https://github.com/rust-lang/rust-clippy/issues/3043, https://github.com/rust-lang/rust-clippy/issues/2918, https://github.com/rust-lang/rust-clippy/issues/2176) and suggestions that make the code worse (https://github.com/rust-lang/rust-clippy/issues/3769, https://github.com/rust-lang/rust-clippy/issues/2749). Overall I believe that this lint is still at nursery quality for now and should not be enabled.
---
changelog: Remove useless_let_if_seq from default set of enabled lints
Reversed empty ranges
This lint checks range expressions with inverted limits which result in empty ranges. This includes also the ranges used to index slices.
The lint reverse_range_loop was covering iteration of reversed ranges in a for loop, which is a subset of what this new lint covers, so it has been removed. I'm not sure if that's the best choice. It would be doable to check in the new lint that we are not in the arguments of a for loop; I went for removing it because the logic was too similar to keep them separated.
changelog: Added reversed_empty_ranges lint that checks for ranges where the limits have been inverted, resulting in empty ranges. Removed reverse_range_loop which was covering a subset of the new lint.
Closes#4192Closes#96
Rustup
Done with
```bash
git subtree push -P src/tools/clippy git@github.com:flip1995/rust-clippy rustup
```
from https://github.com/flip1995/rust/tree/clippyup
A rebase was required to get rid of empty merge commits, that somehow were not empty? 🤔
changelog: none
Allow `use super::*;` glob imports
changelog: Allow super::* glob imports
fixes#5554fixes#5569
A first pass at #5554 - this allows all `use super::*` to pass, which may or may not be desirable. The original issue was around allowing test modules to import their entire parent modules - I'm happy to modify this to do that instead, may just need some guidance on how to implement that (I played around a bit with #[cfg(test)] but from what I can gather, clippy itself isn't in test mode when running, even if the code in question is being checked for the test target).
Extend example for the `unneeded_field_pattern` lint
Current example is incorrect (or pseudo-code) because a struct name is omitted. I have used the code from the tests instead. Perhaps this example can be made less verbose, but I think it is more convenient to see a "real" code as an example.
---
changelog: extend example for the `unneeded_field_pattern` lint
Update to rustc changes
changelog: none
So, turns out `git subtree push` dies in various interesting ways, but the source cause is that the rustc repo looks like
```
--- A --- B --- C ---
\--- D ---/
```
where `B` is the commit where I added clippy to rustc and `D` is an arbitrary other PR and `C` is the master branch (or an earlier commit in it). When we now do `git subtree push`, it doesn't stop looking for things to merge at `B` as it needs to look at `D`, too, but then the bad thing happens, and it doesn't stop at `A` either, and just goes on looking at the entire history of rustc in a recursive bash script. That recursion then quickly runs into a stack overflow. While we can increase the stack size via `ulimit -s 60000`, that just means I was waiting for 30 minutes looking at `git subtree push` counting up the number of commits it has looked at. I aborted that, as a process that needs 30 mins for a push is not reasonable.
This PR cheats by just doing a `cp -r ../rustc/src/tools/clippy/* .` inside my clippy checkout and committing all changes. I'm working on getting us a better workflow, but until then, this workaround will work nicely. Note that this requires a `git subrepo pull` to have occurred in the `rustc` checkout. It's not necessary to merge that pull in order to update clippy, it's just necessary in order to not revert code in the clippy repo that hasn't been synced yet to the rustc repo.
