lint against glob imports of enum variants

[nrc]

It is usually better style to use the prefixed name of an enum variant, rather than importing variants. The glob form of this is particularly poor form.

Two possible lints:

• check use ...::Foo::*; to see if Foo is an enum,
• check all imports to see if they are enum variants.

Bonus points if the lint can suggest renamed versions of the variants in the file.

In fact, we could probably lint against all glob imports too (if there isn't a lint for this already, I couldn't find one by searching for "glob").

[llogiq]

I am unsure about this – it very much depends on how often those Variants are used within a crate and how likely name clashes are IMHO.

OTOH many modules already reexport enum variants, so perhaps that's the way to go.

[Manishearth]

I think this depends on the naming of the variants, too. And how they're intended to be used.

Pedantic lint okay though. Not sure about a Warn lint. A reexport suggestion makes sense too.

[nrc]

I think modern Rust style is to name enum variants without any kind of prefix and assume that they will always be used with the enum name as a prefix.

I can think of two exceptions:

• some of the really common std lib enums - Option, Result. Possibly this extends to user-defined result types too (we can't make this distinction in a lint, however).
• sometimes a function does a lot of juggling of one particular enum, in this case importing all the variants into the scope of the function makes sense. This would be easy to accommodate in a lint by only applying the lint to module-scoped imports.

[Manishearth]

Fair enough

[birkenfeld]

I'd also argue for leaving use self::Enum::* unlinted.

[nrc]

@birkenfeld not sure how customisable lints can be, but it might be nice to separate that out some how. I actually think this is poor style - if variants are named to take the enum name into account, then that applies equally in the module they are defined as elsewhere, and it means less chance of a name clash with other types. I can see how reasonable people can disagree about this though.

[Manishearth]

if variants are named to take the enum name into account,

I'm not talking about that situation, but there are situations where the enum is ubiquitous enough (or the names are obvious enough) that you don't need namespacing

[llogiq]

We're already too opinionated in some cases (shadowing and match_ref_pats on if let being two examples). I'm unsure if the rationale for this is really sound. If you think it through to its conclusion, we should warn on all use statements and ask people to always use full paths for everything.

[Manishearth]

We can make it pedantic and postpone bikeshed to the RfC. clippy_pedantic has a very low bar for inclusion.