Add a lint for not using field pattern shorthands

[ftxqxd]

Closes #17792.

[huonw]

This would be more efficient with the is_shorthand check first.

[ftxqxd]

@huonw Comments addressed.

[ghost]

There's a small subtlety here, which the following example demonstrates:

struct Struct { x: uint }
const x: uint = 42u;
fn main() {
    match (Struct { x: 1u }) {
        Struct { x: x } => (),
        Struct { x: _ } => ()
    }
}

We need to check that the PatIdent here is not resolved to any definition (it can be a constant but I think in some weird cross-crate situations it could also be a variant or a unit struct).

I think this would read well:

        let def_map = cx.tcx.def_map.borrow();
        match pat.node {
            ast::PatStruct(_, ref v, _) => {
                for fieldpat in v.iter()
                    .filter(|fieldpat| !fieldpat.node.is_shorthand)
                    .filter(|fieldpat| !def_map.contains_key(fieldpat.node.pat.id)) {
                    match fieldpat.node.pat.node {
                        ast::PatIdent(_, ident, None) if ident.node.as_str()

[ftxqxd]

I tried that and it looks like the lint just stopped triggering for anything. I’m thinking of trying something like this:

                for fieldpat in v.iter()
                    .filter(|fieldpat| !fieldpat.node.is_shorthand)
                    .filter(|fieldpat| match def_map.find(&fieldpat.node.pat.id) {
                        Some(&def::DefConst(_)) => false,
                        Some(&def::DefVariant(_)) => false,
                        _ => true,
                    }) {

but I’m not sure if that will work, and if it does how I’ll handle the tuple/unit struct case.

[ghost]

@P1start This looks great. I think if you rebased and addressed the def_map issue, we could get someone to r+ it. :)

[ftxqxd]

OK, I’ve made an attempt (that seems to work) at fixing the issue with const and so on. r? someone?

[ghost]

@P1start I see! My bad, I completely forgot about DefLocal. I think this should work:

for fieldpat in v.iter().
                                 .filter(|fieldpat| def_map.find(&fieldpat.node.pat.id) == Some(&def::DefLocal(fieldpat.node.pat.id)))
                                 .filter(|fieldpat| !fieldpat.node.is_shorthand)
{
                    match fieldpat.node.pat.node {

(no need for if fieldpat.node.is_shorthand { continue }).

[ghost]

@P1start r=me with the last two changes pending @huonw's approval.

[ghost]

I'm happy. @huonw?

[ghost]

@P1start This will need another rebase. :(

[ghost]

@P1start Sorry, will need another rebase. Please ping someone on IRC as soon as you rebase so that we can get the PR in ASAP as it's really prone to bitrotting. :)