Update to Pylint 1.7

Fixes #181

Mostly a few changes in pragmas. There are also a few actual code
changes:

- remove reduntant `else return` after `if return`
- Change some `x = foo and bar or baz` into equivalent `bar if foo
  else baz` forms.

.pylintrc

@@ -86,7 +86,7 @@ max-attributes=12
 
 [BASIC]
 bad-functions=input
-# Prospector turns ot unsafe-load-any-extension by default, but
+# Prospector turns on unsafe-load-any-extension by default, but
 # pylint leaves it off. This is the proximal cause of the
 # undefined-all-variable crash.
 #unsafe-load-any-extension = no

.travis.yml

@@ -22,7 +22,7 @@ matrix:
         - RS_MY_DRIVER="C MySQL Connector/Python"
     - python: 3.6
       env: ENV=mysqlconnector
-    - python: pypy-5.4.1
+    - python: pypy-5.6.0
       env: ENV=mysqlconnector
 
     - python: 3.6
@@ -50,7 +50,7 @@ matrix:
       python: 2.7
     - dist: trusty
       env: ENV=postgres
-      python: pypy-5.4.1
+      python: pypy-5.6.0
     - dist: trusty
       env: ENV=postgres
       python: 3.6
@@ -81,7 +81,7 @@ before_install:
 install:
   - pip install -U pip setuptools
   - pip install -U tox coveralls
-  - pip install pylint==1.6.5
+  - pip install -U "pylint>=1.7.1"
   - if [[ $TRAVIS_PYTHON_VERSION == pypy* ]]; then pip install -U python-memcached; fi
   - if [[ $TRAVIS_PYTHON_VERSION != pypy* ]]; then pip install -U pylibmc cffi; fi
   - pip install -U -e ".[test]"

relstorage/_compat.py

@@ -7,7 +7,7 @@ Compatibility shims.
 from __future__ import print_function, absolute_import, division
 
 # pylint:disable=unused-import,invalid-name,no-member,undefined-variable
-# pylint:disable=no-name-in-module,redefined-variable-type
+# pylint:disable=no-name-in-module
 
 import sys
 import platform

relstorage/adapters/connmanager.py

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-
+from __future__ import absolute_import
 from perfmetrics import metricmethod
 from relstorage.adapters.interfaces import IConnectionManager
 from relstorage.adapters.interfaces import ReplicaClosedException

relstorage/adapters/dbiter.py

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-from __future__ import print_function
+from __future__ import print_function, absolute_import
 from relstorage.adapters.interfaces import IDatabaseIterator
 from zope.interface import implementer
 from relstorage._compat import db_binary_to_bytes

relstorage/adapters/interfaces.py

@@ -12,7 +12,7 @@
 #
 ##############################################################################
 """Interfaces provided by RelStorage database adapters"""
-
+from __future__ import absolute_import
 from ZODB.POSException import StorageError
 from zope.interface import Attribute
 from zope.interface import Interface

relstorage/adapters/mover.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """IObjectMover implementation.
 """
-
+from __future__ import absolute_import
 from perfmetrics import Metric
 from relstorage.adapters.batch import RowBatcher
 from relstorage.adapters.interfaces import IObjectMover
@@ -82,8 +82,8 @@ class AbstractObjectMover(object):
             # If it's None, the object's creation has been
             # undone.
             return state, tid
