dap: Fix breakpoint overlap check

The old logic wasn't quite right, and a little hard to follow.
Before this, because breakpoints sent from setBreakpoints don't have an
end line or column they are just a single point the comparison would
fail the check due to the column check.

E.g. if I have a breakpoint on line 30.
The content lines range from 29-31.
Now lets say the end column is at postion 10, but my breakpoint is at
L30 column 11 (still within range).
The overlap check would fail because 11 > 10 but these shouldn't be
compared at all in this case.

Signed-off-by: Brian Goff <[email protected]>

Author: cpuguy83

dap/adapter.go

@@ -612,7 +612,16 @@ func (b *breakpointMap) Intersect(ctx Context, src *pb.Source, ws string) map[di
 
 func (b *breakpointMap) intersect(ctx Context, src *pb.Source, locs *pb.Locations, ws string) int {
 	overlaps := func(r *pb.Range, bp *dap.Breakpoint) bool {
-		return r.Start.Line <= int32(bp.Line) && r.Start.Character <= int32(bp.Column) && r.End.Line >= int32(bp.EndLine) && r.End.Character >= int32(bp.EndColumn)
+		if bp.Line < int(r.Start.Line) || bp.Line > int(r.End.Line) {
+			return false
+		}
+		if bp.Line == int(r.Start.Line) && bp.Column < int(r.Start.Character) {
+			return false
+		}
+		if bp.Line == int(r.End.Line) && bp.Column > int(r.End.Character) {
+			return false
+		}
+		return true
 	}
 
 	for _, loc := range locs.Locations {

dap/adapter_test.go

@@ -3,13 +3,15 @@ package dap
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"io"
 	"path/filepath"
 	"testing"
 	"time"
 
 	"github.com/docker/buildx/dap/common"
 	"github.com/google/go-dap"
+	"github.com/moby/buildkit/solver/pb"
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/sync/errgroup"
 )
@@ -134,6 +136,104 @@ func TestSetBreakpoints(t *testing.T) {
 	eg.Wait()
 }
 
+func TestBreakpointMapIntersectVerified(t *testing.T) {
+	t.Parallel()
+
+	ws := t.TempDir()
+	filename := "Dockerfile"
+	fpath := filepath.Join(ws, filename)
+
+	type breakpointCase struct {
+		desc           string
+		sbp            dap.SourceBreakpoint
+		expectVerified bool
+	}
+
+	docRanges := []*pb.Range{
+		{Start: &pb.Position{Line: 10, Character: 0}, End: &pb.Position{Line: 10, Character: 10}},
+		{Start: &pb.Position{Line: 20, Character: 5}, End: &pb.Position{Line: 20, Character: 5}},
+		{Start: &pb.Position{Line: 30, Character: 0}, End: &pb.Position{Line: 30, Character: 10}},
+		{Start: &pb.Position{Line: 35, Character: 2}, End: &pb.Position{Line: 35, Character: 7}},
+	}
+
+	breakpointCases := []breakpointCase{
+		{desc: "inside range 0", sbp: dap.SourceBreakpoint{Line: 10, Column: 5}, expectVerified: true},
+		{desc: "before range 0", sbp: dap.SourceBreakpoint{Line: 10, Column: -1}},
+		{desc: "range 1 point", sbp: dap.SourceBreakpoint{Line: 20, Column: 5}, expectVerified: true},
+		{desc: "before range 1 point", sbp: dap.SourceBreakpoint{Line: 20, Column: 4}},
+		{desc: "range 2 end", sbp: dap.SourceBreakpoint{Line: 30, Column: 10}, expectVerified: true},
+		{desc: "after range 2", sbp: dap.SourceBreakpoint{Line: 30, Column: 11}},
+		{desc: "inside range 3", sbp: dap.SourceBreakpoint{Line: 35, Column: 4}, expectVerified: true},
+		{desc: "different line", sbp: dap.SourceBreakpoint{Line: 40, Column: 0}},
+	}
+
+	bm := newBreakpointMap()
+	sbps := make([]dap.SourceBreakpoint, len(breakpointCases))
+	for i, bc := range breakpointCases {
+		sbps[i] = bc.sbp
+	}
+	bm.Set(fpath, sbps)
+
+	srcLocs := make(map[string]*pb.Locations, len(docRanges))
+	for i, rng := range docRanges {
+		srcLocs[fmt.Sprintf("doc-%d", i)] = &pb.Locations{
+			Locations: []*pb.Location{{
+				SourceIndex: 0,
+				Ranges:      []*pb.Range{rng},
+			}},
+		}
+	}
+
+	src := &pb.Source{
+		Locations: srcLocs,
+		Infos: []*pb.SourceInfo{
+			{Filename: filename},
+		},
+	}
+
+	ctx := newBreakpointTestContext(t)
+	digests := bm.Intersect(ctx, src, ws)
+	wantMatches := 0
+	for _, bc := range breakpointCases {
+		if bc.expectVerified {
+			wantMatches++
+		}
+	}
+	assert.Len(t, digests, wantMatches)
+
+	expectedEvents := make(map[int]struct{})
+	for i, bp := range bm.byPath[fpath] {
+		if breakpointCases[i].expectVerified {
+			expectedEvents[bp.Id] = struct{}{}
+		}
+	}
+
+	for len(expectedEvents) > 0 {
+		select {
+		case msg := <-ctx.messages:
+			evt, ok := msg.(*dap.BreakpointEvent)
+			if !assert.True(t, ok, "expected breakpoint event message") {
+				continue
+			}
+			if _, ok := expectedEvents[evt.Body.Breakpoint.Id]; ok {
+				delete(expectedEvents, evt.Body.Breakpoint.Id)
+				assert.True(t, evt.Body.Breakpoint.Verified)
+			} else {
+				t.Fatalf("unexpected breakpoint event for id %d", evt.Body.Breakpoint.Id)
+			}
+		case <-time.After(time.Second):
+			t.Fatalf("expected %d more breakpoint events", len(expectedEvents))
+		}
+	}
+
+	stored := bm.byPath[fpath]
+	if assert.Len(t, stored, len(breakpointCases)) {
+		for i, bc := range breakpointCases {
+			assert.Equal(t, bc.expectVerified, stored[i].Verified, "breakpoint %d (%s) mismatch", i, bc.desc)
+		}
+	}
+}
+
 func NewTestAdapter[C LaunchConfig](t *testing.T) (*Adapter[C], Conn, *Client) {
 	t.Helper()
 
@@ -193,3 +293,29 @@ func (c *loggingConn) RecvMsg(ctx context.Context) (dap.Message, error) {
 	c.t.Logf("[%s] recv: %v", c.prefix, string(b))
 	return m, nil
 }
+
+type breakpointTestContext struct {
+	context.Context
+	messages chan dap.Message
+}
+
+func newBreakpointTestContext(t *testing.T) *breakpointTestContext {
+	t.Helper()
+	return &breakpointTestContext{
+		Context:  context.Background(),
+		messages: make(chan dap.Message, 16),
+	}
+}
+
+func (c *breakpointTestContext) C() chan<- dap.Message {
+	return c.messages
+}
+
+func (c *breakpointTestContext) Go(f func(Context)) bool {
+	go f(c)
+	return true
+}
+
+func (c *breakpointTestContext) Request(dap.RequestMessage) dap.ResponseMessage {
+	return nil
+}