fix(items_after_statements): handle cfg_select! arms by saberoueslati · Pull Request #17508 · rust-lang/rust-clippy

saberoueslati · 2026-08-05T14:51:50Z

Fix false positives in items_after_statements for items inside cfg_select! arms by checking token nesting once per block.

fixes #17498

changelog: [items_after_statements]: handle cfg_select! arms without false positives

rustbot · 2026-08-05T14:51:54Z

Thanks for the pull request, and welcome!

You should hear from one of our reviewers after this PR gets at least 2 reviews from the community.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

github-actions · 2026-08-05T15:00:45Z

No changes for c4442e5

DanielEScherzer

community review: it seems that this results in false negative - the lintcheck report shows 2 removals that should be kept

View changes since this review

blyxyas · 2026-08-11T13:11:54Z

Documentation cut from the PR, but useful for reviewing:

cfg_select! splices the tokens of the selected arm into the enclosing block without applying
any expansion marker, so both the span and the syntax context of the resulting items are
indistinguishable from items written directly in the block.

This deliberately gives up on a few true positives, e.g. an item after a statement within the
same cfg_select! arm, or some_macro! { fn f() {} } in statement position, as a false
negative is preferable to a false positive here.

blyxyas

Hmm, seems that you're using Spans, what about using TyCtxt more? ^ↀᴥↀ^

View changes since this review

blyxyas · 2026-08-11T13:10:44Z

+/// `cfg_select!` splices the tokens of the selected arm into the enclosing block without applying
+/// any expansion marker, so both the span and the syntax context of the resulting items are
+/// indistinguishable from items written directly in the block.
+///
+/// This deliberately gives up on a few true positives, e.g. an item after a statement within the
+/// same `cfg_select!` arm, or `some_macro! { fn f() {} }` in statement position, as a false
+/// negative is preferable to a false positive here.


Thanks for documenting this function so much, but it might be a little too verbose 😅 , I'll paste the removed text into a PR comment.

Suggested change

/// `cfg_select!` splices the tokens of the selected arm into the enclosing block without applying

/// any expansion marker, so both the span and the syntax context of the resulting items are

/// indistinguishable from items written directly in the block.

///

/// This deliberately gives up on a few true positives, e.g. an item after a statement within the

/// same `cfg_select!` arm, or `some_macro! { fn f() {} }` in statement position, as a false

/// negative is preferable to a false positive here.

/// `cfg_select!` splices the tokens of the selected arm into the enclosing block without applying

/// any expansion marker, so both the span and the syntax context of the resulting items are

/// indistinguishable from items written directly in the block.

blyxyas · 2026-08-11T13:16:00Z

+    let mut direct_children = vec![false; item_spans.len()];
+    if item_spans.is_empty() {


This could be made a bit simpler :)

Suggested change

let mut direct_children = vec![false; item_spans.len()];

