qapi: Better error messages for duplicated expressions

The previous commit demonstrated that the generator overlooked
duplicate expressions:
- a complex type or command reusing a built-in type name
- redeclaration of a type name, whether by the same or different
metatype
- redeclaration of a command or event
- collision of a type with implicit 'Kind' enum for a union
- collision with an implicit MAX enum constant

Since the c_type() function in the generator treats all names
as being in the same namespace, this patch adds a global array
to track all known names and their source, to prevent collisions
before it can cause further problems.  While valid .json files
won't trigger any of these cases, we might as well be nicer to
developers that make a typo while trying to add new QAPI code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

scripts/qapi.py

@@ -32,6 +32,12 @@ builtin_types = {
     'size':     'QTYPE_QINT',
 }
 
+enum_types = []
+struct_types = []
+union_types = []
+events = []
+all_names = {}
+
 def error_path(parent):
     res = ""
     while parent:
@@ -256,7 +262,14 @@ def discriminator_find_enum_define(expr):
     return find_enum(discriminator_type)
 
 def check_event(expr, expr_info):
+    global events
+    name = expr['event']
     params = expr.get('data')
+
+    if name.upper() == 'MAX':
+        raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
+    events.append(name)
+
     if params:
         for argname, argentry, optional, structured in parse_args(params):
             if structured:
@@ -430,6 +443,9 @@ def check_keys(expr_elem, meta, required, optional=[]):
 
 
 def parse_schema(input_file):
+    global all_names
+    exprs = []
+
     # First pass: read entire file into memory
     try:
         schema = QAPISchema(open(input_file, "r"))
@@ -437,30 +453,34 @@ def parse_schema(input_file):
         print >>sys.stderr, e
         exit(1)
 
-    exprs = []
-
     try:
         # Next pass: learn the types and check for valid expression keys. At
         # this point, top-level 'include' has already been flattened.
+        for builtin in builtin_types.keys():
+            all_names[builtin] = 'built-in'
         for expr_elem in schema.exprs:
             expr = expr_elem['expr']
+            info = expr_elem['info']
             if expr.has_key('enum'):
                 check_keys(expr_elem, 'enum', ['data'])
-                add_enum(expr['enum'], expr['data'])
+                add_enum(expr['enum'], info, expr['data'])
             elif expr.has_key('union'):
                 check_keys(expr_elem, 'union', ['data'],
                            ['base', 'discriminator'])
-                add_union(expr)
+                add_union(expr, info)
             elif expr.has_key('alternate'):
                 check_keys(expr_elem, 'alternate', ['data'])
+                add_name(expr['alternate'], info, 'alternate')
             elif expr.has_key('type'):
                 check_keys(expr_elem, 'type', ['data'], ['base'])
-                add_struct(expr)
+                add_struct(expr, info)
             elif expr.has_key('command'):
                 check_keys(expr_elem, 'command', [],
                            ['data', 'returns', 'gen', 'success-response'])
+                add_name(expr['command'], info, 'command')
             elif expr.has_key('event'):
                 check_keys(expr_elem, 'event', [], ['data'])
+                add_name(expr['event'], info, 'event')
             else:
                 raise QAPIExprError(expr_elem['info'],
                                     "Expression is missing metatype")
@@ -471,9 +491,11 @@ def parse_schema(input_file):
             expr = expr_elem['expr']
             if expr.has_key('union'):
                 if not discriminator_find_enum_define(expr):
-                    add_enum('%sKind' % expr['union'])
+                    add_enum('%sKind' % expr['union'], expr_elem['info'],
+                             implicit=True)
             elif expr.has_key('alternate'):
-                add_enum('%sKind' % expr['alternate'])
+                add_enum('%sKind' % expr['alternate'], expr_elem['info'],
+                         implicit=True)
 
         # Final pass - validate that exprs make sense
         check_exprs(schema)
@@ -567,12 +589,22 @@ def type_name(name):
         return c_list_type(name[0])
     return name
 
-enum_types = []
-struct_types = []
-union_types = []
+def add_name(name, info, meta, implicit = False):
+    global all_names
+    if name in all_names:
+        raise QAPIExprError(info,
+                            "%s '%s' is already defined"
+                            % (all_names[name], name))
+    if not implicit and name[-4:] == 'Kind':
+        raise QAPIExprError(info,
+                            "%s '%s' should not end in 'Kind'"
+                            % (meta, name))
+    all_names[name] = meta
 
-def add_struct(definition):
+def add_struct(definition, info):
     global struct_types
+    name = definition['type']
+    add_name(name, info, 'struct')
     struct_types.append(definition)
 
 def find_struct(name):
@@ -582,8 +614,10 @@ def find_struct(name):
             return struct
     return None
 
-def add_union(definition):
+def add_union(definition, info):
     global union_types
+    name = definition['union']
+    add_name(name, info, 'union')
     union_types.append(definition)
 
 def find_union(name):
@@ -593,8 +627,9 @@ def find_union(name):
             return union
     return None
 
-def add_enum(name, enum_values = None):
+def add_enum(name, info, enum_values = None, implicit = False):
     global enum_types
+    add_name(name, info, 'enum', implicit)
     enum_types.append({"enum_name": name, "enum_values": enum_values})
 
 def find_enum(name):
@@ -636,7 +671,7 @@ def c_type(name, is_param=False):
         return name
     elif name == None or len(name) == 0:
         return 'void'
