diff options
| author | bors <bors@rust-lang.org> | 2024-01-05 15:23:11 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-01-05 15:23:11 +0000 |
| commit | d3dfecbd418e8518f20f3a0660fdc204b07736c1 (patch) | |
| tree | b26ae761843dd5b6a78a141ce7df637993fb6f23 | |
| parent | 2d6c2386f529f33a2e614727b3e41982940abbdd (diff) | |
| parent | ef35e82ea3d40ba271e97764c5b9868f7297d318 (diff) | |
| download | rust-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.rs | 10 | ||||
| -rw-r--r-- | tests/ui/unnecessary_safety_comment.rs | 21 |
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() {} |
