Local variables inside for loops yield an error #173

Open
opened 2025-12-30 01:21:44 +01:00 by adam · 5 comments
Owner

Originally created by @EugZol on GitHub (Jun 25, 2024).

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)
Originally created by @EugZol on GitHub (Jun 25, 2024). Code: ```pkl 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) ```
Author
Owner

@holzensp commented on GitHub (Jun 25, 2024):

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
    }
  }
}
@holzensp commented on GitHub (Jun 25, 2024): 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 } } }
Author
Owner

@EugZol commented on GitHub (Jun 25, 2024):

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 on GitHub (Jun 25, 2024): 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 ```pkl 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 ```diff 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 `when`s second piece of code starts to work.
Author
Owner

@EugZol commented on GitHub (Jun 25, 2024):

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.

@EugZol commented on GitHub (Jun 25, 2024): > 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.
Author
Owner

@holzensp commented on GitHub (Jun 25, 2024):

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)

@holzensp commented on GitHub (Jun 25, 2024): 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 `when`s 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 `when`s back)
Author
Owner

@EugZol commented on GitHub (Jun 25, 2024):

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:

  • for refusing to produce properties even if it is executed once: should be regular "double declaration error" instead
  • local being quasi-property instead of syntax sugar... maybe block form of let statement which could yield multiple members could be introduced instead
  • when not seeing members which are defined at the same level

Hope will be addressed in the future versions!

@EugZol commented on GitHub (Jun 25, 2024): > 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: * `for` refusing to produce properties even if it is executed once: should be regular "double declaration error" instead * `local` being quasi-property instead of syntax sugar... maybe block form of `let` statement which could yield multiple members could be introduced instead * `when` not seeing members which are defined at the same level Hope will be addressed in the future versions!
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pkl#173