Only apply Google Java format to changed lines (#176)

* Only apply Google Java format to changed lines

* Only apply Google Java format to changed lines

* Only apply Google Java format to changed regions

Diffs are relative to origin/master.

Three tasks are added:
- javaIncrementalFormatCheck is added to the build workflow, and
  will abort build if format violations are found.
- javaIncrementalFormatApply needs to be manually invoked to correct
  format violations, the same behavior as spotlessApply.
- javaIncrementalFormatDryRun shows the changes that would happen if
  javaIncrementalFormatApply is invoked.

These tasks work from the root directory and process the buildSrc directory
too.

The Spotless Java config is removed.

* Only apply Google Java format to changed regions

Diffs are relative to origin/master.

Three tasks are added:
- javaIncrementalFormatCheck is added to the build workflow, and
  will abort build if format violations are found.
- javaIncrementalFormatApply needs to be manually invoked to correct
  format violations, the same behavior as spotlessApply.
- javaIncrementalFormatDryRun shows the changes that would happen if
  javaIncrementalFormatApply is invoked.

These tasks work from the root directory and process the buildSrc directory
too.

The Spotless Java config is removed.

* Only apply Google Java format to changed regions

Diffs are relative to origin/master.

Three tasks are added:
- javaIncrementalFormatCheck is added to the build workflow, and
  will abort build if format violations are found.
- javaIncrementalFormatApply needs to be manually invoked to correct
  format violations, the same behavior as spotlessApply.
- javaIncrementalFormatDryRun shows the changes that would happen if
  javaIncrementalFormatApply is invoked.

These tasks work from the root directory and process the buildSrc directory
too.

The Spotless Java config is removed.

* Only apply Google Java format to changed regions

Diffs are relative to origin/master.

Three tasks are added:
- javaIncrementalFormatCheck is added to the build workflow, and
  will abort build if format violations are found.
- javaIncrementalFormatApply needs to be manually invoked to correct
  format violations, the same behavior as spotlessApply.
- javaIncrementalFormatDryRun shows the changes that would happen if
  javaIncrementalFormatApply is invoked.

These tasks work from the root directory and process the buildSrc directory
too.

The Spotless Java config is removed.

build.gradle

@@ -333,3 +333,69 @@ def createGetBuildSrcDirectDepsTask(outputFileName) {
                 "-PdependencyExportFile=${outputFileName}"
   }
 }
+
+rootProject.ext {
+
+  // Executes an arbitrary shell command in bash and returns all output
+  // to stdout as a string. This method allows pipes in shell command.
+  execInBash = { shellCommand, bashWorkingDir ->
+    return new ByteArrayOutputStream().withStream { os ->
+      exec {
+        workingDir bashWorkingDir
+        commandLine 'bash', '-c', "${shellCommand}"
+        standardOutput os
+      }
+      return os
+    }.toString().trim()
+  }
+
+  invokeJavaDiffFormatScript = { action ->
+    def scriptDir = rootDir.path.endsWith('buildSrc')
+            ? "${rootDir}/../java-format"
+            : "${rootDir}/java-format"
+    def workingDir = rootDir.path.endsWith('buildSrc')
+            ? "${rootDir}/.."
+            : rootDir
+    def formatDiffScript = "${scriptDir}/google-java-format-git-diff.sh"
+
+    return ext.execInBash(
+        "${formatDiffScript} ${action}", "${workingDir}")
+  }
+}
+
+// Checks if modified lines in Java source files need reformatting.
+// Note that this task checks modified Java files in the entire repository.
+task javaIncrementalFormatCheck {
+  doLast {
+    def checkResult = invokeJavaDiffFormatScript("check")
+    if (checkResult == 'true') {
+      throw new IllegalStateException(
+          "Some Java files need to be reformatted. You may use the "
+              + "'javaIncrementalFormatDryRun' task to review\n "
+              + "the changes, or the 'javaIncrementalFormatApply' task "
+              + "to reformat.")
+    } else if (checkResult != 'false') {
+      throw new RuntimeException(
+          "Failed to invoke format check script:\n" + checkResult)
+    }
+    println("Incremental Java format check ok.")
+  }
+}
+
+// Shows how modified lines in Java source files will change after formatting.
+// Note that this task checks modified Java files in the entire repository.
+task javaIncrementalFormatDryRun {
+  doLast {
+    println("${invokeJavaDiffFormatScript("show")}")
+  }
+}
+
+// Checks if modified lines in Java source files need reformatting.
+// Note that this task processes modified Java files in the entire repository.
+task javaIncrementalFormatApply {
+  doLast {
+    invokeJavaDiffFormatScript("format")
+  }
+}
+
+tasks.build.dependsOn(tasks.javaIncrementalFormatCheck)

