Skip to content

Commit b224571

Browse files
braincorebraincore
committed
Check for duplicate field name declarations.
Summary: While adding checks to make sure that strunion type tags didn't conflict with field names, I realized that this more basic check wasn't being done. Also added a check for extending from the wrong type. Test Plan: Added tests. Reviewed By: guido
1 parent 89340b6 commit b224571

5 files changed

Lines changed: 211 additions & 40 deletions

File tree

‎babelapi/babel/exception.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import six
2+
3+
class InvalidSpec(Exception):
4+
"""Raise this to indicate there was an error in a specification."""
5+
6+
def __init__(self, msg, lineno, path=None):
7+
"""
8+
Args:
9+
msg: Error message intended for the spec writer to read.
10+
lineno: The line number the error occurred on.
11+
path: Path to the spec file with the error.
12+
"""
13+
assert isinstance(msg, six.text_type)
14+
assert isinstance(lineno, six.integer_types)
15+
self.msg = msg
16+
self.lineno = lineno
17+
self.path = path
18+
19+
def __str__(self):
20+
return repr(self)
21+
22+
def __repr__(self):
23+
return 'InvalidSpec({!r}, {!r}, {!r})'.format(
24+
self.msg,
25+
self.lineno,
26+
self.path,
27+
)

‎babelapi/babel/tower.py‎

Lines changed: 21 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@
99
import six
1010
import sys
1111

