fix(expr): render lists per openjd-specifications #176

Makefile

@@ -218,7 +218,7 @@ test-conformance: ## Run the official OpenJD conformance suite (needs the pinned
 # unpinned upgrade could turn the differential test red without a single sqi
 # commit — and because a divergence report is meaningless without knowing
 # which build of the reference produced it.
-OPENJD_MODEL_VERSION ?= 0.11.4
+OPENJD_MODEL_VERSION ?= 0.11.5
 ORACLE_VENV := .venv-oracle
 
 .PHONY: expr-oracle-venv

internal/openjd/expr/doc.go

@@ -975,26 +975,29 @@
 //     Value.String() rendered a list's string elements unquoted while
 //     string()'s JSON row quoted them (see the float-passthrough bullet
 //     above); that quoting gap closed in this wave, and the two were then
-//     measured to also agree byte-for-byte on ESCAPING for every case
-//     tried: an embedded double quote, non-ASCII, angle brackets and an
-//     ampersand, a newline, a backslash, tab, backspace, formfeed, carriage
-//     return, an emoji, U+2028/U+2029, and the empty string
-//     (TestValueString_VersusStringFunction). That is the full extent of
-//     the claim, and it must not be read as universal agreement: the two
-//     are INDEPENDENT implementations — Value.String() quotes a list's
-//     string elements with strconv.Quote (value.go), string()'s list row
-//     encodes with encoding/json and SetEscapeHTML(false) (funcsconv.go's
-//     writeJSONValue) — and a direct probe of the two encoders against each
-//     other, outside the set above, finds real divergences: on the C0
-//     controls U+0000, U+0001 and U+001B, strconv.Quote emits its own
-//     hex-escape form where the JSON encoder emits a \u00XX escape; on
-//     vertical tab (U+000B), Quote emits its own short escape where JSON
-//     emits \u000b; on DEL (U+007F), Quote escapes it where JSON leaves it
-//     literal; and on invalid UTF-8, Quote preserves the raw bytes where
-//     JSON substitutes U+FFFD — all four measured directly against both
-//     functions and pinned as regression tests, not just measured once
-//     (TestValueString_DivergesFromStringFunctionOutsideMeasuredSet). Do not
-//     widen the agreement claim past the set it was measured on.
+//     measured to also agree byte-for-byte on ESCAPING for thirteen cases:
+//     an embedded double quote, non-ASCII, angle brackets and an ampersand,
+//     a newline, a backslash, tab, backspace, formfeed, carriage return, an
+//     emoji, U+2028/U+2029, and the empty string
+//     (TestValueString_VersusStringFunction). That claim was deliberately
+//     NARROW, because the two were independent implementations —
+//     Value.String() used strconv.Quote while string()'s list row used
+//     encoding/json with SetEscapeHTML(false) — and a direct probe outside
+//     the measured set found real divergences on the C0 controls, vertical
+//     tab, DEL and invalid UTF-8, pinned in the opposite direction.
+//
+//     THAT IS NO LONGER THE SHAPE OF IT. openjd-specifications#176 added
+//     the rule that format-string interpolation "uses this same conversion"
+//     as string() and that the result "must parse as JSON". Go's spelling
+//     of a control character (\x01, and \a/\v for two JSON does not name)
+//     is not JSON at all, so every one of those divergences was a defect
+//     rather than a permitted difference. Both renderers now call
+//     funcsconv.go's jsonQuoteElement, so the agreement holds BY
+//     CONSTRUCTION for every input rather than by measurement over a set,
+//     and TestValueString_ListQuotingIsJSONEverywhere pins the six cases
+//     that used to diverge. The separator still differs from
+//     internal/openjd's canonical STORAGE form ("," there, ", " here), and
+//     paramjson.go states that difference where it lives.
 //
 //     unique() CHARGES PER COMPARISON, NOT PER ELEMENT — and the reason is
 //     a prediction that measurement overturned, not a preference. C1

internal/openjd/expr/funcsconv.go

@@ -175,27 +175,29 @@ var convFuncs = map[string][]Shape{
 				return String(s), nil
 			},
 		},
-		// RFC 0006 calls this "the JSON string representation", and it is a
-		// separate implementation from Value.String() -- two renderings, two
-		// functions, on purpose: this row encodes with encoding/json and
-		// SetEscapeHTML(false) (writeJSONValue), while Value.String() quotes a
-		// list's string elements with strconv.Quote (value.go).
+		// RFC 0006 calls this "the JSON string representation". This row and
+		// Value.String() are two renderings for two purposes, but they now
+		// share ONE quoting rule: both send a list's string-like elements
+		// through jsonQuoteElement (encoding/json with SetEscapeHTML(false)).
 		//
