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

Update Clippy #100419

Merged
merged 94 commits into from
Aug 12, 2022
Merged

Update Clippy #100419

merged 94 commits into from
Aug 12, 2022

Conversation

flip1995
Copy link
Member

AaronC81 and others added 30 commits July 15, 2022 15:49
Fix redundant_closure_call for single-expression async closures

Add Sugg::asyncify

Use Sugg for redundant_closure_call implementation
Address issue rust-lang#99265 by checking each positionally used argument
to see if the argument is named and adding a lint to use the name
instead. This way, when named arguments are used positionally in a
different order than their argument order, the suggested lint is
correct.

For example:
```
println!("{b} {}", a=1, b=2);
```
This will now generate the suggestion:
```
println!("{b} {a}", a=1, b=2);
```

Additionally, this check now also correctly replaces or inserts
only where the positional argument is (or would be if implicit).
Also, width and precision are replaced with their argument names
when they exists.

Since the issues were so closely related, this fix for issue rust-lang#99265
also fixes issue rust-lang#99266.

Fixes rust-lang#99265
Fixes rust-lang#99266
Co-authored-by: Andy Caldwell <andycaldwell@microsoft.com>
…rcho

Read and use deprecated configuration (as well as emitting a warning)

Original change written by `@flip1995` I've simply rebased to master and fixed up the formatting/tests.  This change teaches the configuration parser which config key replaced a deprecated key and attempts to populate the latter from the former.  If both keys are provided this fails with a duplicate key error (rather than attempting to guess which the user intended).

Currently this on affects `cyclomatic-complexity-threshold` -> `cognitive-complexity-threshold` but will also be used in rust-lang#8974 to handle `blacklisted-names` -> `disallowed-names`.

```
changelog: deprecated configuration keys are still applied as if they were provided as their non-deprecated name.
```

- [x] `cargo test` passes locally
- [x] Run `cargo dev fmt`
…rrors

Generate correct suggestion with named arguments used positionally

Address issue rust-lang#99265 by checking each positionally used argument
to see if the argument is named and adding a lint to use the name
instead. This way, when named arguments are used positionally in a
different order than their argument order, the suggested lint is
correct.

For example:
```
println!("{b} {}", a=1, b=2);
```
This will now generate the suggestion:
```
println!("{b} {a}", a=1, b=2);
```

Additionally, this check now also correctly replaces or inserts
only where the positional argument is (or would be if implicit).
Also, width and precision are replaced with their argument names
when they exists.

Since the issues were so closely related, this fix for issue rust-lang#99265
also fixes issue rust-lang#99266.

Fixes rust-lang#99265
Fixes rust-lang#99266
A `TokenStream` contains a `Lrc<Vec<(TokenTree, Spacing)>>`. But this is
not quite right. `Spacing` makes sense for `TokenTree::Token`, but does
not make sense for `TokenTree::Delimited`, because a
`TokenTree::Delimited` cannot be joined with another `TokenTree`.

This commit fixes this problem, by adding `Spacing` to `TokenTree::Token`,
changing `TokenStream` to contain a `Lrc<Vec<TokenTree>>`, and removing the
`TreeAndSpacing` typedef.

The commit removes these two impls:
- `impl From<TokenTree> for TokenStream`
- `impl From<TokenTree> for TreeAndSpacing`

These were useful, but also resulted in code with many `.into()` calls
that was hard to read, particularly for anyone not highly familiar with
the relevant types. This commit makes some other changes to compensate:
- `TokenTree::token()` becomes `TokenTree::token_{alone,joint}()`.
- `TokenStream::token_{alone,joint}()` are added.
- `TokenStream::delimited` is added.

This results in things like this:
```rust
TokenTree::token(token::Semi, stmt.span).into()
```
changing to this:
```rust
TokenStream::token_alone(token::Semi, stmt.span)
```
This makes the type of the result, and its spacing, clearer.

These changes also simplifies `Cursor` and `CursorRef`, because they no longer
need to distinguish between `next` and `next_with_spacing`.
…y, r=Manishearth

Remove "blacklist" terminology

Picking up where rust-lang/rust-clippy#5896 left off, this renames the `blacklisted_name` lint to `disallowed_names` (pluralised for compliance with naming conventions).  The old name is still available though is deprecated (both in the lint name, and in the TOML configuration file).

