Skip to content

Commit d3dfecb

Browse files
committed
Auto merge of rust-lang#12066 - y21:issue12048, r=Alexendoo
Don't look for safety comments in doc tests Fixes rust-lang#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
2 parents 2d6c238 + ef35e82 commit d3dfecb

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

clippy_lints/src/undocumented_unsafe_blocks.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -681,11 +681,19 @@ fn text_has_safety_comment(src: &str, line_starts: &[RelativeBytePos], start_pos
681681
.filter(|(_, text)| !text.is_empty());
682682

683683
let (line_start, line) = lines.next()?;
684+
let mut in_codeblock = false;
684685
// Check for a sequence of line comments.
685686
if line.starts_with("//") {
686687
let (mut line, mut line_start) = (line, line_start);
687688
loop {
688-
if line.to_ascii_uppercase().contains("SAFETY:") {
689+
// Don't lint if the safety comment is part of a codeblock in a doc comment.
690+
// It may or may not be required, and we can't very easily check it (and we shouldn't, since
691+
// the safety comment isn't referring to the node we're currently checking)
692+
if line.trim_start_matches("///").trim_start().starts_with("```") {
693+
in_codeblock = !in_codeblock;
694+
}
695+
696+
if line.to_ascii_uppercase().contains("SAFETY:") && !in_codeblock {
689697
return Some(start_pos + BytePos(u32::try_from(line_start).unwrap()));
690698
}
691699
match lines.next() {

tests/ui/unnecessary_safety_comment.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,25 @@ mod issue_10084 {
7373
}
7474
}
7575

76+
mod issue_12048 {
77+
pub const X: u8 = 0;
78+
79+
/// Returns a pointer to five.
80+
///
81+
/// # Examples
82+
///
83+
/// ```
84+
/// use foo::point_to_five;
85+
///
86+
/// let five_pointer = point_to_five();
87+
/// // Safety: this pointer always points to a valid five.
88+
/// let five = unsafe { *five_pointer };
89+
/// assert_eq!(five, 5);
90+
/// ```
91+
pub fn point_to_five() -> *const u8 {
92+
static FIVE: u8 = 5;
93+
&FIVE
94+
}
95+
}
96+
7697
fn main() {}

0 commit comments

Comments
 (0)