Merge a few checks into opentype/varfont/valid_nameids

Following FontSpector. New check: - opentype/varfont/valid_nameids Old checks: - opentype/varfont/valid_axis_nameid - opentype/varfont/valid_postscript_nameid - opentype/varfont/valid_subfamily_nameid On the OpenType profile. (issue fonttools#4865)
felipesanches · Nov 14, 2024 · 4e7c42e · 4e7c42e
1 parent 3b615af
commit 4e7c42e
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 145 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,8 +8,8 @@ A more detailed list of changes is available in the corresponding milestones for
 
 ### New checks
 #### On the OpenType profile
-  - **[opentype/fvar/axis_ranges_correct]:** Replaces **opentype/varfont/ital_range**, **opentype/varfont/slnt_range**, **opentype/varfont/wdth_valid_range and **opentype/varfont/wght_valid_range**
-
+  - **[opentype/fvar/axis_ranges_correct]:** Replaces **opentype/varfont/ital_range**, **opentype/varfont/slnt_range**, **opentype/varfont/wdth_valid_range** and **opentype/varfont/wght_valid_range**
+  - **[opentype/varfont/valid_nameids]:** Replaces **opentype/varfont/valid_axis_nameid**, **opentype/varfont/valid_postscript_nameid**, and **opentype/varfont/valid_subfamily_nameid**
 
 ### Changes to existing checks
 ### On the Universal profile

diff --git a/Lib/fontbakery/checks/opentype_varfont_valid_nameids.py b/Lib/fontbakery/checks/opentype_varfont_valid_nameids.py
@@ -2,45 +2,20 @@
 
 
 @check(
-    id="opentype/varfont/valid_axis_nameid",
+    id="opentype/varfont/valid_nameids",
     rationale="""
         According to the 'fvar' documentation in OpenType spec v1.9
         https://docs.microsoft.com/en-us/typography/opentype/spec/fvar
 
         The axisNameID field provides a name ID that can be used to obtain strings
         from the 'name' table that can be used to refer to the axis in application
         user interfaces. The name ID must be greater than 255 and less than 32768.
-    """,
-    conditions=["is_variable_font"],
-    proposal="https://github.com/fonttools/fontbakery/issues/3702",
-)
-def check_varfont_valid_axis_nameid(ttFont, has_name_table):
-    """Validates that the value of axisNameID used by each VariationAxisRecord
-    is greater than 255 and less than 32768."""
-
-    if not has_name_table:
-        yield FAIL, Message("lacks-table", "Font lacks 'name' table.")
-        return
-
-    name_table = ttFont["name"]
-    font_axis_nameids = [axis.axisNameID for axis in ttFont["fvar"].axes]
-    invalid_axis_nameids = [val for val in font_axis_nameids if not (255 < val < 32768)]
-
-    for nameid in invalid_axis_nameids:
-        inst_name = name_table.getDebugName(nameid) or "Unnamed"
-
-        yield FAIL, Message(
-            f"invalid-axis-nameid:{nameid}",
-            f"{inst_name!r} instance has an axisNameID value that"
-            " is not greater than 255 and less than 32768.",
-        )
-
 
-@check(
-    id="opentype/varfont/valid_subfamily_nameid",
-    rationale="""
-        According to the 'fvar' documentation in OpenType spec v1.9
-        https://docs.microsoft.com/en-us/typography/opentype/spec/fvar
+        The postScriptNameID field provides a name ID that can be used to obtain
+        strings from the 'name' table that can be treated as equivalent to name
+        ID 6 (PostScript name) strings for the given instance. Values of 6 and
+        "undefined" can be used; otherwise, values must be greater than 255 and
+        less than 32768.
 
         The subfamilyNameID field provides a name ID that can be used to obtain
         strings from the 'name' table that can be treated as equivalent to name
@@ -49,72 +24,57 @@ def check_varfont_valid_axis_nameid(ttFont, has_name_table):
         than 32768.
     """,
     conditions=["is_variable_font"],
-    proposal="https://github.com/fonttools/fontbakery/issues/3703",
+    proposal=[
+        "https://github.com/fonttools/fontbakery/issues/3702",
+        "https://github.com/fonttools/fontbakery/issues/3703",
+    ],
 )
-def check_varfont_valid_subfamily_nameid(ttFont, has_name_table):
-    """Validates that the value of subfamilyNameID used by each InstanceRecord
-    is 2, 17, or greater than 255 and less than 32768."""
-
-    if not has_name_table:
-        yield FAIL, Message("lacks-table", "Font lacks 'name' table.")
-        return
+def check_valid_nameids(ttFont, has_name_table):
+    """Validates that all of the name IDs in an instance record
+    are within the correct range"""
 
-    name_table = ttFont["name"]
-    font_subfam_nameids = [inst.subfamilyNameID for inst in ttFont["fvar"].instances]
-    invalid_subfam_nameids = [
-        val
-        for val in font_subfam_nameids
-        if not (255 < val < 32768) and val not in {2, 17}
-    ]
+    font_axis_nameids = [axis.axisNameID for axis in ttFont["fvar"].axes]
+    invalid_axis_nameids = [val for val in font_axis_nameids if not (255 < val < 32768)]
 
-    for nameid in invalid_subfam_nameids:
-        inst_name = name_table.getDebugName(nameid) or "Unnamed"
+    for nameid in invalid_axis_nameids:
+        name = ("name" in ttFont and ttFont["name"].getDebugName(nameid)) or "Unnamed"
 
         yield FAIL, Message(
-            f"invalid-subfamily-nameid:{nameid}",
-            f"{inst_name!r} instance has a subfamilyNameID value that"
-            " is neither 2, 17, or greater than 255 and less than 32768.",
+            f"invalid-axis-nameid:{nameid}",
+            f"Axis name ID {nameid} ({name}) is out of range."
+            f" It must be greater than 255 and less than 32768.",
         )
 
-
-@check(
-    id="opentype/varfont/valid_postscript_nameid",
-    rationale="""
-        According to the 'fvar' documentation in OpenType spec v1.9
-        https://docs.microsoft.com/en-us/typography/opentype/spec/fvar
-
-        The postScriptNameID field provides a name ID that can be used to obtain
-        strings from the 'name' table that can be treated as equivalent to name
-        ID 6 (PostScript name) strings for the given instance. Values of 6 and
-        0xFFFF can be used; otherwise, values must be greater than 255 and less
-        than 32768.
-    """,
-    conditions=["is_variable_font"],
-    proposal="https://github.com/fonttools/fontbakery/issues/3704",
-)
-def check_varfont_valid_postscript_nameid(ttFont, has_name_table):
-    """Validates that the value of postScriptNameID used by each InstanceRecord
-    is 6, 0xFFFF, or greater than 255 and less than 32768."""
-
-    if not has_name_table:
-        yield FAIL, Message("lacks-table", "Font lacks 'name' table.")
-        return
-
-    name_table = ttFont["name"]
     font_postscript_nameids = [
         inst.postscriptNameID for inst in ttFont["fvar"].instances
     ]
     invalid_postscript_nameids = [
         val
         for val in font_postscript_nameids
-        if not (255 < val < 32768) and val not in {6, 0xFFFF}
+        if val not in [6, 0xFFFF] and not (255 < val < 32768)
     ]
 
     for nameid in invalid_postscript_nameids:
-        inst_name = name_table.getDebugName(nameid) or "Unnamed"
+        name = ("name" in ttFont and ttFont["name"].getDebugName(nameid)) or "Unnamed"
 
         yield FAIL, Message(
             f"invalid-postscript-nameid:{nameid}",
-            f"{inst_name!r} instance has a postScriptNameID value that"
-            f" is neither 6, 0xFFFF, or greater than 255 and less than 32768.",
+            f"PostScript name ID {nameid} ({name}) is out of range."
+            f" It must be greater than 255 and less than 32768, or 6 or 0xFFFF.",
+        )
+
+    font_subfam_nameids = [inst.subfamilyNameID for inst in ttFont["fvar"].instances]
+    invalid_subfam_nameids = [
+        val
+        for val in font_subfam_nameids
+        if not (255 < val < 32768) and val not in {2, 17}
+    ]
+
+    for nameid in invalid_subfam_nameids:
+        name = ("name" in ttFont and ttFont["name"].getDebugName(nameid)) or "Unnamed"
+
+        yield FAIL, Message(
+            f"invalid-subfamily-nameid:{nameid}",
+            f"Instance subfamily name ID {nameid} ({name}) is out of range."
+            f" It must be greater than 255 and less than 32768.",
         )
diff --git a/Lib/fontbakery/profiles/opentype.py b/Lib/fontbakery/profiles/opentype.py
@@ -56,10 +56,8 @@
             "opentype/varfont/regular_wght_coord",
             "opentype/varfont/same_size_instance_records",
             "opentype/varfont/stat_axis_record_for_each_axis",
-            "opentype/varfont/valid_axis_nameid",
             "opentype/varfont/valid_default_instance_nameids",
-            "opentype/varfont/valid_postscript_nameid",
-            "opentype/varfont/valid_subfamily_nameid",
+            "opentype/varfont/valid_nameids",
             "opentype/vendor_id",
             "opentype/weight_class_fvar",
             "opentype/xavgcharwidth",

diff --git a/tests/test_checks_opentype_varfont_valid_nameids.py b/tests/test_checks_opentype_varfont_valid_nameids.py
@@ -8,10 +8,11 @@
 )
 
 
