nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-05-14 02:49:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #995 from oli-obk/oh_the_horror

Browse Source 
lint on `filter(x).map(y)`, `filter(x).flat_map(y)`, `filter_map(x).flat_map(y)`
This commit is contained in:
llogiq 2016-06-22 13:06:12 +02:00 committed by GitHub
commit f81d253c07
8 changed files with 131 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -158,6 +158,7 @@ All notable changes to this project will be documented in this file.
[`explicit_counter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop
[`explicit_iter_loop`]: https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop
[`extend_from_slice`]: https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice
[`filter_map`]: https://github.com/Manishearth/rust-clippy/wiki#filter_map
[`filter_next`]: https://github.com/Manishearth/rust-clippy/wiki#filter_next
[`float_arithmetic`]: https://github.com/Manishearth/rust-clippy/wiki#float_arithmetic
[`float_cmp`]: https://github.com/Manishearth/rust-clippy/wiki#float_cmp

3
README.md
View File

@ -17,7 +17,7 @@ Table of contents:
## Lints
There are 154 lints included in this crate:
There are 155 lints included in this crate:
name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@ -60,6 +60,7 @@ name
[explicit_counter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_counter_loop) | warn | for-looping with an explicit counter when `_.enumerate()` would do
[explicit_iter_loop](https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop) | warn | for-looping over `_.iter()` or `_.iter_mut()` when `&_` or `&mut _` would do
[extend_from_slice](https://github.com/Manishearth/rust-clippy/wiki#extend_from_slice) | warn | `.extend_from_slice(_)` is a faster way to extend a Vec by a slice
[filter_map](https://github.com/Manishearth/rust-clippy/wiki#filter_map) | allow | using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call
[filter_next](https://github.com/Manishearth/rust-clippy/wiki#filter_next) | warn | using `filter(p).next()`, which is more succinctly expressed as `.find(p)`
[float_arithmetic](https://github.com/Manishearth/rust-clippy/wiki#float_arithmetic) | allow | Any floating-point arithmetic statement
[float_cmp](https://github.com/Manishearth/rust-clippy/wiki#float_cmp) | warn | using `==` or `!=` on float values (as floating-point operations usually involve rounding errors, it is always better to check for approximate equality within small bounds)

1
clippy_lints/src/lib.rs
View File

@ -268,6 +268,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
items_after_statements::ITEMS_AFTER_STATEMENTS,
matches::SINGLE_MATCH_ELSE,
mem_forget::MEM_FORGET,
methods::FILTER_MAP,
methods::OPTION_UNWRAP_USED,
methods::RESULT_UNWRAP_USED,
methods::WRONG_PUB_SELF_CONVENTION,

22
clippy_lints/src/lifetimes.rs
View File

@ -71,19 +71,17 @@ enum RefLt {
Named(Name),
}
fn bound_lifetimes(bound: &TyParamBound) -> Option<HirVec<&Lifetime>> {
fn bound_lifetimes(bound: &TyParamBound) -> HirVec<&Lifetime> {
if let TraitTyParamBound(ref trait_ref, _) = *bound {
let lt = trait_ref.trait_ref
.path
.segments
.last()
.expect("a path must have at least one segment")
.parameters
.lifetimes();
Some(lt)
trait_ref.trait_ref
.path
.segments
.last()
.expect("a path must have at least one segment")
.parameters
.lifetimes()
} else {
None
HirVec::new()
}
}
@ -94,7 +92,7 @@ fn check_fn_inner(cx: &LateContext, decl: &FnDecl, generics: &Generics, span: Sp
let bounds_lts = generics.ty_params
.iter()
.flat_map(|ref typ| typ.bounds.iter().filter_map(bound_lifetimes).flat_map(|lts| lts));
.flat_map(|typ| typ.bounds.iter().flat_map(bound_lifetimes));
if could_use_elision(cx, decl, &generics.lifetimes, bounds_lts) {
span_lint(cx,

41
clippy_lints/src/matches.rs
View File

@ -334,31 +334,30 @@ fn check_match_ref_pats(cx: &LateContext, ex: &Expr, arms: &[Arm], source: Match
/// Get all arms that are unbounded `PatRange`s.
fn all_ranges(cx: &LateContext, arms: &[Arm]) -> Vec<SpannedRange<ConstVal>> {
arms.iter()
.filter_map(|arm| {
.flat_map(|arm| {
if let Arm { ref pats, guard: None, .. } = *arm {
Some(pats.iter().filter_map(|pat| {
if_let_chain! {[
let PatKind::Range(ref lhs, ref rhs) = pat.node,
let Ok(lhs) = eval_const_expr_partial(cx.tcx, lhs, ExprTypeChecked, None),
let Ok(rhs) = eval_const_expr_partial(cx.tcx, rhs, ExprTypeChecked, None)
], {
return Some(SpannedRange { span: pat.span, node: (lhs, rhs) });
}}
if_let_chain! {[
let PatKind::Lit(ref value) = pat.node,
let Ok(value) = eval_const_expr_partial(cx.tcx, value, ExprTypeChecked, None)
], {
return Some(SpannedRange { span: pat.span, node: (value.clone(), value) });
}}
None
}))
pats.iter()
} else {
[].iter()
}.filter_map(|pat| {
if_let_chain! {[
let PatKind::Range(ref lhs, ref rhs) = pat.node,
let Ok(lhs) = eval_const_expr_partial(cx.tcx, lhs, ExprTypeChecked, None),
let Ok(rhs) = eval_const_expr_partial(cx.tcx, rhs, ExprTypeChecked, None)
], {
return Some(SpannedRange { span: pat.span, node: (lhs, rhs) });
}}
if_let_chain! {[
let PatKind::Lit(ref value) = pat.node,
let Ok(value) = eval_const_expr_partial(cx.tcx, value, ExprTypeChecked, None)
], {
return Some(SpannedRange { span: pat.span, node: (value.clone(), value) });
}}
None
}
})
})
.flat_map(IntoIterator::into_iter)
.collect()
}

73
clippy_lints/src/methods.rs
View File

@ -159,6 +159,18 @@ declare_lint! {
"using `filter(p).next()`, which is more succinctly expressed as `.find(p)`"
}
/// **What it does:** This lint `Warn`s on `_.filter(_).map(_)`, `_.filter(_).flat_map(_)`, `_.filter_map(_).flat_map(_)` and similar.
///
/// **Why is this bad?** Readability, this can be written more concisely as a single method call
///
/// **Known problems:** Often requires a condition + Option/Iterator creation inside the closure
///
/// **Example:** `iter.filter(|x| x == 0).map(|x| x * 2)`
declare_lint! {
pub FILTER_MAP, Allow,
"using combinations of `filter`, `map`, `filter_map` and `flat_map` which can usually be written as a single method call"
}
/// **What it does:** This lint `Warn`s on an iterator search (such as `find()`, `position()`, or
/// `rposition()`) followed by a call to `is_some()`.
///
@ -356,6 +368,7 @@ impl LintPass for Pass {
SINGLE_CHAR_PATTERN,
SEARCH_IS_SOME,
TEMPORARY_CSTRING_AS_PTR,
FILTER_MAP,
ITER_NTH)
}
}
@ -379,6 +392,14 @@ impl LateLintPass for Pass {
lint_map_unwrap_or_else(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "next"]) {
lint_filter_next(cx, expr, arglists[0]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "map"]) {
lint_filter_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter_map", "map"]) {
lint_filter_map_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter", "flat_map"]) {
lint_filter_flat_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["filter_map", "flat_map"]) {
lint_filter_map_flat_map(cx, expr, arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["find", "is_some"]) {
lint_search_is_some(cx, expr, "find", arglists[0], arglists[1]);
} else if let Some(arglists) = method_chain_args(expr, &["position", "is_some"]) {
@ -813,11 +834,11 @@ fn lint_map_unwrap_or_else(cx: &LateContext, expr: &hir::Expr, map_args: &Method
#[allow(ptr_arg)]
// Type of MethodArgs is potentially a Vec
/// lint use of `filter().next() for Iterators`
/// lint use of `filter().next()` for `Iterators`
fn lint_filter_next(cx: &LateContext, expr: &hir::Expr, filter_args: &MethodArgs) {
// lint if caller of `.filter().next()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(p).next()` on an Iterator. This is more succinctly expressed by calling `.find(p)` \
let msg = "called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` \
instead.";
let filter_snippet = snippet(cx, filter_args[1].span, "..");
if filter_snippet.lines().count() <= 1 {
@ -834,6 +855,52 @@ fn lint_filter_next(cx: &LateContext, expr: &hir::Expr, filter_args: &MethodArgs
}
}
// Type of MethodArgs is potentially a Vec
/// lint use of `filter().map()` for `Iterators`
fn lint_filter_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(p).map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.filter_map(..)` instead.";
span_lint(cx, FILTER_MAP, expr.span, msg);
}
}
// Type of MethodArgs is potentially a Vec
/// lint use of `filter().map()` for `Iterators`
fn lint_filter_map_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) {
// lint if caller of `.filter().map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter_map(p).map(q)` on an `Iterator`. \
This is more succinctly expressed by only calling `.filter_map(..)` instead.";
span_lint(cx, FILTER_MAP, expr.span, msg);
}
}
// Type of MethodArgs is potentially a Vec
/// lint use of `filter().flat_map()` for `Iterators`
fn lint_filter_flat_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) {
// lint if caller of `.filter().flat_map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter(p).flat_map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning an empty Iterator.";
span_lint(cx, FILTER_MAP, expr.span, msg);
}
}
// Type of MethodArgs is potentially a Vec
/// lint use of `filter_map().flat_map()` for `Iterators`
fn lint_filter_map_flat_map(cx: &LateContext, expr: &hir::Expr, _filter_args: &MethodArgs, _map_args: &MethodArgs) {
// lint if caller of `.filter_map().flat_map()` is an Iterator
if match_trait_method(cx, expr, &paths::ITERATOR) {
let msg = "called `filter_map(p).flat_map(q)` on an `Iterator`. \
This is more succinctly expressed by calling `.flat_map(..)` \
and filtering by returning an empty Iterator.";
span_lint(cx, FILTER_MAP, expr.span, msg);
}
}
#[allow(ptr_arg)]
// Type of MethodArgs is potentially a Vec
/// lint searching an Iterator followed by `is_some()`
@ -841,7 +908,7 @@ fn lint_search_is_some(cx: &LateContext, expr: &hir::Expr, search_method: &str,
is_some_args: &MethodArgs) {
// lint if caller of search is an Iterator
if match_trait_method(cx, &*is_some_args[0], &paths::ITERATOR) {
let msg = format!("called `is_some()` after searching an iterator with {}. This is more succinctly expressed \
let msg = format!("called `is_some()` after searching an `Iterator` with {}. This is more succinctly expressed \
by calling `any()`.",
search_method);
let search_snippet = snippet(cx, search_args[1].span, "..");

25
tests/compile-fail/filter_methods.rs Normal file
View File

@ -0,0 +1,25 @@
#![feature(plugin)]
#![plugin(clippy)]
#![deny(clippy, clippy_pedantic)]
fn main() {
let _: Vec<_> = vec![5; 6].into_iter() //~ERROR called `filter(p).map(q)` on an `Iterator`
.filter(|&x| x == 0)
.map(|x| x * 2)
.collect();
let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter(p).flat_map(q)` on an `Iterator`
.filter(|&x| x == 0)
.flat_map(|x| x.checked_mul(2))
.collect();
let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter_map(p).flat_map(q)` on an `Iterator`
.filter_map(|x| x.checked_mul(2))
.flat_map(|x| x.checked_mul(2))
.collect();
let _: Vec<_> = vec![5i8; 6].into_iter() //~ERROR called `filter_map(p).map(q)` on an `Iterator`
.filter_map(|x| x.checked_mul(2))
.map(|x| x.checked_mul(2))
.collect();
}

4
tests/compile-fail/methods.rs
View File

@ -181,11 +181,11 @@ fn filter_next() {
// check single-line case
let _ = v.iter().filter(|&x| *x < 0).next();
//~^ ERROR called `filter(p).next()` on an Iterator.
//~^ ERROR called `filter(p).next()` on an `Iterator`.
//~| NOTE replace `filter(|&x| *x < 0).next()`
// check multi-line case
let _ = v.iter().filter(|&x| { //~ERROR called `filter(p).next()` on an Iterator.
let _ = v.iter().filter(|&x| { //~ERROR called `filter(p).next()` on an `Iterator`.
*x < 0
}
).next();