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

gcl.py

 #!/usr/bin/python
 # Copyright (c) 2006-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.
 #
 # Wrapper script around Rietveld's upload.py that groups files into
 # changelists.
 
 import getpass
 import os
 import random
 import re
 import shutil
 import string
 import subprocess
 import sys
 import tempfile
 import upload
 import urllib2
-import xml.dom.minidom
 
 # gcl now depends on gclient.
 import gclient_scm
 import gclient_utils
 
 __version__ = '1.1.1'
 
 
 CODEREVIEW_SETTINGS = {
   # Default values.
@@ skipping 122 matching lines @@
         url_path = repo_root
       else:
         url_path = dir_info["URL"]
       content = ""
       while True:
         # First, look for a locally modified version of the file.
         local_path = os.path.join(local_dir, local_base)
         r = gclient_scm.CaptureSVNStatus((local_path,))
         rc = -1
         if r:
-          (status, file) = r[0]
+          status = r[0][0]
           rc = 0
         if not rc and status[0] in ('A','M'):
           content = ReadFile(local_path)
           rc = 0
         else:
           # Then look in the repository.
           svn_path = url_path + "/" + filename
           content, rc = RunShellWithReturnCode(["svn", "cat", svn_path])
 
         if not rc:
           # Exit the loop if the file was found. Override content.
           break
         # Make sure to mark settings as empty if not found.
         content = ""
         if url_path == repo_root:
           # Reached the root. Abandoning search.
           break
         # Go up one level to try again.
         url_path = os.path.dirname(url_path)
         local_dir = os.path.dirname(local_dir)
       # Write a cached version even if there isn't a file, so we don't try to
       # fetch it each time.
       WriteFile(cached_file, content)
     else:
-      content = ReadFile(cached_settings_file)
+      content = ReadFile(cached_file)
     # Keep the content cached in memory.
     FILES_CACHE[filename] = content
   return FILES_CACHE[filename]
 
 
 def GetCodeReviewSetting(key):
   """Returns a value for the given key for this repository."""
   # Use '__just_initialized' as a flag to determine if the settings were
   # already initialized.
   global CODEREVIEW_SETTINGS
@@ skipping 45 matching lines @@
   return output, p.returncode
 
 
 def RunShell(command, print_output=False):
   """Executes a command and returns the output."""
   return RunShellWithReturnCode(command, print_output)[0]
 
 
 def ReadFile(filename, flags='r'):
   """Returns the contents of a file."""
-  file = open(filename, flags)
-  result = file.read()
-  file.close()
+  f = open(filename, flags)
+  result = f.read()
+  f.close()
   return result
 
 
 def WriteFile(filename, contents):
   """Overwrites the file with the given contents."""
-  file = open(filename, 'w')
-  file.write(contents)
-  file.close()
+  f = open(filename, 'w')
+  f.write(contents)
+  f.close()
 
 
 def FilterFlag(args, flag):
   """Returns True if the flag is present in args list.
 
   The flag is removed from args if present.
   """
   if flag in args:
     args.remove(flag)
     return True
@@ skipping 30 matching lines @@
     self.patchset = int(patchset)
     self.description = description
     if files is None:
       files = []
     self._files = files
     self.patch = None
     self._local_root = local_root
 
   def GetFileNames(self):
     """Returns the list of file names included in this change."""
-    return [file[1] for file in self._files]
+    return [f[1] for f in self._files]
 
   def GetFiles(self):
     """Returns the list of files included in this change with their status."""
     return self._files
 
   def GetLocalRoot(self):
     """Returns the local repository checkout root directory."""
     return self._local_root
 
   def Exists(self):
     """Returns True if this change already exists (i.e., is not new)."""
     return (self.issue or self.description or self._files)
 
   def _NonDeletedFileList(self):
     """Returns a list of files in this change, not including deleted files."""
-    return [file[1] for file in self.GetFiles()
-            if not file[0].startswith("D")]
+    return [f[1] for f in self.GetFiles()
+            if not f[0].startswith("D")]
 
   def _AddedFileList(self):
     """Returns a list of files added in this change."""
-    return [file[1] for file in self.GetFiles() if file[0].startswith("A")]
+    return [f[1] for f in self.GetFiles() if f[0].startswith("A")]
 
   def Save(self):
     """Writes the changelist information to disk."""
     data = ChangeInfo._SEPARATOR.join([
         "%d, %d" % (self.issue, self.patchset),
         "\n".join([f[0] + f[1] for f in self.GetFiles()]),
         self.description])
     WriteFile(GetChangelistInfoFile(self.name), data)
 
   def Delete(self):
@@ skipping 12 matching lines @@
     ctype, body = upload.EncodeMultipartFormData(data, [])
     SendToRietveld("/%d/description" % self.issue, body, ctype)
 
   def MissingTests(self):
     """Returns True if the change looks like it needs unit tests but has none.
 
     A change needs unit tests if it contains any new source files or methods.
     """
     SOURCE_SUFFIXES = [".cc", ".cpp", ".c", ".m", ".mm"]
     # Ignore third_party entirely.
-    files = [file for file in self._NonDeletedFileList()
-             if file.find("third_party") == -1]
-    added_files = [file for file in self._AddedFileList()
-                   if file.find("third_party") == -1]
+    files = [f for f in self._NonDeletedFileList()
+             if f.find("third_party") == -1]
+    added_files = [f for f in self._AddedFileList()
+                   if f.find("third_party") == -1]
 
     # If the change is entirely in third_party, we're done.
     if len(files) == 0:
       return False
 
     # Any new or modified test files?
     # A test file's name ends with "test.*" or "tests.*".
     test_files = [test for test in files
                   if os.path.splitext(test)[0].rstrip("s").endswith("test")]
     if len(test_files) > 0:
       return False
 
     # Any new source files?
-    source_files = [file for file in added_files
-                    if os.path.splitext(file)[1] in SOURCE_SUFFIXES]
+    source_files = [item for item in added_files
+                    if os.path.splitext(item)[1] in SOURCE_SUFFIXES]
     if len(source_files) > 0:
       return True
 
     # Do the long test, checking the files for new methods.
     return self._HasNewMethod()
 
   def _HasNewMethod(self):
     """Returns True if the changeset contains any new functions, or if a
     function signature has been changed.
 
@@ skipping 65 matching lines @@
     items = split_data[0].split(',')
     issue = 0
     patchset = 0
     if items[0]:
       issue = int(items[0])
     if len(items) > 1:
       patchset = int(items[1])
     files = []
     for line in split_data[1].splitlines():
       status = line[:7]
-      file = line[7:]
-      files.append((status, file))
+      filename = line[7:]
+      files.append((status, filename))
     description = split_data[2]
     save = False
     if update_status:
-      for file in files:
-        filename = os.path.join(local_root, file[1])
+      for item in files:
+        filename = os.path.join(local_root, item[1])
         status_result = gclient_scm.CaptureSVNStatus(filename)
         if not status_result or not status_result[0][0]:
           # File has been reverted.
           save = True
-          files.remove(file)
+          files.remove(item)
           continue
         status = status_result[0][0]
-        if status != file[0]:
+        if status != item[0]:
           save = True
-          files[files.index(file)] = (status, file[1])
+          files[files.index(item)] = (status, item[1])
     change_info = ChangeInfo(changename, issue, patchset, description, files,
                              local_root)
     if save:
       change_info.Save()
     return change_info
 
 
 def GetChangelistInfoFile(changename):
   """Returns the file that stores information about a changelist."""
   if not changename or re.search(r'[^\w-]', changename):
@@ skipping 98 matching lines @@
     password = getpass.getpass("Password for %s: " % email)
     return email, password
 
   server = GetCodeReviewSetting("CODE_REVIEW_SERVER")
   rpc_server = upload.HttpRpcServer(server,
                                     GetUserCredentials,
                                     host_override=server,
                                     save_cookies=True)
   try:
     return rpc_server.Send(request_path, payload, content_type, timeout)
-  except urllib2.URLError, e:
+  except urllib2.URLError:
     if timeout is None:
       ErrorExit("Error accessing url %s" % request_path)
     else:
       return None
 
 
 def GetIssueDescription(issue):
   """Returns the issue description from Rietveld."""
   return SendToRietveld("/%d/description" % issue)
 
 
 def Opened(show_unknown_files):
   """Prints a list of modified files in the current directory down."""
   files = GetModifiedFiles()
   cl_keys = files.keys()
   cl_keys.sort()
   for cl_name in cl_keys:
     if not cl_name:
       continue
     note = ""
     change_info = ChangeInfo.Load(cl_name, GetRepositoryRoot(),
                                   fail_on_not_found=True, update_status=False)
     if len(change_info.GetFiles()) != len(files[cl_name]):
       note = " (Note: this changelist contains files outside this directory)"
     print "\n--- Changelist " + cl_name + note + ":"
-    for file in files[cl_name]:
-      print "".join(file)
+    for filename in files[cl_name]:
+      print "".join(filename)
   if show_unknown_files:
     unknown_files = UnknownFiles([])
   if (files.get('') or (show_unknown_files and len(unknown_files))):
     print "\n--- Not in any changelist:"
-    for file in files.get('', []):
-      print "".join(file)
+    for item in files.get('', []):
+      print "".join(item)
     if show_unknown_files:
-      for file in unknown_files:
-        print "?      %s" % file
+      for filename in unknown_files:
+        print "?      %s" % filename
 
 
 def Help(argv=None):
   if argv:
     if argv[0] == 'try':
       TryChange(None, ['--help'], swallow_exception=False)
       return
     if argv[0] == 'upload':
       upload.RealMain(['upload.py', '--help'])
       return
@@ skipping 85 matching lines @@
   given root. If no root directory is provided, the repository root will be
   used.
   """
   previous_cwd = os.getcwd()
   if root is None:
     os.chdir(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)
-    output = RunShell(["svn", "diff", "--config-dir", bogus_dir, file])
+    output = RunShell(["svn", "diff", "--config-dir", bogus_dir, filename])
     if output:
       diff.append(output)
-    elif IsSVNMoved(file):
+    elif IsSVNMoved(filename):
       #  svn diff on a mv/cp'd file outputs nothing.
       # We put in an empty Index entry so upload.py knows about them.
-      diff.append("\nIndex: %s\n" % file)
+      diff.append("\nIndex: %s\n" % filename)
     else:
       # The file is not modified anymore. It should be removed from the set.
       pass
   os.chdir(previous_cwd)
   return "".join(diff)
 
 
 
 def OptionallyDoPresubmitChecks(change_info, committing, args):
   if FilterFlag(args, "--no_presubmit") or FilterFlag(args, "--force"):
@@ skipping 157 matching lines @@
     return
 
   # We face a problem with svn here: Let's say change 'bleh' modifies
   # svn:ignore on dir1\. but another unrelated change 'pouet' modifies
   # dir1\foo.cc. When the user `gcl commit bleh`, foo.cc is *also committed*.
   # The only fix is to use --non-recursive but that has its issues too:
   # Let's say if dir1 is deleted, --non-recursive must *not* be used otherwise
   # you'll get "svn: Cannot non-recursively commit a directory deletion of a
   # directory with child nodes". Yay...
   commit_cmd = ["svn", "commit"]
-  filename = ''
   if change_info.issue:
     # Get the latest description from Rietveld.
     change_info.description = GetIssueDescription(change_info.issue)
 
   commit_message = change_info.description.replace('\r\n', '\n')
   if change_info.issue:
     commit_message += ('\nReview URL: http://%s/%d' %
                        (GetCodeReviewSetting("CODE_REVIEW_SERVER"),
                         change_info.issue))
 
@@ skipping 114 matching lines @@
       # Update the Rietveld issue with the new description.
       change_info.UpdateRietveldDescription()
 
   new_cl_files = []
   for line in cl_files_text.splitlines():
     if not len(line):
       continue
     if line.startswith("---"):
       break
     status = line[:7]
-    file = line[7:]
-    new_cl_files.append((status, file))
+    filename = line[7:]
+    new_cl_files.append((status, filename))
 
   if (not len(change_info._files)) and (not change_info.issue) and \
       (not len(new_description) and (not new_cl_files)):
     ErrorExit("Empty changelist not saved")
 
   change_info._files = new_cl_files
 
   change_info.Save()
   print change_info.name + " changelist saved."
   if change_info.MissingTests():
@@ skipping 19 matching lines @@
   filenames = cpplint.ParseArguments(args + change_info.GetFileNames())
 
   white_list = GetCodeReviewSetting("LINT_REGEX")
   if not white_list:
     white_list = DEFAULT_LINT_REGEX
   white_regex = re.compile(white_list)
   black_list = GetCodeReviewSetting("LINT_IGNORE_REGEX")
   if not black_list:
     black_list = DEFAULT_LINT_IGNORE_REGEX
   black_regex = re.compile(black_list)
-  for file in filenames:
-    if white_regex.match(file):
-      if black_regex.match(file):
-        print "Ignoring file %s" % file
+  for filename in filenames:
+    if white_regex.match(filename):
+      if black_regex.match(filename):
+        print "Ignoring file %s" % filename
       else:
-        cpplint.ProcessFile(file, cpplint._cpplint_state.verbose_level)
+        cpplint.ProcessFile(filename, cpplint._cpplint_state.verbose_level)
     else:
-      print "Skipping file %s" % file
+      print "Skipping file %s" % filename
 
   print "Total errors found: %d\n" % cpplint._cpplint_state.error_count
   os.chdir(previous_cwd)
 
 
 def DoPresubmitChecks(change_info, committing, may_prompt):
   """Imports presubmit, then calls presubmit.DoPresubmitChecks."""
   # Need to import here to avoid circular dependency.
   import presubmit_support
   root_presubmit = GetCachedFile('PRESUBMIT.py', use_root=True)
@@ skipping 13 matching lines @@
   if not result and may_prompt:
     print "\nPresubmit errors, can't continue (use --no_presubmit to bypass)"
   return result
 
 
 def Changes():
   """Print all the changelists and their files."""
   for cl in GetCLs():
     change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True)
     print "\n--- Changelist " + change_info.name + ":"
