Fix length of listings with computed index (#797)

Motivation:
The following expression evaluates to 2 instead of 1:
new Listing { "value" } { [0 + 0] = "override" }.length

Changes:
- fix length computation in EntriesLiteralNode
- improve `api/listing` tests
- make snippet test failures diffable in IntelliJ

Result:
- fixes https://github.com/apple/pkl/issues/780
- improved dev experience in IntelliJ

pkl-commons-test/src/main/kotlin/org/pkl/commons/test/FileTestUtils.kt

@@ -19,6 +19,7 @@ import java.nio.file.Path
 import java.util.stream.Collectors
 import kotlin.io.path.*
 import org.assertj.core.api.Assertions.fail
+import org.opentest4j.AssertionFailedError
 import org.pkl.commons.*
 
 object FileTestUtils {
@@ -110,5 +111,11 @@ data class SnippetOutcome(val expectedOutFile: Path, val actual: String, val suc
   }
 
   private fun failWithDiff(message: String): Nothing =
-    throw PklAssertionFailedError(message, expected, actual)
+    if (System.getProperty("sun.java.command", "").contains("intellij")) {
+      // IntelliJ only shows diffs for AssertionFailedError
+      throw AssertionFailedError(message, expected, actual)
+    } else {
+      // Gradle test logging/report only shows diffs for PklAssertionFailedError
+      throw PklAssertionFailedError(message, expected, actual)
+    }
 }

pkl-core/src/main/java/org/pkl/core/ast/expression/literal/EntriesLiteralNode.java

@@ -43,15 +43,15 @@ public abstract class EntriesLiteralNode extends SpecializedObjectLiteralNode {
   @Children private final ExpressionNode[] keyNodes;
   private final ObjectMember[] values;
 
-  public EntriesLiteralNode(
+  protected EntriesLiteralNode(
       SourceSection sourceSection,
       VmLanguage language,
-      // contains local properties and default property (if present)
-      // does *not* contain entries with constant keys to maintain definition order of entries
       String qualifiedScopeName,
       boolean isCustomThisScope,
       @Nullable FrameDescriptor parametersDescriptor,
       UnresolvedTypeNode[] parameterTypes,
+      // contains local properties and default property (if present)
+      // does *not* contain entries with constant keys to maintain definition order of entries
       UnmodifiableEconomicMap<Object, ObjectMember> members,
       ExpressionNode[] keyNodes,
       ObjectMember[] values) {
@@ -103,7 +103,8 @@ public abstract class EntriesLiteralNode extends SpecializedObjectLiteralNode {
         frame.materialize(),
         parent,
         createListMembers(frame, parent.getLength()),
-        parent.getLength() + keyNodes.length);
+        // `[x] = y` overrides existing element and doesn't increase length
+        parent.getLength());
   }
 
   @Specialization

pkl-core/src/test/files/LanguageSnippetTests/input/api/listing.pkl

@@ -27,12 +27,20 @@ local altered: Listing<Person> = (base) {
   [0] { name = "Wood Pigeon" }
 }
 
+local computedIndex: Listing<Person> = (base) {
+  [computeIndex()] { name = "Wood Pigeon" }
+}
+
+local function computeIndex() = 0
+
 facts {
   ["isEmpty"] {
     empty.isEmpty
     empty2.isEmpty
     !base.isEmpty
     !derived.isEmpty
+    !altered.isEmpty
+    !computedIndex.isEmpty
   }
   
   ["lastIndex"] {
@@ -41,6 +49,8 @@ facts {
     base.lastIndex == 2
     derived.lastIndex == 4
     duplicate.lastIndex == 5
+    altered.lastIndex == 2
+    computedIndex.lastIndex == 2
   }
 
   ["isDistinct"] {
@@ -49,6 +59,8 @@ facts {
     base.isDistinct
     derived.isDistinct
     !duplicate.isDistinct
+    altered.isDistinct
+    computedIndex.isDistinct
   }
 
   ["isDistinctBy()"] {
@@ -57,18 +69,24 @@ facts {
     base.isDistinctBy((it) -> it)
     derived.isDistinctBy((it) -> it)
     !duplicate.isDistinctBy((it) -> it)
+    altered.isDistinctBy((it) -> it)
+    computedIndex.isDistinctBy((it) -> it)
 
     empty.isDistinctBy((it) -> it.name)
     empty2.isDistinctBy((it) -> it.name)
     base.isDistinctBy((it) -> it.name)
     derived.isDistinctBy((it) -> it.name)
     !duplicate.isDistinctBy((it) -> it.name)
+    altered.isDistinctBy((it) -> it.name)
+    computedIndex.isDistinctBy((it) -> it.name)
 
     empty.isDistinctBy((it) -> it.getClass())
     empty2.isDistinctBy((it) -> it.getClass())
     !base.isDistinctBy((it) -> it.getClass())
     !derived.isDistinctBy((it) -> it.getClass())
     !duplicate.isDistinctBy((it) -> it.getClass())
+    !altered.isDistinctBy((it) -> it.getClass())
+    !computedIndex.isDistinctBy((it) -> it.getClass())
   }
   
   ["getOrNull"] {
@@ -85,24 +103,32 @@ facts {
     module.catch(() -> empty.first) == "Expected a non-empty Listing."
     base.first == base[0]
     derived.first == base[0]
+    altered.first != base[0]
+    computedIndex.first == altered.first
   }
   
   ["firstOrNull"] {
     empty.firstOrNull == null
     base.firstOrNull == base[0]
     derived.firstOrNull == base[0]
+    altered.firstOrNull != base[0]
+    computedIndex.firstOrNull == altered.first
   }
 
   ["last"] {
     module.catch(() -> empty.last) == "Expected a non-empty Listing."
     base.last == base[2]
     derived.last == derived[4]
+    altered.last == base[2]
+    computedIndex.last == base[2]
   }
 
   ["lastOrNull"] {
     empty.lastOrNull == null
     base.lastOrNull == base[2]
     derived.lastOrNull == derived[4]
+    altered.lastOrNull == base[2]
+    computedIndex.lastOrNull == base[2]
   }
 
   ["single"] {
@@ -135,6 +161,7 @@ facts {
     derived.contains(base[1])
     derived.contains(derived[3])
     !altered.contains(base[0])
+    !computedIndex.contains(base[0])
   }
 }
 
@@ -144,6 +171,17 @@ examples {
     empty2.length
     base.length
     derived.length
+    altered.length
+    computedIndex.length
+    
+    local elementsAndEntries = (base) {
+      new { name = "" }
+      [2] { name = "" }
+      [computeIndex()] { name = "" }
+      new { name = "" }
+      [1 + 0] { name = "" }
+    }
+    elementsAndEntries.length
   }
 
   ["toList()"] {
@@ -152,6 +190,8 @@ examples {
     base.toList()
     derived.toList()
     duplicate.toList()
+    altered.toList()
+    computedIndex.toList()
   }
 
   ["toSet()"] {
@@ -160,6 +200,8 @@ examples {
     base.toSet()
     derived.toSet()
     duplicate.toSet()
+    altered.toSet()
+    computedIndex.toSet()
   }
 
   ["distinct"] {
@@ -168,6 +210,8 @@ examples {
     base.distinct
     derived.distinct
     duplicate.distinct
+    altered.distinct
+    computedIndex.distinct
   }
 
   ["distinctBy()"] {
@@ -176,36 +220,48 @@ examples {
     base.distinctBy((it) -> it)
     derived.distinctBy((it) -> it)
     duplicate.distinctBy((it) -> it)
+    altered.distinctBy((it) -> it)
+    computedIndex.distinctBy((it) -> it)
 
     empty.distinctBy((it) -> it.name)
     empty2.distinctBy((it) -> it.name)
     base.distinctBy((it) -> it.name)
     derived.distinctBy((it) -> it.name)
     duplicate.distinctBy((it) -> it.name)
+    altered.distinctBy((it) -> it.name)
+    computedIndex.distinctBy((it) -> it.name)
 
     empty.distinctBy((it) -> it.getClass())
     empty2.distinctBy((it) -> it.getClass())
     base.distinctBy((it) -> it.getClass())
     derived.distinctBy((it) -> it.getClass())
     duplicate.distinctBy((it) -> it.getClass())
+    altered.distinctBy((it) -> it.getClass())
+    computedIndex.distinctBy((it) -> it.getClass())
   }
 
   ["fold"] {
     empty.fold(List(), (l, e) -> l.add(e))
     base.fold(List(), (l, e) -> l.add(e))
     derived.fold(List(), (l, e) -> l.add(e))
+    altered.fold(List(), (l, e) -> l.add(e))
+    computedIndex.fold(List(), (l, e) -> l.add(e))
   }
 
   ["foldIndexed"] {
     empty.foldIndexed(List(), (i, l, e) -> l.add(Pair(i, e)))
     base.foldIndexed(List(), (i, l, e) -> l.add(Pair(i, e)))
     derived.foldIndexed(List(), (i, l, e) -> l.add(Pair(i, e)))
+    altered.foldIndexed(List(), (i, l, e) -> l.add(Pair(i, e)))
+    computedIndex.foldIndexed(List(), (i, l, e) -> l.add(Pair(i, e)))
   }
 
 
   local baseNum = new Listing { 1; 2; 3 }
   local baseString = new Listing { "Pigeon"; "Barn Owl"; "Parrot" }
   local derivedString = (baseString) { "Albatross"; "Elf Owl" }
+  local alteredString = (baseString) { [0] = "Wood Pigeon" }
+  local computedIndexString = (baseString) { [computeIndex()] = "Wood Pigeon" }
 
   ["join"] {
     empty.join("")
@@ -215,5 +271,9 @@ examples {
     baseString.join("---")
     derivedString.join("")
     derivedString.join("\n")
+    alteredString.join("")
+    alteredString.join("\n")
+    computedIndexString.join("")
+    computedIndexString.join("\n")
   }
 }

pkl-core/src/test/files/LanguageSnippetTests/output/api/listing.pcf

@@ -4,6 +4,8 @@ facts {
     true
     true
     true
+    true
+    true
   }
   ["lastIndex"] {
     true
@@ -11,6 +13,8 @@ facts {
     true
     true
     true
+    true
+    true
   }
   ["isDistinct"] {
     true
@@ -18,6 +22,8 @@ facts {
     true
     true
     true
+    true
+    true
   }
   ["isDistinctBy()"] {
     true
@@ -35,6 +41,12 @@ facts {
     true
     true
     true
+    true
+    true
+    true
+    true
+    true
+    true
   }
   ["getOrNull"] {
     true
@@ -49,21 +61,29 @@ facts {
     true
     true
     true
+    true
+    true
   }
   ["firstOrNull"] {
     true
     true
     true
+    true
+    true
   }
   ["last"] {
     true
     true
     true
+    true
+    true
   }
   ["lastOrNull"] {
     true
     true
     true
+    true
+    true
   }
   ["single"] {
     true
@@ -91,6 +111,7 @@ facts {
     true
     true
     true
+    true
   }
 }
 examples {
@@ -99,6 +120,9 @@ examples {
     0
     3
     5
+    3
+    3
+    5
   }
   ["toList()"] {
     List()
@@ -134,6 +158,20 @@ examples {
     }, new {
       name = "Elf Owl"
     })
+    List(new {
+      name = "Wood Pigeon"
+    }, new {
+      name = "Barn Owl"
+    }, new {
+      name = "Parrot"
+    })
+    List(new {
+      name = "Wood Pigeon"
+    }, new {
+      name = "Barn Owl"
+    }, new {
+      name = "Parrot"
+    })
   }
   ["toSet()"] {
     Set()
@@ -167,6 +205,20 @@ examples {
     }, new {
       name = "Elf Owl"
     })
+    Set(new {
+      name = "Wood Pigeon"
+    }, new {
+      name = "Barn Owl"
+    }, new {
+      name = "Parrot"
+    })
+    Set(new {
+      name = "Wood Pigeon"
+    }, new {
+      name = "Barn Owl"
+    }, new {
+      name = "Parrot"
+    })
   }
   ["distinct"] {
     new {}
@@ -216,6 +268,28 @@ examples {
         name = "Elf Owl"
       }
     }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+      new {
+        name = "Barn Owl"
+      }
+      new {
+        name = "Parrot"
+      }
+    }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+      new {
+        name = "Barn Owl"
+      }
+      new {
+        name = "Parrot"
+      }
+    }
   }
   ["distinctBy()"] {
     new {}
@@ -265,6 +339,28 @@ examples {
         name = "Elf Owl"
       }
     }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+      new {
+        name = "Barn Owl"
+      }
+      new {
+        name = "Parrot"
+      }
+    }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+      new {
+        name = "Barn Owl"
+      }
+      new {
+        name = "Parrot"
+      }
+    }
     new {}
     new {}
     new {
@@ -312,6 +408,28 @@ examples {
         name = "Elf Owl"
       }
     }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+      new {
+        name = "Barn Owl"
+      }
+      new {
+        name = "Parrot"
+      }
+    }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+      new {
+        name = "Barn Owl"
+      }
+      new {
+        name = "Parrot"
+      }
+    }
     new {}
     new {}
     new {
@@ -329,6 +447,16 @@ examples {
         name = "Pigeon"
       }
     }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+    }
+    new {
+      new {
+        name = "Wood Pigeon"
+      }
+    }
   }
   ["fold"] {
     List()
@@ -350,6 +478,20 @@ examples {
     }, new {
       name = "Elf Owl"
     })
+    List(new {
+      name = "Wood Pigeon"
+    }, new {
+      name = "Barn Owl"
+    }, new {
+      name = "Parrot"
+    })
+    List(new {
+      name = "Wood Pigeon"
+    }, new {
+      name = "Barn Owl"
+    }, new {
+      name = "Parrot"
+    })
   }
   ["foldIndexed"] {
     List()
@@ -371,6 +513,20 @@ examples {
     }), Pair(4, new {
       name = "Elf Owl"
     }))
+    List(Pair(0, new {
+      name = "Wood Pigeon"
+    }), Pair(1, new {
+      name = "Barn Owl"
+    }), Pair(2, new {
+      name = "Parrot"
+    }))
+    List(Pair(0, new {
+      name = "Wood Pigeon"
+    }), Pair(1, new {
+      name = "Barn Owl"
+    }), Pair(2, new {
+      name = "Parrot"
+    }))
   }
   ["join"] {
     ""
@@ -386,5 +542,17 @@ examples {
     Albatross
     Elf Owl
     """
+    "Wood PigeonBarn OwlParrot"
+    """
+    Wood Pigeon
+    Barn Owl
+    Parrot
+    """
+    "Wood PigeonBarn OwlParrot"
+    """
+    Wood Pigeon
+    Barn Owl
+    Parrot
+    """
   }
 }