[ComposableParameterOrdering] False positives with multiple `@Composable` paramenters

[BraisGabin]

I found this when updating to 1.4

Consider this case:

@Composable
fun Foo(
  state: FooState,
  toolbar: @Composable () -> Unit,
  modifier: Modifier = Modifier,
  bottomBar: @Composable () -> Unit = {},
  showFab: Boolean = false,
  content: @Composable () -> Unit,
)

The current rule requires me to move toolbar down. But I can't see anything on the guidelines about that: https://android.googlesource.com/platform/frameworks/support/+/androidx-main/compose/docs/compose-component-api-guidelines.md#parameters-order

1. Required parameters.
2. Single modifier: Modifier = Modifier.
3. Optional parameters.
4. (optional) trailing @Composable lambda.

In point 4 they refer to only one trailing lambda, no more.

So I'd say that if there is already one @Composable function at the bottom it shouldn't complain.

We could also "work" with this:

(optional) trailing @Composable lambda representing the main content of the component, usually named content.

We could check for the name of the components and only ask them to move it down if it's called content. Because I have other cases like this:

@Composable
fun User(
  name: String,
  followers: Int,
  avatar: @Composable () -> Unit,
  modifier: Modifier = Modifier
)

It's true, that I could move avatar down. But it's not the main content of that composable so it's a bit odd to have something like this:

User(
  name = "Brais",
  followers = "1",
) {
  Image(...)
}

I think that it's better like this:

User(
  name = "Brais",
  followers = "1",
  avatar = { Image(...) },
)

But this last case, for me, is less obvious and probably this case can just have a @Suppress.

[dimsuz]

You are right, this rule should be improved! Although it's quite tricky to satisfy every style -- sometimes there's no content lambda, but there are several slots semantically on the same "level" of importance, and then it's nice to have them grouped together rather than separated by other parameters, also there were other "tricky" cases I can't recall, but I remember this rule having several revisions which changed the rules specifically around the lambdas positioning.

But I guess this only proves that we should simply adhere to the existing styling advice to be able to standardize somehow.

[BraisGabin]

I got two samples from Material:

TextField

Good case for don't move down any of the @Composables

@Composable
fun TextField(
    value: TextFieldValue,
    onValueChange: (TextFieldValue) -> Unit,
    modifier: Modifier = Modifier,
    enabled: Boolean = true,
    readOnly: Boolean = false,
    textStyle: TextStyle = LocalTextStyle.current,
    label: (@Composable () -> Unit)? = null,
    placeholder: (@Composable () -> Unit)? = null,
    leadingIcon: (@Composable () -> Unit)? = null,
    trailingIcon: (@Composable () -> Unit)? = null,
    isError: Boolean = false,
    visualTransformation: VisualTransformation = VisualTransformation.None,
    keyboardOptions: KeyboardOptions = KeyboardOptions.Default,
    keyboardActions: KeyboardActions = KeyboardActions(),
    singleLine: Boolean = false,
    maxLines: Int = if (singleLine) 1 else Int.MAX_VALUE,
    minLines: Int = 1,
    interactionSource: MutableInteractionSource? = null,
    shape: Shape = TextFieldDefaults.TextFieldShape,
    colors: TextFieldColors = TextFieldDefaults.textFieldColors()
): Unit

AlertDialog

This one has the same and the @Composables aren't together because some have a default value and the others don't

@Composable
fun AlertDialog(
    onDismissRequest: () -> Unit,
    buttons: @Composable () -> Unit,
    modifier: Modifier = Modifier,
    title: (@Composable () -> Unit)? = null,
    text: (@Composable () -> Unit)? = null,
    shape: Shape = MaterialTheme.shapes.medium,
    backgroundColor: Color = MaterialTheme.colors.surface,
    contentColor: Color = contentColorFor(backgroundColor),
    properties: DialogProperties = DialogProperties()
): Unit

So my idea for this rule:

• Don't force to move the @Composable functions down.
• Allow to have only one no-default @Composable argument at the bottom of the function definition.
• Add a configuration to allow multiple no-default @Composable arguments at the bottom of the function definition, disable by default.
• Add a configuration to define which lambda parameter names should be at the bottom of the function definition. The default of this list would be content. I would like to have a list because I see on a lot of places in my project that the devs names this parameter mainContent to differentiate it from topContent. But it could be a flag too, I don't see a huge problem to rename those mainContent to just content.
  • This should fix False positive on ModifierParameterPosition and ComposableParametersOrdering #17 too because I didn't enforce that the lambda needs to be @Composable.

What do you think?

[dimsuz]

Sorry for the delay! I think your ideas a great, I'll try to implement them!