Updated source for Pylint 1.5.

Pylint 1.5 brought with it much more than
    pylint.extensions.check_docs
The changes for this PR are
- checking import order
- checking self._foo is set in __init__
- complaining about methods that don't use self
- complaining about not calling super().__init__ (for
  the virtual base class we used)
- complaining about not (self == other) in __ne__,
  suggested (self != other) instead
- complaining about unit tests checking None result from methods
  which are known (by static analysis?) to return None
- complaining about not over-riding virtual method in storage
  _PropertyMixin
- aggressively checking `redefined-variable-type` when variables
  can be optionally float/int/Entity/Key/etc.
- checking that GCloudError constructor is called correctly
  in unit tests
- checking that subclass method overrides use the same arguments
  (this occurred in BigQuery test_job with _makeResource)

gcloud/_helpers.py

@@ -19,23 +19,20 @@
 import calendar
 import datetime
 import os
-import six
+from threading import local as Local
 import socket
 
-try:
-    from threading import local as Local
-except ImportError:     # pragma: NO COVER (who doesn't have it?)
-    class Local(object):
-        """Placeholder for non-threaded applications."""
-
+import six
 from six.moves.http_client import HTTPConnection  # pylint: disable=F0401
 
+from gcloud.environment_vars import PROJECT
+
+# pylint: disable=wrong-import-position
 try:
     from google.appengine.api import app_identity
 except ImportError:
     app_identity = None
-
-from gcloud.environment_vars import PROJECT
+# pylint: enable=wrong-import-position
 
 
 _NOW = datetime.datetime.utcnow  # To be replaced by tests.
@@ -297,7 +294,7 @@ def _to_bytes(value, encoding='ascii'):
 
 
 try:
-    from pytz import UTC  # pylint: disable=unused-import
+    from pytz import UTC  # pylint: disable=unused-import,wrong-import-position
 except ImportError:
     UTC = _UTC()  # Singleton instance to be used throughout.
 

gcloud/bigquery/dataset.py

@@ -60,10 +60,13 @@ class Dataset(object):
     :param access_grants: roles granted to entities for this dataset
     """
 
+    _access_grants = None
+
     def __init__(self, name, client, access_grants=()):
         self.name = name
         self._client = client
         self._properties = {}
+        # Let the @property do validation.
         self.access_grants = access_grants
 
     @property
@@ -283,7 +286,8 @@ def _require_client(self, client):
             client = self._client
         return client
 
-    def _parse_access_grants(self, access):
+    @staticmethod
+    def _parse_access_grants(access):
         """Parse a resource fragment into a set of access grants.
 
         :type access: list of mappings

gcloud/bigquery/job.py

@@ -444,10 +444,14 @@ class LoadTableFromStorageJob(_AsyncJob):
     :type schema: list of :class:`gcloud.bigquery.table.SchemaField`
     :param schema: The job's schema
     """
+
+    _schema = None
+
     def __init__(self, name, destination, source_uris, client, schema=()):
         super(LoadTableFromStorageJob, self).__init__(name, client)
         self.destination = destination
         self.source_uris = source_uris
+        # Let the @property do validation.
         self.schema = schema
         self._configuration = _LoadConfiguration()
 

gcloud/bigquery/table.py

@@ -72,10 +72,13 @@ class Table(object):
     :param schema: The table's schema
     """
 
+    _schema = None
+
     def __init__(self, name, dataset, schema=()):
         self.name = name
         self._dataset = dataset
         self._properties = {}
+        # Let the @property do validation.
         self.schema = schema
 
     @property

gcloud/client.py

@@ -31,9 +31,6 @@ class _ClientFactoryMixin(object):
         This class is virtual.
     """
 
-    def __init__(self, *args, **kwargs):
-        raise NotImplementedError('_ClientFactoryMixin is a virtual class')
-
     @classmethod
     def from_service_account_json(cls, json_credentials_path, *args, **kwargs):
         """Factory to retrieve JSON credentials while creating client.

gcloud/credentials.py

@@ -26,21 +26,22 @@
 from oauth2client.client import _get_application_default_credential_from_file
 from oauth2client import crypt
 from oauth2client import service_account
-
-try:
-    from google.appengine.api import app_identity
-except ImportError:
-    app_identity = None
-
 try:
     from oauth2client.appengine import AppAssertionCredentials as _GAECreds
 except ImportError:
     class _GAECreds(object):
         """Dummy class if not in App Engine environment."""
 
+# pylint: disable=wrong-import-position
+try:
+    from google.appengine.api import app_identity
+except ImportError:
+    app_identity = None
+
 from gcloud._helpers import UTC
 from gcloud._helpers import _NOW
 from gcloud._helpers import _microseconds_from_datetime
