Merge branch 'cli-fix-constraints-indentation-des-291' into main

7 files changed, 245 insertions, 168 deletions

diff --git a/mullvad-cli/src/cmds/bridge.rs b/mullvad-cli/src/cmds/bridge.rs
index 3cc1a95b37..2ee7d9a4f0 100644
--- a/mullvad-cli/src/cmds/bridge.rs
+++ b/mullvad-cli/src/cmds/bridge.rs
@@ -3,8 +3,8 @@ use clap::Subcommand;
 use mullvad_management_interface::MullvadProxyClient;
 use mullvad_types::{
     relay_constraints::{
-        BridgeConstraints, BridgeSettings, BridgeState, Constraint, LocationConstraint, Ownership,
-        Provider, Providers,
+        BridgeConstraints, BridgeConstraintsFormatter, BridgeSettings, BridgeState, Constraint,
+        LocationConstraint, Ownership, Provider, Providers,
     },
     relay_list::RelayEndpointData,
 };
@@ -279,10 +279,14 @@ impl Bridge {
                     Self::print_shadowsocks_proxy(&shadowsocks_proxy)
                 }
             },
-            BridgeSettings::Normal(constraints) => {
-                let mut buf = String::new();
-                let _ = constraints.format(&mut buf, &settings.custom_lists);
-                println!("Bridge constraints: {buf}")
+            BridgeSettings::Normal(ref constraints) => {
+                println!(
+                    "Bridge constraints: {}",
+                    BridgeConstraintsFormatter {
+                        constraints,
+                        custom_lists: &settings.custom_lists
+                    }
+                )
             }
         };
         Ok(())
