Skip to content

Commit 2893485

Browse files
committed
Add lint for misleading_use_of_ok
1 parent a71211d commit 2893485

8 files changed

+126
-5
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5347,6 +5347,7 @@ Released 2018-09-13
53475347
[`min_ident_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars
53485348
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
53495349
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
5350+
[`misleading_use_of_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#misleading_use_of_ok
53505351
[`mismatched_target_os`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_target_os
53515352
[`mismatching_type_param_order`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatching_type_param_order
53525353
[`misnamed_getters`]: https://rust-lang.github.io/rust-clippy/master/index.html#misnamed_getters

clippy_dev/src/update_lints.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -377,14 +377,14 @@ fn remove_lint_declaration(name: &str, path: &Path, lints: &mut Vec<Lint>) -> io
377377

378378
// Some lints have their own directories, delete them
379379
if path.is_dir() {
380-
fs::remove_dir_all(path).ok();
380+
let _ = fs::remove_dir_all(path);
381381
return;
382382
}
383383

384384
// Remove all related test files
385-
fs::remove_file(path.with_extension("rs")).ok();
386-
fs::remove_file(path.with_extension("stderr")).ok();
387-
fs::remove_file(path.with_extension("fixed")).ok();
385+
let _ = fs::remove_file(path.with_extension("rs"));
386+
let _ = fs::remove_file(path.with_extension("stderr"));
387+
let _ = fs::remove_file(path.with_extension("fixed"));
388388
}
389389

390390
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
427427
lint_mod_path.set_file_name(name);
428428
lint_mod_path.set_extension("rs");
429429

430-
fs::remove_file(lint_mod_path).ok();
430+
let _ = fs::remove_file(lint_mod_path);
431431
}
432432

433433
let mut content =

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
477477
crate::misc_early::UNNEEDED_WILDCARD_PATTERN_INFO,
478478
crate::misc_early::UNSEPARATED_LITERAL_SUFFIX_INFO,
479479
crate::misc_early::ZERO_PREFIXED_LITERAL_INFO,
480+
crate::misleading_use_of_ok::MISLEADING_USE_OF_OK_INFO,
480481
crate::mismatching_type_param_order::MISMATCHING_TYPE_PARAM_ORDER_INFO,
481482
crate::missing_assert_message::MISSING_ASSERT_MESSAGE_INFO,
482483
crate::missing_asserts_for_indexing::MISSING_ASSERTS_FOR_INDEXING_INFO,

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ mod min_ident_chars;
212212
mod minmax;
213213
mod misc;
214214
mod misc_early;
215+
mod misleading_use_of_ok;
215216
mod mismatching_type_param_order;
216217
mod missing_assert_message;
217218
mod missing_asserts_for_indexing;
@@ -765,6 +766,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
765766
store.register_late_pass(move |_| Box::new(missing_doc::MissingDoc::new(missing_docs_in_crate_items)));
766767
store.register_late_pass(|_| Box::new(missing_inline::MissingInline));
767768
store.register_late_pass(move |_| Box::new(exhaustive_items::ExhaustiveItems));
769+
store.register_late_pass(|_| Box::new(misleading_use_of_ok::MisleadingUseOfOk));
768770
store.register_late_pass(|_| Box::new(match_result_ok::MatchResultOk));
769771
store.register_late_pass(|_| Box::new(partialeq_ne_impl::PartialEqNeImpl));
770772
store.register_late_pass(|_| Box::new(unused_io_amount::UnusedIoAmount));
+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_context;
3+
use clippy_utils::ty::is_type_diagnostic_item;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{ExprKind, Stmt, StmtKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_session::declare_lint_pass;
8+
use rustc_span::sym;
9+
10+
declare_clippy_lint! {
11+
/// ### What it does
12+
/// Checks for `Result::ok();` when the result is not captured.
13+
///
14+
/// ### Why is this bad?
15+
/// Using `Result::ok()` may look like the result is checked like `unwrap` or `expect` would do
16+
/// but it only silences the warning caused by `#[must_use]` on the `Result`.
17+
///
18+
/// ### Example
19+
/// ```no_run
20+
/// # fn some_function() -> Result<(), ()> { Ok(()) }
21+
/// some_function().ok();
22+
/// ```
23+
/// Use instead:
24+
/// ```no_run
25+
/// # fn some_function() -> Result<(), ()> { Ok(()) }
26+
/// let _ = some_function();
27+
/// ```
28+
#[clippy::version = "1.70.0"]
29+
pub MISLEADING_USE_OF_OK,
30+
style,
31+
"Use of `.ok()` to silence `Result`'s `#[must_use]` is misleading. Use `let _ =` instead."
32+
}
33+
declare_lint_pass!(MisleadingUseOfOk => [MISLEADING_USE_OF_OK]);
34+
35+
impl LateLintPass<'_> for MisleadingUseOfOk {
36+
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
37+
if let StmtKind::Semi(expr) = stmt.kind &&
38+
let ExprKind::MethodCall(ok_path, recv, [], ..) = expr.kind //check is expr.ok() has type Result<T,E>.ok(, _)
39+
&& ok_path.ident.as_str() == "ok"
40+
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result)
41+
{
42+
let ctxt = expr.span.ctxt();
43+
let mut applicability = Applicability::MachineApplicable;
44+
let trimmed_ok = snippet_with_context(cx, recv.span, ctxt, "", &mut applicability).0;
45+
let sugg = format!("let _ = {}", trimmed_ok.trim().trim_end_matches('.'),);
46+
span_lint_and_sugg(
47+
cx,
48+
MISLEADING_USE_OF_OK,
49+
expr.span,
50+
"ignoring a result with `.ok()` is misleading",
51+
"consider using `let _ =` and removing the call to `.ok()` instead",
52+
sugg,
53+
applicability,
54+
);
55+
}
56+
}
57+
}

