Skip to content

Don't suggest @NonRestartableComposable on previews #627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ZacSweers wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Don't suggest @NonRestartableComposable on previews #627

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Changelog
**Unreleased**
--------------

- **Fix**: Don't suggest `@NonRestartableComposable` on `@Preview`-annotated composables.

1.6.0
-----

Expand Down
2 changes: 2 additions & 0 deletions compose-lint-checks/src/main/java/slack/lint/compose/NonRestartableComposableDetector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import slack.lint.compose.util.directCall
import slack.lint.compose.util.findChildrenByClass
import slack.lint.compose.util.isForwardedReceiver
import slack.lint.compose.util.isForwardedValue
import slack.lint.compose.util.isPreview
import slack.lint.compose.util.returnsUnitOrVoid
import slack.lint.compose.util.singleBodyExpression
import slack.lint.compose.util.sourceImplementation
Expand Down Expand Up @@ -63,6 +64,7 @@ class NonRestartableComposableDetector : ComposableFunctionDetector(), SourceCod
if (namedFunction.isLocal() || namedFunction.definedInInterface) return
if (NON_RESTARTABLE_MODIFIERS.any(namedFunction::hasModifier)) return
if (INELIGIBLE_ANNOTATIONS.any(method::hasAnnotation)) return
if (method.isPreview) return

val bodyExpression = namedFunction.singleBodyExpression() ?: return
// A pass-through body has one call and no lambda body of its own. This also rejects calls used
Expand Down
149 changes: 83 additions & 66 deletions compose-lint-checks/src/test/java/slack/lint/compose/NonRestartableComposableDetectorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,99 +3,95 @@
package slack.lint.compose

import com.android.tools.lint.checks.infrastructure.TestFiles.binaryStub
import com.android.tools.lint.checks.infrastructure.TestLintTask
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
import org.intellij.lang.annotations.Language
import org.junit.Test

class NonRestartableComposableDetectorTest : BaseComposeLintTest() {

private val compiledRuntime =
private val compiledStubs =
binaryStub(
"libs/compiled-runtime.jar",
"libs/compiled-stubs.jar",
kotlin(
"""
package androidx.compose.runtime

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.TYPE)
@Retention(AnnotationRetention.BINARY)
@Target(AnnotationTarget.FUNCTION)
annotation class Composable

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.SOURCE)
@Target(AnnotationTarget.FUNCTION)
annotation class NonRestartableComposable

class CompiledContent {
@Composable
@NonRestartableComposable
fun render(value: String) {}
}
"""
)
.indented()
.to("src/androidx/compose/runtime/CompiledRuntime.kt"),
)

private val stubs =
kotlin(
"""
package androidx.compose.runtime

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER, AnnotationTarget.TYPE)
@Retention(AnnotationRetention.BINARY)
annotation class Composable
@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.SOURCE)
annotation class NonSkippableComposable

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.SOURCE)
annotation class NonRestartableComposable
@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.BINARY)
annotation class ReadOnlyComposable

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.SOURCE)
annotation class NonSkippableComposable
@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.SOURCE)
annotation class ExplicitGroupsComposable

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.BINARY)
annotation class ReadOnlyComposable
interface State<out T> {
val value: T
}

@Target(AnnotationTarget.FUNCTION, AnnotationTarget.PROPERTY_GETTER)
@Retention(AnnotationRetention.SOURCE)
annotation class ExplicitGroupsComposable
@Composable
external fun Content(value: String, child: @Composable () -> Unit = {})

interface State<out T> {
val value: T
}
@Composable
external fun VarargContent(vararg values: String)

@Composable
external fun Content(value: String, child: @Composable () -> Unit = {})
@Composable
external fun ValueContent(): String

@Composable
external fun VarargContent(vararg values: String)
@Composable
external fun DefaultValue(): String

@Composable
external fun ValueContent(): String
@Composable
@ReadOnlyComposable
external fun ReadOnlyContent()

@Composable
external fun DefaultValue(): String
@Composable
@NonRestartableComposable
external fun NonRestartableContent()

@Composable
@ReadOnlyComposable
external fun ReadOnlyContent()
@Composable
@NonSkippableComposable
external fun NonSkippableContent()

@Composable
@NonRestartableComposable
external fun NonRestartableContent()
class CompiledContent {
@Composable
@NonRestartableComposable
fun render(value: String) {}
}
"""
)
.indented()
.to("src/androidx/compose/runtime/CompiledStubs.kt"),
kotlin(
"""
package androidx.compose.ui.tooling.preview

@Composable
@NonSkippableComposable
external fun NonSkippableContent()
"""
.trimIndent()
annotation class Preview
"""
)
.indented()
.to("src/androidx/compose/ui/tooling/preview/Preview.kt"),
)

override fun getDetector(): Detector = NonRestartableComposableDetector()

override fun getIssues(): List<Issue> = listOf(NonRestartableComposableDetector.ISSUE)

override fun lint(): TestLintTask = super.lint().allowKotlinClassStubs(true)

@Test
fun `suggests non-restartable for a direct composable call`() {
@Language("kotlin")
Expand All @@ -110,7 +106,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
.trimIndent()

lint()
.files(stubs, kotlin(code))
.files(compiledStubs, kotlin(code))
.run()
.expect(
"""
Expand Down Expand Up @@ -155,7 +151,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
.trimIndent()

lint()
.files(stubs, kotlin(code))
.files(compiledStubs, kotlin(code))
.run()
.expect(
"""
Expand Down Expand Up @@ -188,7 +184,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
.trimIndent()

lint()
.files(stubs, kotlin(code))
.files(compiledStubs, kotlin(code))
.run()
.expect(
"""
Expand Down Expand Up @@ -217,8 +213,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
.trimIndent()

lint()
.files(compiledRuntime, kotlin(code))
.allowKotlinClassStubs(true)
.files(compiledStubs, kotlin(code))
.run()
.expect(
"""
Expand Down Expand Up @@ -284,7 +279,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
"""
.trimIndent()

lint().files(stubs, kotlin(code)).run().expectClean()
lint().files(compiledStubs, kotlin(code)).run().expectClean()
}

@Test
Expand Down Expand Up @@ -353,7 +348,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
"""
.trimIndent()

lint().files(stubs, kotlin(code)).run().expectClean()
lint().files(compiledStubs, kotlin(code)).run().expectClean()
}

@Test
Expand Down Expand Up @@ -385,7 +380,7 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
.trimIndent()

lint()
.files(stubs, kotlin(code))
.files(compiledStubs, kotlin(code))
.run()
.expect(
"""
Expand Down Expand Up @@ -429,6 +424,28 @@ class NonRestartableComposableDetectorTest : BaseComposeLintTest() {
"""
.trimIndent()

lint().files(stubs, kotlin(code)).run().expectClean()
lint().files(compiledStubs, kotlin(code)).run().expectClean()
}

@Test
fun `does not suggest non-restartable on previews`() {
@Language("kotlin")
val code =
"""
import androidx.compose.runtime.Composable
import androidx.compose.ui.tooling.preview.Preview

@Composable
fun AnotherComposable() {}

@Preview
@Composable
fun IgnoredValueChild() {
AnotherComposable()
}
"""
.trimIndent()

lint().files(compiledStubs, kotlin(code)).run().expectClean()
}
}
Loading