Add clippy to Travis integration

[rloomba]

Inspired by #724. This PR fixes 37 warnings caused by using redundant fields and adds clippy integration to Travis to error out if a redundant field is detected. Hopefully we can slowly fix warnings/errors and add those fixed lints to the list of denies or as @casey mentioned, disable specific lints using #![allow(clippy::LINT)] in modules/crates/functions.

Does it make sense to specify clippy to run on 1.30.0 for lightning and 1.39.0 for lightning-net-tokio?

[codecov]

Codecov Report

Merging #739 into main will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #739      +/-   ##
==========================================
+ Coverage   91.94%   91.98%   +0.04%     
==========================================
  Files          37       37              
  Lines       20108    20108              
==========================================
+ Hits        18488    18497       +9     
+ Misses       1620     1611       -9     

Impacted Files	Coverage Δ
lightning/src/lib.rs	100.00% <ø> (ø)
lightning/src/ln/channelmanager.rs	85.64% <ø> (ø)
lightning/src/ln/peer_handler.rs	58.43% <ø> (ø)
lightning/src/chain/channelmonitor.rs	95.07% <100.00%> (ø)
lightning/src/ln/channel.rs	86.34% <100.00%> (ø)
lightning/src/ln/onion_utils.rs	95.13% <100.00%> (ø)
lightning/src/ln/peer_channel_encryptor.rs	93.35% <100.00%> (ø)
lightning/src/util/chacha20poly1305rfc.rs	100.00% <100.00%> (ø)
lightning/src/ln/functional_tests.rs	97.06% <0.00%> (+0.16%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f35a5ce...262172c. Read the comment docs.

[TheBlueMatt]

Cool. Probably worth at least turning on whatever subset already passes instead of just one lint, and definitely add let_underscore_lock, given that was the actual bug for us.

[ariard]

Otherwise ACK 0c950e4

[rloomba]

It looks like unfortunately 1.30.0 doesn't support scoped lints, more at rust-lang/rust#44690:

error[E0658]: scoped lint `clippy::erasing_op` is experimental (see issue #44690)

So if we ever move the MSRV to 1.31.0 or higher, we'll be able to specify lints within individual libs.

But for now, we'll only run the lint on 1.39.0 with the default linter + add allows for erasing_op, never_loop, and if_same_then_else. I think we'll be able to easily refactor so we can remove never_loop and if_same_then_else.

clippy docs

[TheBlueMatt]

I dont think our MSRV needs to extend to clippy - in general test and other infrastructure which isn't required for users I don't think needs to observe MSRV at all.

[TheBlueMatt]

Maybe "Note that our CI enforces a few basic clippy lints - you can test locally if you use rustup, or just push to CI to test." or something like that, given that clippy is only available via rustup and we dont want to encourge people to use rustup when they otherwise wouldn't (as a few other things we use break, eg the bindings generation link-time-LTO, which is way more important than clippy)

[rloomba]

awesome, thanks for the context!

[rloomba]

I dont think our MSRV needs to extend to clippy - in general test and other infrastructure which isn't required for users I don't think needs to observe MSRV at all.

Totally agree. I think if we ever want to support the broader Rust feature of scoped attributes for white-listed tools, e.g., #![allow(clippy::erasing_op)], as seen in 0c950e4, we'll need to increase our MSRV to 1.31.0. Because in currently in 1.30.0, if we include a scoped attribute, there's a compiler error show in #739 (comment) that stops the build.

[TheBlueMatt]

there's a compiler error show in #739 (comment) that stops the build.

Ah, OK, I misunderstood the conversation sorry.

[TheBlueMatt]

Can you add the lint run to github CI as well? We'll probably remove travis soon, I think. Otherwise, ACK!