italics_have_roman_counterparts

Author: simoncozens

fontspector-py/tests/test_checks_googlefonts.py

@@ -2396,53 +2396,52 @@ def test_check_fvar_instances__whats_going_on_here(check):  # TODO: REVIEW THIS.
     assert_PASS(check(ttFont), "with a good font...")
 
 
-@pytest.mark.skip("Check not ported yet.")
 @check_id("googlefonts/family/italics_have_roman_counterparts")
 def test_check_family_italics_have_roman_counterparts(check):
     """Ensure Italic styles have Roman counterparts."""
 
-    # The path used here, "some-crazy.path/", is meant to ensure
-    # that the parsing code does not get lost when trying to
-    # extract the style of a font file.
     fonts = [
-        "some-crazy.path/merriweather/Merriweather-BlackItalic.ttf",
-        "some-crazy.path/merriweather/Merriweather-Black.ttf",
-        "some-crazy.path/merriweather/Merriweather-BoldItalic.ttf",
-        "some-crazy.path/merriweather/Merriweather-Bold.ttf",
-        "some-crazy.path/merriweather/Merriweather-Italic.ttf",
-        "some-crazy.path/merriweather/Merriweather-LightItalic.ttf",
-        "some-crazy.path/merriweather/Merriweather-Light.ttf",
-        "some-crazy.path/merriweather/Merriweather-Regular.ttf",
+        TEST_FILE("merriweather/Merriweather-BlackItalic.ttf"),
+        TEST_FILE("merriweather/Merriweather-Black.ttf"),
+        TEST_FILE("merriweather/Merriweather-BoldItalic.ttf"),
+        TEST_FILE("merriweather/Merriweather-Bold.ttf"),
+        TEST_FILE("merriweather/Merriweather-Italic.ttf"),
+        TEST_FILE("merriweather/Merriweather-LightItalic.ttf"),
+        TEST_FILE("merriweather/Merriweather-Light.ttf"),
+        TEST_FILE("merriweather/Merriweather-Regular.ttf"),
     ]
 
-    assert_PASS(check([MockFont(file=font) for font in fonts]), "with a good family...")
+    assert_PASS(check(fonts), "with a good family...")
 
     fonts.pop(-1)  # remove the last one, which is the Regular
-    assert "some-crazy.path/merriweather/Merriweather-Regular.ttf" not in fonts
-    assert "some-crazy.path/merriweather/Merriweather-Italic.ttf" in fonts
     assert_results_contain(
         check(fonts),
         FAIL,
         "missing-roman",
         "with a family that has an Italic but lacks a Regular.",
     )
 
-    fonts.append("some-crazy.path/merriweather/MerriweatherItalic.ttf")
+    shutil.copy(
+        TEST_FILE("merriweather/Merriweather-Italic.ttf"),
+        TEST_FILE("merriweather/MerriweatherItalic.ttf"),
+    )
+    fonts.append(TEST_FILE("merriweather/MerriweatherItalic.ttf"))
     assert_results_contain(
         check(fonts),
         WARN,
         "bad-filename",
         "with a family that has a non-canonical italic filename.",
     )
+    os.unlink(TEST_FILE("merriweather/MerriweatherItalic.ttf"))
 
     # This check must also be able to deal with variable fonts!
     fonts = [
-        "cabinvfbeta/CabinVFBeta-Italic[wdth,wght].ttf",
-        "cabinvfbeta/CabinVFBeta[wdth,wght].ttf",
+        TEST_FILE("cabinvf/Cabin-Italic[wdth,wght].ttf"),
+        TEST_FILE("cabinvf/Cabin[wdth,wght].ttf"),
     ]
     assert_PASS(check(fonts), "with a good set of varfonts...")
 
