Skip to content

Commit 34d697a

Browse files
mrmans0nmrmans0n
authored
Add ParameterNaming rule (#267)
1 parent a03923b commit 34d697a

File tree

10 files changed

+438
-0
lines changed

10 files changed

+438
-0
lines changed

‎docs/detekt.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,10 @@ Compose:
101101
active: true
102102
MutableStateParam:
103103
active: true
104+
ParameterNaming:
105+
active: true
106+
# -- You can optionally have a list of types to be treated as composable lambdas (e.g. typedefs or fun interfaces not picked up automatically)
107+
# treatAsComposableLambda: MyComposableLambdaType
104108
PreviewAnnotationNaming:
105109
active: true
106110
PreviewPublic:

‎docs/rules.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,23 @@ More information: [Kotlin default arguments](https://kotlinlang.org/docs/functio
268268

269269
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)
270270

271+
### Naming parameters properly
272+
273+
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`.
274+
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.
275+
276+
```kotlin
277+
// ❌
278+
@Composable
279+
fun Avatar(onShown: () -> Unit, onChanged: () -> Unit) { ... }
280+
281+
// ✅
282+
@Composable
283+
fun Avatar(onShow: () -> Unit, onChange: () -> Unit) { ... }
284+
```
285+
286+
Related rule: [compose:parameter-naming](https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ParameterNaming.kt)
287+
271288
### Movable content should be remembered
272289

273290
The methods used to create movable composable content (`movableContentOf` and `movableContentWithReceiverOf`) need to be used inside a `remember` function.

‎rules/common/src/main/kotlin/io/nlopez/compose/rules/ParameterNaming.kt

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
// Copyright 2023 Nacho Lopez
2+
// SPDX-License-Identifier: Apache-2.0
3+
package io.nlopez.compose.rules
4+
5+
import io.nlopez.compose.core.ComposeKtConfig
6+
import io.nlopez.compose.core.ComposeKtVisitor
7+
import io.nlopez.compose.core.Emitter
8+
import io.nlopez.compose.core.report
9+
import io.nlopez.compose.core.util.composableLambdaTypes
10+
import io.nlopez.compose.core.util.isLambda
11+
import org.jetbrains.kotlin.psi.KtFunction
12+
13+
class ParameterNaming : ComposeKtVisitor {
14+
15+
override fun visitComposable(
16+
function: KtFunction,
17+
autoCorrect: Boolean,
18+
emitter: Emitter,
19+
config: ComposeKtConfig,
20+
) = with(config) {
21+
// For lambda parameters only: if it starts with `on`, we want it to not be in past tense, to be all consistent.
22+
// E.g. onClick, onTextChange, onValueChange, and a myriad of other examples in the compose foundation code.
23+
24+
val lambdaTypes = function.containingKtFile.composableLambdaTypes
25+
26+
val errors = function.valueParameters
27+
.filter { it.typeReference?.isLambda(lambdaTypes) == true }
28+
.filter {
29+
// As per why not force lambdas to all start with `on`, we cannot really know when they are used for
30+
// lazy initialization purposes -- and also don't want to be overly annoying.
31+
it.name?.startsWith("on") == true
32+
}
33+
.filter { it.name?.isPastTense == true }
34+
35+
for (error in errors) {
36+
emitter.report(error, LambdaParametersInPresentTense)
37+
}
38+
}
39+
40+
private val String.isPastTense: Boolean
41+
get() = endsWith("ed") || IrregularVerbsInPastTense.any { endsWith(it) }
42+
43+
companion object {
44+
// A list of common irregular verbs in english, excluding those where present tense == past tense,
45+
// according to chatgpt. If you stumble upon one not here, feel free to send a PR to add it.
46+
private val IrregularVerbsInPastTense by lazy {
47+
setOf(
48+
"Arose",
49+
"Arisen",
50+
"Ate",
51+
"Awoke",
52+
"Awoken",
53+
"Beat",
54+
"Beaten",
55+
"Became",
56+
"Been",
57+
"Began",
58+
"Begun",
59+
"Bent",
60+
"Bit",
61+
"Bitten",
62+
"Bled",
63+
"Bled",
64+
"Blew",
65+
"Blown",
66+
"Bore",
67+
"Borne",
68+
"Bought",
69+
"Bound",
70+
"Bred",
71+
"Broke",
72+
"Broken",
73+
"Brought",
74+
"Built",
75+
"Burnt",
76+
"Burst",
77+
"Came",
78+
"Caught",
79+
"Chose",
80+
"Chosen",
81+
"Clung",
82+
"Cost",
83+
"Crept",
84+
"Dealt",
85+
"Did",
86+
"Done",
87+
"Drank",
88+
"Drawn",
89+
"Dreamt",
90+
"Drew",
91+
"Driven",
92+
"Drove",
93+
"Drunk",
94+
"Eaten",
95+
"Fallen",
96+
"Fed",
97+
"Felt",
98+
"Felt",
99+
"Fled",
100+
"Flew",
101+
"Flown",
102+
"Forbade",
103+
"Forbidden",
104+
"Forgave",
105+
"Forgiven",
106+
"Forgot",
107+
"Forgotten",
108+
"Fought",
109+
"Found",
110+
"Froze",
111+
"Frozen",
112+
"Gave",
113+
"Given",
114+
"Gone",
115+
"Got",
116+
"Gotten",
117+
"Grew",
118+
"Grown",
119+
"Had",
120+
"Heard",
121+
"Held",
122+
"Hid",
123+
"Hidden",
124+
"Hit",
125+
"Hung",
126+
"Hurt",
127+
"Kept",
128+
"Knew",
129+
"Known",
130+
"Laid",
131+
"Lain",
132+
"Lay",
133+
"Led",
134+
"Left",
135+
"Lent",
136+
"Let",
137+
"Lit",
138+
"Lost",
139+
"Made",
140+
"Meant",
141+
"Met",
142+
"Paid",
143+
"Ran",
144+
"Rang",
145+
"Ridden",
146+
"Risen",
147+
"Rode",
148+
"Rose",
149+
"Rung",
150+
"Said",
151+
"Sang",
152+
"Sank",
153+
"Sat",
154+
"Saw",
155+
"Seen",
156+
"Sent",
157+
"Set",
158+
"Shaken",
159+
"Shone",
160+
"Shook",
161+
"Shot",
162+
"Showed",
163+
"Shown",
164+
"Shrank",
165+
"Shrunk",
166+
"Slept",
167+
"Slid",
168+
"Sold",
169+
"Sought",
170+
"Spent",
171+
"Spoke",
172+
"Spoken",
173+
"Sprang",
174+
"Sprung",
175+
"Spun",
176+
"Stole",
177+
"Stolen",
178+
"Stood",
179+
"Struck",
180+
"Stuck",
181+
"Stung",
182+
"Sung",
183+
"Sunk",
184+
"Swam",
185+
"Swept",
186+
"Swore",
187+
"Sworn",
188+
"Swum",
189+
"Swung",
190+
"Taken",
191+
"Taught",
192+
"Thought",
193+
"Threw",
194+
"Thrown",
195+
"Told",
196+
"Took",
197+
"Tore",
198+
"Torn",
199+
"Understood",
200+
"Was",
201+
"Went",
202+
"Were",
203+
"Woke",
204+
"Woken",
205+
"Won",
206+
"Wore",
207+
"Worn",
208+
"Wound",
209+
"Written",
210+
"Wrote",
211+
)
212+
}
213+
214+
val LambdaParametersInPresentTense = """
215+
Lambda parameters in a composable function should be in present tense, not past tense.
216+
217+
Examples: `onClick` and not `onClicked`, `onTextChange` and not `onTextChanged`, etc.
218+
219+
See https://mrmans0n.github.io/compose-rules/rules/#naming-parameters-properly for more information.
220+
""".trimIndent()
221+
}
222+
}

‎rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ComposeRuleSetProvider.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class ComposeRuleSetProvider : RuleSetProvider {
3333
MutableStateAutoboxingCheck(config),
3434
MutableStateParameterCheck(config),
3535
NamingCheck(config),
36+
ParameterNamingCheck(config),
3637
ParameterOrderCheck(config),
3738
PreviewAnnotationNamingCheck(config),
3839
PreviewPublicCheck(config),

‎rules/detekt/src/main/kotlin/io/nlopez/compose/rules/detekt/ParameterNamingCheck.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Copyright 2023 Nacho Lopez
2+
// SPDX-License-Identifier: Apache-2.0
3+
package io.nlopez.compose.rules.detekt
4+
5+
import io.gitlab.arturbosch.detekt.api.Config
6+
import io.gitlab.arturbosch.detekt.api.Debt
7+
import io.gitlab.arturbosch.detekt.api.Issue
8+
import io.gitlab.arturbosch.detekt.api.Severity
9+
import io.nlopez.compose.core.ComposeKtVisitor
10+
import io.nlopez.compose.rules.DetektRule
11+
import io.nlopez.compose.rules.ParameterNaming
12+
13+
class ParameterNamingCheck(config: Config) :
14+
DetektRule(config),
15+
ComposeKtVisitor by ParameterNaming() {
16+
override val issue: Issue = Issue(
17+
id = "ParameterNaming",
18+
severity = Severity.CodeSmell,
19+
description = """
20+
Lambda parameters in a composable function should be in present tense, not past tense.
21+
22+
Examples: `onClick` and not `onClicked`, `onTextChange` and not `onTextChanged`, etc.
23+
""".trimIndent(),
24+
debt = Debt.FIVE_MINS,
25+
)
26+
}

‎rules/detekt/src/main/resources/config/config.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ Compose:
4343
active: true
4444
MutableStateParam:
4545
active: true
46+
ParameterNaming:
47+
active: true
4648
PreviewAnnotationNaming:
4749
active: true
4850
PreviewPublic:

‎rules/detekt/src/test/kotlin/io/nlopez/compose/rules/detekt/ParameterNamingCheckTest.kt

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// Copyright 2023 Nacho Lopez
2+
// SPDX-License-Identifier: Apache-2.0
3+
package io.nlopez.compose.rules.detekt
4+
5+
import io.gitlab.arturbosch.detekt.api.SourceLocation
6+
import io.gitlab.arturbosch.detekt.test.TestConfig
7+
import io.gitlab.arturbosch.detekt.test.assertThat
8+
import io.gitlab.arturbosch.detekt.test.lint
9+
import io.nlopez.compose.rules.ParameterNaming
10+
import org.intellij.lang.annotations.Language
11+
import org.junit.jupiter.api.Test
12+
13+
class ParameterNamingCheckTest {
14+
15+
private val testConfig = TestConfig(
16+
"treatAsComposableLambda" to listOf("Potato"),
17+
)
18+
private val rule = ParameterNamingCheck(testConfig)
19+
20+
@Test
21+
fun `errors when a param lambda is in the past tense`() {
22+
@Language("kotlin")
23+
val code =
24+
"""
25+
@Composable
26+
fun A(onClicked: () -> Boolean) { }
27+
@Composable
28+
fun A(onWrote: () -> Boolean) { }
29+
@Composable
30+
fun A(onPotatoed: Potato) { }
31+
""".trimIndent()
32+
33+
val errors = rule.lint(code)
34+
assertThat(errors)
35+
.hasStartSourceLocations(
36+
SourceLocation(2, 7),
37+
SourceLocation(4, 7),
38+
SourceLocation(6, 7),
39+
)
40+
for (error in errors) {
41+
assertThat(error).hasMessage(ParameterNaming.LambdaParametersInPresentTense)
42+
}
43+
}
44+
45+
@Test
46+
fun `ignores lambdas that don't start with on`() {
47+
@Language("kotlin")
48+
val code =
49+
"""
50+
@Composable
51+
fun A(blehWrote: () -> Unit, mehChanged: () -> Unit, potatoed: Potato) {}
52+
""".trimIndent()
53+
54+
val errors = rule.lint(code)
55+
assertThat(errors).isEmpty()
56+
}
57+
58+
@Test
59+
fun `passes when param lambdas are in present tense`() {
60+
@Language("kotlin")
61+
val code =
62+
"""
63+
@Composable
64+
fun A(onClick: () -> Unit, onValueChange: (Int) -> Unit, onWrite: () -> Unit, onPotato: Potato) {}
65+
""".trimIndent()
66+
67+
val errors = rule.lint(code)
68+
assertThat(errors).isEmpty()
69+
}
70+
}

