mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-25 14:10:32 +00:00
Add lint for unused_result_ok
This commit is contained in:
parent
057c4ae287
commit
182c26891e
8 changed files with 200 additions and 5 deletions
|
@ -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
|
||||
|
|
|
@ -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 =
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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));
|
||||
|
|
59
clippy_lints/src/unused_result_ok.rs
Normal file
59
clippy_lints/src/unused_result_ok.rs
Normal file
|
@ -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,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
40
tests/ui/unused_result_ok.fixed
Normal file
40
tests/ui/unused_result_ok.fixed
Normal file
|
@ -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();
|
||||
};
|
||||
}
|
40
tests/ui/unused_result_ok.rs
Normal file
40
tests/ui/unused_result_ok.rs
Normal file
|
@ -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();
|
||||
};
|
||||
}
|
52
tests/ui/unused_result_ok.stderr
Normal file
52
tests/ui/unused_result_ok.stderr
Normal file
|
@ -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
|
||||
|
Loading…
Reference in a new issue