-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
10 changed files
with
264 additions
and
18 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| use clippy_utils::{ | ||
| diagnostics::span_lint_and_note, | ||
| is_from_proc_macro, is_lang_item_or_ctor, | ||
| msrvs::{self, Msrv}, | ||
| }; | ||
| use rustc_hir::{LangItem, PatKind, Stmt, StmtKind}; | ||
| use rustc_lint::{LateContext, LateLintPass, LintContext}; | ||
| use rustc_middle::lint::in_external_macro; | ||
| use rustc_session::{declare_tool_lint, impl_lint_pass}; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Checks for the usage of `Ok` in a `let...else` statement. | ||
| /// | ||
| /// ### Why is this bad? | ||
| /// It's a missed opportunity to handle the `Err` variant gracefully. Alternatively, it can be | ||
| /// propagated to the caller. | ||
| /// | ||
| /// ### Example | ||
| /// ```rust | ||
| /// # fn foo() -> Result<(), ()> { | ||
| /// # Ok(()) | ||
| /// # } | ||
| /// let Ok(foo) = foo() else { | ||
| /// return; | ||
| /// }; | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```rust | ||
| /// # fn foo() -> Result<(), ()> { | ||
| /// # Err(()) | ||
| /// # } | ||
| /// let foo = match foo() { | ||
| /// Ok(foo) => foo, | ||
| /// Err(e) => eprintln!("{e:#?}"), | ||
| /// }; | ||
| /// ``` | ||
| /// ```rust | ||
| /// # fn foo() -> Result<(), ()> { | ||
| /// # Ok(()) | ||
| /// # } | ||
| /// let foo = foo()?; | ||
| /// # Ok::<(), ()>(()) | ||
| /// ``` | ||
| #[clippy::version = "1.72.0"] | ||
| pub LET_ELSE_ON_RESULT_OK, | ||
| pedantic, | ||
| "checks for usage of `Ok` in `let...else` statements" | ||
| } | ||
| impl_lint_pass!(LetElseOnResultOk => [LET_ELSE_ON_RESULT_OK]); | ||
|
|
||
| pub struct LetElseOnResultOk { | ||
| msrv: Msrv, | ||
| } | ||
|
|
||
| impl LetElseOnResultOk { | ||
| #[must_use] | ||
| pub fn new(msrv: Msrv) -> Self { | ||
| Self { msrv } | ||
| } | ||
| } | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for LetElseOnResultOk { | ||
| fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) { | ||
| if !self.msrv.meets(msrvs::LET_ELSE) || in_external_macro(cx.sess(), stmt.span) { | ||
| return; | ||
| } | ||
|
|
||
| if let StmtKind::Local(local) = stmt.kind && let Some(els) = local.els { | ||
| let spans = { | ||
| let mut spans = vec![]; | ||
| local.pat.walk_always(|pat| { | ||
| if let PatKind::TupleStruct(qpath, _, _) = pat.kind | ||
| && let Some(def_id) = cx.qpath_res(&qpath, pat.hir_id).opt_def_id() | ||
| && is_lang_item_or_ctor(cx, def_id, LangItem::ResultOk) | ||
| { | ||
| spans.push(pat.span); | ||
| } | ||
| }); | ||
| spans | ||
| }; | ||
|
|
||
| if !spans.is_empty() && is_from_proc_macro(cx, els) { | ||
| return; | ||
| }; | ||
|
|
||
| for span in spans { | ||
| span_lint_and_note( | ||
| cx, | ||
| LET_ELSE_ON_RESULT_OK, | ||
| span, | ||
| "usage of `let...else` on `Err`", | ||
| None, | ||
| "consider handling the `Err` variant gracefully or propagating it to the caller", | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| extract_msrv_attr!(LateContext); | ||
| } | ||
|
|
||
| // TODO: Add MSRV level to `clippy_utils/src/msrvs.rs` if needed. | ||
| // TODO: Add MSRV test to `tests/ui/min_rust_version_attr.rs`. | ||
| // TODO: Update msrv config comment in `clippy_lints/src/utils/conf.rs` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| //@aux-build:proc_macros.rs | ||
| #![allow(clippy::redundant_pattern_matching, unused)] | ||
| #![warn(clippy::let_else_on_result_ok)] | ||
|
|
||
| #[macro_use] | ||
| extern crate proc_macros; | ||
|
|
||
| struct A(Result<&'static A, ()>); | ||
|
|
||
| enum AnEnum { | ||
| A(Result<&'static A, ()>), | ||
| } | ||
|
|
||
| fn a() -> Result<(), ()> { | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn a_constructor() -> A { | ||
| todo!(); | ||
| } | ||
|
|
||
| fn an_enum_constructor() -> AnEnum { | ||
| todo!(); | ||
| } | ||
|
|
||
| fn main() { | ||
| // Lint | ||
| let Ok(_) = a() else { | ||
| return; | ||
| }; | ||
| let (Ok(_), true) = (a(), true) else { | ||
| return; | ||
| }; | ||
| let [Ok(_), Ok(_)] = [a(), Err(())] else { | ||
| return; | ||
| }; | ||
| let A(Ok(A(Ok(A(Ok(A(Ok(_)))))))) = a_constructor() else { | ||
| return; | ||
| }; | ||
| let AnEnum::A(Ok(A(Err(_)))) = an_enum_constructor() else { | ||
| return; | ||
| }; | ||
| // Don't lint | ||
| let Err(_) = a() else { | ||
| return; | ||
| }; | ||
| match a() { | ||
| Ok(a) => a, | ||
| Err(e) => eprintln!("{e:#?}"), | ||
| }; | ||
| external! { | ||
| let Ok(_) = a() else { | ||
| return; | ||
| }; | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:28:9 | ||
| | | ||
| LL | let Ok(_) = a() else { | ||
| | ^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
| = note: `-D clippy::let-else-on-result-ok` implied by `-D warnings` | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:31:10 | ||
| | | ||
| LL | let (Ok(_), true) = (a(), true) else { | ||
| | ^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:34:10 | ||
| | | ||
| LL | let [Ok(_), Ok(_)] = [a(), Err(())] else { | ||
| | ^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:34:17 | ||
| | | ||
| LL | let [Ok(_), Ok(_)] = [a(), Err(())] else { | ||
| | ^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:37:11 | ||
| | | ||
| LL | let A(Ok(A(Ok(A(Ok(A(Ok(_)))))))) = a_constructor() else { | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:37:16 | ||
| | | ||
| LL | let A(Ok(A(Ok(A(Ok(A(Ok(_)))))))) = a_constructor() else { | ||
| | ^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:37:21 | ||
| | | ||
| LL | let A(Ok(A(Ok(A(Ok(A(Ok(_)))))))) = a_constructor() else { | ||
| | ^^^^^^^^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:37:26 | ||
| | | ||
| LL | let A(Ok(A(Ok(A(Ok(A(Ok(_)))))))) = a_constructor() else { | ||
| | ^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: usage of `let...else` on `Err` | ||
| --> $DIR/let_else_on_result_ok.rs:40:19 | ||
| | | ||
| LL | let AnEnum::A(Ok(A(Err(_)))) = an_enum_constructor() else { | ||
| | ^^^^^^^^^^^^^ | ||
| | | ||
| = note: consider handling the `Err` variant gracefully or propagating it to the caller | ||
|
|
||
| error: aborting due to 9 previous errors | ||
|
|