From fbb84eca7235ca896c0aa6d4a85bfc46e106b17e Mon Sep 17 00:00:00 2001 From: Marc Bonnici Date: Mon, 19 Oct 2020 18:07:21 +0100 Subject: [PATCH] Pylint Fixes Update our version of pylint to use the latest version and update the codebase to comply with the majority of the updates. For now disable the additional checks for `super-with-arguments`, `useless-object-inheritance`, `raise-missing-from`, `no-else-raise`, `no-else-break`, `no-else-continue` to be consistent with the existing codebase. --- .travis.yml | 3 +-- extras/pylintrc | 2 +- wa/commands/process.py | 2 +- wa/framework/configuration/core.py | 3 +-- wa/framework/configuration/parsers.py | 2 +- wa/framework/configuration/tree.py | 1 + wa/framework/execution.py | 2 +- wa/framework/host.py | 1 + wa/framework/plugin.py | 3 ++- wa/framework/pluginloader.py | 1 + wa/framework/resource.py | 2 +- wa/framework/target/runtime_config.py | 2 +- wa/utils/cpustates.py | 4 ++-- wa/utils/log.py | 4 ++-- wa/utils/misc.py | 6 +++--- wa/utils/revent.py | 2 +- wa/utils/terminalsize.py | 3 ++- wa/utils/types.py | 13 ++++++------- wa/workloads/chrome/__init__.py | 2 +- wa/workloads/geekbench/__init__.py | 2 +- wa/workloads/gmail/__init__.py | 2 +- wa/workloads/googlemaps/__init__.py | 2 +- 22 files changed, 33 insertions(+), 31 deletions(-) diff --git a/.travis.yml b/.travis.yml index 54b576e1..4940bc92 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,8 +22,7 @@ install: - pip install nose - pip install nose2 - pip install flake8 - - pip install isort==4.3.21 - - pip install pylint==1.9.2 + - pip install pylint==2.6.0 - git clone -v https://github.com/ARM-software/devlib.git /tmp/devlib && cd /tmp/devlib && python setup.py install - cd $TRAVIS_BUILD_DIR && python setup.py install diff --git a/extras/pylintrc b/extras/pylintrc index 5714c5ac..6bbe0846 100644 --- a/extras/pylintrc +++ b/extras/pylintrc @@ -43,7 +43,7 @@ ignore=external # https://bitbucket.org/logilab/pylint/issue/232/wrong-hanging-indentation-false-positive # TODO: disabling no-value-for-parameter and logging-format-interpolation, as they appear to be broken # in version 1.4.1 and return a lot of false postives; should be re-enabled once fixed. -disable=C0301,C0103,C0111,W0142,R0903,R0904,R0922,W0511,W0141,I0011,R0921,W1401,C0330,no-value-for-parameter,logging-format-interpolation,no-else-return,inconsistent-return-statements,keyword-arg-before-vararg,consider-using-enumerate,no-member +disable=C0301,C0103,C0111,W0142,R0903,R0904,R0922,W0511,W0141,I0011,R0921,W1401,C0330,no-value-for-parameter,logging-format-interpolation,no-else-return,inconsistent-return-statements,keyword-arg-before-vararg,consider-using-enumerate,no-member,super-with-arguments,useless-object-inheritance,raise-missing-from,no-else-raise,no-else-break,no-else-continue [FORMAT] max-module-lines=4000 diff --git a/wa/commands/process.py b/wa/commands/process.py index 236f65de..7d312e60 100644 --- a/wa/commands/process.py +++ b/wa/commands/process.py @@ -78,7 +78,7 @@ class ProcessCommand(Command): if not args.recursive: output_list = [RunOutput(process_directory)] else: - output_list = [output for output in discover_wa_outputs(process_directory)] + output_list = list(discover_wa_outputs(process_directory)) pc = ProcessContext() for run_output in output_list: diff --git a/wa/framework/configuration/core.py b/wa/framework/configuration/core.py index 90ecbf59..4a6c7a9c 100644 --- a/wa/framework/configuration/core.py +++ b/wa/framework/configuration/core.py @@ -252,8 +252,7 @@ class ConfigurationPoint(object): a warning to the user however will continue execution. """ self.name = identifier(name) - if kind in KIND_MAP: - kind = KIND_MAP[kind] + kind = KIND_MAP.get(kind, kind) if kind is not None and not callable(kind): raise ValueError('Kind must be callable.') self.kind = kind diff --git a/wa/framework/configuration/parsers.py b/wa/framework/configuration/parsers.py index a39e6188..4fdac4ba 100644 --- a/wa/framework/configuration/parsers.py +++ b/wa/framework/configuration/parsers.py @@ -297,7 +297,7 @@ def merge_augmentations(raw): raise ConfigError(msg.format(value, n, exc)) # Make sure none of the specified aliases conflict with each other - to_check = [e for e in entries] + to_check = list(entries) while len(to_check) > 1: check_entry = to_check.pop() for e in to_check: diff --git a/wa/framework/configuration/tree.py b/wa/framework/configuration/tree.py index 72d457da..f45c5cdf 100644 --- a/wa/framework/configuration/tree.py +++ b/wa/framework/configuration/tree.py @@ -33,6 +33,7 @@ class JobSpecSource(object): def id(self): return self.config['id'] + @property def name(self): raise NotImplementedError() diff --git a/wa/framework/execution.py b/wa/framework/execution.py index 458fb826..97143280 100644 --- a/wa/framework/execution.py +++ b/wa/framework/execution.py @@ -449,7 +449,7 @@ class Executor(object): for status in reversed(Status.levels): if status in counter: parts.append('{} {}'.format(counter[status], status)) - self.logger.info(status_summary + ', '.join(parts)) + self.logger.info('{}{}'.format(status_summary, ', '.join(parts))) self.logger.info('Results can be found in {}'.format(output.basepath)) diff --git a/wa/framework/host.py b/wa/framework/host.py index 4d4bfb26..973a253f 100644 --- a/wa/framework/host.py +++ b/wa/framework/host.py @@ -50,6 +50,7 @@ def init_user_directory(overwrite_existing=False): # pylint: disable=R0914 # If running with sudo on POSIX, change the ownership to the real user. real_user = os.getenv('SUDO_USER') if real_user: + # pylint: disable=import-outside-toplevel import pwd # done here as module won't import on win32 user_entry = pwd.getpwnam(real_user) uid, gid = user_entry.pw_uid, user_entry.pw_gid diff --git a/wa/framework/plugin.py b/wa/framework/plugin.py index 9fcdcaf5..6c00ef6f 100644 --- a/wa/framework/plugin.py +++ b/wa/framework/plugin.py @@ -157,6 +157,7 @@ class Alias(object): raise ConfigError(msg.format(param, self.name, ext.name)) +# pylint: disable=bad-mcs-classmethod-argument class PluginMeta(type): """ This basically adds some magic to plugins to make implementing new plugins, @@ -367,7 +368,7 @@ class Plugin(with_metaclass(PluginMeta, object)): self._modules.append(module) def __str__(self): - return self.name + return str(self.name) def __repr__(self): params = [] diff --git a/wa/framework/pluginloader.py b/wa/framework/pluginloader.py index 36f486a1..45b1a027 100644 --- a/wa/framework/pluginloader.py +++ b/wa/framework/pluginloader.py @@ -35,6 +35,7 @@ class __LoaderWrapper(object): def reset(self): # These imports cannot be done at top level, because of # sys.modules manipulation below + # pylint: disable=import-outside-toplevel from wa.framework.plugin import PluginLoader from wa.framework.configuration.core import settings self._loader = PluginLoader(settings.plugin_packages, diff --git a/wa/framework/resource.py b/wa/framework/resource.py index 485a86e7..c69ded8c 100644 --- a/wa/framework/resource.py +++ b/wa/framework/resource.py @@ -281,7 +281,7 @@ def apk_version_matches(path, version): version = list_or_string(version) info = get_cacheable_apk_info(path) for v in version: - if info.version_name == v or info.version_code == v: + if v in (info.version_name, info.version_code): return True if loose_version_matching(v, info.version_name): return True diff --git a/wa/framework/target/runtime_config.py b/wa/framework/target/runtime_config.py index 9575af32..c76248d2 100644 --- a/wa/framework/target/runtime_config.py +++ b/wa/framework/target/runtime_config.py @@ -732,7 +732,7 @@ class IdleStateValue(object): '''Checks passed state and converts to its ID''' value = caseless_string(value) for s_id, s_name, s_desc in self.values: - if value == s_id or value == s_name or value == s_desc: + if value in (s_id, s_name, s_desc): return s_id msg = 'Invalid IdleState: "{}"; Must be in {}' raise ValueError(msg.format(value, self.values)) diff --git a/wa/utils/cpustates.py b/wa/utils/cpustates.py index fe088f05..60a16fb2 100755 --- a/wa/utils/cpustates.py +++ b/wa/utils/cpustates.py @@ -151,7 +151,7 @@ class PowerStateProcessor(object): def __init__(self, cpus, wait_for_marker=True, no_idle=None): if no_idle is None: - no_idle = False if cpus[0].cpuidle and cpus[0].cpuidle.states else True + no_idle = not (cpus[0].cpuidle and cpus[0].cpuidle.states) self.power_state = SystemPowerState(len(cpus), no_idle=no_idle) self.requested_states = {} # cpu_id -> requeseted state self.wait_for_marker = wait_for_marker @@ -405,7 +405,7 @@ class ParallelStats(object): for i, clust in enumerate(clusters): self.clusters[str(i)] = set(clust) - self.clusters['all'] = set([cpu.id for cpu in cpus]) + self.clusters['all'] = {cpu.id for cpu in cpus} self.first_timestamp = None self.last_timestamp = None diff --git a/wa/utils/log.py b/wa/utils/log.py index e16afd84..2ad4d151 100644 --- a/wa/utils/log.py +++ b/wa/utils/log.py @@ -78,8 +78,8 @@ def init(verbosity=logging.INFO, color=True, indent_with=4, _console_handler.setFormatter(formatter(regular_fmt)) root_logger.addHandler(_console_handler) - buffer_capacity = os.getenv('WA_LOG_BUFFER_CAPACITY', - DEFAULT_INIT_BUFFER_CAPACITY) + buffer_capacity = int(os.getenv('WA_LOG_BUFFER_CAPACITY', + str(DEFAULT_INIT_BUFFER_CAPACITY))) _init_handler = InitHandler(buffer_capacity) _init_handler.setLevel(logging.DEBUG) root_logger.addHandler(_init_handler) diff --git a/wa/utils/misc.py b/wa/utils/misc.py index f5f23317..06fda7b5 100644 --- a/wa/utils/misc.py +++ b/wa/utils/misc.py @@ -335,7 +335,7 @@ def load_struct_from_yaml(filepath=None, text=None): of basic Python types (strings, ints, lists, dicts, etc.).""" # Import here to avoid circular imports - # pylint: disable=wrong-import-position,cyclic-import + # pylint: disable=wrong-import-position,cyclic-import, import-outside-toplevel from wa.utils.serializer import yaml if not (filepath or text) or (filepath and text): @@ -361,7 +361,7 @@ def load_struct_from_file(filepath): """ extn = os.path.splitext(filepath)[1].lower() - if (extn == '.py') or (extn == '.pyc') or (extn == '.pyo'): + if extn in ('.py', '.pyc', '.pyo'): return load_struct_from_python(filepath) elif extn == '.yaml': return load_struct_from_yaml(filepath) @@ -718,7 +718,7 @@ def lock_file(path, timeout=30): """ # Import here to avoid circular imports - # pylint: disable=wrong-import-position,cyclic-import + # pylint: disable=wrong-import-position,cyclic-import, import-outside-toplevel from wa.framework.exception import ResourceError locked = False diff --git a/wa/utils/revent.py b/wa/utils/revent.py index 2593bdd4..eff082a7 100644 --- a/wa/utils/revent.py +++ b/wa/utils/revent.py @@ -188,7 +188,7 @@ class ReventRecording(object): self._parse_header_and_devices(self.fh) self._events_start = self.fh.tell() if not self.stream: - self._events = [e for e in self._iter_events()] + self._events = list(self._iter_events()) finally: if self._close_when_done: self.close() diff --git a/wa/utils/terminalsize.py b/wa/utils/terminalsize.py index 7b9301a3..0ee8e7dc 100644 --- a/wa/utils/terminalsize.py +++ b/wa/utils/terminalsize.py @@ -45,7 +45,7 @@ def get_terminal_size(): def _get_terminal_size_windows(): - # pylint: disable=unused-variable,redefined-outer-name,too-many-locals + # pylint: disable=unused-variable,redefined-outer-name,too-many-locals, import-outside-toplevel try: from ctypes import windll, create_string_buffer # stdin handle is -10 @@ -77,6 +77,7 @@ def _get_terminal_size_tput(): def _get_terminal_size_linux(): + # pylint: disable=import-outside-toplevel def ioctl_GWINSZ(fd): try: import fcntl diff --git a/wa/utils/types.py b/wa/utils/types.py index 5968e60c..5d7b60a5 100644 --- a/wa/utils/types.py +++ b/wa/utils/types.py @@ -226,7 +226,7 @@ def module_name_set(l): # noqa: E741 modules = set() for m in l: if m and isinstance(m, dict): - modules.update({k for k in m.keys()}) + modules.update(m.keys()) else: modules.add(m) return modules @@ -374,9 +374,8 @@ class prioritylist(object): else: raise ValueError('Invalid index {}'.format(index)) current_global_offset = 0 - priority_counts = {priority: count for (priority, count) in - zip(self.priorities, [len(self.elements[p]) - for p in self.priorities])} + priority_counts = dict(zip(self.priorities, [len(self.elements[p]) + for p in self.priorities])) for priority in self.priorities: if not index_range: break @@ -462,7 +461,7 @@ class toggle_set(set): """ returns a list of enabled items. """ - return set([item for item in self if not item.startswith('~')]) + return {item for item in self if not item.startswith('~')} def conflicts_with(self, other): """ @@ -581,7 +580,7 @@ class level(object): return repr(self) def __str__(self): - return self.name + return str(self.name) def __repr__(self): return '{}({})'.format(self.name, self.value) @@ -806,7 +805,7 @@ class ParameterDict(dict): def update(self, *args, **kwargs): for d in list(args) + [kwargs]: - for k, v in d: + for k, v in d.items(): self[k] = v diff --git a/wa/workloads/chrome/__init__.py b/wa/workloads/chrome/__init__.py index 8de36a14..5c248c53 100644 --- a/wa/workloads/chrome/__init__.py +++ b/wa/workloads/chrome/__init__.py @@ -91,4 +91,4 @@ class Chrome(ApkUiautoWorkload): owner = self.target.execute("{} stat -c '%u' {}".format(self.target.busybox, offline_pages), as_root=True).strip() self.target.execute('{} tar -xvf {} -C {}'.format(self.target.busybox, archives_src, archives_dst), as_root=True) self.target.execute('{} cp {} {}'.format(self.target.busybox, metadata_src, metadata_dst), as_root=True) - self.target.execute('{} chown -R {}:{} {}'.format(self.target.busybox, owner, owner, offline_pages), as_root=True) + self.target.execute('{0} chown -R {1}:{1} {2}'.format(self.target.busybox, owner, offline_pages), as_root=True) diff --git a/wa/workloads/geekbench/__init__.py b/wa/workloads/geekbench/__init__.py index 95581c2b..4962d281 100644 --- a/wa/workloads/geekbench/__init__.py +++ b/wa/workloads/geekbench/__init__.py @@ -257,7 +257,7 @@ class GBScoreCalculator(object): 'memory': 0.1926489, 'stream': 0.1054738, } - # pylint: disable=C0326 + workloads = [ # ID Name Power Mac ST Power Mac MT GBWorkload(101, 'Blowfish', 43971, 40979), # NOQA diff --git a/wa/workloads/gmail/__init__.py b/wa/workloads/gmail/__init__.py index 489d7869..aeb5bd3f 100755 --- a/wa/workloads/gmail/__init__.py +++ b/wa/workloads/gmail/__init__.py @@ -109,4 +109,4 @@ class Gmail(ApkUiautoWorkload): owner = self.target.execute("{} stat -c '%u' {}".format(self.target.busybox, database_dst), as_root=True).strip() self.target.execute('{} rm {}'.format(self.target.busybox, existing_mailstores), as_root=True) self.target.execute('{} tar -xvf {} -C {}'.format(self.target.busybox, database_src, database_dst), as_root=True) - self.target.execute('{} chown -R {}:{} {}'.format(self.target.busybox, owner, owner, database_dst), as_root=True) + self.target.execute('{0} chown -R {1}:{1} {2}'.format(self.target.busybox, owner, database_dst), as_root=True) diff --git a/wa/workloads/googlemaps/__init__.py b/wa/workloads/googlemaps/__init__.py index 91c5349f..ef0ef893 100644 --- a/wa/workloads/googlemaps/__init__.py +++ b/wa/workloads/googlemaps/__init__.py @@ -85,4 +85,4 @@ class GoogleMaps(ApkUiautoWorkload): owner = self.target.execute("{} stat -c '%u' {}".format(self.target.busybox, package_data_dir), as_root=True).strip() self.target.execute('{} tar -xvf {} -C {}'.format(self.target.busybox, databases_src, databases_dst), as_root=True) self.target.execute('{} tar -xvf {} -C {}'.format(self.target.busybox, files_src, files_dst), as_root=True) - self.target.execute('{} chown -R {}:{} {}'.format(self.target.busybox, owner, owner, package_data_dir), as_root=True) + self.target.execute('{0} chown -R {1}:{1} {2}'.format(self.target.busybox, owner, package_data_dir), as_root=True)