Skip to content
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

make sure checked type implements Try trait when linting [question_mark] #12563

Merged
merged 1 commit into from Mar 30, 2024

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
Collaborator

bors commented Mar 30, 2024

📌 Commit 91f3fad has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Mar 30, 2024

⌛ Testing commit 91f3fad with merge e0e7ee1...

@bors
Copy link
Collaborator

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
5 checks passed
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
5 participants