1
Fork 0

Rename lint to option_map_unit_fn

Rust does not have nil.
This commit is contained in:
Philipp Hansch 2018-04-09 08:19:40 +02:00
parent fbd71f901f
commit a3ff21f4d6
No known key found for this signature in database
GPG key ID: B6FA06A6E0E2665B
3 changed files with 39 additions and 36 deletions

View file

@ -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 [`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 [`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 [`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 [`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_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 [`option_map_unwrap_or`]: https://rust-lang-nursery.github.io/rust-clippy/master/index.html#option_map_unwrap_or

View file

@ -145,7 +145,7 @@ pub mod lifetimes;
pub mod literal_representation; pub mod literal_representation;
pub mod loops; pub mod loops;
pub mod map_clone; pub mod map_clone;
pub mod map_nil_fn; pub mod option_map_unit_fn;
pub mod matches; pub mod matches;
pub mod mem_forget; pub mod mem_forget;
pub mod methods; 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 question_mark::QuestionMarkPass);
reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl); 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 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![ 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, if_not_else::IF_NOT_ELSE,
infinite_iter::MAYBE_INFINITE_ITER, infinite_iter::MAYBE_INFINITE_ITER,
items_after_statements::ITEMS_AFTER_STATEMENTS, 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, matches::SINGLE_MATCH_ELSE,
methods::FILTER_MAP, methods::FILTER_MAP,
methods::OPTION_MAP_UNWRAP_OR, methods::OPTION_MAP_UNWRAP_OR,

View file

@ -8,8 +8,8 @@ use utils::paths;
#[derive(Clone)] #[derive(Clone)]
pub struct Pass; pub struct Pass;
/// **What it does:** Checks for usage of `Option.map(f)` where f is a nil /// **What it does:** Checks for usage of `Option.map(f)` where f is a function
/// function or closure /// or closure that returns the unit type.
/// ///
/// **Why is this bad?** Readability, this can be written more clearly with /// **Why is this bad?** Readability, this can be written more clearly with
/// an if statement /// an if statement
@ -17,12 +17,15 @@ pub struct Pass;
/// **Known problems:** None. /// **Known problems:** None.
/// ///
/// **Example:** /// **Example:**
///
/// ```rust /// ```rust
/// let x : Option<&str> = do_stuff(); /// let x : Option<&str> = do_stuff();
/// x.map(log_err_msg); /// x.map(log_err_msg);
/// x.map(|msg| log_err_msg(format_msg(msg))) /// x.map(|msg| log_err_msg(format_msg(msg)))
/// ``` /// ```
///
/// The correct use would be: /// The correct use would be:
///
/// ```rust /// ```rust
/// let x : Option<&str> = do_stuff(); /// let x : Option<&str> = do_stuff();
/// if let Some(msg) = x { /// if let Some(msg) = x {
@ -33,19 +36,19 @@ pub struct Pass;
/// } /// }
/// ``` /// ```
declare_clippy_lint! { declare_clippy_lint! {
pub OPTION_MAP_NIL_FN, pub OPTION_MAP_UNIT_FN,
complexity, 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 { impl LintPass for Pass {
fn get_lints(&self) -> LintArray { 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 { match ty.sty {
ty::TyTuple(slice) => slice.is_empty(), ty::TyTuple(slice) => slice.is_empty(),
ty::TyNever => true, 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); let ty = cx.tables.expr_ty(expr);
if let ty::TyFnDef(id, _) = ty.sty { if let ty::TyFnDef(id, _) = ty.sty {
if let Some(fn_type) = cx.tcx.fn_sig(id).no_late_bound_regions() { 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 false
} }
fn is_nil_expression(cx: &LateContext, expr: &hir::Expr) -> bool { fn is_unit_expression(cx: &LateContext, expr: &hir::Expr) -> bool {
is_nil_type(cx.tables.expr_ty(expr)) is_unit_type(cx.tables.expr_ty(expr))
} }
// The expression inside a closure may or may not have surrounding braces and /// The expression inside a closure may or may not have surrounding braces and
// semicolons, which causes problems when generating a suggestion. Given an /// semicolons, which causes problems when generating a suggestion. Given an
// expression that evaluates to '()' or '!', recursively remove useless braces /// expression that evaluates to '()' or '!', recursively remove useless braces
// and semi-colons until is suitable for including in the suggestion template /// and semi-colons until is suitable for including in the suggestion template
fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option<Span> { fn reduce_unit_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option<Span> {
if !is_nil_expression(cx, expr) { if !is_unit_expression(cx, expr) {
return None; return None;
} }
@ -87,7 +90,7 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option<Sp
match (&block.stmts[..], block.expr.as_ref()) { match (&block.stmts[..], block.expr.as_ref()) {
(&[], Some(inner_expr)) => { (&[], Some(inner_expr)) => {
// Reduce `{ X }` to `X` // Reduce `{ X }` to `X`
reduce_nil_expression(cx, inner_expr) reduce_unit_expression(cx, inner_expr)
}, },
(&[ref inner_stmt], None) => { (&[ref inner_stmt], None) => {
// Reduce `{ X; }` to `X` or `X;` // Reduce `{ X; }` to `X` or `X;`
@ -95,12 +98,12 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option<Sp
hir::StmtDecl(ref d, _) => Some(d.span), hir::StmtDecl(ref d, _) => Some(d.span),
hir::StmtExpr(ref e, _) => Some(e.span), hir::StmtExpr(ref e, _) => Some(e.span),
hir::StmtSemi(ref e, _) => { hir::StmtSemi(ref e, _) => {
if is_nil_expression(cx, e) { if is_unit_expression(cx, e) {
// `X` returns nil so we can strip the // `X` returns unit so we can strip the
// semicolon and reduce further // semicolon and reduce further
reduce_nil_expression(cx, e) reduce_unit_expression(cx, e)
} else { } else {
// `X` doesn't return nil so it needs a // `X` doesn't return unit so it needs a
// trailing semicolon // trailing semicolon
Some(inner_stmt.span) Some(inner_stmt.span)
} }
@ -114,14 +117,14 @@ fn reduce_nil_expression<'a>(cx: &LateContext, expr: &'a hir::Expr) -> Option<Sp
} }
} }
fn nil_closure<'a, 'tcx>(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 { if let hir::ExprClosure(_, ref decl, inner_expr_id, _, _) = expr.node {
let body = cx.tcx.hir.body(inner_expr_id); let body = cx.tcx.hir.body(inner_expr_id);
let body_expr = &body.value; let body_expr = &body.value;
if_chain! { if_chain! {
if decl.inputs.len() == 1; 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(); if let Some(binding) = iter_input_pats(&decl, body).next();
then { then {
return Some((binding, body_expr)); return Some((binding, body_expr));
@ -131,7 +134,7 @@ fn nil_closure<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'a hir::Expr) -> Opt
None 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 var_arg = &map_args[0];
let fn_arg = &map_args[1]; 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; return;
} }
if is_nil_function(cx, fn_arg) { if is_unit_function(cx, fn_arg) {
let msg = "called `map(f)` on an Option value where `f` is a nil function"; let msg = "called `map(f)` on an Option value where `f` is a unit function";
let suggestion = format!("if let Some(...) = {0} {{ {1}(...) }}", let suggestion = format!("if let Some(...) = {0} {{ {1}(...) }}",
snippet(cx, var_arg.span, "_"), snippet(cx, var_arg.span, "_"),
snippet(cx, fn_arg.span, "_")); snippet(cx, fn_arg.span, "_"));
span_lint_and_then(cx, span_lint_and_then(cx,
OPTION_MAP_NIL_FN, OPTION_MAP_UNIT_FN,
expr.span, expr.span,
msg, msg,
|db| { db.span_suggestion(stmt.span, "try this", suggestion); }); |db| { db.span_suggestion(stmt.span, "try this", suggestion); });
} else if let Some((binding, closure_expr)) = nil_closure(cx, fn_arg) { } 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 nil closure"; let msg = "called `map(f)` on an Option value where `f` is a unit closure";
let suggestion = if let Some(expr_span) = reduce_nil_expression(cx, closure_expr) { let suggestion = if let Some(expr_span) = reduce_unit_expression(cx, closure_expr) {
format!("if let Some({0}) = {1} {{ {2} }}", format!("if let Some({0}) = {1} {{ {2} }}",
snippet(cx, binding.pat.span, "_"), snippet(cx, binding.pat.span, "_"),
snippet(cx, var_arg.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, span_lint_and_then(cx,
OPTION_MAP_NIL_FN, OPTION_MAP_UNIT_FN,
expr.span, expr.span,
msg, msg,
|db| { db.span_suggestion(stmt.span, "try this", suggestion); }); |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::StmtSemi(ref expr, _) = stmt.node {
if let hir::ExprMethodCall(_, _, _) = expr.node { if let hir::ExprMethodCall(_, _, _) = expr.node {
if let Some(arglists) = method_chain_args(expr, &["map"]) { 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]);
} }
} }
} }