Easier readability for `needless_late_init` message
Closes#8530
Updated the lint to use a `MultiSpan`, showing where the `let` statement was first used and where the initialisation statement was done, as in the format described, for easier readability.
Was wondering why, when pushing the span label for the initialisation statement, that sometimes the prior statement above the initialisation statement gets pulled into the output as well - any insight is appreciated!
---
changelog: [`needless_late_init`]: Now shows the `let` statement where it was first initialized
[FP] identity_op in front of if
fix#8724
changelog: FP: [`identity_op`]: is now allowed in front of if statements, blocks and other expressions where the suggestion would be invalid.
Resolved simular problems with blocks, mathces, and loops.
identity_op always does NOT suggest reducing `0 + if b { 1 } else { 2 } + 3` into `if b { 1 } else { 2 } + 3` even in the case that the expression is in `f(expr)` or `let x = expr;` for now.
Previously, the `cast_lossless` lint would issue a warning on code that
converted a `usize` value to `f64`, on 32-bit targets.
`usize` to `f64` is a lossless cast on 32-bit targets, however there is
no corresponding `f64::from` that takes a `usize`, so `cast_lossless`'s
suggested replacement does not compile.
This PR disables the lint in the case of casting from `usize` or `isize`.
Fixes#3689.
changelog: [`cast_lossless`] no longer gives wrong suggestion on usize->f64
Only crate root def-ids don't have a parent, and in majority of cases the argument of `DefIdTree::parent` cannot be a crate root.
So we now panic by default in `parent` and introduce a new non-panicing function `opt_parent` for cases where the argument can be a crate root.
Same applies to `local_parent`/`opt_local_parent`.
Change `span_suggestion` (and variants) to take `impl ToString` rather
than `String` for the suggested code, as this simplifies the
requirements on the diagnostic derive.
Signed-off-by: David Wood <david.wood@huawei.com>
ignore `redundant_pub_crate` in `useless_attribute`
changelog: [`useless_attribute`] no longer lints [`redundant_pub_crate`]
As mentioned in https://github.com/rust-lang/rust-clippy/issues/8732#issuecomment-1106489634
> And it turns out I can't even explicitly allow it at the usage site, because then `clippy::useless_attribute` fires (which would also be a FP?), which is deny-by-default.
>
> Though it does work if I then allow `clippy::useless_attribute`. 😂
>
> ```rust
> #[allow(clippy::useless_attribute)]
> #[allow(clippy::redundant_pub_crate)]
> pub(crate) use bit;
> ```
>
> The originally-reported warning now no longer occurs.
`needless_late_init`: ignore `if let`, `let mut` and significant drops
No longer lints `if let`, personal taste on this one is pretty split, so it probably shouldn't be warning by default. Fixes#8613
```rust
let x = if let Some(n) = y {
n
} else {
1
}
```
No longer lints `let mut`, things like the following are not uncommon and look fine as they are
b169c16d86/src/sixty_four.rs (L88-L93)
Avoids changing the drop order in an observable way, where the type of `x` has a drop with side effects and something between `x` and the first use also does, e.g.
48cc6cb791/tests/test_api.rs (L159-L167)
The implementation of `type_needs_ordered_drop_inner` was changed a bit, it now uses `Ty::has_significant_drop` and reordered the ifs to check diagnostic name before checking the implicit drop impl
changelog: [`needless_late_init`]: No longer lints `if let` statements, `let mut` bindings and no longer significantly changes drop order
mistyped_literal_suffix: improve integer suggestions, avoid wrong float suggestions
This PR fixes 2 things:
- The known problem that integer types are always suggested as signed, by suggesting an unsigned suffix for literals that wouldnt fit in the signed type, and ignores any literals too big for the corresponding unsigned type too.
- The lint would only look at the integer part of any floating point literals without an exponent, this causing #6129. This just ignores those literals.
Examples:
```rust
let _ = 2_32; // still 2_i32
let _ = 234_8; // would now suggest 234_u8
// these are now ignored
let _ = 500_8;
let _ = 123_32.123;
```
changelog: suggest correct integer types in [`mistyped_literal_suffix`], ignore float literals without an exponent
fixes#6129
Previously this lint would only look at the integer part of floating
point literals without an exponent, giving wrong suggestions like:
```
|
8 | let _ = 123_32.123;
| ^^^^^^^^^^ help: did you mean to write: `123.123_f32`
|
```
Instead, it now ignores these literals.
Fixes#6129
Instead of just always suggesting signed suffixes regardless of size
of the value, it now suggests an unsigned suffix when the value wouldn't
fit into the corresponding signed type, and ignores the literal entirely
if it is too big for the unsigned type as well.
wrong_self_convention allows `is_*` to take `&mut self`
fix#8480 and #8513
Allowing `is_*` to take `&self` or none is too restrictive.
changelog: FPs: [`wrong_self_convention`] now allows `&mut self` and no self as arguments for `is_*` methods
`manual_split_once`: lint manual iteration of `SplitN`
changelog: `manual_split_once`: lint manual iteration of `SplitN`
Now lints:
```rust
let mut iter = "a.b.c".splitn(2, '.');
let first = iter.next().unwrap();
let second = iter.next().unwrap();
let mut iter = "a.b.c".splitn(2, '.');
let first = iter.next()?;
let second = iter.next()?;
let mut iter = "a.b.c".rsplitn(2, '.');
let first = iter.next().unwrap();
let second = iter.next().unwrap();
let mut iter = "a.b.c".rsplitn(2, '.');
let first = iter.next()?;
let second = iter.next()?;
```
It suggests (minus leftover whitespace):
```rust
let (first, second) = "a.b.c".split_once('.').unwrap();
let (first, second) = "a.b.c".split_once('.')?;
let (second, first) = "a.b.c".rsplit_once('.').unwrap();
let (second, first) = "a.b.c".rsplit_once('.')?;
```
Currently only lints if the statements are next to each other, as detecting the various kinds of shadowing was tricky, so the following won't lint
```rust
let mut iter = "a.b.c".splitn(2, '.');
let something_else = 1;
let first = iter.next()?;
let second = iter.next()?;
```
Less authoritative stable_sort_primitive message
fixes#8241
Hey all - first contribution here so I'm deciding to start with something small.
Updated the linked message to be less authoritative as well as moved the lint grouping from `perf` to `pedantic` as suggested by `@camsteffen` under the issue.
changelog: [`stable_sort_primitive`]: emit less authoritative message and move to `pedantic`
Fix needless_match false positive for if-let when the else block doesn't match to given expr
<!--
Thank you for making Clippy better!
We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
`changelog: none`. Otherwise, please write a short comment
explaining your change. Also, it's helpful for us that
the lint name is put into brackets `[]` and backticks `` ` ` ``,
e.g. ``[`lint_name`]``.
If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[ ] Followed [lint naming conventions][lint_naming]
- \[ ] Added passing UI tests (including committed `.stderr` file)
- \[x] `cargo test` passes locally
- \[x] Executed `cargo dev update_lints`
- \[ ] Added lint documentation
- \[x] Run `cargo dev fmt`
[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR.
--->
fix#8695
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: Fixed ``[`needless_match`]`` false positive when else block expression differs.
Take over: New lint bytes count to len
take over #8375close#8083
This PR adds new lint about considering replacing `.bytes().count()` with `.len()`.
Thank you in advance.
---
r! `@Manishearth`
changelog: adds new lint [`bytes_count_to_len`] to consider replacing `.bytes().count()` with `.len()`
Stop using CRATE_DEF_INDEX outside of metadata encoding.
`CRATE_DEF_ID` and `CrateNum::as_def_id` are almost always what we want. We should not manipulate raw `DefIndex` outside of metadata encoding.
adding test patterns
cargo dev bless
fix comment
add ;
delete :
fix suggestion code
and update stderr in tests.
use match_def_path when checking method name
Add `await_holding_invalid_type` lint
changelog: [`await_holding_invalid_type`]
This lint allows users to create a denylist of types which are not allowed to be
held across await points. This is essentially a re-implementation of the
language-level [`must_not_suspend`
lint](https://github.com/rust-lang/rust/issues/83310). That lint has a lot of
work still to be done before it will reach Rust stable, and in the meantime
there are a lot of types which can trip up developers if they are used
improperly.
I originally implemented this specifically for `tracing::span::Entered`, until I discovered #8434 and read the commentary on that PR. Given this implementation is fully user configurable, doesn't tie clippy to any one particular crate, and introduces no additional dependencies, it seems more appropriate.
Refactor HIR item-like traversal (part 1)
Issue #95004
- Create hir_crate_items query which traverses tcx.hir_crate(()).owners to return a hir::ModuleItems
- use tcx.hir_crate_items in tcx.hir().items() to return an iterator of hir::ItemId
- use tcx.hir_crate_items to introduce a tcx.hir().par_items(impl Fn(hir::ItemId)) to traverse all items in parallel;
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
cc `@cjgillot`
Fix formatting of `cast_abs_to_unsigned` docs
The "use instead" section of the example was not being formatted as Rust code, and the "configuration" documentation was being formatted as Rust code.
changelog: `[cast_abs_to_unsigned]` Fix example/configuration formatting
Implement sym operands for global_asm!
Tracking issue: #93333
This PR is pretty much a complete rewrite of `sym` operand support for inline assembly so that the same implementation can be shared by `asm!` and `global_asm!`. The main changes are:
- At the AST level, `sym` is represented as a special `InlineAsmSym` AST node containing a path instead of an `Expr`.
- At the HIR level, `sym` is split into `SymStatic` and `SymFn` depending on whether the path resolves to a static during AST lowering (defaults to `SynFn` if `get_early_res` fails).
- `SymFn` is just an `AnonConst`. It runs through typeck and we just collect the resulting type at the end. An error is emitted if the type is not a `FnDef`.
- `SymStatic` directly holds a path and the `DefId` of the `static` that it is pointing to.
- The representation at the MIR level is mostly unchanged. There is a minor change to THIR where `SymFn` is a constant instead of an expression.
- At the codegen level we need to apply the target's symbol mangling to the result of `tcx.symbol_name()` depending on the target. This is done by calling the LLVM name mangler, which handles all of the details.
- On Mach-O, all symbols have a leading underscore.
- On x86 Windows, different mangling is used for cdecl, stdcall, fastcall and vectorcall.
- No mangling is needed on other platforms.
r? `@nagisa`
cc `@eddyb`
changelog: [`await_holding_invalid_type`]
This lint allows users to create a denylist of types which are not allowed to be
held across await points. This is essentially a re-implementation of the
language-level [`must_not_suspend`
lint](https://github.com/rust-lang/rust/issues/83310). That lint has a lot of
work still to be done before it will reach Rust stable, and in the meantime
there are a lot of types which can trip up developers if they are used
improperly.
New lint `format_add_strings`
Closes#6261
changelog: Added [`format_add_string`]: recommend using `write!` instead of appending the result of `format!`
Add `usize` cast to `clippy::manual_bits` suggestion
A fix for the suggestion from https://github.com/rust-lang/rust-clippy/pull/8213
changelog: [`manual_bits`]: The suggestion now includes a cast for proper type conversion
This adds test to make sure correct behavior of lint
- The first test's option variable is not a temporary variable
- The second test does not make usage of `take()`
- The third test makes usage of `take()` and uses a temporary variable
Instead of type checking the entire expression (causing a false
positive), only type check for a subset of the expression (the receiver of
the matched function: `take()`)
This lint checks if Option::take() is used on a temporary value (a value
that is not of type &mut Option and that is not a Place expression) to
suggest omitting take()
Check for loops/closures in `local_used_after_expr`
Follow up to #8646, catches when a local is used multiple times because it's in a loop or a closure
changelog: none
errors: lazily load fallback fluent bundle
Addresses (hopefully) https://github.com/rust-lang/rust/pull/95667#issuecomment-1094794087.
Loading the fallback bundle in compilation sessions that won't go on to emit any errors unnecessarily degrades compile time performance, so lazily create the Fluent bundle when it is first required.
r? `@ghost` (just for perf initially)
Use mir constant in thir instead of ty::Const
This is blocked on https://github.com/rust-lang/rust/pull/94059 (does include its changes, the first two commits in this PR correspond to those changes) and https://github.com/rust-lang/rust/pull/93800 being reinstated (which had to be reverted). Mainly opening since `@lcnr` offered to give some feedback and maybe also for a perf-run (if necessary).
This currently contains a lot of duplication since some of the logic of `ty::Const` had to be copied to `mir::ConstantKind`, but with the introduction of valtrees a lot of that functionality will disappear from `ty::Const`.
Only the last commit contains changes that need to be reviewed here. Did leave some `FIXME` comments regarding future implementation decisions and some things that might be incorrectly implemented.
r? `@oli-obk`
Loading the fallback bundle in compilation sessions that won't go on to
emit any errors unnecessarily degrades compile time performance, so
lazily create the Fluent bundle when it is first required.
Signed-off-by: David Wood <david.wood@huawei.com>
fix unnecessary_to_owned about msrv
This PR fixes ``[`unnecessary_owned`]``.
## What
```rust
# sample code
fn _msrv_1_35() {
#![clippy::msrv = "1.35"]
let _ = &["x"][..].to_vec().into_iter();
}
fn _msrv_1_36() {
#![clippy::msrv = "1.36"]
let _ = &["x"][..].to_vec().into_iter();
}
```
If we will check this code using clippy, ``[`unnecessary_owned`]`` will modify the code as follows.
```rust
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:219:14
|
LL | let _ = &["x"][..].to_vec().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
error: unnecessary use of `to_vec`
--> $DIR/unnecessary_to_owned.rs:224:14
|
LL | let _ = &["x"][..].to_vec().into_iter();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `["x"][..].iter().copied()`
```
This is incorrect. Because `Iterator::copied` was estabilished in 1.36.
## Why
This bug was caused by not separating "copied" and "clone" by reference to msrv.
89ee6aa6e3/clippy_lints/src/methods/unnecessary_to_owned.rs (L195)
So, I added a conditional branch and described the corresponding test.
Thank you in advance.
changelog: fix wrong suggestions about msrv in [`unnecessary_to_owned`]
r! `@giraffate`
Don't lint `manual_non_exhaustive` when the enum variant is used
fixes#5714
changelog: Don't lint `manual_non_exhaustive` when the enum variant is used
Do not trigger ``[`rest_pat_in_fully_bound_structs`]`` on `#[non_exhaustive]` structs
fixes#8029
Just adds an additional check to ensure that the`ty::VariantDef` is not marked as `#[non_exhaustive]`.
changelog: Do not apply ``[`rest_pat_in_fully_bound_structs`]`` on structs marked as non exhaustive.
Prevent infinite (exponential) recursion in only_used_in_recursion
This simplifies the visitor code a bit and prevents checking expressions
multiple times. I still think this lint should be removed for now,
because its code isn't really tested.
Fixes#8689
**NOTE:** Before merging this, we should talk about removing and revisiting this lint. See my comment in #8689
changelog: prevent infinite recursion in [`only_used_in_recursion`]
This simplifies the visitor code a bit and prevents checking expressions
multiple times. I still think this lint should be removed for now,
because its code isn't really tested.
adding condition for map_clone message
This PR fixes the message about `map_clone`.
if msrv >= 1.36, the message is correct.
```bash
$ cat main.rs
fn main() {
let x: Vec<&i32> = vec![&1, &2];
let y: Vec<_> = x.iter().map(|i| *i).collect();
println!("{:?}", y);
}
$ cargo clippy
warning: you are using an explicit closure for copying elements
--> main.rs:3:20
|
3 | let y: Vec<_> = x.iter().map(|i| *i).collect();
| ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `copied` method: `x.iter().copied()`
|
= note: `#[warn(clippy::map_clone)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
warning: `test` (build script) generated 1 warning
warning: `test` (bin "test") generated 1 warning (1 duplicate)
Finished dev [unoptimized + debuginfo] target(s) in 0.00s
```
but, if msrv < 1.36, the suggestion is `cloned`, but the message is `copying`.
```bash
$ cat clippy.toml
msrv = "1.35"
$ cargo clippy
warning: you are using an explicit closure for copying elements
--> main.rs:3:20
|
3 | let y: Vec<_> = x.iter().map(|i| *i).collect();
| ^^^^^^^^^^^^^^^^^^^^ help: consider calling the dedicated `cloned` method: `x.iter().cloned()`
```
I think the separation of messages will make it more user-friendly.
thank you in advance.
changelog: Fixed a message in map_clone.
New lint `is_digit_ascii_radix`
Closes#6399
changelog: Added [`is_digit_ascii_radix`]: recommend `is_ascii_digit()` or `is_ascii_hexdigit()` in place of `is_digit(10)` and `is_digit(16)`
Fix subtraction overflow in `cast_possible_truncation`
changelog: Fix false negative due to subtraction overflow in `cast_possible_truncation`
I *think* a false negative is the worst that can happen from this
Don't lint various match lints when expanded by a proc-macro
fixes#4952
As always for proc-macro output this is a hack-job of a fix. It would be really nice if more proc-macro authors would set spans correctly.
changelog: Don't lint various lints on proc-macro output.
Remove overlap between `manual_split_once` and `needless_splitn`
changelog: Remove overlap between [`manual_split_once`] and [`needless_splitn`]. Fixes some incorrect `rsplitn` suggestions for [`manual_split_once`]
Things that can trigger `needless_splitn` no longer trigger `manual_split_once`, e.g.
```rust
s.[r]splitn(2, '=').next();
s.[r]splitn(2, '=').nth(0);
s.[r]splitn(3, '=').next_tuple();
```
Fixes some suggestions:
```rust
let s = "should not match";
s.rsplitn(2, '.').nth(1);
// old -> Some("should not match")
Some(s.rsplit_once('.').map_or(s, |x| x.0));
// new -> None
s.rsplit_once('.').map(|x| x.0);
s.rsplitn(2, '.').nth(1)?;
// old -> "should not match"
s.rsplit_once('.').map_or(s, |x| x.0);
// new -> early returns
s.rsplit_once('.')?.0;
```
Cached stable hash cleanups
r? `@nnethercote`
Add a sanity assertion in debug mode to check that the cached hashes are actually the ones we get if we compute the hash each time.
Add a new data structure that bundles all the hash-caching work to make it easier to re-use it for different interned data structures
- Create hir_crate_items query which traverses tcx.hir_crate(()).owners to return a hir::ModuleItems
- use tcx.hir_crate_items in tcx.hir().items() to return an iterator of hir::ItemId
- add par_items(impl Fn(hir::ItemId)) to traverse all items in parallel
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
ignore `&x | &y` in unnested_or_patterns
replacing it with `&(x | y)` is actually more characters
Fixes#6973
changelog: [`unnested_or_patterns`] ignore `&x | &y`, nesting would result in more characters
Add a lint to detect cast to unsigned for abs() and suggest unsigned_…
…abs()
changelog: Add a [`cast_abs_to_unsigned`] that checks for uses of `abs()` that are cast to the corresponding unsigned integer type and suggest to replace them with `unsigned_abs()`.
