Skip to content

Commit

Permalink
Add ignoreAnnotated/compose_modifier_missing_ignore_annotated to Modi…
Browse files Browse the repository at this point in the history
…fierMissing (#329)
  • Loading branch information
mrmans0n authored Sep 3, 2024
1 parent 03f64f9 commit 935dbb1
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 7 deletions.
2 changes: 2 additions & 0 deletions docs/detekt.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ Compose:
# checkModifiersForVisibility: only_public
# -- You can optionally add your own Modifier types
# customModifiers: BananaModifier,PotatoModifier
# -- You can suppress this check in functions annotated with these annotations
# ignoreAnnotated: ['Potato', 'Banana']
ModifierNaming:
active: true
# -- You can optionally add your own Modifier types
Expand Down
9 changes: 9 additions & 0 deletions docs/ktlint.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ For `compositionlocal-allowlist` rule you can define a list of `CompositionLocal
compose_allowed_composition_locals = LocalSomething,LocalSomethingElse
```

### Ignore annotated functions with specific annotations for missing Modifier checks

In the `modifier-missing-check` rule, you can define a list of annotations that, if present, will make it so the function is exempt from this rule.

```editorconfig
[*.{kt,kts}]
compose_modifier_missing_ignore_annotated = Potato,Banana
```

### Allowing matching function names

The `naming-check` rule requires all composables that return a value to be lowercased. If you want to allow certain patterns though, you can configure a comma-separated list of matching regexes in your `.editorconfig` file:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,6 @@ fun KtElement.isSuppressed(suppression: String): Boolean = parentsWithSelf.filte
.flatMap { it.entries.asSequence() }
.filterIsInstance<KtLiteralStringTemplateEntry>()
.any { it.text == suppression }

fun KtAnnotated.isAnnotatedWith(annotations: Set<String>): Boolean =
annotationEntries.any { it.calleeExpression?.text in annotations }
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import io.nlopez.compose.core.Emitter
import io.nlopez.compose.core.report
import io.nlopez.compose.core.util.definedInInterface
import io.nlopez.compose.core.util.emitsContent
import io.nlopez.compose.core.util.isAnnotatedWith
import io.nlopez.compose.core.util.isInternal
import io.nlopez.compose.core.util.isModifierReceiver
import io.nlopez.compose.core.util.isOverride
Expand Down Expand Up @@ -35,6 +36,9 @@ class ModifierMissing : ComposeKtVisitor {
return
}

// Ignore functions annotated with the given set of names
if (function.isAnnotatedWith(config.getSet("modifierMissingIgnoreAnnotated", emptySet()))) return

// We want to check now the visibility to see whether it's allowed by the configuration
// Possible values:
// - only_public: will check for modifiers only on public composables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import io.gitlab.arturbosch.detekt.api.CorrectableCodeSmell
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Location
import io.gitlab.arturbosch.detekt.api.Rule
import io.nlopez.compose.core.ComposeKtConfig
import io.nlopez.compose.core.ComposeKtVisitor
import io.nlopez.compose.core.Decision
import io.nlopez.compose.core.Emitter
Expand All @@ -24,7 +23,7 @@ abstract class DetektRule(config: Config = Config.empty) :
Rule(config),
ComposeKtVisitor {

private val config: ComposeKtConfig by lazy { DetektComposeKtConfig(this) }
private val config: DetektComposeKtConfig by lazy { DetektComposeKtConfig(this) }

private val emitter: Emitter = Emitter { element, message, canBeAutoCorrected ->
// Grab the named element if there were any, otherwise fall back to the whole PsiElement
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,30 @@ 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.gitlab.arturbosch.detekt.api.config
import io.nlopez.compose.core.ComposeKtConfig
import io.nlopez.compose.core.ComposeKtVisitor
import io.nlopez.compose.core.Emitter
import io.nlopez.compose.core.util.isAnnotatedWith
import io.nlopez.compose.rules.DetektRule
import io.nlopez.compose.rules.ModifierMissing
import org.jetbrains.kotlin.psi.KtFunction

class ModifierMissingCheck(config: Config) :
DetektRule(config),
ComposeKtVisitor by ModifierMissing() {
class ModifierMissingCheck(config: Config) : DetektRule(config) {
override val issue: Issue = Issue(
id = "ModifierMissing",
severity = Severity.Defect,
description = ModifierMissing.MissingModifierContentComposable,
debt = Debt.TEN_MINS,
)
private val visitor: ComposeKtVisitor = ModifierMissing()

// On the docs it looks like this is a common suppressor that should be available everywhere,
// but it doesn't seem to be (according to unit tests). Oh well, I guess I'll just leave the extra check for now.
private val ignoreAnnotated: List<String> by config(emptyList<String>()) { list -> list.map(String::trim) }

override fun visitComposable(function: KtFunction, emitter: Emitter, config: ComposeKtConfig) {
if (function.isAnnotatedWith(ignoreAnnotated.toSet())) return
visitor.visitComposable(function, emitter, config)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,25 @@ class ModifierMissingCheckTest {
}
}

@Test
fun `passes when ignoreAnnotated is used`() {
val rule = ModifierMissingCheck(TestConfig("ignoreAnnotated" to listOf("Potato")))

@Language("kotlin")
val code =
"""
@Potato
@Composable
fun Something1() {
Row {
}
}
""".trimIndent()

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

@Test
fun `errors when a Composable without modifiers has a Composable inside with a modifier`() {
@Language("kotlin")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import com.pinterest.ktlint.rule.engine.core.api.RuleAutocorrectApproveHandler
import com.pinterest.ktlint.rule.engine.core.api.RuleId
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfig
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.EditorConfigProperty
import io.nlopez.compose.core.ComposeKtConfig
import io.nlopez.compose.core.ComposeKtVisitor
import io.nlopez.compose.core.Decision
import io.nlopez.compose.core.Emitter
Expand Down Expand Up @@ -40,7 +39,7 @@ abstract class KtlintRule(id: String, editorConfigProperties: Set<EditorConfigPr
properties = editorConfig
}

private val config: ComposeKtConfig by lazy { KtlintComposeKtConfig(properties, usesEditorConfigProperties) }
private val config: KtlintComposeKtConfig by lazy { KtlintComposeKtConfig(properties, usesEditorConfigProperties) }

override fun beforeVisitChildNodes(
node: ASTNode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,3 +266,21 @@ val disallowUnstableCollections: EditorConfigProperty<Boolean> =
),
defaultValue = false,
)

val modifierMissingIgnoreAnnotated: EditorConfigProperty<String> =
EditorConfigProperty(
type = PropertyType.LowerCasingPropertyType(
"compose_modifier_missing_ignore_annotated",
"A comma separated list of composable functions that should be exempt from the ModifierMissing check",
PropertyValueParser.IDENTITY_VALUE_PARSER,
emptySet(),
),
defaultValue = "",
propertyMapper = { property, _ ->
when {
property?.isUnset == true -> ""
property?.getValueAs<String>() != null -> property.getValueAs<String>()
else -> property?.getValueAs()
}
},
)
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class ModifierMissingCheck :
contentEmittersProperty,
customModifiers,
contentEmittersDenylist,
modifierMissingIgnoreAnnotated,
),
),
ComposeKtVisitor by ModifierMissing()
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,32 @@ class ModifierMissingCheckTest {
)
}

@Test
fun `passes when ignoreAnnotated is used`() {
@Language("kotlin")
val code =
"""
@Potato
@Composable
fun Something1() {
Row {
}
}
@Banana
@Composable
fun Something1() {
Row {
}
}
""".trimIndent()

modifierRuleAssertThat(code)
.withEditorConfigOverride(
modifierMissingIgnoreAnnotated to "Potato,Banana",
)
.hasNoLintViolations()
}

@Test
fun `errors when a Composable without modifiers has a Composable inside with a modifier`() {
@Language("kotlin")
Expand Down

0 comments on commit 935dbb1

Please sign in to comment.