Skip to content

Commit

Permalink
Auto merge of #11061 - Alexendoo:let-and-return-closures, r=llogiq
Browse files Browse the repository at this point in the history
`let_and_return`: lint 'static lifetimes, don't lint borrows in closures

Fixes #11056

Now also ignores functions returning `'static` lifetimes, since I noticed the `stdin.lock()` example was still being linted but doesn't need to be since rust-lang/rust#93965

changelog: none
  • Loading branch information
bors committed Jul 2, 2023
2 parents 17a48c2 + e29a5ac commit 83d0682
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 36 deletions.
6 changes: 3 additions & 3 deletions clippy_lints/src/returns.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::visitors::{for_each_expr, Descend};
use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
use clippy_utils::{fn_def_id, path_to_local_id, span_find_starting_semi};
use core::ops::ControlFlow;
use if_chain::if_chain;
Expand Down Expand Up @@ -328,7 +328,7 @@ fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, semi_spans: Vec<Span>,
}

fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
for_each_expr(expr, |e| {
for_each_expr_with_closures(cx, expr, |e| {
if let Some(def_id) = fn_def_id(cx, e)
&& cx
.tcx
Expand All @@ -337,7 +337,7 @@ fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
.skip_binder()
.output()
.walk()
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_)))
.any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(re) if !re.is_static()))
{
ControlFlow::Break(())
} else {
Expand Down
61 changes: 32 additions & 29 deletions tests/ui/let_and_return.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![allow(unused)]
#![warn(clippy::let_and_return)]

use std::cell::RefCell;

fn test() -> i32 {
let _y = 0; // no warning
let x = 5;
Expand Down Expand Up @@ -65,45 +67,46 @@ macro_rules! tuple_encode {
);
}

fn issue_3792() -> String {
use std::io::{self, BufRead, Stdin};

let stdin = io::stdin();
// `Stdin::lock` returns `StdinLock<'static>` so `line` doesn't borrow from `stdin`
// https://github.com/rust-lang/rust/pull/93965
let line = stdin.lock().lines().next().unwrap().unwrap();
line
}

tuple_encode!(T0, T1, T2, T3, T4, T5, T6, T7);

mod no_lint_if_stmt_borrows {
mod issue_3792 {
use std::io::{self, BufRead, Stdin};
use std::cell::RefCell;
use std::rc::{Rc, Weak};
struct Bar;

fn read_line() -> String {
let stdin = io::stdin();
let line = stdin.lock().lines().next().unwrap().unwrap();
line
impl Bar {
fn new() -> Self {
Bar {}
}
}

mod issue_3324 {
use std::cell::RefCell;
use std::rc::{Rc, Weak};

fn test(value: Weak<RefCell<Bar>>) -> u32 {
let value = value.upgrade().unwrap();
let ret = value.borrow().baz();
ret
fn baz(&self) -> u32 {
0
}
}

struct Bar;
fn issue_3324(value: Weak<RefCell<Bar>>) -> u32 {
let value = value.upgrade().unwrap();
let ret = value.borrow().baz();
ret
}

impl Bar {
fn new() -> Self {
Bar {}
}
fn baz(&self) -> u32 {
0
}
fn borrows_in_closure(value: Weak<RefCell<Bar>>) -> u32 {
fn f(mut x: impl FnMut() -> u32) -> impl FnMut() -> u32 {
x
}

fn main() {
let a = Rc::new(RefCell::new(Bar::new()));
let b = Rc::downgrade(&a);
test(b);
}
let value = value.upgrade().unwrap();
let ret = f(|| value.borrow().baz())();
ret
}

mod free_function {
Expand Down
22 changes: 18 additions & 4 deletions tests/ui/let_and_return.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: returning the result of a `let` binding from a block
--> $DIR/let_and_return.rs:7:5
--> $DIR/let_and_return.rs:9:5
|
LL | let x = 5;
| ---------- unnecessary `let` binding
Expand All @@ -14,7 +14,7 @@ LL ~ 5
|

error: returning the result of a `let` binding from a block
--> $DIR/let_and_return.rs:13:9
--> $DIR/let_and_return.rs:15:9
|
LL | let x = 5;
| ---------- unnecessary `let` binding
Expand All @@ -28,7 +28,21 @@ LL ~ 5
|

error: returning the result of a `let` binding from a block
--> $DIR/let_and_return.rs:164:13
--> $DIR/let_and_return.rs:77:5
|
LL | let line = stdin.lock().lines().next().unwrap().unwrap();
| --------------------------------------------------------- unnecessary `let` binding
LL | line
| ^^^^
|
help: return the expression directly
|
LL ~
LL ~ stdin.lock().lines().next().unwrap().unwrap()
|

error: returning the result of a `let` binding from a block
--> $DIR/let_and_return.rs:167:13
|
LL | let clone = Arc::clone(&self.foo);
| ---------------------------------- unnecessary `let` binding
Expand All @@ -41,5 +55,5 @@ LL ~
LL ~ Arc::clone(&self.foo) as _
|

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

0 comments on commit 83d0682

Please sign in to comment.