diff --git a/mullvad-cli/src/cmds/relay.rs b/mullvad-cli/src/cmds/relay.rs
index 83f4ad4fdd..290c2cacd0 100644
--- a/mullvad-cli/src/cmds/relay.rs
+++ b/mullvad-cli/src/cmds/relay.rs
@@ -5,9 +5,9 @@ use mullvad_management_interface::MullvadProxyClient;
 use mullvad_types::{
     location::Location,
     relay_constraints::{
-        Constraint, GeographicLocationConstraint, LocationConstraint, Match, OpenVpnConstraints,
-        Ownership, Provider, Providers, RelayConstraintsUpdate, RelaySettings, RelaySettingsUpdate,
-        TransportPort, WireguardConstraints,
+        Constraint, GeographicLocationConstraint, LocationConstraint, LocationConstraintFormatter,
+        Match, OpenVpnConstraints, Ownership, Provider, Providers, RelayConstraintsUpdate,
+        RelaySettings, RelaySettingsUpdate, TransportPort, WireguardConstraints,
     },
     relay_list::{RelayEndpointData, RelayListCountry},
     ConnectionConfig, CustomTunnelEndpoint,
@@ -21,6 +21,7 @@ use talpid_types::net::{
 };
 
 use super::{relay_constraints::LocationArgs, BooleanOption};
+use crate::print_option;
 
 #[derive(Subcommand, Debug)]
 pub enum Relay {
@@ -214,9 +215,72 @@ impl Relay {
         let mut rpc = MullvadProxyClient::new().await?;
         let settings = rpc.get_settings().await?;
         let relay_settings = settings.relay_settings;
-        let mut buf = String::new();
-        let _ = relay_settings.format(&mut buf, &settings.custom_lists);
-        println!("Current constraints: \n{}", buf);
+
+        match relay_settings {
+            RelaySettings::CustomTunnelEndpoint(endpoint) => {
+                println!("Custom endpoint: {endpoint}")
+            }
+
+            RelaySettings::Normal(constraints) => {
+                println!("Generic constraints");
+
+                print_option!(
+                    "Location",
+                    constraints
+                        .location
+                        .as_ref()
+                        .map(|location| LocationConstraintFormatter {
+                            constraint: location,
+                            custom_lists: &settings.custom_lists
+                        }),
+                );
+
+                print_option!("Tunnel protocol", constraints.tunnel_protocol,);
+
+                print_option!("Provider(s)", constraints.providers,);
+                print_option!("Ownership", constraints.ownership,);
+
+                println!("OpenVPN constraints");
+
+                match constraints.openvpn_constraints.port {
+                    Constraint::Any => {
+                        print_option!("Port", "any",);
+                        print_option!("Transport", "any",);
+                    }
+                    Constraint::Only(transport_port) => {
+                        print_option!("Port", transport_port.port,);
+                        print_option!("Transport", transport_port.protocol,);
+                    }
+                }
+
+                println!("WireGuard constraints");
+
+                print_option!("Port", constraints.wireguard_constraints.port,);
+
+                print_option!("IP protocol", constraints.wireguard_constraints.ip_version,);
+
+                print_option!(
+                    "Multihop state",
+                    if constraints.wireguard_constraints.use_multihop {
+                        "enabled"
+                    } else {
+                        "disabled"
+                    },
+                );
+                print_option!(
+                    "Multihop entry",
+                    constraints
+                        .wireguard_constraints
+                        .entry_location
+                        .as_ref()
+                        .map(|location| LocationConstraintFormatter {
+                            constraint: location,
+                            custom_lists: &settings.custom_lists
+                        }),
+                );
+            }
+        }
+
         Ok(())
     }
 
diff --git a/mullvad-cli/src/cmds/tunnel.rs b/mullvad-cli/src/cmds/tunnel.rs
index 120fad327d..92e1c89d5c 100644
--- a/mullvad-cli/src/cmds/tunnel.rs
+++ b/mullvad-cli/src/cmds/tunnel.rs
@@ -7,6 +7,7 @@ use mullvad_types::{
 };
 
 use super::BooleanOption;
+use crate::print_option;
 
 #[derive(Subcommand, Debug)]
 pub enum Tunnel {
@@ -70,10 +71,8 @@ impl Tunnel {
 
         println!("OpenVPN options");
 
-        println!(
-            "{:<4}{:<24}{}",
-            "",
-            "mssfix:",
+        print_option!(
+            "mssfix",
             tunnel_options
                 .openvpn
                 .mssfix
@@ -83,33 +82,27 @@ impl Tunnel {
 
         println!("WireGuard options");
 
-        println!(
-            "{:<4}{:<24}{}",
-            "",
-            "MTU:",
+        print_option!(
+            "MTU",
             tunnel_options
                 .wireguard
                 .mtu
                 .map(|val| val.to_string())
                 .unwrap_or("unset".to_string()),
         );
-        println!(
-            "{:<4}{:<24}{}",
-            "", "Quantum resistance:", tunnel_options.wireguard.quantum_resistant,
+        print_option!(
+            "Quantum resistance",
+            tunnel_options.wireguard.quantum_resistant,
         );
 
         let key = rpc.get_wireguard_key().await?;
-        println!("{:<4}{:<24}{}", "", "Public key:", key.key,);
-        println!(
-            "{:<4}{:<24}{}",
-            "",
-            "",
-            format_args!("Created {}", key.created.with_timezone(&chrono::Local)),
-        );
-        println!(
-            "{:<4}{:<24}{}",
-            "",
-            "Rotation interval:",
+        print_option!("Public key", key.key,);
+        print_option!(format_args!(
+            "Created {}",
+            key.created.with_timezone(&chrono::Local)
+        ),);
+        print_option!(
+            "Rotation interval",
             match tunnel_options.wireguard.rotation_interval {
                 Some(interval) => interval.to_string(),
                 None => "unset".to_string(),
@@ -118,11 +111,14 @@ impl Tunnel {
 
         println!("Generic options");
 
-        if tunnel_options.generic.enable_ipv6 {
-            println!("{:<4}{:<24}on", "", "IPv6:");
-        } else {
-            println!("{:<4}{:<24}off", "", "IPv6:");
-        }
+        print_option!(
+            "IPv6",
+            if tunnel_options.generic.enable_ipv6 {
+                "on"
+            } else {
+                "off"
+            }
+        );
 
         Ok(())
     }
diff --git a/mullvad-cli/src/format.rs b/mullvad-cli/src/format.rs
index 0d55c53173..2938842cef 100644
--- a/mullvad-cli/src/format.rs
+++ b/mullvad-cli/src/format.rs
@@ -4,6 +4,16 @@ use talpid_types::{
     tunnel::ErrorState,
 };
 
+#[macro_export]
+macro_rules! print_option {
+    ($value:expr $(,)?) => {{
+        println!("{:<4}{:<24}{}", "", "", $value,)
+    }};
+    ($option:expr, $value:expr $(,)?) => {{
+        println!("{:<4}{:<24}{}", "", concat!($option, ":"), $value,)
+    }};
+}
+
 pub fn print_state(state: &TunnelState, verbose: bool) {
     use TunnelState::*;
 
diff --git a/mullvad-relay-selector/src/lib.rs b/mullvad-relay-selector/src/lib.rs
index 843797c853..4baee72d04 100644
--- a/mullvad-relay-selector/src/lib.rs
+++ b/mullvad-relay-selector/src/lib.rs
@@ -10,8 +10,8 @@ use mullvad_types::{
     relay_constraints::{
         BridgeSettings, BridgeState, Constraint, InternalBridgeConstraints, LocationConstraint,
         Match, ObfuscationSettings, OpenVpnConstraints, Ownership, Providers, RelayConstraints,
-        RelaySettings, ResolvedLocationConstraint, SelectedObfuscation, Set, TransportPort,
-        Udp2TcpObfuscationSettings,
+        RelayConstraintsFormatter, RelaySettings, ResolvedLocationConstraint, SelectedObfuscation,
+        Set, TransportPort, Udp2TcpObfuscationSettings,
     },
     relay_list::{BridgeEndpointData, Relay, RelayEndpointData, RelayList},
     CustomTunnelEndpoint,
@@ -651,9 +651,13 @@ impl RelaySelector {
             );
             Ok(result)
         } else {
-            let mut relay_constraints_string = String::new();
-            let _ = relay_constraints.format(&mut relay_constraints_string, custom_lists);
-            log::warn!("No relays matching {}", &relay_constraints_string);
+            log::warn!(
+                "No relays matching constraints: {}",
+                RelayConstraintsFormatter {
+                    constraints: relay_constraints,
+                    custom_lists,
+                }
+            );
             Err(Error::NoRelay)
         }
     }
diff --git a/mullvad-types/src/relay_constraints.rs b/mullvad-types/src/relay_constraints.rs
index ab78ceca70..dcce9fc950 100644
--- a/mullvad-types/src/relay_constraints.rs
+++ b/mullvad-types/src/relay_constraints.rs
@@ -10,7 +10,7 @@ use crate::{
 #[cfg(target_os = "android")]
 use jnix::{jni::objects::JObject, FromJava, IntoJava, JnixEnv};
 use serde::{Deserialize, Serialize};
-use std::{collections::HashSet, fmt, fmt::Write, str::FromStr};
+use std::{collections::HashSet, fmt, str::FromStr};
 use talpid_types::net::{openvpn::ProxySettings, IpVersion, TransportProtocol, TunnelType};
 
 pub trait Match<T> {
@@ -205,21 +205,6 @@ pub enum RelaySettings {
 }
 
 impl RelaySettings {
-    pub fn format(
-        &self,
-        s: &mut String,
-        custom_lists: &CustomListsSettings,
-    ) -> Result<(), fmt::Error> {
-        match self {
-            RelaySettings::CustomTunnelEndpoint(endpoint) => {
-                write!(s, "custom endpoint {endpoint}")
-            }
-            RelaySettings::Normal(constraints) => constraints.format(s, custom_lists),
-        }
-    }
-}
-
-impl RelaySettings {
     pub fn merge(&self, update: RelaySettingsUpdate) -> Self {
         match update {
             RelaySettingsUpdate::CustomTunnelEndpoint(relay) => {
@@ -235,6 +220,31 @@ impl RelaySettings {
     }
 }
 
+pub struct RelaySettingsFormatter<'a> {
+    pub settings: &'a RelaySettings,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for RelaySettingsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.settings {
+            RelaySettings::CustomTunnelEndpoint(endpoint) => {
+                write!(f, "custom endpoint {endpoint}")
+            }
+            RelaySettings::Normal(constraints) => {
+                write!(
+                    f,
+                    "{}",
+                    RelayConstraintsFormatter {
+                        constraints,
+                        custom_lists: self.custom_lists
+                    }
+                )
+            }
+        }
+    }
+}
+
 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "snake_case")]
 #[cfg_attr(target_os = "android", derive(IntoJava, FromJava))]
@@ -326,26 +336,22 @@ impl Constraint<ResolvedLocationConstraint> {
     }
 }
 
-impl LocationConstraint {
-    fn format(&self, f: &mut String, custom_lists: &CustomListsSettings) -> Result<(), fmt::Error> {
-        match self {
-            Self::Location(location) => writeln!(f, "location - {location}"),
-            Self::CustomList { list_id } => match custom_lists
+pub struct LocationConstraintFormatter<'a> {
+    pub constraint: &'a LocationConstraint,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for LocationConstraintFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.constraint {
+            LocationConstraint::Location(location) => write!(f, "{}", location),
+            LocationConstraint::CustomList { list_id } => self
+                .custom_lists
                 .custom_lists
                 .iter()
                 .find(|custom_list| &custom_list.id == list_id)
-            {
-                Some(list) => {
-                    writeln!(f, "custom list - {}", list.name)?;
-                    for location in &list.locations {
-                        writeln!(f, "\t{}", location)?;
-                    }
-                    Ok(())
-                }
-                None => {
-                    writeln!(f, "custom list - list not found")
-                }
-            },
+                .map(|custom_list| write!(f, "{}", custom_list.name))
+                .unwrap_or_else(|| write!(f, "invalid custom list")),
         }
     }
 }
@@ -400,51 +406,36 @@ impl RelayConstraints {
     }
 }
 
-impl RelayConstraints {
-    pub fn format(
-        &self,
-        f: &mut String,
-        custom_lists: &CustomListsSettings,
-    ) -> Result<(), fmt::Error> {
-        match self.tunnel_protocol {
-            Constraint::Any => {
-                writeln!(
-                    f,
-                    "Tunnel protocol: Any\nOpenVPN constraints: {}\nWireguard constraints: ",
-                    &self.openvpn_constraints,
-                )?;
-                self.wireguard_constraints.format(f, custom_lists)?;
-            }
-            Constraint::Only(ref tunnel_protocol) => {
-                writeln!(f, "Tunnel protocol: {}", tunnel_protocol)?;
-                match tunnel_protocol {
-                    TunnelType::Wireguard => {
-                        writeln!(f, "Wireguard constraints: ")?;
-                        self.wireguard_constraints.format(f, custom_lists)?;
-                    }
-                    TunnelType::OpenVpn => {
-                        writeln!(f, "OpenVPN constraints: {}", &self.openvpn_constraints)?;
-                    }
-                };
-            }
-        }
-        match self.location {
-            Constraint::Any => writeln!(f, "Location: Any")?,
-            Constraint::Only(ref location_constraint) => {
-                write!(f, "Location: ")?;
-                location_constraint.format(f, custom_lists)?;
-            }
-        }
-        match self.providers {
-            Constraint::Any => writeln!(f, "Provider: Any")?,
-            Constraint::Only(ref constraint) => writeln!(f, "Provider: {}", constraint)?,
-        }
-        match self.ownership {
-            Constraint::Any => Ok(()),
-            Constraint::Only(ref constraint) => {
-                write!(f, "Constraints: {constraint}")
-            }
-        }
+pub struct RelayConstraintsFormatter<'a> {
+    pub constraints: &'a RelayConstraints,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for RelayConstraintsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        writeln!(
+            f,
+            "Tunnel protocol: {}\nOpenVPN constraints: {}\nWireguard constraints: {}",
+            self.constraints.tunnel_protocol,
+            self.constraints.openvpn_constraints,
+            WireguardConstraintsFormatter {
+                constraints: &self.constraints.wireguard_constraints,
+                custom_lists: self.custom_lists,
+            },
+        )?;
+        writeln!(
+            f,
+            "Location: {}",
+            self.constraints
+                .location
+                .as_ref()
+                .map(|location| LocationConstraintFormatter {
+                    constraint: location,
+                    custom_lists: self.custom_lists,
+                })
+        )?;
+        writeln!(f, "Provider(s): {}", self.constraints.providers)?;
+        write!(f, "Ownership: {}", self.constraints.ownership)
     }
 }
 
@@ -721,6 +712,33 @@ pub struct WireguardConstraints {
     pub entry_location: Constraint<LocationConstraint>,
 }
 
+pub struct WireguardConstraintsFormatter<'a> {
+    pub constraints: &'a WireguardConstraints,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for WireguardConstraintsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.constraints.port {
+            Constraint::Any => write!(f, "any port")?,
+            Constraint::Only(port) => write!(f, "port {}", port)?,
+        }
+        if let Constraint::Only(ip_version) = self.constraints.ip_version {
+            write!(f, ", {},", ip_version)?;
+        }
+        if self.constraints.use_multihop {
+            let location = self.constraints.entry_location.as_ref().map(|location| {
+                LocationConstraintFormatter {
+                    constraint: location,
+                    custom_lists: self.custom_lists,
+                }
+            });
+            write!(f, ", multihop entry {}", location)?;
+        }
+        Ok(())
+    }
+}
+
 #[cfg(target_os = "android")]
 impl<'env, 'sub_env> FromJava<'env, JObject<'sub_env>> for WireguardConstraints
 where
@@ -749,30 +767,6 @@ where
     }
 }
 
-impl WireguardConstraints {
-    fn format(&self, f: &mut String, custom_lists: &CustomListsSettings) -> Result<(), fmt::Error> {
-        match self.port {
-            Constraint::Any => writeln!(f, "Port: Any")?,
-            Constraint::Only(port) => writeln!(f, "port {port}")?,
-        }
-        match self.ip_version {
-            Constraint::Any => writeln!(f, "Protocol: IPv4 or IPv6")?,
-            Constraint::Only(protocol) => writeln!(f, "Protocol: {protocol}")?,
-        }
-        if self.use_multihop {
-            match &self.entry_location {
-                Constraint::Any => writeln!(f, "Entry location: Any"),
-                Constraint::Only(location) => {
-                    write!(f, "Wireguard entry ")?;
-                    location.format(f, custom_lists)
-                }
-            }
-        } else {
-            Ok(())
-        }
-    }
-}
-
 /// Used for jni conversion.
 #[cfg(target_os = "android")]
 #[derive(Debug, Default, Clone, Eq, PartialEq, FromJava, IntoJava)]
@@ -883,24 +877,30 @@ pub struct BridgeConstraints {
     pub ownership: Constraint<Ownership>,
 }
 
-impl BridgeConstraints {
-    pub fn format(
-        &self,
-        f: &mut String,
-        custom_lists: &CustomListsSettings,
-    ) -> Result<(), fmt::Error> {
-        match self.location {
+pub struct BridgeConstraintsFormatter<'a> {
+    pub constraints: &'a BridgeConstraints,
+    pub custom_lists: &'a CustomListsSettings,
+}
+
+impl<'a> fmt::Display for BridgeConstraintsFormatter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.constraints.location {
             Constraint::Any => write!(f, "any location")?,
-            Constraint::Only(ref location_constraint) => {
-                location_constraint.format(f, custom_lists)?
-            }
+            Constraint::Only(ref constraint) => write!(
+                f,
+                "{}",
+                LocationConstraintFormatter {
+                    constraint,
+                    custom_lists: self.custom_lists,
+                }
+            )?,
         }
         write!(f, " using ")?;
-        match self.providers {
+        match self.constraints.providers {
             Constraint::Any => write!(f, "any provider")?,
             Constraint::Only(ref constraint) => write!(f, "{}", constraint)?,
         }
-        match self.ownership {
+        match self.constraints.ownership {
             Constraint::Any => Ok(()),
             Constraint::Only(ref constraint) => {
                 write!(f, " and {constraint}")
diff --git a/mullvad-types/src/settings/mod.rs b/mullvad-types/src/settings/mod.rs
index 11a19f6552..3b3ca15014 100644
--- a/mullvad-types/src/settings/mod.rs
+++ b/mullvad-types/src/settings/mod.rs
@@ -3,7 +3,7 @@ use crate::{
     relay_constraints::{
         BridgeConstraints, BridgeSettings, BridgeState, Constraint, GeographicLocationConstraint,
         LocationConstraint, ObfuscationSettings, RelayConstraints, RelaySettings,
-        RelaySettingsUpdate, SelectedObfuscation, WireguardConstraints,
+        RelaySettingsFormatter, RelaySettingsUpdate, SelectedObfuscation, WireguardConstraints,
     },
     wireguard,
 };
@@ -153,17 +153,16 @@ impl Settings {
                 self.bridge_state = BridgeState::Auto;
             }
 
-            let mut old_settings_string = String::new();
-            let _ = self
-                .relay_settings
-                .format(&mut old_settings_string, &self.custom_lists);
-            let mut new_settings_string = String::new();
-            let _ = new_settings.format(&mut new_settings_string, &self.custom_lists);
-
             log::debug!(
                 "Changing relay settings:\n\tfrom: {}\n\tto: {}",
-                old_settings_string,
-                new_settings_string,
+                RelaySettingsFormatter {
+                    settings: &self.relay_settings,
+                    custom_lists: &self.custom_lists,
+                },
+                RelaySettingsFormatter {
+                    settings: &new_settings,
+                    custom_lists: &self.custom_lists,
+                },
             );
 
             self.relay_settings = new_settings;