diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index d27383ca332..8d4bb06e59b 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -22,6 +22,7 @@ use syntax::ast; use syntax::source_map::{BytePos, Span}; use syntax::symbol::LocalInternedString; +mod option_map_unwrap_or; mod unnecessary_filter_map; #[derive(Clone)] @@ -836,7 +837,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { ["unwrap", "get_mut"] => lint_get_unwrap(cx, expr, arg_lists[1], true), ["unwrap", ..] => lint_unwrap(cx, expr, arg_lists[0]), ["expect", "ok"] => lint_ok_expect(cx, expr, arg_lists[1]), - ["unwrap_or", "map"] => lint_map_unwrap_or(cx, expr, arg_lists[1], arg_lists[0]), + ["unwrap_or", "map"] => option_map_unwrap_or::lint(cx, expr, arg_lists[1], arg_lists[0]), ["unwrap_or_else", "map"] => lint_map_unwrap_or_else(cx, expr, arg_lists[1], arg_lists[0]), ["map_or", ..] => lint_map_or_none(cx, expr, arg_lists[0]), ["next", "filter"] => lint_filter_next(cx, expr, arg_lists[1]), @@ -1769,49 +1770,6 @@ fn lint_ok_expect(cx: &LateContext<'_, '_>, expr: &hir::Expr, ok_args: &[hir::Ex } } -/// lint use of `map().unwrap_or()` for `Option`s -fn lint_map_unwrap_or(cx: &LateContext<'_, '_>, expr: &hir::Expr, map_args: &[hir::Expr], unwrap_args: &[hir::Expr]) { - // lint if the caller of `map()` is an `Option` - let unwrap_ty = cx.tables.expr_ty(&unwrap_args[1]); - if match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION) && is_copy(cx, unwrap_ty) { - // get snippets for args to map() and unwrap_or() - let map_snippet = snippet(cx, map_args[1].span, ".."); - let unwrap_snippet = snippet(cx, unwrap_args[1].span, ".."); - // lint message - // comparing the snippet from source to raw text ("None") below is safe - // because we already have checked the type. - let arg = if unwrap_snippet == "None" { "None" } else { "a" }; - let suggest = if unwrap_snippet == "None" { - "and_then(f)" - } else { - "map_or(a, f)" - }; - let msg = &format!( - "called `map(f).unwrap_or({})` on an Option value. \ - This can be done more directly by calling `{}` instead", - arg, suggest - ); - // lint, with note if neither arg is > 1 line and both map() and - // unwrap_or() have the same span - let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1; - let same_span = map_args[1].span.ctxt() == unwrap_args[1].span.ctxt(); - if same_span && !multiline { - let suggest = if unwrap_snippet == "None" { - format!("and_then({})", map_snippet) - } else { - format!("map_or({}, {})", unwrap_snippet, map_snippet) - }; - let note = format!( - "replace `map({}).unwrap_or({})` with `{}`", - map_snippet, unwrap_snippet, suggest - ); - span_note_and_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span, ¬e); - } else if same_span && multiline { - span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg); - }; - } -} - /// lint use of `map().flatten()` for `Iterators` fn lint_map_flatten<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr, map_args: &'tcx [hir::Expr]) { // lint if caller of `.map().flatten()` is an Iterator diff --git a/clippy_lints/src/methods/option_map_unwrap_or.rs b/clippy_lints/src/methods/option_map_unwrap_or.rs new file mode 100644 index 00000000000..39bcf415fe9 --- /dev/null +++ b/clippy_lints/src/methods/option_map_unwrap_or.rs @@ -0,0 +1,49 @@ +use crate::utils::paths; +use crate::utils::{is_copy, match_type, snippet, span_lint, span_note_and_lint}; +use rustc::hir; +use rustc::lint::LateContext; + +use super::OPTION_MAP_UNWRAP_OR; + +/// lint use of `map().unwrap_or()` for `Option`s +pub(super) fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr, map_args: &[hir::Expr], unwrap_args: &[hir::Expr]) { + // lint if the caller of `map()` is an `Option` + let unwrap_ty = cx.tables.expr_ty(&unwrap_args[1]); + if match_type(cx, cx.tables.expr_ty(&map_args[0]), &paths::OPTION) && is_copy(cx, unwrap_ty) { + // get snippets for args to map() and unwrap_or() + let map_snippet = snippet(cx, map_args[1].span, ".."); + let unwrap_snippet = snippet(cx, unwrap_args[1].span, ".."); + // lint message + // comparing the snippet from source to raw text ("None") below is safe + // because we already have checked the type. + let arg = if unwrap_snippet == "None" { "None" } else { "a" }; + let suggest = if unwrap_snippet == "None" { + "and_then(f)" + } else { + "map_or(a, f)" + }; + let msg = &format!( + "called `map(f).unwrap_or({})` on an Option value. \ + This can be done more directly by calling `{}` instead", + arg, suggest + ); + // lint, with note if neither arg is > 1 line and both map() and + // unwrap_or() have the same span + let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1; + let same_span = map_args[1].span.ctxt() == unwrap_args[1].span.ctxt(); + if same_span && !multiline { + let suggest = if unwrap_snippet == "None" { + format!("and_then({})", map_snippet) + } else { + format!("map_or({}, {})", unwrap_snippet, map_snippet) + }; + let note = format!( + "replace `map({}).unwrap_or({})` with `{}`", + map_snippet, unwrap_snippet, suggest + ); + span_note_and_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg, expr.span, ¬e); + } else if same_span && multiline { + span_lint(cx, OPTION_MAP_UNWRAP_OR, expr.span, msg); + }; + } +}