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.

Sign up for GitHub

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

build(deps): Bump the kotlin-ksp group with 11 updates #21515

Closed
wants to merge 9 commits into from
Closed

build(deps): Bump the kotlin-ksp group with 11 updates #21515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ffcae27
build(deps): Bump the kotlin-ksp group with 11 updates
dependabot[bot] Dec 10, 2024
57abfc7
Merge branch 'trunk' into dependabot/gradle/kotlin-ksp-31007f7a9e
nbradbury Dec 10, 2024
e41ac16
Use uppercase and lowercase
nbradbury Dec 10, 2024
8b88057
Merge remote-tracking branch 'origin/dependabot/gradle/kotlin-ksp-310…
nbradbury Dec 10, 2024
81024b1
Fixed getSerializableCompat warning
nbradbury Dec 10, 2024
0b94318
Fixed "Specific imports should be used for FluxC Store inner classes"…
nbradbury Dec 10, 2024
cc33adc
Explicitly don't use KSP2
wzieba Dec 11, 2024
631549d
Replace kotlin compile testing with a fork
wzieba Dec 11, 2024
ca1d061
Removed locale
nbradbury Dec 12, 2024
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
7 changes: 4 additions & 3 deletions WordPress/src/main/java/org/wordpress/android/ui/mysite/MySiteFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import org.wordpress.android.WordPress
import org.wordpress.android.analytics.AnalyticsTracker
import org.wordpress.android.databinding.MySiteFragmentBinding
import org.wordpress.android.fluxc.store.AccountStore
import org.wordpress.android.fluxc.store.QuickStartStore
import org.wordpress.android.fluxc.store.QuickStartStore.QuickStartTask
import org.wordpress.android.ui.ActivityLauncher
import org.wordpress.android.ui.ActivityNavigator
import org.wordpress.android.ui.FullScreenDialogFragment
Expand Down Expand Up @@ -86,6 +86,7 @@ import org.wordpress.android.viewmodel.main.WPMainActivityViewModel
import org.wordpress.android.viewmodel.observeEvent
import org.wordpress.android.viewmodel.pages.PageListViewModel
import java.io.File
import java.io.Serializable
import javax.inject.Inject

@Suppress("LargeClass")
Expand Down Expand Up @@ -211,8 +212,8 @@ class MySiteFragment : Fragment(R.layout.my_site_fragment),
override fun onConfirm(result: Bundle?) {
val task = result?.getSerializableCompat(
QuickStartFullScreenDialogFragment.RESULT_TASK
) as? QuickStartStore.QuickStartTask
task?.let { viewModel.onQuickStartTaskCardClick(it) }
) as? Serializable
(task as? QuickStartTask)?.let { viewModel.onQuickStartTaskCardClick(it) }
}

override fun onDismiss() {
Expand Down
2 changes: 2 additions & 0 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@ android.nonTransitiveRClass=true
android.nonFinalResIds=false
android.enableR8.fullMode=false

ksp.useKSP2=false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the commit message and your reasoning about this change @wzieba , makes this all very clear! 🙇

...but it ended up not to be.

I propose setting this flag to false to explicitly describe which KSP version we use, as I haven't found any way to log this.

But just to make it clear here, this ksp.useKSP2 flag is false by default atm, even on Kotlin 2.1.0? We just make this explicit with this change, right? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There was plan to make this true with Kotlin 2.1, but apparently, this changed. So due to this uncertainty, I think it's best we hardcode usage of KSP1 for now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks and I agree, good call @wzieba ! 👍

FYI: Maybe we should propagate the same (explicit) configuration to all other client projects. 🤔

PS: We should also just make sure to revisit that at some point and flip it to true when everything starts working on the KSP2 side again, maybe we should add that as an Apps Infra Roadmap so we don't forget. 🤔


# Dependency Analysis Plugin
dependency.analysis.android.ignored.variants=release,wordpressVanillaDebug,wordpressVanillaRelease,wordpressWasabiDebug,wordpressWasabiRelease,wordpressJalapenoRelease,jetpackVanillaDebug,jetpackVanillaRelease,jetpackWasabiDebug,jetpackWasabiRelease,jetpackJalapenoRelease
10 changes: 5 additions & 5 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ jna = '5.15.0@aar'
json-path = '2.9.0'
jsoup = '1.18.3'
junit = '4.13.2'
kotlin-compile-testing = '1.6.0'
kotlin-main = '2.0.21'
kotlin-compile-testing = '0.7.0'
kotlin-main = '2.1.0'
kotlinx-coroutines = '1.8.1'
kotlinx-kover = '0.7.6'
ksp = '2.0.21-1.0.28'
ksp = '2.1.0-1.0.29'
mockito-android = '5.14.2'
mockito-kotlin = '4.1.0'
mpandroidchart = 'v3.1.0'
Expand Down Expand Up @@ -232,8 +232,8 @@ jna = { module = "net.java.dev.jna:jna", version.ref = "jna" }
json-path = { group = "com.jayway.jsonpath", name = "json-path", version.ref = "json-path" }
jsoup = { group = "org.jsoup", name = "jsoup", version.ref = "jsoup" }
junit = { group = "junit", name = "junit", version.ref = "junit" }
kotlin-compile-testing-ksp = { group = "com.github.tschuchortdev", name = "kotlin-compile-testing-ksp", version.ref = "kotlin-compile-testing" }
kotlin-compile-testing-main = { group = "com.github.tschuchortdev", name = "kotlin-compile-testing", version.ref = "kotlin-compile-testing" }
kotlin-compile-testing-ksp = { group = "dev.zacsweers.kctfork", name = "ksp", version.ref = "kotlin-compile-testing" }
kotlin-compile-testing-main = { group = "dev.zacsweers.kctfork", name = "core", version.ref = "kotlin-compile-testing" }
kotlin-reflect = { group = "org.jetbrains.kotlin", name = "kotlin-reflect", version.ref = "kotlin-main" }
kotlin-test-junit = { group = "org.jetbrains.kotlin", name = "kotlin-test-junit", version.ref = "kotlin-main" }
kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "kotlinx-coroutines" }
Expand Down
4 changes: 1 addition & 3 deletions libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/list/ListOrder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package org.wordpress.android.fluxc.model.list

