keepsimple1 / mdns-sd

Rust library for mDNS based Service Discovery
Apache License 2.0
102 stars 39 forks source link

feat: apply nursery lints #202

Closed CosminPerRam closed 6 months ago

CosminPerRam commented 7 months ago

This PR upgrades clippy to warn about nursery lints on CI and applies said lints.

Applied changes:

This is a 'low-hanging fruit' PR that attempts on making the code a bit more concise (and possible more performant because of the constants and/or dedicated methods for if/match).

Note: the nursery lint group are "new lints that are still under development", if you might not want it, I'll remove it from the CI file.

keepsimple1 commented 6 months ago
  • Add allow(clippy::cognitive_complexity) for the service_daemon module, the lint rates the complexity of a function and warns if it is exceeded, in this case 2 functions pass the default threshold, the solution to this is to make it smaller by refactoring, splitting it into multiple functions etc (or maybe not possible in some cases), I've added this as solving it would be outside of the PR's scope, so lets make CI pass.

It's cool to learn about this (for me). Although I opted not to mandate cognitive_complexity lint, I'm curious what the two functions are. (A quick local run didn't turn up such warnings unless I missed them).

CosminPerRam commented 6 months ago
  • Add allow(clippy::cognitive_complexity) for the service_daemon module, the lint rates the complexity of a function and warns if it is exceeded, in this case 2 functions pass the default threshold, the solution to this is to make it smaller by refactoring, splitting it into multiple functions etc (or maybe not possible in some cases), I've added this as solving it would be outside of the PR's scope, so lets make CI pass.

It's cool to learn about this (for me). Although I opted not to mandate cognitive_complexity lint, I'm curious what the two functions are. (A quick local run didn't turn up such warnings unless I missed them).

Yeah, very interesting to see that clippy nursery suggests splitting big chunks of code/complexity.

    Checking mdns-sd v0.11.0 (/Users/cosminpatrinjan/Github/_contributing/mdns-sd)
error: the function has a cognitive complexity of (50/25)
   --> src/service_daemon.rs:624:8
    |
624 |     fn exec_command(zc: &mut Zeroconf, command: Command, repeating: bool) {
    |        ^^^^^^^^^^^^
    |
    = help: you could split it up into multiple smaller functions
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
    = note: `-D clippy::cognitive-complexity` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::cognitive_complexity)]`

error: the function has a cognitive complexity of (26/25)
    --> src/service_daemon.rs:1970:8
     |
1970 |     fn handle_query(&mut self, msg: DnsIncoming, ip: &IpAddr) {
     |        ^^^^^^^^^^^^
     |
     = help: you could split it up into multiple smaller functions
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity

error: could not compile `mdns-sd` (lib) due to 2 previous errors
keepsimple1 commented 6 months ago

Yeah, very interesting to see that clippy nursery suggests splitting big chunks of code/complexity.

    Checking mdns-sd v0.11.0 (/Users/cosminpatrinjan/Github/_contributing/mdns-sd)
error: the function has a cognitive complexity of (50/25)
   --> src/service_daemon.rs:624:8
    |
624 |     fn exec_command(zc: &mut Zeroconf, command: Command, repeating: bool) {
    |        ^^^^^^^^^^^^
    |
    = help: you could split it up into multiple smaller functions
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
    = note: `-D clippy::cognitive-complexity` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::cognitive_complexity)]`

error: the function has a cognitive complexity of (26/25)
    --> src/service_daemon.rs:1970:8
     |
1970 |     fn handle_query(&mut self, msg: DnsIncoming, ip: &IpAddr) {
     |        ^^^^^^^^^^^^
     |
     = help: you could split it up into multiple smaller functions
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity

error: could not compile `mdns-sd` (lib) due to 2 previous errors

cool! we can look into them. I opened a separate issue #204 to track this .