Largely reduce the number of pylint warnings and fix one typo.

Most of them are style issues or variable aliasing.

TEST=Can almost enable pylint warnings
BUG=none

Review URL: http://codereview.chromium.org/4360002

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@64908 0039d316-1c4b-4281-b951-d872f2087c98

breakpad.py

@@ -46,9 +46,9 @@ def SendStack(last_tb, stack, url=None):
       params['exception'] = str(last_tb)
     except:
       pass
-    print '\n'.join('  %s: %s' % (k, v[0:50]) for k,v in params.iteritems())
+    print('\n'.join('  %s: %s' % (k, v[0:50]) for k, v in params.iteritems()))
     request = urllib.urlopen(url, urllib.urlencode(params))
-    print request.read()
+    print(request.read())
     request.close()
   except IOError:
     print('There was a failure while trying to send the stack trace. Too bad.')

chrome-update.py

@@ -15,9 +15,9 @@ COMPILE_URL = BASE_URL + 'slave/compile.py'
 UTILS_URL = BASE_URL + 'common/chromium_utils.py'
 
 
-def Fetch(url, file):
-  if not os.path.exists(file):
-    urllib.urlretrieve(url, file)
+def Fetch(url, filename):
+  if not os.path.exists(filename):
+    urllib.urlretrieve(url, filename)
 
 
 def GetLastestRevision():
@@ -53,10 +53,10 @@ def DoUpdate(chrome_root):
 
 def DoBuild(chrome_root, args):
   """Download compile.py and run it."""
-  compile = os.path.join(chrome_root, 'compile.py')
-  Fetch(COMPILE_URL, compile)
+  compile_path = os.path.join(chrome_root, 'compile.py')
+  Fetch(COMPILE_URL, compile_path)
   Fetch(UTILS_URL, os.path.join(chrome_root, 'chromium_utils.py'))
-  cmd = ['python', compile] + args
+  cmd = ['python', compile_path] + args
   return subprocess.call(cmd, cwd=chrome_root, shell=IS_WIN)
 
 

drover.py

@@ -9,7 +9,7 @@ import re
 import subprocess
 import sys
 
-import breakpad
+import breakpad  # pylint: disable=W0611
 
 import gclient_utils
 

gcl.py

@@ -35,7 +35,7 @@ except ImportError:
     sys.path.append(os.path.join(os.path.dirname(__file__), 'third_party'))
     import simplejson as json
 
-import breakpad
+import breakpad  # pylint: disable=W0611
 
 # gcl now depends on gclient.
 from scm import SVN

gclient.py

@@ -58,22 +58,21 @@ import os
 import posixpath
 import pprint
 import re
-import subprocess
 import sys
 import urlparse
 import urllib
 
-import breakpad
+import breakpad  # pylint: disable=W0611
 
 import gclient_scm
 import gclient_utils
 from third_party.repo.progress import Progress
 
 
-def attr(attr, data):
+def attr(attribute, data):
   """Sets an attribute on a function."""
   def hook(fn):
-    setattr(fn, attr, data)
+    setattr(fn, attribute, data)
     return fn
   return hook
 

gclient_scm.py

@@ -8,7 +8,6 @@ import logging
 import os
 import posixpath
 import re
-import sys
 import time
 
 import scm

gclient_utils.py

@@ -14,7 +14,6 @@
 
 """Generic utils."""
 
-import copy
 import errno
 import logging
 import os
@@ -300,7 +299,8 @@ def SoftClone(obj):
   """Clones an object. copy.copy() doesn't work on 'file' objects."""
   if obj.__class__.__name__ == 'SoftCloned':
     return obj
-  class SoftCloned(object): pass
+  class SoftCloned(object):
+    pass
   new_obj = SoftCloned()
   for member in dir(obj):
     if member.startswith('_'):
@@ -389,7 +389,7 @@ def MakeFileAnnotated(fileobj):
     try:
       # Detect threads no longer existing.
       indexes = (getattr(t, 'index', None) for t in threading.enumerate())
-      indexed = filter(None, indexes)
+      indexes = filter(None, indexes)
       for index in new_fileobj.output_buffers:
         if not index in indexes:
           orphans.append((index, new_fileobj.output_buffers[index][0]))
@@ -480,7 +480,7 @@ def FindGclientRoot(from_dir, filename='.gclient'):
       # might have failed. In that case, we cannot verify that the .gclient
       # is the one we want to use. In order to not to cause too much trouble,
       # just issue a warning and return the path anyway.
