Skip to content

Commit d71b418

Browse files
bugadanibugadani
committed
Moved to submodule, don't trigger if map_unwrap_or does
1 parent 9c41822 commit d71b418

File tree

2 files changed

+132
-117
lines changed

2 files changed

+132
-117
lines changed

‎clippy_lints/src/methods/mod.rs

Lines changed: 19 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ mod inefficient_to_string;
33
mod manual_saturating_arithmetic;
44
mod option_map_unwrap_or;
55
mod unnecessary_filter_map;
6+
mod unnecessary_lazy_eval;
67

78
use std::borrow::Cow;
89
use std::fmt;
@@ -1436,17 +1437,18 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
14361437
["expect", ..] => lint_expect(cx, expr, arg_lists[0]),
14371438
["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0], method_spans[1]),
14381439
["unwrap_or_else", "map"] => {
1439-
lint_lazy_eval(cx, expr, arg_lists[0], true, "unwrap_or");
1440-
lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]);
1440+
if !lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]) {
1441+
unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "unwrap_or");
1442+
}
14411443
},
14421444
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
14431445
["and_then", ..] => {
1444-
lint_lazy_eval(cx, expr, arg_lists[0], false, "and");
1446+
unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], false, "and");
14451447
bind_instead_of_map::OptionAndThenSome::lint(cx, expr, arg_lists[0]);
14461448
bind_instead_of_map::ResultAndThenOk::lint(cx, expr, arg_lists[0]);
14471449
},
14481450
["or_else", ..] => {
1449-
lint_lazy_eval(cx, expr, arg_lists[0], false, "or");
1451+
unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], false, "or");
14501452
bind_instead_of_map::ResultOrElseErrInfo::lint(cx, expr, arg_lists[0]);
14511453
},
14521454
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
@@ -1490,9 +1492,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
14901492
["is_file", ..] => lint_filetype_is_file(cx, expr, arg_lists[0]),
14911493
["map", "as_ref"] => lint_option_as_ref_deref(cx, expr, arg_lists[1], arg_lists[0], false),
14921494
["map", "as_mut"] => lint_option_as_ref_deref(cx, expr, arg_lists[1], arg_lists[0], true),
1493-
["unwrap_or_else", ..] => lint_lazy_eval(cx, expr, arg_lists[0], true, "unwrap_or"),
1494-
["get_or_insert_with", ..] => lint_lazy_eval(cx, expr, arg_lists[0], true, "get_or_insert"),
1495-
["ok_or_else", ..] => lint_lazy_eval(cx, expr, arg_lists[0], true, "ok_or"),
1495+
["unwrap_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "unwrap_or"),
1496+
["get_or_insert_with", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "get_or_insert"),
1497+
["ok_or_else", ..] => unnecessary_lazy_eval::lint(cx, expr, arg_lists[0], true, "ok_or"),
14961498
_ => {},
14971499
}
14981500

@@ -2708,119 +2710,13 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
27082710
}
27092711
}
27102712