+# pylint: enable=wrong-import-position
 
 
 def get_credentials():

gcloud/datastore/entity.py

@@ -106,7 +106,11 @@ def __ne__(self, other):
         :rtype: boolean
         :returns: False if the entities compare equal, else True.
         """
-        return not self == other
+        eq_val = self.__eq__(other)
+        if eq_val is NotImplemented:
+            return True
+        else:
+            return not eq_val
 
     @property
     def kind(self):

gcloud/datastore/key.py

@@ -99,7 +99,11 @@ def __ne__(self, other):
         :rtype: boolean
         :returns: False if the keys compare equal, else True.
         """
-        return not self == other
+        eq_val = self.__eq__(other)
+        if eq_val is NotImplemented:
+            return True
+        else:
+            return not eq_val
 
     def __hash__(self):
         """Hash a keys for use in a dictionary lookp.

gcloud/datastore/test_client.py

@@ -620,8 +620,7 @@ def test_put_multi_no_batch_w_partial_key(self):
         client = self._makeOne(credentials=creds)
         client.connection._commit.append(_CommitResult(key))
 
-        result = client.put_multi([entity])
-        self.assertTrue(result is None)
+        client.put_multi([entity])
 
         self.assertEqual(len(client.connection._commit_cw), 1)
         dataset_id, mutation, transaction_id = client.connection._commit_cw[0]
@@ -644,9 +643,8 @@ def test_put_multi_existing_batch_w_completed_key(self):
         key = entity.key = _Key(self.DATASET_ID)
 
         with _NoCommitBatch(client) as CURR_BATCH:
-            result = client.put_multi([entity])
+            client.put_multi([entity])
 
-        self.assertEqual(result, None)
         self.assertEqual(len(CURR_BATCH.mutation.insert_auto_id), 0)
         upserts = list(CURR_BATCH.mutation.upsert)
         self.assertEqual(len(upserts), 1)
@@ -675,8 +673,8 @@ def _delete_multi(*args, **kw):
     def test_delete_multi_no_keys(self):
         creds = object()
         client = self._makeOne(credentials=creds)
-        result = client.delete_multi([])
-        self.assertEqual(result, None)
+        client.delete_multi([])
+        self.assertEqual(len(client.connection._commit_cw), 0)
 
     def test_delete_multi_no_batch(self):
         from gcloud.datastore.test_batch import _CommitResult
@@ -688,8 +686,7 @@ def test_delete_multi_no_batch(self):
         client = self._makeOne(credentials=creds)
         client.connection._commit.append(_CommitResult())
 
-        result = client.delete_multi([key])
-        self.assertEqual(result, None)
+        client.delete_multi([key])
         self.assertEqual(len(client.connection._commit_cw), 1)
         dataset_id, mutation, transaction_id = client.connection._commit_cw[0]
         self.assertEqual(dataset_id, self.DATASET_ID)
@@ -704,9 +701,8 @@ def test_delete_multi_w_existing_batch(self):
         key = _Key(self.DATASET_ID)
 
         with _NoCommitBatch(client) as CURR_BATCH:
-            result = client.delete_multi([key])
+            client.delete_multi([key])
 
-        self.assertEqual(result, None)
         self.assertEqual(len(CURR_BATCH.mutation.insert_auto_id), 0)
         self.assertEqual(len(CURR_BATCH.mutation.upsert), 0)
         deletes = list(CURR_BATCH.mutation.delete)
@@ -722,9 +718,8 @@ def test_delete_multi_w_existing_transaction(self):
         key = _Key(self.DATASET_ID)
 
         with _NoCommitTransaction(client) as CURR_XACT:
-            result = client.delete_multi([key])
+            client.delete_multi([key])
 
-        self.assertEqual(result, None)
         self.assertEqual(len(CURR_XACT.mutation.insert_auto_id), 0)
         self.assertEqual(len(CURR_XACT.mutation.upsert), 0)
         deletes = list(CURR_XACT.mutation.delete)

gcloud/search/document.py

@@ -182,7 +182,8 @@ def from_api_repr(cls, resource, index):
         document._parse_fields_resource(resource)
         return document
 
-    def _parse_value_resource(self, resource):
+    @staticmethod
+    def _parse_value_resource(resource):
         """Helper for _parse_fields_resource"""
         if 'stringValue' in resource:
             string_format = resource.get('stringFormat')
@@ -245,7 +246,8 @@ def _require_client(self, client):
             client = self.index._client
         return client
 
-    def _build_value_resource(self, value):
+    @staticmethod
+    def _build_value_resource(value):
         """Helper for _build_fields_resource"""
         result = {}
         if value.value_type == 'string':

gcloud/storage/_helpers.py

@@ -17,9 +17,10 @@
 These are *not* part of the API.
 """
 
