Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detekt: integrate and enable in production code #292

Merged
merged 46 commits into from
Sep 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
05dc2ab
Add Detekt plugin for checking the project.
TWiStErRob Sep 18, 2022
4e5db82
Simplify name of plugin in DSL
TWiStErRob Sep 18, 2022
be6cc1c
Add detekt to gradle/plugins
TWiStErRob Sep 18, 2022
3b67ada
Add detekt to all modules
TWiStErRob Sep 18, 2022
9186afb
Check everything deeper
TWiStErRob Sep 18, 2022
bd8f82a
Configure Detekt report merging
TWiStErRob Sep 18, 2022
75b9690
Some trivial Detekt fixes.
TWiStErRob Sep 18, 2022
187ea10
Detekt: ClassOrdering fixes
TWiStErRob Sep 18, 2022
f1abf4f
Detekt CI
TWiStErRob Sep 18, 2022
f96b9e8
Better handling of uninitialized property by letting Gradle do it.
TWiStErRob Sep 19, 2022
81117be
Some trivial Detekt fixes.
TWiStErRob Sep 19, 2022
c8c452f
CascadingCallWrapping: Introduce helper to simplify code.
TWiStErRob Sep 19, 2022
d25e5b0
Disable test checks
TWiStErRob Sep 19, 2022
0d90d6a
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
30b0fab
UnusedPrivateMember: remove project
TWiStErRob Sep 20, 2022
fb5b3df
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
1303815
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
34efee0
UnnecessaryParentheses: disable to reduce noise for now
TWiStErRob Sep 20, 2022
2ffa570
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
6bdb8a2
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
09cc55b
LateinitUsage: remove by making data flow explicit
TWiStErRob Sep 20, 2022
a974c42
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
dc9e449
UnnecessaryAbstractClass: use conventional initialization
TWiStErRob Sep 20, 2022
394204b
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
7d23590
LateinitUsage: remove by making data flow explicit
TWiStErRob Sep 20, 2022
b500b53
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
0fc6ef2
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
df262b8
Some trivial Detekt fixes.
TWiStErRob Sep 20, 2022
fc8c2cc
Don't fail Detekt on CI
TWiStErRob Sep 20, 2022
ed69fe2
BooleanPropertyNaming: rename and suppress
TWiStErRob Sep 21, 2022
0a62194
Fix some more complex Detekt violations.
TWiStErRob Sep 21, 2022
66e0311
Some trivial Detekt fixes.
TWiStErRob Sep 21, 2022
6d776e1
Extract class
TWiStErRob Sep 21, 2022
0134776
Rework it a bit to reduce nulls
TWiStErRob Sep 21, 2022
ca0c2c8
Ignore failures only on CI
TWiStErRob Sep 21, 2022
c70d4e9
Only check production code
TWiStErRob Sep 21, 2022
ca07e39
Restore CI
TWiStErRob Sep 21, 2022
8d17a0b
Merge remote-tracking branch 'origin/master' into detekt_checks
TWiStErRob Sep 21, 2022
c1d2a42
Fix PMD task
TWiStErRob Sep 22, 2022
43392af
Merge remote-tracking branch 'origin/master' into detekt_checks
TWiStErRob Sep 23, 2022
7632c62
Fix table
TWiStErRob Sep 23, 2022
f47e1e8
Add versions that can be used.
TWiStErRob Sep 23, 2022
207e6cb
Fix a few STOPSHIPs
TWiStErRob Sep 23, 2022
9863866
UseIfInsteadOfWhen: suppress or fix
TWiStErRob Sep 24, 2022
420ed61
Code review
TWiStErRob Sep 24, 2022
762af29
Fix
TWiStErRob Sep 24, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,32 @@ jobs:
run: ./gradlew --no-daemon --no-build-cache --stacktrace assemble check

- name: Build Gradle Quality plugins.
run: ./gradlew --no-daemon --no-build-cache --stacktrace jar validatePlugins
run: >
./gradlew
--no-daemon
--no-build-cache
--stacktrace
--continue
jar
validatePlugins
detektMain
detektReportMergeSarif
detektReportMergeXml

