Commit graph

6822 commits

Author SHA1 Message Date
bors
03113a9f05 Auto merge of #12275 - y21:incompatible_msrv_desugaring, r=Manishearth
[`incompatible_msrv`]: allow expressions that come from desugaring

Fixes #12273

changelog: [`incompatible_msrv`]: don't lint on the `IntoFuture::into_future` call desugared by `.await`
2024-02-13 22:03:15 +00:00
bors
3e264ba13a Auto merge of #11881 - y21:issue11880, r=Alexendoo
[`implied_bounds_in_impls`]: avoid linting on overlapping associated tys

Fixes #11880

Before this change, we were simply ignoring associated types (except for suggestion purposes), because of an incorrect assumption (see the comment that I also removed).

For something like
```rs
trait X { type T; }
trait Y: X { type T; }

// Can't constrain `X::T` through `Y`
fn f() -> impl X<T = i32> + Y<T = u32> { ... }
```
We now avoid linting if the implied bound (`X<T = i32>`) "names" associated types that also exists in the implying trait (`trait Y`). Here that would be the case.
But if we only wrote `impl X + Y<T = u32>` then that's ok because `X::T` was never constrained in the first place.

I haven't really thought about how this interacts with GATs, but I think it's fine. Fine as in, it might create false negatives, but hopefully no false positives.

(The diff is slightly annoying because of formatting things. Really the only thing that changed in the if chain is extracting the `implied_by_def_id` which is needed for getting associated types from the trait, and of course actually checking for overlap)

cc `@Jarcho` ? idk if you want to review this or not. I assume you looked into this code a bit to find this bug.

changelog: [`implied_bounds_in_impls`]: avoid linting when associated type from supertrait can't be constrained through the implying trait bound
2024-02-13 17:05:03 +00:00
bors
3e3a09e2bb Auto merge of #12278 - GabrielBFern:master, r=Jarcho
[`mem_replace_with_default`] No longer triggers on unused expression

changelog:[`mem_replace_with_default`]: No longer triggers on unused expression

Change [`mem_replace_with_default`] to not trigger on unused expression because the lint from `#[must_use]` handle this case better.

fixes: #5586
2024-02-13 04:03:43 +00:00
bors
7dfa6ced9b Auto merge of #12266 - granddaifuku:fix/ice-12253-index-exceeds-usize, r=Manishearth
fix: ICE when array index exceeds usize

fixes #12253

This PR fixes ICE in `indexing_slicing` as it panics when the index of the array exceeds `usize`.

changelog: none
2024-02-12 19:08:14 +00:00
Gabriel Fernandes
83555914ed [mem_replace_with_default] No longer triggers on unused expression 2024-02-11 21:11:13 -03:00
bors
75f57cf4c2 Auto merge of #12248 - GuillaumeGomez:extend-NONMINIMAL_BOOL, r=blyxyas
Extend `NONMINIMAL_BOOL` lint

Fixes #5794.

r? `@blyxyas`

changelog: Extend `NONMINIMAL_BOOL` lint
2024-02-11 23:25:40 +00:00
Guillaume Gomez
5e7c437d41 Extend NONMINIMAL_BOOL to check inverted boolean values 2024-02-11 21:22:33 +01:00
Guillaume Gomez
a18e0a11f7 Extend NONMINIMAL_BOOL lint 2024-02-11 21:22:33 +01:00
y21
92616c0aaa [implied_bounds_in_impls]: avoid linting on overlapping associated types 2024-02-11 20:22:45 +01:00
bors
9b2021235a Auto merge of #12040 - J-ZhengLi:issue12016, r=y21
stop linting [`blocks_in_conditions`] on `match` with weird attr macro case

should fixes: #12016

---

changelog: [`blocks_in_conditions`] - fix FP on `match` with weird attr macro

This might not be the best solution, as the root cause (i think?) is the `span` of block was incorrectly given by the compiler?

I'm open to better solutions
2024-02-11 13:26:18 +00:00
J-ZhengLi
4cc7b7e092 stop linting [blocks_in_conditions] on match on proc macros 2024-02-12 04:16:11 +08:00
y21
67bdb0e11c [incompatible_msrv]: allow expressions that come from desugaring 2024-02-11 13:10:57 +01:00
bors
d29f2eebdd Auto merge of #12261 - Jarcho:issue_12257, r=dswij
Don't lint `incompatible_msrv` in test code

