Commit graph

8507 commits

Author SHA1 Message Date
bors
67fa36a9f2 Auto merge of #12479 - y21:readonly_write_lock_perf, r=Jarcho
move `readonly_write_lock` to perf

[There haven't been any issues](https://github.com/rust-lang/rust-clippy/issues?q=is%3Aissue+readonly_write_lock) since its creation and it's a pretty useful lint I think, so I'd say it's worth giving it a try?

Did a lintcheck run on 300 crates with no results, but I guess `RwLock` is usually not something that's used much in libraries.

changelog: move [`readonly_write_lock`] to perf (now warn-by-default)
2024-03-17 03:35:17 +00:00
bors
12ecaa8367 Auto merge of #12482 - J-ZhengLi:issue12131, r=Jarcho
fix [`dbg_macro`] FN when dbg is inside some complex macros

fixes: #12131

It appears that [`root_macro_call_first_node`] only detects `println!` in the following example:
```rust
println!("{:?}", dbg!(s));
```
---

changelog: fix [`dbg_macro`] FN when `dbg` is inside some complex macros

(re-opening b'cuz bors doesn't like my previous one)
2024-03-17 03:22:20 +00:00
许杰友 Jieyou Xu (Joe)
f9a9c4bf3b Note that type param is chosen by caller when suggesting return impl Trait 2024-03-16 23:20:42 +00:00
bors
59a5ad4840 Auto merge of #12441 - CBSpeir:dedup-else-if-without-else, r=dswij
[`else_if_without_else`]: Fix duplicate diagnostics

Relates to: #12379

changelog:  Fix duplicate lint diagnostic emission from [`else_if_without_else`]
2024-03-16 15:55:11 +00:00
y21
4e72ca31b5 [map_entry]: call the visitor on the local's else block 2024-03-16 14:15:45 +01:00
bors
c9f24827b3 Auto merge of #12493 - y21:issue12491, r=Alexendoo
fix span calculation for non-ascii in `needless_return`

Fixes #12491

Probably fixes #12328 as well, but that one has no reproducer, so 🤷

The bug was that the lint used `rfind()` for finding the byte index of the start of the previous non-whitespace character:
```
// abc\n     return;
     ^
```
... then subtracting one to get the byte index of the actual whitespace (the `\n` here).
(Subtracting instead of adding because it treats this as the length from the `return` token to the `\n`)

That's correct for ascii, like here, and will get us to the `\n`, however for non ascii, the `c` could be multiple bytes wide, which would put us in the middle of a codepoint if we simply subtract 1 and is what caused the ICE.

There's probably a lot of ways we could fix this.
This PR changes it to iterate backwards using bytes instead of characters, so that when `rposition()` finally finds a non-whitespace byte, we *know* that we've skipped exactly 1 byte. This was *probably*(?) what the code was intending to do

changelog: Fix ICE in [`needless_return`] when previous line end in a non-ascii character
2024-03-15 17:30:23 +00:00
y21
d3f8f3e9d7 fix span calculation for non-ascii in needless_return 2024-03-15 16:59:23 +01:00
bors
5a11fefc25 Auto merge of #12432 - Ethiraric:fix-12411, r=y21
[`unused_enumerate_index`]: trigger on method calls

The lint used to check for patterns looking like:
```rs
for (_, x) in some_iter.enumerate() {
    // Index is ignored
}
```

This commit further checks for chained method calls constructs where we
can detect that the index is unused. Currently, this checks only for the
following patterns:
```rs
some_iter.enumerate().map_function(|(_, x)| ..)
let x = some_iter.enumerate();
x.map_function(|(_, x)| ..)
```
where `map_function` is one of `all`, `any`, `filter_map`, `find_map`,
`flat_map`, `for_each` or `map`.

Fixes #12411.

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: [`unused_enumerate_index`]: add detection for method chains such as `iter.enumerate().map(|(_, x)| x)`
2024-03-14 22:26:06 +00:00
Ethiraric
dadcd94b2a [unused_enumerate_index]: Keep explicit element type
Prior to this change, it might be that the lint would remove an explicit
type that was necessary for the type system to keep track of types.

This should be the last change that prevented this lint to be machine
applicable.
2024-03-14 23:12:47 +01:00
bors
b667d02340 Auto merge of #12472 - GuillaumeGomez:fix-10262, r=blyxyas
Don't emit `doc_markdown` lint for missing backticks if it's inside a quote

Fixes #10262.

changelog: Don't emit `doc_markdown` lint for missing backticks if it's inside a quote
2024-03-14 21:39:25 +00:00
bors
8a78128e9d Auto merge of #12386 - Ethiraric:fix-12381, r=blyxyas
[`use_self`]: Make it aware of lifetimes

Have the lint trigger even if `Self` has generic lifetime parameters.

```rs
impl<'a> Foo<'a> {
    type Item = Foo<'a>; // Can be replaced with Self

    fn new() -> Self {
        Foo { // No lifetime, but they are inferred to be that of Self
              // Can be replaced as well
            ...
        }
    }

    // Don't replace `Foo<'b>`, the lifetime is different!
    fn eq<'b>(self, other: Foo<'b>) -> bool {
        ..
    }
