From 74eae0388e333e65c00bdfa882c1eaa80ca9b0cd Mon Sep 17 00:00:00 2001 From: Jen Basch Date: Fri, 5 Jun 2026 09:50:34 -0700 Subject: [PATCH] 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. --- .../runtime/VmUndefinedValueException.java | 2 +- .../input/errors/convertProperty1.pkl | 20 +++++++++++++++++ .../output/errors/convertProperty1.err | 16 ++++++++++++++ .../test/kotlin/org/pkl/core/EvaluatorTest.kt | 22 +++++++++++++++++++ stdlib/Command.pkl | 8 +++---- stdlib/base.pkl | 2 +- 6 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/input/errors/convertProperty1.pkl create mode 100644 pkl-core/src/test/files/LanguageSnippetTests/output/errors/convertProperty1.err diff --git a/pkl-core/src/main/java/org/pkl/core/runtime/VmUndefinedValueException.java b/pkl-core/src/main/java/org/pkl/core/runtime/VmUndefinedValueException.java index f35844558..4a6577750 100644 --- a/pkl-core/src/main/java/org/pkl/core/runtime/VmUndefinedValueException.java +++ b/pkl-core/src/main/java/org/pkl/core/runtime/VmUndefinedValueException.java @@ -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('.'); diff --git a/pkl-core/src/test/files/LanguageSnippetTests/input/errors/convertProperty1.pkl b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/convertProperty1.pkl new file mode 100644 index 000000000..a75d828e3 --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/input/errors/convertProperty1.pkl @@ -0,0 +1,20 @@ +class WrapTypeWithName extends ConvertProperty { + // render: (Pair, BaseValueRenderer) -> Pair + 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 diff --git a/pkl-core/src/test/files/LanguageSnippetTests/output/errors/convertProperty1.err b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/convertProperty1.err new file mode 100644 index 000000000..45e377a7f --- /dev/null +++ b/pkl-core/src/test/files/LanguageSnippetTests/output/errors/convertProperty1.err @@ -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) diff --git a/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt b/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt index cf0bee384..490253e88 100644 --- a/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt +++ b/pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt @@ -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") diff --git a/stdlib/Command.pkl b/stdlib/Command.pkl index 6b4c0b977..8d0802a6f 100644 --- a/stdlib/Command.pkl +++ b/stdlib/Command.pkl @@ -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)? + hidden transformAll: ((List) -> 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)? + hidden transformAll: ((List) -> Any)? /// Specify how this flag should be completed in generated shell completions. /// diff --git a/stdlib/base.pkl b/stdlib/base.pkl index a684ad6ab..bec7084ba 100644 --- a/stdlib/base.pkl +++ b/stdlib/base.pkl @@ -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, BaseValueRenderer) -> Pair + hidden render: (Pair, BaseValueRenderer) -> Pair } /// Base class for rendering Pkl values in some textual output format.