Optional lint for functions that panic without mentioning panics in rustdoc

[joshtriplett]

@mgattozzi's excellent talk at RustConf just now made a really good point: functions that panic should document that panic in their documentation.

While we can't detect this in the completely general case, we could detect explicit calls to panic!(), and RFC 2091 would give the compiler enough information that it could detect calls to functions that (may) panic.

I'd love to see an optional lint for functions that call panic, indirectly or directly, without documentation of that. (Having such documentation would require fairly fuzzy detection, unfortunately, but I still think this is worth considering..)

[killercup]

Casual drive-by comment:

• We recently talked about this in the dev-tools meeting when discussion rustdoc: enhancement: Mark functions that may panic automaticaly steveklabnik/rustdoc#78
• I want to have special headlines in doc comments
• I want this to be linted
• I think a # Panics section makes the most sense to discuss pre/post conditions that are specially checked/assert!ed in the function. Intuitively I'd say most indirect panics do not require documentation (e.g.: a function called by a some function that this function calls uses indexing)

[joshtriplett]

@killercup First: awesome, thank you!

Can you give an example of the kind of "indirect panics" that you don't think need documentation?

I do, for instance, want documentation that using an invalid array index would panic rather than generating an error.

[killercup]

@joshtriplett sure. It may be that I have a different assumption on what I want to document than you do.

Consider this function:

fn foo(x: i32) {
    assert!(x > 1, "Can only foo-ify things great than 1");
    let y = vec![1, 2, 3,];
    let z = helper_thing(&y, x);
    // ...
}

What I'm saying is this: The assert explicitly communicates a possible programmer error, where the API expects its user to check they supply a valid input. Requiring documentation for this assert is very valuable (IMHO) and hopefully not that hard. Analyzing that helper_thing cannot (indirectly) cause a panic may be quite hard (if at all possible!) and in contrast to the explicit panic conditions may only result in a "yep, that'll panic" boolean. (Which is totally fine but a different goal.)

[joshtriplett]

@killercup I actually do want that function to provide documentation that it panics if passed an index too large. As opposed to, for instance, returning an error.

That said, there are occasional cases where a call to .unwrap() or similar has a panic that is in practice unreachable, and in those cases, sure, no need to document that. But I think I'd be OK with having to tag those somehow to ignore the apparent issue.

[brightly-salty]

@rustbot claim