-from Crypto.Hash import MD5
 import base64
 
+from Crypto.Hash import MD5
+
 
 class _PropertyMixin(object):
     """Abstract mixin for cloud storage classes with associated propertties.

gcloud/storage/batch.py

@@ -20,10 +20,10 @@
 from email.mime.application import MIMEApplication
 from email.mime.multipart import MIMEMultipart
 from email.parser import Parser
-import httplib2
 import io
 import json
 
+import httplib2
 import six
 
 from gcloud.exceptions import make_exception

gcloud/storage/test__helpers.py

@@ -28,6 +28,8 @@ def _derivedClass(self, path=None):
 
         class Derived(self._getTargetClass()):
 
+            client = None
+
             @property
             def path(self):
                 return path

gcloud/streaming/transfer.py

@@ -52,6 +52,9 @@ class _Transfer(object):
     :type num_retries: integer
     :param num_retries: how many retries should the transfer attempt
     """
+
+    _num_retries = None
+
     def __init__(self, stream, close_stream=False,
                  chunksize=_DEFAULT_CHUNKSIZE, auto_transfer=True,
                  http=None, num_retries=5):
@@ -61,7 +64,7 @@ def __init__(self, stream, close_stream=False,
         self._stream = stream
         self._url = None
 
-        # Let the @property do validation
+        # Let the @property do validation.
         self.num_retries = num_retries
 
         self.auto_transfer = auto_transfer
@@ -385,7 +388,8 @@ def _normalize_start_end(self, start, end=None):
                 start = max(0, start + self.total_size)
             return start, self.total_size - 1
 
-    def _set_range_header(self, request, start, end=None):
+    @staticmethod
+    def _set_range_header(request, start, end=None):
         """Update the 'Range' header in a request to match a byte range.
 
         :type request: :class:`gcloud.streaming.http_wrapper.Request`
@@ -915,7 +919,8 @@ def refresh_upload_state(self):
         else:
             raise HttpError.from_response(refresh_response)
 
-    def _get_range_header(self, response):
+    @staticmethod
+    def _get_range_header(response):
         """Return a 'Range' header from a response.
 
         :type response: :class:`gcloud.streaming.http_wrapper.Response`
@@ -975,7 +980,8 @@ def initialize_upload(self, http_request, http):
         else:
             return http_response
 
-    def _last_byte(self, range_header):
+    @staticmethod
+    def _last_byte(range_header):
         """Parse the last byte from a 'Range' header.
 
         :type range_header: string

gcloud/test_client.py

@@ -21,11 +21,9 @@ def _getTargetClass(self):
         from gcloud.client import _ClientFactoryMixin
         return _ClientFactoryMixin
 
-    def _makeOne(self, *args, **kw):
-        return self._getTargetClass()(*args, **kw)
-
     def test_virtual(self):
-        self.assertRaises(NotImplementedError, self._makeOne)
+        klass = self._getTargetClass()
+        self.assertFalse('__init__' in klass.__dict__)
 
 
 class TestClient(unittest2.TestCase):

gcloud/test_exceptions.py

@@ -21,8 +21,8 @@ def _getTargetClass(self):
         from gcloud.exceptions import GCloudError
         return GCloudError
 
-    def _makeOne(self, *args):
-        return self._getTargetClass()(*args)
+    def _makeOne(self, message, errors=()):
+        return self._getTargetClass()(message, errors=errors)
 
     def test_ctor_defaults(self):
         e = self._makeOne('Testing')

pylintrc_default

@@ -80,6 +80,11 @@ load-plugins=pylint.extensions.check_docs
 #                      @_lazy_property_deco
 #                      def dataset_id():
 #                          ...
+# - redefined-variable-type: This error is overzealous and complains at e.g.
+#                      if some_prop:
+#                          return int(value)
+#                      else:
+#                          return float(value)
 disable =
     maybe-no-member,
     no-member,
@@ -88,6 +93,7 @@ disable =
     similarities,
     star-args,
     method-hidden,
+    redefined-variable-type,
 
 
 [REPORTS]

run_pylint.py

@@ -51,6 +51,7 @@
     'too-many-locals',
     'too-many-public-methods',
     'unbalanced-tuple-unpacking',
+    'arguments-differ',
 ]
 TEST_RC_ADDITIONS = {
     'MESSAGES CONTROL': {

system_tests/storage.py

@@ -12,11 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import httplib2
 import os
-import six
 import tempfile
 import time
+
+import httplib2
+import six
 import unittest2
 
 from gcloud import _helpers