Merge #47: chore: fix clippy and rust problems

ee84bfc ci: update workflows (Cameron Garnham)
fcd3d96 chore: fix clippy and rust problems (Cameron Garnham)
7a3787c cargo: remove unused deps (Cameron Garnham)

Pull request description:

  Fix Clippy Warnings for entire Project and Update Testing Workflow.

ACKs for top commit:
  da2ce7:
    ACK ee84bfc

Tree-SHA512: b16fc434552649d6c3479ea9f98d1c949454e278851928b2350a2ee3ea7cb4b303067aa52f1a761d8552ef5dab794f07b8141d5fa2a910a0e81848dd4d488842

Author: da2ce7

.github/workflows/coverage.yaml

@@ -19,7 +19,7 @@ jobs:
     steps:
       - id: checkout
         name: Checkout Repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - id: setup
         name: Setup Toolchain
@@ -46,7 +46,7 @@ jobs:
 
       - id: upload
         name: Upload Coverage Report
-        uses: codecov/codecov-action@v3
+        uses: codecov/codecov-action@v4
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: ${{ steps.coverage.outputs.report }}

.github/workflows/testing.yaml

@@ -15,7 +15,7 @@ jobs:
     steps:
       - id: checkout
         name: Checkout Repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - id: setup
         name: Setup Toolchain
@@ -39,71 +39,76 @@ jobs:
 
     strategy:
       matrix:
-        toolchain: [stable, nightly]
+        toolchain: [nightly, stable]
 
     steps:
       - id: checkout
         name: Checkout Repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - id: setup
         name: Setup Toolchain
         uses: dtolnay/rust-toolchain@stable
         with:
-          toolchain: nightly
+          toolchain: ${{ matrix.toolchain }}
           components: clippy
 
       - id: cache
         name: Enable Workflow Cache
         uses: Swatinem/rust-cache@v2
 
+      - id: tools
+        name: Install Tools
+        uses: taiki-e/install-action@v2
+        with:
+          tool: cargo-machete
+
       - id: check
         name: Run Build Checks
-        run: cargo check --workspace --all-targets --all-features
+        run: cargo check --tests --benches --examples --workspace --all-targets --all-features
 
       - id: lint
         name: Run Lint Checks
-        run: cargo clippy --workspace --all-targets --all-features  -- -D clippy::correctness -D clippy::suspicious
+        run: cargo clippy --tests --benches --examples --workspace --all-targets --all-features -- -D clippy::correctness -D clippy::suspicious -D clippy::complexity -D clippy::perf -D clippy::style -D clippy::pedantic
 
-      - id: doc
-        name: Run Documentation Checks
-        run: cargo test --doc
+      - id: docs
+        name: Lint Documentation
+        env:
+          RUSTDOCFLAGS: "-D warnings"
+        run: cargo doc --no-deps --bins --examples --workspace --all-features
+
+      - id: deps
+        name: Check Unused Dependencies
+        run: cargo machete --with-metadata
 
-  internal:
-    name: Units and Integrations
+  unit:
+    name: Units
     runs-on: ubuntu-latest
     needs: check
 
     strategy:
       matrix:
-        toolchain: [stable, nightly]
+        toolchain: [nightly, stable]
 
     steps:
       - id: checkout
         name: Checkout Repository
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
 
       - id: setup
         name: Setup Toolchain
         uses: dtolnay/rust-toolchain@stable
         with:
           toolchain: ${{ matrix.toolchain }}
-          components: llvm-tools-preview
 
       - id: cache
         name: Enable Job Cache
         uses: Swatinem/rust-cache@v2
 
-      - id: test
-        name: Run Code Tests
-        run: cargo test --workspace --all-targets --all-features
-
-      - id: tools
-        name: Install Coverage Tools
-        uses: taiki-e/install-action@v2
-        with:
-          tool: cargo-llvm-cov, cargo-nextest
+      - id: test-docs
+        name: Run Documentation Tests
+        run: cargo test --doc
 
-      - id: coverage
-        name: Generate Coverage Report
-        run: cargo llvm-cov nextest --workspace --all-targets --all-features
+      - id: test
+        name: Run Unit Tests
+        run: cargo test --tests --benches --examples --workspace --all-targets --all-features

examples/get_metadata/Cargo.toml

@@ -16,14 +16,12 @@ version.workspace = true
 [dependencies]
 dht = { path = "../../packages/dht" }
 handshake = { path = "../../packages/handshake" }
