chore: apply lints consistently across workspace (#4357)

Motivated by: #4340.

I also activated
[`clippy::unnnecessary_wraps`](https://rust-lang.github.io/rust-clippy/master/#/unnecessary_wraps)
which does create some false-positives for the platform-specific code
but is IMO overall a net-positive. With the amount of Rust code and
crates increasing, it is good to have tools point out simplifications
like these as they are otherwise hard to spot, especially across crate
boundaries.

rust/Cargo.toml

@@ -50,6 +50,12 @@ firezone-tunnel = { path = "connlib/tunnel"}
 phoenix-channel = { path = "phoenix-channel"}
 http-health-check = { path = "http-health-check"}
 
+[workspace.lints]
+clippy.dbg_macro = "warn"
+clippy.print_stdout = "warn"
+clippy.print_stderr = "warn"
+clippy.unnecessary_wraps = "warn"
+
 [patch.crates-io]
 boringtun = { git = "https://github.com/cloudflare/boringtun", branch = "master" }
 str0m = { git = "https://github.com/firezone/str0m", branch = "main" }

rust/client-tunnel/Cargo.toml

@@ -6,3 +6,6 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+
+[lints]
+workspace = true

rust/client-tunnel/src/lib.rs

@@ -8,6 +8,7 @@
 //! Tauri deb bundler to pick it up easily.
 //! Otherwise we would just make it a normal binary crate.
 
+#[allow(clippy::print_stdout)]
 pub fn run() {
     println!("Firezone Tunnel (library)");
 }

rust/clippy.toml

@@ -0,0 +1 @@
+avoid-breaking-exported-api = false # We don't publish anything to crates.io, hence we don't need to worry about breaking Rust API changes.

rust/connlib/clients/android/Cargo.toml

@@ -29,3 +29,6 @@ tokio = { version = "1.36", default-features = false, features = ["rt"] }
 
 [target.'cfg(target_os = "android")'.dependencies]
 tracing-android = "0.2"
+
+[lints]
+workspace = true

rust/connlib/clients/android/src/lib.rs

@@ -392,7 +392,7 @@ fn connect(
         callback_handler,
         Some(MAX_PARTITION_TIME),
         runtime.handle().clone(),
-    )?;
+    );
 
     Ok(SessionWrapper {
         inner: session,

rust/connlib/clients/apple/Cargo.toml

@@ -30,3 +30,6 @@ tokio = { version = "1.36", default-features = false, features = ["rt"] }
 name = "connlib"
 crate-type = ["staticlib"]
 doc = false
+
+[lints]
+workspace = true

rust/connlib/clients/apple/src/lib.rs

@@ -207,8 +207,7 @@ impl WrappedSession {
             },
             Some(MAX_PARTITION_TIME),
             runtime.handle().clone(),
-        )
-        .map_err(|err| err.to_string())?;
+        );
 
         Ok(Self {
             inner: session,

rust/connlib/clients/shared/Cargo.toml

@@ -39,3 +39,6 @@ tracing-android = "0.2"
 [dev-dependencies]
 serde_json = { version = "1.0", features = ["std"] }
 chrono = { workspace = true }
+
+[lints]
+workspace = true

rust/connlib/clients/shared/examples/on_demand_rolling.rs

@@ -5,6 +5,7 @@ use tracing_subscriber::layer::SubscriberExt;
 use tracing_subscriber::util::SubscriberInitExt;
 use tracing_subscriber::{EnvFilter, Layer};
 
+#[allow(clippy::print_stdout)]
 fn main() {
     let log_dir = Path::new("./target");
 

rust/connlib/clients/shared/src/lib.rs

@@ -44,7 +44,7 @@ impl Session {
         callbacks: CB,
         max_partition_time: Option<Duration>,
         handle: tokio::runtime::Handle,
-    ) -> connlib_shared::Result<Self> {
+    ) -> Self {
         let (tx, rx) = tokio::sync::mpsc::unbounded_channel();
 
         let connect_handle = handle.spawn(connect(
@@ -58,7 +58,7 @@ impl Session {
         ));
         handle.spawn(connect_supervisor(connect_handle, callbacks));
 
-        Ok(Self { channel: tx })
+        Self { channel: tx }
     }
 
     /// Attempts to reconnect a [`Session`].

rust/connlib/shared/Cargo.toml

@@ -72,3 +72,6 @@ version = "0.54.0"
 features = [
   "Win32_Foundation",
 ]
+
+[lints]
+workspace = true

rust/connlib/shared/src/device_id.rs

@@ -78,6 +78,7 @@ mod imp {
     /// `BUNDLE_ID` because we need our own subdir
     ///
     /// `config` to make how Windows has `config` and `data` both under `AppData/Local/$BUNDLE_ID`
+    #[allow(clippy::unnecessary_wraps)] // Needs to be aligned with `cfg(windows)` variant below.
     pub(crate) fn path() -> Option<PathBuf> {
         Some(
             PathBuf::from("/var/lib")

rust/connlib/shared/src/linux/etc_resolv_conf.rs

@@ -309,7 +309,6 @@ nameserver 100.100.111.2
     #[tokio::test]
     async fn no_sentinels() -> Result<()> {
         let (_temp_dir, paths) = create_temp_paths();
-        println!("{}", paths.resolv.display());
 
         write_resolv_conf(&paths.resolv, &[GOOGLE_DNS.into()])?;
 

rust/connlib/snownet/Cargo.toml

@@ -21,3 +21,6 @@ once_cell = "1.17.1"
 backoff = "0.4.0"
 hex = "0.4.0"
 tracing-subscriber = { workspace = true }
+
+[lints]
+workspace = true

rust/connlib/tunnel/Cargo.toml

@@ -63,3 +63,6 @@ features = [
   "Win32_NetworkManagement_Ndis",
   "Win32_Networking_WinSock",
 ]
+
+[lints]
+workspace = true

rust/connlib/tunnel/src/device_channel/tun_darwin.rs

@@ -138,6 +138,7 @@ impl Tun {
         Err(get_last_error())
     }
 
+    #[allow(clippy::unnecessary_wraps)] // fn signature needs to align with other platforms.
     pub fn set_routes(&self, routes: HashSet<IpNetwork>, callbacks: &impl Callbacks) -> Result<()> {
         // This will always be None in macos
         callbacks.on_update_routes(

rust/connlib/tunnel/src/device_channel/tun_linux.rs

@@ -155,6 +155,7 @@ impl Tun {
         })
     }
 
+    #[allow(clippy::unnecessary_wraps)] // fn signature needs to align with other platforms.
     pub fn set_routes(&mut self, new_routes: HashSet<IpNetwork>, _: &impl Callbacks) -> Result<()> {
         if new_routes == self.routes {
             return Ok(());

rust/connlib/tunnel/src/device_channel/tun_windows.rs

@@ -243,6 +243,7 @@ impl Tun {
         self.write(bytes)
     }
 
+    #[allow(clippy::unnecessary_wraps)] // Fn signature must align with other platform implementations.
     fn write(&self, bytes: &[u8]) -> io::Result<usize> {
         let len = bytes
             .len()

rust/connlib/tunnel/src/gateway.rs

@@ -89,7 +89,7 @@ where
             Instant::now(),
         );
 
-        self.new_peer(ips, client_id, id, expires_at, resource_addresses.clone())?;
+        self.new_peer(ips, client_id, id, expires_at, resource_addresses.clone());
 
         Ok(ConnectionAccepted {
             ice_parameters: Answer {
@@ -178,7 +178,7 @@ where
         resource: ResourceId,
         expires_at: Option<DateTime<Utc>>,
         resource_addresses: Vec<IpNetwork>,
-    ) -> Result<()> {
+    ) {
         let mut peer = Peer::new(client_id, PacketTransformGateway::default(), &ips, ());
 
         for address in resource_addresses {
@@ -186,8 +186,6 @@ where
         }
 
         self.role_state.peers.insert(peer, &ips);
-
-        Ok(())
     }
 }
 

rust/firezone-cli-utils/Cargo.toml

@@ -13,3 +13,6 @@ tracing-subscriber = { workspace = true, features = ["env-filter"] }
 tracing = { workspace = true }
 tracing-log = "0.2"
 clap = { version = "4.5", features = ["derive",  "env"] }
+
+[lints]
+workspace = true

rust/gateway/Cargo.toml

@@ -36,3 +36,6 @@ http-health-check = { workspace = true }
 
 [dev-dependencies]
 serde_json = { version = "1.0", default-features = false, features = ["std"] }
+
+[lints]
+workspace = true

rust/gui-client/src-tauri/Cargo.toml

@@ -101,3 +101,6 @@ features = [
 # TODO: We can probably remove this, per <https://github.com/tauri-apps/tauri/releases/tag/tauri-v2.0.0-beta.8>
 # I don't know how to verify this change, so I won't do it right now.
 custom-protocol = ["tauri/custom-protocol"]
+
+[lints]
+workspace = true

rust/gui-client/src-tauri/src/client/auth.rs

@@ -278,7 +278,7 @@ mod tests {
     /// Runs everything in one test so that `cargo test` can't multi-thread it
     /// This should work around a bug we had <https://github.com/firezone/firezone/issues/3256>
     #[test]
-    fn everything() -> anyhow::Result<()> {
+    fn everything() {
         // Run `happy_path` first to make sure it reacts okay if our `data` dir is missing
         // TODO: Re-enable happy path tests once `keyring-rs` is working in CI tests
         // happy_path("");
@@ -286,7 +286,6 @@ mod tests {
         utils();
         no_inflight_request();
         states_dont_match();
-        Ok(())
     }
 
     fn utils() {
@@ -301,7 +300,6 @@ mod tests {
         let hex_string = generate_nonce();
         let hex_string = hex_string.expose_secret();
         assert_eq!(hex_string.len(), NONCE_LENGTH * 2);
-        dbg!(hex_string);
 
         let auth_base_url = Url::parse("https://app.firez.one").unwrap();
         let req = Request {

rust/gui-client/src-tauri/src/client/auth/token_storage_mock.rs

@@ -10,6 +10,7 @@ pub(crate) struct TokenStorage {
     token: Option<SecretString>,
 }
 
+#[allow(clippy::unnecessary_wraps)] // Must align with other cfg'd implementation.
 impl TokenStorage {
     pub(crate) fn new(_key: &'static str) -> Self {
         Self { token: None }

rust/gui-client/src-tauri/src/client/crash_handling.rs

@@ -118,6 +118,7 @@ struct Handler;
 impl minidumper::ServerHandler for Handler {
     /// Called when a crash has been received and a backing file needs to be
     /// created to store it.
+    #[allow(clippy::print_stderr)]
     fn create_minidump_file(&self) -> Result<(File, PathBuf), std::io::Error> {
         let dump_path = known_dirs::logs()
             .expect("Should be able to find logs dir to put crash dump in")

rust/gui-client/src-tauri/src/client/debug_commands.rs

@@ -15,12 +15,14 @@ pub enum Cmd {
 
 pub fn run(cmd: Cmd) -> Result<()> {
     match cmd {
-        Cmd::CheckForUpdates => check_for_updates(),
-        Cmd::Crash => crash(),
-        Cmd::DnsChanges => client::network_changes::run_dns_debug(),
+        Cmd::CheckForUpdates => check_for_updates()?,
+        Cmd::Crash => crash()?,
+        Cmd::DnsChanges => client::network_changes::run_dns_debug()?,
         Cmd::Hostname => hostname(),
-        Cmd::NetworkChanges => client::network_changes::run_debug(),
-    }
+        Cmd::NetworkChanges => client::network_changes::run_debug()?,
+    };
+
+    Ok(())
 }
 
 fn check_for_updates() -> Result<()> {
@@ -41,10 +43,10 @@ fn crash() -> Result<()> {
     panic!("purposely crashing to see if it shows up in logs");
 }
 
-fn hostname() -> Result<()> {
+#[allow(clippy::print_stdout)]
+fn hostname() {
     println!(
         "{:?}",
         hostname::get().ok().and_then(|x| x.into_string().ok())
     );
-    Ok(())
 }

rust/gui-client/src-tauri/src/client/elevation.rs

@@ -4,6 +4,7 @@ pub(crate) use imp::{check, elevate};
 mod imp {
     use anyhow::{Context, Result};
 
+    #[allow(clippy::print_stderr)]
     pub(crate) fn check() -> Result<bool> {
         // Must use `eprintln` here because `tracing` won't be initialized yet.
 

rust/gui-client/src-tauri/src/client/gui.rs

@@ -25,15 +25,18 @@ mod system_tray_menu;
 
 #[cfg(target_os = "linux")]
 #[path = "gui/os_linux.rs"]
+#[allow(clippy::unnecessary_wraps)]
 mod os;
 
 // Stub only
 #[cfg(target_os = "macos")]
 #[path = "gui/os_macos.rs"]
+#[allow(clippy::unnecessary_wraps)]
 mod os;
 
 #[cfg(target_os = "windows")]
 #[path = "gui/os_windows.rs"]
+#[allow(clippy::unnecessary_wraps)]
 mod os;
 
 /// The Windows client doesn't use platform APIs to detect network connectivity changes,
@@ -542,7 +545,7 @@ impl Controller {
             callback_handler.clone(),
             Some(MAX_PARTITION_TIME),
             tokio::runtime::Handle::current(),
-        )?;
+        );
 
         connlib.set_dns(client::resolvers::get().unwrap_or_default());
 

rust/gui-client/src-tauri/src/client/network_changes.rs

@@ -1,13 +1,16 @@
 #[cfg(target_os = "linux")]
 #[path = "network_changes/linux.rs"]
+#[allow(clippy::unnecessary_wraps)]
 mod imp;
 
 #[cfg(target_os = "macos")]
 #[path = "network_changes/macos.rs"]
+#[allow(clippy::unnecessary_wraps)]
 mod imp;
 
 #[cfg(target_os = "windows")]
 #[path = "network_changes/windows.rs"]
+#[allow(clippy::unnecessary_wraps)]
 mod imp;
 
 pub(crate) use imp::{check_internet, run_debug, run_dns_debug, DnsListener, Worker};

rust/gui-client/src-tauri/src/client/resolvers.rs

@@ -9,6 +9,7 @@ mod imp {
 
     // TODO: The code here will depend on the chosen DNS control method.
     // So that will need to be threaded in here somehow.
+    #[allow(clippy::unnecessary_wraps)]
     pub fn get() -> Result<Vec<IpAddr>> {
         tracing::error!("Resolvers module not yet implemented for Linux, returning empty Vec");
         Ok(Vec::default())

rust/gui-client/src-tauri/src/client/wintun_install.rs

@@ -25,8 +25,6 @@ pub(crate) enum Error {
     DllPathInvalid,
     #[error("permission denied")]
     PermissionDenied,
-    #[error("platform not supported")]
-    PlatformNotSupported,
     #[error("write failed: `{0:?}`")]
     WriteFailed(io::Error),
 }
@@ -36,7 +34,7 @@ pub(crate) enum Error {
 /// e.g. `C:\Users\User\AppData\Local\dev.firezone.client\data\wintun.dll`
 /// Also verifies the SHA256 of the DLL on-disk with the expected bytes packed into the exe
 pub(crate) fn ensure_dll() -> Result<PathBuf, Error> {
-    let dll_bytes = get_dll_bytes().ok_or(Error::PlatformNotSupported)?;
+    let dll_bytes = get_dll_bytes();
 
     let path = wintun_dll_path().map_err(|_| Error::CantComputeWintunPath)?;
     // The DLL path should always have a parent
@@ -80,23 +78,18 @@ fn dll_already_exists(path: &Path, dll_bytes: &DllBytes) -> bool {
     expected == actual.as_ref()
 }
 
-/// Returns the platform-specific bytes of wintun.dll, or None if we don't support the compiled platform.
-fn get_dll_bytes() -> Option<DllBytes> {
-    get_platform_dll_bytes()
-}
-
 #[cfg(target_arch = "x86_64")]
-fn get_platform_dll_bytes() -> Option<DllBytes> {
-    Some(DllBytes {
+fn get_dll_bytes() -> DllBytes {
+    DllBytes {
         bytes: include_bytes!("../../../wintun/bin/amd64/wintun.dll"),
         expected_sha256: "e5da8447dc2c320edc0fc52fa01885c103de8c118481f683643cacc3220dafce",
-    })
+    }
 }
 
 #[cfg(target_arch = "aarch64")]
-fn get_platform_dll_bytes() -> Option<DllBytes> {
-    Some(DllBytes {
+fn get_dll_bytes() -> DllBytes {
+    DllBytes {
         bytes: include_bytes!("../../../wintun/bin/arm64/wintun.dll"),
         expected_sha256: "f7ba89005544be9d85231a9e0d5f23b2d15b3311667e2dad0debd344918a3f80",
-    })
+    }
 }

rust/http-health-check/Cargo.toml

@@ -10,3 +10,6 @@ edition = "2021"
 axum = { version = "0.7.5", default-features = false, features = ["http1", "tokio"] }
 tokio = { version = "1.36.0", features = ["net"] }
 clap = { version = "4.5.3", features = ["derive", "env"] }
+
+[lints]
+workspace = true

rust/http-test-server/Cargo.toml

@@ -12,3 +12,6 @@ axum = { version = "0.7.5", features = ["http1", "tokio"] }
 tokio = { version = "1.36.0", features = ["net"] }
 serde = {version = "1", features = ["derive"]}
 futures = "0.3"
+
+[lints]
+workspace = true

rust/linux-client/Cargo.toml

@@ -22,3 +22,6 @@ resolv-conf = "0.7.0"
 thiserror = "1.0.57"
 tokio = { version = "1.36", default-features = false, features = ["rt", "macros", "signal"] }
 url = { version = "2.3.1", default-features = false }
+
+[lints]
+workspace = true

rust/linux-client/src/main.rs

@@ -56,8 +56,7 @@ async fn main() -> Result<()> {
         callbacks.clone(),
         max_partition_time,
         tokio::runtime::Handle::current(),
-    )
-    .unwrap();
+    );
     // TODO: this should be added dynamically
     session.set_dns(system_resolvers(get_dns_control_from_env()).unwrap_or_default());
 
@@ -136,6 +135,7 @@ fn get_system_default_resolvers_resolv_conf() -> Result<Vec<IpAddr>> {
     Ok(nameservers)
 }
 
+#[allow(clippy::unnecessary_wraps)]
 fn get_system_default_resolvers_network_manager() -> Result<Vec<IpAddr>> {
     tracing::error!("get_system_default_resolvers_network_manager not implemented yet");
     Ok(vec![])

rust/phoenix-channel/Cargo.toml

@@ -28,3 +28,6 @@ hostname = "0.3.1" # Hickory already depends on `hostname` so this isn't new
 
 [dev-dependencies]
 tokio = { version = "1.36.0", features = ["macros", "rt"] }
+
+[lints]
+workspace = true

rust/phoenix-channel/src/lib.rs

@@ -122,7 +122,7 @@ pub struct UnexpectedEventDuringInit(String);
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("client error: {0}")]
-    ClientError(StatusCode),
+    Client(StatusCode),
     #[error("token expired")]
     TokenExpired,
     #[error("max retries reached")]
@@ -132,7 +132,7 @@ pub enum Error {
 impl Error {
     pub fn is_authentication_error(&self) -> bool {
         match self {
-            Error::ClientError(s) => s == &StatusCode::UNAUTHORIZED || s == &StatusCode::FORBIDDEN,
+            Error::Client(s) => s == &StatusCode::UNAUTHORIZED || s == &StatusCode::FORBIDDEN,
             Error::TokenExpired => true,
             Error::MaxRetriesReached => false,
         }
@@ -301,7 +301,7 @@ where
                     Poll::Ready(Err(InternalError::WebSocket(
                         tokio_tungstenite::tungstenite::Error::Http(r),
                     ))) if r.status().is_client_error() => {
-                        return Poll::Ready(Err(Error::ClientError(r.status())));
+                        return Poll::Ready(Err(Error::Client(r.status())));
                     }
                     Poll::Ready(Err(e)) => {
                         let Some(backoff) = self.reconnect_backoff.next_backoff() else {

rust/relay/Cargo.toml

@@ -47,3 +47,6 @@ difference = "2.0.0"
 [[test]]
 name = "regression"
 required-features = ["proptest"]
+
+[lints]
+workspace = true

rust/relay/src/main.rs

@@ -130,7 +130,7 @@ async fn main() -> Result<()> {
         None
     };
 
-    let mut eventloop = Eventloop::new(server, channel, public_addr)?;
+    let mut eventloop = Eventloop::new(server, channel, public_addr);
 
     tokio::spawn(http_health_check::serve(
         args.health_check.health_check_addr,
@@ -329,7 +329,7 @@ where
         server: Server<R>,
         channel: Option<PhoenixChannel<JoinMessage, (), ()>>,
         public_address: IpStack,
-    ) -> Result<Self> {
+    ) -> Self {
         let (relay_data_sender, relay_data_receiver) = mpsc::channel(1);
         let (inbound_data_sender, inbound_data_receiver) = mpsc::channel(1000);
         let (outbound_ip4_data_sender, outbound_ip4_data_receiver) = mpsc::channel(1000);
@@ -350,7 +350,7 @@ where
             ));
         }
 
-        Ok(Self {
+        Self {
             inbound_data_receiver,
             outbound_ip4_data_sender,
             outbound_ip6_data_sender,
@@ -362,7 +362,7 @@ where
             sleep: Sleep::default(),
             stats_log_interval: tokio::time::interval(STATS_LOG_INTERVAL),
             last_num_bytes_relayed: 0,
-        })
+        }
     }
 
     fn poll(&mut self, cx: &mut std::task::Context<'_>) -> Poll<Result<()>> {

rust/snownet-tests/Cargo.toml

@@ -22,3 +22,6 @@ tokio = { version = "1", features = ["full"] }
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter", "json"] }
 system-info = { version = "0.1.2", features = ["std"]}
+
+[lints]
+workspace = true