tests/ui/misleading_use_of_ok.fixed

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![warn(clippy::misleading_use_of_ok)]
2+
#![allow(dead_code)]
3+
4+
fn bad_style(x: &str) {
5+
let _ = x.parse::<u32>();
6+
}
7+
8+
fn good_style(x: &str) -> Option<u32> {
9+
x.parse::<u32>().ok()
10+
}
11+
12+
#[rustfmt::skip]
13+
fn strange_parse(x: &str) {
14+
let _ = x . parse::<i32>();
15+
}
16+
17+
fn main() {}

tests/ui/misleading_use_of_ok.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![warn(clippy::misleading_use_of_ok)]
2+
#![allow(dead_code)]
3+
4+
fn bad_style(x: &str) {
5+
x.parse::<u32>().ok();
6+
}
7+
8+
fn good_style(x: &str) -> Option<u32> {
9+
x.parse::<u32>().ok()
10+
}
11+
12+
#[rustfmt::skip]
13+
fn strange_parse(x: &str) {
14+
x . parse::<i32>() . ok ();
15+
}
16+
17+
fn main() {}

tests/ui/misleading_use_of_ok.stderr

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: ignoring a result with `.ok()` is misleading
2+
--> $DIR/misleading_use_of_ok.rs:5:5
3+
|
4+
LL | x.parse::<u32>().ok();
5+
| ^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::misleading-use-of-ok` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::misleading_use_of_ok)]`
9+
help: consider using `let _ =` and removing the call to `.ok()` instead
10+
|
11+
LL | let _ = x.parse::<u32>();
12+
| ~~~~~~~~~~~~~~~~~~~~~~~~
13+
14+
error: ignoring a result with `.ok()` is misleading
15+
--> $DIR/misleading_use_of_ok.rs:14:5
16+
|
17+
LL | x . parse::<i32>() . ok ();
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
|
20+
help: consider using `let _ =` and removing the call to `.ok()` instead
21+
|
22+
LL | let _ = x . parse::<i32>();
23+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
24+
25+
error: aborting due to 2 previous errors
26+

0 commit comments

Comments
 (0)