Skip to content

Commit b66bf1c

Browse files
authored
Add an option to configure ComposePreviewPublic to flag all previews (#102)
By default, `ComposePreviewPublic` will only flag public previews if they use a `@PreviewParameter`, because we know for sure that they are only going to be used for previews. This will continue to be the default behavior, but if people want to flag ALL public previews, they can use `previewPublicOnlyIfParams` to do that now. Closes #95.
1 parent 57cd4c0 commit b66bf1c

10 files changed

Lines changed: 109 additions & 5 deletions

File tree

core-common/src/main/kotlin/com/twitter/rules/core/ComposeKtConfig.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@ interface ComposeKtConfig {
2727

2828
fun ASTNode.config(): ComposeKtConfig = psi.config()
2929

30+
private val PsiElement.hasConfigAttached: Boolean
31+
get() = containingFile.getUserData(Key) != null
32+
3033
fun PsiElement.attach(config: ComposeKtConfig) {
31-
containingFile.putUserData(Key, config)
34+
if (!hasConfigAttached) containingFile.putUserData(Key, config)
3235
}
3336
}
3437
}

core-detekt/src/main/kotlin/com/twitter/rules/core/detekt/TwitterDetektRule.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,13 @@ abstract class TwitterDetektRule(
5858

5959
override fun visitClass(klass: KtClass) {
6060
super<Rule>.visitClass(klass)
61+
klass.attach(config)
6162
visitClass(klass, autoCorrect, emitter)
6263
}
6364

6465
override fun visitKtElement(element: KtElement) {
6566
super.visitKtElement(element)
67+
element.attach(config)
6668
when (element.node.elementType) {
6769
KtStubElementTypes.FUNCTION -> {
6870
val function = element.cast<KtFunction>()

core-ktlint/src/main/kotlin/com/twitter/rules/core/ktlint/KtlintComposeKtConfig.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ internal class KtlintComposeKtConfig(
3737
getValueAsOrPut(key) { getList(key, default.toList()).toSet() } ?: default
3838

3939
override fun getBoolean(key: String, default: Boolean): Boolean =
40-
getValueAsOrPut(key) { properties[ktlintKey(key)]?.getValueAs<String>()?.toBooleanStrictOrNull() } ?: default
40+
getValueAsOrPut(key) { properties[ktlintKey(key)]?.getValueAs<Boolean>() } ?: default
4141

4242
private fun ktlintKey(key: String): String = "twitter_compose_${key.toSnakeCase()}"
4343
}

core-ktlint/src/test/kotlin/com/twitter/rules/core/ktlint/KtlintComposeKtConfigTest.kt

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ package com.twitter.rules.core.ktlint
55
import com.pinterest.ktlint.core.api.EditorConfigProperties
66
import org.assertj.core.api.AssertionsForInterfaceTypes.assertThat
77
import org.ec4j.core.model.Property
8+
import org.ec4j.core.model.PropertyType
9+
import org.ec4j.core.model.PropertyType.LowerCasingPropertyType
10+
import org.ec4j.core.model.PropertyType.PropertyValueParser.BOOLEAN_VALUE_PARSER
811
import org.junit.jupiter.api.Test
912

1013
class KtlintComposeKtConfigTest {
@@ -15,7 +18,7 @@ class KtlintComposeKtConfigTest {
1518
put("twitter_compose_my_list2", "a , b , c,a".prop)
1619
put("twitter_compose_my_set", "a,b,c,a,b,c".prop)
1720
put("twitter_compose_my_set2", " a, b,c ,a , b , c ".prop)
18-
put("twitter_compose_my_bool", "true".prop)
21+
put("twitter_compose_my_bool", true.prop)
1922
}
2023

2124
private val properties: EditorConfigProperties = mapping
@@ -70,7 +73,7 @@ class KtlintComposeKtConfigTest {
7073
mapping["my_list2"] = "z,y".prop
7174
mapping["my_set"] = "a".prop
7275
mapping["my_set2"] = "a, b".prop
73-
mapping["my_bool"] = "false".prop
76+
mapping["my_bool"] = false.prop
7477

7578
assertThat(config.getInt("myInt", 0)).isEqualTo(10)
7679
assertThat(config.getString("myString", null)).isEqualTo("abcd")
@@ -83,4 +86,15 @@ class KtlintComposeKtConfigTest {
8386

8487
private val String.prop: Property
8588
get() = Property.builder().value(this).build()
89+
90+
private val Boolean.prop: Property
91+
get() = Property.builder()
92+
.type(LowerCasingPropertyType("", "", BOOLEAN_VALUE_PARSER, "true", "false"))
93+
.value(
94+
when (this) {
95+
true -> PropertyType.PropertyValue.valid("true", true)
96+
false -> PropertyType.PropertyValue.valid("false", false)
97+
}
98+
)
99+
.build()
86100
}

docs/detekt.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ TwitterCompose:
5252
active: true
5353
PreviewPublic:
5454
active: true
55+
# You can optionally disable that only previews with @PreviewParameter are flagged
56+
# previewPublicOnlyIfParams: false
5557
RememberMissing:
5658
active: true
5759
UnstableCollections:

docs/ktlint.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ For `compositionlocal-allowlist` rule you can define a list of `CompositionLocal
5757
twitter_compose_allowed_composition_locals = LocalSomething,LocalSomethingElse
5858
```
5959

60+
### Make it so that all @Preview composables must be not public, no exceptions
61+
62+
In `preview-public-check`, only previews with a `@PreviewParameter` are required to be non-public by default. However, if you want to make it so ALL `@Preview` composables are non-public, you can add this to your `.editorconfig` file:
63+
64+
```editorconfig
65+
[*.{kt,kts}]
66+
twitter_compose_preview_public_only_if_params = false
67+
```
68+
6069
## Disabling a specific rule
6170

6271
To disable a rule you have to follow the [instructions from the ktlint documentation](https://github.com/pinterest/ktlint#how-do-i-suppress-an-errors-for-a-lineblockfile), and use the id of the rule you want to disable with the `twitter-compose` tag.

rules/common/src/main/kotlin/com/twitter/compose/rules/ComposePreviewPublic.kt

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// SPDX-License-Identifier: Apache-2.0
33
package com.twitter.compose.rules
44

5+
import com.twitter.rules.core.ComposeKtConfig.Companion.config
56
import com.twitter.rules.core.ComposeKtVisitor
67
import com.twitter.rules.core.Emitter
78
import com.twitter.rules.core.util.isPreview
@@ -17,8 +18,12 @@ class ComposePreviewPublic : ComposeKtVisitor {
1718
if (!function.isPreview) return
1819
// We only care about public methods
1920
if (!function.isPublic) return
21+
2022
// If the method is public, none of it's params should be tagged as preview
21-
if (function.valueParameters.none { it.isPreviewParameter }) return
23+
// This is configurable by the `previewPublicOnlyIfParams` config value
24+
if (function.config().getBoolean("previewPublicOnlyIfParams", true)) {
25+
if (function.valueParameters.none { it.isPreviewParameter }) return
26+
}
2227

2328
// If we got here, it's a public method in a @Preview composable with a @PreviewParameter parameter
2429
emitter.report(function, ComposablesPreviewShouldNotBePublic, true)

rules/detekt/src/test/kotlin/com/twitter/compose/rules/detekt/ComposePreviewPublicCheckTest.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package com.twitter.compose.rules.detekt
55
import com.twitter.compose.rules.ComposePreviewPublic
66
import io.gitlab.arturbosch.detekt.api.Config
77
import io.gitlab.arturbosch.detekt.api.SourceLocation
8+
import io.gitlab.arturbosch.detekt.test.TestConfig
89
import io.gitlab.arturbosch.detekt.test.assertThat
910
import io.gitlab.arturbosch.detekt.test.lint
1011
import org.intellij.lang.annotations.Language
@@ -42,6 +43,31 @@ class ComposePreviewPublicCheckTest {
4243
assertThat(errors).isEmpty()
4344
}
4445

46+
@Test
47+
fun `errors when a public preview composable is used when previewPublicOnlyIfParams is false`() {
48+
val config = TestConfig("previewPublicOnlyIfParams" to false)
49+
val ruleWithParams = ComposePreviewPublicCheck(config)
50+
51+
@Language("kotlin")
52+
val code =
53+
"""
54+
@Preview
55+
@Composable
56+
fun MyComposable() { }
57+
@CombinedPreviews
58+
@Composable
59+
fun MyComposable() { }
60+
""".trimIndent()
61+
val errors = ruleWithParams.lint(code)
62+
assertThat(errors).hasSourceLocations(
63+
SourceLocation(3, 5),
64+
SourceLocation(6, 5)
65+
)
66+
for (error in errors) {
67+
assertThat(error).hasMessage(ComposePreviewPublic.ComposablesPreviewShouldNotBePublic)
68+
}
69+
}
70+
4571
@Test
4672
fun `errors when a public preview composable uses preview params`() {
4773
@Language("kotlin")

rules/ktlint/src/main/kotlin/com/twitter/compose/rules/ktlint/EditorConfigProperties.kt

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package com.twitter.rules.core.ktlint
44

55
import com.pinterest.ktlint.core.api.UsesEditorConfigProperties
66
import org.ec4j.core.model.PropertyType
7+
import org.ec4j.core.model.PropertyType.PropertyValueParser
78

89
val contentEmittersProperty: UsesEditorConfigProperties.EditorConfigProperty<String> =
910
UsesEditorConfigProperties.EditorConfigProperty(
@@ -40,3 +41,16 @@ val compositionLocalAllowlistProperty: UsesEditorConfigProperties.EditorConfigPr
4041
}
4142
}
4243
)
44+
45+
val previewPublicOnlyIfParams: UsesEditorConfigProperties.EditorConfigProperty<Boolean> =
46+
UsesEditorConfigProperties.EditorConfigProperty(
47+
type = PropertyType.LowerCasingPropertyType(
48+
"twitter_compose_preview_public_only_if_params",
49+
"If set to true, it means ",
50+
//
51+
PropertyValueParser.BOOLEAN_VALUE_PARSER,
52+
"true",
53+
"false"
54+
),
55+
defaultValue = true
56+
)

rules/ktlint/src/test/kotlin/com/twitter/compose/rules/ktlint/ComposePreviewPublicCheckTest.kt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package com.twitter.compose.rules.ktlint
55
import com.pinterest.ktlint.test.KtLintAssertThat.Companion.assertThatRule
66
import com.pinterest.ktlint.test.LintViolation
77
import com.twitter.compose.rules.ComposePreviewPublic
8+
import com.twitter.rules.core.ktlint.previewPublicOnlyIfParams
89
import org.intellij.lang.annotations.Language
910
import org.junit.jupiter.api.Test
1011

@@ -66,6 +67,34 @@ class ComposePreviewPublicCheckTest {
6667
)
6768
}
6869

70+
@Test
71+
fun `errors when a public preview composable is used when previewPublicOnlyIfParams is false`() {
72+
@Language("kotlin")
73+
val code =
74+
"""
75+
@Preview
76+
@Composable
77+
fun MyComposable() { }
78+
@CombinedPreviews
79+
@Composable
80+
fun MyComposable() { }
81+
""".trimIndent()
82+
ruleAssertThat(code)
83+
.withEditorConfigOverride(previewPublicOnlyIfParams to false)
84+
.hasLintViolations(
85+
LintViolation(
86+
line = 3,
87+
col = 5,
88+
detail = ComposePreviewPublic.ComposablesPreviewShouldNotBePublic
89+
),
90+
LintViolation(
91+
line = 6,
92+
col = 5,
93+
detail = ComposePreviewPublic.ComposablesPreviewShouldNotBePublic
94+
)
95+
)
96+
}
97+
6998
@Test
7099
fun `passes when a non-public preview composable uses preview params`() {
71100
@Language("kotlin")

0 commit comments

Comments
 (0)