Sync Clippy #72300
Merged
Sync Clippy #72300
+1,770
−1,102
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove ast::{Ident, Name} reexports.
The reexport of `Symbol` into `Name` confused me.
Re-remove util/dev file
Rustup Done with ```bash git subtree push -P src/tools/clippy [email protected]: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
Fix ICE caused in unwrap module changelog: Fix ICE in unwrap module with unexpected number of parameters for method of Option/Result Fixes rust-lang#5579
…flip1995 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 rust-lang#4192 Closes rust-lang#96
…ints into `block_in_if_condition` lint
…_map_unwrap_or_else` lints into `map_unwrap` lint
…or_loop_over_fallible` lint
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 (rust-lang/rust-clippy#4124, rust-lang/rust-clippy#3043, rust-lang/rust-clippy#2918, rust-lang/rust-clippy#2176) and suggestions that make the code worse (rust-lang/rust-clippy#3769, rust-lang/rust-clippy#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
Suggest to await future before ? operator Closes rust-lang#71811 cc rust-lang#61076
…oli-obk rustc_driver: factor out computing the exit code In a recent Miri PR I [added a convenience wrapper](https://github.com/rust-lang/miri/pull/1405/files#diff-c3d602c5c8035a16699ce9c015bfeceaR125) around `catch_fatal_errors` and `run_compiler` that @oli-obk suggested I could upstream. However, after seeing what could be shared between `rustc_driver::main`, clippy and Miri, really the only thing I found is computing the exit code -- so that's what this PR does. What prevents using the Miri convenience function in `rustc_driver::main` and clippy is that they do extra work inside `catch_fatal_errors`, and while I could abstract that away, clippy actually *computes the callbacks* inside there, and I fond no good way to abstract that and thus gave up. Maybe the clippy thing could be moved out, I am not sure if it ever can actually raise a `FatalErrorMarker` -- someone more knowledgeable in clippy would have to do that.
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 rust-lang#3430
Fix comparison_chain false positive changelog: comparison_chain: fix false positives when the binary operation is the same. Fixes rust-lang#5212
…reporting_cleanup, r=petrochenkov Literal error reporting cleanup While doing some performance work, I noticed some code duplication in `librustc_parser/lexer/mod.rs`, so I cleaned it up. This PR is probably best reviewed commit by commit. I'm not sure what the API stability practices for `librustc_lexer` are. Four public methods in `unescape.rs` can be removed, but two are used by clippy, so I left them in for now. I could open a PR for Rust-Analyzer when this one lands. But how do I open a PR for clippy? (Git submodules are frustrating to work with)
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 rust-lang#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`
- add `multispan_sugg_with_applicability` - not it gets `&str` instead of `String`, like in `diag.multispan_suggestion`
Maybe someday, git subtree will do it right
Rustup with git subtree The commits from the last rustup rust-lang#5587, are again included in this rustup, since I rebased the rustup. Lesson learned: never rebase, only merge when working with git subtree. changelog: none
- rename it to bind_instead_of_map
…lint, r=phansch Improve `option_and_then_some` lint fixed rust-lang#5492 changelog: Improve and generalize `option_and_then_some` and rename it to `bind_instead_of_map`.
…=flip1995 Rename lint `identity_conversion` to `useless_conversion` Lint name `identity_conversion` was misleading, so this PR renames it to `useless_conversion`. As decision has not really came up in the issue comments, this PR will probably need discussion. fixes rust-lang#3106 changelog: Rename lint `identity_conversion` to `useless_conversion`
rust-highfive
added
the
S-waiting-on-review
|
r=me if you don't want to wait specifically for @oli-obk to check this |
|
@Mark-Simulacrum I don't think a specific review is necessary, since I just ran I'm not in the reviewers list of rust-lang/rust though. |
|
@bors r+ In that case will just approve :) |
|
📌 Commit 4dc83a7 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc rust-lang/rust-clippy#5608
Included changes:
r? @oli-obk