[Qemu-devel] [PATCH v7 04/18] qapi: Clean up qapi.py per pep8

[Eric Blake <eblake@redhat.com>]

Silence pep8, and make pylint a bit happier.  Just style cleanups,
plus killing a useless comment in camel_to_upper(); no semantic
changes.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v7: drop useless (), avoid non-mechanical conversion of lookup within
two sets
v6: split off non-style changes, add in ordereddict.py, and
rework the camel_to_upper conditional
---
 scripts/ordereddict.py |   3 +-
 scripts/qapi.py        | 161 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 106 insertions(+), 58 deletions(-)

diff --git a/scripts/ordereddict.py b/scripts/ordereddict.py
index 7242b50..2d1d813 100644
--- a/scripts/ordereddict.py
+++ b/scripts/ordereddict.py
@@ -22,6 +22,7 @@

 from UserDict import DictMixin

+
 class OrderedDict(dict, DictMixin):

     def __init__(self, *args, **kwds):
@@ -117,7 +118,7 @@ class OrderedDict(dict, DictMixin):
         if isinstance(other, OrderedDict):
             if len(self) != len(other):
                 return False
-            for p, q in  zip(self.items(), other.items()):
+            for p, q in zip(self.items(), other.items()):
                 if p != q:
                     return False
             return True
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a2d869e..4b5d574 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -71,6 +71,7 @@ all_names = {}
 # Parsing the schema into expressions
 #

+
 def error_path(parent):
     res = ""
     while parent:
@@ -79,6 +80,7 @@ def error_path(parent):
         parent = parent['parent']
     return res

+
 class QAPISchemaError(Exception):
     def __init__(self, schema, msg):
         Exception.__init__(self)
@@ -97,6 +99,7 @@ class QAPISchemaError(Exception):
         return error_path(self.info) + \
             "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)

+
 class QAPIExprError(Exception):
     def __init__(self, expr_info, msg):
         Exception.__init__(self)
@@ -107,9 +110,10 @@ class QAPIExprError(Exception):
         return error_path(self.info['parent']) + \
             "%s:%d: %s" % (self.info['file'], self.info['line'], self.msg)

+
 class QAPISchemaParser(object):

-    def __init__(self, fp, previously_included = [], incl_info = None):
+    def __init__(self, fp, previously_included=[], incl_info=None):
         abs_fname = os.path.abspath(fp.name)
         fname = fp.name
         self.fname = fname
@@ -124,13 +128,14 @@ class QAPISchemaParser(object):
         self.exprs = []
         self.accept()

-        while self.tok != None:
+        while self.tok is not None:
             expr_info = {'file': fname, 'line': self.line,
                          'parent': self.incl_info}
             expr = self.get_expr(False)
             if isinstance(expr, dict) and "include" in expr:
                 if len(expr) != 1:
-                    raise QAPIExprError(expr_info, "Invalid 'include' directive")
+                    raise QAPIExprError(expr_info,
+                                        "Invalid 'include' directive")
                 include = expr["include"]
                 if not isinstance(include, str):
                     raise QAPIExprError(expr_info,
@@ -193,7 +198,7 @@ class QAPISchemaParser(object):
                             string += '\t'
                         elif ch == 'u':
                             value = 0
-                            for x in range(0, 4):
+                            for _ in range(0, 4):
                                 ch = self.src[self.cursor]
                                 self.cursor += 1
                                 if ch not in "0123456789abcdefABCDEF":
@@ -215,7 +220,7 @@ class QAPISchemaParser(object):
                             string += ch
                         else:
                             raise QAPISchemaError(self,
-                                                  "Unknown escape \\%s" %ch)
+                                                  "Unknown escape \\%s" % ch)
                         esc = False
                     elif ch == "\\":
                         esc = True
@@ -275,7 +280,7 @@ class QAPISchemaParser(object):
         if self.tok == ']':
             self.accept()
             return expr
-        if not self.tok in "{['tfn":
+        if self.tok not in "{['tfn":
             raise QAPISchemaError(self, 'Expected "{", "[", "]", string, '
                                   'boolean or "null"')
         while True:
@@ -309,15 +314,17 @@ class QAPISchemaParser(object):
 # TODO catching name collisions in generated code would be nice
 #

+
 def find_base_fields(base):
     base_struct_define = find_struct(base)
     if not base_struct_define:
         return None
     return base_struct_define['data']

