Skip to content

Conversation

flip1995
Copy link
Member

bors and others added 30 commits November 3, 2021 17:21
…lip1995

Move if_then_panic to pedantic and rename to manual_assert
[beta] Backport: Move if_then_panic to pedantic and rename to manual_assert

Backport of rust-lang#7810

cc `@camsteffen`

r? `@ghost`

changelog: none (same release)
fix clippy format using `cargo fmt -p clippy_{lints,utils}`
manually revert rustfmt line truncations
rename to hir::Let in clippy
Undo the shadowing of various `expr` variables after renaming `scrutinee`
reduce destructuring of hir::Let to avoid `expr` collisions
cargo fmt -p clippy_{lints,utils}
bless new clippy::author output
By changing `as_str()` to take `&self` instead of `self`, we can just
return `&str`. We're still lying about lifetimes, but it's a smaller lie
than before, where `SymbolStr` contained a (fake) `&'static str`!
Beta branch remerge

r? `@ghost`

changelog: none
Co-authored-by: Takayuki Nakata <[email protected]>
Co-authored-by: Takayuki Nakata <[email protected]>
Update changelog

Apologies for the delay!

changelog: none
…g#7320

When inverting an expression, the output is now like ```foo != 0``` instead of ```!(foo == 0)```, the comparison operator is now replaced.
Fix commits and formatting of CHANGELOG.md

r? `@Manishearth`

Follow up to rust-lang#8136

I think the beta commit update didn't take the backport we've done into account. I fixed the commit ranges. And while I was at it, I also applied my usual formatting to the changelog entries.

changelog: none
Implement let-else type annotations natively

Tracking issue: rust-lang#87335

Fixes rust-lang#89688, fixes rust-lang#89807, edit: fixes  rust-lang#89960 as well

As explained in rust-lang#89688 (comment), the previous desugaring moved the let-else scrutinee into a dummy variable, which meant if you wanted to refer to it again in the else block, it had moved.

This introduces a new hir type, ~~`hir::LetExpr`~~ `hir::Let`, which takes over all the fields of `hir::ExprKind::Let(...)` and adds an optional type annotation. The `hir::Let` is then treated like a `hir::Local` when type checking a function body, specifically:

* `GatherLocalsVisitor` overrides a new `Visitor::visit_let_expr` and does pretty much exactly what it does for `visit_local`, assigning a local type to the `hir::Let` ~~(they could be deduplicated but they are right next to each other, so at least we know they're the same)~~
* It reuses the code in `check_decl_local` to typecheck the `hir::Let`, simply returning 'bool' for the expression type after doing that.

* ~~`FnCtxt::check_expr_let` passes this local type in to `demand_scrutinee_type`, and then imitates check_decl_local's pattern checking~~
* ~~`demand_scrutinee_type` (the blindest change for me, please give this extra scrutiny) uses this local type instead of of creating a new one~~
    * ~~Just realised the `check_expr_with_needs` was passing NoExpectation further down, need to pass the type there too. And apparently this Expectation API already exists.~~

Some other misc notes:

* ~~Is the clippy code supposed to be autoformatted? I tried not to give huge diffs but maybe some rustfmt changes simply haven't hit it yet.~~
* in `rustc_ast_lowering/src/block.rs`, I noticed some existing `self.alias_attrs()` calls in `LoweringContext::lower_stmts` seem to be copying attributes from the lowered locals/etc to the statements. Is that right? I'm new at this, I don't know.
…E, r=xFrednet

Ensure that RETURN_SELF_NOT_MUST_USE is not emitted if the method already has `#[must_use]`

Fixes rust-lang/rust-clippy#8140.

---

Edit:

changelog: none

(The lint is not in beta yet, this should therefore not be included inside the changelog :) )
Remove `SymbolStr`

This was originally proposed in rust-lang#74554 (comment). As well as removing the icky `SymbolStr` type, it allows the removal of a lot of `&` and `*` occurrences.

Best reviewed one commit at a time.

r? `@oli-obk`
Don't emit RETURN_SELF_NOT_MUST_USE lint if `Self` already is marked as `#[must_use]`

New bug discovered with this lint. Hopefully, this is the last one.

---

changelog: none
Fix `SAFETY` comment tag casing in undocumented_unsafe_blocks

This changes the lint introduced in rust-lang#7748 to suggest adding a `SAFETY` comment instead of a `Safety` comment.

Searching for `// Safety:` in rust-lang/rust yields 67 results while `// SAFETY:` yields 1072.
I think it's safe to say that this comment tag is written in upper case, just like `TODO`, `FIXME` and so on are. As such I would expect this lint to follow the official convention as well.

Note that I intentionally introduced some casing diversity in `tests/ui/undocumented_unsafe_blocks.rs` to test more cases than just `Safety:`.

changelog: Capitalize `SAFETY` comment in [`undocumented_unsafe_blocks`]
This makes sure that the tests in clippy_utils are run in CI.

When looking into this I discovered that two tests were failing and
multiple doc tests were failing. This fixes those tests and enables a
few more doc tests.
Test clippy_utils in CI

r? `@xFrednet` Since you did the last refactor of the `str_utils` functions in rust-lang#7873

changelog: Make sure tests in `clippy_utils` are passing by testing it in CI
@rust-highfive
Copy link
Contributor

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 Dec 30, 2021
@Manishearth
Copy link
Member

@bors r+ p=1

@bors
Copy link
Collaborator

bors commented Dec 30, 2021

📌 Commit 4bb6fde has been approved by Manishearth

@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 Dec 30, 2021
@bors
Copy link
Collaborator

bors commented Dec 30, 2021

⌛ Testing commit 4bb6fde with merge 0f4668c9b1ee7c31817228b4c478984c3b0253d1...

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

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

@bors
Copy link
Collaborator

bors commented Dec 30, 2021

💔 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 Dec 30, 2021
@matthiaskrgr
Copy link
Member

@bors retry 🤷 no longs shown

@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 Dec 30, 2021
@bors
Copy link
Collaborator

bors commented Dec 31, 2021

⌛ Testing commit 4bb6fde with merge 8baeddf...

@bors
Copy link
Collaborator

bors commented Dec 31, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 31, 2021
@bors bors merged commit 8baeddf into rust-lang:master Dec 31, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 31, 2021
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8baeddf): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

@rustbot label: -perf-regression

@xFrednet
Copy link
Contributor

cc @GuillaumeGomez FYI, your lint has been synced and should be available in the next nightly 🙃

@GuillaumeGomez
Copy link
Member

Awesome, thanks a lot!

@flip1995 flip1995 deleted the clippyup branch January 2, 2022 17: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.