missing_safety_doc has questionable utility as a default

[camio]

Description

I have concerns with the missing_safety_doc clippy lint defaulting to warn level.

The lint requires unsafe functions have a # Safety documentation section describing "safety preconditions, so that users can be sure they are using them safely". I don't understand the utility of documenting safety preconditions separately from other preconditions. Calling out some preconditions as especially dangerous doesn't seem to save time or improve code in practice. Additionally, documenting all function preconditions (those impacting safety and those that don't) together avoids documentation clutter.

A GitHub code search revealed that most # Safety sections are empty (presumably to defeat the linter). Other developers document all preconditions there which defeats the purpose of the section.

This linter rule should default to "allow". Folks who like it can opt in, but a better practice is to skip the # Safety section and document all preconditions together for unsafe functions.

Version

No response

Additional Labels

No response

[Alexendoo]

A GitHub code search revealed that most # Safety sections are empty

None of the examples on that page contain an empty # Safety section, some only show a single line of context but if you click through the section has content

a better practice is to skip the # Safety section and document all preconditions together for unsafe functions

Do you have an example where this style is an improvement? A safety section is the style used by std itself and much of the ecosystem, it's recommended by the API guidelines and I find generally works very well

[camio]

@Alexendoo, thanks for pointing that there was content in those sections. I totally missed that.

After reviewing the API guidelines, I realized that my documentation comments are following design by contract (DbC), but that's now how the API guidelines work.

For example, with the current Vec.insert documentation, one cannot claim this code is incorrect,

let mut vec = vec![1, 2, 3];
vec.insert(5, 1);

, because insert has no preconditions.

I would instead write insert's documentation like this:

    /// Inserts an element at position `index` within the vector, shifting all
    /// elements after it to the right.
    ///
    /// # Preconditions
    ///
    /// - `index` <= `self.len()`
    ///
    /// # Examples
    ///...

This makes it clear that my example code is incorrect. The absence or presence of the unsafe function modifier indicates whether the function being called out of contract will result in a panic or potentially undefined behavior.

I hope this helps clarify.

[Alexendoo]

For example, with the current Vec.insert documentation, one cannot claim this code is incorrect

It documents clearly that it will panic so I do not understand this claim. Writing the same thing but under a precondition header doesn't really change that, rather it's less clear as it no longer tells me what will happen if I do not uphold the requirements

[camio]

[Documenting preconditions alone] no longer tells me what will happen if I do not uphold the requirements

I agree, but what will you do with this additional information as a consumer of the library? It doesn't seem actionable.