Lint `PhantomData::default()` -> `PhantomData`

[alecmocatta]

What it does

PhantomData::default() is quite common https://github.com/search?q=language%3ARust+%22PhantomData%3A%3Adefault%28%29%22&type=code&ref=advsearch though of course the ::default() is actually redundant.

Perhaps a lint that suggests removing the ::default() would be useful?.

Lint Name

No response

Category

style, pedantic

Advantage

Shorter, ostensibly avoids a function call.

Drawbacks

Perhaps

Foo {
    _marker: PhantomData::default()
}

makes more sense to some people than the slightly special

Foo {
    _marker: PhantomData
}

Example

Foo {
    _marker: PhantomData::default()
}

Could be written as:

Foo {
    _marker: PhantomData
}

[llogiq]

I think it should be complexity and should not lint in macros or if the PhantomData type is introduced by generics.

[Alexendoo]

Could be extended to other unit structs too

[schubart]

@rustbot claim

[asquared31415]

Could be extended to other unit structs too

I would only do this for manually vetted structs that are part of std. Nothing requires Something::default() to have identical semantics to Something, even if the return value might be the same, there could be side effects.

[Alexendoo]

To be conservative the lint can check the Default impl with https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html#method.is_builtin_derived

[llogiq]

@asquared31415 how many different unit struct instances do we expect per type (absent generics perhaps)?

Or do we fear there be other behavior in the function body?

[asquared31415]

I am talking about a case where the body of Default::default does something with side effects that is important.

However, checking if Default is automatically derived is sufficient to catch most normal empty structs and that derive is always well behaved, so that check solves any potential issues that I can think of.

[schubart]

I don't think checking if Default is automatically derived is good enough: It might be trivial and automatically derived today, but the owner of the struct might change it to a hand-crafted implementation in the future.

[schubart]

Apologies, I don't have time to work on this at the moment. I don't want to "hog" this good-first-issue, so I'm un-assigning myself and give someone else the chance to claim it.

@rustbot release-assignment

[Alexendoo]

A Default impl of a unit struct having side effects is already bizarre/unlikely enough that I would be surprised to find any real world examples, I wouldn't worry about a derive changing to one in a future version

[llogiq]

Agreed. I would even go so far as to say the additional check isn't needed. By convention, Default::default() should return a stand-in value, and if the type is public, it cannot rely on not having Default called to construct an instance; if the code wants to do so, it can add #[non_exhaustive] or a private unit field. So for public unit structs, T and T::default() are for all intents and purposes the same.