fixes #12257

changelog: `incompatible_msrv`: Don't lint in test code
2024-02-11 11:55:28 +00:00
granddaifuku
c4d11083d9 fix: ICE when array index exceeds usize 2024-02-11 03:51:26 +09:00
bors
51c89a45d9 Auto merge of #12264 - y21:issue12263, r=Jarcho
[`to_string_trait_impl`]: avoid linting if the impl is a specialization

Fixes #12263

Oh well... https://github.com/rust-lang/rust-clippy/pull/12122#issuecomment-1887765238 🙃

changelog: [`to_string_trait_impl`]: avoid linting if the impl is a specialization
2024-02-10 18:48:02 +00:00
y21
b3b9919d1b [to_string_trait_impl]: take specialization into account 2024-02-10 17:37:51 +01:00
Jason Newcomb
9012d55c02 Don't lint incompatible_msrv in test code 2024-02-10 09:35:29 -05:00
J-ZhengLi
92537a0e5b add test case with a proc macro fake_desugar_await 2024-02-10 22:26:50 +08:00
Trevor Gross
5250afb77d Remove '#[expect(clippy::similar_names)]' where needed to pass dogfood tests 2024-02-09 23:39:36 -06:00
Trevor Gross
09f18f61c6 [similar_names] don't raise if the first character is different
A lot of cases of the "noise" cases of `similar_names` come from two
idents with a different first letter, which is easy enough to
differentiate visually but causes this lint to be raised.

Do not raise the lint in these cases, as long as the first character
does not have a lookalike.

Link: https://github.com/rust-lang/rust-clippy/issues/10926
2024-02-09 23:19:27 -06:00
bors
28443e63fb Auto merge of #12070 - roife:fix/issue-12034, r=Centri3
Fix issue #12034: add autofixes for unnecessary_fallible_conversions

fixes #12034

Currently, the `unnecessary_fallible_conversions` lint was capable of autofixing expressions like `0i32.try_into().unwrap()`. However, it couldn't autofix expressions in the form of `i64::try_from(0i32).unwrap()` or `<i64 as TryFrom<i32>>::try_from(0).unwrap()`.

This pull request extends the functionality to correctly autofix these latter forms as well.

changelog: [`unnecessary_fallible_conversions`]: Add autofixes for more forms
2024-02-09 17:37:26 +00:00
Philipp Krones
d2f76f7e6e
Merge remote-tracking branch 'upstream/master' into rustup 2024-02-08 19:13:13 +01:00
bors
62dcbd672b Auto merge of #12177 - y21:issue12154, r=Jarcho
[`unconditional_recursion`]: compare by `Ty`s instead of `DefId`s

Fixes #12154
Fixes #12181 (this was later edited in, so the rest of the description refers to the first linked issue)

Before this change, the lint would work with `DefId`s and use those to compare types. This PR changes it to compare types directly. It fixes the linked issue, but also other false positives I found in a lintcheck run. For example, one of the issues is that some types don't have `DefId`s (primitives, references, etc., leading to possible FNs), and the helper function used to extract a `DefId` didn't handle type parameters.

Another issue was that the lint would use `.peel_refs()` in a few places where that could lead to false positives (one such FP was in the `http` crate). See the doc comment on one of the added functions and also the test case for what I mean.

The code in the linked issue was linted because the receiver type is `T` (a `ty::Param`), which was not handled in `get_ty_def_id` and returned `None`, so this wouldn't actually *get* to comparing `self_arg != ty_id` here, and skip the early-return:
70573af31e/clippy_lints/src/unconditional_recursion.rs (L171-L178)

This alone could be fixed by doing something like `&& get_ty_def_id(ty).map_or(true, |ty_id)| self_arg != ty_id)`, but we don't really need to work with `DefId`s in the first place, I don't think.

changelog: [`unconditional_recursion`]: avoid linting when the other comparison type is a type parameter
2024-02-07 16:18:33 +00:00
bors
08c8cd5014 Auto merge of #12216 - bpandreotti:redundant-type-annotations-fix, r=Jarcho
Fix false positive in `redundant_type_annotations` lint

