From 4737e4ff52ba736851f7d4b6429e684816989ed7 Mon Sep 17 00:00:00 2001 From: Petros Paraskevopoulos Date: Fri, 21 Mar 2025 16:59:12 +0200 Subject: [PATCH 1/6] Build: Group registered tasks on the root build gradle file --- build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build.gradle b/build.gradle index 5616aa4674c..26e17f9eb8f 100644 --- a/build.gradle +++ b/build.gradle @@ -57,6 +57,8 @@ allprojects { } } +/* DETEKT */ + tasks.register("detektAll", Detekt) { description = "Custom DETEKT build for all modules" parallel = true @@ -80,6 +82,8 @@ dependencies { detektPlugins project(':libs:detektrules') } +/* OTHER */ + tasks.register('clean', Delete) { delete rootProject.layout.buildDirectory } From 9de4465f463f90aa200cd36605056921e9148099 Mon Sep 17 00:00:00 2001 From: Petros Paraskevopoulos Date: Fri, 21 Mar 2025 17:02:26 +0200 Subject: [PATCH 2/6] Build: Add konsist module --- build.gradle | 9 ++++++++ gradle/libs.versions.toml | 2 ++ libs/konsist-tests/build.gradle | 37 +++++++++++++++++++++++++++++++++ settings.gradle | 2 +- 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 libs/konsist-tests/build.gradle diff --git a/build.gradle b/build.gradle index 26e17f9eb8f..0f6fc85264f 100644 --- a/build.gradle +++ b/build.gradle @@ -82,6 +82,15 @@ dependencies { detektPlugins project(':libs:detektrules') } +/* KONSIST */ + +tasks.register("konsist") { + group = "verification" + description = "Runs Konsist static code analysis." + + dependsOn(':libs:konsist-tests:testDebugUnitTest') +} + /* OTHER */ tasks.register('clean', Delete) { diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8a65f6b9d20..c25c42c0ad1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -81,6 +81,7 @@ jna = '5.5.0@aar' json-path = '2.9.0' junit = '4.13.2' lottie = '5.2.0' +konsist = '0.17.3' kotlin = '2.1.10' kotlinx-coroutines = '1.8.1' ksp = '2.1.10-1.0.29' @@ -242,6 +243,7 @@ jna = { module = "net.java.dev.jna:jna", version.ref = "jna" } json-path = { group = "com.jayway.jsonpath", name = "json-path", version.ref = "json-path" } junit = { group = "junit", name = "junit", version.ref = "junit" } lottie-compose = { group = "com.airbnb.android", name = "lottie-compose", version.ref = "lottie" } +konsist = { module = "com.lemonappdev:konsist", version.ref = "konsist" } kotlin-test-junit = { group = "org.jetbrains.kotlin", name = "kotlin-test-junit", version.ref = "kotlin" } kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "kotlinx-coroutines" } kotlinx-coroutines-core = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" } diff --git a/libs/konsist-tests/build.gradle b/libs/konsist-tests/build.gradle new file mode 100644 index 00000000000..15fe49c0478 --- /dev/null +++ b/libs/konsist-tests/build.gradle @@ -0,0 +1,37 @@ +import org.jetbrains.kotlin.gradle.dsl.JvmTarget + +plugins { + alias(libs.plugins.android.library) + alias(libs.plugins.kotlin.android) + alias(libs.plugins.dependency.analysis) +} + +kotlin { + sourceCompatibility = JvmTarget.fromTarget(libs.versions.java.get()).target + targetCompatibility = JvmTarget.fromTarget(libs.versions.java.get()).target +} + +android { + namespace 'com.woocommerce.android.konsist.tests' + + compileOptions { + sourceCompatibility libs.versions.java.get() + targetCompatibility libs.versions.java.get() + } + + defaultConfig { + minSdkVersion gradle.ext.minSdkVersion + targetSdkVersion gradle.ext.targetSdkVersion + compileSdk gradle.ext.compileSdkVersion + } +} + +dependencies { + testImplementation(libs.junit) + testImplementation(libs.konsist) +} + +// See: https://docs.konsist.lemonappdev.com/advanced/isolate-konsist-tests#solution-1-module-is-always-out-of-date +tasks.withType(Test).configureEach { + outputs.upToDateWhen { false } +} diff --git a/settings.gradle b/settings.gradle index 6c3c2926eb9..a276cfbe5ec 100644 --- a/settings.gradle +++ b/settings.gradle @@ -176,7 +176,7 @@ include ':libs:commons', ':libs:cardreader', ':libs:apifaker' include ':libs:fluxc', ':libs:fluxc-annotations', ':libs:fluxc-processor', ':libs:fluxc-plugin', ':libs:fluxc-tests' include ':libs:login' include ':WooCommerce', ':WooCommerce-Wear' -include ':libs:detektrules' +include ':libs:detektrules', ':libs:konsist-tests' gradle.ext.fluxCBinaryPath = "org.wordpress:fluxc" gradle.ext.fluxCWooCommercePluginBinaryPath = "org.wordpress.fluxc.plugins:woocommerce" From e9bbab1d9e33872c4cc956ca69272df64c0a2fd1 Mon Sep 17 00:00:00 2001 From: Petros Paraskevopoulos Date: Fri, 21 Mar 2025 17:02:52 +0200 Subject: [PATCH 3/6] Analysis: Add konsist tests --- libs/konsist-tests/build.gradle | 10 +++ .../konsist/test/KonsistAnnotationsTest.kt | 32 ++++++++ .../konsist/test/KonsistComposeTest.kt | 17 ++++ .../android/konsist/test/KonsistFieldsTest.kt | 19 +++++ .../android/konsist/test/KonsistFilesTest.kt | 14 ++++ .../konsist/test/KonsistFunctionsTest.kt | 25 ++++++ .../konsist/test/KonsistGenericsTest.kt | 21 +++++ .../konsist/test/KonsistImportsTest.kt | 14 ++++ .../android/konsist/test/KonsistLogsTest.kt | 26 ++++++ .../android/konsist/test/KonsistNamingTest.kt | 46 +++++++++++ .../konsist/test/KonsistPackagesTest.kt | 36 +++++++++ .../konsist/test/KonsistParametersTest.kt | 18 +++++ .../konsist/test/KonsistPropertiesTest.kt | 38 +++++++++ .../android/konsist/test/KonsistTestsTest.kt | 81 +++++++++++++++++++ .../konsist/test/KonsistUseCasesTest.kt | 21 +++++ 15 files changed, 418 insertions(+) create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistAnnotationsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFieldsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFilesTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFunctionsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistGenericsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistLogsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistNamingTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPackagesTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistParametersTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPropertiesTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistTestsTest.kt create mode 100644 libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistUseCasesTest.kt diff --git a/libs/konsist-tests/build.gradle b/libs/konsist-tests/build.gradle index 15fe49c0478..f6811736f87 100644 --- a/libs/konsist-tests/build.gradle +++ b/libs/konsist-tests/build.gradle @@ -27,6 +27,16 @@ android { } dependencies { + implementation(libs.androidx.lifecycle.viewmodel.savedstate) + implementation(libs.androidx.appcompat) + implementation(libs.androidx.fragment.ktx) + implementation(libs.google.dagger.compiler) + implementation(libs.google.dagger.hilt.compiler) + implementation(libs.google.dagger.hilt.android.main) + implementation(libs.google.dagger.hilt.android.testing) + implementation(platform(libs.androidx.compose.bom)) + implementation(libs.androidx.compose.ui.tooling.main) + testImplementation(libs.junit) testImplementation(libs.konsist) } diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistAnnotationsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistAnnotationsTest.kt new file mode 100644 index 00000000000..fde4b391917 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistAnnotationsTest.kt @@ -0,0 +1,32 @@ +package com.woocommerce.android.konsist.test + +import android.app.Application +import androidx.fragment.app.Fragment +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.properties +import com.lemonappdev.konsist.api.ext.list.withoutAllParentsNamed +import com.lemonappdev.konsist.api.ext.list.withoutAllParentsOf +import com.lemonappdev.konsist.api.ext.list.withoutAnnotationOf +import com.lemonappdev.konsist.api.ext.list.withoutName +import com.lemonappdev.konsist.api.ext.provider.hasAnnotationOf +import com.lemonappdev.konsist.api.verify.assertFalse +import dagger.hilt.android.AndroidEntryPoint +import dagger.hilt.android.HiltAndroidApp +import dagger.hilt.android.testing.HiltAndroidTest +import org.junit.Test +import javax.inject.Inject + +class KonsistAnnotationsTest { + @Test // Or suppress 'AppInitializer' class: @Suppress("konsist.no class should use field injection") + fun `no class should use field injection`() { + Konsist.scopeFromProject() + .classes() + .withoutAnnotationOf(HiltAndroidApp::class, AndroidEntryPoint::class, HiltAndroidTest::class) + .withoutAllParentsOf(Application::class) + .withoutAllParentsOf(Fragment::class) + .withoutAllParentsNamed("BaseFragment") + .withoutName("AppInitializer") + .properties() + .assertFalse { it.hasAnnotationOf() } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt new file mode 100644 index 00000000000..c0bbd8fa17f --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt @@ -0,0 +1,17 @@ +package com.woocommerce.android.konsist.test + +import androidx.compose.ui.tooling.preview.Preview +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.withAnnotationOf +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistComposeTest { + @Test + fun `all jetpack compose previews contain 'preview' in method name`() { + Konsist.scopeFromProject() + .functions() + .withAnnotationOf(Preview::class) + .assertTrue { it.hasNameContaining("Preview") } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFieldsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFieldsTest.kt new file mode 100644 index 00000000000..883e53c8623 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFieldsTest.kt @@ -0,0 +1,19 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.properties +import com.lemonappdev.konsist.api.verify.assertFalse +import org.junit.Test + +class KonsistFieldsTest { + @Test + fun `no field should have 'm' prefix`() { + Konsist.scopeFromProject() + .classes() + .properties() + .assertFalse { + val secondCharacterIsUppercase = it.name.getOrNull(1)?.isUpperCase() ?: false + it.name.startsWith('m') && secondCharacterIsUppercase + } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFilesTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFilesTest.kt new file mode 100644 index 00000000000..0fabe72050b --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFilesTest.kt @@ -0,0 +1,14 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.verify.assertFalse +import org.junit.Test + +class KonsistFilesTest { + @Test + fun `no empty files allowed`() { + Konsist.scopeFromProject() + .files + .assertFalse { it.text.isEmpty() } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFunctionsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFunctionsTest.kt new file mode 100644 index 00000000000..be243819949 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistFunctionsTest.kt @@ -0,0 +1,25 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.functions +import com.lemonappdev.konsist.api.ext.list.returnTypes +import com.lemonappdev.konsist.api.ext.list.withoutSourceSet +import com.lemonappdev.konsist.api.verify.assertFalse +import org.junit.Test + +class KonsistFunctionsTest { + @Test // Or suppress all files: @file:Suppress("konsist.return type of all functions are immutable") + fun `return type of all functions - expect in a test - are immutable`() { + Konsist.scopeFromProject() + .files + .filter { file -> + !file.path.contains("FlowExt.kt") && + !file.path.contains("LiveDataExt.kt") && + !file.path.contains("SavedStateFlow.kt") + } + .functions() + .returnTypes + .withoutSourceSet("test") + .assertFalse { it.isMutableType } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistGenericsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistGenericsTest.kt new file mode 100644 index 00000000000..fd1cf520191 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistGenericsTest.kt @@ -0,0 +1,21 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.declaration.flatten +import com.lemonappdev.konsist.api.ext.list.types +import com.lemonappdev.konsist.api.verify.assertFalse +import org.junit.Test + +class KonsistGenericsTest { + @Test + fun `property generic type does not contains star projection`() { + Konsist.scopeFromProduction() + .properties() + .types + .assertFalse { type -> + type.typeArguments + ?.flatten() + ?.any { it.isStarProjection } + } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt new file mode 100644 index 00000000000..8ded98c82f0 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt @@ -0,0 +1,14 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.verify.assertFalse +import org.junit.Test + +class KonsistImportsTest { + @Test // Detekt: WildcardImport (https://detekt.dev/docs/rules/style/#wildcardimport) + fun `no wildcard imports allowed`() { + Konsist.scopeFromProject() + .imports + .assertFalse { it.isWildcard } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistLogsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistLogsTest.kt new file mode 100644 index 00000000000..9852e3fa8ae --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistLogsTest.kt @@ -0,0 +1,26 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.withoutSourceSet +import com.lemonappdev.konsist.api.verify.assertFalse +import org.junit.Test + +class KonsistLogsTest { + @Test + fun `no class should use java util logging`() { + Konsist.scopeFromProject() + .files + .assertFalse { it.hasImport { import -> import.name == "java.util.logging.." } } + } + + @Test // Or suppress 'WooLog' file: @file:Suppress("konsist.return type of all functions are immutable") + fun `no class should use android util logging`() { + Konsist.scopeFromProject() + .files + .filter { file -> + !file.path.contains("WooLog.kt") + } + .withoutSourceSet("androidTest") + .assertFalse { it.hasImport { import -> import.name == "android.util.Log" } } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistNamingTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistNamingTest.kt new file mode 100644 index 00000000000..f52988141fe --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistNamingTest.kt @@ -0,0 +1,46 @@ +package com.woocommerce.android.konsist.test + +import androidx.appcompat.app.AppCompatActivity +import androidx.fragment.app.Fragment +import androidx.lifecycle.ViewModel +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.withAllParentsNamed +import com.lemonappdev.konsist.api.ext.list.withAllParentsOf +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistNamingTest { + @Test + fun `android activity class name ends with 'activity'`() { + Konsist.scopeFromProject() + .classes() + .withAllParentsOf(AppCompatActivity::class) + .assertTrue { it.name.endsWith("Activity") } + } + + @Test + fun `android fragment class name ends with 'fragment'`() { + Konsist.scopeFromProject() + .classes() + .withAllParentsOf(Fragment::class) + .plus( + Konsist.scopeFromProject() + .classes() + .withAllParentsNamed("BaseFragment") + ) + .assertTrue { it.name.endsWith("Fragment") } + } + + @Test + fun `android view model class name ends with 'viewmodel'`() { + Konsist.scopeFromProject() + .classes() + .withAllParentsOf(ViewModel::class) + .plus( + Konsist.scopeFromProject() + .classes() + .withAllParentsNamed("ScopedViewModel") + ) + .assertTrue { it.name.endsWith("ViewModel") } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPackagesTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPackagesTest.kt new file mode 100644 index 00000000000..ab39545f4f3 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPackagesTest.kt @@ -0,0 +1,36 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.withNameEndingWith +import com.lemonappdev.konsist.api.ext.list.withPackage +import com.lemonappdev.konsist.api.ext.list.withoutName +import com.lemonappdev.konsist.api.ext.list.withoutSourceSet +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistPackagesTest { + @Test + fun `files in 'extensions' package must have name ending with 'ext'`() { + Konsist.scopeFromProject() + .files + .withPackage("..extensions..") + .withoutSourceSet("test") + .assertTrue { it.hasNameEndingWith("Ext") } + } + + @Test // Detekt: InvalidPackageDeclaration (https://detekt.dev/docs/rules/naming/#invalidpackagedeclaration) + fun `package name must match file path`() { + Konsist.scopeFromProject() + .packages + .withoutName("org.wordpress.android.util.config") + .assertTrue { it.hasMatchingPath } + } + + @Test + fun `classes with 'usecase' suffix should reside in 'usecases' package`() { + Konsist.scopeFromProject() + .classes() + .withNameEndingWith("UseCase") + .assertTrue { it.resideInPackage("..usecases..") } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistParametersTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistParametersTest.kt new file mode 100644 index 00000000000..b0c18b2fe8f --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistParametersTest.kt @@ -0,0 +1,18 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.modifierprovider.withValueModifier +import com.lemonappdev.konsist.api.ext.list.primaryConstructors +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistParametersTest { + @Test + fun `every value class has parameter named 'value'`() { + Konsist.scopeFromProject() + .classes() + .withValueModifier() + .primaryConstructors + .assertTrue { it.hasParameterWithName("value") } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPropertiesTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPropertiesTest.kt new file mode 100644 index 00000000000..3fe95ff87b0 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistPropertiesTest.kt @@ -0,0 +1,38 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.declaration.KoFunctionDeclaration +import com.lemonappdev.konsist.api.declaration.KoPropertyDeclaration +import com.lemonappdev.konsist.api.ext.list.indexOfFirstInstance +import com.lemonappdev.konsist.api.ext.list.indexOfLastInstance +import com.lemonappdev.konsist.api.ext.list.withoutSourceSet +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistPropertiesTest { + @Test + fun `properties - expect in a test - are declared before functions`() { + Konsist.scopeFromProject() + .classes() + .withoutSourceSet("test") + .assertTrue { + val lastKoPropertyDeclarationIndex = it + .declarations( + includeNested = false, + includeLocal = false, + ) + .indexOfLastInstance() + val firstKoFunctionDeclarationIndex = it + .declarations( + includeNested = false, + includeLocal = false, + ) + .indexOfFirstInstance() + if (lastKoPropertyDeclarationIndex != -1 && firstKoFunctionDeclarationIndex != -1) { + lastKoPropertyDeclarationIndex < firstKoFunctionDeclarationIndex + } else { + true + } + } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistTestsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistTestsTest.kt new file mode 100644 index 00000000000..07a2e4bd449 --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistTestsTest.kt @@ -0,0 +1,81 @@ +package com.woocommerce.android.konsist.test + +import androidx.lifecycle.ViewModel +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.withAllParentsNamed +import com.lemonappdev.konsist.api.ext.list.withAllParentsOf +import com.lemonappdev.konsist.api.ext.list.withName +import com.lemonappdev.konsist.api.ext.list.withNameEndingWith +import com.lemonappdev.konsist.api.ext.list.withoutName +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistTestsTest { + @Test // Or suppress 'ScopedViewModel' class: @Suppress("konsist.every view model class has test") + fun `every view model class has test`() { + Konsist.scopeFromProduction() + .classes() + .withAllParentsOf(ViewModel::class) + .plus( + Konsist.scopeFromProject() + .classes() + .withAllParentsNamed("ScopedViewModel") + ) + .withoutName("ScopedViewModel") + .assertTrue { viewModelClass -> + Konsist.scopeFromProject() + .classes() + .withName("${viewModelClass.name}Test") + .isNotEmpty() + } + } + + @Test + fun `every use case class has test`() { + Konsist.scopeFromProduction() + .classes() + .withNameEndingWith("UseCase") + .assertTrue { useCaseClass -> + Konsist.scopeFromProject() + .classes() + .withName("${useCaseClass.name}Test") + .isNotEmpty() + } + } + + @Test + fun `every repository class has test`() { + Konsist.scopeFromProduction() + .classes() + .withNameEndingWith("Repository") + .assertTrue { repositoryClass -> + Konsist.scopeFromProject() + .classes() + .withName("${repositoryClass.name}Test") + .isNotEmpty() + } + } + + @Test + fun `test classes should have test subject named sut`() { + Konsist.scopeFromTest() + .classes() + .assertTrue { + val type = it.name.removeSuffix("Test") + val sut = it + .properties() + .firstOrNull { property -> property.name == "sut" } + sut != null && (sut.type?.name == type || sut.text.contains("$type(")) + } + } + + @Test + fun `classes with 'test' annotation should have 'test' suffix`() { + Konsist.scopeFromSourceSet("test") + .classes() + .filter { + it.functions().any { func -> func.hasAnnotationOf(Test::class) } + } + .assertTrue { it.hasNameEndingWith("Test") } + } +} diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistUseCasesTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistUseCasesTest.kt new file mode 100644 index 00000000000..3949abb83bf --- /dev/null +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistUseCasesTest.kt @@ -0,0 +1,21 @@ +package com.woocommerce.android.konsist.test + +import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.withNameEndingWith +import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Test + +class KonsistUseCasesTest { + @Test + fun `classes with 'usecase' suffix should have single 'public operator' method named 'invoke'`() { + Konsist.scopeFromProject() + .classes() + .withNameEndingWith("UseCase") + .assertTrue { + val hasSingleInvokeOperatorMethod = it.hasFunction { function -> + function.name == "invoke" && function.hasPublicOrDefaultModifier && function.hasOperatorModifier + } + hasSingleInvokeOperatorMethod && it.countFunctions { item -> item.hasPublicOrDefaultModifier } == 1 + } + } +} From 1e8d3bbdfc65a1343419af9a00ae0d70433c6f58 Mon Sep 17 00:00:00 2001 From: Petros Paraskevopoulos Date: Fri, 21 Mar 2025 17:04:21 +0200 Subject: [PATCH 4/6] CI: Add konsist jon on ci FYI: This change includes adding this extra script related '-x libs:konsist-test:testDebugUnitTest' configuration on unit tests so that 'Konsist' tests are excluded from running as part of all the other "normal" unit tests. --- .buildkite/commands/run-unit-tests.sh | 2 +- .buildkite/pipeline.yml | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.buildkite/commands/run-unit-tests.sh b/.buildkite/commands/run-unit-tests.sh index a464acf103c..ebb57965585 100755 --- a/.buildkite/commands/run-unit-tests.sh +++ b/.buildkite/commands/run-unit-tests.sh @@ -10,7 +10,7 @@ bundle exec fastlane run configure_apply echo "--- ๐Ÿงช Testing" set +e -./gradlew testJalapenoDebugUnitTest testDebugUnitTest +./gradlew testJalapenoDebugUnitTest testDebugUnitTest -x libs:konsist-test:testDebugUnitTest TESTS_EXIT_STATUS=$? set -e diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 5547a08e657..c713cb857b0 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -34,6 +34,13 @@ steps: agents: queue: "linter" + - label: "konsist" + command: ./gradlew konsist + plugins: [$CI_TOOLKIT] + artifact_paths: + - "**/build/reports/tests/**/*" + - "**/build/test-results/*/*.xml" + - label: "detekt" command: ./gradlew detektAll plugins: [$CI_TOOLKIT] From bbe79c1a67e7d551f94ca7911894fdb9c12cf456 Mon Sep 17 00:00:00 2001 From: Petros Paraskevopoulos Date: Fri, 28 Mar 2025 15:23:01 +0200 Subject: [PATCH 5/6] Analysis: Convert woopos design system btn/text usage rules to konsist This commit demonstrates how an existing custom Detekt rule can be easily converted to Konsist, and, at the same time showcases how Konsist's declarative nature makes it easier to write and maintain such checks. --- .../ui/woopos/home/cart/WooPosCartScreen.kt | 12 ++++++---- .../konsist/test/KonsistImportsTest.kt | 24 +++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt index dba93662e94..b6dfb2c380b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt @@ -25,9 +25,11 @@ import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material3.Button import androidx.compose.material3.Icon import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -60,7 +62,6 @@ import com.woocommerce.android.R import com.woocommerce.android.extensions.isNotNullOrEmpty import com.woocommerce.android.ui.woopos.common.composeui.WooPosPreview import com.woocommerce.android.ui.woopos.common.composeui.component.ShadowType -import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosButton import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosCard import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosLazyColumn import com.woocommerce.android.ui.woopos.common.composeui.component.WooPosOutlinedButtonSmall @@ -162,10 +163,13 @@ private fun WooPosCartScreen( end.linkTo(parent.end) } ) { - WooPosButton( - text = stringResource(R.string.woopos_checkout_button), + Button( onClick = { onUIEvent(WooPosCartUIEvent.CheckoutClicked) } - ) + ) { + Text( + text = stringResource(R.string.woopos_checkout_button) + ) + } } } } diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt index 8ded98c82f0..a31d10f5f5e 100644 --- a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistImportsTest.kt @@ -1,9 +1,11 @@ package com.woocommerce.android.konsist.test import com.lemonappdev.konsist.api.Konsist +import com.lemonappdev.konsist.api.ext.list.imports import com.lemonappdev.konsist.api.verify.assertFalse import org.junit.Test +@Suppress("MaxLineLength") class KonsistImportsTest { @Test // Detekt: WildcardImport (https://detekt.dev/docs/rules/style/#wildcardimport) fun `no wildcard imports allowed`() { @@ -11,4 +13,26 @@ class KonsistImportsTest { .imports .assertFalse { it.isWildcard } } + + @Test // Custom Detekt: WooPosDesignSystemButtonUsageRule (https://github.com/woocommerce/woocommerce-android/blob/trunk/libs/detektrules/src/main/kotlin/com/woocommerce/android/detektrules/woopos/WooPosDesignSystemButtonUsageRule.kt#L13) + fun `woopos package - no standard compose buttons imports allowed - use woopos button instead`() { + Konsist.scopeFromPackage("com.woocommerce.android.ui.woopos..") + .files + .filter { file -> + !file.path.contains("WooPosButtons.kt") + } + .imports + .assertFalse { it.hasNameMatching("""^androidx\.compose\.material3\.Button${'$'}""".toRegex()) } + } + + @Test // Custom Detekt: WooPosDesignSystemTextUsageRule (https://github.com/woocommerce/woocommerce-android/blob/trunk/libs/detektrules/src/main/kotlin/com/woocommerce/android/detektrules/woopos/WooPosDesignSystemTextUsageRule.kt#L13) + fun `woopos package - no standard compose text imports allowed - use woopos text instead`() { + Konsist.scopeFromPackage("com.woocommerce.android.ui.woopos..") + .files + .filter { file -> + !file.path.contains("WooPosTexts.kt") + } + .imports + .assertFalse { it.hasNameMatching("""^androidx\.compose\.material3\.Text${'$'}""".toRegex()) } + } } From 5f5445c3dd72fab261b8d7f240b1a7ef01069007 Mon Sep 17 00:00:00 2001 From: Petros Paraskevopoulos Date: Fri, 28 Mar 2025 16:18:29 +0200 Subject: [PATCH 6/6] Analysis: Convert compose view model guideline to konsist This commit demonstrates how an existing documented Compose guideline can be easily converted to Konstist, and, at the same time showcases how adding a KDoc on top of this custom test/rule make this a living document. --- .../ui/woopos/home/cart/WooPosCartScreen.kt | 7 ++-- .../konsist/test/KonsistComposeTest.kt | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt index b6dfb2c380b..1bdcf86e867 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/cart/WooPosCartScreen.kt @@ -74,9 +74,10 @@ import com.woocommerce.android.ui.woopos.common.composeui.designsystem.WooPosTyp import com.woocommerce.android.ui.woopos.common.composeui.designsystem.toAdaptivePadding @Composable -fun WooPosCartScreen(modifier: Modifier = Modifier) { - val viewModel: WooPosCartViewModel = hiltViewModel() - +fun WooPosCartScreen( + modifier: Modifier = Modifier, + viewModel: WooPosCartViewModel = hiltViewModel() +) { viewModel.state.observeAsState().value?.let { WooPosCartScreen(modifier, it, viewModel::onUIEvent) } diff --git a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt index c0bbd8fa17f..c0655f3965a 100644 --- a/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt +++ b/libs/konsist-tests/src/test/java/com/woocommerce/android/konsist/test/KonsistComposeTest.kt @@ -1,11 +1,13 @@ package com.woocommerce.android.konsist.test +import androidx.compose.runtime.Composable import androidx.compose.ui.tooling.preview.Preview import com.lemonappdev.konsist.api.Konsist import com.lemonappdev.konsist.api.ext.list.withAnnotationOf import com.lemonappdev.konsist.api.verify.assertTrue import org.junit.Test +@Suppress("MaxLineLength") class KonsistComposeTest { @Test fun `all jetpack compose previews contain 'preview' in method name`() { @@ -14,4 +16,39 @@ class KonsistComposeTest { .withAnnotationOf(Preview::class) .assertTrue { it.hasNameContaining("Preview") } } + + /** + * [Composable Functions Best Practices โœ…](https://github.com/woocommerce/woocommerce-android/blob/trunk/docs/compose.md#composable-functions-best-practices--) + * + * Don't acquire the viewModel inside a composable function, this will make testing harder. Inject it as a parameter + * and provide a default value to facilitate reusability: + * + * โŒ + * + * ``` + * @Composable + * fun MyComposable() { + * val viewModel by viewModel() + * ... + * } + * ``` + * + * โœ… + * + * ``` + * @Composable + * fun MyComposable(viewModel : MyViewModel = getViewModel()) { + * ... + * } + * ``` + */ + @Test + fun `composable functions should not acquire view model directly`() { + Konsist.scopeFromProject() + .functions() + .withAnnotationOf(Composable::class) + .assertTrue { function -> + !function.text.contains("""(?i)val\s+\w*viewModel""".toRegex()) + } + } }