Rename and allow `cast_ref_to_mut` lint
This PR is a small subset of https://github.com/rust-lang/rust/pull/112431, that is the renaming of the lint (`cast_ref_to_mut` -> `invalid_reference_casting`).
BUT also temporarily change the default level of the lint from deny-by-default to allow-by-default until https://github.com/rust-lang/rust/pull/112431 is merged.
r? `@Nilstrieb`
new lint: [`readonly_write_lock`]
Closes#8555
A new lint that catches `RwLock::write` calls to acquire a write lock only to read from it and not actually do any writes (mutations).
changelog: new lint: [`readonly_write_lock`]
Now `option_env_unwrap` warns even if a variable isn't set at compiletime
Fixes#10742
changelog: Fix false negative where `option_env_unwrap` wouldn't warn if the env variable isn't set at compile-time.
[`slow_vector_initialization`]: catch `Vec::new()` followed by `.resize(len, 0)`
Closes#10938
changelog: [`slow_vector_initialization`]: catch `Vec::new()` followed by `.resize(len, 0)`
New lint [`needless_return_with_try`]
Closes#10902
Rather than having a config option, this will just suggest removing the "return"; if `try_err` is used as well, then it'll be added again but without the `?`.
changelog: New lint [`needless_return_with_try`]
ptr_arg should ignore extern functions
Fixes: #11181
changelog: [`ptr_arg`]: ignore extern functions that are not
I am not sure whether we should ignore other Rust calling conventions like `rust-intrinsic`, `rust-call` or `rust-cold`.
`unwrap_or_else_default` -> `unwrap_or_default` and improve resulting lint
Resolves#10080 (though it doesn't implement exactly what's described there)
This PR does the following:
1. Merges `unwrap_or_else_default.rs`'s code into `or_fun_call.rs`
2. Extracts the code to handle `unwrap_or(/* default value */)` and similar, and moves it into `unwrap_or_else_default`
3. Implements the missing functionality from #9342, e.g.,, to handle `or_insert_with(Default::default)`
4. Renames `unwrap_or_else_default` to `unwrap_or_default` (since the "new" lint handles both `unwrap_or` and `unwrap_or_else`, it seemed sensible to use the shortened name)
This PR is currently two commits. The first implements 1-3, the second implements 4.
A word about 2: the `or_fun_call` lint currently produces warnings like the following:
```
error: use of `unwrap_or` followed by a call to `new`
--> $DIR/or_fun_call.rs:56:14
|
LL | with_new.unwrap_or(Vec::new());
| ^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
```
To me, such warnings look like they should come from `unwrap_or_else_default`, not `or_fun_call`, especially since `or_fun_call` is [in the nursery](https://github.com/rust-lang/rust-clippy/pull/9829).
---
changelog: Move: Renamed `unwrap_or_else_default` to [`unwrap_or_default`]
[#10120](https://github.com/rust-lang/rust-clippy/pull/10120)
changelog: Enhancement: [`unwrap_or_default`]: Now handles more functions, like `or_insert_with`
[#10120](https://github.com/rust-lang/rust-clippy/pull/10120)
<!-- changelog_checked-->
check that the types are equal in `SpanlessEq::eq_expr`
Fixes#11213
changelog: [`if_same_then_else`]: don't lint for integer literals of different types
Fix async functions handling for `needless_pass_by_ref_mut` lint
Fixes https://github.com/rust-lang/rust-clippy/issues/11179.
The problem with async is that "internals" are actually inside a closure from the `ExprUseVisitor` point of view, meaning we need to actually run the check on the closures' body as well.
changelog: none
r? `@llogiq`
Make `comparison_to_empty` work on `if let`/`let` chains
This adds `LetChain` to `clippy_utils::higher`, other lints may benefit from such a change as well :D
changelog: Enhancement: [`comparison_to_empty`]: Now lints on `if let`
[`unused_async`]: don't lint if paths reference async fn without immediate call
Fixes#9695Fixes#9359
Clippy shouldn't lint unused `async` if there are paths referencing them if that path isn't the receiver of a function call, because that means that the function might be passed to some other function:
```rs
async fn f() {} // No await statements, so unused at this point
fn requires_fn_future<F: Future<Output = ()>>(_: fn() -> F) {}
requires_fn_future(f); // `f`'s asyncness is actually not unused.
```
(This isn't limited to just passing the function as a parameter to another function, it could also first be stored in a variable and later passed to another function as an argument)
This requires delaying the linting until post-crate and collecting path references to local async functions along the way.
changelog: [`unused_async`]: don't lint if paths reference async fn that require asyncness
fix dogfood lints in `redundant_local`
keep `redundant_local` from running in proc macros
rewrite `redundant_local` as late pass
make redundant_local's `find_binding` more readable
pluralize `redundant_locals` name
add test for `redundant_locals` in macros
test `redundant_locals` in proc macros
use more destructuring in `redundant_locals`
fix: format redundant_locals.rs
ignore needless_pass_by_mut_ref in redundant_locals test
Allow `Self::cmp(self, other)` as a correct impl
Fixes#11178
Also no longer checks if the method name is *just* cmp, but the path. That was an oversight on my part ^^
r? `@xFrednet`
(and `@blyxyas` too!)
changelog: [`incorrect_partial_ord_impl_on_ord_type`]: Now allows non-method calls to `cmp` like `Self::cmp(self, other)`
fix: false positive for `option_env!` in `ifs_same_cond`
Clippy had a false positive for with `ifs_same_cond` when two if-let expressions have an `option_env!` macro. The fix is similar to the `env!` macro fix.
The following example had a clippy error:
```rust
if let Some(env1) = option_env!("ENV1") {
// ...
} else if let Some(env2) = option_env!("ENV2") {
// ...
}
```
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01b85c61b56ddd900117fb247af04824
changelog: [`ifs_same_cond`]: fix false positive when using `option_env!` in if-let expressions.
[`unnecessary_literal_unwrap`]: Fix ICE on None.unwrap_or_default()
Fixes#11099Fixes#11064
I'm running into #11099 (cc `@y21)` on my Rust codebase. Clippy ICEs on this code when evaluating the `unnecessary_literal_unwrap` lint:
```rust
fn main() {
let val1: u8 = None.unwrap_or_default();
}
```
This fixes that ICE and adds an message specifically for that case:
```
error: used `unwrap_or_default()` on `None` value
--> $DIR/unnecessary_literal_unwrap.rs:26:5
|
LL | None::<String>.unwrap_or_default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the `None` and `unwrap_or_default()`: `String::default()`
```
This PR also fixes the same ICE with `None.unwrap_or_else` (by giving the generic error message for the lint in that case).
changelog: Fix ICE in `unnecessary_literal_unwrap` on `None.unwrap_or_default()`
Clippy had a false positive for with `ifs_same_cond` when two
if-let expressions have an `option_env!` macro. The fix is similar to the
`env!` macro fix.
The following example had a clippy error:
```rust
if let Some(env1) = option_env!("ENV1") {
// ...
} else if let Some(env2) = option_env!("ENV2") {
// ...
}
```
See https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=01b85c61b56ddd900117fb247af04824
changelog: Fix [`ifs_same_cond`] false positive when using `option_env!` in if-let expressions.
[`manual_filter_map`]: lint on `matches` and pattern matching
Fixes#8010
Previously this lint only worked specifically for a very limited set of methods on the filter call (`.filter(|opt| opt.is_some())` and `.filter(|res| res.is_ok())`). This PR extends it to also recognize `matches!` in the `filter` and pattern matching with `if let` or `match` in the `map`.
Example:
```rs
enum Enum {
A(i32),
B,
}
let _ = [Enum::A(123), Enum::B].into_iter()
.filter(|x| matches!(x, Enum::A(_)))
.map(|x| if let Enum::A(s) = x { s } else { unreachable!() });
```
Now suggests:
```diff
- .filter(|x| matches!(x, Enum::A(_))).map(if let Enum::A(s) = x { s } else { unreachable!() })
+ .filter_map(|x| match x { Enum::A(s) => Some(s), _ => None })
```
Adding this required a somewhat large change in code because it originally seemed to be specifically written with only method calls in the filter in mind, and `matches!` has different behavior in the map, so this new setup should make it possible to support more "generic" cases that need different handling for the filter and map calls.
changelog: [`manual_filter_map`]: lint on `matches` and pattern matching (and some internal refactoring)
Fix `unwrap_or_else_default` false positive
This PR fixes a false positive in the handling of `unwrap_or_else` with a default value when the value is needed for type inference.
An easy example to exhibit the false positive is the following:
```rust
let option = None;
option.unwrap_or_else(Vec::new).push(1);
```
The following code would not compile, because the fact that the value is a `Vec` has been lost:
```rust
let option = None;
option.unwrap_or_default().push(1);
```
The fix is to:
- implement a heuristic to tell whether an expression's type can be determined purely from its subexpressions, and the arguments and locals they use;
- apply the heuristic to `unwrap_or_else`'s receiver.
The heuristic returns false when applied to `option` in the above example, but it returns true when applied to `option` in either of the following examples:
```rust
let option: Option<Vec<u64>> = None;
option.unwrap_or_else(Vec::new).push(1);
```
```rust
let option = None::<Vec<u64>>;
option.unwrap_or_else(Vec::new).push(1);
```
(Aside: https://github.com/rust-lang/rust-clippy/pull/10120 unfairly contained multiple changes in one PR. I am trying to break that PR up into smaller pieces.)
---
changelog: FP: [`unwrap_or_else_default`]: No longer lints if the default value is needed for type inference
Rewrite [`tuple_array_conversions`]
Fixes#11100Fixes#11144Fixes#11124#11082 still needs discussion and #11085 likely can't be fixed.
changelog: [`tuple_array_conversions`]: Move to `pedantic`
changelog: [`tuple_array_conversions`]: Don't lint if mutability of references changes
changelog: [`tuple_array_conversions`]: Don't lint if bindings don't come from the exact same pattern
changelog: [`tuple_array_conversions`]: Don't lint if bindings are used for more than just the conversion
Add `imports_granularity = "Module"` to rustfmt.toml
This lets rustfmt split/merge imports, `Module` seems to be the most common style in clippy
https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#imports_granularity
changelog: none
Almost all the updates other than the config file change are from `cargo dev fmt` or blessed tests, the exceptions being
- `tests/ui/single_component_path_imports.rs`
- `tests/ui/single_component_path_imports_nested_first.rs`
- `tests/ui/single_component_path_imports_self_after.rs`
- `tests/ui/single_component_path_imports_self_before.rs`
- `tests/ui/unsafe_removed_from_name.rs` (added a test with merged imports as a drive by)
- `tests/ui/wildcard_imports.rs`
- `tests/ui/wildcard_imports_2021.rs`
[`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>`
Fixes#11145
This lint has a list of allowed types, one of which is `Wrapping<T>`, but it was only actually allowed if the type on the right hand side was also `Wrapping<T>`, which meant that, for example, `Wrapping<u32> += u32` would still lint. It now allows binary ops involving `Wrapping<T>` regardless of the type on the rhs.
These impls have only existed since Rust 1.60.0, so that is probably why the lint was previously not handling this correctly
changelog: [`arithmetic_side_effect`]: allow different types on the right hand side for `Wrapping<T>` (e.g. `Wrapping<T> += T`)
This commit fixes#11025 by removing checks for `todo!`,
`unimplemented!` and `unreachable!`.
Signed-off-by: Panagiotis Foliadis <pfoliadis@hotmail.com>
Uplift `clippy::fn_null_check` lint
This PR aims at uplifting the `clippy::fn_null_check` lint into rustc.
## `incorrect_fn_null_checks`
(warn-by-default)
The `incorrect_fn_null_checks` lint checks for expression that checks if a function pointer is null.
### Example
```rust
let fn_ptr: fn() = /* somehow obtained nullable function pointer */
if (fn_ptr as *const ()).is_null() { /* ... */ }
```
### Explanation
Function pointers are assumed to be non-null, checking for their nullity is incorrect.
-----
Mostly followed the instructions for uplifting a clippy lint described here: https://github.com/rust-lang/rust/pull/99696#pullrequestreview-1134072751
`@rustbot` label: +I-lang-nominated
r? compiler
[`unnecessary_literal_unwrap`]: don't lint if binding initializer comes from expansion
Fixes https://github.com/rust-lang/rust-clippy/discussions/11109
changelog: [`unnecessary_literal_unwrap`]: don't lint if binding initializer comes from expansion
"try this" -> "try"
Current help messages contain a mix of "try", "try this", and one "try this instead". In the spirit of #10631, this PR adopts the first, as it is the most concise.
It also updates the `lint_message_conventions` test to catch cases of "try this".
(Aside: #10120 unfairly contained multiple changes in one PR. I am trying to break that PR up into smaller pieces.)
changelog: Make help messages more concise ("try this" -> "try").
Add `needless_pass_by_ref_mut` lint
changelog: [`needless_pass_by_ref_mut`]: This PR add a new lint `needless_pass_by_ref_mut` which emits a warning in case a `&mut` function argument isn't used mutably. It doesn't warn on trait and trait impls functions.
Fixes#8863.
cargo dev fmt
cargo test passes
cargo test passes
refactor a lil
Update bool_comparison.stderr
heavily refactor + bump `clippy::version`
refactor
refactor
check bounds to increase accuracy, and add todos
new lint: `read_line_without_trim`
This adds a new lint that checks for calls to `Stdin::read_line` with a reference to a string that is then attempted to parse into an integer type without first trimming it, which is always going to fail at runtime.
This is something that I've seen happen a lot to beginners, because it's easy to run into when following the example of chapter 2 in the book where it shows how to program a guessing game.
It would be nice if we could point beginners to clippy and tell them "let's see what clippy has to say" and have clippy explain to them why it fails 👀
I think this lint can later be "generalized" to work not just for `Stdin` but also any `BufRead` (which seems to be where the guarantee about the trailing newline comes from) and also, matching/comparing it to a string slice that doesn't end in a newline character (e.g. `input == "foo"` is always going to fail)
changelog: new lint: [`read_line_without_trim`]
[`useless_vec`]: add more tests and don't lint inside of macros
Closes#11084.
I realized that the fix I added in #11081 itself also causes an error in a suggestion when inside of a macro. Example:
```rs
macro_rules! x {
() => {
for _ in vec![1, 2] {}
}
}
x!();
```
Here it would suggest replacing `vec![1, 2]` with `[x!()]`, because that's what the source callsite is (reminder: it does this to get the correct span of `x!()` for code like `for _ in vec![x!()]`), but that's wrong when *inside* macros, so I decided to make it not lint if the whole loop construct is inside a macro to avoid this issue.
changelog: [`useless_vec`]: add more tests and don't lint inside of macros
r? `@Alexendoo` since these were your tests, I figured it makes most sense to assign you
Don't lint manual_let_else in cases where ? would work
Don't lint `manual_let_else` where the question mark operator `?` would be sufficient, that is, mostly in cases like:
```Rust
let v = if let Some(v) = ex { v } else { return None };
```
Also, this PR emits the `question_mark` lint for `let...else` patterns that could be written with `?` (also, only `return None` like cases).
```
changelog: [`manual_let_else`]: don't lint in cases where question_mark already lints
changelog: [`question_mark`]: lint for `let Some(...) = ex else { return None };`
```
Fixes #8755
[`useless_vec`]: use the source span for initializer
Fixes#11075.
changelog: [`useless_vec`]: use the source span for the initializer expression when inside of a macro
[`arc_with_non_send_sync`]: don't lint if type has nested type parameters
Fixes#11076
changelog: [`arc_with_non_send_sync`]: don't lint if type has nested type parameters
r? `@Manishearth`
new lint: `type_id_on_box`
Closes#7687.
A new lint that detects calling `.type_id()` on `Box<dyn Any>` (and not on the underlying `dyn Any`), which can make up for some pretty confusing bugs!
changelog: new lint: [`type_id_on_box`]
`let_and_return`: lint 'static lifetimes, don't lint borrows in closures
Fixes#11056
Now also ignores functions returning `'static` lifetimes, since I noticed the `stdin.lock()` example was still being linted but doesn't need to be since https://github.com/rust-lang/rust/pull/93965
changelog: none
New lint [`tuple_array_conversions`]
Closes#10748
PS, the implementation is a bit ugly 😅 ~~I will likely refactor soon enough :)~~ Done :D
changelog: New lint [`tuple_array_conversions`]
Also, lint question_mark for `let...else` clauses that can be simplified to use `?`.
This lint isn't perfect as it doesn't support the unstable try blocks.
[significant_drop_tightening] Fix#10413Fix#10413
This is quite a rewrite that unfortunately took a large amount of time. I tried my best to comment what is going on to easy review but feel free to ask any question.
The problem basically is that the current algorithm is only taking into consideration single blocks which means that things like the following don't work or show unpredictable results.
```rust
let mutex = Mutex::new(1);
{
let lock = mutex.lock().unwrap();
{
let _ = *lock;
}
}
```
The solve the issue, each path that refers a lock is now being tracked individually.
```
changelog: [`significant_drop_tightening`]: Lift the restriction of only considerate single blocks
```
New lint [`redundant_at_rest_pattern`]
Closes#11011
It's always a great feeling when a new lint triggers on clippy itself 😄
changelog: New lint [`redundant_at_rest_pattern`]
suggests `is_some_and` over `map().unwrap`
changelog: Enhancement: [`option_map_unwrap_or`] now considers the [`msrv`] config when creating the suggestion.
* modified option_map_unwrap_or lint to recognise when an `Option<T>` is mapped to an `Option<bool>` with false being used when `None` is detected; suggests the use of `is_some_and` instead
* msrv is set to 1.70.0 for this lint; when `is_some_and` was stabilised
fixes#9125
[`question_mark`]: don't lint inside of `try` block
Fixes#8628.
Diff looks a bit noisy because I had to move the two functions into an impl, because they now need to access the structs `try_block_depth` field to see if they're inside a try block.
changelog: [`question_mark`]: don't lint inside of `try` block
[`option_if_let_else`]: suggest `.as_ref()` if scrutinee is of type `&Option<_>`
Fixes#10729
`Option::map_or` takes ownership, so if matching on an `&Option<_>`, we need to suggest `.as_ref()` before calling `map_or` to get the same effect and to not cause a borrowck error.
changelog: [`option_if_let_else`]: suggest `.as_ref()`/`.as_mut()` if scrutinee is of type `&Option<_>`/`&mut Option<_>`
[`unused_async`]: don't lint if function is part of a trait
Fixes#10459.
We shouldn't lint if the function is part of a trait, because the user won't be able to easily remove the `async`, as this will then not match with the function signature in the trait definition
changelog: [`unused_async`]: don't lint if function is part of a trait
Port clippy away from compiletest to ui_test
Reasons to do this:
* runs completely on stable Rust
* is easier to extend with new features
* has its own dogfood test suite, so changes can be tested in [the `ui_test` repo](https://github.com/oli-obk/ui_test)
* supports dependencies from crates.io without having to manually fiddle with command line flags
* supports `ui-cargo`, `ui`, `ui-toml` out of the box, no need to find and run the tests ourselves
One thing that is a big difference to `compiletest` is that if a test emits *any* error, you need to mark all of them with `//~ ERROR:` annotations. Since many clippy tests did not have annotations, I changed many lints to be `warn` in their test so that only the `stderr` output is tested.
TODO:
* [ ] check that this still works as a subtree in the rustc repo
changelog: none
<!-- changelog_checked -->
Note: at present the latest changes needed for clippy are only available as a git dependency, but I expect to publish a new crates.io version soon
Check if `if` conditions always evaluate to true in `never_loop`
This fixes the example provided in #11004, but it shouldn't be closed as this is still an issue on like
```rust
let x = true;
if x { /* etc */ }`
```
This also makes `clippy_utils::consts::constant` handle `ConstBlock` and `DropTemps`.
changelog: [`never_loop`]: Check if `if` conditions always evaluate to true
Lint `mem_forget` if any fields are `Drop`
Closes#9298
I think this way of doing it (`needs_drop`) should be fine.
---
changelog: Enhancement: [`mem_forget`]: Now lints on types with fields that implement `Drop`
[#10996](https://github.com/rust-lang/rust-clippy/pull/10996)
[`format_push_string`]: look through `match` and `if` expressions
Closes#9493.
changelog: [`format_push_string`]: look through `match` and `if` expressions
[`get_unwrap`]: include a borrow in the suggestion if argument is not an integer literal
Fixes#9909
I have to say, I don't really understand what the previous logic was trying to do, but this fixes the linked bug.
It was checking if the argument passed to `.get()` can be parsed as a usize (i.e. if it's an integer literal, probably?), and if not, it wouldn't include a borrow? I don't know how we came to that conclusion, but that logic doesn't work:
```rs
let slice = &[1, 2];
let _r: &i32 = slice.get({ 1 }).unwrap();
// previous suggestion: slice[{ 1 }]
// the suggestion should be: &slice[{ 1 }]
```
Here the argument passed to it isn't an integer literal, but it should still include a borrow, because it would otherwise change the type from `&i32` to `i32`.
The exception is that if the parent of the `get().unwrap()` expr is a dereference or a method call or the like, we don't need an explicit borrow because it's automatically inserted by the compiler
changelog: [`get_unwrap`]: include a borrow in the suggestion if argument is not an integer literal
Don't lint [`iter_nth_zero`] in `next`
Closes#9820
This also *slightlyy* modifies the output of `iter_nth`, as I noticed the types' names weren't in backticks
changelog: [`iter_nth_zero`]: No longer lints in implementations of `Iterator::next`
[`single_match`]: don't lint if block contains comments
Fixes#8634
It now ignores matches with a comment in the "else" arm
changelog: [`single_match`]: don't lint if block contains comments
[`redundant_closure_call`]: handle nested closures
Fixes#9956.
This ended up being a much larger change than I'd thought, and I ended up having to pretty much rewrite it as a late lint pass, because it needs access to certain things that I don't think are available in early lint passes (e.g. getting the parent expr). I think this'll be required to fi-x #10922 anyway, so this is probably fine.
(edit: had to write "fi-x" because "fix" makes github think that this PR fixes it, which it doesn't 😅 )
Previously, it would suggest changing `(|| || 42)()()` to `|| 42()`, which is a type error (it needs parens: `(|| 42)()`). In my opinion, though, the suggested fix should have really been `42`, so that's what this PR changes.
changelog: [`redundant_closure_call`]: handle nested closures and rewrite as a late lint pass
[`arithmetic_side_effects`] Fix#10792Fix#10792
```
changelog: [`arithmetic_side_effects`]: Retrieve field values of structures that are in constant environments
```
Ignore more type aliases in `unnecessary_cast`
This is potentially the worst code I've ever written, and even if not, it's very close to being on par with starb. This will ignore `call() as i32` and `local_obtained_from_call as i32` now.
This should fix every reasonable way to reproduce #10555, but likely not entirely.
changelog: Ignore more type aliases in `unnecessary_cast`
[`missing_panics_doc`]: pickup expect method
close#10240
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`missing_panics_doc`]: pickup expect method
new lint: `drain_collect`
Closes#10818.
This adds a new lint that looks for `.drain(..).collect()` and suggests replacing it with `mem::take`.
changelog: [`drain_collect`]: new lint
[`match_same_arms`]: don't lint if `non_exhaustive_omitted_patterns`
Fixes#10327
changelog: [`match_same_arms`]: Don't lint if `non_exhaustive_omitted_patterns` is `warn` or `deny`
from_over_into: Show suggestions for non-Self expanded paths
changelog: [`from_over_into`]: Show suggestions when the body contains macros not expanding to `Self`
Currently any path in a macro expansion causes the suggestion to be hidden, meaning most macro calls cause it to be hidden
Now it's only hidden if the expansion contains `Self`
[`unnecessary_fold`]: suggest turbofish if necessary
Fixes#10000
This adds turbofish `::<T>` to the suggestion in `unnecessary_fold`. This is necessary because the `Sum` trait is generic, which breaks inference when changing `fold()` to `sum()`.
changelog: [`unnecessary_fold`]: suggest turbofish if necessary
new lint [`single_range_in_vec_init`]
Lints on `vec![0..200]` (or `[0..200]`), suggesting either `(0..200).collect::<Vec<i32>>()` or `[0; 200]`.
Haven't tested it with anything that isn't primitive. Probably should!
Closes#10932
changelog: new lint [`single_range_in_vec_init`]
[`derivable_impls`]: don't lint if `default()` call expr unsize-coerces to trait object
Fixes#10158.
This fixes a FP where the derive-generated Default impl would have different behavior because of unsize coercion from `Box<T>` to `Box<dyn Trait>`:
```rs
struct S {
x: Box<dyn std::fmt::Debug>
}
impl Default for S {
fn default() -> Self {
Self {
x: Box::<()>::default()
// ^~ Box<()> coerces to Box<dyn Debug>
// #[derive(Default)] would call Box::<dyn Debug>::default()
}
}
}
```
(this intentionally only looks for trait objects `dyn` specifically, and not any unsize coercion, e.g. `&[i32; 5]` to `&[i32]`, because that breaks existing tests and isn't actually problematic, as far as I can tell)
changelog: [`derivable_impls`]: don't lint if `default()` call expression unsize-coerces to trait object
[`needless_doctest_main`]: ignore `main()` in `no_test` code fences
close#10491
*Please write a short comment explaining your change (or "none" for internal only changes)*
changelog: [`needless_doctest_main`]: ignore `main()` in `no_test` code fence
[`map_unwrap_or`]: don't lint when referenced variable is moved
Fixes#10579.
The previous way of checking if changing `map(f).unwrap_or(a)` to `map_or(a, f)` is safe had a flaw when the argument to `unwrap_or` moves a binding and the `map` closure references that binding in some way.
It used to simply check if any of the identifiers in the `unwrap_or` argument are referenced in the `map` closure, but it didn't consider the case where the moved binding is referred to through references, for example:
```rs
let x = vec![1, 2];
let x_ref = &x;
Some(()).map(|_| x_ref.clone()).unwrap_or(x);
```
This compiles as is, but we cannot change it to `map_or`. This lint however did suggest changing it, because the simple way of checking if `x` is referenced anywhere in the `map` closure fails here. The safest thing to do here imo (what this PR does) is check if the moved value `x` is referenced *anywhere* in the body (before the `unwrap_or` call). One can always create a reference to the value and smuggle them into the closure, without actually referring to `x`. The original, linked issue shows another one such example:
```rs
let x = vec![1,2,3,0];
let y = x.strip_suffix(&[0]).map(|s| s.to_vec()).unwrap_or(x);
```
`x.strip_suffix(&[0])` creates a reference to `x` that is available through `s` inside of the `map` closure, so we can't change it to `map_or`.
changelog: [`map_unwrap_or`]: don't lint when referenced variable is moved
[`no_effect`]: Suggest adding `return` if applicable
Closes#10941
Unfortunately doesn't catch anything complex as `no_effect` already wouldn't, but I'm fine with that (it catches `ControlFlow` at least :D)
changelog: [`no_effect`]: Suggest adding `return` if statement has same type as function's return type and is the last statement in a block
Improve suggestion for [`needless_lifetimes`]
Fixes#10093
changelog: [`needless_lifetimes`]: Suggestion now points at the elidable lifetimes, rather than the entire function declaration
[`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`
this will check every local for `TerminatorKind::Drop` to ensure they can be evaluated at compile time, not sure if this is the best way to do this but MIR is confusing and it works so...
fixes#10617
changelog: [`missing_const_for_fn`]: Ensure dropped locals are `~const Destruct`
[`useless_vec`]: lint on `vec![_]` invocations that adjust to a slice
Fixes#2262 (well, actually my PR over at #10901 did do most of the stuff, but this PR implements the one last other case mentioned in the comments that my PR didn't fix)
Before this change, it would lint `(&vec![1]).iter().sum::<i32>()`, but not `vec![1].iter().sum::<i32>()`. This PR handles this case.
This also refactors a few things that I wanted to do in my other PR but forgot about.
changelog: [`useless_vec`]: lint on `vec![_]` invocations that adjust to a slice
Don't linting `as_conversions` in proc macros
Don't linting `as_conversions` if code was generated by procedural macro.
This PR fixes https://github.com/rust-lang/rust-clippy/issues/9657
I implemented the fix changing the lint code to be a `LateLintPass` in order to be able to use the `is_from_proc_macro` out of the box. If the reviwer thinks that it would be better to do the other way (implementing `WithSearchPat`) just let me know. I might need some help in implementing it for the `ustc_ast::ast::Expr`
changelog: [`as_conversions`] avoiding warnings in macro-generated code
Extend `explicit_iter_loop` and `explicit_into_iter_loop`
fixes#1518
Some included cleanups
* Split `for_loop` test into different files for each lint (partially).
* Move handling of some `into_iter` cases from `explicit_into_iter`.
---
changelog: Enhancement: [`explicit_iter_loop`]: Now also handles types that implement `IntoIterator`.
[#10416](https://github.com/rust-lang/rust-clippy/pull/10416)
changelog: Sugg: [`explicit_into_iter_loop`]: The suggestion now works on mutable references.
[#10416](https://github.com/rust-lang/rust-clippy/pull/10416)
<!-- changelog_checked -->
Add `needless_if` lint
first off: Sorry about the large diff. Seems a ton of tests do this (understandably so).
this is basically everything I wanted in #10868, while it doesn't lint *all* unnecessary empty blocks, it lints needless if statements; which are basically the crux of the issue (for me) anyway. I've committed code that includes this far too many times 😅 hopefully clippy can help me out soon
closes#10868
changelog: New lint [`needless_if`]
handle exponent without digits in `numeric_literal`
Fixes#10912
The numeric literal util module didn't check for exponents with no digits.
So:
384cf37612/clippy_utils/src/numeric_literal.rs (L163-L168)
`exponent` here would be the empty string, which passed the `!= "0"` check (when it shouldn't have, it should probably be treated as if the user wrote `E0`), then later fails when counting the digits and subtracting one (0 - 1 = overflow).
Also, interestingly I can't even write a test for this because exponents with no digits is some kind of error by itself and `cargo dev fmt` fails on it.
changelog: [`unreadable_literal`]: don't (debug) ICE on numeric literal with empty exponent
Fix `diverging_sub_expression` not checking body of block
Fixes#10776
This also adds a warning to the test `ui/never_loop.rs`, not sure if this is correct or not.
changelog: [`diverging_sub_expression`]: Fix false negatives with body of block
[`unnecessary_to_owned`]: check that the adjusted type matches target
Fixes#10033.
Before this change, the lint would assume that removing the `.to_string()` in `f(&x.to_string())` would be ok if x is of some type that implements `Deref<Target = str>` and `f` takes a `&str`.
This turns out to not actually be ok if the `to_string` call is some method that exists on `x` directly, which happens if it implements `Display`/`ToString` itself.
changelog: [`unnecessary_to_owned`]: only lint if the adjusted receiver type actually matches
Ignore more pointer types in `unnecessary_cast`
Spotted this because
e2c655b4c0/tests/ui/suspicious_to_owned.rs (L9-L10)
currently fails on `aarch64-unknown-linux-gnu` as `c_char` is `u8` there
The current implementation checks for `as alias`, `as _`. This adds things like
- `as *const alias`
- `as *const cfg_dependant`
- `as *const _`
changelog: none
[`redundant_closure`]: special case inclusive ranges
Fixes#10684.
`x..=y` ranges need a bit of special handling in this lint because it desugars to a call to the lang item `RangeInclusiveNew`, where the callee span would be the same as the range expression itself, so the suggestion looked a bit weird. It now correctly suggests `RangeInclusive::new`.
changelog: [`redundant_closure`]: special case `RangeInclusive`
Adds new lint `arc_with_non_send_or_sync`
Fixes#653
Adds a new lint to check for uses of non-Send/Sync types within Arc.
```
changelog: [`arc_with_non_send_sync`]: Added a lint to detect uses of non-Send/Sync types within Arc.
```
[`useless_vec`]: lint `vec!` invocations when a slice or an array would do
First off, sorry for that large diff in tests. *A lot* of tests seem to trigger the lint with this new change, so I decided to `#![allow()]` the lint in the affected tests to make reviewing this easier, and also split the commits up so that the first commit is the actual logic of the lint and the second commit contains all the test changes. The stuff that changed in the tests is mostly just line numbers now. So, as large as the diff looks, it's not actually that bad. 😅
I manually went through all of these to find out about edge cases and decided to put them in `tests/ui/vec.rs`.
For more context, I wrote about the idea of this PR here: https://github.com/rust-lang/rust-clippy/issues/2262#issuecomment-1579155257 (that explains the logic)
Basically, it now also considers the case where a `Vec` is put in a local variable and the user only ever does things with it that one could also do with a slice or an array. This should catch a lot more cases, and (at least from looking at the tests) it does.
changelog: [`useless_vec`]: lint `vec!` invocations when a slice or an array would do (also considering local variables now)
`suspicious_else_formatting`: Don't warn if there is a comment between else and curly bracket
This PR fixes https://github.com/rust-lang/rust-clippy/issues/10273
The idea is that if the only thing after `else` and before `{` is a comment, we will not warn because, probably, the line break was "made" by rustfmt.
changelog: [`suspicious_else_formatting`]: Don't warn if the only thing between `else` and curly bracket is a comment
consider autoderef through user-defined `Deref` in `eager_or_lazy`
Fixes#10462
This PR handles autoderef in the `eager_or_lazy` util module and stops suggesting to change lazy to eager if autoderef in an expression goes through user defined `Deref` impls, e.g.
```rs
struct S;
impl Deref for S {
type Target = ();
fn deref(&self) -> &Self::Target { &() }
}
let _ = Some(()).as_ref().unwrap_or_else(|| &S); // autoderef `&S` -> `&()`
```
changelog: [`unnecessary_lazy_evaluations`]: don't suggest changing lazy evaluation to eager if autoderef goes through user-defined `Deref`
r? `@xFrednet` (because of the earlier review in #10864, might help for context here)
[`let_with_type_underscore`]: Don't emit on locals from procedural macros
closes#10498
changelog: [`let_with_type_underscore`]: Don't emit on locals from procedural macros
make cast_possible_wrap work correctly for 16 bit {u,i}size
These changes make `cast_possible_wrap` aware of the different pointer widths and fixes the implementation to print the correct pointer widths.
Fixes#9337
changelog: `cast_possible_wrap` does not lint on `u8 as isize` or `usize as i8`, since these can never wrap.
`cast_possible_wrap` now properly considers 16 bit pointer size and prints the correct bit widths.
Add redundant type annotations lint
Hello, I'm trying to add the `redundat_type_annotations` lint.
It's still WIP but I'd like to start gathering some feedbacks to be sure that I'm not doing things 100% wrong :)
Right now it still misses lints like:
- [x] `let foo: u32 = 5_u32`,
- [x] `let foo: String = STest2::func()`
- [x] `let foo: String = self.func()` (`MethodCall`)
- [x] refs
- [ ] Generics
I've some problems regarding the second example above, in the `init` part of the `Local` I have:
```rust
init: Some(
Expr {
hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).58),
kind: Call(
Expr {
hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).59),
kind: Path(
TypeRelative(
Ty {
hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).61),
kind: Path(
Resolved(
None,
Path {
span: src/main.rs:77:21: 77:27 (#0),
res: Def(
Struct,
DefId(0:17 ~ playground[e1bd]::STest2),
),
segments: [
PathSegment {
ident: STest2#0,
hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).60),
res: Def(
Struct,
DefId(0:17 ~ playground[e1bd]::STest2),
),
args: None,
infer_args: true,
},
],
},
),
),
span: src/main.rs:77:21: 77:27 (#0),
},
PathSegment {
ident: get_numb#0,
hir_id: HirId(DefId(0:24 ~ playground[e1bd]::main).62),
res: Err,
args: None,
infer_args: true,
},
),
),
span: src/main.rs:77:21: 77:37 (#0),
},
[],
),
span: src/main.rs:77:21: 77:39 (#0),
},
),
```
And I'm not sure how to get the return type of the function `STest2::func()` since the resolved path `DefId` points to the struct itself and not the function. Do you have any idea on how I could get this information in this case?
Thanks!
changelog: changelog: [`redundant_type_annotations`]: New lint to warn on redundant type annotations
fixes#9155
[`unnecessary_lazy_eval`]: don't lint on types with deref impl
Fixes#10437.
This PR changes clippy's util module `eager_or_lazy` to also consider deref expressions whose type has a non-builtin deref impl and not suggest replacing it as that might have observable side effects.
A prominent example might be the `lazy_static` macro, which creates a newtype with a `Deref` impl that you need to go through to get access to the inner value. Going from lazy to eager can make a difference there.
changelog: [`unnecessary_lazy_eval`]: don't lint on types with non-builtin deref impl