-    elif name == name.upper():
+    elif name in events:
         return '%sEvent *%s' % (camel_case(name), eatspace)
     else:
         return '%s *%s' % (name, eatspace)

tests/qapi-schema/command-int.err

@@ -0,0 +1 @@
+tests/qapi-schema/command-int.json:2: built-in 'int' is already defined

tests/qapi-schema/command-int.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/command-int.json

@@ -1,3 +1,3 @@
-# FIXME: we should reject collisions between commands and types
+# we reject collisions between commands and types
 { 'command': 'int', 'data': { 'character': 'str' },
   'returns': { 'value': 'int' } }

tests/qapi-schema/command-int.out

@@ -1,3 +0,0 @@
-[OrderedDict([('command', 'int'), ('data', OrderedDict([('character', 'str')])), ('returns', OrderedDict([('value', 'int')]))])]
-[]
-[]

tests/qapi-schema/enum-union-clash.err

@@ -0,0 +1 @@
+tests/qapi-schema/enum-union-clash.json:2: enum 'UnionKind' should not end in 'Kind'

tests/qapi-schema/enum-union-clash.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/enum-union-clash.json

@@ -1,4 +1,4 @@
-# FIXME: we should reject types that would conflict with implicit union enum
+# we reject types that would conflict with implicit union enum
 { 'enum': 'UnionKind', 'data': [ 'oops' ] }
 { 'union': 'Union',
   'data': { 'a': 'int' } }

tests/qapi-schema/enum-union-clash.out

@@ -1,5 +0,0 @@
-[OrderedDict([('enum', 'UnionKind'), ('data', ['oops'])]),
- OrderedDict([('union', 'Union'), ('data', OrderedDict([('a', 'int')]))])]
-[{'enum_name': 'UnionKind', 'enum_values': ['oops']},
- {'enum_name': 'UnionKind', 'enum_values': None}]
-[]

tests/qapi-schema/event-max.err

@@ -0,0 +1 @@
+tests/qapi-schema/event-max.json:2: Event name 'MAX' cannot be created

tests/qapi-schema/event-max.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/event-max.json

@@ -1,2 +1,2 @@
-# FIXME: an event named 'MAX' would conflict with implicit C enum
+# an event named 'MAX' would conflict with implicit C enum
 { 'event': 'MAX' }

tests/qapi-schema/event-max.out

@@ -1,3 +0,0 @@
-[OrderedDict([('event', 'MAX')])]
-[]
-[]

tests/qapi-schema/redefined-builtin.err

@@ -0,0 +1 @@
+tests/qapi-schema/redefined-builtin.json:2: built-in 'size' is already defined

tests/qapi-schema/redefined-builtin.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/redefined-builtin.json

@@ -1,2 +1,2 @@
-# FIXME: we should reject types that duplicate builtin names
+# we reject types that duplicate builtin names
 { 'type': 'size', 'data': { 'myint': 'size' } }

tests/qapi-schema/redefined-builtin.out

@@ -1,3 +0,0 @@
-[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])]
-[]
-[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])]

tests/qapi-schema/redefined-command.err

@@ -0,0 +1 @@
+tests/qapi-schema/redefined-command.json:3: command 'foo' is already defined

tests/qapi-schema/redefined-command.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/redefined-command.json

@@ -1,3 +1,3 @@
-# FIXME: we should reject commands defined more than once
+# we reject commands defined more than once
 { 'command': 'foo', 'data': { 'one': 'str' } }
 { 'command': 'foo', 'data': { '*two': 'str' } }

tests/qapi-schema/redefined-command.out

@@ -1,4 +0,0 @@
-[OrderedDict([('command', 'foo'), ('data', OrderedDict([('one', 'str')]))]),
- OrderedDict([('command', 'foo'), ('data', OrderedDict([('*two', 'str')]))])]
-[]
-[]

tests/qapi-schema/redefined-event.err

@@ -0,0 +1 @@
+tests/qapi-schema/redefined-event.json:3: event 'EVENT_A' is already defined

tests/qapi-schema/redefined-event.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/redefined-event.json

@@ -1,3 +1,3 @@
-# FIXME: we should reject duplicate events
+# we reject duplicate events
 { 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
 { 'event': 'EVENT_A', 'data': { 'myint': 'int' } }

tests/qapi-schema/redefined-event.out

@@ -1,4 +0,0 @@
-[OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))]),
- OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))])]
-[]
-[]

tests/qapi-schema/redefined-type.err

@@ -0,0 +1 @@
+tests/qapi-schema/redefined-type.json:3: struct 'foo' is already defined

tests/qapi-schema/redefined-type.exit

@@ -1 +1 @@
-0
+1

tests/qapi-schema/redefined-type.json

@@ -1,3 +1,3 @@
-# FIXME: we should reject types defined more than once
+# we reject types defined more than once
 { 'type': 'foo', 'data': { 'one': 'str' } }
 { 'enum': 'foo', 'data': [ 'two' ] }

tests/qapi-schema/redefined-type.out

@@ -1,4 +0,0 @@
-[OrderedDict([('type', 'foo'), ('data', OrderedDict([('one', 'str')]))]),
- OrderedDict([('enum', 'foo'), ('data', ['two'])])]
-[{'enum_name': 'foo', 'enum_values': ['two']}]
-[OrderedDict([('type', 'foo'), ('data', OrderedDict([('one', 'str')]))])]