appKODE / detekt-rules-compose

A collection of Detekt rules for Jetpack Compose
MIT License
136 stars 8 forks source link

False positive on `UnnecessaryEventHandlerParameter` #13

Closed BraisGabin closed 1 year ago

BraisGabin commented 2 years ago

Given this code:

internal sealed class State {
    object Loading : State()
    data class Data(val id: String) : State()
}

@Composable
internal fun MyButton(
    state: State,
    onClick: (String) -> Unit,
) {
    when (state) {
        State.Loading -> Text("Loading")
        is State.Data -> Button(onClick = { onClick(state.id) }) { Text("Click here") }
    }
}

UnnecessaryEventHandlerParameter raises an issue. But in this case the parameter is necessary because state.id is accesible because of the smart cast that you don't have in the function that call this component.

dimsuz commented 2 years ago

Ugh, right. Not sure how to solve this without converting this rule to use type resolution (to check that State is a sealed class - it could be defined in other file/subproject).

(I'm still a bit hesitant to use type resolution, because I feel that it's not widely adopted at the moment, but perhaps I'm wrong. And maybe putting some direct advice + instructions on enabling it in the README would help)

BraisGabin commented 2 years ago

Yes, to fix this we will need type resolution.

Right now I'm working on this on the detekt side:

With those type resolution will not be experimental anymore and we will inform users about the rules that they have active but they aren't working.

dimsuz commented 2 years ago

Great work! So I'll get started. Both in adopting type resolution in our work projects and learning to write rules for it (only saw examples in the Detekt repo, didn't try myself).

BraisGabin commented 2 years ago

To be honest, they are more complex. But the good part is that there are enoght examples in the detekt repo to find what you need.