refactor: move into methods module
This commit is contained in:
parent
fd2c860171
commit
34ad33c57a
7 changed files with 112 additions and 107 deletions
|
@ -181,6 +181,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
|
||||||
LintId::of(methods::OPTION_FILTER_MAP),
|
LintId::of(methods::OPTION_FILTER_MAP),
|
||||||
LintId::of(methods::OPTION_MAP_OR_NONE),
|
LintId::of(methods::OPTION_MAP_OR_NONE),
|
||||||
LintId::of(methods::OR_FUN_CALL),
|
LintId::of(methods::OR_FUN_CALL),
|
||||||
|
LintId::of(methods::OR_THEN_UNWRAP),
|
||||||
LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),
|
LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),
|
||||||
LintId::of(methods::SEARCH_IS_SOME),
|
LintId::of(methods::SEARCH_IS_SOME),
|
||||||
LintId::of(methods::SHOULD_IMPLEMENT_TRAIT),
|
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(only_used_in_recursion::ONLY_USED_IN_RECURSION),
|
||||||
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
|
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
|
||||||
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
|
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(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
|
||||||
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
|
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
|
||||||
LintId::of(precedence::PRECEDENCE),
|
LintId::of(precedence::PRECEDENCE),
|
||||||
|
|
|
@ -47,6 +47,7 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
|
||||||
LintId::of(methods::NEEDLESS_SPLITN),
|
LintId::of(methods::NEEDLESS_SPLITN),
|
||||||
LintId::of(methods::OPTION_AS_REF_DEREF),
|
LintId::of(methods::OPTION_AS_REF_DEREF),
|
||||||
LintId::of(methods::OPTION_FILTER_MAP),
|
LintId::of(methods::OPTION_FILTER_MAP),
|
||||||
|
LintId::of(methods::OR_THEN_UNWRAP),
|
||||||
LintId::of(methods::SEARCH_IS_SOME),
|
LintId::of(methods::SEARCH_IS_SOME),
|
||||||
LintId::of(methods::SKIP_WHILE_NEXT),
|
LintId::of(methods::SKIP_WHILE_NEXT),
|
||||||
LintId::of(methods::UNNECESSARY_FILTER_MAP),
|
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::NO_EFFECT),
|
||||||
LintId::of(no_effect::UNNECESSARY_OPERATION),
|
LintId::of(no_effect::UNNECESSARY_OPERATION),
|
||||||
LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
|
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(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
|
||||||
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
|
LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
|
||||||
LintId::of(precedence::PRECEDENCE),
|
LintId::of(precedence::PRECEDENCE),
|
||||||
|
|
|
@ -319,6 +319,7 @@ store.register_lints(&[
|
||||||
methods::OPTION_FILTER_MAP,
|
methods::OPTION_FILTER_MAP,
|
||||||
methods::OPTION_MAP_OR_NONE,
|
methods::OPTION_MAP_OR_NONE,
|
||||||
methods::OR_FUN_CALL,
|
methods::OR_FUN_CALL,
|
||||||
|
methods::OR_THEN_UNWRAP,
|
||||||
methods::RESULT_MAP_OR_INTO_OPTION,
|
methods::RESULT_MAP_OR_INTO_OPTION,
|
||||||
methods::SEARCH_IS_SOME,
|
methods::SEARCH_IS_SOME,
|
||||||
methods::SHOULD_IMPLEMENT_TRAIT,
|
methods::SHOULD_IMPLEMENT_TRAIT,
|
||||||
|
@ -404,7 +405,6 @@ store.register_lints(&[
|
||||||
open_options::NONSENSICAL_OPEN_OPTIONS,
|
open_options::NONSENSICAL_OPEN_OPTIONS,
|
||||||
option_env_unwrap::OPTION_ENV_UNWRAP,
|
option_env_unwrap::OPTION_ENV_UNWRAP,
|
||||||
option_if_let_else::OPTION_IF_LET_ELSE,
|
option_if_let_else::OPTION_IF_LET_ELSE,
|
||||||
or_then_unwrap::OR_THEN_UNWRAP,
|
|
||||||
overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
|
overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,
|
||||||
panic_in_result_fn::PANIC_IN_RESULT_FN,
|
panic_in_result_fn::PANIC_IN_RESULT_FN,
|
||||||
panic_unimplemented::PANIC,
|
panic_unimplemented::PANIC,
|
||||||
|
|
|
@ -322,7 +322,6 @@ mod only_used_in_recursion;
|
||||||
mod open_options;
|
mod open_options;
|
||||||
mod option_env_unwrap;
|
mod option_env_unwrap;
|
||||||
mod option_if_let_else;
|
mod option_if_let_else;
|
||||||
mod or_then_unwrap;
|
|
||||||
mod overflow_check_conditional;
|
mod overflow_check_conditional;
|
||||||
mod panic_in_result_fn;
|
mod panic_in_result_fn;
|
||||||
mod panic_unimplemented;
|
mod panic_unimplemented;
|
||||||
|
@ -867,7 +866,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||||
ignore_publish: cargo_ignore_publish,
|
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`
|
// add lints here, do not remove this comment, it's used in `new_lint`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -45,6 +45,7 @@ mod option_as_ref_deref;
|
||||||
mod option_map_or_none;
|
mod option_map_or_none;
|
||||||
mod option_map_unwrap_or;
|
mod option_map_unwrap_or;
|
||||||
mod or_fun_call;
|
mod or_fun_call;
|
||||||
|
mod or_then_unwrap;
|
||||||
mod search_is_some;
|
mod search_is_some;
|
||||||
mod single_char_add_str;
|
mod single_char_add_str;
|
||||||
mod single_char_insert_string;
|
mod single_char_insert_string;
|
||||||
|
@ -778,6 +779,42 @@ declare_clippy_lint! {
|
||||||
"using any `*or` method with a function call, which suggests `*or_else`"
|
"using any `*or` method with a function call, which suggests `*or_else`"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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! {
|
declare_clippy_lint! {
|
||||||
/// ### What it does
|
/// ### What it does
|
||||||
/// Checks for calls to `.expect(&format!(...))`, `.expect(foo(..))`,
|
/// Checks for calls to `.expect(&format!(...))`, `.expect(foo(..))`,
|
||||||
|
@ -2039,6 +2076,7 @@ impl_lint_pass!(Methods => [
|
||||||
OPTION_MAP_OR_NONE,
|
OPTION_MAP_OR_NONE,
|
||||||
BIND_INSTEAD_OF_MAP,
|
BIND_INSTEAD_OF_MAP,
|
||||||
OR_FUN_CALL,
|
OR_FUN_CALL,
|
||||||
|
OR_THEN_UNWRAP,
|
||||||
EXPECT_FUN_CALL,
|
EXPECT_FUN_CALL,
|
||||||
CHARS_NEXT_CMP,
|
CHARS_NEXT_CMP,
|
||||||
CHARS_LAST_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], _)) => {
|
Some(("get_mut", [recv, get_arg], _)) => {
|
||||||
get_unwrap::check(cx, expr, recv, get_arg, true);
|
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);
|
unwrap_used::check(cx, expr, recv);
|
||||||
|
|
68
clippy_lints/src/methods/or_then_unwrap.rs
Normal file
68
clippy_lints/src/methods/or_then_unwrap.rs
Normal file
|
@ -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
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -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
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
Loading…
Add table
Reference in a new issue