+
 # Return the qtype of an alternate branch, or None on error.
 def find_alternate_member_qtype(qapi_type):
-    if builtin_types.has_key(qapi_type):
+    if qapi_type in builtin_types:
         return builtin_types[qapi_type]
     elif find_struct(qapi_type):
         return "QTYPE_QDICT"
@@ -327,6 +334,7 @@ def find_alternate_member_qtype(qapi_type):
         return "QTYPE_QDICT"
     return None

+
 # Return the discriminator enum define if discriminator is specified as an
 # enum type, otherwise return None.
 def discriminator_find_enum_define(expr):
@@ -346,11 +354,14 @@ def discriminator_find_enum_define(expr):

     return find_enum(discriminator_type)

+
 # FIXME should enforce "other than downstream extensions [...], all
 # names should begin with a letter".
 valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$')
-def check_name(expr_info, source, name, allow_optional = False,
-               enum_member = False):
+
+
+def check_name(expr_info, source, name, allow_optional=False,
+               enum_member=False):
     global valid_name
     membername = name

@@ -371,7 +382,8 @@ def check_name(expr_info, source, name, allow_optional = False,
         raise QAPIExprError(expr_info,
                             "%s uses invalid name '%s'" % (source, name))

-def add_name(name, info, meta, implicit = False):
+
+def add_name(name, info, meta, implicit=False):
     global all_names
     check_name(info, "'%s'" % meta, name)
     # FIXME should reject names that differ only in '_' vs. '.'
@@ -386,12 +398,14 @@ def add_name(name, info, meta, implicit = False):
                             % (meta, name))
     all_names[name] = meta

+
 def add_struct(definition, info):
     global struct_types
     name = definition['struct']
     add_name(name, info, 'struct')
     struct_types.append(definition)

+
 def find_struct(name):
     global struct_types
     for struct in struct_types:
@@ -399,12 +413,14 @@ def find_struct(name):
             return struct
     return None

+
 def add_union(definition, info):
     global union_types
     name = definition['union']
     add_name(name, info, 'union')
     union_types.append(definition)

+
 def find_union(name):
     global union_types
     for union in union_types:
@@ -412,11 +428,13 @@ def find_union(name):
             return union
     return None

-def add_enum(name, info, enum_values = None, implicit = False):
+
+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):
     global enum_types
     for enum in enum_types:
@@ -424,12 +442,14 @@ def find_enum(name):
             return enum
     return None

+
 def is_enum(name):
-    return find_enum(name) != None
+    return find_enum(name) is not None

-def check_type(expr_info, source, value, allow_array = False,
-               allow_dict = False, allow_optional = False,
-               allow_metas = []):
+
+def check_type(expr_info, source, value, allow_array=False,
+               allow_dict=False, allow_optional=False,
+               allow_metas=[]):
     global all_names

     if value is None:
@@ -448,7 +468,7 @@ def check_type(expr_info, source, value, allow_array = False,

     # Check if type name for value is okay
     if isinstance(value, str):
-        if not value in all_names:
+        if value not in all_names:
             raise QAPIExprError(expr_info,
                                 "%s uses unknown type '%s'"
                                 % (source, value))
@@ -477,7 +497,8 @@ def check_type(expr_info, source, value, allow_array = False,
                    allow_metas=['built-in', 'union', 'alternate', 'struct',
                                 'enum'])

-def check_member_clash(expr_info, base_name, data, source = ""):
+
+def check_member_clash(expr_info, base_name, data, source=""):
     base = find_struct(base_name)
     assert base
     base_members = base['data']
@@ -491,6 +512,7 @@ def check_member_clash(expr_info, base_name, data, source = ""):
     if base.get('base'):
         check_member_clash(expr_info, base['base'], data, source)

+
 def check_command(expr, expr_info):
     name = expr['command']

@@ -504,6 +526,7 @@ def check_command(expr, expr_info):
                expr.get('returns'), allow_array=True,
                allow_optional=True, allow_metas=returns_meta)

+
 def check_event(expr, expr_info):
     global events
     name = expr['event']
@@ -515,19 +538,20 @@ def check_event(expr, expr_info):
                expr.get('data'), allow_dict=True, allow_optional=True,
                allow_metas=['struct'])