-		// CORRECTION (final whole-branch review, sub-project E1): an earlier
-		// revision said Value.String() "renders a list's string elements
-		// unquoted ('[a, b]') as a diagnostic form, which is a known
-		// divergence deferred to sub-project E". That was true when written
-		// and is FALSE now -- sub-project E1 CLOSED the divergence rather than
-		// deferring it, and Value.String() has quoted list string elements
-		// since. What is true today: the two renderings agree byte-for-byte on
-		// quoting AND on escaping for every case measured
-		// (TestValueString_VersusStringFunction's thirteen), but they remain
-		// independent implementations and do NOT agree universally -- they
-		// diverge on the C0 controls, vertical tab, DEL and invalid UTF-8,
-		// pinned in the opposite direction by
-		// TestValueString_DivergesFromStringFunctionOutsideMeasuredSet. Do not
-		// restate the old claim, and do not widen the new one past the set it
-		// was measured on; doc.go carries the full statement.
+		// HISTORY, because two earlier revisions of this comment were each
+		// true when written and false later. (1) The oldest said
+		// Value.String() "renders a list's string elements unquoted" as a
+		// diagnostic form -- sub-project E1 closed that. (2) Its replacement
+		// said the two were INDEPENDENT implementations that agreed on the
+		// thirteen cases measured (TestValueString_VersusStringFunction) and
+		// provably diverged outside them -- on the C0 controls, vertical tab,
+		// DEL and invalid UTF-8 -- and warned against widening the claim.
+		// That was correct until openjd-specifications#176, which states that
+		// format-string interpolation "uses this same conversion" and that
+		// the result "must parse as JSON": Go's \x00 and \v forms are not
+		// JSON, so the divergence became a defect and the two renderers were
+		// merged onto one quoter. The agreement is now universal BY
+		// CONSTRUCTION rather than by measurement, and
+		// TestValueString_ListQuotingIsJSONEverywhere pins the six cases that
+		// used to diverge. What still differs is the SEPARATOR in
+		// internal/openjd's canonical storage form ("," there, ", " here) --
+		// see paramjson.go.
 		//
 		// Cost{ArgElements: {0}} — a DELIBERATE divergence from the reference,
 		// which measures a flat 1 for string([1,2,3]) AND for a 10-element
@@ -348,6 +350,37 @@ func jsonList(v Value) (string, error) {
 	return b.String(), nil
 }
 
+// jsonQuoteElement renders one string-like list element as a JSON string.
+//
+// The HTML-escaping note above this function's only other caller applies
+// here: json.Marshal escapes "<", ">" and "&" by default, because its output
+// is meant to be safe inside an HTML document, which is not this context.
+// Neither the reference implementation nor Python's json.dumps does that, and
+// RFC 0006 asks for "the JSON string representation" without qualification.
+// An Encoder with SetEscapeHTML(false) is the supported way to turn it off;
+// it appends a newline, which is trimmed.
+//
+// Value.String() renders a list's elements through this same function, and
+// must keep doing so: openjd-specifications#176 states that format-string
+// interpolation with surrounding text "uses this same conversion", so
+// "items: {{ MyList }}" and "items: " + string(MyList) are required to agree.
+// They are two renderers sharing one quoting rule, not two rules -- see
+// TestValueString_ListQuotingIsJSONEverywhere, which pins the six classes of
+// input (C0 controls, vertical tab, DEL, invalid UTF-8) where a second rule
+// would show through.
+func jsonQuoteElement(s string) string {
+	var buf bytes.Buffer
+	enc := json.NewEncoder(&buf)
+	enc.SetEscapeHTML(false)
+	// Encode reports unsupported types, encoding cycles and writer failures,
+	// none of which a string written into a bytes.Buffer can produce.
+	// Invalid UTF-8 is not an error either -- the encoder substitutes U+FFFD
+	// -- so there is no failure left for a caller to handle, and an
+	// infallible signature is what lets Value.String() share this function.
+	_ = enc.Encode(s) //nolint:errcheck // cannot fail for a string into a buffer; see above
+	return string(bytes.TrimRight(buf.Bytes(), "\n"))
+}
+
 // writeJSONValue is jsonList's recursive worker.
 func writeJSONValue(b *strings.Builder, v Value) error {
 	switch v.Type.Code {
@@ -371,13 +404,7 @@ func writeJSONValue(b *strings.Builder, v Value) error {
 		// rendering into a surprising one. An Encoder with SetEscapeHTML(false)
 		// is the supported way to turn it off; it appends a newline, which is
 		// trimmed.
-		var buf bytes.Buffer
-		enc := json.NewEncoder(&buf)
-		enc.SetEscapeHTML(false)
-		if err := enc.Encode(v.s); err != nil {
-			return err
-		}
-		b.Write(bytes.TrimRight(buf.Bytes(), "\n"))
+		b.WriteString(jsonQuoteElement(v.s))
 	default:
 		// bool, int, float and null all render bare, and Value.String()
 		// already spells each of them the way JSON does.

internal/openjd/expr/funcsreprshell.go

@@ -83,11 +83,12 @@ var reprShellFuncs = map[string][]Shape{
 		}},
 		// DIVERGENCE from the reference: see this var block's own COST comment.
 		{Params: []Type{ListOf(varT)}, Ret: TString, Cost: Cost{ArgElements: []int{0}}, Fn: func(args []Value) (Value, error) {
-			body, err := joinValues(args[0].AsList(), ", ", pwshElement)
+			elems := args[0].AsList()
+			body, err := joinValues(elems, ", ", pwshElement)
 			if err != nil {
 				return Value{}, err
 			}
-			return boundedString("@(" + body.AsStr() + ")")
+			return boundedString("@(" + pwshUnaryComma(elems) + body.AsStr() + ")")
 		}},
 	},
 }