config/presubmits.py

@@ -78,7 +78,8 @@ PRESUBMITS = {
         r".*Copyright 20\d{2} The Nomulus Authors\. All Rights Reserved\.",
         ("java", "js", "soy", "sql", "py", "sh", "gradle"), {
             ".git", "/build/", "/generated/", "node_modules/",
-            "JUnitBackports.java", "registrar_bin.", "registrar_dbg."
+            "JUnitBackports.java", "registrar_bin.", "registrar_dbg.",
+            "google-java-format-diff.py"
         }, REQUIRED):
         "File did not include the license header.",
 

java-format/google-java-format-diff.py

@@ -0,0 +1,130 @@
+#!/usr/bin/env python2.7
+#
+#===- google-java-format-diff.py - google-java-format Diff Reformatter -----===#
+#
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+#===------------------------------------------------------------------------===#
+
+"""
+google-java-format Diff Reformatter
+============================
+This script reads input from a unified diff and reformats all the changed
+lines. This is useful to reformat all the lines touched by a specific patch.
+Example usage for git/svn users:
+  git diff -U0 HEAD^ | google-java-format-diff.py -p1 -i
+  svn diff --diff-cmd=diff -x-U0 | google-java-format-diff.py -i
+For perforce users:
+  P4DIFF="git --no-pager diff --no-index" p4 diff | ./google-java-format-diff.py -i -p7
+"""
+
+import argparse
+import difflib
+import re
+import string
+import subprocess
+import StringIO
+import sys
+from distutils.spawn import find_executable
+
+def main():
+  parser = argparse.ArgumentParser(description=
+                                   'Reformat changed lines in diff. Without -i '
+                                   'option just output the diff that would be '
+                                   'introduced.')
+  parser.add_argument('-i', action='store_true', default=False,
+                      help='apply edits to files instead of displaying a diff')
+
+  parser.add_argument('-p', metavar='NUM', default=0,
+                      help='strip the smallest prefix containing P slashes')
+  parser.add_argument('-regex', metavar='PATTERN', default=None,
+                      help='custom pattern selecting file paths to reformat '
+                      '(case sensitive, overrides -iregex)')
+  parser.add_argument('-iregex', metavar='PATTERN', default=r'.*\.java',
+                      help='custom pattern selecting file paths to reformat '
+                      '(case insensitive, overridden by -regex)')
+  parser.add_argument('-v', '--verbose', action='store_true',
+                      help='be more verbose, ineffective without -i')
+  parser.add_argument('-a', '--aosp', action='store_true',
+                      help='use AOSP style instead of Google Style (4-space indentation)')
+  parser.add_argument('--skip-sorting-imports', action='store_true',
+                      help='do not fix the import order')
+  parser.add_argument('-b', '--binary', help='path to google-java-format binary')
+  parser.add_argument('--google-java-format-jar', metavar='ABSOLUTE_PATH', default=None,
+                      help='use a custom google-java-format jar')
+
+  args = parser.parse_args()
+
+  # Extract changed lines for each file.
+  filename = None
+  lines_by_file = {}
+
+  for line in sys.stdin:
+    match = re.search('^\+\+\+\ (.*?/){%s}(\S*)' % args.p, line)
+    if match:
+      filename = match.group(2)
+    if filename is None:
+      continue
+
+    if args.regex is not None:
+      if not re.match('^%s$' % args.regex, filename):
+        continue
+    else:
+      if not re.match('^%s$' % args.iregex, filename, re.IGNORECASE):
+        continue
+
+    match = re.search('^@@.*\+(\d+)(,(\d+))?', line)
+    if match:
+      start_line = int(match.group(1))
+      line_count = 1
+      if match.group(3):
+        line_count = int(match.group(3))
+      if line_count == 0:
+        continue
+      end_line = start_line + line_count - 1;
+      lines_by_file.setdefault(filename, []).extend(
+          ['-lines', str(start_line) + ':' + str(end_line)])
+
+  if args.binary:
+    base_command = [args.binary]
+  elif args.google_java_format_jar:
+    base_command = ['java', '-jar', args.google_java_format_jar]
+  else:
+    binary = find_executable('google-java-format') or '/usr/bin/google-java-format'
+    base_command = [binary]
+
+  # Reformat files containing changes in place.
+  for filename, lines in lines_by_file.iteritems():
+    if args.i and args.verbose:
+      print 'Formatting', filename
+    command = base_command[:]
+    if args.i:
+      command.append('-i')
+    if args.aosp:
+      command.append('--aosp')
+    if args.skip_sorting_imports:
+      command.append('--skip-sorting-imports')
+    command.extend(lines)
+    command.append(filename)
+    p = subprocess.Popen(command, stdout=subprocess.PIPE,
+                         stderr=None, stdin=subprocess.PIPE)
+    stdout, stderr = p.communicate()
+    if p.returncode != 0:
+      sys.exit(p.returncode);
+
+    if not args.i:
+      with open(filename) as f:
+        code = f.readlines()
+      formatted_code = StringIO.StringIO(stdout).readlines()
+      diff = difflib.unified_diff(code, formatted_code,
+                                  filename, filename,
+                                  '(before formatting)', '(after formatting)')
+      diff_string = string.join(diff, '')
+      if len(diff_string) > 0:
+        sys.stdout.write(diff_string)
+
+if __name__ == '__main__':
+  main()

