Skip to content

Commit

Permalink
unnecessary_semicolon: do not lint if it may cause borrow errors
Browse files Browse the repository at this point in the history
Before edition 2024, some temporaries used in scrutinees in a `match`
used as the last expression of a block may outlive some referenced
local variables. Prevent those cases from happening by checking that
alive temporaries with significant drop do have a static lifetime.

The check is performed only for edition 2021 and earlier, and for the
last statement if it would become the last expression of the block.
  • Loading branch information
samueltardieu committed Jan 22, 2025
1 parent 71ba2cf commit 9dca770
Show file tree
Hide file tree
Showing 7 changed files with 243 additions and 6 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +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));
store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::default());
// add lints here, do not remove this comment, it's used in `new_lint`
}
56 changes: 51 additions & 5 deletions clippy_lints/src/unnecessary_semicolon.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::leaks_droppable_temporary_with_limited_lifetime;
use rustc_errors::Applicability;
use rustc_hir::{ExprKind, MatchSource, Stmt, StmtKind};
use rustc_hir::{Block, ExprKind, HirId, MatchSource, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;
use rustc_span::edition::Edition::Edition2021;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -33,10 +35,50 @@ declare_clippy_lint! {
"unnecessary semicolon after expression returning `()`"
}

declare_lint_pass!(UnnecessarySemicolon => [UNNECESSARY_SEMICOLON]);
#[derive(Default)]
pub struct UnnecessarySemicolon {
last_statements: Vec<HirId>,
}

impl_lint_pass!(UnnecessarySemicolon => [UNNECESSARY_SEMICOLON]);

impl UnnecessarySemicolon {
/// Enter or leave a block, remembering the last statement of the block.
fn handle_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>, enter: bool) {
// Up to edition 2021, removing the semicolon of the last statement of a block
// may result in the scrutinee temporary values to live longer than the block
// variables. To avoid this problem, we do not lint the last statement of an
// expressionless block.
if cx.tcx.sess.edition() <= Edition2021
&& block.expr.is_none()
&& let Some(last_stmt) = block.stmts.last()
{
if enter {
self.last_statements.push(last_stmt.hir_id);
} else {
self.last_statements.pop();
}
}
}

/// Checks if `stmt` is the last statement in an expressionless block for edition ≤ 2021.
fn is_last_in_block(&self, stmt: &Stmt<'_>) -> bool {
self.last_statements
.last()
.is_some_and(|last_stmt_id| last_stmt_id == &stmt.hir_id)
}
}

impl<'tcx> LateLintPass<'tcx> for UnnecessarySemicolon {
fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
self.handle_block(cx, block, true);
}

