Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clippy complains about clippy::match_single_binding when deriving Debug #58

Closed
imp opened this issue Feb 17, 2020 · 6 comments
Closed
Labels
bug The crate does not work as expected

Comments

@imp
Copy link

imp commented Feb 17, 2020

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

@imp imp added the bug The crate does not work as expected label Feb 17, 2020
@UkonnRa
Copy link

UkonnRa commented Feb 27, 2020

+1

@mcarton
Copy link
Owner

mcarton commented Mar 9, 2020

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.

@znewman01
Copy link

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

znewman01 added a commit to znewman01/spectrum-impl that referenced this issue Mar 23, 2020
- mcarton/rust-derivative#58
- some misc. stuff I just introduced
@oli-cosmian
Copy link

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.

@mcarton
Copy link
Owner

mcarton commented Apr 2, 2020

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

@mcarton mcarton reopened this Apr 2, 2020
@mcarton
Copy link
Owner

mcarton commented Apr 11, 2020

Fixed in #71.

@mcarton mcarton closed this as completed Apr 11, 2020
tdyas pushed a commit to tdyas/pants that referenced this issue May 2, 2020
fixed by upgrading derivative to latest

Clippy output:

    Checking process_execution v0.0.1 (/Users/tdyas/TC/pants/src/rust/engine/process_execution)
error: this match could be written as a `let` statement
  --> process_execution/src/remote.rs:37:1
   |
37 | #[derivative(Debug)]
   | ^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> process_execution/src/lib.rs:7:3
   |
7  |   clippy::all,
   |   ^^^^^^^^^^^
   = note: `#[deny(clippy::match_single_binding)]` implied by `#[deny(clippy::all)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
   |
37 | let Derivative = #[derivative(Debug)];
38 | Derivative

See mcarton/rust-derivative#58. Fix was released in vw2.1.1.
tdyas pushed a commit to tdyas/pants that referenced this issue May 4, 2020
fixed by upgrading derivative to latest

Clippy output:

    Checking process_execution v0.0.1 (/Users/tdyas/TC/pants/src/rust/engine/process_execution)
error: this match could be written as a `let` statement
  --> process_execution/src/remote.rs:37:1
   |
37 | #[derivative(Debug)]
   | ^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> process_execution/src/lib.rs:7:3
   |
7  |   clippy::all,
   |   ^^^^^^^^^^^
   = note: `#[deny(clippy::match_single_binding)]` implied by `#[deny(clippy::all)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
   |
37 | let Derivative = #[derivative(Debug)];
38 | Derivative

See mcarton/rust-derivative#58. Fix was released in vw2.1.1.
tdyas pushed a commit to tdyas/pants that referenced this issue May 13, 2020
fixed by upgrading derivative to latest

Clippy output:

    Checking process_execution v0.0.1 (/Users/tdyas/TC/pants/src/rust/engine/process_execution)
error: this match could be written as a `let` statement
  --> process_execution/src/remote.rs:37:1
   |
37 | #[derivative(Debug)]
   | ^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> process_execution/src/lib.rs:7:3
   |
7  |   clippy::all,
   |   ^^^^^^^^^^^
   = note: `#[deny(clippy::match_single_binding)]` implied by `#[deny(clippy::all)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
   |
37 | let Derivative = #[derivative(Debug)];
38 | Derivative

See mcarton/rust-derivative#58. Fix was released in vw2.1.1.
tdyas pushed a commit to tdyas/pants that referenced this issue May 16, 2020
fixed by upgrading derivative to latest

Clippy output:

    Checking process_execution v0.0.1 (/Users/tdyas/TC/pants/src/rust/engine/process_execution)
error: this match could be written as a `let` statement
  --> process_execution/src/remote.rs:37:1
   |
37 | #[derivative(Debug)]
   | ^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> process_execution/src/lib.rs:7:3
   |
7  |   clippy::all,
   |   ^^^^^^^^^^^
   = note: `#[deny(clippy::match_single_binding)]` implied by `#[deny(clippy::all)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
   |
37 | let Derivative = #[derivative(Debug)];
38 | Derivative

See mcarton/rust-derivative#58. Fix was released in vw2.1.1.
stuhood pushed a commit to tdyas/pants that referenced this issue May 28, 2020
fixed by upgrading derivative to latest

Clippy output:

    Checking process_execution v0.0.1 (/Users/tdyas/TC/pants/src/rust/engine/process_execution)
error: this match could be written as a `let` statement
  --> process_execution/src/remote.rs:37:1
   |
37 | #[derivative(Debug)]
   | ^^^^^^^^^^^^^^^^^^^^
   |
note: the lint level is defined here
  --> process_execution/src/lib.rs:7:3
   |
7  |   clippy::all,
   |   ^^^^^^^^^^^
   = note: `#[deny(clippy::match_single_binding)]` implied by `#[deny(clippy::all)]`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
   |
37 | let Derivative = #[derivative(Debug)];
38 | Derivative

See mcarton/rust-derivative#58. Fix was released in vw2.1.1.
# Delete this line to force CI to run the JVM tests.
[ci skip-jvm-tests]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug The crate does not work as expected
Projects
None yet
Development

No branches or pull requests

5 participants