From 9c5796d4a0fc73d5d29a001e77e8c1a6204256ef Mon Sep 17 00:00:00 2001 From: Charlie Chang Date: Sat, 7 Mar 2026 23:10:11 +0800 Subject: [PATCH 1/4] Add `undocumented_as_casts` lint to enforce comments on `as` casts --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/undocumented_as_casts.rs | 137 ++++++++++++ tests/ui/undocumented_as_casts.rs | 256 ++++++++++++++++++++++ tests/ui/undocumented_as_casts.stderr | 129 +++++++++++ 6 files changed, 526 insertions(+) create mode 100644 clippy_lints/src/undocumented_as_casts.rs create mode 100644 tests/ui/undocumented_as_casts.rs create mode 100644 tests/ui/undocumented_as_casts.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 45bfb65b2e67..21b66820dda8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7197,6 +7197,7 @@ Released 2018-09-13 [`unchecked_duration_subtraction`]: https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_subtraction [`unchecked_time_subtraction`]: https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_time_subtraction [`unconditional_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#unconditional_recursion +[`undocumented_as_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_as_casts [`undocumented_unsafe_blocks`]: https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks [`undropped_manually_drops`]: https://rust-lang.github.io/rust-clippy/master/index.html#undropped_manually_drops [`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 441b907eaf2f..8b50287648a6 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -751,6 +751,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[ crate::types::TYPE_COMPLEXITY_INFO, crate::types::VEC_BOX_INFO, crate::unconditional_recursion::UNCONDITIONAL_RECURSION_INFO, + crate::undocumented_as_casts::UNDOCUMENTED_AS_CASTS_INFO, crate::undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS_INFO, crate::undocumented_unsafe_blocks::UNNECESSARY_SAFETY_COMMENT_INFO, crate::unicode::INVISIBLE_CHARACTERS_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 950fcf0db0a4..78ae722428c4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -361,6 +361,7 @@ mod transmute; mod tuple_array_conversions; mod types; mod unconditional_recursion; +mod undocumented_as_casts; mod undocumented_unsafe_blocks; mod unicode; mod uninhabited_references; @@ -866,6 +867,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))), Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)), Box::new(move |_| Box::new(manual_pop_if::ManualPopIf::new(conf))), + Box::new(|_| Box::new(undocumented_as_casts::UndocumentedAsCasts)), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/clippy_lints/src/undocumented_as_casts.rs b/clippy_lints/src/undocumented_as_casts.rs new file mode 100644 index 000000000000..6ae180eaa7fd --- /dev/null +++ b/clippy_lints/src/undocumented_as_casts.rs @@ -0,0 +1,137 @@ +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::is_from_proc_macro; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_session::declare_lint_pass; +use rustc_span::{Pos, Span}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `as` casts that do not have a preceding `// CAST:` comment. + /// + /// ### Why is this bad? + /// `as` casts are powerful and potentially dangerous. Requiring a documentation comment + /// ensures the developer has considered the safety and necessity of the conversion. + /// + /// ### Example + /// ```no_run + /// let x = y as u32; + /// ``` + /// Use instead: + /// ```no_run + /// // CAST: reason for the cast + /// let x = y as u32; + /// + /// /* CAST: reason for the cast */ + /// let x = y as u32; + /// ``` + #[clippy::version = "1.96.0"] + pub UNDOCUMENTED_AS_CASTS, + nursery, + "`as` casts without a `CAST:` explanation" +} + +declare_lint_pass!(UndocumentedAsCasts => [UNDOCUMENTED_AS_CASTS]); + +impl<'tcx> LateLintPass<'tcx> for UndocumentedAsCasts { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + if let ExprKind::Cast(_, _) = expr.kind + && !expr.span.in_external_macro(cx.sess().source_map()) + && !is_from_proc_macro(cx, expr) + && !has_preceding_cast_comment(cx, expr.span) + { + #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")] + span_lint_and_then( + cx, + UNDOCUMENTED_AS_CASTS, + expr.span, + "`as` casts without a `// CAST:` explanation", + |diag| { + diag.help("consider adding a cast comment on the preceding line"); + }, + ); + } + } +} + +/// Checks if there is a `// CAST:` or `/* CAST:` comment preceding the cast expression. +fn has_preceding_cast_comment(cx: &LateContext<'_>, span: Span) -> bool { + let source_map = cx.sess().source_map(); + + // Try to get the file and line information + let Ok(line_info) = source_map.lookup_line(span.lo()) else { + return false; + }; + + let Some(src) = line_info.sf.src.as_deref() else { + return false; + }; + + let lines = line_info.sf.lines(); + let mut block_comment_end_idx = None; + let mut found_line_comment = false; + + // Find the preceding lines that start with `//` until hitting a non-comment line + for line_idx in (0..line_info.line).rev() { + let start = lines[line_idx].to_usize(); + let end = if line_idx + 1 < lines.len() { + lines[line_idx + 1].to_usize() + } else { + src.len() + }; + + if let Some(prev_text) = src.get(start..end) { + // Check line comment + let trimmed = prev_text.trim_start(); + + if let Some(description) = trimmed.strip_prefix("//") { + found_line_comment = true; + if description.trim().to_ascii_uppercase().starts_with("CAST:") { + return true; + } + // If it's a comment but not `CAST:`, continue checking previous lines + continue; + } + + // Stop if already found line comments but the current line is not a line comment + if found_line_comment { + break; + } + + // Stop if hit non-empty line, but keep track of the end of block comment + if !trimmed.is_empty() { + if trimmed.trim_end().ends_with("*/") { + block_comment_end_idx = Some(line_idx); + } + break; + } + } + } + + // Find `CAST:` in block comments if found the end of a block comment until hitting the start of + // the block comment + if let Some(line_end_idx) = block_comment_end_idx { + for line_idx in (0..=line_end_idx).rev() { + let start = lines[line_idx].to_usize(); + let end = if line_idx + 1 < lines.len() { + lines[line_idx + 1].to_usize() + } else { + src.len() + }; + if let Some(prev_text) = src.get(start..end) { + let trimmed = prev_text.trim_start(); + + if trimmed.to_ascii_uppercase().contains("CAST:") { + return true; + } + + // Stop if hit the start of the block comment + if trimmed.starts_with("/*") { + break; + } + } + } + } + + false +} diff --git a/tests/ui/undocumented_as_casts.rs b/tests/ui/undocumented_as_casts.rs new file mode 100644 index 000000000000..bdb486cc791d --- /dev/null +++ b/tests/ui/undocumented_as_casts.rs @@ -0,0 +1,256 @@ +//@aux-build:proc_macros.rs + +#![warn(clippy::undocumented_as_casts)] + +extern crate proc_macros; +use proc_macros::{external, with_span}; + +fn ignore_external() { + external!(0u32 as u64); // Should not lint +} + +fn ignore_proc_macro() { + with_span!( + span + + fn converting() { + let x = 0u32 as u64; // Should not lint + } + ); +} + +// Valid Comments + +fn line_comment() { + // CAST: reason + let _ = 0u32 as u64; +} + +fn line_comment_newlines() { + // CAST: reason + + let _ = 0u32 as u64; +} + +fn line_comment_empty() { + // CAST: reason + // + // + // + let _ = 0u32 as u64; +} + +fn line_comment_with_extras() { + // This is a description + // CAST: reason + let _ = 0u32 as u64; +} + +fn line_comment_multiple_casts_same_line() { + // CAST: reason for both casts + let _ = 0u32 as u64 + 1u16 as u64; +} + +fn line_comment_multiple_casts() { + // CAST: reason for first cast + let x = 0u32 as u64; + // CAST: reason for second cast + let y = 0u8 as u64; +} + +fn line_comment_function_return() -> u64 { + // CAST: reason + 0u32 as u64 +} + +fn line_comment_match_block_multiple_arms() { + let x = 0u32; + match x { + 0 => { + // CAST: reason for first cast + let _ = x as u64; + }, + _ => { + // CAST: reason for second cast + let _ = x as u64; + }, + } +} + +fn line_comment_let_match_block_multiple_arms() { + let x = 0u32; + let y = match x { + 0 => { + // CAST: reason for first cast + x as u64 + }, + _ => { + // CAST: reason for second cast + x as u64 + }, + }; +} + +fn block_comment() { + /* CAST: reason */ + let _ = 0u32 as u64; +} + +fn block_comment_newlines() { + /* CAST: reason */ + + let _ = 0u32 as u64; +} + +fn block_comment_multiple_line() { + /* This is a description + * CAST: reason + */ + let _ = 0u32 as u64; +} + +fn block_comment_multiple_casts_same_line() { + /* CAST: reason for both casts */ + let _ = 0u32 as u64 + 1u16 as u64; +} + +fn block_comment_multiple_casts() { + /* CAST: reason for first cast */ + let x = 0u32 as u64; + /* CAST: reason for second cast */ + let y = 0u8 as u64; +} + +fn block_comment_function_return() -> u64 { + /* CAST: reason */ + 0u32 as u64 +} + +// Invalid Comment + +fn no_comment() { + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn trailing_cast_comment() { + let _ = 0u32 as u64; // CAST: reason + // + //~^^ undocumented_as_casts +} + +fn non_cast_comment() { + // This is a description + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn non_cast_comment_newlines() { + // This is a description + + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn non_cast_comment_with_extras() { + // This is a description + // This is more description + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn non_cast_block_comment() { + /* This is a description */ + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn non_cast_block_comment_newlines() { + /* This is a description */ + + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn non_cast_block_comment_with_extras() { + /* This is a description + * This is more description */ + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn no_comment_first_cast() { + let x = 0u32 as u64; + //~^ undocumented_as_casts + + // CAST: reason + let y = 1u32 as u64; +} + +fn no_comment_following_cast() { + // CAST: reason + let x = 0u32 as u64; + + let y = 1u32 as u64; + //~^ undocumented_as_casts +} + +fn line_cast_comment_before_block_comment() { + // CAST: reason + /* This is a description */ + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn block_cast_comment_before_line_comment() { + /* CAST: reason */ + // This is a description + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn block_cast_comment_before_block_comment() { + /* CAST: reason */ + /* This is a description */ + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn newline_between_cast_comment_and_line_comment() { + // CAST: reason + + // This is a description + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +// Need Improved Detection + +fn line_comment_let_match_then_cast() { + let x = 0u32; + // CAST: reason for match block cast + let y = match x { + 0 => { + // CAST: reason for first cast + x as u64 + }, + _ => { + // CAST: reason for second cast + x as u64 + }, + } as usize; + + let x = 0u32; + let y = match x { + //~^ undocumented_as_casts + 0 => { + // CAST: reason for first cast + x as u64 + }, + _ => { + // CAST: reason for second cast + x as u64 + }, + // CAST: reason for final cast + } as usize; +} diff --git a/tests/ui/undocumented_as_casts.stderr b/tests/ui/undocumented_as_casts.stderr new file mode 100644 index 000000000000..b75983d003b5 --- /dev/null +++ b/tests/ui/undocumented_as_casts.stderr @@ -0,0 +1,129 @@ +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:132:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + = note: `-D clippy::undocumented-as-casts` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::undocumented_as_casts)]` + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:137:13 + | +LL | let _ = 0u32 as u64; // CAST: reason + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:144:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:151:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:158:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:164:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:171:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:178:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:183:13 + | +LL | let x = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:194:13 + | +LL | let y = 1u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:201:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:208:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:215:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:223:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:244:13 + | +LL | let y = match x { + | _____________^ +LL | | +LL | | 0 => { +... | +LL | | } as usize; + | |______________^ + | + = help: consider adding a cast comment on the preceding line + +error: aborting due to 15 previous errors + From 27a4bddf8eb4272adfd7a18ae5da2c04cea26346 Mon Sep 17 00:00:00 2001 From: Charlie Chang Date: Sat, 7 Mar 2026 23:53:42 +0800 Subject: [PATCH 2/4] Fix doctest fail on example code --- clippy_lints/src/undocumented_as_casts.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/undocumented_as_casts.rs b/clippy_lints/src/undocumented_as_casts.rs index 6ae180eaa7fd..91338e571090 100644 --- a/clippy_lints/src/undocumented_as_casts.rs +++ b/clippy_lints/src/undocumented_as_casts.rs @@ -14,16 +14,16 @@ declare_clippy_lint! { /// ensures the developer has considered the safety and necessity of the conversion. /// /// ### Example - /// ```no_run - /// let x = y as u32; + /// ```rust,ignore + /// let x = 0u32 as usize; /// ``` /// Use instead: - /// ```no_run + /// ```rust,ignore /// // CAST: reason for the cast - /// let x = y as u32; + /// let x = 0u32 as usize; /// /// /* CAST: reason for the cast */ - /// let x = y as u32; + /// let y = 1u32 as usize; /// ``` #[clippy::version = "1.96.0"] pub UNDOCUMENTED_AS_CASTS, From 882856d08690e65428d463c3366b797f52cdd43f Mon Sep 17 00:00:00 2001 From: Charlie Chang Date: Wed, 11 Mar 2026 22:30:37 +0800 Subject: [PATCH 3/4] Share check marked comment logic with `undocumented_unsafe_blocks`, add test case for declared macro --- clippy_lints/src/undocumented_as_casts.rs | 108 ++----------- .../src/undocumented_unsafe_blocks.rs | 146 ++++-------------- clippy_utils/src/source.rs | 106 +++++++++++++ tests/ui/undocumented_as_casts.rs | 143 ++++++++++++----- tests/ui/undocumented_as_casts.stderr | 78 ++++++---- 5 files changed, 302 insertions(+), 279 deletions(-) diff --git a/clippy_lints/src/undocumented_as_casts.rs b/clippy_lints/src/undocumented_as_casts.rs index 91338e571090..b6a8264b04c8 100644 --- a/clippy_lints/src/undocumented_as_casts.rs +++ b/clippy_lints/src/undocumented_as_casts.rs @@ -1,9 +1,9 @@ -use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::is_from_proc_macro; +use clippy_utils::source::text_has_marked_comment; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::declare_lint_pass; -use rustc_span::{Pos, Span}; declare_clippy_lint! { /// ### What it does @@ -27,7 +27,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.96.0"] pub UNDOCUMENTED_AS_CASTS, - nursery, + restriction, "`as` casts without a `CAST:` explanation" } @@ -35,103 +35,29 @@ declare_lint_pass!(UndocumentedAsCasts => [UNDOCUMENTED_AS_CASTS]); impl<'tcx> LateLintPass<'tcx> for UndocumentedAsCasts { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { + let source_map = cx.sess().source_map(); if let ExprKind::Cast(_, _) = expr.kind && !expr.span.in_external_macro(cx.sess().source_map()) && !is_from_proc_macro(cx, expr) - && !has_preceding_cast_comment(cx, expr.span) + && let Ok(line_info) = source_map.lookup_line(expr.span.lo()) + && let Some(src) = line_info.sf.src.as_deref() + && text_has_marked_comment( + src, + &line_info.sf.lines()[..=line_info.line], + line_info.sf.start_pos, + "CAST:", + false, + ) + .is_none() { - #[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")] - span_lint_and_then( + span_lint_and_help( cx, UNDOCUMENTED_AS_CASTS, expr.span, "`as` casts without a `// CAST:` explanation", - |diag| { - diag.help("consider adding a cast comment on the preceding line"); - }, + None, + "consider adding a cast comment on the preceding line", ); } } } - -/// Checks if there is a `// CAST:` or `/* CAST:` comment preceding the cast expression. -fn has_preceding_cast_comment(cx: &LateContext<'_>, span: Span) -> bool { - let source_map = cx.sess().source_map(); - - // Try to get the file and line information - let Ok(line_info) = source_map.lookup_line(span.lo()) else { - return false; - }; - - let Some(src) = line_info.sf.src.as_deref() else { - return false; - }; - - let lines = line_info.sf.lines(); - let mut block_comment_end_idx = None; - let mut found_line_comment = false; - - // Find the preceding lines that start with `//` until hitting a non-comment line - for line_idx in (0..line_info.line).rev() { - let start = lines[line_idx].to_usize(); - let end = if line_idx + 1 < lines.len() { - lines[line_idx + 1].to_usize() - } else { - src.len() - }; - - if let Some(prev_text) = src.get(start..end) { - // Check line comment - let trimmed = prev_text.trim_start(); - - if let Some(description) = trimmed.strip_prefix("//") { - found_line_comment = true; - if description.trim().to_ascii_uppercase().starts_with("CAST:") { - return true; - } - // If it's a comment but not `CAST:`, continue checking previous lines - continue; - } - - // Stop if already found line comments but the current line is not a line comment - if found_line_comment { - break; - } - - // Stop if hit non-empty line, but keep track of the end of block comment - if !trimmed.is_empty() { - if trimmed.trim_end().ends_with("*/") { - block_comment_end_idx = Some(line_idx); - } - break; - } - } - } - - // Find `CAST:` in block comments if found the end of a block comment until hitting the start of - // the block comment - if let Some(line_end_idx) = block_comment_end_idx { - for line_idx in (0..=line_end_idx).rev() { - let start = lines[line_idx].to_usize(); - let end = if line_idx + 1 < lines.len() { - lines[line_idx + 1].to_usize() - } else { - src.len() - }; - if let Some(prev_text) = src.get(start..end) { - let trimmed = prev_text.trim_start(); - - if trimmed.to_ascii_uppercase().contains("CAST:") { - return true; - } - - // Stop if hit the start of the block comment - if trimmed.starts_with("/*") { - break; - } - } - } - } - - false -} diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 2bf1d8be4653..3fa8754b55d5 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -5,15 +5,14 @@ use clippy_config::Conf; use clippy_utils::consts::const_item_rhs_to_expr; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_lint_allowed; -use clippy_utils::source::walk_span_to_context; +use clippy_utils::source::{text_has_marked_comment, walk_span_to_context}; use clippy_utils::visitors::{Descend, for_each_expr}; use hir::HirId; use rustc_errors::Applicability; use rustc_hir::{self as hir, Block, BlockCheckMode, FnSig, Impl, ItemKind, Node, UnsafeSource}; -use rustc_lexer::{FrontmatterAllowed, TokenKind, tokenize}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::impl_lint_pass; -use rustc_span::{BytePos, Pos, RelativeBytePos, Span, SyntaxContext}; +use rustc_span::{BytePos, Pos, Span, SyntaxContext}; declare_clippy_lint! { /// ### What it does @@ -589,13 +588,17 @@ fn item_has_safety_comment( return if comment_start_line.line >= unsafe_line.line { HasSafetyComment::No } else { - text_has_safety_comment( + match text_has_marked_comment( src, &unsafe_line.sf.lines() [(comment_start_line.line + usize::from(!include_first_line_of_file))..=unsafe_line.line], unsafe_line.sf.start_pos, + "SAFETY:", accept_comment_above_attributes, - ) + ) { + Some((b, d)) => HasSafetyComment::Yes(b, d), + None => HasSafetyComment::No, + } }; } HasSafetyComment::Maybe @@ -632,12 +635,16 @@ fn stmt_has_safety_comment( return if comment_start_line.line >= unsafe_line.line { HasSafetyComment::No } else { - text_has_safety_comment( + match text_has_marked_comment( src, &unsafe_line.sf.lines()[comment_start_line.line + 1..=unsafe_line.line], unsafe_line.sf.start_pos, + "SAFETY:", accept_comment_above_attributes, - ) + ) { + Some((b, d)) => HasSafetyComment::Yes(b, d), + None => HasSafetyComment::No, + } }; } HasSafetyComment::Maybe @@ -710,12 +717,16 @@ fn span_from_macro_expansion_has_safety_comment( && let Some(src) = unsafe_line.sf.src.as_deref() { if macro_line.line < unsafe_line.line { - text_has_safety_comment( + match text_has_marked_comment( src, &unsafe_line.sf.lines()[macro_line.line + 1..=unsafe_line.line], unsafe_line.sf.start_pos, + "SAFETY:", accept_comment_above_attributes, - ) + ) { + Some((b, d)) => HasSafetyComment::Yes(b, d), + None => HasSafetyComment::No, + } } else { HasSafetyComment::No } @@ -774,15 +785,14 @@ fn span_has_safety_comment(cx: &LateContext<'_>, span: Span, accept_comment_abov // fn foo() { some_stuff; unsafe { stuff }; other_stuff; } // ^-------------^ body_line.line < unsafe_line.line - && matches!( - text_has_safety_comment( - src, - &unsafe_line.sf.lines()[body_line.line + 1..=unsafe_line.line], - unsafe_line.sf.start_pos, - accept_comment_above_attributes, - ), - HasSafetyComment::Yes(..) + && text_has_marked_comment( + src, + &unsafe_line.sf.lines()[body_line.line + 1..=unsafe_line.line], + unsafe_line.sf.start_pos, + "SAFETY:", + accept_comment_above_attributes, ) + .is_some() } else { // Problem getting source text. Pretend a comment was found. true @@ -792,108 +802,6 @@ fn span_has_safety_comment(cx: &LateContext<'_>, span: Span, accept_comment_abov } } -/// Checks if the given text has a safety comment for the immediately proceeding line. -/// -/// If `accept_comment_above_attributes` is true, it will ignore attributes inbetween blocks of -/// comments -fn text_has_safety_comment( - src: &str, - line_starts: &[RelativeBytePos], - start_pos: BytePos, - accept_comment_above_attributes: bool, -) -> HasSafetyComment { - let mut lines = line_starts - .array_windows::<2>() - .rev() - .map_while(|[start, end]| { - let start = start.to_usize(); - let end = end.to_usize(); - let text = src.get(start..end)?; - let trimmed = text.trim_start(); - Some((start + (text.len() - trimmed.len()), trimmed)) - }) - .filter(|(_, text)| !(text.is_empty() || (accept_comment_above_attributes && is_attribute(text)))); - - let Some((line_start, line)) = lines.next() else { - return HasSafetyComment::No; - }; - - let mut in_codeblock = false; - // Check for a sequence of line comments. - if line.starts_with("//") { - let (mut line, mut line_start) = (line, line_start); - loop { - // Don't lint if the safety comment is part of a codeblock in a doc comment. - // It may or may not be required, and we can't very easily check it (and we shouldn't, since - // the safety comment isn't referring to the node we're currently checking) - if let Some(doc) = line.strip_prefix("///").or_else(|| line.strip_prefix("//!")) - && doc.trim_start().starts_with("```") - { - in_codeblock = !in_codeblock; - } - - if !in_codeblock && let Some(safety_pos) = line.to_ascii_uppercase().find("SAFETY:") { - return HasSafetyComment::Yes( - start_pos - + BytePos(u32::try_from(line_start).unwrap()) - + BytePos(u32::try_from(safety_pos).unwrap()), - line.starts_with("///"), - ); - } - match lines.next() { - Some((s, x)) if x.starts_with("//") => (line, line_start) = (x, s), - _ => return HasSafetyComment::No, - } - } - } - // Check for a comment that appears after other code on the same line (e.g., `let x = // SAFETY:`) - // This handles cases in macros where the comment is on the same line as preceding code. - // We only check the first (immediate preceding) line for this pattern. - // Only whitespace is allowed between the comment marker and `SAFETY:`. - if let Some(comment_start) = [line.find("//"), line.find("/*")].into_iter().flatten().min() - && let after_marker = &line[comment_start + 2..] // skip marker - && let trimmed = after_marker.trim_start() // skip whitespace - && trimmed.get(..7).is_some_and(|s| s.eq_ignore_ascii_case("SAFETY:")) - { - let safety_offset = 2 + (after_marker.len() - trimmed.len()); - return HasSafetyComment::Yes( - start_pos - + BytePos(u32::try_from(line_start).unwrap()) - + BytePos(u32::try_from(comment_start + safety_offset).unwrap()), - false, - ); - } - // No line comments; look for the start of a block comment. - // This will only find them if they are at the start of a line. - let (mut line_start, mut line) = (line_start, line); - loop { - if line.starts_with("/*") { - let src = &src[line_start..line_starts.last().unwrap().to_usize()]; - let mut tokens = tokenize(src, FrontmatterAllowed::No); - let a = tokens.next(); - if let Some(safety_pos) = src[..a.unwrap().len as usize].to_ascii_uppercase().find("SAFETY:") - && tokens.all(|t| t.kind == TokenKind::Whitespace) - { - return HasSafetyComment::Yes( - start_pos - + BytePos(u32::try_from(line_start).unwrap()) - + BytePos(u32::try_from(safety_pos).unwrap()), - line.starts_with("/**"), - ); - } - return HasSafetyComment::No; - } - match lines.next() { - Some(x) => (line_start, line) = x, - None => return HasSafetyComment::No, - } - } -} - -fn is_attribute(text: &str) -> bool { - (text.starts_with("#[") || text.starts_with("#![")) && text.trim_end().ends_with(']') -} - fn span_and_hid_of_item_alike_node(node: &Node<'_>) -> Option<(Span, HirId)> { match node { Node::Item(item) => Some((item.span, item.owner_id.into())), diff --git a/clippy_utils/src/source.rs b/clippy_utils/src/source.rs index f30f26f3a70b..52aae7bc9fdb 100644 --- a/clippy_utils/src/source.rs +++ b/clippy_utils/src/source.rs @@ -803,6 +803,112 @@ pub fn str_literal_to_char_literal( } } +/// Checks if the given text contains a comment with the given marker (e.g., `SAFETY:`) +/// that applies to the given position. If so, returns the position of the comment and +/// whether it's a doc comment. +/// +/// If `accept_comment_above_attributes` is true, it will ignore attributes inbetween +/// blocks of comments +pub fn text_has_marked_comment( + src: &str, + line_starts: &[RelativeBytePos], + start_pos: BytePos, + comment_marker: &str, + accept_comment_above_attributes: bool, +) -> Option<(BytePos, bool)> { + let mut lines = line_starts + .array_windows::<2>() + .rev() + .map_while(|[start, end]| { + let start = start.to_usize(); + let end = end.to_usize(); + let text = src.get(start..end)?; + let trimmed = text.trim_start(); + Some((start + (text.len() - trimmed.len()), trimmed)) + }) + .filter(|(_, text)| { + !(text.is_empty() + || (accept_comment_above_attributes + // is the line an attribute + && ((text.starts_with("#[") || text.starts_with("#![")) && text.trim_end().ends_with(']')))) + }); + + let (line_start, line) = lines.next()?; + + let mut in_codeblock = false; + // Check for a sequence of line comments. + if line.starts_with("//") { + let (mut line, mut line_start) = (line, line_start); + loop { + // Check for the start or end of a code block in the documentation comment. + // We want to ignore marked comment that appear in code blocks, since the + // marked comment isn't referring to the node we're currently checking. + if let Some(doc) = line.strip_prefix("///").or_else(|| line.strip_prefix("//!")) + && doc.trim_start().starts_with("```") + { + in_codeblock = !in_codeblock; + } + + if !in_codeblock && let Some(marker_pos) = line.to_ascii_uppercase().find(comment_marker) { + return Some(( + start_pos + + BytePos(u32::try_from(line_start).unwrap()) + + BytePos(u32::try_from(marker_pos).unwrap()), + line.starts_with("///"), + )); + } + match lines.next() { + Some((s, x)) if x.starts_with("//") => (line, line_start) = (x, s), + _ => return None, + } + } + } + // Check for a comment that appears after other code on the same line (e.g., `let x = // SAFETY:`) + // This handles cases in macros where the comment is on the same line as preceding code. + // We only check the first (immediate preceding) line for this pattern. + // Only whitespace is allowed between the comment marker and `SAFETY:`. + if let Some(comment_start) = [line.find("//"), line.find("/*")].into_iter().flatten().min() + && let after_marker = &line[comment_start + 2..] // skip marker + && let trimmed = after_marker.trim_start() // skip whitespace + && trimmed + .get(..comment_marker.len()) + .is_some_and(|s| s.eq_ignore_ascii_case(comment_marker)) + { + let marker_offset = 2 + (after_marker.len() - trimmed.len()); + return Some(( + start_pos + + BytePos(u32::try_from(line_start).unwrap()) + + BytePos(u32::try_from(comment_start + marker_offset).unwrap()), + false, + )); + } + // No line comments; look for the start of a block comment. + // This will only find them if they are at the start of a line. + let (mut line_start, mut line) = (line_start, line); + loop { + if line.starts_with("/*") { + let src = &src[line_start..line_starts.last().unwrap().to_usize()]; + let mut tokens = tokenize(src, FrontmatterAllowed::No); + let a = tokens.next(); + if let Some(marker_pos) = src[..a.unwrap().len as usize].to_ascii_uppercase().find(comment_marker) + && tokens.all(|t| t.kind == TokenKind::Whitespace) + { + return Some(( + start_pos + + BytePos(u32::try_from(line_start).unwrap()) + + BytePos(u32::try_from(marker_pos).unwrap()), + line.starts_with("/**"), + )); + } + return None; + } + match lines.next() { + Some(x) => (line_start, line) = x, + None => return None, + } + } +} + #[cfg(test)] mod test { use super::reindent_multiline; diff --git a/tests/ui/undocumented_as_casts.rs b/tests/ui/undocumented_as_casts.rs index bdb486cc791d..11d6145e689b 100644 --- a/tests/ui/undocumented_as_casts.rs +++ b/tests/ui/undocumented_as_casts.rs @@ -19,6 +19,48 @@ fn ignore_proc_macro() { ); } +fn declared_macro() { + macro_rules! cast { + ($x:expr, $t:ty) => { + // CAST: reason for the cast + $x as $t + }; + } + + cast!(0u32, u64); + + // CAST: reason for the cast + cast!(0u32 as u64, u32); + + macro_rules! cast_no_comment { + ($x:expr, $t:ty) => { + $x as $t + //~^ undocumented_as_casts + }; + } + + cast_no_comment!(0u32, u64); + + macro_rules! cast_with_comment_after { + ($x:expr, $t:ty) => { + $x as $t // CAST: reason for the cast + // + //~^^ undocumented_as_casts + }; + } + + cast_with_comment_after!(0u32, u64); + + macro_rules! add_one { + ($x:expr) => { + $x + 1 + }; + } + + // CAST: reason for the cast + add_one!(0u32 as u64); +} + // Valid Comments fn line_comment() { @@ -26,6 +68,16 @@ fn line_comment() { let _ = 0u32 as u64; } +fn line_comment_lowercase() { + // cast: reason + let _ = 0u32 as u64; +} + +fn line_comment_mixed_case() { + // CaSt: reason + let _ = 0u32 as u64; +} + fn line_comment_newlines() { // CAST: reason @@ -91,6 +143,28 @@ fn line_comment_let_match_block_multiple_arms() { }; } +fn newline_between_cast_line_comment_and_line_comment() { + // CAST: reason + + // This is a description + let _ = 0u32 as u64; +} + +fn line_comment_let_match_then_cast() { + let x = 0u32; + // CAST: reason for match block cast + let y = match x { + 0 => { + // CAST: reason for first cast + x as u64 + }, + _ => { + // CAST: reason for second cast + x as u64 + }, + } as usize; +} + fn block_comment() { /* CAST: reason */ let _ = 0u32 as u64; @@ -126,6 +200,21 @@ fn block_comment_function_return() -> u64 { 0u32 as u64 } +fn block_comment_let_match_then_cast() { + let x = 0u32; + /* CAST: reason for match block cast */ + let y = match x { + 0 => { + /* CAST: reason for first cast */ + x as u64 + }, + _ => { + /* CAST: reason for second cast */ + x as u64 + }, + } as usize; +} + // Invalid Comment fn no_comment() { @@ -202,45 +291,9 @@ fn line_cast_comment_before_block_comment() { //~^ undocumented_as_casts } -fn block_cast_comment_before_line_comment() { - /* CAST: reason */ - // This is a description - let _ = 0u32 as u64; - //~^ undocumented_as_casts -} - -fn block_cast_comment_before_block_comment() { - /* CAST: reason */ - /* This is a description */ - let _ = 0u32 as u64; - //~^ undocumented_as_casts -} - -fn newline_between_cast_comment_and_line_comment() { - // CAST: reason - - // This is a description - let _ = 0u32 as u64; - //~^ undocumented_as_casts -} - -// Need Improved Detection - -fn line_comment_let_match_then_cast() { +fn line_comment_let_match_then_cast_invalid() { let x = 0u32; - // CAST: reason for match block cast - let y = match x { - 0 => { - // CAST: reason for first cast - x as u64 - }, - _ => { - // CAST: reason for second cast - x as u64 - }, - } as usize; - let x = 0u32; let y = match x { //~^ undocumented_as_casts 0 => { @@ -251,6 +304,20 @@ fn line_comment_let_match_then_cast() { // CAST: reason for second cast x as u64 }, - // CAST: reason for final cast + // CAST: reason for match block cast } as usize; } + +fn block_cast_comment_before_line_comment() { + /* CAST: reason */ + // This is a description + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} + +fn block_cast_comment_before_block_comment() { + /* CAST: reason */ + /* This is a description */ + let _ = 0u32 as u64; + //~^ undocumented_as_casts +} diff --git a/tests/ui/undocumented_as_casts.stderr b/tests/ui/undocumented_as_casts.stderr index b75983d003b5..a3ece86cff32 100644 --- a/tests/ui/undocumented_as_casts.stderr +++ b/tests/ui/undocumented_as_casts.stderr @@ -1,23 +1,31 @@ error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:132:13 + --> tests/ui/undocumented_as_casts.rs:37:13 | -LL | let _ = 0u32 as u64; - | ^^^^^^^^^^^ +LL | $x as $t + | ^^^^^^^^ +... +LL | cast_no_comment!(0u32, u64); + | --------------------------- in this macro invocation | = help: consider adding a cast comment on the preceding line = note: `-D clippy::undocumented-as-casts` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::undocumented_as_casts)]` + = note: this error originates in the macro `cast_no_comment` (in Nightly builds, run with -Z macro-backtrace for more info) error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:137:13 + --> tests/ui/undocumented_as_casts.rs:46:13 | -LL | let _ = 0u32 as u64; // CAST: reason - | ^^^^^^^^^^^ +LL | $x as $t // CAST: reason for the cast + | ^^^^^^^^ +... +LL | cast_with_comment_after!(0u32, u64); + | ----------------------------------- in this macro invocation | = help: consider adding a cast comment on the preceding line + = note: this error originates in the macro `cast_with_comment_after` (in Nightly builds, run with -Z macro-backtrace for more info) error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:144:13 + --> tests/ui/undocumented_as_casts.rs:221:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -25,15 +33,15 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:151:13 + --> tests/ui/undocumented_as_casts.rs:226:13 | -LL | let _ = 0u32 as u64; +LL | let _ = 0u32 as u64; // CAST: reason | ^^^^^^^^^^^ | = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:158:13 + --> tests/ui/undocumented_as_casts.rs:233:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -41,7 +49,7 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:164:13 + --> tests/ui/undocumented_as_casts.rs:240:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -49,7 +57,7 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:171:13 + --> tests/ui/undocumented_as_casts.rs:247:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -57,7 +65,7 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:178:13 + --> tests/ui/undocumented_as_casts.rs:253:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -65,23 +73,15 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:183:13 - | -LL | let x = 0u32 as u64; - | ^^^^^^^^^^^ + --> tests/ui/undocumented_as_casts.rs:260:13 | - = help: consider adding a cast comment on the preceding line - -error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:194:13 - | -LL | let y = 1u32 as u64; +LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ | = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:201:13 + --> tests/ui/undocumented_as_casts.rs:267:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -89,23 +89,23 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:208:13 + --> tests/ui/undocumented_as_casts.rs:272:13 | -LL | let _ = 0u32 as u64; +LL | let x = 0u32 as u64; | ^^^^^^^^^^^ | = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:215:13 + --> tests/ui/undocumented_as_casts.rs:283:13 | -LL | let _ = 0u32 as u64; +LL | let y = 1u32 as u64; | ^^^^^^^^^^^ | = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:223:13 + --> tests/ui/undocumented_as_casts.rs:290:13 | LL | let _ = 0u32 as u64; | ^^^^^^^^^^^ @@ -113,7 +113,7 @@ LL | let _ = 0u32 as u64; = help: consider adding a cast comment on the preceding line error: `as` casts without a `// CAST:` explanation - --> tests/ui/undocumented_as_casts.rs:244:13 + --> tests/ui/undocumented_as_casts.rs:297:13 | LL | let y = match x { | _____________^ @@ -125,5 +125,21 @@ LL | | } as usize; | = help: consider adding a cast comment on the preceding line -error: aborting due to 15 previous errors +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:314:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: `as` casts without a `// CAST:` explanation + --> tests/ui/undocumented_as_casts.rs:321:13 + | +LL | let _ = 0u32 as u64; + | ^^^^^^^^^^^ + | + = help: consider adding a cast comment on the preceding line + +error: aborting due to 16 previous errors From 46c9dc2538afe5feda5f6a8c8b29a795aff724ff Mon Sep 17 00:00:00 2001 From: Charlie Chang Date: Wed, 11 Mar 2026 22:57:12 +0800 Subject: [PATCH 4/4] Update safety comment handling to use more descriptive variable names --- clippy_lints/src/undocumented_unsafe_blocks.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 3fa8754b55d5..0d75b115a6cf 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -596,7 +596,7 @@ fn item_has_safety_comment( "SAFETY:", accept_comment_above_attributes, ) { - Some((b, d)) => HasSafetyComment::Yes(b, d), + Some((pos, is_doc)) => HasSafetyComment::Yes(pos, is_doc), None => HasSafetyComment::No, } }; @@ -642,7 +642,7 @@ fn stmt_has_safety_comment( "SAFETY:", accept_comment_above_attributes, ) { - Some((b, d)) => HasSafetyComment::Yes(b, d), + Some((pos, is_doc)) => HasSafetyComment::Yes(pos, is_doc), None => HasSafetyComment::No, } }; @@ -724,7 +724,7 @@ fn span_from_macro_expansion_has_safety_comment( "SAFETY:", accept_comment_above_attributes, ) { - Some((b, d)) => HasSafetyComment::Yes(b, d), + Some((pos, is_doc)) => HasSafetyComment::Yes(pos, is_doc), None => HasSafetyComment::No, } } else {