‎rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ComposeRuleSetProvider.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class ComposeRuleSetProvider : RuleSetProviderV3(
3232
RuleProvider { MutableStateAutoboxingCheck() },
3333
RuleProvider { MutableStateParameterCheck() },
3434
RuleProvider { NamingCheck() },
35+
RuleProvider { ParameterNamingCheck() },
3536
RuleProvider { ParameterOrderCheck() },
3637
RuleProvider { PreviewAnnotationNamingCheck() },
3738
RuleProvider { PreviewPublicCheck() },

‎rules/ktlint/src/main/kotlin/io/nlopez/compose/rules/ktlint/ParameterNamingCheck.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright 2023 Nacho Lopez
2+
// SPDX-License-Identifier: Apache-2.0
3+
package io.nlopez.compose.rules.ktlint
4+
5+
import io.nlopez.compose.core.ComposeKtVisitor
6+
import io.nlopez.compose.rules.KtlintRule
7+
import io.nlopez.compose.rules.ParameterNaming
8+
9+
class ParameterNamingCheck :
10+
KtlintRule(
11+
id = "compose:parameter-naming",
12+
editorConfigProperties = setOf(treatAsComposableLambda),
13+
),
14+
ComposeKtVisitor by ParameterNaming()

0 commit comments

Comments
 (0)