2711-
/// lint use of `<fn>_else(simple closure)` for `Option`s and `Result`s that can be
2712-
/// replaced with `<fn>(return value of simple closure)`
2713-
fn lint_lazy_eval<'tcx>(
2714-
cx: &LateContext<'tcx>,
2715-
expr: &'tcx hir::Expr<'_>,
2716-
args: &'tcx [hir::Expr<'_>],
2717-
allow_variant_calls: bool,
2718-
simplify_using: &str,
2719-
) {
2720-
let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&args[0]), sym!(option_type));
2721-
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&args[0]), sym!(result_type));
2722-
2723-
if !is_option && !is_result {
2724-
return;
2725-
}
2726-
2727-
// Return true if the expression is an accessor of any of the arguments
2728-
fn expr_uses_argument(expr: &hir::Expr<'_>, params: &[hir::Param<'_>]) -> bool {
2729-
params.iter().any(|arg| {
2730-
if_chain! {
2731-
if let hir::PatKind::Binding(_, _, ident, _) = arg.pat.kind;
2732-
if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = expr.kind;
2733-
if let [p, ..] = path.segments;
2734-
then {
2735-
ident.name == p.ident.name
2736-
} else {
2737-
false
2738-
}
2739-
}
2740-
})
2741-
}
2742-
2743-
fn match_any_qpath(path: &hir::QPath<'_>, paths: &[&[&str]]) -> bool {
2744-
paths.iter().any(|candidate| match_qpath(path, candidate))
2745-
}
2746-
2747-
fn can_simplify(expr: &hir::Expr<'_>, params: &[hir::Param<'_>], variant_calls: bool) -> bool {
2748-
match expr.kind {
2749-
// Closures returning literals can be unconditionally simplified
2750-
hir::ExprKind::Lit(_) => true,
2751-
2752-
hir::ExprKind::Index(ref object, ref index) => {
2753-
// arguments are not being indexed into
2754-
if !expr_uses_argument(object, params) {
2755-
// arguments are not used as index
2756-
!expr_uses_argument(index, params)
2757-
} else {
2758-
false
2759-
}
2760-
},
2761-
2762-
// Reading fields can be simplified if the object is not an argument of the closure
2763-
hir::ExprKind::Field(ref object, _) => !expr_uses_argument(object, params),
2764-
2765-
// Paths can be simplified if the root is not the argument, this also covers None
2766-
hir::ExprKind::Path(_) => !expr_uses_argument(expr, params),
2767-
2768-
// Calls to Some, Ok, Err can be considered literals if they don't derive an argument
2769-
hir::ExprKind::Call(ref func, ref args) => if_chain! {
2770-
if variant_calls; // Disable lint when rules conflict with bind_instead_of_map
2771-
if let hir::ExprKind::Path(ref path) = func.kind;
2772-
if match_any_qpath(path, &[&["Some"], &["Ok"], &["Err"]]);
2773-
then {
2774-
// Recursively check all arguments
2775-
args.iter().all(|arg| can_simplify(arg, params, variant_calls))
2776-
} else {
2777-
false
2778-
}
2779-
},
2780-
2781-
// For anything more complex than the above, a closure is probably the right solution,
2782-
// or the case is handled by an other lint
2783-
_ => false,
2784-
}
2785-
}
2786-
2787-
if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind {
2788-
let body = cx.tcx.hir().body(eid);
2789-
let ex = &body.value;
2790-
let params = &body.params;
2791-
2792-
if can_simplify(ex, params, allow_variant_calls) {
2793-
let msg = if is_option {
2794-
"unnecessary closure used to substitute value for `Option::None`"
2795-
} else {
2796-
"unnecessary closure used to substitute value for `Result::Err`"
2797-
};
2798-
2799-
span_lint_and_sugg(
2800-
cx,
2801-
UNNECESSARY_LAZY_EVALUATION,
2802-
expr.span,
2803-
msg,
2804-
&format!("Use `{}` instead", simplify_using),
2805-
format!(
2806-
"{0}.{1}({2})",
2807-
snippet(cx, args[0].span, ".."),
2808-
simplify_using,
2809-
snippet(cx, ex.span, ".."),
2810-
),
2811-
Applicability::MachineApplicable,
2812-
);
2813-
}
2814-
}
2815-
}
2816-
28172713
/// lint use of `map().unwrap_or_else()` for `Option`s and `Result`s
28182714
fn lint_map_unwrap_or_else<'tcx>(
28192715
cx: &LateContext<'tcx>,
28202716
expr: &'tcx hir::Expr<'_>,
28212717
map_args: &'tcx [hir::Expr<'_>],
28222718
unwrap_args: &'tcx [hir::Expr<'_>],
2823-
) {
2719+
) -> bool {
28242720
// lint if the caller of `map()` is an `Option`
28252721
let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym!(option_type));
28262722
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&map_args[0]), sym!(result_type));
@@ -2832,10 +2728,10 @@ fn lint_map_unwrap_or_else<'tcx>(
28322728
let unwrap_mutated_vars = mutated_variables(&unwrap_args[1], cx);
28332729
if let (Some(map_mutated_vars), Some(unwrap_mutated_vars)) = (map_mutated_vars, unwrap_mutated_vars) {
28342730
if map_mutated_vars.intersection(&unwrap_mutated_vars).next().is_some() {
2835-
return;
2731+
return false;
28362732
}
28372733
} else {
2838-
return;
2734+
return false;
28392735
}
28402736

28412737
// lint message
@@ -2865,9 +2761,15 @@ fn lint_map_unwrap_or_else<'tcx>(
28652761
map_snippet, unwrap_snippet,
28662762
),
28672763
);
2764+
true
28682765
} else if same_span && multiline {
28692766
span_lint(cx, MAP_UNWRAP_OR, expr.span, msg);
2870-
};
2767+
true
2768+
} else {
2769+
false
2770+
}
2771+
} else {
2772+
false
28712773
}
28722774
}
28732775