-      print >>sys.stderr, ("%s file in parent directory %s might not be the "
+      print >> sys.stderr, ("%s file in parent directory %s might not be the "
           "file you want to use" % (filename, path))
       return path
     scope = {}

git-try

@@ -7,7 +7,7 @@
 import logging
 import sys
 
-import breakpad
+import breakpad  # pylint: disable=W0611
 
 import gclient_utils
 from scm import GIT

git_cl_hooks.py

@@ -3,11 +3,10 @@
 # found in the LICENSE file.
 
 import os
-import re
 import subprocess
 import sys
 
-import breakpad
+import breakpad  # pylint: disable=W0611
 
 from git_cl_repo import git_cl
 from git_cl_repo import upload
@@ -25,11 +24,11 @@ def Backquote(cmd, cwd=None):
                           cwd=cwd,
                           stdout=subprocess.PIPE).communicate()[0].strip()
 
-def ConvertToInteger(input):
+def ConvertToInteger(inputval):
   """Convert a string to integer, but returns either an int or None."""
   try:
-    return int(input)
-  except TypeError, ValueError:
+    return int(inputval)
+  except (TypeError, ValueError):
     return None
 
 

presubmit_canned_checks.py

@@ -275,11 +275,11 @@ def CheckLongLines(input_api, output_api, maxlen=80, source_file_filter=None):
     return []
 
 
