We have released a set of Detekt rules which help us find common errors while working on Compose code. For now there are few rules there, which seem more or less generally applicable (of course, they can be turned on/off individually if they don't work for you): https://github.com/appKODE/detekt-rules-compose

ModifierArgumentPosition

and

ModifierParameterPosition

ensure that

modifier

is declared and passed as a first parameter/argument

Why would you want Modifier being passed as a first argument to a Composable? 🤔 I personally pass it always as a last parameter because some modifier chains are long and I want to see the "more important" model parameters first. Nothing against your conventions but I would be interested in the reasoning.

This seems to be the convention which Material components also use. I guess this was inspired by that and we had this from the start. I thought about whether to include these rules, but then decided that they can be turned off if not desirable... Alternatively I guess we can also provide an option to that rule which either forces it to be the first argument or the last one: those seem to be the most commonly used presets.

Yeah, I've been looking through the material components and I see it. Weird, I've never noticed it 😅 And i agree that it's better to have it with ability to turn it off rather than not having it at all. I was just surprised that someone would make a lint about something I explicitly try not to do 😄 Anyways, thank you for making the rules public. I will consider using them 👍 Especially the

ReusedModifierInstance

is nice.

Yeah, we had a lot of inconsistencies in larger teams where everyone was placing modifier parameters randomly, so we've decided to stick to something 🙂 Thank you, please report any issues or suggestions!

ReusedModifierInstance

is indeed the most useful one of those. Others are more or less minor, but occasionally helpful.

The official convention is that the modifier param is the first optional parameter in the list. The idea is that you never have to type

modifier =

in the parameter list, it can always be specified positionally if the only other parameters you're using are required

Yep, this check only checks the calls/declarations which have it. I.e. if no

modifier

is mentioned, it won't be enforced at all. This is just to stay consistent when it's used.

Yes, that is the composable definition. But is there a convention on position of modifier parameter when using the composable? I 100% agree with the composable definition parameter order convention

When calling it the convention is to match the parameter order of the function definition, but I'd probably shake my head at anyone trying to formally enforce that 🙂

🙂 Indeed the one which checks an argument order at the call site is probably not as useful (I actually don't remember it ever firing), perhaps it should be removed. I guess it was conceived out of calls for perfect symmetry :)

I wouldn't mind having a rule to enforce using named parameters 😄 🙃 But yeah, I asked about it in the office and we didn't agree on anything. Someone wants it as first parameter, it does not matter for others and I like it as a last parameter. So it's probably highly subjective and situational.

Personally, I put

modifier

as the first optional parameter (this can be arguable btw) when it comes to signature, but I always pass it as last (but before any

@Composable

param) when it comes to function invocation - I completely agree with Filip.

Agreed. I will remove the

ModifierArgumentPosition

then and leave only the one which enforces the parameter:

ModifierParameterPosition

. It's also arguably subjective, but can be enabled/disabled depending on codebase conventions.

We considered modifier always first but the counterexample is that having to specify the named parameter in

Text(text = "hello")

as opposed to

Text("hello")

is silly

Notice to myself: Never use

var showButton = remember { mutableStateOf(false) }.value

. Although

.value

is nice for

val

, because it supports data flow analysis (eg smart casts), with

var

the state won't update 🤦‍♂️ Adding this rule would be nice too 😄

I wonder how widespread this error is?... by the way, you can always use

var showButton by remember { ... }

(note the

by

)

Yes, you could use

by

, beside

val (showButton, setValue) =

. In my codebase, I found it only once, but it easy to make the same mistake again, I think.