Merge 7cea42adb477b049f024752b9b67b3a09e78244c on remote branch

Change-Id: Iaa7c1e679642899822752d4bb847e8c235311545
diff --git a/README.md b/README.md
index dd6ffc6..1954d0a 100644
--- a/README.md
+++ b/README.md
@@ -170,6 +170,9 @@
 This section allows for turning on common/builtin hooks.  There are a bunch of
 canned hooks already included geared towards AOSP style guidelines.
 
+* `aidl_format`: Run AIDL files (.aidl) through `aidl-format`.
+* `android_test_mapping_format`: Validate TEST_MAPPING files in Android source
+  code. Refer to go/test-mapping for more details.
 * `bpfmt`: Run Blueprint files (.bp) through `bpfmt`.
 * `checkpatch`: Run commits through the Linux kernel's `checkpatch.pl` script.
 * `clang_format`: Run git-clang-format against the commit. The default style is
@@ -195,8 +198,6 @@
 * `pylint3`: Run Python code through `pylint` using Python 3.
 * `rustfmt`: Run Rust code through `rustfmt`.
 * `xmllint`: Run XML code through `xmllint`.
-* `android_test_mapping_format`: Validate TEST_MAPPING files in Android source
-  code. Refer to go/test-mapping for more details.
 
 Note: Builtin hooks tend to match specific filenames (e.g. `.json`).  If no
 files match in a specific commit, then the hook will be skipped for that commit.
@@ -263,6 +264,9 @@
 provide consistent behavior for developers across different OS and Linux
 distros/versions.  The following tools are recognized:
 
+* `aidl-format`: used for the `aidl_format` builtin hook.
+* `android-test-mapping-format`: used for the `android_test_mapping_format`
+  builtin hook.
 * `bpfmt`: used for the `bpfmt` builtin hook.
 * `clang-format`: used for the `clang_format` builtin hook.
 * `cpplint`: used for the `cpplint` builtin hook.
@@ -272,8 +276,6 @@
 * `google-java-format-diff`: used for the `google_java_format` builtin hook.
 * `pylint`: used for the `pylint` builtin hook.
 * `rustfmt`: used for the `rustfmt` builtin hook.
-* `android-test-mapping-format`: used for the `android_test_mapping_format`
-  builtin hook.
 
 See [Placeholders](#Placeholders) for variables you can expand automatically.
 
diff --git a/pre-upload.py b/pre-upload.py
index 0e032f4..7eb11b8 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -26,8 +26,8 @@
 
 
 # Assert some minimum Python versions as we don't test or support any others.
-if sys.version_info < (3, 5):
-    print('repohooks: error: Python-3.5+ is required', file=sys.stderr)
+if sys.version_info < (3, 6):
+    print('repohooks: error: Python-3.6+ is required', file=sys.stderr)
     sys.exit(1)
 
 
diff --git a/rh/config.py b/rh/config.py
index ed75007..2c7b4af 100644
--- a/rh/config.py
+++ b/rh/config.py
@@ -63,21 +63,9 @@
                 return default
             raise
 
-    def get(self, section, option, default=_UNSET):
-        """Return the value for |option| in |section| (with |default|)."""
-        try:
-            return configparser.RawConfigParser.get(self, section, option)
-        except (configparser.NoSectionError, configparser.NoOptionError):
-            if default is not _UNSET:
-                return default
-            raise
-
     def items(self, section=_UNSET, default=_UNSET):
         """Return a list of (key, value) tuples for the options in |section|."""
         if section is _UNSET:
-            # Python 3 compat logic.  Return a dict of section-to-options.
-            if sys.version_info.major < 3:
-                return [(x, self.items(x)) for x in self.sections()]
             return super(RawConfigParser, self).items()
 
         try:
@@ -87,15 +75,6 @@
                 return default
             raise
 
-    if sys.version_info.major < 3:
-        def read_dict(self, dictionary):
-            """Store |dictionary| into ourselves."""
-            for section, settings in dictionary.items():
-                for option, value in settings:
-                    if not self.has_section(section):
-                        self.add_section(section)
-                    self.set(section, option, value)
-
 
 class PreUploadConfig(object):
     """A single (abstract) config used for `repo upload` hooks."""
@@ -138,7 +117,8 @@
 
     def custom_hook(self, hook):
         """The command to execute for |hook|."""
-        return shlex.split(self.config.get(self.CUSTOM_HOOKS_SECTION, hook, ''))
+        return shlex.split(self.config.get(
+            self.CUSTOM_HOOKS_SECTION, hook, fallback=''))
 
     @property
     def builtin_hooks(self):
@@ -148,13 +128,13 @@
 
     def builtin_hook_option(self, hook):
         """The options to pass to |hook|."""
-        return shlex.split(self.config.get(self.BUILTIN_HOOKS_OPTIONS_SECTION,
-                                           hook, ''))
+        return shlex.split(self.config.get(
+            self.BUILTIN_HOOKS_OPTIONS_SECTION, hook, fallback=''))
 
     def builtin_hook_exclude_paths(self, hook):
         """List of paths for which |hook| should not be executed."""
-        return shlex.split(self.config.get(self.BUILTIN_HOOKS_EXCLUDE_SECTION,
-                                           hook, ''))
+        return shlex.split(self.config.get(
+            self.BUILTIN_HOOKS_EXCLUDE_SECTION, hook, fallback=''))
 
     @property
     def tool_paths(self):
@@ -186,7 +166,7 @@
         """Whether to skip hooks for merged commits."""
         return rh.shell.boolean_shell_value(
             self.config.get(self.OPTIONS_SECTION,
-                            self.OPTION_IGNORE_MERGED_COMMITS, None),
+                            self.OPTION_IGNORE_MERGED_COMMITS, fallback=None),
             False)
 
     def update(self, preupload_config):
