X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fdoc.rs;h=133a20b669a17720faab96f0cb22031939e01f2a;hb=0ee393cf01b59d55a54e9e4299797bd0f7339d58;hp=bd959427403b712235f0db80387592d6ef9b3dfd;hpb=23a9c021201df810f40d0ad1fe3815dda5da8462;p=rust.git

diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs
index bd959427403..133a20b669a 100644
--- a/clippy_lints/src/doc.rs
+++ b/clippy_lints/src/doc.rs
@@ -1,13 +1,15 @@
-use crate::utils::span_lint;
+use crate::utils::{get_trait_def_id, implements_trait, is_entrypoint_fn, match_type, paths, return_ty, span_lint};
 use itertools::Itertools;
-use pulldown_cmark;
-use rustc::lint::{EarlyContext, EarlyLintPass, LintArray, LintPass};
-use rustc::{declare_tool_lint, impl_lint_pass};
+use rustc::lint::in_external_macro;
+use rustc::ty::TyKind;
 use rustc_data_structures::fx::FxHashSet;
+use rustc_hir as hir;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::source_map::{BytePos, MultiSpan, Span};
+use rustc_span::Pos;
 use std::ops::Range;
-use syntax::ast;
-use syntax::source_map::{BytePos, Span};
-use syntax_pos::Pos;
+use syntax::ast::{AttrKind, Attribute};
 use url::Url;
 
 declare_clippy_lint! {
@@ -43,7 +45,7 @@
     ///
     /// **Known problems:** None.
     ///
-    /// **Examples**:
+    /// **Examples:**
     /// ```rust
     ///# type Universe = ();
     /// /// This function should really be documented
@@ -68,6 +70,35 @@
     "`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
     ///
@@ -100,34 +131,119 @@
 #[derive(Clone)]
 pub struct DocMarkdown {
     valid_idents: FxHashSet<String>,
+    in_trait_impl: bool,
 }
 
 impl DocMarkdown {
     pub fn new(valid_idents: FxHashSet<String>) -> Self {
-        Self { valid_idents }
+        Self {
+            valid_idents,
+            in_trait_impl: false,
+        }
     }
 }
 
-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 EarlyLintPass for DocMarkdown {
-    fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &ast::Crate) {
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
+    fn check_crate(&mut self, cx: &LateContext<'a, 'tcx>, krate: &'tcx hir::Crate<'_>) {
         check_attrs(cx, &self.valid_idents, &krate.attrs);
     }
 
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
-        if check_attrs(cx, &self.valid_idents, &item.attrs) {
+    fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) {
+        let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
+        match item.kind {
+            hir::ItemKind::Fn(ref sig, ..) => {
+                if !(is_entrypoint_fn(cx, cx.tcx.hir().local_def_id(item.hir_id))
+                    || in_external_macro(cx.tcx.sess, item.span))
+                {
+                    lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
+                }
+            },
+            hir::ItemKind::Impl {
+                of_trait: ref trait_ref,
+                ..
+            } => {
+                self.in_trait_impl = trait_ref.is_some();
+            },
+            _ => {},
+        }
+    }
+
+    fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) {
+        if let hir::ItemKind::Impl { .. } = item.kind {
+            self.in_trait_impl = false;
+        }
+    }
+
+    fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem<'_>) {
+        let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
+        if let hir::TraitItemKind::Method(ref sig, ..) = item.kind {
+            if !in_external_macro(cx.tcx.sess, item.span) {
+                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<'_>) {
+        let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
+        if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
             return;
         }
-        // no safety header
-        if let ast::ItemKind::Fn(_, ref header, ..) = item.kind {
-            if item.vis.node.is_pub() && header.unsafety == ast::Unsafety::Unsafe {
-                span_lint(
-                    cx,
-                    MISSING_SAFETY_DOC,
-                    item.span,
-                    "unsafe function's docs miss `# Safety` section",
-                );
+        if let hir::ImplItemKind::Method(ref sig, ..) = item.kind {
+            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 {
+        if 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",
+            );
+        } else {
+            let def_id = cx.tcx.hir().local_def_id(hir_id);
+            let mir = cx.tcx.optimized_mir(def_id);
+            if let Some(future) = get_trait_def_id(cx, &paths::FUTURE) {
+                if implements_trait(cx, mir.return_ty(), future, &[]) {
+                    use TyKind::*;
+
+                    if let Opaque(_, subs) = mir.return_ty().kind {
+                        if let Some(ty) = subs.types().next() {
+                            if let Generator(_, subs, _) = ty.kind {
+                                if match_type(cx, subs.as_generator().return_ty(def_id, cx.tcx), &paths::RESULT) {
+                                    span_lint(
+                                        cx,
+                                        MISSING_ERRORS_DOC,
+                                        span,
+                                        "docs for function returning `Result` missing `# Errors` section",
+                                    );
+                                }
+                            }
+                        }
+                    }
+                }
             }
         }
     }
@@ -140,6 +256,7 @@ fn check_item(&mut self, cx: &EarlyContext<'_>, item: &ast::Item) {
 /// need to keep track of
 /// the spans but this function is inspired from the later.
 #[allow(clippy::cast_possible_truncation)]
+#[must_use]
 pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<(usize, Span)>) {
     // one-line comments lose their prefix
     const ONELINERS: &[&str] = &["///!", "///", "//!", "//"];
@@ -190,21 +307,29 @@ pub fn strip_doc_comment_decoration(comment: &str, span: Span) -> (String, Vec<(
     panic!("not a doc-comment: {}", comment);
 }
 
-pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, attrs: &'a [ast::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![];
 
     for attr in attrs {
-        if attr.is_sugared_doc {
-            if let Some(ref current) = attr.value_str() {
-                let current = current.to_string();
-                let (current, current_spans) = strip_doc_comment_decoration(&current, attr.span);
-                spans.extend_from_slice(&current_spans);
-                doc.push_str(&current);
-            }
+        if let AttrKind::DocComment(ref comment) = attr.kind {
+            let comment = comment.to_string();
+            let (comment, current_spans) = strip_doc_comment_decoration(&comment, attr.span);
+            spans.extend_from_slice(&current_spans);
+            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,
+            };
         }
     }
 
@@ -216,7 +341,10 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, 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();
@@ -240,16 +368,19 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>,
 }
 
 fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize>)>>(
-    cx: &EarlyContext<'_>,
+    cx: &LateContext<'_, '_>,
     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;
@@ -272,7 +403,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,
@@ -283,21 +415,24 @@ fn check_doc<'a, Events: Iterator<Item = (pulldown_cmark::Event<'a>, Range<usize
                 } else {
                     // Adjust for the beginning of the current `Event`
                     let span = span.with_lo(span.lo() + BytePos::from_usize(range.start - begin));
+
                     check_text(cx, valid_idents, &text, span);
                 }
             },
         }
     }
