Bug 1571899 - Use google-java-format via spotless to enforce Java for…

…matting. r=ahal,owlish

This change adds a new lint `android-format` which enforces formatting of Java
code using google-java-format.

To run the lint simply run:

./mach lint -l android-format

This command also support automatically fixing all errors running by adding
--fix:

./mach lint -l android-format --fix

This change also removes all the formatting-related checkstyle checks which are
now implicitly enforced by the formatter.

Differential Revision: https://phabricator.services.mozilla.com/D127734

build.gradle

@@ -105,6 +105,7 @@ buildscript {
         classpath 'com.android.tools.build:gradle:4.2.0'
         classpath 'com.getkeepsafe.dexcount:dexcount-gradle-plugin:0.8.2'
         classpath 'org.apache.commons:commons-exec:1.3'
+        classpath 'com.diffplug.spotless:spotless-plugin-gradle:5.16.0'
         classpath 'org.tomlj:tomlj:1.0.0'
         classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"
     }
@@ -268,3 +269,17 @@ idea {
         }
     }
 }
+
+subprojects {
+    apply plugin: "com.diffplug.spotless"
+
+    spotless {
+        java {
+            target project.fileTree(project.projectDir) {
+                include '**/*.java'
+                exclude '**/thirdparty/**'
+            }
+            googleJavaFormat('1.7')
+        }
+    }
+}

docs/code-quality/lint/linters/android-format.rst

@@ -0,0 +1,28 @@
+Spotless
+========
+
+`Spotless <https://github.com/diffplug/spotless>`__ is a pluggable formatter
+for Gradle and Android.
+
+In our current configuration, Spotless includes the
+`Google Java Format plug-in https://github.com/google/google-java-format`__
+which formats all our Java code using the Google Java coding style guidelines.
+
+
+Run Locally
+-----------
+
+The mozlint integration of spotless can be run using mach:
+
+.. parsed-literal::
+
+    $ mach lint --linter android-format
+
+Alternatively, omit the ``--linter android-format`` and run all configured linters, which will include
+spotless.
+
+
+Autofix
+-------
+
+The spotless linter provides a ``--fix`` option.

mobile/android/geckoview/checkstyle.xml

@@ -8,18 +8,6 @@
 
 <module name="Checker">
     <property name="charset" value="UTF-8"/>
-
-    <!-- Disallow tabs -->
-    <module name="FileTabCharacter">
-        <property name="eachLine" value="true"/>
-    </module>
-
-    <!-- Disallow trailing whitespace -->
-    <module name="RegexpSingleline">
-        <property name="format" value="\s+$"/>
-        <property name="message" value="Trailing whitespace"/>
-    </module>
-
     <module name="SuppressionFilter">
       <property name="file" value="${config_loc}/checkstyle-suppressions.xml"/>
       <property name="optional" value="false"/>
@@ -35,9 +23,6 @@
         </module>
         <module name="ParameterName"/>
         <module name="StaticVariableName"/>
-        <module name="LeftCurly"/>
-        <module name="RightCurly"/>
-        <module name="Indentation"/>
         <module name="OneStatementPerLine"/>
         <module name="AvoidStarImport"/>
         <module name="UnusedImports"/>
@@ -64,19 +49,10 @@
         </module>
         <module name="LocalVariableName"/>
 
-        <module name="GenericWhitespace"/>
-        <module name="NoLineWrap">
-            <property name="tokens" value="IMPORT,PACKAGE_DEF"/>
-        </module>
         <module name="OuterTypeFilename"/>
         <module name="WhitespaceAfter">
             <property name="tokens" value="COMMA, SEMI"/>
         </module>
-        <module name="WhitespaceAround">
-            <property name="allowEmptyConstructors" value="true"/>
-            <property name="allowEmptyMethods" value="true"/>
-            <property name="allowEmptyTypes" value="true"/>
-        </module>
         <module name="OneTopLevelClass"/>
     </module>
 

mobile/android/gradle.configure

@@ -213,6 +213,30 @@ def gradle_android_api_lint_tasks(build_config):
 set_config("GRADLE_ANDROID_API_LINT_TASKS", gradle_android_api_lint_tasks)
 
 
+set_config("GRADLE_ANDROID_FORMAT_LINT_FIX_TASKS", ["spotlessJavaApply"])
+
+
+@dependable
+def gradle_android_format_lint_check_tasks():
+    return ["spotlessJavaCheck"]
+
+
+set_config(
+    "GRADLE_ANDROID_FORMAT_LINT_CHECK_TASKS", gradle_android_format_lint_check_tasks
+)
+
+set_config(
+    "GRADLE_ANDROID_FORMAT_LINT_FOLDERS",
+    [
+        "mobile/android/annotations",
+        "mobile/android/geckoview",
+        "mobile/android/geckoview_example",
+        "mobile/android/examples/messaging_example",
+        "mobile/android/examples/port_messaging_example",
+    ],
+)
+
+
 @depends(gradle_android_build_config)
 def gradle_android_checkstyle_tasks(build_config):
     """Gradle tasks run by |mach android checkstyle|."""
