Use pylint 2.7 for depot_tools

This includes a few fixes for specific errors, and disables several new
warnings introduced in this version, in order to allow for an incremental migration.

Bug:1262286
Change-Id: I4b8f8fc521386419a3121bbb07edc8ac83170a94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3413679
Reviewed-by: Josip Sokcevic <[email protected]>
Commit-Queue: Aravind Vasudevan <[email protected]>

Author: Aravind Vasudevan

PRESUBMIT.py

@@ -57,15 +57,27 @@ def CheckPylint(input_api, output_api):
       files_to_skip.extend([fnmatch.translate(l) for l in lines if
                          l and not l.startswith('#')])
   disabled_warnings = [
-    'R0401',  # Cyclic import
-    'W0613',  # Unused argument
+      'R0401',  # Cyclic import
+      'W0613',  # Unused argument
+      'C0415',  # import-outside-toplevel
+      'R1710',  # inconsistent-return-statements
+      'E1101',  # no-member
+      'E1120',  # no-value-for-parameter
+      'R1708',  # stop-iteration-return
+      'W1510',  # subprocess-run-check
+      # Checks which should be re-enabled after Python 2 support is removed.
+      'R0205',  # useless-object-inheritance
+      'R1725',  # super-with-arguments
+      'W0707',  # raise-missing-from
+      'W1113',  # keyword-arg-before-vararg
   ]
   return input_api.RunTests(input_api.canned_checks.GetPylint(
       input_api,
       output_api,
       files_to_check=files_to_check,
       files_to_skip=files_to_skip,
-      disabled_warnings=disabled_warnings))
+      disabled_warnings=disabled_warnings,
+      version='2.7'), parallel=False)
 
 
 def CheckRecipes(input_api, output_api):

autoninja.py

@@ -51,15 +51,15 @@
   elif arg.startswith('-C'):
     # Support -Cout/Default
     output_dir = arg[2:]
-  elif arg == '-o' or arg == '--offline':
+  elif arg in ('-o', '--offline'):
     offline = True
   elif arg == '-h':
     print('autoninja: Use -o/--offline to temporary disable goma.',
           file=sys.stderr)
     print(file=sys.stderr)
 
 # Strip -o/--offline so ninja doesn't see them.
-input_args = [ arg for arg in input_args if arg != '-o' and arg != '--offline']
+input_args = [ arg for arg in input_args if arg not in ('-o', '--offline')]
 
 use_goma = False
 use_remoteexec = False

cit.py

@@ -83,8 +83,8 @@ def is_exe(filename):
   """Given a full filepath, return true if the file is an executable."""
   if sys.platform.startswith('win'):
     return filename.endswith('.exe')
-  else:
-    return os.path.isfile(filename) and os.access(filename, os.X_OK)
+
+  return os.path.isfile(filename) and os.access(filename, os.X_OK)
 
 
 def get_available_tools():

clang_format_merge_driver.py

@@ -28,7 +28,9 @@ def main():
           sys.argv[0])
     sys.exit(1)
 
+  # pylint: disable=unbalanced-tuple-unpacking
   base, current, others, file_name_in_tree = sys.argv[1:5]
+  # pylint: enable=unbalanced-tuple-unpacking
 
   if file_name_in_tree == '%P':
     print(file=sys.stderr)

compile_single_file.py

@@ -23,7 +23,7 @@ def path_to_source_root(path):
   # to break when we rename directories.
   fingerprints = ['chrome', 'net', 'v8', 'build', 'skia']
   while candidate and not all(
-      [os.path.isdir(os.path.join(candidate, fp)) for fp in fingerprints]):
+      os.path.isdir(os.path.join(candidate, fp)) for fp in fingerprints):
     new_candidate = os.path.dirname(candidate)
     if new_candidate == candidate:
       raise Exception("Couldn't find source-dir from %s" % path)

cpplint.py

@@ -5263,8 +5263,8 @@ def ExpectingFunctionArgs(clean_lines, linenum):
 _re_pattern_headers_maybe_templates = []
 for _header, _templates in _HEADERS_MAYBE_TEMPLATES:
   for _template in _templates:
