cover: Update report to include ranges (#8752)

Coverage Range records the source span of each expression instead of
just row.

Additive API Changes:
- cover.Position.Col field
- cover.Range.Compare function
- ast.Location End() function
- ast.Location HasFile() function

covered_lines / not_covered_lines count unique rows across ranges, same
numbers as before.

I have also broken down the cover package a bit to aid future work.

Fixes https://github.com/open-policy-agent/opa/issues/8748

---------

Signed-off-by: Charlie Egan <charlie_egan@apple.com>
This commit is contained in:
Charlie Egan
2026-06-09 17:05:19 +01:00
committed by GitHub
parent a8f8cea073
commit e7909e1875
10 changed files with 527 additions and 184 deletions
+4 -1
View File
@@ -21,7 +21,10 @@ func New() *Cover {
type Position = v1.Position
// PositionSlice is a collection of position that can be sorted.
type PositionSlice = v1.PositionSlice
//
// Deprecated: PositionSlice is unused inside OPA and will be removed in a
// future release.
type PositionSlice = v1.PositionSlice //nolint:staticcheck
// Range represents a range of positions in a file.
type Range = v1.Range
+32
View File
@@ -6,6 +6,7 @@ import (
"encoding/json"
"errors"
"fmt"
"unicode/utf8"
astJSON "github.com/open-policy-agent/opa/v1/ast/json"
"github.com/open-policy-agent/opa/v1/util"
@@ -92,6 +93,37 @@ func (loc *Location) StringLength() (n int) {
return n
}
// HasFile reports whether loc carries a non-empty File. Safe to call on a
// nil receiver.
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.
func (loc *Location) End() (row, col int) {
if loc == nil {
return 0, 0
}
if len(loc.Text) == 0 {
return loc.Row, loc.Col
}
row = loc.Row + bytes.Count(loc.Text, []byte{'\n'})
col = loc.Col
lastLine := loc.Text
if row != loc.Row {
col = 1
lastLine = loc.Text[bytes.LastIndex(loc.Text, []byte{'\n'})+1:]
}
return row, col + utf8.RuneCount(lastLine)
}
// Compare returns -1, 0, or 1 to indicate if this loc is less than, equal to,
// or greater than the other. Comparison is performed on the file, row, and
// column of the Location (but not on the text.) Nil locations are greater than
+69
View File
@@ -183,6 +183,75 @@ func TestLocationString(t *testing.T) {
}
}
func TestLocationHasFile(t *testing.T) {
tests := map[string]struct {
loc *Location
exp bool
}{
"nil receiver": {loc: nil, exp: false},
"empty file": {loc: &Location{Row: 1, Col: 1}, exp: false},
"with file": {loc: &Location{File: "x.rego", Row: 1, Col: 1}, exp: true},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
if got := tc.loc.HasFile(); got != tc.exp {
t.Fatalf("Expected %v but got %v", tc.exp, got)
}
})
}
}
func TestLocationEnd(t *testing.T) {
tests := map[string]struct {
loc *Location
expRow int
expCol int
}{
"single-line text": {
loc: &Location{Text: []byte("false"), Row: 3, Col: 10},
expRow: 3,
expCol: 15,
},
"multi-line text": {
loc: &Location{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},
expRow: 1,
expCol: 5,
},
"multi-byte runes across a newline": {
loc: &Location{Text: []byte("café\nñ"), Row: 1, Col: 1},
expRow: 2,
expCol: 2,
},
"single multi-byte rune": {
loc: &Location{Text: []byte("é"), Row: 1, Col: 1},
expRow: 1,
expCol: 2,
},
"nil receiver": {
loc: nil,
expRow: 0,
expCol: 0,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
row, col := tc.loc.End()
if row != tc.expRow || col != tc.expCol {
t.Fatalf("Expected (%d, %d) but got (%d, %d)", tc.expRow, tc.expCol, row, col)
}
})
}
}
// Verify zero allocations for Location.AppendText.
func BenchmarkLocationAppendText(b *testing.B) {
locs := []*Location{
+21 -170
View File
@@ -6,26 +6,23 @@
package cover
import (
"fmt"
"slices"
"strings"
"sync"
"github.com/open-policy-agent/opa/v1/ast"
"github.com/open-policy-agent/opa/v1/topdown"
"github.com/open-policy-agent/opa/v1/util"
)
// Cover computes and reports on coverage.
type Cover struct {
mu sync.Mutex
hits map[string]map[Position]struct{}
hits map[string]map[Range]struct{}
}
// New returns a new Cover object.
func New() *Cover {
return &Cover{
hits: map[string]map[Position]struct{}{},
hits: map[string]map[Range]struct{}{},
}
}
@@ -45,24 +42,24 @@ func (*Cover) Config() topdown.TraceConfig {
func (c *Cover) Report(modules map[string]*ast.Module) (report Report) {
report.Files = map[string]*FileReport{}
for file, hits := range c.hits {
covered := make(PositionSlice, 0, len(hits))
for pos := range hits {
covered = append(covered, pos)
covered := make([]Range, 0, len(hits))
for r := range hits {
covered = append(covered, r)
}
covered.Sort()
slices.SortFunc(covered, Range.Compare)
fr, ok := report.Files[file]
if !ok {
fr = &FileReport{}
report.Files[file] = fr
}
fr.Covered = sortedPositionSliceToRangeSlice(covered)
fr.Covered = covered
}
for file, module := range modules {
notCovered := PositionSlice{}
notCovered := map[Range]struct{}{}
ast.WalkRules(module, func(x *ast.Rule) bool {
if hasFileLocation(x.Head.Location) {
if x.Head.Location.HasFile() {
if !report.IsCovered(x.Location.File, x.Location.Row) {
notCovered = append(notCovered, Position{x.Head.Location.Row})
notCovered[rangeOf(x.Head.Location)] = struct{}{}
}
}
return false
@@ -70,18 +67,22 @@ func (c *Cover) Report(modules map[string]*ast.Module) (report Report) {
ast.WalkExprs(module, func(x *ast.Expr) bool {
if includeExprInCoverage(x) {
if !report.IsCovered(x.Location.File, x.Location.Row) {
notCovered = append(notCovered, Position{x.Location.Row})
notCovered[rangeOf(x.Location)] = struct{}{}
}
}
return false
})
notCovered.Sort()
ranges := make([]Range, 0, len(notCovered))
for r := range notCovered {
ranges = append(ranges, r)
}
slices.SortFunc(ranges, Range.Compare)
fr, ok := report.Files[file]
if !ok {
fr = &FileReport{}
report.Files[file] = fr
}
fr.NotCovered = sortedPositionSliceToRangeSlice(notCovered)
fr.NotCovered = ranges
}
var coveredLoc, notCoveredLoc int
@@ -128,110 +129,18 @@ func (c *Cover) TraceEvent(event topdown.Event) {
}
func (c *Cover) setHit(loc *ast.Location) {
if hasFileLocation(loc) {
if loc.HasFile() {
c.mu.Lock()
defer c.mu.Unlock()
hits, ok := c.hits[loc.File]
if !ok {
hits = map[Position]struct{}{}
hits = map[Range]struct{}{}
c.hits[loc.File] = hits
}
hits[Position{loc.Row}] = struct{}{}
hits[rangeOf(loc)] = struct{}{}
}
}
// Position represents a file location.
type Position struct {
Row int `json:"row"`
}
// PositionSlice is a collection of position that can be sorted.
type PositionSlice []Position
// Sort sorts the slice by line number.
func (sl PositionSlice) Sort() {
slices.SortFunc(sl, func(a, b Position) int {
return a.Row - b.Row
})
}
// Range represents a range of positions in a file.
type Range struct {
Start Position `json:"start"`
End Position `json:"end"`
}
// In returns true if the row is inside the range.
func (r Range) In(row int) bool {
return row >= r.Start.Row && row <= r.End.Row
}
// FileReport represents a coverage report for a single file.
type FileReport struct {
Covered []Range `json:"covered,omitempty"`
NotCovered []Range `json:"not_covered,omitempty"`
CoveredLines int `json:"covered_lines,omitempty"`
NotCoveredLines int `json:"not_covered_lines,omitempty"`
Coverage float64 `json:"coverage,omitempty"`
}
// IsCovered returns true if the row is marked as covered in the report.
func (fr *FileReport) IsCovered(row int) bool {
if fr == nil {
return false
}
for _, r := range fr.Covered {
if r.In(row) {
return true
}
}
return false
}
// IsNotCovered returns true if the row is marked as NOT covered in the report.
// This is not the same as simply not being reported. For example, certain
// statements like imports are not included in the report.
func (fr *FileReport) IsNotCovered(row int) bool {
if fr == nil {
return false
}
for _, r := range fr.NotCovered {
if r.In(row) {
return true
}
}
return false
}
// locCovered returns the number of lines of code covered by tests
func (fr *FileReport) locCovered() (loc int) {
for _, r := range fr.Covered {
loc += r.End.Row - r.Start.Row + 1
}
return
}
// locNotCovered returns the number of lines of code not covered by tests
func (fr *FileReport) locNotCovered() (loc int) {
for _, r := range fr.NotCovered {
loc += r.End.Row - r.Start.Row + 1
}
return
}
// computeCoveragePercentage returns the code coverage percentage of the file
func (fr *FileReport) computeCoveragePercentage() float64 {
coveredLoc := fr.locCovered()
notCoveredLoc := fr.locNotCovered()
totalLoc := coveredLoc + notCoveredLoc
if totalLoc == 0 {
return 0.0
}
return 100.0 * float64(coveredLoc) / float64(totalLoc)
}
// Report represents a coverage report for a set of files.
type Report struct {
Files map[string]*FileReport `json:"files"`
@@ -245,67 +154,9 @@ func (r Report) IsCovered(file string, row int) bool {
return r.Files[file].IsCovered(row)
}
// CoverageThresholdError represents an error raised when the global
// code coverage percentage is lower than the specified threshold.
type CoverageThresholdError struct {
Coverage float64
Threshold float64
Report *Report
}
func (e *CoverageThresholdError) Error() string {
sb := &strings.Builder{}
fmt.Fprintf(sb,
"Code coverage threshold not met: got %.2f instead of %.2f",
e.Coverage,
e.Threshold,
)
if e.Report != nil && len(e.Report.Files) > 0 {
sb.WriteString("\nLines not covered:")
for _, file := range util.KeysSorted(e.Report.Files) {
report := e.Report.Files[file]
for _, r := range report.NotCovered {
if r.Start.Row == r.End.Row {
fmt.Fprintf(sb, "\n\t%s:%d", file, r.Start.Row)
} else {
fmt.Fprintf(sb, "\n\t%s:%d-%d", file, r.Start.Row, r.End.Row)
}
}
}
}
return sb.String()
}
func sortedPositionSliceToRangeSlice(sorted []Position) (result []Range) {
if len(sorted) == 0 {
return
}
start, end := sorted[0], sorted[0]
for i := 1; i < len(sorted); i++ {
curr := sorted[i]
switch {
case curr.Row == end.Row: // skip
case curr.Row == end.Row+1:
end = curr
default:
result = append(result, Range{start, end})
start, end = curr, curr
}
}
result = append(result, Range{start, end})
return
}
func hasFileLocation(loc *ast.Location) bool {
return loc != nil && loc.File != ""
}
// Check the expression and return true if it should be included in the coverage report
func includeExprInCoverage(x *ast.Expr) bool {
_, excludeExprType := x.Terms.(*ast.SomeDecl)
return !excludeExprType && hasFileLocation(x.Location)
return !excludeExprType && x.Location.HasFile()
}
+11 -11
View File
@@ -76,18 +76,18 @@ p if {
}
expectedCovered := []Position{
{5}, // foo head
{6}, {7}, {8}, // foo body
{11}, // bar head
{12}, {13}, {14}, // bar body
{18}, {19}, // baz body hits
{23}, // p head
{25}, {26}, // p body
{Row: 5}, // foo head
{Row: 6}, {Row: 7}, {Row: 8}, // foo body
{Row: 11}, // bar head
{Row: 12}, {Row: 13}, {Row: 14}, // bar body
{Row: 18}, {Row: 19}, // baz body hits
{Row: 23}, // p head
{Row: 25}, {Row: 26}, // p body
}
expectedNotCovered := []Position{
{17}, // baz head
{20}, // baz body miss
{Row: 17}, // baz head
{Row: 20}, // baz body miss
}
for _, exp := range expectedCovered {
@@ -178,11 +178,11 @@ allow if { true }
}
expectedCovered := []Position{
{6}, // allow
{Row: 6}, // allow
}
expectedNotCovered := []Position{
{4}, // foo
{Row: 4}, // foo
}
for _, exp := range expectedCovered {
+65
View File
@@ -0,0 +1,65 @@
// Copyright 2018 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 cover
// FileReport represents a coverage report for a single file.
type FileReport struct {
Covered []Range `json:"covered,omitempty"`
NotCovered []Range `json:"not_covered,omitempty"`
CoveredLines int `json:"covered_lines,omitempty"`
NotCoveredLines int `json:"not_covered_lines,omitempty"`
Coverage float64 `json:"coverage,omitempty"`
}
// IsCovered returns true if the row is marked as covered in the report.
func (fr *FileReport) IsCovered(row int) bool {
if fr == nil {
return false
}
for _, r := range fr.Covered {
if r.In(row) {
return true
}
}
return false
}
// IsNotCovered returns true if the row is marked as NOT covered in the report.
// This is not the same as simply not being reported. For example, certain
// statements like imports are not included in the report.
func (fr *FileReport) IsNotCovered(row int) bool {
if fr == nil {
return false
}
for _, r := range fr.NotCovered {
if r.In(row) {
return true
}
}
return false
}
// locCovered returns the number of unique rows of code covered by tests
func (fr *FileReport) locCovered() int {
return uniqueRowCount(fr.Covered)
}
// locNotCovered returns the number of unique rows of code not covered by tests
func (fr *FileReport) locNotCovered() int {
return uniqueRowCount(fr.NotCovered)
}
// computeCoveragePercentage returns the code coverage percentage of the file
func (fr *FileReport) computeCoveragePercentage() float64 {
coveredLoc := fr.locCovered()
notCoveredLoc := fr.locNotCovered()
totalLoc := coveredLoc + notCoveredLoc
if totalLoc == 0 {
return 0.0
}
return 100.0 * float64(coveredLoc) / float64(totalLoc)
}
+119
View File
@@ -0,0 +1,119 @@
// Copyright 2018 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 cover
import (
"slices"
"github.com/open-policy-agent/opa/v1/ast"
)
// Position represents a file location.
type Position struct {
Row int `json:"row"`
Col int `json:"col,omitempty"`
}
// PositionSlice is a collection of position that can be sorted.
//
// Deprecated: PositionSlice is unused inside OPA and will be removed in a
// future release.
type PositionSlice []Position
// Sort sorts the slice by row, then column.
//
// Deprecated: see PositionSlice.
func (sl PositionSlice) Sort() {
slices.SortFunc(sl, func(a, b Position) int {
if a.Row != b.Row {
return a.Row - b.Row
}
return a.Col - b.Col
})
}
// Range represents a range of positions in a file.
type Range struct {
Start Position `json:"start"`
End Position `json:"end"`
}
// In returns true if the row is inside the range.
func (r Range) In(row int) bool {
return row >= r.Start.Row && row <= r.End.Row
}
// Compare orders ranges by start, then end, comparing row before col.
func (r Range) Compare(other Range) int {
if r.Start.Row != other.Start.Row {
return r.Start.Row - other.Start.Row
}
if r.Start.Col != other.Start.Col {
return r.Start.Col - other.Start.Col
}
if r.End.Row != other.End.Row {
return r.End.Row - other.End.Row
}
return r.End.Col - other.End.Col
}
// rangeOf returns a Range for loc, deriving the end row/col from loc.Text via
// (*ast.Location).End.
func rangeOf(loc *ast.Location) Range {
endRow, endCol := loc.End()
return Range{
Start: Position{Row: loc.Row, Col: loc.Col},
End: Position{Row: endRow, Col: endCol},
}
}
// uniqueRowCount returns the number of distinct rows touched by any range
// in rs. Used for line-level coverage statistics, where overlapping
// per-expression ranges must not double-count.
func uniqueRowCount(rs []Range) int {
if len(rs) == 0 {
return 0
}
rows := make(map[int]struct{}, len(rs))
for _, r := range rs {
for row := r.Start.Row; row <= r.End.Row; row++ {
rows[row] = struct{}{}
}
}
return len(rows)
}
// rowSpans returns sorted [start, end] row pairs covering the same set of
// rows as rs, with adjacent rows collapsed into a single span. Intended
// for line-oriented output (e.g. "file.rego:3-5") where overlapping or
// touching ranges should print as one entry.
func rowSpans(rs []Range) [][2]int {
if len(rs) == 0 {
return nil
}
rows := make([]int, 0, len(rs))
seen := make(map[int]struct{}, len(rs))
for _, r := range rs {
for row := r.Start.Row; row <= r.End.Row; row++ {
if _, ok := seen[row]; ok {
continue
}
seen[row] = struct{}{}
rows = append(rows, row)
}
}
slices.Sort(rows)
out := make([][2]int, 0, len(rows))
start, end := rows[0], rows[0]
for i := 1; i < len(rows); i++ {
if rows[i] == end+1 {
end = rows[i]
continue
}
out = append(out, [2]int{start, end})
start, end = rows[i], rows[i]
}
return append(out, [2]int{start, end})
}
+158
View File
@@ -0,0 +1,158 @@
// 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 cover
import (
"reflect"
"testing"
"github.com/open-policy-agent/opa/v1/ast"
)
func TestRangeIn(t *testing.T) {
r := Range{Start: Position{Row: 3}, End: Position{Row: 5}}
tests := map[string]struct {
row int
exp bool
}{
"row before start": {row: 2, exp: false},
"row at start": {row: 3, exp: true},
"row inside": {row: 4, exp: true},
"row at end inclusive": {row: 5, exp: true},
"row after end": {row: 6, exp: false},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
if got := r.In(tc.row); got != tc.exp {
t.Fatalf("Expected %v but got %v", tc.exp, got)
}
})
}
}
func TestRangeCompare(t *testing.T) {
mk := func(sr, sc, er, ec int) Range {
return Range{
Start: Position{Row: sr, Col: sc},
End: Position{Row: er, Col: ec},
}
}
tests := map[string]struct {
a Range
b Range
exp int
}{
"equal": {
a: mk(3, 1, 3, 5),
b: mk(3, 1, 3, 5),
exp: 0,
},
"earlier start row sorts first": {
a: mk(2, 10, 2, 12),
b: mk(3, 1, 3, 1),
exp: -1,
},
"same start, later end col sorts last": {
a: mk(3, 1, 3, 4),
b: mk(3, 1, 3, 5),
exp: -1,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := tc.a.Compare(tc.b)
if (got < 0) != (tc.exp < 0) || (got > 0) != (tc.exp > 0) || (got == 0) != (tc.exp == 0) {
t.Fatalf("Expected sign matching %d but got %d", tc.exp, got)
}
})
}
}
func TestRangeOf(t *testing.T) {
loc := &ast.Location{Text: []byte("false"), File: "x.rego", Row: 3, Col: 10}
got := rangeOf(loc)
want := Range{
Start: Position{Row: 3, Col: 10},
End: Position{Row: 3, Col: 15},
}
if got != want {
t.Fatalf("Expected %+v but got %+v", want, got)
}
}
func TestUniqueRowCount(t *testing.T) {
mkRange := func(startRow, endRow int) Range {
return Range{
Start: Position{Row: startRow},
End: Position{Row: endRow},
}
}
tests := map[string]struct {
rs []Range
exp int
}{
"empty": {
rs: nil,
exp: 0,
},
"two ranges sharing a row": {
rs: []Range{mkRange(3, 3), mkRange(3, 3)},
exp: 1,
},
"multi-row range plus overlap": {
rs: []Range{mkRange(2, 4), mkRange(3, 3), mkRange(7, 7)},
exp: 4, // rows 2,3,4,7
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
if got := uniqueRowCount(tc.rs); got != tc.exp {
t.Fatalf("Expected %d but got %d", tc.exp, got)
}
})
}
}
func TestRowSpans(t *testing.T) {
mkRange := func(startRow, endRow int) Range {
return Range{
Start: Position{Row: startRow},
End: Position{Row: endRow},
}
}
tests := map[string]struct {
rs []Range
exp [][2]int
}{
"empty": {
rs: nil,
exp: nil,
},
"adjacent rows merge into one span": {
rs: []Range{mkRange(3, 3), mkRange(4, 4), mkRange(5, 5)},
exp: [][2]int{{3, 5}},
},
"gaps split into separate spans": {
rs: []Range{mkRange(7, 7), mkRange(2, 4), mkRange(3, 3)},
exp: [][2]int{{2, 4}, {7, 7}},
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
got := rowSpans(tc.rs)
if !reflect.DeepEqual(got, tc.exp) {
t.Fatalf("Expected %v but got %v", tc.exp, got)
}
})
}
}
+46
View File
@@ -0,0 +1,46 @@
// Copyright 2018 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 cover
import (
"fmt"
"strings"
"github.com/open-policy-agent/opa/v1/util"
)
// CoverageThresholdError represents an error raised when the global
// code coverage percentage is lower than the specified threshold.
type CoverageThresholdError struct {
Coverage float64
Threshold float64
Report *Report
}
func (e *CoverageThresholdError) Error() string {
sb := &strings.Builder{}
fmt.Fprintf(sb,
"Code coverage threshold not met: got %.2f instead of %.2f",
e.Coverage,
e.Threshold,
)
if e.Report != nil && len(e.Report.Files) > 0 {
sb.WriteString("\nLines not covered:")
for _, file := range util.KeysSorted(e.Report.Files) {
report := e.Report.Files[file]
for _, span := range rowSpans(report.NotCovered) {
if span[0] == span[1] {
fmt.Fprintf(sb, "\n\t%s:%d", file, span[0])
} else {
fmt.Fprintf(sb, "\n\t%s:%d-%d", file, span[0], span[1])
}
}
}
}
return sb.String()
}
+2 -2
View File
@@ -674,7 +674,7 @@ func (s *session) handleEvent(t *thread, stackIndex int, e *topdown.Event, ts th
return skipAction, state, nil
}
if s.properties.StopOnEntry && !state.entered && e.Location != nil && e.Location.File != "" {
if s.properties.StopOnEntry && !state.entered && e.Location.HasFile() {
state.entered = true
s.d.logger.Info("Thread %d stopped at entry", t.id)
s.d.sendEvent(Event{Type: StoppedEventType, Thread: t.id, Message: "entry", stackIndex: stackIndex, stackEvent: e})
@@ -687,7 +687,7 @@ func (s *session) handleEvent(t *thread, stackIndex int, e *topdown.Event, ts th
return breakAction, state, nil
}
if e.Location != nil && e.Location.File != "" {
if e.Location.HasFile() {
for _, bp := range s.breakpoints.allForFilePath(e.Location.File) {
if bp.Location().Row == e.Location.Row {
// if the last event also caused a breakpoint AND we're still on the same line, skip this breakpoint.