apple / apple/pkl

ValueMapper/PObjectToDataObject converter cannot deserialize to records with generic components AND with custom constructors

Open
#1,485 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
11.5k
Forks
402
Avg merge
1d 15h
Merged PRs (30d)
20

Description

`ValueMapper`, or, to be precise, the `PObjectToDataObject` converter, analyzes constructors of classes to figure out how to map Pkl structures to them. This allows one to use `ValueMapper` to decode Pkl structures to any class, as long as its constructor matches the decoded Pkl value. It works even if you don't use codegen, and if you, say, define a record with appropriate components, it will "just work" (in most cases).

Now, consider these two records:
```java
record Foo1(List input) {}

record Foo2(List input) {
public Foo2 {
if (input.isEmpty()) throw new IllegalArgumentException("input is empty");
}
}
```

and this Pkl module:
```pkl
class Foo {
input: Listing
}

x: Foo = new { input { "abc" } }
```

if the `x` property of this Pkl module is decoded into `Foo1`, everything works. If it is decoded into `Foo2`, however, you get an extremely confusing error:
```
Target type `interface java.util.List` is missing type arguments.
java.lang.IllegalArgumentException: Target type `interface java.util.List` is missing type arguments.
at org.pkl.config.java.mapper.ValueMapperImpl.getConverter(ValueMapperImpl.java:83)
at org.pkl.config.java.mapper.PObjectToDataObject$ConverterImpl.convert(PObjectToDataObject.java:204)
at org.pkl.config.java.mapper.PObjectToDataObject$ConverterImpl.convert(PObjectToDataObject.java:154)
at org.pkl.config.java.mapper.ValueMapperImpl.map(ValueMapperImpl.java:57)
at org.pkl.config.java.AbstractConfig.as(AbstractConfig.java:57)
at org.pkl.config.java.AbstractConfig.as(AbstractConfig.java:52)
```

Simple reproducer code:
```java
package com.example;

import org.pkl.config.java.ConfigEvaluator;
import org.pkl.core.ModuleSource;

import java.util.List;

public class DecodingIssueTest {
public record Foo1(List input) {}

public record Foo2(List input) {
public Foo2 {
if (input.isEmpty()) throw new IllegalArgumentException("input is empty");
}
}

public static void main(String[] args) {
final var moduleText = """
class Foo {
input: Listing
}

x: Foo = new { input { "abc" } }
""";

try (final var evaluator = ConfigEvaluator.preconfigured()) {
// This works fine, you can see `foo1` being printed.
final var foo1 = evaluator.evaluate(ModuleSource.text(moduleText)).get("x").as(Foo1.class);
System.out.println(foo1);
// This throws.
final var foo2 = evaluator.evaluate(ModuleSource.text(moduleText)).get("x").as(Foo2.class);
System.out.println(foo2);
}
}
}
```
---
The reason, as I've discovered, is in this call:

https://github.com/apple/pkl/blob/82afa8b90bd3a7a954bbd5f9d213cf82f867449a/pkl-config-java/src/main/java/org/pkl/config/java/mapper/Reflection.java#L142-L143

`GenericTypeReflector` comes from the geantyref library, and internally it does [this](https://github.com/leangen/geantyref/blob/d1584975ca208bc60e79ea35a6451278b24c14d7/src/main/java/io/leangen/geantyref/GenericTypeReflector.java#L703):
```java
AnnotatedType[] parameterTypes = exe.getAnnotatedParameterTypes();
```

`exe` here is a `java.lang.reflect.Constructor` instance. `Executable.getAnnotatedParameterTypes()` returns an array of parameter types which may or may not be parameterized; they might as well be erased. JDK does not guarantee that they will not be erased (see discussion [here](https://stackoverflow.com/questions/23025363/how-to-get-generic-type-information-from-getannotatedparametertypes-in-java-8); only `getGenericParameterTypes()` explicitly guarantees to return generic information). And in fact, on modern JDKs, up to and including JDK 25, this is exactly what may happen: internally, in the `Executable` implementation, there is this piece of logic:
```java
if (param.isSynthetic() || param.isImplicit()) {
// If we hit a synthetic or mandated parameter,
// use the non generic parameter info.
out[i] = nonGenericParamTypes[i];
} else {
// Otherwise, use the generic parameter info.
out[i] = genericParamTypes[fromidx];
fromidx++;
}
```
which forces synthetic and implicit parameters to be returned as erased types.

The problem is, in the original `Foo2` record, because of the presence of the custom constructor, all its parameters are annotated as implicit, and therefore this method returns erased information about constructor parameters. This, in turn, breaks Pkl's converter resolution, since it relies on generic parameter info to find the appropriate converter.
`Foo1` does not have a custom constructor, so its parameters are not marked as implicit, and thus everything works fine.

Ideally, the geantyref library should probably take this into account and use `getGenericParameterTypes()`, but maybe there is a way to work around it in Pkl directly.

Contributor guide

Open the contributing guide

Research direction

Start with pkl-config-java/src/main/java/org/pkl/config/java/mapper/Reflection.java at the linked lines, then trace how PObjectToDataObject resolves constructor parameter converters. Run the supplied Foo1/Foo2 reproducer and compare reflection results for their constructors. Done means the custom-constructor record with a generic List component deserializes successfully without the missing-type-arguments error.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.