mrmans0n / compose-rules

Lint rules for ktlint/detekt aimed to contribute to a healthier usage of Compose. Actively maintained and evolved fork of the Twitter Compose rules.
https://mrmans0n.github.io/compose-rules
Other
543 stars 20 forks source link

compose:vm-injection-check rule work don't properly #212

Closed APXANGEL93 closed 5 months ago

APXANGEL93 commented 5 months ago

Let's say we have the following function:

@Composable
fun ScreenRoute(viewModel: ScreenViewModel)

although there is no default value, it is still correct. Here the rule works fine. Then we use it for example from the navigation declaration like this:

composable(destination = Destination.ScreenRoute) {
  ScreenRoute(
    viewModel = hiltViewModel()
  )
}

Here, too, everything works correctly. But then we decide to use assistedfactory to add parameters to the viewmodel and change the code like this:

composable(destination = Destination.ScreenRoute) {
  val viewModel = hiltViewModel(
       creationCallback = { factory: ScreenViewModel.Factory ->
           factory.create(
               parameter = 5,
               parameter2 = null
           )
       }
   )
   ScreenRoute(
       viewModel = viewModel
   )
}

This is where the rule sounds the alarm, although everything still remains correct. Let's go further and explicitly declare the type of the viewModel variable:

composable(destination = Destination.ScreenRoute) {
  val viewModel: ScreenViewModel = hiltViewModel(
       creationCallback = { factory: ScreenViewModel.Factory ->
           factory.create(
               parameter = 5,
               parameter2 = null
           )
       }
   )
   ScreenRoute(
       viewModel = viewModel
   )
}

Here the rule finally loses its mind and it puts the variable with the viewmodel into the parameter of the function in which all the screens are located:

Navigation(
  ...
  navController: NavHostController = rememberNavController()
  **<here>**
) {
NavHost(
    navController = navController,
    startDestination = startDestination
) {
    composable(destination = Destination.ScreenRoute) {...}
    composable(destination = Destination.Screen1Route) {...}
  }
}

kotlin = "1.9.23" compose = "1.6.4" ktlint = "12.1.0" ktlintComposeRules = "0.3.12"

mrmans0n commented 5 months ago

So the autofix that has the issues in your case, right? And the detection fails in the navigation DSL?

APXANGEL93 commented 5 months ago

Well, the triggering of the rule itself is also false. The dependency is passed to the screen explicitly in the constructor

mrmans0n commented 5 months ago

Yeah the rule won't check the cases where the factory function is instantiated directly in a KtCallableReference's parameters, it will only check if they are being instantiated in a property.

I'm gonna to submit a fix that filters it out when it's instantiated inside a NavHost DSL, so those won't be reported. Also I'll get rid of the autofix if the factory function has params, as we can't be 100% sure it won't screw things up.