Forbid empty classes (take 2) (#2446)
This commit is contained in:
Simon Cozens
GitHub
parent
00f37ea6b6
commit
af9dfc94e7
@ -1331,10 +1331,16 @@ class PairPosStatement(Statement):
|
||||
"""
|
||||
if self.enumerated:
|
||||
g = [self.glyphs1.glyphSet(), self.glyphs2.glyphSet()]
|
||||
seen_pair = False
|
||||
for glyph1, glyph2 in itertools.product(*g):
|
||||
seen_pair = True
|
||||
builder.add_specific_pair_pos(
|
||||
self.location, glyph1, self.valuerecord1, glyph2, self.valuerecord2
|
||||
)
|
||||
if not seen_pair:
|
||||
raise FeatureLibError(
|
||||
"Empty glyph class in positioning rule", self.location
|
||||
)
|
||||
return
|
||||
|
||||
is_specific = isinstance(self.glyphs1, GlyphName) and isinstance(
|
||||
|
||||
@ -1195,39 +1195,6 @@ class Builder(object):
|
||||
for glyph in glyphs:
|
||||
self.attachPoints_.setdefault(glyph, set()).update(contourPoints)
|
||||
|
||||
def add_chain_context_pos(self, location, prefix, glyphs, suffix, lookups):
|
||||
lookup = self.get_lookup_(location, ChainContextPosBuilder)
|
||||
lookup.rules.append(
|
||||
ChainContextualRule(
|
||||
prefix, glyphs, suffix, self.find_lookup_builders_(lookups)
|
||||
)
|
||||
)
|
||||
|
||||
def add_chain_context_subst(self, location, prefix, glyphs, suffix, lookups):
|
||||
lookup = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
lookup.rules.append(
|
||||
ChainContextualRule(
|
||||
prefix, glyphs, suffix, self.find_lookup_builders_(lookups)
|
||||
)
|
||||
)
|
||||
|
||||
def add_alternate_subst(self, location, prefix, glyph, suffix, replacement):
|
||||
if self.cur_feature_name_ == "aalt":
|
||||
alts = self.aalt_alternates_.setdefault(glyph, set())
|
||||
alts.update(replacement)
|
||||
return
|
||||
if prefix or suffix:
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
lookup = self.get_chained_lookup_(location, AlternateSubstBuilder)
|
||||
chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [lookup]))
|
||||
else:
|
||||
lookup = self.get_lookup_(location, AlternateSubstBuilder)
|
||||
if glyph in lookup.alternates:
|
||||
raise FeatureLibError(
|
||||
'Already defined alternates for glyph "%s"' % glyph, location
|
||||
)
|
||||
lookup.alternates[glyph] = replacement
|
||||
|
||||
def add_feature_reference(self, location, featureName):
|
||||
if self.cur_feature_name_ != "aalt":
|
||||
raise FeatureLibError(
|
||||
@ -1272,53 +1239,9 @@ class Builder(object):
|
||||
key = (script, lang, self.cur_feature_name_)
|
||||
self.features_.setdefault(key, [])
|
||||
|
||||
def add_ligature_subst(
|
||||
self, location, prefix, glyphs, suffix, replacement, forceChain
|
||||
):
|
||||
if prefix or suffix or forceChain:
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
lookup = self.get_chained_lookup_(location, LigatureSubstBuilder)
|
||||
chain.rules.append(ChainContextualRule(prefix, glyphs, suffix, [lookup]))
|
||||
else:
|
||||
lookup = self.get_lookup_(location, LigatureSubstBuilder)
|
||||
|
||||
# OpenType feature file syntax, section 5.d, "Ligature substitution":
|
||||
# "Since the OpenType specification does not allow ligature
|
||||
# substitutions to be specified on target sequences that contain
|
||||
# glyph classes, the implementation software will enumerate
|
||||
# all specific glyph sequences if glyph classes are detected"
|
||||
for g in sorted(itertools.product(*glyphs)):
|
||||
lookup.ligatures[g] = replacement
|
||||
|
||||
def add_multiple_subst(
|
||||
self, location, prefix, glyph, suffix, replacements, forceChain=False
|
||||
):
|
||||
if prefix or suffix or forceChain:
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
sub = self.get_chained_lookup_(location, MultipleSubstBuilder)
|
||||
sub.mapping[glyph] = replacements
|
||||
chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [sub]))
|
||||
return
|
||||
lookup = self.get_lookup_(location, MultipleSubstBuilder)
|
||||
if glyph in lookup.mapping:
|
||||
if replacements == lookup.mapping[glyph]:
|
||||
log.info(
|
||||
"Removing duplicate multiple substitution from glyph"
|
||||
' "%s" to %s%s',
|
||||
glyph,
|
||||
replacements,
|
||||
f" at {location}" if location else "",
|
||||
)
|
||||
else:
|
||||
raise FeatureLibError(
|
||||
'Already defined substitution for glyph "%s"' % glyph, location
|
||||
)
|
||||
lookup.mapping[glyph] = replacements
|
||||
|
||||
def add_reverse_chain_single_subst(self, location, old_prefix, old_suffix, mapping):
|
||||
lookup = self.get_lookup_(location, ReverseChainSingleSubstBuilder)
|
||||
lookup.rules.append((old_prefix, old_suffix, mapping))
|
||||
# GSUB rules
|
||||
|
||||
# GSUB 1
|
||||
def add_single_subst(self, location, prefix, suffix, mapping, forceChain):
|
||||
if self.cur_feature_name_ == "aalt":
|
||||
for (from_glyph, to_glyph) in mapping.items():
|
||||
@ -1347,7 +1270,87 @@ class Builder(object):
|
||||
)
|
||||
lookup.mapping[from_glyph] = to_glyph
|
||||
|
||||
# GSUB 2
|
||||
def add_multiple_subst(
|
||||
self, location, prefix, glyph, suffix, replacements, forceChain=False
|
||||
):
|
||||
if prefix or suffix or forceChain:
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
sub = self.get_chained_lookup_(location, MultipleSubstBuilder)
|
||||
sub.mapping[glyph] = replacements
|
||||
chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [sub]))
|
||||
return
|
||||
lookup = self.get_lookup_(location, MultipleSubstBuilder)
|
||||
if glyph in lookup.mapping:
|
||||
if replacements == lookup.mapping[glyph]:
|
||||
log.info(
|
||||
"Removing duplicate multiple substitution from glyph"
|
||||
' "%s" to %s%s',
|
||||
glyph,
|
||||
replacements,
|
||||
f" at {location}" if location else "",
|
||||
)
|
||||
else:
|
||||
raise FeatureLibError(
|
||||
'Already defined substitution for glyph "%s"' % glyph, location
|
||||
)
|
||||
lookup.mapping[glyph] = replacements
|
||||
|
||||
# GSUB 3
|
||||
def add_alternate_subst(self, location, prefix, glyph, suffix, replacement):
|
||||
if self.cur_feature_name_ == "aalt":
|
||||
alts = self.aalt_alternates_.setdefault(glyph, set())
|
||||
alts.update(replacement)
|
||||
return
|
||||
if prefix or suffix:
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
lookup = self.get_chained_lookup_(location, AlternateSubstBuilder)
|
||||
chain.rules.append(ChainContextualRule(prefix, [{glyph}], suffix, [lookup]))
|
||||
else:
|
||||
lookup = self.get_lookup_(location, AlternateSubstBuilder)
|
||||
if glyph in lookup.alternates:
|
||||
raise FeatureLibError(
|
||||
'Already defined alternates for glyph "%s"' % glyph, location
|
||||
)
|
||||
# We allow empty replacement glyphs here.
|
||||
lookup.alternates[glyph] = replacement
|
||||
|
||||
# GSUB 4
|
||||
def add_ligature_subst(
|
||||
self, location, prefix, glyphs, suffix, replacement, forceChain
|
||||
):
|
||||
if prefix or suffix or forceChain:
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
lookup = self.get_chained_lookup_(location, LigatureSubstBuilder)
|
||||
chain.rules.append(ChainContextualRule(prefix, glyphs, suffix, [lookup]))
|
||||
else:
|
||||
lookup = self.get_lookup_(location, LigatureSubstBuilder)
|
||||
|
||||
if not all(glyphs):
|
||||
raise FeatureLibError("Empty glyph class in substitution", location)
|
||||
|
||||
# OpenType feature file syntax, section 5.d, "Ligature substitution":
|
||||
# "Since the OpenType specification does not allow ligature
|
||||
# substitutions to be specified on target sequences that contain
|
||||
# glyph classes, the implementation software will enumerate
|
||||
# all specific glyph sequences if glyph classes are detected"
|
||||
for g in sorted(itertools.product(*glyphs)):
|
||||
lookup.ligatures[g] = replacement
|
||||
|
||||
# GSUB 5/6
|
||||
def add_chain_context_subst(self, location, prefix, glyphs, suffix, lookups):
|
||||
if not all(glyphs) or not all(prefix) or not all(suffix):
|
||||
raise FeatureLibError("Empty glyph class in contextual substitution", location)
|
||||
lookup = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
lookup.rules.append(
|
||||
ChainContextualRule(
|
||||
prefix, glyphs, suffix, self.find_lookup_builders_(lookups)
|
||||
)
|
||||
)
|
||||
|
||||
def add_single_subst_chained_(self, location, prefix, suffix, mapping):
|
||||
if not mapping or not all(prefix) or not all(suffix):
|
||||
raise FeatureLibError("Empty glyph class in contextual substitution", location)
|
||||
# https://github.com/fonttools/fonttools/issues/512
|
||||
chain = self.get_lookup_(location, ChainContextSubstBuilder)
|
||||
sub = chain.find_chainable_single_subst(set(mapping.keys()))
|
||||
@ -1358,7 +1361,54 @@ class Builder(object):
|
||||
ChainContextualRule(prefix, [list(mapping.keys())], suffix, [sub])
|
||||
)
|
||||
|
||||
# GSUB 8
|
||||
def add_reverse_chain_single_subst(self, location, old_prefix, old_suffix, mapping):
|
||||
if not mapping:
|
||||
raise FeatureLibError("Empty glyph class in substitution", location)
|
||||
lookup = self.get_lookup_(location, ReverseChainSingleSubstBuilder)
|
||||
lookup.rules.append((old_prefix, old_suffix, mapping))
|
||||
|
||||
# GPOS rules
|
||||
|
||||
# GPOS 1
|
||||
def add_single_pos(self, location, prefix, suffix, pos, forceChain):
|
||||
if prefix or suffix or forceChain:
|
||||
self.add_single_pos_chained_(location, prefix, suffix, pos)
|
||||
else:
|
||||
lookup = self.get_lookup_(location, SinglePosBuilder)
|
||||
for glyphs, value in pos:
|
||||
if not glyphs:
|
||||
raise FeatureLibError("Empty glyph class in positioning rule", location)
|
||||
otValueRecord = self.makeOpenTypeValueRecord(location, value, pairPosContext=False)
|
||||
for glyph in glyphs:
|
||||
try:
|
||||
lookup.add_pos(location, glyph, otValueRecord)
|
||||
except OpenTypeLibError as e:
|
||||
raise FeatureLibError(str(e), e.location) from e
|
||||
|
||||
# GPOS 2
|
||||
def add_class_pair_pos(self, location, glyphclass1, value1, glyphclass2, value2):
|
||||
if not glyphclass1 or not glyphclass2:
|
||||
raise FeatureLibError(
|
||||
"Empty glyph class in positioning rule", location
|
||||
)
|
||||
lookup = self.get_lookup_(location, PairPosBuilder)
|
||||
v1 = self.makeOpenTypeValueRecord(location, value1, pairPosContext=True)
|
||||
v2 = self.makeOpenTypeValueRecord(location, value2, pairPosContext=True)
|
||||
lookup.addClassPair(location, glyphclass1, v1, glyphclass2, v2)
|
||||
|
||||
def add_specific_pair_pos(self, location, glyph1, value1, glyph2, value2):
|
||||
if not glyph1 or not glyph2:
|
||||
raise FeatureLibError("Empty glyph class in positioning rule", location)
|
||||
lookup = self.get_lookup_(location, PairPosBuilder)
|
||||
v1 = self.makeOpenTypeValueRecord(location, value1, pairPosContext=True)
|
||||
v2 = self.makeOpenTypeValueRecord(location, value2, pairPosContext=True)
|
||||
lookup.addGlyphPair(location, glyph1, v1, glyph2, v2)
|
||||
|
||||
# GPOS 3
|
||||
def add_cursive_pos(self, location, glyphclass, entryAnchor, exitAnchor):
|
||||
if not glyphclass:
|
||||
raise FeatureLibError("Empty glyph class in positioning rule", location)
|
||||
lookup = self.get_lookup_(location, CursivePosBuilder)
|
||||
lookup.add_attachment(
|
||||
location,
|
||||
@ -1367,34 +1417,23 @@ class Builder(object):
|
||||
self.makeOpenTypeAnchor(location, exitAnchor),
|
||||
)
|
||||
|
||||
def add_marks_(self, location, lookupBuilder, marks):
|
||||
"""Helper for add_mark_{base,liga,mark}_pos."""
|
||||
for _, markClass in marks:
|
||||
for markClassDef in markClass.definitions:
|
||||
for mark in markClassDef.glyphs.glyphSet():
|
||||
if mark not in lookupBuilder.marks:
|
||||
otMarkAnchor = self.makeOpenTypeAnchor(location, markClassDef.anchor)
|
||||
lookupBuilder.marks[mark] = (markClass.name, otMarkAnchor)
|
||||
else:
|
||||
existingMarkClass = lookupBuilder.marks[mark][0]
|
||||
if markClass.name != existingMarkClass:
|
||||
raise FeatureLibError(
|
||||
"Glyph %s cannot be in both @%s and @%s"
|
||||
% (mark, existingMarkClass, markClass.name),
|
||||
location,
|
||||
)
|
||||
|
||||
# GPOS 4
|
||||
def add_mark_base_pos(self, location, bases, marks):
|
||||
builder = self.get_lookup_(location, MarkBasePosBuilder)
|
||||
self.add_marks_(location, builder, marks)
|
||||
if not bases:
|
||||
raise FeatureLibError("Empty glyph class in positioning rule", location)
|
||||
for baseAnchor, markClass in marks:
|
||||
otBaseAnchor = self.makeOpenTypeAnchor(location, baseAnchor)
|
||||
for base in bases:
|
||||
builder.bases.setdefault(base, {})[markClass.name] = otBaseAnchor
|
||||
|
||||
# GPOS 5
|
||||
def add_mark_lig_pos(self, location, ligatures, components):
|
||||
builder = self.get_lookup_(location, MarkLigPosBuilder)
|
||||
componentAnchors = []
|
||||
if not ligatures:
|
||||
raise FeatureLibError("Empty glyph class in positioning rule", location)
|
||||
for marks in components:
|
||||
anchors = {}
|
||||
self.add_marks_(location, builder, marks)
|
||||
@ -1404,9 +1443,12 @@ class Builder(object):
|
||||
for glyph in ligatures:
|
||||
builder.ligatures[glyph] = componentAnchors
|
||||
|
||||
# GPOS 6
|
||||
def add_mark_mark_pos(self, location, baseMarks, marks):
|
||||
builder = self.get_lookup_(location, MarkMarkPosBuilder)
|
||||
self.add_marks_(location, builder, marks)
|
||||
if not baseMarks:
|
||||
raise FeatureLibError("Empty glyph class in positioning rule", location)
|
||||
for baseAnchor, markClass in marks:
|
||||
otBaseAnchor = self.makeOpenTypeAnchor(location, baseAnchor)
|
||||
for baseMark in baseMarks:
|
||||
@ -1414,35 +1456,20 @@ class Builder(object):
|
||||
markClass.name
|
||||
] = otBaseAnchor
|
||||
|
||||
def add_class_pair_pos(self, location, glyphclass1, value1, glyphclass2, value2):
|
||||
lookup = self.get_lookup_(location, PairPosBuilder)
|
||||
v1 = self.makeOpenTypeValueRecord(location, value1, pairPosContext=True)
|
||||
v2 = self.makeOpenTypeValueRecord(location, value2, pairPosContext=True)
|
||||
lookup.addClassPair(location, glyphclass1, v1, glyphclass2, v2)
|
||||
|
||||
def add_subtable_break(self, location):
|
||||
self.cur_lookup_.add_subtable_break(location)
|
||||
|
||||
def add_specific_pair_pos(self, location, glyph1, value1, glyph2, value2):
|
||||
lookup = self.get_lookup_(location, PairPosBuilder)
|
||||
v1 = self.makeOpenTypeValueRecord(location, value1, pairPosContext=True)
|
||||
v2 = self.makeOpenTypeValueRecord(location, value2, pairPosContext=True)
|
||||
lookup.addGlyphPair(location, glyph1, v1, glyph2, v2)
|
||||
|
||||
def add_single_pos(self, location, prefix, suffix, pos, forceChain):
|
||||
if prefix or suffix or forceChain:
|
||||
self.add_single_pos_chained_(location, prefix, suffix, pos)
|
||||
else:
|
||||
lookup = self.get_lookup_(location, SinglePosBuilder)
|
||||
for glyphs, value in pos:
|
||||
otValueRecord = self.makeOpenTypeValueRecord(location, value, pairPosContext=False)
|
||||
for glyph in glyphs:
|
||||
try:
|
||||
lookup.add_pos(location, glyph, otValueRecord)
|
||||
except OpenTypeLibError as e:
|
||||
raise FeatureLibError(str(e), e.location) from e
|
||||
# GPOS 7/8
|
||||
def add_chain_context_pos(self, location, prefix, glyphs, suffix, lookups):
|
||||
if not all(glyphs) or not all(prefix) or not all(suffix):
|
||||
raise FeatureLibError("Empty glyph class in contextual positioning rule", location)
|
||||
lookup = self.get_lookup_(location, ChainContextPosBuilder)
|
||||
lookup.rules.append(
|
||||
ChainContextualRule(
|
||||
prefix, glyphs, suffix, self.find_lookup_builders_(lookups)
|
||||
)
|
||||
)
|
||||
|
||||
def add_single_pos_chained_(self, location, prefix, suffix, pos):
|
||||
if not pos or not all(prefix) or not all(suffix):
|
||||
raise FeatureLibError("Empty glyph class in contextual positioning rule", location)
|
||||
# https://github.com/fonttools/fonttools/issues/514
|
||||
chain = self.get_lookup_(location, ChainContextPosBuilder)
|
||||
targets = []
|
||||
@ -1466,6 +1493,26 @@ class Builder(object):
|
||||
ChainContextualRule(prefix, [g for g, v in pos], suffix, subs)
|
||||
)
|
||||
|
||||
def add_marks_(self, location, lookupBuilder, marks):
|
||||
"""Helper for add_mark_{base,liga,mark}_pos."""
|
||||
for _, markClass in marks:
|
||||
for markClassDef in markClass.definitions:
|
||||
for mark in markClassDef.glyphs.glyphSet():
|
||||
if mark not in lookupBuilder.marks:
|
||||
otMarkAnchor = self.makeOpenTypeAnchor(location, markClassDef.anchor)
|
||||
lookupBuilder.marks[mark] = (markClass.name, otMarkAnchor)
|
||||
else:
|
||||
existingMarkClass = lookupBuilder.marks[mark][0]
|
||||
if markClass.name != existingMarkClass:
|
||||
raise FeatureLibError(
|
||||
"Glyph %s cannot be in both @%s and @%s"
|
||||
% (mark, existingMarkClass, markClass.name),
|
||||
location,
|
||||
)
|
||||
|
||||
def add_subtable_break(self, location):
|
||||
self.cur_lookup_.add_subtable_break(location)
|
||||
|
||||
def setGlyphClass_(self, location, glyph, glyphClass):
|
||||
oldClass, oldLocation = self.glyphClassDefs_.get(glyph, (None, None))
|
||||
if oldClass and oldClass != glyphClass:
|
||||
|
||||
@ -684,6 +684,8 @@ class Parser(object):
|
||||
assert self.is_cur_keyword_("markClass")
|
||||
location = self.cur_token_location_
|
||||
glyphs = self.parse_glyphclass_(accept_glyphname=True)
|
||||
if not glyphs.glyphSet():
|
||||
raise FeatureLibError("Empty glyph class in mark class definition", location)
|
||||
anchor = self.parse_anchor_()
|
||||
name = self.expect_class_name_()
|
||||
self.expect_symbol_(";")
|
||||
@ -872,7 +874,7 @@ class Parser(object):
|
||||
num_lookups = len([l for l in lookups if l is not None])
|
||||
|
||||
is_deletion = False
|
||||
if len(new) == 1 and len(new[0].glyphSet()) == 0:
|
||||
if len(new) == 1 and isinstance(new[0], ast.NullGlyph):
|
||||
new = [] # Deletion
|
||||
is_deletion = True
|
||||
|
||||
@ -918,6 +920,9 @@ class Parser(object):
|
||||
and max([len(n.glyphSet()) for n in new]) == 1
|
||||
and num_lookups == 0
|
||||
):
|
||||
for n in new:
|
||||
if not list(n.glyphSet()):
|
||||
raise FeatureLibError("Empty class in replacement", location)
|
||||
return self.ast.MultipleSubstStatement(
|
||||
old_prefix,
|
||||
tuple(old[0].glyphSet())[0],
|
||||
|
||||
@ -925,6 +925,45 @@ class BuilderTest(unittest.TestCase):
|
||||
assert "GPOS" not in font
|
||||
assert "GSUB" not in font
|
||||
|
||||
def test_disable_empty_classes(self):
|
||||
for test in [
|
||||
"sub a by c []",
|
||||
"sub f f [] by f",
|
||||
"ignore sub a []'",
|
||||
"ignore sub [] a'",
|
||||
"sub a []' by b",
|
||||
"sub [] a' by b",
|
||||
"rsub [] by a",
|
||||
"pos [] 120",
|
||||
"pos a [] 120",
|
||||
"enum pos a [] 120",
|
||||
"pos cursive [] <anchor NULL> <anchor NULL>",
|
||||
"pos base [] <anchor NULL> mark @TOPMARKS",
|
||||
"pos ligature [] <anchor NULL> mark @TOPMARKS",
|
||||
"pos mark [] <anchor NULL> mark @TOPMARKS",
|
||||
"ignore pos a []'",
|
||||
"ignore pos [] a'",
|
||||
]:
|
||||
self.assertRaisesRegex(
|
||||
FeatureLibError,
|
||||
"Empty ",
|
||||
self.build,
|
||||
f"markClass a <anchor 150 -10> @TOPMARKS; lookup foo {{ {test}; }} foo;",
|
||||
)
|
||||
self.assertRaisesRegex(
|
||||
FeatureLibError,
|
||||
"Empty glyph class in mark class definition",
|
||||
self.build,
|
||||
"markClass [] <anchor 150 -10> @TOPMARKS;"
|
||||
)
|
||||
self.assertRaisesRegex(
|
||||
FeatureLibError,
|
||||
'Expected a glyph class with 1 elements after "by", but found a glyph class with 0 elements',
|
||||
self.build,
|
||||
"feature test { sub a by []; test};"
|
||||
)
|
||||
|
||||
|
||||
|
||||
def generate_feature_file_test(name):
|
||||
return lambda self: self.check_feature_file(name)
|
||||
|
||||
@ -4,7 +4,6 @@ languagesystem latn dflt;
|
||||
@group1 = [b o];
|
||||
@group2 = [c d];
|
||||
@group3 = [v w];
|
||||
@group4 = [];
|
||||
|
||||
lookup kernlookup {
|
||||
pos A V -34;
|
||||
@ -13,9 +12,6 @@ lookup kernlookup {
|
||||
subtable;
|
||||
pos @group1 @group3 -10;
|
||||
pos @group3 @group2 -20;
|
||||
subtable;
|
||||
pos @group4 @group1 -10;
|
||||
pos @group4 @group4 -10;
|
||||
} kernlookup;
|
||||
|
||||
feature kern {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user