Rollup of 11 pull requests #5438
Merged
Conversation
This file contains 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
… -> Option Result<T, E> has an `ok()` method that adapts a Result<T,E> into an Option<T>. It's possible to get around this adapter by writing Result<T,E>.map_or(None, Some). This lint is implemented as a new variant of the existing [`option_map_none` lint](rust-lang#2128)
Fix update_lints This fixes a bug in update_lints, where `internal` lints were not registered properly. This also cleans up some code. For example: The code generation functions no longer filter the lints the are given. This is now the task of the caller. This way, it is more obvious in the `replace_in_file` calls which lints will be included in which part of a file. This also turns the lint modules private. There is no need for them to be public, since shared code should be in the utils module anyway. And last but not least, this fixes the `register_lints` code generation, so also internal lints get registered. changelog: none
Downgrade let_unit_value to pedantic Given that the false positive in rust-lang#1502 is marked E-hard and I don't have much hope of it getting fixed, I think it would be wise to disable this lint by default. I have had to suppress this lint in every substantial codebase (\>100k line) I have worked in. Any time this lint is being triggered, it's always the false positive case. The motivation for this lint is documented as: > A unit value cannot usefully be used anywhere. So binding one is kind of pointless. with this example: > ```rust > let x = { > 1; > }; > ``` Sure, but the author would find this out via an unused_variable warning or from `x` not being the type that they need further down. If there ends up being a type error on `x`, clippy's advice isn't going to help get the code compiling because it can only run if the code already compiles. changelog: Remove let_unit_value from default set of enabled lints
Downgrade trivially_copy_pass_by_ref to pedantic
The rationale for this lint is documented as:
> In many calling conventions instances of structs will be passed through registers if they fit into two or less general purpose registers.
I think the purported performance benefits of clippy's recommendation are overstated. This isn't worth asking people to sprinkle code with more `*``*``&``*``&` to chase the alleged performance.
This should be a pedantic lint that is disabled by default and opted in if some specific performance sensitive codebase determines that it is worthwhile.
As a reminder, a typical place that a reference to a primitive would come up is if the function is used as a filter. Triggering a performance-oriented lint on this type of code is the definition of pedantic.
```rust
fn filter(_n: &i32) -> bool {
true
}
fn main() {
let v = vec![1, 2, 3];
v.iter().copied().filter(filter).for_each(drop);
}
```
```console
warning: this argument (4 byte) is passed by reference, but would be more efficient if passed by value (limit: 8 byte)
--> src/main.rs:1:15
|
1 | fn filter(_n: &i32) -> bool {
| ^^^^ help: consider passing by value instead: `i32`
```
changelog: Remove trivially_copy_pass_by_ref from default set of enabled lints
Downgrade inefficient_to_string to pedantic From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string): > ```diff > - ["foo", "bar"].iter().map(|s| s.to_string()); > > + ["foo", "bar"].iter().map(|&s| s.to_string()); > ``` I feel like saving 10 nanoseconds from the formatting machinery isn't worth asking the programmer to insert extra `&` / `*` noise in the *vast* majority of cases. This is a pedantic lint. changelog: Remove inefficient_to_string from default set of enabled lints
Add new lint for `Result<T, E>.map_or(None, Some(T))` Fixes rust-lang#5414 PR Checklist --- - [x] Followed lint naming conventions (the name is a bit awkward, but it seems to conform) - [x] Added passing UI tests (including committed .stderr file) - [x] cargo test passes locally - [x] Executed cargo dev update_lints - [x] Added lint documentation - [x] Run cargo dev fmt `Result<T, E>` has an [`ok()`](https://doc.rust-lang.org/std/result/enum.Result.html#method.ok) method that adapts a `Result<T,E>` into an `Option<T>`. It's possible to get around this adapter by writing `Result<T,E>.map_or(None, Some)`. This lint is implemented as a new variant of the existing [`option_map_none` lint](rust-lang#2128)
Update doc links and mentioned names in docs changelog: none
Downgrade unreadable_literal to pedantic As motivated by rust-lang#5418. This is the top most commonly suppressed Clippy style lint, which indicates that the community has decided they don't share Clippy's opinion on the best style of this. I've left the lint in as pedantic, though it could be that "restriction" would be better -- I can see this lint being useful as an opt-in restriction in some codebases. changelog: Remove unreadable_literal from default set of enabled lints
Downgrade new_ret_no_self to pedantic As motivated by rust-lang#5418. This is the second most widely suppressed Clippy style lint, and [this grep.app search](https://grep.app/search?q=%5C%5Ballow%5C%28.%2Aclippy%3A%3Anew_ret_no_self%5Cb®exp=true&case=true&filter[lang][0]=Rust) shows a large number of diverse reasonable signatures for a `new` method. changelog: Remove new_ret_no_self from default set of enabled lints
…lip1995 CONTRIBUTING.md: fix broken triage link Fixes rust-lang#5421
…l, r=flip1995 Incorrect suspicious_op_assign_impl fixes rust-lang#5255 changelog: In suspicious_op_assign_impl ignore all operators in expression if it's part of AssignOp
Ehance opt_as_ref_deref lint. - [x] Added passing UI tests (including committed `.stderr` file) - [x] `cargo test` passes locally - [x] Run `cargo dev fmt` Lint on opt.as_ref().map(|x| &**x). Fixes rust-lang#5367. changelog: lint on opt.as_ref().map(|x| &**x)
|
@bors r+ p=2 |
|
📌 Commit 381f9cb has been approved by |
|
🌲 The tree is currently closed for pull requests below priority 2, this pull request will be tested once the tree is reopened |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
This was referenced Apr 8, 2020
Merged
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.
Successful merges:
Result<T, E>.map_or(None, Some(T))#5415 (Add new lint forResult<T, E>.map_or(None, Some(T)))Failed merges:
r? @ghost
changelog: rollup