Skip to content

Commit

Permalink
Improve heuristics for determining whether eager of lazy evaluation i…
Browse files Browse the repository at this point in the history
…s preferred
  • Loading branch information
Jarcho committed Sep 16, 2021
1 parent 2316f4d commit bafe94e
Show file tree
Hide file tree
Showing 17 changed files with 406 additions and 238 deletions.
4 changes: 2 additions & 2 deletions clippy_lints/src/functions/too_many_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ pub(super) fn check_fn(
continue;
}
} else {
let multi_idx = line.find("/*").unwrap_or_else(|| line.len());
let single_idx = line.find("//").unwrap_or_else(|| line.len());
let multi_idx = line.find("/*").unwrap_or(line.len());
let single_idx = line.find("//").unwrap_or(line.len());
code_in_line |= multi_idx > 0 && single_idx > 0;
// Implies multi_idx is below line.len()
if multi_idx < single_idx {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/from_iter_instead_of_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn extract_turbofish(cx: &LateContext<'_>, expr: &hir::Expr<'_>, ty: Ty<'tcx>) -
// i.e.: 2 wildcards in `std::collections::BTreeMap<&i32, &char>`
let ty_str = ty.to_string();
let start = ty_str.find('<').unwrap_or(0);
let end = ty_str.find('>').unwrap_or_else(|| ty_str.len());
let end = ty_str.find('>').unwrap_or(ty_str.len());
let nb_wildcard = ty_str[start..end].split(',').count();
let wildcards = format!("_{}", ", _".repeat(nb_wildcard - 1));
format!("{}<{}>", elements.join("::"), wildcards)
Expand Down
59 changes: 22 additions & 37 deletions clippy_lints/src/methods/or_fun_call.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eager_or_lazy::is_lazyness_candidate;
use clippy_utils::is_trait_item;
use clippy_utils::eager_or_lazy::switch_to_lazy_eval;
use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_macro_callsite};
use clippy_utils::ty::implements_trait;
use clippy_utils::ty::{is_type_diagnostic_item, match_type};
use clippy_utils::{contains_return, last_path_segment, paths};
use clippy_utils::ty::{implements_trait, match_type};
use clippy_utils::{contains_return, is_trait_item, last_path_segment, paths};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir as hir;
use rustc_hir::{BlockCheckMode, UnsafeSource};
use rustc_lint::LateContext;
use rustc_middle::ty;
use rustc_span::source_map::Span;
use rustc_span::symbol::{kw, sym};
use std::borrow::Cow;
Expand Down Expand Up @@ -96,25 +92,10 @@ pub(super) fn check<'tcx>(
(&paths::RESULT, true, &["or", "unwrap_or"], "else"),
];

if let hir::ExprKind::MethodCall(path, _, [self_arg, ..], _) = &arg.kind {
if path.ident.name == sym::len {
let ty = cx.typeck_results().expr_ty(self_arg).peel_refs();

match ty.kind() {
ty::Slice(_) | ty::Array(_, _) | ty::Str => return,
_ => (),
}

if is_type_diagnostic_item(cx, ty, sym::vec_type) {
return;
}
}
}

if_chain! {
if KNOW_TYPES.iter().any(|k| k.2.contains(&name));

if is_lazyness_candidate(cx, arg);
if switch_to_lazy_eval(cx, arg);
if !contains_return(arg);

let self_ty = cx.typeck_results().expr_ty(self_expr);
Expand Down Expand Up @@ -166,26 +147,30 @@ pub(super) fn check<'tcx>(
}
}

if args.len() == 2 {
match args[1].kind {
if let [self_arg, arg] = args {
let inner_arg = if let hir::ExprKind::Block(
hir::Block {
stmts: [],
expr: Some(expr),
..
},
_,
) = arg.kind
{
expr
} else {
arg
};
match inner_arg.kind {
hir::ExprKind::Call(fun, or_args) => {
let or_has_args = !or_args.is_empty();
if !check_unwrap_or_default(cx, name, fun, &args[0], &args[1], or_has_args, expr.span) {
if !check_unwrap_or_default(cx, name, fun, self_arg, arg, or_has_args, expr.span) {
let fun_span = if or_has_args { None } else { Some(fun.span) };
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, fun_span);
check_general_case(cx, name, method_span, self_arg, arg, expr.span, fun_span);
}
},
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
},
hir::ExprKind::Block(block, _) => {
if let BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided) = block.rules {
if let Some(block_expr) = block.expr {
if let hir::ExprKind::MethodCall(..) = block_expr.kind {
check_general_case(cx, name, method_span, &args[0], &args[1], expr.span, None);
}
}
}
check_general_case(cx, name, method_span, self_arg, arg, expr.span, None);
},
_ => (),
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/unnecessary_lazy_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(super) fn check<'tcx>(
return;
}

if eager_or_lazy::is_eagerness_candidate(cx, body_expr) {
if eager_or_lazy::switch_to_eager_eval(cx, body_expr) {
let msg = if is_option {
"unnecessary closure used to substitute value for `Option::None`"
} else {
Expand Down
6 changes: 1 addition & 5 deletions clippy_lints/src/option_if_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,7 @@ fn detect_option_if_let_else<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) ->
let capture_mut = if bind_annotation == &BindingAnnotation::Mutable { "mut " } else { "" };
let some_body = extract_body_from_expr(if_then)?;
let none_body = extract_body_from_expr(if_else)?;
let method_sugg = if eager_or_lazy::is_eagerness_candidate(cx, none_body) {
"map_or"
} else {
"map_or_else"
};
let method_sugg = if eager_or_lazy::switch_to_eager_eval(cx, none_body) { "map_or" } else { "map_or_else" };
let capture_name = id.name.to_ident_string();
let (as_ref, as_mut) = match &let_expr.kind {
ExprKind::AddrOf(_, Mutability::Not, _) => (true, false),
Expand Down
Loading

0 comments on commit bafe94e

Please sign in to comment.