```

Fixes #12381

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: [`use_self`]: Have the lint trigger even if `Self` has generic lifetime parameters
2024-03-14 16:28:57 +00:00
maekawatoshiki
c5d3b62cfc
Fix conflict 2024-03-14 12:40:33 +09:00
maekawatoshiki
20e4c74521 Handle false positive with map_clone 2024-03-14 12:36:54 +09:00
J-ZhengLi
fe4e0aca73 checks dbg inside other macros as well (but no ext macro);
some refractoring;
2024-03-14 09:32:05 +08:00
J-ZhengLi
0535f55831 lint nested dbg! macros, split tests 2024-03-14 09:29:29 +08:00
J-ZhengLi
1fe884401a fix [dbg_macro] FN when dbg is inside some complex macros 2024-03-14 09:29:29 +08:00
y21
adcbb4a9b7 move readonly_write_lock to perf 2024-03-13 20:56:53 +01:00
Ethiraric
7cdeac5773 [unused_enumerate_index]: trigger on method calls
The lint used to check for patterns looking like:
```rs
for (_, x) in some_iter.enumerate() {
    // Index is ignored
}
```

This commit further checks for chained method calls constructs where we
can detect that the index is unused. Currently, this checks only for the
following patterns:
```rs
some_iter.enumerate().map_function(|(_, x)| ..)
let x = some_iter.enumerate();
x.map_function(|(_, x)| ..)
```
where `map_function` is one of `all`, `any`, `filter_map`, `find_map`,
`flat_map`, `for_each` or `map`.

Fixes #12411.
2024-03-13 20:28:01 +01:00
Guillaume Gomez
03d7ae8153 Also handle <blockquote> and <q> HTML tags 2024-03-13 19:31:01 +01:00
Guillaume Gomez
cd36b25c4f Add regression test for #10262 2024-03-13 19:25:36 +01:00
bors
73be4863f0 Auto merge of #12459 - y21:unconditional_recursion_from_into, r=Jarcho
lint when calling the blanket `Into` impl from a `From` impl

Closes #11150
```
warning: function cannot return without recursing
  --> x.rs:9:9
   |
9  | /         fn from(value: f32) -> Self {
10 | |             value.into()
11 | |         }
   | |_________^
   |
note: recursive call site
  --> x.rs:10:13
   |
10 |             value.into()
   |             ^^^^^^^^^^^^
```

I'm also thinking that we can probably generalize this lint to #11032 at some point (instead of hardcoding a bunch of impls), like how rustc's `unconditional_recursion` works, at least up to one indirect call, but this still seems useful for now :)

I've also noticed that we use `fn_def_id` in a bunch of lints and then try to get the node args of the call in a separate step, so I made a helper function that does both in one. I intend to refactor a bunch of uses of `fn_def_id` to use this later

I can add more test cases, but this is already using much of the same logic that exists for the other impls that this lint looks for (e.g. making sure that there are no conditional returns).

changelog: [`unconditional_recursion`]: emit a warning inside of `From::from` when unconditionally calling the blanket `.into()` impl
2024-03-13 16:32:25 +00:00
y21
65defdb474 [unconditional_recursion]: catch From -> Into -> From 2024-03-13 17:22:54 +01:00
bors
99e8000b92 Auto merge of #12466 - J-ZhengLi:issue12377, r=blyxyas
fix [`empty_docs`] trigger in proc-macro

fixes: #12377

---

changelog: fix [`empty_docs`] trigger in proc-macros
2024-03-12 19:23:26 +00:00
Catherine Flores
244d7da817 [cast_lossless]: Suggest type alias instead of the aliased type 2024-03-12 10:16:27 -05:00
Arthur Carcano
cbbb0ae7d2 fix: allow-one-hash-in-raw-strings option of needless_raw_string_hashes was ignored
Fixes: https://github.com/rust-lang/rust-clippy/issues/11481

changelog: Fix `allow-one-hash-in-raw-strings` option of [`needless_raw_string_hashes`] was ignored
2024-03-12 10:59:41 +01:00
bors
a8a7371728 Auto merge of #12417 - Alexendoo:iter-nth, r=flip1995
Move `iter_nth` to `style`, add machine applicable suggestion

There's no `O(n)` involved with `.iter().nth()` on the linted types since the iterator implementations provide `nth` and/or `advance_by` that operate in `O(1)`

For slice iterators the codegen is equivalent, `VecDeque`'s iterator seems to codegen differently but that doesn't seem significant enough to keep it as a perf lint

changelog: [`iter_nth`] Move to `style`

r? `@flip1995`
2024-03-12 08:54:26 +00:00
J-ZhengLi
3cd6fd15a5 fix [empty_docs] trigger in proc-macro 2024-03-12 10:40:40 +08:00
J-ZhengLi
10677d6901 add with_empty_docs attr macro & test cases for issue #12377 2024-03-12 09:19:35 +08:00
Jacherr
100ab4993e new restriction lint: division_remainder_used 2024-03-11 22:51:03 +00:00
bors
e22ca03627 Auto merge of #12452 - CBSpeir:dedup-manual-retain, r=blyxyas
[`manual_retain`]: Fix duplicate diagnostics

Relates to: #12379

The first lint guard executed in `LateLintPass::check_expr` was testing if the parent was of type `ExprKind::Assign`.  This meant the lint emitted on both sides of the assignment operator when `check_expr` is called on either `Expr`.  The guard in the fix only lints once when the `Expr` is of kind `Assign`.

changelog:  Fix duplicate lint diagnostic emission from [`manual_retain`]
2024-03-11 22:21:27 +00:00
bors
f685a4b3f1 Auto merge of #12378 - GuillaumeGomez:duplicated_attr, r=blyxyas
Add new `duplicated_attributes` lint

It's a lint idea that `@llogiq` gave me while reviewing another PR.

There are some limitations, in particular for the "output". Initially I wanted to make it possible for directly lint against the whole attribute if its parts were all duplicated, but then I realized that the output would be chaotic if the duplicates were coming from different attributes, so I preferred to go to the simplest way and simply emit a warning for each entry. Not the best, but makes the implementation much easier.

Another limitation is that `cfg_attr` would be a bit more tricky to implement because we need to check if two `cfg` sets are exactly the same. I added a FIXME and will likely come back to it later.

And finally, I updated the `cargo dev update_lints` command because the generated `tests/ui/rename.rs` file was emitting the `duplicated_attributes` lint, so I allowed this lint inside it to prevent it from working.

changelog: Add new `duplicated_attributes` lint
2024-03-11 18:36:21 +00:00
WeiTheShinobi
8e55bbf115 [single_match]: Fix duplicate diagnostics 2024-03-11 01:08:37 +08:00
bors
86717f2f0c Auto merge of #12445 - y21:document-diagnostic-utils, r=xFrednet
add documentation to the `span_lint_hir` functions

As far as I could tell, these weren't documented anywhere, and since this is sometimes needed over `span_lint` for `#[allow]` attrs to work, I thought I would add a little bit of documentation.
When I started with clippy development, I also had no idea what these functions were for.

