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

gclient_scm.py

 # Copyright 2009 Google Inc.  All Rights Reserved.
 #
 # 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
 #
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
@@ skipping 78 matching lines @@
           command, self.scm_name))
 
     return getattr(self, command)(options, args, file_list)
 
 
 class GitWrapper(SCMWrapper):
   """Wrapper for Git"""
 
   def cleanup(self, options, args, file_list):
     """Cleanup working copy."""
+    __pychecker__ = 'unusednames=args,file_list,options'
     self._RunGit(['prune'], redirect_stdout=False)
     self._RunGit(['fsck'], redirect_stdout=False)
     self._RunGit(['gc'], redirect_stdout=False)
 
   def diff(self, options, args, file_list):
-    # NOTE: This function does not currently modify file_list.
+    __pychecker__ = 'unusednames=args,file_list,options'
     merge_base = self._RunGit(['merge-base', 'HEAD', 'origin'])
     self._RunGit(['diff', merge_base], redirect_stdout=False)
 
   def export(self, options, args, file_list):
+    __pychecker__ = 'unusednames=file_list,options'
     assert len(args) == 1
     export_path = os.path.abspath(os.path.join(args[0], self.relpath))
     if not os.path.exists(export_path):
       os.makedirs(export_path)
     self._RunGit(['checkout-index', '-a', '--prefix=%s/' % export_path],
                  redirect_stdout=False)
 
   def update(self, options, args, file_list):
     """Runs git to update or transparently checkout the working copy.
 
@@ skipping 41 matching lines @@
     files = self._RunGit(['diff', new_base, '--name-only']).split()
     file_list.extend([os.path.join(self.checkout_path, f) for f in files])
     self._RunGit(['rebase', '-v', new_base], redirect_stdout=False)
     print "Checked out revision %s." % self.revinfo(options, (), None)
 
   def revert(self, options, args, file_list):
     """Reverts local modifications.
 
     All reverted files will be appended to file_list.
     """
+    __pychecker__ = 'unusednames=args'
     path = os.path.join(self._root_dir, self.relpath)
     if not os.path.isdir(path):
       # revert won't work if the directory doesn't exist. It needs to
       # checkout instead.
       print("\n_____ %s is missing, synching instead" % self.relpath)
       # Don't reuse the args.
       return self.update(options, [], file_list)
     merge_base = self._RunGit(['merge-base', 'HEAD', 'origin'])
     files = self._RunGit(['diff', merge_base, '--name-only']).split()
     self._RunGit(['reset', '--hard', merge_base], redirect_stdout=False)
     file_list.extend([os.path.join(self.checkout_path, f) for f in files])
 
   def revinfo(self, options, args, file_list):
     """Display revision"""
+    __pychecker__ = 'unusednames=args,file_list,options'
     return self._RunGit(['rev-parse', 'HEAD'])
 
   def runhooks(self, options, args, file_list):
     self.status(options, args, file_list)
 
   def status(self, options, args, file_list):
     """Display status information."""
+    __pychecker__ = 'unusednames=args,options'
     if not os.path.isdir(self.checkout_path):
       print('\n________ couldn\'t run status in %s:\nThe directory '
-            'does not exist.' % checkout_path)
+            'does not exist.' % self.checkout_path)
     else:
       merge_base = self._RunGit(['merge-base', 'HEAD', 'origin'])
       self._RunGit(['diff', '--name-status', merge_base], redirect_stdout=False)
       files = self._RunGit(['diff', '--name-only', merge_base]).split()
       file_list.extend([os.path.join(self.checkout_path, f) for f in files])
 
   def _RunGit(self, args, cwd=None, checkrc=True, redirect_stdout=True):
     stdout=None
     if redirect_stdout:
       stdout=subprocess.PIPE
     if cwd == None:
       cwd = self.checkout_path
     cmd = ['git']
     cmd.extend(args)
     sp = subprocess.Popen(cmd, cwd=cwd, stdout=stdout)
     if checkrc and sp.returncode:
       raise gclient_utils.Error('git command %s returned %d' %
                                 (args[0], sp.returncode))
     output = sp.communicate()[0]
     if output != None:
       return output.strip()
 
 
 class SVNWrapper(SCMWrapper):
   """ Wrapper for SVN """
 
   def cleanup(self, options, args, file_list):
     """Cleanup working copy."""
+    __pychecker__ = 'unusednames=file_list,options'
     command = ['cleanup']
     command.extend(args)
     RunSVN(command, os.path.join(self._root_dir, self.relpath))
 
   def diff(self, options, args, file_list):
     # NOTE: This function does not currently modify file_list.
+    __pychecker__ = 'unusednames=file_list,options'
     command = ['diff']
     command.extend(args)
     RunSVN(command, os.path.join(self._root_dir, self.relpath))
 
   def export(self, options, args, file_list):
+    __pychecker__ = 'unusednames=file_list,options'
     assert len(args) == 1
     export_path = os.path.abspath(os.path.join(args[0], self.relpath))
     try:
       os.makedirs(export_path)
     except OSError:
       pass
     assert os.path.exists(export_path)
     command = ['export', '--force', '.']
     command.append(export_path)
     RunSVN(command, os.path.join(self._root_dir, self.relpath))
@@ skipping 111 matching lines @@
     if revision:
       command.extend(['--revision', str(revision)])
     RunSVNAndGetFileList(options, command, self._root_dir, file_list)
 
   def revert(self, options, args, file_list):
     """Reverts local modifications. Subversion specific.
 
     All reverted files will be appended to file_list, even if Subversion
     doesn't know about them.
     """
+    __pychecker__ = 'unusednames=args'
     path = os.path.join(self._root_dir, self.relpath)
     if not os.path.isdir(path):
       # svn revert won't work if the directory doesn't exist. It needs to
       # checkout instead.
       print("\n_____ %s is missing, synching instead" % self.relpath)
       # Don't reuse the args.
       return self.update(options, [], file_list)
 
-    for file in CaptureSVNStatus(path):
-      file_path = os.path.join(path, file[1])
-      if file[0][0] == 'X':
+    for file_status in CaptureSVNStatus(path):
+      file_path = os.path.join(path, file_status[1])
+      if file_status[0][0] == 'X':
         # Ignore externals.
         logging.info('Ignoring external %s' % file_path)
         continue
 
       if logging.getLogger().isEnabledFor(logging.INFO):
         logging.info('%s%s' % (file[0], file[1]))
       else:
         print(file_path)
-      if file[0].isspace():
+      if file_status[0].isspace():
         logging.error('No idea what is the status of %s.\n'
                       'You just found a bug in gclient, please ping '
                       'maruel@chromium.org ASAP!' % file_path)
       # svn revert is really stupid. It fails on inconsistent line-endings,
       # on switched directories, etc. So take no chance and delete everything!
       try:
         if not os.path.exists(file_path):
           pass
         elif os.path.isfile(file_path):
           logging.info('os.remove(%s)' % file_path)
@@ skipping 12 matching lines @@
       # "svn up --revision BASE" achieve the same effect.
       RunSVNAndGetFileList(options, ['update', '--revision', 'BASE'], path,
                            file_list)
     except OSError, e:
       # Maybe the directory disapeared meanwhile. We don't want it to throw an
       # exception.
       logging.error('Failed to update:\n%s' % str(e))
 
   def revinfo(self, options, args, file_list):
     """Display revision"""
+    __pychecker__ = 'unusednames=args,file_list,options'
     return CaptureSVNHeadRevision(self.url)
 
   def runhooks(self, options, args, file_list):
     self.status(options, args, file_list)
 
   def status(self, options, args, file_list):
     """Display status information."""
     path = os.path.join(self._root_dir, self.relpath)
     command = ['status']
     command.extend(args)
     if not os.path.isdir(path):
       # svn status won't work if the directory doesn't exist.
       print("\n________ couldn't run \'%s\' in \'%s\':\nThe directory "
             "does not exist."
             % (' '.join(command), path))
       # There's no file list to retrieve.
     else:
       RunSVNAndGetFileList(options, command, path, file_list)
 
   def pack(self, options, args, file_list):
     """Generates a patch file which can be applied to the root of the
     repository."""
+    __pychecker__ = 'unusednames=file_list,options'
     path = os.path.join(self._root_dir, self.relpath)
     command = ['diff']
     command.extend(args)
     # Simple class which tracks which file is being diffed and
     # replaces instances of its file name in the original and
     # working copy lines of the svn diff output.
     class DiffFilterer(object):
       index_string = "Index: "
       original_prefix = "--- "
       working_prefix = "+++ "
@@ skipping 302 matching lines @@
     'normal': ' ',
     'obstructed': '~',
     'replaced': 'R',
     'unversioned': '?',
   }
   dom = gclient_utils.ParseXML(CaptureSVN(command))
   results = []
   if dom:
     # /status/target/entry/(wc-status|commit|author|date)
     for target in dom.getElementsByTagName('target'):
-      base_path = target.getAttribute('path')
       for entry in target.getElementsByTagName('entry'):
-        file = entry.getAttribute('path')
+        file_path = entry.getAttribute('path')
         wc_status = entry.getElementsByTagName('wc-status')
         assert len(wc_status) == 1
         # Emulate svn 1.5 status ouput...
-        statuses = [' ' for i in range(7)]
+        statuses = [' '] * 7
         # Col 0
         xml_item_status = wc_status[0].getAttribute('item')
         if xml_item_status in status_letter:
           statuses[0] = status_letter[xml_item_status]
         else:
           raise Exception('Unknown item status "%s"; please implement me!' %
                           xml_item_status)
         # Col 1
         xml_props_status = wc_status[0].getAttribute('props')
         if xml_props_status == 'modified':
           statuses[1] = 'M'
         elif xml_props_status == 'conflicted':
           statuses[1] = 'C'
         elif (not xml_props_status or xml_props_status == 'none' or
               xml_props_status == 'normal'):
           pass
         else:
           raise Exception('Unknown props status "%s"; please implement me!' %
                           xml_props_status)
         # Col 2
         if wc_status[0].getAttribute('wc-locked') == 'true':
           statuses[2] = 'L'
         # Col 3
         if wc_status[0].getAttribute('copied') == 'true':
           statuses[3] = '+'
         # Col 4
         if wc_status[0].getAttribute('switched') == 'true':
           statuses[4] = 'S'
         # TODO(maruel): Col 5 and 6
-        item = (''.join(statuses), file)
+        item = (''.join(statuses), file_path)
         results.append(item)
   return results