New lint for unused public items?

help
1

I'm thinking about trying to contribute a new feature, and I wanted to ask if it makes sense.

The idea is to create a new opt-in (allow by default) lint for unused public items. Just like unused_items, but for the purpose of opting in for specific items or modules, in circumstances where the items should generally only be constructed by the crate.

For example, for use in bespoke error enums:

#[warn(unused_public_items)]
#[derive(Error, Debug)]
pub enum FunctionSpecificError {
    #[error("All good here")]
    VariantThatIsUsed
    #[error("I would be caught by the lint")]
    VariantThatIsntUsed
}

pub fn foo() -> Result<(), FunctionSpecificError> {
    Err(FunctionSpecificError::VariantThatIsUsed)
}

Does this make sense to add? I would love to use this for my own stuff. Would it make sense as a compiler lint or a clippy lint? Thanks for your feedback.

3

I didn't find unused_items, but I suppose you mean the different rules for unused variables, mut, macros, labels, and so on, and possibly dead_code. For that new lint, do you mean an additional attribute argument unused_enum_variants? Or do you have other public items in mind?

In general, I can't think of a situation where all the enum variants should be explicitly used in the code that uses a crate. The code using that dependency may not use any of the functions that return that enum: should that count as unused? Or must it be coupled to using a function that returns that enum (directly or embedded). And if the code uses one of those functions, it may ignore the specifics. In your example, it could use Err(_) to regroup all the FunctionSpecificError errors and generate its own unique return error variant for that category.

Do you see a real situation where the crate user must examine each enum variant instead?

4

I would like to see this lint — I would find it useful in many places. When working on a multi-crate project where many things have to be pub, sometimes for not great reasons, I find that it’s easy to end up with something that is public but not meaningful any more.

It's also possible for there to be an item that should be read internally:

struct Config {
    foo: bool,
    bar: bool,
}
fn do_thing(config: Config) {
    if config.foo {/* ... */}
    // Forgot to use bar — the option does nothing
}

I think the lint should be defined like: “If this item would emit the dead_code lint if were not pub, then emit this lint”.

I think you misunderstand the idea. In that example, the crate itself (not its dependent) should be constructing all variants of the error enum. If there is a variant that is not constructed — an error that can’t happen — then either the enum should have that variant deleted, or the code that is supposed to emit it is missing or unreachable.

2 Likes