Skip to content

Commit bbb9788

Browse files
committed
Add lint for 'toilet closures'
1 parent 04849bd commit bbb9788

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+354
-187
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5976,6 +5976,7 @@ Released 2018-09-13
59765976
[`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
59775977
[`to_string_trait_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_trait_impl
59785978
[`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
5979+
[`toilet_closures`]: https://rust-lang.github.io/rust-clippy/master/index.html#toilet_closures
59795980
[`too_long_first_doc_paragraph`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_long_first_doc_paragraph
59805981
[`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
59815982
[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
692692
crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
693693
crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
694694
crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO,
695+
crate::toilet_closures::TOILET_CLOSURES_INFO,
695696
crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO,
696697
crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO,
697698
crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ mod temporary_assignment;
352352
mod tests_outside_test_module;
353353
mod to_digit_is_some;
354354
mod to_string_trait_impl;
355+
mod toilet_closures;
355356
mod trailing_empty_array;
356357
mod trait_bounds;
357358
mod transmute;
@@ -949,5 +950,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
949950
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
950951
store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));
951952
store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp));
953+
store.register_late_pass(|_| Box::new(toilet_closures::ToiletClosures));
952954
// add lints here, do not remove this comment, it's used in `new_lint`
953955
}

clippy_lints/src/returns.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,9 @@ impl RetReplacement<'_> {
153153
fn applicability(&self) -> Applicability {
154154
match self {
155155
Self::Expr(_, ap) | Self::NeedsPar(_, ap) => *ap,
156-
_ => Applicability::MachineApplicable,
156+
Self::Empty | Self::Unit => Applicability::MachineApplicable,
157+
// This should be MachineApplicable, but it can trigger `toilet_closures`.
158+
Self::Block => Applicability::MaybeIncorrect,
157159
}
158160
}
159161
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_applicability;
3+
use rustc_errors::Applicability;
4+
use rustc_hir::{Block, Closure, ClosureKind, Expr, ExprKind, TyKind};
5+
use rustc_lint::{LateContext, LateLintPass};
6+
use rustc_middle::ty;
7+
use rustc_session::declare_lint_pass;
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
///
12+
/// Detects a closure which takes one argument and returns unit.
13+
///
14+
/// ### Why is this bad?
15+
///
16+
/// This is a less clear way to write `drop`.
17+
///
18+
/// ### Example
19+
/// ```no_run
20+
/// fn drop_ok<T, E>(result: Result<T, E>) -> Result<(), E> {
21+
/// result.map_err(|_| ())
22+
/// }
23+
/// ```
24+
/// Use instead:
25+
/// ```no_run
26+
/// fn drop_ok<T, E>(result: Result<T, E>) -> Result<(), E> {
27+
/// result.map_err(drop)
28+
/// }
29+
/// ```
30+
#[clippy::version = "1.83.0"]
31+
pub TOILET_CLOSURES,
32+
style,
33+
"checks for 'toilet closure' usage"
34+
}
35+
36+
declare_lint_pass!(ToiletClosures => [TOILET_CLOSURES]);
37+
38+
fn is_empty_expr(expr: &Expr<'_>) -> bool {
39+
matches!(
40+
expr.kind,
41+
ExprKind::Tup([])
42+
| ExprKind::Block(
43+
Block {
44+
stmts: [],
45+
expr: None,
46+
..
47+
},
48+
None,
49+
)
50+
)
51+
}
52+
53+
impl<'tcx> LateLintPass<'tcx> for ToiletClosures {
54+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
55+
let ExprKind::Closure(Closure {
56+
fn_decl,
57+
body,
58+
kind: ClosureKind::Closure,
59+
..
60+
}) = expr.kind
61+
else {
62+
return;
63+
};
64+
65+
// Check that the closure takes one argument
66+
if let [arg_ty] = fn_decl.inputs
67+
// and returns `()` or `{}`
68+
&& is_empty_expr(cx.tcx.hir().body(*body).value)
69+
// and the closure is not higher ranked, which `drop` cannot replace
70+
&& let ty::Closure(_, generic_args) = cx.typeck_results().expr_ty(expr).kind()
71+
&& generic_args.as_closure().sig().bound_vars().is_empty()
72+
{
73+
let mut applicability = Applicability::MachineApplicable;
74+
span_lint_and_sugg(
75+
cx,
76+
TOILET_CLOSURES,
77+
expr.span,
78+
"defined a 'toilet closure'",
79+
"replace with",
80+
if matches!(arg_ty.kind, TyKind::Infer) {
81+
String::from("drop")
82+
} else {
83+
let arg_ty_snippet = snippet_with_applicability(cx, arg_ty.span, "...", &mut applicability);
84+
format!("drop::<{arg_ty_snippet}>",)
85+
},
86+
applicability,
87+
);
88+
}
89+
}
90+
}

tests/ui/crashes/ice-11337.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#![feature(trait_alias)]
2+
#![allow(clippy::toilet_closures)]
23

34
trait Confusing<F> = Fn(i32) where F: Fn(u32);
45

tests/ui/explicit_auto_deref.fixed

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
clippy::too_many_arguments,
1212
clippy::borrow_deref_ref,
1313
clippy::let_unit_value,
14-
clippy::needless_lifetimes
14+
clippy::needless_lifetimes,
15+
clippy::toilet_closures
1516
)]
1617

1718
trait CallableStr {

tests/ui/explicit_auto_deref.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
clippy::too_many_arguments,
1212
clippy::borrow_deref_ref,
1313
clippy::let_unit_value,
14-
clippy::needless_lifetimes
14+
clippy::needless_lifetimes,
15+
clippy::toilet_closures
1516
)]
1617

1718
trait CallableStr {

0 commit comments

Comments
 (0)