diff --git a/CHANGELOG.md b/CHANGELOG.md index 14438edcb98..0065ae1a0af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -705,7 +705,7 @@ All notable changes to this project will be documented in this file. [`nonsensical_open_options`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#nonsensical_open_options [`not_unsafe_ptr_arg_deref`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref [`ok_expect`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#ok_expect -[`option_map_nil_fn`]: https://github.com/Manishearth/rust-clippy/wiki#option_map_nil_fn +[`option_map_unit_fn`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#option_map_unit_fn [`op_ref`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#op_ref [`option_map_or_none`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#option_map_or_none [`option_map_unwrap_or`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#option_map_unwrap_or diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 755fc26bc9b..87781e4e5f7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -145,7 +145,7 @@ pub mod lifetimes; pub mod literal_representation; pub mod loops; pub mod map_clone; -pub mod map_nil_fn; +pub mod option_map_unit_fn; pub mod matches; pub mod mem_forget; pub mod methods; @@ -406,7 +406,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { reg.register_late_lint_pass(box question_mark::QuestionMarkPass); reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl); reg.register_late_lint_pass(box redundant_field_names::RedundantFieldNames); - reg.register_late_lint_pass(box map_nil_fn::Pass); + reg.register_late_lint_pass(box option_map_unit_fn::Pass); reg.register_lint_group("clippy_restriction", vec![ @@ -443,7 +443,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { if_not_else::IF_NOT_ELSE, infinite_iter::MAYBE_INFINITE_ITER, items_after_statements::ITEMS_AFTER_STATEMENTS, - map_nil_fn::OPTION_MAP_NIL_FN, + option_map_unit_fn::OPTION_MAP_UNIT_FN, matches::SINGLE_MATCH_ELSE, methods::FILTER_MAP, methods::OPTION_MAP_UNWRAP_OR, diff --git a/clippy_lints/src/map_nil_fn.rs b/clippy_lints/src/option_map_unit_fn.rs similarity index 69% rename from clippy_lints/src/map_nil_fn.rs rename to clippy_lints/src/option_map_unit_fn.rs index b72c0a01eda..df3c78a344b 100644 --- a/clippy_lints/src/map_nil_fn.rs +++ b/clippy_lints/src/option_map_unit_fn.rs @@ -8,8 +8,8 @@ use utils::paths; #[derive(Clone)] pub struct Pass; -/// **What it does:** Checks for usage of `Option.map(f)` where f is a nil -/// function or closure +/// **What it does:** Checks for usage of `Option.map(f)` where f is a function +/// or closure that returns the unit type. /// /// **Why is this bad?** Readability, this can be written more clearly with /// an if statement @@ -17,12 +17,15 @@ pub struct Pass; /// **Known problems:** None. /// /// **Example:** +/// /// ```rust /// let x : Option<&str> = do_stuff(); /// x.map(log_err_msg); /// x.map(|msg| log_err_msg(format_msg(msg))) /// ``` +/// /// The correct use would be: +/// /// ```rust /// let x : Option<&str> = do_stuff(); /// if let Some(msg) = x { @@ -33,19 +36,19 @@ pub struct Pass; /// } /// ``` declare_clippy_lint! { - pub OPTION_MAP_NIL_FN, + pub OPTION_MAP_UNIT_FN, complexity, - "using `Option.map(f)`, where f is a nil function or closure" + "using `Option.map(f)`, where f is a function or closure that returns ()" } impl LintPass for Pass { fn get_lints(&self) -> LintArray { - lint_array!(OPTION_MAP_NIL_FN) + lint_array!(OPTION_MAP_UNIT_FN) } } -fn is_nil_type(ty: ty::Ty) -> bool { +fn is_unit_type(ty: ty::Ty) -> bool { match ty.sty { ty::TyTuple(slice) => slice.is_empty(), ty::TyNever => true, @@ -53,27 +56,27 @@ fn is_nil_type(ty: ty::Ty) -> bool { } } -fn is_nil_function(cx: &LateContext, expr: &hir::Expr) -> bool { +fn is_unit_function(cx: &LateContext, expr: &hir::Expr) -> bool { let ty = cx.tables.expr_ty(expr); if let ty::TyFnDef(id, _) = ty.sty { if let Some(fn_type) = cx.tcx.fn_sig(id).no_late_bound_regions() { - return is_nil_type(fn_type.output()); + return is_unit_type(fn_type.output()); } } false } -fn is_nil_expression(cx: &LateContext, expr: &hir::Expr) -> bool { - is_nil_type(cx.tables.expr_ty(expr)) +fn is_unit_expression(cx: &LateContext, expr: &hir::Expr) -> bool { + is_unit_type(cx.tables.expr_ty(expr)) } -// The expression inside a closure may or may not have surrounding braces and -// semicolons, which causes problems when generating a suggestion. Given an -// expression that evaluates to '()' or '!', recursively remove useless braces -// and semi-colons until is suitable for including in the suggestion template -fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option { - if !is_nil_expression(cx, expr) { +/// The expression inside a closure may or may not have surrounding braces and +/// semicolons, which causes problems when generating a suggestion. Given an +/// expression that evaluates to '()' or '!', recursively remove useless braces +/// and semi-colons until is suitable for including in the suggestion template +fn reduce_unit_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option { + if !is_unit_expression(cx, expr) { return None; } @@ -87,7 +90,7 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option { // Reduce `{ X }` to `X` - reduce_nil_expression(cx, inner_expr) + reduce_unit_expression(cx, inner_expr) }, (&[ref inner_stmt], None) => { // Reduce `{ X; }` to `X` or `X;` @@ -95,12 +98,12 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option Some(d.span), hir::StmtExpr(ref e, _) => Some(e.span), hir::StmtSemi(ref e, _) => { - if is_nil_expression(cx, e) { - // `X` returns nil so we can strip the + if is_unit_expression(cx, e) { + // `X` returns unit so we can strip the // semicolon and reduce further - reduce_nil_expression(cx, e) + reduce_unit_expression(cx, e) } else { - // `X` doesn't return nil so it needs a + // `X` doesn't return unit so it needs a // trailing semicolon Some(inner_stmt.span) } @@ -114,14 +117,14 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Option<(&'tcx hir::Arg, &'a hir::Expr)> { +fn unit_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Option<(&'tcx hir::Arg, &'a hir::Expr)> { if let hir::ExprClosure(_, ref decl, inner_expr_id, _, _) = expr.node { let body = cx.tcx.hir.body(inner_expr_id); let body_expr = &body.value; if_chain! { if decl.inputs.len() == 1; - if is_nil_expression(cx, body_expr); + if is_unit_expression(cx, body_expr); if let Some(binding) = iter_input_pats(&decl, body).next(); then { return Some((binding, body_expr)); @@ -131,7 +134,7 @@ fn nil_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Opt None } -fn lint_map_nil_fn(cx: &LateContext, stmt: &hir::Stmt, expr: &hir::Expr, map_args: &[hir::Expr]) { +fn lint_map_unit_fn(cx: &LateContext, stmt: &hir::Stmt, expr: &hir::Expr, map_args: &[hir::Expr]) { let var_arg = &map_args[0]; let fn_arg = &map_args[1]; @@ -139,20 +142,20 @@ fn lint_map_nil_fn(cx: &LateContext, stmt: &hir::Stmt, expr: &hir::Expr, map_arg return; } - if is_nil_function(cx, fn_arg) { - let msg = "called `map(f)` on an Option value where `f` is a nil function"; + if is_unit_function(cx, fn_arg) { + let msg = "called `map(f)` on an Option value where `f` is a unit function"; let suggestion = format!("if let Some(...) = {0} {{ {1}(...) }}", snippet(cx, var_arg.span, "_"), snippet(cx, fn_arg.span, "_")); span_lint_and_then(cx, - OPTION_MAP_NIL_FN, + OPTION_MAP_UNIT_FN, expr.span, msg, |db| { db.span_suggestion(stmt.span, "try this", suggestion); }); - } else if let Some((binding, closure_expr)) = nil_closure(cx, fn_arg) { - let msg = "called `map(f)` on an Option value where `f` is a nil closure"; - let suggestion = if let Some(expr_span) = reduce_nil_expression(cx, closure_expr) { + } else if let Some((binding, closure_expr)) = unit_closure(cx, fn_arg) { + let msg = "called `map(f)` on an Option value where `f` is a unit closure"; + let suggestion = if let Some(expr_span) = reduce_unit_expression(cx, closure_expr) { format!("if let Some({0}) = {1} {{ {2} }}", snippet(cx, binding.pat.span, "_"), snippet(cx, var_arg.span, "_"), @@ -164,7 +167,7 @@ fn lint_map_nil_fn(cx: &LateContext, stmt: &hir::Stmt, expr: &hir::Expr, map_arg }; span_lint_and_then(cx, - OPTION_MAP_NIL_FN, + OPTION_MAP_UNIT_FN, expr.span, msg, |db| { db.span_suggestion(stmt.span, "try this", suggestion); }); @@ -180,7 +183,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { if let hir::StmtSemi(ref expr, _) = stmt.node { if let hir::ExprMethodCall(_, _, _) = expr.node { if let Some(arglists) = method_chain_args(expr, &["map"]) { - lint_map_nil_fn(cx, stmt, expr, arglists[0]); + lint_map_unit_fn(cx, stmt, expr, arglists[0]); } } }