Merge pull request #2 from felipesanches/new_check_arabic_spacing_symbols

new check: arabic_spacing_symbols

Author: simoncozens

fontspector-checkapi/src/constants.rs

@@ -1 +1,20 @@
 // pub const RIBBI_STYLE_NAMES: [&str; 5] = ["Regular", "Italic", "Bold", "BoldItalic", "Bold Italic"];
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum GlyphClass {
+    Base,
+    Ligature,
+    Mark,
+    Component,
+}
+impl GlyphClass {
+    pub fn from_u16(class: u16) -> Option<Self> {
+        match class {
+            1 => Some(Self::Base),
+            2 => Some(Self::Ligature),
+            3 => Some(Self::Mark),
+            4 => Some(Self::Component),
+            _ => None,
+        }
+    }
+}

fontspector-checkapi/src/font.rs

@@ -1,5 +1,7 @@
-use crate::{filetype::FileTypeConvert, FileType, Testable};
+use crate::{constants::GlyphClass, filetype::FileTypeConvert, CheckError, FileType, Testable};
 use read_fonts::{
+    tables::cmap::Cmap,
+    tables::gdef::Gdef,
     tables::{os2::SelectionFlags, post::DEFAULT_GLYPH_NAMES},
     types::Version16Dot16,
     TableProvider,
@@ -66,7 +68,32 @@ impl TestFont<'_> {
         self.font().table_data(Tag::new(table)).is_some()
     }
 
-    pub fn get_os2_fsselection(&self) -> Result<SelectionFlags, Box<dyn Error>> {
+    pub fn get_cmap(&self) -> Result<Cmap, CheckError> {
+        let cmap = self
+            .font()
+            .cmap()
+            .map_err(|_| CheckError::Error("Font lacks a cmap table".to_string()))?;
+        Ok(cmap)
+    }
+
+    pub fn get_gdef(&self) -> Result<Gdef, CheckError> {
+        let gdef = self
+            .font()
+            .gdef()
+            .map_err(|_| CheckError::Error("Font lacks a GDEF table".to_string()))?;
+        Ok(gdef)
+    }
+
+    pub fn gdef_class(&self, glyph_id: GlyphId) -> Option<GlyphClass> {
+        self.get_gdef()
+            .ok()
+            .and_then(|gdef| gdef.glyph_class_def())?
+            .ok()
+            .map(|class_def| class_def.get(glyph_id))
+            .and_then(GlyphClass::from_u16)
+    }
+
+    pub fn get_os2_fsselection(&self) -> Result<SelectionFlags, CheckError> {
         let os2 = self.font().os2()?;
         Ok(os2.fs_selection())
     }

fontspector-checkapi/src/lib.rs

@@ -1,7 +1,7 @@
 #![deny(clippy::unwrap_used, clippy::expect_used)]
 mod check;
 mod checkresult;
-mod constants;
+pub mod constants;
 mod context;
 mod filetype;
 mod font;

profile-universal/src/checks/arabic_spacing_symbols.rs

@@ -0,0 +1,61 @@
+use fontspector_checkapi::constants::GlyphClass;
+use fontspector_checkapi::{prelude::*, testfont, FileTypeConvert};
+
+const ARABIC_SPACING_SYMBOLS: [u16; 17] = [
+    0xFBB2, // Dot Above
+    0xFBB3, // Dot Below
+    0xFBB4, // Two Dots Above
+    0xFBB5, // Two Dots Below
+    0xFBB6, // Three Dots Above
+    0xFBB7, // Three Dots Below
+    0xFBB8, // Three Dots Pointing Downwards Above
+    0xFBB9, // Three Dots Pointing Downwards Below
+    0xFBBA, // Four Dots Above
+    0xFBBB, // Four Dots Below
+    0xFBBC, // Double Vertical Bar Below
+    0xFBBD, // Two Dots Vertically Above
+    0xFBBE, // Two Dots Vertically Below
+    0xFBBF, // Ring
+    0xFBC0, // Small Tah Above
+    0xFBC1, // Small Tah Below
+    0xFBC2, // Wasla Above
+];
+
+#[check(
+    id = "com.google.fonts/check/arabic_spacing_symbols",
+    title = "Check that Arabic spacing symbols U+FBB2–FBC1 aren't classified as marks.",
+    rationale = "
+        Unicode has a few spacing symbols representing Arabic dots and other marks,
+        but they are purposefully not classified as marks.
+
+        Many fonts mistakenly classify them as marks, making them unsuitable
+        for their original purpose as stand-alone symbols to used in pedagogical
+        contexts discussing Arabic consonantal marks.
+    ",
+    proposal = "https://github.com/googlefonts/fontbakery/issues/4295"
+)]
+fn arabic_spacing_symbols(t: &Testable, _context: &Context) -> CheckFnResult {
+    let mut problems: Vec<Status> = vec![];
+    let f = testfont!(t);
+    let cmap = f.get_cmap()?;
+
+    for codepoint in ARABIC_SPACING_SYMBOLS {
+        if let Some(gid) = cmap.map_codepoint(codepoint) {
+            if f.gdef_class(gid) == Some(GlyphClass::Mark) {
+                problems.push(Status::fail(
+                    "gdef-mark",
+                    &format!(
+                        "U+{:04X} is defined in GDEF as a mark (class 3).",
+                        codepoint
+                    ),
+                ));
+            }
+        }
+    }
+
+    if problems.is_empty() {
+        Ok(Status::just_one_pass())
+    } else {
+        return_result(problems)
+    }
+}

profile-universal/src/checks/mod.rs

@@ -1,3 +1,4 @@
+pub mod arabic_spacing_symbols;
 pub mod bold_italic_unique;
 pub mod fvar;
 pub mod glyphnames;

profile-universal/src/lib.rs

@@ -6,6 +6,7 @@ pub struct Universal;
 
 impl fontspector_checkapi::Plugin for Universal {
     fn register(&self, cr: &mut Registry) -> Result<(), String> {
+        cr.register_check(checks::arabic_spacing_symbols::arabic_spacing_symbols);
         cr.register_check(checks::fvar::axis_ranges_correct);
         cr.register_check(checks::fvar::regular_coords_correct);
         cr.register_check(checks::glyphnames::valid_glyphnames);