Skip to content

Commit

Permalink
Add checking for unnecessary delims in closure body
Browse files Browse the repository at this point in the history
  • Loading branch information
chenyukang committed Feb 13, 2025
1 parent 3a34ad7 commit 9ec13a6
Show file tree
Hide file tree
Showing 16 changed files with 124 additions and 29 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3449,7 +3449,7 @@ pub fn is_case_difference(sm: &SourceMap, suggested: &str, sp: Span) -> bool {
// All the chars that differ in capitalization are confusable (above):
let confusable = iter::zip(found.chars(), suggested.chars())
.filter(|(f, s)| f != s)
.all(|(f, s)| (ascii_confusables.contains(&f) || ascii_confusables.contains(&s)));
.all(|(f, s)| ascii_confusables.contains(&f) || ascii_confusables.contains(&s));
confusable && found.to_lowercase() == suggested.to_lowercase()
// FIXME: We sometimes suggest the same thing we already have, which is a
// bug, but be defensive against that here.
Expand Down
16 changes: 15 additions & 1 deletion compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::ops::ControlFlow;

use rustc_ast as ast;
use rustc_ast::util::{classify, parser};
use rustc_ast::{ExprKind, StmtKind};
use rustc_ast::{ExprKind, FnRetTy, StmtKind};
use rustc_errors::{MultiSpan, pluralize};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
Expand Down Expand Up @@ -594,6 +594,7 @@ enum UnusedDelimsCtx {
AnonConst,
MatchArmExpr,
IndexExpr,
ClosureBody,
}

impl From<UnusedDelimsCtx> for &'static str {
Expand All @@ -615,6 +616,7 @@ impl From<UnusedDelimsCtx> for &'static str {
UnusedDelimsCtx::ArrayLenExpr | UnusedDelimsCtx::AnonConst => "const expression",
UnusedDelimsCtx::MatchArmExpr => "match arm expression",
UnusedDelimsCtx::IndexExpr => "index expression",
UnusedDelimsCtx::ClosureBody => "closure body",
}
}
}
Expand Down Expand Up @@ -933,6 +935,18 @@ trait UnusedDelimLint {
let (args_to_check, ctx) = match *call_or_other {
Call(_, ref args) => (&args[..], UnusedDelimsCtx::FunctionArg),
MethodCall(ref call) => (&call.args[..], UnusedDelimsCtx::MethodArg),
Closure(ref closure)
if matches!(closure.fn_decl.output, FnRetTy::Default(_))
// skip `#[core::contracts::requires(...)]` and `#[core::contracts::ensures(...)]` which generate closure
&& !cx
.sess()
.source_map()
.span_to_snippet(closure.fn_decl_span)
.unwrap_or_default()
.contains("core::contracts") =>
{
(&[closure.body.clone()][..], UnusedDelimsCtx::ClosureBody)
}
// actual catch-all arm
_ => {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
debug_assert!(
span_edges
.iter_enumerated()
.all(|(node, span_edge)| { span_edge.is_some() <= self.is_supernode(node) }),
.all(|(node, span_edge)| span_edge.is_some() <= self.is_supernode(node)),
"only supernodes can have a span edge",
);
debug_assert!(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ fn create_mappings(extracted_mappings: &ExtractedMappings) -> Vec<Mapping> {
condition_info: _,
true_index: _,
false_index: _,
}| { Mapping { kind: MappingKind::Branch { true_bcb, false_bcb }, span } },
}| Mapping { kind: MappingKind::Branch { true_bcb, false_bcb }, span },
));

for (decision, branches) in mcdc_mappings {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2189,7 +2189,7 @@ impl<'a> Parser<'a> {

if self.look_ahead(1, |t| *t == token::Not) && self.look_ahead(2, |t| t.is_ident()) {
return IsMacroRulesItem::Yes { has_bang: true };
} else if self.look_ahead(1, |t| (t.is_ident())) {
} else if self.look_ahead(1, |t| t.is_ident()) {
// macro_rules foo
self.dcx().emit_err(errors::MacroRulesMissingBang {
span: macro_rules_span,
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
let module_did = mod_prefix.as_ref().and_then(Res::mod_def_id);

let mod_prefix =
mod_prefix.map_or_else(String::new, |res| (format!("{} ", res.descr())));

mod_prefix.map_or_else(String::new, |res| format!("{} ", res.descr()));
(mod_prefix, format!("`{}`", Segment::names_to_string(mod_path)), module_did, None)
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1440,7 +1440,7 @@ impl clean::FnDecl {
fmt::from_fn(move |f| {
// First, generate the text form of the declaration, with no line wrapping, and count the bytes.
let mut counter = WriteCounter(0);
write!(&mut counter, "{:#}", fmt::from_fn(|f| { self.inner_full_print(None, f, cx) }))
write!(&mut counter, "{:#}", fmt::from_fn(|f| self.inner_full_print(None, f, cx)))
.unwrap();
// If the text form was over 80 characters wide, we will line-wrap our output.
let line_wrapping_indent =
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_lints/src/unused_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedAsync {
let iter = self
.unused_async_fns
.iter()
.filter(|UnusedAsyncFn { def_id, .. }| (!self.async_fns_as_value.contains(def_id)));
.filter(|UnusedAsyncFn { def_id, .. }| !self.async_fns_as_value.contains(def_id));

for fun in iter {
span_lint_hir_and_then(
Expand Down
2 changes: 1 addition & 1 deletion src/tools/clippy/clippy_utils/src/higher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl<'hir> IfLetOrMatch<'hir> {
if_then,
if_else,
let_span,
}| { Self::IfLet(let_expr, let_pat, if_then, if_else, let_span) },
}| Self::IfLet(let_expr, let_pat, if_then, if_else, let_span),
),
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_utils/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ impl AdtVariantInfo {
.enumerate()
.map(|(i, f)| (i, approx_ty_size(cx, f.ty(cx.tcx, subst))))
.collect::<Vec<_>>();
fields_size.sort_by(|(_, a_size), (_, b_size)| (a_size.cmp(b_size)));
fields_size.sort_by(|(_, a_size), (_, b_size)| a_size.cmp(b_size));

Self {
ind: i,
Expand All @@ -936,7 +936,7 @@ impl AdtVariantInfo {
}
})
.collect::<Vec<_>>();
variants_size.sort_by(|a, b| (b.size.cmp(&a.size)));
variants_size.sort_by(|a, b| b.size.cmp(&a.size));
variants_size
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/miri/src/concurrency/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,7 +641,7 @@ impl<'tcx> ThreadManager<'tcx> {
assert!(
self.threads
.iter()
.all(|thread| { !thread.state.is_blocked_on(BlockReason::Join(joined_thread_id)) }),
.all(|thread| !thread.state.is_blocked_on(BlockReason::Join(joined_thread_id))),
"this thread already has threads waiting for its termination"
);

Expand Down
1 change: 0 additions & 1 deletion tests/ui/async-await/issues/issue-54752-async-block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
//@ pp-exact

fn main() { let _a = (async { }); }
//~^ WARNING unnecessary parentheses around assigned value
15 changes: 0 additions & 15 deletions tests/ui/async-await/issues/issue-54752-async-block.stderr

This file was deleted.

13 changes: 13 additions & 0 deletions tests/ui/lint/unused/closure-body-issue-136741.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ run-rustfix
#![deny(unused_parens)]
#![deny(unused_braces)]
pub fn main() {
let _closure = |x: i32, y: i32| x * (x + (y * 2)); //~ ERROR unnecessary braces around closure body
let _ = || 0 == 0; //~ ERROR unnecessary parentheses around closure body
let _ = (0..).find(|n| n % 2 == 0); //~ ERROR unnecessary parentheses around closure body
let _ = (0..).find(|n| n % 2 == 0); //~ ERROR unnecessary braces around closure body
let _ = || {
_ = 0;
0 == 0 //~ ERROR unnecessary parentheses around block return value
};
}
13 changes: 13 additions & 0 deletions tests/ui/lint/unused/closure-body-issue-136741.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//@ run-rustfix
#![deny(unused_parens)]
#![deny(unused_braces)]
pub fn main() {
let _closure = |x: i32, y: i32| { x * (x + (y * 2)) }; //~ ERROR unnecessary braces around closure body
let _ = || (0 == 0); //~ ERROR unnecessary parentheses around closure body
let _ = (0..).find(|n| (n % 2 == 0)); //~ ERROR unnecessary parentheses around closure body
let _ = (0..).find(|n| {n % 2 == 0}); //~ ERROR unnecessary braces around closure body
let _ = || {
_ = 0;
(0 == 0) //~ ERROR unnecessary parentheses around block return value
};
}
72 changes: 72 additions & 0 deletions tests/ui/lint/unused/closure-body-issue-136741.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
error: unnecessary braces around closure body
--> $DIR/closure-body-issue-136741.rs:5:37
|
LL | let _closure = |x: i32, y: i32| { x * (x + (y * 2)) };
| ^^ ^^
|
note: the lint level is defined here
--> $DIR/closure-body-issue-136741.rs:3:9
|
LL | #![deny(unused_braces)]
| ^^^^^^^^^^^^^
help: remove these braces
|
LL - let _closure = |x: i32, y: i32| { x * (x + (y * 2)) };
LL + let _closure = |x: i32, y: i32| x * (x + (y * 2));
|

error: unnecessary parentheses around closure body
--> $DIR/closure-body-issue-136741.rs:6:16
|
LL | let _ = || (0 == 0);
| ^ ^
|
note: the lint level is defined here
--> $DIR/closure-body-issue-136741.rs:2:9
|
LL | #![deny(unused_parens)]
| ^^^^^^^^^^^^^
help: remove these parentheses
|
LL - let _ = || (0 == 0);
LL + let _ = || 0 == 0;
|

error: unnecessary parentheses around closure body
--> $DIR/closure-body-issue-136741.rs:7:28
|
LL | let _ = (0..).find(|n| (n % 2 == 0));
| ^ ^
|
help: remove these parentheses
|
LL - let _ = (0..).find(|n| (n % 2 == 0));
LL + let _ = (0..).find(|n| n % 2 == 0);
|

error: unnecessary braces around closure body
--> $DIR/closure-body-issue-136741.rs:8:28
|
LL | let _ = (0..).find(|n| {n % 2 == 0});
| ^ ^
|
help: remove these braces
|
LL - let _ = (0..).find(|n| {n % 2 == 0});
LL + let _ = (0..).find(|n| n % 2 == 0);
|

error: unnecessary parentheses around block return value
--> $DIR/closure-body-issue-136741.rs:11:9
|
LL | (0 == 0)
| ^ ^
|
help: remove these parentheses
|
LL - (0 == 0)
LL + 0 == 0
|

error: aborting due to 5 previous errors

0 comments on commit 9ec13a6

Please sign in to comment.