Merge opentype/fsselection_matches_macstyle into opentype/fsselection

Backporting from FontSpector.
(issue fonttools#4865)

Lib/fontbakery/checks/opentype/fsselection.py

@@ -1,74 +1,30 @@
+from fontbakery.constants import FsSelection, MacStyle
 from fontbakery.prelude import check, Message, FAIL
 
 
 @check(
     id="opentype/fsselection",
     conditions=["style"],
     rationale="""
-    The OS/2.fsSelection field is a bit field used to specify the stylistic
-    qualities of the font - in particular, it specifies to some operating
-    systems whether the font is italic (bit 0), bold (bit 5) or regular
-    (bit 6).
+        The OS/2.fsSelection field is a bit field used to specify the stylistic
+        qualities of the font - in particular, it specifies to some operating
+        systems whether the font is italic (bit 0), bold (bit 5) or regular
+        (bit 6).
 
-    This check verifies that the fsSelection field is set correctly for the
-    font style. For a family of static fonts created in GlyphsApp, this is
-    set by using the style linking checkboxes in the exports settings.
+        This check verifies that the fsSelection field is set correctly for the
+        font style. For a family of static fonts created in GlyphsApp, this is
+        set by using the style linking checkboxes in the exports settings.
+
+        Additionally, the bold and italic bits in OS/2.fsSelection must match the
+        bold and italic bits in head.macStyle per the OpenType spec.
     """,
-    proposal="https://github.com/fonttools/fontbakery/issues/4829",  # legacy check
+    proposal=[
+        "https://github.com/fonttools/fontbakery/issues/4829",  # legacy check
+        "https://github.com/fonttools/fontbakery/pull/2382",
+    ],
 )
 def check_fsselection(ttFont, style):
     """Checking OS/2 fsSelection value."""
-    from fontbakery.constants import RIBBI_STYLE_NAMES, STATIC_STYLE_NAMES, FsSelection
-    from fontbakery.utils import check_bit_entry
-
-    # Checking fsSelection REGULAR bit:
-    expected = "Regular" in style or (
-        style in STATIC_STYLE_NAMES
-        and style not in RIBBI_STYLE_NAMES
-        and "Italic" not in style
-    )
-    yield check_bit_entry(
-        ttFont,
-        "OS/2",
-        "fsSelection",
-        expected,
-        bitmask=FsSelection.REGULAR,
-        bitname="REGULAR",
-    )
-
-    # Checking fsSelection ITALIC bit:
-    expected = "Italic" in style
-    yield check_bit_entry(
-        ttFont,
-        "OS/2",
-        "fsSelection",
-        expected,
-        bitmask=FsSelection.ITALIC,
-        bitname="ITALIC",
-    )
-
-    # Checking fsSelection BOLD bit:
-    expected = style in ["Bold", "BoldItalic"]
-    yield check_bit_entry(
-        ttFont,
-        "OS/2",
-        "fsSelection",
-        expected,
-        bitmask=FsSelection.BOLD,
-        bitname="BOLD",
-    )
-
-
-@check(
-    id="opentype/fsselection_matches_macstyle",
-    rationale="""
-        The bold and italic bits in OS/2.fsSelection must match the bold and italic
-        bits in head.macStyle per the OpenType spec.
-    """,
-    proposal="https://github.com/fonttools/fontbakery/pull/2382",
-)
-def check_fsselection_matches_macstyle(ttFont):
-    """Check if OS/2 fsSelection matches head macStyle bold and italic bits."""
 
     # Check both OS/2 and head are present.
     missing_tables = False
@@ -81,23 +37,34 @@ def check_fsselection_matches_macstyle(ttFont):
     if missing_tables:
         return
 
-    from fontbakery.constants import FsSelection, MacStyle
+    bold_expected = style in set("Bold", "BoldItalic")
+    italic_expected = "Italic" in style
+    regular_expected = (not bold_expected) and (not italic_expected)
+    bold_seen = bool(ttFont["OS/2"].fsSelection & FsSelection.BOLD)
+    italic_seen = bool(ttFont["OS/2"].fsSelection & FsSelection.ITALIC)
+    regular_seen = bool(ttFont["OS/2"].fsSelection & FsSelection.REGULAR)
+
+    for flag, expected, label in [
+        (bold_seen, bold_expected, "Bold"),
+        (italic_seen, italic_expected, "Italic"),
+        (regular_seen, regular_expected, "Regular"),
+    ]:
+        if flag != expected:
+            yield FAIL, Message(
+                f"bad-{label.upper()}",
+                f"fsSelection {label} flag {flag}"
+                f" does not match font style {style}",
+            )
 
-    head_bold = (ttFont["head"].macStyle & MacStyle.BOLD) != 0
-    os2_bold = (ttFont["OS/2"].fsSelection & FsSelection.BOLD) != 0
-    if head_bold != os2_bold:
-        yield FAIL, Message(
-            "fsselection-macstyle-bold",
-            "The OS/2.fsSelection and head.macStyle bold settings do not match:\n\n"
-            f"* OS/2.fsSelection: BOLD is {'not ' if not os2_bold else ''}set\n"
-            f"* head.macStyle: BOLD is {'not ' if not head_bold else ''}set",
-        )
-    head_italic = (ttFont["head"].macStyle & MacStyle.ITALIC) != 0
-    os2_italic = (ttFont["OS/2"].fsSelection & FsSelection.ITALIC) != 0
-    if head_italic != os2_italic:
-        yield FAIL, Message(
-            "fsselection-macstyle-italic",
-            "The OS/2.fsSelection and head.macStyle italic settings do not match.\n\n"
-            f"* OS/2.fsSelection: ITALIC is {'not ' if not os2_italic else ''}set\n"
-            f"* head.macStyle: ITALIC is {'not ' if not head_italic else ''}set",
-        )
+    mac_bold = bool(ttFont["head"].macStyle & MacStyle.BOLD)
+    mac_italic = bool(ttFont["head"].macStyle & MacStyle.ITALIC)
+    for flag, expected, label in [
+        (bold_seen, mac_bold, "Bold"),
+        (italic_seen, mac_italic, "Italic"),
+    ]:
+        if flag != expected:
+            yield FAIL, Message(
+                f"fsselection-macstyle-{label.lower()}",
+                f"fsSelection {label} flag {flag}"
+                f" does not match macStyle {expected} flag",
+            )

Lib/fontbakery/legacy_checkids.py

@@ -219,7 +219,7 @@
     "com.google.fonts/check/family/underline_thickness":                    "opentype/family/underline_thickness",
     "com.google.fonts/check/font_version TODO: double-check this!":         "opentype/font_version",
     "com.google.fonts/check/fsselection":                                   "opentype/fsselection",
-    "com.adobe.fonts/check/fsselection_matches_macstyle":                   "opentype/fsselection_matches_macstyle",
+    "com.adobe.fonts/check/fsselection_matches_macstyle":                   "opentype/fsselection",
     "com.typenetwork/check/varfont/ital_range":                             "opentype/fvar/axis_ranges_correct",
     "com.google.fonts/check/varfont/slnt_range":                            "opentype/fvar/axis_ranges_correct",
     "com.google.fonts/check/varfont/wdth_valid_range":                      "opentype/fvar/axis_ranges_correct",

Lib/fontbakery/profiles/opentype.py

@@ -17,7 +17,6 @@
             "opentype/family/underline_thickness",
             "opentype/font_version",
             "opentype/fsselection",
-            "opentype/fsselection_matches_macstyle",
             "opentype/fvar/axis_ranges_correct",
             "opentype/fvar/regular_coords_correct",
             "opentype/gdef_mark_chars",

tests/test_checks_opentype_os2.py

@@ -14,7 +14,6 @@
     assert_results_contain,
     portable_path,
     TEST_FILE,
-    MockFont,
 )
 
 
