default_trait_access should only fire on simple types

[adrianheine]

Calling std::cell::RefCell<std::collections::HashMap<R, futures::future::Shared<std::pin::Pin<std::boxed::Box<dyn futures::Future<Output = ()>>>>>>::default() is more clear than this expression

I don't agree

Calling std::marker::PhantomData<(L, B)>::default() is more clear than this expression

For PhantomData I think I always prefer Default::default().

[flip1995]

Can you provide example code please?

[adrianheine]

Sure!

#![deny(clippy::default_trait_access)]
use std::cell::RefCell;
use std::collections::HashMap;
use std::future::Future;
use std::pin::Pin;

struct X {
    c: RefCell<HashMap<String, Pin<Box<dyn Future<Output = ()>>>>>
}

fn main() {
    let x = X { c: Default::default() };
}

gives

error: calling `std::cell::RefCell<std::collections::HashMap<std::string::String, std::pin::Pin<std::boxed::Box<dyn std::future::Future<Output = ()>>>>>::default()` is more clear than this expression
  --> src/main.rs:12:20
   |
12 |     let x = X { c: Default::default() };
   |                    ^^^^^^^^^^^^^^^^^^ help: try: `std::cell::RefCell<std::collections::HashMap<std::string::String, std::pin::Pin<std::boxed::Box<dyn std::future::Future<Output = ()>>>>>::default()`
   |

[flip1995]

Thanks!

We should add a configuration option where the type depth can be specified. About the PhantomData case: This lint is pedantic and therefore meant to be opinionated, so I wouldn't change that.

[taiki-e]

| ^^^^^^^^^^^^^^^^^^ help: try: std::cell::RefCell<std::collections::HashMap<std::string::String, std::pin::Pin<std::boxed::Box<dyn std::future::Future<Output = ()>>>>>::default()

I think the underlying problem is clippy suggests code with complete parameters, not clippy triggers this lint even for complex types. AFAIK, If code compiles with Default::default, it doesn't need to specify any parameters, as type inference is working. (So, in this case, default_trait_access should suggest RefCell::default.)

[adrianheine]

That's a very good observation, thanks! See also #5990 about this.

I think the nicest would be to explicitly include all types up to the first type that can be »empty«, i. e. whose Default implementation does not depend on some other non-optional type parameter being Default. In this case, that would be RefCell<HashMap>::default(), but that's not valid syntax. RefCell::<HashMap<_, _>>::default() looks ugly. As such, I agree with your suggestion to just use the out-most type name. That's also something the lint already accepts, so this is really just about the suggested solution.

[taiki-e]

This and #5990 will be fixed in #5993