Auto merge of #6659 - phlip9:let_and_return_fix, r=phansch

Fix let_and_return false positive

The issue:

See this Rust playground link: https://play.rust-lang.org/?edition=2018&gist=12cb5d1e7527f8c37743b87fc4a53748

Run the above with clippy to see the following warning:

```
warning: returning the result of a `let` binding from a block
  --> src/main.rs:24:5
   |
23 |     let value = Foo::new(&x).value();
   |     --------------------------------- unnecessary `let` binding
24 |     value
   |     ^^^^^
   |
   = note: `#[warn(clippy::let_and_return)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
   |
23 |
24 |     Foo::new(&x).value()
   |
```

Implementing the suggested fix, removing the temporary let binding,
yields a compiler error:

```
error[E0597]: `x` does not live long enough
  --> src/main.rs:23:14
   |
23 |     Foo::new(&x).value()
   |     ---------^^-
   |     |        |
   |     |        borrowed value does not live long enough
   |     a temporary with access to the borrow is created here ...
24 | }
   | -
   | |
   | `x` dropped here while still borrowed
   | ... and the borrow might be used here, when that temporary is dropped and runs the `Drop` code for type `Foo`
   |
   = note: the temporary is part of an expression at the end of a block;
           consider forcing this temporary to be dropped sooner, before the block's local variables are dropped
help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block
   |
23 |     let x = Foo::new(&x).value(); x
   |     ^^^^^^^                     ^^^
```

The fix:

Of course, clippy looks like it should already handle this edge case;
however, it appears `utils::fn_def_id` is not returning a `DefId` for
`Foo::new`. Changing the `qpath_res` lookup to use the child Path
`hir_id` instead of the parent Call `hir_id` fixes the issue.

changelog: none
This commit is contained in:
bors 2021-02-02 15:04:35 +00:00
commit f870876d92
3 changed files with 14 additions and 3 deletions

View file

@ -1532,10 +1532,11 @@ pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<DefId> {
ExprKind::Call(
Expr {
kind: ExprKind::Path(qpath),
hir_id: path_hir_id,
..
},
..,
) => cx.typeck_results().qpath_res(qpath, expr.hir_id).opt_def_id(),
) => cx.typeck_results().qpath_res(qpath, *path_hir_id).opt_def_id(),
_ => None,
}
}

View file

@ -117,7 +117,11 @@ mod no_lint_if_stmt_borrows {
fn drop(&mut self) {}
}
impl Foo<'_> {
impl<'a> Foo<'a> {
fn new(inner: &'a Inner) -> Self {
Self { inner }
}
fn value(&self) -> i32 {
42
}
@ -132,6 +136,12 @@ mod no_lint_if_stmt_borrows {
let value = some_foo(&x).value();
value
}
fn test2() -> i32 {
let x = Inner {};
let value = Foo::new(&x).value();
value
}
}
}

View file

@ -28,7 +28,7 @@ LL | 5
|
error: returning the result of a `let` binding from a block
--> $DIR/let_and_return.rs:154:13
--> $DIR/let_and_return.rs:164:13
|
LL | let clone = Arc::clone(&self.foo);
| ---------------------------------- unnecessary `let` binding