Skip to content

Commit

Permalink
Updates ktlint to 0.46.1
Browse files Browse the repository at this point in the history
ktlint had a significant update in 0.46.x, including a bunch of breaking changes in tests. The new API is nicer though.
This patch includes all the migration to the new test style, and to pass the default tests.

NOTE: At the moment of writing this, spotless hasn't been updated yet to support the new ktlint version. diffplug/spotless#1239
  • Loading branch information
mrmans0n committed Jun 22, 2022
1 parent c2a63e7 commit 66bc796
Show file tree
Hide file tree
Showing 18 changed files with 190 additions and 363 deletions.
18 changes: 9 additions & 9 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ Instead pass down the relevant data to the function, and optional lambdas for ca

More information: [State and Jetpack Compose](https://developer.android.com/jetpack/compose/state)

Related rule: [compose-vm-forwarding-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeViewModelForwardingCheck.kt)
Related rule: [twitter-compose:vm-forwarding-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeViewModelForwardingCheck.kt)

### State should be remembered in composables

Be careful when using `mutableStateOf` (or any of the other state builders) to make sure that you `remember` the instance. If you don't `remember` the state instance, a new state instance will be created when the function is recomposed.

Related rule: [compose-remember-missing-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeRememberMissingCheck.kt)
Related rule: [twitter-compose:remember-missing-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeRememberMissingCheck.kt)

### Use Immutable annotation whenever possible

Expand All @@ -40,7 +40,7 @@ There are a few reasons for this, but the main one is that it is very easy to us

Passing `ArrayList<T>`, `MutableState<T>`, `ViewModel` are common examples of this (but not limited to those types).

Related rule: [compose-mutable-params-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeMutableParametersCheck.kt)
Related rule: [twitter-compose:mutable-params-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeMutableParametersCheck.kt)

### Do not emit content and return a result

Expand All @@ -50,7 +50,7 @@ If a composable should offer additional control surfaces to its caller, those co

More info: [Compose API guidelines](https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-api-guidelines.md#emit-xor-return-a-value)

Related rule: [compose-multiple-emitters-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeMultipleContentEmittersCheck.kt)
Related rule: [twitter-compose:multiple-emitters-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeMultipleContentEmittersCheck.kt)

### Do not emit multiple pieces of content

Expand Down Expand Up @@ -96,7 +96,7 @@ private fun ColumnScope.InnerContent() {
```
This effectively ties the function to be called from a Column, but is still not recommended (although permitted).

Related rule: [compose-multiple-emitters-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeMultipleContentEmittersCheck.kt)
Related rule: [twitter-compose:multiple-emitters-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeMultipleContentEmittersCheck.kt)

### Naming @Composable functions properly

Expand All @@ -106,7 +106,7 @@ However, Composable functions that return a value should start with a lowercase

More information: [Naming Unit @Composable functions as entities](https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-api-guidelines.md#naming-unit-composable-functions-as-entities) and [Naming @Composable functions that return values](https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-api-guidelines.md#naming-composable-functions-that-return-values)

Related rule: [compose-naming-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeNamingCheck.kt)
Related rule: [twitter-compose:naming-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeNamingCheck.kt)

### Make dependencies explicit

Expand Down Expand Up @@ -137,7 +137,7 @@ private fun MyComposable(

```

Related rule: [compose-vm-injection-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeViewModelInjectionCheck.kt)
Related rule: [twitter-compose:vm-injection-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeViewModelInjectionCheck.kt)

## Modifiers

Expand All @@ -149,7 +149,7 @@ They are especially important for your public components, as they allow callers

More info: [Always provide a Modifier parameter](https://chris.banes.dev/always-provide-a-modifier/)

Related rule: [compose-modifier-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeModifierMissingCheck.kt)
Related rule: [twitter-compose:modifier-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeModifierMissingCheck.kt)

### Don't re-use modifiers

Expand Down Expand Up @@ -180,4 +180,4 @@ private fun InnerContent(modifier: Modifier = Modifier) {
}
```

Related rule: [compose-modifier-used-once-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeModifierUsedOnceCheck.kt)
Related rule: [twitter-compose:modifier-used-once-check](https://github.com/twitter/compose-rules/blob/main/rules/ktlint/src/main/kotlin/com/twitter/rules/ktlint/compose/ComposeModifierUsedOnceCheck.kt)
4 changes: 2 additions & 2 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[versions]
kotlin = "1.6.21"
ktlint = "0.45.2"
kotlin = "1.7.0"
ktlint = "0.46.1"
junit = "5.8.2"

[libraries]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.psiUtil.isPublic

class ComposeModifierMissingCheck : TwitterKtRule("compose-modifier-check") {
class ComposeModifierMissingCheck : TwitterKtRule("twitter-compose:modifier-check") {

override fun visitFile(file: KtFile, autoCorrect: Boolean, emitter: Emitter) {
file.findChildrenByClass<KtFunction>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import org.jetbrains.kotlin.psi.KtValueArgumentName
import org.jetbrains.kotlin.psi.psiUtil.siblings
import org.jetbrains.kotlin.psi.psiUtil.startOffset

class ComposeModifierUsedOnceCheck : TwitterKtRule("compose-modifier-used-once-check") {
class ComposeModifierUsedOnceCheck : TwitterKtRule("twitter-compose:modifier-used-once-check") {

override fun visitFunction(function: KtFunction, autoCorrect: Boolean, emitter: Emitter) {
if (!function.isComposable) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtFunction

class ComposeMultipleContentEmittersCheck : TwitterKtRule("compose-multiple-emitters-check") {
class ComposeMultipleContentEmittersCheck : TwitterKtRule("twitter-compose:multiple-emitters-check") {

override fun visitFile(file: KtFile, autoCorrect: Boolean, emitter: Emitter) {
// CHECK #1 : We want to find the composables first that are at risk of emitting content from multiple sources.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import com.twitter.rules.core.ktlint.TwitterKtRule
import com.twitter.rules.core.ktlint.report
import org.jetbrains.kotlin.psi.KtFunction

class ComposeMutableParametersCheck : TwitterKtRule("compose-mutable-params-check") {
class ComposeMutableParametersCheck : TwitterKtRule("twitter-compose:mutable-params-check") {

override fun visitFunction(function: KtFunction, autoCorrect: Boolean, emitter: Emitter) {
if (!function.isComposable) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import com.twitter.rules.core.ktlint.report
import com.twitter.rules.core.returnsValue
import org.jetbrains.kotlin.psi.KtFunction

class ComposeNamingCheck : TwitterKtRule("compose-naming-check") {
class ComposeNamingCheck : TwitterKtRule("twitter-compose:naming-check") {

override fun visitFunction(function: KtFunction, autoCorrect: Boolean, emitter: Emitter) {
if (!function.isComposable) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.psiUtil.startOffset

class ComposeRememberMissingCheck : TwitterKtRule("compose-remember-missing-check") {
class ComposeRememberMissingCheck : TwitterKtRule("twitter-compose:remember-missing-check") {

override fun visitFunction(function: KtFunction, autoCorrect: Boolean, emitter: Emitter) {
if (!function.isComposable) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.KtReferenceExpression
import org.jetbrains.kotlin.psi.psiUtil.startOffset

class ComposeViewModelForwardingCheck : TwitterKtRule("compose-vm-forwarding-check") {
class ComposeViewModelForwardingCheck : TwitterKtRule("twitter-compose:vm-forwarding-check") {

override fun visitFile(file: KtFile, autoCorrect: Boolean, emitter: Emitter) {
file.findChildrenByClass<KtFunction>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import org.jetbrains.kotlin.psi.KtFunctionType
import org.jetbrains.kotlin.psi.KtProperty
import org.jetbrains.kotlin.psi.KtPsiFactory

class ComposeViewModelInjectionCheck : TwitterKtRule("compose-vm-injection-check") {
class ComposeViewModelInjectionCheck : TwitterKtRule("twitter-compose:vm-injection-check") {

override fun visitFile(file: KtFile, autoCorrect: Boolean, emitter: Emitter) {
file.findChildrenByClass<KtFunction>()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
package com.twitter.rules.ktlint.compose

import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.test.format
import com.pinterest.ktlint.test.lint
import org.assertj.core.api.Assertions.assertThat
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThat
import com.pinterest.ktlint.test.LintViolation
import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.Test

class ComposeModifierMissingCheckTest {

private val rule = ComposeModifierMissingCheck()
private val modifierRuleAssertThat = ComposeModifierMissingCheck().assertThat()

@Test
fun `errors when a Composable has a layout inside and it doesn't have a modifier`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
@Composable
fun Something() {
Expand All @@ -40,37 +38,30 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
)
val expectedErrors = listOf(
LintError(

modifierRuleAssertThat(code).hasLintViolationsWithoutAutoCorrect(
LintViolation(
line = 2,
col = 5,
ruleId = "compose-modifier-check",
detail = ComposeModifierMissingCheck.MissingModifierContentComposable,
canBeAutoCorrected = false
),
LintError(
LintViolation(
line = 7,
col = 5,
ruleId = "compose-modifier-check",
detail = ComposeModifierMissingCheck.MissingModifierContentComposable,
canBeAutoCorrected = false
),
LintError(
LintViolation(
line = 12,
col = 5,
ruleId = "compose-modifier-check",
detail = ComposeModifierMissingCheck.MissingModifierContentComposable,
canBeAutoCorrected = false
)
)
assertThat(errors).isEqualTo(expectedErrors)
}

@Test
fun `errors when a Composable without modifiers has a Composable inside with a modifier`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
@Composable
fun Something() {
Expand All @@ -85,30 +76,23 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
)
val expectedErrors = listOf(
LintError(

modifierRuleAssertThat(code).hasLintViolationsWithoutAutoCorrect(
LintViolation(
line = 2,
col = 5,
ruleId = "compose-modifier-check",
detail = ComposeModifierMissingCheck.MissingModifierContentComposable,
canBeAutoCorrected = false
),
LintError(
LintViolation(
line = 7,
col = 5,
ruleId = "compose-modifier-check",
detail = ComposeModifierMissingCheck.MissingModifierContentComposable,
canBeAutoCorrected = false
)
)
assertThat(errors).isEqualTo(expectedErrors)
}

@Test
fun `errors when a Composable has modifiers but without default values, and is able to auto fixing it`() {
val check = rule

@Language("kotlin")
val composableCode = """
@Composable
Expand All @@ -117,33 +101,28 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
val errors = check.lint(composableCode)
val expectedErrors = listOf(
LintError(

modifierRuleAssertThat(composableCode)
.hasLintViolation(
line = 2,
col = 15,
ruleId = "compose-modifier-check",
detail = ComposeModifierMissingCheck.MissingModifierDefaultParam,
canBeAutoCorrected = true
)
)
assertThat(errors).isEqualTo(expectedErrors)
val autoFixCode = check.format(composableCode)
assertThat(autoFixCode).isEqualTo(
"""
.isFormattedAs(
"""
@Composable
fun Something(modifier: Modifier = Modifier) {
Row(modifier = modifier) {
}
}
""".trimIndent()
)
)
}

@Test
fun `passes when a Composable has modifiers with defaults`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
@Composable
fun Something(modifier: Modifier = Modifier) {
Expand All @@ -161,14 +140,13 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
)
assertThat(errors).isEmpty()
modifierRuleAssertThat(code).hasNoLintViolations()
}

@Test
fun `non-public visibility Composables are ignored`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
@Composable
private fun Something() {
Expand All @@ -193,14 +171,13 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
)
assertThat(errors).isEmpty()
modifierRuleAssertThat(code).hasNoLintViolations()
}

@Test
fun `interface Composables are ignored`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
interface MyInterface {
@Composable
Expand All @@ -216,14 +193,13 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
)
assertThat(errors).isEmpty()
modifierRuleAssertThat(code).hasNoLintViolations()
}

@Test
fun `overridden Composables are ignored`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
@Composable
override fun Content() {
Expand All @@ -241,22 +217,20 @@ class ComposeModifierMissingCheckTest {
}
}
""".trimIndent()
)
assertThat(errors).isEmpty()
modifierRuleAssertThat(code).hasNoLintViolations()
}

@Test
fun `Composables that return a type that is not Unit shouldn't be processed`() {
@Language("kotlin")
val errors = rule.lint(
val code =
"""
@Composable
fun Something(): Int {
Row {
}
}
""".trimIndent()
)
assertThat(errors).isEmpty()
modifierRuleAssertThat(code).hasNoLintViolations()
}
}
Loading

0 comments on commit 66bc796

Please sign in to comment.