Rust shouldn't warn about snake case in patterns

[Timmmm]

For example:

enum Animal {
  Spider {
    iHateSnakes: bool,
  },
  ...
}

...

match pet {
  Animal::Spider { iHateSnakes } =>
  ...

This gives you a warning that iHateSnakes should be snake case in the match arm. But at that point you don't really have much choice about it. Variable name warnings should be where the variable is named, and it isn't really named there. (Maybe there's some syntax like { i_hate_snakes @ iHateSnakes } but who is going to bother with that?)

This issue has been assigned to @jumbatm via this comment.

[hellow554]

Please provide a working example without ... next time. It makes it easier to see what's going on here.

Let's assume this one:

enum Animal {
    Spider { iHateSnakes: bool },
}

fn main() {
    let pet = Animal::Spider { iHateSnakes: false };

    match pet {
        Animal::Spider { iHateSnakes } => dbg!(iHateSnakes),
    };
}

You could instead write

match pet {
    Animal::Spider { iHateSnakes: i_hate_snakes } => dbg!(i_hate_snakes),
};

and have full control over the names of the binding and that's why the compiler is complaining.

You are right in the way, that we should lint the original issue. I'm not sure how the the suggestion goes in terms of machine applicability and renaming of further instances of this variable.
Let's see what the future™ will bring :)

[scottmcm]

Direct repro link: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=f1197162b57e77fff49f0e9536aa6366

Current warning:

warning: variable `iHateSnakes` should have a snake case name
  --> src/main.rs:10:26
   |
10 |         Animal::Spider { iHateSnakes } => dbg!(iHateSnakes),
   |                          ^^^^^^^^^^^ help: convert the identifier to snake case: `i_hate_snakes`
   |
   = note: `#[warn(non_snake_case)]` on by default

Seems like a diagnostics fix to change the help to

help: convert the identifier to snake case: `iHateSnakes: i_hate_snakes`

[joshtriplett]

We discussed this in the lang team meeting, and we agree with the proposal here. We shouldn't warn twice over the same field name; we should only warn about the definition, and not again when you match it.

Note, in particular, that it seems like a relatively common pattern to disable these lints only in a module written by bindgen or otherwise written to match some external spec/API, but leave them enabled in the rest of your code.

So, 👍 to not linting for field puns on field names that don't match the recommended style. (We should still lint when not punning, if you define a new local name that doesn't match the recommended style.)

[jumbatm]

Hi, I'd like to take this one. Just to make sure I'm understanding this right: We want to warn on identifier used to define the field name in the struct itself, and not on the match binding. Is that correct?

[scottmcm]

@jumbatm We definitely want to warn on the struct definition. My understanding is that we don't want to warn on the binding when you didn't actually write the binding ("for field puns"). So we don't warn on Foo { iHateSnakes } but would warn on Foo { iHateSnakes: iHateSnakes } (and definitely on Foo { i_hate_snakes: iHateSnakes }).

[jumbatm]

That makes a lot of sense. Thanks!

@rustbot claim

[joshtriplett]

On November 21, 2019 1:56:42 PM PST, Jon F ***@***.***> wrote: Hi, I'd like to take this one. Just to make sure I'm understanding this right: We want to warn on identifier used to define the struct itself, and not on the match binding. Is that correct?

Right, exactly. I would suggest starting by writing some (failing) test cases for the desired behavior including warning output. I would be happy to review that step and confirm.

[jumbatm]

@joshtriplett I've put up a PR with the failing test case. How's this look? Are there any other cases I should test?