search

twitter / compose-rules

Static checks to aid with a healthy adoption of Compose
https://twitter.github.io/compose-rules
Other
1.35k stars 92 forks source link

modifier-missing-check: Wrong report for fake content emitters #109

Closed PaulWoitaschek closed 1 year ago

PaulWoitaschek commented 1 year ago

Currently, the modifier-missing-check rule shows an error for things that are technically content emitters but don't emit real content in terms that the content is a different window.

In the following example it does not really make sense to supply a modifier because the composable is really encapsulating it's contents.

@Composable
public fun MyDialog() {
  AlertDialog(
    onDismissRequest = { /*TODO*/ },
    buttons = { Text(text = "Button") },
    text = { Text(text = "Body") },
  )
}

The same applies for composables like ModalBottomSheetLayout.

One solution could be be to extend the current rule logic and check if the function has a single direct content emitter which is one of these window-style content emitters.

Tested with 0.0.22

mrmans0n commented 1 year ago

Hi! Feel free to submit a PR deleting these kind of emitters from https://github.com/twitter/compose-rules/blob/main/core-common/src/main/kotlin/com/twitter/rules/core/util/Composables.kt (or add another list of these types, and then add an exclusion for those in the rule)

mrmans0n commented 1 year ago

Now that this is in, I'm gonna kickstart the publishing process so the changes will be in maven central in version 0.0.23 in an hour or so.