- name: Upload "Detekt Results" artifact.
if: success() || failure()
uses: actions/upload-artifact@v3
with:
name: Detekt Results
path: |
${{ github.workspace }}/**/build/reports/detekt/detekt.*
${{ github.workspace }}/build/reports/detekt/merge.*

- name: Publish "Code scanning results / detekt".
uses: github/codeql-action/upload-sarif@v2
if: success() || failure()
with:
sarif_file: ${{ github.workspace }}/build/reports/detekt/merge.sarif

- name: Cancel other jobs on Build failure, no need to check / test it.
if: failure()
Expand Down
6 changes: 6 additions & 0 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import java.time.format.DateTimeFormatter
plugins {
@Suppress("DSL_SCOPE_VIOLATION")
alias(libs.plugins.nexus)
id("net.twisterrob.gradle.build.detekt.root")
}

val projectVersion: String by project
Expand Down Expand Up @@ -199,6 +200,11 @@ if (project.property("net.twisterrob.gradle.build.includeExamples").toString().t
}
}

tasks.register("check") {
description = "Delegate task for checking included builds too."
dependsOn(gradle.includedBuild("plugins").task(":check"))
}

project.tasks.register<TestReport>("testReport") {
group = LifecycleBasePlugin.VERIFICATION_GROUP
description = "Run and report on all tests in the project. Add `-x test` to just generate report."
Expand Down
1 change: 1 addition & 0 deletions checkers/checkstyle/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ plugins {
id("org.jetbrains.kotlin.jvm")
id("org.gradle.java-test-fixtures")
id("net.twisterrob.gradle.build.publishing")
id("net.twisterrob.gradle.build.detekt")
}

base.archivesName.set("twister-quality-checkstyle")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package net.twisterrob.gradle.checkstyle

import net.twisterrob.gradle.common.BaseQualityExtension
import org.gradle.api.Project

open class CheckStyleExtension(project: Project) : BaseQualityExtension<CheckStyleTask>(project)
open class CheckStyleExtension : BaseQualityExtension<CheckStyleTask>()
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ open class CheckStyleTask : Checkstyle(), TargetChecker {

private fun setupProperties() {
// partially based on https://github.com/jshiell/checkstyle-idea#eclipse-cs-variable-support
configProperties = (configProperties ?: emptyMap()) + mapOf(
configProperties = configProperties.orEmpty() + mapOf(
"basedir" to project.projectDir, // TODO or rootDir?
"project_loc" to project.rootDir,
"workspace_loc" to project.rootDir
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class CheckStyleTaskCreator(project: Project) : VariantTaskCreator<CheckStyleTas
task.configFile = rootConfig
}
}
@Suppress("UseIfInsteadOfWhen") // Preparing for future new version ranges.
when {
GradleVersion.current().baseVersion < GradleVersion.version("6.0") -> {
// Keep using Checkstyle.setConfigDir instead of getConfigDirectory() for backward compatibility.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,13 @@ class CheckStyleTaskTest_ConfigLocation : BaseIntgTest() {

val result = executeBuild()
assertEquals(TaskOutcome.FAILED, result.task(":module:checkstyleDebug")!!.outcome)
if (gradle.gradleVersion < GradleVersion.version("5.0"))
assertThat(result.failReason, containsString("Unable to create a Checker: configLocation"))
else
assertThat(result.failReason, containsString("Unable to create Root Module: config"))
val expectedError = when {
gradle.gradleVersion.baseVersion < GradleVersion.version("5.0") ->
"Unable to create a Checker: configLocation"
else ->
"Unable to create Root Module: config"
}
assertThat(result.failReason, containsString(expectedError))
result.assertHasOutputLine("""While auto-configuring configFile for task ':module:checkstyleDebug', there was no configuration found at:""")
}

Expand Down
1 change: 1 addition & 0 deletions checkers/pmd/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ plugins {
id("org.jetbrains.kotlin.jvm")
id("org.gradle.java-test-fixtures")
id("net.twisterrob.gradle.build.publishing")
id("net.twisterrob.gradle.build.detekt")
}

base.archivesName.set("twister-quality-pmd")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package net.twisterrob.gradle.pmd

import net.twisterrob.gradle.common.BaseQualityExtension
import org.gradle.api.Project

open class PmdExtension(project: Project) : BaseQualityExtension<PmdTask>(project)
open class PmdExtension : BaseQualityExtension<PmdTask>()
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class PmdTaskCreator(project: Project) : VariantTaskCreator<PmdTask>(
object : VariantTaskCreator<PmdTask>.DefaultTaskConfig() {

override fun setupConfigLocations(task: PmdTask) {
task.ruleSets = listOf() // default is java-basic
task.ruleSets = emptyList() // default is java-basic
val rootConfig = task.project.rootProject.file("config/pmd/pmd.xml")
val subConfig = task.project.file("config/pmd/pmd.xml")
val config: File? = listOf(subConfig, rootConfig).firstOrNull { it.exists() }
Expand Down Expand Up @@ -59,22 +59,30 @@ class PmdTaskCreator(project: Project) : VariantTaskCreator<PmdTask>(
return
}

// Note: Kotlin 1.4 introduced Sequence.flatMap(()->Iterable), Gradle <6.8 uses Kotlin 1.3.x

task.include(variants
.flatMap { it.sourceSets }
.flatMap { it.resDirectories }
.asSequence()
.flatMap { it.sourceSets.asSequence() }
.flatMap { it.resDirectories.asSequence() }
.map { dir ->
// build relative path (e.g. src/main/res) and
// append a trailing "/" for include to treat it as recursive
projectPath.relativize(dir.toPath()).toString() + File.separator
})
}
.toList()
)

task.include(variants
.flatMap { it.sourceSets }
.asSequence()
.flatMap { it.sourceSets.asSequence() }
.map { it.manifestFile }
.map { file ->
// build relative path (e.g. src/main/AndroidManifest.xml)
projectPath.relativize(file.toPath()).toString()
})
}
.toList()
)
}
}
}
1 change: 1 addition & 0 deletions common/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ plugins {
id("org.jetbrains.kotlin.jvm")
id("org.gradle.groovy")
id("net.twisterrob.gradle.build.publishing")
id("net.twisterrob.gradle.build.detekt")
}