if item_spans.is_empty() {

if item_spans.is_empty() {

return vec![];

};

let mut direct_children = vec![false; item_spans.len()];

blyxyas · 2026-08-11T13:24:00Z

+    block.span.with_source_text(cx, |src| {
+        let mut item_offsets = item_spans
+            .iter()
+            .enumerate()


We don't really need the enumerate be so early, we can wait until later to enumerate this Vec

Done, the revised code no longer needs enumerate at all.

blyxyas · 2026-08-11T13:28:17Z

+    block.span.with_source_text(cx, |src| {
+        let mut item_offsets = item_spans
+            .iter()
+            .enumerate()
+            .filter(|(_, item_span)| item_span.lo() >= block.span.lo() && item_span.lo() <= block.span.hi())
+            .map(|(index, item_span)| (index, (item_span.lo() - block.span.lo()).to_usize()))
+            .filter(|&(_, offset)| offset <= src.len())
+            .collect::<Vec<_>>();
+        item_offsets.sort_unstable_by_key(|&(_, offset)| offset);
+
+        let mut depth = 0i32;
+        let mut offset = 0;
+        let mut next_item = 0;
+        for token in tokenize(src, FrontmatterAllowed::No) {
+            while let Some(&(index, item_offset)) = item_offsets.get(next_item)
+                && item_offset <= offset
+            {
+                direct_children[index] = depth == 1;
+                next_item += 1;
+            }
+
+            match token.kind {
+                TokenKind::OpenParen | TokenKind::OpenBrace | TokenKind::OpenBracket => depth += 1,
+                TokenKind::CloseParen | TokenKind::CloseBrace | TokenKind::CloseBracket => depth -= 1,
+                _ => {},
+            }
+            offset += token.len as usize;
+        }
+
+        while let Some(&(index, _)) = item_offsets.get(next_item) {
+            direct_children[index] = depth == 1;
+            next_item += 1;
+        }
+    });


Spans and source text is frail, it gets shifted, proc-macro'ed, replaced by include_str!, and all manners of other modifications. Is there any chance that we could be made stronger?

What about, instead of operating with spans, we query to tcx one of its parent functions? Like tcx.hir_parent_iter

I looked into this. cfg_select! splices the selected arm's tokens into the enclosing block during expansion, keeping the original spans and root syntax context, so by the time we reach HIR, the resulting items are indistinguishable direct children of that block.

I instrumented the lint to compare an item inside a cfg_select! arm with a regular item after a statement:

cfg_select item ctxt == block ctxt: true from_expansion: false outer_expn: Root
genuine item ctxt == block ctxt: true from_expansion: false outer_expn: Root

Their hir_parent_iter chains are identical too. I couldn't find a tcx query that separates them; if there's one I've missed, I'm happy to switch to it.

Given that, source nesting seems to be the only available signal. I've made that path fail open:

it now uses the full statement span instead of each leaf item's span; and

if the source text or spans can't be mapped, it keeps the lint rather than suppressing it.

The first change also restores the grouped-use diagnostics that lintcheck flagged as removed.

Known tradeoff: this still misses an item after a statement within the same cfg_select! arm. I'd rather take that false negative than the false positive.

rustbot · 2026-08-11T13:28:37Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

saberoueslati · 2026-08-11T17:43:34Z

@blyxyas I investigated the tcx/HIR route: cfg_select! fully splices the selected arm into the enclosing block, so its items have the same root syntax context and identical hir_parent_iter chain as regular items. I couldn't find a tcx query that distinguishes them.

I therefore kept source nesting as the discriminator, but made it fail open: it uses full statement spans, and unmappable source/spans retain the lint instead of suppressing it. This also restores the grouped-use diagnostics from lintcheck. I added the detailed findings to the inline thread.

saberoueslati · 2026-08-11T17:49:59Z

@DanielEScherzer Thank you, the two removals were the leaf imports in a grouped use statement. I now use the full statement span for nesting detection, which preserves those diagnostics, and added a UI regression test covering the list stem and both leaf imports.

blyxyas · 2026-08-12T11:38:08Z

I see, I'm not sure if this is even intended behavior. I think that this is a bug in cfg_select!'s behavior. Macros should update their SyntaxContext, and cfg_select is not doing that.

fix(items_after_statements): handle cfg_select! arms

7d94db5

rustbot added the S-waiting-on-community-reviews Status: This is awaiting for positive reviews from the community before a maintainer is assigned. label Aug 5, 2026

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 5, 2026

fixed CI/CD issue

c2858a0

DanielEScherzer suggested changes Aug 8, 2026

View reviewed changes

blyxyas requested changes Aug 11, 2026

View reviewed changes

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 Aug 11, 2026

adressed review comments

c4442e5

saberoueslati requested review from DanielEScherzer and blyxyas August 11, 2026 17:50

		let mut direct_children = vec![false; item_spans.len()];
		if item_spans.is_empty() {

Uh oh!

Uh oh!

Conversation

saberoueslati commented Aug 5, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 5, 2026

Uh oh!

github-actions Bot commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielEScherzer left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blyxyas commented Aug 11, 2026

Uh oh!

blyxyas left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Aug 11, 2026

Uh oh!

saberoueslati commented Aug 11, 2026

Uh oh!

saberoueslati commented Aug 11, 2026

Uh oh!

blyxyas commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

saberoueslati commented Aug 5, 2026 •

edited by rustbot

Loading

github-actions Bot commented Aug 5, 2026 •

edited

Loading

DanielEScherzer left a comment •

edited

Loading

blyxyas left a comment •

edited

Loading

blyxyas commented Aug 12, 2026 •

edited

Loading