fix: bind self/$ to outer self in super[name] lookup (#829)

sjsonnet/src/sjsonnet/Evaluator.scala

@@ -1213,11 +1213,20 @@ class Evaluator(
     }
   }
 
+  // DO NOT CHANGE: `self` MUST be passed explicitly to `sup.value(...)`.
+  // WHY: `Val.Obj.value`'s `self` parameter defaults to `this` (= the parent
+  // `sup`). Omitting `self` here would bind `$`/`self` of the retrieved field
+  // to the parent in the merge chain, hiding fields contributed by mixins
+  // applied *after* the parent — see issue #829 (grafana/mimir's
+  // `overrideSuperIfExists` pattern: `super[name] + override`). This must
+  // mirror `visitSelectSuper` (`super.name`) so static-key and computed-key
+  // super lookups agree, matching google/jsonnet, go-jsonnet, and jrsonnet.
   def visitLookupSuper(e: LookupSuper)(implicit scope: ValScope): Val = {
     var sup = scope.bindings(e.selfIdx + 1).asInstanceOf[Val.Obj]
     val key = visitExpr(e.index).cast[Val.Str]
-    if (sup == null) sup = scope.bindings(e.selfIdx).asInstanceOf[Val.Obj]
-    sup.value(key.str, e.pos)
+    val self = scope.bindings(e.selfIdx).asInstanceOf[Val.Obj]
+    if (sup == null) sup = self
+    sup.value(key.str, e.pos, self)
   }
 
   def visitImportStr(e: ImportStr): Val.Str = {

sjsonnet/test/resources/new_test_suite/super_lookup_dollar_binding.jsonnet

@@ -0,0 +1,99 @@
+// Regression test for https://github.com/databricks/sjsonnet/issues/829
+//
+// `super[name]` (computed-key super lookup) must bind `$`/`self` of the
+// retrieved field to the *outer* self (the merge result), NOT the parent.
+// Otherwise fields defined in mixins applied AFTER the parent become
+// unreachable from inside fields retrieved through `super[name]`.
+//
+// This mirrors the semantics of `super.name` (static-key super lookup) and
+// matches behavior of google/jsonnet (C++), go-jsonnet, and jrsonnet.
+//
+// The bug surfaced via grafana/mimir's `overrideSuperIfExists(name, override)`
+// helper which uses `super[name] + override` to layer mixins.
+
+// ============================================================
+// SECTION 1: Static vs dynamic super key parity for $ binding
+// ============================================================
+
+// 1.1 Computed-key super lookup must see fields from later mixins via $
+local a1 = {
+  _config+:: { a_enabled: false },
+  field: { flag: $._config.b_enabled },
+};
+local b1 = {
+  _config+:: { b_enabled: false },
+  field: super['field'] + { extra: 1 },
+};
+local r1 = (a1 + b1).field;
+assert r1 == { flag: false, extra: 1 } : 'super[name] must preserve $: ' + std.toString(r1);
+
+// 1.2 Static-key super lookup baseline (was already correct in sjsonnet)
+local a2 = {
+  _config+:: { a_enabled: false },
+  field: { flag: $._config.b_enabled },
+};
+local b2 = {
+  _config+:: { b_enabled: false },
+  field: super.field + { extra: 1 },
+};
+local r2 = (a2 + b2).field;
+assert r2 == r1 : 'super.name and super[name] must agree: ' + std.toString(r2);
+
+// ============================================================
+// SECTION 2: The grafana/mimir overrideSuperIfExists pattern
+// ============================================================
+
+local mixinA = {
+  _config+:: { a_only: 'a' },
+  // Field references _config field that only exists after merging mixinB.
+  alertmanager_statefulset: {
+    label: $._config.b_only,
+  },
+};
+local mixinB = {
+  _config+:: { b_only: 'b' },
+  local overrideSuperIfExists(name, override) =
+    if !(name in super) || super[name] == null || super[name] == {} then null
+    else super[name] + override,
+  alertmanager_statefulset: overrideSuperIfExists(
+    'alertmanager_statefulset',
+    { extra: 'mixed-in' },
+  ),
+};
+local merged = mixinA + mixinB;
+assert merged.alertmanager_statefulset == { label: 'b', extra: 'mixed-in' }
+       : 'overrideSuperIfExists must see merged $._config: '
+         + std.toString(merged.alertmanager_statefulset);
+
+// ============================================================
+// SECTION 3: Edge cases
+// ============================================================
+
+// 3.1 super[name] with no override (just rebinding) preserves $
+local n1 = { _config+:: { x: 1 }, value: $._config.x };
+local n2 = { value: super['value'] };
+local r5 = (n1 + n2).value;
+assert r5 == 1 : 'bare super[name] must preserve binding';
+
+// 3.2 Chained mixins: 3 levels deep, $ must reach the outermost
+local l1 = { _config+:: { l1: 1 }, item: { sum: $._config.l1 + $._config.l2 + $._config.l3 } };
+local l2 = { _config+:: { l2: 2 }, item: super['item'] + { mid: true } };
+local l3 = { _config+:: { l3: 3 }, item: super['item'] + { top: true } };
+local r6 = (l1 + l2 + l3).item;
+assert r6 == { sum: 6, mid: true, top: true }
+       : 'chained super[name] across 3 mixins: ' + std.toString(r6);
+
+// 3.3 Computed key from a variable (not a literal string)
+local k = 'item';
+local v1 = { _config+:: { x: 'hello' }, item: { val: $._config.x } };
+local v2 = { item: super[k] + { wrapped: true } };
+local r7 = (v1 + v2).item;
+assert r7 == { val: 'hello', wrapped: true }
+       : 'super[var-key] must preserve binding: ' + std.toString(r7);
+
+// 3.4 Object-extension syntax {a + b} also exercises super lookup
+local oa = { _config+:: { y: 'yes' }, label: $._config.y };
+local ob = oa { label: super['label'] };
+assert ob.label == 'yes' : 'super[name] in object-extension must preserve binding';
+
+true

sjsonnet/test/resources/new_test_suite/super_lookup_dollar_binding.jsonnet.golden

@@ -0,0 +1,2 @@
+true
+

sjsonnet/test/resources/new_test_suite/super_lookup_mimir_autoscaling.libsonnet

@@ -0,0 +1,9 @@
+// Mimics grafana/mimir's ingest-storage-ingester-autoscaling.libsonnet —
+// adds the `_config` field that the rollout mixin's fields reference via $.
+// The whole point of the bug: this mixin is applied LAST, so super[name]
+// inside memberlist must still see this field through $._config.
+{
+  _config+:: {
+    ingest_storage_ingester_autoscaling_enabled: false,
+  },
+}

sjsonnet/test/resources/new_test_suite/super_lookup_mimir_memberlist.libsonnet

@@ -0,0 +1,12 @@
+// Mimics grafana/mimir's memberlist.libsonnet — uses the
+// `super[name] + override` pattern through the local helper. This is the
+// site that triggered issue #829.
+{
+  // Verbatim pattern from operations/mimir/memberlist.libsonnet:171.
+  local overrideSuperIfExists(name, override) =
+    if !(name in super) || super[name] == null || super[name] == {} then null
+    else super[name] + override,
+
+  alertmanager_statefulset:
+    overrideSuperIfExists('alertmanager_statefulset', { gossip: true }),
+}

sjsonnet/test/resources/new_test_suite/super_lookup_mimir_pattern.jsonnet

@@ -0,0 +1,29 @@
+// Regression test for https://github.com/databricks/sjsonnet/issues/829
+//
+// End-to-end multi-file reproduction of the grafana/mimir failure:
+//
+//   sjsonnet.Error: Field does not exist: ingest_storage_ingester_autoscaling_enabled
+//       at [overrideSuperIfExists].(mimir/rollout-operator.libsonnet:18:64)
+//       at [<root>].(mimir/memberlist.libsonnet:106:50)
+//
+// Faithfully mirrors the structure: three libsonnet mixins merged with `+`,
+// where the last mixin's `_config` adds the field that an earlier mixin's
+// field references through `super[name] + override`.
+
+local rollout = import 'super_lookup_mimir_rollout.libsonnet';
+local memberlist = import 'super_lookup_mimir_memberlist.libsonnet';
+local autoscaling = import 'super_lookup_mimir_autoscaling.libsonnet';
+
+local merged = rollout + memberlist + autoscaling;
+
+// Without the fix this throws:
+//   Field does not exist: ingest_storage_ingester_autoscaling_enabled
+local rt = merged.rollout_operator_replica_template_access_enabled;
+assert rt == false : 'rollout enabled flag must evaluate via $._config across mixins';
+
+// memberlist's overrideSuperIfExists must surface the rebuilt statefulset
+local sts = merged.alertmanager_statefulset;
+assert sts.label == 'autoscaling-disabled' : 'super[name] must rebind $: ' + std.toString(sts);
+assert sts.gossip == true : 'override must merge on top of super[name]: ' + std.toString(sts);
+
+true

sjsonnet/test/resources/new_test_suite/super_lookup_mimir_pattern.jsonnet.golden

@@ -0,0 +1,2 @@
+true
+

sjsonnet/test/resources/new_test_suite/super_lookup_mimir_rollout.libsonnet

@@ -0,0 +1,22 @@
+// Mimics grafana/mimir's rollout-operator.libsonnet — defines a field that
+// references _config keys defined in a later mixin.
+{
+  _config+:: {
+    multi_zone_ingester_enabled: false,
+    multi_zone_store_gateway_enabled: false,
+    cortex_compactor_concurrent_rollout_enabled: false,
+  },
+
+  // Line equivalent to rollout-operator.libsonnet:18 — references
+  // ingest_storage_ingester_autoscaling_enabled which is added by the
+  // autoscaling mixin appended LATER in the merge chain.
+  rollout_operator_replica_template_access_enabled:
+    $._config.ingest_storage_ingester_autoscaling_enabled,
+
+  alertmanager_statefulset: {
+    label:
+      if $._config.ingest_storage_ingester_autoscaling_enabled
+      then 'autoscaling-enabled'
+      else 'autoscaling-disabled',
+  },
+}