From 984487e81b960acd2caf052f21900057f1a0a236 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 26 Nov 2024 03:27:08 +0000 Subject: [PATCH 1/7] Update dependency com.lemonappdev:konsist to v0.17.0 --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 7d29344cbc..6ef4e632ce 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -150,7 +150,7 @@ test_arch_core = "androidx.arch.core:core-testing:2.2.0" test_junit = "junit:junit:4.13.2" test_runner = "androidx.test:runner:1.6.2" test_mockk = "io.mockk:mockk:1.13.13" -test_konsist = "com.lemonappdev:konsist:0.16.1" +test_konsist = "com.lemonappdev:konsist:0.17.0" test_turbine = "app.cash.turbine:turbine:1.2.0" test_truth = "com.google.truth:truth:1.4.4" test_parameter_injector = "com.google.testparameterinjector:test-parameter-injector:1.18" From c433bab0a8d06e94b988a2cffbc42449ffd01cd2 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Nov 2024 10:15:53 +0100 Subject: [PATCH 2/7] Ensure `Sealed interface used in Composable MUST be Immutable or Stable` is detecting error by adding a failing case. --- ...mposableWithNonImmutableSealedInterface.kt | 20 +++++++++++++++++++ .../tests/konsist/KonsistArchitectureTest.kt | 12 +++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FailingComposableWithNonImmutableSealedInterface.kt diff --git a/tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FailingComposableWithNonImmutableSealedInterface.kt b/tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FailingComposableWithNonImmutableSealedInterface.kt new file mode 100644 index 0000000000..1e90577ca6 --- /dev/null +++ b/tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FailingComposableWithNonImmutableSealedInterface.kt @@ -0,0 +1,20 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.tests.konsist.failures + +import androidx.compose.runtime.Composable + +// Make test `Sealed interface used in Composable MUST be Immutable or Stable` fails + +sealed interface SealedInterface + +@Composable +fun FailingComposableWithNonImmutableSealedInterface( + sealedInterface: SealedInterface +) { +} diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt index 7cc848d825..365449a91d 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt @@ -22,6 +22,7 @@ import com.lemonappdev.konsist.api.ext.list.withoutName import com.lemonappdev.konsist.api.ext.list.withoutParents import com.lemonappdev.konsist.api.verify.assertEmpty import com.lemonappdev.konsist.api.verify.assertTrue +import org.junit.Assert.assertTrue import org.junit.Test class KonsistArchitectureTest { @@ -66,18 +67,18 @@ class KonsistArchitectureTest { @Test fun `Sealed interface used in Composable MUST be Immutable or Stable`() { + var failingTestFound = false // List all sealed interface without Immutable nor Stable annotation in the project val forbiddenInterfacesForComposableParameter = Konsist.scopeFromProject() .interfaces() .withSealedModifier() .withoutAnnotationOf(Immutable::class, Stable::class) .map { it.fullyQualifiedName } - Konsist.scopeFromProject() .functions() .withAnnotationOf(Composable::class) .assertTrue(additionalMessage = "Consider adding the @Immutable or @Stable annotation to the sealed interface") { - it.parameters.all { param -> + val result = it.parameters.all { param -> val type = param.type.text return@all if (type.startsWith("@") || type.startsWith("(") || type.startsWith("suspend")) { true @@ -94,6 +95,13 @@ class KonsistArchitectureTest { fullyQualifiedName !in forbiddenInterfacesForComposableParameter } } + if (!result && !failingTestFound && it.name == "FailingComposableWithNonImmutableSealedInterface") { + failingTestFound = true + true + } else { + result + } } + assertTrue("FailingComposableWithNonImmutableSealedInterface should make this test fail.", failingTestFound) } } From 1b490488a27e1fe23b0fe39e233fafb82e45cc56 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Nov 2024 10:29:54 +0100 Subject: [PATCH 3/7] Exclude non-prod code from quality checks. --- build.gradle.kts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 187b09de54..31edb5b08c 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -52,6 +52,10 @@ allprojects { detektPlugins("io.nlopez.compose.rules:detekt:0.4.19") } + tasks.withType().configureEach { + exclude("io/element/android/tests/konsist/failures/**") + } + // KtLint apply { plugin("org.jlleitschuh.gradle.ktlint") @@ -75,6 +79,7 @@ allprojects { val generatedPath = "${layout.buildDirectory.asFile.get()}/generated/" filter { exclude { element -> element.file.path.contains(generatedPath) } + exclude("io/element/android/tests/konsist/failures/**") } } // Dependency check From 878bd29b67572cc9d85f80df06b047ee830e9d80 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Nov 2024 10:48:01 +0100 Subject: [PATCH 4/7] Ensure `Fake classes must be named using Fake and the interface it fakes` is detecting error by adding failing cases. --- .../konsist/failures/FakeWrongClassName.kt | 22 +++++++++++++++++++ .../tests/konsist/KonsistClassNameTest.kt | 14 +++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FakeWrongClassName.kt diff --git a/tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FakeWrongClassName.kt b/tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FakeWrongClassName.kt new file mode 100644 index 0000000000..68ea557b53 --- /dev/null +++ b/tests/konsist/src/main/kotlin/io/element/android/tests/konsist/failures/FakeWrongClassName.kt @@ -0,0 +1,22 @@ +/* + * Copyright 2024 New Vector Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only + * Please see LICENSE in the repository root for full details. + */ + +package io.element.android.tests.konsist.failures + +// Make test `Fake classes must be named using Fake and the interface it fakes` fails + +interface MyInterface + +// This class should be named FakeMyInterface +class FakeWrongClassName : MyInterface + +class MyClass { + interface MyFactory +} + +// This class should be named FakeMyClassMyFactory +class FakeWrongClassSubInterfaceName : MyClass.MyFactory diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt index adc2b9ceb7..cc76c3fedd 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt @@ -73,6 +73,11 @@ class KonsistClassNameTest { @Test fun `Fake classes must be named using Fake and the interface it fakes`() { + var failingCases = 0 + val failingCasesList = listOf( + "FakeWrongClassName", + "FakeWrongClassSubInterfaceName", + ) Konsist.scopeFromProject() .classes() .withNameContaining("Fake") @@ -84,7 +89,7 @@ class KonsistClassNameTest { val interfaceName = it.name .replace("FakeRust", "") .replace("Fake", "") - (it.name.startsWith("Fake") || it.name.startsWith("FakeRust")) && + val result = (it.name.startsWith("Fake") || it.name.startsWith("FakeRust")) && it.parents().any { parent -> // Workaround to get the parent name. For instance: // parent.name used to return `UserListPresenter.Factory` but is now returning `Factory`. @@ -93,7 +98,14 @@ class KonsistClassNameTest { val parentName = parent.fullyQualifiedName!!.substringAfter("$packageName.").replace(".", "") parentName == interfaceName } + if (!result && it.name in failingCasesList) { + failingCases++ + true + } else { + result + } } + assertThat(failingCases).isEqualTo(failingCasesList.size) } @Test From e261f6f36ec1b07c32f9465689da9188519c3c3b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Nov 2024 11:09:38 +0100 Subject: [PATCH 5/7] Fix API break in `Fake classes must be named using Fake and the interface it fakes` It seems that the workaround is not necessary anymore. --- .../element/android/tests/konsist/KonsistClassNameTest.kt | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt index cc76c3fedd..9d8e23e7dc 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistClassNameTest.kt @@ -91,11 +91,7 @@ class KonsistClassNameTest { .replace("Fake", "") val result = (it.name.startsWith("Fake") || it.name.startsWith("FakeRust")) && it.parents().any { parent -> - // Workaround to get the parent name. For instance: - // parent.name used to return `UserListPresenter.Factory` but is now returning `Factory`. - // So we need to retrieve the name of the parent class differently. - val packageName = parent.packagee!!.name - val parentName = parent.fullyQualifiedName!!.substringAfter("$packageName.").replace(".", "") + val parentName = parent.name.replace(".", "") parentName == interfaceName } if (!result && it.name in failingCasesList) { From a84ca7ee8e29157a63fd17afd143bfebe2d64d46 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Nov 2024 11:39:33 +0100 Subject: [PATCH 6/7] Fix API break in `Sealed interface used in Composable MUST be Immutable or Stable` --- .../tests/konsist/KonsistArchitectureTest.kt | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt index 365449a91d..d334d88b55 100644 --- a/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt +++ b/tests/konsist/src/test/kotlin/io/element/android/tests/konsist/KonsistArchitectureTest.kt @@ -80,19 +80,22 @@ class KonsistArchitectureTest { .assertTrue(additionalMessage = "Consider adding the @Immutable or @Stable annotation to the sealed interface") { val result = it.parameters.all { param -> val type = param.type.text - return@all if (type.startsWith("@") || type.startsWith("(") || type.startsWith("suspend")) { + return@all if (type.startsWith("@") || type.contains("->") || type.startsWith("suspend")) { true } else { - var typePackage = param.type.declaration.packagee?.name - if (typePackage == type) { - // Workaround, now that packagee.fullyQualifiedName is not available anymore - // It seems that when the type in in the same package as the function, - // the package is equal to the type (which is wrong). - // So in this case, use the package of the function - typePackage = it.packagee?.name + val typePackage = param.type.sourceDeclaration?.let { declaration -> + declaration.asTypeParameterDeclaration()?.packagee + ?: declaration.asExternalDeclaration()?.packagee + ?: declaration.asClassOrInterfaceDeclaration()?.packagee + ?: declaration.asKotlinTypeDeclaration()?.packagee + ?: declaration.asObjectDeclaration()?.packagee + }?.name + if (typePackage == null) { + false + } else { + val fullyQualifiedName = "$typePackage.$type" + fullyQualifiedName !in forbiddenInterfacesForComposableParameter } - val fullyQualifiedName = "$typePackage.$type" - fullyQualifiedName !in forbiddenInterfacesForComposableParameter } } if (!result && !failingTestFound && it.name == "FailingComposableWithNonImmutableSealedInterface") { From c209245c2b657548859c746afc9914815596b45a Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 26 Nov 2024 15:59:29 +0100 Subject: [PATCH 7/7] Exclude Konsist code from Kover. --- plugins/src/main/kotlin/extension/KoverExtension.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/src/main/kotlin/extension/KoverExtension.kt b/plugins/src/main/kotlin/extension/KoverExtension.kt index ccc077b0a3..6fbb18e187 100644 --- a/plugins/src/main/kotlin/extension/KoverExtension.kt +++ b/plugins/src/main/kotlin/extension/KoverExtension.kt @@ -98,6 +98,8 @@ fun Project.setupKover() { "io.element.android.libraries.designsystem.theme.components.bottomsheet.*", // Test presenters "io.element.android.features.leaveroom.fake.FakeLeaveRoomPresenter", + // Konsist code to make test fails + "io.element.android.tests.konsist.failures", ) annotatedBy( "androidx.compose.ui.tooling.preview.Preview",