-    for file in change_info.GetFiles():
-      print "".join(file)
+    for filename in change_info.GetFiles():
+      print "".join(filename)
 
 
 def DeleteEmptyChangeLists():
   """Delete all changelists that have no files."""
   print "\n--- Deleting:"
   for cl in GetCLs():
     change_info = ChangeInfo.Load(cl, GetRepositoryRoot(), True, True)
     if not len(change_info._files):
       print change_info.name
       change_info.Delete()
@@ skipping 10 matching lines @@
   try:
     # Create the directories where we store information about changelists if it
     # doesn't exist.
     if not os.path.exists(GetInfoDir()):
       os.mkdir(GetInfoDir())
     if not os.path.exists(GetChangesDir()):
       os.mkdir(GetChangesDir())
       # For smooth upgrade support, move the files in GetInfoDir() to
       # GetChangesDir().
       # TODO(maruel): Remove this code in August 2009.
-      for file in os.listdir(unicode(GetInfoDir())):
-        file_path = os.path.join(unicode(GetInfoDir()), file)
-        if os.path.isfile(file_path) and file != CODEREVIEW_SETTINGS_FILE:
+      for filename in os.listdir(unicode(GetInfoDir())):
+        file_path = os.path.join(unicode(GetInfoDir()), filename)
+        if os.path.isfile(file_path) and filename != CODEREVIEW_SETTINGS_FILE:
           shutil.move(file_path, GetChangesDir())
     if not os.path.exists(GetCacheDir()):
       os.mkdir(GetCacheDir())
   except gclient_utils.Error:
     # Will throw an exception if not run in a svn checkout.
     pass
 
   # Commands that don't require an argument.
   command = argv[1]
   if command == "opened" or command == "status":
     Opened(command == "status")
     return 0
   if command == "nothave":
