New lint: unnecessary_semicolon (#14032)

This lint detects and removes the unnecessary semicolon after a `match`
or `if` statement returning `()`. It seems to be quite a common
"mistake", given the number of hits (88) we had in the Clippy sources
themselves.

The lint doesn't bother about loops, as `rustfmt` already removes the
extra semicolon. It doesn't handle blocks either, as an extra block
level, followed or not by a semicolon, is likely intentional.

I propose to put the lint in `pedantic`, as putting it in `style` seems
quite hazardous given the number of hits.

Note: there exists a `redundant-semicolon` lint in the compiler, but it
is an early lint and cannot check that the expression evaluates to `()`,
so it ignores the cases we're handling here.

----

changelog: [`unnecessary_semicolon`]: new lint
This commit is contained in:
llogiq 2025-01-20 17:39:37 +00:00 committed by GitHub
commit 8f1b4bb87a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
83 changed files with 236 additions and 88 deletions

View file

@ -257,7 +257,7 @@ fn build_sugg<'tcx>(
// The receiver may have been a value type, so we need to add an `&` to
// be sure the argument to clone_from will be a reference.
arg_sugg = arg_sugg.addr();
};
}
format!("{receiver_sugg}.clone_from({arg_sugg})")
},

View file

@ -60,7 +60,7 @@ pub(super) fn check(cx: &EarlyContext<'_>, item_span: Span, attrs: &[Attribute])
}
outer_attr_kind.insert(kind);
},
};
}
}
}

View file

@ -84,6 +84,6 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_from: Ty<'_>, ca
diag
.note("`usize` and `isize` may be as small as 16 bits on some platforms")
.note("for more information see https://doc.rust-lang.org/reference/types/numeric.html#machine-dependent-integer-types");
};
}
});
}

View file

@ -205,7 +205,7 @@ fn expr_muldiv_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign {
// - uncertain if there are any uncertain values (because they could be negative or positive),
Sign::Uncertain => return Sign::Uncertain,
Sign::ZeroOrPositive => (),
};
}
}
// A mul/div is:
@ -236,7 +236,7 @@ fn expr_add_sign(cx: &LateContext<'_>, expr: &Expr<'_>) -> Sign {
// - uncertain if there are any uncertain values (because they could be negative or positive),
Sign::Uncertain => return Sign::Uncertain,
Sign::ZeroOrPositive => positive_count += 1,
};
}
}
// A sum is:

View file

@ -273,7 +273,7 @@ fn get_types_from_cast<'a>(
},
_ => {},
}
};
}
None
}

View file

@ -757,6 +757,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
crate::unnecessary_semicolon::UNNECESSARY_SEMICOLON_INFO,
crate::unnecessary_struct_initialization::UNNECESSARY_STRUCT_INITIALIZATION_INFO,
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
crate::unneeded_struct_pattern::UNNEEDED_STRUCT_PATTERN_INFO,

View file

@ -80,6 +80,6 @@ impl LateLintPass<'_> for DefaultConstructedUnitStructs {
String::new(),
Applicability::MachineApplicable,
);
};
}
}
}

View file

@ -331,7 +331,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing<'tcx> {
deref_count += 1;
},
None => break None,
};
}
};
let use_node = use_cx.use_node(cx);

View file

@ -70,7 +70,7 @@ impl<'tcx> LateLintPass<'tcx> for UnportableVariant {
var.span,
"C-like enum variant discriminant is not portable to 32-bit targets",
);
};
}
}
}
}

View file

@ -183,7 +183,7 @@ impl<'cx, 'tcx> TypeWalker<'cx, 'tcx> {
.collect()
};
self.emit_sugg(spans, msg, help);
};
}
}
}

View file

@ -45,7 +45,7 @@ pub(super) fn check_fn<'tcx>(cx: &LateContext<'_>, kind: &'tcx FnKind<'_>, body:
for param in generics.params {
if param.is_impl_trait() {
report(cx, param, generics);
};
}
}
}
}

View file

