From 53578e129b97f901cd95dedd5bca9cc76faf0a60 Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Wed, 12 Aug 2026 14:40:39 +0100 Subject: [PATCH 1/3] ir: Add end locations in IR plan statements This will allow sub line coverage reporting in IR evaluators. Follows/related to https://github.com/open-policy-agent/opa/pull/8752 to allow feature parity with topdown reporting. Signed-off-by: Charlie Egan --- cmd/build_jsonv2_test.go | 2 +- cmd/build_test.go | 2 +- internal/cmd/genplanschema/main.go | 4 +- internal/planner/planner.go | 2 +- internal/planner/planner_test.go | 2 +- v1/ast/location/location.go | 28 +-- v1/ast/location/location_test.go | 49 ++++-- v1/bundle/v1pb/manifest.pb.go | 2 +- v1/format/format.go | 8 +- v1/ir/ir.go | 23 ++- v1/ir/marshal.go | 8 +- v1/ir/marshal_test.go | 68 +++++++- v1/ir/plan.proto | 19 +- v1/ir/plan.schema.json | 272 +++++++++++++++++++++++++++++ v1/ir/proto.go | 8 +- v1/ir/proto_test.go | 2 +- v1/ir/v1pb/plan.pb.go | 47 +++-- 17 files changed, 479 insertions(+), 67 deletions(-) diff --git a/cmd/build_jsonv2_test.go b/cmd/build_jsonv2_test.go index 2b9f3f0f07f..d4d9424bc3e 100644 --- a/cmd/build_jsonv2_test.go +++ b/cmd/build_jsonv2_test.go @@ -3475,7 +3475,7 @@ func TestBuildPlanJSONOutputBytes(t *testing.T) { got := strings.ReplaceAll(string(planBytes), root, "TEMPDIR") - expected := `{"static":{"strings":[{"value":"result"},{"value":"p"},{"value":"1"},{"value":"test"}],"files":[{"value":"TEMPDIR/test.rego"}]},"plans":{"plans":[{"name":"test","blocks":[{"stmts":[{"type":"MakeObjectStmt","stmt":{"target":2,"file":0,"col":0,"row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"CallStmt","stmt":{"func":"g0.data.test.p","args":[{"type":"local","value":0},{"type":"local","value":1}],"result":3,"file":0,"col":0,"row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":1},"value":{"type":"local","value":3},"object":2,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"DotStmt","stmt":{"source":{"type":"local","value":1},"key":{"type":"string_index","value":3},"target":5,"file":0,"col":0,"row":0}},{"type":"ObjectMergeStmt","stmt":{"a":5,"b":2,"target":4,"file":0,"col":0,"row":0}},{"type":"BreakStmt","stmt":{"index":1,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":2},"target":4,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":4},"target":6,"file":0,"col":0,"row":0}},{"type":"MakeObjectStmt","stmt":{"target":7,"file":0,"col":0,"row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":0},"value":{"type":"local","value":6},"object":7,"file":0,"col":0,"row":0}},{"type":"ResultSetAddStmt","stmt":{"value":7,"file":0,"col":0,"row":0}}]}]}]},"funcs":{"funcs":[{"name":"g0.data.test.p","params":[0,1],"return":2,"blocks":[{"stmts":[{"type":"ResetLocalStmt","stmt":{"target":3,"file":0,"col":4,"row":3}},{"type":"MakeNumberRefStmt","stmt":{"file":0,"col":4,"row":3,"index":2,"Index":2,"target":4}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":4},"target":3,"file":0,"col":4,"row":3}}]},{"stmts":[{"type":"IsDefinedStmt","stmt":{"source":3,"file":0,"col":4,"row":3}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":3},"target":2,"file":0,"col":4,"row":3}}]},{"stmts":[{"type":"ReturnLocalStmt","stmt":{"source":2,"file":0,"col":4,"row":3}}]}],"path":["g0","test","p"]}]}}` + expected := `{"static":{"strings":[{"value":"result"},{"value":"p"},{"value":"1"},{"value":"test"}],"files":[{"value":"TEMPDIR/test.rego"}]},"plans":{"plans":[{"name":"test","blocks":[{"stmts":[{"type":"MakeObjectStmt","stmt":{"target":2,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"CallStmt","stmt":{"func":"g0.data.test.p","args":[{"type":"local","value":0},{"type":"local","value":1}],"result":3,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":1},"value":{"type":"local","value":3},"object":2,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"DotStmt","stmt":{"source":{"type":"local","value":1},"key":{"type":"string_index","value":3},"target":5,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectMergeStmt","stmt":{"a":5,"b":2,"target":4,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BreakStmt","stmt":{"index":1,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":2},"target":4,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":4},"target":6,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"MakeObjectStmt","stmt":{"target":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":0},"value":{"type":"local","value":6},"object":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ResultSetAddStmt","stmt":{"value":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}]}]},"funcs":{"funcs":[{"name":"g0.data.test.p","params":[0,1],"return":2,"blocks":[{"stmts":[{"type":"ResetLocalStmt","stmt":{"target":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}},{"type":"MakeNumberRefStmt","stmt":{"file":0,"col":4,"row":3,"end_col":9,"end_row":3,"index":2,"Index":2,"target":4}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":4},"target":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]},{"stmts":[{"type":"IsDefinedStmt","stmt":{"source":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":3},"target":2,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]},{"stmts":[{"type":"ReturnLocalStmt","stmt":{"source":2,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]}],"path":["g0","test","p"]}]}}` if diff := cmp.Diff(expected, got); diff != "" { t.Errorf("unexpected result (-want, +got):\n%s", diff) diff --git a/cmd/build_test.go b/cmd/build_test.go index 5edcc2046ff..d1ec16c1928 100644 --- a/cmd/build_test.go +++ b/cmd/build_test.go @@ -3475,7 +3475,7 @@ func TestBuildPlanJSONOutputBytes(t *testing.T) { got := strings.ReplaceAll(string(planBytes), root, "TEMPDIR") - expected := `{"static":{"strings":[{"value":"result"},{"value":"p"},{"value":"1"},{"value":"test"}],"files":[{"value":"TEMPDIR/test.rego"}]},"plans":{"plans":[{"name":"test","blocks":[{"stmts":[{"type":"MakeObjectStmt","stmt":{"target":2,"file":0,"col":0,"row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"CallStmt","stmt":{"func":"g0.data.test.p","args":[{"type":"local","value":0},{"type":"local","value":1}],"result":3,"file":0,"col":0,"row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":1},"value":{"type":"local","value":3},"object":2,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"DotStmt","stmt":{"source":{"type":"local","value":1},"key":{"type":"string_index","value":3},"target":5,"file":0,"col":0,"row":0}},{"type":"ObjectMergeStmt","stmt":{"a":5,"b":2,"target":4,"file":0,"col":0,"row":0}},{"type":"BreakStmt","stmt":{"index":1,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":2},"target":4,"file":0,"col":0,"row":0}}]}],"file":0,"col":0,"row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":4},"target":6,"file":0,"col":0,"row":0}},{"type":"MakeObjectStmt","stmt":{"target":7,"file":0,"col":0,"row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":0},"value":{"type":"local","value":6},"object":7,"file":0,"col":0,"row":0}},{"type":"ResultSetAddStmt","stmt":{"value":7,"file":0,"col":0,"row":0}}]}]}]},"funcs":{"funcs":[{"name":"g0.data.test.p","params":[0,1],"return":2,"blocks":[{"stmts":[{"type":"ResetLocalStmt","stmt":{"target":3,"file":0,"col":4,"row":3}},{"type":"MakeNumberRefStmt","stmt":{"file":0,"col":4,"row":3,"index":2,"Index":2,"target":4}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":4},"target":3,"file":0,"col":4,"row":3}}]},{"stmts":[{"type":"IsDefinedStmt","stmt":{"source":3,"file":0,"col":4,"row":3}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":3},"target":2,"file":0,"col":4,"row":3}}]},{"stmts":[{"type":"ReturnLocalStmt","stmt":{"source":2,"file":0,"col":4,"row":3}}]}],"path":["g0","test","p"]}]}}` + expected := `{"static":{"strings":[{"value":"result"},{"value":"p"},{"value":"1"},{"value":"test"}],"files":[{"value":"TEMPDIR/test.rego"}]},"plans":{"plans":[{"name":"test","blocks":[{"stmts":[{"type":"MakeObjectStmt","stmt":{"target":2,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"CallStmt","stmt":{"func":"g0.data.test.p","args":[{"type":"local","value":0},{"type":"local","value":1}],"result":3,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":1},"value":{"type":"local","value":3},"object":2,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"BlockStmt","stmt":{"blocks":[{"stmts":[{"type":"DotStmt","stmt":{"source":{"type":"local","value":1},"key":{"type":"string_index","value":3},"target":5,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectMergeStmt","stmt":{"a":5,"b":2,"target":4,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"BreakStmt","stmt":{"index":1,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":2},"target":4,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}],"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"AssignVarStmt","stmt":{"source":{"type":"local","value":4},"target":6,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"MakeObjectStmt","stmt":{"target":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ObjectInsertStmt","stmt":{"key":{"type":"string_index","value":0},"value":{"type":"local","value":6},"object":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}},{"type":"ResultSetAddStmt","stmt":{"value":7,"file":0,"col":0,"row":0,"end_col":0,"end_row":0}}]}]}]},"funcs":{"funcs":[{"name":"g0.data.test.p","params":[0,1],"return":2,"blocks":[{"stmts":[{"type":"ResetLocalStmt","stmt":{"target":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}},{"type":"MakeNumberRefStmt","stmt":{"file":0,"col":4,"row":3,"end_col":9,"end_row":3,"index":2,"Index":2,"target":4}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":4},"target":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]},{"stmts":[{"type":"IsDefinedStmt","stmt":{"source":3,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}},{"type":"AssignVarOnceStmt","stmt":{"source":{"type":"local","value":3},"target":2,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]},{"stmts":[{"type":"ReturnLocalStmt","stmt":{"source":2,"file":0,"col":4,"row":3,"end_col":9,"end_row":3}}]}],"path":["g0","test","p"]}]}}` if diff := cmp.Diff(expected, got); diff != "" { t.Errorf("unexpected result (-want, +got):\n%s", diff) diff --git a/internal/cmd/genplanschema/main.go b/internal/cmd/genplanschema/main.go index 410b6c6f31c..849faec1d01 100644 --- a/internal/cmd/genplanschema/main.go +++ b/internal/cmd/genplanschema/main.go @@ -215,6 +215,8 @@ func makeNumberRefStmtSchema() genjsonschema.OrderedMap { "col", genjsonschema.Map("type", "integer"), "file", genjsonschema.Map("type", "integer"), "row", genjsonschema.Map("type", "integer"), + "end_col", genjsonschema.Map("type", "integer"), + "end_row", genjsonschema.Map("type", "integer"), "index", genjsonschema.Map("type", "integer"), "Index", genjsonschema.Map( "type", "integer", @@ -223,7 +225,7 @@ func makeNumberRefStmtSchema() genjsonschema.OrderedMap { ), "target", genjsonschema.Map("type", "integer"), ), - "required", []string{"col", "file", "index", "row", "target"}, + "required", []string{"col", "end_col", "end_row", "file", "index", "row", "target"}, "additionalProperties", false, ) } diff --git a/internal/planner/planner.go b/internal/planner/planner.go index a7d62542c24..296c0288428 100644 --- a/internal/planner/planner.go +++ b/internal/planner/planner.go @@ -2489,7 +2489,7 @@ func (p *Planner) appendStmtToBlock(s ir.Stmt, b *ir.Block) { if str == "" { str = `` } - s.SetLocation(p.getFileConst(str), p.loc.Row, p.loc.Col, str, string(p.loc.Text)) + s.SetLocation(p.getFileConst(str), p.loc.Row, p.loc.Col, str, p.loc.Text) } b.Stmts = append(b.Stmts, s) } diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index 5b9fb30592f..8e9ef6bbafb 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -474,7 +474,7 @@ func getLocation(x any) string { v := reflect.ValueOf(x).Elem().FieldByName("Location") li := v.Interface() file := v.FieldByName("file").String() - text := v.FieldByName("text").String() + text := v.FieldByName("text").Bytes() if loc, ok := li.(ir.Location); ok { return fmt.Sprintf("%s:%d:%d: %s", file, loc.Row, loc.Col, text) } diff --git a/v1/ast/location/location.go b/v1/ast/location/location.go index e08088cff15..9c2213bc5b3 100644 --- a/v1/ast/location/location.go +++ b/v1/ast/location/location.go @@ -97,29 +97,31 @@ func (loc *Location) HasFile() bool { return loc != nil && loc.File != "" } -// End returns the (row, col) one past the last rune of loc.Text — an -// exclusive end matching the scanner's offset calculation, so [Start, End) -// covers the text. Columns are counted per rune. Returns (Row, Col) for -// empty text and (0, 0) for a nil receiver. +// End determines the end position of loc. func (loc *Location) End() (row, col int) { if loc == nil { return 0, 0 } + return EndOf(loc.Row, loc.Col, loc.Text) +} - if len(loc.Text) == 0 { - return loc.Row, loc.Col +// EndOf returns the end (row, col) position reached by starting at (row, col) +// and advancing through text. +func EndOf(row, col int, text []byte) (endRow, endCol int) { + if len(text) == 0 { + return row, col } - row = loc.Row + bytes.Count(loc.Text, []byte{'\n'}) - col = loc.Col + endRow = row + bytes.Count(text, []byte{'\n'}) + endCol = col - lastLine := loc.Text - if row != loc.Row { - col = 1 - lastLine = loc.Text[bytes.LastIndex(loc.Text, []byte{'\n'})+1:] + lastLine := text + if endRow != row { + endCol = 1 + lastLine = text[bytes.LastIndex(text, []byte{'\n'})+1:] } - return row, col + utf8.RuneCount(lastLine) + return endRow, endCol + utf8.RuneCount(lastLine) } // Compare returns -1, 0, or 1 to indicate if this loc is less than, equal to, diff --git a/v1/ast/location/location_test.go b/v1/ast/location/location_test.go index 59f7e572f7f..e918698d592 100644 --- a/v1/ast/location/location_test.go +++ b/v1/ast/location/location_test.go @@ -243,39 +243,68 @@ func TestLocationHasFile(t *testing.T) { } } -func TestLocationEnd(t *testing.T) { +func TestEndOf(t *testing.T) { tests := map[string]struct { - loc *Location - expRow int - expCol int + row, col int + text []byte + expRow int + expCol int }{ "single-line text": { - loc: &Location{Text: []byte("false"), Row: 3, Col: 10}, + text: []byte("false"), row: 3, col: 10, expRow: 3, expCol: 15, }, "multi-line text": { - loc: &Location{Text: []byte("a\nbc"), Row: 5, Col: 2}, + text: []byte("a\nbc"), row: 5, col: 2, expRow: 6, expCol: 3, }, "multi-byte runes count as one column each": { // "café" is 5 bytes but 4 runes; the scanner advances Col - // per rune (see scanner.next), so End must too. - loc: &Location{Text: []byte("café"), Row: 1, Col: 1}, + // per rune (see scanner.next), so EndOf must too. + text: []byte("café"), row: 1, col: 1, expRow: 1, expCol: 5, }, "multi-byte runes across a newline": { - loc: &Location{Text: []byte("café\nñ"), Row: 1, Col: 1}, + text: []byte("café\nñ"), row: 1, col: 1, expRow: 2, expCol: 2, }, "single multi-byte rune": { - loc: &Location{Text: []byte("é"), Row: 1, Col: 1}, + text: []byte("é"), row: 1, col: 1, expRow: 1, expCol: 2, }, + "empty text": { + text: nil, row: 4, col: 7, + expRow: 4, + expCol: 7, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + row, col := EndOf(tc.row, tc.col, tc.text) + if row != tc.expRow || col != tc.expCol { + t.Fatalf("Expected (%d, %d) but got (%d, %d)", tc.expRow, tc.expCol, row, col) + } + }) + } +} + +func TestLocationEnd(t *testing.T) { + tests := map[string]struct { + loc *Location + expRow int + expCol int + }{ + "delegates to EndOf": { + loc: &Location{Text: []byte("a\nbc"), Row: 5, Col: 2}, + expRow: 6, + expCol: 3, + }, "nil receiver": { loc: nil, expRow: 0, diff --git a/v1/bundle/v1pb/manifest.pb.go b/v1/bundle/v1pb/manifest.pb.go index 018001f2d96..195e08eb00f 100644 --- a/v1/bundle/v1pb/manifest.pb.go +++ b/v1/bundle/v1pb/manifest.pb.go @@ -42,7 +42,7 @@ type Manifest struct { // Free-form metadata object. Modeled as `Struct` because the Go field // is `map[string]any`. Metadata *structpb.Struct `protobuf:"bytes,6,opt,name=metadata" json:"metadata,omitempty"` - // True iff `bundle.Manifest.Roots` was non-nil. `repeated string` can't + // True if `bundle.Manifest.Roots` was non-nil. `repeated string` can't // distinguish nil (default to [""]) from explicit-empty (owns no paths). RootsSet *bool `protobuf:"varint,7,opt,name=roots_set,json=rootsSet" json:"roots_set,omitempty"` unknownFields protoimpl.UnknownFields diff --git a/v1/format/format.go b/v1/format/format.go index e2f74591aa4..da044ea2dbf 100644 --- a/v1/format/format.go +++ b/v1/format/format.go @@ -16,6 +16,7 @@ import ( "github.com/open-policy-agent/opa/internal/future" "github.com/open-policy-agent/opa/v1/ast" + "github.com/open-policy-agent/opa/v1/ast/location" "github.com/open-policy-agent/opa/v1/types" "github.com/open-policy-agent/opa/v1/util" ) @@ -668,7 +669,7 @@ func (w *writer) writeRule(rule *ast.Rule, isElse bool, comments []*ast.Comment) // Additionally, a single set term must not be stripped of the outer body // braces, as that would semantically change the inner set to a body: // `p if { { x } }` -> p if { x } - headEndRow := rule.Head.Location.Row + strings.Count(string(rule.Head.Location.Text), "\n") + headEndRow, _ := location.EndOf(rule.Head.Location.Row, rule.Head.Location.Col, rule.Head.Location.Text) if rule.Body[0].Location.Row == headEndRow && !isSetTerm(rule.Body[0]) { w.write(" ") var err error @@ -1056,7 +1057,8 @@ func exprTermsEndRow(expr *ast.Expr) int { } } text = bytes.TrimRight(text, " \t\r\n") - return loc.Row + bytes.Count(text, []byte{'\n'}) + endRow, _ := location.EndOf(loc.Row, loc.Col, text) + return endRow } // isSetTerm reports whether expr is a non-negated set term. @@ -1343,7 +1345,7 @@ func (w *writer) writeTerm(term *ast.Term, comments []*ast.Comment) ([]*ast.Comm // If beforeEnd refers to a comment within the source text range, clear it // This prevents the comment from being written twice if w.beforeEnd != nil && len(term.Location.Text) > 0 { - endRow := term.Location.Row + bytes.Count(term.Location.Text, []byte{'\n'}) + endRow, _ := location.EndOf(term.Location.Row, term.Location.Col, term.Location.Text) if w.beforeEnd.Location.Row >= term.Location.Row && w.beforeEnd.Location.Row <= endRow { w.beforeEnd = nil } diff --git a/v1/ir/ir.go b/v1/ir/ir.go index df6bbdcc1d2..972890faae8 100644 --- a/v1/ir/ir.go +++ b/v1/ir/ir.go @@ -15,6 +15,7 @@ package ir import ( "fmt" + "github.com/open-policy-agent/opa/v1/ast/location" "github.com/open-policy-agent/opa/v1/types" ) @@ -84,7 +85,7 @@ type ( } locationStmt interface { - SetLocation(index, row, col int, file, text string) + SetLocation(index, row, col int, file string, text []byte) GetLocation() *Location } @@ -467,14 +468,18 @@ type ResultSetAddStmt struct { // Location records the filen index, and the row and column inside that file // that a statement can be connected to. type Location struct { - File int `json:"file"` // filename string constant index - Col int `json:"col"` - Row int `json:"row"` - file, text string // only used for debugging + File int `json:"file"` // filename string constant index + Col int `json:"col"` + Row int `json:"row"` + EndCol int `json:"end_col"` + EndRow int `json:"end_row"` + + file string // only used for debugging + text []byte // only used for debugging } // SetLocation sets the Location for a given Stmt. -func (l *Location) SetLocation(index, row, col int, file, text string) { +func (l *Location) SetLocation(index, row, col int, file string, text []byte) { *l = Location{ File: index, Row: row, @@ -482,6 +487,12 @@ func (l *Location) SetLocation(index, row, col int, file, text string) { file: file, text: text, } + l.EndRow, l.EndCol = l.End() +} + +// End determines the end position of l. +func (l *Location) End() (row, col int) { + return location.EndOf(l.Row, l.Col, l.text) } // GetLocation returns a Stmt's Location. diff --git a/v1/ir/marshal.go b/v1/ir/marshal.go index f395fb2b622..6e13635f6ca 100644 --- a/v1/ir/marshal.go +++ b/v1/ir/marshal.go @@ -113,6 +113,8 @@ func (m *MakeNumberRefStmt) MarshalJSON() ([]byte, error) { File int `json:"file"` Col int `json:"col"` Row int `json:"row"` + EndCol int `json:"end_col"` + EndRow int `json:"end_row"` Index int `json:"index"` IndexLegacy int `json:"Index"` // deprecated; remove in next major Target Local `json:"target"` @@ -120,6 +122,8 @@ func (m *MakeNumberRefStmt) MarshalJSON() ([]byte, error) { File: m.File, Col: m.Col, Row: m.Row, + EndCol: m.EndCol, + EndRow: m.EndRow, Index: m.Index, IndexLegacy: m.Index, Target: m.Target, @@ -133,6 +137,8 @@ func (m *MakeNumberRefStmt) UnmarshalJSON(bs []byte) error { File int `json:"file"` Col int `json:"col"` Row int `json:"row"` + EndCol int `json:"end_col"` + EndRow int `json:"end_row"` Index *int `json:"index"` IndexLegacy *int `json:"Index"` Target Local `json:"target"` @@ -140,7 +146,7 @@ func (m *MakeNumberRefStmt) UnmarshalJSON(bs []byte) error { if err := json.Unmarshal(bs, &raw); err != nil { return err } - m.File, m.Col, m.Row, m.Target = raw.File, raw.Col, raw.Row, raw.Target + m.File, m.Col, m.Row, m.EndCol, m.EndRow, m.Target = raw.File, raw.Col, raw.Row, raw.EndCol, raw.EndRow, raw.Target switch { case raw.Index != nil: m.Index = *raw.Index diff --git a/v1/ir/marshal_test.go b/v1/ir/marshal_test.go index c5aeb15d3c4..5991b3747bb 100644 --- a/v1/ir/marshal_test.go +++ b/v1/ir/marshal_test.go @@ -2,6 +2,7 @@ package ir import ( "encoding/json" + "fmt" "strings" "testing" ) @@ -11,7 +12,7 @@ func TestMakeNumberRefStmtMarshalsBothKeys(t *testing.T) { Index: 7, Target: 3, } - stmt.SetLocation(2, 11, 5, "test.rego", "") + stmt.SetLocation(2, 11, 5, "test.rego", nil) bs, err := json.Marshal(stmt) if err != nil { @@ -33,6 +34,69 @@ func TestMakeNumberRefStmtMarshalsBothKeys(t *testing.T) { } } +func TestLocationEndRowColSetAndMarshalled(t *testing.T) { + cases := map[string]struct { + text []byte + wantEndRow int + wantEndCol int + }{ + "single line": { + text: []byte("foo"), + wantEndRow: 11, + wantEndCol: 8, + }, + "multi line": { + text: []byte("foo\nbar"), + wantEndRow: 12, + wantEndCol: 4, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + stmt := &MakeNumberRefStmt{ + Index: 7, + Target: 3, + } + stmt.SetLocation(2, 11, 5, "test.rego", tc.text) + + endRow, endCol := stmt.Location.End() + if endRow != tc.wantEndRow { + t.Errorf("expected end row %d, got %d", tc.wantEndRow, endRow) + } + if endCol != tc.wantEndCol { + t.Errorf("expected end col %d, got %d", tc.wantEndCol, endCol) + } + + bs, err := json.Marshal(stmt) + if err != nil { + t.Fatalf("marshal: %v", err) + } + got := string(bs) + + for _, want := range []string{ + fmt.Sprintf(`"end_row":%d`, tc.wantEndRow), + fmt.Sprintf(`"end_col":%d`, tc.wantEndCol), + } { + if !strings.Contains(got, want) { + t.Errorf("output missing %q\n got: %s", want, got) + } + } + + var roundTripped MakeNumberRefStmt + if err := json.Unmarshal(bs, &roundTripped); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if roundTripped.EndRow != tc.wantEndRow { + t.Errorf("round-tripped end row = %d, want %d", roundTripped.EndRow, tc.wantEndRow) + } + if roundTripped.EndCol != tc.wantEndCol { + t.Errorf("round-tripped end col = %d, want %d", roundTripped.EndCol, tc.wantEndCol) + } + }) + } +} + func TestMakeNumberRefStmtUnmarshalAcceptsBothKeys(t *testing.T) { cases := []struct { name string @@ -76,7 +140,7 @@ func TestMakeNumberRefStmtUnmarshalAcceptsBothKeys(t *testing.T) { func TestMakeNumberRefStmtRoundTrip(t *testing.T) { orig := &MakeNumberRefStmt{Index: 13, Target: 4} - orig.SetLocation(1, 2, 3, "", "") + orig.SetLocation(1, 2, 3, "", nil) bs, err := json.Marshal(orig) if err != nil { diff --git a/v1/ir/plan.proto b/v1/ir/plan.proto index 9bd5541e8f6..f49ed9b06bb 100644 --- a/v1/ir/plan.proto +++ b/v1/ir/plan.proto @@ -99,23 +99,26 @@ message Val { } // Stmt mirrors the `ir.Stmt` interface in v1/ir/ir.go. Every Stmt carries -// the source-location triple (file, col, row) on this envelope; the body -// messages below describe only the kind-specific payload. +// the source-location quintuple (file, col, row, end_col, end_row) on this +// envelope; the body messages below describe only the kind-specific +// payload. // // On the Go side, `ir.Location` is embedded into every concrete Stmt -// implementation, so `encoding/json` flattens File/Col/Row into the -// emitted JSON body. The proto promotes those fields to the envelope -// because that's both more idiomatic protobuf and lets every body +// implementation, so `encoding/json` flattens File/Col/Row/EndCol/EndRow +// into the emitted JSON body. The proto promotes those fields to the +// envelope because that's both more idiomatic protobuf and lets every body // message start its own field numbering at 1. // // Case-number assignments (4–37) are a stability commitment. Field -// numbers 1–3 are reserved for the location triple. New cases must be -// added with the next unused number; existing numbers must never be -// repurposed. +// numbers 1–3 and 38–39 are reserved for the location fields. New cases +// must be added with the next unused number; existing numbers must never +// be repurposed. message Stmt { int32 file = 1; int32 col = 2; int32 row = 3; + int32 end_col = 38; + int32 end_row = 39; oneof kind { ArrayAppendStmt array_append_stmt = 4; AssignIntStmt assign_int_stmt = 5; diff --git a/v1/ir/plan.schema.json b/v1/ir/plan.schema.json index e4af18ba231..70cffd06b4e 100644 --- a/v1/ir/plan.schema.json +++ b/v1/ir/plan.schema.json @@ -11,6 +11,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -27,6 +33,8 @@ "required": [ "array", "col", + "end_col", + "end_row", "file", "row", "value" @@ -39,6 +47,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -54,6 +68,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "target", @@ -67,6 +83,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -82,6 +104,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source", @@ -95,6 +119,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -110,6 +140,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source", @@ -138,6 +170,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -157,6 +195,8 @@ "required": [ "blocks", "col", + "end_col", + "end_row", "file", "row" ], @@ -168,6 +208,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -180,6 +226,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "index", "row" @@ -212,6 +260,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -243,6 +297,8 @@ "required": [ "args", "col", + "end_col", + "end_row", "file", "path", "result", @@ -256,6 +312,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -281,6 +343,8 @@ "required": [ "args", "col", + "end_col", + "end_row", "file", "func", "result", @@ -294,6 +358,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -312,6 +382,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "key", "row", @@ -326,6 +398,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -343,6 +421,8 @@ "a", "b", "col", + "end_col", + "end_row", "file", "row" ], @@ -414,6 +494,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -426,6 +512,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source" @@ -438,6 +526,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -450,6 +544,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source" @@ -462,6 +558,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -474,6 +576,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source" @@ -486,6 +590,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -498,6 +608,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source" @@ -510,6 +622,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -522,6 +640,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source" @@ -534,6 +654,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -549,6 +675,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source", @@ -562,6 +690,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -578,6 +712,8 @@ "required": [ "capacity", "col", + "end_col", + "end_row", "file", "row", "target" @@ -590,6 +726,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -602,6 +744,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "target" @@ -614,6 +758,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -629,6 +779,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "target", @@ -648,6 +800,12 @@ "row": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "index": { "type": "integer" }, @@ -662,6 +820,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "index", "row", @@ -675,6 +835,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -687,6 +853,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "target" @@ -699,6 +867,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -711,6 +885,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "target" @@ -723,6 +899,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -732,6 +914,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row" ], @@ -743,6 +927,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -760,6 +950,8 @@ "a", "b", "col", + "end_col", + "end_row", "file", "row" ], @@ -771,6 +963,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -791,6 +989,8 @@ "required": [ "block", "col", + "end_col", + "end_row", "file", "row" ], @@ -802,6 +1002,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -820,6 +1026,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "key", "object", @@ -834,6 +1042,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -852,6 +1066,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "key", "object", @@ -866,6 +1082,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -886,6 +1108,8 @@ "a", "b", "col", + "end_col", + "end_row", "file", "row", "target" @@ -956,6 +1180,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -968,6 +1198,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "target" @@ -980,6 +1212,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -992,6 +1230,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "value" @@ -1004,6 +1244,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -1016,6 +1262,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "source" @@ -1028,6 +1276,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -1057,6 +1311,8 @@ "required": [ "block", "col", + "end_col", + "end_row", "file", "key", "row", @@ -1071,6 +1327,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -1086,6 +1348,8 @@ }, "required": [ "col", + "end_col", + "end_row", "file", "row", "set", @@ -1735,6 +1999,12 @@ "col": { "type": "integer" }, + "end_col": { + "type": "integer" + }, + "end_row": { + "type": "integer" + }, "file": { "type": "integer" }, @@ -1770,6 +2040,8 @@ "required": [ "block", "col", + "end_col", + "end_row", "file", "local", "path", diff --git a/v1/ir/proto.go b/v1/ir/proto.go index 0c89dcb1059..b5a497c5aaf 100644 --- a/v1/ir/proto.go +++ b/v1/ir/proto.go @@ -189,9 +189,11 @@ func stmtToProto(s Stmt) *pb.Stmt { } loc := s.GetLocation() out := &pb.Stmt{ - File: proto.Int32(toInt32(loc.File)), - Col: proto.Int32(toInt32(loc.Col)), - Row: proto.Int32(toInt32(loc.Row)), + File: proto.Int32(toInt32(loc.File)), + Col: proto.Int32(toInt32(loc.Col)), + Row: proto.Int32(toInt32(loc.Row)), + EndCol: proto.Int32(toInt32(loc.EndCol)), + EndRow: proto.Int32(toInt32(loc.EndRow)), } switch x := s.(type) { case *ArrayAppendStmt: diff --git a/v1/ir/proto_test.go b/v1/ir/proto_test.go index d14942e3464..f9f42a86fb9 100644 --- a/v1/ir/proto_test.go +++ b/v1/ir/proto_test.go @@ -18,7 +18,7 @@ import ( func TestPolicyToProtoRoundTrip(t *testing.T) { loc := ir.Location{File: 1, Row: 2, Col: 3} withLoc := func(s ir.Stmt) ir.Stmt { - s.SetLocation(loc.File, loc.Row, loc.Col, "", "") + s.SetLocation(loc.File, loc.Row, loc.Col, "", nil) return s } diff --git a/v1/ir/v1pb/plan.pb.go b/v1/ir/v1pb/plan.pb.go index a47afbcc0f4..fe55e67fc0f 100644 --- a/v1/ir/v1pb/plan.pb.go +++ b/v1/ir/v1pb/plan.pb.go @@ -668,24 +668,27 @@ func (*Val_Local) isVal_Kind() {} func (*Val_StringIndex) isVal_Kind() {} // Stmt mirrors the `ir.Stmt` interface in v1/ir/ir.go. Every Stmt carries -// the source-location triple (file, col, row) on this envelope; the body -// messages below describe only the kind-specific payload. +// the source-location quintuple (file, col, row, end_col, end_row) on this +// envelope; the body messages below describe only the kind-specific +// payload. // // On the Go side, `ir.Location` is embedded into every concrete Stmt -// implementation, so `encoding/json` flattens File/Col/Row into the -// emitted JSON body. The proto promotes those fields to the envelope -// because that's both more idiomatic protobuf and lets every body +// implementation, so `encoding/json` flattens File/Col/Row/EndCol/EndRow +// into the emitted JSON body. The proto promotes those fields to the +// envelope because that's both more idiomatic protobuf and lets every body // message start its own field numbering at 1. // // Case-number assignments (4–37) are a stability commitment. Field -// numbers 1–3 are reserved for the location triple. New cases must be -// added with the next unused number; existing numbers must never be -// repurposed. +// numbers 1–3 and 38–39 are reserved for the location fields. New cases +// must be added with the next unused number; existing numbers must never +// be repurposed. type Stmt struct { - state protoimpl.MessageState `protogen:"open.v1"` - File *int32 `protobuf:"varint,1,opt,name=file" json:"file,omitempty"` - Col *int32 `protobuf:"varint,2,opt,name=col" json:"col,omitempty"` - Row *int32 `protobuf:"varint,3,opt,name=row" json:"row,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + File *int32 `protobuf:"varint,1,opt,name=file" json:"file,omitempty"` + Col *int32 `protobuf:"varint,2,opt,name=col" json:"col,omitempty"` + Row *int32 `protobuf:"varint,3,opt,name=row" json:"row,omitempty"` + EndCol *int32 `protobuf:"varint,38,opt,name=end_col,json=endCol" json:"end_col,omitempty"` + EndRow *int32 `protobuf:"varint,39,opt,name=end_row,json=endRow" json:"end_row,omitempty"` // Types that are valid to be assigned to Kind: // // *Stmt_ArrayAppendStmt @@ -778,6 +781,20 @@ func (x *Stmt) GetRow() int32 { return 0 } +func (x *Stmt) GetEndCol() int32 { + if x != nil && x.EndCol != nil { + return *x.EndCol + } + return 0 +} + +func (x *Stmt) GetEndRow() int32 { + if x != nil && x.EndRow != nil { + return *x.EndRow + } + return 0 +} + func (x *Stmt) GetKind() isStmt_Kind { if x != nil { return x.Kind @@ -3100,11 +3117,13 @@ const file_v1_ir_plan_proto_rawDesc = "" + "\x04bool\x18\x01 \x01(\bH\x00R\x04bool\x12\x16\n" + "\x05local\x18\x02 \x01(\x05H\x00R\x05local\x12#\n" + "\fstring_index\x18\x03 \x01(\x05H\x00R\vstringIndexB\x06\n" + - "\x04kind\"\xf5\x11\n" + + "\x04kind\"\xa7\x12\n" + "\x04Stmt\x12\x12\n" + "\x04file\x18\x01 \x01(\x05R\x04file\x12\x10\n" + "\x03col\x18\x02 \x01(\x05R\x03col\x12\x10\n" + - "\x03row\x18\x03 \x01(\x05R\x03row\x12H\n" + + "\x03row\x18\x03 \x01(\x05R\x03row\x12\x17\n" + + "\aend_col\x18& \x01(\x05R\x06endCol\x12\x17\n" + + "\aend_row\x18' \x01(\x05R\x06endRow\x12H\n" + "\x11array_append_stmt\x18\x04 \x01(\v2\x1a.opa.ir.v1.ArrayAppendStmtH\x00R\x0farrayAppendStmt\x12B\n" + "\x0fassign_int_stmt\x18\x05 \x01(\v2\x18.opa.ir.v1.AssignIntStmtH\x00R\rassignIntStmt\x12O\n" + "\x14assign_var_once_stmt\x18\x06 \x01(\v2\x1c.opa.ir.v1.AssignVarOnceStmtH\x00R\x11assignVarOnceStmt\x12B\n" + From eeb9415ae0571a3a86d9d5f51d18e758834230e9 Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Thu, 13 Aug 2026 10:37:00 +0100 Subject: [PATCH 2/3] ir: Only compute end location when text present This mean End() will work after unmarshalling too. Signed-off-by: Charlie Egan --- v1/ir/ir.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/v1/ir/ir.go b/v1/ir/ir.go index 972890faae8..f7e571955e8 100644 --- a/v1/ir/ir.go +++ b/v1/ir/ir.go @@ -487,12 +487,14 @@ func (l *Location) SetLocation(index, row, col int, file string, text []byte) { file: file, text: text, } - l.EndRow, l.EndCol = l.End() + + l.EndRow, l.EndCol = location.EndOf(row, col, l.Text) } -// End determines the end position of l. +// End returns the end row and col of the location range, expected to be called +// after SetLocation or unmarshalling. func (l *Location) End() (row, col int) { - return location.EndOf(l.Row, l.Col, l.text) + return l.EndRow, l.EndCol } // GetLocation returns a Stmt's Location. From fe0cd8c29b9311d92993e0e1f74c7daf3058c2c3 Mon Sep 17 00:00:00 2001 From: Charlie Egan Date: Thu, 13 Aug 2026 10:57:13 +0100 Subject: [PATCH 3/3] ir: Implement the printing of embedded Location txt This is tricky as the type is embedded and so we need to have a custom type to handle the printing of text rather than registering String on Location itself. Signed-off-by: Charlie Egan --- e2e/proto/plan_test.go | 1 + internal/planner/planner_test.go | 3 +-- v1/ir/ir.go | 16 ++++++++++++++-- v1/ir/pretty_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 v1/ir/pretty_test.go diff --git a/e2e/proto/plan_test.go b/e2e/proto/plan_test.go index 3a83cb77a2a..3ab2dff0b9a 100644 --- a/e2e/proto/plan_test.go +++ b/e2e/proto/plan_test.go @@ -264,6 +264,7 @@ func TestPlanProtoRoundTripYAMLSuite(t *testing.T) { func roundTripCmpOpts() []cmp.Option { return []cmp.Option{ cmpopts.IgnoreUnexported(ir.Location{}), + cmpopts.IgnoreFields(ir.Location{}, "Text"), cmpopts.IgnoreFields(ir.BuiltinFunc{}, "Decl"), cmpopts.EquateEmpty(), } diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index 8e9ef6bbafb..9b698f1f582 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -474,9 +474,8 @@ func getLocation(x any) string { v := reflect.ValueOf(x).Elem().FieldByName("Location") li := v.Interface() file := v.FieldByName("file").String() - text := v.FieldByName("text").Bytes() if loc, ok := li.(ir.Location); ok { - return fmt.Sprintf("%s:%d:%d: %s", file, loc.Row, loc.Col, text) + return fmt.Sprintf("%s:%d:%d: %s", file, loc.Row, loc.Col, loc.Text) } return "unknown" } diff --git a/v1/ir/ir.go b/v1/ir/ir.go index f7e571955e8..6718acc7e6c 100644 --- a/v1/ir/ir.go +++ b/v1/ir/ir.go @@ -474,8 +474,19 @@ type Location struct { EndCol int `json:"end_col"` EndRow int `json:"end_row"` + // Text is only used for location ranges and debug prints. + // A named type is used so that its String method is called during printing. + // String cannot be set on Location since it is embedded and impacts parent + // structs if registered here. + Text locationText `json:"-"` + file string // only used for debugging - text []byte // only used for debugging +} + +type locationText []byte + +func (d locationText) String() string { + return string(d) } // SetLocation sets the Location for a given Stmt. @@ -484,8 +495,9 @@ func (l *Location) SetLocation(index, row, col int, file string, text []byte) { File: index, Row: row, Col: col, + Text: text, + file: file, - text: text, } l.EndRow, l.EndCol = location.EndOf(row, col, l.Text) diff --git a/v1/ir/pretty_test.go b/v1/ir/pretty_test.go new file mode 100644 index 00000000000..df4b33e1cb4 --- /dev/null +++ b/v1/ir/pretty_test.go @@ -0,0 +1,27 @@ +// Copyright 2026 The OPA Authors. All rights reserved. +// Use of this source code is governed by an Apache2 +// license that can be found in the LICENSE file. + +package ir + +import ( + "bytes" + "testing" +) + +func TestPrettyRenderStmt(t *testing.T) { + stmt := &ReturnLocalStmt{Source: Local(1)} + + stmt.SetLocation(0, 1, 1, "test.rego", []byte("p = 1")) + + var buf bytes.Buffer + if err := Pretty(&buf, stmt); err != nil { + t.Fatal(err) + } + + want := "*ir.ReturnLocalStmt &{Source:Local<1> Location:{File:0 Col:1 Row:1 EndCol:6 EndRow:1 Text:p = 1 file:test.rego}}\n" + got := buf.String() + if got != want { + t.Errorf("unexpected output:\nwant: %q\ngot: %q", want, got) + } +}