-metainfo = { path = "../../packages/metainfo" }
 peer = { path = "../../packages/peer" }
 select = { path = "../../packages/select" }
 
 clap = "3"
 futures = "0.1"
 tokio-core = "0.1"
-tokio-io = "0.1"
 tokio-codec = "0.1"
 pendulum = "0.3"
 hex = "0.4"

examples/get_metadata/src/main.rs

@@ -21,8 +21,9 @@ use peer::protocols::{NullProtocol, PeerExtensionProtocol, PeerWireProtocol};
 use peer::{IPeerManagerMessage, OPeerManagerMessage, PeerInfo, PeerManagerBuilder, PeerProtocolCodec};
 use pendulum::future::TimerBuilder;
 use pendulum::HashedWheelBuilder;
+use select::discovery::error::DiscoveryError;
 use select::discovery::{IDiscoveryMessage, ODiscoveryMessage, UtMetadataModule};
-use select::{ControlMessage, IExtendedMessage, IUberMessage, OExtendedMessage, OUberMessage, UberModuleBuilder};
+use select::{ControlMessage, DiscoveryTrait, IExtendedMessage, IUberMessage, OExtendedMessage, OUberMessage, UberModuleBuilder};
 use tokio_core::reactor::Core;
 
 // Legacy Handshaker, when bip_dht is migrated, it will accept S directly
@@ -69,6 +70,7 @@ where
     fn metadata(&mut self, _data: ()) {}
 }
 
