[code-health] Migrate //buildtools/checkdeps to python3

Bug: 1211960
Test: Ran all scripts locally.
Change-Id: I7ff140ba5aeb4a6b013d392e3538467d5e03c0e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2983371
Commit-Queue: Fabrice de Gans <fdegans@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@google.com>
Cr-Commit-Position: refs/heads/master@{#895427}

buildtools/checkdeps/PRESUBMIT.py

@@ -5,6 +5,8 @@
 """Presubmit script for checkdeps tool.
 """
 
+USE_PYTHON3 = True
+
 
 def CheckChange(input_api, output_api):
   results = []

buildtools/checkdeps/builddeps.py

@@ -9,13 +9,12 @@
 See README.md for the format of the deps file.
 """
 
-from __future__ import print_function
+
 
 import copy
 import os.path
 import posixpath
 import subprocess
-import sys
 
 from rules import Rule, Rules
 
@@ -250,14 +249,12 @@ def Lookup(self, var_name):
     if os.path.isfile(deps_file_path) and not (
         self._under_test and
         os.path.basename(dir_path_local_abs) == 'checkdeps'):
-      if sys.version_info.major == 2:
-        execfile(deps_file_path, global_scope, local_scope)
-      else:
-        try:
-          exec(open(deps_file_path).read(), global_scope, local_scope)
-        except Exception as e:
-          print(' Error reading %s: %s' % (deps_file_path, str(e)))
-          raise
+      try:
+        with open(deps_file_path) as file:
+          exec(file.read(), global_scope, local_scope)
+      except Exception as e:
+        print(' Error reading %s: %s' % (deps_file_path, str(e)))
+        raise
     elif self.verbose:
       print('  No deps file found in', dir_path_local_abs)
 
@@ -311,7 +308,6 @@ def GetAllRulesAndFiles(self, dir_name=None):
         self._git_source_directories.update(_GitSourceDirectories(repo_path))
 
     # Collect a list of all files and directories to check.
-    files_to_check = []
     if dir_name and not os.path.isabs(dir_name):
       dir_name = os.path.join(self.base_directory, dir_name)
     dirs_to_check = [dir_name or self.base_directory]

buildtools/checkdeps/checkdeps.py

@@ -12,7 +12,7 @@
 See README.md for a detailed description of the DEPS format.
 """
 
-from __future__ import print_function
+
 
 import os
 import optparse
@@ -32,7 +32,7 @@ def _IsTestFile(filename):
   """Does a rudimentary check to try to skip test files; this could be
   improved but is good enough for now.
   """
-  return re.match('(test|mock|dummy)_.*|.*_[a-z]*test\.(cc|mm|java)', filename)
+  return re.match(r'(test|mock|dummy)_.*|.*_[a-z]*test\.(cc|mm|java)', filename)
 
 
 class DepsChecker(DepsBuilder):

buildtools/checkdeps/checkdeps_test.py

@@ -31,9 +31,9 @@ def ImplTestRegularCheckDepsRun(self, ignore_temp_rules, skip_tests):
 
     problems = self.deps_checker.results_formatter.GetResults()
     if skip_tests:
-      self.failUnlessEqual(4, len(problems))
+      self.assertEqual(4, len(problems))
     else:
-      self.failUnlessEqual(5, len(problems))
+      self.assertEqual(5, len(problems))
 
     def VerifySubstringsInProblems(key_path, substrings_in_sequence):
       """Finds the problem in |problems| that contains |key_path|,
@@ -48,7 +48,7 @@ def VerifySubstringsInProblems(key_path, substrings_in_sequence):
         if index != -1:
           for substring in substrings_in_sequence:
             index = problem.find(substring, index + 1)
-            self.failUnless(index != -1, '%s in %s' % (substring, problem))
+            self.assertTrue(index != -1, '%s in %s' % (substring, problem))
           found = True
           break
       if not found:
@@ -103,28 +103,28 @@ def CountViolations(self, ignore_temp_rules):
     return self.deps_checker.results_formatter.GetResults()
 
   def testCountViolations(self):
-    self.failUnlessEqual('11', self.CountViolations(False))
+    self.assertEqual('11', self.CountViolations(False))
 
   def testCountViolationsIgnoringTempRules(self):
-    self.failUnlessEqual('12', self.CountViolations(True))
+    self.assertEqual('12', self.CountViolations(True))
 
   def testCountViolationsWithRelativePath(self):
     self.deps_checker.results_formatter = results.CountViolationsFormatter()
     self.deps_checker.CheckDirectory(
         os.path.join('buildtools', 'checkdeps', 'testdata', 'allowed'))
-    self.failUnlessEqual('4', self.deps_checker.results_formatter.GetResults())
+    self.assertEqual('4', self.deps_checker.results_formatter.GetResults())
 
   def testTempRulesGenerator(self):
     self.deps_checker.results_formatter = results.TemporaryRulesFormatter()
     self.deps_checker.CheckDirectory(
         os.path.join(self.deps_checker.base_directory,
                      'buildtools/checkdeps/testdata/allowed'))
     temp_rules = self.deps_checker.results_formatter.GetResults()
-    expected = [u'  "!buildtools/checkdeps/testdata/disallowed/bad.h",',
-                u'  "!buildtools/checkdeps/testdata/disallowed/teststuff/bad.h",',
-                u'  "!third_party/explicitly_disallowed/bad.h",',
-                u'  "!third_party/no_rule/bad.h",']
-    self.failUnlessEqual(expected, temp_rules)
+    expected = ['  "!buildtools/checkdeps/testdata/disallowed/bad.h",',
+                '  "!buildtools/checkdeps/testdata/disallowed/teststuff/bad.h",',
+                '  "!third_party/explicitly_disallowed/bad.h",',
+                '  "!third_party/no_rule/bad.h",']
+    self.assertEqual(expected, temp_rules)
 
   def testBadBaseDirectoryNotCheckoutRoot(self):
     # This assumes git. It's not a valid test if buildtools is fetched via svn.
@@ -138,34 +138,34 @@ def testCheckAddedIncludesAllGood(self):
         ['#include "buildtools/checkdeps/testdata/allowed/good.h"',
          '#include "buildtools/checkdeps/testdata/disallowed/allowed/good.h"']
       ]])
-    self.failIf(problems)
+    self.assertFalse(problems)
 
   def testCheckAddedIncludesManyGarbageLines(self):
     garbage_lines = ["My name is Sam%d\n" % num for num in range(50)]
     problems = self.deps_checker.CheckAddedCppIncludes(
       [['buildtools/checkdeps/testdata/allowed/test.cc', garbage_lines]])
-    self.failIf(problems)
+    self.assertFalse(problems)
 
   def testCheckAddedIncludesNoRule(self):
     problems = self.deps_checker.CheckAddedCppIncludes(
       [['buildtools/checkdeps/testdata/allowed/test.cc',
         ['#include "no_rule_for_this/nogood.h"']
       ]])
-    self.failUnless(problems)
+    self.assertTrue(problems)
 
   def testCheckAddedIncludesSkippedDirectory(self):
     problems = self.deps_checker.CheckAddedCppIncludes(
       [['buildtools/checkdeps/testdata/disallowed/allowed/skipped/test.cc',
         ['#include "whatever/whocares.h"']
       ]])
-    self.failIf(problems)
+    self.assertFalse(problems)
 
   def testCheckAddedIncludesTempAllowed(self):
     problems = self.deps_checker.CheckAddedCppIncludes(
       [['buildtools/checkdeps/testdata/allowed/test.cc',
         ['#include "buildtools/checkdeps/testdata/disallowed/temporarily_allowed.h"']
       ]])
-    self.failUnless(problems)
+    self.assertTrue(problems)
 
   def testCopyIsDeep(self):
     # Regression test for a bug where we were making shallow copies of
@@ -187,55 +187,55 @@ def testCopyIsDeep(self):
        ]])
     # With the bug in place, there would be two problems reported, and
     # the second would be for foo_unittest.cc.
-    self.failUnless(len(problems) == 1)
-    self.failUnless(problems[0][0].endswith('/test.cc'))
+    self.assertTrue(len(problems) == 1)
+    self.assertTrue(problems[0][0].endswith('/test.cc'))
 
   def testTraversalIsOrdered(self):
     dirs_traversed = []
     for rules, filenames in self.deps_checker.GetAllRulesAndFiles(dir_name='buildtools'):
-      self.failUnlessEqual(type(filenames), list)
-      self.failUnlessEqual(filenames, sorted(filenames))
+      self.assertEqual(type(filenames), list)
+      self.assertEqual(filenames, sorted(filenames))
       if filenames:
         dir_names = set(os.path.dirname(file) for file in filenames)
-        self.failUnlessEqual(1, len(dir_names))
+        self.assertEqual(1, len(dir_names))
         dirs_traversed.append(dir_names.pop())
-    self.failUnlessEqual(dirs_traversed, sorted(dirs_traversed))
+    self.assertEqual(dirs_traversed, sorted(dirs_traversed))
 
   def testCheckPartialImportsAreAllowed(self):
     problems = self.deps_checker.CheckAddedProtoImports(
       [['buildtools/checkdeps/testdata/test.proto',
         ['import "no_rule_for_this/nogood.proto"']
       ]])
-    self.failIf(problems)
+    self.assertFalse(problems)
 
   def testCheckAddedFullPathImportsAllowed(self):
     problems = self.deps_checker.CheckAddedProtoImports(
       [['buildtools/checkdeps/testdata/test.proto',
         ['import "buildtools/checkdeps/testdata/allowed/good.proto"',
          'import "buildtools/checkdeps/testdata/disallowed/sub_folder/good.proto"']
       ]])
-    self.failIf(problems)
+    self.assertFalse(problems)
 
   def testCheckAddedFullPathImportsDisallowed(self):
     problems = self.deps_checker.CheckAddedProtoImports(
       [['buildtools/checkdeps/testdata/test.proto',
         ['import "buildtools/checkdeps/testdata/disallowed/bad.proto"']
       ]])
-    self.failUnless(problems)
+    self.assertTrue(problems)
 
   def testCheckAddedFullPathImportsManyGarbageLines(self):
     garbage_lines = ["My name is Sam%d\n" % num for num in range(50)]
     problems = self.deps_checker.CheckAddedProtoImports(
       [['buildtools/checkdeps/testdata/test.proto',
         garbage_lines]])
-    self.failIf(problems)
+    self.assertFalse(problems)
 
   def testCheckAddedIncludesNoRuleFullPath(self):
     problems = self.deps_checker.CheckAddedProtoImports(
       [['buildtools/checkdeps/testdata/test.proto',
         ['import "tools/some.proto"']
       ]])
-    self.failUnless(problems)
+    self.assertTrue(problems)
 
 if __name__ == '__main__':
   unittest.main()

buildtools/checkdeps/cpp_checker.py

@@ -4,7 +4,7 @@
 
 """Checks C++ and Objective-C files for illegal includes."""
 
-from __future__ import print_function
+
 
 import codecs
 import os
@@ -34,7 +34,7 @@ class CppChecker(object):
   # This regular expression will be used to extract filenames from include
   # statements.
   _EXTRACT_INCLUDE_PATH = re.compile(
-      '[ \t]*#[ \t]*(?:include|import)[ \t]+"(.*)"')
+      r'[ \t]*#[ \t]*(?:include|import)[ \t]+"(.*)"')
 
   def __init__(self, verbose, resolve_dotdot=False, root_dir=''):
     self._verbose = verbose

buildtools/checkdeps/graphdeps.py

@@ -233,7 +233,7 @@ def _DumpDependenciesImpl(self, deps, out):
 
     # Reformat the computed raw node attributes into a final DOT representation.
     nodes = []
-    for (node, attrs) in node_props.iteritems():
+    for (node, attrs) in node_props.items():
       attr_strs = []
       if attrs['hilite']:
         attr_strs.append('style=filled,fillcolor=%s' % attrs['hilite'])
@@ -253,7 +253,7 @@ def _DumpDependenciesImpl(self, deps, out):
 
 
 def PrintUsage():
-  print """Usage: python graphdeps.py [--root <root>]
+  print("""Usage: python graphdeps.py [--root <root>]
 
   --root ROOT Specifies the repository root. This defaults to "../../.."
               relative to the script file. This will be correct given the
@@ -280,7 +280,7 @@ def PrintUsage():
       --excl='.*->third_party' \
       --fanin='^(apps|content/browser/renderer_host)$' \
       --ignore-specific-rules \
-      --ignore-temp-rules"""
+      --ignore-temp-rules""")
 
 
 def main():
@@ -392,11 +392,11 @@ def main():
     PrintUsage()
     return 1
 
-  print 'Using base directory: ', deps_grapher.base_directory
-  print 'include nodes       : ', options.incl
-  print 'exclude nodes       : ', options.excl
-  print 'highlight fanins of : ', options.hilite_fanins
-  print 'highlight fanouts of: ', options.hilite_fanouts
+  print('Using base directory: ', deps_grapher.base_directory)
+  print('include nodes       : ', options.incl)
+  print('exclude nodes       : ', options.excl)
+  print('highlight fanins of : ', options.hilite_fanins)
+  print('highlight fanouts of: ', options.hilite_fanouts)
 
   deps_grapher.DumpDependencies()
   return 0

buildtools/checkdeps/java_checker.py

@@ -4,7 +4,7 @@
 
 """Checks Java files for illegal imports."""
 
-from __future__ import print_function
+
 
 import codecs
 import os

buildtools/checkdeps/proto_checker.py

@@ -4,7 +4,7 @@
 
 """Checks protobuf files for illegal imports."""
 
-from __future__ import print_function
+
 
 import codecs
 import os
@@ -30,7 +30,7 @@ class ProtoChecker(object):
   # This regular expression will be used to extract filenames from import
   # statements.
   _EXTRACT_IMPORT_PATH = re.compile(
-      '[ \t]*[ \t]*import[ \t]+"(.*)"')
+      r'[ \t]*[ \t]*import[ \t]+"(.*)"')
 
   def __init__(self, verbose, resolve_dotdot=False, root_dir=''):
     self._verbose = verbose
@@ -39,7 +39,7 @@ def __init__(self, verbose, resolve_dotdot=False, root_dir=''):
 
   def IsFullPath(self, import_path):
     """Checks if the given path is a valid path starting from |_root_dir|."""
-    match = re.match('(.*)/([^/]*\.proto)', import_path)
+    match = re.match(r'(.*)/([^/]*\.proto)', import_path)
     if not match:
       return False
     return os.path.isdir(self._root_dir + "/" + match.group(1))

buildtools/checkdeps/results.py

@@ -4,7 +4,7 @@
 
 """Results object and results formatters for checkdeps tool."""
 
-from __future__ import print_function
+
 
 import json
 

buildtools/checkdeps/rules.py

@@ -112,7 +112,7 @@ def __init__(self):
   def __str__(self):
     result = ['Rules = {\n    (apply to all files): [\n%s\n    ],' % '\n'.join(
         '      %s' % x for x in self._general_rules)]
-    for regexp, rules in self._specific_rules.items():
+    for regexp, rules in list(self._specific_rules.items()):
       result.append('    (limited to files matching %s): [\n%s\n    ]' % (
           regexp, '\n'.join('      %s' % x for x in rules)))
     result.append('  }')
@@ -132,7 +132,7 @@ def AddDependencyTuplesImpl(deps, rules, extra_dependent_suffix=""):
     if include_general_rules:
       AddDependencyTuplesImpl(deps, self._general_rules)
     if include_specific_rules:
-      for regexp, rules in self._specific_rules.items():
+      for regexp, rules in list(self._specific_rules.items()):
         AddDependencyTuplesImpl(deps, rules, "/" + regexp)
     return deps
 
@@ -175,7 +175,7 @@ def RuleApplyingTo(self, include_path, dependee_path):
     file located at |dependee_path|.
     """
     dependee_filename = os.path.basename(dependee_path)
-    for regexp, specific_rules in self._specific_rules.items():
+    for regexp, specific_rules in list(self._specific_rules.items()):
       if re.match(regexp, dependee_filename):
         for rule in specific_rules:
           if rule.ChildOrMatch(include_path):