-@check_id("opentype/varfont/valid_axis_nameid")
-def test_check_varfont_valid_axis_nameid(check):
-    """The value of axisNameID used by each VariationAxisRecord must
-    be greater than 255 and less than 32768."""
+@check_id("opentype/varfont/valid_nameids")
+def test_check_varfont_valid_nameids(check):
+    ####
+    # The value of axisNameID used by each VariationAxisRecord must
+    # be greater than 255 and less than 32768.
 
     # The axisNameID values in the reference varfont are all valid
     ttFont = TTFont("data/test/cabinvf/Cabin[wdth,wght].ttf")
@@ -29,33 +30,21 @@ def test_check_varfont_valid_axis_nameid(check):
     # Change the axes' axisNameID to invalid values
     # (32768 is greater than the maximum, and 255 is less than the minimum)
     wght_axis.axisNameID = 32768
-    wdth_axis.axisNameID = 255
     assert_results_contain(check(ttFont), FAIL, "invalid-axis-nameid:32768")
-    msg = assert_results_contain(check(ttFont), FAIL, "invalid-axis-nameid:255")
-    assert msg == (
-        "'Unnamed' instance has an axisNameID value that"
-        " is not greater than 255 and less than 32768."
-    )
+
+    wdth_axis.axisNameID = 255
+    assert_results_contain(check(ttFont), FAIL, "invalid-axis-nameid:255")
 
     # Another set of invalid values
     wght_axis.axisNameID = 128
