diff options
| author | Max Baumann <max@bmn.dev> | 2022-03-18 21:11:54 +0100 |
|---|---|---|
| committer | Max Baumann <max@bmn.dev> | 2022-03-18 21:11:54 +0100 |
| commit | 34ad33c57aa9db1b0c8473925affef08fadc7b8a (patch) | |
| tree | e69aa1c2af0c8a20a67b7004d19f88afbc325ff4 | |
| parent | fd2c8601711554d31f0c836d5aa96623a25e63b6 (diff) | |
| download | rust-34ad33c57aa9db1b0c8473925affef08fadc7b8a.tar.gz rust-34ad33c57aa9db1b0c8473925affef08fadc7b8a.zip | |
refactor: move into methods module
| -rw-r--r-- | clippy_lints/src/lib.register_all.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_complexity.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/lib.register_lints.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/lib.rs | 2 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 41 | ||||
| -rw-r--r-- | clippy_lints/src/methods/or_then_unwrap.rs | 68 | ||||
| -rw-r--r-- | clippy_lints/src/or_then_unwrap.rs | 102 |
7 files changed, 112 insertions, 107 deletions
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 9ff5f26c1fa..033efcb8a89 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -181,6 +181,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(methods::OPTION_FILTER_MAP), LintId::of(methods::OPTION_MAP_OR_NONE), LintId::of(methods::OR_FUN_CALL), + LintId::of(methods::OR_THEN_UNWRAP), LintId::of(methods::RESULT_MAP_OR_INTO_OPTION), LintId::of(methods::SEARCH_IS_SOME), LintId::of(methods::SHOULD_IMPLEMENT_TRAIT), @@ -238,7 +239,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION), LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP), - LintId::of(or_then_unwrap::OR_THEN_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL), LintId::of(precedence::PRECEDENCE), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index 34d764e810d..a2ce69065f9 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -47,6 +47,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(methods::NEEDLESS_SPLITN), LintId::of(methods::OPTION_AS_REF_DEREF), LintId::of(methods::OPTION_FILTER_MAP), + LintId::of(methods::OR_THEN_UNWRAP), LintId::of(methods::SEARCH_IS_SOME), LintId::of(methods::SKIP_WHILE_NEXT), LintId::of(methods::UNNECESSARY_FILTER_MAP), @@ -66,7 +67,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(no_effect::NO_EFFECT), LintId::of(no_effect::UNNECESSARY_OPERATION), LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION), - LintId::of(or_then_unwrap::OR_THEN_UNWRAP), LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL), LintId::of(precedence::PRECEDENCE), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 0b6e3c7e7df..ce5a1170504 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -319,6 +319,7 @@ store.register_lints(&[ methods::OPTION_FILTER_MAP, methods::OPTION_MAP_OR_NONE, methods::OR_FUN_CALL, + methods::OR_THEN_UNWRAP, methods::RESULT_MAP_OR_INTO_OPTION, methods::SEARCH_IS_SOME, methods::SHOULD_IMPLEMENT_TRAIT, @@ -404,7 +405,6 @@ store.register_lints(&[ open_options::NONSENSICAL_OPEN_OPTIONS, option_env_unwrap::OPTION_ENV_UNWRAP, option_if_let_else::OPTION_IF_LET_ELSE, - or_then_unwrap::OR_THEN_UNWRAP, overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, panic_in_result_fn::PANIC_IN_RESULT_FN, panic_unimplemented::PANIC, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index fdab58935ef..504235d0d1e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -322,7 +322,6 @@ 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; @@ -867,7 +866,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ignore_publish: cargo_ignore_publish, }) }); - 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/methods/mod.rs b/clippy_lints/src/methods/mod.rs index aa9f86f292c..1e76428858b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -45,6 +45,7 @@ mod option_as_ref_deref; mod option_map_or_none; mod option_map_unwrap_or; mod or_fun_call; +mod or_then_unwrap; mod search_is_some; mod single_char_add_str; mod single_char_insert_string; @@ -780,6 +781,42 @@ declare_clippy_lint! { 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::<Error>(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_clippy_lint! { + /// ### What it does /// Checks for calls to `.expect(&format!(...))`, `.expect(foo(..))`, /// etc., and suggests to use `unwrap_or_else` instead /// @@ -2039,6 +2076,7 @@ impl_lint_pass!(Methods => [ OPTION_MAP_OR_NONE, BIND_INSTEAD_OF_MAP, OR_FUN_CALL, + OR_THEN_UNWRAP, EXPECT_FUN_CALL, CHARS_NEXT_CMP, CHARS_LAST_CMP, @@ -2474,6 +2512,9 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio Some(("get_mut", [recv, get_arg], _)) => { get_unwrap::check(cx, expr, recv, get_arg, true); }, + Some(("or", [recv, or_arg], or_span)) => { + or_then_unwrap::check(cx, expr, recv, or_arg, or_span); + }, _ => {}, } unwrap_used::check(cx, expr, recv); diff --git a/clippy_lints/src/methods/or_then_unwrap.rs b/clippy_lints/src/methods/or_then_unwrap.rs new file mode 100644 index 00000000000..02fa5887f67 --- /dev/null +++ b/clippy_lints/src/methods/or_then_unwrap.rs @@ -0,0 +1,68 @@ +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; +use rustc_span::{sym, Span}; + +use super::OR_THEN_UNWRAP; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + unwrap_expr: &Expr<'_>, + recv: &'tcx Expr<'tcx>, + or_arg: &'tcx Expr<'_>, + or_span: Span, +) { + let ty = cx.typeck_results().expr_ty(recv); // 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; + } + + let unwrap_span = if let ExprKind::MethodCall(_, _, span) = unwrap_expr.kind { + span + } else { + // unreachable. but fallback to ident's span ("()" are missing) + unwrap_expr.span + }; + + 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/or_then_unwrap.rs b/clippy_lints/src/or_then_unwrap.rs deleted file mode 100644 index d467fbdfe02..00000000000 --- a/clippy_lints/src/or_then_unwrap.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::<Error>(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 - } - } -} |
