Add support for automatically applying fixes am: dab680c723 am: b7b810ad5a am: 93bfe8098e

am: d7f8346125

Change-Id: I81ae436edfa08a4b17d64ec6585a8f9ce8179ade

4 files changed, 146 insertions, 9 deletions

diff --git a/pre-upload.py b/pre-upload.py
index 447b8bf..fe4cdd8 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -161,6 +161,37 @@ def _get_project_config():
     return config
 
 
+def _attempt_fixes(fixup_func_list, commit_list):
+    """Attempts to run |fixup_func_list| given |commit_list|."""
+    if len(fixup_func_list) != 1:
+        # Only single fixes will be attempted, since various fixes might
+        # interact with each other.
+        return
+
+    hook_name, commit, fixup_func = fixup_func_list[0]
+
+    if commit != commit_list[0]:
+        # If the commit is not at the top of the stack, git operations might be
+        # needed and might leave the working directory in a tricky state if the
+        # fix is attempted to run automatically (e.g. it might require manual
+        # merge conflict resolution). Refuse to run the fix in those cases.
+        return
+
+    prompt = ('An automatic fix can be attempted for the "%s" hook. '
+              'Do you want to run it?' % hook_name)
+    if not rh.terminal.boolean_prompt(prompt):
+        return
+
+    result = fixup_func()
+    if result:
+        print('Attempt to fix "%s" for commit "%s" failed: %s' %
+              (hook_name, commit, result),
+              file=sys.stderr)
+    else:
+        print('Fix successfully applied. Amend the current commit before '
+              'attempting to upload again.\n', file=sys.stderr)
+
+
 def _run_project_hooks(project_name, proj_dir=None,
                        commit_list=None):
     """For each project run its project specific hook from the hooks dictionary.
@@ -226,6 +257,7 @@ def _run_project_hooks(project_name, proj_dir=None,
             ignore_merged_commits=config.ignore_merged_commits)
 
     ret = True
+    fixup_func_list = []
 
     for commit in commit_list:
         # Mix in some settings for our hooks.
@@ -244,6 +276,13 @@ def _run_project_hooks(project_name, proj_dir=None,
             if error:
                 ret = False
                 output.hook_error(name, error)
+                for result in hook_results:
+                    if result.fixup_func:
+                        fixup_func_list.append((name, commit,
+                                                result.fixup_func))
+
+    if fixup_func_list:
+        _attempt_fixes(fixup_func_list, commit_list)
 
     output.finish()
     os.chdir(pwd)
diff --git a/rh/hooks.py b/rh/hooks.py
index 2c0dc1d..47926bc 100644
--- a/rh/hooks.py
+++ b/rh/hooks.py
@@ -250,10 +250,26 @@ def _get_build_os_name():
         return 'linux-x86'
 
 
-def _check_cmd(hook_name, project, commit, cmd, **kwargs):
+def _fixup_func_caller(cmd, **kwargs):
+    """Wraps |cmd| around a callable automated fixup.
+
+    For hooks that support automatically fixing errors after running (e.g. code
+    formatters), this function provides a way to run |cmd| as the |fixup_func|
+    parameter in HookCommandResult.
+    """
+    def wrapper():
+        result = _run_command(cmd, **kwargs)
+        if result.returncode not in (None, 0):
+            return result.output
+        return None
+    return wrapper
+
+
+def _check_cmd(hook_name, project, commit, cmd, fixup_func=None, **kwargs):
     """Runs |cmd| and returns its result as a HookCommandResult."""
     return [rh.results.HookCommandResult(hook_name, project, commit,
-                                         _run_command(cmd, **kwargs))]
+                                         _run_command(cmd, **kwargs),
+                                         fixup_func=fixup_func)]
 
 
 # Where helper programs exist.
@@ -284,10 +300,13 @@ def check_clang_format(project, commit, _desc, diff, options=None):
     tool = get_helper_path('clang-format.py')
     clang_format = options.tool_path('clang-format')
     git_clang_format = options.tool_path('git-clang-format')
-    cmd = ([tool, '--clang-format', clang_format, '--git-clang-format',
-            git_clang_format] +
-           options.args(('--style', 'file', '--commit', commit), diff))
-    return _check_cmd('clang-format', project, commit, cmd)
+    tool_args = (['--clang-format', clang_format, '--git-clang-format',
+                  git_clang_format] +
+                 options.args(('--style', 'file', '--commit', commit), diff))
+    cmd = [tool] + tool_args
+    fixup_func = _fixup_func_caller([tool, '--fix'] + tool_args)
+    return _check_cmd('clang-format', project, commit, cmd,
+                      fixup_func=fixup_func)
 
 
 def check_google_java_format(project, commit, _desc, _diff, options=None):
diff --git a/rh/results.py b/rh/results.py
index ecf7f50..8fa956f 100644
--- a/rh/results.py
+++ b/rh/results.py
@@ -29,12 +29,27 @@ del _path
 class HookResult(object):
     """A single hook result."""
 
-    def __init__(self, hook, project, commit, error, files=()):
+    def __init__(self, hook, project, commit, error, files=(), fixup_func=None):
+        """Initialize.
+
+        Args:
+          hook: The name of the hook.
+          project: The name of the project.
+          commit: The git commit sha.
+          error: A string representation of the hook's result.  Empty on
+              success.
+          files: The list of files that were involved in the hook execution.
+          fixup_func: A callable that will attempt to automatically fix errors
+              found in the hook's execution.  Returns an non-empty string if
+              this, too, fails.  Can be None if the hook does not support
+              automatically fixing errors.
+        """
         self.hook = hook
         self.project = project
         self.commit = commit
         self.error = error
         self.files = files
+        self.fixup_func = fixup_func
 
     def __bool__(self):
         return bool(self.error)
@@ -47,10 +62,11 @@ class HookResult(object):
 class HookCommandResult(HookResult):
     """A single hook result based on a CommandResult."""
 
-    def __init__(self, hook, project, commit, result, files=()):
+    def __init__(self, hook, project, commit, result, files=(),
+                 fixup_func=None):
         HookResult.__init__(self, hook, project, commit,
                             result.error if result.error else result.output,
-                            files=files)
+                            files=files, fixup_func=fixup_func)
         self.result = result
 
     def __bool__(self):
diff --git a/rh/terminal.py b/rh/terminal.py
index 49da992..9eda0ec 100644
--- a/rh/terminal.py
+++ b/rh/terminal.py
@@ -136,3 +136,66 @@ def print_status_line(line, print_newline=False):
 
     sys.stderr.write(output)
     sys.stderr.flush()
+
+
+def get_input(prompt):
+    """Python 2/3 glue for raw_input/input differences."""
+    try:
+        return raw_input(prompt)
+    except NameError:
+        # Python 3 renamed raw_input() to input(), which is safe to call since
+        # it does not evaluate the input.
+        # pylint: disable=bad-builtin
+        return input(prompt)
+
+
+def boolean_prompt(prompt='Do you want to continue?', default=True,
+                   true_value='yes', false_value='no', prolog=None):
+    """Helper function for processing boolean choice prompts.
+
+    Args:
+      prompt: The question to present to the user.
+      default: Boolean to return if the user just presses enter.
+      true_value: The text to display that represents a True returned.
+      false_value: The text to display that represents a False returned.
+      prolog: The text to display before prompt.
+
+    Returns:
+      True or False.
+    """
+    true_value, false_value = true_value.lower(), false_value.lower()
+    true_text, false_text = true_value, false_value
+    if true_value == false_value:
+        raise ValueError('true_value and false_value must differ: got %r'
+                         % true_value)
+
+    if default:
+        true_text = true_text[0].upper() + true_text[1:]
+    else:
+        false_text = false_text[0].upper() + false_text[1:]
+
+    prompt = ('\n%s (%s/%s)? ' % (prompt, true_text, false_text))
+
+    if prolog:
+        prompt = ('\n%s\n%s' % (prolog, prompt))
+
+    while True:
+        try:
+            response = get_input(prompt).lower()
+        except EOFError:
+            # If the user hits CTRL+D, or stdin is disabled, use the default.
+            print()
+            response = None
+        except KeyboardInterrupt:
+            # If the user hits CTRL+C, just exit the process.
+            print()
+            raise
+
+        if not response:
+            return default
+        if true_value.startswith(response):
+            if not false_value.startswith(response):
+                return True
+            # common prefix between the two...
+        elif false_value.startswith(response):
+            return False