Skip to content

Commit 8e1adbd

Browse files
committed
format: Add error message for comment merge conflict.
This commit adds a basic check for comment merge conflicts, and provides a helpful error message if a conflict happens during formatting. Fixes open-policy-agent#6330 Signed-off-by: Philip Conrad <[email protected]>
1 parent 89e02ef commit 8e1adbd

File tree

3 files changed

+31
-4
lines changed

3 files changed

+31
-4
lines changed

Diff for: format/format.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,9 @@ func (w *writer) insertComments(comments []*ast.Comment, loc *ast.Location) []*a
523523
w.blankLine()
524524
}
525525

526-
w.beforeLineEnd(at)
526+
if err := w.beforeLineEnd(at); err != nil {
527+
w.errs = append(w.errs, err.(*ast.Error))
528+
}
527529
return comments
528530
}
529531

@@ -1332,14 +1334,15 @@ func (w *writer) endLine() {
13321334
}
13331335

13341336
// beforeLineEnd registers a comment to be printed at the end of the current line.
1335-
func (w *writer) beforeLineEnd(c *ast.Comment) {
1337+
func (w *writer) beforeLineEnd(c *ast.Comment) error {
13361338
if w.beforeEnd != nil {
13371339
if c == nil {
1338-
return
1340+
return nil
13391341
}
1340-
panic("overwriting non-nil beforeEnd")
1342+
return ast.NewError(ast.ParseErr, c.Location, "could not move comment from line %d, because an existing comment was already present at line %d", w.beforeEnd.Location.Row, c.Location.Row)
13411343
}
13421344
w.beforeEnd = c
1345+
return nil
13431346
}
13441347

13451348
func (w *writer) delayBeforeEnd() {

Diff for: format/format_test.go

+20
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,26 @@ func TestFormatSourceError(t *testing.T) {
7777
}
7878
}
7979

80+
func TestFormatConflictingCommentsError(t *testing.T) {
81+
rego := "testfiles/test_issue_6330.rego.error"
82+
contents, err := os.ReadFile(rego)
83+
if err != nil {
84+
t.Fatalf("Failed to read rego source: %v", err)
85+
}
86+
87+
_, err = Source(rego, contents)
88+
if err == nil {
89+
t.Fatal("Expected parsing error, not nil")
90+
}
91+
92+
// Has the filename prefix because it's an issue during formatting, not parsing.
93+
exp := "testfiles/test_issue_6330.rego.error: 1 error occurred: testfiles/test_issue_6330.rego.error:4: rego_parse_error: could not move comment from line 3, because an existing comment was already present at line 4"
94+
95+
if !strings.HasPrefix(err.Error(), exp) {
96+
t.Fatalf("Expected error message '%s', got '%s'", exp, err.Error())
97+
}
98+
}
99+
80100
func TestFormatSource(t *testing.T) {
81101
regoFiles, err := filepath.Glob("testfiles/*.rego")
82102
if err != nil {

Diff for: format/testfiles/test_issue_6330.rego.error

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
package a
2+
3+
value := {"a": #
4+
"b"} #

0 commit comments

Comments
 (0)