mcarton/rust-derivative

clippy complains about clippy::match_single_binding when deriving Debug

imp opened this issue · 6 comments

imp commented

Fresh clippy [as of 2020-02-16] is not happy about match code generated when deriving Debug

warning: this match could be written as a `let` statement
 --> examples/let.rs:6:1
  |
6 | #[derivative(Debug)]
  | ^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(clippy::match_single_binding)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
  |
6 | let Derivative = #[derivative(Debug)];
7 | Derivative
  |

To Reproduce
Here is the code that shows the problem

use derivative::Derivative;

#[derive(Default, Derivative)]
#[derivative(Debug)]
pub struct Foo {
    foo: u8,
}

fn main() {
    let foo1 = Foo::default();
    println!("foo = {:?}", foo1);
}

Expected behavior

warning-clean code

Errors
warning: this match could be written as a let statement
--> examples/let.rs:6:1
|
6 | #[derivative(Debug)]
| ^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(clippy::match_single_binding)] on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using let statement
|
6 | let Derivative = #[derivative(Debug)];
7 | Derivative
|

Version (please complete the following information):

Please include the output of the following commands, and any other version you think is relevant:

rustup 1.21.1 (7832b2ebe 2019-12-20)
cargo 1.43.0-nightly (3c53211c3 2020-02-07)
rustc 1.43.0-nightly (5e7af4669 2020-02-16)
  • Version of derivative: 1.0.3

Additional context
running cargo expand shows what makes clippy unhappy

#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::v1::*;
#[macro_use]
extern crate std;
extern crate derivative;
use derivative::Derivative;
#[derivative(Debug)]
pub struct Foo {
    foo: u8,
}
#[automatically_derived]
#[allow(unused_qualifications)]
impl ::core::default::Default for Foo {
    #[inline]
    fn default() -> Foo {
        Foo {
            foo: ::core::default::Default::default(),
        }
    }
}
#[allow(unused_qualifications)]
impl ::std::fmt::Debug for Foo {
    fn fmt(&self, __f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
        match *self {
            Foo { foo: ref __arg_0 } => {
                let mut __debug_trait_builder = __f.debug_struct("Foo");
                let _ = __debug_trait_builder.field("foo", &__arg_0);
                __debug_trait_builder.finish()
            }
        }
    }
}
fn main() {
    let foo1 = Foo::default();
    {
        ::std::io::_print(::core::fmt::Arguments::new_v1(
            &["foo = ", "\n"],
            &match (&foo1,) {
                (arg0,) => [::core::fmt::ArgumentV1::new(arg0, ::core::fmt::Debug::fmt)],
            },
        ));
    };
}

The match *self seems to be the source of grief

+1

Hi,

Thanks for the repport, but I consider this to be a bug in Clippy: It should not generate that kind of warning in generated code. Please report the issue over there.

FYI I filed a clippy issue, as I didn't see one: rust-lang/rust-clippy#5362

I'm not sure this is a clippy issue. Clippy already checks for expansions (https://github.com/rust-lang/rust-clippy/blob/09fe163c9290b8aa573decb25d5b4b02f33e481e/clippy_lints/src/matches.rs#L367), but since you are reusing the derivative(Debug) span in

quote_spanned! {input.span=>
#[allow(unused_qualifications)]
impl #impl_generics #debug_trait_path for #name #ty_generics #where_clause {
fn fmt(&self, #formatter: &mut #fmt_path::Formatter) -> #fmt_path::Result {
match *self {
#body
}
}
}
we can't detect that you are doing an expansion.

Shouldn't it work just fine if you used quote! instead of quote_spanned!? The derive macro should expand everything with a new span attached that mentions that the error is coming from a derive.

@oli-cosmian The thing with making new spans is that it usually makes worse error messages.

Fixed in #71.