java-format/google-java-format-git-diff.sh

@@ -0,0 +1,88 @@
+#!/bin/bash
+# Copyright 2019 The Nomulus Authors. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# This script applies Google Java format to modified regions in Java source
+# files in a Git repository. It assumes that the repository has a 'master'
+# branch that is only used for merging and is never directly worked on.
+#
+# If invoked on the master branch, this script will format the modified lines
+# relative to HEAD. Otherwise, it uses the
+# 'git merge-base --fork-point origin/master' command to find the latest
+# fork point from origin/master, and formats the modified lines between
+# the fork point and the HEAD of the current branch.
+#
+# Background: existing code base does not conform to Google Java format. Since
+# the team wants to keep the 'blame' feature (find last modifier of a line)
+# usable, we do not want to reformat existing code.
+
+set -e
+
+USAGE="
+$(basename "$0") [--help] check|format|show
+Incrementally format modified java lines in Git.
+
+where:
+    --help  show this help text
+    check  check if formatting is necessary
+    format format files in place
+    show   show the effect of the formatting as unified diff"
+
+SCRIPT_DIR="$(realpath $(dirname $0))"
+
+function callGoogleJavaFormatDiff() {
+  local forkPoint
+  forkPoint=$(git merge-base --fork-point origin/master)
+
+  local callResult
+  case "$1" in
+    "check")
+      callResult=$(git diff -U0 ${forkPoint} | \
+          ${SCRIPT_DIR}/google-java-format-diff.py -p1 | wc -l)
+      ;;
+    "format")
+      callResult=$(git diff -U0 ${forkPoint} | \
+          ${SCRIPT_DIR}/google-java-format-diff.py -p1 -i)
+      ;;
+    "show")
+      callResult=$(git diff -U0 ${forkPoint} | \
+          ${SCRIPT_DIR}/google-java-format-diff.py -p1)
+      ;;
+  esac
+  echo "${callResult}"
+  exit 0
+}
+
+function isJavaFormatNeededOnDiffs() {
+  local modifiedLineCount
+  modifiedLineCount=$(callGoogleJavaFormatDiff "check")
+
+  if [[ ${modifiedLineCount} -ne 0 ]]; then
+    echo "true"
+  else
+    echo "false"
+  fi
+  exit 0
+}
+
+# The main function of this script:
+if [[ $# -eq 1 && $1 == "check" ]]; then
+  isJavaFormatNeededOnDiffs
+elif [[ $# -eq 1 && $1 == "format" ]]; then
+  callGoogleJavaFormatDiff "format"
+elif [[ $# -eq 1 && $1 == "show" ]]; then
+  callGoogleJavaFormatDiff "show"
+else
+  echo "${USAGE}"
+fi

java_common.gradle

@@ -75,16 +75,6 @@ gradleLint.rules = [
 // - Check: ./gradlew spotlessCheck
 // - Format in place: ./gradlew spotlessApply
 spotless {
-  java {
-    clearSteps()
-    target project.fileTree("${project.rootDir}/") {
-      include "src/main/java/**/*.java"
-      include "src/test/java/**/*.java"
-      // there are some files that have degenerate interactions with the GJF formatter
-      exclude "**/flows/FlowComponent.java"
-    }
-    googleJavaFormat('1.7')
-  }
   format 'misc', {
     clearSteps()
     target '**/*.gradle'