Skip to content

Commit

Permalink
fix: Clippy warnings (#1231)
Browse files Browse the repository at this point in the history
Fix almost all clippy warnings (some of them were automatically fixed
with `cargo clippy --fix`).

Note: there are still some warnings that require more work to get rid of
them:

<details>
<summary><i>$ cargo clippy</i></summary>

~~~
cargo clippy
Checking agama-server v0.1.0
(/home/ivan/projects/suse/code/agama/rust/agama-server)
warning: module has the same name as its containing module
 --> agama-server/src/l10n.rs:5:1
  |
5 | pub mod l10n;
  | ^^^^^^^^^^^^^
  |
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
  = note: `#[warn(clippy::module_inception)]` on by default

warning: very complex type used. Consider factoring parts into `type`
definitions
   --> agama-server/src/network/nm/proxies.rs:849:28
    |
849 | fn addresses(&self) -> zbus::Result<Vec<(Vec<u8>, u32, Vec<u8>)>>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
    = note: `#[warn(clippy::type_complexity)]` on by default

warning: very complex type used. Consider factoring parts into `type`
definitions
   --> agama-server/src/network/nm/proxies.rs:879:25
    |
879 | fn routes(&self) -> zbus::Result<Vec<(Vec<u8>, u32, Vec<u8>,
u32)>>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity

warning: `agama-server` (lib) generated 3 warnings
warning: current MSRV (Minimum Supported Rust Version) is `1.74.0` but
this item is stable since `1.75.0`
   --> agama-server/src/agama-web-server.rs:233:22
    |
233 |         if addr.ip().to_canonical().is_loopback() {
    |                      ^^^^^^^^^^^^^^
    |
= help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#incompatible_msrv
    = note: `#[warn(clippy::incompatible_msrv)]` on by default
~~~

</details>
  • Loading branch information
joseivanlopez authored May 27, 2024
2 parents 8fc9271 + d7202e8 commit 208d111
Show file tree
Hide file tree
Showing 17 changed files with 118 additions and 146 deletions.
2 changes: 1 addition & 1 deletion rust/agama-cli/src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ async fn get_jwt(url: String, password: String) -> anyhow::Result<String> {
let body = response
.json::<std::collections::HashMap<String, String>>()
.await?;
let value = body.get(&"token".to_string());
let value = body.get("token");

if let Some(token) = value {
return Ok(token.clone());
Expand Down
14 changes: 5 additions & 9 deletions rust/agama-lib/src/dbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ macro_rules! property_from_dbus {
pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap<String, OwnedValue> {
let mut owned = HashMap::new();
for (key, value) in source.iter() {
if let Ok(owned_value) = value.try_into() {
owned.insert(key.to_string(), owned_value);
}
owned.insert(key.to_string(), value.into());
}
owned
}
Expand All @@ -74,7 +72,7 @@ pub fn to_owned_hash(source: &HashMap<&str, Value<'_>>) -> HashMap<String, Owned
/// TODO: should we merge this feature with the "DeviceSid"?
pub fn extract_id_from_path(path: &OwnedObjectPath) -> Result<u32, zvariant::Error> {
path.as_str()
.rsplit_once("/")
.rsplit_once('/')
.and_then(|(_, id)| id.parse::<u32>().ok())
.ok_or_else(|| {
zvariant::Error::Message(format!("Could not extract the ID from {}", path.as_str()))
Expand All @@ -92,7 +90,7 @@ mod tests {
#[test]
fn test_get_property() {
let data: HashMap<String, OwnedValue> = HashMap::from([
("Id".to_string(), (1 as u8).into()),
("Id".to_string(), 1_u8.into()),
("Device".to_string(), Str::from_static("/dev/sda").into()),
]);
let id: u8 = get_property(&data, "Id").unwrap();
Expand All @@ -104,16 +102,14 @@ mod tests {

#[test]
fn test_get_property_wrong_type() {
let data: HashMap<String, OwnedValue> =
HashMap::from([("Id".to_string(), (1 as u8).into())]);
let data: HashMap<String, OwnedValue> = HashMap::from([("Id".to_string(), 1_u8.into())]);
let result: Result<u16, _> = get_property(&data, "Id");
assert_eq!(result, Err(zvariant::Error::IncorrectType));
}

#[test]
fn test_get_optional_property() {
let data: HashMap<String, OwnedValue> =
HashMap::from([("Id".to_string(), (1 as u8).into())]);
let data: HashMap<String, OwnedValue> = HashMap::from([("Id".to_string(), 1_u8.into())]);
let id: Option<u8> = get_optional_property(&data, "Id").unwrap();
assert_eq!(id, Some(1));

Expand Down
2 changes: 1 addition & 1 deletion rust/agama-lib/src/software/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ impl<'a> SoftwareClient<'a> {
.await?
.into_iter()
.filter_map(|(id, reason)| match SelectedBy::try_from(reason) {
Ok(reason) if reason == SelectedBy::User => Some(id),
Ok(SelectedBy::User) => Some(id),
Ok(_reason) => None,
Err(e) => {
log::warn!("Ignoring pattern {}. Error: {}", &id, e);
Expand Down
2 changes: 1 addition & 1 deletion rust/agama-lib/src/storage/client/iscsi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<'a> ISCSIClient<'a> {

let result = self
.initiator_proxy
.discover(address, port as u32, options_ref)
.discover(address, port, options_ref)
.await?;
Ok(result == 0)
}
Expand Down
98 changes: 46 additions & 52 deletions rust/agama-lib/src/storage/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,9 @@ impl TryFrom<i32> for DeviceSid {
type Error = zbus::zvariant::Error;

fn try_from(value: i32) -> Result<Self, Self::Error> {
u32::try_from(value).map(|v| v.into()).or_else(|_| {
Err(Self::Error::Message(format!(
"Cannot convert sid from {}",
value
)))
})
u32::try_from(value)
.map(|v| v.into())
.map_err(|_| Self::Error::Message(format!("Cannot convert sid from {}", value)))
}
}

Expand All @@ -48,7 +45,7 @@ impl TryFrom<zbus::zvariant::ObjectPath<'_>> for DeviceSid {

fn try_from(path: zbus::zvariant::ObjectPath) -> Result<Self, Self::Error> {
path.as_str()
.rsplit_once("/")
.rsplit_once('/')
.and_then(|(_, sid)| sid.parse::<u32>().ok())
.ok_or_else(|| Self::Error::Message(format!("Cannot parse sid from {}", path)))
.map(DeviceSid)
Expand All @@ -68,12 +65,9 @@ impl TryFrom<i64> for DeviceSize {
type Error = zbus::zvariant::Error;

fn try_from(value: i64) -> Result<Self, Self::Error> {
u64::try_from(value).map(|v| v.into()).or_else(|_| {
Err(Self::Error::Message(format!(
"Cannot convert size from {}",
value
)))
})
u64::try_from(value)
.map(|v| v.into())
.map_err(|_| Self::Error::Message(format!("Cannot convert size from {}", value)))
}
}

Expand All @@ -94,9 +88,9 @@ impl TryFrom<zbus::zvariant::Value<'_>> for DeviceSize {
}
}

impl<'a> Into<zbus::zvariant::Value<'a>> for DeviceSize {
fn into(self) -> Value<'a> {
Value::new(self.0)
impl<'a> From<DeviceSize> for zbus::zvariant::Value<'a> {
fn from(val: DeviceSize) -> Self {
Value::new(val.0)
}
}

Expand Down Expand Up @@ -188,11 +182,11 @@ impl TryFrom<zbus::zvariant::Value<'_>> for SpaceActionSettings {
}
}

impl<'a> Into<zbus::zvariant::Value<'a>> for SpaceActionSettings {
fn into(self) -> zbus::zvariant::Value<'a> {
impl<'a> From<SpaceActionSettings> for zbus::zvariant::Value<'a> {
fn from(val: SpaceActionSettings) -> Self {
let result: HashMap<&str, Value> = HashMap::from([
("Device", Value::new(self.device)),
("Action", Value::new(self.action.as_dbus_string())),
("Device", Value::new(val.device)),
("Action", Value::new(val.action.as_dbus_string())),
]);

Value::new(result)
Expand All @@ -218,41 +212,41 @@ pub struct ProposalSettingsPatch {
pub volumes: Option<Vec<Volume>>,
}

impl<'a> Into<HashMap<&'static str, Value<'a>>> for ProposalSettingsPatch {
fn into(self) -> HashMap<&'static str, Value<'a>> {
impl<'a> From<ProposalSettingsPatch> for HashMap<&'static str, Value<'a>> {
fn from(val: ProposalSettingsPatch) -> Self {
let mut result = HashMap::new();
if let Some(target) = self.target {
if let Some(target) = val.target {
result.insert("Target", Value::new(target.as_dbus_string()));
}
if let Some(dev) = self.target_device {
if let Some(dev) = val.target_device {
result.insert("TargetDevice", Value::new(dev));
}
if let Some(devs) = self.target_pv_devices {
if let Some(devs) = val.target_pv_devices {
result.insert("TargetPVDevices", Value::new(devs));
}
if let Some(value) = self.configure_boot {
if let Some(value) = val.configure_boot {
result.insert("ConfigureBoot", Value::new(value));
}
if let Some(value) = self.boot_device {
if let Some(value) = val.boot_device {
result.insert("BootDevice", Value::new(value));
}
if let Some(value) = self.encryption_password {
if let Some(value) = val.encryption_password {
result.insert("EncryptionPassword", Value::new(value));
}
if let Some(value) = self.encryption_method {
if let Some(value) = val.encryption_method {
result.insert("EncryptionMethod", Value::new(value));
}
if let Some(value) = self.encryption_pbkd_function {
if let Some(value) = val.encryption_pbkd_function {
result.insert("EncryptionPBKDFunction", Value::new(value));
}
if let Some(value) = self.space_policy {
if let Some(value) = val.space_policy {
result.insert("SpacePolicy", Value::new(value));
}
if let Some(value) = self.space_actions {
if let Some(value) = val.space_actions {
let list: Vec<Value> = value.into_iter().map(|a| a.into()).collect();
result.insert("SpaceActions", Value::new(list));
}
if let Some(value) = self.volumes {
if let Some(value) = val.volumes {
let list: Vec<Value> = value.into_iter().map(|a| a.into()).collect();
result.insert("Volumes", Value::new(list));
}
Expand Down Expand Up @@ -338,14 +332,14 @@ pub enum VolumeTarget {
Filesystem,
}

impl<'a> Into<zbus::zvariant::Value<'a>> for VolumeTarget {
fn into(self) -> zbus::zvariant::Value<'a> {
let str = match self {
Self::Default => "default",
Self::NewPartition => "new_partition",
Self::NewVg => "new_vg",
Self::Device => "device",
Self::Filesystem => "filesystem",
impl<'a> From<VolumeTarget> for zbus::zvariant::Value<'a> {
fn from(val: VolumeTarget) -> Self {
let str = match val {
VolumeTarget::Default => "default",
VolumeTarget::NewPartition => "new_partition",
VolumeTarget::NewVg => "new_vg",
VolumeTarget::Device => "device",
VolumeTarget::Filesystem => "filesystem",
};

Value::new(str)
Expand Down Expand Up @@ -419,23 +413,23 @@ pub struct Volume {
outline: Option<VolumeOutline>,
}

impl<'a> Into<zbus::zvariant::Value<'a>> for Volume {
fn into(self) -> zbus::zvariant::Value<'a> {
impl<'a> From<Volume> for zbus::zvariant::Value<'a> {
fn from(val: Volume) -> Self {
let mut result: HashMap<&str, Value> = HashMap::from([
("MountPath", Value::new(self.mount_path)),
("MountOptions", Value::new(self.mount_options)),
("Target", self.target.into()),
("FsType", Value::new(self.fs_type)),
("AutoSize", Value::new(self.auto_size)),
("Snapshots", Value::new(self.snapshots)),
("MountPath", Value::new(val.mount_path)),
("MountOptions", Value::new(val.mount_options)),
("Target", val.target.into()),
("FsType", Value::new(val.fs_type)),
("AutoSize", Value::new(val.auto_size)),
("Snapshots", Value::new(val.snapshots)),
]);
if let Some(dev) = self.target_device {
if let Some(dev) = val.target_device {
result.insert("TargetDevice", Value::new(dev));
}
if let Some(value) = self.min_size {
if let Some(value) = val.min_size {
result.insert("MinSize", value.into());
}
if let Some(value) = self.max_size {
if let Some(value) = val.max_size {
result.insert("MaxSize", value.into());
}
// intentionally skip outline as it is not send to dbus and act as read only parameter
Expand Down
1 change: 1 addition & 0 deletions rust/agama-server/src/agama-web-server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ fn write_token(path: &str, secret: &str) -> io::Result<()> {
let token = generate_token(secret);
let mut file = fs::OpenOptions::new()
.create(true)
.truncate(true)
.write(true)
.mode(0o400)
.open(path)?;
Expand Down
10 changes: 5 additions & 5 deletions rust/agama-server/src/dbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl DBusObjectChangesStream {
.path(manager_path.clone())?
.interface("org.freedesktop.DBus.ObjectManager")?
.build();
let stream = MessageStream::for_match_rule(rule, &connection, None).await?;
let stream = MessageStream::for_match_rule(rule, connection, None).await?;
Ok(stream)
}

Expand All @@ -168,7 +168,7 @@ impl DBusObjectChangesStream {
.interface("org.freedesktop.DBus.Properties")?
.member("PropertiesChanged")?
.build();
let stream = MessageStream::for_match_rule(rule, &connection, None).await?;
let stream = MessageStream::for_match_rule(rule, connection, None).await?;
Ok(stream)
}
}
Expand All @@ -182,10 +182,10 @@ impl Stream for DBusObjectChangesStream {
let item = ready!(pinned.inner.as_mut().poll_next(cx));
let next_value = match item {
Some((PROPERTIES_CHANGED, message)) => {
Self::handle_properties_changed(message, &pinned.interface)
Self::handle_properties_changed(message, pinned.interface)
}
Some((OBJECTS_MANAGER, message)) => {
Self::handle_added_or_removed(message, &pinned.interface)
Self::handle_added_or_removed(message, pinned.interface)
}
_ => None,
};
Expand Down Expand Up @@ -216,7 +216,7 @@ where
}

pub fn remove(&mut self, path: &OwnedObjectPath) -> Option<T> {
self.objects.remove(&path)
self.objects.remove(path)
}
}

Expand Down
18 changes: 9 additions & 9 deletions rust/agama-server/src/l10n/dbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ impl L10nInterface {
"The locales list cannot be empty".to_string(),
));
}
backend.set_locales(&locales).map_err(|e| {
zbus::fdo::Error::Failed(format!("Could not set the locales: {}", e.to_string()))
})?;
backend
.set_locales(&locales)
.map_err(|e| zbus::fdo::Error::Failed(format!("Could not set the locales: {}", e)))?;
Ok(())
}

Expand Down Expand Up @@ -59,9 +59,9 @@ impl L10nInterface {
.parse()
.map_err(|_e| zbus::fdo::Error::InvalidArgs("Cannot parse keymap ID".to_string()))?;

backend.set_keymap(keymap_id).map_err(|e| {
zbus::fdo::Error::Failed(format!("Could not set the keymap: {}", e.to_string()))
})?;
backend
.set_keymap(keymap_id)
.map_err(|e| zbus::fdo::Error::Failed(format!("Could not set the keymap: {}", e)))?;

Ok(())
}
Expand All @@ -76,9 +76,9 @@ impl L10nInterface {
pub fn set_timezone(&mut self, timezone: &str) -> Result<(), zbus::fdo::Error> {
let mut backend = self.backend.write().unwrap();

backend.set_timezone(timezone).map_err(|e| {
zbus::fdo::Error::Failed(format!("Could not set the timezone: {}", e.to_string()))
})?;
backend
.set_timezone(timezone)
.map_err(|e| zbus::fdo::Error::Failed(format!("Could not set the timezone: {}", e)))?;
Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions rust/agama-server/src/l10n/l10n.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl L10n {
return Err(LocaleError::UnknownLocale(loc.to_string()))?;
}
}
self.locales = locales.clone();
self.locales.clone_from(locales);
Ok(())
}

Expand All @@ -153,7 +153,7 @@ impl L10n {
if !self.timezones_db.exists(&timezone.to_string()) {
return Err(LocaleError::UnknownTimezone(timezone.to_string()))?;
}
self.timezone = timezone.to_owned();
timezone.clone_into(&mut self.timezone);
Ok(())
}

Expand All @@ -168,7 +168,7 @@ impl L10n {

// TODO: use LocaleError
pub fn translate(&mut self, locale: &LocaleId) -> Result<(), Error> {
helpers::set_service_locale(&locale);
helpers::set_service_locale(locale);
self.timezones_db.read(&locale.language)?;
self.locales_db.read(&locale.language)?;
self.ui_locale = locale.clone();
Expand Down
5 changes: 2 additions & 3 deletions rust/agama-server/src/l10n/locale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ impl LocalesDatabase {
fn get_locales_list() -> Result<Vec<LocaleId>, Error> {
const LOCALES_LIST_PATH: &str = "/etc/agama.d/locales";

let locales = fs::read_to_string(LOCALES_LIST_PATH)
.map(|content| Self::get_locales_from_string(content));
let locales = fs::read_to_string(LOCALES_LIST_PATH).map(Self::get_locales_from_string);

if let Ok(locales) = locales {
if !locales.is_empty() {
Expand All @@ -123,7 +122,7 @@ impl LocalesDatabase {
.context("Failed to get the list of locales")?;

let locales = String::from_utf8(result.stdout)
.map(|content| Self::get_locales_from_string(content))
.map(Self::get_locales_from_string)
.context("Invalid UTF-8 sequence from list-locales")?;

Ok(locales)
Expand Down
Loading

0 comments on commit 208d111

Please sign in to comment.