impl LateLintPass<'_> for UnnecessarySemicolon {
fn check_stmt(&mut self, cx: &LateContext<'_>, stmt: &Stmt<'_>) {
fn check_block_post(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
self.handle_block(cx, block, false);
}

fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) {
// rustfmt already takes care of removing semicolons at the end
// of loops.
if let StmtKind::Semi(expr) = stmt.kind
Expand All @@ -48,6 +90,10 @@ impl LateLintPass<'_> for UnnecessarySemicolon {
)
&& cx.typeck_results().expr_ty(expr) == cx.tcx.types.unit
{
if self.is_last_in_block(stmt) && leaks_droppable_temporary_with_limited_lifetime(cx, expr) {
return;
}

let semi_span = expr.span.shrink_to_hi().to(stmt.span.shrink_to_hi());
span_lint_and_sugg(
cx,
Expand Down
57 changes: 57 additions & 0 deletions tests/ui/unnecessary_semicolon.edition2021.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//@revisions: edition2021 edition2024
//@[edition2021] edition:2021
//@[edition2024] edition:2024

#![warn(clippy::unnecessary_semicolon)]
#![feature(postfix_match)]
#![allow(clippy::single_match)]

fn no_lint(mut x: u32) -> Option<u32> {
Some(())?;

{
let y = 3;
dbg!(x + y)
};

{
let (mut a, mut b) = (10, 20);
(a, b) = (b + 1, a + 1);
}

Some(0)
}

fn main() {
let mut a = 3;
if a == 2 {
println!("This is weird");
}
//~^ ERROR: unnecessary semicolon

a.match {
3 => println!("three"),
_ => println!("not three"),
}
//~^ ERROR: unnecessary semicolon
}

// This is a problem in edition 2021 and below
fn borrow_issue() {
let v = std::cell::RefCell::new(Some(vec![1]));
match &*v.borrow() {
Some(v) => {
dbg!(v);
},
None => {},
};
}

fn no_borrow_issue(a: u32, b: u32) {
match Some(a + b) {
Some(v) => {
dbg!(v);
},
None => {},
}
}
23 changes: 23 additions & 0 deletions tests/ui/unnecessary_semicolon.edition2021.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:29:6
|
LL | };
| ^ help: remove
|
= note: `-D clippy::unnecessary-semicolon` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_semicolon)]`

error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:35:6
|
LL | };
| ^ help: remove

error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:56:6
|
LL | };
| ^ help: remove

error: aborting due to 3 previous errors

57 changes: 57 additions & 0 deletions tests/ui/unnecessary_semicolon.edition2024.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//@revisions: edition2021 edition2024
//@[edition2021] edition:2021
//@[edition2024] edition:2024

#![warn(clippy::unnecessary_semicolon)]
#![feature(postfix_match)]
#![allow(clippy::single_match)]

fn no_lint(mut x: u32) -> Option<u32> {
Some(())?;

{
let y = 3;
dbg!(x + y)
};

{
let (mut a, mut b) = (10, 20);
(a, b) = (b + 1, a + 1);
}

Some(0)
}

fn main() {
let mut a = 3;
if a == 2 {
println!("This is weird");
}
//~^ ERROR: unnecessary semicolon

a.match {
3 => println!("three"),
_ => println!("not three"),
}
//~^ ERROR: unnecessary semicolon
}

// This is a problem in edition 2021 and below
fn borrow_issue() {
let v = std::cell::RefCell::new(Some(vec![1]));
match &*v.borrow() {
Some(v) => {
dbg!(v);
},
None => {},
}
}

fn no_borrow_issue(a: u32, b: u32) {
match Some(a + b) {
Some(v) => {
dbg!(v);
},
None => {},
}
}
29 changes: 29 additions & 0 deletions tests/ui/unnecessary_semicolon.edition2024.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:29:6
|
LL | };
| ^ help: remove
|
= note: `-D clippy::unnecessary-semicolon` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_semicolon)]`

error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:35:6
|
LL | };
| ^ help: remove

error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:47:6
|
LL | };
| ^ help: remove

error: unnecessary semicolon
--> tests/ui/unnecessary_semicolon.rs:56:6
|
LL | };
| ^ help: remove

error: aborting due to 4 previous errors

25 changes: 25 additions & 0 deletions tests/ui/unnecessary_semicolon.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
//@revisions: edition2021 edition2024
//@[edition2021] edition:2021
//@[edition2024] edition:2024

#![warn(clippy::unnecessary_semicolon)]
#![feature(postfix_match)]
#![allow(clippy::single_match)]

fn no_lint(mut x: u32) -> Option<u32> {
Some(())?;
Expand Down Expand Up @@ -30,3 +35,23 @@ fn main() {
};
//~^ ERROR: unnecessary semicolon
}

// This is a problem in edition 2021 and below
fn borrow_issue() {
let v = std::cell::RefCell::new(Some(vec![1]));
match &*v.borrow() {
Some(v) => {
dbg!(v);
},
None => {},
};
}

fn no_borrow_issue(a: u32, b: u32) {
match Some(a + b) {
Some(v) => {
dbg!(v);
},
None => {},
};
}

0 comments on commit 9dca770

Please sign in to comment.