Implement new lint
This commit is contained in:
Dániel Buga
parent
3bd98895f1
commit
a3ea65c2d9
7 changed files with 494 additions and 1 deletions
|
|
@ -672,6 +672,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
&methods::OK_EXPECT,
|
||||
&methods::OPTION_AS_REF_DEREF,
|
||||
&methods::OPTION_MAP_OR_NONE,
|
||||
&methods::UNNECESSARY_LAZY_EVALUATION,
|
||||
&methods::OR_FUN_CALL,
|
||||
&methods::RESULT_MAP_OR_INTO_OPTION,
|
||||
&methods::SEARCH_IS_SOME,
|
||||
|
|
@ -1360,6 +1361,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&methods::UNINIT_ASSUMED_INIT),
|
||||
LintId::of(&methods::UNNECESSARY_FILTER_MAP),
|
||||
LintId::of(&methods::UNNECESSARY_FOLD),
|
||||
LintId::of(&methods::UNNECESSARY_LAZY_EVALUATION),
|
||||
LintId::of(&methods::USELESS_ASREF),
|
||||
LintId::of(&methods::WRONG_SELF_CONVENTION),
|
||||
LintId::of(&methods::ZST_OFFSET),
|
||||
|
|
@ -1610,6 +1612,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&methods::FILTER_NEXT),
|
||||
LintId::of(&methods::FLAT_MAP_IDENTITY),
|
||||
LintId::of(&methods::OPTION_AS_REF_DEREF),
|
||||
LintId::of(&methods::UNNECESSARY_LAZY_EVALUATION),
|
||||
LintId::of(&methods::SEARCH_IS_SOME),
|
||||
LintId::of(&methods::SKIP_WHILE_NEXT),
|
||||
LintId::of(&methods::SUSPICIOUS_MAP),
|
||||
|
|
|
|||
|
|
@ -1329,6 +1329,32 @@ declare_clippy_lint! {
|
|||
"`push_str()` used with a single-character string literal as parameter"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Looks for unnecessary lazily evaluated closures on `Option` and `Result`.
|
||||
///
|
||||
/// **Why is this bad?** Using eager evaluation is shorter and simpler in some cases.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
///
|
||||
/// ```rust
|
||||
/// // example code where clippy issues a warning
|
||||
/// let opt: Option<u32> = None;
|
||||
///
|
||||
/// opt.unwrap_or_else(|| 42);
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// let opt: Option<u32> = None;
|
||||
///
|
||||
/// opt.unwrap_or(42);
|
||||
/// ```
|
||||
pub UNNECESSARY_LAZY_EVALUATION,
|
||||
style,
|
||||
"using unnecessary lazy evaluation, which can be replaced with simpler eager evaluation"
|
||||
}
|
||||
|
||||
declare_lint_pass!(Methods => [
|
||||
UNWRAP_USED,
|
||||
EXPECT_USED,
|
||||
|
|
@ -1378,6 +1404,7 @@ declare_lint_pass!(Methods => [
|
|||
ZST_OFFSET,
|
||||
FILETYPE_IS_FILE,
|
||||
OPTION_AS_REF_DEREF,
|
||||
UNNECESSARY_LAZY_EVALUATION,
|
||||
]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for Methods {
|
||||
|
|
@ -1398,13 +1425,18 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
|
|||
["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]),
|
||||
["expect", ..] => lint_expect(cx, expr, arg_lists[0]),
|
||||
["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0], method_spans[1]),
|
||||
["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]),
|
||||
["unwrap_or_else", "map"] => {
|
||||
lint_lazy_eval(cx, expr, arg_lists[0], true, "unwrap_or");
|
||||
lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]);
|
||||
},
|
||||
["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]),
|
||||
["and_then", ..] => {
|
||||
lint_lazy_eval(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]);
|
||||
},
|
||||
["or_else", ..] => {
|
||||
lint_lazy_eval(cx, expr, arg_lists[0], false, "or");
|
||||
bind_instead_of_map::ResultOrElseErrInfo::lint(cx, expr, arg_lists[0]);
|
||||
},
|
||||
["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]),
|
||||
|
|
@ -1448,6 +1480,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", ..] => lint_lazy_eval(cx, expr, arg_lists[0], true, "unwrap_or"),
|
||||
["get_or_insert_with", ..] => lint_lazy_eval(cx, expr, arg_lists[0], true, "get_or_insert"),
|
||||
["ok_or_else", ..] => lint_lazy_eval(cx, expr, arg_lists[0], true, "ok_or"),
|
||||
_ => {},
|
||||
}
|
||||
|
||||
|
|
@ -2663,6 +2698,99 @@ fn lint_map_flatten<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, map
|
|||
}
|
||||
}
|
||||
|
||||
/// lint use of `<fn>_else(simple closure)` for `Option`s and `Result`s that can be
|
||||
/// replaced with `<fn>(return value of simple closure)`
|
||||
fn lint_lazy_eval<'a, 'tcx>(
|
||||
cx: &LateContext<'a, '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.tables.expr_ty(&args[0]), sym!(option_type));
|
||||
let is_result = is_type_diagnostic_item(cx, cx.tables.expr_ty(&args[0]), sym!(result_type));
|
||||
|
||||
if !is_option && !is_result {
|
||||
return;
|
||||
}
|
||||
|
||||
// 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))
|
||||
}
|
||||
|
||||
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 simplify = match ex.kind {
|
||||
// Closures returning literals can be unconditionally simplified
|
||||
hir::ExprKind::Lit(_) => true,
|
||||
|
||||
// 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(ex, 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 allow_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 {
|
||||
!args.iter().any(|arg| expr_uses_argument(arg, params))
|
||||
} 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,
|
||||
};
|
||||
|
||||
if simplify {
|
||||
let msg = if is_option {
|
||||
"unnecessary closure used to substitute value for `Option::None`"
|
||||
} else {
|
||||
"unnecessary closure used to substitute value for `Result::Err`"
|
||||
};
|
||||
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
UNNECESSARY_LAZY_EVALUATION,
|
||||
expr.span,
|
||||
msg,
|
||||
&format!("Use `{}` instead", simplify_using),
|
||||
format!(
|
||||
"{0}.{1}({2})",
|
||||
snippet(cx, args[0].span, ".."),
|
||||
simplify_using,
|
||||
snippet(cx, ex.span, ".."),
|
||||
),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// lint use of `map().unwrap_or_else()` for `Option`s and `Result`s
|
||||
fn lint_map_unwrap_or_else<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue