Skip to content

Commit

Permalink
Add ParameterNaming rule (#267)
Browse files Browse the repository at this point in the history
  • Loading branch information
mrmans0n committed May 23, 2024
1 parent a03923b commit 34d697a
Show file tree
Hide file tree
Showing 10 changed files with 438 additions and 0 deletions.
4 changes: 4 additions & 0 deletions docs/detekt.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ Compose:
active: true
MutableStateParam:
active: true
ParameterNaming:
active: true
# -- You can optionally have a list of types to be treated as composable lambdas (e.g. typedefs or fun interfaces not picked up automatically)
# treatAsComposableLambda: MyComposableLambdaType
PreviewAnnotationNaming:
active: true
PreviewPublic:
Expand Down
17 changes: 17 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,23 @@ More information: [Kotlin default arguments](https://kotlinlang.org/docs/functio

Related rule: [compose:param-order-check](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ParameterOrder.kt)

### Naming parameters properly

The parameters in composable functions that send events are typically named `on` + verb in the present tense, like in the very common examples in Compose foundation code: `onClick` or `onTextChange`.
It a common mistake to use the past tense in some of these events, so for consistency’s sake, we'll want to adjust the tense of the verbs to present.

```kotlin
//
@Composable
fun Avatar(onShown: () -> Unit, onChanged: () -> Unit) { ... }

//
@Composable
fun Avatar(onShow: () -> Unit, onChange: () -> Unit) { ... }
```

Related rule: [compose:parameter-naming](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ParameterNaming.kt)

### Movable content should be remembered

The methods used to create movable composable content (`movableContentOf` and `movableContentWithReceiverOf`) need to be used inside a `remember` function.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,222 @@
// Copyright 2023 Nacho Lopez
// SPDX-License-Identifier: Apache-2.0
package io.nlopez.compose.rules

import io.nlopez.compose.core.ComposeKtConfig
import io.nlopez.compose.core.ComposeKtVisitor
import io.nlopez.compose.core.Emitter
import io.nlopez.compose.core.report
import io.nlopez.compose.core.util.composableLambdaTypes
import io.nlopez.compose.core.util.isLambda
import org.jetbrains.kotlin.psi.KtFunction

class ParameterNaming : ComposeKtVisitor {

override fun visitComposable(
function: KtFunction,
autoCorrect: Boolean,
emitter: Emitter,
config: ComposeKtConfig,
) = with(config) {
// For lambda parameters only: if it starts with `on`, we want it to not be in past tense, to be all consistent.
// E.g. onClick, onTextChange, onValueChange, and a myriad of other examples in the compose foundation code.

val lambdaTypes = function.containingKtFile.composableLambdaTypes

val errors = function.valueParameters
.filter { it.typeReference?.isLambda(lambdaTypes) == true }
.filter {
// As per why not force lambdas to all start with `on`, we cannot really know when they are used for
// lazy initialization purposes -- and also don't want to be overly annoying.
it.name?.startsWith("on") == true
}
.filter { it.name?.isPastTense == true }

for (error in errors) {
emitter.report(error, LambdaParametersInPresentTense)
}
}

private val String.isPastTense: Boolean
get() = endsWith("ed") || IrregularVerbsInPastTense.any { endsWith(it) }

companion object {
// A list of common irregular verbs in english, excluding those where present tense == past tense,
// according to chatgpt. If you stumble upon one not here, feel free to send a PR to add it.
private val IrregularVerbsInPastTense by lazy {
setOf(
"Arose",
"Arisen",
"Ate",
"Awoke",
"Awoken",
"Beat",
"Beaten",
"Became",
"Been",
"Began",
"Begun",
"Bent",
"Bit",
"Bitten",
"Bled",
"Bled",
"Blew",
"Blown",
"Bore",
"Borne",
"Bought",
"Bound",
"Bred",
"Broke",
"Broken",
"Brought",
"Built",
"Burnt",
"Burst",
"Came",
"Caught",
"Chose",
"Chosen",
"Clung",
"Cost",
"Crept",
"Dealt",
"Did",
"Done",
"Drank",
"Drawn",
"Dreamt",
"Drew",
"Driven",
"Drove",
"Drunk",
"Eaten",
"Fallen",
"Fed",
"Felt",
"Felt",
"Fled",
"Flew",
"Flown",
"Forbade",
"Forbidden",
"Forgave",
"Forgiven",
"Forgot",
"Forgotten",
"Fought",
"Found",
"Froze",
"Frozen",
"Gave",
"Given",
"Gone",
"Got",
"Gotten",
"Grew",
"Grown",
"Had",
"Heard",
"Held",
"Hid",
"Hidden",
"Hit",
"Hung",
"Hurt",
"Kept",
"Knew",
"Known",
"Laid",
"Lain",
"Lay",
"Led",
"Left",
"Lent",
"Let",
"Lit",
"Lost",
"Made",
"Meant",
"Met",
"Paid",
"Ran",
"Rang",
"Ridden",
"Risen",
"Rode",
"Rose",
"Rung",
"Said",
"Sang",
"Sank",
"Sat",
"Saw",
"Seen",
"Sent",
"Set",
"Shaken",
"Shone",
"Shook",
"Shot",
"Showed",
"Shown",
"Shrank",
"Shrunk",
"Slept",
"Slid",
"Sold",
"Sought",
"Spent",
"Spoke",
"Spoken",
"Sprang",
"Sprung",
"Spun",
"Stole",
"Stolen",
"Stood",
"Struck",
"Stuck",
"Stung",
"Sung",
"Sunk",
"Swam",
"Swept",
"Swore",
"Sworn",
"Swum",
"Swung",
"Taken",
"Taught",
"Thought",
"Threw",
"Thrown",
"Told",
"Took",
"Tore",
"Torn",
"Understood",
"Was",
"Went",
"Were",
"Woke",
"Woken",
"Won",
"Wore",
"Worn",
"Wound",
"Written",
"Wrote",
)
}

val LambdaParametersInPresentTense = """
Lambda parameters in a composable function should be in present tense, not past tense.
Examples: `onClick` and not `onClicked`, `onTextChange` and not `onTextChanged`, etc.
See https://mrmans0n.github.io/compose-rules/rules/#naming-parameters-properly for more information.
""".trimIndent()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class ComposeRuleSetProvider : RuleSetProvider {
MutableStateAutoboxingCheck(config),
MutableStateParameterCheck(config),
NamingCheck(config),
ParameterNamingCheck(config),
ParameterOrderCheck(config),
PreviewAnnotationNamingCheck(config),
PreviewPublicCheck(config),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright 2023 Nacho Lopez
// SPDX-License-Identifier: Apache-2.0
package io.nlopez.compose.rules.detekt

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Severity
import io.nlopez.compose.core.ComposeKtVisitor
import io.nlopez.compose.rules.DetektRule
import io.nlopez.compose.rules.ParameterNaming

class ParameterNamingCheck(config: Config) :
DetektRule(config),
ComposeKtVisitor by ParameterNaming() {
override val issue: Issue = Issue(
id = "ParameterNaming",
severity = Severity.CodeSmell,
description = """
Lambda parameters in a composable function should be in present tense, not past tense.
Examples: `onClick` and not `onClicked`, `onTextChange` and not `onTextChanged`, etc.
""".trimIndent(),
debt = Debt.FIVE_MINS,
)
}
2 changes: 2 additions & 0 deletions rules/detekt/src/main/resources/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ Compose:
active: true
MutableStateParam:
active: true
ParameterNaming:
active: true
PreviewAnnotationNaming:
active: true
PreviewPublic:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
// Copyright 2023 Nacho Lopez
// SPDX-License-Identifier: Apache-2.0
package io.nlopez.compose.rules.detekt

import io.gitlab.arturbosch.detekt.api.SourceLocation
import io.gitlab.arturbosch.detekt.test.TestConfig
import io.gitlab.arturbosch.detekt.test.assertThat
import io.gitlab.arturbosch.detekt.test.lint
import io.nlopez.compose.rules.ParameterNaming
import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.Test

class ParameterNamingCheckTest {

private val testConfig = TestConfig(
"treatAsComposableLambda" to listOf("Potato"),
)
private val rule = ParameterNamingCheck(testConfig)

@Test
fun `errors when a param lambda is in the past tense`() {
@Language("kotlin")
val code =
"""
@Composable
fun A(onClicked: () -> Boolean) { }
@Composable
fun A(onWrote: () -> Boolean) { }
@Composable
fun A(onPotatoed: Potato) { }
""".trimIndent()

val errors = rule.lint(code)
assertThat(errors)
.hasStartSourceLocations(
SourceLocation(2, 7),
SourceLocation(4, 7),
SourceLocation(6, 7),
)
for (error in errors) {
assertThat(error).hasMessage(ParameterNaming.LambdaParametersInPresentTense)
}
}

@Test
fun `ignores lambdas that don't start with on`() {
@Language("kotlin")
val code =
"""
@Composable
fun A(blehWrote: () -> Unit, mehChanged: () -> Unit, potatoed: Potato) {}
""".trimIndent()

val errors = rule.lint(code)
assertThat(errors).isEmpty()
}

@Test
fun `passes when param lambdas are in present tense`() {
@Language("kotlin")
val code =
"""
@Composable
fun A(onClick: () -> Unit, onValueChange: (Int) -> Unit, onWrite: () -> Unit, onPotato: Potato) {}
""".trimIndent()

val errors = rule.lint(code)
assertThat(errors).isEmpty()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class ComposeRuleSetProvider : RuleSetProviderV3(
RuleProvider { MutableStateAutoboxingCheck() },
RuleProvider { MutableStateParameterCheck() },
RuleProvider { NamingCheck() },
RuleProvider { ParameterNamingCheck() },
RuleProvider { ParameterOrderCheck() },
RuleProvider { PreviewAnnotationNamingCheck() },
RuleProvider { PreviewPublicCheck() },
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2023 Nacho Lopez
// SPDX-License-Identifier: Apache-2.0
package io.nlopez.compose.rules.ktlint

import io.nlopez.compose.core.ComposeKtVisitor
import io.nlopez.compose.rules.KtlintRule
import io.nlopez.compose.rules.ParameterNaming

class ParameterNamingCheck :
KtlintRule(
id = "compose:parameter-naming",
editorConfigProperties = setOf(treatAsComposableLambda),
),
ComposeKtVisitor by ParameterNaming()
Loading

0 comments on commit 34d697a

Please sign in to comment.