From 8e8a0d68c7cf2e6e778daa05ae63ffe6c760f4b8 Mon Sep 17 00:00:00 2001
From: Sascha Brawer <sascha@brawer.ch>
Date: Mon, 7 Sep 2015 13:33:44 +0200
Subject: [PATCH] [feaLib] Prohibit `script` and `language` statements within
 named lookups

---
 Lib/fontTools/feaLib/ast.py          |  6 ++++++
 Lib/fontTools/feaLib/builder.py      | 30 +++++++++++++++++++++++++++-
 Lib/fontTools/feaLib/builder_test.py | 20 +++++++++++++++++++
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/Lib/fontTools/feaLib/ast.py b/Lib/fontTools/feaLib/ast.py
index 401c4cb60..17e2f7f1d 100644
--- a/Lib/fontTools/feaLib/ast.py
+++ b/Lib/fontTools/feaLib/ast.py
@@ -42,6 +42,12 @@ class LookupBlock(Block):
         Block.__init__(self, location)
         self.name, self.use_extension = name, use_extension
 
+    def build(self, builder):
+        # TODO(sascha): Handle use_extension.
+        builder.start_lookup_block(self.location, self.name)
+        Block.build(self, builder)
+        builder.end_lookup_block()
+
 
 class GlyphClassDefinition(Statement):
     def __init__(self, location, name, glyphs):
diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py
index 8e9974b24..701618482 100644
--- a/Lib/fontTools/feaLib/builder.py
+++ b/Lib/fontTools/feaLib/builder.py
@@ -18,7 +18,9 @@ class Builder(object):
         self.script_ = None
         self.lookup_flag_ = 0
         self.language_systems = set()
+        self.named_lookups_ = {}
         self.cur_lookup_ = None
+        self.cur_lookup_name_ = None
         self.lookups_ = []
 
     def build(self):
@@ -28,10 +30,14 @@ class Builder(object):
         self.gsub = self.font['GSUB'] = self.makeTable('GSUB')
 
     def get_lookup_(self, location, builder_class):
-        if self.cur_lookup_ and type(self.cur_lookup_) == builder_class:
+        if (self.cur_lookup_ and
+                type(self.cur_lookup_) == builder_class and
+                self.cur_lookup_.lookup_flag == self.lookup_flag_):
             return self.cur_lookup_
         self.cur_lookup_ = builder_class(location, self.lookup_flag_)
         self.lookups_.append(self.cur_lookup_)
+        if self.cur_lookup_name_:
+            self.named_lookups_[self.cur_lookup_name_] = self.cur_lookup_
         return self.cur_lookup_
 
     def makeTable(self, tag):
@@ -88,12 +94,30 @@ class Builder(object):
 
     def start_feature(self, location, name):
         self.language_systems = self.get_default_language_systems_()
+        self.cur_lookup_ = None
 
     def end_feature(self):
         self.language_systems = None
         self.cur_lookup_ = None
 
+    def start_lookup_block(self, location, name):
+        if name in self.named_lookups_:
+            raise FeatureLibError(
+                'Lookup "%s" has already been defined' % name, location)
+        self.cur_lookup_name_ = name
+        self.named_lookups_[name] = None
+        self.cur_lookup_ = None
+
+    def end_lookup_block(self):
+        assert self.cur_lookup_name_ is not None
+        self.cur_lookup_name_ = None
+        self.cur_lookup_ = None
+
     def set_language(self, location, language, include_default):
+        if self.cur_lookup_name_:
+            raise FeatureLibError(
+                "Within a named lookup block, it is not allowed "
+                "to change the language", location)
         self.cur_lookup_ = None
         if include_default:
             langsys = set(self.get_default_language_systems_())
@@ -103,6 +127,10 @@ class Builder(object):
         self.language_systems = frozenset(langsys)
 
     def set_script(self, location, script):
+        if self.cur_lookup_name_:
+            raise FeatureLibError(
+                "Within a named lookup block, it is not allowed "
+                "to change the script", location)
         self.cur_lookup_ = None
         self.script_ = script
         self.lookup_flag_ = 0
diff --git a/Lib/fontTools/feaLib/builder_test.py b/Lib/fontTools/feaLib/builder_test.py
index 572baaa53..0d91f9746 100644
--- a/Lib/fontTools/feaLib/builder_test.py
+++ b/Lib/fontTools/feaLib/builder_test.py
@@ -120,6 +120,13 @@ class BuilderTest(unittest.TestCase):
         self.assertEqual(builder.language_systems,
                          {('DFLT', 'dflt'), ('cyrl', 'dflt')})
 
+    def test_script_in_lookup_block(self):
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Within a named lookup block, it is not allowed "
+            "to change the script",
+            self.build, "lookup Foo { script latn; } Foo;")
+
     def test_language(self):
         builder = Builder(None, TTFont())
         builder.add_language_system(None, 'latn', 'FRA')
@@ -133,6 +140,19 @@ class BuilderTest(unittest.TestCase):
         self.assertEqual(builder.language_systems,
                          {('latn', 'FRA'), ('cyrl', 'BGR')})
 
+    def test_language_in_lookup_block(self):
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Within a named lookup block, it is not allowed "
+            "to change the language",
+            self.build, "lookup Foo { language RUS; } Foo;")
+
+    def test_lookup_already_defined(self):
+        self.assertRaisesRegex(
+            FeatureLibError,
+            "Lookup \"foo\" has already been defined",
+            self.build, "lookup foo {} foo; lookup foo {} foo;")
+
 
 if __name__ == "__main__":
     unittest.main()