-        else:
-            return None, None
+
+        return None, None
 
     _load_revision_query = """
         SELECT state
@@ -142,8 +142,8 @@ class AbstractObjectMover(object):
             state = db_binary_to_bytes(state)
             # None in state means The object's creation has been undone
             return state, tid
-        else:
-            return None, None
+
+        return None, None
 
 
     @metricmethod_sampled
@@ -164,8 +164,6 @@ class AbstractObjectMover(object):
         row = cursor.fetchone()
         if row:
             return row[0]
-        else:
-            return None
 
     # NOTE: These are not database param escapes, they are Python
     # escapes, so they shouldn't be translated to :1, etc.

relstorage/adapters/mysql/adapter.py

@@ -132,7 +132,6 @@ class MySQLAdapter(object):
         self.connmanager.add_on_load_opened(self._prepare_get_latest_tid)
         self.connmanager.add_on_store_opened(self._prepare_get_latest_tid)
 
-        # pylint:disable=redefined-variable-type
         if self.keep_history:
             self.packundo = MySQLHistoryPreservingPackUndo(
                 database_type='mysql',

relstorage/adapters/mysql/connmanager.py

@@ -14,6 +14,8 @@
 """
 MySQL adapter for RelStorage.
 """
+
+
 from __future__ import print_function, absolute_import
 import logging
 
@@ -52,6 +54,7 @@ class MySQLdbConnectionManager(AbstractConnectionManager):
     def open(self, transaction_mode="ISOLATION LEVEL READ COMMITTED",
              replica_selector=None, **kwargs):
         """Open a database connection and return (conn, cursor)."""
+        # pylint:disable=arguments-differ
         if replica_selector is None:
             replica_selector = self.replica_selector
 

relstorage/adapters/mysql/drivers.py

@@ -16,7 +16,7 @@
 MySQL IDBDriver implementations.
 """
 from __future__ import print_function, absolute_import
-# pylint:disable=redefined-variable-type
+
 
 import os
 import sys
@@ -364,7 +364,7 @@ else:
             self._umysql_conn = umysql.Connection() # pylint:disable=no-member
             self._connect()  # Potentially this could raise again?
 
-        def connect(self, *_args, **_kwargs): # pragma: no cover
+        def connect(self, *_args, **_kwargs): # pragma: no cover pylint:disable=arguments-differ
             # Redirect the PyMySQL connect method to the umysqldb method, that's
             # already captured the host and port. (XXX: Why do we do this?)
             return self._connect()

relstorage/adapters/mysql/locker.py

@@ -49,7 +49,7 @@ class MySQLLocker(AbstractLocker):
 
     @metricmethod
     def hold_commit_lock(self, cursor, ensure_current=False, nowait=False):
-        timeout = not nowait and self.commit_lock_timeout or 0
+        timeout = self.commit_lock_timeout if not nowait else 0
         stmt = "SELECT GET_LOCK(CONCAT(DATABASE(), '.commit'), %s)"
         cursor.execute(stmt, (timeout,))
         try:

relstorage/adapters/mysql/mover.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """IObjectMover implementation.
 """
-from __future__ import print_function
+from __future__ import print_function, absolute_import
 
 from relstorage.adapters.interfaces import IObjectMover
 from zope.interface import implementer

relstorage/adapters/oracle/adapter.py

@@ -130,7 +130,6 @@ class OracleAdapter(object):
             revert_when_stale=options.revert_when_stale,
         )
 