@ -160,7 +160,7 @@ fn check_needless_must_use(
&& !is_must_use_ty(cx, future_ty)
{
return;
};
}
}
span_lint_and_help(

View file

@ -120,7 +120,7 @@ fn get_const<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(u128, B
let ecx = ConstEvalCtxt::new(cx);
if let Some(Constant::Int(c)) = ecx.eval(r) {
return Some((c, op.node, l));
};
}
if let Some(Constant::Int(c)) = ecx.eval(l) {
return Some((c, invert_op(op.node)?, r));
}

View file

@ -350,7 +350,7 @@ fn check_with_condition<'tcx>(
if cx.typeck_results().expr_ty(cond_left).is_signed() {
} else {
print_lint_and_sugg(cx, var_name, expr);
};
}
}
},
ExprKind::Path(QPath::TypeRelative(_, name)) => {

View file

@ -65,6 +65,6 @@ impl LateLintPass<'_> for IterOverHashType {
expr.span,
"iteration over unordered hash-based type",
);
};
}
}
}

View file

@ -41,6 +41,6 @@ impl<'tcx> LateLintPass<'tcx> for UnderscoreTyped {
Some(ty.span.with_lo(local.pat.span.hi())),
"remove the explicit type `_` declaration",
);
};
}
}
}

View file

@ -372,6 +372,7 @@ mod unnecessary_literal_bound;
mod unnecessary_map_on_constructor;
mod unnecessary_owned_empty_strings;
mod unnecessary_self_imports;
mod unnecessary_semicolon;
mod unnecessary_struct_initialization;
mod unnecessary_wraps;
mod unneeded_struct_pattern;
@ -972,5 +973,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(unnecessary_literal_bound::UnnecessaryLiteralBound));
store.register_late_pass(move |_| Box::new(arbitrary_source_item_ordering::ArbitrarySourceItemOrdering::new(conf)));
store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern));
store.register_late_pass(|_| Box::new(unnecessary_semicolon::UnnecessarySemicolon));
// add lints here, do not remove this comment, it's used in `new_lint`
}

View file

@ -784,7 +784,7 @@ fn report_elidable_lifetimes(
|diag| {
if !include_suggestions {
return;
};
}
if let Some(suggestions) = elision_suggestions(cx, generics, elidable_lts, usages) {
diag.multipart_suggestion("elide the lifetimes", suggestions, Applicability::MachineApplicable);

View file

@ -251,7 +251,7 @@ impl LiteralDigitGrouping {
);
if !consistent {
return Err(WarningType::InconsistentDigitGrouping);
};
}
}
Ok(())

View file

@ -106,7 +106,7 @@ impl<'tcx> QuestionMark {
emit_manual_let_else(cx, stmt.span, match_expr, &ident_map, pat_arm.pat, diverging_arm.body);
},
}
};
}
}
}
@ -295,7 +295,7 @@ fn pat_allowed_for_else(cx: &LateContext<'_>, pat: &'_ Pat<'_>, check_types: boo
PatKind::Struct(..) | PatKind::TupleStruct(..) | PatKind::Path(..)
) {
return;
};
}
let ty = typeck_results.pat_ty(pat);
// Option and Result are allowed, everything else isn't.
if !(is_type_diagnostic_item(cx, ty, sym::Option) || is_type_diagnostic_item(cx, ty, sym::Result)) {

View file

@ -85,7 +85,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualRemEuclid {
}
},
_ => return,
};
}
let mut app = Applicability::MachineApplicable;
let rem_of = snippet_with_context(cx, rem2_lhs.span, ctxt, "_", &mut app).0;

View file

@ -86,7 +86,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualStrip {
let target_res = cx.qpath_res(target_path, target_arg.hir_id);
if target_res == Res::Err {
return;
};
}
if let Res::Local(hir_id) = target_res
&& let Some(used_mutably) = mutated_variables(then, cx)

View file

@ -34,7 +34,7 @@ fn get_cond_expr<'tcx>(
needs_negated: is_none_expr(cx, then_expr), /* if the `then_expr` resolves to `None`, need to negate the
* cond */
});
};
}
None
}
@ -45,7 +45,7 @@ fn peels_blocks_incl_unsafe_opt<'a>(expr: &'a Expr<'a>) -> Option<&'a Expr<'a>>
if block.stmts.is_empty() {
return block.expr;
}
};
}
None
}
@ -68,14 +68,14 @@ fn is_some_expr(cx: &LateContext<'_>, target: HirId, ctxt: SyntaxContext, expr:
&& is_res_lang_ctor(cx, path_res(cx, callee), OptionSome)
&& path_to_local_id(arg, target);
}
};
}
false
}
fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let Some(inner_expr) = peels_blocks_incl_unsafe_opt(expr) {
return is_res_lang_ctor(cx, path_res(cx, inner_expr), OptionNone);
};
}
false
}

