diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_dev/src/update_lints.rs | 10 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/unused_result_ok.rs | 59 | ||||
| -rw-r--r-- | tests/ui/unused_result_ok.fixed | 40 | ||||
| -rw-r--r-- | tests/ui/unused_result_ok.rs | 40 | ||||
| -rw-r--r-- | tests/ui/unused_result_ok.stderr | 52 |
8 files changed, 200 insertions, 5 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 55281f3cbec..b6bbe6f6e49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5955,6 +5955,7 @@ Released 2018-09-13 [`unused_io_amount`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_io_amount [`unused_label`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_label [`unused_peekable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_peekable +[`unused_result_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_result_ok [`unused_rounding`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_rounding [`unused_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_self [`unused_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#unused_unit diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 45353901c98..757fe98c57d 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -377,14 +377,14 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io // Some lints have their own directories, delete them if path.is_dir() { - fs::remove_dir_all(path).ok(); + let _ = fs::remove_dir_all(path); return; } // Remove all related test files - fs::remove_file(path.with_extension("rs")).ok(); - fs::remove_file(path.with_extension("stderr")).ok(); - fs::remove_file(path.with_extension("fixed")).ok(); + let _ = fs::remove_file(path.with_extension("rs")); + let _ = fs::remove_file(path.with_extension("stderr")); + let _ = fs::remove_file(path.with_extension("fixed")); } fn remove_impl_lint_pass(lint_name_upper: &str, content: &mut String) { @@ -427,7 +427,7 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io lint_mod_path.set_file_name(name); lint_mod_path.set_extension("rs"); - fs::remove_file(lint_mod_path).ok(); + let _ = fs::remove_file(lint_mod_path); } let mut content = diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index eabc67601a2..73912841278 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -740,6 +740,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::unused_async::UNUSED_ASYNC_INFO, crate::unused_io_amount::UNUSED_IO_AMOUNT_INFO, crate::unused_peekable::UNUSED_PEEKABLE_INFO, + crate::unused_result_ok::UNUSED_RESULT_OK_INFO, crate::unused_rounding::UNUSED_ROUNDING_INFO, crate::unused_self::UNUSED_SELF_INFO, crate::unused_unit::UNUSED_UNIT_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 25cd7610400..cfd456e08ed 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -371,6 +371,7 @@ mod unsafe_removed_from_name; mod unused_async; mod unused_io_amount; mod unused_peekable; +mod unused_result_ok; mod unused_rounding; mod unused_self; mod unused_unit; @@ -668,6 +669,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(conf))); store.register_late_pass(|_| Box::new(missing_inline::MissingInline)); store.register_late_pass(move |_| Box::new(exhaustive_items::ExhaustiveItems)); + store.register_late_pass(|_| Box::new(unused_result_ok::UnusedResultOk)); store.register_late_pass(|_| Box::new(match_result_ok::MatchResultOk)); store.register_late_pass(|_| Box::new(partialeq_ne_impl::PartialEqNeImpl)); store.register_late_pass(|_| Box::new(unused_io_amount::UnusedIoAmount)); diff --git a/clippy_lints/src/unused_result_ok.rs b/clippy_lints/src/unused_result_ok.rs new file mode 100644 index 00000000000..297288db0a5 --- /dev/null +++ b/clippy_lints/src/unused_result_ok.rs @@ -0,0 +1,59 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_context; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_errors::Applicability; +use rustc_hir::{ExprKind, Stmt, StmtKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_session::declare_lint_pass; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for calls to `Result::ok()` without using the returned `Option`. + /// + /// ### Why is this bad? + /// Using `Result::ok()` may look like the result is checked like `unwrap` or `expect` would do + /// but it only silences the warning caused by `#[must_use]` on the `Result`. + /// + /// ### Example + /// ```no_run + /// # fn some_function() -> Result<(), ()> { Ok(()) } + /// some_function().ok(); + /// ``` + /// Use instead: + /// ```no_run + /// # fn some_function() -> Result<(), ()> { Ok(()) } + /// let _ = some_function(); + /// ``` + #[clippy::version = "1.70.0"] + pub UNUSED_RESULT_OK, + restriction, + "Use of `.ok()` to silence `Result`'s `#[must_use]` is misleading. Use `let _ =` instead." +} +declare_lint_pass!(UnusedResultOk => [UNUSED_RESULT_OK]); + +impl LateLintPass<'_> for UnusedResultOk { + fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) { + if let StmtKind::Semi(expr) = stmt.kind + && let ExprKind::MethodCall(ok_path, recv, [], ..) = expr.kind //check is expr.ok() has type Result<T,E>.ok(, _) + && ok_path.ident.as_str() == "ok" + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result) + && !in_external_macro(cx.sess(), stmt.span) + { + let ctxt = expr.span.ctxt(); + let mut applicability = Applicability::MaybeIncorrect; + let snippet = snippet_with_context(cx, recv.span, ctxt, "", &mut applicability).0; + let sugg = format!("let _ = {snippet}"); + span_lint_and_sugg( + cx, + UNUSED_RESULT_OK, + expr.span, + "ignoring a result with `.ok()` is misleading", + "consider using `let _ =` and removing the call to `.ok()` instead", + sugg, + applicability, + ); + } + } +} diff --git a/tests/ui/unused_result_ok.fixed b/tests/ui/unused_result_ok.fixed new file mode 100644 index 00000000000..e78fde5c9e3 --- /dev/null +++ b/tests/ui/unused_result_ok.fixed @@ -0,0 +1,40 @@ +//@aux-build:proc_macros.rs +#![warn(clippy::unused_result_ok)] +#![allow(dead_code)] + +#[macro_use] +extern crate proc_macros; + +fn bad_style(x: &str) { + let _ = x.parse::<u32>(); +} + +fn good_style(x: &str) -> Option<u32> { + x.parse::<u32>().ok() +} + +#[rustfmt::skip] +fn strange_parse(x: &str) { + let _ = x . parse::<i32>(); +} + +macro_rules! v { + () => { + Ok::<(), ()>(()) + }; +} + +macro_rules! w { + () => { + let _ = Ok::<(), ()>(()); + }; +} + +fn main() { + let _ = v!(); + w!(); + + external! { + Ok::<(),()>(()).ok(); + }; +} diff --git a/tests/ui/unused_result_ok.rs b/tests/ui/unused_result_ok.rs new file mode 100644 index 00000000000..117d64c4cec --- /dev/null +++ b/tests/ui/unused_result_ok.rs @@ -0,0 +1,40 @@ +//@aux-build:proc_macros.rs +#![warn(clippy::unused_result_ok)] +#![allow(dead_code)] + +#[macro_use] +extern crate proc_macros; + +fn bad_style(x: &str) { + x.parse::<u32>().ok(); +} + +fn good_style(x: &str) -> Option<u32> { + x.parse::<u32>().ok() +} + +#[rustfmt::skip] +fn strange_parse(x: &str) { + x . parse::<i32>() . ok (); +} + +macro_rules! v { + () => { + Ok::<(), ()>(()) + }; +} + +macro_rules! w { + () => { + Ok::<(), ()>(()).ok(); + }; +} + +fn main() { + v!().ok(); + w!(); + + external! { + Ok::<(),()>(()).ok(); + }; +} diff --git a/tests/ui/unused_result_ok.stderr b/tests/ui/unused_result_ok.stderr new file mode 100644 index 00000000000..241e0c71261 --- /dev/null +++ b/tests/ui/unused_result_ok.stderr @@ -0,0 +1,52 @@ +error: ignoring a result with `.ok()` is misleading + --> tests/ui/unused_result_ok.rs:9:5 + | +LL | x.parse::<u32>().ok(); + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::unused-result-ok` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unused_result_ok)]` +help: consider using `let _ =` and removing the call to `.ok()` instead + | +LL | let _ = x.parse::<u32>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~ + +error: ignoring a result with `.ok()` is misleading + --> tests/ui/unused_result_ok.rs:18:5 + | +LL | x . parse::<i32>() . ok (); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `let _ =` and removing the call to `.ok()` instead + | +LL | let _ = x . parse::<i32>(); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: ignoring a result with `.ok()` is misleading + --> tests/ui/unused_result_ok.rs:34:5 + | +LL | v!().ok(); + | ^^^^^^^^^ + | +help: consider using `let _ =` and removing the call to `.ok()` instead + | +LL | let _ = v!(); + | ~~~~~~~~~~~~ + +error: ignoring a result with `.ok()` is misleading + --> tests/ui/unused_result_ok.rs:29:9 + | +LL | Ok::<(), ()>(()).ok(); + | ^^^^^^^^^^^^^^^^^^^^^ +... +LL | w!(); + | ---- in this macro invocation + | + = note: this error originates in the macro `w` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider using `let _ =` and removing the call to `.ok()` instead + | +LL | let _ = Ok::<(), ()>(()); + | ~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 4 previous errors + |