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

Why new_without_default is shown for structs with private fields? #1878

Closed
ozkriff opened this issue Jul 9, 2017 · 5 comments
Closed

Why new_without_default is shown for structs with private fields? #1878

ozkriff opened this issue Jul 9, 2017 · 5 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types

Comments

@ozkriff
Copy link

ozkriff commented Jul 9, 2017

No description provided.

@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2017

Can you elaborate on the issue you have with this? If you have an fn new() -> Self that you can call as T::new(), there's no disadvantage to also offering a T::default().

@ozkriff
Copy link
Author

ozkriff commented Jul 10, 2017

It requires me to write five lines of code (for each such class) which most likely will be useless for me, as I don't often see Default used with complex objects with private fields and internal invariants. I think that useful Default implementation for such objects is an exception rather than the rule.

impl Default for Simulator {
    fn default() -> Simulator {
        Simulator::new()
    }
}

(Ahem. As always, sorry for my English)

@oli-obk
Copy link
Contributor

oli-obk commented Jul 10, 2017

which most likely will be useless for me

Emphasis (mine): most likely. For public types this can be a serious usability problem. A user of your object might need to manually impl Default with many fields just because that one field doesn't impl Default, when they otherwise could have slapped a #[derive(Default)] onto their struct.

I'd be totally fine only linting for pub types and ignoring all pub(crate) and friends.

@oli-obk oli-obk added good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages T-middle Type: Probably requires verifiying types labels Jul 10, 2017
@rail-rain
Copy link
Contributor

Hello, is there any work needed to close this issue? Since the author +1-ing oli-obk's last comment and that feature had already been implemented in #972 IIUC, I believe you can close this issue.

@oli-obk
Copy link
Contributor

oli-obk commented Dec 11, 2020

Thanks! you're right, this is indeed fixed

@oli-obk oli-obk closed this as completed Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

4 participants