View file

@ -109,7 +109,7 @@ where
}
},
None => return None,
};
}
let mut app = Applicability::MachineApplicable;

View file

@ -117,7 +117,7 @@ where
if let ty::Ref(..) = cx.typeck_results().expr_ty(ex_inner).kind() {
ex_new = ex_inner;
}
};
}
span_lint_and_sugg(
cx,
MATCH_LIKE_MATCHES_MACRO,

View file

@ -170,7 +170,7 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
);
});
},
};
}
}
enum CommonPrefixSearcher<'a> {

View file

@ -46,7 +46,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine
err_ty = ty;
} else {
return;
};
}
span_lint_and_then(
cx,

View file

@ -13,7 +13,7 @@ pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arms: &[Arm<'_>]) {
&& has_non_exhaustive_attr(cx.tcx, *adt_def)
{
return;
};
}
for arm in arms {
if let PatKind::Or(fields) = arm.pat.kind {
// look for multiple fields in this arm that contains at least one Wild pattern

View file

@ -62,5 +62,5 @@ pub(super) fn check<'tcx>(
),
applicability,
);
};
}
}

View file

@ -32,5 +32,5 @@ pub(super) fn check<'tcx>(
),
applicability,
);
};
}
}

View file

@ -46,5 +46,5 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx E
format!("{receiver}.as_bytes().get({n}).copied()"),
applicability,
);
};
}
}

View file

@ -32,7 +32,7 @@ pub fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span: Span,
// &T where T: Copy
ty::Ref(_, ty, _) if is_copy(cx, *ty) => {},
_ => return,
};
}
span_lint_and_sugg(
cx,
CLONED_INSTEAD_OF_COPIED,

View file

@ -37,7 +37,7 @@ pub(super) fn check(
"expect_err".to_string(),
Applicability::MachineApplicable,
);
};
}
}
/// Given a `Result<T, E>` type, return its data (`T`).

View file

@ -58,7 +58,7 @@ pub(super) fn check<'tcx>(
if ty.is_str() && can_be_static_str(cx, arg) {
return false;
}
};
}
true
}

View file

@ -25,5 +25,5 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, span
"into_iter()".to_string(),
Applicability::MaybeIncorrect,
);
};
}
}

View file

@ -37,7 +37,7 @@ pub(super) fn check(
let _ = write!(sugg, r#".extension().is_some_and(|ext| ext == "{path}")"#);
} else {
let _ = write!(sugg, r#".extension().map_or(false, |ext| ext == "{path}")"#);
};
}
span_lint_and_sugg(
cx,

View file

@ -87,7 +87,7 @@ pub fn check_for_loop_iter(
// skip lint
return true;
}
};
}
// the lint should not be executed if no violation happens
let snippet = if let ExprKind::MethodCall(maybe_iter_method_name, collection, [], _) = receiver.kind

View file

@ -214,7 +214,7 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
);
},
);
};
}
if let StmtKind::Semi(expr) = stmt.kind
&& let ExprKind::Binary(ref binop, a, b) = expr.kind
&& (binop.node == BinOpKind::And || binop.node == BinOpKind::Or)
@ -236,7 +236,7 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
);
},
);
};
}
}
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {

View file

@ -66,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for TypeParamMismatch {
}) => impl_params.push((path.segments[0].ident.to_string(), path.span)),
GenericArg::Type(_) => return,
_ => (),
};
}
}
// find the type that the Impl is for

View file

@ -220,7 +220,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc {
| hir::ItemKind::GlobalAsm(..)
| hir::ItemKind::Impl { .. }
| hir::ItemKind::Use(..) => note_prev_span_then_ret!(self.prev_span, it.span),
};
}
let (article, desc) = cx.tcx.article_and_description(it.owner_id.to_def_id());

View file

@ -135,7 +135,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingInline {
| hir::ItemKind::ForeignMod { .. }
| hir::ItemKind::Impl { .. }
| hir::ItemKind::Use(..) => {},
};
}
}
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx hir::ImplItem<'_>) {

View file

@ -56,10 +56,10 @@ impl EarlyLintPass for MultiAssignments {
if let ExprKind::Assign(target, source, _) = &expr.kind {
if let ExprKind::Assign(_target, _source, _) = &strip_paren_blocks(target).kind {
span_lint(cx, MULTI_ASSIGNMENTS, expr.span, "assignments don't nest intuitively");
};
}
if let ExprKind::Assign(_target, _source, _) = &strip_paren_blocks(source).kind {
span_lint(cx, MULTI_ASSIGNMENTS, expr.span, "assignments don't nest intuitively");
}
};
}
}
}

View file

@ -171,7 +171,7 @@ fn collect_unsafe_exprs<'tcx>(
},
_ => {},
};
}
Continue::<(), _>(Descend::Yes)
});

View file

@ -91,7 +91,7 @@ impl<'tcx> LateLintPass<'tcx> for Mutex {
ty::Uint(t) if t != UintTy::Usize => span_lint(cx, MUTEX_INTEGER, expr.span, msg),
ty::Int(t) if t != IntTy::Isize => span_lint(cx, MUTEX_INTEGER, expr.span, msg),
_ => span_lint(cx, MUTEX_ATOMIC, expr.span, msg),
};
}
}
}
}

View file

@ -336,7 +336,7 @@ fn check<'tcx>(
);
},
_ => {},
};
}
Some(())
}

View file

@ -73,7 +73,7 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions {
}
},
_ => {},
};
}
}
}

View file

@ -104,7 +104,7 @@ impl ArithmeticSideEffects {
if !tcx.is_diagnostic_item(sym::NonZero, adt.did()) {
return false;
};
}
let int_type = substs.type_at(0);
let unsigned_int_types = [
@ -214,7 +214,7 @@ impl ArithmeticSideEffects {
| hir::BinOpKind::Sub
) {
return;
};
}
let (mut actual_lhs, lhs_ref_counter) = peel_hir_expr_refs(lhs);
let (mut actual_rhs, rhs_ref_counter) = peel_hir_expr_refs(rhs);
actual_lhs = expr_or_init(cx, actual_lhs);

View file

@ -104,7 +104,7 @@ fn check_op(cx: &LateContext<'_>, expr: &Expr<'_>, other: &Expr<'_>, left: bool)
} else {
expr_snip = arg_snip.to_string();
eq_impl = without_deref;
};
}
let span;
let hint;

View file

@ -127,7 +127,7 @@ pub(super) fn check<'tcx>(
None,
note,
);
};
}
}
}

View file

@ -49,5 +49,5 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, op: BinOpKind, lhs: &'tcx Expr
lint_double_comparison!(==);
},
_ => (),
};
}
}

View file

@ -120,7 +120,7 @@ fn is_float(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
if let ty::Array(arr_ty, _) = value {
return matches!(arr_ty.kind(), ty::Float(_));
};
}
matches!(value, ty::Float(_))
}

View file

@ -30,7 +30,7 @@ pub(super) fn check<'tcx>(
} else {
check_non_const_operands(cx, e, lhs);
}
};
}
}
fn used_in_comparison_with_zero(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {

View file

@ -21,6 +21,6 @@ pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, op: BinOpKind, right:
"any number modulo -1 will panic/overflow or result in 0",
);
}
};
}
}
}

View file

@ -53,6 +53,6 @@ impl<'tcx> LateLintPass<'tcx> for PartialEqNeImpl {
);
}
}
};
}
}
}

View file

@ -208,7 +208,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall {
// avoid clippy::double_parens
if !is_in_fn_call_arg {
hint = hint.maybe_par();
};
}
diag.span_suggestion(full_expr.span, "try doing something like", hint, applicability);
}

View file

@ -215,6 +215,6 @@ impl LateLintPass<'_> for RedundantTypeAnnotations {
},
_ => (),
}
};
}
}
}

View file

@ -97,7 +97,7 @@ fn get_pointee_ty_and_count_expr<'tcx>(
&& let Some(pointee_ty) = cx.typeck_results().node_args(func.hir_id).types().next()
{
return Some((pointee_ty, count));
};
}
if let ExprKind::MethodCall(method_path, ptr_self, [.., count], _) = expr.kind
// Find calls to copy_{from,to}{,_nonoverlapping} and write_bytes methods
&& let method_ident = method_path.ident.as_str()
@ -108,7 +108,7 @@ fn get_pointee_ty_and_count_expr<'tcx>(
cx.typeck_results().expr_ty(ptr_self).kind()
{
return Some((*pointee_ty, count));
};
}
None
}
@ -130,6 +130,6 @@ impl<'tcx> LateLintPass<'tcx> for SizeOfInElementCount {
&& pointee_ty == ty_used_for_size_of
{
span_lint_and_help(cx, SIZE_OF_IN_ELEMENT_COUNT, count_expr.span, LINT_MSG, None, HELP_MSG);
};
}
}
}

