about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2024-01-05 15:23:11 +0000
committerbors <bors@rust-lang.org>2024-01-05 15:23:11 +0000
commitd3dfecbd418e8518f20f3a0660fdc204b07736c1 (patch)
treeb26ae761843dd5b6a78a141ce7df637993fb6f23
parent2d6c2386f529f33a2e614727b3e41982940abbdd (diff)
parentef35e82ea3d40ba271e97764c5b9868f7297d318 (diff)
downloadrust-d3dfecbd418e8518f20f3a0660fdc204b07736c1.tar.gz
rust-d3dfecbd418e8518f20f3a0660fdc204b07736c1.zip
Auto merge of #12066 - y21:issue12048, r=Alexendoo
Don't look for safety comments in doc tests

Fixes #12048.

What happened in the linked issue is that the lint checks for lines that start with `//` and have `SAFETY:` somewhere in it above the function item.
This works for regular comments, but when the `//` is the start of a doc comment (e.g. `/// // SAFETY: ...`) and it's part of a doc test (i.e. within \`\`\`), we probably shouldn't lint that, since the user most likely meant to refer to a different node than the one currently being checked. For example in the linked issue, the safety comment refers to `unsafe { *five_pointer }`, but the lint believes it's part of the function item.

We also can't really easily test whether the `// SAFETY:` comment within a doc comment is necessary or not, since I think that would require creating a new compiler session to re-parse the contents of the doc comment. We already do this for one of the doc markdown lints, to look for a main function in doc tests, but I don't know how to feel about doing that in more places, so probably best to just ignore them?

changelog: [`unnecessary_safety_comment`]: don't look for safety comments in doc tests
-rw-r--r--clippy_lints/src/undocumented_unsafe_blocks.rs10
-rw-r--r--tests/ui/unnecessary_safety_comment.rs21
2 files changed, 30 insertions, 1 deletions
diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs
index e5bc3b5a25f..add4b3e5637 100644
--- a/clippy_lints/src/undocumented_unsafe_blocks.rs
+++ b/clippy_lints/src/undocumented_unsafe_blocks.rs
@@ -681,11 +681,19 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
         .filter(|(_, text)| !text.is_empty());
 
     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 {
-            if line.to_ascii_uppercase().contains("SAFETY:") {
+            // 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 line.trim_start_matches("///").trim_start().starts_with("```") {
+                in_codeblock = !in_codeblock;
+            }
+
+            if line.to_ascii_uppercase().contains("SAFETY:") && !in_codeblock {
                 return Some(start_pos + BytePos(u32::try_from(line_start).unwrap()));
             }
             match lines.next() {
diff --git a/tests/ui/unnecessary_safety_comment.rs b/tests/ui/unnecessary_safety_comment.rs
index d9a7ad8e56c..bdc6fa0f46b 100644
--- a/tests/ui/unnecessary_safety_comment.rs
+++ b/tests/ui/unnecessary_safety_comment.rs
@@ -73,4 +73,25 @@ mod issue_10084 {
     }
 }
 
+mod issue_12048 {
+    pub const X: u8 = 0;
+
+    /// Returns a pointer to five.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use foo::point_to_five;
+    ///
+    /// let five_pointer = point_to_five();
+    /// // Safety: this pointer always points to a valid five.
+    /// let five = unsafe { *five_pointer };
+    /// assert_eq!(five, 5);
+    /// ```
+    pub fn point_to_five() -> *const u8 {
+        static FIVE: u8 = 5;
+        &FIVE
+    }
+}
+
 fn main() {}