ufoLib groupnames conversion 2 → 3 (#1762)

* If a group name is the same as a glyph name, it cannot be a kerning group name
* Add test

Lib/fontTools/ufoLib/__init__.py

@@ -339,7 +339,8 @@ class UFOReader(_UFOBaseIO):
 			# convert kerning and groups
 			kerning, groups, conversionMaps = convertUFO1OrUFO2KerningToUFO3Kerning(
 				self._upConvertedKerningData["originalKerning"],
-				deepcopy(self._upConvertedKerningData["originalGroups"])
+				deepcopy(self._upConvertedKerningData["originalGroups"]),
+				self.getGlyphSet()
 			)
 			# store
 			self._upConvertedKerningData["kerning"] = kerning
@@ -637,7 +638,7 @@ class UFOReader(_UFOBaseIO):
 
 		``validateRead`` will validate the read data, by default it is set to the
 		class's validate value, can be overridden.
-		``validateWrte`` will validate the written data, by default it is set to the
+		``validateWrite`` will validate the written data, by default it is set to the
 		class's validate value, can be overridden.
 		"""
 		from fontTools.ufoLib.glifLib import GlyphSet

Lib/fontTools/ufoLib/converters.py

@@ -6,16 +6,16 @@ Conversion functions.
 
 # adapted from the UFO spec
 
-def convertUFO1OrUFO2KerningToUFO3Kerning(kerning, groups):
+def convertUFO1OrUFO2KerningToUFO3Kerning(kerning, groups, glyphSet=()):
     # gather known kerning groups based on the prefixes
     firstReferencedGroups, secondReferencedGroups = findKnownKerningGroups(groups)
     # Make lists of groups referenced in kerning pairs.
     for first, seconds in list(kerning.items()):
-        if first in groups:
+        if first in groups and first not in glyphSet:
             if not first.startswith("public.kern1."):
                 firstReferencedGroups.add(first)
         for second in list(seconds.keys()):
-            if second in groups:
+            if second in groups and second not in glyphSet:
                 if not second.startswith("public.kern2."):
                     secondReferencedGroups.add(second)
     # Create new names for these groups.
@@ -154,7 +154,7 @@ def test():
     ...     "DGroup" : ["D"],
     ... }
     >>> kerning, groups, maps = convertUFO1OrUFO2KerningToUFO3Kerning(
-    ...     testKerning, testGroups)
+    ...     testKerning, testGroups, [])
     >>> expected = {
     ...     "A" : {
     ...         "A": 1,
@@ -220,7 +220,7 @@ def test():
     ...     "@MMK_R_XGroup" : ["X"],
     ... }
     >>> kerning, groups, maps = convertUFO1OrUFO2KerningToUFO3Kerning(
-    ...     testKerning, testGroups)
+    ...     testKerning, testGroups, [])
     >>> expected = {
     ...     "A" : {
     ...         "A": 1,
@@ -293,7 +293,7 @@ def test():
     ...     "DGroup" : ["D"],
     ... }
     >>> kerning, groups, maps = convertUFO1OrUFO2KerningToUFO3Kerning(
-    ...     testKerning, testGroups)
+    ...     testKerning, testGroups, [])
     >>> expected = {
     ...     "A" : {
     ...         "A": 1,

Lib/fontTools/ufoLib/validators.py

@@ -883,7 +883,7 @@ def groupsValidator(value):
 			return False, "A group has an empty name."
 		if groupName.startswith("public."):
 			if not groupName.startswith("public.kern1.") and not groupName.startswith("public.kern2."):
-				# unknown pubic.* name. silently skip.
+				# unknown public.* name. silently skip.
 				continue
 			else:
 				if len("public.kernN.") == len(groupName):

Tests/ufoLib/UFOConversion_test.py

@@ -137,7 +137,9 @@ class KerningUpConversionTestCase(unittest.TestCase):
 		("A", "public.kern2.CGroup"): 3,
 		("A", "public.kern2.DGroup"): 4,
 		("A", "A"): 1,
-		("A", "B"): 2
+		("A", "B"): 2,
+		("X", "A"): 13,
+		("X", "public.kern2.CGroup"): 14
 	}
 
 	expectedGroups = {
@@ -148,7 +150,8 @@ class KerningUpConversionTestCase(unittest.TestCase):
 		"public.kern1.CGroup": ["C", "Ccedilla"],
 		"public.kern2.CGroup": ["C", "Ccedilla"],
 		"public.kern2.DGroup": ["D"],
-		"Not A Kerning Group" : ["A"]
+		"Not A Kerning Group" : ["A"],
+		"X": ["X", "X.sc"]
 	}
 
 	def setUp(self):
@@ -163,6 +166,20 @@ class KerningUpConversionTestCase(unittest.TestCase):
 		self.clearUFO()
 		if not os.path.exists(self.ufoPath):
 			os.mkdir(self.ufoPath)
+
+		# glyphs
+		glyphsPath = os.path.join(self.ufoPath, "glyphs")
+		if not os.path.exists(glyphsPath):
+			os.mkdir(glyphsPath)
+		glyphFile = "X_.glif"
+		glyphsContents = dict(X=glyphFile)
+		path = os.path.join(glyphsPath, "contents.plist")
+		with open(path, "wb") as f:
+			plistlib.dump(glyphsContents, f)
+		path = os.path.join(glyphsPath, glyphFile)
+		with open(path, "w") as f:
+			f.write('<?xml version="1.0" encoding="UTF-8"?>\n')
+
 		# metainfo.plist
 		metaInfo = dict(creator="test", formatVersion=formatVersion)
 		path = os.path.join(self.ufoPath, "metainfo.plist")
@@ -187,6 +204,10 @@ class KerningUpConversionTestCase(unittest.TestCase):
 				"B" : 10,
 				"CGroup" : 11,
 				"DGroup" : 12
+			},
+			"X": {
+				"A" : 13,
+				"CGroup" : 14
 			}
 		}
 		path = os.path.join(self.ufoPath, "kerning.plist")
@@ -197,7 +218,8 @@ class KerningUpConversionTestCase(unittest.TestCase):
 			"BGroup" : ["B"],
 			"CGroup" : ["C", "Ccedilla"],
 			"DGroup" : ["D"],
-			"Not A Kerning Group" : ["A"]
+			"Not A Kerning Group" : ["A"],
+			"X" : ["X", "X.sc"]  # a group with a name that is also a glyph name
 		}
 		path = os.path.join(self.ufoPath, "groups.plist")
 		with open(path, "wb") as f: