Skip to content

Commit ab21870

Browse files
authored
format: Keep rule body inline when the head spans multiple lines (#8904)
### Why the changes in this PR are needed? Fixes #8894. `opa fmt` expands a one-line `if` condition into a block whenever the rule head's *value* expression spans multiple lines, even though the condition itself is a single simple term. For example: ```rego foo := sprintf( "%d", [1], ) if allow ``` was reformatted to: ```rego foo := sprintf( "%d", [1], ) if { allow } ``` ### What are the changes in this PR? The inline-`if` path in `writeRule` decides whether to keep `if <term>` on one line by comparing the body term's row to the rule head's row: ```go if rule.Body[0].Location.Row == rule.Head.Location.Row { ``` `rule.Head.Location.Row` is the head's **start** row. Once the head value wraps onto later lines, the single body term sits on a later row than the head start, the equality fails, and formatting falls through to the block form. The fix compares against the head's **end** row instead (start row plus the number of newlines in the head's location text), so a single body term on the same line as `if` stays inline regardless of how many lines the head value occupies. Single-line heads are unaffected (end row == start row), and genuinely multi-statement bodies still expand as before (they don't hit the `len(rule.Body) == 1` branch). ### Notes to assist PR review: Added `v1/format/testfiles/v1/test_issue_8894.rego` (+`.formatted`) with the exact repro from the issue; it fails on `master` (expands to a block) and passes with this change. The rest of the format golden suite is unchanged. Signed-off-by: Kunalbehbud <b.kunal2002@gmail.com>
1 parent 95090fa commit ab21870

3 files changed

Lines changed: 18 additions & 1 deletion

File tree

v1/format/format.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,12 @@ func (w *writer) writeRule(rule *ast.Rule, isElse bool, comments []*ast.Comment)
640640
if (w.fmtOpts.regoV1 || w.fmtOpts.ifs) && partialSetException {
641641
w.write(" if")
642642
if len(rule.Body) == 1 {
643-
if rule.Body[0].Location.Row == rule.Head.Location.Row {
643+
// Keep `if <term>` on one line when the single body term sits on the
644+
// same line as the end of the head. Comparing against the head's
645+
// start row would wrongly expand the condition into a block whenever
646+
// the head value spans multiple lines (e.g. a multi-line call).
647+
headEndRow := rule.Head.Location.Row + strings.Count(string(rule.Head.Location.Text), "\n")
648+
if rule.Body[0].Location.Row == headEndRow {
644649
w.write(" ")
645650
var err error
646651
comments, err = w.writeExpr(rule.Body[0], comments)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package test
2+
3+
foo := sprintf(
4+
"%d",
5+
[1],
6+
) if allow
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package test
2+
3+
foo := sprintf(
4+
"%d",
5+
[1],
6+
) if allow

0 commit comments

Comments
 (0)