Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clippy subtree update #122149

Merged
merged 107 commits into from
Mar 7, 2024
Merged

Clippy subtree update #122149

merged 107 commits into from
Mar 7, 2024

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Mar 7, 2024

Urgau and others added 30 commits February 17, 2024 13:59
…leLapkin

Implement RFC 3373: Avoid non-local definitions in functions

This PR implements [RFC 3373: Avoid non-local definitions in functions](rust-lang#120363).
Add `ErrorGuaranteed` to `ast::ExprKind::Err`

See rust-lang#119967 for context
```
      \
       \
          _~^~^~_
      \) /  o o  \ (/
        '_   -   _'
        / '-----' \
```

r? fmease
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 rust-lang#11935
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 rust-lang#11968
[`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 rust-lang#11968

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

changelog: [`unnecessary_cast`]: Avoid breaking precedence with unary operators (`(*x as T).foo()` --  before: `*x.foo()` -- now: `{*x}.foo()`)
chore: rebase master

chore: replace $DIR

fix: check bases does not contain reference to loop index

fix: grammatical mistake

fix: style
[`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 rust-lang#11935

----

changelog: [`map_entry`]: Fix false positive when borrowing the map in the `insert` call
Much better!

Note that this involves renaming (and updating the value of)
`DIAGNOSTIC_BUILDER` in clippy.
It's a specialized form of the `UntranslatableDiagnostic` lint that is
deny-by-default.

Now that `UntranslatableDiagnostic` has been changed from
allow-by-default to deny-by-default, the trivial variant is no longer
needed.
This prevents a follow-up type error in a test, which seems fine.
Improve `is_lint_level` code

Since rust-lang#121230 was merged, we can now rely on `Level` directly instead of keeping the list of symbols to check in clippy.

changelog: Improve `is_lint_level` code
…gression, r=flip1995

Fix `nonminimal_bool` lint regression

Fixes rust-lang#12371.
Fixes rust-lang#12369.

cc `@RalfJung`

The problem was an invalid condition. Shame on me...

changelog: Fix `nonminimal_bool` lint regression
…nishearth

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
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 rust-lang#12358
[`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 rust-lang#12358

----

changelog: [`redundant_closure_call`]: If `x!()` returns a closure, don't suggest replacing `x!()()` with `x!()`
bors and others added 15 commits March 5, 2024 10:47
Handle plural acronyms in `doc_markdown`

Prevent `doc_markdown` from triggering on words like `OSes` and `UXes`.

changelog: handle plural acronyms in `doc_markdown`
…r=Alexendoo

Don't emit "missing backticks" lint if the element is wrapped in `<code>` HTML tags

Fixes rust-lang#9473.

changelog: Don't emit "missing backticks" lint if the element is wrapped in `<code>` HTML tags
[`misrefactored_assign_op`]: Fix duplicate diagnostics

Relate to rust-lang#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
Add `assigning_clones` lint

This PR is a "revival" of rust-lang/rust-clippy#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`]
…blyxyas

Remove double expr lint

Related to rust-lang#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.
…-ctxt, r=Manishearth

Don't lint `redundant_field_names` across macro boundaries

Fixes rust-lang#12426

The `field.span.eq_ctxt(field.ident.span)` addition is the relevant line for the bugfix

The current implementation checks that the field's name and the path are in the same context by comparing the idents, but not that the two are in the same context as the entire field itself, so in local macros `SomeStruct { $ident: $ident }` would get linted

changelog: none
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 7, 2024
@rustbot
Copy link
Collaborator

rustbot commented Mar 7, 2024

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 7, 2024

📌 Commit 73f7e79 has been approved by Manishearth

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 7, 2024
@Manishearth
Copy link
Member

@bors p=1

@bors
Copy link
Contributor

bors commented Mar 7, 2024

⌛ Testing commit 73f7e79 with merge 735f758...

@bors
Copy link
Contributor

bors commented Mar 7, 2024

☀️ Test successful - checks-actions
Approved by: Manishearth
Pushing 735f758 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 7, 2024
@bors bors merged commit 735f758 into rust-lang:master Mar 7, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 7, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (735f758): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.6% [-2.6%, -2.6%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-3.0% [-3.0%, -3.0%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -3.0% [-3.0%, -3.0%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 647.822s -> 647.318s (-0.08%)
Artifact size: 172.63 MiB -> 172.63 MiB (-0.00%)

@flip1995 flip1995 deleted the clippy-subtree-update branch March 8, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.