This PR changes the `redundant_type_annotations` lint to allow slice type annotations (i.e., `&[u8]`) for byte string literals. It will still consider _array_ type annotations (i.e., `&[u8; 4]`) as redundant. The reasoning behind this is that the type of byte string literals is by default a reference to an array, but, by using a type annotation, you can force it to be a slice. For example:
```rust
let a: &[u8; 4] = b"test";
let b: &[u8] = b"test";
```

Now, the type annotation for `a` will still be linted (as it is still redundant), but the type annotation for `b` will not.

Fixes #12212.

changelog: [`redundant_type_annotations`]: Fix false positive with byte string literals
2024-02-07 16:02:28 +00:00
bors
b1e5a58427 Auto merge of #11812 - Jarcho:issue_11786, r=Alexendoo
Return `Some` from `walk_to_expr_usage` more

fixes #11786
supersedes #11097

The code removed in the first commit would have needed changes due to the second commit. Since it's useless it just gets removed instead.

changelog: `needless_borrow`: Fix linting in tuple and array expressions.
2024-02-06 15:20:07 +00:00
Michael Goulet
7895b98712 Add CoroutineClosure to TyKind, AggregateKind, UpvarArgs 2024-02-06 02:22:58 +00:00
bors
fdf819df9a Auto merge of #12227 - y21:issue12225, r=Manishearth
[`redundant_locals`]: take by-value closure captures into account

Fixes #12225

The same problem in the linked issue can happen to regular closures too, and conveniently async blocks are closures in the HIR so fixing closures will fix async blocks as well.

changelog: [`redundant_locals`]: avoid linting when redefined variable is captured by-value
2024-02-05 18:37:05 +00:00
y21
7f80b449f5 new lint: manual_c_str_literals 2024-02-05 18:51:49 +01:00
y21
6807977153 also check for coroutines 2024-02-05 15:41:59 +01:00
Matthias Krüger
d13ce192c9 Rollup merge of #116284 - RalfJung:no-nan-match, r=cjgillot
make matching on NaN a hard error, and remove the rest of illegal_floating_point_literal_pattern

These arms would never be hit anyway, so the pattern makes little sense. We have had a future-compat lint against float matches in general for a *long* time, so I hope we can get away with immediately making this a hard error.

This is part of implementing https://github.com/rust-lang/rfcs/pull/3535.

Closes https://github.com/rust-lang/rust/issues/41620 by removing the lint.

https://github.com/rust-lang/reference/pull/1456 updates the reference to match.
2024-02-05 11:07:26 +01:00
y21
7c3908f86c [redundant_locals]: take by-value closure captures into account 2024-02-04 20:19:27 +01:00
bors
34e4c9fa4a Auto merge of #12087 - marcin-serwin:ref_as_ptr_cast, r=blyxyas
Add new lint: `ref_as_ptr`

Fixes #10130

Added new lint `ref_as_ptr` that checks for conversions from references to pointers and suggests using `std::ptr::from_{ref, mut}` instead.

The name is different than suggested in the issue (`as_ptr_cast`) since there were some other lints with similar names (`ptr_as_ptr`, `borrow_as_ptr`) and I wanted to follow the convention.

