Skip to content

Commit d663353

Browse files
committed
WIP: add and_then_then_some lint
1 parent 0ce07f6 commit d663353

File tree

7 files changed

+214
-0
lines changed

7 files changed

+214
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5199,6 +5199,7 @@ Released 2018-09-13
51995199
[`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range
52005200
[`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range
52015201
[`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
5202+
[`and_then_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#and_then_then_some
52025203
[`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
52035204
[`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
52045205
[`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
// noise reduction, remove before committing!
2+
#![allow(unused_variables)]
3+
4+
use rustc_errors::Applicability;
5+
use rustc_hir::*;
6+
use rustc_hir::def::Res;
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::declare_lint_pass;
9+
use rustc_span::Span;
10+
use clippy_utils::{match_def_path, fn_def_id};
11+
use clippy_utils::diagnostics::span_lint_and_sugg;
12+
use clippy_utils::source::snippet_with_applicability;
13+
14+
declare_clippy_lint! {
15+
/// ### What it does
16+
/// Detects usage of `Option::and_then` and `bool::then_some` that could
17+
/// be replaced with `Option::filter`.
18+
///
19+
/// ### Why is this bad?
20+
/// Needless complexity, uses recent and uncommon stdlib funtions instead of
21+
/// one older function.
22+
///
23+
/// ### Example
24+
/// ```no_run
25+
/// // example code where clippy issues a warning
26+
/// ```
27+
/// Use instead:
28+
/// ```no_run
29+
/// // example code which does not raise clippy warning
30+
/// ```
31+
#[clippy::version = "1.81.0"]
32+
pub AND_THEN_THEN_SOME,
33+
nursery,
34+
"default lint description"
35+
}
36+
37+
// note: `Option::filter` is older than `bool::then_some`,
38+
// so no msrv check is required.
39+
declare_lint_pass!(AndThenThenSome => [AND_THEN_THEN_SOME]);
40+
41+
impl<'tcx> LateLintPass<'tcx> for AndThenThenSome {
42+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
43+
match expr.kind {
44+
ExprKind::MethodCall(method_name, selfarg, [ arg ], span) => {
45+
//let option_id = cx.tcx.get_diagnostic_item(sym::Option);
46+
// TODO: check if type of reciever is diagnostic item Option.
47+
let tckr = cx.typeck_results();
48+
let def_id = tckr.type_dependent_def_id(expr.hir_id).unwrap();
49+
//dbg!(method_name, selfarg, arg);
50+
if match_def_path(cx, def_id,
51+
&["core", "option", "Option", "and_then"])
52+
{
53+
if let Some((closure_args, predicate)) = then_some_closure_arg(cx, arg) {
54+
//dbg!(predicate);
55+
show_sugg(cx, expr.span, selfarg, closure_args, predicate);
56+
}
57+
}
58+
}
59+
// TODO: check for call as associated function
60+
_ => {},
61+
}
62+
}
63+
}
64+
65+
// `|v| X.then_some(v)` -> Some((span"|v|", X))
66+
fn then_some_closure_arg<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>)
67+
-> Option<(Span, &'tcx Expr<'tcx>)>
68+
{
69+
match expr.kind {
70+
ExprKind::Closure(Closure{
71+
fn_decl: FnDecl{ inputs: [ Ty{ hir_id: arg_id, ..} ], .. },
72+
body,
73+
..
74+
}) => {
75+
if let Node::Expr(expr) = cx.tcx.hir_node(body.hir_id) {
76+
//dbg!(arg_id);
77+
if let Some(body) = peel_closure_body(cx, expr, *arg_id) {
78+
Some((cx.tcx.hir().span(*arg_id), body))
79+
} else {
80+
None
81+
}
82+
} else {
83+
None
84+
}
85+
},
86+
_ => None,
87+
}
88+
}
89+
90+
fn peel_closure_body<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, closure_arg_id: HirId) -> Option<&'tcx Expr<'tcx>> {
91+
92+
//dbg!(cx.tcx.hir_node(closure_arg_id));
93+
match expr.kind {
94+
ExprKind::Block(_block, _) =>
95+
// recurse somehow, maybe lift { x; y.a() } into { x; y }.a()
96+
todo!(),
97+
ExprKind::MethodCall(_path, selfarg, [ arg ], _span) => {
98+
if is_then_some(cx, expr) &&
99+
is_local_defined_at(cx, arg, closure_arg_id)
100+
{
101+
// the argument to then_some is the same as that given to the closure
102+
Some(selfarg)
103+
} else {
104+
None
105+
}
106+
}
107+
ExprKind::Call(func /*@ Expr{ kind: ExprKind::Path(QPath::Resolved(_, Path{ res: Res::Local(local_hid), ..})), ..},*/, [ pred, arg ]) => {
108+
//dbg!(func, fn_def_id(cx, expr));
109+
//todo!();
110+
if is_then_some(cx, func) && dbg!(is_local_defined_at(cx, arg, closure_arg_id)) {
111+
//todo!("it worked!!");
112+
Some(pred)
113+
114+
} else {
115+
//todo!("nope");
116+
None
117+
}
118+
}
119+
_ => None,
120+
}
121+
}
122+
123+
fn is_local_defined_at<'tcx>(cx: &LateContext<'tcx>, local: &Expr<'_>, arg_hid: HirId) -> bool {
124+
//dbg!(local);
125+
match local.kind {
126+
ExprKind::Path(QPath::Resolved(_, Path{ res: Res::Local(local_hid), .. })) => {
127+
// XXX" this is the best way i could find to compare if a local refers to a specific closure argument.
128+
if let Node::Pat(Pat{ span: local_span, .. }) = cx.tcx.hir_node(*local_hid) &&
129+
let Node::Ty(Ty{ span: arg_span, .. }) = cx.tcx.hir_node(arg_hid) &&
130+
local_span == arg_span
131+
{
132+
true
133+
} else {
134+
false
135+
}
136+
}
137+
// is not local at all, so definitly isn't a local defined at the given position
138+
_ => false,
139+
}
140+
}
141+
142+
fn show_sugg(cx: &LateContext<'_>, span: Span, selfarg: &Expr<'_>, closure_args: Span, predicate: &Expr<'_>) {
143+
let mut appl = Applicability::MachineApplicable;
144+
let sugg = format!(
145+
"{}.filter(|{}| {})",
146+
snippet_with_applicability(cx, selfarg.span, "<OPTION>", &mut appl),
147+
snippet_with_applicability(cx, closure_args, "<ARGS>", &mut appl),
148+
snippet_with_applicability(cx, predicate.span, "<PREDICATE>", &mut appl));
149+
span_lint_and_sugg(cx, AND_THEN_THEN_SOME, span,
150+
"use of `and_then` + `then_some` is equivelent to `filter`",
151+
"use `Option::filter` instead",
152+
sugg, appl);
153+
}
154+
155+
fn is_then_some(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
156+
if let Some(def_id) = fn_def_id(cx, expr) {
157+
match_def_path(
158+
cx, dbg!(def_id),
159+
&["core", "bool", "<impl bool>", "then_some"])
160+
} else {
161+
//todo!("not type dependent");
162+
false
163+
}
164+
}
165+

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
4040
crate::absolute_paths::ABSOLUTE_PATHS_INFO,
4141
crate::allow_attributes::ALLOW_ATTRIBUTES_INFO,
4242
crate::almost_complete_range::ALMOST_COMPLETE_RANGE_INFO,
43+
crate::and_then_then_some::AND_THEN_THEN_SOME_INFO,
4344
crate::approx_const::APPROX_CONSTANT_INFO,
4445
crate::arc_with_non_send_sync::ARC_WITH_NON_SEND_SYNC_INFO,
4546
crate::as_conversions::AS_CONVERSIONS_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ mod renamed_lints;
7373
mod absolute_paths;
7474
mod allow_attributes;
7575
mod almost_complete_range;
76+
mod and_then_then_some;
7677
mod approx_const;
7778
mod arc_with_non_send_sync;
7879
mod as_conversions;
@@ -1171,6 +1172,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11711172
});
11721173
store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
11731174
store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
1175+
store.register_late_pass(|_| Box::new(and_then_then_some::AndThenThenSome));
11741176
// add lints here, do not remove this comment, it's used in `new_lint`
11751177
}
11761178

