Make git_cl pass pylint cleanly

git_cl/upload.py

Index: git_cl/upload.py
diff --git a/git_cl/upload.py b/git_cl/upload.py
index ff7d3f33e750518f3bcf0dd2ae7a5ef67401d3a8..7c8c2f1c8c4fb8179c5b32bbc8083c4cda94ad4b 100644
--- a/git_cl/upload.py
+++ b/git_cl/upload.py
@@ -54,15 +54,18 @@ except ImportError:
   from md5 import md5
 
 try:
-  import readline
+  import readline  # pylint: disable=W0611

[M-A Ruel, 2011/03/13 20:45:48]

Don't modify this file here, you need to modify it upstream in rietveld. For now
just add it to the blacklist.

 except ImportError:
   pass
 
 try:
-  import keyring
+  import keyring  # pylint: disable=F0401
 except ImportError:
   keyring = None
 
+# don't worry about methods that could be functions
+# pylint: disable=R0201
+
 # The logging verbosity:
 #  0: Errors only.
 #  1: Status messages.
@@ -122,7 +125,7 @@ def GetEmail(prompt):
       last_email = last_email_file.readline().strip("\n")
       last_email_file.close()
       prompt += " [%s]" % last_email
-    except IOError, e:
+    except IOError:
       pass
   email = raw_input(prompt + ": ").strip()
   if email:
@@ -130,7 +133,7 @@ def GetEmail(prompt):
       last_email_file = open(last_email_file_name, "w")
       last_email_file.write(email)
       last_email_file.close()
-    except IOError, e:
+    except IOError:
       pass
   else:
     email = last_email
@@ -151,7 +154,7 @@ def StatusUpdate(msg):
 
 def ErrorExit(msg):
   """Print an error message to stderr and exit."""
-  print >>sys.stderr, msg
+  print >> sys.stderr, msg
   sys.exit(1)
 
 
@@ -167,8 +170,9 @@ class ClientLoginError(urllib2.HTTPError):
 class AbstractRpcServer(object):
   """Provides a common interface for a simple RPC server."""
 
-  def __init__(self, host, auth_function, host_override=None, extra_headers={},
-               save_cookies=False, account_type=AUTH_ACCOUNT_TYPE):
+  def __init__(self, host, auth_function, host_override=None,
+               extra_headers=None, save_cookies=False,
+               account_type=AUTH_ACCOUNT_TYPE):
     """Creates a new HttpRpcServer.
 
     Args:
@@ -184,6 +188,7 @@ class AbstractRpcServer(object):
       account_type: Account type used for authentication. Defaults to
         AUTH_ACCOUNT_TYPE.
     """
+    extra_headers = extra_headers or {}
     self.host = host
     if (not self.host.startswith("http://") and
         not self.host.startswith("https://")):