-        # pylint:disable=redefined-variable-type
         if self.keep_history:
             self.packundo = OracleHistoryPreservingPackUndo(
                 database_type='oracle',

relstorage/adapters/oracle/connmanager.py

@@ -70,6 +70,7 @@ class CXOracleConnectionManager(AbstractConnectionManager):
     def open(self, transaction_mode="ISOLATION LEVEL READ COMMITTED",
              twophase=False, replica_selector=None, **kwargs):
         """Open a database connection and return (conn, cursor)."""
+        # pylint:disable=arguments-differ
         if replica_selector is None:
             replica_selector = self.replica_selector
 

relstorage/adapters/oracle/locker.py

@@ -37,7 +37,7 @@ class OracleLocker(AbstractLocker):
     def hold_commit_lock(self, cursor, ensure_current=False, nowait=False):
         # Hold commit_lock to prevent concurrent commits
         # (for as short a time as possible).
-        timeout = not nowait and self.commit_lock_timeout or 0
+        timeout = self.commit_lock_timeout if not nowait else 0
         status = cursor.callfunc(
             "DBMS_LOCK.REQUEST",
             self.inputsize_NUMBER, (

relstorage/adapters/oracle/mover.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """IObjectMover implementation.
 """
-
+from __future__ import absolute_import
 
 from relstorage.adapters.interfaces import IObjectMover
 
@@ -115,8 +115,6 @@ class OracleObjectMover(AbstractObjectMover):
             # with superclass.
             assert len(rows) == 1
             return rows[0][0]
-        else:
-            return None
 
     # no store connection initialization needed for Oracle
     def on_store_opened(self, cursor, restart=False):

relstorage/adapters/oracle/scriptrunner.py

@@ -75,7 +75,7 @@ class OracleScriptRunner(ScriptRunner):
                     params[k] = v
         else:
             stmt = generic_stmt % self.script_vars
-            params = () # pylint:disable=redefined-variable-type
+            params = ()
 
         try:
             cursor.execute(stmt, params)

relstorage/adapters/packundo.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """Pack/Undo implementations.
 """
-
+from __future__ import absolute_import
 # pylint:disable=too-many-lines,unused-argument
 
 from ZODB.POSException import UndoError
@@ -648,7 +648,7 @@ class HistoryPreservingPackUndo(PackUndo):
             res = [tid for (tid,) in cursor]
         finally:
             self.connmanager.close(conn, cursor)
-        return res and res[0] or 0
+        return res[0] if res else 0
 
 
     @metricmethod
@@ -702,8 +702,8 @@ class HistoryPreservingPackUndo(PackUndo):
                     if time.time() >= start + self.options.pack_batch_timeout:
                         conn.commit()
                         if packed_func is not None:
-                            for oid, tid in packed_list:
-                                packed_func(oid, tid)
+                            for poid, ptid in packed_list:
+                                packed_func(poid, ptid)
                         statecounter += len(packed_list)
                         if counter >= lastreport + reportstep:
                             log.info("pack: packed %d (%.1f%%) transaction(s), "

relstorage/adapters/poller.py

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-
+from __future__ import absolute_import
 from ZODB.POSException import ReadConflictError
 from ZODB.POSException import Unsupported
 from relstorage.adapters.interfaces import IPoller

relstorage/adapters/postgresql/adapter.py

@@ -107,7 +107,7 @@ class PostgreSQLAdapter(object):
         )
         self.connmanager.add_on_load_opened(self._prepare_get_latest_tid)
         self.connmanager.add_on_store_opened(self._prepare_get_latest_tid)
-        # pylint:disable=redefined-variable-type
+
         if self.keep_history:
             self.packundo = HistoryPreservingPackUndo(
                 database_type='postgresql',

relstorage/adapters/postgresql/connmanager.py

@@ -50,6 +50,7 @@ class Psycopg2ConnectionManager(AbstractConnectionManager):
     @metricmethod
     def open(self, isolation=None, replica_selector=None, **kwargs):
         """Open a database connection and return (conn, cursor)."""
+        # pylint:disable=arguments-differ
         if isolation is None:
             isolation = self.isolation_read_committed
 

relstorage/adapters/postgresql/drivers.py

@@ -17,7 +17,7 @@ PostgreSQL IDBDriver implementations.
 """
 
 from __future__ import print_function, absolute_import
-# pylint:disable=redefined-variable-type
+
 import sys
 import os
 

relstorage/adapters/postgresql/mover.py

@@ -13,7 +13,7 @@
 ##############################################################################
 """IObjectMover implementation.
 """
-
+from __future__ import absolute_import
 from ..mover import AbstractObjectMover
 from relstorage.adapters.interfaces import IObjectMover
 

relstorage/adapters/replica.py

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-
+from __future__ import absolute_import
 from perfmetrics import metricmethod
 from relstorage.adapters.interfaces import IReplicaSelector
 from zope.interface import implementer

relstorage/blobhelper.py

@@ -149,8 +149,7 @@ class BlobHelper(object):
         try:
             if blob is None:
                 return open(blob_filename, 'rb')
-            else:
-                return ZODB.blob.BlobFile(blob_filename, 'r', blob)
+            return ZODB.blob.BlobFile(blob_filename, 'r', blob)
         except IOError:
             # The file got removed while we were opening.
             # Fall through and try again with the protection of the lock.
@@ -171,8 +170,7 @@ class BlobHelper(object):
             _accessed(blob_filename)
             if blob is None:
                 return open(blob_filename, 'rb')
-            else:
-                return ZODB.blob.BlobFile(blob_filename, 'r', blob)
+            return ZODB.blob.BlobFile(blob_filename, 'r', blob)
         finally:
             lock.close()
 

relstorage/cache/cache_ring.py

@@ -23,7 +23,7 @@ try:
     izip = itertools.izip
 except AttributeError:
     # Python 3
-    izip = zip # pylint:disable=redefined-variable-type
+    izip = zip
 
 from relstorage.cache import _cache_ring
 

relstorage/cache/storage_cache.py

@@ -140,6 +140,7 @@ class StorageCache(object):
             self._trace = self._tracer.trace
             self._trace_store_current = self._tracer.trace_store_current
 
+    # XXX: Note that our __bool__ and __len__ are NOT consistent
     def __bool__(self):
         return True
     __nonzero__ = __bool__
@@ -213,7 +214,8 @@ class StorageCache(object):
         """
         Store any persistent client data.
         """
-        if self.options.cache_local_dir and len(self):
+        if self.options.cache_local_dir and len(self): # pylint:disable=len-as-condition
+            # (our __bool__ is not consistent with our len)
             return persistence.save_local_cache(self.options, self.prefix, self)
 
 

relstorage/cache/tests/benchmarks.py

@@ -15,7 +15,7 @@ from __future__ import print_function, absolute_import, division
 
 from relstorage.options import Options
 
-# pylint:disable=unused-argument,redefined-variable-type
+# pylint:disable=unused-argument
 
 class MockOptions(Options):
     cache_module_name = ''
@@ -444,9 +444,9 @@ class StorageTraceSimulator(object):
         self._report_one(stats, f, cache_local_mb, now, done)
 
         if hasattr(root_cache.clients_local_first[0], 'operations'):
-            with open(f + '.' + str(options.cache_local_mb) + '.ctrace', 'w') as f:
+            with open(f + '.' + str(options.cache_local_mb) + '.ctrace', 'w') as fp:
                 for o in root_cache.clients_local_first[0].operations:
-                    f.write("%s,%s,%d\n" % o)
+                    fp.write("%s,%s,%d\n" % o)
         return stats
 
     def simulate(self, s_type='local'):

relstorage/cache/tests/test_cache.py

@@ -82,7 +82,7 @@ class StorageCacheTests(unittest.TestCase):
         data.clear()
         for inst in self._instances:
             inst.close()
-            assert len(inst) == 0
+            assert len(inst) == 0 # pylint:disable=len-as-condition
             assert bool(inst)
             assert inst.size == 0
             assert inst.limit == 0

relstorage/pylibmc_wrapper.py

@@ -17,8 +17,8 @@ One way to use this is to add 'cache-module-name relstorage.pylibmc_wrapper'
 to zope.conf and set the 'cache-servers' parameter as well.
 """
 
-import pylibmc
-from pylibmc import Error as MemcachedError # pylint:disable=no-name-in-module
+import pylibmc # pylint:disable=import-error
+from pylibmc import Error as MemcachedError # pylint:disable=no-name-in-module,import-error
 import logging
 from functools import wraps
 

relstorage/storage.py

@@ -1627,7 +1627,7 @@ class RelStorageTransactionRecord(TransactionRecord):
         self._trans_iter = trans_iter
         self._tid_int = tid_int
         tid = p64(tid_int)
-        status = packed and 'p' or ' '
+        status = 'p' if packed else ' '
         user = user or b''
         description = desc or b''
         if ext:

relstorage/tests/RecoveryStorage.py

@@ -38,10 +38,10 @@ if not PY3:
 
 class IteratorDeepCompare(object):
 
-    def compare(self, storage1, storage2):
+    def compare(self, src, dest):
         # override this for storages that truncate on restore (because
         # they do not store history).
-        self.compare_exact(storage1, storage2)
+        self.compare_exact(src, dest)
 
     def compare_exact(self, storage1, storage2):
         """Confirm that storage1 and storage2 contain equivalent data"""

relstorage/tests/blob/testblob.py

@@ -1,4 +1,4 @@
-from __future__ import print_function
+from __future__ import print_function, absolute_import
 ##############################################################################
 #
 # Copyright (c) 2004 Zope Foundation and Contributors.
@@ -634,13 +634,13 @@ def clean(tmp):
 def rmtree(path):
     """Remove a tree without causing Windows file access errors"""
     # copied from setupstack.py
-    for path, dirs, files in os.walk(path, False):
+    for cpath, dirs, files in os.walk(path, False):
         for fname in files:
-            fname = os.path.join(path, fname)
+            fname = os.path.join(cpath, fname)
             os.chmod(fname, stat.S_IWUSR)
             os.remove(fname)
         for dname in dirs:
-            dname = os.path.join(path, dname)
+            dname = os.path.join(cpath, dname)
             os.rmdir(dname)
     os.rmdir(path)
 

relstorage/tests/hftestbase.py

@@ -12,7 +12,7 @@
 #
 ##############################################################################
 """A foundation for history-free RelStorage tests"""
-
+from __future__ import absolute_import
 from relstorage.tests.RecoveryStorage import BasicRecoveryStorage
 from relstorage.tests.RecoveryStorage import UndoableRecoveryStorage
 from relstorage.tests.reltestbase import GenericRelStorageTests

relstorage/tests/hptestbase.py

@@ -45,6 +45,7 @@ class HistoryPreservingRelStorageTests(GenericRelStorageTests,
     keep_history = True
 
     def checkUndoMultipleConflictResolution(self, *_args, **_kwargs):
+        # pylint:disable=arguments-differ
         # 4.2.3 and above add this. it's an exotic feature according to jimfulton.
         raise unittest.SkipTest("conflict-resolving undo not supported")
 
@@ -142,7 +143,7 @@ class HistoryPreservingRelStorageTests(GenericRelStorageTests,
         if isinstance(ugly_string, bytes):
             # Always text. Use latin 1 because it can decode any arbitrary
             # bytes.
-            ugly_string = ugly_string.decode('latin-1') # pylint:disable=redefined-variable-type
+            ugly_string = ugly_string.decode('latin-1')
 
         # The storage layer is defined to take bytes (implicitly in
         # older ZODB releases, explicitly in ZODB 5.something), but historically

relstorage/tests/packstresstest.py

@@ -15,7 +15,7 @@ logging.getLogger().setLevel(logging.DEBUG)
 
 use = 'oracle'
 keep_history = True
-# pylint:disable=redefined-variable-type
+
 if use == 'mysql':
     from relstorage.adapters.mysql import MySQLAdapter
     a = MySQLAdapter(db='packtest',

relstorage/tests/reltestbase.py

@@ -12,6 +12,7 @@
 #
 ##############################################################################
 """A foundation for RelStorage tests"""
+from __future__ import absolute_import
 # pylint:disable=too-many-ancestors,abstract-method,too-many-public-methods
 from ZODB.DB import DB
 from ZODB.POSException import ReadConflictError

relstorage/tests/test_blobhelper.py

@@ -43,8 +43,7 @@ class BlobHelperTest(unittest.TestCase):
                 if download_action == 'write':
                     write_file(filename, 'blob here')
                     return 9
-                else:
-                    return 0
+                return 0
 
             def upload_blob(self, cursor, oid_int, tid_int, filename):
                 test.uploaded = (oid_int, tid_int, filename)

relstorage/tests/test_zodbconvert.py

@@ -11,7 +11,7 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-from __future__ import print_function
+from __future__ import print_function, absolute_import
 
 from contextlib import contextmanager
 import os

relstorage/zodburi_resolver.py

@@ -30,9 +30,8 @@ class SuffixMultiplier(object):
     def __init__(self, d, default=1):
         self._d = dict(d)
         self._default = default
-        # all keys must be the same size
-        self._keysz = None
 
+        # all keys must be the same size
         def check(a, b):
             if len(a) != len(b):
                 raise ValueError("suffix length mismatch")