-def CheckLicense(input_api, output_api, license, source_file_filter=None,
+def CheckLicense(input_api, output_api, license_re, source_file_filter=None,
     accept_empty_files=True):
   """Verifies the license header.
   """
-  license_re = input_api.re.compile(license, input_api.re.MULTILINE)
+  license_re = input_api.re.compile(license_re, input_api.re.MULTILINE)
   bad_files = []
   for f in input_api.AffectedSourceFiles(source_file_filter):
     contents = input_api.ReadFile(f, 'rb')
@@ -483,7 +483,7 @@ def CheckRietveldTryJobExecution(input_api, output_api, host_url, platforms,
   for platform in platforms:
     values.setdefault(platform, ['not started', ''])
   message = None
-  non_success = [k.upper() for k,v in values.iteritems() if v[0] != 'success']
+  non_success = [k.upper() for k, v in values.iteritems() if v[0] != 'success']
   if 'failure' in [v[0] for v in values.itervalues()]:
     message = 'Try job failures on %s!\n' % ', '.join(non_success)
   elif non_success:

presubmit_support.py

@@ -1,5 +1,5 @@
 #!/usr/bin/python
-# Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
+# Copyright (c) 2010 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.
 
@@ -32,7 +32,7 @@ import traceback  # Exposed through the API.
 import types
 import unittest  # Exposed through the API.
 import urllib2  # Exposed through the API.
-import warnings
+from warnings import warn
 
 try:
   import simplejson as json
@@ -48,7 +48,6 @@ except ImportError:
     import simplejson as json
 
 # Local imports.
-import gcl
 import gclient_utils
 import presubmit_canned_checks
 import scm
@@ -75,11 +74,23 @@ def normpath(path):
   path = path.replace(os.sep, '/')
   return os.path.normpath(path)
 
+
 def PromptYesNo(input_stream, output_stream, prompt):
   output_stream.write(prompt)
   response = input_stream.readline().strip().lower()
   return response == 'y' or response == 'yes'
 
+
+def _RightHandSideLinesImpl(affected_files):
+  """Implements RightHandSideLines for InputApi and GclChange."""
+  for af in affected_files:
+    lines = af.NewContents()
+    line_number = 0
+    for line in lines:
+      line_number += 1
+      yield (af, line_number, line)
+
+
 class OutputApi(object):
   """This class (more like a module) gets passed to presubmit scripts so that
   they can specify various types of results.
@@ -158,7 +169,8 @@ class OutputApi(object):
   class MailTextResult(PresubmitResult):
     """A warning that should be included in the review request email."""
     def __init__(self, *args, **kwargs):
-      raise NotImplementedException()  # TODO(joi) Implement.
+      super(OutputApi.MailTextResult, self).__init__()
+      raise NotImplementedException()
 
 
 class InputApi(object):
@@ -309,10 +321,10 @@ class InputApi(object):
     thereof.
     """
     if include_deletes is not None:
-      warnings.warn("AffectedTextFiles(include_deletes=%s)"
-                        " is deprecated and ignored" % str(include_deletes),
-                    category=DeprecationWarning,
-                    stacklevel=2)
+      warn("AffectedTextFiles(include_deletes=%s)"
+               " is deprecated and ignored" % str(include_deletes),
+           category=DeprecationWarning,
+           stacklevel=2)
     return filter(lambda x: x.IsTextFile(),
                   self.AffectedFiles(include_dirs=False, include_deletes=False))
 
@@ -327,8 +339,8 @@ class InputApi(object):
 
     Note: Copy-paste this function to suit your needs or use a lambda function.
     """
-    def Find(affected_file, list):
-      for item in list:
+    def Find(affected_file, items):
+      for item in items:
         local_path = affected_file.LocalPath()
         if self.re.match(item, local_path):
           logging.debug("%s matched %s" % (item, local_path))
@@ -364,7 +376,7 @@ class InputApi(object):
     Note: The cariage return (LF or CR) is stripped off.
     """
     files = self.AffectedSourceFiles(source_file_filter)
-    return InputApi._RightHandSideLinesImpl(files)
+    return _RightHandSideLinesImpl(files)
 
   def ReadFile(self, file_item, mode='r'):
     """Reads an arbitrary file.
@@ -377,16 +389,6 @@ class InputApi(object):
       raise IOError('Access outside the repository root is denied.')
     return gclient_utils.FileRead(file_item, mode)
 
-  @staticmethod
-  def _RightHandSideLinesImpl(affected_files):
-    """Implements RightHandSideLines for InputApi and GclChange."""
-    for af in affected_files:
-      lines = af.NewContents()
-      line_number = 0
-      for line in lines:
-        line_number += 1
-        yield (af, line_number, line)
-
 
 class AffectedFile(object):
   """Representation of a file in a change."""
@@ -665,10 +667,10 @@ class Change(object):
   def AffectedTextFiles(self, include_deletes=None):
     """Return a list of the existing text files in a change."""
     if include_deletes is not None:
-      warnings.warn("AffectedTextFiles(include_deletes=%s)"
-                        " is deprecated and ignored" % str(include_deletes),
-                    category=DeprecationWarning,
-                    stacklevel=2)
+      warn("AffectedTextFiles(include_deletes=%s)"
+               " is deprecated and ignored" % str(include_deletes),
+           category=DeprecationWarning,
+           stacklevel=2)
     return filter(lambda x: x.IsTextFile(),
                   self.AffectedFiles(include_dirs=False, include_deletes=False))
 
@@ -698,9 +700,9 @@ class Change(object):
         integer line number (1-based); and
         the contents of the line as a string.
     """
-    return InputApi._RightHandSideLinesImpl(
-        filter(lambda x: x.IsTextFile(),
-               self.AffectedFiles(include_deletes=False)))
+    return _RightHandSideLinesImpl(
+        x for x in self.AffectedFiles(include_deletes=False)
+        if x.IsTextFile())
 
 
 class SvnChange(Change):
@@ -716,6 +718,8 @@ class SvnChange(Change):
     if self._changelists == None:
       previous_cwd = os.getcwd()
       os.chdir(self.RepositoryRoot())
+      # Need to import here to avoid circular dependency.
+      import gcl
       self._changelists = gcl.GetModifiedFiles()
       os.chdir(previous_cwd)
     return self._changelists
@@ -792,7 +796,8 @@ def ListRelevantPresubmitFiles(files, root):
 
 
 class GetTrySlavesExecuter(object):
-  def ExecPresubmitScript(self, script_text):
+  @staticmethod
+  def ExecPresubmitScript(script_text):
     """Executes GetPreferredTrySlaves() from a single presubmit script.
 
     Args:

pylintrc

@@ -36,6 +36,7 @@ load-plugins=
 # C0111: Missing docstring
 # C0302: Too many lines in module (N)
 # I0011: Locally disabling WNNNN
+# R0801: Similar lines in N files
 # R0901: Too many ancestors (8/7)
 # R0902: Too many instance attributes (N/7)
 # R0903: Too few public methods (N/2)
@@ -46,10 +47,13 @@ load-plugins=
 # R0915: Too many statements (N/50)
 # W0122: Use of the exec statement
 # W0141: Used builtin function ''
+# W0142: Used * or ** magic
+# W0402: Uses of a deprecated module 'string'
+# W0511: TODO
 # W0603: Using the global statement
 # W0613: Unused argument ''
 # W6501: Specify string format arguments as logging function parameters
-disable=C0103,C0111,C0302,I0011,R0901,R0902,R0903,R0911,R0912,R0913,R0914,R0915,W0122,W0141,W0603,W0613,W6501
+disable=C0103,C0111,C0302,I0011,R0801,R0901,R0902,R0903,R0911,R0912,R0913,R0914,R0915,W0122,W0141,W0142,W0402,W0511,W0603,W0613,W6501
 
 
 [REPORTS]

tests/gclient_scm_test.py

@@ -17,6 +17,7 @@ import __builtin__
 # Fixes include path.
 from super_mox import mox, StdoutCheck, TestCaseUtils, SuperMoxTestBase
 
+import sys
 import gclient_scm
 
 # Shortcut since this function is used often
@@ -789,11 +790,11 @@ from :3
     end = ('] test\n 1 files changed, 1 insertions(+), '
          '1 deletions(-)\n\n_____ . at refs/heads/master\n'
          'Attempting rebase onto refs/remotes/origin/master...\n')
-    self.assertTrue(gclient_scm.sys.stdout.getvalue().startswith(start))
-    self.assertTrue(gclient_scm.sys.stdout.getvalue().endswith(end))
-    self.assertEquals(len(gclient_scm.sys.stdout.getvalue()),
+    self.assertTrue(sys.stdout.getvalue().startswith(start))
+    self.assertTrue(sys.stdout.getvalue().endswith(end))
+    self.assertEquals(len(sys.stdout.getvalue()),
                       len(start) + len(end) + 7)
-    gclient_scm.sys.stdout.close()
+    sys.stdout.close()
 
   def testUpdateNotGit(self):
     if not self.enabled:

tests/gclient_utils_test.py

@@ -30,7 +30,7 @@ class GclientUtilsUnittest(GclientUtilBase):
         'ParseXML', 'Popen',
         'PrintableObject', 'RemoveDirectory', 'SoftClone', 'SplitUrlRevision',
         'SyntaxErrorToError', 'WorkItem',
-        'copy', 'errno', 'logging', 'os', 'Queue', 're', 'stat', 'subprocess',
+        'errno', 'logging', 'os', 'Queue', 're', 'stat', 'subprocess',
         'sys','threading', 'time', 'xml',
     ]
     # If this test fails, you should add the relevant test.

tests/presubmit_unittest.py

@@ -37,7 +37,7 @@ def GetPreferredTrySlaves():
   def setUp(self):
     SuperMoxTestBase.setUp(self)
     self.mox.StubOutWithMock(presubmit, 'random')
-    self.mox.StubOutWithMock(presubmit, 'warnings')
+    self.mox.StubOutWithMock(presubmit, 'warn')
     presubmit._ASKED_FOR_FEEDBACK = False
     self.fake_root_dir = self.RootDir()
     # Special mocks.
@@ -69,11 +69,11 @@ class PresubmitUnittest(PresubmitTestsBase):
       'NotImplementedException', 'OutputApi', 'ParseFiles',
       'PresubmitExecuter', 'PromptYesNo', 'ScanSubDirs',
       'SvnAffectedFile', 'SvnChange', 'cPickle', 'cStringIO',
-      'exceptions', 'fnmatch', 'gcl', 'gclient_utils', 'glob', 'json',
+      'exceptions', 'fnmatch', 'gclient_utils', 'glob', 'json',
       'logging', 'marshal', 'normpath', 'optparse', 'os', 'pickle',
       'presubmit_canned_checks', 'random', 're', 'scm', 'subprocess',
       'sys', 'tempfile', 'time', 'traceback', 'types', 'unittest', 'urllib2',
-      'warnings',
+      'warn',
     ]
     # If this test fails, you should add the relevant test.
     self.compareMembers(presubmit, members)
@@ -898,8 +898,7 @@ class InputApiUnittest(PresubmitTestsBase):
                        normpath(join(self.fake_root_dir, 'isdir', 'blat.cc')))
 
   def testDeprecated(self):
-    presubmit.warnings.warn(mox.IgnoreArg(), category=mox.IgnoreArg(),
-                            stacklevel=2)
+    presubmit.warn(mox.IgnoreArg(), category=mox.IgnoreArg(), stacklevel=2)
     self.mox.ReplayAll()
 
     change = presubmit.Change('mychange', '', self.fake_root_dir, [], 0, 0)

trychange.py

@@ -2,6 +2,7 @@
 # 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.
@@ -29,7 +30,7 @@ except ImportError:
     json = None
 
 try:
-  import breakpad
+  import breakpad  # pylint: disable=W0611
 except ImportError:
   pass
 
@@ -205,11 +206,9 @@ class SVN(SCM):
       return data
 
     # Try to search on the subversion repository for the file.
-    try:
-      from gcl import GetCachedFile
-    except ImportError:
+    if not gcl:
       return None
-    data = GetCachedFile(filename)
+    data = gcl.GetCachedFile(filename)
     logging.debug('%s:\n%s' % (filename, data))
     return data
 

watchlists.py

@@ -106,9 +106,11 @@ class Watchlists(object):
     for path in paths:
       path = path.replace(os.sep, '/')
       for name, rule in self._defns.iteritems():
-        if name not in self._watchlists: continue
+        if name not in self._watchlists:
+          continue
         rex_str = rule.get('filepath')
-        if not rex_str: continue
+        if not rex_str:
+          continue
         if re.search(rex_str, path):
           map(watchers.add, self._watchlists[name])
     return list(watchers)