+#[allow(clippy::too_many_lines)]
 fn main() {
     let matches = clap_app!(myapp =>
         (version: "1.0")
@@ -100,7 +102,7 @@ fn main() {
             // Set a low handshake timeout so we don't wait on peers that aren't listening on tcp
             HandshakerConfig::default().with_connect_timeout(Duration::from_millis(500)),
         )
-        .build(TcpTransport, core.handle())
+        .build(TcpTransport, &core.handle())
         .unwrap()
         .into_parts();
     // Create a peer manager that will hold our peers and heartbeat/send messages to them
@@ -109,7 +111,7 @@ fn main() {
     // Hook up a future that feeds incoming (handshaken) peers over to the peer manager
     core.handle().spawn(
         handshaker_recv
-            .map_err(|_| ())
+            .map_err(|()| ())
             .map(|complete_msg| {
                 // Our handshaker finished handshaking some peer, get
                 // the peer info as well as the peer itself (socket)
@@ -141,11 +143,22 @@ fn main() {
     );
 
     // Create our UtMetadata selection module
-    let (uber_send, uber_recv) = UberModuleBuilder::new()
-        .with_extended_builder(Some(ExtendedMessageBuilder::new()))
-        .with_discovery_module(UtMetadataModule::new())
-        .build()
-        .split();
+    let (uber_send, uber_recv) = {
+        let mut this = UberModuleBuilder::new().with_extended_builder(Some(ExtendedMessageBuilder::new()));
+        let module = UtMetadataModule::new();
+        this.discovery.push(Box::new(module)
+            as Box<
+                dyn DiscoveryTrait<
+                    SinkItem = IDiscoveryMessage,
+                    SinkError = Box<DiscoveryError>,
+                    Item = ODiscoveryMessage,
+                    Error = Box<DiscoveryError>,
+                >,
+            >);
+        this
+    }
+    .build()
+    .split();
 
     // Tell the uber module we want to download metainfo for the given hash
     let uber_send = core
@@ -159,7 +172,7 @@ fn main() {
     let timer = TimerBuilder::default().build(HashedWheelBuilder::default().build());
     let timer_recv = timer.sleep_stream(Duration::from_millis(100)).unwrap().map(Either::B);
 
-    let merged_recv = peer_manager_recv.map(Either::A).map_err(|_| ()).select(timer_recv);
+    let merged_recv = peer_manager_recv.map(Either::A).map_err(|()| ()).select(timer_recv);
 
     // Hook up a future that receives messages from the peer manager
     core.handle().spawn(future::loop_fn(
@@ -183,23 +196,23 @@ fn main() {
                             info, message,
                         ))),
                         Either::A(OPeerManagerMessage::PeerAdded(info)) => {
-                            println!("Connected To Peer: {:?}", info);
+                            println!("Connected To Peer: {info:?}");
                             Some(IUberMessage::Control(ControlMessage::PeerConnected(info)))
                         }
                         Either::A(OPeerManagerMessage::PeerRemoved(info)) => {
-                            println!("We Removed Peer {:?} From The Peer Manager", info);
+                            println!("We Removed Peer {info:?} From The Peer Manager");
                             Some(IUberMessage::Control(ControlMessage::PeerDisconnected(info)))
                         }
                         Either::A(OPeerManagerMessage::PeerDisconnect(info)) => {
-                            println!("Peer {:?} Disconnected From Us", info);
+                            println!("Peer {info:?} Disconnected From Us");
                             Some(IUberMessage::Control(ControlMessage::PeerDisconnected(info)))
                         }
                         Either::A(OPeerManagerMessage::PeerError(info, error)) => {
-                            println!("Peer {:?} Disconnected With Error: {:?}", info, error);
+                            println!("Peer {info:?} Disconnected With Error: {error:?}");
                             Some(IUberMessage::Control(ControlMessage::PeerDisconnected(info)))
                         }
-                        Either::B(_) => Some(IUberMessage::Control(ControlMessage::Tick(Duration::from_millis(100)))),
-                        _ => None,
+                        Either::B(()) => Some(IUberMessage::Control(ControlMessage::Tick(Duration::from_millis(100)))),
+                        Either::A(_) => None,
                     };
 
                     match opt_message {

examples/simple_torrent/Cargo.toml

@@ -23,6 +23,4 @@ peer = { path = "../../packages/peer" }
 clap = "3"
 futures = "0.1"
 tokio-core = "0.1"
-tokio-io = "0.1"
-tokio-timer = "0.1"
 tokio-codec = "0.1"

examples/simple_torrent/src/main.rs

@@ -67,6 +67,7 @@ impl<S> Handshaker for LegacyHandshaker<S> where S: Sink<SinkItem=InitiateMessag
 const MAX_PENDING_BLOCKS: usize = 50;
 
 // Some enum to store our selection state updates
+#[allow(dead_code)]
 #[derive(Debug)]
 enum SelectState {
     Choke(PeerInfo),
@@ -84,9 +85,11 @@ enum SelectState {
     TorrentAdded,
 }
 
+#[allow(clippy::too_many_lines)]
 fn main() {
     // Command line argument parsing
     let matches = clap_app!(myapp =>
+
         (version: "1.0")
         (author: "Andrew <[email protected]>")
         (about: "Simple torrent downloading")
@@ -130,7 +133,7 @@ fn main() {
         // block when we reach our max peers). Setting these to low
         // values so we don't have more than 2 unused tcp connections.
         .with_config(HandshakerConfig::default().with_wait_buffer_size(0).with_done_buffer_size(0))
-        .build::<TcpTransport>(TcpTransport, core.handle()) // Will handshake over TCP (could swap this for UTP in the future)
+        .build::<TcpTransport>(TcpTransport, &core.handle()) // Will handshake over TCP (could swap this for UTP in the future)
         .unwrap()
         .into_parts();
     // Create a peer manager that will hold our peers and heartbeat/send messages to them
@@ -146,7 +149,7 @@ fn main() {
     let map_peer_manager_send = peer_manager_send.clone().sink_map_err(|_| ());
     core.handle().spawn(
         handshaker_recv
-            .map_err(|_| ())
+            .map_err(|()| ())
             .map(|complete_msg| {
                 // Our handshaker finished handshaking some peer, get
                 // the peer info as well as the peer itself (socket)
@@ -173,7 +176,7 @@ fn main() {
 
     // Map out the errors for these sinks so they match
     let map_select_send = select_send.clone().sink_map_err(|_| ());
-    let map_disk_manager_send = disk_manager_send.clone().sink_map_err(|_| ());
+    let map_disk_manager_send = disk_manager_send.clone().sink_map_err(|()| ());
 
     // Hook up a future that receives messages from the peer manager, and forwards request to the disk manager or selection manager (using loop fn
     // here because we need to be able to access state, like request_map and a different future combinator wouldn't let us keep it around to access)
@@ -241,15 +244,15 @@ fn main() {
                         OPeerManagerMessage::PeerAdded(info) => Some(Either::A(SelectState::NewPeer(info))),
                         OPeerManagerMessage::SentMessage(_, _) => None,
                         OPeerManagerMessage::PeerRemoved(info) => {
-                            println!("We Removed Peer {:?} From The Peer Manager", info);
+                            println!("We Removed Peer {info:?} From The Peer Manager");
                             Some(Either::A(SelectState::RemovedPeer(info)))
                         }
                         OPeerManagerMessage::PeerDisconnect(info) => {
-                            println!("Peer {:?} Disconnected From Us", info);
+                            println!("Peer {info:?} Disconnected From Us");
                             Some(Either::A(SelectState::RemovedPeer(info)))
                         }
                         OPeerManagerMessage::PeerError(info, error) => {
-                            println!("Peer {:?} Disconnected With Error: {:?}", info, error);
+                            println!("Peer {info:?} Disconnected With Error: {error:?}");
                             Some(Either::A(SelectState::RemovedPeer(info)))
                         }
                     };
@@ -305,6 +308,7 @@ fn main() {
                             let peer_info = peer_list.remove(1);
 
                             // Pack up our block into a peer wire protocol message and send it off to the peer
+                            #[allow(clippy::cast_possible_truncation)]
                             let piece =
                                 PieceMessage::new(metadata.piece_index() as u32, metadata.block_offset() as u32, block.freeze());
                             let pwp_message = PeerWireProtocolMessage::Piece(piece);
@@ -360,13 +364,13 @@ fn main() {
                         match opt_item.unwrap() {
                             // Disk manager identified a good piece already downloaded
                             SelectState::GoodPiece(index) => {
-                                piece_requests.retain(|req| req.piece_index() != index as u32);
+                                piece_requests.retain(|req| u64::from(req.piece_index()) != index);
                                 Loop::Continue((select_recv, piece_requests, cur_pieces + 1))
                             }
                             // Disk manager is finished identifying good pieces, torrent has been added
                             SelectState::TorrentAdded => Loop::Break((select_recv, piece_requests, cur_pieces)),
                             // Shouldn't be receiving any other messages...
-                            message => panic!("Unexpected Message Received In Selection Receiver: {:?}", message),
+                            message => panic!("Unexpected Message Received In Selection Receiver: {message:?}"),
                         }
                     })
                     .map_err(|_| ())
@@ -467,14 +471,14 @@ fn main() {
                                 vec![IPeerManagerMessage::SendMessage(
                                     peer,
                                     0,
-                                    PeerWireProtocolMessage::Have(HaveMessage::new(piece as u32)),
+                                    PeerWireProtocolMessage::Have(HaveMessage::new(piece.try_into().unwrap())),
                                 )]
                             } else {
                                 vec![]
                             }
                         }
                         // Decided not to handle these two cases here
-                        SelectState::RemovedPeer(info) => panic!("Peer {:?} Got Disconnected", info),
+                        SelectState::RemovedPeer(info) => panic!("Peer {info:?} Got Disconnected"),
                         SelectState::BadPiece(_) => panic!("Peer Gave Us Bad Piece"),
                         _ => vec![],
                     };
@@ -507,11 +511,11 @@ fn main() {
                         Box::new(
                             map_peer_manager_send
                                 .send_all(stream::iter_result(send_messages.into_iter().map(Ok::<_, ()>)))
-                                .map_err(|_| ())
+                                .map_err(|()| ())
                                 .and_then(|(map_peer_manager_send, _)| {
                                     map_peer_manager_send.send_all(stream::iter_result(next_piece_requests))
                                 })
-                                .map_err(|_| ())
+                                .map_err(|()| ())
                                 .map(move |(map_peer_manager_send, _)| {
                                     Loop::Continue((
                                         select_recv,
@@ -567,6 +571,7 @@ fn generate_requests(info: &Info, block_size: usize) -> Vec<RequestMessage> {
         for block_index in 0..whole_blocks {
             let block_offset = block_index * block_size as u64;
 
+            #[allow(clippy::cast_possible_truncation)]
             requests.push(RequestMessage::new(piece_index as u32, block_offset as u32, block_size));
         }
 
@@ -576,9 +581,9 @@ fn generate_requests(info: &Info, block_size: usize) -> Vec<RequestMessage> {
             let block_offset = whole_blocks * block_size as u64;
 
             requests.push(RequestMessage::new(
-                piece_index as u32,
-                block_offset as u32,
-                partial_block_length as usize,
+                piece_index.try_into().unwrap(),
+                block_offset.try_into().unwrap(),
+                partial_block_length.try_into().unwrap(),
             ));
         }