Run pychecker over most scripts in depot_tools. Catched a few bugs.

trychange.py

 #!/usr/bin/python
 # Copyright (c) 2009 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 """Client-side script to send a try job to the try server. It communicates to
 the try server by either writting to a svn repository or by directly connecting
 to the server by HTTP.
 """
 
 
 import datetime
 import getpass
 import logging
 import optparse
 import os
 import shutil
 import socket
 import subprocess
 import sys
 import tempfile
-import traceback
 import urllib
 
 import gcl
-import gclient
 import gclient_scm
 import presubmit_support
 import upload
 
 __version__ = '1.1.1'
 
 
 # Constants
 HELP_STRING = "Sorry, Tryserver is not available."
 USAGE = r"""%prog [change_name] [options]
@@ skipping 91 matching lines @@
     raise NoTryServerAccess(' '.join(command) + '\nOuput:\n' + output)
   return output
 
 
 class SCM(object):
   """Simplistic base class to implement one function: ProcessOptions."""
   def __init__(self, options):
     self.options = options
 
   def ProcessOptions(self):
-    raise Unimplemented
+    raise NotImplementedError
 
 
 class SVN(SCM):
   """Gathers the options and diff for a subversion checkout."""
   def GenerateDiff(self, files, root):
     """Returns a string containing the diff for the given file list.
 
     The files in the list should either be absolute paths or relative to the
     given root. If no root directory is provided, the repository root will be
     used.
     """
     previous_cwd = os.getcwd()
     if root is None:
       os.chdir(gcl.GetRepositoryRoot())
     else:
       os.chdir(root)
 
     diff = []
-    for file in files:
+    for filename in files:
       # Use svn info output instead of os.path.isdir because the latter fails
       # when the file is deleted.
-      if gclient_scm.CaptureSVNInfo(file).get("Node Kind") in ("dir",
-                                                               "directory"):
+      if gclient_scm.CaptureSVNInfo(filename).get("Node Kind") in (
+          "dir", "directory"):
         continue
       # If the user specified a custom diff command in their svn config file,
       # then it'll be used when we do svn diff, which we don't want to happen
       # since we want the unified diff.  Using --diff-cmd=diff doesn't always
       # work, since they can have another diff executable in their path that
       # gives different line endings.  So we use a bogus temp directory as the
       # config directory, which gets around these problems.
       if sys.platform.startswith("win"):
         parent_dir = tempfile.gettempdir()
       else:
         parent_dir = sys.path[0]  # tempdir is not secure.
       bogus_dir = os.path.join(parent_dir, "temp_svn_config")
       if not os.path.exists(bogus_dir):
         os.mkdir(bogus_dir)
       # Grabs the diff data.
-      data = gcl.RunShell(["svn", "diff", "--config-dir", bogus_dir, file])
+      data = gcl.RunShell(["svn", "diff", "--config-dir", bogus_dir, filename])
 
       # We know the diff will be incorrectly formatted. Fix it.
-      if gcl.IsSVNMoved(file):
+      if gcl.IsSVNMoved(filename):
         # The file is "new" in the patch sense. Generate a homebrew diff.
         # We can't use ReadFile() since it's not using binary mode.
-        file_handle = open(file, 'rb')
+        file_handle = open(filename, 'rb')
         file_content = file_handle.read()
         file_handle.close()
         # Prepend '+' to every lines.
         file_content = ['+' + i for i in file_content.splitlines(True)]
         nb_lines = len(file_content)
         # We need to use / since patch on unix will fail otherwise.
-        file = file.replace('\\', '/')
-        data = "Index: %s\n" % file
+        filename = filename.replace('\\', '/')
+        data = "Index: %s\n" % filename
         data += ("============================================================="
                  "======\n")
         # Note: Should we use /dev/null instead?
-        data += "--- %s\n" % file
-        data += "+++ %s\n" % file
+        data += "--- %s\n" % filename
+        data += "+++ %s\n" % filename
         data += "@@ -0,0 +1,%d @@\n" % nb_lines
         data += ''.join(file_content)
       diff.append(data)
     os.chdir(previous_cwd)
     return "".join(diff)
 
   def GetFileNames(self):
     """Return the list of files in the diff."""
     return self.change_info.GetFileNames()
 
