From 4bf9efa4af7421d4b3d04553437fc35c403ff87b Mon Sep 17 00:00:00 2001
From: Sean Griffin <sean@seantheprogrammer.com>
Date: Sat, 7 Apr 2018 14:28:54 -0600
Subject: [PATCH] Allow feature names to begin with numbers

Diesel 1.2 had planned on renaming our `large-tables` and `huge-tables`
features to `32-column-tables` and `64-column-tables` respectively,
while also introducing the `128-column-tables` feature. This change was
made several months ago in Diesel. Cargo will happily accept those
as feature names, and resolve them properly from other crates.

However while publishing Diesel 1.2, I ran into a snag mid-release when
I realized that Cargo is incorrectly assuming that a feature name must
be the same as a crate name. I suspect this is an artifact of the fact
that feature names often are crate names (and perhaps in the past that
was the only form of feature?).

However, now they are an entirely separate thing, and we should allow
the same set of feature names that Cargo does. (As an aside, do we want
to apply the same 64 character limit that we apply to crate names to
feature names?)

Fixes #1329.
---
 src/models/krate.rs        | 19 ++++++++++++-------
 src/tests/krate.rs         | 11 ++++++-----
 src/views/krate_publish.rs | 18 +++++++++++++++++-
 3 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/models/krate.rs b/src/models/krate.rs
index 146faad160..b0e4a27c40 100644
--- a/src/models/krate.rs
+++ b/src/models/krate.rs
@@ -250,24 +250,29 @@ impl Crate {
     }
 
     fn valid_ident(name: &str) -> bool {
-        if name.is_empty() {
-            return false;
-        }
-        name.chars().next().unwrap().is_alphabetic()
+        Self::valid_feature_name(name)
+            && name.chars()
+                .nth(0)
+                .map(char::is_alphabetic)
+                .unwrap_or(false)
+    }
+
+    pub fn valid_feature_name(name: &str) -> bool {
+        !name.is_empty()
             && name.chars()
                 .all(|c| c.is_alphanumeric() || c == '_' || c == '-')
             && name.chars().all(|c| c.is_ascii())
     }
 
-    pub fn valid_feature_name(name: &str) -> bool {
+    pub fn valid_feature(name: &str) -> bool {
         let mut parts = name.split('/');
         match parts.next() {
-            Some(part) if !Crate::valid_ident(part) => return false,
+            Some(part) if !Crate::valid_feature_name(part) => return false,
             None => return false,
             _ => {}
         }
         match parts.next() {
-            Some(part) if !Crate::valid_ident(part) => return false,
+            Some(part) if !Crate::valid_feature_name(part) => return false,
             _ => {}
         }
         parts.next().is_none()
diff --git a/src/tests/krate.rs b/src/tests/krate.rs
index de46ad4aea..3726852489 100644
--- a/src/tests/krate.rs
+++ b/src/tests/krate.rs
@@ -753,11 +753,12 @@ fn new_krate_bad_name() {
 
 #[test]
 fn valid_feature_names() {
-    assert!(Crate::valid_feature_name("foo"));
-    assert!(!Crate::valid_feature_name(""));
-    assert!(!Crate::valid_feature_name("/"));
-    assert!(!Crate::valid_feature_name("%/%"));
-    assert!(Crate::valid_feature_name("a/a"));
+    assert!(Crate::valid_feature("foo"));
+    assert!(!Crate::valid_feature(""));
+    assert!(!Crate::valid_feature("/"));
+    assert!(!Crate::valid_feature("%/%"));
+    assert!(Crate::valid_feature("a/a"));
+    assert!(Crate::valid_feature("32-column-tables"));
 }
 
 #[test]
diff --git a/src/views/krate_publish.rs b/src/views/krate_publish.rs
index dd8f045fd3..051d6f16be 100644
--- a/src/views/krate_publish.rs
+++ b/src/views/krate_publish.rs
@@ -18,7 +18,7 @@ pub struct NewCrate {
     pub name: CrateName,
     pub vers: CrateVersion,
     pub deps: Vec<CrateDependency>,
-    pub features: HashMap<CrateName, Vec<Feature>>,
+    pub features: HashMap<FeatureName, Vec<Feature>>,
     pub authors: Vec<String>,
     pub description: Option<String>,
     pub homepage: Option<String>,
@@ -51,6 +51,8 @@ pub struct CategoryList(pub Vec<Category>);
 pub struct Category(pub String);
 #[derive(Serialize, Debug, Deref)]
 pub struct Feature(pub String);
+#[derive(PartialEq, Eq, Hash, Serialize, Debug, Deref)]
+pub struct FeatureName(pub String);
 
 #[derive(Serialize, Deserialize, Debug)]
 pub struct CrateDependency {
@@ -102,6 +104,20 @@ impl<'de> Deserialize<'de> for Keyword {
     }
 }
 
+impl<'de> Deserialize<'de> for FeatureName {
+    fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
+        let s = String::deserialize(d)?;
+        if !Crate::valid_feature_name(&s) {
+            let value = de::Unexpected::Str(&s);
+            let expected = "a valid feature name containing only letters, \
+                            numbers, hyphens, or underscores";
+            Err(de::Error::invalid_value(value, &expected))
+        } else {
+            Ok(FeatureName(s))
+        }
+    }
+}
+
 impl<'de> Deserialize<'de> for Feature {
     fn deserialize<D: Deserializer<'de>>(d: D) -> Result<Feature, D::Error> {
         let s = String::deserialize(d)?;