diff --git a/clippy_lints/src/items_after_statements.rs b/clippy_lints/src/items_after_statements.rs index afb8863f1b94..0e5f9f12432f 100644 --- a/clippy_lints/src/items_after_statements.rs +++ b/clippy_lints/src/items_after_statements.rs @@ -1,7 +1,10 @@ use clippy_utils::diagnostics::span_lint_hir; +use clippy_utils::source::SpanExt as _; use rustc_hir::{Block, ItemKind, StmtKind}; +use rustc_lexer::{FrontmatterAllowed, TokenKind, tokenize}; use rustc_lint::{LateContext, LateLintPass, LintContext as _}; use rustc_session::declare_lint_pass; +use rustc_span::{Pos as _, Span}; declare_clippy_lint! { /// ### What it does @@ -49,25 +52,87 @@ declare_clippy_lint! { declare_lint_pass!(ItemsAfterStatements => [ITEMS_AFTER_STATEMENTS]); +/// `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. +fn are_direct_children_of_block(cx: &LateContext<'_>, block: &Block<'_>, stmt_spans: &[Span]) -> Vec { + if stmt_spans.is_empty() { + return vec![]; + } + let mut direct_children = vec![true; stmt_spans.len()]; + + // Only the block's own opening delimiter may be left open at a statement's position. Comments and + // string literals are single tokens, so delimiters inside them can't skew the count. + block.span.with_source_text(cx, |src| { + let Some(stmt_offsets) = stmt_spans + .iter() + .map(|stmt_span| { + (stmt_span.lo() >= block.span.lo() && stmt_span.lo() <= block.span.hi()) + .then(|| (stmt_span.lo() - block.span.lo()).to_usize()) + .filter(|&offset| offset <= src.len()) + }) + .collect::>>() + else { + return; + }; + if !stmt_offsets.is_sorted() { + return; + } + + let mut depth = 0i32; + let mut offset = 0; + let mut next_item = 0; + for token in tokenize(src, FrontmatterAllowed::No) { + while let Some(&stmt_offset) = stmt_offsets.get(next_item) + && stmt_offset <= offset + { + direct_children[next_item] = 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 stmt_offsets.get(next_item).is_some() { + direct_children[next_item] = depth == 1; + next_item += 1; + } + }); + + // If source text isn't available or the spans can't be mapped, preserve the lint. + direct_children +} + impl LateLintPass<'_> for ItemsAfterStatements { fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) { if block.stmts.len() > 1 { let ctxt = block.span.ctxt(); let mut in_external = None; - block + let items = block .stmts .iter() .skip_while(|stmt| matches!(stmt.kind, StmtKind::Item(..))) .filter_map(|stmt| match stmt.kind { - StmtKind::Item(id) => Some(cx.tcx.hir_item(id)), + StmtKind::Item(id) => Some((stmt.span, cx.tcx.hir_item(id))), _ => None, }) // Ignore macros since they can only see previously defined locals. - .filter(|item| !matches!(item.kind, ItemKind::Macro(..))) + .filter(|(_, item)| !matches!(item.kind, ItemKind::Macro(..))) // Stop linting if macros define items. - .take_while(|item| item.span.ctxt() == ctxt) - // Don't use `next` due to the complex filter chain. - .for_each(|item| { + .take_while(|(_, item)| item.span.ctxt() == ctxt) + .collect::>(); + let stmt_spans = items.iter().map(|(stmt_span, _)| *stmt_span).collect::>(); + let direct_children = are_direct_children_of_block(cx, block, &stmt_spans); + items + .into_iter() + .zip(direct_children) + .filter(|(_, direct)| *direct) + .for_each(|((_, item), _)| { // Only do the macro check once, but delay it until it's needed. if !*in_external.get_or_insert_with(|| block.span.in_external_macro(cx.sess().source_map())) { span_lint_hir( diff --git a/tests/ui/items_after_statement.rs b/tests/ui/items_after_statement.rs index a308d8d78934..60d50ec4de58 100644 --- a/tests/ui/items_after_statement.rs +++ b/tests/ui/items_after_statement.rs @@ -68,6 +68,47 @@ fn item_from_macro() { static_assert_size!(u32, 4); } +fn cfg_select_arm() { + let x = 1; + // `cfg_select!` splices the tokens of the arm into this block, without any expansion marker + std::cfg_select! { + true => { + use std::{cmp, mem}; + let _ = (x, cmp::max(0, 1), mem::size_of::()); + }, + } +} + +fn item_after_cfg_select() { + let x = 1; + std::cfg_select! { + true => { + use std::{cmp, mem}; + let _ = (x, cmp::max(0, 1), mem::size_of::()); + }, + } + fn foo() {} + //~^ items_after_statements +} + +fn grouped_use_after_statement() { + let x = 1; + use std::cmp::{max, min}; + //~^ items_after_statements + //~| items_after_statements + //~| items_after_statements + let _ = (x, max(0, 1), min(0, 1)); +} + +fn nested_block() { + let _ = 1; + { + let _ = 2; + fn foo() {} + //~^ items_after_statements + } +} + fn allow_attribute() { let _ = 1; #[allow(clippy::items_after_statements)] diff --git a/tests/ui/items_after_statement.stderr b/tests/ui/items_after_statement.stderr index 7a5ae2c8d47d..105ea1e5bb3b 100644 --- a/tests/ui/items_after_statement.stderr +++ b/tests/ui/items_after_statement.stderr @@ -35,5 +35,35 @@ LL | b!(); | = note: this error originates in the macro `b` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 3 previous errors +error: adding items after statements is confusing, since items exist from the start of the scope + --> tests/ui/items_after_statement.rs:90:5 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + +error: adding items after statements is confusing, since items exist from the start of the scope + --> tests/ui/items_after_statement.rs:96:5 + | +LL | use std::cmp::{max, min}; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: adding items after statements is confusing, since items exist from the start of the scope + --> tests/ui/items_after_statement.rs:96:20 + | +LL | use std::cmp::{max, min}; + | ^^^ + +error: adding items after statements is confusing, since items exist from the start of the scope + --> tests/ui/items_after_statement.rs:96:25 + | +LL | use std::cmp::{max, min}; + | ^^^ + +error: adding items after statements is confusing, since items exist from the start of the scope + --> tests/ui/items_after_statement.rs:107:9 + | +LL | fn foo() {} + | ^^^^^^^^^^^ + +error: aborting due to 8 previous errors