tests/ui/and_then_then_some.fixed

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#![warn(clippy::and_then_then_some)]
2+
3+
fn main() {
4+
let x = Some("foo".to_string());
5+
6+
let _y = x.clone().filter(|v| v.starts_with('f'));
7+
8+
let ts = bool::then_some;
9+
// even if the function is renamed, it should still fire
10+
let _z = x.clone().and_then(|v| (ts(v.starts_with('f'), v)));
11+
// even if it's called as an associated method with a block body
12+
/*let _w = Option::and_then(x, |v: String| {
13+
bool::then_some(v.starts_with('f'), v)
14+
});*/
15+
}

tests/ui/and_then_then_some.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#![warn(clippy::and_then_then_some)]
2+
3+
fn main() {
4+
let x = Some("foo".to_string());
5+
6+
//let _y = x.clone().and_then(|v| v.starts_with('f')
7+
// .then_some(v));
8+
9+
//let ts = bool::then_some;
10+
//use std::primative::bool::then_some as ts;
11+
// even if the function is renamed, it should still fire
12+
let _z = x.clone().and_then(|v| bool::then_some(v.starts_with('f'), v));
13+
// even if it's called as an associated method with a block body
14+
let _w = Option::and_then(x, |v: String| {
15+
bool::then_some(v.starts_with('f'), v)
16+
});
17+
}

tests/ui/and_then_then_some.stderr

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
error: use of `and_then` + `then_some` is equivelent to `filter`
2+
--> tests/ui/and_then_then_some.rs:6:14
3+
|
4+
LL | let _y = x.clone().and_then(|v| v.starts_with('f')
5+
| ______________^
6+
LL | | .then_some(v));
7+
| |______________________^ help: use `Option::filter` instead: `x.clone().filter(|v| v.starts_with('f'))`
8+
|
9+
= note: `-D clippy::and-then-then-some` implied by `-D warnings`
10+
= help: to override `-D warnings` add `#[allow(clippy::and_then_then_some)]`
11+
12+
error: aborting due to 1 previous error
13+

0 commit comments

Comments
 (0)