chore: run cargo clippy --fix + manual fixes (#20)

Author: Boshen

.github/workflows/ci.yml

@@ -6,11 +6,8 @@ on:
   pull_request:
     branches: ["main"]
 
-env:
-  CARGO_TERM_COLOR: always
-
 jobs:
-  build:
+  test:
     strategy:
       matrix:
         os:
@@ -29,6 +26,19 @@ jobs:
 
       - run: cargo test
 
+  clippy:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: taiki-e/checkout-action@v1
+
+      - uses: oxc-project/[email protected]
+        with:
+          save-cache: ${{ github.ref_name == 'main' }}
+          cache-key: clippy
+          components: clippy
+
+      - run: cargo clippy --all-targets --all-features -- -D warnings
+
   fmt:
     runs-on: ubuntu-latest
     steps:

src/fs.rs

@@ -80,16 +80,14 @@ pub enum Error {
 
 #[cfg(feature = "mmap")]
 pub fn open_zip_via_mmap<P: AsRef<Path>>(p: P) -> Result<Zip<mmap_rs::Mmap>, std::io::Error> {
-    let file = fs::File::open(p)?;
+    let file = std::fs::File::open(p)?;
 
     let mmap_builder =
         mmap_rs::MmapOptions::new(file.metadata().unwrap().len().try_into().unwrap()).unwrap();
 
-    let mmap = unsafe { mmap_builder.with_file(file, 0).map().unwrap() };
+    let mmap = unsafe { mmap_builder.with_file(&file, 0).map().unwrap() };
 
-    let zip = Zip::new(mmap).map_err(|_| {
-        std::io::Error::new(std::io::ErrorKind::Other, "Failed to read the zip file")
-    })?;
+    let zip = Zip::new(mmap).map_err(|_| std::io::Error::other("Failed to read the zip file"))?;
 
     Ok(zip)
 }
@@ -102,9 +100,7 @@ pub fn open_zip_via_mmap_p(p: &Path) -> Result<Zip<mmap_rs::Mmap>, std::io::Erro
 pub fn open_zip_via_read<P: AsRef<Path>>(p: P) -> Result<Zip<Vec<u8>>, std::io::Error> {
     let data = std::fs::read(p)?;
 
-    let zip = Zip::new(data).map_err(|_| {
-        std::io::Error::new(std::io::ErrorKind::Other, "Failed to read the zip file")
-    })?;
+    let zip = Zip::new(data).map_err(|_| std::io::Error::other("Failed to read the zip file"))?;
 
     Ok(zip)
 }
@@ -167,7 +163,7 @@ where
         p: P,
         cb: F,
     ) -> Result<T, std::io::Error> {
-        let zip = self.lru.get_or_try_init(p.as_ref().to_path_buf(), 1, |p| (self.open)(&p))?;
+        let zip = self.lru.get_or_try_init(p.as_ref().to_path_buf(), 1, |p| (self.open)(p))?;
 
         Ok(cb(zip.value()))
     }
@@ -315,7 +311,7 @@ mod tests {
 
     #[test]
     fn test_zip_type_api() {
-        let zip = open_zip_via_read(&PathBuf::from(
+        let zip = open_zip_via_read(PathBuf::from(
             "data/@babel-plugin-syntax-dynamic-import-npm-7.8.3-fb9ff5634a-8.zip",
         ))
         .unwrap();
@@ -327,7 +323,7 @@ mod tests {
     #[test]
     #[should_panic(expected = "Kind(NotFound)")]
     fn test_zip_type_api_not_exist_dir_with_slash() {
-        let zip = open_zip_via_read(&PathBuf::from(
+        let zip = open_zip_via_read(PathBuf::from(
             "data/@babel-plugin-syntax-dynamic-import-npm-7.8.3-fb9ff5634a-8.zip",
         ))
         .unwrap();
@@ -338,7 +334,7 @@ mod tests {
     #[test]
     #[should_panic(expected = "Kind(NotFound)")]
     fn test_zip_type_api_not_exist_dir_without_slash() {
-        let zip = open_zip_via_read(&PathBuf::from(
+        let zip = open_zip_via_read(PathBuf::from(
             "data/@babel-plugin-syntax-dynamic-import-npm-7.8.3-fb9ff5634a-8.zip",
         ))
         .unwrap();
@@ -348,7 +344,7 @@ mod tests {
 
     #[test]
     fn test_zip_list() {
-        let zip = open_zip_via_read(&PathBuf::from(
+        let zip = open_zip_via_read(PathBuf::from(
             "data/@babel-plugin-syntax-dynamic-import-npm-7.8.3-fb9ff5634a-8.zip",
         ))
         .unwrap();
@@ -382,7 +378,7 @@ mod tests {
 
     #[test]
     fn test_zip_read() {
-        let zip = open_zip_via_read(&PathBuf::from(
+        let zip = open_zip_via_read(PathBuf::from(
             "data/@babel-plugin-syntax-dynamic-import-npm-7.8.3-fb9ff5634a-8.zip",
         ))
         .unwrap();
@@ -472,10 +468,10 @@ mod tests {
 
         match vpath(&PathBuf::from(input)) {
             Ok(res) => {
-                assert_eq!(res, expectation, "input='{:?}'", input);
+                assert_eq!(res, expectation, "input='{input:?}'");
             }
             Err(err) => {
-                panic!("{:?}: {}", input, err);
+                panic!("{input:?}: {err}");
             }
         }
     }

src/lib.rs

@@ -1,3 +1,5 @@
+#![expect(clippy::result_large_err)] // TODO: FIXME
+
 pub mod fs;
 
 mod builtins;
@@ -11,6 +13,7 @@ use serde::{Deserialize, Serialize};
 use serde_with::{DefaultOnNull, serde_as};
 use std::{
     collections::{HashMap, HashSet, hash_map::Entry},
+    fmt,
     path::{Path, PathBuf},
 };
 use util::RegexDef;
@@ -63,15 +66,16 @@ pub enum Error {
     },
 }
 
-impl ToString for Error {
-    fn to_string(&self) -> String {
-        match &self {
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let message = match &self {
             Error::BadSpecifier { message, .. } => message.clone(),
             Error::FailedManifestHydration { message, .. } => message.clone(),
             Error::MissingPeerDependency { message, .. } => message.clone(),
             Error::UndeclaredDependency { message, .. } => message.clone(),
             Error::MissingDependency { message, .. } => message.clone(),
-        }
+        };
+        write!(f, "{message}")
     }
 }
 
@@ -82,6 +86,7 @@ pub enum Resolution {
 }
 
 pub struct ResolutionHost {
+    #[allow(clippy::type_complexity)]
     pub find_pnp_manifest: Box<dyn Fn(&Path) -> Result<Option<Manifest>, Error>>,
 }
 
@@ -172,13 +177,9 @@ pub struct Manifest {
 fn parse_scoped_package_name(specifier: &str) -> Option<(String, Option<String>)> {
     let mut segments = specifier.splitn(3, '/');
 
-    let Some(scope) = segments.next() else {
-        return None;
-    };
+    let scope = segments.next()?;
 
-    let Some(name) = segments.next() else {
-        return None;
-    };
+    let name = segments.next()?;
 
     let package_name = specifier[..scope.len() + name.len() + 1].to_string();
 
@@ -190,9 +191,7 @@ fn parse_scoped_package_name(specifier: &str) -> Option<(String, Option<String>)
 fn parse_global_package_name(specifier: &str) -> Option<(String, Option<String>)> {
     let mut segments = specifier.splitn(2, '/');
 
-    let Some(name) = segments.next() else {
-        return None;
-    };
+    let name = segments.next()?;
 
     let package_name = name.to_string();
 
@@ -229,8 +228,7 @@ pub fn load_pnp_manifest<P: AsRef<Path>>(p: P) -> Result<Manifest, Error> {
     let manifest_content =
         std::fs::read_to_string(p.as_ref()).map_err(|err| Error::FailedManifestHydration {
             message: format!(
-                "We failed to read the content of the manifest.\n\nOriginal error: {}",
-                err.to_string()
+                "We failed to read the content of the manifest.\n\nOriginal error: {err}"
             ),
             manifest_path: p.as_ref().to_path_buf(),
         })?;
@@ -270,7 +268,7 @@ pub fn load_pnp_manifest<P: AsRef<Path>>(p: P) -> Result<Manifest, Error> {
 
     let mut manifest: Manifest = serde_json::from_str(&json_string.to_owned())
         .map_err(|err| Error::FailedManifestHydration {
-            message: format!("We failed to parse the PnP data payload as proper JSON; Did you manually edit the file?\n\nOriginal error: {}", err.to_string()),
+            message: format!("We failed to parse the PnP data payload as proper JSON; Did you manually edit the file?\n\nOriginal error: {err}"),
             manifest_path: p.as_ref().to_path_buf(),
         })?;
 
@@ -288,7 +286,7 @@ pub fn init_pnp_manifest<P: AsRef<Path>>(manifest: &mut Manifest, p: P) {
         for (reference, info) in ranges.iter_mut() {
             let package_location = manifest.manifest_dir.join(info.package_location.clone());
 
-            let normalized_location = util::normalize_path(&package_location.to_string_lossy());
+            let normalized_location = util::normalize_path(package_location.to_string_lossy());
 
             info.package_location = PathBuf::from(normalized_location);
 
@@ -366,7 +364,7 @@ pub fn find_broken_peer_dependencies(
     _dependency: &str,
     _initial_package: &PackageLocator,
 ) -> Vec<PackageLocator> {
-    vec![].to_vec()
+    [].to_vec()
 }
 
 pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
@@ -425,32 +423,30 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
                         issuer_path = parent.as_ref().to_string_lossy(),
                     )
                 }
+            } else if is_dependency_tree_root(manifest, parent_locator) {
+                format!(
+                    "Your application tried to access {dependency_name}, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.\n\nRequired package: {dependency_name}{via}\nRequired by: {issuer_path}",
+                    dependency_name = &ident,
+                    via = if ident != specifier {
+                        format!(" (via \"{}\")", &specifier)
+                    } else {
+                        String::from("")
+                    },
+                    issuer_path = parent.as_ref().to_string_lossy(),
+                )
             } else {
-                if is_dependency_tree_root(manifest, parent_locator) {
-                    format!(
-                        "Your application tried to access {dependency_name}, but it isn't declared in your dependencies; this makes the require call ambiguous and unsound.\n\nRequired package: {dependency_name}{via}\nRequired by: {issuer_path}",
-                        dependency_name = &ident,
-                        via = if ident != specifier {
-                            format!(" (via \"{}\")", &specifier)
-                        } else {
-                            String::from("")
-                        },
-                        issuer_path = parent.as_ref().to_string_lossy(),
-                    )
-                } else {
-                    format!(
-                        "{issuer_locator_name} tried to access {dependency_name}, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.\n\nRequired package: {dependency_name}{via}\nRequired by: {issuer_locator_name}@{issuer_locator_reference} (via {issuer_path})",
-                        issuer_locator_name = &parent_locator.name,
-                        issuer_locator_reference = &parent_locator.reference,
-                        dependency_name = &ident,
-                        via = if ident != specifier {
-                            format!(" (via \"{}\")", &specifier)
-                        } else {
-                            String::from("")
-                        },
-                        issuer_path = parent.as_ref().to_string_lossy(),
-                    )
-                }
+                format!(
+                    "{issuer_locator_name} tried to access {dependency_name}, but it isn't declared in its dependencies; this makes the require call ambiguous and unsound.\n\nRequired package: {dependency_name}{via}\nRequired by: {issuer_locator_name}@{issuer_locator_reference} (via {issuer_path})",
+                    issuer_locator_name = &parent_locator.name,
+                    issuer_locator_reference = &parent_locator.reference,
+                    dependency_name = &ident,
+                    via = if ident != specifier {
+                        format!(" (via \"{}\")", &specifier)
+                    } else {
+                        String::from("")
+                    },
+                    issuer_path = parent.as_ref().to_string_lossy(),
+                )
             };
 
             return Err(Error::UndeclaredDependency {
@@ -474,7 +470,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
 
             Ok(Resolution::Resolved(dependency_pkg.package_location.clone(), module_path))
         } else {
-            let broken_ancestors = find_broken_peer_dependencies(&specifier, parent_locator);
+            let broken_ancestors = find_broken_peer_dependencies(specifier, parent_locator);
 
             let message = if is_dependency_tree_root(manifest, parent_locator) {
                 format!(
@@ -523,7 +519,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(
                 dependency_name: ident,
                 issuer_locator: parent_locator.clone(),
                 issuer_path: parent.as_ref().to_path_buf(),
-                broken_ancestors: vec![].to_vec(),
+                broken_ancestors: [].to_vec(),
             })
         }
     } else {

src/lib_tests.rs

@@ -1,6 +1,6 @@
 use serde::Deserialize;
 
-use crate::{Manifest, Resolution, ResolutionConfig};
+use crate::{Manifest, Resolution};
 
 #[derive(Deserialize)]
 struct Test {
@@ -22,20 +22,18 @@ mod tests {
 
     use super::*;
     use crate::{
-        ResolutionHost, init_pnp_manifest, load_pnp_manifest, parse_bare_identifier,
-        resolve_to_unqualified, resolve_to_unqualified_via_manifest,
+        ResolutionConfig, ResolutionHost, init_pnp_manifest, load_pnp_manifest,
+        parse_bare_identifier, resolve_to_unqualified, resolve_to_unqualified_via_manifest,
     };
 
     #[test]
     fn example() {
         let manifest = load_pnp_manifest("data/pnp-yarn-v3.cjs").unwrap();
 
-        let host = ResolutionHost {
-            find_pnp_manifest: Box::new(move |_| Ok(Some(manifest.clone()))),
-            ..Default::default()
-        };
+        let host =
+            ResolutionHost { find_pnp_manifest: Box::new(move |_| Ok(Some(manifest.clone()))) };
 
-        let config = ResolutionConfig { host, ..Default::default() };
+        let config = ResolutionConfig { host };
 
         let resolution = resolve_to_unqualified(
             "lodash/cloneDeep",
@@ -85,7 +83,7 @@ mod tests {
 
         for test_suite in test_suites.iter_mut() {
             let manifest = &mut test_suite.manifest;
-            init_pnp_manifest(manifest, &PathBuf::from("/path/to/project/.pnp.cjs"));
+            init_pnp_manifest(manifest, PathBuf::from("/path/to/project/.pnp.cjs"));
 
             for test in test_suite.tests.iter() {
                 let specifier = &test.imported;
@@ -95,10 +93,9 @@ mod tests {
 
                 let host = ResolutionHost {
                     find_pnp_manifest: Box::new(move |_| Ok(Some(manifest_copy.clone()))),
-                    ..Default::default()
                 };
 
-                let config = ResolutionConfig { host, ..Default::default() };
+                let config = ResolutionConfig { host };
 
                 let resolution = resolve_to_unqualified(specifier, parent, &config);
 
@@ -110,7 +107,7 @@ mod tests {
                         assert_eq!(specifier, &test.expected, "{}", test.it);
                     }
                     Err(err) => {
-                        assert_eq!(test.expected, "error!", "{}: {}", test.it, err.to_string());
+                        assert_eq!(test.expected, "error!", "{}: {err}", test.it);
                     }
                 }
             }