Skip to content

derivable_impl false positive when Default is implemented manually to structs with a dyn field #10158

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

Closed
Y-Nak opened this issue Jan 4, 2023 · 0 comments · Fixed by #10954
Closed
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@Y-Nak
Copy link
Contributor

Y-Nak commented Jan 4, 2023

Summary

false positive with derivable_impl if

  1. Default is implemented manually to a struct having Box<dyn Trait> field, and
  2. A type being used in the Default implementation implements Default

Lint Name

derivable_impl

Reproducer

I tried this code:

pub trait T {}

#[derive(Default)]
pub struct S {}
impl T for S {}

pub struct Outer {
    pub inner: Box<dyn T>,
}

impl Default for Outer {
    fn default() -> Self {
        Outer {
            inner: Box::<S>::default(),
        }
    }
}

I saw this happen:

warning: this `impl` can be derived
  --> src/lib.rs:11:1
   |
11 | / impl Default for Outer {
12 | |     fn default() -> Self {
13 | |         Outer {
14 | |             inner: Box::<S>::default(),
15 | |         }
16 | |     }
17 | | }
   | |_^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls
   = note: `#[warn(clippy::derivable_impls)]` on by default
   = help: remove the manual implementation...
help: ...and instead derive it
   |
7  | #[derive(Default)]
   |

I expected to see this happen: derivable_impl is not invoked.

Version

rustc 1.66.0 (69f9c33d7 2022-12-12)
binary: rustc
commit-hash: 69f9c33d71c871fc16ac445211281c6e7a340943
commit-date: 2022-12-12
host: aarch64-apple-darwin
release: 1.66.0
LLVM version: 15.0.2

Additional Labels

No response

@Y-Nak Y-Nak added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jan 4, 2023
Xiretza referenced this issue in fish-shell/fish-shell Mar 5, 2023
bool_assert_comparison is stupid, the reason they give is "it's shorter". Well,
`assert!(!foo)` is nowhere near as readable as `assert_eq!(foo, false)` because
of the ! noise from the macro.

Uninlined format args is a stupid lint that Rust actually walked back when they
made it an official warning because you still have to use a mix of inlined and
un-inlined format args (the latter of which won't complain) since only idents
can be inlined.
@bors bors closed this as completed in ffe9525 Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant