Refactor attribute value access and error handling

- Replaced `values.get(0)` with `values.first()` for improved clarity and consistency across multiple files.
- Simplified conditional checks with `.is_empty()` and `format_args!` to enhance readability.
- Implemented `Default` trait for `VariableMode` directly within the enum.
- Updated error handling by boxing errors in `RepositoryError` and `CatalogError`.
- Replaced redundant `else` blocks with streamlined logic.

libips/src/api.rs

@@ -77,7 +77,7 @@ pub use crate::transformer::TransformRule;
 pub enum IpsError {
     #[error(transparent)]
     #[diagnostic(transparent)]
-    Repository(#[from] RepositoryError),
+    Repository(Box<RepositoryError>),
 
     #[error(transparent)]
     #[diagnostic(transparent)]
@@ -635,7 +635,7 @@ impl Resolver {
 fn manifest_fmri(manifest: &Manifest) -> Option<Fmri> {
     for attr in &manifest.attributes {
         if attr.key == "pkg.fmri" {
-            if let Some(val) = attr.values.get(0) {
+            if let Some(val) = attr.values.first() {
                 if let Ok(f) = Fmri::parse(val) {
                     return Some(f);
                 }
@@ -747,7 +747,7 @@ pub mod lint {
             for attr in &manifest.attributes {
                 if attr.key == "pkg.fmri" {
                     fmri_attr_count += 1;
-                    if let Some(v) = attr.values.get(0) {
+                    if let Some(v) = attr.values.first() {
                         fmri_text = Some(v.clone());
                     }
                 }
@@ -1017,13 +1017,13 @@ mod tests {
         assert!(m.attributes.iter().any(|a| {
             a.key == "pkg.fmri"
                 && a.values
-                    .get(0)
+                    .first()
                     .map(|v| v == &fmri.to_string())
                     .unwrap_or(false)
         }));
         assert!(
             m.attributes.iter().any(|a| a.key == "pkg.summary"
-                && a.values.get(0).map(|v| v == "Summary").unwrap_or(false))
+                && a.values.first().map(|v| v == "Summary").unwrap_or(false))
         );
 
         // Validate license
@@ -1053,3 +1053,9 @@ mod tests {
         assert_eq!(df.version.as_ref().unwrap().to_string(), "1.2");
     }
 }
+
+impl From<RepositoryError> for IpsError {
+    fn from(err: RepositoryError) -> Self {
+        Self::Repository(Box::new(err))
+    }
+}

libips/src/depend/mod.rs

@@ -425,7 +425,6 @@ pub fn resolve_dependencies<R: ReadableRepository>(
                             facets: HashMap::new(),
                         });
                     }
-                } else {
                 }
             }
             FileDepKind::Python {
@@ -467,7 +466,6 @@ pub fn resolve_dependencies<R: ReadableRepository>(
                             facets: HashMap::new(),
                         });
                     }
-                } else {
                 }
             }
         }
@@ -478,7 +476,7 @@ pub fn resolve_dependencies<R: ReadableRepository>(
 
 fn normalize_join(dir: &str, base: &str) -> String {
     if dir.ends_with('/') {
-        format!("{}{}", dir.trim_end_matches('/'), format!("/{}", base))
+        format!("{}{}", dir.trim_end_matches('/'), format_args!("/{}", base))
     } else {
         format!("{}/{}", dir, base)
     }

libips/src/image/catalog.rs

@@ -251,7 +251,7 @@ impl ImageCatalog {
                                 .attributes
                                 .iter()
                                 .find(|attr| attr.key == "pkg.fmri")
-                                .and_then(|attr| attr.values.get(0).cloned())
+                                .and_then(|attr| attr.values.first().cloned())
                                 .unwrap_or_else(|| "unknown".to_string());
 
                             println!("Key: {}", key_str);
@@ -990,7 +990,7 @@ impl ImageCatalog {
     /// Check if a package is obsolete
     fn is_package_obsolete(&self, manifest: &Manifest) -> bool {
         manifest.attributes.iter().any(|attr| {
-            attr.key == "pkg.obsolete" && attr.values.get(0).map_or(false, |v| v == "true")
+            attr.key == "pkg.obsolete" && attr.values.first().map_or(false, |v| v == "true")
         })
     }
 
@@ -1059,7 +1059,7 @@ impl ImageCatalog {
                 .iter()
                 .find(|attr| attr.key == "pkg.fmri")
                 .map(|attr| {
-                    if let Some(fmri_str) = attr.values.get(0) {
+                    if let Some(fmri_str) = attr.values.first() {
                         // Parse the FMRI string
                         match Fmri::parse(fmri_str) {
                             Ok(fmri) => fmri.publisher.unwrap_or_else(|| "unknown".to_string()),

libips/src/image/installed.rs

@@ -110,7 +110,7 @@ impl InstalledPackages {
                                 .attributes
                                 .iter()
                                 .find(|attr| attr.key == "pkg.fmri")
-                                .and_then(|attr| attr.values.get(0).cloned())
+                                .and_then(|attr| attr.values.first().cloned())
                                 .unwrap_or_else(|| "unknown".to_string());
 
                             println!("Key: {}", key_str);

libips/src/image/tests.rs

@@ -163,7 +163,7 @@ fn test_catalog_methods() {
     // Verify that the obsolete package's manifest has the obsolete attribute
     let manifest = manifest.unwrap();
     let is_obsolete = manifest.attributes.iter().any(|attr| {
-        attr.key == "pkg.obsolete" && attr.values.get(0).map_or(false, |v| v == "true")
+        attr.key == "pkg.obsolete" && attr.values.first().map_or(false, |v| v == "true")
     });
     assert!(is_obsolete);
 

libips/src/repository/catalog.rs

@@ -615,7 +615,7 @@ mod tests {
 
                     let packages = part.packages.get("openindiana.org").unwrap();
                     println!("Found {} packages for openindiana.org", packages.len());
-                    assert!(packages.len() > 0, "Should have loaded packages");
+                    assert!(!packages.is_empty(), "Should have loaded packages");
                 }
                 Err(e) => panic!("Failed to load catalog part: {}", e),
             }

libips/src/repository/mod.rs

@@ -138,7 +138,7 @@ pub enum RepositoryError {
 
     #[error(transparent)]
     #[diagnostic(transparent)]
-    CatalogError(#[from] catalog::CatalogError),
+    CatalogError(Box<catalog::CatalogError>),
 
     #[error("path prefix error: {0}")]
     #[diagnostic(
@@ -441,3 +441,9 @@ pub trait WritableRepository {
 /// This trait combines both ReadableRepository and WritableRepository traits
 /// for backward compatibility.
 pub trait Repository: ReadableRepository + WritableRepository {}
+
+impl From<catalog::CatalogError> for RepositoryError {
+    fn from(err: catalog::CatalogError) -> Self {
+        Self::CatalogError(Box::new(err))
+    }
+}

libips/src/solver/mod.rs

@@ -162,7 +162,7 @@ impl<'a> IpsProvider<'a> {
             let mut pushed = false;
             if let Ok(manifest) = decode_manifest_bytes_local(v.value()) {
                 if let Some(attr) = manifest.attributes.iter().find(|a| a.key == "pkg.fmri") {
-                    if let Some(fmri_str) = attr.values.get(0) {
+                    if let Some(fmri_str) = attr.values.first() {
                         if let Ok(mut fmri) = Fmri::parse(fmri_str) {
                             // Ensure publisher is present; if missing/empty, use image default publisher
                             let missing_pub = fmri

userland/src/lib.rs

@@ -35,18 +35,13 @@ pub struct MakefileVariable {
     pub mode: VariableMode,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Clone, Default)]
 pub enum VariableMode {
     Add,
+    #[default]
     Set,
 }
 
-impl Default for VariableMode {
-    fn default() -> Self {
-        Self::Set
-    }
-}
-
 #[derive(Error, Debug)]
 pub enum ParserError {
     #[error("cannot parse {file}: {reason}")]
@@ -194,7 +189,7 @@ impl Makefile {
     }
 }
 
-fn vars_to_string(vars: &Vec<String>) -> String {
+fn vars_to_string(vars: &[String]) -> String {
     if vars.is_empty() {
         String::new()
     } else if vars.len() == 1 {