-    unknown_files = UnknownFiles(argv[2:])
-    for file in unknown_files:
-      print "?      " + "".join(file)
+    __pychecker__ = 'no-returnvalues'
+    for filename in UnknownFiles(argv[2:]):
+      print "?      " + "".join(filename)
     return 0
   if command == "changes":
     Changes()
     return 0
   if command == "help":
     Help(argv[2:])
     return 0
   if command == "diff" and len(argv) == 2:
     files = GetFilesNotInCL()
     print GenerateDiff([x[1] for x in files])
     return 0
   if command == "settings":
-    ignore = GetCodeReviewSetting("UNKNOWN");
+    # Force load settings
+    GetCodeReviewSetting("UNKNOWN");
     del CODEREVIEW_SETTINGS['__just_initialized']
     print '\n'.join(("%s: %s" % (str(k), str(v))
                      for (k,v) in CODEREVIEW_SETTINGS.iteritems()))
     return 0
   if command == "deleteempties":
     DeleteEmptyChangeLists()
     return 0
 
   if command == "change":
     if len(argv) == 2:
@@ skipping 50 matching lines @@
       return 0
     args =["svn", command]
     root = GetRepositoryRoot()
     args.extend([os.path.join(root, x) for x in change_info.GetFileNames()])
     RunShell(args, True)
   return 0
 
 
 if __name__ == "__main__":
   sys.exit(main())