View file

@ -191,7 +191,7 @@ impl SlowVectorInit {
InitializationType::Extend(e) | InitializationType::Resize(e) => {
Self::emit_lint(cx, e, vec_alloc, "slow zero-filling initialization");
},
};
}
}
fn emit_lint(cx: &LateContext<'_>, slow_fill: &Expr<'_>, vec_alloc: &VecAllocation<'_>, msg: &'static str) {

View file

@ -296,7 +296,7 @@ fn check_xor_swap<'tcx>(cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
{
let span = s1.span.to(s3.span);
generate_swap_warning(block, cx, lhs0, rhs0, rhs1, rhs2, span, true);
};
}
}
}

View file

@ -24,7 +24,7 @@ pub(super) fn check<'tcx>(
if !tcx.is_diagnostic_item(sym::NonZero, adt.did()) {
return false;
};
}
let int_ty = substs.type_at(0);
if from_ty != int_ty {

View file

@ -119,7 +119,7 @@ fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &
&& let LitKind::Int(val, _) = lit.node
{
return (val == i as u128).then_some(lhs);
};
}
None
})

View file

@ -71,7 +71,7 @@ pub(super) fn check(cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>, lt: &Lifetime, m
Applicability::Unspecified,
);
return true;
};
}
false
},
_ => false,

View file

@ -0,0 +1,63 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use rustc_errors::Applicability;
use rustc_hir::{ExprKind, MatchSource, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
declare_clippy_lint! {
/// ### What it does
/// Checks for the presence of a semicolon at the end of
/// a `match` or `if` statement evaluating to `()`.
///
/// ### Why is this bad?
/// The semicolon is not needed, and may be removed to
/// avoid confusion and visual clutter.
///
/// ### Example
/// ```no_run
/// # let a: u32 = 42;
/// if a > 10 {
/// println!("a is greater than 10");
/// };
/// ```
/// Use instead:
/// ```no_run
/// # let a: u32 = 42;
/// if a > 10 {
/// println!("a is greater than 10");
/// }
/// ```
#[clippy::version = "1.86.0"]
pub UNNECESSARY_SEMICOLON,
pedantic,
"unnecessary semicolon after expression returning `()`"
}
declare_lint_pass!(UnnecessarySemicolon => [UNNECESSARY_SEMICOLON]);
impl LateLintPass<'_> for UnnecessarySemicolon {
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
// rustfmt already takes care of removing semicolons at the end
// of loops.
if let StmtKind::Semi(expr) = stmt.kind
&& !stmt.span.from_expansion()
&& !expr.span.from_expansion()
&& matches!(
expr.kind,
ExprKind::If(..) | ExprKind::Match(_, _, MatchSource::Normal | MatchSource::Postfix)
)
&& cx.typeck_results().expr_ty(expr) == cx.tcx.types.unit
{
let semi_span = expr.span.shrink_to_hi().to(stmt.span.shrink_to_hi());
span_lint_and_sugg(
cx,
UNNECESSARY_SEMICOLON,
semi_span,
"unnecessary semicolon",
"remove",
String::new(),
Applicability::MachineApplicable,
);
}
}
}

View file

@ -182,7 +182,7 @@ fn check_expr<'a>(cx: &LateContext<'a>, expr: &'a hir::Expr<'a>) {
emit_lint(cx, expr.span, expr.hir_id, op, &[]);
},
_ => {},
};
}
}
fn should_lint<'a>(cx: &LateContext<'a>, mut inner: &'a hir::Expr<'a>) -> Option<IoOp> {

View file

@ -69,6 +69,6 @@ impl<'tcx> LateLintPass<'tcx> for SlowSymbolComparisons {
),
applicability,
);
};
}
}
}

View file

@ -69,7 +69,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec {
};
if self.allow_in_test && is_in_test(cx.tcx, expr.hir_id) {
return;
};
}
// the parent callsite of this `vec!` expression, or span to the borrowed one such as `&vec!`
let callsite = expr.span.parent_callsite().unwrap_or(expr.span);