Merge branch 'clippy-ci-and-cleanup'

13 files changed, 101 insertions, 77 deletions

diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml
new file mode 100644
index 0000000000..0b76562513
--- /dev/null
+++ b/.github/workflows/clippy.yml
@@ -0,0 +1,37 @@
+name: Rust - Run Clippy to check lints
+on:
+    # Check whenever a file that affects Clippy is changed in a pull request
+    pull_request:
+        paths:
+            - .github/workflows/clippy.yml
+            - ci/check-clippy.sh
+            - clippy.toml
+            - '**/*.rs'
+    # Check if requested manually from the Actions tab
+    workflow_dispatch:
+jobs:
+  clippy_check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      - name: Checkout binaries submodule
+        run: git submodule update --init --depth=1 dist-assets/binaries
+
+      - uses: actions-rs/toolchain@v1.0.6
+        with:
+            toolchain: stable
+            components: clippy
+            override: true
+
+      - name: Install build dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install libdbus-1-dev
+
+      - name: Clippy check
+        run: |
+          export RUSTFLAGS="--deny warnings"
+          source env.sh
+          time cargo clippy --locked --verbose
diff --git a/talpid-core/build.rs b/talpid-core/build.rs
index 8c5c74911b..b46304d5a0 100644
--- a/talpid-core/build.rs
+++ b/talpid-core/build.rs
@@ -69,6 +69,10 @@ fn main() {
     let link_type = match target_os.as_str() {
         "android" => "",
         "linux" | "macos" => "=static",
+        // We would like to avoid panicing on windows even if we can not link correctly
+        // because we would like to be able to run check and clippy.
+        // This does not allow for correct linking or building.
+        "windows" => "",
         _ => panic!("Unsupported platform: {}", target_os),
     };
 
diff --git a/talpid-core/src/offline/android.rs b/talpid-core/src/offline/android.rs
index 65f0e7cf58..9aa4e33e8a 100644
--- a/talpid-core/src/offline/android.rs
+++ b/talpid-core/src/offline/android.rs
@@ -100,7 +100,7 @@ impl MonitorHandle {
         })
     }
 