Note that this lint conflicts with the `borrow_as_ptr` lint in the sense that it recommends changing `&foo as *const _` to `std::ptr::from_ref(&foo)` instead of `std::ptr::addr_of!(foo)`. Personally, I think the former is more readable and, in contrast to `addr_of` macro, can be also applied to temporaries (cf. #9884).

---

changelog: New lint: [`ref_as_ptr`]
[#12087](https://github.com/rust-lang/rust-clippy/pull/12087)
2024-02-04 17:07:18 +00:00
Marcin Serwin
a3baebcb31
Add ref_as_ptr lint
Author:    Marcin Serwin <marcin.serwin0@protonmail.com>
2024-02-04 17:38:09 +01:00
bors
9fb41079ca Auto merge of #12219 - sanxiyn:labeled-block, r=blyxyas
Avoid deleting labeled blocks

Fix #11575.

changelog: [`unnecessary_operation`]: skip labeled blocks
2024-02-03 22:48:46 +00:00
Seo Sanghyeon
abced206d7
Avoid deleting labeled blocks 2024-02-03 22:35:14 +01:00
bors
9b6f86643f Auto merge of #12217 - PartiallyTyped:12208, r=blyxyas
Fixed FP in `unused_io_amount` for Ok(lit), unrachable! and unwrap de…

…sugar

Fixes fp caused by linting on Ok(_) for all cases outside binding.

We introduce the following rules for match exprs.
- `panic!` and `unreachable!` are treated as consumed.
- `Ok( )` patterns outside `DotDot` and `Wild` are treated as consuming.

changelog: FP [`unused_io_amount`] when matching Ok(literal) or unreachable

fixes #12208

r? `@blyxyas`
2024-02-02 19:43:01 +00:00
Quinn Sinclair
fe8c2e24bd Fixed FP in unused_io_amount for Ok(lit), unrachable!
We introduce the following rules for match exprs.
- `panic!` and `unreachable!` are treated as consumption.
- guard expressions in any arm imply consumption.

For match exprs:
- Lint only if exacrtly 2 non-consuming arms exist
- Lint only if one arm is an `Ok(_)` and the other is `Err(_)`

Added additional requirement that for a block return expression
that is a match, the source must be `Normal`.

changelog: FP [`unused_io_amount`] when matching Ok(literal)
2024-02-01 17:05:12 +01:00
Nicholas Nethercote
ae0f0fd655 Don't hash lints differently to non-lints.
`Diagnostic::keys`, which is used for hashing and equating diagnostics,
has a surprising behaviour: it ignores children, but only for lints.
This was added in #88493 to fix some duplicated diagnostics, but it
doesn't seem necessary any more.

This commit removes the special case and only four tests have changed
output, with additional errors. And those additional errors aren't
exact duplicates, they're just similar. For example, in
src/tools/clippy/tests/ui/same_name_method.rs we currently have this
error:
```
error: method's name is the same as an existing method in a trait
  --> $DIR/same_name_method.rs:75:13
   |
LL |             fn foo() {}
   |             ^^^^^^^^^^^
   |
note: existing `foo` defined here
  --> $DIR/same_name_method.rs:79:9
   |
LL |         impl T1 for S {}
   |         ^^^^^^^^^^^^^^^^
```
and with this change we also get this error:
```
error: method's name is the same as an existing method in a trait
  --> $DIR/same_name_method.rs:75:13
   |
LL |             fn foo() {}
   |             ^^^^^^^^^^^
   |
note: existing `foo` defined here
  --> $DIR/same_name_method.rs:81:9
   |
LL |         impl T2 for S {}
   |         ^^^^^^^^^^^^^^^^
```
I think printing this second argument is reasonable, possibly even
preferable to hiding it. And the other cases are similar.
2024-01-31 08:25:29 +11:00
Bruno Andreotti
3106219e24
Don't lint slice type annotations for byte strings 2024-01-30 16:17:02 -03:00
bors
455c07b7cc Auto merge of #12210 - GuillaumeGomez:add-regression-test-2371, r=blyxyas
Add regression ui test for #2371

Fixes #2371.

#2371 seems to already be handled correctly in the lint. This PR adds a ui regression test so we can close it.

r? `@blyxyas`

changelog: Add regression ui test for #2371
2024-01-29 22:06:58 +00:00
bors
3cd713a9f8 Auto merge of #11370 - modelflat:suggest-relpath-in-redundant-closure-for-method-calls, r=blyxyas
[fix] [`redundant_closure_for_method_calls`] Suggest relative paths for local modules

Fixes #10854.

Currently, `redundant_closure_for_method_calls` suggest incorrect paths when a method defined on a struct within inline mod is referenced (see the description in the aforementioned issue for an example; also see [this playground link](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=f7d3c5b2663c9bd3ab7abdb0bd38ee43) for the current-version output for the test cases added in this PR). It will now try to construct a relative path path to the module and suggest it instead.

changelog: [`redundant_closure_for_method_calls`] Fix incorrect path suggestions for types within local modules
2024-01-29 14:53:54 +00:00
Guillaume Gomez
b2f2080942 Add regression test for #2371 2024-01-29 14:53:29 +01:00
modelflat
b3d53774e7 Make redundant_closure_for_method_calls suggest relative paths
Fixes #10854.

Co-authored-by: Alejandra González <blyxyas@gmail.com>
2024-01-29 12:34:59 +01:00
bors
e7a3cb7ab0 Auto merge of #12021 - PartiallyTyped:11982, r=flip1995
FP: `needless_return_with_question_mark` with implicit Error Conversion

Return with a question mark was triggered in situations where the `?` desuraging was performing error conversion via `Into`/`From`.

The desugared `?` produces a match over an expression with type `std::ops::ControlFlow<B,C>` with `B:Result<Infallible, E:Error>` and `C:Result<_, E':Error>`, and the arms perform the conversion. The patch adds another check in the lint that checks that `E == E'`. If `E == E'`, then the `?` is indeed unnecessary.

changelog: False Positive: [`needless_return_with_question_mark`] when implicit Error Conversion occurs.

fixes: #11982
2024-01-29 09:10:02 +00:00
Quinn Sinclair
3aa2c279c8 rewrote to match only Result::err cons 2024-01-28 22:43:40 +01:00
bors
8ccf6a61ee Auto merge of #12084 - yuxqiu:manual_retain, r=Alexendoo
fix: incorrect suggestions generated by `manual_retain` lint

fixes #10393, fixes #11457, fixes #12081

#10393: In the current implementation of `manual_retain`, if the argument to the closure is matched using tuple, they are all treated as the result of a call to `map.into_iter().filter(<f>)`. However, such tuple pattern matching can also occur in many different containers that stores tuples internally. The correct approach is to apply different lint policies depending on whether the receiver of `into_iter` is a map or not.

#11457 and #12081: In the current implementation of `manual_retain`, if the argument to the closure is `Binding`, the closure will be used directly in the `retain` method, which will result in incorrect suggestion because the first argument to the `retain` closure may be of a different type. In addition, if the argument to the closure is `Ref + Binding`, the lint will simply remove the `Ref` part and use the `Binding` part as the argument to the new closure, which will lead to bad suggestion for the same reason. The correct approach is to detect each of these cases and apply lint suggestions conservatively.

changelog: [`manual_retain`] refactor and add check for various patterns
2024-01-27 18:20:06 +00:00
bors
276ce3936b Auto merge of #12083 - cocodery:fix/issue11932, r=Alexendoo
Fix/Issue11932: assert* in multi-condition after unrolling will cause lint `nonminimal_bool` emit warning

fixes [Issue#11932](https://github.com/rust-lang/rust-clippy/issues/11932)

After `assert`, `assert_eq`, `assert_ne`, etc, assert family marcos unrolling in multi-condition expressions, lint `nonminimal_bool` will recognize whole expression as a entirety, analyze each simple condition expr of them, and check whether can simplify them.

But `assert` itself is a entirety to programmers, we don't need to lint on `assert`. This commit add check whether lint snippet contains `assert` when try to warning to an expression.

changelog: [`nonminimal_bool`] add check for condition expression
2024-01-27 17:44:24 +00:00
bors
18e1f25a9f Auto merge of #12206 - y21:issue12205, r=Alexendoo
[`never_loop`]: recognize desugared `try` blocks

Fixes #12205

The old code assumed that only blocks with an explicit label can be jumped to (using `break`). This is mostly correct except for `try` desugaring, where the `?` operator is rewritten to a `break` to that block, even without a label on the block. `Block::targeted_by_break` is a little more accurate than just checking if a block has a label in that regard, so we should just use that instead

changelog: [`never_loop`]: avoid linting when `?` is used inside of a try block
2024-01-27 17:27:38 +00:00
y21
ff5afac616 [never_loop]: recognize ? desugaring in try blocks 2024-01-27 17:07:10 +01:00
bors
85e08cd3b9 Auto merge of #12169 - GuillaumeGomez:unnecessary_result_map_or_else, r=llogiq
Add new `unnecessary_result_map_or_else` lint

Fixes https://github.com/rust-lang/rust-clippy/issues/7328.

r? `@llogiq`

changelog: Add new `unnecessary_result_map_or_else` lint
2024-01-27 12:42:18 +00:00