Fix error rendering @ConvertProperty annotations during error reporting (#1648)

The added snippet test originally produced error "A value of type
`Function2` cannot be exported."

This PR actually fixes the bug twice:
* By marking `ConvertProperty.render` as `hidden` so that it is skipped
when the enclosing object is exported. This broke any attempts to obtain
the module schema because this requires exporting all annotations on all
class properties.
* By changing the way that `VmUndefinedValueException.fillInHint()`
obtains the module URI to avoid obtaining the module schema (and
triggering the more expensive module schema generation process).

It also makes function-typed annotation properties in `pkl:Command` hidden to avoid similar issues there.
This commit is contained in:
Jen Basch
2026-06-05 09:50:34 -07:00
committed by GitHub
parent 8b6b90d889
commit 74eae0388e
6 changed files with 64 additions and 6 deletions
@@ -74,7 +74,7 @@ public final class VmUndefinedValueException extends VmEvalException {
if (topLevelValue instanceof VmTyped typed && typed.isModuleObject()) {
builder
.append(" of module `")
.append(typed.getModuleInfo().getModuleSchema(typed).getModuleUri())
.append(typed.getModuleInfo().getModuleKey().getUri())
.append('`');
}
builder.append('.');
@@ -0,0 +1,20 @@
class WrapTypeWithName extends ConvertProperty {
// render: (Pair<String, Any>, BaseValueRenderer) -> Pair<String, Any>
render = (p, _) ->
Pair(p.key, new Mapping {
[p.value.getClass().simpleName] = p.value
})
}
class Something {
@WrapTypeWithName
prop1: String
prop2: Int
}
a: Something = new Something {
prop2 = 1
}
// this should result in: Tried to read property `prop1` but its value is undefined.
// any other error would result from an error in the reporting path
@@ -0,0 +1,16 @@
–– Pkl Error ––
Tried to read property `prop1` but its value is undefined.
xx | prop1: String
^^^^^
at convertProperty1#Something.prop1 (file:///$snippetsDir/input/errors/convertProperty1.pkl)
The above error occurred when rendering path `a.prop1` of module `file:///$snippetsDir/input/errors/convertProperty1.pkl`.
xxx | renderer.renderDocument(value)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at pkl.base#Module.output.text (pkl:base)
xxx | if (renderer is BytesRenderer) renderer.renderDocument(value) else text.encodeToBytes("UTF-8")
^^^^
at pkl.base#Module.output.bytes (pkl:base)
@@ -750,6 +750,28 @@ class EvaluatorTest {
.hasMessageContaining("Cannot resolve dependency because there is no project found.")
}
@Test
fun `eval schema when property has a ConvertProperty annotation`() {
// this fails when `ConvertProperty.render` is not hidden
// because module schema generation exports all annotations
// and this annotation has a Function2 property that is not exportable
val evaluator = Evaluator.preconfigured()
assertThatCode {
evaluator.evaluateSchema(
text(
"""
@ConvertProperty {
render = (prop, ) -> prop
}
foo: String
"""
.trimIndent()
)
)
}
.doesNotThrowAnyException()
}
private fun checkModule(module: PModule) {
assertThat(module.properties.size).isEqualTo(2)
assertThat(module.getProperty("name")).isEqualTo("pigeon")
+4 -4
View File
@@ -165,7 +165,7 @@ class Flag extends BaseFlag {
/// | [Listing], [List], [Set] | Element values are parsed based on the above primitive types |
/// | [Mapping], [Map], [Pair] | Value is split into a [Pair] on the first `"="` and each substring is parsed based on the above primitive types |
/// | Other types | An error is thrown; `convert` should be defined explicitly |
convert: ((String) -> Any)?
hidden convert: ((String) -> Any)?
/// Specifies whether the flag may be specified more than once.
///
@@ -188,7 +188,7 @@ class Flag extends BaseFlag {
/// | [Listing], [List] | Result is all option values in the order specified |
/// | [Set] | Result is all unique option values |
/// | Other types | Result is the last value specified option value |
transformAll: ((List<Any>) -> Any)?
hidden transformAll: ((List<Any>) -> Any)?
/// Specify how this flag should be completed in generated shell completions.
///
@@ -226,7 +226,7 @@ class Argument extends Annotation {
///
/// If no value is provided, each option value is transformed using the same rules as
/// [Flag.convert].
convert: ((String) -> Any)?
hidden convert: ((String) -> Any)?
/// Specifies whether the argument may be specified more than once.
///
@@ -245,7 +245,7 @@ class Argument extends Annotation {
///
/// If no value is provided, all option values are transformed using the same rules as
/// [Flag.transformAll].
transformAll: ((List<Any>) -> Any)?
hidden transformAll: ((List<Any>) -> Any)?
/// Specify how this flag should be completed in generated shell completions.
///
+1 -1
View File
@@ -414,7 +414,7 @@ abstract class BaseValueRenderer {
open class ConvertProperty extends Annotation {
/// Function called by [BaseValueRenderer] types during rendering to transform property
/// names and values.
render: (Pair<String, Any>, BaseValueRenderer) -> Pair<String, Any>
hidden render: (Pair<String, Any>, BaseValueRenderer) -> Pair<String, Any>
}
/// Base class for rendering Pkl values in some textual output format.