From 20d0298a8a7cb180f57fb7fd3e8d4eb24407397f Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 21 Oct 2025 13:54:20 +1100 Subject: [PATCH] chore: fix clippy warnings about HashMap iteration (#10661) Not quite sure how these didn't get picked up by CI but they showed in my local IDE. --------- Signed-off-by: Thomas Eizinger Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- rust/connlib/phoenix-channel/src/lib.rs | 4 ++-- rust/connlib/snownet/src/node/connections.rs | 16 +++++++------- rust/connlib/tunnel/src/tests/sim_client.rs | 10 +-------- rust/connlib/tunnel/src/tests/transition.rs | 22 ++++++++++++++++++++ rust/relay/server/src/server.rs | 2 +- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/rust/connlib/phoenix-channel/src/lib.rs b/rust/connlib/phoenix-channel/src/lib.rs index d2498d474..d357da683 100644 --- a/rust/connlib/phoenix-channel/src/lib.rs +++ b/rust/connlib/phoenix-channel/src/lib.rs @@ -4,7 +4,7 @@ mod get_user_agent; mod login_url; use anyhow::{Context as _, Result}; -use std::collections::{HashMap, VecDeque}; +use std::collections::{BTreeMap, VecDeque}; use std::net::{IpAddr, SocketAddr, ToSocketAddrs as _}; use std::sync::Arc; use std::time::{Duration, Instant}; @@ -48,7 +48,7 @@ pub struct PhoenixChannel { _phantom: PhantomData, - pending_join_requests: HashMap, + pending_join_requests: BTreeMap, // Stored here to allow re-connecting. url_prototype: Secret>, diff --git a/rust/connlib/snownet/src/node/connections.rs b/rust/connlib/snownet/src/node/connections.rs index cb84f013f..a595a9569 100644 --- a/rust/connlib/snownet/src/node/connections.rs +++ b/rust/connlib/snownet/src/node/connections.rs @@ -1,5 +1,5 @@ use std::{ - collections::{BTreeMap, HashMap, VecDeque}, + collections::{BTreeMap, VecDeque}, fmt, hash::Hash, iter, @@ -23,9 +23,9 @@ pub struct Connections { established_by_wireguard_session_index: BTreeMap, - disconnected_ids: HashMap, - disconnected_public_keys: HashMap<[u8; 32], Instant>, - disconnected_session_indices: HashMap, + disconnected_ids: BTreeMap, + disconnected_public_keys: BTreeMap<[u8; 32], Instant>, + disconnected_session_indices: BTreeMap, } impl Default for Connections { @@ -359,9 +359,9 @@ pub struct UnknownConnection { } impl UnknownConnection { - fn by_id(id: TId, disconnected_ids: &HashMap, now: Instant) -> Self + fn by_id(id: TId, disconnected_ids: &BTreeMap, now: Instant) -> Self where - TId: fmt::Display + Eq + Hash, + TId: fmt::Display + Eq + Ord, { Self { id: id.to_string(), @@ -372,7 +372,7 @@ impl UnknownConnection { } } - fn by_index(id: usize, disconnected_indices: &HashMap, now: Instant) -> Self { + fn by_index(id: usize, disconnected_indices: &BTreeMap, now: Instant) -> Self { Self { id: id.to_string(), kind: "index", @@ -384,7 +384,7 @@ impl UnknownConnection { fn by_public_key( key: [u8; 32], - disconnected_public_keys: &HashMap<[u8; 32], Instant>, + disconnected_public_keys: &BTreeMap<[u8; 32], Instant>, now: Instant, ) -> Self { Self { diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index 8b595a23a..f670b875b 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -483,7 +483,7 @@ pub struct RefClient { /// The expected TCP connections. #[debug(skip)] - pub(crate) expected_tcp_connections: HashMap<(IpAddr, Destination, SPort, DPort), ResourceId>, + pub(crate) expected_tcp_connections: BTreeMap<(IpAddr, Destination, SPort, DPort), ResourceId>, /// The expected UDP DNS handshakes. #[debug(skip)] @@ -703,10 +703,6 @@ impl RefClient { } } - #[expect( - clippy::disallowed_methods, - reason = "We don't care about the ordering of the expected TCP connections." - )] pub(crate) fn expected_resource_status( &self, has_failed_tcp_connection: impl Fn((SPort, DPort)) -> bool, @@ -1223,10 +1219,6 @@ impl RefClient { .contains_key(&(src, dst, sport, dport)) } - #[expect( - clippy::disallowed_methods, - reason = "Iteration order does not matter here." - )] pub(crate) fn tcp_connection_tuple_to_resource( &self, resource: ResourceId, diff --git a/rust/connlib/tunnel/src/tests/transition.rs b/rust/connlib/tunnel/src/tests/transition.rs index ec4685ebf..cd2450dfc 100644 --- a/rust/connlib/tunnel/src/tests/transition.rs +++ b/rust/connlib/tunnel/src/tests/transition.rs @@ -144,6 +144,28 @@ pub(crate) enum Destination { IpAddr(IpAddr), } +impl Ord for Destination { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + match (self, other) { + ( + Destination::DomainName { name: left, .. }, + Destination::DomainName { name: right, .. }, + ) => left.cmp(right), + (Destination::IpAddr(left), Destination::IpAddr(right)) => left.cmp(right), + + // These are according to variant order. + (Destination::DomainName { .. }, Destination::IpAddr(_)) => std::cmp::Ordering::Less, + (Destination::IpAddr(_), Destination::DomainName { .. }) => std::cmp::Ordering::Greater, + } + } +} + +impl PartialOrd for Destination { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl Eq for Destination {} impl std::hash::Hash for Destination { diff --git a/rust/relay/server/src/server.rs b/rust/relay/server/src/server.rs index d92035600..a9283a7a9 100644 --- a/rust/relay/server/src/server.rs +++ b/rust/relay/server/src/server.rs @@ -54,7 +54,7 @@ pub struct Server { public_address: IpStack, /// All client allocations, indexed by client's socket address. - allocations: HashMap, + allocations: BTreeMap, clients_by_allocation: HashMap, /// Redundant mapping so we can look route data with a single lookup. channel_and_client_by_port_and_peer: