Fix issues found by pylint 2.0

Resolve:
 - no-else-return
 - useless-object-inheritance
 - useless-return
 - consider-using-set-comprehension
 - consider-using-in
 - logging-not-lazy

Ignore:
 - not-an-iterable - false possitives for asyncio coroutines

Ignore all the above in qubespolicy/__init__.py, as the file will be
moved to separate repository (core-qrexec) - it already has a copy
there, don't desynchronize them.

ci/pylintrc

@@ -6,6 +6,8 @@ ignore=tests
 # abstract-class-little-used: see http://www.logilab.org/ticket/111138
 # deprecated-method:
 #   enable again after disabling py-3.4.3 asyncio.ensure_future compat hack
+# not-an-iterable:
+#   a lot of false possitives for asyncio (yield from (some coroutine))
 disable=
   abstract-class-little-used,
   bad-continuation,
@@ -18,6 +20,7 @@ disable=
   locally-enabled,
   logging-format-interpolation,
   missing-docstring,
+  not-an-iterable,
   star-args,
   wrong-import-order
 

qubes/__init__.py

@@ -42,7 +42,7 @@ __license__ = 'GPLv2 or later'
 __version__ = 'R3'
 
 
-class Label(object):
+class Label:
     '''Label definition for virtual machines
 
     Label specifies colour of the padlock displayed next to VM's name.
@@ -134,7 +134,7 @@ class Label(object):
             self.icon_dispvm) + ".png"
 
 
-class property(object):  # pylint: disable=redefined-builtin,invalid-name
+class property:  # pylint: disable=redefined-builtin,invalid-name
     '''Qubes property.
 
     This class holds one property that can be saved to and loaded from
@@ -350,7 +350,6 @@ class property(object):  # pylint: disable=redefined-builtin,invalid-name
                 raise qubes.exc.QubesValueError
             if self.type is bool:
                 return self.bool(None, None, untrusted_newvalue)
-            else:
             try:
                 return self.type(untrusted_newvalue)
             except ValueError:

qubes/api/__init__.py

@@ -97,7 +97,7 @@ def apply_filters(iterable, filters):
     return iterable
 
 
-class AbstractQubesAPI(object):
+class AbstractQubesAPI:
     '''Common code for Qubes Management Protocol handling
 
     Different interfaces can expose different API call sets, however they share

qubes/api/admin.py

@@ -44,7 +44,7 @@ import qubes.vm.adminvm
 import qubes.vm.qubesvm
 
 
-class QubesMgmtEventsDispatcher(object):
+class QubesMgmtEventsDispatcher:
     def __init__(self, filters, send_event):
         self.filters = filters
         self.send_event = send_event

qubes/app.py

@@ -66,7 +66,7 @@ import qubes.vm.qubesvm
 import qubes.vm.templatevm
 # pylint: enable=wrong-import-position
 
-class VirDomainWrapper(object):
+class VirDomainWrapper:
     # pylint: disable=too-few-public-methods
 
     def __init__(self, connection, vm):
@@ -97,7 +97,7 @@ class VirDomainWrapper(object):
         return wrapper
 
 
-class VirConnectWrapper(object):
+class VirConnectWrapper:
     # pylint: disable=too-few-public-methods
 
     def __init__(self, uri):
@@ -134,7 +134,7 @@ class VirConnectWrapper(object):
         return wrapper
 
 
-class VMMConnection(object):
+class VMMConnection:
     '''Connection to Virtual Machine Manager (libvirt)'''
 
     def __init__(self, offline_mode=None):
@@ -229,7 +229,7 @@ class VMMConnection(object):
         self._xc = None  # and pray it will get garbage-collected
 
 
-class QubesHost(object):
+class QubesHost:
     '''Basic information about host machine
 
     :param qubes.Qubes app: Qubes application context (must have \
@@ -363,7 +363,7 @@ class QubesHost(object):
         return (current_time, current)
 
 
-class VMCollection(object):
+class VMCollection:
     '''A collection of Qubes VMs
 
     VMCollection supports ``in`` operator. You may test for ``qid``, ``name``
@@ -493,7 +493,7 @@ class VMCollection(object):
         self.app.fire_event('domain-delete', vm=vm)
 
     def __contains__(self, key):
-        return any((key == vm or key == vm.qid or key == vm.name)
+        return any((key in (vm, vm.qid, vm.name))
                    for vm in self)
 
 
@@ -557,7 +557,7 @@ def _default_pool(app):
     '''
     if 'default' in app.pools:
         return app.pools['default']
-    else:
+
     if 'DEFAULT_LVM_POOL' in os.environ:
         thin_pool = os.environ['DEFAULT_LVM_POOL']
         for pool in app.pools.values():

qubes/backup.py

@@ -75,7 +75,7 @@ class BackupCanceledError(qubes.exc.QubesException):
         self.tmpdir = tmpdir
 
 
-class BackupHeader(object):
+class BackupHeader:
     '''Structure describing backup-header file included as the first file in
     backup archive
     '''
@@ -123,7 +123,7 @@ class BackupHeader(object):
                 f_header.write("{!s}={!s}\n".format(key, getattr(self, attr)))
 
 
-class SendWorker(object):
+class SendWorker:
     # pylint: disable=too-few-public-methods
     def __init__(self, queue, base_dir, backup_stdout):
         super(SendWorker, self).__init__()
@@ -147,6 +147,7 @@ class SendWorker(object):
             # verified before untaring.
             tar_final_cmd = ["tar", "-cO", "--posix",
                              "-C", self.base_dir, filename]
+            # pylint: disable=not-an-iterable
             final_proc = yield from asyncio.create_subprocess_exec(
                 *tar_final_cmd,
                 stdout=self.backup_stdout)
@@ -182,6 +183,7 @@ def launch_proc_with_pty(args, stdin=None, stdout=None, stderr=None, echo=True):
             termios_p[3] &= ~termios.ECHO
             termios.tcsetattr(ctty_fd, termios.TCSANOW, termios_p)
     (pty_master, pty_slave) = os.openpty()
+    # pylint: disable=not-an-iterable
     p = yield from asyncio.create_subprocess_exec(*args,
         stdin=stdin,
         stdout=stdout,
@@ -227,7 +229,7 @@ def launch_scrypt(action, input_name, output_name, passphrase):
     return p
 
 
-class Backup(object):
+class Backup:
     '''Backup operation manager. Usage:
 
     >>> app = qubes.Qubes()
@@ -250,7 +252,7 @@ class Backup(object):
 
     '''
     # pylint: disable=too-many-instance-attributes
-    class FileToBackup(object):
+    class FileToBackup:
         # pylint: disable=too-few-public-methods
         def __init__(self, file_path, subdir=None, name=None, size=None):
             if size is None:
@@ -280,7 +282,7 @@ class Backup(object):
             if name is not None:
                 self.name = name
 
-    class VMToBackup(object):
+    class VMToBackup:
         # pylint: disable=too-few-public-methods
         def __init__(self, vm, files, subdir):
             self.vm = vm
@@ -637,6 +639,7 @@ class Backup(object):
 
                 # Pipe: tar-sparse | scrypt | tar | backup_target
                 # TODO: log handle stderr
+                # pylint: disable=not-an-iterable
                 tar_sparse = yield from asyncio.create_subprocess_exec(
                     *tar_cmdline, stdout=subprocess.PIPE)
 

qubes/devices.py

@@ -67,7 +67,7 @@ class DeviceAlreadyAttached(qubes.exc.QubesException, KeyError):
     '''Trying to attach already attached device'''
     pass
 
-class DeviceInfo(object):
+class DeviceInfo:
     ''' Holds all information about a device '''
     # pylint: disable=too-few-public-methods
     def __init__(self, backend_domain, ident, description=None,
@@ -117,7 +117,7 @@ class DeviceInfo(object):
         return '{!s}:{!s}'.format(self.backend_domain, self.ident)
 
 
-class DeviceAssignment(object): # pylint: disable=too-few-public-methods
+class DeviceAssignment: # pylint: disable=too-few-public-methods
     ''' Maps a device to a frontend_domain. '''
 
     def __init__(self, backend_domain, ident, options=None, persistent=False,
@@ -158,7 +158,7 @@ class DeviceAssignment(object): # pylint: disable=too-few-public-methods
         return self.backend_domain.devices[self.bus][self.ident]
 
 
-class DeviceCollection(object):
+class DeviceCollection:
     '''Bag for devices.
 
     Used as default value for :py:meth:`DeviceManager.__missing__` factory.
@@ -357,7 +357,6 @@ class DeviceCollection(object):
             if persistent is True:
                 # don't break app.save()
                 return self._set
-            else:
             raise
         result = set()
         for dev, options in devices:
@@ -433,7 +432,7 @@ class UnknownDevice(DeviceInfo):
             frontend_domain)
 
 
-class PersistentCollection(object):
+class PersistentCollection:
 
     ''' Helper object managing persistent `DeviceAssignment`s.
     '''

qubes/dochelpers.py

@@ -48,7 +48,7 @@ SUBCOMMANDS_TITLE = 'COMMANDS'
 OPTIONS_TITLE = 'OPTIONS'
 
 
-class GithubTicket(object):
+class GithubTicket:
     # pylint: disable=too-few-public-methods
     def __init__(self, data):
         self.number = data['number']
@@ -418,7 +418,7 @@ def parse_event(env, sig, signode):
 #
 
 
-def break_to_pdb(app, *dummy):
+def break_to_pdb(app, *_dummy):
     if not app.config.break_to_pdb:
         return
     import pdb

qubes/events.py

@@ -94,7 +94,7 @@ class EmitterMeta(type):
                 cls.__handlers__[event].add(attr)
 
 
-class Emitter(object, metaclass=EmitterMeta):
+class Emitter(metaclass=EmitterMeta):
     '''Subject that can emit events.
 
     By default all events are disabled not to interfere with loading from XML.

qubes/ext/__init__.py

@@ -29,7 +29,7 @@ import pkg_resources
 import qubes.events
 
 
-class Extension(object):
+class Extension:
     '''Base class for all extensions
     '''  # pylint: disable=too-few-public-methods
 

qubes/firewall.py

@@ -34,7 +34,7 @@ import qubes
 import qubes.vm.qubesvm
 
 
-class RuleOption(object):
+class RuleOption:
     def __init__(self, untrusted_value):
         # subset of string.punctuation
         safe_set = string.ascii_letters + string.digits + \
@@ -208,7 +208,7 @@ class Expire(RuleOption):
 
     @property
     def rule(self):
-        return None
+        pass
 
     @property
     def api_rule(self):
@@ -232,7 +232,7 @@ class Comment(RuleOption):
 
     @property
     def rule(self):
-        return None
+        pass
 
     @property
     def api_rule(self):
@@ -449,7 +449,7 @@ class Rule(qubes.PropertyHolder):
         return hash(self.api_rule)
 
 
-class Firewall(object):
+class Firewall:
     def __init__(self, vm, load=True):
         assert hasattr(vm, 'firewall_conf')
         self.vm = vm

qubes/rngdoc.py

@@ -26,7 +26,7 @@ import textwrap
 
 import lxml.etree
 
-class Element(object):
+class Element:
     def __init__(self, schema, xml):
         self.schema = schema
         self.xml = xml
@@ -157,7 +157,7 @@ class Element(object):
             write_rst_table(stream, childtable, ('element', 'number'))
 
 
-class Schema(object):
+class Schema:
     # pylint: disable=too-few-public-methods
     nsmap = {
         'rng': 'http://relaxng.org/ns/structure/1.0',

qubes/storage/__init__.py

@@ -47,7 +47,7 @@ class StoragePoolException(qubes.exc.QubesException):
     pass
 
 
-class BlockDevice(object):
+class BlockDevice:
     ''' Represents a storage block device. '''
     # pylint: disable=too-few-public-methods
     def __init__(self, path, name, script=None, rw=True, domain=None,
@@ -62,7 +62,7 @@ class BlockDevice(object):
         self.devtype = devtype
 
 
-class Volume(object):
+class Volume:
     ''' Encapsulates all data about a volume for serialization to qubes.xml and
         libvirt config.
 
@@ -334,14 +334,14 @@ class Volume(object):
         msg = msg.format(str(self.__class__.__name__), method_name)
         return NotImplementedError(msg)
 
-class Storage(object):
+class Storage:
     ''' Class for handling VM virtual disks.
 
     This is base class for all other implementations, mostly with Xen on Linux
     in mind.
     '''
 
-    AVAILABLE_FRONTENDS = set(['xvd' + c for c in string.ascii_lowercase])
+    AVAILABLE_FRONTENDS = {'xvd' + c for c in string.ascii_lowercase}
 
     def __init__(self, vm):
         #: Domain for which we manage storage
@@ -655,9 +655,9 @@ class Storage(object):
         ''' Used device names '''
         xml = self.vm.libvirt_domain.XMLDesc()
         parsed_xml = lxml.etree.fromstring(xml)
-        return set([target.get('dev', None)
+        return {target.get('dev', None)
                     for target in parsed_xml.xpath(
-                        "//domain/devices/disk/target")])
+                        "//domain/devices/disk/target")}
 
     def export(self, volume):
         ''' Helper function to export volume (pool.export(volume))'''
@@ -688,7 +688,7 @@ class Storage(object):
         return self.vm.volumes[volume].import_data_end(success=success)
 
 
-class VolumesCollection(object):
+class VolumesCollection:
     '''Convenient collection wrapper for pool.get_volume and
     pool.list_volumes
     '''
@@ -706,7 +706,6 @@ class VolumesCollection(object):
         if isinstance(item, Volume):
             if item.pool == self._pool:
                 return self[item.vid]
-            else:
             raise KeyError(item)
         try:
             return self._pool.get_volume(item)
@@ -740,7 +739,7 @@ class VolumesCollection(object):
         return [vol for vol in self]
 
 
-class Pool(object):
+class Pool:
     ''' A Pool is used to manage different kind of volumes (File
         based/LVM/Btrfs/...).
 
@@ -760,7 +759,7 @@ class Pool(object):
     def __eq__(self, other):
         if isinstance(other, Pool):
             return self.name == other.name
-        elif isinstance(other, str):
+        if isinstance(other, str):
             return self.name == other
         return NotImplemented
 
@@ -829,12 +828,12 @@ class Pool(object):
     @property
     def size(self):
         ''' Storage pool size in bytes, or None if unknown '''
-        return None
+        pass
 
     @property
     def usage(self):
         ''' Space used in the pool in bytes, or None if unknown '''
-        return None
+        pass
 
     def _not_implemented(self, method_name):
         ''' Helper for emitting helpful `NotImplementedError` exceptions '''
@@ -898,7 +897,7 @@ def search_pool_containing_dir(pools, dir_path):
     return None
 
 
-class VmCreationManager(object):
+class VmCreationManager:
     ''' A `ContextManager` which cleans up if volume creation fails.
     '''  # pylint: disable=too-few-public-methods
     def __init__(self, vm):

qubes/storage/file.py

@@ -356,9 +356,9 @@ class FileVolume(qubes.storage.Volume):
     def script(self):
         if not self.snap_on_start and not self.save_on_stop:
             return None
-        elif not self.snap_on_start and self.save_on_stop:
+        if not self.snap_on_start and self.save_on_stop:
             return 'block-origin'
-        elif self.snap_on_start:
+        if self.snap_on_start:
             return 'block-snapshot'
         return None
 

qubes/utils.py

@@ -41,7 +41,7 @@ def get_timezone():
     if os.path.islink('/etc/localtime'):
         return '/'.join(os.readlink('/etc/localtime').split('/')[-2:])
     # <=fc17
-    elif os.path.exists('/etc/sysconfig/clock'):
+    if os.path.exists('/etc/sysconfig/clock'):
         clock_config = open('/etc/sysconfig/clock', "r")
         clock_config_lines = clock_config.readlines()
         clock_config.close()
@@ -50,7 +50,6 @@ def get_timezone():
             line_match = zone_re.match(line)
             if line_match:
                 return line_match.group(1)
-    else:
     # last resort way, some applications makes /etc/localtime
     # hardlink instead of symlink...
     tz_info = os.stat('/etc/localtime')
@@ -61,7 +60,7 @@ def get_timezone():
             '-inum', str(tz_info.st_ino), '-print', '-quit'],
             stdout=subprocess.PIPE)
         tz_path = p.communicate()[0].strip()
-            return tz_path.replace('/usr/share/zoneinfo/', '')
+        return tz_path.replace(b'/usr/share/zoneinfo/', b'')
     return None
 
 
@@ -132,9 +131,9 @@ def size_to_human(size):
     """Humane readable size, with 1/10 precision"""
     if size < 1024:
         return str(size)
-    elif size < 1024 * 1024:
+    if size < 1024 * 1024:
         return str(round(size / 1024.0, 1)) + ' KiB'
-    elif size < 1024 * 1024 * 1024:
+    if size < 1024 * 1024 * 1024:
         return str(round(size / (1024.0 * 1024), 1)) + ' MiB'
 
     return str(round(size / (1024.0 * 1024 * 1024), 1)) + ' GiB'
@@ -181,6 +180,6 @@ def match_vm_name_with_special(vm, name):
     or @type:...'''
     if name.startswith('@tag:'):
         return name[len('@tag:'):] in vm.tags
-    elif name.startswith('@type:'):
+    if name.startswith('@type:'):
         return name[len('@type:'):] == vm.__class__.__name__
     return name == vm.name

qubes/vm/__init__.py

@@ -568,7 +568,6 @@ class VMProperty(qubes.property):
             if self.allow_none:
                 super(VMProperty, self).__set__(instance, value)
                 return
-            else:
             raise ValueError(
                 'Property {!r} does not allow setting to {!r}'.format(
                     self.__name__, value))

qubes/vm/adminvm.py

@@ -147,7 +147,6 @@ class AdminVM(qubes.vm.BaseVM):
         if self.app.vmm.offline_mode:
             # default value passed on xen cmdline
             return 4096
-        else:
         try:
             return self.app.vmm.libvirt_conn.getInfo()[1]
         except libvirt.libvirtError as e:
@@ -181,7 +180,7 @@ class AdminVM(qubes.vm.BaseVM):
 
     @property
     def icon_path(self):
-        return None
+        pass
 
     @property
     def untrusted_qdb(self):

qubes/vm/qubesvm.py

@@ -518,7 +518,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
         except libvirt.libvirtError as e:
             if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                 return -1
-            else:
             self.log.exception('libvirt error code: {!r}'.format(
                 e.get_error_code()))
             raise
@@ -1575,7 +1574,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             except libvirt.libvirtError as e:
                 if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                     return 'Halted'
-                else:
                 raise
 
         libvirt_domain = self.libvirt_domain
@@ -1587,18 +1585,16 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                 # pylint: disable=line-too-long
                 if libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_PAUSED:
                     return "Paused"
-                elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_CRASHED:
+                if libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_CRASHED:
                     return "Crashed"
-                elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_SHUTDOWN:
+                if libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_SHUTDOWN:
                     return "Halting"
-                elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_SHUTOFF:
+                if libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_SHUTOFF:
                     return "Dying"
-                elif libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_PMSUSPENDED:  # nopep8
+                if libvirt_domain.state()[0] == libvirt.VIR_DOMAIN_PMSUSPENDED:  # nopep8
                     return "Suspended"
-                else:
                 if not self.is_fully_usable():
                     return "Transient"
-
                 return "Running"
 
             return 'Halted'
@@ -1639,7 +1635,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             except libvirt.libvirtError as e:
                 if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
                     return False
-                else:
                 raise
 
         return bool(self.libvirt_domain.isActive())
@@ -1662,7 +1657,7 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
             :py:obj:`False` otherwise.
         :rtype: bool
         '''
-        if self.xid < 0:
+        if self.xid < 0:  # pylint: disable=comparison-with-callable
             return False
         return os.path.exists('/var/run/qubes/qrexec.%s' % self.name)
 
@@ -1706,7 +1701,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     libvirt.VIR_ERR_INTERNAL_ERROR):
                 return 0
 
-            else:
             self.log.exception(
                 'libvirt error code: {!r}'.format(e.get_error_code()))
             raise
@@ -1733,7 +1727,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     libvirt.VIR_ERR_INTERNAL_ERROR):
                 return 0
 
-            else:
             self.log.exception(
                 'libvirt error code: {!r}'.format(e.get_error_code()))
             raise
@@ -1772,7 +1765,6 @@ class QubesVM(qubes.vm.mix.net.NetVMMixin, qubes.vm.BaseVM):
                     libvirt.VIR_ERR_INTERNAL_ERROR):
                 return 0
 
-            else:
             self.log.exception(
                 'libvirt error code: {!r}'.format(e.get_error_code()))
             raise

qubespolicy/__init__.py

@@ -18,6 +18,8 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library; if not, see <https://www.gnu.org/licenses/>.
 
+# pylint: disable=no-else-return,useless-object-inheritance,try-except-raise
+
 ''' Qrexec policy parser and evaluator '''
 import enum
 import itertools

qubespolicy/agent.py

@@ -33,7 +33,7 @@ import qubespolicy.rpcconfirmation
 import qubespolicy.policycreateconfirmation
 # pylint: enable=wrong-import-position
 
-class PolicyAgent(object):
+class PolicyAgent:
     # pylint: disable=too-few-public-methods
     dbus = """
     <node>

qubespolicy/cli.py

@@ -77,7 +77,7 @@ def main(args=None):
     try:
         system_info = qubespolicy.get_system_info()
     except qubespolicy.QubesMgmtException as e:
-        log.error(log_prefix + 'error getting system info: ' + str(e))
+        log.error('%s error getting system info: %s', log_prefix, str(e))
         return 1
     try:
         try:
@@ -130,13 +130,13 @@ def main(args=None):
                 action.handle_user_response(True, response)
             else:
                 action.handle_user_response(False)
-        log.info(log_prefix + 'allowed to {}'.format(action.target))
+        log.info('%s allowed to %s', log_prefix, action.target)
         action.execute(caller_ident)
     except qubespolicy.PolicySyntaxError as e:
-        log.error(log_prefix + 'error loading policy: ' + str(e))
+        log.error('%s error loading policy: %s', log_prefix, str(e))
         return 1
     except qubespolicy.AccessDenied as e:
-        log.info(log_prefix + 'denied: ' + str(e))
+        log.info('%s denied: %s', log_prefix, str(e))
         return 1
     return 0
 

qubespolicy/policycreateconfirmation.py

@@ -28,7 +28,7 @@ gi.require_version('Gtk', '3.0')
 from gi.repository import Gtk
 # pylint: enable=import-error
 
-class PolicyCreateConfirmationWindow(object):
+class PolicyCreateConfirmationWindow:
     # pylint: disable=too-few-public-methods
     _source_file = pkg_resources.resource_filename('qubespolicy',
         os.path.join('glade', "PolicyCreateConfirmationWindow.glade"))