Unused parameters in default trait impls

[derekdreery]

Note I'm not sure what the correct tag for topics about linting, let me know if I miscategorised.

So if you make a function like

fn oneify(a: u8) -> u8 {
    1
}

you will get a warning telling you that you didn't use a, and that you can suppress the warning by using _a instead. All good.

Now suppose that you are writing a trait, and your trait looks like the following:

trait Drawable {
    fn draw(&self, ctx: &mut DrawContext) {}
}

where the default implementation does nothing. If you compile this you will get the same warning, telling you that you should use _ctx, which in practice many people do.

The problem is that the name of the variable appears in documentation, and so variable names look messy and it's slightly harder to quickly parse what's going on when you look at the documentation. For a particularly severe example see the documentation for EventHandler in ggez.

I think there are 3 options for solving this issue

1. Do nothing. Maybe it is only an aesthetic issue (although I find it distracting).
2. Modify the lint to allow unused parameters when the method is a default implementation for a trait.
3. Modify the generated documentation to strip the underscore, or maybe not even show the parameter name, just the type a la C (void top_decl(int, int, char);, note I'm just including for completeness I don't think this is a good idea.)

What does everyone think?

[Nemo157]

What I've started doing to hide the fact the argument is unused from the docs is to discard the argument inside the function:

fn oneify(a: u8) -> u8 {
    let _ = a;
    1
}

I'm 50/50 on whether the lint should be suppressed around trait methods, on one hand a lot of the time you're correctly not using the argument and it's a papercut to have to explicitly mark it unused. On the other hand sometimes it's a bug where you just forgot to use the argument, and having this little reminder is useful.

[kornel]

#[allow(unused)] sounds fine to me.

[bill_myers]

Stripping the underscore in the generated documentation (for all functions, I think) seems the right approach, since the underscore is part of the implementation while the documentation is supposed to describe the interface.

[ohAitch]

In the oneify case "this parameter will never be used" seems like pretty useful documentation actually, though I definitely agree for traits where an interesting implementation would be using the param. Probably still easier to follow an explicit #allow(unused) than implicit name rewriting.

[PO8]

As I have learned the hard way, #[allow(unused)] turns off a whole group of lints. #[allow(unused_variables)] might be safer.

I was a little surprised that #[allow(unused_parameters)] isn't a separate thing, but it doesn't appear to be. Using #[allow(unused_variables)] with a function definition will also turn off warnings for unused variables in the function body.

One can't turn the warnings just on in the body with #![warn(unused_variables)] because that reaches back into the head apparently and turns it back on there: also, that's starting to get ridiculous.

[Nemo157]

On beta (so 1.39 releasing in ~6 weeks) you can scope the lint attribute to the parameter:

pub fn oneify(#[allow(unused_variables)] a: u8) -> u8 {
    1
}