Findings from running Clippy on Servo

[Manishearth]

(I'll update as I move forward)

• float_cmp/single_match needs in_external_macro, otherwise it lints on all derived thingies. We should find other lints that are prone to lint on derive too.
• option_unwrap_unused has too many false positives (places where it is known to exist, for example), and in general unwrap seems pervasive throughout Servo and other Rust codebases. I vote we move this to Allow.
• let_unit_value also has a problem with derives, even though it uses in_macro. Will investigate

PRs: servo/servo#7224, servo/servo#7536, servo/servo#7519

[Manishearth]

I think our macro checking should be stronger, perhaps with some specific checks to totally ignore derive or even all plugins

[birkenfeld]

Although I don't like it, I think it's fair enough to let people use unwrap by default.

[llogiq]

I'd really like to see how the spans for derive-macros look. Will have to investigate.

[Manishearth]

This seems to be a rustc bug btw

[Manishearth]

Our in_external_macro is fine, but the spans for certain serde impls are broken in Servo. I have a MWE which I'll file a bug with

[llogiq]

Even a lint that is allow by default can have some value – if people care enough to use it, e.g. per-project.

[Manishearth]

rust-lang/rust#27817

[Manishearth]

Until that gets fixed, keeping the let lint on allow for servo

[Manishearth]

Btw, I'd love for a way to configure allows/warns dynamically

[birkenfeld]

What do you mean by "dynamically"?

[Manishearth]

I'm running the "servo" branch of Clippy on servo, btw. Not all lints changed to Allow should be changed to Allow in Clippy itself, in some cases it's because Servo is special snowflake-y (like float_cmp -- we have a lot of legit reasons to compare floats), or just because Servo is an old codebase and some lints are noisy (eg the for loop iter ones), but should be turned back to warn when servo fixes them.

[Manishearth]

Oh, sorry, that was a drive-by comment without context.

Basically, it would be nice if once could specify the matrix of allow/warn/deny when plugin_registrar is run. In Servo I just run Clippy's plugin_registrar from our existing plugins module.

Even if there was a way to do this as a Cargo feature it would be fine. The idea is that the set of allow/warn/denys can be changed for an entire codebase, instead of per-lib.

[Manishearth]

https://github.com/Manishearth/rust-clippy/compare/servo

Feel free to pick up fixes from here as they appear and hoist them to master

[llogiq]

I have an open cargo issue here (actually huon filed it then, after I had asked how to do it on StackOverflow).

[Manishearth]

First batch of Servo improvements. This is a small chunk of overall linted stuff.

https://github.com/servo/servo/pull/7224/files

Full list of warnings (with some filtered out): https://gist.github.com/Manishearth/f127d1353586b8ebb271