Merge ~juergh/+git/annotations-tools:wip/lint into ~arighi/+git/annotations-tools:master

[Juerg Haefliger]

Add pylint checks and fixes.

diff --git a/Makefile b/Makefile
index 68a710b..317440a 100644
--- a/Makefile
+++ b/Makefile
@@ -1,6 +1,11 @@
-.PHONY: all lint
+.PHONY: all lint flake8 pylint
 
 all:  lint
 
-lint:
+lint: flake8 pylint
+
+flake8:
 	flake8 annotations .
+
+pylint:
+	pylint annotations kconfig
diff --git a/annotations b/annotations
index 7e7ff5e..ea6bc9b 100755
--- a/annotations
+++ b/annotations
@@ -8,9 +8,10 @@ sys.dont_write_bytecode = True
 import os
 import argparse
 import json
-from kconfig.annotations import Annotation, KConfig
 from signal import signal, SIGPIPE, SIG_DFL
 
+from kconfig.annotations import Annotation, KConfig
+
 VERSION = '0.1'
 
 SKIP_CONFIGS = (
@@ -76,7 +77,7 @@ _ARGPARSER = make_parser()
 def arg_fail(message):
     print(message)
     _ARGPARSER.print_usage()
-    exit(1)
+    sys.exit(1)
 
 
 def print_result(config, res):
@@ -96,7 +97,8 @@ def do_query(args):
 def do_autocomplete(args):
     a = Annotation(args.file)
     res = (c.removeprefix('CONFIG_') for c in a.search_config())
-    print('complete -W "{}" annotations'.format(' '.join(res)))
+    res_str = ' '.join(res)
+    print(f'complete -W "{res_str}" annotations')
 
 
 def do_source(args):
@@ -104,7 +106,7 @@ def do_source(args):
         arg_fail('error: --source requires --config')
     if not os.path.exists('tags'):
         print('tags not found in the current directory, try: `make tags`')
-        exit(1)
+        sys.exit(1)
     os.system(f'vim -t {args.config}')
 
 
@@ -221,7 +223,7 @@ def do_check(args):
         total += 1
 
     print(f"check-config: {good}/{total} checks passed -- exit {ret}")
-    exit(ret)
+    sys.exit(ret)
 
 
 def autodetect_annotations(args):
@@ -230,9 +232,9 @@ def autodetect_annotations(args):
     # If --file/-f isn't specified try to automatically determine the right
     # location of the annotations file looking at debian/debian.env.
     try:
-        with open('debian/debian.env', 'rt') as fd:
+        with open('debian/debian.env', 'rt', encoding='utf-8') as fd:
             args.file = fd.read().rstrip().split('=')[1] + '/config/annotations'
-    except Exception:
+    except (FileNotFoundError, IndexError):
         arg_fail('error: could not determine DEBDIR, try using: --file/-f')
 
 
diff --git a/kconfig/annotations.py b/kconfig/annotations.py
index 19fb544..a1ad282 100644
--- a/kconfig/annotations.py
+++ b/kconfig/annotations.py
@@ -7,21 +7,26 @@ import json
 import re
 import shutil
 import tempfile
+
+from abc import abstractmethod
 from ast import literal_eval
 from os.path import dirname, abspath
 
 
-class Config(object):
-    def __init__(self, fname: str, arch: str = None, flavour: str = None):
+class Config():
+    def __init__(self, fname):
         """
         Basic configuration file object
         """
         self.fname = fname
+        self.config = {}
+
         raw_data = self._load(fname)
         self._parse(raw_data)
 
-    def _load(self, fname: str) -> str:
-        with open(fname, 'rt') as fd:
+    @staticmethod
+    def _load(fname: str) -> str:
+        with open(fname, 'rt', encoding='utf-8') as fd:
             data = fd.read()
         return data.rstrip()
 
@@ -29,13 +34,17 @@ class Config(object):
         """ Return a JSON representation of the config """
         return json.dumps(self.config, indent=4)
 
+    @abstractmethod
+    def _parse(self, data: str):
+        pass
+
 
 class KConfig(Config):
     """
     Parse a .config file, individual config options can be accessed via
     .config[<CONFIG_OPTION>]
     """
-    def _parse(self, data: str) -> dict:
+    def _parse(self, data: str):
         self.config = {}
         for line in data.splitlines():
             m = re.match(r'^# (CONFIG_.*) is not set$', line)
@@ -107,17 +116,17 @@ class Annotation(Config):
                         raise Exception('syntax error')
                     self.config[conf] = entry
                 except Exception as e:
-                    raise Exception(str(e) + f', line = {line}')
+                    raise Exception(str(e) + f', line = {line}') from e
                 continue
 
             # Invalid line
             raise Exception(f'invalid line: {line}')
 
-    """
-    Parse main annotations file, individual config options can be accessed via
-    self.config[<CONFIG_OPTION>]
-    """
-    def _parse(self, data: str) -> dict:
+    def _parse(self, data: str):
+        """
+        Parse main annotations file, individual config options can be accessed
+        via self.config[<CONFIG_OPTION>]
+        """
         self.config = {}
         self.arch = []
         self.flavour = []
@@ -138,7 +147,7 @@ class Annotation(Config):
                     self.flavour = list(m.group(1).split(' '))
                 m = re.match(r'^# FLAVOUR_DEP: (.*)', line)
                 if m:
-                    self.flavour_dep = eval(m.group(1))
+                    self.flavour_dep = literal_eval(m.group(1))
                 self.header += line + "\n"
             else:
                 break
@@ -183,12 +192,12 @@ class Annotation(Config):
                     flavour = arch
                 self.config[config]['policy'][flavour] = value
             else:
-                for arch in self.arch:
-                    self.config[config]['policy'][arch] = value
+                for a in self.arch:
+                    self.config[config]['policy'][a] = value
         if note is not None:
             self.config[config]['note'] = "'" + note.replace("'", '') + "'"
 
-    def update(self, c: KConfig, arch: str, flavour: str = None, configs: list = []):
+    def update(self, c: KConfig, arch: str, flavour: str = None, configs: list = None):
         """ Merge configs from a Kconfig object into Annotation object """
 
         # Determine if we need to import all configs or a single config
@@ -260,7 +269,7 @@ class Annotation(Config):
                     for flavour in arch_flavours:
                         if flavour not in self.config[conf]['policy']:
                             break
-                        elif value is None:
+                        if value is None:
                             value = self.config[conf]['policy'][flavour]
                         elif value != self.config[conf]['policy'][flavour]:
                             break
@@ -282,7 +291,8 @@ class Annotation(Config):
                 if not self.config[conf]['policy']:
                     del self.config[conf]
 
-    def _sorted(self, config):
+    @staticmethod
+    def _sorted(config):
         """ Sort configs alphabetically but return configs with a note first """
         w_note = []
         wo_note = []
@@ -362,31 +372,31 @@ class Annotation(Config):
         if config is None and arch is None:
             # Get all config options for all architectures
             return self.config
-        elif config is None and arch is not None:
+        if config is None and arch is not None:
             # Get config options of a specific architecture
             ret = {}
-            for c in self.config:
-                if 'policy' not in self.config[c]:
+            for c, val in self.config.items():
+                if 'policy' not in val:
                     continue
-                if flavour in self.config[c]['policy']:
-                    ret[c] = self.config[c]['policy'][flavour]
-                elif generic != flavour and generic in self.config[c]['policy']:
-                    ret[c] = self.config[c]['policy'][generic]
-                elif arch in self.config[c]['policy']:
-                    ret[c] = self.config[c]['policy'][arch]
+                if flavour in val['policy']:
+                    ret[c] = val['policy'][flavour]
+                elif generic != flavour and generic in val['policy']:
+                    ret[c] = val['policy'][generic]
+                elif arch in val['policy']:
+                    ret[c] = val['policy'][arch]
             return ret
-        elif config is not None and arch is None:
+        if config is not None and arch is None:
             # Get a specific config option for all architectures
             return self.config[config] if config in self.config else None
-        elif config is not None and arch is not None:
+        if config is not None and arch is not None:
             # Get a specific config option for a specific architecture
             if config in self.config:
                 if 'policy' in self.config[config]:
                     if flavour in self.config[config]['policy']:
                         return {config: self.config[config]['policy'][flavour]}
-                    elif generic != flavour and generic in self.config[config]['policy']:
+                    if generic != flavour and generic in self.config[config]['policy']:
                         return {config: self.config[config]['policy'][generic]}
-                    elif arch in self.config[config]['policy']:
+                    if arch in self.config[config]['policy']:
                         return {config: self.config[config]['policy'][arch]}
         return None
 
diff --git a/setup.cfg b/setup.cfg
index 6fff888..00a726a 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -3,3 +3,33 @@ max_line_length = 120
 per-file-ignores =
     # E402 module level import not at top of file
     annotations: E402
+
+
+[pylint.FORMAT]
+max-line-length = 120
+
+[pylint]
+# These are the default disables but for some reason we seem to loose them
+# due to the above statement (huh?). So redefine them.
+disable = invalid-name,
+          missing-module-docstring,
+          missing-class-docstring,
+          missing-function-docstring,
+          wrong-import-position,
+          raw-checker-failed,
+          bad-inline-option,
+          locally-disabled,
+          file-ignored,
+          suppressed-message,
+          useless-suppression,
+          deprecated-pragma,
+          use-symbolic-message-instead,
+          too-many-instance-attributes,
+          too-many-arguments,
+          too-many-locals,
+          too-many-statements,
+          redefined-outer-name,
+          # End of default disables
+          too-many-branches,
+          too-many-return-statements,
+          too-few-public-methods