Re: [PATCH 4/4] qapi: Brush off some (py)lint

[John Snow]

On 2/27/20 9:45 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <address@hidden>

I wrote some pylint cleanup for iotests recently, too. Are you targeting
a subset of pylint errors to clean here?

(Do any files pass 100%?)

Consider checking in a pylintrc file that lets others run the same
subset of pylint tests as you are doing so that we can prevent future
regressions.

Take a peek at [PATCH v6 0/9] iotests: use python logging​

Thanks for this series. I had a very similar series sitting waiting to
go out, but this goes further in a few places.

--js

> ---
>  scripts/qapi/commands.py   | 2 +-
>  scripts/qapi/expr.py       | 3 +--
>  scripts/qapi/gen.py        | 9 ++++++---
>  scripts/qapi/introspect.py | 2 --
>  scripts/qapi/parser.py     | 6 ++----
>  scripts/qapi/schema.py     | 9 ++++-----
>  6 files changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 8bb6316061..0e13e82989 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -274,7 +274,7 @@ class 
> QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
>  
>  void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
>  ''',
> -                       c_prefix=c_name(self._prefix, protect=False)))
> +                             c_prefix=c_name(self._prefix, protect=False)))
>          self._genc.preamble_add(mcgen('''
>  #include "qemu/osdep.h"
>  #include "%(prefix)sqapi-commands.h"
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index d7a289eded..fecf466fa7 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -35,7 +35,6 @@ def check_name_is_str(name, info, source):
>  def check_name_str(name, info, source,
>                     allow_optional=False, enum_member=False,
>                     permit_upper=False):
> -    global valid_name
>      membername = name
>  
>      if allow_optional and name.startswith('*'):
> @@ -249,7 +248,7 @@ def check_union(expr, info):
>  def check_alternate(expr, info):
>      members = expr['data']
>  
> -    if len(members) == 0:
> +    if not members:
>          raise QAPISemError(info, "'data' must not be empty")
>      for (key, value) in members.items():
>          source = "'data' member '%s'" % key
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index e17354392b..33690bfa3b 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -45,10 +45,10 @@ class QAPIGen:
>  
>      def write(self, output_dir):
>          pathname = os.path.join(output_dir, self.fname)
> -        dir = os.path.dirname(pathname)
> -        if dir:
> +        odir = os.path.dirname(pathname)
> +        if odir:
>              try:
> -                os.makedirs(dir)
> +                os.makedirs(odir)
>              except os.error as e:
>                  if e.errno != errno.EEXIST:
>                      raise
> @@ -261,6 +261,9 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
>              genc.write(output_dir)
>              genh.write(output_dir)
>  
> +    def _begin_system_module(self, name):
> +        pass
> +
>      def _begin_user_module(self, name):
>          pass
>  
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 0cc655fd9f..b5537eddc0 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -10,8 +10,6 @@ This work is licensed under the terms of the GNU GPL, 
> version 2.
>  See the COPYING file in the top-level directory.
>  """
>  
> -import string
> -
>  from qapi.common import *
>  from qapi.gen import QAPISchemaMonolithicCVisitor
>  from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType,
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 340f7c4633..abadacbb0e 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -282,8 +282,7 @@ class QAPISchemaParser:
>                  doc.end_comment()
>                  self.accept()
>                  return doc
> -            else:
> -                doc.append(self.val)
> +            doc.append(self.val)
>              self.accept(False)
>  
>          raise QAPIParseError(self, "documentation comment must end with 
> '##'")
> @@ -492,7 +491,7 @@ class QAPIDoc:
>              raise QAPIParseError(self._parser,
>                                   "'%s' can't follow '%s' section"
>                                   % (name, self.sections[0].name))
> -        elif self._is_section_tag(name):
> +        if self._is_section_tag(name):
>              line = line[len(name)+1:]
>              self._start_section(name[:-1])
>  
> @@ -556,7 +555,6 @@ class QAPIDoc:
>              raise QAPISemError(feature.info,
>                                 "feature '%s' lacks documentation"
>                                 % feature.name)
> -            self.features[feature.name] = QAPIDoc.ArgSection(feature.name)
>          self.features[feature.name].connect(feature)
>  
>      def check_expr(self, expr):
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index e132442c04..cfbb9758c4 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -19,7 +19,7 @@ import re
>  from collections import OrderedDict
>  
>  from qapi.common import c_name, pointer_suffix
> -from qapi.error import QAPIError, QAPIParseError, QAPISemError
> +from qapi.error import QAPIError, QAPISemError
>  from qapi.expr import check_exprs
>  from qapi.parser import QAPISchemaParser
>  
> @@ -96,14 +96,14 @@ class QAPISchemaVisitor:
>      def visit_end(self):
>          pass
>  
> -    def visit_module(self, fname):
> +    def visit_module(self, name):
>          pass
>  
>      def visit_needed(self, entity):
>          # Default to visiting everything
>          return True
>  
> -    def visit_include(self, fname, info):
> +    def visit_include(self, name, info):
>          pass
>  
>      def visit_builtin_type(self, name, info, json_type):
> @@ -576,7 +576,7 @@ class QAPISchemaObjectTypeVariants:
>              assert self.tag_member.ifcond == []
>          if self._tag_name:    # flat union
>              # branches that are not explicitly covered get an empty type
> -            cases = set([v.name for v in self.variants])
> +            cases = {v.name for v in self.variants}
>              for m in self.tag_member.type.members:
>                  if m.name not in cases:
>                      v = QAPISchemaObjectTypeVariant(m.name, self.info,
> @@ -1098,7 +1098,6 @@ class QAPISchema:
>  
>      def visit(self, visitor):
>          visitor.visit_begin(self)
> -        module = None
>          for mod in self._module_dict.values():
>              mod.visit(visitor)
>          visitor.visit_end()
> 

-- 
—js