From d4be754c39b634b1a2ea04b8dedae2b91ee61da5 Mon Sep 17 00:00:00 2001 From: Aaditya Dhingra Date: Tue, 21 Jul 2026 05:41:20 +0000 Subject: [PATCH] Bug 2052887 - Move detekt to top level as a plugin r=geckoview-reviewers,android-reviewers,nalexander,ahochheiden Update the temporary detekt ktlint aliases with top-level detekt plugin. With current implementation, the mach instances are overwriting and corrupting the same objdir causing failures even without touching code. This is not caught by try as it runs subproject linters sequentially. Differential Revision: https://phabricator.services.mozilla.com/D311097 --- build.gradle | 3 +- gradle/libs.versions.toml | 1 - .../android/android-components/build.gradle | 88 +++----------- mobile/android/fenix/.buildconfig.yml | 2 + mobile/android/fenix/app/build.gradle | 20 +++- mobile/android/fenix/build.gradle | 57 --------- mobile/android/focus-android/.buildconfig.yml | 1 + mobile/android/focus-android/app/build.gradle | 17 ++- mobile/android/focus-android/build.gradle | 51 -------- .../mozilla/conventions/ProjectExtension.kt | 6 + .../org/mozilla/conventions/ProjectPlugin.kt | 112 ++++++++++++++++++ settings.gradle | 2 + 12 files changed, 165 insertions(+), 195 deletions(-) diff --git a/build.gradle b/build.gradle index babf9ea5ec83..a2853b65d686 100644 --- a/build.gradle +++ b/build.gradle @@ -33,7 +33,6 @@ plugins { id 'org.mozilla.conventions.mach-tasks' alias(libs.plugins.android.library) apply false alias(libs.plugins.dependency.analysis) - alias(libs.plugins.detekt) alias(libs.plugins.kotlin.compose) apply false alias(libs.plugins.ksp) alias(libs.plugins.spotless) @@ -399,7 +398,7 @@ tasks.named("ktlintFormat").configure { mobileProjects.each { dependsOn "${it}:ktlintFormat" } } tasks.named("detekt").configure { - // NOTE: We are replacing the default 'detekt' task. + group = "verification" description = "Runs detekt on all mobile projects." mobileProjects.each { dependsOn "${it}:detekt" } } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index b888498acce5..2bc5aea596a6 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -318,7 +318,6 @@ android-lint-plugin = { id = "com.android.lint", version.ref = "android-gradle-p # No version: supplied by the buildscript classpath, which the plugins DSL must not version-check. androidx-safeargs = { id = "androidx.navigation.safeargs.kotlin" } dependency-analysis = { id = "com.autonomousapps.dependency-analysis", version.ref = "dependency-analysis" } -detekt = { id = "io.gitlab.arturbosch.detekt", version.ref = "detekt" } kotlin-compose = { id = "org.jetbrains.kotlin.plugin.compose", version.ref = "kotlin" } # No version: bundled in the Kotlin Gradle plugin classpath, which the plugins DSL must not version-check. kotlin-parcelize = { id = "org.jetbrains.kotlin.plugin.parcelize" } diff --git a/mobile/android/android-components/build.gradle b/mobile/android/android-components/build.gradle index 5b2f73bfd6c9..fe1e6533b81a 100644 --- a/mobile/android/android-components/build.gradle +++ b/mobile/android/android-components/build.gradle @@ -1,8 +1,5 @@ // Top-level build file where you can add configuration options common to all sub-projects/modules. -import io.gitlab.arturbosch.detekt.Detekt -import io.gitlab.arturbosch.detekt.DetektCreateBaselineTask - buildscript { repositories { gradle.configureMavenRepositories(delegate) @@ -16,13 +13,25 @@ buildscript { plugins { alias(libs.plugins.dependency.analysis) - alias(libs.plugins.detekt) alias(libs.plugins.kotlin.compose) apply false alias(libs.plugins.ksp) } mozilla { ktlintSourcePaths = ["components/**/*.kt", "samples/**/*.kt", "buildSrc/**/*.kt", "!**/build/**/*.kt"] + detektSourcePaths = [ + "**/components/**/*.kt", "**/buildSrc/**/*.kt", "**/samples/**/*.kt", + "!**/build.gradle.kts", "!**/build/**", "!**/docs/**", "!**/resources/**", + "!**/src/androidTest/**", "!**/src/iosTest/**", "!**/src/main/assets/extensions/**", + "!**/src/test/**", "!**/test/src/**", "!**/tmp/**", "!**/tooling/fetch-tests/**", + ] + detektConfig = "$projectDir/config/detekt.yml" + detektBaseline = "$projectDir/config/detekt-baseline.xml" + detektReports = [ + "xml": "$projectDir/build/reports/detekt/detekt.xml", + "html": "$projectDir/build/reports/detekt.html", + "suppression-count": "$projectDir/build/reports/suppressions.txt", + ] } allprojects { @@ -139,56 +148,7 @@ if (findProject(":geckoview") == null) { } } -detekt { - input = files("$projectDir/components", "$projectDir/buildSrc", "$projectDir/samples") - config = files("$projectDir/config/detekt.yml") - baseline = file("$projectDir/config/detekt-baseline.xml") - - reports { - html { - enabled = true - destination = file("$projectDir/build/reports/detekt.html") - } - xml { - enabled = false - } - txt { - enabled = false - } - } -} - -tasks.named("detekt").configure { - reports { - custom { - reportId = "suppression-count" - outputLocation.set(file("$projectDir/build/reports/suppressions.txt")) - } - } -} - -tasks.withType(Detekt).configureEach() { - // Custom detekt rules should be built before. - // See https://detekt.dev/docs/introduction/extensions#pitfalls - dependsOn(":components:tooling-detekt:assemble") - - autoCorrect = true - - exclude "**/build.gradle.kts" - exclude "**/build/**" - exclude "**/docs/**" - exclude "**/resources/**" - exclude "**/src/androidTest/**" - exclude "**/src/iosTest/**" - exclude "**/src/main/assets/extensions/**" - exclude "**/src/test/**" - exclude "**/test/src/**" - exclude "**/tmp/**" - exclude "**/tooling/fetch-tests/**" -} - -// Apply same path exclusions as for the main task -tasks.withType(DetektCreateBaselineTask).configureEach() { +tasks.named("detektBaseline").configure { dependsOn(":components:browser-icons:updateBuiltInExtensionVersion") dependsOn(":components:feature-accounts:updateBuiltInExtensionVersion") dependsOn(":components:feature-readerview:updateBuiltInExtensionVersion") @@ -198,30 +158,10 @@ tasks.withType(DetektCreateBaselineTask).configureEach() { dependsOn(":components:samples-browser:updateTestExtensionVersion") dependsOn(":components:samples-compose-browser:updateBorderifyExtensionVersion") dependsOn(":components:samples-compose-browser:updateTestExtensionVersion") - dependsOn(":components:tooling-detekt:assemble") - - exclude "**/build.gradle.kts" - exclude "**/build/**" - exclude "**/docs/**" - exclude "**/resources/**" - exclude "**/src/androidTest/**" - exclude "**/src/iosTest/**" - exclude "**/src/main/assets/extensions/**" - exclude "**/src/test/**" - exclude "**/test/src/**" - exclude "**/tmp/**" - exclude "**/tooling/fetch-tests/**" -} - -configurations { - detektDependencies } dependencies { detektPlugins project(":components:tooling-detekt") - detekt libs.detekt.cli - - detektDependencies libs.detekt.cli } tasks.register("lint") { diff --git a/mobile/android/fenix/.buildconfig.yml b/mobile/android/fenix/.buildconfig.yml index 3c3703d71096..810636c8420e 100644 --- a/mobile/android/fenix/.buildconfig.yml +++ b/mobile/android/fenix/.buildconfig.yml @@ -115,6 +115,7 @@ projects: - components:support-test-libstate - components:support-utils - components:support-webextensions + - components:tooling-detekt - components:tooling-lint - components:ui-animation - components:ui-autocomplete @@ -123,6 +124,7 @@ projects: - components:ui-richtext - components:ui-tabcounter - components:ui-widgets + - mozilla-detekt-rules variants: - apks: - abi: arm64-v8a diff --git a/mobile/android/fenix/app/build.gradle b/mobile/android/fenix/app/build.gradle index 62514ba0b341..b72facf2d1f2 100644 --- a/mobile/android/fenix/app/build.gradle +++ b/mobile/android/fenix/app/build.gradle @@ -777,11 +777,21 @@ def getSupportedLocales() { mozilla { ktlintSourcePaths = ["src/**/*.kt", "!**/build/**/*.kt"] + detektSourcePaths = [ + "**/src/**/*.kt", + "!**/test/**", "!**/androidTest/**", "!**/build/**", "!**/resources/**", "!**/tmp/**", + "!**/longfox/**" + ] + detektConfig = "$projectDir/../config/detekt.yml" + detektBaseline = "$projectDir/../config/detekt-baseline.xml" + detektReports = [ + "xml": "$projectDir/../build/reports/detekt/detekt.xml", + "html": "$projectDir/../build/reports/detekt.html", + "suppression-count": "$projectDir/../build/reports/suppressions.txt", + ] } -tasks.register("detekt", Exec) { - group = "verification" - description = "Run detekt static analysis." - workingDir file("${gradle.mozconfig.topsrcdir}") - commandLine "./mach", "gradle", "-p", "mobile/android/fenix", ":detekt" +dependencies { + detektPlugins project(":components:tooling-detekt") + detektPlugins project(":mozilla-detekt-rules") } diff --git a/mobile/android/fenix/build.gradle b/mobile/android/fenix/build.gradle index 486776d98eaf..64f905230596 100644 --- a/mobile/android/fenix/build.gradle +++ b/mobile/android/fenix/build.gradle @@ -1,9 +1,5 @@ // Top-level build file where you can add configuration options common to all sub-projects/modules. - -import io.gitlab.arturbosch.detekt.Detekt -import io.gitlab.arturbosch.detekt.DetektCreateBaselineTask - buildscript { repositories { gradle.configureMavenRepositories(delegate) @@ -25,7 +21,6 @@ buildscript { plugins { id "mozac.ConfigPlugin" alias(libs.plugins.dependency.analysis) - alias(libs.plugins.detekt) alias(libs.plugins.kotlin.compose) apply false alias(libs.plugins.ksp) } @@ -59,58 +54,6 @@ dependencyAnalysis { } } -detekt { - input = files("$projectDir/app/src") - config = files("$projectDir/config/detekt.yml") - baseline = file("$projectDir/config/detekt-baseline.xml") - - reports { - html { - enabled = true - destination = file("$projectDir/build/reports/detekt.html") - } - xml { - enabled = false - } - txt { - enabled = false - } - } -} - -tasks.withType(Detekt).configureEach() { - autoCorrect = true - - exclude "**/test/**" - exclude "**/androidTest/**" - exclude "**/build/**" - exclude "**/resources/**" - exclude "**/tmp/**" - - reports { - custom { - reportId = "suppression-count" - outputLocation.set(file("$projectDir/build/reports/suppressions.txt")) - } - } -} - -// Apply same path exclusions as for the main task -tasks.withType(DetektCreateBaselineTask).configureEach() { - exclude "**/test/**" - exclude "**/androidTest/**" - exclude "**/build/**" - exclude "**/resources/**" - exclude "**/tmp/**" -} - -dependencies { - detekt project(":components:tooling-detekt") - detekt project(":mozilla-detekt-rules") - detekt libs.detekt.cli -} - - tasks.register("listRepositories") { def reposData = project.provider { project.repositories.collect { repo -> diff --git a/mobile/android/focus-android/.buildconfig.yml b/mobile/android/focus-android/.buildconfig.yml index 52cbafd82cb4..7ee5875eac9c 100644 --- a/mobile/android/focus-android/.buildconfig.yml +++ b/mobile/android/focus-android/.buildconfig.yml @@ -66,6 +66,7 @@ projects: - components:support-test-libstate - components:support-utils - components:support-webextensions + - components:tooling-detekt - components:tooling-lint - components:ui-autocomplete - components:ui-colors diff --git a/mobile/android/focus-android/app/build.gradle b/mobile/android/focus-android/app/build.gradle index 584d480a4508..fcea79f6aca3 100644 --- a/mobile/android/focus-android/app/build.gradle +++ b/mobile/android/focus-android/app/build.gradle @@ -636,11 +636,18 @@ if (project.hasProperty("coverage")) { mozilla { ktlintSourcePaths = ["src/**/*.kt", "!**/build/**/*.kt"] + detektSourcePaths = [ + "**/src/**/*.kt", + "!**/test/**", "!**/androidTest/**", "!**/build/**", "!**/resources/**", "!**/tmp/**", + ] + detektConfig = "$projectDir/../quality/detekt.yml" + detektBaseline = "$projectDir/../quality/detekt-baseline.xml" + detektReports = [ + "xml": "$projectDir/../build/reports/detekt/detekt.xml", + "html": "$projectDir/../build/reports/detekt.html", + ] } -tasks.register("detekt", Exec) { - group = "verification" - description = "Run detekt static analysis." - workingDir file("${gradle.mozconfig.topsrcdir}") - commandLine "./mach", "gradle", "-p", "mobile/android/focus-android", ":detekt" +dependencies { + detektPlugins project(":components:tooling-detekt") } diff --git a/mobile/android/focus-android/build.gradle b/mobile/android/focus-android/build.gradle index c3c3db0ab3c2..2d279053f23c 100644 --- a/mobile/android/focus-android/build.gradle +++ b/mobile/android/focus-android/build.gradle @@ -1,7 +1,3 @@ -import io.gitlab.arturbosch.detekt.Detekt -import io.gitlab.arturbosch.detekt.DetektCreateBaselineTask - - // Top-level build file where you can add configuration options common to all sub-projects/modules. buildscript { @@ -23,7 +19,6 @@ buildscript { plugins { id "mozac.ConfigPlugin" alias(libs.plugins.dependency.analysis) - alias(libs.plugins.detekt) alias(libs.plugins.kotlin.compose) apply false alias(libs.plugins.ksp) } @@ -50,56 +45,10 @@ dependencyAnalysis { } } -detekt { - input = files("$projectDir/app") - config = files("$projectDir/quality/detekt.yml") - baseline = file("$projectDir/quality/detekt-baseline.xml") - - reports { - html { - enabled = true - destination = file("$projectDir/build/reports/detekt.html") - } - xml { - enabled = false - } - txt { - enabled = false - } - } -} - -tasks.withType(Detekt).configureEach() { - // Custom detekt rules should be built before. - // See https://detekt.dev/docs/introduction/extensions#pitfalls - dependsOn(":components:tooling-detekt:assemble") - - autoCorrect = true - - exclude "**/test/**" - exclude "**/androidTest/**" - exclude "**/build/**" - exclude "**/resources/**" - exclude "**/tmp/**" -} - -// Apply same path exclusions as for the main task -tasks.withType(DetektCreateBaselineTask).configureEach() { - exclude "**/test/**" - exclude "**/androidTest/**" - exclude "**/build/**" - exclude "**/resources/**" - exclude "**/tmp/**" -} - tasks.register('clean', Delete) { delete rootProject.layout.buildDirectory } -dependencies { - detektPlugins project(':components:tooling-detekt') -} - tasks.register("listRepositories") { def reposData = project.provider { project.repositories.collect { repo -> diff --git a/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectExtension.kt b/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectExtension.kt index 8babe9eede43..a960eb0f6f46 100644 --- a/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectExtension.kt +++ b/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectExtension.kt @@ -5,9 +5,15 @@ package org.mozilla.conventions import org.gradle.api.provider.ListProperty +import org.gradle.api.provider.MapProperty import org.gradle.api.provider.Property interface ProjectExtension { val androidComponentsProject: Property val ktlintSourcePaths: ListProperty + val detektSourcePaths: ListProperty + val detektConfig: Property + val detektBaseline: Property + val detektAutoCorrect: Property + val detektReports: MapProperty } diff --git a/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectPlugin.kt b/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectPlugin.kt index ed7009910ba4..d7b3b325d85c 100644 --- a/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectPlugin.kt +++ b/mobile/android/gradle/plugins/conventions/src/main/java/org/mozilla/conventions/ProjectPlugin.kt @@ -24,6 +24,7 @@ import org.gradle.api.tasks.testing.TestListener import org.gradle.api.tasks.testing.TestOutputEvent import org.gradle.api.tasks.testing.TestOutputListener import org.gradle.api.tasks.testing.TestResult +import org.gradle.process.CommandLineArgumentProvider import java.io.File class ProjectPlugin : Plugin { @@ -32,6 +33,9 @@ class ProjectPlugin : Plugin { val mozilla = project.extensions.create("mozilla", ProjectExtension::class.java) mozilla.androidComponentsProject.convention(false) mozilla.ktlintSourcePaths.convention(emptyList()) + mozilla.detektSourcePaths.convention(emptyList()) + mozilla.detektAutoCorrect.convention(true) + mozilla.detektReports.convention(emptyMap()) val extraProperties = project.gradle.extensions.extraProperties val mozconfig = extraProperties["mozconfig"] as Map @@ -55,6 +59,7 @@ class ProjectPlugin : Plugin { configureGleanSubstitution(project, extraProperties) configureGleanVersionResolution(project) configureKtlint(project, mozilla) + configureDetekt(project, mozilla) configureTestOutputFormatting(project) configurePackagingResourcesExcludes(project) registerPrintVariantsTask(project) @@ -385,6 +390,113 @@ class ProjectPlugin : Plugin { } } + private fun configureDetekt(project: Project, mozilla: ProjectExtension) { + val sourcePaths = mozilla.detektSourcePaths + + val detektConfig = project.configurations.register("detektCli") + val detektDep = project.provider { + val versionCatalogs = project.extensions.getByType(VersionCatalogsExtension::class.java) + val libs = versionCatalogs.named("libs") + project.dependencies.create(libs.findLibrary("detekt-cli").get().get()) + } + detektConfig.configure { dependencies.addLater(detektDep) } + val detektClasspath = project.files(detektConfig) + + // Subproject's build.gradle can add dependencies like + // detektPlugins project(":components:tooling-detekt") + val detektPlugins = project.configurations.register("detektPlugins") { + isCanBeConsumed = false + isCanBeResolved = true + } + val detektPluginFiles = project.files(detektPlugins) + + val projectDir = project.projectDir + + fun JavaExec.configureCommon() { + classpath = detektClasspath + mainClass.set("io.gitlab.arturbosch.detekt.cli.Main") + // Resolve the include/exclude globs (with leading "!" meaning exclude) + // into a FileTree rooted at projectDir, so Gradle can use the actual + // Kotlin source set to compute UP-TO-DATE / build cache keys. + val detektSourceTree = + if (sourcePaths.get().none { !it.startsWith("!") }) { + project.files() + } else { + project.fileTree(projectDir).matching { + sourcePaths.get().forEach { pattern -> + if (pattern.startsWith("!")) { + exclude(pattern.removePrefix("!")) + } else { + include(pattern) + } + } + } + } + onlyIf { !detektSourceTree.isEmpty } + + mozilla.detektConfig.orNull?.let { + val file = project.file(it) + args("--config", file.absolutePath) + inputs.file(file).withPropertyName("detektConfig") + } + + inputs.files(detektSourceTree) + .withPropertyName("detektSources") + .withPathSensitivity(org.gradle.api.tasks.PathSensitivity.RELATIVE) + .skipWhenEmpty() + inputs.files(detektPluginFiles) + .withPropertyName("detektPlugins") + .optional(true) + + val includes = sourcePaths.get().filter { !it.startsWith("!") } + val excludes = sourcePaths.get().filter { it.startsWith("!") }.map {it.removePrefix("!") } + args("--input", projectDir.absolutePath) + if (includes.isNotEmpty()) args("--includes", includes.joinToString(",")) + if (excludes.isNotEmpty()) args("--excludes", excludes.joinToString(",")) + + argumentProviders.add(CommandLineArgumentProvider { + val plugins = detektPluginFiles.files.filter { it.exists() } + if (plugins.isNotEmpty()) { + listOf("--plugins", plugins.joinToString(",") { it.absolutePath }) + } else { + emptyList() + } + }) + } + + project.tasks.register("detekt", JavaExec::class.java) { + group = "verification" + description = "Run detekt static analysis." + configureCommon() + + if (mozilla.detektAutoCorrect.get()) { + args("--auto-correct") + jvmArgs("--add-opens", "java.base/java.lang=ALL-UNNAMED") + } + + mozilla.detektBaseline.orNull?.let { + val file = project.file(it) + if (file.exists()) { + args("--baseline", file.absolutePath) + inputs.file(file).withPropertyName("detektBaseline").optional(true) + } + } + mozilla.detektReports.get().forEach { (id, path) -> args("--report", "$id:$path") } + mozilla.detektReports.get().values.forEach { outputs.file(project.file(it)) } + outputs.cacheIf { true } + } + + project.tasks.register("detektBaseline", JavaExec::class.java) { + group = "verification" + description = "Regenerate the detekt baseline." + configureCommon() + val baselineFile = mozilla.detektBaseline.orNull?.let { project.file(it) } + onlyIf { baselineFile != null } + args("--create-baseline") + baselineFile?.let { args("--baseline", it.absolutePath) } + } + } + // Translates JUnit test events into Mozilla's TBPL-like textual format that Taskcluster // log parsing expects. See also: testing/mozbase/mozlog/mozlog/formatters/tbplformatter.py private fun configureTestOutputFormatting(project: Project) { diff --git a/settings.gradle b/settings.gradle index d149d222b95b..5f09bd4db149 100644 --- a/settings.gradle +++ b/settings.gradle @@ -55,6 +55,8 @@ project(':android-components').projectDir = new File("${gradle.mozconfig.topsrcd if (!gradle.mozconfig.substs.MOZ_ANDROID_SUBPROJECT || gradle.mozconfig.substs.MOZ_ANDROID_SUBPROJECT == "fenix") { include ':fenix' project(':fenix').projectDir = new File("${gradle.mozconfig.topsrcdir}/mobile/android/fenix/app") + include ':mozilla-detekt-rules' + project(':mozilla-detekt-rules').projectDir = new File("${gradle.mozconfig.topsrcdir}/mobile/android/fenix/mozilla-detekt-rules") include ':fenix:longfox' project(':fenix:longfox').projectDir = new File("${gradle.mozconfig.topsrcdir}/mobile/android/fenix/app/longfox") include ':fenix:benchmark'