From 1dcdacbe3281374e3cabf602c92a3383f98295a2 Mon Sep 17 00:00:00 2001 From: Shuvam Pal <64604251+unichronic@users.noreply.github.com> Date: Mon, 22 Jun 2026 19:51:48 +0530 Subject: [PATCH] fmt: preserve the multiline but single entry iterables (#8663) Fixes #8557. `opa fmt` collapsed single-entry arrays/objects into one line even when the original source spanned multiple rows, making deeply nested structures hard to read. This maintains the structure. Signed-off-by: unichronic --- v1/format/format.go | 12 ++++++-- v1/format/testfiles/v0/test.rego.formatted | 18 +++++++++--- .../v0/test_issue_6330.rego.formatted | 24 ++++++++++++++-- .../testfiles/v0/test_with.rego.formatted | 4 ++- v1/format/testfiles/v1/test.rego.formatted | 18 +++++++++--- .../v1/test_issue_6330.rego.formatted | 28 ++++++++++++++++--- v1/format/testfiles/v1/test_issue_8557.rego | 19 +++++++++++++ .../v1/test_issue_8557.rego.formatted | 19 +++++++++++++ .../testfiles/v1/test_with.rego.formatted | 4 ++- 9 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 v1/format/testfiles/v1/test_issue_8557.rego create mode 100644 v1/format/testfiles/v1/test_issue_8557.rego.formatted diff --git a/v1/format/format.go b/v1/format/format.go index b281d78a4c..b2ea4a2ee6 100644 --- a/v1/format/format.go +++ b/v1/format/format.go @@ -1875,9 +1875,13 @@ func (w *writer) writeIterable(elements []any, last *ast.Location, close *ast.Lo return nil, err } + hasBoundaryComment := false // If there are comments within the single line, don't collapse it and keep it as-is // Return an error so that writeTerm will write the original formatting if len(lines) == 1 { + if w.beforeEnd != nil { + hasBoundaryComment = w.beforeEnd.Location.Row >= last.Row && w.beforeEnd.Location.Row <= close.Row + } for _, c := range comments { if c.Location.Row > last.Row && c.Location.Row < close.Row { return comments, unexpectedCommentError{ @@ -1885,10 +1889,14 @@ func (w *writer) writeIterable(elements []any, last *ast.Location, close *ast.Lo newCommentRow: c.Location.Row, } } + if c.Location.Row == last.Row || c.Location.Row == close.Row { + hasBoundaryComment = true + } } } - if len(lines) > 1 { + isMultiline := len(lines) > 1 || (len(lines) == 1 && len(elements) > 0 && close.Row > last.Row && !hasBoundaryComment) + if isMultiline { w.delayBeforeEnd() w.startMultilineSeq() } @@ -1910,7 +1918,7 @@ func (w *writer) writeIterable(elements []any, last *ast.Location, close *ast.Lo return nil, err } - if len(lines) > 1 { + if isMultiline { w.write(",") w.endLine() comments, err = w.insertComments(comments, close) diff --git a/v1/format/testfiles/v0/test.rego.formatted b/v1/format/testfiles/v0/test.rego.formatted index 9f316c5e85..831d33d4af 100644 --- a/v1/format/testfiles/v0/test.rego.formatted +++ b/v1/format/testfiles/v0/test.rego.formatted @@ -44,9 +44,15 @@ r = y { # Comment on else else = y { y = ["howdy"] - x = {"x": {"y": "z"}} + x = { + "x": { + "y": "z", + }, + } a = { - "a": {"b": "c"}, + "a": { + "b": "c", + }, "b": "c", "c": [ 1, 2, 3, 4, @@ -231,11 +237,15 @@ union_list := [(declare3 | declare4)] union_set_2 := {(declare3 | declare4)} -union_object_multi_line := {"response": (declare3 | declare4)} +union_object_multi_line := { + "response": (declare3 | declare4), +} union_object_key := {(declare3 | declare4): "foo"} -union_object_key_multi_line := {(declare3 | declare4): "foo"} +union_object_key_multi_line := { + (declare3 | declare4): "foo", +} # more comments! # more comments! diff --git a/v1/format/testfiles/v0/test_issue_6330.rego.formatted b/v1/format/testfiles/v0/test_issue_6330.rego.formatted index 0209af21c7..90800f7e22 100644 --- a/v1/format/testfiles/v0/test_issue_6330.rego.formatted +++ b/v1/format/testfiles/v0/test_issue_6330.rego.formatted @@ -1,20 +1,38 @@ package a -value := {"a": {"b": {"c": "d"}}} +value := { + "a": { + "b": { + "c": "d", + }, + }, +} value := {"a": # test 1 "b"} # test 2 value := {"a": "b"} # test 1 -value := {"a": {"b": {"c": "d"}}} +value := { + "a": { + "b": { + "c": "d", + }, + }, +} value := {"a": # this is {"b": # my ridiculous {"c": # way of "d"}}} # commenting code -value := {"a": {"b": {"c": "d"}}} +value := { + "a": { + "b": { + "c": "d", + }, + }, +} p[{"a": # "b"} # diff --git a/v1/format/testfiles/v0/test_with.rego.formatted b/v1/format/testfiles/v0/test_with.rego.formatted index 220a2d79be..461bc377b7 100644 --- a/v1/format/testfiles/v0/test_with.rego.formatted +++ b/v1/format/testfiles/v0/test_with.rego.formatted @@ -7,7 +7,9 @@ single_line_with { multi_line_with { fn(1) with input.a as "a" with input.b as "b" - with input.c as {"foo": "bar"} + with input.c as { + "foo": "bar", + } with input.d as [ 1, 2, diff --git a/v1/format/testfiles/v1/test.rego.formatted b/v1/format/testfiles/v1/test.rego.formatted index 53b2f71f55..94206869b0 100644 --- a/v1/format/testfiles/v1/test.rego.formatted +++ b/v1/format/testfiles/v1/test.rego.formatted @@ -41,9 +41,15 @@ r := y if { # Comment on else else := y if { y = ["howdy"] - x = {"x": {"y": "z"}} + x = { + "x": { + "y": "z", + }, + } a = { - "a": {"b": "c"}, + "a": { + "b": "c", + }, "b": "c", "c": [ 1, 2, 3, 4, @@ -222,11 +228,15 @@ union_list := [(declare3 | declare4)] union_set_2 := {(declare3 | declare4)} -union_object_multi_line := {"response": (declare3 | declare4)} +union_object_multi_line := { + "response": (declare3 | declare4), +} union_object_key := {(declare3 | declare4): "foo"} -union_object_key_multi_line := {(declare3 | declare4): "foo"} +union_object_key_multi_line := { + (declare3 | declare4): "foo", +} # more comments! # more comments! diff --git a/v1/format/testfiles/v1/test_issue_6330.rego.formatted b/v1/format/testfiles/v1/test_issue_6330.rego.formatted index 47af265862..33fe6a71f3 100644 --- a/v1/format/testfiles/v1/test_issue_6330.rego.formatted +++ b/v1/format/testfiles/v1/test_issue_6330.rego.formatted @@ -4,14 +4,26 @@ p[{"a": # "b"} # ] := true -value := {"a": {"b": {"c": "d"}}} +value := { + "a": { + "b": { + "c": "d", + }, + }, +} value := {"a": # test 1 "b"} # test 2 value := {"a": "b"} # test 1 -value := {"a": {"b": {"c": "d"}}} +value := { + "a": { + "b": { + "c": "d", + }, + }, +} value := {"a": # this is {"b": # my ridiculous @@ -23,7 +35,13 @@ p := {"a": # str := "my \n string" } -value := {"a": {"b": {"c": "d"}}} +value := { + "a": { + "b": { + "c": "d", + }, + }, +} f(_) := value if { value := {"a": # this is @@ -63,7 +81,9 @@ p := {"a": str} if { # str := "my \n string" } -p := {"a": str} if { +p := { + "a": str, +} if { # str := "my \n string" } diff --git a/v1/format/testfiles/v1/test_issue_8557.rego b/v1/format/testfiles/v1/test_issue_8557.rego new file mode 100644 index 0000000000..33f3d804c5 --- /dev/null +++ b/v1/format/testfiles/v1/test_issue_8557.rego @@ -0,0 +1,19 @@ +package test + +test_with_and_indentation if { + my_mock_object := { + "the_first_key": { + "a_nested_key": { + "and_sub_nested_key": [ + { + "yet_another_key": [ + "and_sub_nested_key_value", "foooo", "baaar", + ], + }, + ], + } + } + } + + _ = my_mock_object +} diff --git a/v1/format/testfiles/v1/test_issue_8557.rego.formatted b/v1/format/testfiles/v1/test_issue_8557.rego.formatted new file mode 100644 index 0000000000..700ba72ae9 --- /dev/null +++ b/v1/format/testfiles/v1/test_issue_8557.rego.formatted @@ -0,0 +1,19 @@ +package test + +test_with_and_indentation if { + my_mock_object := { + "the_first_key": { + "a_nested_key": { + "and_sub_nested_key": [ + { + "yet_another_key": [ + "and_sub_nested_key_value", "foooo", "baaar", + ], + }, + ], + }, + }, + } + + _ = my_mock_object +} diff --git a/v1/format/testfiles/v1/test_with.rego.formatted b/v1/format/testfiles/v1/test_with.rego.formatted index 0b1f2796fc..ce3bacecc2 100644 --- a/v1/format/testfiles/v1/test_with.rego.formatted +++ b/v1/format/testfiles/v1/test_with.rego.formatted @@ -7,7 +7,9 @@ single_line_with if { multi_line_with if { fn(1) with input.a as "a" with input.b as "b" - with input.c as {"foo": "bar"} + with input.c as { + "foo": "bar", + } with input.d as [ 1, 2,