Further enhance `needless_borrow`, mildly refactor `redundant_clone`
This PR does the following:
* Moves some code from `redundant_clone` into a new `clippy_utils` module called `mir`, and wraps that code in a function called `dropped_without_further_use`.
* Relaxes the "is copyable" condition condition from #9136 by also suggesting to remove borrows from values dropped without further use. The changes involve the just mentioned function.
* Separates `redundant_clone` into modules.
Strictly speaking, the last bullet is independent of the others. `redundant_clone` is somewhat hairy, IMO. Separating it into modules makes it slightly less so, by helping to delineate what depends upon what.
I've tried to break everything up into digestible commits.
r? `@Jarcho`
(`@Jarcho` I hope you don't mind.)
changelog: continuation of #9136
Add `manual_filter` lint for `Option`
Share much of its implementation with `manual_map` and should greatly benefit from its previous feedback.
I'm sure it's possible to even more refactor both and would gladly take input on that as well as any clippy idiomatic usage, since this is my first lint addition.
I've added the lint to the complexity section for now, I don't know if every new lint needs to go in nursery first.
The matching could be expanded to more than `Some(<value>)` to lint on arbitrary struct matching inside the `Some` but I've left it like it was for `manual_map` for now. `needless_match::pat_same_as_expr` provides a more generic match example.
close https://github.com/rust-lang/rust-clippy/issues/8822
changelog: Add lint [`manual_filter`] for `Option`
extend `box-default` lint, add suggestion
This extends the recently added `box-default` lint to also cover `Box::new(vec![])`, `Box::new(String::from(""))` and `Box::new(Vec::from([]))`. Also the lint now suggests a suitable replacement. I did not find a simple way to check whether the type is fully determined by the outside, so I at least checked for some variations to remove the turbofish in those cases.
---
changelog: none
lint::unsafe_removed_from_name: fix false positive result when allowed
changelog: [`unsafe_removed_from_name`] Fix allowing on imports produces a false positive on `useless_attribute`.
Fixes: #9197
Signed-off-by: Andy-Python-Programmer <andypythonappdeveloper@gmail.com>
FormatArgsExpn: Find comma spans and ignore weird proc macro spans
Fixes the following cases:
A missing `, 1` from the `expect_fun_call` suggestion:
```rust
Some(()).expect(&format!("{x} {}", 1));
```
```
warning: use of `expect` followed by a function call
--> t.rs:7:14
|
7 | Some(()).expect(&format!("{x} {}", 1));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or_else(|| panic!("{x} {}"))`
```
The suggestion removing from the comma in the comment rather than the one after the format string:
```rust
println!(
"{}",
// a comment, with a comma in it
x
);
```
```
warning: variables can be used directly in the `format!` string
--> t.rs:9:5
|
9 | / println!(
10 | | "{}",
11 | | // a comment, with a comma in it
12 | | x
13 | | );
| |_____^
|
help: change this to
|
10 ~ "{x}",
11 ~ // a comment
|
```
It also no longer accepts expansions where a format string or argument has a "weird" proc macro span, that is one where the literal/expression it outputs has the span of one of its inputs. Kind of like a `format_args` specific `clippy_utils::is_from_proc_macro`, e.g. `format!(indoc! {" ... "})`
changelog: [`expect_fun_call`]: Fix suggestion for `format!` using captured variables
changelog: [`print_literal`], [`write_literal`], [`uninlined_format_args`]: Fix suggestion when following a comment including a comma
[arithmetic-side-effects] Do not ignore literal references
To my utter surprise, `rustc` does does not warn stuff like `let n: u8 = &255 + &1` or `let n: u8 = 255 + &1`.
changelog: [arithmetic-side-effects] Do not ignore literal references
fallout: fix tests to allow uninlined_format_args
In order to switch `clippy::uninlined_format_args` from pedantic to style, all existing tests must not raise a warning. I did not want to change the actual tests, so this is a relatively minor change that:
* add `#![allow(clippy::uninlined_format_args)]` where needed
* normalizes all allow/deny/warn attributes
* all allow attributes are grouped together
* sorted alphabetically
* the `clippy::*` attributes are listed separate from the other ones.
* deny and warn attributes are listed before the allowed ones
See also https://github.com/rust-lang/rust-clippy/pull/9233, https://github.com/rust-lang/rust-clippy/pull/9525, https://github.com/rust-lang/rust-clippy/pull/9527
cc: `@llogiq` `@Alexendoo`
changelog: none
Fix and improve `match_type_on_diagnostic_item`
This extracts the fix for the lint out of #7647. There's still a couple of other functions to check, but at least this will get lint working again.
The two added util functions (`is_diagnostic_item` and `is_lang_item`) are needed to handle `DefId` for unit and tuple struct/variant constructors. The `rustc_diagnostic_item` and `lang` attributes are attached to the struct/variant `DefId`, but most of the time they are used through their constructors which have a different `DefId`. The two utility functions will check if the `DefId` is for a constructor and switch to the associated struct/variant `DefId`.
There does seem to be a bug on rustc's side where constructor `DefId`s from external crates seem to be returning `DefKind::Variant` instead of `DefKind::Ctor()`. There's a workaround put in right.
changelog: None
In order to switch `clippy::uninlined_format_args` from pedantic to
style, all existing tests must not raise a warning. I did not want to
change the actual tests, so this is a relatively minor change that:
* add `#![allow(clippy::uninlined_format_args)]` where needed
* normalizes all allow/deny/warn attributes
* all allow attributes are grouped together
* sorted alphabetically
* the `clippy::*` attributes are listed separate from the other ones.
* deny and warn attributes are listed before the allowed ones
changelog: none
Remove unused `.fixed` files, only run asm_syntax doctests on x86
Two small changes, removes some unused `.fixed` and makes `clippy_lints` doctests pass on non x86 arches
changelog: none
* Check for `const`s and `static`s from external crates
* Check for `LangItem`s
* Handle inherent functions which have the same name as a field
* Also check the following functions:
* `match_trait_method`
* `match_def_path`
* `is_expr_path_def_path`
* `is_qpath_def_path`
* Handle checking for a constructor to a diagnostic item or `LangItem`
let unnecessary_cast work for trivial non_literal expressions
Signed-off-by: TennyZhuang <zty0826@gmail.com>
---
changelog: [`unnecessary_cast`]: fix for trivial non_literal expressions
Fixes#9562
[`unnecessary_cast`] add parenthesis when negative number uses a method
fix#9563
The issue was probably introduced by 90fe3bea52
changelog: [`unnecessary_cast`] add parenthesis when negative number uses a method
r? llogiq
Implement `manual_clamp` lint
Fixes#9477Fixes#6751
Identifies common patterns where usage of the `clamp` function would be more succinct and clear, and suggests using the `clamp` function instead.
changelog: [`manual_clamp`]: Implement manual_clamp lint
This lint detects calls to a `&self`-taking `as_ptr` method, where
the result is then immediately cast to a `*mut T`. Code like this
is probably invalid, as that pointer will not have write permissions,
and `*mut T` is usually used to write through.
fix [`needless_borrow`], [`explicit_auto_deref`] FPs on unions
fix https://github.com/rust-lang/rust-clippy/issues/9383
changelog: fix [`needless_borrow`] false positive on unions
changelog: fix [`explicit_auto_deref`] false positive on unions
Left a couple debug derived impls on purpose I needed to debug as I don't think it's noise
Don't lint unstable moves in `std_instead_of_core`
Fixes#9515
changelog: [`std_instead_of_core`]: No longer suggests unstable modules such as `core::error`
add `box-default` lint
This adds a `box-default` lint to suggest using `Box::default()` instead of `Box::new(Default::default())`, which offers less moving parts and potentially better performance according to [the perf book](https://nnethercote.github.io/perf-book/standard-library-types.html#box).
---
changelog: add [`box_default`] lint
[`needless_return`] Recursively remove unneeded semicolons
fix#8336,
fix#8156,
fix https://github.com/rust-lang/rust-clippy/issues/7358,
fix#9192,
fix https://github.com/rust-lang/rust-clippy/issues/9503
changelog: [`needless_return`] Recursively remove unneeded semicolons
For now the suggestion about removing the semicolons are hidden because they would be very noisy and should be obvious if the user wants to apply the lint manually instead of using `--fix`. This could be an issue for beginner, but haven't found better way to display it.
[arithmetic-side-effects] Consider references
Takes into consideration integer references like `&i32::MAX` because currently things like `let _ = &1 + 0` trigger the lint.
changelog: FP: [`arithmetic_side_effects`]: Now ignores references
[9507](https://github.com/rust-lang/rust-clippy/pull/9507)
Don't lint `*_interior_mutable_const` on unions due to potential ICE.
fixes#9445
cc rust-lang/rust#101113
This started ICE'ing sometime last month due to stricter UB checks. I'm not sure how we could check the value of a union as MIRI doesn't seem to store which field is currently active.
changelog: Don't ICE on const unions containing a `!Freeze` type.
Silence [`question_mark`] in const context
fix https://github.com/rust-lang/rust-clippy/issues/9175
When `const_try` is stabilised can be turned into a MSRV
changelog: Silence [`question_mark`] in const context
Stabilize const `BTree{Map,Set}::new`
The FCP was completed in #71835.
Since `len` and `is_empty` are not const stable yet, this also creates a new feature for them since they previously used the same `const_btree_new` feature.
Implement https://github.com/rust-lang/rust-clippy/issues/8368 - a new
lint to inline format arguments such as `print!("{}", var)` into
`print!("{var}")`.
code | suggestion | comment
---|---|---
`print!("{}", var)` | `print!("{var}")` | simple variables
`print!("{0}", var)` | `print!("{var}")` | positional variables
`print!("{v}", v=var)` | `print!("{var}")` | named variables
`print!("{0} {0}", var)` | `print!("{var} {var}")` | aliased variables
`print!("{0:1$}", var, width)` | `print!("{var:width$}")` | width
support
`print!("{0:.1$}", var, prec)` | `print!("{var:.prec$}")` | precision
support
`print!("{:.*}", prec, var)` | `print!("{var:.prec$}")` | asterisk
support
code | suggestion | comment
---|---|---
`print!("{0}={1}", var, 1+2)` | `print!("{var}={0}", 1+2)` | Format
string uses an indexed argument that cannot be inlined. Supporting this
case requires re-indexing of the format string.
changelog: [`uninlined_format_args`]: A new lint to inline format
arguments, i.e. `print!("{}", var)` into `print!("{var}")`
Since `len` and `is_empty` are not const stable yet, this also
creates a new feature for them since they previously used the same
`const_btree_new` feature.
[`never_loop`]: Fix FP with let..else statements.
Fixes#9356
This has been bugging me for a while, so I thought I'd take a stab at it! I'm completely uncertain about the quality of my code, but I think it's an alright start, so opening this PR to get some feedback from more experienced clippy people :)
changelog: [`never_loop`]: Fix FP with let..else statements
Fixes#9504
Compiler generated call `into_iter` nodes return empty substs
which we need when checking it's predicates. Handle this by
simply exitting when we encounter one. This change introduces
false negatives in place of the ICEs.
[arithmetic-side-effects] Finish non-overflowing ops
Extends https://github.com/rust-lang/rust-clippy/pull/9474 to also take into consideration "raw" binary operations. For example, `let a = b / 2` and `let a = 1 * b` won't trigger the lint.
changelog: [arithmetic-side-effects] Finish non-overflowing ops
Make module-style lints resilient to --remap-path-prefix
changelog: [`self_named_module_files`], [`mod_module_files`]: Make module-style lints resilient to `--remap-path-prefix`
Without this if a user has configured `--remap-path-prefix` to be used for a prefix containing the current source directory the lints would silently fail to generate a warning.
Migrate write.rs to a late pass
changelog: Migrates write.rs from a pre expansion pass to a late pass
changelog: [`positional_named_format_parameters`] is renamed in favour of the rustc lint `named_arguments_used_positionally`
- Macros are now identified by diagnostic items, so will no longer lint user defined macros named, e.g. a custom `print!`
- `print_literal`/`write_literal` no longer lint no longer lint literals that come from macro expansions, e.g. `env!("FOO")`
- `print_with_newline`/`write_with_newline` no longer lint strings with any internal `\r` or `\n`s
~~A false negative, `print_literal`/`write_literal` don't lint format strings that produce `FormatSpec`s, e.g. ones containing pretty print/width/align specifiers~~
Suggestion changes:
- ~~`print_literal`/`write_literal` no longer have suggestions, as the spans for the `{}`s were not easily obtainable~~
- `print_with_newline`/`write_with_newline` has a better suggestion for a sole literal newline, but no longer has suggestions for len > 1 strings that end in a literal newline
- ~~`use_debug` spans are less precise, now point to the whole format string~~
The diff for write.rs is pretty unwieldy, other than for the `declare_clippy_lint!`s I think you'd be better off viewing it as a brand new file rather than looking at the diff, as it's mostly written from scratch
cc #6610, fixes#5721, fixes#7195, fixes#8615
Fix `unused_peekable` closure and `f(&mut peekable)` false positives
changelog: Fix [`unused_peekable`] false positive when peeked in a closure or called as `f(&mut peekable)`
The `return`/`break` changes aren't part of the fix, they allow an earlier return in some cases. `break` is replaced with `return` for style purposes as they do the same thing in this case
Fixes#9456Fixes#9462
Don't panic on invalid shift while constfolding
Instead of panicking on invalid shifts while folding constants we simply give up. Fixes#9463
Notice the "attempt to shift right by `1316134912_u32`", which seems weird. AFAICS it comes from rustc itself.
changelog: none
Don't lint `large_stack_array` inside static items
We now check if the linted `Expr` is inside an `ItemKind::Static`, which can't take the suggested `Box<[...]`. I _think_ this is the correct fix for #9460
I removed `if_chain` while I was at it.
changelog: Don't lint `large_stack_array` inside static items
Use macro callsite when creating `Sugg` helper
Closes#9375
changelog: Improvement: [`collapsible_if`]: Suggestions now work with macros, by taking the call site into account.
Replace u128 with u64 in large_enum_variant uitest
A u128 has [an 8 byte alignment on x86](https://github.com/rust-lang/rust/issues/54341), but a 16 byte alignment on aarch64 which changes the size of the enums due to extra padding. This means the test fails on aarch64
changelog: none
Fix `range_{plus,minus}_one` bad suggestions
Fixes#9431.
The current `range_plus_one` and `range_minus_one` suggestions are completely incorrect when macros are involved.
This commit resolves this by disabling the lints for any range expression that is expanded from a macro. The reasons for this are that it is very difficult to create a correct suggestion in this case and that false negatives are less important for pedantic lints.
changelog: Fix `range_{plus,minus}_one` bad suggestions
Fixes#9431.
The current `range_plus_one` and `range_minus_one` suggestions
are completely incorrect when macros are involved.
This commit resolves this by disabling the lints for any range
expression that is expanded from a macro. The reasons for this
are that it is very difficult to create a correct suggestion in
this case and that false negatives are less important for
pedantic lints.
[Arithmetic] Consider literals
Fixes https://github.com/rust-lang/rust-clippy/issues/9307 and makes the `arithmetic` lint behave like `integer_arithmetic`.
It is worth noting that literal integers of a binary operation (`1 + 1`, `i32::MAX + 1`), **regardless if they are in a constant environment**, won't trigger the lint. Assign operations also have similar reasoning.
changelog: Consider literals in the arithmetic lint
Suggest `unwrap_or_default` when closure returns `"".to_string`
Closes https://github.com/rust-lang/rust-clippy/issues/9420
changelog: [`unwrap_or_else_default`]: suggest `unwrap_or_default()` instead of `unwrap_or_else` with a closure that returns an empty `to_string`.
`BindingAnnotation` refactor
* `ast::BindingMode` is deleted and replaced with `hir::BindingAnnotation` (which is moved to `ast`)
* `BindingAnnotation` is changed from an enum to a tuple struct e.g. `BindingAnnotation(ByRef::No, Mutability::Mut)`
* Associated constants added for convenience `BindingAnnotation::{NONE, REF, MUT, REF_MUT}`
One goal is to make it more clear that `BindingAnnotation` merely represents syntax `ref mut` and not the actual binding mode. This was especially confusing since we had `ast::BindingMode`->`hir::BindingAnnotation`->`thir::BindingMode`.
I wish there were more symmetry between `ByRef` and `Mutability` (variant) naming (maybe `Mutable::Yes`?), and I also don't love how long the name `BindingAnnotation` is, but this seems like the best compromise. Ideas welcome.
Unlike past similar work done in #6228, expand the existing `or_fun_call`
lint to detect `or_insert` calls with a `T::new()` or `T::default()`
argument, much like currently done for `unwrap_or` calls. In that case,
suggest the use of `or_default`, which is more idiomatic.
Note that even with this change, `or_insert_with(T::default)` calls
aren't detected as candidates for `or_default()`, in the same manner
that currently `unwrap_or_else(T::default)` calls aren't detected as
candidates for `unwrap_or_default()`.
Also, as a nearby cleanup, change `KNOW_TYPES` from `static` to `const`,
since as far as I understand it's preferred (should Clippy have a lint
for that?).
Fixes#3812.
Fixes#9351.
Note that this commit reworks that fix for #9317. The change
is to check that the type implements `AsRef<str>` before regarding
`to_string` as an equivalent of `to_owned`. This was suggested
by Jarcho in the #9317 issue comments.
The benefit of this is that it moves some complexity out of
`check_other_call_arg` and simplifies the module as a whole.
Fix `mut_mutex_lock` when Mutex is behind immutable deref
I *think* the problem here is the `if let ty::Ref(_, _, Mutability::Mut) = cx.typeck_results().expr_ty(recv).kind()` line tries to check if the `Mutex` can be mutably borrowed (there already is a test for `Arc<Mutex<_>>`), but gets bamboozled by the `&mut Arc` indirection. And I *think* checking the deref-adjustment to filter immutable-adjust (the deref through the `Arc`, starting from `&mut Arc`) is the correct fix.
Fixes#9415
changelog: Fix `mut_mutex_lock` when Mutex is behind immutable deref
Don't use `hir_ty_to_ty` in `result_large_err`
fixes#9414
This occurs starting with 2022-09-01. I checked that this does fix the ICE on rust-lang/rust@9353538. Not sure which pr caused the late-bound region to leak through `hir_ty_to_ty`.
changelog: None
Fix `suboptimal_float` not linting on `{const}.powf({const})`
There used to be an early return if the receiver was an effective const but the method was not linted, not taking into account later cases where the receiver and the arguments are both effective consts for different methods. Removed the early return.
Fixes#9402Fixes#9201
changelog: Fix `suboptimal_flops`, `imprecise_flops` not linting on `{const}.powf({const})` et al
Fix the emission order of `trait_duplication_in_bounds`
Makes the lint emit in source order rather than whatever order the hash map happens to be in. This is currently blocking the sync into rustc.
changelog: None
Strengthen invalid_value lint to forbid uninit primitives, adjust docs to say that's UB
For context: https://github.com/rust-lang/rust/issues/66151#issuecomment-1174477404=
This does not make it a FCW, but it does explicitly state in the docs that uninit integers are UB.
This also doesn't affect any runtime behavior, uninit u32's will still successfully be created through mem::uninitialized.
Fix missing parens in `suboptimal_flops` suggestion
Fixes#9391. The problem is simple enough, I didn't check if the same problem occurs elsewhere, though.
changelog: fix missing parenthesis in `suboptimal_flops` suggestion
Ignore `match_like_matches_macro` when there is comment
Closes#9164
changelog: [`match_like_matches_macro`] is ignored when there is some comment inside the match block.
Also add `span_contains_comment` util to check if given span contains comments.
Implemented `suspicious_to_owned` lint to check if `to_owned` is called on a `Cow`
changelog: Add lint ``[`suspicious_to_owned`]``
-----------------
Hi,
posting this unsolicited PR as I've been burned by this issue :)
Being unsolicited, feel free to reject it or reassign a different lint level etc.
This lint checks whether `to_owned` is called on `Cow<'_, _>`. This is done because `to_owned` is very similarly named to `into_owned`, but the effect of calling those two methods is completely different (one makes the `Cow::Borrowed` into a `Cow::Owned`, the other just clones the `Cow`). If the cow is then passed to code for which the type is not checked (e.g. generics, closures, etc.) it might slip through and if the cow data is coming from an unsafe context there is the potential for accidentally cause undefined behavior.
Even if not falling into this painful case, there's really no reason to call `to_owned` on a `Cow` other than confusing people reading the code: either `into_owned` or `clone` should be called.
Note that this overlaps perfectly with `implicit_clone` as a warning, but `implicit_clone` is classified pedantic (while the consequences for `Cow` might be of a wider blast radius than just pedantry); given the overlap, I set-up the lint so that if `suspicious_to_owned` triggers `implicit_clone` will not trigger. I'm not 100% sure this is done in the correct way (I tried to copy what other lints were doing) so please provide feedback on it if it isn't.
### Checklist
- \[x] Followed [lint naming conventions][lint_naming]
- \[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`
This is done because `to_owned` is very similarly named to `into_owned`, but the
effect of calling those two methods is completely different. This creates
confusion (stemming from the ambiguity of the 'owned' term in the context of
`Cow`s) and might not be what the writer intended.
new lint
This fixes#6576
If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.
- \[x] Followed [lint naming conventions][lint_naming]
- \[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`
---
changelog: add [`multi_assignments`] lint
feat(fix): Do not lint if the target code is inside a loop
close#8753
we consider the following code.
```rust
fn main() {
let vec = vec![1];
let w: Vec<usize> = vec.iter().map(|i| i * i).collect(); // <- once.
for i in 0..2 {
let _ = w.contains(&i);
}
}
```
and the clippy will issue the following warning.
```rust
warning: avoid using `collect()` when not needed
--> src/main.rs:3:51
|
3 | let w: Vec<usize> = vec.iter().map(|i| i * i).collect();
| ^^^^^^^
...
6 | let _ = w.contains(&i);
| -------------- the iterator could be used here instead
|
= note: `#[warn(clippy::needless_collect)]` on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_collect
help: check if the original Iterator contains an element instead of collecting then checking
|
3 ~
4 |
5 | for i in 0..2 {
6 ~ let _ = vec.iter().map(|i| i * i).any(|x| x == i);
```
Rewrite the code as indicated.
```rust
fn main() {
let vec = vec![1];
for i in 0..2 {
let _ = vec.iter().map(|i| i * i).any(|x| x == i); // <- execute `map` every loop.
}
}
```
this code is valid in the compiler, but, it is different from the code before the rewrite.
So, we should not lint, If `collect` is outside of a loop.
Thank you in advance.
---
changelog: Do not lint if the target code is inside a loop in `needless_collect`
