diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4e747524e9e..a9448a57f7f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1092,6 +1092,7 @@ Released 2018-09-13
[`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op
[`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn
[`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items
+[`missing_errors_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc
[`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
[`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
diff --git a/README.md b/README.md
index e8818412e90..97a7c97b49a 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
-[There are 338 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 339 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs
index 0fcc9763d37..8f8d8ad96bb 100644
--- a/clippy_lints/src/doc.rs
+++ b/clippy_lints/src/doc.rs
@@ -1,4 +1,4 @@
-use crate::utils::span_lint;
+use crate::utils::{match_type, paths, return_ty, span_lint};
use itertools::Itertools;
use pulldown_cmark;
use rustc::hir;
@@ -8,7 +8,7 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_session::declare_tool_lint;
use std::ops::Range;
use syntax::ast::{AttrKind, Attribute};
-use syntax::source_map::{BytePos, Span};
+use syntax::source_map::{BytePos, MultiSpan, Span};
use syntax_pos::Pos;
use url::Url;
@@ -45,7 +45,7 @@ declare_clippy_lint! {
///
/// **Known problems:** None.
///
- /// **Examples**:
+ /// **Examples:**
/// ```rust
///# type Universe = ();
/// /// This function should really be documented
@@ -70,6 +70,35 @@ declare_clippy_lint! {
"`pub unsafe fn` without `# Safety` docs"
}
+declare_clippy_lint! {
+ /// **What it does:** Checks the doc comments of publicly visible functions that
+ /// return a `Result` type and warns if there is no `# Errors` section.
+ ///
+ /// **Why is this bad?** Documenting the type of errors that can be returned from a
+ /// function can help callers write code to handle the errors appropriately.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Examples:**
+ ///
+ /// Since the following function returns a `Result` it has an `# Errors` section in
+ /// its doc comment:
+ ///
+ /// ```rust
+ ///# use std::io;
+ /// /// # Errors
+ /// ///
+ /// /// Will return `Err` if `filename` does not exist or the user does not have
+ /// /// permission to read it.
+ /// pub fn read(filename: String) -> io::Result<String> {
+ /// unimplemented!();
+ /// }
+ /// ```
+ pub MISSING_ERRORS_DOC,
+ pedantic,
+ "`pub fn` returns `Result` without `# Errors` in doc comment"
+}
+
declare_clippy_lint! {
/// **What it does:** Checks for `fn main() { .. }` in doctests
///
@@ -114,7 +143,7 @@ impl DocMarkdown {
}
}
-impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN, MISSING_SAFETY_DOC, NEEDLESS_DOCTEST_MAIN]);
+impl_lint_pass!(DocMarkdown => [DOC_MARKDOWN, MISSING_SAFETY_DOC, MISSING_ERRORS_DOC, NEEDLESS_DOCTEST_MAIN]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
fn check_crate(&mut self, cx: &LateContext<'a, 'tcx>, krate: &'tcx hir::Crate) {
@@ -122,20 +151,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
}
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
- if check_attrs(cx, &self.valid_idents, &item.attrs) {
- return;
- }
- // no safety header
+ let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
match item.kind {
hir::ItemKind::Fn(ref sig, ..) => {
- if cx.access_levels.is_exported(item.hir_id) && sig.header.unsafety == hir::Unsafety::Unsafe {
- span_lint(
- cx,
- MISSING_SAFETY_DOC,
- item.span,
- "unsafe function's docs miss `# Safety` section",
- );
- }
+ lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
},
hir::ItemKind::Impl(_, _, _, _, ref trait_ref, ..) => {
self.in_trait_impl = trait_ref.is_some();
@@ -151,40 +170,51 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
}
fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) {
- if check_attrs(cx, &self.valid_idents, &item.attrs) {
- return;
- }
- // no safety header
+ let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
if let hir::TraitItemKind::Method(ref sig, ..) = item.kind {
- if cx.access_levels.is_exported(item.hir_id) && sig.header.unsafety == hir::Unsafety::Unsafe {
- span_lint(
- cx,
- MISSING_SAFETY_DOC,
- item.span,
- "unsafe function's docs miss `# Safety` section",
- );
- }
+ lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
}
}
fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::ImplItem) {
- if check_attrs(cx, &self.valid_idents, &item.attrs) || self.in_trait_impl {
+ let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
+ if self.in_trait_impl {
return;
}
- // no safety header
if let hir::ImplItemKind::Method(ref sig, ..) = item.kind {
- if cx.access_levels.is_exported(item.hir_id) && sig.header.unsafety == hir::Unsafety::Unsafe {
- span_lint(
- cx,
- MISSING_SAFETY_DOC,
- item.span,
- "unsafe function's docs miss `# Safety` section",
- );
- }
+ lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
}
}
}
+fn lint_for_missing_headers<'a, 'tcx>(
+ cx: &LateContext<'a, 'tcx>,
+ hir_id: hir::HirId,
+ span: impl Into<MultiSpan> + Copy,
+ sig: &hir::FnSig,
+ headers: DocHeaders,
+) {
+ if !cx.access_levels.is_exported(hir_id) {
+ return; // Private functions do not require doc comments
+ }
+ if !headers.safety && sig.header.unsafety == hir::Unsafety::Unsafe {
+ span_lint(
+ cx,
+ MISSING_SAFETY_DOC,
+ span,
+ "unsafe function's docs miss `# Safety` section",
+ );
+ }
+ if !headers.errors && match_type(cx, return_ty(cx, hir_id), &paths::RESULT) {
+ span_lint(
+ cx,
+ MISSING_ERRORS_DOC,
+ span,
+ "docs for function returning `Result` missing `# Errors` section",
+ );
+ }
+}
+
/// Cleanup documentation decoration (`///` and such).
///
/// We can't use `syntax::attr::AttributeMethods::with_desugared_doc` or
@@ -243,7 +273,13 @@ pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<(
panic!("not a doc-comment: {}", comment);
}
-pub fn check_attrs<'a>(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String>, attrs: &'a [Attribute]) -> bool {
+#[derive(Copy, Clone)]
+struct DocHeaders {
+ safety: bool,
+ errors: bool,
+}
+
+fn check_attrs<'a>(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String>, attrs: &'a [Attribute]) -> DocHeaders {
let mut doc = String::new();
let mut spans = vec![];
@@ -255,7 +291,11 @@ pub fn check_attrs<'a>(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String
doc.push_str(&comment);
} else if attr.check_name(sym!(doc)) {
// ignore mix of sugared and non-sugared doc
- return true; // don't trigger the safety check
+ // don't trigger the safety or errors check
+ return DocHeaders {
+ safety: true,
+ errors: true,
+ };
}
}
@@ -267,7 +307,10 @@ pub fn check_attrs<'a>(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String
}
if doc.is_empty() {
- return false;
+ return DocHeaders {
+ safety: false,
+ errors: false,
+ };
}
let parser = pulldown_cmark::Parser::new(&doc).into_offset_iter();
@@ -295,12 +338,15 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
valid_idents: &FxHashSet<String>,
events: Events,
spans: &[(usize, Span)],
-) -> bool {
+) -> DocHeaders {
// true if a safety header was found
use pulldown_cmark::Event::*;
use pulldown_cmark::Tag::*;
- let mut safety_header = false;
+ let mut headers = DocHeaders {
+ safety: false,
+ errors: false,
+ };
let mut in_code = false;
let mut in_link = None;
let mut in_heading = false;
@@ -323,7 +369,8 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
// text "http://example.com" by pulldown-cmark
continue;
}
- safety_header |= in_heading && text.trim() == "Safety";
+ headers.safety |= in_heading && text.trim() == "Safety";
+ headers.errors |= in_heading && text.trim() == "Errors";
let index = match spans.binary_search_by(|c| c.0.cmp(&range.start)) {
Ok(o) => o,
Err(e) => e - 1,
@@ -340,7 +387,7 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
},
}
}
- safety_header
+ headers
}
fn check_code(cx: &LateContext<'_, '_>, text: &str, span: Span) {
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 432a38a11a5..d14f946c8eb 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -488,6 +488,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
&derive::DERIVE_HASH_XOR_EQ,
&derive::EXPL_IMPL_CLONE_ON_COPY,
&doc::DOC_MARKDOWN,
+ &doc::MISSING_ERRORS_DOC,
&doc::MISSING_SAFETY_DOC,
&doc::NEEDLESS_DOCTEST_MAIN,
&double_comparison::DOUBLE_COMPARISONS,
@@ -1013,6 +1014,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
LintId::of(&doc::DOC_MARKDOWN),
+ LintId::of(&doc::MISSING_ERRORS_DOC),
LintId::of(&empty_enum::EMPTY_ENUM),
LintId::of(&enum_glob_use::ENUM_GLOB_USE),
LintId::of(&enum_variants::MODULE_NAME_REPETITIONS),
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 5b08571c258..1f1c24b2c30 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 338] = [
+pub const ALL_LINTS: [Lint; 339] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
@@ -1127,6 +1127,13 @@ pub const ALL_LINTS: [Lint; 338] = [
deprecation: None,
module: "missing_doc",
},
+ Lint {
+ name: "missing_errors_doc",
+ group: "pedantic",
+ desc: "`pub fn` returns `Result` without `# Errors` in doc comment",
+ deprecation: None,
+ module: "doc",
+ },
Lint {
name: "missing_inline_in_public_items",
group: "restriction",
diff --git a/tests/ui/doc_errors.rs b/tests/ui/doc_errors.rs
new file mode 100644
index 00000000000..408cf573896
--- /dev/null
+++ b/tests/ui/doc_errors.rs
@@ -0,0 +1,64 @@
+#![warn(clippy::missing_errors_doc)]
+
+use std::io;
+
+pub fn pub_fn_missing_errors_header() -> Result<(), ()> {
+ unimplemented!();
+}
+
+/// This is not sufficiently documented.
+pub fn pub_fn_returning_io_result() -> io::Result<()> {
+ unimplemented!();
+}
+
+/// # Errors
+/// A description of the errors goes here.
+pub fn pub_fn_with_errors_header() -> Result<(), ()> {
+ unimplemented!();
+}
+
+/// This function doesn't require the documentation because it is private
+fn priv_fn_missing_errors_header() -> Result<(), ()> {
+ unimplemented!();
+}
+
+pub struct Struct1;
+
+impl Struct1 {
+ /// This is not sufficiently documented.
+ pub fn pub_method_missing_errors_header() -> Result<(), ()> {
+ unimplemented!();
+ }
+
+ /// # Errors
+ /// A description of the errors goes here.
+ pub fn pub_method_with_errors_header() -> Result<(), ()> {
+ unimplemented!();
+ }
+
+ /// This function doesn't require the documentation because it is private.
+ fn priv_method_missing_errors_header() -> Result<(), ()> {
+ unimplemented!();
+ }
+}
+
+pub trait Trait1 {
+ /// This is not sufficiently documented.
+ fn trait_method_missing_errors_header() -> Result<(), ()>;
+
+ /// # Errors
+ /// A description of the errors goes here.
+ fn trait_method_with_errors_header() -> Result<(), ()>;
+}
+
+impl Trait1 for Struct1 {
+ fn trait_method_missing_errors_header() -> Result<(), ()> {
+ unimplemented!();
+ }
+
+ fn trait_method_with_errors_header() -> Result<(), ()> {
+ unimplemented!();
+ }
+}
+
+fn main() {}
diff --git a/tests/ui/doc_errors.stderr b/tests/ui/doc_errors.stderr
new file mode 100644
index 00000000000..f1d321cf909
--- /dev/null
+++ b/tests/ui/doc_errors.stderr
@@ -0,0 +1,34 @@
+error: docs for function returning `Result` missing `# Errors` section
+ --> $DIR/doc_errors.rs:5:1
+ |
+LL | / pub fn pub_fn_missing_errors_header() -> Result<(), ()> {
+LL | | unimplemented!();
+LL | | }
+ | |_^
+ |
+ = note: `-D clippy::missing-errors-doc` implied by `-D warnings`
+
+error: docs for function returning `Result` missing `# Errors` section
+ --> $DIR/doc_errors.rs:10:1
+ |
+LL | / pub fn pub_fn_returning_io_result() -> io::Result<()> {
+LL | | unimplemented!();
+LL | | }
+ | |_^
+
+error: docs for function returning `Result` missing `# Errors` section
+ --> $DIR/doc_errors.rs:29:5
+ |
+LL | / pub fn pub_method_missing_errors_header() -> Result<(), ()> {
+LL | | unimplemented!();
+LL | | }
+ | |_____^
+
+error: docs for function returning `Result` missing `# Errors` section
+ --> $DIR/doc_errors.rs:47:5
+ |
+LL | fn trait_method_missing_errors_header() -> Result<(), ()>;
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 4 previous errors
+