Skip to content

Conversation

@J-ZhengLi
Copy link
Member

@J-ZhengLi J-ZhengLi commented Mar 26, 2024

(indirectly) fixes: #12412 and fixes: #11983


changelog: make sure checked type implements Try trait when linting [question_mark]

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2024

r? @Alexendoo

rustbot has assigned @Alexendoo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 26, 2024
@J-ZhengLi J-ZhengLi marked this pull request as draft March 26, 2024 03:40
@J-ZhengLi J-ZhengLi force-pushed the issue11513 branch 2 times, most recently from 731985b to 765a52c Compare March 26, 2024 07:04
@J-ZhengLi J-ZhengLi changed the title ignore borrowed option/result when linting [question_mark] make sure checked type implements Try trait when linting [question_mark] Mar 26, 2024
@J-ZhengLi J-ZhengLi marked this pull request as ready for review March 26, 2024 07:15
Comment on lines +122 to +128
fn init_expr_can_use_question_mark(cx: &LateContext<'_>, init_expr: &Expr<'_>) -> bool {
let init_ty = cx.typeck_results().expr_ty_adjusted(init_expr);
cx.tcx
.lang_items()
.try_trait()
.map_or(false, |did| implements_trait(cx, init_ty, did, &[]))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this pattern come up often? I think I have seen something like this before. Perhaps we could have this in the utils mod.

Copy link
Member

Choose a reason for hiding this comment

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

cx.tcx.lang_items.something().map_or(false, |id| some_util(...)) comes up fairly often, some utils have different variants that take lang items or diagnostic names, maybe we could do something like

trait Definition {
    fn def_id(&self) -> Option<DefId>;
}

And impl it for DefId, LangItem, Symbol (for diagnostic names)

Then you could do implements_trait(..., LangItem::Try) or implements_trait(.., sym::Default). Could use it to get unify a good few utils, but things taking a trait is going to be slightly more difficult to understand

@Alexendoo
Copy link
Member

Thanks! @bors r+

@bors
Copy link
Contributor

bors commented Mar 30, 2024

📌 Commit 91f3fad has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 30, 2024

⌛ Testing commit 91f3fad with merge e0e7ee1...

@bors
Copy link
Contributor

bors commented Mar 30, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing e0e7ee1 to master...

@bors bors merged commit e0e7ee1 into rust-lang:master Mar 30, 2024
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.

question_mark lint gives invalid suggestion on nightly Incorrect suggestion made by question_mark when match ergonomics involved

5 participants