changelog: none
2024-03-10 10:49:05 +00:00
Christopher B. Speir
ed6e6291dc Fix duplicate lint emission from [manual_retain] 2024-03-09 18:58:40 -06:00
Guillaume Gomez
98ac5f1e8c Rename into manual_unwrap_or_default 2024-03-10 01:23:28 +01:00
Guillaume Gomez
b0f358fd3c Update ui test 2024-03-10 01:23:28 +01:00
Guillaume Gomez
fadb254073 Add new ui test for match_option_and_default 2024-03-10 01:15:23 +01:00
kcz
a92037fce8
[option_option]: Fix duplicate diagnostics 2024-03-09 18:08:40 -05:00
bors
7ee75f896f Auto merge of #12447 - MarcusGrass:mg/fix-12438-regression, r=y21
Fix #12438 std_instead_of_core regression

Fixes #12438.

Boy-scouting removed two paths that checks for duplication since I thought they were unused. However, that's just because I didn't spot it in the diff.

I installed [difftastic](https://github.com/Wilfred/difftastic) and ran it on the old one:

![image](https://github.com/rust-lang/rust-clippy/assets/34198073/5c51276c-055a-49a3-9425-6f7da0590fb0)

And the new one (fixed):

![image](https://github.com/rust-lang/rust-clippy/assets/34198073/6e10f29c-6d6b-4f64-893f-de526424f1cd)

New one (stderr):
![image](https://github.com/rust-lang/rust-clippy/assets/34198073/c4c07776-ee0f-47ba-996f-6b632de47c81)

Good teachings for the future when inspecting diffs with a lot of line changes, should've thought of that before, sorry for the trouble!

changelog: [`std_instead_of_core`] Fix false positive for crates that are in `std` but not `core`
2024-03-09 21:23:38 +00:00
Jacherr
0e59259add add new lint zero_repeat_side_effects 2024-03-09 18:53:14 +00:00
y21
eb5ce85932 mention span_lint_hir in span_lint and add a reason to disallowed_methods 2024-03-09 19:40:39 +01:00
MarcusGrass
b44ab66156
Fix #12438 false positive regression 2024-03-09 18:37:18 +01:00
y21
ced8bc5b8f use span_lint_hir instead of span_lint in more lints 2024-03-09 17:43:04 +01:00
bors
b2f9c4cbc7 Auto merge of #12442 - cookie-s:fix-mutmut-duplicate-diags, r=y21
[`mut_mut`]: Fix duplicate diags

Relates to #12379

The `mut_mut` lint produced two diagnostics for each `mut mut` pattern in `ty` inside  `block`s because `MutVisitor::visit_ty` was called from `MutMut::check_ty` and  `MutMut::check_block` independently. This PR fixes the issue.

---

changelog: [`mut_mut`]: Fix duplicate diagnostics
2024-03-09 13:13:28 +00:00
bors
099e2c0033 Auto merge of #12443 - cookie-s:noeffectreoplace-fix-dup-diags, r=Alexendoo
[`no_effect_replace`]: Fix duplicate diagnostics

Relates to #12379

Fixes `no_effect_replace` duplicate diagnostics

---

changelog: [`no_effect_replace`]: Fix duplicate diagnostics
2024-03-09 13:02:47 +00:00
Guillaume Gomez
ae52a9d428 Update ui tests 2024-03-09 12:43:19 +01:00
Guillaume Gomez
d57d001543 Update cargo dev update_lints command to fix new warning emitted by duplicated_attributes 2024-03-09 12:43:19 +01:00
Guillaume Gomez
749e225cd6 Add ui test for new duplicated_attributes lint 2024-03-09 12:43:18 +01:00
bors
453242cbde Auto merge of #12310 - samueltardieu:issue-12307, r=xFrednet
New lint `const_is_empty`

This lint detects calls to `.is_empty()` on an entity initialized from a string literal and flag them as suspicious. To avoid triggering on macros called from generated code, it checks that the `.is_empty()` receiver, the call itself and the initialization come from the same context.

Fixes #12307

changelog: [`const_is_empty`]: new lint
2024-03-09 09:56:37 +00:00
Christopher B. Speir
8d78cd17e1 Fix duplicate lint emission from [else_if_without_else] 2024-03-08 22:36:17 -06:00
kcz
7473f0522c
[no_effect_replace]: Fix duplicate diagnostics 2024-03-08 23:05:16 -05:00
kcz
4e95b4a026
[mut_mut]: Fix duplicate diags 2024-03-08 22:39:36 -05:00
Nadrieril
2dd085fb00 Allow lint where we don't care 2024-03-09 01:13:42 +01:00
bors
0b4b684b46 Auto merge of #12433 - J-ZhengLi:issue12197, r=dswij
fix [`missing_docs_in_private_items`] on some proc macros

fixes: #12197

---

changelog: [`missing_docs_in_private_items`] support manually search for docs as fallback method
2024-03-08 11:05:50 +00:00
J-ZhengLi
adc91e4913 support manually search for docs in case attr was removed by proc macros 2024-03-08 16:32:47 +08:00
J-ZhengLi
3a6cac7c06 add rewrite_struct proc-macro and test case 2024-03-08 09:32:38 +08:00
Ethiraric
f3879b3630 [use_self]: Make it aware of lifetimes
Have the lint trigger even if `Self` has generic lifetime parameters.

```rs
impl<'a> Foo<'a> {
    type Item = Foo<'a>; // Can be replaced with Self

    fn new() -> Self {
        Foo { // No lifetime, but they are inferred to be that of Self
              // Can be replaced as well
            ...
        }
    }

    // Don't replace `Foo<'b>`, the lifetime is different!
    fn eq<'b>(self, other: Foo<'b>) -> bool {
        ..
    }
```

Fixes #12381
2024-03-07 20:45:59 +01:00
Philipp Krones
7e83df4068 Merge commit '93f0a9a91f58c9b2153868f458402155fb6265bb' into clippy-subtree-update 2024-03-07 17:19:29 +01:00
Philipp Krones
a6df0277ea
Merge remote-tracking branch 'upstream/master' into rustup 2024-03-07 17:14:36 +01:00
Alex Macleod
ac643a278b Don't lint redundant_field_names across macro boundaries 2024-03-07 12:21:16 +00:00
bors
a79db2aa51 Auto merge of #12401 - MarcusGrass:dedup-nonminimal-bool, r=blyxyas
Remove double expr lint

Related to #12379.

Previously the code manually checked nested binop exprs in unary exprs, but those were caught anyway by `check_expr`. Removed that code path, the path is used in the tests.

---

changelog: [`nonminimal_bool`] Remove duplicate output on nested Binops in Unary exprs.
2024-03-06 13:32:53 +00:00
bors
e485a02ef2 Auto merge of #12077 - Kobzol:assigning-clones, r=blyxyas
Add `assigning_clones` lint

This PR is a "revival" of https://github.com/rust-lang/rust-clippy/pull/10613 (with `@kpreid's` permission).

I tried to resolve most of the unresolved things from the mentioned PR:
1) The lint now checks properly if we indeed call the functions `std::clone::Clone::clone` or `std::borrow::ToOwned::to_owned`.
2) It now supports both method and function (UFCS) calls.
3) A heuristic has been added to decide if the lint should apply. It will only apply if the type on which the method is called has a custom implementation of `clone_from/clone_into`. Notably, it will not trigger for types that use `#[derive(Clone)]`.
4) `Deref` handling has been (hopefully) a bit improved, but I'm not sure if it's ideal yet.