-    wdth_axis.axisNameID = 36000
     assert_results_contain(check(ttFont), FAIL, "invalid-axis-nameid:128")
-    msg = assert_results_contain(check(ttFont), FAIL, "invalid-axis-nameid:36000")
-    assert msg == (
-        "'Unnamed' instance has an axisNameID value that"
-        " is not greater than 255 and less than 32768."
-    )
-
-    # Confirm the check yields FAIL if the font doesn't have a required table
-    del ttFont["name"]
-    assert_results_contain(check(ttFont), FAIL, "lacks-table")
 
+    wdth_axis.axisNameID = 36000
+    assert_results_contain(check(ttFont), FAIL, "invalid-axis-nameid:36000")
 
-@check_id("opentype/varfont/valid_postscript_nameid")
-def test_check_varfont_valid_postscript_nameid(check):
-    """The value of postScriptNameID used by each InstanceRecord must
-    be 6, 0xFFFF, or greater than 255 and less than 32768."""
+    ####
+    # The value of postScriptNameID used by each InstanceRecord must
+    # be 6, 0xFFFF, or greater than 255 and less than 32768.
 
     # The postScriptNameID values in the reference varfont are all valid
     ttFont = TTFont("data/test/cabinvf/Cabin[wdth,wght].ttf")
@@ -78,36 +67,24 @@ def test_check_varfont_valid_postscript_nameid(check):
     # Change two instances' postScriptNameID to invalid values
     # (32768 is greater than the maximum, and 255 is less than the minimum)
     inst_3.postscriptNameID = 255
