Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling introspected methods / constructors with default arguments in Kotlin #4379

Closed
morki opened this issue Oct 25, 2020 · 9 comments
Closed
Labels
lang: kotlin Issues or features specific to Kotlin relates-to: native-kotlin

Comments

@morki
Copy link
Contributor

morki commented Oct 25, 2020

Hi, I just found little bit annoying to workaround missing support for kotlin default parameters in binding query parameters.

Instead of using this:

data class Query(
  val search: String?,
  val orderBy: OrderBy = OrderBy.NAME,
  val page: Int = 1,
  val resultsOnPage: Int = 20,
)

I have to define auxiliary getters like:

data class Query(
  val search: String?,
  private val orderBy: OrderBy?,
  private val page: Int?,
  private val resultsOnPage: Int?,
) {
  val realOrderBy get() = orderBy ?: OrderBy.NAME
  val realPage get() = page ?: 1
  val realResultsOnPage get() = resultsOnPage ?: 20
}

It gets very complicated with many arguments and little bit unreadable.

I have read that default arguments are not available for reflection, because they can be arbitrary expressions.
But I thought that maybe it can be solved differently using this approach I have found on SO:

https://stackoverflow.com/questions/44792787/reflectively-calling-function-and-using-default-parameters

What do you think? Will it work in native image? Maybe with some relflection configuration...

It is just an idea to discuss, if you have another less verbose alternative it would be awesome.

EDIT: Kotlin module for Jackson is using the same approach:

https://github.com/FasterXML/jackson-module-kotlin/blob/130f0944be72d5dc3d3c88cd76abddb6dc9359a7/src/main/kotlin/com/fasterxml/jackson/module/kotlin/KotlinValueInstantiator.kt#L171

EDIT 2: When using default values in body deserialization (JSON), it works only if @Introspected annotation is not used, othewise it throws following exception: Failed to convert argument [body] for value [null] due to: Missing required creator property.

@morki
Copy link
Contributor Author

morki commented Oct 27, 2020

Maybe answer to my question on SO can be helpful, maybe not. I will add it here for reference.

https://stackoverflow.com/questions/64539969/how-to-use-kotlin-reflection-from-java/64543134?noredirect=1#comment114146639_64543134

@Flassie
Copy link

Flassie commented Feb 2, 2021

Bad thing about default values in kotlin is that they are not stored somewhere and can't be obtained from Metadata annotations.
As I can see now there is no solution to solve that problem but generating kotlin introspection classes for kotlin types where instantiateInternal method will accept named/marked arguments and call kotlin constructor/method with explicit arguments naming or calling reflection functions like callBy. Both methods, I think, will lead to codebase rework to call instantiateInternal with named args.

@morki
Copy link
Contributor Author

morki commented Feb 3, 2021

Just for information for others wanting this feature, I resolved this by disabling Jackson introspection module:

jackson:
  bean-introspection-module: false

For native image to work properly, adding Kotlin metadata resources and some reflection configuration solved it nicely.
Adding this resources and reflection info to native image added only 2MB to our 100MB application, so it was worth it:

reflect-config.json

[
  {
    "name": "java.util.List"
  },
  {
    "name": "java.util.Set"
  },
  {
    "name": "java.util.Map"
  },
  {
    "name": "java.lang.String"
  },
  {
    "name": "kotlin.Metadata",
    "allDeclaredMethods": true
  },
  {
    "name": "kotlin.KotlinVersion",
    "allDeclaredFields": true,
    "allDeclaredMethods": true,
    "allDeclaredConstructors": true
  },
  {
    "name": "kotlin.KotlinVersion[]"
  },
  {
    "name": "kotlin.KotlinVersion$Companion"
  },
  {
    "name": "kotlin.KotlinVersion$Companion[]"
  },
  {
    "name": "kotlin.jvm.internal.DefaultConstructorMarker"
  },
  {
    "name": "kotlin.reflect.jvm.internal.ReflectionFactoryImpl",
    "allDeclaredConstructors": true
  }
]

resource-config.json

{
  "resources": {
    "includes": [
      {
        "pattern": "META-INF/.*.kotlin_module$"
      },
      {
        "pattern": ".*.kotlin_builtins"
      }
    ]
  }
}

Source here: oracle/graal#2306 (comment)

It is now integrated to Spring, see comment below. Can we make this contribution to micronaut-kotlin project?

@graemerocher
Copy link
Contributor

Sure PRs, welcome. Might want to combine it with micronaut-projects/micronaut-kotlin#163 (comment)

@morki
Copy link
Contributor Author

morki commented Feb 3, 2021

Thank you @graemerocher, will provide PR. I have another questions, if you like to help me with answer.

Now I combine this with already provided @Introspected annotation, which is registering annotated classes for reflection but also creates BeanIntrospections, which are useless in this case.

Would you think it is ok (useless classes) or shall we create another annotation, which will only do reflection registration and not introspection?

@graemerocher
Copy link
Contributor

probably it makes sense to allow ReflectiveAccess to be declared on types since that is what you are doing essentially

@graemerocher
Copy link
Contributor

Note that the real solution is to support default argument values in the AST but I am not sure if that is possible with KAPT

@graemerocher graemerocher added the lang: kotlin Issues or features specific to Kotlin label Feb 3, 2021
@morki
Copy link
Contributor Author

morki commented Feb 3, 2021

Default argument values are not presented in kotlin reflection metadata (see @Flassie comment), and can contain references to themselves (see example), so I think it is not doable this way. But I will be happy if I am wrong.

Example:

data class Test(
  val first: Int = 8,
  val second: String = "I contain $first",
)

@Flassie
Copy link

Flassie commented Feb 3, 2021

Not only. It can contain anything:

data class Test(
    val a: String,
    val b: List<Int> = run {
        (0 until 100).map { it+2 }
    }
)

Kotlin generates only stubs for that and there is nothing interesting inside them
Default arguments are proceed inside binary code. For example code that was generated from example above:

    public TestExample1(@NotNull String a, @NotNull List<Integer> b) {
        Intrinsics.checkNotNullParameter((Object)a, (String)"a");
        Intrinsics.checkNotNullParameter(b, (String)"b");
        this.a = a;
        this.b = b;
    }

    /*
     * WARNING - void declaration
     */
    public /* synthetic */ TestExample1(String string, List list, int n, DefaultConstructorMarker defaultConstructorMarker) {
        if ((n & 2) != 0) {
            void $this$mapTo$iv$iv;
            boolean bl = false;
            boolean bl2 = false;
            boolean bl3 = false;
            Iterable $this$map$iv = (Iterable)RangesKt.until((int)0, (int)100);
            boolean $i$f$map = false;
            Iterable iterable = $this$map$iv;
            Collection destination$iv$iv = new ArrayList(CollectionsKt.collectionSizeOrDefault((Iterable)$this$map$iv, (int)10));
            boolean $i$f$mapTo = false;
            Iterator iterator = $this$mapTo$iv$iv.iterator();
            while (iterator.hasNext()) {
                void it;
                int item$iv$iv;
                int n2 = item$iv$iv = ((IntIterator)iterator).nextInt();
                Collection collection = destination$iv$iv;
                boolean bl4 = false;
                Integer n3 = (int)(it + 2);
                collection.add(n3);
            }
            list = (List)destination$iv$iv;
        }
        this(string, list);
    }

So yes. I think it is not doable using kapt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang: kotlin Issues or features specific to Kotlin relates-to: native-kotlin
Projects
None yet
Development

No branches or pull requests

4 participants