base.archivesName.set("twister-quality-common")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package net.twisterrob.gradle.common

import com.android.build.gradle.AppExtension
import com.android.build.gradle.BaseExtension
import com.android.build.gradle.LibraryExtension
import com.android.build.gradle.TestExtension
import org.gradle.api.Action
Expand All @@ -15,30 +16,28 @@ class AndroidVariantApplier(val project: Project) {
variantsClosure: Action<DomainObjectSet<out com.android.build.gradle.api.BaseVariant>>
) {
project.plugins.withId("com.android.application") {
val android = project.extensions.getByName<AppExtension>("android")
variantsClosure.execute(android.applicationVariants)
variantsClosure.execute(android<AppExtension>().applicationVariants)
}
project.plugins.withId("com.android.library") {
val android = project.extensions.getByName<LibraryExtension>("android")
variantsClosure.execute(android.libraryVariants)
variantsClosure.execute(android<LibraryExtension>().libraryVariants)
}
project.plugins.withId("com.android.feature") {
// These types of feature modules were deprecated and removed in AGP 4.x.
//val android = project.extensions["android"] as FeatureExtension
//variantsClosure.execute(android.featureVariants)
//variantsClosure.execute(android<FeatureExtension>().featureVariants)
}
project.plugins.withId("com.android.dynamic-feature") {
val android = project.extensions.getByName<AppExtension>("android")
variantsClosure.execute(android.applicationVariants)
variantsClosure.execute(android<AppExtension>().applicationVariants)
}
project.plugins.withId("com.android.test") {
val android = project.extensions.getByName<TestExtension>("android")
variantsClosure.execute(android.applicationVariants)
variantsClosure.execute(android<TestExtension>().applicationVariants)
}
project.plugins.withId("com.android.instantapp") {
//val android = project.extensions.getByName<InstantAppExtension>("android")
//android<InstantAppExtension>()
// has no variants, but don't call back, because there's no way to tell if this happened
//variantsClosure.execute(new DefaultDomainObjectSet<>(BaseVariant))
}
}