diff --git a/rh/hooks.py b/rh/hooks.py
index 8cea251..0b3bb29 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -974,9 +974,30 @@
     return _check_cmd('android-test-mapping-format', project, commit, cmd)
 
 
+def check_aidl_format(project, commit, _desc, diff, options=None):
+    """Checks that AIDL files are formatted with aidl-format."""
+    # All *.aidl files except for those under aidl_api directory.
+    filtered = _filter_diff(diff, [r'\.aidl$'], [r'/aidl_api/'])
+    if not filtered:
+        return None
+    aidl_format = options.tool_path('aidl-format')
+    cmd = [aidl_format, '-d'] + options.args((), filtered)
+    ret = []
+    for d in filtered:
+        data = rh.git.get_file_content(commit, d.file)
+        result = _run(cmd, input=data)
+        if result.stdout:
+            fixup_func = _fixup_func_caller([aidl_format, '-w', d.file])
+            ret.append(rh.results.HookResult(
+                'aidl-format', project, commit, error=result.stdout,
+                files=(d.file,), fixup_func=fixup_func))
+    return ret
+
+
 # Hooks that projects can opt into.
 # Note: Make sure to keep the top level README.md up to date when adding more!
 BUILTIN_HOOKS = {
+    'aidl_format': check_aidl_format,
     'android_test_mapping_format': check_android_test_mapping,
     'bpfmt': check_bpfmt,
     'checkpatch': check_checkpatch,
@@ -1002,6 +1023,7 @@
 # Additional tools that the hooks can call with their default values.
 # Note: Make sure to keep the top level README.md up to date when adding more!
 TOOL_PATHS = {
+    'aidl-format': 'aidl-format',
     'android-test-mapping-format':
         os.path.join(TOOLS_DIR, 'android_test_mapping_format.py'),
     'bpfmt': 'bpfmt',
diff --git a/rh/hooks_unittest.py b/rh/hooks_unittest.py
index 716e1da..8466319 100755
--- a/rh/hooks_unittest.py
+++ b/rh/hooks_unittest.py
@@ -823,6 +823,20 @@
             self.project, 'commit', 'desc', diff, options=self.options)
         self.assertIsNotNone(ret)
 
+    def test_aidl_format(self, mock_check, _mock_run):
+        """Verify the aidl_format builtin hook."""
+        # First call should do nothing as there are no files to check.
+        ret = rh.hooks.check_aidl_format(
+            self.project, 'commit', 'desc', (), options=self.options)
+        self.assertIsNone(ret)
+        self.assertFalse(mock_check.called)
+
+        # Second call will have some results.
+        diff = [rh.git.RawDiffEntry(file='IFoo.go')]
+        ret = rh.hooks.check_gofmt(
+            self.project, 'commit', 'desc', diff, options=self.options)
+        self.assertIsNotNone(ret)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/rh/utils.py b/rh/utils.py
index 1b36c7a..52a6828 100644
--- a/rh/utils.py
+++ b/rh/utils.py
@@ -183,12 +183,8 @@
             print('Ignoring unhandled exception in _kill_child_process: %s' % e,
                   file=sys.stderr)
 
-        # Ensure our child process has been reaped.
-        kwargs = {}
-        if sys.version_info.major >= 3:
-            # ... but don't wait forever.
-            kwargs['timeout'] = 60
-        proc.wait_lock_breaker(**kwargs)
+        # Ensure our child process has been reaped, but don't wait forever.
+        proc.wait_lock_breaker(timeout=60)
 
     if not rh.signals.relay_signal(original_handler, signum, frame):
         # Mock up our own, matching exit code for signaling.
@@ -310,13 +306,8 @@
     kill_timeout = float(kill_timeout)
 
     def _get_tempfile():
-        kwargs = {}
-        if sys.version_info.major < 3:
-            kwargs['bufsize'] = 0
-        else:
-            kwargs['buffering'] = 0
         try:
-            return tempfile.TemporaryFile(**kwargs)
+            return tempfile.TemporaryFile(buffering=0)
         except EnvironmentError as e:
             if e.errno != errno.ENOENT:
                 raise
@@ -325,7 +316,7 @@
             # issue in this particular case since our usage gurantees deletion,
             # and since this is primarily triggered during hard cgroups
             # shutdown.
-            return tempfile.TemporaryFile(dir='/tmp', **kwargs)
+            return tempfile.TemporaryFile(dir='/tmp', buffering=0)
 
     # Modify defaults based on parameters.
     # Note that tempfiles must be unbuffered else attempts to read
diff --git a/tools/android_test_mapping_format.py b/tools/android_test_mapping_format.py
index b87b886..1e654e6 100755
--- a/tools/android_test_mapping_format.py
+++ b/tools/android_test_mapping_format.py
@@ -53,13 +53,6 @@
 _COMMENTS_RE = re.compile(r'^\s*//')
 
 
-if sys.version_info.major < 3:
-    # pylint: disable=basestring-builtin,undefined-variable
-    string_types = basestring
-else:
-    string_types = str
-
-
 class Error(Exception):
     """Base exception for all custom exceptions in this module."""
 
@@ -125,14 +118,14 @@
             'Failed test config: %s' % (test_mapping_file, test))
     preferred_targets = test.get(PREFERRED_TARGETS, [])
     if (not isinstance(preferred_targets, list) or
-            any(not isinstance(t, string_types) for t in preferred_targets)):
+            any(not isinstance(t, str) for t in preferred_targets)):
         raise InvalidTestMappingError(
             'Invalid test config in test mapping file %s. `preferred_targets` '
             'setting in test config can only be a list of strings. Failed test '
             'config: %s' % (test_mapping_file, test))
     file_patterns = test.get(FILE_PATTERNS, [])
     if (not isinstance(file_patterns, list) or
-            any(not isinstance(p, string_types) for p in file_patterns)):
+            any(not isinstance(p, str) for p in file_patterns)):
         raise InvalidTestMappingError(
             'Invalid test config in test mapping file %s. `file_patterns` '
             'setting in test config can only be a list of strings. Failed test '
diff --git a/tools/pylint.py b/tools/pylint.py
index 60f4b67..570f055 100755
--- a/tools/pylint.py
+++ b/tools/pylint.py
@@ -23,8 +23,8 @@
 import subprocess
 
 
-assert (sys.version_info.major, sys.version_info.minor) >= (3, 5), (
-    'Python 3.5 or newer is required; found %s' % (sys.version,))
+assert (sys.version_info.major, sys.version_info.minor) >= (3, 6), (
+    'Python 3.6 or newer is required; found %s' % (sys.version,))
 
 
 DEFAULT_PYLINTRC_PATH = os.path.join(