feat: Source fixing

Cargo.toml

@@ -54,6 +54,10 @@ kurbo = "0.11"                                     # For italic angle check
 
 regex = "1.10.6"
 
+# Source fixes
+norad = "0.16.0"
+glyphslib = { git = "https://github.com/simoncozens/glyphslib-rs" }
+
 [workspace.lints.clippy]
 unwrap_used = "deny"
 expect_used = "deny"

fontspector-checkapi/Cargo.toml

@@ -44,6 +44,8 @@ itertools = { workspace = true }
 
 kurbo = { workspace = true, optional = true }
 thiserror = "2.0.12"
+glyphslib.workspace = true
+norad.workspace = true
 
 [lints]
 workspace = true

fontspector-checkapi/src/check.rs

@@ -4,6 +4,7 @@ use crate::{
     context::Context,
     error::FontspectorError,
     prelude::FixFnResult,
+    source::SourceFile,
     status::CheckFnResult,
     testable::{TestableCollection, TestableType},
     CheckResult, Registry, Status, Testable,
@@ -50,6 +51,9 @@ pub enum CheckImplementation<'a> {
 /// The function signature for a hotfix function
 pub type HotfixFunction = dyn Fn(&mut Testable) -> FixFnResult;
 
+/// The function signature for a source fix function
+pub type FixSourceFunction = dyn Fn(&mut SourceFile) -> FixFnResult;
+
 #[derive(Clone)]
 /// A check definition
 pub struct Check<'a> {
@@ -66,7 +70,7 @@ pub struct Check<'a> {
     /// Function pointer implementing a hotfix to the binary file
     pub hotfix: Option<&'a HotfixFunction>,
     /// Function pointer implementing a hotfix to the font source file
-    pub fix_source: Option<&'a dyn Fn(&Testable) -> FixFnResult>,
+    pub fix_source: Option<&'a FixSourceFunction>,
     /// A registered file type that this check applies to
     pub applies_to: &'a str,
     /// Additional flags for the check

fontspector-checkapi/src/checkresult.rs

@@ -4,7 +4,7 @@ use serde::{ser::SerializeStruct, Serialize};
 
 use crate::{Check, CheckId, Status, StatusCode};
 
-#[derive(Debug, Clone, Serialize)]
+#[derive(Debug, Clone, Serialize, PartialEq)]
 /// The result of a fix operation.
 pub enum FixResult {
     /// A fix was available, but not requested

fontspector-checkapi/src/error.rs

@@ -1,4 +1,4 @@
-use std::sync::PoisonError;
+use std::{path::PathBuf, sync::PoisonError};
 
 use thiserror::Error;
 
@@ -38,6 +38,21 @@ pub enum FontspectorError {
         /// Additional details about the error
         more_details: String,
     },
+    /// A file was not found
+    #[error("File not found: {0}")]
+    FileNotFound(PathBuf),
+    /// Could not load a UFO file
+    #[error("Could not load UFO file: {0}")]
+    UfoLoad(Arc<norad::error::FontLoadError>),
+    /// Could not save a UFO file
+    #[error("Could not save UFO file: {0}")]
+    UfoSave(Arc<norad::error::FontWriteError>),
+    /// Could not load a designspace file
+    #[error("Could not load designspace file: {0}")]
+    DesignspaceLoad(Arc<norad::error::DesignSpaceLoadError>),
+    /// A file was not recognized as a source
+    #[error("Unrecognized source file: {0}")]
+    UnrecognizedSource(PathBuf),
     /// Something went wrong doing Python things
     #[error("Python error: {0}")]
     Python(String),
@@ -71,6 +86,14 @@ pub enum FontspectorError {
     /// Something else happened when fixing the font
     #[error("Something went wrong while fixing: {0}")]
     Fix(String),
+    /// Something else happened when saving the font
+    #[error("Something went wrong while saving {path}: {error}")]
+    SaveError {
+        /// The path to the file that could not be saved
+        path: PathBuf,
+        /// The error that occurred while saving
+        error: String,
+    },
 }
 
 impl From<std::string::FromUtf8Error> for FontspectorError {
@@ -97,6 +120,30 @@ impl<T> From<PoisonError<T>> for FontspectorError {
     }
 }
 
+use std::sync::Arc;
+impl From<norad::error::DesignSpaceLoadError> for FontspectorError {
+    fn from(err: norad::error::DesignSpaceLoadError) -> Self {
+        FontspectorError::DesignspaceLoad(Arc::new(err))
+    }
+}
+
+impl From<norad::error::FontLoadError> for FontspectorError {
+    fn from(err: norad::error::FontLoadError) -> Self {
+        FontspectorError::UfoLoad(Arc::new(err))
+    }
+}
+
+impl From<norad::error::FontWriteError> for FontspectorError {
+    fn from(err: norad::error::FontWriteError) -> Self {
+        FontspectorError::UfoSave(Arc::new(err))
+    }
+}
+impl From<Box<dyn std::error::Error>> for FontspectorError {
+    fn from(err: Box<dyn std::error::Error>) -> Self {
+        FontspectorError::General(err.to_string())
+    }
+}
+
 impl FontspectorError {
     /// Create a skip error with a code and message
     pub fn skip(code: &'static str, message: &'static str) -> Self {

fontspector-checkapi/src/lib.rs

@@ -37,13 +37,19 @@ pub mod pens;
 mod profile;
 /// The registry of checks and profiles
 mod registry;
+
+/// Source files which can be fixed
+pub mod source;
 /// Data structures representing the most basic elements of a check's result
 mod status;
 /// Wraps a file or "thing" to be tested
 mod testable;
 /// Common utility functions for check implementors
 mod utils;
-pub use check::{return_result, Check, CheckFlags, CheckId, CheckImplementation, HotfixFunction};
+pub use check::{
+    return_result, Check, CheckFlags, CheckId, CheckImplementation, FixSourceFunction,
+    HotfixFunction,
+};
 pub use checkresult::{CheckResult, FixResult};
 pub use context::Context;
 pub use error::FontspectorError;
@@ -55,6 +61,7 @@ pub use font::{
 pub use gsub::{GetSubstitutionMap, SubstitutionMap};
 pub use profile::{Override, Profile, ProfileBuilder};
 pub use registry::Registry;
+pub use source::{Source, SourceFile};
 pub use status::{CheckFnResult, Status, StatusCode, StatusList};
 pub use testable::{Testable, TestableCollection, TestableType};
 

fontspector-checkapi/src/source.rs

@@ -0,0 +1,159 @@
+use std::{
+    collections::HashMap,
+    ffi::OsStr,
+    path::{Path, PathBuf},
+};
+
+use glyphslib::Font;
+use norad::{designspace::DesignSpaceDocument, Font as UfoFont};
+
+use crate::{prelude::FixFnResult, FontspectorError};
+
+#[derive(Debug, Clone)]
+/// Our representation of a DesignSpace document, which contains multiple UFO sources.
+pub struct DesignSpace {
+    /// The DesignSpace document.
+    pub document: DesignSpaceDocument,
+    /// The source fonts
+    pub sources: HashMap<String, Box<UfoFont>>,
+}
+
+impl DesignSpace {
+    /// Load a DesignSpace document from a path.
+    pub fn load(path: &Path) -> Result<Self, FontspectorError> {
+        let document = DesignSpaceDocument::load(path)?;
+        let mut sources = HashMap::new();
+        for source in &document.sources {
+            let ufo_font = UfoFont::load(path.with_file_name(&source.filename))?;
+            sources.insert(source.filename.clone(), Box::new(ufo_font));
+        }
+        Ok(Self { document, sources })
+    }
+
+    /// Save the DesignSpace document to a path.
+    pub fn save(&self, path: &Path) -> Result<(), FontspectorError> {
+        self.document
+            .save(path)
+            .map_err(|e| FontspectorError::SaveError {
+                path: path.to_path_buf(),
+                error: e.to_string(),
+            })?;
+        // Now save all the UFO sources
+        for (name, font) in &self.sources {
+            font.save(path.with_file_name(name))?;
+        }
+        Ok(())
+    }
+
+    /// Apply to fix to each UFO source in the DesignSpace document.
+    pub fn apply_fix(&mut self, fix: &dyn Fn(&mut UfoFont) -> FixFnResult) -> FixFnResult {
+        let mut changed = false;
+        for font in self.sources.values_mut() {
+            if fix(font)? {
+                changed = true;
+            }
+        }
+        Ok(changed)
+    }
+}
+
+/// A source of a font, which can be a Glyphs file, a UFO file, or a DesignSpace document.
+#[derive(Debug)]
+pub enum Source {
+    /// A source that contains a Glyphs font.
+    ///
+    /// This may be either Glyphs 2 or Glyphs 3 format.
+    Glyphs(Box<Font>),
+    /// A source that contains a UFO font.
+    Ufo(Box<UfoFont>),
+    /// A source that contains a DesignSpace document.
+    Designspace(Box<DesignSpace>),
+}
+
+/// A source file that can be loaded (and saved) from a path.
+pub struct SourceFile {
+    /// The source of the font.
+    pub source: Source,
+    /// The path to the file, if available.
+    pub file: PathBuf,
+}
+
+impl SourceFile {
+    /// Load a source file from a given path.
+    pub fn new(path: &Path) -> Result<Self, FontspectorError> {
+        if !path.exists() {
+            return Err(FontspectorError::FileNotFound(path.to_path_buf()));
+        }
+        let ext = path
+            .extension()
+            .and_then(OsStr::to_str)
+            .ok_or_else(|| FontspectorError::UnrecognizedSource(path.to_path_buf()))?;
+        let source = match ext {
+            "designspace" => Ok(Source::Designspace(Box::new(DesignSpace::load(path)?))),
+            "ufo" => Ok(Source::Ufo(Box::new(UfoFont::load(path)?))),
+            "glyphs" | "glyphspackage" => Ok(Source::Glyphs(Box::new(Font::load(path)?))),
+            _ => Err(FontspectorError::UnrecognizedSource(path.to_path_buf())),
+        }?;
+        Ok(Self {
+            source,
+            file: path.to_path_buf(),
+        })
+    }
+
+    /// Returns the filename of the source file.
+    pub fn filename(&self) -> String {
+        // Displaying a PathBuf is a chore, let's have a method for it.
+        self.file
+            .file_name()
+            .and_then(OsStr::to_str)
+            .map_or_else(|| "unknown".to_string(), |s| s.to_string())
+    }
+
+    /// Saves the source file.
+    pub fn save(&self) -> Result<(), FontspectorError> {
+        match &self.source {
+            Source::Glyphs(font) => {
+                font.save(&self.file)
+                    .map_err(|e| FontspectorError::SaveError {
+                        path: self.file.clone(),
+                        error: e.to_string(),
+                    })
+            }
+            Source::Ufo(font) => Ok(font.save(&self.file)?),
+            Source::Designspace(doc) => {
+                doc.save(&self.file)
+                    .map_err(|e| FontspectorError::SaveError {
+                        path: self.file.clone(),
+                        error: e.to_string(),
+                    })
+            }
+        }
+    }
+}
+
+// Utility functions for source fixes
+
+/// Find or add a custom parameter in a Glyphs font.
+pub fn find_or_add_cp(
+    cps: &mut Vec<glyphslib::common::CustomParameter>,
+    name: &str,
+    value: glyphslib::Plist,
+) -> FixFnResult {
+    if let Some(cp) = cps.iter_mut().find(|cp| cp.name == name) {
+        if cp.value != value {
+            log::info!("Setting {name} custom parameter to {value:?} in Glyphs font.");
+            cp.value = value;
+            Ok(true)
+        } else {
+            Ok(false)
+        }
+    } else {
+        log::info!("Adding {name} custom parameter with value {value:?} in Glyphs font.");
+        cps.push(glyphslib::common::CustomParameter {
+            name: name.to_string(),
+            value,
+            disabled: false,
+        });
+        Ok(true)
+    }
+}

fontspector-checkapi/src/status.rs

@@ -9,14 +9,19 @@ use crate::{error::FontspectorError, Override};
 /// A severity level for a single check subresult
 pub enum StatusCode {
     /// Skip: the check didn't run because some condition was not met
+    #[cfg_attr(feature = "clap", value(alias("SKIP"), hide = true))]
     Skip,
     /// Pass: there's no problem here
+    #[cfg_attr(feature = "clap", value(alias("PASS"), hide = true))]
     Pass,
     /// Info: the check returned some useful information, but no problems
+    #[cfg_attr(feature = "clap", value(alias("INFO"), hide = true))]
     Info,
     /// Warn: a problem which should be manually reviewed
+    #[cfg_attr(feature = "clap", value(alias("WARN"), hide = true))]
     Warn,
     /// Fail: a problem materially affects the correctness of the font
+    #[cfg_attr(feature = "clap", value(alias("FAIL"), hide = true))]
     Fail,
     /// Error: something went wrong
     ///
@@ -25,6 +30,7 @@ pub enum StatusCode {
     /// parsed, even though we did our best to check for things. In
     /// other words, it's something so bad there's no point continuing
     /// with the check; it's equivalent to a Fontbakery FATAL.
+    #[cfg_attr(feature = "clap", value(alias("ERROR"), hide = true))]
     Error,
 }
 

fontspector-cli/src/args.rs

@@ -112,6 +112,32 @@ pub struct Args {
     #[clap(long, help_heading = "Fix problems")]
     pub fix_sources: bool,
 
+    #[arg(long, help_heading = "Fix problems", value_parser = parse_source_map)]
+    pub source_map: Vec<(String, String)>,
+
     /// Input files
     pub inputs: Vec<String>,
 }
+
+fn parse_source_map(s: &str) -> Result<(String, String), String> {
+    let parts: Vec<&str> = s.splitn(2, ':').collect();
+    if parts.len() != 2 {
+        return Err(format!(
+            "Invalid source map entry (should be binary_file.ttf:source.glyphs): {s}"
+        ));
+    }
+    #[allow(clippy::indexing_slicing)] // We know there are exactly two parts
+    if parts[0].ends_with(".glyphs")
+        || parts[0].ends_with(".glyphspackage")
+        || parts[0].ends_with(".ufo")
+        || parts[0].ends_with(".designspace")
+        || parts[1].ends_with(".ttf")
+        || parts[1].ends_with(".otf")
+    {
+        return Err(format!(
+            "Invalid source map key (the binary font should go on the left): {s}",
+        ));
+    }
+    #[allow(clippy::indexing_slicing)] // We know there are exactly two parts
+    Ok((parts[0].to_string(), parts[1].to_string()))
+}

fontspector-cli/src/configuration.rs

@@ -17,6 +17,8 @@ pub(crate) struct UserConfigurationFile {
 
     #[serde(flatten)]
     pub per_check_config: HashMap<CheckId, Value>,
+    #[serde(default)]
+    pub source_map: HashMap<String, String>,
 }
 
 pub(crate) fn load_configuration(args: &Args) -> UserConfigurationFile {