appKODE / detekt-rules-compose

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

New rule: `Modifier` Should have default value #4

Closed BraisGabin closed 2 years ago

BraisGabin commented 2 years ago

Enforce that modifier always have a default value non compilant: modifier: Modifier

compilant: modifier: Modifier = Modifier modifier: Modifier = Modifier.padding(4.dp)

dimsuz commented 2 years ago

I have also thought about adding a separate rule which would enforce putting optional parameters after required parameters, (+not allowing to intersperse them). Looks like this makes sense specifically for composable functions (not generally) and follows Compose internal style: so that you can write e.g. Text("Hello", TextStyle.H2). This would require named args if at least one optional parameter is present before required ones. Not yet sure how useful such rule would be, thought about trying it on our private codebases first.

BraisGabin commented 2 years ago

😂 I had on my TODO list "Open an issue to enforce required parameters first". I just crosslined it (Unless you want the issue to track this kind of things)

dimsuz commented 2 years ago

Hah, nice! No need then, it's on my TODO list also, will do soon.

dimsuz commented 2 years ago

I found a thread in compose slack in which Google's Leland Richardson said that the convention is to have the default value be simply Modifier, not Modifier.fillMaxWidth() or something else, as this could lead to unwanted problems, as that thread indicates.

This rule can check this also, and report error a) in case there's no default value b) in case default value is something more than Modifier

BraisGabin commented 2 years ago

Sounds good!

dimsuz commented 2 years ago

I have actually made two new rules: