Skip to content

Commit 74eae03

Browse files
authored
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.
1 parent 8b6b90d commit 74eae03

6 files changed

Lines changed: 64 additions & 6 deletions

File tree

pkl-core/src/main/java/org/pkl/core/runtime/VmUndefinedValueException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public VmUndefinedValueException fillInHint(Deque<Object> path, Object topLevelV
7474
if (topLevelValue instanceof VmTyped typed && typed.isModuleObject()) {
7575
builder
7676
.append(" of module `")
77-
.append(typed.getModuleInfo().getModuleSchema(typed).getModuleUri())
77+
.append(typed.getModuleInfo().getModuleKey().getUri())
7878
.append('`');
7979
}
8080
builder.append('.');
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
class WrapTypeWithName extends ConvertProperty {
2+
// render: (Pair<String, Any>, BaseValueRenderer) -> Pair<String, Any>
3+
render = (p, _) ->
4+
Pair(p.key, new Mapping {
5+
[p.value.getClass().simpleName] = p.value
6+
})
7+
}
8+
9+
class Something {
10+
@WrapTypeWithName
11+
prop1: String
12+
prop2: Int
13+
}
14+
15+
a: Something = new Something {
16+
prop2 = 1
17+
}
18+
19+
// this should result in: Tried to read property `prop1` but its value is undefined.
20+
// any other error would result from an error in the reporting path
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
–– Pkl Error ––
2+
Tried to read property `prop1` but its value is undefined.
3+
4+
xx | prop1: String
5+
^^^^^
6+
at convertProperty1#Something.prop1 (file:///$snippetsDir/input/errors/convertProperty1.pkl)
7+
8+
The above error occurred when rendering path `a.prop1` of module `file:///$snippetsDir/input/errors/convertProperty1.pkl`.
9+
10+
xxx | renderer.renderDocument(value)
11+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
at pkl.base#Module.output.text (pkl:base)
13+
14+
xxx | if (renderer is BytesRenderer) renderer.renderDocument(value) else text.encodeToBytes("UTF-8")
15+
^^^^
16+
at pkl.base#Module.output.bytes (pkl:base)

pkl-core/src/test/kotlin/org/pkl/core/EvaluatorTest.kt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -750,6 +750,28 @@ class EvaluatorTest {
750750
.hasMessageContaining("Cannot resolve dependency because there is no project found.")
751751
}
752752

753+
@Test
754+
fun `eval schema when property has a ConvertProperty annotation`() {
755+
// this fails when `ConvertProperty.render` is not hidden
756+
// because module schema generation exports all annotations
757+
// and this annotation has a Function2 property that is not exportable
758+
val evaluator = Evaluator.preconfigured()
759+
assertThatCode {
760+
evaluator.evaluateSchema(
761+
text(
762+
"""
763+
@ConvertProperty {
764+
render = (prop, ) -> prop
765+
}
766+
foo: String
767+
"""
768+
.trimIndent()
769+
)
770+
)
771+
}
772+
.doesNotThrowAnyException()
773+
}
774+
753775
private fun checkModule(module: PModule) {
754776
assertThat(module.properties.size).isEqualTo(2)
755777
assertThat(module.getProperty("name")).isEqualTo("pigeon")

stdlib/Command.pkl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class Flag extends BaseFlag {
165165
/// | [Listing], [List], [Set] | Element values are parsed based on the above primitive types |
166166
/// | [Mapping], [Map], [Pair] | Value is split into a [Pair] on the first `"="` and each substring is parsed based on the above primitive types |
167167
/// | Other types | An error is thrown; `convert` should be defined explicitly |
168-
convert: ((String) -> Any)?
168+
hidden convert: ((String) -> Any)?
169169

170170
/// Specifies whether the flag may be specified more than once.
171171
///
@@ -188,7 +188,7 @@ class Flag extends BaseFlag {
188188
/// | [Listing], [List] | Result is all option values in the order specified |
189189
/// | [Set] | Result is all unique option values |
190190
/// | Other types | Result is the last value specified option value |
191-
transformAll: ((List<Any>) -> Any)?
191+
hidden transformAll: ((List<Any>) -> Any)?
192192

193193
/// Specify how this flag should be completed in generated shell completions.
194194
///
@@ -226,7 +226,7 @@ class Argument extends Annotation {
226226
///
227227
/// If no value is provided, each option value is transformed using the same rules as
228228
/// [Flag.convert].
229-
convert: ((String) -> Any)?
229+
hidden convert: ((String) -> Any)?
230230

231231
/// Specifies whether the argument may be specified more than once.
232232
///
@@ -245,7 +245,7 @@ class Argument extends Annotation {
245245
///
246246
/// If no value is provided, all option values are transformed using the same rules as
247247
/// [Flag.transformAll].
248-
transformAll: ((List<Any>) -> Any)?
248+
hidden transformAll: ((List<Any>) -> Any)?
249249

250250
/// Specify how this flag should be completed in generated shell completions.
251251
///

stdlib/base.pkl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ abstract class BaseValueRenderer {
414414
open class ConvertProperty extends Annotation {
415415
/// Function called by [BaseValueRenderer] types during rendering to transform property
416416
/// names and values.
417-
render: (Pair<String, Any>, BaseValueRenderer) -> Pair<String, Any>
417+
hidden render: (Pair<String, Any>, BaseValueRenderer) -> Pair<String, Any>
418418
}
419419

420420
/// Base class for rendering Pkl values in some textual output format.

0 commit comments

Comments
 (0)