I also added a bunch of additional tests.

There are a few things that could be improved, but shouldn't be blockers:
1) When the right-hand side is a function call, it is transformed into e.g. `::std::clone::Clone::clone(...)`. It would be nice to either auto-import the `Clone` trait or use the original path and modify it (e.g. `clone::Clone::clone` -> `clone::Clone::clone_from`). I don't know how to modify the `QPath` to do that though.
2) The lint currently does not trigger when the left-hand side is a local variable without an initializer. This is overly conservative, since it could trigger when the variable has no initializer, but it has been already initialized at the moment of the function call, e.g.
```rust
let mut a;
...
a = Foo;
...
a = b.clone(); // Here the lint should trigger, but currently doesn't
```
These cases probably won't be super common, but it would be nice to make the lint more precise. I'm not sure how to do that though, I'd need access to some dataflow analytics or something like that.

changelog: new lint [`assigning_clones`]
2024-03-05 15:26:57 +00:00
bors
2d9d404448 Auto merge of #12413 - high-cloud:fix_assign_ops2, r=flip1995
[`misrefactored_assign_op`]: Fix duplicate diagnostics

Relate to #12379

The following diagnostics appear twice
```
  --> tests/ui/assign_ops2.rs:26:5
   |
LL |     a *= a * a;
   |     ^^^^^^^^^^
   |
help: did you mean `a = a * a` or `a = a * a * a`? Consider replacing it with
```

because `a` (lhs) appears in both left operand and right operand in the right hand side.
This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.

changelog: [`misrefactored_assign_op`]: Fix duplicate diagnostics
2024-03-05 14:34:56 +00:00
bors
21efd39b04 Auto merge of #12423 - GuillaumeGomez:code-wrapped-element, r=Alexendoo
Don't emit "missing backticks" lint if the element is wrapped in `<code>` HTML tags

Fixes #9473.

changelog: Don't emit "missing backticks" lint if the element is wrapped in `<code>` HTML tags
2024-03-05 14:02:54 +00:00
Yaodong Yang
3c5008e8de [misrefactored_assign_op]: Fix duplicate diagnostics 2024-03-05 19:53:26 +08:00
Guillaume Gomez
bd9b9ffa04 Add regression test for #9473 2024-03-05 12:07:37 +01:00
Nicholas Nethercote
dc37dddeb5 Rename DiagnosticMessage as DiagMessage. 2024-03-05 12:14:49 +11:00
Samuel Moelius
cc4c8db0fd Handle plural acronyms in doc_markdown 2024-03-04 22:00:24 +00:00
Alex Macleod
301d293ef6 Move iter_nth to style, add machine applicable suggestion 2024-03-04 16:39:15 +00:00
bors
c0939b18b8 Auto merge of #12409 - cookie-s:fix-identityop-duplicate-errors, r=Alexendoo
[`identity_op`]: Fix duplicate diagnostics

Relates to #12379

In the `identity_op` lint, the following diagnostic was emitted two times

```
  --> tests/ui/identity_op.rs:156:5
   |
LL |     1 * 1;
   |     ^^^^^ help: consider reducing it to: `1`
   |
```

because both of the left operand and the right operand are the identity element of the multiplication.

This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.

---

changelog: [`identity_op`]: Fix duplicate diagnostics
2024-03-04 14:43:38 +00:00
bors
e970fa52e7 Auto merge of #12341 - y21:issue12337, r=dswij
Check for try blocks in `question_mark` more consistently

Fixes #12337

