diff options
Diffstat (limited to 'clippy_lints/src/methods')
| -rw-r--r-- | clippy_lints/src/methods/bind_instead_of_map.rs | 21 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 66 | ||||
| -rw-r--r-- | clippy_lints/src/methods/unnecessary_lazy_eval.rs | 76 |
3 files changed, 42 insertions, 121 deletions
diff --git a/clippy_lints/src/methods/bind_instead_of_map.rs b/clippy_lints/src/methods/bind_instead_of_map.rs index 498f12518f8..ae37942e55a 100644 --- a/clippy_lints/src/methods/bind_instead_of_map.rs +++ b/clippy_lints/src/methods/bind_instead_of_map.rs @@ -12,6 +12,7 @@ use rustc_middle::hir::map::Map; use rustc_span::Span; pub(crate) struct OptionAndThenSome; + impl BindInsteadOfMap for OptionAndThenSome { const TYPE_NAME: &'static str = "Option"; const TYPE_QPATH: &'static [&'static str] = &paths::OPTION; @@ -24,6 +25,7 @@ impl BindInsteadOfMap for OptionAndThenSome { } pub(crate) struct ResultAndThenOk; + impl BindInsteadOfMap for ResultAndThenOk { const TYPE_NAME: &'static str = "Result"; const TYPE_QPATH: &'static [&'static str] = &paths::RESULT; @@ -36,6 +38,7 @@ impl BindInsteadOfMap for ResultAndThenOk { } pub(crate) struct ResultOrElseErrInfo; + impl BindInsteadOfMap for ResultOrElseErrInfo { const TYPE_NAME: &'static str = "Result"; const TYPE_QPATH: &'static [&'static str] = &paths::RESULT; @@ -120,9 +123,9 @@ pub(crate) trait BindInsteadOfMap { } } - fn lint_closure(cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) { + fn lint_closure(cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool { let mut suggs = Vec::new(); - let can_sugg = find_all_ret_expressions(cx, closure_expr, |ret_expr| { + let can_sugg: bool = find_all_ret_expressions(cx, closure_expr, |ret_expr| { if_chain! { if !in_macro(ret_expr.span); if let hir::ExprKind::Call(ref func_path, ref args) = ret_expr.kind; @@ -153,12 +156,13 @@ pub(crate) trait BindInsteadOfMap { ) }); } + can_sugg } /// Lint use of `_.and_then(|x| Some(y))` for `Option`s - fn lint(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) { + fn lint(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) -> bool { if !match_type(cx, cx.typeck_results().expr_ty(&args[0]), Self::TYPE_QPATH) { - return; + return false; } match args[1].kind { @@ -166,8 +170,10 @@ pub(crate) trait BindInsteadOfMap { let closure_body = cx.tcx.hir().body(body_id); let closure_expr = remove_blocks(&closure_body.value); - if !Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) { - Self::lint_closure(cx, expr, closure_expr); + if Self::lint_closure_autofixable(cx, expr, args, closure_expr, closure_args_span) { + true + } else { + Self::lint_closure(cx, expr, closure_expr) } }, // `_.and_then(Some)` case, which is no-op. @@ -181,8 +187,9 @@ pub(crate) trait BindInsteadOfMap { snippet(cx, args[0].span, "..").into(), Applicability::MachineApplicable, ); + true }, - _ => {}, + _ => false, } } } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index de966cccd11..184aee95efa 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -25,14 +25,15 @@ use rustc_span::source_map::Span; use rustc_span::symbol::{sym, SymbolStr}; use crate::consts::{constant, Constant}; +use crate::utils::eager_or_lazy::is_lazyness_candidate; use crate::utils::usage::mutated_variables; use crate::utils::{ contains_ty, get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait, in_macro, - is_copy, is_ctor_or_promotable_const_function, is_expn_of, is_type_diagnostic_item, iter_input_pats, - last_path_segment, match_def_path, match_qpath, match_trait_method, match_type, match_var, method_calls, - method_chain_args, paths, remove_blocks, return_ty, single_segment_path, snippet, snippet_with_applicability, - snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, - span_lint_and_then, sugg, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq, + is_copy, is_expn_of, is_type_diagnostic_item, iter_input_pats, last_path_segment, match_def_path, match_qpath, + match_trait_method, match_type, match_var, method_calls, method_chain_args, paths, remove_blocks, return_ty, + single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, + span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, sugg, walk_ptrs_ty, + walk_ptrs_ty_depth, SpanlessEq, }; declare_clippy_lint! { @@ -1454,18 +1455,21 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0], method_spans[1]), ["unwrap_or_else", "map"] => { if !lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]) { - unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "unwrap_or"); + unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "unwrap_or"); } }, ["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]), ["and_then", ..] => { - unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], false, "and"); - bind_instead_of_map::OptionAndThenSome::lint(cx, expr, arg_lists[0]); - bind_instead_of_map::ResultAndThenOk::lint(cx, expr, arg_lists[0]); + let biom_option_linted = bind_instead_of_map::OptionAndThenSome::lint(cx, expr, arg_lists[0]); + let biom_result_linted = bind_instead_of_map::ResultAndThenOk::lint(cx, expr, arg_lists[0]); + if !biom_option_linted && !biom_result_linted { + unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "and"); + } }, ["or_else", ..] => { - unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], false, "or"); - bind_instead_of_map::ResultOrElseErrInfo::lint(cx, expr, arg_lists[0]); + if !bind_instead_of_map::ResultOrElseErrInfo::lint(cx, expr, arg_lists[0]) { + unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "or"); + } }, ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]), ["next", "skip_while"] => lint_skip_while_next(cx, expr, arg_lists[1]), @@ -1508,9 +1512,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods { ["is_file", ..] => lint_filetype_is_file(cx, expr, arg_lists[0]), ["map", "as_ref"] => lint_option_as_ref_deref(cx, expr, arg_lists[1], arg_lists[0], false), ["map", "as_mut"] => lint_option_as_ref_deref(cx, expr, arg_lists[1], arg_lists[0], true), - ["unwrap_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "unwrap_or"), - ["get_or_insert_with", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "get_or_insert"), - ["ok_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "ok_or"), + ["unwrap_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "unwrap_or"), + ["get_or_insert_with", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "get_or_insert"), + ["ok_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], "ok_or"), _ => {}, } @@ -1714,37 +1718,6 @@ fn lint_or_fun_call<'tcx>( name: &str, args: &'tcx [hir::Expr<'_>], ) { - // Searches an expression for method calls or function calls that aren't ctors - struct FunCallFinder<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - found: bool, - } - - impl<'a, 'tcx> intravisit::Visitor<'tcx> for FunCallFinder<'a, 'tcx> { - type Map = Map<'tcx>; - - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - let call_found = match &expr.kind { - // ignore enum and struct constructors - hir::ExprKind::Call(..) => !is_ctor_or_promotable_const_function(self.cx, expr), - hir::ExprKind::MethodCall(..) => true, - _ => false, - }; - - if call_found { - self.found |= true; - } - - if !self.found { - intravisit::walk_expr(self, expr); - } - } - - fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> { - intravisit::NestedVisitorMap::None - } - } - /// Checks for `unwrap_or(T::new())` or `unwrap_or(T::default())`. fn check_unwrap_or_default( cx: &LateContext<'_>, @@ -1825,8 +1798,7 @@ fn lint_or_fun_call<'tcx>( if_chain! { if know_types.iter().any(|k| k.2.contains(&name)); - let mut finder = FunCallFinder { cx: &cx, found: false }; - if { finder.visit_expr(&arg); finder.found }; + if is_lazyness_candidate(cx, arg); if !contains_return(&arg); let self_ty = cx.typeck_results().expr_ty(self_expr); diff --git a/clippy_lints/src/methods/unnecessary_lazy_eval.rs b/clippy_lints/src/methods/unnecessary_lazy_eval.rs index 31517659c34..08b3eab9b7c 100644 --- a/clippy_lints/src/methods/unnecessary_lazy_eval.rs +++ b/clippy_lints/src/methods/unnecessary_lazy_eval.rs @@ -1,78 +1,17 @@ -use crate::utils::{is_type_diagnostic_item, match_qpath, snippet, span_lint_and_sugg}; -use if_chain::if_chain; +use crate::utils::{eager_or_lazy, usage}; +use crate::utils::{is_type_diagnostic_item, snippet, span_lint_and_sugg}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use super::UNNECESSARY_LAZY_EVALUATIONS; -// Return true if the expression is an accessor of any of the arguments -fn expr_uses_argument(expr: &hir::Expr<'_>, params: &[hir::Param<'_>]) -> bool { - params.iter().any(|arg| { - if_chain! { - if let hir::PatKind::Binding(_, _, ident, _) = arg.pat.kind; - if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = expr.kind; - if let [p, ..] = path.segments; - then { - ident.name == p.ident.name - } else { - false - } - } - }) -} - -fn match_any_qpath(path: &hir::QPath<'_>, paths: &[&[&str]]) -> bool { - paths.iter().any(|candidate| match_qpath(path, candidate)) -} - -fn can_simplify(expr: &hir::Expr<'_>, params: &[hir::Param<'_>], variant_calls: bool) -> bool { - match expr.kind { - // Closures returning literals can be unconditionally simplified - hir::ExprKind::Lit(_) => true, - - hir::ExprKind::Index(ref object, ref index) => { - // arguments are not being indexed into - if expr_uses_argument(object, params) { - false - } else { - // arguments are not used as index - !expr_uses_argument(index, params) - } - }, - - // Reading fields can be simplified if the object is not an argument of the closure - hir::ExprKind::Field(ref object, _) => !expr_uses_argument(object, params), - - // Paths can be simplified if the root is not the argument, this also covers None - hir::ExprKind::Path(_) => !expr_uses_argument(expr, params), - - // Calls to Some, Ok, Err can be considered literals if they don't derive an argument - hir::ExprKind::Call(ref func, ref args) => if_chain! { - if variant_calls; // Disable lint when rules conflict with bind_instead_of_map - if let hir::ExprKind::Path(ref path) = func.kind; - if match_any_qpath(path, &[&["Some"], &["Ok"], &["Err"]]); - then { - // Recursively check all arguments - args.iter().all(|arg| can_simplify(arg, params, variant_calls)) - } else { - false - } - }, - - // For anything more complex than the above, a closure is probably the right solution, - // or the case is handled by an other lint - _ => false, - } -} - /// lint use of `<fn>_else(simple closure)` for `Option`s and `Result`s that can be /// replaced with `<fn>(return value of simple closure)` pub(super) fn lint<'tcx>( cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, args: &'tcx [hir::Expr<'_>], - allow_variant_calls: bool, simplify_using: &str, ) { let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&args[0]), sym!(option_type)); @@ -81,10 +20,13 @@ pub(super) fn lint<'tcx>( if is_option || is_result { if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind { let body = cx.tcx.hir().body(eid); - let ex = &body.value; - let params = &body.params; + let body_expr = &body.value; + + if usage::BindingUsageFinder::are_params_used(cx, body) { + return; + } - if can_simplify(ex, params, allow_variant_calls) { + if eager_or_lazy::is_eagerness_candidate(cx, body_expr) { let msg = if is_option { "unnecessary closure used to substitute value for `Option::None`" } else { @@ -101,7 +43,7 @@ pub(super) fn lint<'tcx>( "{0}.{1}({2})", snippet(cx, args[0].span, ".."), simplify_using, - snippet(cx, ex.span, ".."), + snippet(cx, body_expr.span, ".."), ), Applicability::MachineApplicable, ); |