import java.util.Locale

enum class ListOrder(val value: String) {
ASC("ASC"),
DESC("DESC");
companion object {
fun fromValue(value: String): ListOrder? {
return values().firstOrNull { it.value.toLowerCase(Locale.ROOT) == value.toLowerCase(Locale.ROOT) }
return values().firstOrNull { it.value.lowercase() == value.lowercase() }
}
}
}
3 changes: 1 addition & 2 deletions libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/list/PostListDescriptor.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import org.wordpress.android.fluxc.model.list.ListOrder.DESC
import org.wordpress.android.fluxc.model.list.PostListOrderBy.DATE
import org.wordpress.android.fluxc.model.post.PostStatus
import org.wordpress.android.fluxc.store.PostStore.DEFAULT_POST_STATUS_LIST
import java.util.Locale

sealed class PostListDescriptor(
val site: SiteModel,
Expand Down Expand Up @@ -105,7 +104,7 @@ enum class PostListOrderBy(val value: String) {

companion object {
fun fromValue(value: String): PostListOrderBy? {
return values().firstOrNull { it.value.toLowerCase(Locale.ROOT) == value.toLowerCase(Locale.ROOT) }
return values().firstOrNull { it.value.lowercase() == value.lowercase() }
}
}
}
5 changes: 2 additions & 3 deletions libs/fluxc/src/main/java/org/wordpress/android/fluxc/model/notification/NotificationModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package org.wordpress.android.fluxc.model.notification

import org.wordpress.android.fluxc.tools.FormattableContent
import org.wordpress.android.fluxc.tools.FormattableMeta
import java.util.Locale

data class NotificationModel(
val noteId: Int = 0,
Expand Down Expand Up @@ -45,7 +44,7 @@ data class NotificationModel(
companion object {
private val reverseMap = values().associateBy(
Kind::name)
fun fromString(type: String) = reverseMap[type.uppercase(Locale.US)] ?: UNKNOWN
fun fromString(type: String) = reverseMap[type.uppercase()] ?: UNKNOWN
}
}

Expand All @@ -62,7 +61,7 @@ data class NotificationModel(
return if (type.isEmpty()) {
NONE
} else {
reverseMap[type.toUpperCase(Locale.US)] ?: UNKNOWN
reverseMap[type.uppercase()] ?: UNKNOWN
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions ...c/src/main/java/org/wordpress/android/fluxc/network/rest/wpcom/site/BooleanTypeAdapter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.google.gson.JsonDeserializer
import com.google.gson.JsonElement
import com.google.gson.JsonParseException
import java.lang.reflect.Type
import java.util.Locale

internal class BooleanTypeAdapter : JsonDeserializer<Boolean?> {
@Suppress("VariableNaming") private val TRUE_STRINGS: Set<String> = HashSet(listOf("true", "1", "yes"))
Expand All @@ -16,9 +15,9 @@ internal class BooleanTypeAdapter : JsonDeserializer<Boolean?> {
return when {
jsonPrimitive.isBoolean -> jsonPrimitive.asBoolean
jsonPrimitive.isNumber -> jsonPrimitive.asNumber.toInt() == 1
jsonPrimitive.isString -> TRUE_STRINGS.contains(jsonPrimitive.asString.toLowerCase(
Locale.getDefault()
))
jsonPrimitive.isString -> TRUE_STRINGS.contains(
jsonPrimitive.asString.lowercase()
)
else -> false
}
}
Expand Down
5 changes: 2 additions & 3 deletions libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/NotificationStore.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import org.wordpress.android.fluxc.utils.PreferenceUtils
import org.wordpress.android.util.AppLog
import org.wordpress.android.util.AppLog.T
import java.util.Date
import java.util.Locale
import java.util.UUID
import javax.inject.Inject
import javax.inject.Singleton
Expand Down Expand Up @@ -74,7 +73,7 @@ class NotificationStore @Inject constructor(

companion object {
private val reverseMap = values().associateBy(DeviceRegistrationErrorType::name)
fun fromString(type: String) = reverseMap[type.toUpperCase(Locale.US)] ?: GENERIC_ERROR
fun fromString(type: String) = reverseMap[type.uppercase()] ?: GENERIC_ERROR
}
}

Expand Down Expand Up @@ -147,7 +146,7 @@ class NotificationStore @Inject constructor(

companion object {
private val reverseMap = values().associateBy(NotificationErrorType::name)
fun fromString(type: String) = reverseMap[type.toUpperCase(Locale.US)] ?: GENERIC_ERROR
fun fromString(type: String) = reverseMap[type.uppercase()] ?: GENERIC_ERROR
}
}

Expand Down
4 changes: 1 addition & 3 deletions libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/PageStore.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import org.wordpress.android.fluxc.tools.CoroutineEngine
import org.wordpress.android.util.AppLog
import org.wordpress.android.util.DateTimeUtils
import java.util.Calendar
import java.util.Locale
import javax.inject.Inject
import javax.inject.Singleton
import kotlin.coroutines.Continuation
Expand Down Expand Up @@ -83,8 +82,7 @@ class PageStore @Inject constructor(
suspend fun search(site: SiteModel, searchQuery: String): List<PageModel> =
coroutineEngine.withDefaultContext(AppLog.T.POSTS, this, "search") {
getPagesFromDb(site).filter {
it.title.toLowerCase(Locale.ROOT)
.contains(searchQuery.toLowerCase(Locale.ROOT))
it.title.lowercase().contains(searchQuery.lowercase())
}
}

Expand Down
2 changes: 1 addition & 1 deletion libs/fluxc/src/main/java/org/wordpress/android/fluxc/store/SiteStore.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,7 @@ open class SiteStore @Inject constructor(
private const val SITE = "SITE"
@JvmStatic fun fromString(string: String): NewSiteErrorType {
if (!TextUtils.isEmpty(string)) {
val siteString = string.toUpperCase(Locale.US).replace(BLOG, SITE)
val siteString = string.uppercase().replace(BLOG, SITE)
for (v in values()) {
if (siteString.equals(v.name, ignoreCase = true)) {
return v
Expand Down
11 changes: 7 additions & 4 deletions libs/processors/src/test/kotlin/org/wordpress/android/processor/RemoteConfigProcessorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ package org.wordpress.android.processor

import com.tschuchort.compiletesting.KotlinCompilation
import com.tschuchort.compiletesting.SourceFile
import com.tschuchort.compiletesting.kspWithCompilation
import com.tschuchort.compiletesting.symbolProcessorProviders
import com.tschuchort.compiletesting.configureKsp
import org.assertj.core.api.Assertions.assertThat
import org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi
import org.jetbrains.kotlin.utils.addToStdlib.UnsafeCastFunction
Expand Down Expand Up @@ -119,10 +118,14 @@ class RemoteConfigProcessorTest {

private fun compile(src: List<SourceFile>) = KotlinCompilation().apply {
sources = src + fakeAppConfig
symbolProcessorProviders = listOf(RemoteConfigProcessorProvider())
kspWithCompilation = true
inheritClassPath = true
verbose = true
messageOutputStream = System.out
configureKsp(useKsp2 = false) {
symbolProcessorProviders += RemoteConfigProcessorProvider()
withCompilation = true
languageVersion = "1.9"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional reference:

To use KSP, you must set KotlinCompilation.languageVersion to 1.9.

ZacSweers/kotlin-compile-testing#196 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, nice find @wzieba ! 🥇

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, I have no idea why you "must" set KotlinCompilation.languageVersion to 1.9 for this to work, if you know, please share some more details, since this looks weird to me, and I am not sure that I am missing something basic here... 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU this is because KSP1 was meant to support language features up Kotlin 2.0+ (not included). So if -language-version compiler option (this is what KotlinCompilation.languageVersion sets) is set to higher, I'm guessing Kotlin Compiler forcefully chooses KSP2, which doesn't work with out processor. This is just a theory though, understanding it fully would diving into source of this logic or asking the author. Or find a way to instruct Kotlin compiler to print out which KSP it uses, but I haven't found a way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZacSweers/kotlin-compile-testing#309 🙂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sharing this theory @wzieba , it makes (some) sense indeed... 👍 + 🤔

Copy link
Contributor

@ParaskP7 ParaskP7 Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZacSweers/kotlin-compile-testing#309 🙂

Awesome, thanks for that, upvoted/subscribed! 🙇

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We got the answer, I think it's aligned with what I described above 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, 100%, great to have clarity and verification on that! 💯

}
}.compile()

// Fake AppConfig is needed, as it's a class that is expected to be present in the classpath. Originally, this class
Expand Down
Loading