From 79f93a655a9a3849c0653a72a5b547776a1522b3 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Wed, 14 Jun 2023 22:07:31 +0900 Subject: [PATCH 1/5] missing_panics_doc: pickup expect method --- clippy_lints/src/doc.rs | 4 ++-- tests/ui/doc/missing_panics_doc.rs | 31 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tests/ui/doc/missing_panics_doc.rs diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 0617c3ea5..11de7e606 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -916,8 +916,8 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> { } } - // check for `unwrap` - if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { + // check for `unwrap` and `expect` both `Option` and `Result` + if let Some(arglists) = method_chain_args(expr, &["unwrap"]).or(method_chain_args(expr, &["expect"])) { let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs(); if is_type_diagnostic_item(self.cx, receiver_ty, sym::Option) || is_type_diagnostic_item(self.cx, receiver_ty, sym::Result) diff --git a/tests/ui/doc/missing_panics_doc.rs b/tests/ui/doc/missing_panics_doc.rs new file mode 100644 index 000000000..5cb8a9691 --- /dev/null +++ b/tests/ui/doc/missing_panics_doc.rs @@ -0,0 +1,31 @@ +#![warn(clippy::missing_panics_doc)] + +pub fn option_unwrap(v: &[T]) -> &T { + let o: Option<&T> = v.last(); + o.unwrap() +} + +pub fn option_expect(v: &[T]) -> &T { + let o: Option<&T> = v.last(); + o.expect("passed an empty thing") +} + +pub fn result_unwrap(v: &[T]) -> &T { + let res: Result<&T, &str> = v.last().ok_or("oh noes"); + res.unwrap() +} + +pub fn result_expect(v: &[T]) -> &T { + let res: Result<&T, &str> = v.last().ok_or("oh noes"); + res.expect("passed an empty thing") +} + +pub fn last_unwrap(v: &[u32]) -> u32 { + *v.last().unwrap() +} + +pub fn last_expect(v: &[u32]) -> u32 { + *v.last().expect("passed an empty thing") +} + +fn main() {} From 2fa72c7dfe89febb091699fe5d43fb4893d38d7f Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Wed, 14 Jun 2023 22:16:20 +0900 Subject: [PATCH 2/5] bless --- tests/ui/doc/missing_panics_doc.stderr | 75 ++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tests/ui/doc/missing_panics_doc.stderr diff --git a/tests/ui/doc/missing_panics_doc.stderr b/tests/ui/doc/missing_panics_doc.stderr new file mode 100644 index 000000000..f3e613bc7 --- /dev/null +++ b/tests/ui/doc/missing_panics_doc.stderr @@ -0,0 +1,75 @@ +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:3:1 + | +LL | pub fn option_unwrap(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:5:5 + | +LL | o.unwrap() + | ^^^^^^^^^^ + = note: `-D clippy::missing-panics-doc` implied by `-D warnings` + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:8:1 + | +LL | pub fn option_expect(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:10:5 + | +LL | o.expect("passed an empty thing") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:13:1 + | +LL | pub fn result_unwrap(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:15:5 + | +LL | res.unwrap() + | ^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:18:1 + | +LL | pub fn result_expect(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:20:5 + | +LL | res.expect("passed an empty thing") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:23:1 + | +LL | pub fn last_unwrap(v: &[u32]) -> u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:24:6 + | +LL | *v.last().unwrap() + | ^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:27:1 + | +LL | pub fn last_expect(v: &[u32]) -> u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:28:6 + | +LL | *v.last().expect("passed an empty thing") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + From aff9c01ab99dd7bb4a185df3a08e27f364fecf50 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Wed, 14 Jun 2023 22:26:10 +0900 Subject: [PATCH 3/5] address or allow clippy::missing_panics_doc in clippy-dev --- clippy_dev/src/fmt.rs | 1 + clippy_dev/src/lib.rs | 6 ++++++ clippy_dev/src/new_lint.rs | 1 + 3 files changed, 8 insertions(+) diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs index 256231441..ee559d45d 100644 --- a/clippy_dev/src/fmt.rs +++ b/clippy_dev/src/fmt.rs @@ -35,6 +35,7 @@ struct FmtContext { } // the "main" function of cargo dev fmt +#[allow(clippy::missing_panics_doc)] pub fn run(check: bool, verbose: bool) { fn try_run(context: &FmtContext) -> Result { let mut success = true; diff --git a/clippy_dev/src/lib.rs b/clippy_dev/src/lib.rs index 56a269288..8aaa029f7 100644 --- a/clippy_dev/src/lib.rs +++ b/clippy_dev/src/lib.rs @@ -29,6 +29,10 @@ static CARGO_CLIPPY_EXE: &str = "cargo-clippy"; static CARGO_CLIPPY_EXE: &str = "cargo-clippy.exe"; /// Returns the path to the `cargo-clippy` binary +/// +/// # Panics +/// +/// Panics if the path of current executable could not be retrieved. #[must_use] pub fn cargo_clippy_path() -> PathBuf { let mut path = std::env::current_exe().expect("failed to get current executable name"); @@ -61,6 +65,8 @@ pub fn clippy_project_root() -> PathBuf { panic!("error: Can't determine root of project. Please run inside a Clippy working dir."); } +/// # Panics +/// Panics if given command result was failed. pub fn exit_if_err(status: io::Result) { match status.expect("failed to run command").code() { Some(0) => {}, diff --git a/clippy_dev/src/new_lint.rs b/clippy_dev/src/new_lint.rs index f970a3272..f0ccdb0fe 100644 --- a/clippy_dev/src/new_lint.rs +++ b/clippy_dev/src/new_lint.rs @@ -36,6 +36,7 @@ impl Context for io::Result { /// # Errors /// /// This function errors out if the files couldn't be created or written to. +#[allow(clippy::missing_panics_doc)] pub fn create( pass: &String, lint_name: Option<&String>, From 73cd2cde8b96b31b423a33e06dafa0e71e9f2589 Mon Sep 17 00:00:00 2001 From: Kisaragi Marine Date: Fri, 16 Jun 2023 16:37:19 +0900 Subject: [PATCH 4/5] merge test --- tests/ui/doc/missing_panics_doc.rs | 31 ----------- tests/ui/doc/missing_panics_doc.stderr | 75 -------------------------- tests/ui/missing_panics_doc.rs | 32 +++++++++++ tests/ui/missing_panics_doc.stderr | 74 ++++++++++++++++++++++++- 4 files changed, 105 insertions(+), 107 deletions(-) delete mode 100644 tests/ui/doc/missing_panics_doc.rs delete mode 100644 tests/ui/doc/missing_panics_doc.stderr diff --git a/tests/ui/doc/missing_panics_doc.rs b/tests/ui/doc/missing_panics_doc.rs deleted file mode 100644 index 5cb8a9691..000000000 --- a/tests/ui/doc/missing_panics_doc.rs +++ /dev/null @@ -1,31 +0,0 @@ -#![warn(clippy::missing_panics_doc)] - -pub fn option_unwrap(v: &[T]) -> &T { - let o: Option<&T> = v.last(); - o.unwrap() -} - -pub fn option_expect(v: &[T]) -> &T { - let o: Option<&T> = v.last(); - o.expect("passed an empty thing") -} - -pub fn result_unwrap(v: &[T]) -> &T { - let res: Result<&T, &str> = v.last().ok_or("oh noes"); - res.unwrap() -} - -pub fn result_expect(v: &[T]) -> &T { - let res: Result<&T, &str> = v.last().ok_or("oh noes"); - res.expect("passed an empty thing") -} - -pub fn last_unwrap(v: &[u32]) -> u32 { - *v.last().unwrap() -} - -pub fn last_expect(v: &[u32]) -> u32 { - *v.last().expect("passed an empty thing") -} - -fn main() {} diff --git a/tests/ui/doc/missing_panics_doc.stderr b/tests/ui/doc/missing_panics_doc.stderr deleted file mode 100644 index f3e613bc7..000000000 --- a/tests/ui/doc/missing_panics_doc.stderr +++ /dev/null @@ -1,75 +0,0 @@ -error: docs for function which may panic missing `# Panics` section - --> $DIR/missing_panics_doc.rs:3:1 - | -LL | pub fn option_unwrap(v: &[T]) -> &T { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: first possible panic found here - --> $DIR/missing_panics_doc.rs:5:5 - | -LL | o.unwrap() - | ^^^^^^^^^^ - = note: `-D clippy::missing-panics-doc` implied by `-D warnings` - -error: docs for function which may panic missing `# Panics` section - --> $DIR/missing_panics_doc.rs:8:1 - | -LL | pub fn option_expect(v: &[T]) -> &T { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: first possible panic found here - --> $DIR/missing_panics_doc.rs:10:5 - | -LL | o.expect("passed an empty thing") - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: docs for function which may panic missing `# Panics` section - --> $DIR/missing_panics_doc.rs:13:1 - | -LL | pub fn result_unwrap(v: &[T]) -> &T { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: first possible panic found here - --> $DIR/missing_panics_doc.rs:15:5 - | -LL | res.unwrap() - | ^^^^^^^^^^^^ - -error: docs for function which may panic missing `# Panics` section - --> $DIR/missing_panics_doc.rs:18:1 - | -LL | pub fn result_expect(v: &[T]) -> &T { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: first possible panic found here - --> $DIR/missing_panics_doc.rs:20:5 - | -LL | res.expect("passed an empty thing") - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: docs for function which may panic missing `# Panics` section - --> $DIR/missing_panics_doc.rs:23:1 - | -LL | pub fn last_unwrap(v: &[u32]) -> u32 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: first possible panic found here - --> $DIR/missing_panics_doc.rs:24:6 - | -LL | *v.last().unwrap() - | ^^^^^^^^^^^^^^^^^ - -error: docs for function which may panic missing `# Panics` section - --> $DIR/missing_panics_doc.rs:27:1 - | -LL | pub fn last_expect(v: &[u32]) -> u32 { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -note: first possible panic found here - --> $DIR/missing_panics_doc.rs:28:6 - | -LL | *v.last().expect("passed an empty thing") - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 6 previous errors - diff --git a/tests/ui/missing_panics_doc.rs b/tests/ui/missing_panics_doc.rs index 10aa436d6..d544a0b1a 100644 --- a/tests/ui/missing_panics_doc.rs +++ b/tests/ui/missing_panics_doc.rs @@ -151,3 +151,35 @@ pub fn debug_assertions() { debug_assert_eq!(1, 2); debug_assert_ne!(1, 2); } + +// all function must be triggered the lint. +// `pub` is required, because the lint does not consider unreachable items +pub mod issue10240 { + pub fn option_unwrap(v: &[T]) -> &T { + let o: Option<&T> = v.last(); + o.unwrap() + } + + pub fn option_expect(v: &[T]) -> &T { + let o: Option<&T> = v.last(); + o.expect("passed an empty thing") + } + + pub fn result_unwrap(v: &[T]) -> &T { + let res: Result<&T, &str> = v.last().ok_or("oh noes"); + res.unwrap() + } + + pub fn result_expect(v: &[T]) -> &T { + let res: Result<&T, &str> = v.last().ok_or("oh noes"); + res.expect("passed an empty thing") + } + + pub fn last_unwrap(v: &[u32]) -> u32 { + *v.last().unwrap() + } + + pub fn last_expect(v: &[u32]) -> u32 { + *v.last().expect("passed an empty thing") + } +} diff --git a/tests/ui/missing_panics_doc.stderr b/tests/ui/missing_panics_doc.stderr index 183c262ce..39d97f083 100644 --- a/tests/ui/missing_panics_doc.stderr +++ b/tests/ui/missing_panics_doc.stderr @@ -83,5 +83,77 @@ note: first possible panic found here LL | assert_ne!(x, 0); | ^^^^^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:158:5 + | +LL | pub fn option_unwrap(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:160:9 + | +LL | o.unwrap() + | ^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:163:5 + | +LL | pub fn option_expect(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:165:9 + | +LL | o.expect("passed an empty thing") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:168:5 + | +LL | pub fn result_unwrap(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:170:9 + | +LL | res.unwrap() + | ^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:173:5 + | +LL | pub fn result_expect(v: &[T]) -> &T { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:175:9 + | +LL | res.expect("passed an empty thing") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:178:5 + | +LL | pub fn last_unwrap(v: &[u32]) -> u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:179:10 + | +LL | *v.last().unwrap() + | ^^^^^^^^^^^^^^^^^ + +error: docs for function which may panic missing `# Panics` section + --> $DIR/missing_panics_doc.rs:182:5 + | +LL | pub fn last_expect(v: &[u32]) -> u32 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: first possible panic found here + --> $DIR/missing_panics_doc.rs:183:10 + | +LL | *v.last().expect("passed an empty thing") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 13 previous errors From 73c0c145269a6226f46047c2e9cf5e5810a08dc7 Mon Sep 17 00:00:00 2001 From: Kisaragi <48310258+KisaragiEffective@users.noreply.github.com> Date: Fri, 16 Jun 2023 16:39:09 +0900 Subject: [PATCH 5/5] improve grammer in comment sentence Co-authored-by: dswij --- clippy_lints/src/doc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 11de7e606..815495a7b 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -916,7 +916,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> { } } - // check for `unwrap` and `expect` both `Option` and `Result` + // check for `unwrap` and `expect` for both `Option` and `Result` if let Some(arglists) = method_chain_args(expr, &["unwrap"]).or(method_chain_args(expr, &["expect"])) { let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs(); if is_type_diagnostic_item(self.cx, receiver_ty, sym::Option)