+
 def check_union(expr, expr_info):
     name = expr['union']
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
-    values = { 'MAX': '(automatic)' }
+    values = {'MAX': '(automatic)'}

     # Two types of unions, determined by discriminator.

     # With no discriminator it is a simple union.
     if discriminator is None:
         enum_define = None
-        allow_metas=['built-in', 'union', 'alternate', 'struct', 'enum']
+        allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
         if base is not None:
             raise QAPIExprError(expr_info,
                                 "Simple union '%s' must not have a base"
@@ -557,7 +581,7 @@ def check_union(expr, expr_info):
                                 "struct '%s'"
                                 % (discriminator, base))
         enum_define = find_enum(discriminator_type)
-        allow_metas=['struct']
+        allow_metas = ['struct']
         # Do not allow string discriminator
         if not enum_define:
             raise QAPIExprError(expr_info,
@@ -581,7 +605,7 @@ def check_union(expr, expr_info):
         # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
         if enum_define:
-            if not key in enum_define['enum_values']:
+            if key not in enum_define['enum_values']:
                 raise QAPIExprError(expr_info,
                                     "Discriminator value '%s' is not found in "
                                     "enum '%s'" %
@@ -596,10 +620,11 @@ def check_union(expr, expr_info):
                                     % (name, key, values[c_key]))
             values[c_key] = key

+
 def check_alternate(expr, expr_info):
     name = expr['alternate']
     members = expr['data']
-    values = { 'MAX': '(automatic)' }
+    values = {'MAX': '(automatic)'}
     types_seen = {}

     # Check every branch
@@ -627,11 +652,12 @@ def check_alternate(expr, expr_info):
                                 % (name, key, types_seen[qtype]))
         types_seen[qtype] = key

+
 def check_enum(expr, expr_info):
     name = expr['enum']
     members = expr.get('data')
     prefix = expr.get('prefix')
