Check for clone-on-copy in argument positions
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.
Fixes#5436
changelog: apply clone_on_copy lint to func args as well
Earlier if arguments to method calls matched the above pattern they were
not reported. This patch ensures such arguments are checked as well.
Fixes#5436
Downgrade implicit_hasher to pedantic
From the [documentation](https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher), this lint is intended to suggest:
```diff
- pub fn foo(map: &mut HashMap<i32, i32>) { }
+ pub fn foo<S: BuildHasher>(map: &mut HashMap<i32, i32, S>) { }
```
I think this is pedantic. I get that this lint can benefit core libraries like serde, but that's exactly the use case for pedantic lints; a library like serde will [enable clippy_pedantic](fd6741f4b0/src/lib.rs (L304)) and take the time to go through everything possible. Similar for libraries doing a libz blitz style checkup before committing to a 1.0 release; it would make sense to run through all the available pedantic lints then.
But otherwise, for most codebases and certainly for industrial codebases, the above suggested change just makes the codebase more obtuse for questionable benefit.
changelog: Remove implicit_hasher from default set of enabled lints
Check fn header along with decl when suggesting to implement trait
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.
Fixes#5413, #4290
changelog: check fn header along with decl for should_implement_trait
Move cognitive_complexity to nursery
As discussed in https://github.com/rust-lang/rust-clippy/issues/5418#issuecomment-610054361; Clippy's current understanding of cognitive complexity is not good enough yet at analyzing code for understandability to have this lint be enabled by default.
changelog: Remove cognitive_complexity from default set of enabled lints
When checking for functions that are potential candidates for trait
implementations check the function header to make sure modifiers like
asyncness, constness and safety match before triggering the lint.
Fixes#5413, #4290
Downgrade unreadable_literal to pedantic
As motivated by #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
Add new lint for `Result<T, E>.map_or(None, Some(T))`
Fixes#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](https://github.com/rust-lang/rust-clippy/pull/2128)
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
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 let_unit_value to pedantic
Given that the false positive in #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
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
Use assoc int and float consts instead of module level ones
changelog: Recommend primitive type associated constants instead of module level constants
In Rust 1.43 integer and float primitive types will have a number of new associated constants. For example `MAX`, `MIN` and a number of constants related to the machine representation of floats. https://github.com/rust-lang/rust/pull/68952
These new constants are preferred over the module level constants in `{core,std}::{f*, u*, i*}`. I have in the last few days made sure that the documentation in the main rust repository uses the new constants in every place I could find (https://github.com/rust-lang/rust/pull/69860, https://github.com/rust-lang/rust/pull/70782). So the next step is naturally to make the linter recommend the new constants as well.
This PR only changes two lints. There are more. But I did not want the PR to be too big. And since I have not contributed to clippy before it felt saner to start with a small PR so I see if there are any quirks. More will come later.
rustup: update for the new Ty::walk interface.
The first commit fixes a portability bug in `setup-toolchain.sh`, while the second rewrites the handling of "trait impl methods" in `use_self` - even if `Ty::walk` could've still been used, it was IMO a misuse.
This could also serve as a PSA: *please* use `hir_ty_to_ty` instead of trying to compare `hir::Ty`s between themselves or against semantic `Ty`s. Its "quasi-deprecation" is 3 years old and doesn't really mean anything, just that it's currently uncached and that we should eventually querify it (either for a single HIR node, or for all of the nodes in an entire definition).
---
changelog: none
