Skip to content

Commit b4bd2b1

Browse files
committed
fix: make sure coverage point starts with the current token start
Before that coverage point spans from the end of previous token till the end of the last token in the statement
1 parent 67caad6 commit b4bd2b1

3 files changed

Lines changed: 50 additions & 34 deletions

File tree

internal/instrument/instrumenter.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,12 @@ type executableSegment struct {
201201
func findExecutableSegments(bodyContent string, tokens []*pgquery.ScanToken) []executableSegment {
202202
var segments []executableSegment
203203

204-
segmentStart := 0
205204
hasExecutableContent := false
205+
firstExecutableTokenPos := -1
206206

207207
for idx, token := range tokens {
208208
if token.Token == pgquery.Token_BEGIN_P { // Skip until BEGIN token
209209
tokens = tokens[idx+1:]
210-
segmentStart = int(token.End)
211210
break
212211
}
213212
}
@@ -220,39 +219,42 @@ func findExecutableSegments(bodyContent string, tokens []*pgquery.ScanToken) []e
220219

221220
// Check if this is a semicolon (statement separator)
222221
if token.Token == pgquery.Token_ASCII_59 { // Token_ASCII_59 - ";"
223-
if hasExecutableContent {
222+
if hasExecutableContent && firstExecutableTokenPos >= 0 {
224223
// Check if this segment represents an executable statement
225224
segmentEnd := int(token.Start)
226-
segmentContent := bodyContent[segmentStart:segmentEnd]
225+
segmentContent := bodyContent[firstExecutableTokenPos:segmentEnd]
227226

228227
if isExecutableSegment(segmentContent) {
229228
segment := executableSegment{
230-
startPos: segmentStart,
229+
startPos: firstExecutableTokenPos,
231230
endPos: segmentEnd,
232-
lineStart: convertByteOffsetToLine(bodyContent, segmentStart),
231+
lineStart: convertByteOffsetToLine(bodyContent, firstExecutableTokenPos),
233232
lineEnd: convertByteOffsetToLine(bodyContent, segmentEnd),
234233
}
235234
segments = append(segments, segment)
236235
}
237236
}
238237

239238
// Reset for next segment
240-
segmentStart = int(token.End)
241239
hasExecutableContent = false
240+
firstExecutableTokenPos = -1
242241
} else {
243242
// This is some non-comment token, so we have content
243+
if !hasExecutableContent {
244+
firstExecutableTokenPos = int(token.Start)
245+
}
244246
hasExecutableContent = true
245247
}
246248
}
247249

248250
// Handle the last segment if there's remaining content
249-
if hasExecutableContent && segmentStart < len(bodyContent) {
250-
segmentContent := bodyContent[segmentStart:]
251+
if hasExecutableContent && firstExecutableTokenPos >= 0 && firstExecutableTokenPos < len(bodyContent) {
252+
segmentContent := bodyContent[firstExecutableTokenPos:]
251253
if isExecutableSegment(segmentContent) {
252254
segment := executableSegment{
253-
startPos: segmentStart,
255+
startPos: firstExecutableTokenPos,
254256
endPos: len(bodyContent),
255-
lineStart: convertByteOffsetToLine(bodyContent, segmentStart),
257+
lineStart: convertByteOffsetToLine(bodyContent, firstExecutableTokenPos),
256258
lineEnd: convertByteOffsetToLine(bodyContent, len(bodyContent)),
257259
}
258260
segments = append(segments, segment)

internal/instrument/location.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -161,18 +161,3 @@ func GetUniqueFiles(locations []CoveragePoint) []string {
161161
}
162162
return files
163163
}
164-
165-
// ConvertPositionToLine converts a byte position in source text to a line number (1-indexed)
166-
func ConvertPositionToLine(sourceText string, startPos int) int {
167-
if startPos < 0 || startPos > len(sourceText) {
168-
return 1
169-
}
170-
171-
line := 1
172-
for i := 0; i < startPos && i < len(sourceText); i++ {
173-
if sourceText[i] == '\n' {
174-
line++
175-
}
176-
}
177-
return line
178-
}

internal/instrument/plpgsql_ast_test.go

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,45 @@ $$ LANGUAGE plpgsql;`
6262
t.Errorf("Expected 4 coverage points, got %d", len(instrumented.Locations))
6363
}
6464

65-
// Verify coverage points are at the correct lines (by converting positions to lines)
66-
expectedLines := []int{7, 9, 11, 14}
65+
// Verify coverage points are positioned at the expected executable statements
66+
sqlContent, _ := os.ReadFile(tmpFile)
67+
sqlText := string(sqlContent)
68+
69+
// Expected executable statements that should have coverage points
70+
expectedStatements := []string{
71+
"discount_rate := 0.20",
72+
"discount_rate := 0.10",
73+
"discount_rate := 0.05",
74+
"RETURN total_amount * discount_rate",
75+
}
76+
77+
if len(instrumented.Locations) != len(expectedStatements) {
78+
t.Errorf("Expected %d coverage points, got %d", len(expectedStatements), len(instrumented.Locations))
79+
}
80+
6781
for i, cp := range instrumented.Locations {
68-
// Convert position to line number for validation
69-
// For this test, we'll use the original SQL content
70-
sqlContent, _ := os.ReadFile(tmpFile)
71-
actualLine := ConvertPositionToLine(string(sqlContent), cp.StartPos)
72-
if actualLine != expectedLines[i] {
73-
t.Errorf("Coverage point %d: expected line %d, got %d", i, expectedLines[i], actualLine)
82+
if i >= len(expectedStatements) {
83+
break
7484
}
85+
86+
// Extract the actual code at the coverage point position
87+
if cp.StartPos < 0 || cp.StartPos >= len(sqlText) {
88+
t.Errorf("Coverage point %d: invalid start position %d", i, cp.StartPos)
89+
continue
90+
}
91+
92+
// Get a reasonable chunk of text around the coverage point to verify it's correct
93+
endPos := cp.StartPos + cp.Length
94+
if endPos > len(sqlText) {
95+
endPos = len(sqlText)
96+
}
97+
actualText := strings.TrimSpace(sqlText[cp.StartPos:endPos])
98+
99+
// Check if the coverage point contains the expected statement
100+
if !strings.Contains(actualText, expectedStatements[i]) {
101+
t.Errorf("Coverage point %d: expected to contain %q, got %q", i, expectedStatements[i], actualText)
102+
}
103+
75104
if cp.ImplicitCoverage {
76105
t.Errorf("Coverage point %d: should not be implicit", i)
77106
}

0 commit comments

Comments
 (0)