-    values = { 'MAX': '(automatic)' }
+    values = {'MAX': '(automatic)'}

     if not isinstance(members, list):
         raise QAPIExprError(expr_info,
@@ -640,7 +666,7 @@ def check_enum(expr, expr_info):
         raise QAPIExprError(expr_info,
                             "Enum '%s' requires a string for 'prefix'" % name)
     for member in members:
-        check_name(expr_info, "Member of enum '%s'" %name, member,
+        check_name(expr_info, "Member of enum '%s'" % name, member,
                    enum_member=True)
         key = camel_to_upper(member)
         if key in values:
@@ -649,6 +675,7 @@ def check_enum(expr, expr_info):
                                 % (name, member, values[key]))
         values[key] = member

+
 def check_struct(expr, expr_info):
     name = expr['struct']
     members = expr['data']
@@ -660,6 +687,7 @@ def check_struct(expr, expr_info):
     if expr.get('base'):
         check_member_clash(expr_info, expr['base'], expr['data'])

+
 def check_keys(expr_elem, meta, required, optional=[]):
     expr = expr_elem['expr']
     info = expr_elem['info']
@@ -667,22 +695,23 @@ def check_keys(expr_elem, meta, required, optional=[]):
     if not isinstance(name, str):
         raise QAPIExprError(info,
                             "'%s' key must have a string value" % meta)
-    required = required + [ meta ]
+    required = required + [meta]
     for (key, value) in expr.items():
-        if not key in required and not key in optional:
+        if key not in required and key not in optional:
             raise QAPIExprError(info,
                                 "Unknown key '%s' in %s '%s'"
                                 % (key, meta, name))
-        if (key == 'gen' or key == 'success-response') and value != False:
+        if (key == 'gen' or key == 'success-response') and value is not False:
             raise QAPIExprError(info,
                                 "'%s' of %s '%s' should only use false value"
                                 % (key, meta, name))
     for key in required:
-        if not expr.has_key(key):
+        if key not in expr:
             raise QAPIExprError(info,
                                 "Key '%s' is missing from %s '%s'"
                                 % (key, meta, name))

+
 def check_exprs(exprs):
     global all_names

@@ -692,24 +721,24 @@ def check_exprs(exprs):
     for expr_elem in exprs:
         expr = expr_elem['expr']
         info = expr_elem['info']
-        if expr.has_key('enum'):
+        if 'enum' in expr:
             check_keys(expr_elem, 'enum', ['data'], ['prefix'])
             add_enum(expr['enum'], info, expr['data'])
-        elif expr.has_key('union'):
+        elif 'union' in expr:
             check_keys(expr_elem, 'union', ['data'],
                        ['base', 'discriminator'])
             add_union(expr, info)
-        elif expr.has_key('alternate'):
+        elif 'alternate' in expr:
             check_keys(expr_elem, 'alternate', ['data'])
             add_name(expr['alternate'], info, 'alternate')
-        elif expr.has_key('struct'):
+        elif 'struct' in expr:
             check_keys(expr_elem, 'struct', ['data'], ['base'])
             add_struct(expr, info)
-        elif expr.has_key('command'):
+        elif 'command' in expr:
             check_keys(expr_elem, 'command', [],
                        ['data', 'returns', 'gen', 'success-response'])
             add_name(expr['command'], info, 'command')
-        elif expr.has_key('event'):
+        elif 'event' in expr:
             check_keys(expr_elem, 'event', [], ['data'])
             add_name(expr['event'], info, 'event')
         else:
@@ -719,11 +748,11 @@ def check_exprs(exprs):
     # Try again for hidden UnionKind enum
     for expr_elem in exprs:
         expr = expr_elem['expr']
-        if expr.has_key('union'):
+        if 'union' in expr:
             if not discriminator_find_enum_define(expr):
                 add_enum('%sKind' % expr['union'], expr_elem['info'],
                          implicit=True)
-        elif expr.has_key('alternate'):
+        elif 'alternate' in expr:
             add_enum('%sKind' % expr['alternate'], expr_elem['info'],
                      implicit=True)

@@ -732,17 +761,17 @@ def check_exprs(exprs):
         expr = expr_elem['expr']
         info = expr_elem['info']

-        if expr.has_key('enum'):
+        if 'enum' in expr:
             check_enum(expr, info)
-        elif expr.has_key('union'):
+        elif 'union' in expr:
             check_union(expr, info)
-        elif expr.has_key('alternate'):
+        elif 'alternate' in expr:
             check_alternate(expr, info)
-        elif expr.has_key('struct'):
+        elif 'struct' in expr:
             check_struct(expr, info)
-        elif expr.has_key('command'):
+        elif 'command' in expr:
             check_command(expr, info)
-        elif expr.has_key('event'):
+        elif 'event' in expr:
             check_event(expr, info)
         else:
             assert False, 'unexpected meta type'
@@ -994,6 +1023,7 @@ class QAPISchemaObjectTypeVariants(object):
             vseen = dict(seen)
             v.check(schema, self.tag_member.type, vseen)

+
 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     def __init__(self, name, typ):
         QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
@@ -1293,6 +1323,7 @@ def camel_case(name):
             new_name += ch.lower()
     return new_name

+
 # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
 # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
 # ENUM24_Name -> ENUM24_NAME
@@ -1307,14 +1338,14 @@ def camel_to_upper(value):
         c = c_fun_str[i]
         # When c is upper and no "_" appears before, do more checks
         if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
-            # Case 1: next string is lower
-            # Case 2: previous string is digit
-            if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
-            c_fun_str[i - 1].isdigit():
+            if i < l - 1 and c_fun_str[i + 1].islower():
+                new_name += '_'
+            elif c_fun_str[i - 1].isdigit():
                 new_name += '_'
         new_name += c
     return new_name.lstrip('_').upper()

+
 def c_enum_const(type_name, const_name, prefix=None):
     if prefix is not None:
         type_name = prefix
@@ -1322,6 +1353,7 @@ def c_enum_const(type_name, const_name, prefix=None):

 c_name_trans = string.maketrans('.-', '__')

+
 # Map @name to a valid C identifier.
 # If @protect, avoid returning certain ticklish identifiers (like
 # C keywords) by prepending "q_".
@@ -1334,15 +1366,16 @@ c_name_trans = string.maketrans('.-', '__')
 def c_name(name, protect=True):
     # ANSI X3J11/88-090, 3.1.1
     c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
-                     'default', 'do', 'double', 'else', 'enum', 'extern', 'float',
-                     'for', 'goto', 'if', 'int', 'long', 'register', 'return',
-                     'short', 'signed', 'sizeof', 'static', 'struct', 'switch',
-                     'typedef', 'union', 'unsigned', 'void', 'volatile', 'while'])
+                     'default', 'do', 'double', 'else', 'enum', 'extern',
+                     'float', 'for', 'goto', 'if', 'int', 'long', 'register',
+                     'return', 'short', 'signed', 'sizeof', 'static',
+                     'struct', 'switch', 'typedef', 'union', 'unsigned',
+                     'void', 'volatile', 'while'])
     # ISO/IEC 9899:1999, 6.4.1
     c99_words = set(['inline', 'restrict', '_Bool', '_Complex', '_Imaginary'])
     # ISO/IEC 9899:2011, 6.4.1