-    fonts = ["cabinvfbeta/CabinVFBeta-Italic[wdth,wght].ttf"]
+    fonts = [TEST_FILE("cabinvf/Cabin-Italic[wdth,wght].ttf")]
     assert_results_contain(
         check(fonts),
         FAIL,

profile-googlefonts/src/checks/googlefonts/family/italics_have_roman_counterparts.rs

@@ -0,0 +1,74 @@
+use fontspector_checkapi::prelude::*;
+
+#[check(
+    id = "googlefonts/family/italics_have_roman_counterparts",
+    rationale = "
+        
+        For each font family on Google Fonts, every Italic style must have
+        a Roman sibling.
+
+        This kind of problem was first observed at [1] where the Bold style was
+        missing but BoldItalic was included.
+
+        [1] https://github.com/google/fonts/pull/1482
+    
+    ",
+    proposal = "https://github.com/fonttools/fontbakery/issues/1733",
+    title = "Ensure Italic styles have Roman counterparts.",
+    implementation = "all"
+)]
+fn italics_have_roman_counterparts(c: &TestableCollection, context: &Context) -> CheckFnResult {
+    let filenames: Vec<String> = c
+        .testables
+        .iter()
+        .map(|t| t.filename.to_string_lossy().to_string())
+        .filter(|x| x.ends_with(".ttf"))
+        .collect();
+    let italics: Vec<&String> = filenames
+        .iter()
+        .filter(|x| x.contains("Italic") && x.find("-") < x.find("Italic"))
+        .collect();
+    let mut missing_roman = vec![];
+    let mut problems = vec![];
+    for italic in italics {
+        if !italic.contains("-") {
+            problems.push(Status::warn(
+                "bad-filename",
+                &format!("Filename seems to be incorrect: '{}'", italic),
+            ));
+            continue;
+        }
+        #[allow(clippy::unwrap_used)] // We just tested for one
+        let after_hyphen = italic.split("-").last().unwrap();
+        let Some(style_from_filename) = after_hyphen.split(".").next() else {
+            problems.push(Status::warn(
+                "bad-filename",
+                &format!("Filename seems to be incorrect: '{}'", italic),
+            ));
+            continue;
+        };
+        let is_varfont = style_from_filename.contains("[");
+        let roman_counterpart = if style_from_filename == "Italic" {
+            if is_varfont {
+                italic.replace("-Italic", "")
+            } else {
+                italic.replace("Italic", "Regular")
+            }
+        } else {
+            italic.replace("-Italic", "")
+        };
+        if !filenames.contains(&roman_counterpart) {
+            missing_roman.push(italic);
+        }
+    }
+    if !missing_roman.is_empty() {
+        problems.push(Status::fail(
+            "missing-roman",
+            &format!(
+                "Italics missing a Roman counterpart:\n{}",
+                bullet_list(context, missing_roman)
+            ),
+        ));
+    }
+    return_result(problems)
+}

profile-googlefonts/src/checks/googlefonts/family/mod.rs

@@ -2,3 +2,5 @@ mod equal_codepoint_coverage;
 pub use equal_codepoint_coverage::equal_codepoint_coverage;
 mod has_license;
 pub use has_license::has_license;
+mod italics_have_roman_counterparts;
+pub use italics_have_roman_counterparts::italics_have_roman_counterparts;

profile-googlefonts/src/lib.rs

@@ -102,7 +102,7 @@ impl fontspector_checkapi::Plugin for GoogleFonts {
             .add_and_register_check(checks::googlefonts::description::valid_html)
             .add_section("Family Checks")
             .add_and_register_check(checks::googlefonts::family::equal_codepoint_coverage)
-            //            .add_and_register_check(checks::googlefonts::family::italics_have_roman_counterparts)
+            .add_and_register_check(checks::googlefonts::family::italics_have_roman_counterparts)
             //            .add_and_register_check(checks::googlefonts::family::tnum_horizontal_metrics)
             .add_section("Name table checks")
             .add_and_register_check(checks::googlefonts::name::family_name_compliance)