‎clippy_lints/src/methods/unnecessary_lazy_eval.rs

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
use crate::utils::{match_qpath, span_lint_and_sugg, snippet, is_type_diagnostic_item};
2+
use if_chain::if_chain;
3+
use rustc_errors::Applicability;
4+
use rustc_hir as hir;
5+
use rustc_lint::LateContext;
6+
7+
use super::UNNECESSARY_LAZY_EVALUATION;
8+
9+
/// lint use of `<fn>_else(simple closure)` for `Option`s and `Result`s that can be
10+
/// replaced with `<fn>(return value of simple closure)`
11+
pub(super) fn lint<'tcx>(
12+
cx: &LateContext<'tcx>,
13+
expr: &'tcx hir::Expr<'_>,
14+
args: &'tcx [hir::Expr<'_>],
15+
allow_variant_calls: bool,
16+
simplify_using: &str,
17+
) {
18+
let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&args[0]), sym!(option_type));
19+
let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(&args[0]), sym!(result_type));
20+
21+
if !is_option && !is_result {
22+
return;
23+
}
24+
25+
// Return true if the expression is an accessor of any of the arguments
26+
fn expr_uses_argument(expr: &hir::Expr<'_>, params: &[hir::Param<'_>]) -> bool {
27+
params.iter().any(|arg| {
28+
if_chain! {
29+
if let hir::PatKind::Binding(_, _, ident, _) = arg.pat.kind;
30+
if let hir::ExprKind::Path(hir::QPath::Resolved(_, ref path)) = expr.kind;
31+
if let [p, ..] = path.segments;
32+
then {
33+
ident.name == p.ident.name
34+
} else {
35+
false
36+
}
37+
}
38+
})
39+
}
40+
41+
fn match_any_qpath(path: &hir::QPath<'_>, paths: &[&[&str]]) -> bool {
42+
paths.iter().any(|candidate| match_qpath(path, candidate))
43+
}
44+
45+
fn can_simplify(expr: &hir::Expr<'_>, params: &[hir::Param<'_>], variant_calls: bool) -> bool {
46+
match expr.kind {
47+
// Closures returning literals can be unconditionally simplified
48+
hir::ExprKind::Lit(_) => true,
49+
50+
hir::ExprKind::Index(ref object, ref index) => {
51+
// arguments are not being indexed into
52+
if !expr_uses_argument(object, params) {
53+
// arguments are not used as index
54+
!expr_uses_argument(index, params)
55+
} else {
56+
false
57+
}
58+
},
59+
60+
// Reading fields can be simplified if the object is not an argument of the closure
61+
hir::ExprKind::Field(ref object, _) => !expr_uses_argument(object, params),
62+
63+
// Paths can be simplified if the root is not the argument, this also covers None
64+
hir::ExprKind::Path(_) => !expr_uses_argument(expr, params),
65+
66+
// Calls to Some, Ok, Err can be considered literals if they don't derive an argument
67+
hir::ExprKind::Call(ref func, ref args) => if_chain! {
68+
if variant_calls; // Disable lint when rules conflict with bind_instead_of_map
69+
if let hir::ExprKind::Path(ref path) = func.kind;
70+
if match_any_qpath(path, &[&["Some"], &["Ok"], &["Err"]]);
71+
then {
72+
// Recursively check all arguments
73+
args.iter().all(|arg| can_simplify(arg, params, variant_calls))
74+
} else {
75+
false
76+
}
77+
},
78+
79+
// For anything more complex than the above, a closure is probably the right solution,
80+
// or the case is handled by an other lint
81+
_ => false,
82+
}
83+
}
84+
85+
if let hir::ExprKind::Closure(_, _, eid, _, _) = args[1].kind {
86+
let body = cx.tcx.hir().body(eid);
87+
let ex = &body.value;
88+
let params = &body.params;
89+
90+
if can_simplify(ex, params, allow_variant_calls) {
91+
let msg = if is_option {
92+
"unnecessary closure used to substitute value for `Option::None`"
93+
} else {
94+
"unnecessary closure used to substitute value for `Result::Err`"
95+
};
96+
97+
span_lint_and_sugg(
98+
cx,
99+
UNNECESSARY_LAZY_EVALUATION,
100+
expr.span,
101+
msg,
102+
&format!("Use `{}` instead", simplify_using),
103+
format!(
104+
"{0}.{1}({2})",
105+
snippet(cx, args[0].span, ".."),
106+
simplify_using,
107+
snippet(cx, ex.span, ".."),
108+
),
109+
Applicability::MachineApplicable,
110+
);
111+
}
112+
}
113+
}

0 commit comments

Comments
 (0)