From 4dfea00356b5e0597f8293ce6b6ecb60c4569d89 Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <cosimo@anthrotype.com>
Date: Tue, 21 Nov 2017 11:42:28 +0100
Subject: [PATCH] [feaLib] report error with multiple runs of marked ' glyphs

As Martin Hosken reported in https://github.com/fonttools/fonttools/pull/1096,
feaLib currently incorrectly handles the case where a marked input
glyph sequence in a contextual chaining sub/pos rule is split into
multiple runs, rather than being a single continuous run of ' marked
glyphs.

The consensus there was to raise a syntax error like makeotf instead of
second-guessing and silently fixing it like fontforge does.
---
 Lib/fontTools/feaLib/parser.py |  8 ++++++++
 Tests/feaLib/parser_test.py    | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py
index 7902773f2..bb33006ae 100644
--- a/Lib/fontTools/feaLib/parser.py
+++ b/Lib/fontTools/feaLib/parser.py
@@ -340,6 +340,14 @@ class Parser(object):
                 self.expect_symbol_("'")
                 hasMarks = marked = True
             if marked:
+                if suffix:
+                    # makeotf also reports this as an error, while FontForge
+                    # silently inserts ' in all the intervening glyphs.
+                    # https://github.com/fonttools/fonttools/pull/1096
+                    raise FeatureLibError(
+                        "Unsupported contextual target sequence: at most "
+                        "one run of marked (') glyph/class names allowed",
+                        self.cur_token_location_)
                 glyphs.append(gc)
             elif glyphs:
                 suffix.append(gc)
diff --git a/Tests/feaLib/parser_test.py b/Tests/feaLib/parser_test.py
index 54814dc4d..614986525 100644
--- a/Tests/feaLib/parser_test.py
+++ b/Tests/feaLib/parser_test.py
@@ -1220,6 +1220,42 @@ class ParserTest(unittest.TestCase):
         self.assertEqual(sub.glyph, "f_f_i")
         self.assertEqual(sub.replacement, ("f", "f", "i"))
 
+    def test_split_marked_glyphs_runs(self):
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Unsupported contextual target sequence",
+            self.parse, "feature test{"
+                        "    ignore pos a' x x A';"
+                        "} test;")
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Unsupported contextual target sequence",
+            self.parse, "lookup shift {"
+                        "    pos a <0 -10 0 0>;"
+                        "    pos A <0 10 0 0>;"
+                        "} shift;"
+                        "feature test {"
+                        "    sub a' lookup shift x x A' lookup shift;"
+                        "} test;")
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Unsupported contextual target sequence",
+            self.parse, "feature test {"
+                        "    ignore sub a' x x A';"
+                        "} test;")
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Unsupported contextual target sequence",
+            self.parse, "lookup upper {"
+                        "    sub a by A;"
+                        "} upper;"
+                        "lookup lower {"
+                        "    sub A by a;"
+                        "} lower;"
+                        "feature test {"
+                        "    sub a' lookup upper x x A' lookup lower;"
+                        "} test;")
+
     def test_substitute_from(self):  # GSUB LookupType 3
         doc = self.parse("feature test {"
                          "  substitute a from [a.1 a.2 a.3];"