clippy::let_and_return false positive

[CreepySkeleton]

Summary

The clippy::let_and_return lint triggers from code generated by a macro-derive.

Reproducer

use structopt::StructOpt;

#[derive(StructOpt)]
pub struct A {
    #[structopt(flatten)]
    pub b: B,
}

#[derive(StructOpt)]
pub struct B {}

fn main() {}

structopt v0.3.2 does generate let_and_return in this case but I don't think this lint makes much sense when originated from a macro expansion.

cargo clippy -V
clippy 0.0.212 (e3cb40e4 2019-06-25)

[llogiq]

That's strange. Gotta look into the code that structopt generates. Sometimes the expr spans aren't the same as their stmt spans.

[CreepySkeleton]

Sometimes the expr spans aren't the same as their stmt spans.

Yes, structopt propagates source span info to the generated code. This particular piece of generated code has a span that points to #[structopt(flatten)] in source code.

[CreepySkeleton]

Relevant piece of expansion:

let app = <B>::augment_clap(app);
let app = if <B>::is_subcommand() {
    app.setting(::structopt::clap::AppSettings::SubcommandRequiredElseHelp)
} else {
    app
};
app

These let statements get a span pointing to flatten in source code.

The warning:

warning: returning the result of a let binding from a block
 --> src\main.rs:3:10
  |
3 | #[derive(StructOpt)]
  |          ^^^^^^^^^
4 | pub struct A {
5 |     #[structopt(flatten)]
  |                 ------- unnecessary let binding
  |
  = 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

[CreepySkeleton]

Full expansion

[llogiq]

I think checking if !in_external_macro(cx.sess(), retexpr.span); in returns.rs:210 should suffice. The return expression gets its own span from structopt, as far as I can see.

[CreepySkeleton]

@llogiq Yes, you're right.

Perhaps this will be more appropriate:

if !in_external_macro(cx.sess(), retexpr.span);
if !in_external_macro(cx.sess(), local.span);