Respect fixed properties when converting Map or Dynamic to Typed

[nileshpatil6]

Fixes #573

The bug: toTyped() on Map and Dynamic builds a set of delegating members that
read their value from the source Map/Dynamic. This is done the same way for
every property, regardless of whether the property is marked fixed. So
calling Map("id", 99).toTyped(SomeClass) on a class where id is declared
fixed id: Int = 1 produces a Typed object with id = 99, silently
overriding the fixed value.

Root cause: VmClass.createDelegatingMembers already special-cases hidden
properties by skipping them when building the delegating member map, so the
resulting Typed instance falls back to its prototype's value. It does not do
the same for fixed properties, even though getMapToTypedMembers() and
getDynamicToTypedMembers() both go through the same method as
getTypedToDynamicMembers().

Fix: createDelegatingMembers now takes a boolean telling it whether it is
building members for a conversion into Typed (dynamic/map -> typed) versus
out of Typed (typed -> dynamic). When converting into Typed, fixed
properties are skipped the same way hidden ones already are, so the class's
own fixed value wins instead of whatever happens to be in the source Map or
Dynamic. The typed -> dynamic direction is untouched, since that path already
reads off the already-evaluated Typed value and does not need this check.

Testing: I added cases to the existing snippet tests in
pkl-core/src/test/files/LanguageSnippetTests/input/api/map.pkl and
dynamic.pkl, with a small FixedPerson class that has a fixed id: Int = 1
property, converting from both Map and Dynamic with a different id value in
the source and asserting the output keeps id = 1. I could not finish
running the full Gradle test suite in this environment, the build was still
in the dependency resolution/compile phase after several minutes and I did
not want to sit on the fix while it might get picked up by someone else. I did
carefully review the change by hand: it's a two-line addition to an existing
early-continue check plus a threaded boolean parameter, all three call sites
updated consistently, and I confirmed ClassProperty.isFixed() already
exists and is used elsewhere in the codebase (UnresolvedPropertyNode), so no
new imports or methods were needed.

[HT154]

A couple questions:

1. Should attempts to set fixed members be silently dropped (current implementation) or should they be errors?
2. How are const properties affected? Should they be treated similarly?

[nileshpatil6]

Good questions.

1. Right now it's silently dropped, same as how hidden already works in this same method (I just followed that existing pattern rather than inventing new behavior). I don't have a strong opinion here honestly, silent felt safer since it matches what hidden already does and I didn't want to introduce a new kind of error in a bug fix PR. But if you'd rather it be a hard error when someone tries to set a fixed member through Dynamic/Map, I'm happy to change it, just let me know which way you want it and I'll update.

2. I don't think const needs the same treatment, and it's not really the same kind of thing. fixed is about preventing a value from being overridden (that's the whole bug here). const is a caching/purity thing, it just means the property gets evaluated once on the prototype and that value gets reused for every child in the amends chain, see VmUtils.doReadMember, it doesn't say anything about whether the value can be overridden from a Dynamic/Map source. So a const property going through this conversion should still just take whichever value is correct for it, nothing to special case.

Let me know on point 1 and I'll push a change if needed.

[HT154]

I'm interested in input from other maintainers on (1), so hold tight there.

For (2), this seems like blatantly incorrect behavior in the exact same way that this PR is addressing for fixed:

class Foo {
  const a = 1
}

b = Map("a", 2).toTyped(Foo)

Result:

b {
  a = 2
}

[nileshpatil6]

Good catch, you're right and I was wrong in my earlier reply, const does need the same treatment here, it's the same bug shape as fixed.

I dug into it more and found that Member already has an isConstOrFixed() helper, and it's used exactly this way elsewhere, e.g. GeneratorMemberNode.checkIsValidTypedProperty throws cannotAssignConstProperty/cannotAssignFixedProperty when a typed object literal tries to override either one. So the codebase already treats const and fixed as the same category for this kind of thing, I just missed applying that here.

Pushed a commit that swaps the isFixed() check in createDelegatingMembers to isConstOrFixed(), so const properties now get the same protection. Added matching const test cases right next to the existing fixed ones in map.pkl and dynamic.pkl. Ran the full api snippet test suite locally, 158 tests, 0 failures, including both files.

Still waiting on other maintainers for point 1 like you said, just wanted to get point 2 sorted since your repro made it obvious it was actually broken, not a style question.