Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust API Guideline: Function docs include panic conditions in "Panics" section #1791

Closed
mcarton opened this issue May 27, 2017 · 3 comments
Closed
Assignees
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group L-guidelines Lint: Related to the Rust API Guidelines T-middle Type: Probably requires verifiying types

Comments

@mcarton
Copy link
Member

mcarton commented May 27, 2017

Only for functions calling panic! and other known-panicky functions (eg. Option::unwrap).

As per Function docs include error, panic, and safety considerations.

@mcarton mcarton added L-correctness Lint: Belongs in the correctness lint group E-medium Call for participation: Medium difficulty level problem and requires some initial experience. A-lint Area: New lints T-middle Type: Probably requires verifiying types labels May 27, 2017
@dtolnay
Copy link
Member

dtolnay commented May 28, 2017

I think requiring this when calling known-panicky functions is extreme. Often something is unwrapped because it is expected to never fail. That's not something you can explain to the caller. "This function panics if... the code is wrong"?

Requiring a "Panics" section if the function uses specifically panic! would be good. If it can panic, it should be documented. If it can't panic, they should have used unreachable! instead.

@clarfonthey
Copy link
Contributor

clarfonthey commented May 28, 2017

Don't forget assert! and co. as well. This shouldn't include debug_assert! because those shouldn't ever panic unless the code is broken.

@mcarton mcarton added the L-guidelines Lint: Related to the Rust API Guidelines label May 28, 2017
@mcarton mcarton self-assigned this Jun 18, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 5, 2017

We also usually don't want to document panics in logic provided by the caller.

/// # Panics
///
/// This function panics if `T`'s implementation of `Display` panics.
pub fn print<T: Display>(t: T) {
    println!("{}", t.to_string());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group L-guidelines Lint: Related to the Rust API Guidelines T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

4 participants