Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add empty closure lint #5355

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1242,6 +1242,7 @@ Released 2018-09-13
[`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument
[`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
[`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
[`empty_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_closure
[`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
[`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr
[`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
48 changes: 48 additions & 0 deletions clippy_lints/src/empty_closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use crate::utils::{match_def_path, paths, span_lint};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Checks for empty closure.
ThibsG marked this conversation as resolved.
Show resolved Hide resolved
///
/// **Why is this bad?** Empty closure does nothing, it can be removed.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// std::thread::spawn(|| {});
/// ```
pub EMPTY_CLOSURE,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the name should probably be spawn_empty_closure

style,
"closure with empty body"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably mention spawning

}

declare_lint_pass!(EmptyClosure => [EMPTY_CLOSURE]);

impl LateLintPass<'_, '_> for EmptyClosure {
fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &'_ Expr<'_>) {
if_chain! {
if let ExprKind::Call(ref path_expr, ref args) = expr.kind;
if args.len() == 1;
if let ExprKind::Path(ref qpath) = path_expr.kind;
if let Some(def_id) = cx.tables.qpath_res(qpath, path_expr.hir_id).opt_def_id();
if match_def_path(cx, def_id, &paths::THREAD_SPAWN);
if let ExprKind::Closure(_, _, body_id, ..) = args[0].kind;
let body = cx.tcx.hir().body(body_id);
if let ExprKind::Block(ref b, _) = body.value.kind;
if b.stmts.is_empty() && b.expr.is_none();
then {
span_lint(
cx,
EMPTY_CLOSURE,
args[0].span,
"Empty closure may be removed",
);
}
}
}
}
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ pub mod drop_bounds;
pub mod drop_forget_ref;
pub mod duration_subsec;
pub mod else_if_without_else;
pub mod empty_closure;
pub mod empty_enum;
pub mod entry;
pub mod enum_clike;
Expand Down Expand Up @@ -527,6 +528,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&drop_forget_ref::FORGET_REF,
&duration_subsec::DURATION_SUBSEC,
&else_if_without_else::ELSE_IF_WITHOUT_ELSE,
&empty_closure::EMPTY_CLOSURE,
&empty_enum::EMPTY_ENUM,
&entry::MAP_ENTRY,
&enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,
Expand Down Expand Up @@ -1027,6 +1029,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| box macro_use::MacroUseImports);
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
store.register_late_pass(|| box empty_closure::EmptyClosure);

store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
Expand Down Expand Up @@ -1172,6 +1175,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&drop_forget_ref::FORGET_COPY),
LintId::of(&drop_forget_ref::FORGET_REF),
LintId::of(&duration_subsec::DURATION_SUBSEC),
LintId::of(&empty_closure::EMPTY_CLOSURE),
LintId::of(&entry::MAP_ENTRY),
LintId::of(&enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
LintId::of(&enum_variants::ENUM_VARIANT_NAMES),
Expand Down Expand Up @@ -1404,6 +1408,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&comparison_chain::COMPARISON_CHAIN),
LintId::of(&doc::MISSING_SAFETY_DOC),
LintId::of(&doc::NEEDLESS_DOCTEST_MAIN),
LintId::of(&empty_closure::EMPTY_CLOSURE),
LintId::of(&enum_variants::ENUM_VARIANT_NAMES),
LintId::of(&enum_variants::MODULE_INCEPTION),
LintId::of(&eq_op::OP_REF),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ pub const STRING: [&str; 3] = ["alloc", "string", "String"];
pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"];
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
pub const THREAD_SPAWN: [&str; 3] = ["std", "thread", "spawn"];
pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"];
pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];
pub const TO_STRING: [&str; 3] = ["alloc", "string", "ToString"];
Expand Down
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 361] = [
pub const ALL_LINTS: [Lint; 362] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -427,6 +427,13 @@ pub const ALL_LINTS: [Lint; 361] = [
deprecation: None,
module: "else_if_without_else",
},
Lint {
name: "empty_closure",
group: "style",
desc: "closure with empty body",
deprecation: None,
module: "empty_closure",
},
Lint {
name: "empty_enum",
group: "pedantic",
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/empty_closure.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![warn(clippy::empty_closure)]

fn main() {
// Lint
std::thread::spawn(|| {});
// No lint
vec![0, 1, 2].iter().map(|_| {}).collect::<Vec<()>>();
}
10 changes: 10 additions & 0 deletions tests/ui/empty_closure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: Empty closure may be removed
--> $DIR/empty_closure.rs:5:24
|
LL | std::thread::spawn(|| {});
| ^^^^^
|
= note: `-D clippy::empty-closure` implied by `-D warnings`

error: aborting due to previous error

1 change: 1 addition & 0 deletions tests/ui/unused_unit.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ fn return_unit() { }
#[allow(clippy::needless_return)]
#[allow(clippy::never_loop)]
#[allow(clippy::unit_cmp)]
#[allow(clippy::empty_closure)]
fn main() {
let u = Unitter;
assert_eq!(u.get_unit(|| {}, return_unit), u.into());
Expand Down
1 change: 1 addition & 0 deletions tests/ui/unused_unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ fn return_unit() -> () { () }
#[allow(clippy::needless_return)]
#[allow(clippy::never_loop)]
#[allow(clippy::unit_cmp)]
#[allow(clippy::empty_closure)]
fn main() {
let u = Unitter;
assert_eq!(u.get_unit(|| {}, return_unit), u.into());
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/unused_unit.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ LL | fn return_unit() -> () { () }
| ^^ help: remove the final `()`

error: unneeded `()`
--> $DIR/unused_unit.rs:44:14
--> $DIR/unused_unit.rs:45:14
|
LL | break();
| ^^ help: remove the `()`

error: unneeded `()`
--> $DIR/unused_unit.rs:46:11
--> $DIR/unused_unit.rs:47:11
|
LL | return();
| ^^ help: remove the `()`
Expand Down