Make git_cl pass pylint cleanly

git_cl/upload.py

 #!/usr/bin/env python
 #
 # Copyright 2007 Google Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
 #     http://www.apache.org/licenses/LICENSE-2.0
 #
@@ skipping 36 matching lines @@
 import urllib2
 import urlparse
 
 # The md5 module was deprecated in Python 2.5.
 try:
   from hashlib import md5
 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.
 #  2: Info logs.
 #  3: Debug logs.
 verbosity = 1
 
 # The account type used for authentication.
 # This line could be changed by the review server (see handler for
 # upload.py).
@@ skipping 39 matching lines @@
 
   """
   last_email_file_name = os.path.expanduser("~/.last_codereview_email_address")
   last_email = ""
   if os.path.exists(last_email_file_name):
     try:
       last_email_file = open(last_email_file_name, "r")
       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:
     try:
       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
   return email
 
 
 def StatusUpdate(msg):
   """Print a status message to stdout.
 
   If 'verbosity' is greater than 0, print the message.
 
   Args:
     msg: The string to print.
   """
   if verbosity > 0:
     print msg
 
 
 def ErrorExit(msg):
   """Print an error message to stderr and exit."""
-  print >>sys.stderr, msg
+  print >> sys.stderr, msg
   sys.exit(1)
 
 
 class ClientLoginError(urllib2.HTTPError):
   """Raised to indicate there was an error authenticating with ClientLogin."""
 
   def __init__(self, url, code, msg, headers, args):
     urllib2.HTTPError.__init__(self, url, code, msg, headers, None)
     self.args = args
     self.reason = args["Error"]
 
 
 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:
       host: The host to send requests to.
       auth_function: A function that takes no arguments and returns an
         (email, password) tuple when called. Will be called if authentication
         is required.
       host_override: The host header to send to the server (defaults to host).
       extra_headers: A dict of extra headers to append to every request.
       save_cookies: If True, save the authentication cookies to local disk.
         If False, use an in-memory cookiejar instead.  Subclasses must
         implement this functionality.  Defaults to False.
       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://")):
       self.host = "http://" + self.host
     assert re.match(r'^[a-z]+://[a-z0-9\.-_]+(|:[0-9]+)$', self.host), (
         '%s is malformed' % host)
     self.host_override = host_override
     self.auth_function = auth_function
     self.authenticated = False
     self.extra_headers = extra_headers
@@ skipping 113 matching lines @@
      2) We use ClientLogin to obtain an AUTH token for the user
         (see http://code.google.com/apis/accounts/AuthForInstalledApps.html).
      3) We pass the auth token to /_ah/login on the server to obtain an
         authentication cookie. If login was successful, it tries to redirect
         us to the URL we provided.
 
     If we attempt to access the upload API without first obtaining an
     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"
               "If you are using a Google Apps account the URL is:\n"
               "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)
       return
 
   def Send(self, request_path, payload=None,
            content_type="application/octet-stream",
            timeout=None,
            extra_headers=None,
            **kwargs):
@@ skipping 80 matching lines @@
     Returns:
       A urllib2.OpenerDirector object.
     """
     opener = urllib2.OpenerDirector()
     opener.add_handler(urllib2.ProxyHandler())
     opener.add_handler(urllib2.UnknownHandler())
     opener.add_handler(urllib2.HTTPHandler())
     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)
       if os.path.exists(self.cookie_file):
         try:
           self.cookie_jar.load()
           self.authenticated = True
           StatusUpdate("Loaded authentication cookies from %s" %
                        self.cookie_file)
         except (cookielib.LoadError, IOError):
@@ skipping 204 matching lines @@
 
 def GetContentType(filename):
   """Helper to guess the content-type from the filename."""
   return mimetypes.guess_type(filename)[0] or 'application/octet-stream'
 
 
 # Use a shell for subcommands on Windows to get a PATH search.
 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:
     command: Command to execute.
     print_output: If True, the output is printed to stdout.
                   If False, both stdout and stderr are ignored.
     universal_newlines: Use universal_newlines flag (default: True).
 
   Returns:
     Tuple (output, return code)
   """
   logging.info("Running %s", command)
   p = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
                        shell=use_shell, universal_newlines=universal_newlines,
                        env=env)
   if print_output:
     output_array = []
     while True:
       line = p.stdout.readline()
       if not line:
         break
       print line.strip("\n")
       output_array.append(line)
     output = "".join(output_array)
   else:
     output = p.stdout.read()
   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:
     ErrorExit("No output from %s" % command)
   return data
 
 
 class VersionControlSystem(object):
   """Abstract base class providing an interface to the VCS."""
 
@@ skipping 57 matching lines @@
     """Helper that calls GetBase file for each file in the patch.
 
     Returns:
       A dictionary that maps from filename to GetBaseFile's tuple.  Filenames
       are retrieved based on lines that start with "Index:" or
       "Property changes on:".
     """
     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('\\', '/')
         files[filename] = self.GetBaseFile(filename)
     return files
 
 
   def UploadBaseFiles(self, issue, rpc_server, patch_list, patchset, options,
                       files):
     """Uploads the base files (and if necessary, the current ones as well)."""
 
     def UploadFile(filename, file_id, content, is_binary, status, is_base):
       """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))
         file_too_large = True
         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),
                      ("checksum", checksum),
                      ("is_binary", str(is_binary)),
                      ("is_current", str(not is_base)),
                     ]
       if file_too_large:
         form_fields.append(("file_too_large", "1"))
       if options.email:
         form_fields.append(("user", options.email))
       ctype, body = EncodeMultipartFormData(form_fields,
                                             [("data", filename, content)])
       response_body = rpc_server.Send(url, body,
                                       content_type=ctype)
       if not response_body.startswith("OK"):
         StatusUpdate("  --> %s" % response_body)
         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)
       if file_id_str.find("nobase") != -1:
         base_content = None
         file_id_str = file_id_str[file_id_str.rfind("_") + 1:]
       file_id = int(file_id_str)
       if base_content != None:
         UploadFile(filename, file_id, base_content, is_binary, status, True)
       if new_content != None:
