Skip to content

[unused_async]: don't lint if function is part of a trait #11042

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

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

y21
Copy link
Member

@y21 y21 commented Jun 27, 2023

Fixes #10459.

We shouldn't lint if the function is part of a trait, because the user won't be able to easily remove the async, as this will then not match with the function signature in the trait definition

changelog: [unused_async]: don't lint if function is part of a trait

@rustbot
Copy link
Collaborator

rustbot commented Jun 27, 2023

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 27, 2023
/// Checks if the `def_id` belongs to a function and that function is part of a trait impl,
/// in which case we shouldn't lint because `async` is part of the trait definition and therefore
/// can't be removed.
fn is_in_trait_impl(cx: &LateContext<'_>, def_id: LocalDefId) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: this should be in utils somewhere. there's also a chance we already have it

(@Centri3 got opinions where?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we already have it, we have some for MethodCalls and such but not DefIds themselves

I think just the top-level clippy_utils is fine. The other is_trait_method function is there too. We could probably rename this to is_def_id_trait_method or something for consistency's sake.

@Manishearth
Copy link
Member

@bors r+

thanks!

@bors
Copy link
Contributor

bors commented Jun 28, 2023

📌 Commit b713cd5 has been approved by Manishearth

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 28, 2023

⌛ Testing commit b713cd5 with merge 750c7a1...

@bors
Copy link
Contributor

bors commented Jun 28, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 750c7a1 to master...

@bors bors merged commit 750c7a1 into rust-lang:master Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unused_async false positive with async_fn_in_trait
5 participants