This has been proposed/requested a few times, most recently in rust-lang/rust-clippy#7582 where `@xFrednet` suggested pinging the Clippy team when a PR was created hence...

cc: `@rust-lang/clippy`

changelog: [`disallowed_names`] lint replaces `blacklisted_name`
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
The expect_used lint is allow-by-default, so it would be better to show the case where this is enabled.

Co-authored-by: Takayuki Nakata <f.seasons017@gmail.com>
…ochenkov

Remove `TreeAndSpacing`.

A `TokenStream` contains a `Lrc<Vec<(TokenTree, Spacing)>>`. But this is
not quite right. `Spacing` makes sense for `TokenTree::Token`, but does
not make sense for `TokenTree::Delimited`, because a
`TokenTree::Delimited` cannot be joined with another `TokenTree`.

This commit fixes this problem, by adding `Spacing` to `TokenTree::Token`,
changing `TokenStream` to contain a `Lrc<Vec<TokenTree>>`, and removing the
`TreeAndSpacing` typedef.

The commit removes these two impls:
- `impl From<TokenTree> for TokenStream`
- `impl From<TokenTree> for TreeAndSpacing`

These were useful, but also resulted in code with many `.into()` calls
that was hard to read, particularly for anyone not highly familiar with
the relevant types. This commit makes some other changes to compensate:
- `TokenTree::token()` becomes `TokenTree::token_{alone,joint}()`.
- `TokenStream::token_{alone,joint}()` are added.
- `TokenStream::delimited` is added.

This results in things like this:
```rust
TokenTree::token(token::Semi, stmt.span).into()
```
changing to this:
```rust
TokenStream::token_alone(token::Semi, stmt.span)
```
This makes the type of the result, and its spacing, clearer.

These changes also simplifies `Cursor` and `CursorRef`, because they no longer
need to distinguish between `next` and `next_with_spacing`.

r? `@petrochenkov`
Rollup of 5 pull requests

Successful merges:

 - rust-lang#99311 (change maybe_body_owned_by to take local def id)
 - rust-lang#99862 (Improve type mismatch w/ function signatures)
 - rust-lang#99895 (don't call type ascription "cast")
 - rust-lang#99900 (remove some manual hash stable impls)
 - rust-lang#99903 (Add diagnostic when using public instead of pub)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@rustbot
Copy link
Collaborator

rustbot commented Aug 11, 2022

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 11, 2022
@Manishearth
Copy link
Member

@bors r+ p=1

@bors
Copy link
Contributor

bors commented Aug 11, 2022

📌 Commit ea90d4b 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 Aug 11, 2022
@bors
Copy link
Contributor

bors commented Aug 11, 2022

⌛ Testing commit ea90d4b with merge 00c071d1229639228176e80837672e56f58f9215...

@bors
Copy link
Contributor

bors commented Aug 11, 2022

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 11, 2022
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@flip1995
Copy link
Member Author

 downloading https://static.rust-lang.org/dist/2022-07-21/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to static.rust-lang.org:443 

seems spurious

@bors retry

@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 Aug 11, 2022
@bors
Copy link
Contributor

bors commented Aug 12, 2022

⌛ Testing commit ea90d4b with merge b998821...

@bors
Copy link
Contributor

bors commented Aug 12, 2022

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 12, 2022
@bors bors merged commit b998821 into rust-lang:master Aug 12, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 12, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b998821): comparison url.

Instruction count

  • Primary benchmarks: ❌ relevant regressions found
  • Secondary benchmarks: ❌ relevant regression found
mean1 max count2
Regressions ❌
(primary)
0.3% 0.9% 6
Regressions ❌
(secondary)
0.2% 0.2% 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% 0.9% 6

Max RSS (memory usage)

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

Cycles

Results
  • Primary benchmarks: ❌ relevant regression found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions ❌
(primary)
4.3% 4.3% 1
Regressions ❌
(secondary)
7.3% 7.3% 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.7% -4.2% 3
All ❌✅ (primary) 4.3% 4.3% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Footnotes

  1. the arithmetic mean of the percent change 2

  2. number of relevant changes 2

@rustbot rustbot added the perf-regression Performance regression. label Aug 12, 2022
@Mark-Simulacrum Mark-Simulacrum removed the perf-regression Performance regression. label Aug 16, 2022
@Mark-Simulacrum
Copy link
Member

Untagging as a regression. This PR didn't change any benchmarked code (including e.g. for PGO purposes).

Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 29, 2022
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.