From a913431ecdb32f0a4a203fee9e40f1808f129c07 Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <clupo@google.com>
Date: Fri, 3 Apr 2020 12:38:35 +0100
Subject: [PATCH] feaLib.parser: if no glyphNames, treat dash as names, not
 ranges

But print a warning about the possible ambiguity.

Fixes https://github.com/fonttools/fonttools/issues/1768
---
 Lib/fontTools/feaLib/parser.py | 11 ++++++++---
 Tests/feaLib/parser_test.py    | 14 ++++++++++++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py
index bda34e261..1d9c717fd 100644
--- a/Lib/fontTools/feaLib/parser.py
+++ b/Lib/fontTools/feaLib/parser.py
@@ -299,7 +299,7 @@ class Parser(object):
             if self.next_token_type_ is Lexer.NAME:
                 glyph = self.expect_glyph_()
                 location = self.cur_token_location_
-                if '-' in glyph and glyph not in self.glyphNames_:
+                if '-' in glyph and self.glyphNames_ and glyph not in self.glyphNames_:
                     start, limit = self.split_glyph_range_(glyph, location)
                     self.check_glyph_name_in_glyph_set(start, limit)
                     glyphs.add_range(
@@ -314,6 +314,11 @@ class Parser(object):
                         start, limit,
                         self.make_glyph_range_(location, start, limit))
                 else:
+                    if not self.glyphNames_:
+                        log.warning(str(FeatureLibError(
+                            f"Ambiguous glyph name that looks like a range: {glyph!r}",
+                            location
+                        )))
                     self.check_glyph_name_in_glyph_set(glyph)
                     glyphs.append(glyph)
             elif self.next_token_type_ is Lexer.CID:
@@ -350,8 +355,8 @@ class Parser(object):
                 glyphs.add_class(gc)
             else:
                 raise FeatureLibError(
-                    "Expected glyph name, glyph range, "
-                    "or glyph class reference",
+                    f"Expected glyph name, glyph range, "
+                    f"or glyph class reference, found {self.next_token_!r}",
                     self.next_token_location_)
         self.expect_symbol_("]")
         return glyphs
diff --git a/Tests/feaLib/parser_test.py b/Tests/feaLib/parser_test.py
index 927c7e4c7..402a11dd4 100644
--- a/Tests/feaLib/parser_test.py
+++ b/Tests/feaLib/parser_test.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+from fontTools.misc.loggingTools import CapturingLogHandler
 from fontTools.feaLib.error import FeatureLibError
 from fontTools.feaLib.parser import Parser, SymbolTable
 from io import StringIO
@@ -346,6 +347,19 @@ class ParserTest(unittest.TestCase):
         [gc] = self.parse("@range = [A-foo.sc - C-foo.sc];", gn).statements
         self.assertEqual(gc.glyphSet(), ("A-foo.sc", "B-foo.sc", "C-foo.sc"))
 
+    def test_glyphclass_ambiguous_dash_no_glyph_names(self):
+        # If Parser is initialized without a glyphNames parameter (or with empty one)
+        # it cannot distinguish between a glyph name containing an hyphen, or a
+        # range of glyph names; thus it will interpret them as literal glyph names
+        # while also outputting a logging warning to alert user about the ambiguity.
+        # https://github.com/fonttools/fonttools/issues/1768
+        glyphNames = ()
+        with CapturingLogHandler("fontTools.feaLib.parser", level="WARNING") as caplog:
+            [gc] = self.parse("@class = [A-foo.sc B-foo.sc];", glyphNames).statements
+        self.assertEqual(gc.glyphSet(), ("A-foo.sc", "B-foo.sc"))
+        self.assertEqual(len(caplog.records), 2)
+        caplog.assertRegex("Ambiguous glyph name that looks like a range:")
+
     def test_glyphclass_glyph_name_should_win_over_range(self):
         # The OpenType Feature File Specification v1.20 makes it clear
         # that if a dashed name could be interpreted either as a glyph name