Skip to content

Commit

Permalink
initial draft. fixes rust-lang#6061.
Browse files Browse the repository at this point in the history
  • Loading branch information
Eric Loren committed Nov 29, 2020
1 parent 68cf94f commit f722bb5
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1988,6 +1988,7 @@ Released 2018-09-13
[`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
[`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
[`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
[`option_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_filter_map
[`option_if_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
[`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
[`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&methods::NEW_RET_NO_SELF,
&methods::OK_EXPECT,
&methods::OPTION_AS_REF_DEREF,
&methods::OPTION_FILTER_MAP,
&methods::OPTION_MAP_OR_NONE,
&methods::OR_FUN_CALL,
&methods::RESULT_MAP_OR_INTO_OPTION,
Expand Down Expand Up @@ -1469,6 +1470,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::NEW_RET_NO_SELF),
LintId::of(&methods::OK_EXPECT),
LintId::of(&methods::OPTION_AS_REF_DEREF),
LintId::of(&methods::OPTION_FILTER_MAP),
LintId::of(&methods::OPTION_MAP_OR_NONE),
LintId::of(&methods::OR_FUN_CALL),
LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION),
Expand Down Expand Up @@ -1668,6 +1670,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
LintId::of(&methods::NEW_RET_NO_SELF),
LintId::of(&methods::OK_EXPECT),
LintId::of(&methods::OPTION_FILTER_MAP),
LintId::of(&methods::OPTION_MAP_OR_NONE),
LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION),
LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
Expand Down
50 changes: 43 additions & 7 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
mod bind_instead_of_map;
mod inefficient_to_string;
mod manual_saturating_arithmetic;
mod option_filter_map;
mod option_map_unwrap_or;
mod unnecessary_filter_map;
mod unnecessary_lazy_eval;
Expand Down Expand Up @@ -816,6 +817,28 @@ declare_clippy_lint! {
"using `Iterator::step_by(0)`, which will panic at runtime"
}

declare_clippy_lint! {
/// **What it does:** Checks for indirect collection of populated `Option`
///
/// **Why is this bad?** `Option` is like a collection of 0-1 things, so `flatten`
/// automatically does this without suspicious-looking `unwrap` calls.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// my_iter.filter(Option::is_some).map(Option::unwrap)
/// ```
/// Use instead:
/// ```rust
/// my_iter.flatten()
/// ```
pub OPTION_FILTER_MAP,
style,
"filtering `Option` for `Some` then force-unwrapping, which can be one type-safe operation"
}

declare_clippy_lint! {
/// **What it does:** Checks for the use of `iter.nth(0)`.
///
Expand Down Expand Up @@ -1441,6 +1464,7 @@ impl_lint_pass!(Methods => [
FILTER_NEXT,
SKIP_WHILE_NEXT,
FILTER_MAP,
OPTION_FILTER_MAP,
FILTER_MAP_NEXT,
FLAT_MAP_IDENTITY,
FIND_MAP,
Expand Down Expand Up @@ -2912,14 +2936,26 @@ fn lint_skip_while_next<'tcx>(
fn lint_filter_map<'tcx>(
cx: &LateContext<'tcx>,
expr: &'tcx hir::Expr<'_>,
_filter_args: &'tcx [hir::Expr<'_>],
_map_args: &'tcx [hir::Expr<'_>],
filter_args: &'tcx [hir::Expr<'_>],
map_args: &'tcx [hir::Expr<'_>],
) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(..).map(..)` on an `Iterator`";
let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead";
span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
// is it specifically `.filter(Option::is_some).map(Option::unwrap)`?
if option_filter_map::in_scope(filter_args, map_args) {
span_lint_and_help(
cx,
OPTION_FILTER_MAP,
expr.span,
"`filter` for `Some` followed by `unwrap`",
None,
"consider using `flatten` instead",
);
} else {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(..).map(..)` on an `Iterator`";
let hint = "this is more succinctly expressed by calling `.filter_map(..)` instead";
span_lint_and_help(cx, FILTER_MAP, expr.span, msg, None, hint);
}
}
}

Expand Down
14 changes: 14 additions & 0 deletions clippy_lints/src/methods/option_filter_map.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use crate::utils::{match_qpath, paths};
use rustc_hir::{self, Expr, ExprKind};
use std::iter;

fn calls_fn<'tcx>(expr: &'tcx [Expr<'_>], fn_name: &str) -> bool {
expr.get(1).map_or(false, |x| match &x.kind {
ExprKind::Path(qp) => match_qpath(qp, &paths::OPTION.iter().cloned().chain(iter::once(fn_name)).collect::<Vec<&str>>()),
_ => false,
})
}

pub fn in_scope<'tcx>(filter_args: &'tcx [Expr<'_>], map_args: &'tcx [Expr<'_>]) -> bool {
calls_fn(map_args, "unwrap") && calls_fn(filter_args, "is_some")
}
16 changes: 16 additions & 0 deletions tests/ui/option_filter_map.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
fn odds_out(x: i32) -> Option<i32> {
if x % 2 == 0 {
Some(x)
} else {
None
}
}

fn main() {
let evens: Vec<i32> = vec![1, 2, 3]
.into_iter()
.map(odds_out)
.filter(Option::is_some)
.map(Option::unwrap)
.collect();
}
16 changes: 16 additions & 0 deletions tests/ui/option_filter_map.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: `filter` for `Some` followed by `unwrap`
--> $DIR/option_filter_map.rs:10:27
|
LL | let evens: Vec<i32> = vec![1, 2, 3]
| ___________________________^
LL | | .into_iter()
LL | | .map(odds_out)
LL | | .filter(Option::is_some)
LL | | .map(Option::unwrap)
| |____________________________^
|
= note: `-D clippy::option-filter-map` implied by `-D warnings`
= help: consider using `flatten` instead

error: aborting due to previous error

0 comments on commit f722bb5

Please sign in to comment.