Skip to content

Commit 9c2f9c9

Browse files
committed
Add unnecessary_safety_doc lint
1 parent c4fbe54 commit 9c2f9c9

File tree

4 files changed

+79
-25
lines changed

4 files changed

+79
-25
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4449,6 +4449,7 @@ Released 2018-09-13
44494449
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
44504450
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
44514451
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
4452+
[`unnecessary_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_safety_doc
44524453
[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
44534454
[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
44544455
[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
127127
crate::doc::MISSING_PANICS_DOC_INFO,
128128
crate::doc::MISSING_SAFETY_DOC_INFO,
129129
crate::doc::NEEDLESS_DOCTEST_MAIN_INFO,
130+
crate::doc::UNNECESSARY_SAFETY_DOC_INFO,
130131
crate::double_parens::DOUBLE_PARENS_INFO,
131132
crate::drop_forget_ref::DROP_COPY_INFO,
132133
crate::drop_forget_ref::DROP_NON_DROP_INFO,

clippy_lints/src/doc.rs

Lines changed: 69 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,42 @@ declare_clippy_lint! {
221221
"possible typo for an intra-doc link"
222222
}
223223

224+
declare_clippy_lint! {
225+
/// ### What it does
226+
/// Checks for the doc comments of publicly visible
227+
/// safe functions and traits and warns if there is a `# Safety` section.
228+
///
229+
/// ### Why is this bad?
230+
/// Safe functions and traits are safe to implement and therefore do not
231+
/// need to describe safety preconditions that users are required to uphold.
232+
///
233+
/// ### Examples
234+
/// ```rust
235+
///# type Universe = ();
236+
/// /// # Safety
237+
/// ///
238+
/// /// This function should not be called before the horsemen are ready.
239+
/// pub fn start_apocalypse_but_safely(u: &mut Universe) {
240+
/// unimplemented!();
241+
/// }
242+
/// ```
243+
///
244+
/// The function is safe, so there shouldn't be any preconditions
245+
/// that have to be explained for safety reasons.
246+
///
247+
/// ```rust
248+
///# type Universe = ();
249+
/// /// This function should really be documented
250+
/// pub fn start_apocalypse(u: &mut Universe) {
251+
/// unimplemented!();
252+
/// }
253+
/// ```
254+
#[clippy::version = "1.66.0"]
255+
pub UNNECESSARY_SAFETY_DOC,
256+
style,
257+
"`pub fn` or `pub trait` with `# Safety` docs"
258+
}
259+
224260
#[expect(clippy::module_name_repetitions)]
225261
#[derive(Clone)]
226262
pub struct DocMarkdown {
@@ -243,7 +279,8 @@ impl_lint_pass!(DocMarkdown => [
243279
MISSING_SAFETY_DOC,
244280
MISSING_ERRORS_DOC,
245281
MISSING_PANICS_DOC,
246-
NEEDLESS_DOCTEST_MAIN
282+
NEEDLESS_DOCTEST_MAIN,
283+
UNNECESSARY_SAFETY_DOC,
247284
]);
248285

249286
impl<'tcx> LateLintPass<'tcx> for DocMarkdown {
@@ -254,7 +291,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown {
254291

255292
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) {
256293
let attrs = cx.tcx.hir().attrs(item.hir_id());
257-
let headers = check_attrs(cx, &self.valid_idents, attrs);
294+
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else { return };
258295
match item.kind {
259296
hir::ItemKind::Fn(ref sig, _, body_id) => {
260297
if !(is_entrypoint_fn(cx, item.def_id.to_def_id()) || in_external_macro(cx.tcx.sess, item.span)) {
@@ -271,15 +308,20 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown {
271308
hir::ItemKind::Impl(impl_) => {
272309
self.in_trait_impl = impl_.of_trait.is_some();
273310
},
274-
hir::ItemKind::Trait(_, unsafety, ..) => {
275-
if !headers.safety && unsafety == hir::Unsafety::Unsafe {
276-
span_lint(
277-
cx,
278-
MISSING_SAFETY_DOC,
279-
cx.tcx.def_span(item.def_id),
280-
"docs for unsafe trait missing `# Safety` section",
281-
);
282-
}
311+
hir::ItemKind::Trait(_, unsafety, ..) => match (headers.safety, unsafety) {
312+
(false, hir::Unsafety::Unsafe) => span_lint(
313+
cx,
314+
MISSING_SAFETY_DOC,
315+
cx.tcx.def_span(item.def_id),
316+
"docs for unsafe trait missing `# Safety` section",
317+
),
318+
(true, hir::Unsafety::Normal) => span_lint(
319+
cx,
320+
UNNECESSARY_SAFETY_DOC,
321+
cx.tcx.def_span(item.def_id),
322+
"docs for safe trait have unnecessary `# Safety` section",
323+
),
324+
_ => (),
283325
},
284326
_ => (),
285327
}
@@ -293,7 +335,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown {
293335

294336
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::TraitItem<'_>) {
295337
let attrs = cx.tcx.hir().attrs(item.hir_id());
296-
let headers = check_attrs(cx, &self.valid_idents, attrs);
338+
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else { return };
297339
if let hir::TraitItemKind::Fn(ref sig, ..) = item.kind {
298340
if !in_external_macro(cx.tcx.sess, item.span) {
299341
lint_for_missing_headers(cx, item.def_id.def_id, sig, headers, None, None);
@@ -303,7 +345,7 @@ impl<'tcx> LateLintPass<'tcx> for DocMarkdown {
303345

304346
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::ImplItem<'_>) {
305347
let attrs = cx.tcx.hir().attrs(item.hir_id());
306-
let headers = check_attrs(cx, &self.valid_idents, attrs);
348+
let Some(headers) = check_attrs(cx, &self.valid_idents, attrs) else { return };
307349
if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
308350
return;
309351
}
@@ -343,14 +385,20 @@ fn lint_for_missing_headers(
343385
}
344386

345387
let span = cx.tcx.def_span(def_id);
346-
347-
if !headers.safety && sig.header.unsafety == hir::Unsafety::Unsafe {
348-
span_lint(
388+
match (headers.safety, sig.header.unsafety) {
389+
(false, hir::Unsafety::Unsafe) => span_lint(
349390
cx,
350391
MISSING_SAFETY_DOC,
351392
span,
352393
"unsafe function's docs miss `# Safety` section",
353-
);
394+
),
395+
(true, hir::Unsafety::Normal) => span_lint(
396+
cx,
397+
UNNECESSARY_SAFETY_DOC,
398+
span,
399+
"safe function's docs have unnecessary `# Safety` section",
400+
),
401+
_ => (),
354402
}
355403
if !headers.panics && panic_span.is_some() {
356404
span_lint_and_note(
@@ -452,7 +500,7 @@ struct DocHeaders {
452500
panics: bool,
453501
}
454502

455-
fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[Attribute]) -> DocHeaders {
503+
fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[Attribute]) -> Option<DocHeaders> {
456504
use pulldown_cmark::{BrokenLink, CowStr, Options};
457505
/// We don't want the parser to choke on intra doc links. Since we don't
458506
/// actually care about rendering them, just pretend that all broken links are
@@ -473,11 +521,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
473521
} else if attr.has_name(sym::doc) {
474522
// ignore mix of sugared and non-sugared doc
475523
// don't trigger the safety or errors check
476-
return DocHeaders {
477-
safety: true,
478-
errors: true,
479-
panics: true,
480-
};
524+
return None;
481525
}
482526
}
483527

@@ -489,7 +533,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
489533
}
490534

491535
if doc.is_empty() {
492-
return DocHeaders::default();
536+
return Some(DocHeaders::default());
493537
}
494538

495539
let mut cb = fake_broken_link_callback;
@@ -512,7 +556,7 @@ fn check_attrs(cx: &LateContext<'_>, valid_idents: &FxHashSet<String>, attrs: &[
512556
(previous, current) => Err(((previous, previous_range), (current, current_range))),
513557
}
514558
});
515-
check_doc(cx, valid_idents, events, &spans)
559+
Some(check_doc(cx, valid_idents, events, &spans))
516560
}
517561

518562
const RUST_CODE: &[&str] = &["rust", "no_run", "should_panic", "compile_fail"];

tests/ui/auxiliary/doc_unsafe_macros.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,11 @@ macro_rules! undocd_unsafe {
66
}
77
};
88
}
9+
#[macro_export]
10+
macro_rules! undocd_safe {
11+
() => {
12+
pub fn vey_oy() {
13+
unimplemented!();
14+
}
15+
};
16+
}

0 commit comments

Comments
 (0)