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

Expand missing_errors_doc to also work on async functions #5181

Merged
merged 5 commits into from
Feb 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 39 additions & 13 deletions clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::utils::{is_entrypoint_fn, match_type, paths, return_ty, span_lint};
use crate::utils::{get_trait_def_id, implements_trait, is_entrypoint_fn, match_type, paths, return_ty, span_lint};
use if_chain::if_chain;
use itertools::Itertools;
use rustc::lint::in_external_macro;
use rustc::ty;
use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -152,11 +154,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
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, ..) => {
hir::ItemKind::Fn(ref sig, _, body_id) => {
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);
lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers, Some(body_id));
}
},
hir::ItemKind::Impl {
Expand All @@ -179,7 +181,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
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);
lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers, None);
}
}
}
Expand All @@ -189,8 +191,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
if self.in_trait_impl || in_external_macro(cx.tcx.sess, item.span) {
return;
}
if let hir::ImplItemKind::Method(ref sig, ..) = item.kind {
lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
if let hir::ImplItemKind::Method(ref sig, body_id) = item.kind {
lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers, Some(body_id));
}
}
}
Expand All @@ -201,6 +203,7 @@ fn lint_for_missing_headers<'a, 'tcx>(
span: impl Into<MultiSpan> + Copy,
sig: &hir::FnSig<'_>,
headers: DocHeaders,
body_id: Option<hir::BodyId>,
) {
if !cx.access_levels.is_exported(hir_id) {
return; // Private functions do not require doc comments
Expand All @@ -213,13 +216,36 @@ fn lint_for_missing_headers<'a, 'tcx>(
"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",
);
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 {
if_chain! {
if let Some(body_id) = body_id;
if let Some(future) = get_trait_def_id(cx, &paths::FUTURE);
let def_id = cx.tcx.hir().body_owner_def_id(body_id);
let mir = cx.tcx.optimized_mir(def_id);
let ret_ty = mir.return_ty();
if implements_trait(cx, ret_ty, future, &[]);
if let ty::Opaque(_, subs) = ret_ty.kind;
if let Some(gen) = subs.types().next();
if let ty::Generator(_, subs, _) = gen.kind;
if match_type(cx, subs.as_generator().return_ty(def_id, cx.tcx), &paths::RESULT);
then {
span_lint(
cx,
MISSING_ERRORS_DOC,
span,
"docs for function returning `Result` missing `# Errors` section",
);
}
}
}
}
}

Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments
pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"];
pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"];
pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"];
pub const FUTURE: [&str; 3] = ["std", "future", "Future"];
pub const HASH: [&str; 2] = ["hash", "Hash"];
pub const HASHMAP: [&str; 5] = ["std", "collections", "hash", "map", "HashMap"];
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/doc_errors.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// compile-flags: --edition 2018
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
#![warn(clippy::missing_errors_doc)]

use std::io;
Expand All @@ -6,22 +7,42 @@ pub fn pub_fn_missing_errors_header() -> Result<(), ()> {
unimplemented!();
}

pub async fn async_pub_fn_missing_errors_header() -> Result<(), ()> {
unimplemented!();
}

/// This is not sufficiently documented.
pub fn pub_fn_returning_io_result() -> io::Result<()> {
unimplemented!();
}

/// This is not sufficiently documented.
pub async fn async_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!();
}

/// # Errors
/// A description of the errors goes here.
pub async fn async_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!();
}

/// This function doesn't require the documentation because it is private
async fn async_priv_fn_missing_errors_header() -> Result<(), ()> {
unimplemented!();
}

pub struct Struct1;

impl Struct1 {
Expand All @@ -30,16 +51,32 @@ impl Struct1 {
unimplemented!();
}

/// This is not sufficiently documented.
pub async fn async_pub_method_missing_errors_header() -> Result<(), ()> {
unimplemented!();
}

/// # Errors
/// A description of the errors goes here.
pub fn pub_method_with_errors_header() -> Result<(), ()> {
unimplemented!();
}

/// # Errors
/// A description of the errors goes here.
pub async fn async_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!();
}

/// This function doesn't require the documentation because it is private.
async fn async_priv_method_missing_errors_header() -> Result<(), ()> {
unimplemented!();
}
}

pub trait Trait1 {
Expand Down
32 changes: 28 additions & 4 deletions tests/ui/doc_errors.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:5:1
--> $DIR/doc_errors.rs:6:1
|
LL | / pub fn pub_fn_missing_errors_header() -> Result<(), ()> {
LL | | unimplemented!();
Expand All @@ -11,24 +11,48 @@ LL | | }
error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:10:1
|
LL | / pub async fn async_pub_fn_missing_errors_header() -> Result<(), ()> {
LL | | unimplemented!();
LL | | }
| |_^

error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:15: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
--> $DIR/doc_errors.rs:20:1
|
LL | / pub async fn async_pub_fn_returning_io_result() -> io::Result<()> {
LL | | unimplemented!();
LL | | }
| |_^

error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:50: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
--> $DIR/doc_errors.rs:55:5
|
LL | / pub async fn async_pub_method_missing_errors_header() -> Result<(), ()> {
LL | | unimplemented!();
LL | | }
| |_____^

error: docs for function returning `Result` missing `# Errors` section
--> $DIR/doc_errors.rs:84:5
|
LL | fn trait_method_missing_errors_header() -> Result<(), ()>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 4 previous errors
error: aborting due to 7 previous errors