12-
from babelapi.babel.parser import BabelParser
13-
from babelapi.data_type import (
12+
from ..api import (
13+
Api,
14+
ApiRoute,
15+
)
16+
from ..data_type import (
1417
Binary,
1518
Boolean,
1619
DataType,
@@ -31,47 +34,20 @@
3134
Union,
3235
Void,
3336
)
34-
from babelapi.api import (
35-
Api,
36-
ApiRoute,
37-
)
38-
from babelapi.babel.parser import (
37+
38+
from .exception import InvalidSpec
39+
from .parser import (
3940
BabelAlias,
4041
BabelInclude,
4142
BabelNamespace,
43+
BabelParser,
4244
BabelRouteDef,
4345
BabelVoidField,
4446
BabelTagRef,
4547
BabelTypeDef,
4648
BabelTypeRef,
4749
)
4850

49-
class InvalidSpec(Exception):
50-
"""Raise this to indicate there was an error in a specification."""
51-
52-
def __init__(self, msg, lineno, path=None):
53-
"""
54-
Args:
55-
msg: Error message intended for the spec writer to read.
56-
lineno: The line number the error occurred on.
57-
path: Path to the spec file with the error.
58-
"""
59-
assert isinstance(msg, six.text_type)
60-
assert isinstance(lineno, six.integer_types)
61-
self.msg = msg
62-
self.lineno = lineno
63-
self.path = path
64-
65-
def __str__(self):
66-
return repr(self)
67-
68-
def __repr__(self):
69-
return 'InvalidSpec({!r}, {!r}, {!r})'.format(
70-
self.msg,
71-
self.lineno,
72-
self.path,
73-
)
74-
7551
def quote(s):
7652
assert s.replace('_', '').isalnum(), \
7753
'Only use quote() with names or IDs in Babel.'
@@ -203,8 +179,12 @@ def _create_type(self, env, item):
203179
raise InvalidSpec(
204180
'Data type %s is undefined.' % quote(item.extends),
205181
item.lineno)
206-
else:
207-
supertype = env.get(item.extends)
182+
supertype = env[item.extends]
183+
if not isinstance(supertype, Struct):
184+
raise InvalidSpec(
185+
'A struct can only extend another struct: '
186+
'%s is not a struct.' % quote(item.extends),
187+
item.lineno)
208188
api_type_fields = []
209189
for babel_field in item.fields:
210190
api_type_field = self._create_struct_field(env, babel_field)
@@ -218,8 +198,12 @@ def _create_type(self, env, item):
218198
raise InvalidSpec(
219199
'Data type %s is undefined.' % quote(item.extends),
220200
item.lineno)
221-
else:
222-
subtype = env.get(item.extends)
201+
subtype = env[item.extends]
202+
if not isinstance(subtype, Union):
203+
raise InvalidSpec(
204+
'A union can only extend another union: '
205+
'%s is not a union.' % quote(item.extends),
206+
item.lineno)
223207
api_type_fields = []
224208
catch_all_field = None
225209
for babel_field in item.fields:
@@ -243,7 +227,6 @@ def _create_type(self, env, item):
243227

244228
catch_all_field = api_type_field
245229
api_type_fields.append(api_type_field)
246-
247230
api_type = Union(item.name, item.doc, api_type_fields, item,
248231
subtype, catch_all_field)
249232
else:

‎babelapi/cli.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
import sys
1212
import traceback
1313

14-
from babelapi.compiler import Compiler, GeneratorException
15-
from babelapi.babel.tower import InvalidSpec, TowerOfBabel
14+
from .babel.exception import InvalidSpec
15+
from .babel.tower import TowerOfBabel
16+
from .compiler import Compiler, GeneratorException
1617

1718
# The parser for command line arguments
1819
_cmdline_parser = argparse.ArgumentParser(description='BabelAPI')

‎babelapi/data_type.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import re
1717
import six
1818

19+
from .babel.exception import InvalidSpec
20+
1921
class ParameterError(Exception):
2022
"""Raised when a data type is parameterized with a bad type or value."""
2123
pass
@@ -456,6 +458,16 @@ def __init__(self, name, doc, fields, token):
456458
self.fields = fields
457459
self._token = token
458460
self.examples = {}
461+
self._fields_by_name = {} # Dict[str, Field]
462+
463+
# Check that no two fields share the same name.
464+
for field in self.fields:
465+
if field.name in self._fields_by_name:
466+
orig_lineno = self._fields_by_name[field.name]._token.lineno
467+
raise InvalidSpec("Field '%s' already defined on line %s." %
468+
(field.name, orig_lineno),
469+
field._token.lineno)
470+
self._fields_by_name[field.name] = field
459471

460472
@property
461473
def name(self):
@@ -501,6 +513,19 @@ def __init__(self, name, doc, fields, token, supertype=None,
501513
self.coverage = []
502514
self.supertype = supertype
503515

516+
# Check that the fields for this type do not match any of the fields of
517+
# its parents.
518+
cur_type = self.supertype
519+
while cur_type:
520+
for field in self.fields:
521+
if field.name in cur_type._fields_by_name:
522+
lineno = cur_type._fields_by_name[field.name]._token.lineno
523+
raise InvalidSpec(
524+
"Field '%s' already defined in parent '%s' on line %d."
525+
% (field.name, cur_type.name, lineno),
526+
field._token.lineno)
527+
cur_type = cur_type.supertype
528+
504529
def has_coverage(self):
505530
return bool(self.coverage_names)
506531

@@ -673,6 +698,19 @@ def __init__(self, name, doc, fields, token, subtype=None,
673698
self.catch_all_field = catch_all_field
674699
self.subtype = subtype
675700

701+
# TODO(kelkabany): When supertype/subtype are renamed to parent_type,
702+
# this logic can be moved to the CompositeType super class.
703+
cur_type = self.subtype
704+
while cur_type:
705+
for field in self.fields:
706+
if field.name in cur_type._fields_by_name:
707+
lineno = cur_type._fields_by_name[field.name]._token.lineno
708+
raise InvalidSpec(
709+
"Field '%s' already defined in parent '%s' on line %d."
710+
% (field.name, cur_type.name, lineno),
711+
field._token.lineno)
712+
cur_type = cur_type.subtype
713+
676714
def check(self, val):
677715
if not isinstance(val, TagRef):
678716
raise ValueError('%r is not a tag of %s' % (val, self.name))

‎test/test_babel.py‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,125 @@ def test_parsing_errors(self):
407407
with self.assertRaises(InvalidSpec) as cm:
408408
t.add_to_api('test.babel', res)
409409
self.assertIn("Symbol 'Blah' is undefined", cm.exception.msg)
410+
411+
def test_struct_semantics(self):
412+
# Test duplicate fields
413+
text = """\
414+
namespace test
415+
416+
struct A
417+
a UInt64
418+
a String
419+
"""
420+
t = TowerOfBabel([('test.babel', text)])
421+
with self.assertRaises(InvalidSpec) as cm:
422+
t.parse()
423+
self.assertIn('already defined', cm.exception.msg)
424+
425+
# Test duplicate field name -- earlier being in a parent type
426+
text = """\
427+
namespace test
428+
429+
struct A
430+
a UInt64
431+
432+
struct B extends A
433+
b String
434+
435+
struct C extends B
436+
a String
437+
"""
438+
t = TowerOfBabel([('test.babel', text)])
439+
with self.assertRaises(InvalidSpec) as cm:
440+
t.parse()
441+
self.assertIn('already defined in parent', cm.exception.msg)
442+
443+
# Test extending from wrong type
444+
text = """\
445+
namespace test
446+
447+
union A
448+
a
449+
450+
struct B extends A
451+
b UInt64
452+
"""
453+
t = TowerOfBabel([('test.babel', text)])
454+
with self.assertRaises(InvalidSpec) as cm:
455+
t.parse()
456+
self.assertIn('struct can only extend another struct', cm.exception.msg)
457+
458+
def test_union_semantics(self):
459+
# Test duplicate fields
460+
text = """\
461+
namespace test
462+
463+
union A
464+
a UInt64
465+
a String
466+
"""
467+
t = TowerOfBabel([('test.babel', text)])
468+
with self.assertRaises(InvalidSpec) as cm:
469+
t.parse()
470+
self.assertIn('already defined', cm.exception.msg)
471+
472+
# Test duplicate field name -- earlier being in a parent type
473+
text = """\
474+
namespace test
475+
476+
union A
477+
a UInt64
478+
479+
union B extends A
480+
b String
481+
482+
union C extends B
483+
a String
484+
"""
485+
t = TowerOfBabel([('test.babel', text)])
486+
with self.assertRaises(InvalidSpec) as cm:
487+
t.parse()
488+
self.assertIn('already defined in parent', cm.exception.msg)
489+
490+
# Test two catch-alls
491+
text = """\
492+
namespace test
493+
494+
union A
495+
a*
496+
b*
497+
"""
498+
t = TowerOfBabel([('test.babel', text)])
499+
with self.assertRaises(InvalidSpec) as cm:
500+
t.parse()
501+
self.assertIn('Only one catch-all tag', cm.exception.msg)
502+
503+
# Test existing catch-all in parent type
504+
text = """\
505+
namespace test
506+
507+
union A
508+
a*
509+
510+
union B extends A
511+
b*
512+
"""
513+
t = TowerOfBabel([('test.babel', text)])
514+
with self.assertRaises(InvalidSpec) as cm:
515+
t.parse()
516+
self.assertIn('already declared a catch-all tag', cm.exception.msg)
517+
518+
# Test extending from wrong type
519+
text = """\
520+
namespace test
521+
522+
struct A
523+
a UInt64
524+
525+
union B extends A
526+
b UInt64
527+
"""
528+
t = TowerOfBabel([('test.babel', text)])
529+
with self.assertRaises(InvalidSpec) as cm:
530+
t.parse()
531+
self.assertIn('union can only extend another union', cm.exception.msg)

0 commit comments

Comments
 (0)