@@ -400,6 +424,7 @@ def gradle_android_dependencies():
 
 @depends(
     gradle_android_api_lint_tasks,
+    gradle_android_format_lint_check_tasks,
     gradle_android_checkstyle_tasks,
     gradle_android_dependencies,
 )

taskcluster/ci/source-test/mozlint-android.yml

@@ -70,6 +70,7 @@ lints:
             ./mach --log-no-times build pre-export export &&
             ./mach --log-no-times lint -f treeherder -f json:/builds/worker/mozlint.json
             --linter android-api-lint
+            --linter android-format
             --linter android-javadoc
             --linter android-checkstyle
             --linter android-lint

tools/lint/android-format.yml

@@ -0,0 +1,15 @@
+---
+android-format:
+    description: Android formatting lint
+    include: ['mobile/android']
+    exclude: []
+    extensions: ['java']
+    support-files:
+        - 'mobile/android/**/Makefile.in'
+        - 'mobile/android/config/**'
+        - 'mobile/android/gradle.configure'
+        - 'mobile/android/**/moz.build'
+        - '**/*.gradle'
+    type: global
+    payload: android.lints:format
+    setup: android.lints:setup

tools/lint/android/lints.py

@@ -6,6 +6,7 @@
 
 import itertools
 import json
+import glob
 import os
 import re
 import six
@@ -18,7 +19,6 @@
 import mozpack.path as mozpath
 from mozlint import result
 
-
 # The Gradle target invocations are serialized with a simple locking file scheme.  It's fine for
 # them to take a while, since the first will compile all the Java, etc, and then perform
 # potentially expensive static analyses.
@@ -84,6 +84,45 @@ def gradle(log, topsrcdir=None, topobjdir=None, tasks=[], extra_args=[], verbose
             raise
 
 
+def format(config, fix=None, **lintargs):
+    topsrcdir = lintargs["root"]
+    topobjdir = lintargs["topobjdir"]
+
+    if fix:
+        tasks = lintargs["substs"]["GRADLE_ANDROID_FORMAT_LINT_FIX_TASKS"]
+    else:
+        tasks = lintargs["substs"]["GRADLE_ANDROID_FORMAT_LINT_CHECK_TASKS"]
+
+    gradle(
+        lintargs["log"],
+        topsrcdir=topsrcdir,
+        topobjdir=topobjdir,
+        tasks=tasks,
+        extra_args=lintargs.get("extra_args") or [],
+    )
+
+    results = []
+    for path in lintargs["substs"]["GRADLE_ANDROID_FORMAT_LINT_FOLDERS"]:
+        folder = os.path.join(
+            topobjdir, "gradle", "build", path, "spotless", "spotlessJava"
+        )
+        for filename in glob.iglob(folder + "/**/*.java", recursive=True):
+            err = {
+                "rule": "spotless-java",
+                "path": os.path.join(path, mozpath.relpath(filename, folder)),
+                "lineno": 0,
+                "column": 0,
+                "message": "Formatting error, please run ./mach lint -l android-format --fix",
+                "level": "error",
+            }
+            results.append(result.from_config(config, **err))
+
+    # If --fix was passed, we just report the number of files that were changed
+    if fix:
+        return {"results": [], "fixed": len(results)}
+    return results
+
+
 def api_lint(config, **lintargs):
     topsrcdir = lintargs["root"]
     topobjdir = lintargs["topobjdir"]

tools/lint/mach_commands.py

@@ -8,6 +8,7 @@
 
 from mozbuild.base import (
     BuildEnvironmentNotFoundException,
+    MachCommandConditions as conditions,
 )
 
 
@@ -31,6 +32,7 @@
 GLOBAL_EXCLUDES = ["node_modules", "tools/lint/test/files", ".hg", ".git"]
 
 VALID_FORMATTERS = {"black", "clang-format", "rustfmt"}
+VALID_ANDROID_FORMATTERS = {"android-format"}
 
 
 def setup_argument_parser():
@@ -143,16 +145,20 @@ def eslint(command_context, paths, extra_args=[], **kwargs):
 def format_files(command_context, paths, extra_args=[], **kwargs):
     linters = kwargs["linters"]
 
+    formatters = VALID_FORMATTERS
+    if conditions.is_android(command_context):
+        formatters |= VALID_ANDROID_FORMATTERS
+
     if not linters:
-        linters = VALID_FORMATTERS
+        linters = formatters
     else:
-        invalid_linters = set(linters) - VALID_FORMATTERS
+        invalid_linters = set(linters) - formatters
         if invalid_linters:
             print(
                 "error: One or more linters passed are not valid formatters. "
                 "Note that only the following linters are valid formatters:"
             )
-            print("\n".join(sorted(VALID_FORMATTERS)))
+            print("\n".join(sorted(formatters)))
             return 1
 
     kwargs["linters"] = list(linters)