@@ -152,6 +153,9 @@ func pwshQuote(s string) string {
 // runnable PowerShell for anything that expects array elements. Recursing
 // through pwshElement instead builds a nested "@(...)" literal, matching how
 // the top-level ListOf(varT) row itself is built.
+//
+// A nested list also picks up its own unary comma when it needs one, since
+// the decision is per-list; see pwshUnaryComma.
 func pwshElement(v Value) string {
 	switch v.Type.Code {
 	case CodeBool:
@@ -164,12 +168,33 @@ func pwshElement(v Value) string {
 	case CodeNull:
 		return "$null"
 	case CodeList:
-		parts := make([]string, len(v.AsList()))
-		for i, elem := range v.AsList() {
+		elems := v.AsList()
+		parts := make([]string, len(elems))
+		for i, elem := range elems {
 			parts[i] = pwshElement(elem)
 		}
-		return "@(" + strings.Join(parts, ", ") + ")"
+		return "@(" + pwshUnaryComma(elems) + strings.Join(parts, ", ") + ")"
 	default:
 		return pwshQuote(v.String())
 	}
 }
+
+// pwshUnaryComma returns the "," that section 2.2.6 requires in front of a
+// one-element array whose only element is itself an array, and "" for every
+// other list.
+//
+// PowerShell flattens "@(@(1, 2))" to "@(1, 2)", so the nesting is lost on
+// the round trip; the unary comma operator, "@(,@(1, 2))", preserves it. The
+// test is on the element COUNT, not on depth: "@(@(1, 2), @(3))" needs no
+// comma because two elements already force an array, and a one-element list
+// of SCALARS must not get one -- "@('a')" is unambiguous already.
+//
+// This is a spec rule sqi missed until openjd-specifications#176 stated it
+// outright; the reference implementation refuses lists nested more than two
+// deep, so it cannot answer the recursive case at all.
+func pwshUnaryComma(elems []Value) string {
+	if len(elems) == 1 && elems[0].Type.Code == CodeList {
+		return ","
+	}
+	return ""
+}

internal/openjd/expr/funcsreprshell_internal_test.go

@@ -100,6 +100,24 @@ func TestReprPwsh(t *testing.T) {
 		// list of text, not a nested array. A nested list must become a
 		// nested "@(...)" array literal instead.
 		{"nested list becomes a nested array literal", `repr_pwsh([['a'], ['b']])`, "@(@('a'), @('b'))"},
+		// Section 2.2.6, as restated by openjd-specifications#176: a
+		// ONE-element list whose element is itself a list takes the unary
+		// comma form, because "@(@(1, 2))" flattens to "@(1, 2)" under
+		// PowerShell's array-flattening rules while "@(,@(1, 2))" preserves
+		// the nesting. The rule is about the number of ELEMENTS, not the
+		// depth: a two-element list of lists needs no comma, and a
+		// one-element list of SCALARS must not get one (@('a') is already
+		// unambiguous, and @(,'a') would be a different, wronger thing to
+		// write).
+		{"single nested list takes the unary comma form", `repr_pwsh([[1, 2]])`, "@(,@(1, 2))"},
+		{"single nested EMPTY list takes the unary comma form", `repr_pwsh([[]])`, "@(,@())"},
+		{"two nested lists take no comma", `repr_pwsh([[1, 2], [3]])`, "@(@(1, 2), @(3))"},
+		{"single scalar element takes no comma", `repr_pwsh(['a'])`, "@('a')"},
+		// The rule recurses: each list decides for itself. The reference
+		// implementation refuses three levels of nesting outright ("Lists
+		// may be nested at most 2 levels deep"), so this row's ground truth
+		// is section 2.2.6's own wording, not the oracle.
+		{"unary comma applies at depth too", `repr_pwsh([[['a']], [['b']]])`, "@(@(,@('a')), @(,@('b')))"},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {

internal/openjd/expr/value.go

@@ -274,7 +274,14 @@ func (v Value) String() string {
 			// the regex family and the path .parts/.suffixes properties.
 			switch elem.Type.Code {
 			case CodeString, CodePath, CodeRangeExpr:
-				parts[i] = strconv.Quote(elem.s)
+				// jsonQuoteElement, NOT strconv.Quote: Go's quoting spells a
+				// control character "\x01" and has "\a"/"\v" forms JSON does
+				// not define, so it produced output no JSON parser accepts --
+				// which openjd-specifications#176 forbids outright, and which
+				// also made this rendering disagree with string()'s for the
+				// same list. Sharing funcsconv.go's quoter is what keeps
+				// section 2.2.1's "same conversion" true by construction.
+				parts[i] = jsonQuoteElement(elem.s)
 			default:
 				parts[i] = elem.String()
 			}