appKODE / detekt-rules-compose

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

False positive on `ModifierParameterPosition` and `ComposableParametersOrdering` #17

Open AfigAliyev opened 2 years ago

AfigAliyev commented 2 years ago
@Composable
fun MyComposable(
    modifier: Modifier = Modifier,
    content: LazyListScope.() -> Unit
) = LazyColumn(modifier = modifier, content = content)

ModifierParameterPosition and ComposableParametersOrdering raise an issue. The content parameter should be the last parameter.

dimsuz commented 2 years ago

Currently these rules ignore only @Composable trailing lambdas, because non-composable trailing lambdas are usually event handlers and they should follow the optional/required placement rule. We have discussed this in #12.

LazyColumn seems to be an exception here, because it's a DSL for building content, instead of content being a @Composable function.

I'm not sure yet how to best approach this:

BraisGabin commented 2 years ago

Brainstorming (they are not good ideas, they are just random ideas that maybe help to find a solution):

dimsuz commented 2 years ago

I like the second option (ignore specified receivers) and the fourth one (ignore all trailing lambdas). Maybe they both can be added as configuration parameters, although the ignoring all lambdas set to true will effectively override whatever is configured with second option, but this should not be too confusing.

AfigAliyev commented 2 years ago

I like the second option too, but I would like to add that it only makes sense to apply it to the last parameter of a function due to the DSL.

aptech73 commented 3 months ago

Did a little search. There is a similar library for compose [https://github.com/mrmans0n/compose-rules] (fork from Twitter library). And there this rule for argument order works correctly for lambdas (ContentTrailingLambda: https://mrmans0n.github.io/compose-rules/rules/#ordering-composable-parameters-properly).

Code: https://github.com/mrmans0n/compose-rules/blob/main/rules/common/src/main/kotlin/io/nlopez/compose/rules/ParameterOrder.kt

dimsuz commented 3 months ago

Thank you!

I plan to also merge the rule ModifierParameterPosition to the rule ComposableParameterOrdering, because the latter supersedes the former.