Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #26844: Update ktlint to 0.47.0. #26846

Merged
merged 6 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion .editorconfig
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
[*.{kt,kts}]
# Disabling rules that were added in the latest versions of ktlint
# tracking here: https://github.com/mozilla-mobile/fenix/issues/4861
disabled_rules=import-ordering

ktlint_disabled_rules=import-ordering

ij_kotlin_allow_trailing_comma_on_call_site=true
ij_kotlin_allow_trailing_comma=true

[*]
insert_final_newline = true
4 changes: 2 additions & 2 deletions app/src/main/java/org/mozilla/fenix/compose/cfr/CFRPopup.kt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CFRPopup(
// This is just a facade for the CFRPopupFullScreenLayout composable offering a cleaner API.

@VisibleForTesting
internal var popup: WeakReference<CFRPopupFullScreenLayout>? = null
internal var popup: WeakReference<CFRPopupFullscreenLayout>? = null

/**
* Construct and display a styled CFR popup shown at the coordinates of [anchor].
Expand All @@ -72,7 +72,7 @@ class CFRPopup(
*/
fun show() {
anchor.post {
CFRPopupFullScreenLayout(text, anchor, properties, onDismiss, action).apply {
CFRPopupFullscreenLayout(text, anchor, properties, onDismiss, action).apply {
this.show()
popup = WeakReference(this)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private data class PopupHorizontalBounds(
* @param action Optional other composable to show just below the popup text.
*/
@SuppressLint("ViewConstructor") // Intended to be used only in code, don't need a View constructor
internal class CFRPopupFullScreenLayout(
internal class CFRPopupFullscreenLayout(
mcarare marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

This is an aside thought, but do we know why the ktlint in CI didn't complain and block the initial merge when the file name rule failed for this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the changelog it seems the rule has been added in 0.46.0. (It was later modified in 0.47.0 to allow different naming for files with just a single top-level declaration of type function - especially in the case of extension functions).

I do remember similar warnings about file names, but that could have been from lint or IDE (?)

private val text: String,
private val anchor: View,
private val properties: CFRPopupProperties,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ import org.junit.runner.RunWith
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner

@RunWith(FenixRobolectricTestRunner::class)
class CFRPopupFullScreenLayoutTest {
class CFRPopupFullscreenLayoutTest {
@Test
fun `WHEN the popup is constructed THEN setup lifecycle owners`() {
val anchor = View(testContext).apply {
ViewTreeLifecycleOwner.set(this, mockk())
this.setViewTreeSavedStateRegistryOwner(mockk())
}

val popupView = spyk(CFRPopupFullScreenLayout("", anchor, mockk(), mockk()) {})
val popupView = spyk(CFRPopupFullscreenLayout("", anchor, mockk(), mockk()) {})

assertNotNull(popupView.findViewTreeLifecycleOwner())
assertEquals(
Expand All @@ -59,7 +59,7 @@ class CFRPopupFullScreenLayoutTest {
}
val windowManager = spyk(context.getSystemService(Context.WINDOW_SERVICE) as WindowManager)
every { context.getSystemService(Context.WINDOW_SERVICE) } returns windowManager
val popupView = CFRPopupFullScreenLayout("", anchor, mockk(), mockk()) {}
val popupView = CFRPopupFullscreenLayout("", anchor, mockk(), mockk()) {}
popupView.show()
assertNotNull(popupView.findViewTreeLifecycleOwner())
assertNotNull(popupView.findViewTreeSavedStateRegistryOwner())
Expand All @@ -77,7 +77,7 @@ class CFRPopupFullScreenLayoutTest {
val anchor = View(context)
val windowManager = spyk(context.getSystemService(Context.WINDOW_SERVICE))
every { context.getSystemService(Context.WINDOW_SERVICE) } returns windowManager
val popupView = CFRPopupFullScreenLayout("", anchor, mockk(), mockk()) {}
val popupView = CFRPopupFullscreenLayout("", anchor, mockk(), mockk()) {}
val layoutParamsCaptor = slot<LayoutParams>()

popupView.show()
Expand All @@ -97,7 +97,7 @@ class CFRPopupFullScreenLayoutTest {
@Test
fun `WHEN creating layout params THEN get fullscreen translucent layout params`() {
val anchor = View(testContext)
val popupView = CFRPopupFullScreenLayout("", anchor, mockk(), mockk()) {}
val popupView = CFRPopupFullscreenLayout("", anchor, mockk(), mockk()) {}

val result = popupView.createLayoutParams()

Expand Down
6 changes: 3 additions & 3 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ configurations {
}

dependencies {
ktlint("com.pinterest:ktlint:0.41.0") {
ktlint("com.pinterest:ktlint:0.47.0") {
attributes {
attribute(Bundling.BUNDLING_ATTRIBUTE, getObjects().named(Bundling, Bundling.EXTERNAL))
}
Expand All @@ -210,14 +210,14 @@ tasks.register('ktlint', JavaExec) {
description = "Check Kotlin code style."
classpath = configurations.ktlint
main = "com.pinterest.ktlint.Main"
args "app/src/**/*.kt", "!**/build/**/*.kt"
args "app/src/**/*.kt", "!**/build/**/*.kt", "--baseline=ktlint-baseline.xml"
}

task ktlintFormat(type: JavaExec, group: "formatting") {
description = "Fix Kotlin code style deviations."
classpath = configurations.ktlint
main = "com.pinterest.ktlint.Main"
args "-F", "app/src/**/*.kt", "!**/build/**/*.kt"
args "-F", "app/src/**/*.kt", "!**/build/**/*.kt", "--baseline=ktlint-baseline.xml"
}

tasks.withType(io.gitlab.arturbosch.detekt.Detekt.class).configureEach {
Expand Down
Loading