-    safety_header
+    headers
 }
 
-fn check_code(cx: &EarlyContext<'_>, text: &str, span: Span) {
-    if text.contains("fn main() {") {
+static LEAVE_MAIN_PATTERNS: &[&str] = &["static", "fn main() {}", "extern crate"];
+
+fn check_code(cx: &LateContext<'_, '_>, text: &str, span: Span) {
+    if text.contains("fn main() {") && !LEAVE_MAIN_PATTERNS.iter().any(|p| text.contains(p)) {
         span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
     }
 }
 
-fn check_text(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) {
+fn check_text(cx: &LateContext<'_, '_>, valid_idents: &FxHashSet<String>, text: &str, span: Span) {
     for word in text.split(|c: char| c.is_whitespace() || c == '\'') {
         // Trim punctuation as in `some comment (see foo::bar).`
         //                                                   ^^
@@ -320,7 +455,7 @@ fn check_text(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>, text: &st
     }
 }
 
-fn check_word(cx: &EarlyContext<'_>, word: &str, span: Span) {
+fn check_word(cx: &LateContext<'_, '_>, word: &str, span: Span) {
     /// Checks if a string is camel-case, i.e., contains at least two uppercase
     /// letters (`Clippy` is ok) and one lower-case letter (`NASA` is ok).
     /// Plurals are also excluded (`IDs` is ok).