apple / pkl

A configuration as code language with rich validation and tooling.
https://pkl-lang.org
Apache License 2.0
9.84k stars 259 forks source link

Local variables inside for loops yield an error #549

Open EugZol opened 1 week ago

EugZol commented 1 week ago

Code:

hidden xs = new Listing { 1 2 3 }

object {
  for (x in xs) {
    local plusOne = x + 1
    plusOne
  }
}

On evaluation yields the following error:

A for-generator cannot generate object properties (only entries and elements).

Instead, expected to produce object with entries 2 3 4.

Version:

% pkl --version
Pkl 0.26.0 (macOS 14.3.1, native)
holzensp commented 1 week ago

This is a short-coming we have at the moment. local is a modifier for a property; not a "variable." It really behaves like a property, which is why you can't have one in a for generator (because it's the same as generating the same property - possibly with different values - over and over).

If, as in this case, you only use it in one expression, you can use let;

object {
  for (x in XS) {
    let (plusOne = x + 1)
      plusOne
  }
}

If you do need it in multiple places, a (admittedly sub-optimal) workaround is to use a for generator again;


object {
  for (x in XS) {
    for (plusOne in List(x + 1)) {
      plusOne
    }
  }
}
EugZol commented 1 week ago

My case was initially trying to group objects and map each subgroup under different properties of the result, but don't add the result property if the corresponding subgroup is empty. And another loop on top of that.

It went like this

Attempt 1: local variable in for-loop

class Entity {
  selectors: Listing<Selector>
}

class Selector {
  enabled: Boolean
  index: Int
}

hidden objects = new Listing<Entity> {
  new {
    selectors {
      new {
        enabled = false
        index = 1
      }
      new {
        enabled = true
        index = 2
      }
    }
  }
  new {
    selectors {
      new {
        enabled = false
        index = 3
      }
      new {
        enabled = true
        index = 4
      }
    }
  }
}

result: Listing = new {
  for (o in objects) {
    local grouped = o.selectors.toList().groupBy((x) -> x.enabled)

    new {
      when (grouped.containsKey(true)) {
        all_enabled {
          for (a in grouped[true]) {
            a.index
          }
        }
      }

      when (grouped.containsKey(false)) {
        all_disabled {
          for (a in grouped[false]) {
            a.index
          }
        }
      }
    }
  }
}

Error:

A for-generator cannot generate object properties (only entries and elements).

Attempt 2: local variable inside leaf object

result: Listing = new {
  for (o in objects) {
-
    new {
+      local grouped = o.selectors.toList().groupBy((x) -> x.enabled)

      when (grouped.containsKey(true)) {

Error:

Cannot find property `grouped`.

42 | when (grouped.containsKey(true)) {
           ^^^^^^^

Attempt 3: instance methods

I eventually was able to solve the problem with some instance methods on Entity.

However both errors above were rather unexpected for me. Curiously, removing whens second piece of code starts to work.

EugZol commented 1 week ago

If you do need it in multiple places, a (admittedly sub-optimal) workaround is to use a for generator again;

I actually tried that as well! Doesn't work either, apparently because all_enabled/all_enabled are properties as well, and that artificial for-loop would need to generate them.

holzensp commented 1 week ago

Works for me with that for-as-let, because all_enabled/all_disabled live inside that new {...} (which, btw, do you realise/intend that to be Dynamic)?

result: Listing = new {
  for (o in objects) {
    for (grouped in List(o.selectors.toList().groupBy((x) -> x.enabled))) {
      new {
        when (grouped.containsKey(true)) {
          all_enabled {
            for (a in grouped[true]) {
              a.index
            }
          }
        }

        when (grouped.containsKey(false)) {
          all_disabled {
            for (a in grouped[false]) {
              a.index
            }
          }
        }
      }
    }
  }
}

but also... this pattern of splitting a collection in two sub-collections based on a property is commonly called partitioning. Pkl has a partition method Collection, so I'd

result: Listing = new {
  for (o in objects) {
    let (partitionedSelectors = o.selectors.toList().partition((it) -> it.enabled))
      new {
        all_enabled {
          ...partitionedSelectors.first.map((it) -> it.index)
        }
        all_disabled {
          ...partitionedSelectors.second.map((it) -> it.index)
        }
      }
  }
}

(I'm omitting the whens around all_enabled and all_disabled, assuming that them being empty and them being null are equivalent; if not; you'd need to bring some whens back)

EugZol commented 1 week ago

Works for me with that for-as-let, because all_enabled/all_disabled live inside that new {...}

Should've tried that! I've put it inside new before.

which, btw, do you realise/intend that to be Dynamic?

That's an example. Does it matter?

but also... this pattern of splitting a collection in two sub-collections based on a property is commonly called partitioning

That's nice! Thanks for the hint.

To summarize what I felt was inconvenient:

Hope will be addressed in the future versions!