@@ -176,10 +175,9 @@ def test_check_xavgcharwidth(check):
     assert_results_contain(check(test_font), FATAL, "missing-glyphs")
 
 
-@check_id("opentype/fsselection_matches_macstyle")
+@check_id("opentype/fsselection")
 def test_check_fsselection_matches_macstyle(check):
     """Check if OS/2 fsSelection matches head macStyle bold and italic bits."""
-
     from fontbakery.constants import FsSelection
 
     test_font_path = TEST_FILE("nunito/Nunito-Regular.ttf")
@@ -193,7 +191,7 @@ def test_check_fsselection_matches_macstyle(check):
     message = assert_results_contain(
         check(test_font), FAIL, "fsselection-macstyle-bold"
     )
-    assert "bold" in message
+    assert "Bold" in message
 
     # now turn off bold in OS/2.fsSelection so we can focus on italic
     test_font["OS/2"].fsSelection &= ~FsSelection.BOLD
@@ -203,7 +201,7 @@ def test_check_fsselection_matches_macstyle(check):
     message = assert_results_contain(
         check(test_font), FAIL, "fsselection-macstyle-italic"
     )
-    assert "italic" in message
+    assert "Italic" in message
 
 
 @check_id("opentype/family/bold_italic_unique_for_nameid1")
@@ -282,7 +280,6 @@ def test_check_vendor_id(check):
 @check_id("opentype/fsselection")
 def test_check_fsselection(check):
     """Checking OS/2 fsSelection value."""
-
     from fontbakery.constants import FsSelection
 
     ttFont = TTFont(TEST_FILE("cabin/Cabin-Regular.ttf"))
@@ -355,19 +352,23 @@ def test_check_fsselection(check):
         ],
     ]
 
-    for fsSelection_value, style, expected, expected_message in test_cases:
+    for fsSelection_value, style, expected, _expected_message in test_cases:
         ttFont["OS/2"].fsSelection = fsSelection_value
 
+        ttFont.reader.file.name = f"Test-{style}.ttf"
+        print(f"Testing {ttFont.reader.file.name}...")
         if expected == PASS:
+            results = list(check(ttFont))
+            # Only care about results which refer to the old FontBakery test here
+            results = [r for r in results if "fsselection" not in r.message.code]
             assert_PASS(
-                check(MockFont(ttFont=ttFont, style=style)),
+                results,
                 "with fsSelection:{fsSelection_value} style:{style}...",
             )
         else:
             message = assert_results_contain(
-                check(MockFont(ttFont=ttFont, style=style)),
+                check(ttFont),
                 FAIL,
                 expected,
                 f"with fsSelection:{fsSelection_value} style:{style}...",
             )
-            assert message == expected_message