New lint `match_vec_item`
Added new lint to warn a match on index item which can panic. It's always better to use `get(..)` instead.
Closes#5500
changelog: New lint `match_on_vec_items`
- Show just one error message with multiple suggestions in case of
using multiple times an OS in target family position
- Only suggest #[cfg(unix)] when the OS is in the Unix family
- Test all the operating systems
Don't trigger while_let_on_iterator when the iterator is recreated every iteration
r? @phansch
Fixes#1654
changelog: Fix false positive in [`while_let_on_iterator`]
Downgrade match_bool to pedantic
I don't quite buy the justification in https://rust-lang.github.io/rust-clippy/. The justification is:
> It makes the code less readable.
In the Rust codebases I've worked in, I have found people were comfortable using `match bool` (selectively) to make code more readable. For example, initializing struct fields is a place where the indentation of `match` can work better than the indentation of `if`:
```rust
let _ = Struct {
v: {
...
},
w: match doing_w {
true => ...,
false => ...,
},
x: Nested {
c: ...,
b: ...,
a: ...,
},
y: if doing_y {
...
} else { // :(
...
},
z: ...,
};
```
Or sometimes people prefer something a bit less pithy than `if` when the meaning of the bool doesn't read off clearly from the condition:
```rust
if set.insert(...) {
... // ???
} else {
...
}
match set.insert(...) {
// set.insert returns false if already present
false => ...,
true => ...,
}
```
Or `match` can be a better fit when the bool is playing the role more of a value than a branch condition:
```rust
impl ErrorCodes {
pub fn from(b: bool) -> Self {
match b {
true => ErrorCodes::Yes,
false => ErrorCodes::No,
}
}
}
```
And then there's plain old it's-1-line-shorter, which means we get 25% more content on a screen when stacking a sequence of conditions:
```rust
let old_noun = match old_binding.is_import() {
true => "import",
false => "definition",
};
let new_participle = match new_binding.is_import() {
true => "imported",
false => "defined",
};
```
Bottom line is I think this lint fits the bill better as a pedantic lint; I don't think linting on this by default is justified.
changelog: Remove match_bool from default set of enabled lints
Fixes#4226
This introduces the lint await_holding_lock. For async functions, we iterate
over all types in generator_interior_types and look for types named MutexGuard,
RwLockReadGuard, or RwLockWriteGuard. If we find one then we emit a lint.
If let else mutex
changelog: Adds lint to catch incorrect use of `Mutex::lock` in `if let` expressions with lock calls in any of the blocks.
closes: #5219
Fix issue #2907.
Update the "borrow box" lint to avoid recommending the following
conversion:
```
// Old
pub fn f(&mut Box<T>) {...}
// New
pub fn f(&mut T) {...}
```
Given a mutable reference to a box, functions may want to change
"which" object the Box is pointing at.
This change avoids recommending removing the "Box" parameter
for mutable references.
changelog: Don't trigger [`borrow_box`] lint on `&mut Box` references
Update the "borrow box" lint to avoid recommending the following
conversion:
```
// Old
pub fn f(&mut Box<T>) {...}
// New
pub fn f(&mut T) {...}
```
Given a mutable reference to a box, functions may want to change
"which" object the Box is pointing at.
This change avoids recommending removing the "Box" parameter
for mutable references.
add lint futures_not_send
changelog: add lint futures_not_send
fixes#5379
~Remark: one thing that can (should?) still be improved is to directly include the error message from the `Send` check so that the programmer stays in the flow. Currently, getting the actual error message requires a restructuring of the code to make the `Send` constraint explicit.~
It now shows all unmet constraints for allowing the Future to be Send.
Do not lint in macros for match lints
Don't lint in macros for match lints, more precisely in `check_pat` and `check_local` where it was not the case.
changelog: none
fixes: #5362
large_enum_variant: Report sizes of variants
This reports the sizes of the largest and second-largest variants.
Closes#5459
changelog: `large_enum_variant`: Report the sizes of the largest and second-largest variants.
Disallow bit-shifting in integer_arithmetic
Make the `integer_arithmetic` lint detect all the operations that are defined as being capable of overflow in the [Rust Reference](https://doc.rust-lang.org/reference/expressions/operator-expr.html#overflow), by also linting for bit-shifting operations (`<<`, `>>`).
changelog: Disallow bit-shifting in `integer_arithmetic`
Add lint on large non scalar const
This PR adds the new lint `non_scalar_const` that aims to warn against `const` declaration of large arrays. For performance, because of inlining, large arrays should be preferably declared as `static`.
Note: i made this one to warn on all const arrays, whether they are in a body function or not. I don't know if this is really necessary, i could just reduce this lint to variables out of function scope.
Fixes: #400
changelog: add new lint for large non-scalar types declared as const
Add lint for float in array comparison
Fixes#4277
changelog:
- Added new handler for expression of index kind (e.g. `arr[i]`). It returns a constant when both array and index are constant, or when the array is constant and all values are equal.
- Trigger float_cmp and float_cmp_const lint when comparing arrays. Allow for comparison when one of the arrays contains only zeros or infinities.
- Added appropriate tests for such cases.
This change adds a check to the `inconsistent_digit_grouping` to add a check for
NumericLiterals that follow the UUID format of 8-4-4-4-12.
If the NumericLiteral matches the UUID format, no further inconsistent grouping checks
will be performed.
Closes#5431
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
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 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
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](https://github.com/rust-lang/rust-clippy/pull/2128)
useless Rc<Rc<T>>, Rc<Box<T>>, Rc<&T>, Box<&T>
refers to #2394
changelog: Add lints for Rc<Rc<T>> and Rc<Box<T>> and Rc<&T>, Box<&T>
this is based on top of another change #5310 so probably should go after that one.
Downgrade option_option to pedantic
Based on a search of my work codebase (\>500k lines) for `Option<Option<`, it looks like a bunch of reasonable uses to me. The documented motivation for this lint is:
> an optional optional value is logically the same thing as an optional value but has an unneeded extra level of wrapping
which seems a bit bogus in practice. For example a typical usage would look like:
```rust
let mut host: Option<String> = None;
let mut port: Option<i32> = None;
let mut payload: Option<Option<String>> = None;
for each field {
match field.name {
"host" => host = Some(...),
"port" => port = Some(...),
"payload" => payload = Some(...), // can be null or string
_ => return error,
}
}
let host = host.ok_or(...)?;
let port = port.ok_or(...)?;
let payload = payload.ok_or(...)?;
do_thing(host, port, payload)
```
This lint seems to fit right in with the pedantic group; I don't think linting on occurrences of `Option<Option<T>>` by default is justified.
---
changelog: Remove option_option from default set of enabled lints
Lint unnamed address comparisons
Functions and vtables have an insignificant address. Attempts to compare such addresses will lead to very surprising behaviour. For example: addresses of different functions could compare equal; two trait object pointers representing the same object and the same type could be unequal.
Lint against unnamed address comparisons to avoid issues like those in rust-lang/rust#69757 and rust-lang/rust#54685.
changelog: New lints: [`fn_address_comparisons`] [#5294](https://github.com/rust-lang/rust-clippy/pull/5294), [`vtable_address_comparisons`] [#5294](https://github.com/rust-lang/rust-clippy/pull/5294)
`unused_self` false positive
fixes#5351
Remove the for loop in `unused_self` so that lint enabled for one method doesn't trigger on another method.
changelog: Fix false positive in `unused_self` around lint gates on impl items
Lint for `pub(crate)` items that are not crate visible due to the visibility of the module that contains them
changelog: Add `redundant_pub_crate` lint
Closes#5274.
Improvement: Don't show function body in needless_lifetimes
Changes the span on which the lint is reported to point to only the
function return type instead of the entire function body.
Fixes#5284
changelog: none
