From 7b51be44e1171b88fe83e9636260bae819e845d5 Mon Sep 17 00:00:00 2001 From: Aaditya Dhingra Date: Mon, 19 Aug 2024 15:22:48 +0000 Subject: [PATCH] Bug 1907129 - Modify linters to run ./gradlew ktlint detekt on Fenix and AC. r=android-reviewers,nalexander,gl,pollymce Differential Revision: https://phabricator.services.mozilla.com/D216999 --- .../android/android-components/build.gradle | 2 +- mobile/android/fenix/build.gradle | 2 +- mobile/android/focus-android/build.gradle | 2 +- .../kinds/source-test/mozlint-android.yml | 3 + tools/lint/android-ac.yml | 15 ++ tools/lint/android-fenix.yml | 15 ++ tools/lint/android-focus.yml | 15 ++ tools/lint/android/lints.py | 166 ++++++++++++++++++ 8 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 tools/lint/android-ac.yml create mode 100644 tools/lint/android-fenix.yml create mode 100644 tools/lint/android-focus.yml diff --git a/mobile/android/android-components/build.gradle b/mobile/android/android-components/build.gradle index a5bdcefb483a..ac31e2226f85 100644 --- a/mobile/android/android-components/build.gradle +++ b/mobile/android/android-components/build.gradle @@ -335,7 +335,7 @@ tasks.register("ktlint", JavaExec) { description = "Check Kotlin code style." classpath = configurations.ktlint mainClass.set("com.pinterest.ktlint.Main") - args "components/**/*.kt" , "samples/**/*.kt", "!**/build/**/*.kt", "buildSrc/**/*.kt", "--baseline=ktlint-baseline.xml" + args "components/**/*.kt" , "samples/**/*.kt", "!**/build/**/*.kt", "buildSrc/**/*.kt", "--baseline=ktlint-baseline.xml", "--reporter=json,output=build/reports/ktlint/ktlint.json" } tasks.register("ktlintFormat", JavaExec) { diff --git a/mobile/android/fenix/build.gradle b/mobile/android/fenix/build.gradle index 097d30e9371c..ee71351cd054 100644 --- a/mobile/android/fenix/build.gradle +++ b/mobile/android/fenix/build.gradle @@ -177,7 +177,7 @@ tasks.register('ktlint', JavaExec) { description = "Check Kotlin code style." classpath = configurations.ktlint mainClass.set("com.pinterest.ktlint.Main") - args "app/src/**/*.kt", "!**/build/**/*.kt", "--baseline=ktlint-baseline.xml" + args "app/src/**/*.kt", "!**/build/**/*.kt", "--baseline=ktlint-baseline.xml", "--reporter=json,output=build/reports/ktlint/ktlint.json" } tasks.register('ktlintFormat', JavaExec) { diff --git a/mobile/android/focus-android/build.gradle b/mobile/android/focus-android/build.gradle index 528b1f93cbbe..af93f6e57bf8 100644 --- a/mobile/android/focus-android/build.gradle +++ b/mobile/android/focus-android/build.gradle @@ -161,7 +161,7 @@ tasks.register('ktlint', JavaExec) { description = "Check Kotlin code style." classpath = configurations.ktlint mainClass.set("com.pinterest.ktlint.Main") - args "app/**/*.kt", "!**/build/**/*.kt", "buildSrc/**/*.kt" + args "app/**/*.kt", "!**/build/**/*.kt", "buildSrc/**/*.kt", "--reporter=json,output=build/reports/ktlint/ktlint.json" } diff --git a/taskcluster/kinds/source-test/mozlint-android.yml b/taskcluster/kinds/source-test/mozlint-android.yml index 81e4ff94d2a6..b6a40b98466f 100644 --- a/taskcluster/kinds/source-test/mozlint-android.yml +++ b/taskcluster/kinds/source-test/mozlint-android.yml @@ -75,6 +75,9 @@ lints: --linter android-checkstyle --linter android-lint --linter android-test + --linter android-fenix + --linter android-ac + --linter android-focus * tooltool-downloads: internal # For internal toolchains. fetches: diff --git a/tools/lint/android-ac.yml b/tools/lint/android-ac.yml new file mode 100644 index 000000000000..94ce5e2ab945 --- /dev/null +++ b/tools/lint/android-ac.yml @@ -0,0 +1,15 @@ +--- +android-ac: + description: Android formatting lint for Android Components + include: ['mobile/android/android-components'] + exclude: [] + extensions: ['java', 'kt'] + support-files: + - 'mobile/android/**/Makefile.in' + - 'mobile/android/config/**' + - 'mobile/android/gradle.configure' + - 'mobile/android/**/moz.build' + - '**/*.gradle' + type: global + payload: android.lints:ac_format + setup: android.lints:setup diff --git a/tools/lint/android-fenix.yml b/tools/lint/android-fenix.yml new file mode 100644 index 000000000000..5a0cf43b78db --- /dev/null +++ b/tools/lint/android-fenix.yml @@ -0,0 +1,15 @@ +--- +android-fenix: + description: Android formatting lint for Fenix + include: ['mobile/android/fenix'] + exclude: [] + extensions: ['java', 'kt'] + support-files: + - 'mobile/android/**/Makefile.in' + - 'mobile/android/config/**' + - 'mobile/android/gradle.configure' + - 'mobile/android/**/moz.build' + - '**/*.gradle' + type: global + payload: android.lints:fenix_format + setup: android.lints:setup diff --git a/tools/lint/android-focus.yml b/tools/lint/android-focus.yml new file mode 100644 index 000000000000..7e8a584cfe28 --- /dev/null +++ b/tools/lint/android-focus.yml @@ -0,0 +1,15 @@ +--- +android-fenix: + description: Android formatting lint for Focus + include: ['mobile/android/focus-android'] + exclude: [] + extensions: ['java', 'kt'] + support-files: + - 'mobile/android/**/Makefile.in' + - 'mobile/android/config/**' + - 'mobile/android/gradle.configure' + - 'mobile/android/**/moz.build' + - '**/*.gradle' + type: global + payload: android.lints:focus_format + setup: android.lints:setup diff --git a/tools/lint/android/lints.py b/tools/lint/android/lints.py index 53c86260137a..e58ad345f780 100644 --- a/tools/lint/android/lints.py +++ b/tools/lint/android/lints.py @@ -23,6 +23,11 @@ from mozpack.files import FileFinder # potentially expensive static analyses. GRADLE_LOCK_MAX_WAIT_SECONDS = 20 * 60 +EXCLUSION_FILES = [ + os.path.join("tools", "rewriting", "Generated.txt"), + os.path.join("tools", "rewriting", "ThirdPartyPaths.txt"), +] + def setup(root, **setupargs): if setupargs.get("substs", {}).get("MOZ_BUILD_APP") != "mobile/android": @@ -85,6 +90,41 @@ def gradle(log, topsrcdir=None, topobjdir=None, tasks=[], extra_args=[], verbose return proc.returncode +def gradlew(log, topsrcdir=None, topobjdir=None, tasks=[], cwd=None): + sys.path.insert(0, os.path.join(topsrcdir, "mobile", "android")) + from gradle import gradle_lock + + with gradle_lock(topobjdir, max_wait_seconds=GRADLE_LOCK_MAX_WAIT_SECONDS): + # The android-lint parameter can be used by gradle tasks to run special + # logic when they are run for a lint using + # project.hasProperty('android-lint') + cmd_args = ["./gradlew"] + tasks + + cmd = " ".join(shlex.quote(arg) for arg in cmd_args) + log.debug(cmd) + + # Gradle and mozprocess do not get along well, so we use subprocess + # directly. + proc = subprocess.Popen(cmd_args, cwd=cwd) + status = None + # Leave it to the subprocess to handle Ctrl+C. If it terminates as a result + # of Ctrl+C, proc.wait() will return a status code, and, we get out of the + # loop. If it doesn't, like e.g. gdb, we continue waiting. + while status is None: + try: + status = proc.wait() + except KeyboardInterrupt: + pass + + try: + proc.wait() + except KeyboardInterrupt: + proc.kill() + raise + + return proc.returncode + + def format(config, fix=None, **lintargs): topsrcdir = lintargs["root"] topobjdir = lintargs["topobjdir"] @@ -153,6 +193,132 @@ def format(config, fix=None, **lintargs): return results +def fenix_format(config, fix=None, **lintargs): + return report_gradlew( + config, + fix, + os.path.join("mobile", "android", "fenix"), + **lintargs, + ) + + +def ac_format(config, fix=None, **lintargs): + return report_gradlew( + config, + fix, + os.path.join("mobile", "android", "android-components"), + **lintargs, + ) + + +def focus_format(config, fix=None, **lintargs): + return report_gradlew( + config, + fix, + os.path.join("mobile", "android", "focus-android"), + **lintargs, + ) + + +def report_gradlew(config, fix, subdir, **lintargs): + topsrcdir = lintargs["root"] + topobjdir = lintargs["topobjdir"] + + if fix: + tasks = ["ktlintFormat", "detekt"] + else: + tasks = ["ktlint", "detekt"] + + for task in tasks: + gradlew( + lintargs["log"], + topsrcdir=topsrcdir, + topobjdir=topobjdir, + tasks=[task], + cwd=os.path.join(topsrcdir, subdir), + ) + + reports = os.path.join(topsrcdir, subdir, "build", "reports") + results = [] + + excludes = [] + for path in EXCLUSION_FILES: + with open(os.path.join(topsrcdir, path), "r") as fh: + for f in fh.readlines(): + if "*" in f: + excludes.extend(glob.glob(f.strip())) + elif f.startswith(subdir): + excludes.append(f.strip()) + + try: + tree = ET.parse( + open( + os.path.join( + reports, + "detekt", + "detekt.xml", + ), + "rt", + ) + ) + root = tree.getroot() + + for file in root.findall("file"): + name = file.get("name") + if is_excluded_file(topsrcdir, excludes, name): + continue + for error in file: + err = { + "rule": error.get("source"), + "path": name, + "lineno": int(error.get("line") or 0), + "column": int(error.get("column") or 0), + "message": error.get("message"), + "level": "error", + } + results.append(result.from_config(config, **err)) + except FileNotFoundError: + pass + + try: + issues = json.load( + open( + os.path.join( + reports, + "ktlint", + "ktlint.json", + ), + "rt", + ) + ) + + for issue in issues: + name = issue["file"] + if is_excluded_file(topsrcdir, excludes, name): + continue + for error in issue["errors"]: + err = { + "rule": error["rule"], + "path": name, + "lineno": error["line"], + "column": error["column"], + "message": error["message"], + "level": "error", + } + results.append(result.from_config(config, **err)) + except FileNotFoundError: + pass + + return results + + +def is_excluded_file(topsrcdir, excludes, file): + for path in excludes: + if file.startswith(os.path.join(topsrcdir, path)): + return True + return False + + def api_lint(config, **lintargs): topsrcdir = lintargs["root"] topobjdir = lintargs["topobjdir"]