From 05e05eaed7f512bf2a1f7f236fc4b484d4a52aa5 Mon Sep 17 00:00:00 2001 From: Max Baumann Date: Fri, 18 Mar 2022 01:04:33 +0100 Subject: refactor: rename lint to or_then_unwrap --- CHANGELOG.md | 2 +- clippy_lints/src/lib.register_all.rs | 2 +- clippy_lints/src/lib.register_complexity.rs | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.rs | 4 +- clippy_lints/src/or_then_unwrap.rs | 102 ++++++++++++++++++++++++++++ clippy_lints/src/use_unwrap_or.rs | 102 ---------------------------- tests/ui/or_then_unwrap.rs | 45 ++++++++++++ tests/ui/or_then_unwrap.stderr | 19 ++++++ tests/ui/use_unwrap_or.rs | 45 ------------ tests/ui/use_unwrap_or.stderr | 19 ------ 11 files changed, 172 insertions(+), 172 deletions(-) create mode 100644 clippy_lints/src/or_then_unwrap.rs delete mode 100644 clippy_lints/src/use_unwrap_or.rs create mode 100644 tests/ui/or_then_unwrap.rs create mode 100644 tests/ui/or_then_unwrap.stderr delete mode 100644 tests/ui/use_unwrap_or.rs delete mode 100644 tests/ui/use_unwrap_or.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 9475c674983..b45be38bf4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3538,7 +3538,7 @@ Released 2018-09-13 [`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self -[`use_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_unwrap_or +[`or_then_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#or_then_unwrap [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref [`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index a42f4cb6d70..5c3e352d09f 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -310,7 +310,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(unwrap::PANICKING_UNWRAP), LintId::of(unwrap::UNNECESSARY_UNWRAP), LintId::of(upper_case_acronyms::UPPER_CASE_ACRONYMS), - LintId::of(use_unwrap_or::USE_UNWRAP_OR), + LintId::of(or_then_unwrap::OR_THEN_UNWRAP), LintId::of(useless_conversion::USELESS_CONVERSION), LintId::of(vec::USELESS_VEC), LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index 94ff53c2a60..45ad1520396 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -94,7 +94,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(unit_types::UNIT_ARG), LintId::of(unnecessary_sort_by::UNNECESSARY_SORT_BY), LintId::of(unwrap::UNNECESSARY_UNWRAP), - LintId::of(use_unwrap_or::USE_UNWRAP_OR), + LintId::of(or_then_unwrap::OR_THEN_UNWRAP), LintId::of(useless_conversion::USELESS_CONVERSION), LintId::of(zero_div_zero::ZERO_DIVIDED_BY_ZERO), ]) diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index d1e13647e7e..aad0c8735ee 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -528,7 +528,7 @@ store.register_lints(&[ unwrap_in_result::UNWRAP_IN_RESULT, upper_case_acronyms::UPPER_CASE_ACRONYMS, use_self::USE_SELF, - use_unwrap_or::USE_UNWRAP_OR, + or_then_unwrap::OR_THEN_UNWRAP, useless_conversion::USELESS_CONVERSION, vec::USELESS_VEC, vec_init_then_push::VEC_INIT_THEN_PUSH, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9c9e9643bc9..fdab58935ef 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -322,6 +322,7 @@ mod only_used_in_recursion; mod open_options; mod option_env_unwrap; mod option_if_let_else; +mod or_then_unwrap; mod overflow_check_conditional; mod panic_in_result_fn; mod panic_unimplemented; @@ -394,7 +395,6 @@ mod unwrap; mod unwrap_in_result; mod upper_case_acronyms; mod use_self; -mod use_unwrap_or; mod useless_conversion; mod vec; mod vec_init_then_push; @@ -867,7 +867,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ignore_publish: cargo_ignore_publish, }) }); - store.register_late_pass(|| Box::new(use_unwrap_or::UseUnwrapOr)); + store.register_late_pass(|| Box::new(or_then_unwrap::OrThenUnwrap)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/or_then_unwrap.rs b/clippy_lints/src/or_then_unwrap.rs new file mode 100644 index 00000000000..d467fbdfe02 --- /dev/null +++ b/clippy_lints/src/or_then_unwrap.rs @@ -0,0 +1,102 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::ty::is_type_diagnostic_item; +use if_chain::if_chain; +use rustc_hir::{Expr, ExprKind, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `.or(…).unwrap()` calls to Options and Results. + /// + /// ### Why is this bad? + /// You should use `.unwrap_or(…)` instead for clarity. + /// + /// ### Example + /// ```rust + /// # let fallback = "fallback"; + /// // Result + /// # type Error = &'static str; + /// # let result: Result<&str, Error> = Err("error"); + /// let value = result.or::(Ok(fallback)).unwrap(); + /// + /// // Option + /// # let option: Option<&str> = None; + /// let value = option.or(Some(fallback)).unwrap(); + /// ``` + /// Use instead: + /// ```rust + /// # let fallback = "fallback"; + /// // Result + /// # let result: Result<&str, &str> = Err("error"); + /// let value = result.unwrap_or(fallback); + /// + /// // Option + /// # let option: Option<&str> = None; + /// let value = option.unwrap_or(fallback); + /// ``` + #[clippy::version = "1.61.0"] + pub OR_THEN_UNWRAP, + complexity, + "checks for `.or(…).unwrap()` calls to Options and Results." +} +declare_lint_pass!(OrThenUnwrap => [OR_THEN_UNWRAP]); + +impl<'tcx> LateLintPass<'tcx> for OrThenUnwrap { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // look for x.or().unwrap() + if_chain! { + if let ExprKind::MethodCall(path, [unwrap_self], unwrap_span) = &expr.kind; + if path.ident.name == sym::unwrap; + if let ExprKind::MethodCall(caller_path, [or_self, or_arg], or_span) = &unwrap_self.kind; + if caller_path.ident.name == sym::or; + then { + let ty = cx.typeck_results().expr_ty(or_self); // get type of x (we later check if it's Option or Result) + let title; + + if is_type_diagnostic_item(cx, ty, sym::Option) { + title = ".or(Some(…)).unwrap() found"; + if !is(or_arg, "Some") { + return; + } + } else if is_type_diagnostic_item(cx, ty, sym::Result) { + title = ".or(Ok(…)).unwrap() found"; + if !is(or_arg, "Ok") { + return; + } + } else { + // Someone has implemented a struct with .or(...).unwrap() chaining, + // but it's not an Option or a Result, so bail + return; + } + + span_lint_and_help( + cx, + OR_THEN_UNWRAP, + or_span.to(*unwrap_span), + title, + None, + "use `unwrap_or()` instead" + ); + } + } + } +} + +/// is expr a Call to name? +/// name might be "Some", "Ok", "Err", etc. +fn is<'a>(expr: &Expr<'a>, name: &str) -> bool { + if_chain! { + if let ExprKind::Call(some_expr, _some_args) = expr.kind; + if let ExprKind::Path(QPath::Resolved(_, path)) = &some_expr.kind; + if let Some(path_segment) = path.segments.first(); + if path_segment.ident.name.as_str() == name; + then { + true + } + else { + false + } + } +} diff --git a/clippy_lints/src/use_unwrap_or.rs b/clippy_lints/src/use_unwrap_or.rs deleted file mode 100644 index 3e40014f50f..00000000000 --- a/clippy_lints/src/use_unwrap_or.rs +++ /dev/null @@ -1,102 +0,0 @@ -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::is_type_diagnostic_item; -use if_chain::if_chain; -use rustc_hir::{Expr, ExprKind, QPath}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for `.or(…).unwrap()` calls to Options and Results. - /// - /// ### Why is this bad? - /// You should use `.unwrap_or(…)` instead for clarity. - /// - /// ### Example - /// ```rust - /// # let fallback = "fallback"; - /// // Result - /// # type Error = &'static str; - /// # let result: Result<&str, Error> = Err("error"); - /// let value = result.or::(Ok(fallback)).unwrap(); - /// - /// // Option - /// # let option: Option<&str> = None; - /// let value = option.or(Some(fallback)).unwrap(); - /// ``` - /// Use instead: - /// ```rust - /// # let fallback = "fallback"; - /// // Result - /// # let result: Result<&str, &str> = Err("error"); - /// let value = result.unwrap_or(fallback); - /// - /// // Option - /// # let option: Option<&str> = None; - /// let value = option.unwrap_or(fallback); - /// ``` - #[clippy::version = "1.61.0"] - pub USE_UNWRAP_OR, - complexity, - "checks for `.or(…).unwrap()` calls to Options and Results." -} -declare_lint_pass!(UseUnwrapOr => [USE_UNWRAP_OR]); - -impl<'tcx> LateLintPass<'tcx> for UseUnwrapOr { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - // look for x.or().unwrap() - if_chain! { - if let ExprKind::MethodCall(path, [unwrap_self], unwrap_span) = &expr.kind; - if path.ident.name == sym::unwrap; - if let ExprKind::MethodCall(caller_path, [or_self, or_arg], or_span) = &unwrap_self.kind; - if caller_path.ident.name == sym::or; - then { - let ty = cx.typeck_results().expr_ty(or_self); // get type of x (we later check if it's Option or Result) - let title; - - if is_type_diagnostic_item(cx, ty, sym::Option) { - title = ".or(Some(…)).unwrap() found"; - if !is(or_arg, "Some") { - return; - } - } else if is_type_diagnostic_item(cx, ty, sym::Result) { - title = ".or(Ok(…)).unwrap() found"; - if !is(or_arg, "Ok") { - return; - } - } else { - // Someone has implemented a struct with .or(...).unwrap() chaining, - // but it's not an Option or a Result, so bail - return; - } - - span_lint_and_help( - cx, - USE_UNWRAP_OR, - or_span.to(*unwrap_span), - title, - None, - "use `unwrap_or()` instead" - ); - } - } - } -} - -/// is expr a Call to name? -/// name might be "Some", "Ok", "Err", etc. -fn is<'a>(expr: &Expr<'a>, name: &str) -> bool { - if_chain! { - if let ExprKind::Call(some_expr, _some_args) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, path)) = &some_expr.kind; - if let Some(path_segment) = path.segments.first(); - if path_segment.ident.name.as_str() == name; - then { - true - } - else { - false - } - } -} diff --git a/tests/ui/or_then_unwrap.rs b/tests/ui/or_then_unwrap.rs new file mode 100644 index 00000000000..cbc3c387da0 --- /dev/null +++ b/tests/ui/or_then_unwrap.rs @@ -0,0 +1,45 @@ +#![warn(clippy::or_then_unwrap)] +#![allow(clippy::map_identity)] + +struct SomeStruct {} +impl SomeStruct { + fn or(self, _: Option) -> Self { + self + } + fn unwrap(&self) {} +} + +struct SomeOtherStruct {} +impl SomeOtherStruct { + fn or(self) -> Self { + self + } + fn unwrap(&self) {} +} + +fn main() { + let option: Option<&str> = None; + let _ = option.or(Some("fallback")).unwrap(); // should trigger lint + + let result: Result<&str, &str> = Err("Error"); + let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint + + // Not Option/Result + let instance = SomeStruct {}; + let _ = instance.or(Some(SomeStruct {})).unwrap(); // should not trigger lint + + let instance = SomeOtherStruct {}; + let _ = instance.or().unwrap(); // should not trigger lint and should not panic + + // None in or + let option: Option<&str> = None; + let _ = option.or(None).unwrap(); // should not trigger lint + + // Not Err in or + let result: Result<&str, &str> = Err("Error"); + let _ = result.or::<&str>(Err("Other Error")).unwrap(); // should not trigger lint + + // other function between + let option: Option<&str> = None; + let _ = option.or(Some("fallback")).map(|v| v).unwrap(); // should not trigger lint +} diff --git a/tests/ui/or_then_unwrap.stderr b/tests/ui/or_then_unwrap.stderr new file mode 100644 index 00000000000..fdd718b3580 --- /dev/null +++ b/tests/ui/or_then_unwrap.stderr @@ -0,0 +1,19 @@ +error: .or(Some(…)).unwrap() found + --> $DIR/or_then_unwrap.rs:22:20 + | +LL | let _ = option.or(Some("fallback")).unwrap(); // should trigger lint + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::or-then-unwrap` implied by `-D warnings` + = help: use `unwrap_or()` instead + +error: .or(Ok(…)).unwrap() found + --> $DIR/or_then_unwrap.rs:25:20 + | +LL | let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `unwrap_or()` instead + +error: aborting due to 2 previous errors + diff --git a/tests/ui/use_unwrap_or.rs b/tests/ui/use_unwrap_or.rs deleted file mode 100644 index dd55b33739d..00000000000 --- a/tests/ui/use_unwrap_or.rs +++ /dev/null @@ -1,45 +0,0 @@ -#![warn(clippy::use_unwrap_or)] -#![allow(clippy::map_identity)] - -struct SomeStruct {} -impl SomeStruct { - fn or(self, _: Option) -> Self { - self - } - fn unwrap(&self) {} -} - -struct SomeOtherStruct {} -impl SomeOtherStruct { - fn or(self) -> Self { - self - } - fn unwrap(&self) {} -} - -fn main() { - let option: Option<&str> = None; - let _ = option.or(Some("fallback")).unwrap(); // should trigger lint - - let result: Result<&str, &str> = Err("Error"); - let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint - - // Not Option/Result - let instance = SomeStruct {}; - let _ = instance.or(Some(SomeStruct {})).unwrap(); // should not trigger lint - - let instance = SomeOtherStruct {}; - let _ = instance.or().unwrap(); // should not trigger lint and should not panic - - // None in or - let option: Option<&str> = None; - let _ = option.or(None).unwrap(); // should not trigger lint - - // Not Err in or - let result: Result<&str, &str> = Err("Error"); - let _ = result.or::<&str>(Err("Other Error")).unwrap(); // should not trigger lint - - // other function between - let option: Option<&str> = None; - let _ = option.or(Some("fallback")).map(|v| v).unwrap(); // should not trigger lint -} diff --git a/tests/ui/use_unwrap_or.stderr b/tests/ui/use_unwrap_or.stderr deleted file mode 100644 index 796778a293d..00000000000 --- a/tests/ui/use_unwrap_or.stderr +++ /dev/null @@ -1,19 +0,0 @@ -error: .or(Some(…)).unwrap() found - --> $DIR/use_unwrap_or.rs:22:20 - | -LL | let _ = option.or(Some("fallback")).unwrap(); // should trigger lint - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::use-unwrap-or` implied by `-D warnings` - = help: use `unwrap_or()` instead - -error: .or(Ok(…)).unwrap() found - --> $DIR/use_unwrap_or.rs:25:20 - | -LL | let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: use `unwrap_or()` instead - -error: aborting due to 2 previous errors - -- cgit 1.4.1-3-g733a5