Changes to allow non navigation arguments with default values.

raamcosta · Sep 9, 2021 · 281cc15 · 281cc15
1 parent dbb011e
commit 281cc15
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 70 deletions.
diff --git a/README.md b/README.md
@@ -8,10 +8,9 @@ will make adding new destinations have less boilerplate and be less error-prone,
 ### Why?
 
 Because:
-- How nice would it be if navigation compose did not rely as much on bundles, strings and other not type-safe stuff?
+- How nice would it be if navigation compose did not rely as much on bundles, strings and other "non-type-safe" stuff?
 - What if we could simply add parameters to the `Composable` function to define the destination arguments?
-- And what if we did not have to do anything else other than create the screen composable when we add a new screen? (Instead of remembering 
-to add a `composable` call in the `NavHost` and most likely adding the destination to some kind of `sealed class`)
+- And what if the navigation graph could be built/updated automatically when we add new or remove old screen composables?
 
 ### How?
 
@@ -76,29 +75,14 @@ fun UserScreen(
 
 Now the IDE will even tell you the default arguments of the composable when calling the `withArgs` method!
 
-NOTE: Currently, arguments do have some limitations:
-- They must be one of `String`, `Boolean`, `Int`, `Float`, `Long`, `NavController`, `NavBackStackEntry` or `ScaffoldState` (only if you are using `Scaffold`).
-- All `Composable` function arguments will be considered navigation arguments except for `NavController`, `NavBackStackEntry` or `ScaffoldState`.
-If you have a view model that you want to access with, for example `hiltViewModel()`, it cannot be a Destination argument. The good practice is to call other
-`Composable` function and use the Destination Composable as a wrapper to the testable `Composable` screen.
-- Default values must be resolvable from the generated `DestinationSpec` class. The code written after the "`=`" will be exactly what is used inside the generated classes. Unfortunately, this means you won't be able to use a private constant as the default value. We'll be looking for ways to improve this.
+Notes about arguments:
+- They must be one of `String`, `Boolean`, `Int`, `Float`, `Long` to be considered navigation arguments.
+  `NavController`, `NavBackStackEntry` or `ScaffoldState` (only if you are using `Scaffold`) can also be used by all destinations.
+- Navigation arguments with default values must be resolvable from the generated `DestinationSpec` class since the code written after the "`=`" 
+  will be copied into the generated classes. 
+Unfortunately, this means you won't be able to use a private constant as the default value. We'll be looking for ways to improve this.
 
-### Going deeper:
-
-- All annotated composables will generate an implementation of `DestinationSpec` which is a sealed interface that contains the full route, navigation arguments,
-`Content` composable function that will call your `Composable` and the `withArgs` implementation.
-- `Scaffold` composable lambda parameters will be given a current `DestinationSpec`. This makes it trivial to have top bar, bottom bar and drawer depend on the current destination.
-- If you would like to have additional properties/functions in the `DestinationSpec` (for example a "title" which will be shown to the user for each screen) you can make an extension 
-property/function of `DestinationSpec`. Since it is a sealed interface, a sealed `when` expression will make sure you always have a definition for each screen (check 
-`DestinationSpecExtensions.kt` file in the sample app).
-
-### Current state
-
-At the moment I haven't done any release because I don't really want people to be using this just yet on real projects. I want to be able to change the APIs without worries.
-I'm looking for all kinds of feedback, issues, feature requests and help in improving the code or even this README. So please, if you find this interesting, try it out in some sample projects and let me know how it goes! 
-
-
-### Import it
+### Dependencies
 
 For now, in order to try Compose Destinations, add jitpack repository and point to a specific commit. For the public releases, I may change it to maven central.
 
@@ -122,11 +106,39 @@ plugins {
 
 Add the dependencies:
 ```gradle
-    implementation 'com.github.raamcosta.compose-destinations:core:e5ff2ae7db'
-    ksp 'com.github.raamcosta.compose-destinations:ksp:e5ff2ae7db'
+implementation 'com.github.raamcosta.compose-destinations:core:e5ff2ae7db'
+ksp 'com.github.raamcosta.compose-destinations:ksp:e5ff2ae7db'
 
 ```
 
+And finally, you need to make sure the IDE looks at the generated folder.
+See KSP related [issue](https://github.com/google/ksp/issues/37).
+An example for the debug variant would be:
+```gradle
+sourceSets {
+    //...
+    main {
+        java.srcDir(file("build/generated/ksp/debug/kotlin"))
+    }
+}
+```
+
+### Going deeper:
+
+- All annotated composables will generate an implementation of `DestinationSpec` which is a sealed interface that contains the full route, navigation arguments,
+  `Content` composable function and the `withArgs` implementation.
+- `Scaffold` composable lambda parameters will be given a current `DestinationSpec`. This makes it trivial to have top bar, bottom bar and drawer depend on the current destination.
+- Besides the `NavHost` and `Scaffold` wrappers, the generated `Destinations` class contains a collection of all `DestinationSpec`s as well as the `startDestination`.
+- If you would like to have additional properties/functions in the `DestinationSpec` (for example a "title" which will be shown to the user for each screen) you can make an extension
+  property/function of `DestinationSpec`. Since it is a sealed interface, a `when` expression will make sure you always have a definition for each screen (check
+  `DestinationSpecExtensions.kt` file in the sample app).
+
+### Current state
+
+This lib is still in its alpha stage, APIs can change.
+I'm looking for all kinds of feedback, issues, feature requests and help in improving the code or even this README. So please, if you find this interesting, try it out in
+some sample projects and let me know how it goes!
+
 ## License
 
     Copyright 2021 Rafael Costa

diff --git a/app/src/main/java/com/ramcosta/samples/destinationstodosample/destinations/Greeting.kt b/app/src/main/java/com/ramcosta/samples/destinationstodosample/destinations/Greeting.kt
@@ -17,15 +17,16 @@ import com.ramcosta.composedestinations.GreetingDestination
 import com.ramcosta.composedestinations.ProfileDestination
 import com.ramcosta.composedestinations.annotation.Destination
 import com.ramcosta.samples.destinationstodosample.title
+import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 
 @Destination(route = "greeting", start = true)
 @Composable
 fun Greeting(
     navController: NavController,
-    scaffoldState: ScaffoldState
+    scaffoldState: ScaffoldState,
+    coroutineScope: CoroutineScope = rememberCoroutineScope()
 ) {
-    val coroutineScope = rememberCoroutineScope()
     Box(
         modifier = Modifier
             .fillMaxSize()

diff --git a/app/src/main/java/com/ramcosta/samples/destinationstodosample/destinations/Settings.kt b/app/src/main/java/com/ramcosta/samples/destinationstodosample/destinations/Settings.kt
@@ -31,7 +31,7 @@ fun Profile(
             .background(Color.Green)
     ) {
         Text(
-            text = "Profile ${ProfileDestination.route} " +
+            text = "Profile route: ${ProfileDestination.route} " +
                     "\n\nARGS =" +
                     "\n " +
                     "\n arg0= $arg0" +

diff --git a/...ions-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/DefaultValue.kt b/...ions-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/DefaultValue.kt
diff --git a/...tions-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/Destination.kt b/...tions-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/Destination.kt
@@ -1,6 +1,6 @@
 package com.ramcosta.composedestinations.codegen.model
 
-class Destination(
+data class Destination(
     val name: String,
     val qualifiedName: String,
     val composableName: String,

diff --git a/.../src/main/java/com/ramcosta/composedestinations/codegen/model/GeneratedDestinationFile.kt b/.../src/main/java/com/ramcosta/composedestinations/codegen/model/GeneratedDestinationFile.kt
@@ -1,6 +1,6 @@
 package com.ramcosta.composedestinations.codegen.model
 
-class GeneratedDestinationFile(
+data class GeneratedDestinationFile(
     val qualifiedName: String,
     val simpleName: String,
     val isStartDestination: Boolean

diff --git a/...nations-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/Parameter.kt b/...nations-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/Parameter.kt
@@ -1,9 +1,11 @@
 package com.ramcosta.composedestinations.codegen.model
 
-class Parameter(
+data class Parameter(
     val name: String,
     val type: Type,
-    val defaultValue: DefaultValue
+    val defaultValueSrc: String?
 ) {
-    val isMandatory: Boolean get() = !type.isNullable && defaultValue is None
+    val hasDefault get() = defaultValueSrc != null
+
+    val isMandatory: Boolean get() = !type.isNullable && !hasDefault
 }
diff --git a/...destinations-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/Type.kt b/...destinations-codegen/src/main/java/com/ramcosta/composedestinations/codegen/model/Type.kt
@@ -1,6 +1,6 @@
 package com.ramcosta.composedestinations.codegen.model
 
-class Type(
+data class Type(
     val simpleName: String,
     val qualifiedName: String,
     val isNullable: Boolean

diff --git a/...in/java/com/ramcosta/composedestinations/codegen/processors/SingleDestinationProcessor.kt b/...in/java/com/ramcosta/composedestinations/codegen/processors/SingleDestinationProcessor.kt
@@ -11,6 +11,7 @@ import com.ramcosta.composedestinations.codegen.templates.NAV_ARGUMENTS
 import com.ramcosta.composedestinations.codegen.templates.SYMBOL_QUALIFIED_NAME
 import com.ramcosta.composedestinations.codegen.templates.WITH_ARGS_METHOD
 import com.ramcosta.composedestinations.codegen.templates.destinationTemplate
+import java.lang.IllegalStateException
 
 class SingleDestinationProcessor(
     private val codeGenerator: CodeOutputStreamMaker,
@@ -33,7 +34,7 @@ class SingleDestinationProcessor(
             .replace(DESTINATION_NAME, fileName)
             .replace(COMPOSED_ROUTE, constructRoute())
             .replace(NAV_ARGUMENTS, navArgumentsDeclarationCode())
-            .replace(CONTENT_FUNCION_CODE, callActualComposable())
+            .replace(CONTENT_FUNCION_CODE, contentFunctionCode())
             .replace(WITH_ARGS_METHOD, withArgsMethod())
 
         outputStream.close()
@@ -93,8 +94,8 @@ class SingleDestinationProcessor(
 
     private fun defaultValueWithArgs(it: Parameter): String {
         return when {
-            it.defaultValue is Known -> {
-                " = ${it.defaultValue.srcCode}"
+            it.hasDefault -> {
+                " = ${it.defaultValueSrc}"
             }
 
             it.type.isNullable -> " = null"
@@ -118,39 +119,50 @@ class SingleDestinationProcessor(
         return destination.cleanRoute + mandatoryArgs.toString() + optionalArgs.toString()
     }
 
-    private fun callActualComposable(): String = with(destination) {
+    private fun contentFunctionCode(): String = with(destination) {
         return "${composableName}(${prepareArguments(parameters)})"
     }
 
     private fun prepareArguments(parameters: List<Parameter>): String {
         var argsCode = ""
 
         parameters.forEachIndexed { i, it ->
-            if (i != 0) argsCode += ", "
+            val argumentResolver = resolveArgumentForTypeAndName(it)
 
-            argsCode += "\n\t\t\t${it.name} = ${resolveArgumentForTypeAndName(it)}"
+            if (argumentResolver != null) {
+                if (i != 0) argsCode += ", "
+
+                argsCode += "\n\t\t\t${it.name} = $argumentResolver"
+
+            } else if (!it.hasDefault) {
+                throw IllegalStateException("Unresolvable argument without default value: $it")
+            }
 
             if (i == parameters.lastIndex) argsCode += "\n\t\t"
         }
 
         return argsCode
     }
 
-    private fun resolveArgumentForTypeAndName(parameter: Parameter): String {
+    private fun resolveArgumentForTypeAndName(parameter: Parameter): String? {
         return when (parameter.type.qualifiedName) {
             NAV_CONTROLLER_QUALIFIED_NAME -> "navController"
             NAV_BACK_STACK_ENTRY_QUALIFIED_NAME -> "navBackStackEntry"
             SCAFFOLD_STATE_QUALIFIED_NAME -> "scaffoldState ?: throw RuntimeException(\"'scaffoldState' was requested but we don't have it. Is this screen a part of a Scaffold?\")"
-            else -> "navBackStackEntry.arguments?.${parameter.type.toNavBackStackEntryArgGetter(parameter.name)}${defaultCodeIfArgNotPresent(parameter)}"
+            else -> {
+                if (navArgs.contains(parameter)) {
+                    "navBackStackEntry.arguments?.${parameter.type.toNavBackStackEntryArgGetter(parameter.name)}${defaultCodeIfArgNotPresent(parameter)}"
+                } else null
+            }
         }
     }
 
     private fun defaultCodeIfArgNotPresent(parameter: Parameter): String {
 
-        if (parameter.defaultValue is Known) {
-            return if (parameter.defaultValue.srcCode == "null") {
+        if (parameter.hasDefault) {
+            return if (parameter.defaultValueSrc == "null") {
                 ""
-            } else " ?: ${parameter.defaultValue.srcCode}"
+            } else " ?: ${parameter.defaultValueSrc}"
         }
 
         if (parameter.type.isNullable) {
@@ -171,7 +183,7 @@ class SingleDestinationProcessor(
             code += "navArgument(\"${it.name}\") {\n\t\t\t"
             code += "type = ${it.type.toNavTypeCode()}\n\t\t\t"
             code += "nullable = ${it.type.isNullable}\n\t\t"
-            code += navArgDefaultCode(it.defaultValue)
+            code += navArgDefaultCode(it.defaultValueSrc)
             code += "}"
 
             code += if (i != navArgs.lastIndex) {
@@ -184,8 +196,8 @@ class SingleDestinationProcessor(
         return code.toString()
     }
 
-    private fun navArgDefaultCode(argDefault: DefaultValue): String {
-        return if (argDefault is Known) "\tdefaultValue = ${argDefault.srcCode}\n\t\t" else ""
+    private fun navArgDefaultCode(argDefault: String?): String {
+        return if (argDefault != null) "\tdefaultValue = ${argDefault}\n\t\t" else ""
     }
 
     private fun Type.toNavBackStackEntryArgGetter(argName: String): String {

diff --git a/...p/src/main/kotlin/com/ramcosta/composedestinations/commons/DefaultParameterValueReader.kt b/...p/src/main/kotlin/com/ramcosta/composedestinations/commons/DefaultParameterValueReader.kt
@@ -2,9 +2,6 @@ package com.ramcosta.composedestinations.commons
 
 import com.google.devtools.ksp.symbol.FileLocation
 import com.google.devtools.ksp.symbol.KSValueParameter
-import com.ramcosta.composedestinations.codegen.model.DefaultValue
-import com.ramcosta.composedestinations.codegen.model.Known
-import com.ramcosta.composedestinations.codegen.model.None
 import java.io.File
 
 internal class DefaultParameterValueReader {
@@ -13,7 +10,7 @@ internal class DefaultParameterValueReader {
         lineText: String,
         argName: String,
         argType: String,
-    ): DefaultValue {
+    ): String? {
         var auxText = lineText
 
         val anchors = arrayOf(
@@ -37,14 +34,14 @@ internal class DefaultParameterValueReader {
         if (index != -1)
             auxText = auxText.removeRange(index, auxText.length)
 
-        return Known(auxText)
+        return auxText
     }
 }
 
 internal val reader = DefaultParameterValueReader()
 
-internal fun KSValueParameter.getDefaultValue(): DefaultValue {
-    if (!hasDefault) return None
+internal fun KSValueParameter.getDefaultValue(): String? {
+    if (!hasDefault) return null
 
     /*
         This is not ideal: having to read the first n lines of the file,

diff --git a/...c/test/kotlin/com/ramcosta/composedestinations/commons/DefaultParameterValueReaderTest.kt b/...c/test/kotlin/com/ramcosta/composedestinations/commons/DefaultParameterValueReaderTest.kt
@@ -1,7 +1,5 @@
 package com.ramcosta.composedestinations.commons
 
-import com.ramcosta.composedestinations.codegen.model.DefaultValue
-import com.ramcosta.composedestinations.codegen.model.Known
 import org.junit.Test
 
 class DefaultParameterValueReaderTest {
@@ -13,19 +11,19 @@ class DefaultParameterValueReaderTest {
             lineText = "    arg1: String? = \"defaultArg\") {",
             argName = "arg1",
             argType = "String",
-            expected = Known("\"defaultArg\"")
+            expected = "\"defaultArg\""
         ),
         TestCase(
             lineText = "    arg1: String? = \"defaultArg\"",
             argName = "arg1",
             argType = "String",
-            expected = Known("\"defaultArg\"")
+            expected = "\"defaultArg\""
         ),
         TestCase(
             lineText = "    arg1: String? = \"defaultArg\",",
             argName = "arg1",
             argType = "String",
-            expected = Known("\"defaultArg\"")
+            expected = "\"defaultArg\""
         )
     )
 
@@ -46,6 +44,6 @@ class DefaultParameterValueReaderTest {
         val lineText: String,
         val argName: String,
         val argType: String,
-        val expected: DefaultValue
+        val expected: String?
     )
 }