inline fun <reified T : BaseExtension> android(): T =
project.extensions.getByName<T>("android")
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ import java.io.File

open class BasePlugin : Plugin<Project> {

@Suppress("PropertyName", "MemberVisibilityCanBePrivate")
@Suppress("PropertyName", "MemberVisibilityCanBePrivate", "VariableNaming")
protected val LOG: Logger = LoggerFactory.getLogger(javaClass)

@Suppress("LateinitUsage") // Too many usages to fix right now. TODO consider removing this field, make dep explicit.
protected lateinit var project: Project

override fun apply(target: Project) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package net.twisterrob.gradle.common

import org.gradle.api.Action
import org.gradle.api.Project
import org.gradle.api.internal.file.pattern.PatternMatcherFactory
import org.gradle.api.tasks.SourceTask
import org.gradle.api.tasks.util.PatternSet

open class BaseQualityExtension<T>(
private val project: Project,
internal var taskConfigurator: Action<TaskConfigurator<T>> = Action {}
) where T : SourceTask {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ open class BaseQualityPlugin(
override fun apply(target: Project) {
super.apply(target)

@Suppress("CastToNullableType") // This is a lazy creation, so findByName is very likely null.
val quality = project.extensions.findByName("quality") as ExtensionAware?
?: project.extensions.create("quality", FakeQualityExtension::class.java) as ExtensionAware
quality.extensions.create(extensionName, extensionType, project)
quality.extensions.create(extensionName, extensionType)

// level of indirection with base is to prevent loading classes in project not having Android
project.plugins.withId("com.android.base") {
Expand Down
10 changes: 5 additions & 5 deletions common/src/main/kotlin/net/twisterrob/gradle/common/Utils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,16 @@ fun safeAdd(a: Int?, b: Int?): Int? =
fun nullSafeSum(): Collector<Int?, *, Int?> =
nullSafeSum(Function.identity())

fun <T> nullSafeSum(mapper: Function<T?, Int?>): Collector<T?, *, Int?> {
return Collectors.reducing(null, mapper, BinaryOperator(::safeAdd))
}
fun <T> nullSafeSum(mapper: Function<T?, Int?>): Collector<T?, *, Int?> =
Collectors.reducing(null, mapper, BinaryOperator(::safeAdd))

fun File.listFilesInDirectory(filter: ((File) -> Boolean)? = null): Array<File> {
val listFiles: Array<File>? =
if (filter != null)
if (filter != null) {
this.listFiles(filter)
else
} else {
this.listFiles()
}

return listFiles ?: error(
"$this does not denote a directory or an error occurred" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@ T : Reporting<out ReportContainer<out ConfigurableReport>>,
T : TargetChecker,
T : VerificationTask {

private lateinit var eachTask: TaskProvider<*>

private val checkerExtension: BaseQualityExtension<T>
get() {
val quality = project.extensions.getByName("quality") as ExtensionAware
Expand All @@ -45,8 +43,8 @@ T : VerificationTask {
variants: DomainObjectSet<out @Suppress("TYPEALIAS_EXPANSION_DEPRECATION" /* AGP 7.0 */) BaseVariant>
) {
project.plugins.apply(pluginName)
createGlobalTask()
variants.all(this::createTaskForVariant)
val eachTask = createGlobalTask()
variants.all { createTaskForVariant(it, eachTask) }
project.afterEvaluate {
project.tasks.register("${baseName}All", taskClass, variantsConfig(variants))
}
Expand All @@ -65,19 +63,20 @@ T : VerificationTask {
open fun taskConfigurator(): VariantTaskCreator<T>.DefaultTaskConfig =
DefaultTaskConfig()

private fun createGlobalTask() {
private fun createGlobalTask(): TaskProvider<Task> {
val globalTaskName = "${baseName}Each"
if (globalTaskName in project.tasks.names) {
return
return project.tasks.named(globalTaskName)
}
eachTask = project.tasks.register(globalTaskName) { task: Task ->
return project.tasks.register(globalTaskName) { task: Task ->
task.group = JavaBasePlugin.VERIFICATION_GROUP
task.description = "Run ${baseName} on each variant separately"
}
}

private fun createTaskForVariant(
variant: @Suppress("TYPEALIAS_EXPANSION_DEPRECATION" /* AGP 7.0 */) BaseVariant
variant: @Suppress("TYPEALIAS_EXPANSION_DEPRECATION" /* AGP 7.0 */) BaseVariant,
eachTask: TaskProvider<*>,
) {
val taskName = "${baseName}${variant.name.capitalize()}"
val variantTask = project.tasks.register(taskName, taskClass, variantConfig(variant))
Expand Down Expand Up @@ -181,13 +180,13 @@ T : VerificationTask {
?: error("Cannot find reporting extension, did you apply `reporting` plugin?")
val reportsDir = reporting.baseDirectory
with(task.reports) {
with(getByName("xml")) {
setRequired(true)
setOutputLocationCompat(reportsDir.file("${baseName}${fullSuffix}.xml"))
getByName("xml") { xml ->
xml.setRequired(true)
xml.setOutputLocationCompat(reportsDir.file("${baseName}${fullSuffix}.xml"))
}
with(getByName("html")) {
setRequired(true)
setOutputLocationCompat(reportsDir.file("${baseName}${fullSuffix}.html"))
getByName("html") { html ->
html.setRequired(true)
html.setOutputLocationCompat(reportsDir.file("${baseName}${fullSuffix}.html"))
}
}
}
Expand Down
1 change: 1 addition & 0 deletions compat/agp-40x/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
id("org.gradle.java-library")
id("org.jetbrains.kotlin.jvm")
id("net.twisterrob.gradle.build.publishing")
id("net.twisterrob.gradle.build.detekt")
}

base.archivesName.set("twister-compat-agp-4.0.x")
Expand Down
1 change: 1 addition & 0 deletions compat/agp-41x/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
id("org.gradle.java-library")
id("org.jetbrains.kotlin.jvm")
id("net.twisterrob.gradle.build.publishing")
id("net.twisterrob.gradle.build.detekt")
}

base.archivesName.set("twister-compat-agp-4.1.x")
Expand Down
1 change: 1 addition & 0 deletions compat/agp-42x/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ plugins {
id("org.gradle.java-library")
id("org.jetbrains.kotlin.jvm")
id("net.twisterrob.gradle.build.publishing")
id("net.twisterrob.gradle.build.detekt")
}

base.archivesName.set("twister-compat-agp-4.2.x")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ val Project.lintGlobalTasks: TaskCollection<LintGlobalTask>
get() = this.tasks.withType(LintGlobalTask::class.java)

fun TaskCollection<LintGlobalTask>.configureXmlReport() {
this.configureEach {
it.lintOptions.isAbortOnError = it.wasLaunchedExplicitly
this.configureEach { task ->
task.lintOptions.isAbortOnError = task.wasLaunchedExplicitly
// make sure we have xml output, otherwise can't figure out if it failed
it.lintOptions.xmlReport = true
task.lintOptions.xmlReport = true
}
}

fun TaskCollection<LintGlobalTask>.collectXmlReport(reportDiscovered: (RegularFileProperty) -> Unit) {
this.configureEach {
reportDiscovered(it.xmlOutputProperty)
this.configureEach { task ->
reportDiscovered(task.xmlOutputProperty)
}
}

Expand Down
Loading