Split postscriptname checks (#80)

miguelsousa · Oct 8, 2023 · fa8793a · fa8793a
2 parents f530926 + f5a4e7a
commit fa8793a
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,7 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `com.google.fonts/check/valid_glyphnames`: The check now takes into account that OpenType-CFF2 fonts with `post` table format 3 contain no glyph names, and will yield SKIP (https://github.com/miguelsousa/openbakery/pull/38).
 - `com.google.fonts/check/unique_glyphnames`: The check now takes into account that OpenType-CFF2 fonts with `post` table format 3 contain no glyph names, and will yield SKIP (https://github.com/miguelsousa/openbakery/pull/38).
 - `com.google.fonts/check/STAT_in_statics`: The check now skips fonts that do not have a `STAT` table (https://github.com/miguelsousa/openbakery/pull/38).
-- `com.google.fonts/check/family_naming_recommendations`: Two validations of PostScript name were moved out of this check and into `com.adobe.fonts/check/postscript_name` which yields FAIL (https://github.com/miguelsousa/openbakery/pull/62).
+- `com.google.fonts/check/family_naming_recommendations`: Two validations of PostScript name were moved out of this check and into separate new checks `com.adobe.fonts/check/postscript_name_characters` and `com.adobe.fonts/postscript_name_hyphens` which yield FAIL (https://github.com/miguelsousa/openbakery/pull/62).
 - `com.google.fonts/check/cjk_not_enough_glyphs`: This check is now only run when a font has CJK codepages or ranges declared in the `OS/2` table. Other CJK-related checks are run on fonts with a minimum of 150 CJK glyphs (https://github.com/fonttools/fontbakery/issues/3846).
 - `com.google.fonts/check/family/panose_familytype` and `com.google.fonts/check/family/panose_proportion`: Failures have been downgraded to warnings (https://github.com/fonttools/fontbakery/issues/4192).
 

diff --git a/Lib/openbakery/profiles/adobefonts.py b/Lib/openbakery/profiles/adobefonts.py
@@ -26,7 +26,7 @@
 SET_EXPLICIT_CHECKS = {
     # This is the set of explict checks that will be invoked
     # when openbakery is run with the 'adobefonts' subcommand.
-    # The contents of this set were last updated on September 6, 2023.
+    # The contents of this set were last updated on October 6, 2023.
     #
     # =======================================
     # From adobefonts.py (this file)
@@ -132,9 +132,10 @@
     "com.adobe.fonts/check/family/max_4_fonts_per_family_name",
     "com.adobe.fonts/check/family/consistent_family_name",
     "com.adobe.fonts/check/name/empty_records",
-    "com.adobe.fonts/check/postscript_name",
     "com.adobe.fonts/check/name/postscript_name_consistency",
     "com.adobe.fonts/check/name/postscript_vs_cff",
+    "com.adobe.fonts/check/postscript_name_characters",
+    "com.adobe.fonts/check/postscript_name_hyphens",
     "com.google.fonts/check/family_naming_recommendations",
     "com.google.fonts/check/monospace",
     #

diff --git a/Lib/openbakery/profiles/name.py b/Lib/openbakery/profiles/name.py
@@ -419,48 +419,55 @@ def com_google_fonts_check_name_match_familyname_fullfont(ttFont):
 
 
 @check(
-    id="com.adobe.fonts/check/postscript_name",
+    id="com.adobe.fonts/check/postscript_name_characters",
     proposal="https://github.com/miguelsousa/openbakery/issues/62",
 )
-def com_adobe_fonts_check_postscript_name(ttFont):
-    """PostScript name follows OpenType specification requirements?"""
+def com_adobe_fonts_check_postscript_name_characters(ttFont):
+    """PostScript name contains only allowed characters a-ZA-Z and hyphen?"""
     import re
     from openbakery.utils import get_name_entry_strings
 
-    bad_entries = []
+    bad_entry_count = 0
 
     # <Postscript name> may contain only a-zA-Z0-9
-    # and one hyphen
     bad_psname = re.compile("[^A-Za-z0-9-]")
     for string in get_name_entry_strings(ttFont, NameID.POSTSCRIPT_NAME):
         if bad_psname.search(string):
-            bad_entries.append(
-                {
-                    "field": "PostScript Name",
-                    "value": string,
-                    "rec": ("May contain only a-zA-Z0-9 characters and a hyphen."),
-                }
+            bad_entry_count += 1
+            yield FAIL, Message(
+                "bad-psname-characters",
+                f"PostScript name {string} contains disallowed characters.",
             )
+
+    if bad_entry_count == 0:
+        yield PASS, Message(
+            "psname-characters-ok", "PostScript name contains only allowed characters."
+        )
+
+
+@check(
+    id="com.adobe.fonts/check/postscript_name_hyphens",
+    proposal="https://github.com/miguelsousa/openbakery/issues/62",
+)
+def com_adobe_fonts_check_postscript_name_hyphens(ttFont):
+    """PostScript name contains at most one hyphen?"""
+    from openbakery.utils import get_name_entry_strings
+
+    bad_entry_count = 0
+
+    # <Postscript name> may contain only one hyphen
+    for string in get_name_entry_strings(ttFont, NameID.POSTSCRIPT_NAME):
         if string.count("-") > 1:
-            bad_entries.append(
-                {
-                    "field": "Postscript Name",
-                    "value": string,
-                    "rec": ("May contain not more than a single hyphen."),
-                }
+            bad_entry_count += 1
+            yield FAIL, Message(
+                "bad-psname-hyphens",
+                f"PostScript name {string} contains more than one hyphen.",
             )
 
-    if len(bad_entries) > 0:
-        table = "| Field | Value | Recommendation |\n"
-        table += "|:----- |:----- |:-------------- |\n"
-        for bad in bad_entries:
-            table += "| {} | {} | {} |\n".format(bad["field"], bad["value"], bad["rec"])
-        yield FAIL, Message(
-            "bad-psname-entries",
-            f"PostScript name does not follow requirements:\n\n{table}",
+    if bad_entry_count == 0:
+        yield PASS, Message(
+            "psname-hyphens-ok", "PostScript name contains at most one hyphen."
         )
-    else:
-        yield PASS, Message("psname-ok", "PostScript name follows requirements.")
 
 
 @check(

diff --git a/Lib/openbakery/profiles/opentype.py b/Lib/openbakery/profiles/opentype.py
@@ -70,7 +70,8 @@
     "com.google.fonts/check/glyf_unused_data",
     "com.google.fonts/check/family_naming_recommendations",
     "com.google.fonts/check/maxadvancewidth",
-    "com.adobe.fonts/check/postscript_name",
+    "com.adobe.fonts/check/postscript_name_characters",
+    "com.adobe.fonts/check/postscript_name_hyphens",
     "com.google.fonts/check/points_out_of_bounds",
     "com.google.fonts/check/glyf_non_transformed_duplicate_components",
     "com.google.fonts/check/code_pages",

diff --git a/tests/profiles/adobefonts_test.py b/tests/profiles/adobefonts_test.py
@@ -61,7 +61,7 @@ def test_get_family_checks():
 def test_profile_check_set():
     """Confirm that the profile has the correct number of checks and the correct
     set of check IDs."""
-    assert len(SET_EXPLICIT_CHECKS) == 81
+    assert len(SET_EXPLICIT_CHECKS) == 82
     explicit_with_overrides = sorted(
         f"{check_id}{OVERRIDE_SUFFIX}" if check_id in OVERRIDDEN_CHECKS else check_id
         for check_id in SET_EXPLICIT_CHECKS

diff --git a/tests/profiles/name_test.py b/tests/profiles/name_test.py
@@ -648,35 +648,45 @@ def set_name(font, nameID, string):
     assert_results_contain(check(ttFont), FAIL, "bad-typographicsubfamilyname")
 
 
-def test_check_name_postscript():
-    check = CheckTester(opentype_profile, "com.adobe.fonts/check/postscript_name")
+def test_check_name_postscript_characters():
+    check = CheckTester(
+        opentype_profile, "com.adobe.fonts/check/postscript_name_characters"
+    )
 
-    # Test a font that has OK psname. Check should PASS.
+    # Test a font that has psname with allowed characters. Check should PASS.
     ttFont = TTFont(TEST_FILE("source-sans-pro/OTF/SourceSansPro-Bold.otf"))
-    assert_PASS(check(ttFont), "psname-ok")
+    assert_results_contain(check(ttFont), PASS, "psname-characters-ok")
 
-    # Change the PostScript name string to more than one hyphen. Should FAIL.
-    bad_ps_name = "more-than-one-hyphen".encode("utf-16-be")
+    # Change it to a string with disallowed characters. Should FAIL.
+    bad_ps_name = "(disallowed) characters".encode("utf-16-be")
     ttFont["name"].setName(
         bad_ps_name,
         NameID.POSTSCRIPT_NAME,
         PlatformID.WINDOWS,
         WindowsEncodingID.UNICODE_BMP,
         WindowsLanguageID.ENGLISH_USA,
     )
-    msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-entries")
-    assert "PostScript name does not follow requirements" in msg
-    assert "May contain not more than a single hyphen." in msg
+    msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-characters")
+    assert "name (disallowed) characters contains disallowed characters." in msg
+
+
+def test_check_name_postscript_hyphens():
+    check = CheckTester(
+        opentype_profile, "com.adobe.fonts/check/postscript_name_hyphens"
+    )
 
-    # Now change it to a string with illegal characters. Should FAIL.
-    bad_ps_name = "(illegal) characters".encode("utf-16-be")
+    # Test a font that has OK psname. Check should PASS.
+    ttFont = TTFont(TEST_FILE("source-sans-pro/OTF/SourceSansPro-Bold.otf"))
+    assert_results_contain(check(ttFont), PASS, "psname-hyphens-ok")
+
+    # Change the PostScript name string to more than one hyphen. Should FAIL.
+    bad_ps_name = "more-than-one-hyphen".encode("utf-16-be")
     ttFont["name"].setName(
         bad_ps_name,
         NameID.POSTSCRIPT_NAME,
         PlatformID.WINDOWS,
         WindowsEncodingID.UNICODE_BMP,
         WindowsLanguageID.ENGLISH_USA,
     )
-    msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-entries")
-    assert "PostScript name does not follow requirements" in msg
-    assert "May contain only a-zA-Z0-9 characters and a hyphen." in msg
+    msg = assert_results_contain(check(ttFont), FAIL, "bad-psname-hyphens")
+    assert "PostScript name more-than-one-hyphen contains more than one hyphen." in msg