"if let &foo = bar" triggers match_ref_pats

[karyon]

in servo, if let &DisplayItem::StackingContextClass(ref stacking_context) = kid { triggers match_ref_pats.

complete output:

/servo/components/gfx/paint_task.rs:519:17: 527:18 warning: instead of prefixing all patterns with `&`, you can dereference the expression: `if let ... = *kid {`, #[warn(match_ref_pats)] on by default
/servo/components/gfx/paint_task.rs:519                 if let &DisplayItem::StackingContextClass(ref stacking_context) = kid {
/servo/components/gfx/paint_task.rs:520                     build_from_stacking_context(properties,
/servo/components/gfx/paint_task.rs:521                                                 &stacking_context,
/servo/components/gfx/paint_task.rs:522                                                 &parent_origin,
/servo/components/gfx/paint_task.rs:523                                                 &transform,
/servo/components/gfx/paint_task.rs:524                                                 &perspective,
                                                                 ...
/servo/components/gfx/paint_task.rs:519:17: 527:18 help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats

also, match-patterns where only one arm does the dereferencing also triggers it:

/servo/components/script/dom/htmlanchorelement.rs:62:9: 65:10 warning: instead of prefixing all patterns with `&`, you can dereference the expression: `match *name { ...`, #[warn(match_ref_pats)] on by default
/servo/components/script/dom/htmlanchorelement.rs:62         match name {
/servo/components/script/dom/htmlanchorelement.rs:63             &atom!("rel") => AttrValue::from_serialized_tokenlist(value),
/servo/components/script/dom/htmlanchorelement.rs:64             _ => self.super_type().unwrap().parse_plain_attribute(name, value),
/servo/components/script/dom/htmlanchorelement.rs:65         }
/servo/components/script/dom/htmlanchorelement.rs:62:9: 65:10 help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#match_ref_pats

[Manishearth]

I believe both situations are intentional?

[karyon]

do you mean intentional by servo or by clippy? imo changing one & to a * doesn't make the code any more readable, so i thought it was not intended by clippy... and in servo, i have about 80 of these warnings, so i thought it was intentionally written like this in servo :)

[Manishearth]

Intentional by clippy. & patterns aren't often used by the rest of the community, and readability issues come into play when mixed with ref bindings.

Servo has tons of cases of nonstandard code style since it comes from a time when Rust had different syntax and features and has incrementally updated over Rust changes over time. So there's a lot of old-Rust style left over.

For example, Servo had tons of cases of & patterns being used in match statements as well until I removed them. You don't see this anywhere else in the Rust ecosystem.

[karyon]

alright, i'll just have to believe you on this one :)

but i think the documentation can be improved then. it says "checks for matches where all arms match a reference", which doesn't apply to if lets intuitively (although maybe technically). also, an explanation why this makes sense even if there is only one arm would be helpful for newbies.

[Manishearth]

I think it's mostly a style choice for uniformity. But it's something that we should bikeshed when we rfc clippy; see #533

cc @llogiq