Per code review, explicitly enumerate known scalars

font-codegen/src/lib.rs

@@ -1,6 +1,6 @@
 //! Generating types from the opentype spec
 
-use log::{debug};
+use log::debug;
 use quote::quote;
 
 mod error;

font-codegen/src/parsing.rs

@@ -1,6 +1,6 @@
 //! raw parsing code
 
-use std::{collections::HashMap, ops::Deref};
+use std::{collections::HashMap, ops::Deref, str::FromStr};
 
 use log::{debug, trace};
 use proc_macro2::{Span, TokenStream};
@@ -614,28 +614,63 @@ impl Parse for FieldType {
     }
 }
 
-fn is_wellknown_scalar(path: &syn::PathSegment) -> bool {
-    if !path.arguments.is_empty() {
-        return false;
+// https://learn.microsoft.com/en-us/typography/opentype/spec/otff#data-types
+// Offset(16,24,32) get special handling, not listed here
+// GlyphId and MajorMinor are *not* spec names for scalar but are captured here
+#[derive(Debug, PartialEq)]
+enum WellKnownScalar {
+    UInt8,
+    Int8,
+    UInt16,
+    Int16,
+    UInt24,
+    UInt32,
+    Int32,
+    Fixed,
+    FWord,
+    UFWord,
+    F2Dot14,
+    LongDateTime,
+    Tag,
+    Version16Dot16,
+    GlyphId,
+    MajorMinor,
+}
+
+impl FromStr for WellKnownScalar {
+    type Err = ();
+
+    // TODO(https://github.com/googlefonts/fontations/issues/84) use spec names
+    fn from_str(str: &str) -> Result<WellKnownScalar, ()> {
+        match str {
+            "u8" => Ok(WellKnownScalar::UInt8),
+            "i8" => Ok(WellKnownScalar::Int8),
+            "u16" => Ok(WellKnownScalar::UInt16),
+            "i16" => Ok(WellKnownScalar::Int16),
+            "u24" => Ok(WellKnownScalar::UInt24),
+            "u32" => Ok(WellKnownScalar::UInt32),
+            "i32" => Ok(WellKnownScalar::Int32),
+            "Fixed" => Ok(WellKnownScalar::Fixed),
+            "FWord" => Ok(WellKnownScalar::FWord),
+            "UFWord" => Ok(WellKnownScalar::UFWord),
+            "F2Dot14" => Ok(WellKnownScalar::F2Dot14),
+            "LongDateTime" => Ok(WellKnownScalar::LongDateTime),
+            "Tag" => Ok(WellKnownScalar::Tag),
+            "Version16Dot16" => Ok(WellKnownScalar::Version16Dot16),
+            "GlyphId" => Ok(WellKnownScalar::GlyphId),
+            "MajorMinor" => Ok(WellKnownScalar::MajorMinor),
+            _ => Err(()),
+        }
+    }
+}
+
+impl WellKnownScalar {
+    fn from_path(path: &syn::PathSegment) -> Result<WellKnownScalar, ()> {
+        if !path.arguments.is_empty() {
+            return Err(());
+        }
+        WellKnownScalar::from_str(path.ident.to_string().as_str())
     }
-    // TODO use spec types not Rust types
-    // TODO feels like this should be an enum, know it's size, etc
-    matches!(
-        path.ident.to_string().as_str(),
-        "u8" | "u16"
-            | "Uint24"
-            | "u32"
-            | "i16"
-            | "GlyphId"
-            | "Fixed"
-            | "FWord"
-            | "UfWord"
-            | "F2Dot14"
-            | "Tag"
-            | "MajorMinor"
-            | "LongDateTime"
-            | "Version16Dot16"
-    )
 }
 
 // TODO use an explicit declaration of user defined types and delete this
@@ -686,7 +721,7 @@ impl FieldType {
             }
         }
 
-        if is_wellknown_scalar(last) {
+        if WellKnownScalar::from_path(last).is_ok() {
             return Ok(FieldType::Scalar {
                 typ: last.ident.clone(),
             });
@@ -1055,14 +1090,14 @@ impl Items {
         Ok(())
     }
 
-    pub (crate) fn resolve_pending(&mut self) -> Result<(), syn::Error> {
+    pub(crate) fn resolve_pending(&mut self) -> Result<(), syn::Error> {
         // We should know what some stuff is now
         // In theory we could repeat resolution until we succeed or stop learning
         // but I don't think ever need that currently
         let known = build_type_map(self);
-    
+
         known.iter().for_each(|(k, v)| trace!("{} => {:?}", k, v));
-    
+
         // Try to resolve everything pending against the known world
         for item in &mut self.items {
             let fields = match item {