@@ -317,16 +322,16 @@ class AbstractRpcServer(object):
     authentication cookie, it returns a 401 response (or a 302) and
     directs us to authenticate ourselves with ClientLogin.
     """
-    for i in range(3):
+    for i in range(3):  # pylint: disable=W0612
       credentials = self.auth_function()
       try:
         auth_token = self._GetAuthToken(host, credentials[0], credentials[1])
       except ClientLoginError, e:
         if e.reason == "BadAuthentication":
-          print >>sys.stderr, "Invalid username or password."
+          print >> sys.stderr, "Invalid username or password."
           continue
         if e.reason == "CaptchaRequired":
-          print >>sys.stderr, (
+          print >> sys.stderr, (
               "Please go to\n"
               "https://www.google.com/accounts/DisplayUnlockCaptcha\n"
               "and verify you are a human.  Then try again.\n"
@@ -334,23 +339,23 @@ class AbstractRpcServer(object):
               "https://www.google.com/a/yourdomain.com/UnlockCaptcha")
           break
         if e.reason == "NotVerified":
-          print >>sys.stderr, "Account not verified."
+          print >> sys.stderr, "Account not verified."
           break
         if e.reason == "TermsNotAgreed":
-          print >>sys.stderr, "User has not agreed to TOS."
+          print >> sys.stderr, "User has not agreed to TOS."
           break
         if e.reason == "AccountDeleted":
-          print >>sys.stderr, "The user account has been deleted."
+          print >> sys.stderr, "The user account has been deleted."
           break
         if e.reason == "AccountDisabled":
-          print >>sys.stderr, "The user account has been disabled."
+          print >> sys.stderr, "The user account has been disabled."
           break
         if e.reason == "ServiceDisabled":
-          print >>sys.stderr, ("The user's access to the service has been "
+          print >> sys.stderr, ("The user's access to the service has been "
                                "disabled.")
           break
         if e.reason == "ServiceUnavailable":
-          print >>sys.stderr, "The service is not available; try again later."
+          print >> sys.stderr, "The service is not available; try again later."
           break
         raise
       self._GetAuthCookie(host, auth_token)
@@ -451,6 +456,7 @@ class HttpRpcServer(AbstractRpcServer):
     opener.add_handler(urllib2.HTTPDefaultErrorHandler())
     opener.add_handler(urllib2.HTTPSHandler())
     opener.add_handler(urllib2.HTTPErrorProcessor())
+    # pylint: disable=W0201
     if self.save_cookies:
       self.cookie_file = os.path.expanduser("~/.codereview_upload_cookies")
       self.cookie_jar = cookielib.MozillaCookieJar(self.cookie_file)
@@ -675,8 +681,7 @@ def GetContentType(filename):
 use_shell = sys.platform.startswith("win")
 
 def RunShellWithReturnCode(command, print_output=False,
-                           universal_newlines=True,
-                           env=os.environ):
+                           universal_newlines=True, env=None):
   """Executes a command and returns the output from stdout and the return code.
 
   Args:
@@ -706,16 +711,16 @@ def RunShellWithReturnCode(command, print_output=False,
   p.wait()
   errout = p.stderr.read()
   if print_output and errout:
-    print >>sys.stderr, errout
+    print >> sys.stderr, errout
   p.stdout.close()
   p.stderr.close()
   return output, p.returncode
 
 
 def RunShell(command, silent_ok=False, universal_newlines=True,
-             print_output=False, env=os.environ):
+             print_output=False, env=None):
   data, retcode = RunShellWithReturnCode(command, print_output,
-                                         universal_newlines, env)
+                                         universal_newlines, env=env)
   if retcode:
     ErrorExit("Got error status from %s:\n%s" % (command, data))
   if not silent_ok and not data:
@@ -793,7 +798,7 @@ class VersionControlSystem(object):
     files = {}
     for line in diff.splitlines(True):
       if line.startswith('Index:') or line.startswith('Property changes on:'):
-        unused, filename = line.split(':', 1)
+        _, filename = line.split(':', 1)
         # On Windows if a file has property changes its filename uses '\'
         # instead of '/'.
         filename = filename.strip().replace('\\', '/')
@@ -809,9 +814,9 @@ class VersionControlSystem(object):
       """Uploads a file to the server."""
       file_too_large = False
       if is_base:
-        type = "base"
+        file_type = "base"
       else:
-        type = "current"
+        file_type = "current"
       if len(content) > MAX_UPLOAD_SIZE:
         print ("Not uploading the %s file for %s because it's too large." %
                (type, filename))
@@ -819,7 +824,7 @@ class VersionControlSystem(object):
         content = ""
       checksum = md5(content).hexdigest()
       if options.verbose > 0 and not file_too_large:
-        print "Uploading %s file for %s" % (type, filename)
+        print "Uploading %s file for %s" % (file_type, filename)
       url = "/%d/upload_content/%d/%d" % (int(issue), int(patchset), file_id)
       form_fields = [("filename", filename),
                      ("status", status),
@@ -840,7 +845,8 @@ class VersionControlSystem(object):
         sys.exit(1)
 
     patches = dict()
-    [patches.setdefault(v, k) for k, v in patch_list]
+    for k, v in patch_list:
+      patches.setdefault(v, k)
     for filename in patches.keys():
       base_content, new_content, is_binary, status = files[filename]
       file_id_str = patches.get(filename)
@@ -966,9 +972,9 @@ class SubversionVCS(VersionControlSystem):
     }
 
     def repl(m):
-       if m.group(2):
-         return "$%s::%s$" % (m.group(1), " " * len(m.group(3)))
-       return "$%s$" % m.group(1)
+      if m.group(2):
+        return "$%s::%s$" % (m.group(1), " " * len(m.group(3)))
+      return "$%s$" % m.group(1)
     keywords = [keyword
                 for name in keyword_str.split(" ")
                 for keyword in svn_keywords.get(name, [])]
@@ -984,12 +990,12 @@ class SubversionVCS(VersionControlSystem):
 
   def ReadFile(self, filename):
     """Returns the contents of a file."""
-    file = open(filename, 'rb')
+    f = open(filename, 'rb')
     result = ""
     try:
-      result = file.read()
+      result = f.read()
     finally:
-      file.close()
+      f.close()
     return result
 
   def GetStatus(self, filename):
@@ -1212,7 +1218,8 @@ class GitVCS(VersionControlSystem):
     # this by overriding the environment (but there is still a problem if the
     # git config key "diff.external" is used).
     env = os.environ.copy()
-    if 'GIT_EXTERNAL_DIFF' in env: del env['GIT_EXTERNAL_DIFF']
+    if 'GIT_EXTERNAL_DIFF' in env:
+      del env['GIT_EXTERNAL_DIFF']
     return RunShell(["git", "diff", "--no-ext-diff", "--full-index", "-M"]
                     + extra_args, env=env)
 
@@ -1230,7 +1237,7 @@ class GitVCS(VersionControlSystem):
     return data
 
   def GetBaseFile(self, filename):
-    hash_before, hash_after = self.hashes.get(filename, (None,None))
+    hash_before, hash_after = self.hashes.get(filename, (None, None))
     base_content = None
     new_content = None
     is_binary = self.IsBinary(filename)
@@ -1313,7 +1320,6 @@ class MercurialVCS(VersionControlSystem):
 
   def GetUnknownFiles(self):
     """Return a list of files unknown to the VCS."""
-    args = []
     status = RunShell(["hg", "status", "--rev", self.base_rev, "-u", "."],
         silent_ok=True)
     unknown_files = []
@@ -1381,10 +1387,10 @@ def SplitPatch(data):
   for line in data.splitlines(True):
     new_filename = None
     if line.startswith('Index:'):
-      unused, new_filename = line.split(':', 1)
+      _, new_filename = line.split(':', 1)
       new_filename = new_filename.strip()
     elif line.startswith('Property changes on:'):
-      unused, temp_filename = line.split(':', 1)
+      _, temp_filename = line.split(':', 1)
       # When a file is modified, paths use '/' between directories, however
       # when a property is modified '\' is used on Windows.  Make them the same
       # otherwise the file shows up twice.
@@ -1452,8 +1458,8 @@ def GuessVCSName():
     out, returncode = RunShellWithReturnCode(["hg", "root"])
     if returncode == 0:
       return (VCS_MERCURIAL, out.strip())
-  except OSError, (errno, message):
-    if errno != 2:  # ENOENT -- they don't have hg installed.
+  except OSError, e:
+    if e.errno != 2:  # ENOENT -- they don't have hg installed.
       raise
 
   # Subversion has a .svn in all working directories.
@@ -1468,8 +1474,8 @@ def GuessVCSName():
                                               "--is-inside-work-tree"])
     if returncode == 0:
       return (VCS_GIT, None)
-  except OSError, (errno, message):
-    if errno != 2:  # ENOENT -- they don't have git installed.
+  except OSError, e:
+    if e.errno != 2:  # ENOENT -- they don't have git installed.
       raise
 
   return (VCS_UNKNOWN, None)
@@ -1718,20 +1724,20 @@ def RealMain(argv, data=None):
   if options.description_file:
     if options.description:
       ErrorExit("Can't specify description and description_file")
-    file = open(options.description_file, 'r')
-    description = file.read()
-    file.close()
+    f = open(options.description_file, 'r')
+    description = f.read()
+    f.close()
   if description:
     form_fields.append(("description", description))
   # Send a hash of all the base file so the server can determine if a copy
   # already exists in an earlier patchset.
   base_hashes = ""
-  for file, info in files.iteritems():
+  for filename, info in files.iteritems():
     if not info[0] is None:
       checksum = md5(info[0]).hexdigest()
       if base_hashes:
         base_hashes += "|"
-      base_hashes += checksum + ":" + file
+      base_hashes += checksum + ":" + filename
   form_fields.append(("base_hashes", base_hashes))
   if options.private:
     if options.issue: