Skip to content

Commit e5463a7

Browse files
blipinskZacSweers
andauthored
ComposeUnstableReceiver only for non-Unit returning functions (#468)
* ComposeUnstableReceiver detector updated * Tests updated * Rules warning * Spotless --------- Co-authored-by: Zac Sweers <[email protected]>
1 parent 0665b8a commit e5463a7

File tree

3 files changed

+71
-38
lines changed

3 files changed

+71
-38
lines changed

compose-lint-checks/src/main/java/slack/lint/compose/UnstableReceiverDetector.kt

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import org.jetbrains.uast.UMethod
1818
import org.jetbrains.uast.getContainingUClass
1919
import slack.lint.compose.util.Priorities
2020
import slack.lint.compose.util.isStable
21+
import slack.lint.compose.util.returnsUnitOrVoid
2122
import slack.lint.compose.util.sourceImplementation
2223

2324
class UnstableReceiverDetector : ComposableFunctionDetector(), SourceCodeScanner {
@@ -65,30 +66,34 @@ class UnstableReceiverDetector : ComposableFunctionDetector(), SourceCodeScanner
6566
else -> null
6667
}
6768

68-
if (receiverType?.isStable(context.evaluator) == false) {
69-
context.report(
70-
ISSUE,
71-
nodeToReport,
72-
context.getLocation(nodeToReport),
73-
ISSUE.getExplanation(TextFormat.TEXT),
74-
)
75-
} else if (!method.isTopLevelKtOrJavaMember() && !method.isStatic) {
76-
// We check both the receiver and the containing class, as classes could have
77-
// extension functions in their declarations too.
78-
val containingClass = method.getContainingUClass()
79-
val containingClassType =
80-
containingClass
81-
// If the containing class is an object, it will never be passed as a receiver arg
82-
?.takeUnless { it.sourcePsi is KtObjectDeclaration }
83-
?.let(context.evaluator::getClassType) ?: return
84-
85-
if (!containingClassType.isStable(context.evaluator, resolveUClass = { containingClass })) {
69+
// Only non-Unit returning functions can be skippable.
70+
// Non-skippable functions will always be recomposed regardless of the receiver type.
71+
if (method.returnsUnitOrVoid(context.evaluator)) {
72+
if (receiverType?.isStable(context.evaluator) == false) {
8673
context.report(
8774
ISSUE,
88-
method,
89-
context.getNameLocation(method),
75+
nodeToReport,
76+
context.getLocation(nodeToReport),
9077
ISSUE.getExplanation(TextFormat.TEXT),
9178
)
79+
} else if (!method.isTopLevelKtOrJavaMember() && !method.isStatic) {
80+
// We check both the receiver and the containing class, as classes could have
81+
// extension functions in their declarations too.
82+
val containingClass = method.getContainingUClass()
83+
val containingClassType =
84+
containingClass
85+
// If the containing class is an object, it will never be passed as a receiver arg
86+
?.takeUnless { it.sourcePsi is KtObjectDeclaration }
87+
?.let(context.evaluator::getClassType) ?: return
88+
89+
if (!containingClassType.isStable(context.evaluator, resolveUClass = { containingClass })) {
90+
context.report(
91+
ISSUE,
92+
method,
93+
context.getNameLocation(method),
94+
ISSUE.getExplanation(TextFormat.TEXT),
95+
)
96+
}
9297
}
9398
}
9499
}

compose-lint-checks/src/test/java/slack/lint/compose/UnstableReceiverDetectorTest.kt

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,6 @@ class UnstableReceiverDetectorTest : BaseComposeLintTest() {
4343
@Composable
4444
fun Example.OtherContent() {}
4545
46-
@get:Composable
47-
val Example.OtherContentProperty get() {}
48-
4946
@Stable
5047
enum class EnumExample {
5148
TEST;
@@ -108,16 +105,13 @@ class UnstableReceiverDetectorTest : BaseComposeLintTest() {
108105
@Composable
109106
fun Example.OtherContent() {}
110107
111-
@get:Composable
112-
val Example.OtherContentProperty get() {}
113-
114108
// Supertypes
115-
interface Presenter<T> {
116-
@Composable fun present(): T
109+
interface Presenter {
110+
@Composable fun present()
117111
}
118112
119-
class HomePresenter : Presenter<String> {
120-
@Composable override fun present(): String { return "hi" }
113+
class HomePresenter : Presenter {
114+
@Composable override fun present() { println("hi") }
121115
}
122116
"""
123117
.trimIndent()
@@ -135,18 +129,49 @@ class UnstableReceiverDetectorTest : BaseComposeLintTest() {
135129
src/ExampleInterface.kt:12: Warning: Instance composable functions on non-stable classes will always be recomposed. If possible, make the receiver type stable or refactor this function if that isn't possible. See https://slackhq.github.io/compose-lints/rules/#unstable-receivers for more information. [ComposeUnstableReceiver]
136130
fun Example.OtherContent() {}
137131
~~~~~~~
138-
src/ExampleInterface.kt:15: Warning: Instance composable functions on non-stable classes will always be recomposed. If possible, make the receiver type stable or refactor this function if that isn't possible. See https://slackhq.github.io/compose-lints/rules/#unstable-receivers for more information. [ComposeUnstableReceiver]
139-
val Example.OtherContentProperty get() {}
140-
~~~~~~~
141-
src/ExampleInterface.kt:19: Warning: Instance composable functions on non-stable classes will always be recomposed. If possible, make the receiver type stable or refactor this function if that isn't possible. See https://slackhq.github.io/compose-lints/rules/#unstable-receivers for more information. [ComposeUnstableReceiver]
142-
@Composable fun present(): T
132+
src/ExampleInterface.kt:16: Warning: Instance composable functions on non-stable classes will always be recomposed. If possible, make the receiver type stable or refactor this function if that isn't possible. See https://slackhq.github.io/compose-lints/rules/#unstable-receivers for more information. [ComposeUnstableReceiver]
133+
@Composable fun present()
143134
~~~~~~~
144-
src/ExampleInterface.kt:23: Warning: Instance composable functions on non-stable classes will always be recomposed. If possible, make the receiver type stable or refactor this function if that isn't possible. See https://slackhq.github.io/compose-lints/rules/#unstable-receivers for more information. [ComposeUnstableReceiver]
145-
@Composable override fun present(): String { return "hi" }
135+
src/ExampleInterface.kt:20: Warning: Instance composable functions on non-stable classes will always be recomposed. If possible, make the receiver type stable or refactor this function if that isn't possible. See https://slackhq.github.io/compose-lints/rules/#unstable-receivers for more information. [ComposeUnstableReceiver]
136+
@Composable override fun present() { println("hi") }
146137
~~~~~~~
147-
0 errors, 6 warnings
138+
0 errors, 5 warnings
148139
"""
149140
.trimIndent()
150141
)
151142
}
143+
144+
@Test
145+
fun `unstable receiver types with non-Unit return type report no errors`() {
146+
@Language("kotlin")
147+
val code =
148+
"""
149+
import androidx.compose.runtime.Composable
150+
151+
interface ExampleInterface {
152+
@Composable fun Content(): String
153+
}
154+
155+
class Example {
156+
@Composable fun Content(): String { return "hi" }
157+
}
158+
159+
@Composable
160+
fun Example.OtherContent(): String { return "hi" }
161+
162+
@get:Composable
163+
val Example.OtherContentProperty get() {}
164+
165+
// Supertypes
166+
interface Presenter<T> {
167+
@Composable fun present(): T
168+
}
169+
170+
class HomePresenter : Presenter<String> {
171+
@Composable override fun present(): String { return "hi" }
172+
}
173+
"""
174+
.trimIndent()
175+
lint().files(*commonStubs, kotlin(code)).run().expectClean()
176+
}
152177
}

docs/rules.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ Related rule: [`ComposeMutableParameters`](https://github.com/slackhq/compose-li
7171
In compose, all parameters must be stable or immutable in order for a composable function to be
7272
_restartable_ or _skippable_. This _includes_ the containing class or receiver, which the compose-compiler will treat as the 0th argument. Using an unstable receiver is usually a bug, so this lint offers a warning to raise this issue.
7373

74+
!!! warning "Use with Strong Skipping"
75+
If you have Strong Skipping enabled, we recommend disabling this rule to avoid potential false negatives.
76+
7477
More info: [Compose API Stability](https://developer.android.com/jetpack/compose/performance/stability)
7578

7679
Related rule: [`ComposeUnstableReceiver`](https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/UnstableReceiverDetector.kt)

0 commit comments

Comments
 (0)