-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix: async trait method for unnecessary_async
#13508
Conversation
// Do nothing if the method is an async member of trait. | ||
if let Some(fname) = function.name() { | ||
if let Some(trait_item) = find_corresponding_trait_member(ctx, fname.to_string()) { | ||
if let AssocItem::Function(method) = trait_item { | ||
if method.is_async(ctx.db()) { | ||
return None; | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can simplify this a bit via
// Check if this node is contained in a trait, second ancestors: ast::Fn -> ast::AssocItemList -> ast::Trait
if let Some(_) = function.syntax().ancestors().nth(2).and_then(ast::Trait::cast) {
return None
}
We basically check the ancestor nodes of the function, take the second ancestor which is the Trait it is contained in (if it is a trait function), and if that is indeed the case we are done here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Veykril Thank you for review! I see, it make sense.
I tried as you suggested but found ast::Trait
only matches trait definitions.
(the ancestor of a method in a trait implementation is ast::Impl
, which is indistinguishable from impl
for struct.)
So I think we would need at least the following procedure, right?
if let Some(impl_) = ctx.find_node_at_offset::<ast::Impl>() {
if let Some(_) = resolve_target_trait(&ctx.sema, &impl_) {
return None;
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, we can do it with the following:
if let Some(impl_) = function.syntax().ancestors().nth(2).and_then(ast::Impl::cast) {
if let Some(_) = impl_.trait_() {
return None;
}
}
fix: remove unused import
@bors r+ |
☀️ Test successful - checks-actions |
Fix #13492