@@ skipping 105 matching lines @@
       'Id':                  ['Id'],
 
       # Aliases
       'LastChangedDate':     ['LastChangedDate', 'Date'],
       'LastChangedRevision': ['LastChangedRevision', 'Rev', 'Revision'],
       'LastChangedBy':       ['LastChangedBy', 'Author'],
       'URL':                 ['URL', 'HeadURL'],
     }
 
     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, [])]
     return re.sub(r"\$(%s):(:?)([^\$]+)\$" % '|'.join(keywords), repl, content)
 
   def GetUnknownFiles(self):
     status = RunShell(["svn", "status", "--ignore-externals"], silent_ok=True)
     unknown_files = []
     for line in status.split("\n"):
       if line and line[0] == "?":
         unknown_files.append(line)
     return unknown_files
 
   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):
     """Returns the status of a file."""
     if not self.options.revision:
       status = RunShell(["svn", "status", "--ignore-externals", filename])
       if not status:
         ErrorExit("svn status returned no output for %s" % filename)
       status_lines = status.splitlines()
       # If file is in a cl, the output will begin with
@@ skipping 202 matching lines @@
     if self.options.revision:
       if ":" in self.options.revision:
         extra_args = self.options.revision.split(":", 1) + extra_args
       else:
         extra_args = [self.options.revision] + extra_args
 
     # --no-ext-diff is broken in some versions of Git, so try to work around
     # 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)
 
   def GetUnknownFiles(self):
     status = RunShell(["git", "ls-files", "--exclude-standard", "--others"],
                       silent_ok=True)
     return status.splitlines()
 
   def GetFileContent(self, file_hash, is_binary):
     """Returns the content of a file identified by its git hash."""
     data, retcode = RunShellWithReturnCode(["git", "show", file_hash],
                                             universal_newlines=not is_binary)
     if retcode:
       ErrorExit("Got error status from 'git show %s'" % file_hash)
     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)
     status = None
 
     if filename in self.renames:
       status = "A +"  # Match svn attribute name for renames.
       if filename not in self.hashes:
         # If a rename doesn't change the content, we never get a hash.
         base_content = RunShell(["git", "show", "HEAD:" + filename])
@@ skipping 62 matching lines @@
         filecount += 1
         logging.info(line)
       else:
         svndiff.append(line)
     if not filecount:
       ErrorExit("No valid patches found in output from hg diff")
     return "\n".join(svndiff) + "\n"
 
   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 = []
     for line in status.splitlines():
       st, fn = line.split(" ", 1)
       if st == "?":
         unknown_files.append(fn)
     return unknown_files
 
   def GetBaseFile(self, filename):
@@ skipping 47 matching lines @@
   Returns:
     A list of 2-tuple (filename, text) where text is the svn diff output
       pertaining to filename.
   """
   patches = []
   filename = None
   diff = []
   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.
       temp_filename = temp_filename.strip().replace('\\', '/')
       if temp_filename != filename:
         # File has property changes but no modifications, create a new diff.
         new_filename = temp_filename
     if new_filename:
       if filename and diff:
         patches.append((filename, ''.join(diff)))
@@ skipping 47 matching lines @@
     output is a string containing any interesting output from the vcs
     detection routine, or None if there is nothing interesting.
   """
   # Mercurial has a command to get the base directory of a repository
   # Try running it, but don't die if we don't have hg installed.
   # NOTE: we try Mercurial first as it can sit on top of an SVN working copy.
   try:
     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.
   if os.path.isdir('.svn'):
     logging.info("Guessed VCS = Subversion")
     return (VCS_SUBVERSION, None)
 
   # Git has a command to test if you're in a git tree.
   # Try running it, but don't die if we don't have git installed.
   try:
     out, returncode = RunShellWithReturnCode(["git", "rev-parse",
                                               "--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)
 
 
 def GuessVCS(options):
   """Helper to guess the version control system.
 
   This verifies any user-specified VersionControlSystem (by command line
   or environment variable).  If the user didn't specify one, this examines
@@ skipping 228 matching lines @@
       CheckReviewer(reviewer)
     form_fields.append(("reviewers", options.reviewers))
   if options.cc:
     for cc in options.cc.split(','):
       CheckReviewer(cc)
     form_fields.append(("cc", options.cc))
   description = options.description
   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:
       print "Warning: Private flag ignored when updating an existing issue."
     else:
       form_fields.append(("private", "1"))
   # If we're uploading base files, don't send the email before the uploads, so
   # that it contains the file status.
   if options.send_mail and options.download_base:
     form_fields.append(("send_mail", "1"))
@@ skipping 40 matching lines @@
   try:
     RealMain(sys.argv)
   except KeyboardInterrupt:
     print
     StatusUpdate("Interrupted.")
     sys.exit(1)
 
 
 if __name__ == "__main__":
   main()