From dee469e6db40960e8647544d0464c204f9eaa997 Mon Sep 17 00:00:00 2001
From: rsheeter <rsheeter@google.com>
Date: Wed, 8 Jan 2025 13:38:12 -0800
Subject: [PATCH] Tweak based on IM

---
 fea-rs/src/compile/compile_ctx.rs   | 77 +++++++++--------------------
 fea-rs/src/compile/validate.rs      |  2 +-
 fea-rs/src/parse/grammar/metrics.rs | 18 +++++++
 fea-rs/src/token_tree/typed.rs      | 11 ++++-
 4 files changed, 50 insertions(+), 58 deletions(-)

diff --git a/fea-rs/src/compile/compile_ctx.rs b/fea-rs/src/compile/compile_ctx.rs
index 1eb5dbe54..267f33e2d 100644
--- a/fea-rs/src/compile/compile_ctx.rs
+++ b/fea-rs/src/compile/compile_ctx.rs
@@ -1819,7 +1819,28 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
     }
 
     fn resolve_name_spec(&mut self, node: &typed::NameSpec) -> super::tables::NameSpec {
-        resolve_name_spec(node)
+        const WIN_DEFAULT_IDS: (u16, u16) = (1, 0x0409);
+        const MAC_DEFAULT_IDS: (u16, u16) = (0, 0);
+
+        let platform_id = node
+            .platform_id()
+            .map(|n| n.parse().unwrap())
+            .unwrap_or(tags::WIN_PLATFORM_ID);
+
+        let (encoding_id, language_id) = match node.platform_and_language_ids() {
+            Some((platform, language)) => (platform.parse().unwrap(), language.parse().unwrap()),
+            None => match platform_id {
+                tags::MAC_PLATFORM_ID => MAC_DEFAULT_IDS,
+                tags::WIN_PLATFORM_ID => WIN_DEFAULT_IDS,
+                _ => panic!("missed validation"),
+            },
+        };
+        super::tables::NameSpec {
+            platform_id,
+            encoding_id,
+            language_id,
+            string: node.string().into(),
+        }
     }
 
     fn resolve_lookup_ref(&mut self, lookup: typed::LookupRef) {
@@ -2119,43 +2140,6 @@ impl<'a, F: FeatureProvider, V: VariationInfo> CompilationCtx<'a, F, V> {
     }
 }
 
-// testing and free fn's, friends forever
-fn resolve_name_spec(node: &typed::NameSpec) -> super::tables::NameSpec {
-    const WIN_DEFAULT_IDS: (u16, u16) = (1, 0x0409);
-    const MAC_DEFAULT_IDS: (u16, u16) = (0, 0);
-
-    let platform_id = node
-        .platform_id()
-        .map(|n| n.parse().unwrap())
-        .unwrap_or(tags::WIN_PLATFORM_ID);
-
-    let (encoding_id, language_id) = match node.platform_and_language_ids() {
-        Some((platform, language)) => (platform.parse().unwrap(), language.parse().unwrap()),
-        None => match platform_id {
-            tags::MAC_PLATFORM_ID => MAC_DEFAULT_IDS,
-            tags::WIN_PLATFORM_ID => WIN_DEFAULT_IDS,
-            _ => panic!("missed validation"),
-        },
-    };
-
-    // Drop wrapping quotes around names if present, it confuses subsequent processing of string
-    let string = match &node.string().text {
-        quoted if quoted.starts_with('"') && quoted.ends_with('"') && quoted.len() > 1 => quoted
-            .strip_prefix('"')
-            .unwrap()
-            .strip_suffix('"')
-            .unwrap()
-            .into(),
-        unquoted => unquoted.clone(),
-    };
-    super::tables::NameSpec {
-        platform_id,
-        encoding_id,
-        language_id,
-        string,
-    }
-}
-
 fn sequence_enumerator(sequence: &[GlyphOrClass]) -> Vec<Vec<GlyphId16>> {
     assert!(sequence.len() >= 2);
     let split = sequence.split_first();
@@ -2238,21 +2222,4 @@ mod tests {
             ]
         );
     }
-
-    fn parse_name_spec(fea: &str) -> crate::compile::tables::NameSpec {
-        resolve_name_spec(&typed::NameSpec {
-            inner: crate::parse::parse_string(fea).0.root,
-        })
-    }
-
-    #[test]
-    fn parse_name_spec_drop_quotes() {
-        assert_eq!(
-            vec!["", "duck"],
-            vec![
-                parse_name_spec("3 1 0x409 \"\"").string,
-                parse_name_spec("3 1 0x409 \"duck\"").string
-            ]
-        );
-    }
 }
diff --git a/fea-rs/src/compile/validate.rs b/fea-rs/src/compile/validate.rs
index f4d9bffa2..09298aa43 100644
--- a/fea-rs/src/compile/validate.rs
+++ b/fea-rs/src/compile/validate.rs
@@ -486,7 +486,7 @@ impl<'a, V: VariationInfo> ValidationCtx<'a, V> {
 
         let platform = platform.unwrap_or(WIN_PLATFORM_ID);
 
-        if let Err((range, err)) = validate_name_string_encoding(platform, spec.string()) {
+        if let Err((range, err)) = validate_name_string_encoding(platform, spec.string_token()) {
             self.error(range, err);
         }
         if let Some((platspec, language)) = spec.platform_and_language_ids() {
diff --git a/fea-rs/src/parse/grammar/metrics.rs b/fea-rs/src/parse/grammar/metrics.rs
index 7faedea29..3217172e3 100644
--- a/fea-rs/src/parse/grammar/metrics.rs
+++ b/fea-rs/src/parse/grammar/metrics.rs
@@ -516,4 +516,22 @@ mod tests {
         assert!(valrecord.advance().is_none());
         assert!(valrecord.placement().is_some());
     }
+
+    #[test]
+    fn name_string_omits_quotes() {
+        let parse_name = |fea| {
+            let (token, _err, _) = debug_parse_output(fea, |parser| {
+                expect_name_record(parser, TokenSet::EMPTY);
+            });
+
+            crate::typed::NameSpec::cast(&token).unwrap()
+        };
+        assert_eq!(
+            ["", "duck"],
+            [
+                parse_name("3 1 0x409 \"\"").string(),
+                parse_name("3 1 0x409 \"duck\"").string(),
+            ]
+        );
+    }
 }
diff --git a/fea-rs/src/token_tree/typed.rs b/fea-rs/src/token_tree/typed.rs
index 931910ae3..8343dda73 100644
--- a/fea-rs/src/token_tree/typed.rs
+++ b/fea-rs/src/token_tree/typed.rs
@@ -89,7 +89,7 @@ macro_rules! ast_node {
         #[derive(Clone, Debug)]
         #[allow(missing_docs)]
         pub struct $typ {
-            pub(crate) inner: Node,
+            inner: Node,
         }
 
         impl $typ {
@@ -1487,9 +1487,16 @@ impl NameSpec {
         }
     }
 
-    pub(crate) fn string(&self) -> &Token {
+    pub(crate) fn string_token(&self) -> &Token {
+        // There is always a string
         self.find_token(Kind::String).unwrap()
     }
+
+    pub(crate) fn string(&self) -> &str {
+        // The value is always doublequoted so slice out the actual string
+        let s = self.string_token().as_str();
+        &s[1..s.len() - 1]
+    }
 }
 
 impl DecOctHex {