Merge pull request #139 from hit9/check-field-id-name-dupl

Check duplicate field id or name

tests/parser-cases/e_duplicate_field_id.thrift

@@ -0,0 +1,4 @@
+struct Foo {
+1: required i32 abc
+1: required i32 efg
+}

tests/parser-cases/e_duplicate_field_name.thrift

@@ -0,0 +1,4 @@
+struct Foo {
+1: required i32 abc
+2: required i32 abc
+}

tests/test_parser.py

@@ -203,6 +203,15 @@ def test_e_use_thrift_reserved_keywords():
     assert 'Cannot use reserved language keyword' in str(excinfo.value)
 
 
+def test_e_duplicate_field_id_or_name():
+    with pytest.raises(ThriftGrammerError) as excinfo:
+        load('parser-cases/e_duplicate_field_id.thrift')
+    assert 'field identifier/name has already been used' in str(excinfo.value)
+    with pytest.raises(ThriftGrammerError) as excinfo:
+        load('parser-cases/e_duplicate_field_name.thrift')
+    assert 'field identifier/name has already been used' in str(excinfo.value)
+
+
 def test_thrift_meta():
     thrift = load('parser-cases/tutorial.thrift')
     meta = thrift.__thrift_meta__

thriftpy/parser/parser.py

@@ -661,6 +661,10 @@ def _make_struct(name, fields, ttype=TType.STRUCT, base_cls=TPayload,
     _tspec = {}
 
     for field in fields:
+        if field[0] in thrift_spec or field[3] in _tspec:
+            raise ThriftGrammerError(('\'%d:%s\' field identifier/name has '
+                                      'already been used') % (
+                                          field[0], field[3]))
         ttype = field[2]
         thrift_spec[field[0]] = _ttype_spec(ttype, field[3], field[1])
         default_spec.append((field[3], field[4]))