-    inst_4.postscriptNameID = 32768
     assert_results_contain(check(ttFont), FAIL, "invalid-postscript-nameid:255")
-    msg = assert_results_contain(check(ttFont), FAIL, "invalid-postscript-nameid:32768")
-    assert msg == (
-        "'Unnamed' instance has a postScriptNameID value that"
-        " is neither 6, 0xFFFF, or greater than 255 and less than 32768."
-    )
+
+    inst_4.postscriptNameID = 32768
+    assert_results_contain(check(ttFont), FAIL, "invalid-postscript-nameid:32768")
 
     # Reset two postScriptNameID to valid values,
     # then set two other postScriptNameID to invalid values
     inst_3.postscriptNameID = 256  # valid
     inst_4.postscriptNameID = 32767  # valid
     inst_1.postscriptNameID = 3
-    inst_2.postscriptNameID = 18
     assert_results_contain(check(ttFont), FAIL, "invalid-postscript-nameid:3")
-    msg = assert_results_contain(check(ttFont), FAIL, "invalid-postscript-nameid:18")
-    assert msg == (
-        "'Unnamed' instance has a postScriptNameID value that"
-        " is neither 6, 0xFFFF, or greater than 255 and less than 32768."
-    )
-
-    # Confirm the check yields FAIL if the font doesn't have a required table
-    del ttFont["name"]
-    assert_results_contain(check(ttFont), FAIL, "lacks-table")
 
+    inst_2.postscriptNameID = 18
+    assert_results_contain(check(ttFont), FAIL, "invalid-postscript-nameid:18")
 
-@check_id("opentype/varfont/valid_subfamily_nameid")
-def test_check_varfont_valid_subfamily_nameid(check):
-    """The value of subfamilyNameID used by each InstanceRecord must
-    be 2, 17, or greater than 255 and less than 32768."""
+    ####
+    # The value of subfamilyNameID used by each InstanceRecord must
+    # be 2, 17, or greater than 255 and less than 32768.
 
     # The subfamilyNameID values in the reference varfont are all valid
     ttFont = TTFont("data/test/cabinvf/Cabin[wdth,wght].ttf")
@@ -130,27 +107,17 @@ def test_check_varfont_valid_subfamily_nameid(check):
     # Change two instances' subfamilyNameID to invalid values
     # (32768 is greater than the maximum, and 255 is less than the minimum)
     inst_3.subfamilyNameID = 255
-    inst_4.subfamilyNameID = 32768
     assert_results_contain(check(ttFont), FAIL, "invalid-subfamily-nameid:255")
-    msg = assert_results_contain(check(ttFont), FAIL, "invalid-subfamily-nameid:32768")
-    assert msg == (
-        "'Unnamed' instance has a subfamilyNameID value that"
-        " is neither 2, 17, or greater than 255 and less than 32768."
-    )
+
+    inst_4.subfamilyNameID = 32768
+    assert_results_contain(check(ttFont), FAIL, "invalid-subfamily-nameid:32768")
 
     # Reset two subfamilyNameID to valid values,
     # then set two other subfamilyNameID to invalid values
     inst_3.subfamilyNameID = 256  # valid
     inst_4.subfamilyNameID = 32767  # valid
     inst_1.subfamilyNameID = 3
-    inst_2.subfamilyNameID = 18
     assert_results_contain(check(ttFont), FAIL, "invalid-subfamily-nameid:3")
-    msg = assert_results_contain(check(ttFont), FAIL, "invalid-subfamily-nameid:18")
-    assert msg == (
-        "'Unnamed' instance has a subfamilyNameID value that"
-        " is neither 2, 17, or greater than 255 and less than 32768."
-    )
-
-    # Confirm the check yields FAIL if the font doesn't have a required table
-    del ttFont["name"]
-    assert_results_contain(check(ttFont), FAIL, "lacks-table")
+
+    inst_2.subfamilyNameID = 18
+    assert_results_contain(check(ttFont), FAIL, "invalid-subfamily-nameid:18")