Dependency keys now have validation via Alias newtype.

src/commands/update.rs

@@ -1,5 +1,4 @@
 use std::collections::BTreeSet;
-use std::convert::TryInto;
 use std::path::PathBuf;
 use std::str::FromStr;
 use std::time::Duration;

src/lockfile.rs

@@ -9,6 +9,7 @@ use fs_err::File;
 use semver::Version;
 use serde::{Deserialize, Serialize};
 
+use crate::manifest::Alias;
 use crate::package_id;
 use crate::{
     manifest::Manifest, package_id::PackageId, package_name::PackageName, resolution::Resolve,
@@ -26,11 +27,8 @@ pub struct Lockfile {
 
 fn grab_dependencies(
     package_id: &PackageId,
-    dependencies: &BTreeMap<
-        package_id::PackageId,
-        BTreeMap<std::string::String, package_id::PackageId>,
-    >,
-) -> Vec<(String, PackageId)> {
+    dependencies: &BTreeMap<package_id::PackageId, BTreeMap<Alias, PackageId>>,
+) -> Vec<(Alias, PackageId)> {
     dependencies
         .get(package_id)
         .map(|dependencies| {
@@ -127,7 +125,7 @@ pub struct RegistryLockPackage {
     pub checksum: Option<String>,
 
     #[serde(default)]
-    pub dependencies: Vec<(String, PackageId)>,
+    pub dependencies: Vec<(Alias, PackageId)>,
 }
 
 #[derive(Debug, Serialize, Deserialize)]

src/manifest.rs

@@ -1,7 +1,10 @@
 use std::collections::BTreeMap;
+use std::convert::TryFrom;
+use std::fmt::{self, Display};
 use std::path::Path;
+use std::str::FromStr;
 
-use anyhow::Context;
+use anyhow::{ensure, Context};
 use semver::Version;
 use serde::{Deserialize, Serialize};
 
@@ -21,13 +24,13 @@ pub struct Manifest {
     pub place: PlaceInfo,
 
     #[serde(default)]
-    pub dependencies: BTreeMap<String, PackageReq>,
+    pub dependencies: BTreeMap<Alias, PackageReq>,
 
     #[serde(default)]
-    pub server_dependencies: BTreeMap<String, PackageReq>,
+    pub server_dependencies: BTreeMap<Alias, PackageReq>,
 
     #[serde(default)]
-    pub dev_dependencies: BTreeMap<String, PackageReq>,
+    pub dev_dependencies: BTreeMap<Alias, PackageReq>,
 }
 
 impl Manifest {
@@ -173,3 +176,125 @@ impl Realm {
         )
     }
 }
+
+/// This newtype struct represents a valid alias for a given dependency.
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Serialize, Deserialize)]
+#[serde(try_from = "String", into = "String")]
+pub struct Alias(String);
+
+impl Display for Alias {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(self.0.as_str())
+    }
+}
+
+impl Alias {
+    pub fn new<T>(input: T) -> anyhow::Result<Alias>
+    where
+        T: Into<String>,
+    {
+        let inner = input.into();
+        Self::is_valid(&inner)?;
+        Ok(Alias(inner))
+    }
+
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+
+    pub fn is_valid(alias: &str) -> anyhow::Result<()> {
+        // An empty alias is nonsensical (doesn't map to Lua well and doesn't map to the file system at all).
+        let is_not_empty = !alias.is_empty();
+        ensure!(is_not_empty, "Cannot have an empty alias.");
+
+        // The following two ensures that the alias is a valid Lua identifier.
+        let does_not_start_with_digit = !alias.chars().next().unwrap().is_ascii_digit();
+        let only_valid_chars = alias.chars().all(|c| c.is_ascii_alphanumeric() || c == '_');
+
+        // Windows' filesystem doesn't like the following names.
+        // https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces
+        let not_reserved_windows_name = !matches!(
+            alias,
+            "con"
+                | "prn"
+                | "aux"
+                | "nul"
+                | "com0"
+                | "com1"
+                | "com2"
+                | "com3"
+                | "com4"
+                | "com5"
+                | "com6"
+                | "com7"
+                | "com8"
+                | "com9"
+                | "lpt0"
+                | "lpt1"
+                | "lpt2"
+                | "lpt3"
+                | "lpt4"
+                | "lpt5"
+                | "lpt6"
+                | "lpt7"
+                | "lpt8"
+                | "lpt9"
+        );
+
+        ensure!(
+            does_not_start_with_digit,
+            "The alias {} is invalid since it starts with a digit."
+        );
+        ensure!(only_valid_chars, "The alias {} is invalid since it has invalid characters (can only have a-Z, A-Z, _, 0-9).");
+        ensure!(
+            not_reserved_windows_name,
+            "The alias {} is invalid since it matches a reserved Windows name."
+        );
+
+        Ok(())
+    }
+}
+
+impl FromStr for Alias {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> anyhow::Result<Self> {
+        Self::new(s)
+    }
+}
+
+impl TryFrom<String> for Alias {
+    type Error = anyhow::Error;
+
+    fn try_from(value: String) -> anyhow::Result<Self> {
+        Self::new(value)
+    }
+}
+
+impl From<Alias> for String {
+    fn from(value: Alias) -> Self {
+        value.0
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::Alias;
+
+    #[test]
+    fn reject_bad_aliases() {
+        assert!(
+            Alias::new(":").is_err(),
+            "consist only of valid ascii characters"
+        );
+        assert!(Alias::new("").is_err(), "empty alias is bad.");
+        assert!(
+            Alias::new("1hello").is_err(),
+            "starting with a digit grinds against lua"
+        );
+        assert!(
+            Alias::new("aux").is_err(),
+            "windows will reject this file name since it's reserved"
+        )
+    }
+}

src/resolution.rs

@@ -6,7 +6,7 @@ use anyhow::format_err;
 use semver::Version;
 use serde::Serialize;
 
-use crate::manifest::{Manifest, Realm};
+use crate::manifest::{Alias, Manifest, Realm};
 use crate::package_id::PackageId;
 use crate::package_req::PackageReq;
 use crate::package_source::{PackageSourceId, PackageSourceMap, PackageSourceProvider};
@@ -26,17 +26,17 @@ pub struct Resolve {
     pub metadata: BTreeMap<PackageId, ResolvePackageMetadata>,
 
     /// Graph of all dependencies originating from the "shared" dependency realm.
-    pub shared_dependencies: BTreeMap<PackageId, BTreeMap<String, PackageId>>,
+    pub shared_dependencies: BTreeMap<PackageId, BTreeMap<Alias, PackageId>>,
 
     /// Graph of all dependencies originating from the "server" dependency realm.
-    pub server_dependencies: BTreeMap<PackageId, BTreeMap<String, PackageId>>,
+    pub server_dependencies: BTreeMap<PackageId, BTreeMap<Alias, PackageId>>,
 
     /// Graph of all dependencies originating from the "dev" dependency realm.
-    pub dev_dependencies: BTreeMap<PackageId, BTreeMap<String, PackageId>>,
+    pub dev_dependencies: BTreeMap<PackageId, BTreeMap<Alias, PackageId>>,
 }
 
 impl Resolve {
-    fn activate(&mut self, source: PackageId, dep_name: String, dep_realm: Realm, dep: PackageId) {
+    fn activate(&mut self, source: PackageId, dep_name: Alias, dep_realm: Realm, dep: PackageId) {
         self.activated.insert(dep.clone());
 
         let dependencies = match dep_realm {
@@ -314,7 +314,7 @@ pub struct DependencyRequest {
     request_source: PackageId,
     request_realm: Realm,
     origin_realm: Realm,
-    package_alias: String,
+    package_alias: Alias,
     package_req: PackageReq,
 }
 

src/test_package.rs

@@ -59,23 +59,27 @@ impl PackageBuilder {
 
     pub fn with_dep<A, R>(mut self, alias: A, package_req: R) -> Self
     where
-        A: Into<String>,
+        A: AsRef<str>,
         R: AsRef<str>,
     {
         let req: PackageReq = package_req.as_ref().parse().expect("invalid PackageReq");
 
-        self.manifest.dependencies.insert(alias.into(), req);
+        self.manifest
+            .dependencies
+            .insert(alias.as_ref().parse().expect("invalid alias"), req);
         self
     }
 
     pub fn with_server_dep<A, R>(mut self, alias: A, package_req: R) -> Self
     where
-        A: Into<String>,
+        A: AsRef<str>,
         R: AsRef<str>,
     {
         let req: PackageReq = package_req.as_ref().parse().expect("invalid PackageReq");
 
-        self.manifest.server_dependencies.insert(alias.into(), req);
+        self.manifest
+            .server_dependencies
+            .insert(alias.as_ref().parse().expect("invalid alias"), req);
         self
     }