-    pub async fn is_offline(&self) -> bool {
+    pub async fn host_is_offline(&self) -> bool {
         match self.get_is_connected() {
             Ok(is_connected) => !is_connected,
             Err(error) => {
diff --git a/talpid-core/src/offline/linux.rs b/talpid-core/src/offline/linux.rs
index 08ee97a7fe..913202f08c 100644
--- a/talpid-core/src/offline/linux.rs
+++ b/talpid-core/src/offline/linux.rs
@@ -25,7 +25,7 @@ const PUBLIC_INTERNET_ADDRESS_V6: IpAddr =
     IpAddr::V6(Ipv6Addr::new(0x2001, 0xdb8, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6));
 
 impl MonitorHandle {
-    pub async fn is_offline(&mut self) -> bool {
+    pub async fn host_is_offline(&self) -> bool {
         match public_ip_unreachable(&self.route_manager).await {
             Ok(is_offline) => is_offline,
             Err(err) => {
diff --git a/talpid-core/src/offline/macos.rs b/talpid-core/src/offline/macos.rs
index 8ba649856a..20e2145e4f 100644
--- a/talpid-core/src/offline/macos.rs
+++ b/talpid-core/src/offline/macos.rs
@@ -37,7 +37,7 @@ pub struct MonitorHandle {
 impl MonitorHandle {
     /// Host is considered to be offline if the IPv4 internet is considered to be unreachable by the
     /// given reachability flags *or* there are no active physical interfaces.
-    pub async fn is_offline(&self) -> bool {
+    pub async fn host_is_offline(&self) -> bool {
         !exists_non_tunnel_default_route().await
     }
 }
diff --git a/talpid-core/src/offline/mod.rs b/talpid-core/src/offline/mod.rs
index 6fc4f46d99..b07fb3d8c9 100644
--- a/talpid-core/src/offline/mod.rs
+++ b/talpid-core/src/offline/mod.rs
@@ -34,9 +34,9 @@ pub use self::imp::Error;
 pub struct MonitorHandle(Option<imp::MonitorHandle>);
 
 impl MonitorHandle {
-    pub async fn is_offline(&mut self) -> bool {
-        match self.0.as_mut() {
-            Some(monitor) => monitor.is_offline().await,
+    pub async fn host_is_offline(&self) -> bool {
+        match self.0.as_ref() {
+            Some(monitor) => monitor.host_is_offline().await,
             None => false,
         }
     }
diff --git a/talpid-core/src/offline/windows.rs b/talpid-core/src/offline/windows.rs
index f0b7b478ce..bbe9d951a9 100644
--- a/talpid-core/src/offline/windows.rs
+++ b/talpid-core/src/offline/windows.rs
@@ -136,7 +136,7 @@ impl BroadcastListener {
         state.apply_change(change);
     }
 
-    pub async fn is_offline(&self) -> bool {
+    pub async fn host_is_offline(&self) -> bool {
         let state = self.system_state.lock();
         state.is_offline_currently()
     }
diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index f6ada1c2cf..b4e6170220 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -104,12 +104,20 @@ where
     // L: (Fn(TunnelEvent) -> std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send>>)
     L: (Fn(TunnelEvent) -> BoxFuture<'static, ()>) + Send + Clone + Sync + 'static,
 {
-    /// Resource directory.
+    /// Toktio runtime handle.
+    pub runtime: tokio::runtime::Handle,
+    /// Resource directory path.
     pub resource_dir: &'a Path,
     /// Callback function called when an event happens.
     pub on_event: L,
     /// Receiver oneshot channel for closing the tunnel.
     pub tunnel_close_rx: oneshot::Receiver<()>,
+    /// Mutex to tunnel provider.
+    pub tun_provider: Arc<Mutex<TunProvider>>,
+    /// Connection retry attempts.
+    pub retry_attempt: u32,
+    /// Route manager handle.
+    pub route_manager: RouteManagerHandle,
 }
 
 // TODO(emilsp) move most of the openvpn tunnel details to OpenVpnTunnelMonitor
@@ -118,13 +126,9 @@ impl TunnelMonitor {
     /// on tunnel state changes.
     #[cfg_attr(any(target_os = "android", windows), allow(unused_variables))]
     pub fn start<L>(
-        runtime: tokio::runtime::Handle,
         tunnel_parameters: &mut TunnelParameters,
         log_dir: &Option<PathBuf>,
-        tun_provider: Arc<Mutex<TunProvider>>,
-        retry_attempt: u32,
-        route_manager: RouteManagerHandle,
-        init_args: TunnelArgs<'_, L>,
+        args: TunnelArgs<'_, L>,
     ) -> Result<Self>
     where
         L: (Fn(TunnelEvent) -> std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send>>)
@@ -138,27 +142,21 @@ impl TunnelMonitor {
 
         match tunnel_parameters {
             #[cfg(not(target_os = "android"))]
-            TunnelParameters::OpenVpn(config) => runtime.block_on(Self::start_openvpn_tunnel(
+            TunnelParameters::OpenVpn(config) => args.runtime.block_on(Self::start_openvpn_tunnel(
                 config,
                 log_file,
-                init_args.resource_dir,
-                init_args.on_event,
-                init_args.tunnel_close_rx,
+                args.resource_dir,
+                args.on_event,
+                args.tunnel_close_rx,
                 #[cfg(target_os = "linux")]
-                route_manager,
+                args.route_manager,
             )),
             #[cfg(target_os = "android")]
             TunnelParameters::OpenVpn(_) => Err(Error::UnsupportedPlatform),
 
-            TunnelParameters::Wireguard(ref mut config) => Self::start_wireguard_tunnel(
-                runtime,
-                config,
-                log_file,
-                tun_provider,
-                retry_attempt,
-                route_manager,
-                init_args,
-            ),
+            TunnelParameters::Wireguard(ref mut config) => {
+                Self::start_wireguard_tunnel(config, log_file, args)
+            }
         }
     }
 
@@ -185,13 +183,9 @@ impl TunnelMonitor {
     }
 
     fn start_wireguard_tunnel<L>(
-        runtime: tokio::runtime::Handle,
         params: &mut wireguard_types::TunnelParameters,
         log: Option<PathBuf>,
-        tun_provider: Arc<Mutex<TunProvider>>,
-        retry_attempt: u32,
-        route_manager: RouteManagerHandle,
-        init_args: TunnelArgs<'_, L>,
+        args: TunnelArgs<'_, L>,
     ) -> Result<Self>
     where
         L: (Fn(TunnelEvent) -> std::pin::Pin<Box<dyn std::future::Future<Output = ()> + Send>>)
@@ -201,10 +195,10 @@ impl TunnelMonitor {
             + 'static,
     {
         #[cfg(target_os = "linux")]
-        runtime.block_on(Self::assign_mtu(&route_manager, params));
+        args.runtime
+            .block_on(Self::assign_mtu(&args.route_manager, params));
         let config = wireguard::config::Config::from_parameters(params)?;
         let monitor = wireguard::WireguardMonitor::start(
-            runtime,
             config,
             if params.options.use_pq_safe_psk {
                 Some(
@@ -213,16 +207,13 @@ impl TunnelMonitor {
                         .exit_peer
                         .as_ref()
                         .map(|peer| peer.public_key.clone())
-                        .unwrap_or(params.connection.peer.public_key.clone()),
+                        .unwrap_or_else(|| params.connection.peer.public_key.clone()),
                 )
             } else {
                 None
             },
             log.as_deref(),
-            tun_provider,
-            retry_attempt,
-            route_manager,
-            init_args,
+            args,
         )?;
         Ok(TunnelMonitor {
             monitor: InternalTunnelMonitor::Wireguard(monitor),
diff --git a/talpid-core/src/tunnel/wireguard/mod.rs b/talpid-core/src/tunnel/wireguard/mod.rs
index e49286cb30..e15896d160 100644
--- a/talpid-core/src/tunnel/wireguard/mod.rs
+++ b/talpid-core/src/tunnel/wireguard/mod.rs
@@ -2,7 +2,7 @@ use self::config::Config;
 #[cfg(not(windows))]
 use super::tun_provider;
 use super::{tun_provider::TunProvider, TunnelArgs, TunnelEvent, TunnelMetadata};
-use crate::routing::{self, RequiredRoute, RouteManagerHandle};
+use crate::routing::{self, RequiredRoute};
 use futures::future::{abortable, AbortHandle as FutureAbortHandle, BoxFuture, Future};
 #[cfg(windows)]
 use futures::{channel::mpsc, StreamExt};
@@ -196,32 +196,29 @@ impl WireguardMonitor {
             + Clone
             + 'static,
     >(
-        runtime: tokio::runtime::Handle,
         mut config: Config,
         psk_negotiation: Option<PublicKey>,
         log_path: Option<&Path>,
-        tun_provider: Arc<Mutex<TunProvider>>,
-        retry_attempt: u32,
-        route_manager: RouteManagerHandle,
-        init_args: TunnelArgs<'_, F>,
+        args: TunnelArgs<'_, F>,
     ) -> Result<WireguardMonitor> {
-        let on_event = init_args.on_event;
+        let on_event = args.on_event;
 
         let endpoint_addrs: Vec<IpAddr> =
             config.peers.iter().map(|peer| peer.endpoint.ip()).collect();
         let (close_msg_sender, close_msg_receiver) = sync_mpsc::channel();
 
-        let obfuscator = maybe_create_obfuscator(&runtime, &mut config, close_msg_sender.clone())?;
+        let obfuscator =
+            maybe_create_obfuscator(&args.runtime, &mut config, close_msg_sender.clone())?;
 
         #[cfg(target_os = "windows")]
         let (setup_done_tx, setup_done_rx) = mpsc::channel(0);
 
         let tunnel = Self::open_tunnel(
-            runtime.clone(),
+            args.runtime.clone(),
             &Self::patch_allowed_ips(&config, psk_negotiation.is_some()),
             log_path,
-            init_args.resource_dir,
-            tun_provider,
+            args.resource_dir,
+            args.tun_provider,
             #[cfg(target_os = "windows")]
             setup_done_tx,
         )?;
@@ -230,7 +227,7 @@ impl WireguardMonitor {
         let event_callback = Box::new(on_event.clone());
         let (pinger_tx, pinger_rx) = sync_mpsc::channel();
         let monitor = WireguardMonitor {
-            runtime: runtime.clone(),
+            runtime: args.runtime.clone(),
             tunnel: Arc::new(Mutex::new(Some(tunnel))),
             event_callback,
             close_msg_receiver,
@@ -269,7 +266,7 @@ impl WireguardMonitor {
 
             // Add non-default routes before establishing the tunnel.
             #[cfg(target_os = "linux")]
-            route_manager
+            args.route_manager
                 .create_routing_rules(config.enable_ipv6)
                 .await
                 .map_err(Error::SetupRoutingError)
@@ -278,14 +275,15 @@ impl WireguardMonitor {
             let routes = Self::get_pre_tunnel_routes(&iface_name, &config)
                 .chain(Self::get_endpoint_routes(&endpoint_addrs))
                 .collect();
-            route_manager
+            args.route_manager
                 .add_routes(routes)
                 .await
                 .map_err(Error::SetupRoutingError)
                 .map_err(CloseMsg::SetupError)?;
 
             if let Some(pubkey) = psk_negotiation {
-                Self::perform_psk_negotiation(tunnel, retry_attempt, pubkey, &mut config).await?;
+                Self::perform_psk_negotiation(tunnel, args.retry_attempt, pubkey, &mut config)
+                    .await?;
                 (on_event)(TunnelEvent::InterfaceUp(
                     metadata.clone(),
                     AllowedTunnelTraffic::All,
@@ -294,7 +292,7 @@ impl WireguardMonitor {
             }
 
             let mut connectivity_monitor = tokio::task::spawn_blocking(move || {
-                match connectivity_monitor.establish_connectivity(retry_attempt) {
+                match connectivity_monitor.establish_connectivity(args.retry_attempt) {
                     Ok(true) => Ok(connectivity_monitor),
                     Ok(false) => {
                         log::warn!("Timeout while checking tunnel connection");
@@ -313,7 +311,7 @@ impl WireguardMonitor {
             .unwrap()?;
 
             // Add any default route(s) that may exist.
-            route_manager
+            args.route_manager
                 .add_routes(Self::get_post_tunnel_routes(&iface_name, &config).collect())
                 .await
                 .map_err(Error::SetupRoutingError)
@@ -343,7 +341,7 @@ impl WireguardMonitor {
         });
 
         tokio::spawn(async move {
-            if init_args.tunnel_close_rx.await.is_ok() {
+            if args.tunnel_close_rx.await.is_ok() {
                 monitor_handle.abort();
                 let _ = close_msg_sender.send(CloseMsg::Stop);
             }
@@ -354,7 +352,7 @@ impl WireguardMonitor {
 
     /// Replace `0.0.0.0/0`/`::/0` with the gateway IPs when `gateway_only` is true.
     /// Used to block traffic to other destinations while connecting on Android.
-    fn patch_allowed_ips<'a>(config: &'a Config, gateway_only: bool) -> Cow<'a, Config> {
+    fn patch_allowed_ips(config: &Config, gateway_only: bool) -> Cow<'_, Config> {
         if gateway_only {
             let mut patched_config = config.clone();
             let gateway_net_v4 = ipnetwork::IpNetwork::from(IpAddr::from(config.ipv4_gateway));
@@ -370,12 +368,10 @@ impl WireguardMonitor {
                         if allowed_ip.prefix() == 0 {
                             if allowed_ip.is_ipv4() {
                                 allowed_ip = gateway_net_v4;
+                            } else if let Some(net) = gateway_net_v6 {
+                                allowed_ip = net;
                             } else {
-                                if let Some(net) = gateway_net_v6 {
-                                    allowed_ip = net;
-                                } else {
-                                    return None;
-                                }
+                                return None;
                             }
                         }
                         Some(allowed_ip)
diff --git a/talpid-core/src/tunnel/wireguard/wireguard_kernel/wg_message.rs b/talpid-core/src/tunnel/wireguard/wireguard_kernel/wg_message.rs
index 7c767f25ed..03e754a525 100644
--- a/talpid-core/src/tunnel/wireguard/wireguard_kernel/wg_message.rs
+++ b/talpid-core/src/tunnel/wireguard/wireguard_kernel/wg_message.rs
@@ -88,7 +88,7 @@ impl DeviceMessage {
                 PeerNla::Flags(WGPEER_F_REPLACE_ALLOWEDIPS),
             ];
             if let Some(psk) = peer.psk.as_ref() {
-                peer_nlas.push(PeerNla::PresharedKey(psk.as_bytes().clone()));
+                peer_nlas.push(PeerNla::PresharedKey(*psk.as_bytes()));
             }
             peers.push(PeerMessage(peer_nlas));
         }
diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs
index e166f15986..5fc02d1052 100644
--- a/talpid-core/src/tunnel_state_machine/connected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connected_state.rs
@@ -123,7 +123,7 @@ impl ConnectedState {
         let dns_ips = self.get_dns_servers(shared_values);
 
         #[cfg(any(target_os = "linux", target_os = "windows"))]
-        let dns_ips = &dns_ips
+        let dns_ips = dns_ips
             .into_iter()
             .filter(|ip| {
                 !crate::firewall::is_local_address(ip)
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index a671f32558..5a83bd6b76 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -144,21 +144,17 @@ impl ConnectingState {
                 }
             };
 
-            let init_args = TunnelArgs {
+            let args = TunnelArgs {
+                runtime,
                 resource_dir: &resource_dir,
                 on_event: on_tunnel_event,
                 tunnel_close_rx,
-            };
-
-            let block_reason = match TunnelMonitor::start(
-                runtime,
-                &mut tunnel_parameters,
-                &log_dir,
                 tun_provider,
                 retry_attempt,
-                route_manager_handle,
-                init_args,
-            ) {
+                route_manager: route_manager_handle,
+            };
+
+            let block_reason = match TunnelMonitor::start(&mut tunnel_parameters, &log_dir, args) {
                 Ok(monitor) => {
                     let reason = Self::wait_for_tunnel_monitor(monitor, retry_attempt);
                     log::debug!("Tunnel monitor exited with block reason: {:?}", reason);
diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs
index 25fa469349..061798b1e2 100644
--- a/talpid-core/src/tunnel_state_machine/mod.rs
+++ b/talpid-core/src/tunnel_state_machine/mod.rs
@@ -305,7 +305,7 @@ impl TunnelStateMachine {
                 let _ = args.offline_state_tx.unbounded_send(offline);
             }
         });
-        let mut offline_monitor = offline::spawn_monitor(
+        let offline_monitor = offline::spawn_monitor(
             offline_tx,
             #[cfg(target_os = "linux")]
             route_manager
@@ -318,7 +318,7 @@ impl TunnelStateMachine {
         )
         .await
         .map_err(Error::OfflineMonitorError)?;
-        let is_offline = offline_monitor.is_offline().await;
+        let is_offline = offline_monitor.host_is_offline().await;
         let _ = initial_offline_state_tx.unbounded_send(is_offline);
 
         #[cfg(windows)]