@@ skipping 42 matching lines @@
     # TODO: check for errors here?
     root = upload.RunShell(['git', 'rev-parse', '--show-cdup']).strip()
     return os.path.abspath(root)
 
   def GetPatchName(self):
     """Construct a name for this patch."""
     # TODO: perhaps include the hash of the current commit, to distinguish
     # patches?
     branch = upload.RunShell(['git', 'symbolic-ref', 'HEAD']).strip()
     if not branch.startswith('refs/heads/'):
-      raise "Couldn't figure out branch name"
+      # TODO(maruel): Find a better type.
+      raise NoTryServerAccess("Couldn't figure out branch name")
     branch = branch[len('refs/heads/'):]
     return branch
 
   def ProcessOptions(self):
     if not self.options.diff:
       self.options.diff = self.GenerateDiff()
     if not self.options.name:
       self.options.name = self.GetPatchName()
     if not self.options.email:
       self.options.email = self.GetEmail()
@@ skipping 101 matching lines @@
     try:
       RunCommand(['svn', 'ls', full_url])
       file_found = True
     except NoTryServerAccess:
       pass
     if file_found:
       # The file already exists in the repo. Note that commiting a file is a
       # no-op if the file's content (the diff) is not modified. This is why the
       # file name contains the date and time.
       RunCommand(['svn', 'update', full_path])
-      file = open(full_path, 'wb')
-      file.write(options.diff)
-      file.close()
+      f = open(full_path, 'wb')
+      f.write(options.diff)
+      f.close()
     else:
       # Add the file to the repo
-      file = open(full_path, 'wb')
-      file.write(options.diff)
-      file.close()
+      f = open(full_path, 'wb')
+      f.write(options.diff)
+      f.close()
       RunCommand(["svn", "add", full_path])
     temp_file.write(description)
     temp_file.flush()
     RunCommand(["svn", "commit", full_path, '--file',
                 temp_file_name])
   finally:
     temp_file.close()
     shutil.rmtree(temp_dir, True)
 
 
@@ skipping 167 matching lines @@
       options.diff = urllib.urlopen(options.url).read()
     elif options.diff:
       options.diff = gcl.ReadFile(options.diff)
     # Process the VCS in any case at least to retrieve the email address.
     try:
       options.scm = GuessVCS(options)
       options.scm.ProcessOptions()
     except NoTryServerAccess, e:
       # If we got the diff, we don't care.
       if not options.diff:
+        # TODO(maruel): Raise what?
         raise
 
     # Get try slaves from PRESUBMIT.py files if not specified.
     if not options.bot:
       if options.url:
         print('You need to specify which bots to use.')
         return 1
       root_presubmit = gcl.GetCachedFile('PRESUBMIT.py', use_root=True)
       options.bot = presubmit_support.DoGetTrySlaves(options.scm.GetFileNames(),
                                                      options.scm.GetLocalRoot(),
                                                      root_presubmit,
                                                      False,
                                                      sys.stdout)
 
     if options.name is None:
       if options.issue:
-        patch_name = 'Issue %s' % options.issue
+        options.name = 'Issue %s' % options.issue
       else:
         options.name = 'Unnamed'
         print('Note: use --name NAME to change the try job name.')
     if not options.email:
       print('Warning: TRYBOT_RESULTS_EMAIL_ADDRESS is not set. Try server '
             'results might\ngo to: %s@google.com.\n' % options.user)
     else:
       print('Results will be emailed to: ' + options.email)
 
     # Send the patch.
     options.send_patch(options)
     print 'Patch \'%s\' sent to try server: %s' % (options.name,
                                                    ', '.join(options.bot))
   except (InvalidScript, NoTryServerAccess), e:
     if swallow_exception:
       return 1
     print e
     return 1
   return 0
 
 
 if __name__ == "__main__":
   sys.exit(TryChange(None, None, False))