apple
Behavior differs between local and non-local properties with no default value
Repro:
class A {}
a: A
// result: a {}
local aLocal: A
// error: Missing property value.
I would expect aLocal to be initialized to the default value of A here instead.
This error is thrown during parsing, so it's not even necessary to have aLocal in the evaluation path to trigger it.
Also pkl-intellij does not register this as an error, only pkl eval.
I'm actually not sure why it behaves this way. I can't think of a good reason for the language to do this; might okay to make this behave like a normal property (default to the type's default value)
I think that you are just putting the value without just explaining the old value which is not providing the first value any point
@HT154 What’s the use case? Unlike a regular property, a local property cannot be amended in place. I can’t think of many valid reasons to set a local property to the default object value, so it might pay off to be explicit.
I think amending is orthogonal here; normal properties will default to the type's default value.
a: A
Is effectively the same as:
a: A = new A {}
It's strange that this works:
a: A
b = a
But this suddenly breaks:
local a: A
b = a
Also, const and fixed properties also cannot be amended either, but they, like other properties, default to the type's default value.
By the way, welcome back!
normal properties will default to the type's default value.
True, except that many types don’t have default values and always need to be assigned explicitly.
Also, const and fixed properties also cannot be amended either, but they, like other properties, default to the type's default value.
Under the assumption that default values are rarely useful for local/fixed/const properties, requiring explicit assignment will avoid mistakes. I don’t have a strong opinion on this issue, but being selectively more restrictive can sometimes be a tolerable/beneficial form of inconsistency.
My use case here is, once again, abusing the language to get more namespacing without more modules. Sorry!
I have a bunch of similar looking but unrelated types and I want to build sets of mixins for them. To that end, I have a module that look like this:
// mixins.pkl
class MixinsForA {
const mix1: Mixin<A> = new { ... }
const function mix2(arg): Mixin<A> = new { ... }
}
class MixinsForB {
const mix1: Mixin<B> = new { ... }
const function mix2(arg): Mixin<B> = new { ... }
}
Usage looks like this:
import "mixins.pkl"
local a: mixins.MixinsForA = new {}
local b: mixins.MixinsForB = new {}
// someA |> a.mix1 |> a.mix2("foo")
A couple key goals here are having a single import point and reducing the amount of code needed to use/reference each mixin.
Our Kubernetes package has fixed properties without explicitly assigned values: https://github.com/apple/pkl-k8s/blob/65907044bcf76ff0347abbb84e307fd291e0ad70/generated-package/api/core/v1/Pod.pkl#L29-L31
I'm not sure what errors we might be preventing here; you still get an error if the property's type has no default, but only when accessed. I think that better follows Pkl's lazy ethos. Also, parse-time checks like this can't be caught using test.catch().
We can (and probably should) add an IDE error if a local property's type has no default value, which is the same as what we do with const and fixed:
I don’t have a strong opinion on this issue, but being selectively more restrictive can sometimes be a tolerable/beneficial form of inconsistency.
Yeah, makes sense, good food for thought.
I'm not sure what errors we might be preventing here
When I see local a: A, I immediately wonder if the author forgot to set a value (and they might have). To me it looks strange that a property that can’t be set anywhere else doesn’t have an explicit value. The analogy that comes to mind is immutable locals in GPLs, which typically require an explicit value.
For module properties, defaulting to new {} avoids a lot of noise. For locals, it’s not clear why new {} is a useful default.
We can (and probably should) add an IDE error if a local property's type has no default value
I can’t tell if you were implying otherwise, but I believe that IntelliJ should only give an error if parsing/evaluation is guaranteed to produce an error. Otherwise, IntelliJ should give a warning.
I can’t tell if you were implying otherwise, but I believe that IntelliJ should only give an error if parsing/evaluation is guaranteed to produce an error. Otherwise, IntelliJ should give a warning.
We only show an error annotation if we know it will error during eval. And, a local property whose type has no default, will indeed raise an error during eval (as long as the property is executed).
I don't have a super strong opinion here, but I agree with odenix that it's better to be explicit in this case. As an empty local might be a legitimate error from the user side, but for normal properties you can't be sure. We could change IntelliJ's and tree-sitter's grammar to not allow an empty local, as, right now, this will always throw in the ast builder, regardless if the variable is read or not.