-    c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic', '_Noreturn',
-                     '_Static_assert', '_Thread_local'])
+    c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic',
+                     '_Noreturn', '_Static_assert', '_Thread_local'])
     # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
     # excluding _.*
     gcc_words = set(['asm', 'typeof'])
@@ -1358,29 +1391,34 @@ def c_name(name, protect=True):
                      'not_eq', 'or', 'or_eq', 'xor', 'xor_eq'])
     # namespace pollution:
     polluted_words = set(['unix', 'errno'])
-    if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words):
+    if protect and (name in c89_words | c99_words | c11_words | gcc_words
+                    | cpp_words | polluted_words):
         return "q_" + name
     return name.translate(c_name_trans)

 eatspace = '\033EATSPACE.'
 pointer_suffix = ' *' + eatspace

+
 def genindent(count):
     ret = ""
-    for i in range(count):
+    for _ in range(count):
         ret += " "
     return ret

 indent_level = 0

+
 def push_indent(indent_amount=4):
     global indent_level
     indent_level += indent_amount

+
 def pop_indent(indent_amount=4):
     global indent_level
     indent_level -= indent_amount

+
 # Generate @code with @kwds interpolated.
 # Obey indent_level, and strip eatspace.
 def cgen(code, **kwds):
@@ -1393,6 +1431,7 @@ def cgen(code, **kwds):
         raw = raw[0]
     return re.sub(re.escape(eatspace) + ' *', '', raw)

+
 def mcgen(code, **kwds):
     if code[0] == '\n':
         code = code[1:]
@@ -1402,6 +1441,7 @@ def mcgen(code, **kwds):
 def guardname(filename):
     return c_name(filename, protect=False).upper()

+
 def guardstart(name):
     return mcgen('''

@@ -1411,6 +1451,7 @@ def guardstart(name):
 ''',
                  name=guardname(name))

+
 def guardend(name):
     return mcgen('''

@@ -1419,6 +1460,7 @@ def guardend(name):
 ''',
                  name=guardname(name))

+
 def gen_enum_lookup(name, values, prefix=None):
     ret = mcgen('''

@@ -1440,6 +1482,7 @@ const char *const %(c_name)s_lookup[] = {
                  max_index=max_index)
     return ret

+
 def gen_enum(name, values, prefix=None):
     # append automatically generated _MAX value
     enum_values = values + ['MAX']
@@ -1471,6 +1514,7 @@ extern const char *const %(c_name)s_lookup[];
                  c_name=c_name(name))
     return ret

+
 def gen_params(arg_type, extra):
     if not arg_type:
         return extra
@@ -1491,7 +1535,8 @@ def gen_params(arg_type, extra):
 # Common command line parsing
 #

-def parse_command_line(extra_options = "", extra_long_options = []):
+
+def parse_command_line(extra_options="", extra_long_options=[]):

     try:
         opts, args = getopt.gnu_getopt(sys.argv[1:],
@@ -1542,6 +1587,7 @@ def parse_command_line(extra_options = "", extra_long_options = []):
 # Generate output files with boilerplate
 #

+
 def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
                 c_comment, h_comment):
     guard = guardname(prefix + h_file)
@@ -1569,7 +1615,7 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
 /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
 %(comment)s
 ''',
-                     comment = c_comment))
+                     comment=c_comment))

     fdecl.write(mcgen('''
 /* AUTOMATICALLY GENERATED, DO NOT MODIFY */
@@ -1578,10 +1624,11 @@ def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
 #define %(guard)s

 ''',
-                      comment = h_comment, guard = guard))
+                      comment=h_comment, guard=guard))

     return (fdef, fdecl)

+
 def close_output(fdef, fdecl):
     fdecl.write('''
 #endif
-- 
2.4.3