-    # Match max<type>(..., ...), max(..., ...), but not foo->max, foo.max or
-    # type::max().
+    # Match max<type>(..., ...), max(..., ...), but not foo->max, foo.max
+    # or type::max().
     _re_pattern_headers_maybe_templates.append(
         (re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'),
             _template,
@@ -5380,9 +5380,9 @@ def CheckForIncludeWhatYouUse(filename, clean_lines, include_state, error,
     io: The IO factory to use to read the header file. Provided for unittest
         injection.
   """
-  required = {}  # A map of header name to linenumber and the template entity.
-                 # Example of required: { '<functional>': (1219, 'less<>') }
-
+  # A map of header name to linenumber and the template entity.
+  # Example of required: { '<functional>': (1219, 'less<>') }
+  required = {}
   for linenum in range(clean_lines.NumLines()):
     line = clean_lines.elided[linenum]
     if not line or line[0] == '#':
@@ -5865,9 +5865,9 @@ def ProcessConfigOverrides(filename):
           elif name == 'linelength':
             global _line_length
             try:
-                _line_length = int(val)
+              _line_length = int(val)
             except ValueError:
-                sys.stderr.write('Line length must be numeric.')
+              sys.stderr.write('Line length must be numeric.')
           else:
             sys.stderr.write(
                 'Invalid configuration option (%s) in file %s\n' %
@@ -5881,7 +5881,7 @@ def ProcessConfigOverrides(filename):
   # Apply all the accumulated filters in reverse order (top-level directory
   # config options having the least priority).
   for filter in reversed(cfg_filters):
-     _AddFilters(filter)
+    _AddFilters(filter)
 
   return True
 
@@ -6053,15 +6053,15 @@ def ParseArguments(args):
     elif opt == '--linelength':
       global _line_length
       try:
-          _line_length = int(val)
+        _line_length = int(val)
       except ValueError:
-          PrintUsage('Line length must be digits.')
+        PrintUsage('Line length must be digits.')
     elif opt == '--extensions':
       global _valid_extensions
       try:
-          _valid_extensions = set(val.split(','))
+        _valid_extensions = set(val.split(','))
       except ValueError:
-          PrintUsage('Extensions must be comma separated list.')
+        PrintUsage('Extensions must be comma separated list.')
 
   if not filenames:
     PrintUsage('No files were specified.')

download_from_google_storage.py

@@ -44,9 +44,10 @@
     'aix7': 'aix',
 }
 
-
-class FileNotFoundError(IOError):
-  pass
+if sys.version_info.major == 2:
+  # pylint: disable=redefined-builtin
+  class FileNotFoundError(IOError):
+    pass
 
 
 class InvalidFileError(IOError):

fetch.py

@@ -56,7 +56,6 @@ def __init__(self, options, spec, root):
 
   def exists(self):
     """Check does this checkout already exist on desired location"""
-    pass
 
   def init(self):
     pass
@@ -67,18 +66,18 @@ def run(self, cmd, return_stdout=False, **kwargs):
       return ''
     if return_stdout:
       return subprocess.check_output(cmd, **kwargs).decode()
-    else:
-      try:
-        subprocess.check_call(cmd, **kwargs)
-      except subprocess.CalledProcessError as e:
-        # If the subprocess failed, it likely emitted its own distress message
-        # already - don't scroll that message off the screen with a stack trace
-        # from this program as well. Emit a terse message and bail out here;
-        # otherwise a later step will try doing more work and may hide the
-        # subprocess message.
-        print('Subprocess failed with return code %d.' % e.returncode)
-        sys.exit(e.returncode)
-      return ''
+
+    try:
+      subprocess.check_call(cmd, **kwargs)
+    except subprocess.CalledProcessError as e:
+      # If the subprocess failed, it likely emitted its own distress message
+      # already - don't scroll that message off the screen with a stack trace
+      # from this program as well. Emit a terse message and bail out here;
+      # otherwise a later step will try doing more work and may hide the
+      # subprocess message.
+      print('Subprocess failed with return code %d.' % e.returncode)
+      sys.exit(e.returncode)
+    return ''
 
 
 class GclientCheckout(Checkout):

gclient.py

@@ -238,7 +238,7 @@ def run(self):
         not gclient_eval.EvaluateCondition(self._condition, self._variables)):
       return
 
-    cmd = [arg for arg in self._action]
+    cmd = list(self._action)
 
     if cmd[0] == 'python':
       cmd[0] = 'vpython'
@@ -373,8 +373,8 @@ def condition(self):
   def target_os(self):
     if self.local_target_os is not None:
       return tuple(set(self.local_target_os).union(self.parent.target_os))
-    else:
-      return self.parent.target_os
+
+    return self.parent.target_os
 
   @property
   def target_cpu(self):
@@ -1420,7 +1420,7 @@ def __init__(self, root_dir, options):
     if 'all' in enforced_os:
       enforced_os = self.DEPS_OS_CHOICES.values()
     self._enforced_os = tuple(set(enforced_os))
-    self._enforced_cpu = detect_host_arch.HostArch(),
+    self._enforced_cpu = (detect_host_arch.HostArch(), )
     self._root_dir = root_dir
     self._cipd_root = None
     self.config_content = None
@@ -1830,7 +1830,7 @@ def RunOnDeps(self, command, args, ignore_requirements=False, progress=True):
     if command == 'update':
       gn_args_dep = self.dependencies[0]
       if gn_args_dep._gn_args_from:
-        deps_map = dict([(dep.name, dep) for dep in gn_args_dep.dependencies])
+        deps_map = {dep.name: dep for dep in gn_args_dep.dependencies}
         gn_args_dep = deps_map.get(gn_args_dep._gn_args_from)
       if gn_args_dep and gn_args_dep.HasGNArgsFile():
         gn_args_dep.WriteGNArgsFile()
@@ -1874,11 +1874,12 @@ def ShouldPrintRevision(dep):
         entries = {}
         def GrabDeps(dep):
           """Recursively grab dependencies."""
-          for d in dep.dependencies:
-            d.PinToActualRevision()
-            if ShouldPrintRevision(d):
-              entries[d.name] = d.url
-            GrabDeps(d)
+          for rec_d in dep.dependencies:
+            rec_d.PinToActualRevision()
+            if ShouldPrintRevision(rec_d):
+              entries[rec_d.name] = rec_d.url
+            GrabDeps(rec_d)
+
         GrabDeps(d)
         json_output.append({
             'name': d.name,
@@ -2275,7 +2276,7 @@ def _flatten_dep(self, dep):
       if key not in self._vars:
         continue
       # Don't "override" existing vars if it's actually the same value.
-      elif self._vars[key][1] == value:
+      if self._vars[key][1] == value:
         continue
       # Anything else is overriding a default value from the DEPS.
       self._vars[key] = (hierarchy + ' [custom_var override]', value)