Optimize lockfile

src/command/build.rs

@@ -7,7 +7,7 @@ use crate::command::utils::{create_pkg_dir, get_crate_path};
 use crate::emoji;
 use crate::install::{self, InstallMode, Tool};
 use crate::license;
-use crate::lockfile::Lockfile;
+use crate::lockfile;
 use crate::manifest;
 use crate::readme;
 use crate::wasm_opt;
@@ -406,8 +406,8 @@ impl Build {
 
     fn step_install_wasm_bindgen(&mut self) -> Result<()> {
         info!("Identifying wasm-bindgen dependency...");
-        let lockfile = Lockfile::new(&self.crate_data)?;
-        let bindgen_version = lockfile.require_wasm_bindgen()?;
+        let [package] = lockfile::Package::get(&self.crate_data, ["wasm-bindgen"])?;
+        let bindgen_version = package.require_version_or_suggest("dependencies", "0.2")?;
         info!("Installing wasm-bindgen-cli...");
         let bindgen = install::download_prebuilt_or_cargo_install(
             Tool::WasmBindgen,

src/command/test.rs

@@ -4,12 +4,11 @@ use crate::build;
 use crate::cache;
 use crate::command::utils::get_crate_path;
 use crate::install::{self, InstallMode, Tool};
-use crate::lockfile::Lockfile;
+use crate::lockfile;
 use crate::manifest;
 use crate::test::{self, webdriver};
 use anyhow::{bail, Result};
 use binary_install::Cache;
-use console::style;
 use log::info;
 use std::path::PathBuf;
 use std::str::FromStr;
@@ -282,22 +281,16 @@ impl Test {
 
     fn step_install_wasm_bindgen(&mut self) -> Result<()> {
         info!("Identifying wasm-bindgen dependency...");
-        let lockfile = Lockfile::new(&self.crate_data)?;
-        let bindgen_version = lockfile.require_wasm_bindgen()?;
+        let [bindgen, bindgen_test] =
+            lockfile::Package::get(&self.crate_data, ["wasm-bindgen", "wasm-bindgen-test"])?;
+        let bindgen_version = bindgen.require_version_or_suggest("dependencies", "0.2")?;
 
         // Unlike `wasm-bindgen` and `wasm-bindgen-cli`, `wasm-bindgen-test`
         // will work with any semver compatible `wasm-bindgen-cli`, so just make
         // sure that it is depended upon, so we can run tests on
         // `wasm32-unkown-unknown`. Don't enforce that it is the same version as
         // `wasm-bindgen`.
-        if lockfile.wasm_bindgen_test_version().is_none() {
-            bail!(
-                "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\
-                 [dev-dependencies]\n\
-                 wasm-bindgen-test = \"0.2\"",
-                style("wasm-bindgen-test").bold().dim(),
-            )
-        }
+        bindgen_test.require_version_or_suggest("dev-dependencies", "0.2")?;
 
         let status = install::download_prebuilt_or_cargo_install(
             Tool::WasmBindgen,

src/lockfile.rs

@@ -2,104 +2,163 @@
 
 #![allow(clippy::new_ret_no_self)]
 
-use std::fs;
 use std::path::PathBuf;
+use std::{borrow::Cow, fs};
 
 use crate::manifest::CrateData;
 use anyhow::{anyhow, bail, Context, Result};
 use console::style;
+use serde::{
+    de::{DeserializeSeed, Error, IgnoredAny, Visitor},
+    Deserializer,
+};
 use toml;
 
-const PACKAGE_NAMES: &[&str] = &["wasm-bindgen", "wasm-bindgen-test"];
-
-/// This struct represents the contents of `Cargo.lock`.
-#[derive(Clone, Debug, Deserialize)]
-pub struct Lockfile {
-    #[serde(deserialize_with = "deserialize_package_list")]
-    package: Vec<Package>,
-}
-
 /// This struct represents a single package entry in `Cargo.lock`
-#[derive(Clone, Debug, Deserialize)]
-struct Package {
-    name: String,
-    version: String,
+pub struct Package<'a> {
+    name: &'a str,
+    version: Option<String>,
 }
 
-/// Deserializer that only includes packages with a name starting with "wasm-bindgen"
-fn deserialize_package_list<'de, D>(deserializer: D) -> std::result::Result<Vec<Package>, D::Error>
-where
-    D: serde::Deserializer<'de>,
-{
-    deserializer.deserialize_seq(PackagesVisitor)
+impl<'a> Package<'a> {
+    /// Read the `Cargo.lock` file for the crate at the given path and get the versions of the named dependencies.
+    pub fn get<const N: usize>(
+        crate_data: &CrateData,
+        dep_names: [&'a str; N],
+    ) -> Result<[Self; N]> {
+        let lock_path = get_lockfile_path(crate_data)?;
+        let lockfile = fs::read_to_string(&lock_path)
+            .with_context(|| anyhow!("failed to read: {}", lock_path.display()))?;
+        toml::Deserializer::new(&lockfile)
+            .deserialize_struct("Lockfile", &["package"], LockfileVisitor { dep_names })
+            .with_context(|| anyhow!("failed to parse: {}", lock_path.display()))
+    }
+
+    /// Get the version of this package used in the `Cargo.lock`.
+    pub fn version(&self) -> Option<&str> {
+        self.version.as_deref()
+    }
+
+    /// Like `version`, except it returns an error instead of `None`. `suggested_version` is only used when showing an example of adding the dependency to `Cargo.toml`.
+    pub fn require_version_or_suggest(
+        &self,
+        section: &str,
+        suggested_version: &str,
+    ) -> Result<&str> {
+        self.version().ok_or_else(|| {
+            anyhow!(
+                "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\
+                 [{}]\n\
+                 {} = \"{}\"",
+                style(self.name).bold().dim(),
+                section,
+                self.name,
+                suggested_version,
+            )
+        })
+    }
 }
 
-struct PackagesVisitor;
+struct LockfileVisitor<'a, const N: usize> {
+    dep_names: [&'a str; N],
+}
 
-impl<'de> serde::de::Visitor<'de> for PackagesVisitor {
-    type Value = Vec<Package>;
+impl<'de, 'a, const N: usize> Visitor<'de> for LockfileVisitor<'a, N> {
+    type Value = [Package<'a>; N];
 
     fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
-        formatter.write_str("a sequence")
+        formatter.write_str("struct Lockfile")
     }
 
-    fn visit_seq<A>(self, mut seq: A) -> std::result::Result<Self::Value, A::Error>
+    fn visit_map<A>(self, mut map: A) -> std::result::Result<Self::Value, A::Error>
     where
-        A: serde::de::SeqAccess<'de>,
+        A: serde::de::MapAccess<'de>,
     {
-        let mut result = Vec::new();
-        while let Some(package) = seq.next_element::<Package>()? {
-            if PACKAGE_NAMES.contains(&package.name.as_str()) {
-                result.push(package);
+        #[derive(Deserialize)]
+        #[serde(field_identifier, rename_all = "lowercase")]
+        enum Field {
+            Package,
+            Other(IgnoredAny),
+        }
+
+        while let Some(key) = map.next_key()? {
+            match key {
+                Field::Package => {
+                    return map.next_value_seed(PackagesSeed {
+                        dep_names: self.dep_names,
+                    })
+                }
+                Field::Other(_) => {
+                    map.next_value::<IgnoredAny>()?;
+                }
             }
         }
-        Ok(result)
+        Err(A::Error::missing_field("package"))
     }
 }
 
-impl Lockfile {
-    /// Read the `Cargo.lock` file for the crate at the given path.
-    pub fn new(crate_data: &CrateData) -> Result<Lockfile> {
-        let lock_path = get_lockfile_path(crate_data)?;
-        let lockfile = fs::read_to_string(&lock_path)
-            .with_context(|| anyhow!("failed to read: {}", lock_path.display()))?;
-        let lockfile = toml::from_str(&lockfile)
-            .with_context(|| anyhow!("failed to parse: {}", lock_path.display()))?;
-        Ok(lockfile)
-    }
+struct PackagesSeed<'a, const N: usize> {
+    dep_names: [&'a str; N],
+}
 
-    /// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
-    pub fn wasm_bindgen_version(&self) -> Option<&str> {
-        self.get_package_version("wasm-bindgen")
-    }
+impl<'de, 'a, const N: usize> DeserializeSeed<'de> for PackagesSeed<'a, N> {
+    type Value = [Package<'a>; N];
 
-    /// Like `wasm_bindgen_version`, except it returns an error instead of
-    /// `None`.
-    pub fn require_wasm_bindgen(&self) -> Result<&str> {
-        self.wasm_bindgen_version().ok_or_else(|| {
-            anyhow!(
-                "Ensure that you have \"{}\" as a dependency in your Cargo.toml file:\n\
-                 [dependencies]\n\
-                 wasm-bindgen = \"0.2\"",
-                style("wasm-bindgen").bold().dim(),
-            )
-        })
+    fn deserialize<D>(self, deserializer: D) -> std::result::Result<Self::Value, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        deserializer.deserialize_seq(self)
     }
+}
 
-    /// Get the version of `wasm-bindgen` dependency used in the `Cargo.lock`.
-    pub fn wasm_bindgen_test_version(&self) -> Option<&str> {
-        self.get_package_version("wasm-bindgen-test")
+impl<'de, 'a, const N: usize> Visitor<'de> for PackagesSeed<'a, N> {
+    type Value = [Package<'a>; N];
+
+    fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
+        formatter.write_str("a sequence")
     }
 
-    fn get_package_version(&self, package: &str) -> Option<&str> {
-        debug_assert!(PACKAGE_NAMES.contains(&package));
-        self.package
-            .iter()
-            .find(|p| p.name == package)
-            .map(|p| &p.version[..])
+    fn visit_seq<A>(self, mut seq: A) -> std::result::Result<Self::Value, A::Error>
+    where
+        A: serde::de::SeqAccess<'de>,
+    {
+        let mut packages = self.dep_names.map(|name| Package {
+            name,
+            version: None,
+        });
+        while let Some(PackageValue { name, version }) = seq.next_element()? {
+            #[cfg(test)]
+            assert_eq!(
+                (true, true),
+                (
+                    matches!(name, Cow::Borrowed(_)),
+                    matches!(version, Cow::Borrowed(_))
+                )
+            );
+            for package in &mut packages {
+                if package.name == name {
+                    package.version = Some(version.into_owned());
+                    if packages.iter().all(|i| i.version.is_some()) {
+                        return Ok(packages);
+                    } else {
+                        break;
+                    }
+                }
+            }
+        }
+        Ok(packages)
     }
 }
 
+#[derive(Deserialize)]
+struct PackageValue<'a> {
+    #[serde(borrow)]
+    name: Cow<'a, str>,
+    #[serde(borrow)]
+    version: Cow<'a, str>,
+}
+
 /// Given the path to the crate that we are building, return a `PathBuf`
 /// containing the location of the lock file, by finding the workspace root.
 fn get_lockfile_path(crate_data: &CrateData) -> Result<PathBuf> {

tests/all/lockfile.rs

@@ -1,23 +1,23 @@
 use crate::utils::fixture;
-use wasm_pack::lockfile::Lockfile;
+use wasm_pack::lockfile;
 use wasm_pack::manifest::CrateData;
 
 #[test]
 fn it_gets_wasm_bindgen_version() {
     let fixture = fixture::js_hello_world().build();
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path, None).unwrap();
-    let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.74"),);
+    let [package] = lockfile::Package::get(&data, ["wasm-bindgen"]).unwrap();
+    assert_eq!(package.version(), Some("0.2.74"),);
 }
 
 #[test]
 fn it_gets_wasm_bindgen_test_version() {
     let fixture = fixture::wbg_test_node().build();
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path, None).unwrap();
-    let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_test_version(), Some("0.3.24"),);
+    let [package] = lockfile::Package::get(&data, ["wasm-bindgen-test"]).unwrap();
+    assert_eq!(package.version(), Some("0.3.24"),);
 }
 
 #[test]
@@ -61,8 +61,8 @@ fn it_gets_wasm_bindgen_version_in_crate_inside_workspace() {
         .build();
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path.join("blah"), None).unwrap();
-    let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.74"),);
+    let [package] = lockfile::Package::get(&data, ["wasm-bindgen"]).unwrap();
+    assert_eq!(package.version(), Some("0.2.74"),);
 }
 
 #[test]
@@ -129,6 +129,6 @@ fn it_gets_wasm_bindgen_version_from_dependencies() {
         .build();
     fixture.cargo_check();
     let data = CrateData::new(&fixture.path.join("parent"), None).unwrap();
-    let lock = Lockfile::new(&data).unwrap();
-    assert_eq!(lock.wasm_bindgen_version(), Some("0.2.74"),);
+    let [package] = lockfile::Package::get(&data, ["wasm-bindgen"]).unwrap();
+    assert_eq!(package.version(), Some("0.2.74"),);
 }