Skip to content

New by_ref_peekable_peek lint#17042

Open
samueltardieu wants to merge 1 commit into
rust-lang:masterfrom
samueltardieu:lints/by_ref_peekable_peek
Open

New by_ref_peekable_peek lint#17042
samueltardieu wants to merge 1 commit into
rust-lang:masterfrom
samueltardieu:lints/by_ref_peekable_peek

Conversation

@samueltardieu
Copy link
Copy Markdown
Member

@samueltardieu samueltardieu commented May 20, 2026

changelog: [by_ref_peekable_peek]: new lint

Closes #17020

@rustbot rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 20, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 20, 2026

r? @Jarcho

rustbot has assigned @Jarcho.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: 7 candidates
  • 7 candidates expanded to 7 candidates
  • Random selection from Jarcho, llogiq

@samueltardieu samueltardieu force-pushed the lints/by_ref_peekable_peek branch from 274ead3 to bad070b Compare May 20, 2026 15:36
use super::BY_REF_PEEKABLE_PEEK;

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>) {
if let Some(recvs) = method_chain_args(recv, &[sym::by_ref, sym::peekable])
Copy link
Copy Markdown
Contributor

@Jarcho Jarcho May 21, 2026

Choose a reason for hiding this comment

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

Can we not use method_chain_args. It's only mildly shorter to write, but it's notably slower and does a hidden from_expansion check on everything.

View changes since the review

Comment on lines +12 to +16
&& cx
.typeck_results()
.expr_ty(recv)
.peel_refs()
.is_diag_item(cx, sym::IterPeekable)
Copy link
Copy Markdown
Contributor

@Jarcho Jarcho May 21, 2026

Choose a reason for hiding this comment

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

This doesn't check that we called the correct peekable function.

View changes since the review

Comment on lines +19 to +20
&& let Some(def_id) = cx.typeck_results().type_dependent_def_id(recvs[1].0.hir_id)
&& paths::ITERATOR_BY_REF.matches(cx, def_id)
Copy link
Copy Markdown
Contributor

@Jarcho Jarcho May 21, 2026

Choose a reason for hiding this comment

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

cx.ty_based_def(recvs[1].0).opt_parent().is_diag_item(sym::Iterator)

View changes since the review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels May 21, 2026
@Jarcho
Copy link
Copy Markdown
Contributor

Jarcho commented May 21, 2026

@rustbot label lint-nominated

@Jarcho Jarcho closed this May 21, 2026
@rustbot rustbot added the lint-nominated Create an FCP-thread on Zulip for this PR label May 21, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 21, 2026

This lint has been nominated for inclusion.

A FCP topic has been created on Zulip.

@Jarcho Jarcho reopened this May 21, 2026
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 21, 2026
@samueltardieu samueltardieu force-pushed the lints/by_ref_peekable_peek branch from bad070b to 1eb259c Compare May 22, 2026 08:06
@samueltardieu
Copy link
Copy Markdown
Member Author

I've changed the code to not use method_chain_args, and have it check that method calls to .by_ref() and .peekable() are both Iterator's trait methods.

@samueltardieu samueltardieu requested a review from Jarcho May 22, 2026 08:07
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label May 22, 2026
@samueltardieu samueltardieu force-pushed the lints/by_ref_peekable_peek branch from 1eb259c to c6e0832 Compare May 22, 2026 08:11
@samueltardieu samueltardieu force-pushed the lints/by_ref_peekable_peek branch from c6e0832 to 59f63dd Compare May 23, 2026 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lint-nominated Create an FCP-thread on Zulip for this PR needs-fcp PRs that add, remove, or rename lints and need an FCP 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.

Incorrect usage by_ref peekable peek

3 participants