I split this PR up into two commits since this moves a method out of an `impl`, which makes for a pretty bad diff (the `&self` parameter is now unused, and there isn't a reason for that function to be part of the `impl` now).

The first commit is the actual relevant change and the 2nd commit just moves stuff (github's "hide whitespace" makes the diff easier to look at)

------------
Now for the actual issue:

`?` within `try {}` blocks desugars to a `break` to the block, rather than a `return`, so that changes behavior in those cases.

The lint has multiple patterns to look for and in *some* of them it already does correctly check whether we're in a try block, but this isn't done for all of its patterns.

We could add another `self.inside_try_block()` check to the function that looks for `let-else-return`, but I chose to actually just move those checks out and instead have them in `LintPass::check_{stmt,expr}`. This has the advantage that we can't (easily) accidentally forget to add that check in new patterns that might be added in the future.

(There's also a bit of a subtle interaction between two lints, where `question_mark`'s LintPass calls into `manual_let_else`, so I added a check to make sure we don't avoid linting for something that doesn't have anything to do with `?`)

changelog: [`question_mark`]: avoid linting on try blocks in more cases
2024-03-04 14:34:56 +00:00
bors
f75d8c7f1b Auto merge of #12393 - J-ZhengLi:issue9413, r=dswij
fix [`derive_partial_eq_without_eq`] FP on trait projection

fixes: #9413 #9319

---

changelog: fix [`derive_partial_eq_without_eq`] FP on trait projection

Well, this is awkward, it works but I don't understand why, why `clippy_utils::ty::implements_trait` couldn't detects the existance of `Eq` trait, even thought it's obviously present in the derive attribute.
2024-03-04 06:43:59 +00:00
kcz
3b9939e83b
[identity_op]: Fix duplicate errors 2024-03-03 19:25:51 -05:00
bors
c2dd413c79 Auto merge of #12403 - samueltardieu:issue-12402, r=blyxyas
Pointers cannot be converted to integers at compile time

Fix #12402

changelog: [`transmutes_expressible_as_ptr_casts`]: do not suggest invalid const casts
2024-03-03 23:09:54 +00:00
bors
aceeb54b75 Auto merge of #12405 - PartiallyTyped:12404, r=blyxyas
Added msrv to threadlocal initializer check

closes: #12404
changelog:[`thread_local_initializer_can_be_made_const`]: Check for MSRV (>= 1.59) before processing.
2024-03-03 23:01:37 +00:00
Samuel Tardieu
6e5332cd9c Pointers cannot be converted to integers at compile time 2024-03-03 23:55:01 +01:00
bors
30642113b2 Auto merge of #12406 - MarcusGrass:fix-duplicate-std-instead-of-core, r=Alexendoo
Dedup std_instead_of_core by using first segment span for uniqueness

Relates to #12379.

Instead of checking that the paths have an identical span, it checks that the relevant `std` part of the path segment's span is identical. Added a multiline test, because my first implementation was worse and failed that, then I realized that you could grab the span off the first_segment `Ident`.

I did find another bug that isn't addressed by this, and that exists on master as well.

The path:
```Rust
use std::{io::Write, fmt::Display};
```

Will get fixed into:
```Rust
use core::{io::Write, fmt::Display};
```

Which doesn't compile since `io::Write` isn't in `core`, if any of those paths are present in `core` it'll do the replace and cause a miscompilation. Do you think I should file a separate bug for that? Since `rustfmt` default splits those up it isn't that big of a deal.

Rustfmt:
```Rust
// Pre
use std::{io::Write, fmt::Display};
// Post
use std::fmt::Display;
use std::io::Write;
```
---

changelog: [`std_instead_of_core`]: Fix duplicated output on multiple imports
2024-03-03 18:22:40 +00:00
MarcusGrass
3735bf93ff
Working but not with mixed imports 2024-03-03 17:23:11 +01:00
MarcusGrass
a2e4ef294c
dump bugged fix 2024-03-03 17:17:58 +01:00
Quinn Sinclair
08459b4dae Added msrv to threadlocal initializer 2024-03-03 15:43:09 +01:00
MarcusGrass
74cba639e9
Dedup std_instead_of_core by using first segment span for uniqueness 2024-03-03 15:30:49 +01:00
Samuel Tardieu
1df2854ac3 [let_underscore_untyped]: fix false positive on async function 2024-03-03 14:03:02 +01:00
MarcusGrass
8e3ad2e545
Remove double expr lint 2024-03-03 08:32:30 +01:00
J-ZhengLi
dde2552b11 add test cases for #9319 2024-03-02 01:06:59 +08:00
J-ZhengLi
1a97d1460b fix [derive_partial_eq_without_eq] FP on trait projection 2024-03-02 00:37:35 +08:00
Jakub Beránek
24de0be29b
Fix tests 2024-03-01 16:37:23 +01:00
Jakub Beránek
bc551b9a70
Do not run the lint on macro-generated code 2024-03-01 16:36:05 +01:00
Jakub Beránek
f7356f2a8f
Fix lint errors 2024-03-01 16:36:05 +01:00
Jakub Beránek
41f5ee1189
React to review 2024-03-01 16:36:05 +01:00
Jakub Beránek
0fcc33e29c
Fix tests 2024-03-01 16:36:04 +01:00
Jakub Beránek
0656d28f6b
Add assigning_clones lint 2024-03-01 16:35:28 +01:00
clubby789
aa1c9a5993 If suggestion would leave an empty line, delete it 2024-03-01 13:48:20 +00:00
bors
e865dca4d7 Auto merge of #12010 - granddaifuku:fix/manual-memcpy-indexing-for-multi-dimension-arrays, r=Alexendoo
fix: `manual_memcpy` wrong indexing for multi dimensional arrays

fixes: #9334

This PR fixes an invalid suggestion for multi-dimensional arrays.

For example,
```rust
let src = vec![vec![0; 5]; 5];
let mut dst = vec![0; 5];

for i in 0..5 {
    dst[i] = src[i][i];
}
```

For the above code, Clippy suggests `dst.copy_from_slice(&src[i]);`, but it is not compilable because `i` is only used to loop the array.
I adjusted it so that Clippy `manual_memcpy` works properly for multi-dimensional arrays.

changelog: [`manual_memcpy`]: Fixes invalid indexing suggestions for multi-dimensional arrays
2024-03-01 12:05:03 +00:00
Quinn Sinclair
bb1ee8746e Fixed FP for thread_local_initializer_can_be_made_const for os_local
`os_local` impl of `thread_local` — regardless of whether it is const and
unlike other implementations — includes an `fn __init(): EXPR`.

Existing implementation of the lint checked for the presence of said
function and whether the expr can be made const. Because for `os_local`
we always have an `__init()`, it triggers for const implementations.

The solution is to check whether the `__init()` function is already const.
If it is `const`, there is nothing to do. Otherwise, we verify that we can
make it const.

Co-authored-by: Alejandra González <blyxyas@gmail.com>
2024-03-01 00:41:14 +01:00
Guillaume Gomez
7cfe7d6223 Rollup merge of #121669 - nnethercote:count-stashed-errs-again, r=estebank
Count stashed errors again

Stashed diagnostics are such a pain. Their "might be emitted, might not" semantics messes with lots of things.

#120828 and #121206 made some big changes to how they work, improving some things, but still leaving some problems, as seen by the issues caused by #121206. This PR aims to fix all of them by restricting them in a way that eliminates the "might be emitted, might not" semantics while still allowing 98% of their benefit. Details in the individual commit logs.

r? `@oli-obk`
2024-02-29 17:08:38 +01:00
bors
00ff8c92d3 Auto merge of #12354 - GuillaumeGomez:mixed_attributes_style, r=llogiq
Add new `mixed_attributes_style` lint

Add a new lint to detect cases where both inner and outer attributes are used on a same item.

r? `@llogiq`

----

changelog: Add new [`mixed_attributes_style`] lint
2024-02-29 11:44:51 +00:00
Nicholas Nethercote
81783fbf89 Overhaul how stashed diagnostics work, again.
Stashed errors used to be counted as errors, but could then be
cancelled, leading to `ErrorGuaranteed` soundness holes. #120828 changed
that, closing the soundness hole. But it introduced other difficulties
because you sometimes have to account for pending stashed errors when
making decisions about whether errors have occured/will occur and it's
easy to overlook these.

This commit aims for a middle ground.
- Stashed errors (not warnings) are counted immediately as emitted
  errors, avoiding the possibility of forgetting to consider them.
- The ability to cancel (or downgrade) stashed errors is eliminated, by
  disallowing the use of `steal_diagnostic` with errors, and introducing
  the more restrictive methods `try_steal_{modify,replace}_and_emit_err`
  that can be used instead.

Other things:
- `DiagnosticBuilder::stash` and `DiagCtxt::stash_diagnostic` now both
  return `Option<ErrorGuaranteed>`, which enables the removal of two
  `delayed_bug` calls and one `Ty::new_error_with_message` call. This is
  possible because we store error guarantees in
  `DiagCtxt::stashed_diagnostics`.
- Storing the guarantees also saves us having to maintain a counter.
- Calls to the `stashed_err_count` method are no longer necessary
  alongside calls to `has_errors`, which is a nice simplification, and
  eliminates two more `span_delayed_bug` calls and one FIXME comment.
- Tests are added for three of the four fixed PRs mentioned below.
- `issue-121108.rs`'s output improved slightly, omitting a non-useful
  error message.

Fixes #121451.
Fixes #121477.
Fixes #121504.
Fixes #121508.
2024-02-29 11:08:27 +11:00
Ethiraric
0d59345907 [redundant_closure_call]: Don't lint if closure origins from a macro
The following code used to trigger the lint:
```rs
 macro_rules! make_closure {
     () => {
         (|| {})
     };
 }
 make_closure!()();
```
The lint would suggest to replace `make_closure!()()` with
`make_closure!()`, which changes the code and removes the call to the
closure from the macro. This commit fixes that.

Fixes #12358
2024-02-28 19:17:37 +01:00
bors
af91e6ea8c Auto merge of #12374 - Alexendoo:duplicate-diagnostics, r=Manishearth
Show duplicate diagnostics in UI tests by default

Duplicated diagnostics can indicate where redundant work is being done, this PR doesn't fix any of that but does indicate in which tests they're occurring for future investigation or to catch issues in future lints

changelog: none
2024-02-28 16:19:08 +00:00
Alex Macleod
733e1d43c7 Show duplicate diagnostics in UI tests by default 2024-02-28 13:24:14 +00:00
Guillaume Gomez
8473716f6e Add ui regression test for #12371 2024-02-28 12:48:00 +01:00
Nicholas Nethercote
1ba47ea06c Use LitKind::Err for floats with empty exponents.
This prevents a follow-up type error in a test, which seems fine.
2024-02-28 20:59:27 +11:00
y21
1430623e04 move methods out of impl and remove unused &self param 2024-02-28 00:53:25 +01:00
y21
0671d78283 check for try blocks in LintPass methods 2024-02-27 23:49:07 +01:00
bors
4c1d05cfa1 Auto merge of #12362 - Ethiraric:fix-11935, r=llogiq
[`map_entry`]: Check insert expression for map use

The lint makes sure that the map is not used (borrowed) before the call to `insert`. Since the lint creates a mutable borrow on the map with the `Entry`, it wouldn't be possible to replace such code with `Entry`. However, expressions up to the `insert` call are checked, but not expressions for the arguments of the `insert` call itself. This commit fixes that.

Fixes #11935

----

changelog: [`map_entry`]: Fix false positive when borrowing the map in the `insert` call
2024-02-27 18:53:25 +00:00
Yudai Fukushima
dfedadc179 fix: manual_memcpy wrong suggestion for multi dimensional arrays
chore: rebase master

chore: replace $DIR

fix: check bases does not contain reference to loop index

fix: grammatical mistake

fix: style
2024-02-28 03:48:49 +09:00
Ethiraric
c6cb0e99f3 [unnecessary_cast]: Avoid breaking precedence
If the whole cast expression is a unary expression (`(*x as T)`) or an
addressof expression (`(&x as T)`), then not surrounding the suggestion
into a block risks us changing the precedence of operators if the cast
expression is followed by an operation with higher precedence than the
unary operator (`(*x as T).foo()` would become `*x.foo()`, which changes
what the `*` applies on).
The same is true if the expression encompassing the cast expression is a
unary expression or an addressof expression.

The lint supports the latter case, but missed the former one. This PR
fixes that.

Fixes #11968
2024-02-27 16:27:12 +01:00
Philipp Krones
7be6e2178e Merge commit '10136170fe9ed01e46aeb4f4479175b79eb0e3c7' into clippy-subtree-update 2024-02-27 15:50:17 +01:00
Guillaume Gomez
f30138623b Update ui tests 2024-02-27 15:22:39 +01:00
Guillaume Gomez
28738234ac Add ui test for mixed_attributes_style 2024-02-27 15:22:39 +01:00
Ethiraric
03bb7908b9 [map_entry]: Check insert expression for map use
The lint makes sure that the map is not used (borrowed) before the call
to `insert`. Since the lint creates a mutable borrow on the map with the
`Entry`, it wouldn't be possible to replace such code with `Entry`.
However, expressions up to the `insert` call are checked, but not
expressions for the arguments of the `insert` call itself. This commit
fixes that.

Fixes #11935
2024-02-27 15:20:49 +01:00
Oli Scherer
592fe89997 lower bstr version requirement to 1.6.0 2024-02-27 13:34:01 +00:00
bors
e33cba523a Auto merge of #12126 - teor2345:patch-1, r=llogiq
Fix sign-handling bugs and false negatives in `cast_sign_loss`

**Note: anyone should feel free to move this PR forward, I might not see notifications from reviewers.**

changelog: [`cast_sign_loss`]: Fix sign-handling bugs and false negatives

This PR fixes some arithmetic bugs and false negatives in PR #11883 (and maybe earlier PRs).
Cc `@J-ZhengLi`

I haven't updated the tests yet. I was hoping for some initial feedback before adding tests to cover the cases listed below.

Here are the issues I've attempted to fix:

#### `abs()` can return a negative value in release builds

Example:
```rust
i32::MIN.abs()
```
https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=022d200f9ef6ee72f629c0c9c1af11b8

Docs: https://doc.rust-lang.org/std/primitive.i32.html#method.abs

Other overflows that produce negative values could cause false negatives (and underflows could produce false positives), but they're harder to detect.

#### Values with uncertain signs can be positive or negative

Any number of values with uncertain signs cause the whole expression to have an uncertain sign, because an uncertain sign can be positive or negative.

Example (from UI tests):
```rust
fn main() {
    foo(a: i32, b: i32, c: i32) -> u32 {
        (a * b * c * c) as u32
        //~^ ERROR: casting `i32` to `u32` may lose the sign of the value
    }

    println!("{}", foo(1, -1, 1));
}
```
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=165d2e2676ee8343b1b9fe60db32aadd

#### Handle `expect()` the same way as `unwrap()`

Since we're ignoring `unwrap()` we might as well do the same with `expect()`.

This doesn't seem to have tests but I'm happy to add some like `Some(existing_test).unwrap() as u32`.

#### A negative base to an odd exponent is guaranteed to be negative

An integer `pow()`'s sign is only uncertain when its operants are uncertain. (Ignoring overflow.)

Example:
```rust
((-2_i32).pow(3) * -2) as u32
```

This offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. (Rather than just an odd number of uncertain signs.)

#### Both sides of a multiply or divide should be peeled recursively

I'm not sure why the lhs was peeled recursively, and the rhs was left intact. But the sign of any sequence of multiplies and divides is determined by the signs of its operands. (Ignoring overflow.)

I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.

But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, these should all lint:
```rust
fn peel_all(x: i32) {
    (-p(x) * -p(x) * -p(x)) as u32;
    ((-p(x) * -p(x)) * -p(x)) as u32;
    (-p(x) * (-p(x) * -p(x))) as u32;
}
```

#### The right hand side of a Rem doesn't change the sign

Unlike Mul and Div,
> Given remainder = dividend % divisor, the remainder will have the same sign as the dividend.
https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators

I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.

But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, only the first six expressions should lint.

The expressions that start with a constant should lint (or not lint) regardless of whether the lint supports `p()` or unary negation, because only the dividend's sign matters.

Example:
```rust
fn rem_lhs(x: i32) {
    (-p(x) % -1) as u32;
    (-p(x) % 1) as u32;
    (-1 % -p(x)) as u32;
    (-1 % p(x)) as u32;
    (-1 % -x) as u32;
    (-1 % x) as u32;
    // These shouldn't lint:
    (p(x) % -1) as u32;
    (p(x) % 1) as u32;
    (1 % -p(x)) as u32;
    (1 % p(x)) as u32;
    (1 % -x) as u32;
    (1 % x) as u32;
}
```

#### There's no need to bail on other expressions

When peeling, any other operators or expressions can be left intact and sent to the constant evaluator.

If these expressions can be evaluated, this offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. If not, they end up marked as having uncertain sign.
2024-02-27 05:38:40 +00:00
bors
fb060815b3 Auto merge of #11136 - y21:enhance_read_line_without_trim, r=dswij
[`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls

This lint now also realizes that a comparison like `s == "foo"` and calls such as `s.ends_with("foo")` will fail if `s` was initialized by a call to `Stdin::read_line` (because of the trailing newline).

changelog: [`read_line_without_trim`]: detect string literal comparison and `.ends_with()` calls

r? `@giraffate` assigning you because you reviewed #10970 that added this lint, so this is kinda a followup PR ^^
2024-02-27 03:36:12 +00:00
bors
d12b53e481 Auto merge of #12116 - J-ZhengLi:issue12101, r=Alexendoo
fix suggestion error in [`useless_vec`]

fixes: #12101

---

changelog: fix suggestion error in [`useless_vec`]

r+ `@matthiaskrgr` since they opened the issue?
2024-02-26 22:38:24 +00:00
y21
fd85db3636 restructure lint code, update description, more cases 2024-02-26 20:24:46 +01:00
bors
1c5094878b Auto merge of #12342 - lucarlig:empty-docs, r=llogiq
Empty docs

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

changelog: [`empty_doc`]: Detects documentation that is empty.
changelog: Doc comment lints now trigger for struct field and enum variant documentation
2024-02-26 18:03:13 +00:00
Ethiraric
97dc4b22c6 [box_default]: Preserve required path segments
When encountering code such as:
```
Box::new(outer::Inner::default())
```
clippy would suggest replacing with `Box::<Inner>::default()`, dropping
the `outer::` segment. This behavior is incorrect and that commit fixes
it.

What it does is it checks the contents of the `Box::new` and, if it is
of the form `A::B::default`, does a text replacement, inserting `A::B`
in the `Box`'s quickfix generic list.
If the source does not match that pattern (including `Vec::from(..)`
or other `T::new()` calls), we then fallback to the original code.

Fixes #11927
2024-02-26 17:39:00 +01:00
Samuel Tardieu
898ed8825d feat: make const_is_empty lint ignore external constants 2024-02-26 09:54:19 +01:00
Samuel Tardieu
1159e2c00f feat: add more tests for the const_is_empty lint
Suggested by @xFredNet and @matthiaskrgr.
2024-02-26 09:03:46 +01:00
Samuel Tardieu
288497093b feat: extend const_is_empty with many kinds of constants 2024-02-26 08:58:18 +01:00
Samuel Tardieu
89b334d47c chore: update some tests to allow const_is_empty 2024-02-26 08:51:28 +01:00
Samuel Tardieu
dbfbd0e77f feat: add const_is_empty lint 2024-02-26 08:51:28 +01:00
bors
aa2c94e416 Auto merge of #12308 - y21:more_implied_bounds, r=xFrednet
Look for `implied_bounds_in_impls` in more positions

With this, we lint `impl Trait` implied bounds in more positions:
- Type alias impl trait
- Associated type position impl trait
- Argument position impl trait
  - these are not opaque types, but instead are desugared to `where` clauses, so we need extra logic for finding them (`check_generics`), however the rest of the logic is the same

Before this, we'd only lint RPIT `impl Trait`s.
"Hide whitespaces" and reviewing commits individually might make this easier

changelog: [`implied_bounds_in_impls`]: start linting implied bounds in APIT, ATPIT, TAIT
2024-02-25 22:20:37 +00:00
y21
bbfe1c1ec3 lint implied bounds in APIT 2024-02-25 23:12:28 +01:00
y21
ec29b0d6b8 lint implied bounds in *all* opaque impl Trait types 2024-02-25 23:09:59 +01:00
bors
b8fb8907ba Auto merge of #120393 - Urgau:rfc3373-non-local-defs, r=WaffleLapkin
Implement RFC 3373: Avoid non-local definitions in functions

This PR implements [RFC 3373: Avoid non-local definitions in functions](https://github.com/rust-lang/rust/issues/120363).
2024-02-25 19:11:06 +00:00
lucarlig
d84d9d32f1 lint on variant and fields as well 2024-02-25 22:33:16 +04:00
lucarlig
f32e92cdc9 add 1 more test and dont trim other code 2024-02-25 21:18:38 +04:00
y21
9a56153c5e [single_call_fn]: merge post-crate visitor into lint pass 2024-02-25 17:13:47 +01:00
bors
c469cb0023 Auto merge of #12336 - not-elm:fix/issue-12243, r=y21
FIX(12243): redundant_guards

Fixed #12243

changelog: Fix[`redundant_guards`]

I have made a correction so that no warning does  appear when y.is_empty() is used within a constant function as follows.

```rust
pub const fn const_fn(x: &str) {
    match x {
        // Shouldn't lint.
        y if y.is_empty() => {},
        _ => {},
    }
}
```
2024-02-25 14:56:07 +00:00
lucarlig
5a50cede29 add single letter test 2024-02-25 18:01:53 +04:00
lucarlig
ee0cbeaa77 ignore empty comment in semicolon_if_nothing_returned 2024-02-25 17:41:41 +04:00
lucarlig
a3fea80a68 bless tests 2024-02-25 17:04:58 +04:00
lucarlig
84219f45a3 working naive with outside check_attrs 2024-02-25 16:11:14 +04:00
not-elm
5a63cd82cb FIX(12243): redundant_guard
A warning is now suppressed when "<str_va> if <str_var>.is_empty" is used in a constant function.

FIX: instead of clippy_util::in_const

FIX: Merged `redundant_guards_const_fn.rs` into `redundant_guards.rs`.
2024-02-25 15:38:18 +09:00
lucarlig
3093b291f6 WIP: empty doc span is still broken 2024-02-25 09:55:58 +04:00
Guillaume Gomez
a1971414ec Update ui tests 2024-02-24 15:02:10 +01:00
Guillaume Gomez
40cff2d99f Add ui test for unnecessary_get_then_check 2024-02-24 15:02:10 +01:00
bors
a2c1d565e5 Auto merge of #12259 - GuillaumeGomez:multiple-bound-locations, r=llogiq
Add new `multiple_bound_locations` lint

Fixes #7181.

r? `@llogiq`

changelog: Add new `multiple_bound_locations` lint
2024-02-24 13:43:34 +00:00
bors
64054693eb Auto merge of #12322 - sanxiyn:expression-with-attribute, r=llogiq
Be careful with expressions with attributes

Fix #9949.

changelog: [`unused_unit`]: skip expressions with attributes
2024-02-24 10:52:55 +00:00
Jason Newcomb
5ab42d8e6a Take lifetime extension into account in ref_as_ptr 2024-02-23 21:33:53 -05:00
Guillaume Gomez
762448bc55 Update ui tests 2024-02-23 17:38:39 +01:00
J-ZhengLi
929f746b5c fix the actual bug 2024-02-24 00:35:48 +08:00
Guillaume Gomez
6955a8ac4e Add ui test for multiple_bound_locations lint 2024-02-23 16:40:21 +01:00
MarcusGrass
f1974593c9
Remove double unused_imports check 2024-02-22 23:12:38 +01:00
MarcusGrass
97a3ac5b86
Allow unused_imports, and unused_import_braces on use 2024-02-22 21:53:04 +01:00
Philipp Krones
8a58b7613d
Update i686 asm test stderr 2024-02-22 16:27:24 +01:00
Philipp Krones
dc0bb69e66
Merge remote-tracking branch 'upstream/master' into rustup 2024-02-22 15:59:29 +01:00
bors
d554bcad79 Auto merge of #12303 - GuillaumeGomez:unneedeed_clippy_cfg_attr, r=flip1995
Add `unnecessary_clippy_cfg` lint

Follow-up of https://github.com/rust-lang/rust-clippy/pull/12292.

r? `@flip1995`

changelog: Add `unnecessary_clippy_cfg` lint
2024-02-22 11:00:52 +00:00