Add notable_trait predicate to CompletionRelevance#16274
Conversation
implement the predicate set on the case function from traits
6fdf60c to
df5c647
Compare
Veykril
left a comment
There was a problem hiding this comment.
Let's add a test to crates\ide-completion\src\render.rs that checks that the relevance fetching works
| .map_or(false, |trait_| completion.is_ops_trait(trait_)); | ||
|
|
||
| let is_item_from_notable_trait = func | ||
| .as_assoc_item(ctx.db()) |
There was a problem hiding this comment.
Let's bind the as_assoc_item result to var and re-use that as multiple things in this function are interested in it (and the call is not completely free as it accesses the database)
There was a problem hiding this comment.
Is it okay, if bind the result of the and_then as well, since seems like that's call to the db as well? I assume both methods (containing_trait and containing_trait_or_trait_impl) return the needed Trait.
Also is ctx.db() same as the db in the parameter of the render function?
There was a problem hiding this comment.
The db is the same ye. You can (and actually want) to use containing_trait_or_trait_impl for both here, I missed that actually (since the attribuite is only on the trait, not trait impls).
There was a problem hiding this comment.
I thought the same too but just now saw the implementation and figured to ask the question.
|
My bad that I didn't add tests. If there any more checks to be made on tests, please suggest them. I will add it. |
No worries, I tend to forget to add tests myself from time to time. That one should suffice I think. Thanks! |
|
☀️ Test successful - checks-actions |
Given a score of 1 for now, will change as per reviews needed.