Skip to content

Commit c171216

Browse files
pelletierclaude
pelletier
and
claude
authored
Fix incorrect error positions in unstable parser Range() (#1047) (#1056)
Parser.subsliceOffset() assumed its input was always a suffix of the document, computing offset as len(data)-len(b). Error highlights from NewParserError are arbitrary subslices (e.g. b[0:1]), not suffixes, causing wrong offsets after comments and other consumed content. Replace the suffix-length calculation with reflect-based pointer arithmetic, matching the safe approach already used in errors.go. --------- Co-authored-by: Claude <[email protected]>
1 parent f36a3ec commit c171216

3 files changed

Lines changed: 214 additions & 3 deletions

File tree

‎errors_test.go‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,123 @@ OtherMissing = 1
301301
assert.Equal(t, 2, len(strictErr.Unwrap()))
302302
}
303303

304+
func TestDecodeError_PositionAfterComments(t *testing.T) {
305+
examples := []struct {
306+
name string
307+
doc string
308+
expectedRow int
309+
expectedCol int
310+
}{
311+
{
312+
name: "comment then invalid key start",
313+
doc: "# comment\n= \"value\"",
314+
expectedRow: 2,
315+
expectedCol: 1,
316+
},
317+
{
318+
name: "no comment invalid key start",
319+
doc: "= \"value\"",
320+
expectedRow: 1,
321+
expectedCol: 1,
322+
},
323+
{
324+
name: "multiple comments then error",
325+
doc: "# c1\n# c2\n= \"val\"",
326+
expectedRow: 3,
327+
expectedCol: 1,
328+
},
329+
{
330+
name: "valid line then invalid key start",
331+
doc: "a = 1\n= \"val\"",
332+
expectedRow: 2,
333+
expectedCol: 1,
334+
},
335+
{
336+
name: "blank lines then error",
337+
doc: "\n\n= \"val\"",
338+
expectedRow: 3,
339+
expectedCol: 1,
340+
},
341+
{
342+
name: "expected newline but got invalid char",
343+
doc: "a = 1 b = 2",
344+
expectedRow: 1,
345+
expectedCol: 7,
346+
},
347+
}
348+
349+
for _, e := range examples {
350+
t.Run(e.name, func(t *testing.T) {
351+
var v interface{}
352+
err := Unmarshal([]byte(e.doc), &v)
353+
354+
var derr *DecodeError
355+
if !errors.As(err, &derr) {
356+
t.Fatal("error not in expected format")
357+
}
358+
359+
row, col := derr.Position()
360+
if row != e.expectedRow {
361+
t.Errorf("row: got %d, want %d", row, e.expectedRow)
362+
}
363+
if col != e.expectedCol {
364+
t.Errorf("col: got %d, want %d", col, e.expectedCol)
365+
}
366+
})
367+
}
368+
}
369+
370+
func TestErrorPositionConsistency(t *testing.T) {
371+
// Verify that the unstable parser API and the public Unmarshal API
372+
// report the same error positions for identical documents.
373+
documents := []string{
374+
"# comment\n= \"value\"",
375+
"= \"value\"",
376+
"# c1\n# c2\n= \"val\"",
377+
"a = 1\n= \"val\"",
378+
"\n\n= \"val\"",
379+
"a = 1 b = 2",
380+
}
381+
382+
for _, doc := range documents {
383+
t.Run(doc, func(t *testing.T) {
384+
// Get position from unstable API
385+
p := unstable.Parser{}
386+
p.Reset([]byte(doc))
387+
for p.NextExpression() {
388+
}
389+
err := p.Error()
390+
if err == nil {
391+
t.Fatal("expected parser error")
392+
}
393+
var perr *unstable.ParserError
394+
if !errors.As(err, &perr) {
395+
t.Fatalf("expected *ParserError, got %T", err)
396+
}
397+
r := p.Range(perr.Highlight)
398+
shape := p.Shape(r)
399+
unstableLine := shape.Start.Line
400+
unstableCol := shape.Start.Column
401+
402+
// Get position from public API
403+
var v interface{}
404+
pubErr := Unmarshal([]byte(doc), &v)
405+
var derr *DecodeError
406+
if !errors.As(pubErr, &derr) {
407+
t.Fatal("error not in expected format")
408+
}
409+
pubRow, pubCol := derr.Position()
410+
411+
if unstableLine != pubRow {
412+
t.Errorf("line mismatch: unstable=%d, public=%d", unstableLine, pubRow)
413+
}
414+
if unstableCol != pubCol {
415+
t.Errorf("column mismatch: unstable=%d, public=%d", unstableCol, pubCol)
416+
}
417+
})
418+
}
419+
}
420+
304421
func ExampleDecodeError() {
305422
doc := `name = 123__456`
306423

‎unstable/parser.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package unstable
33
import (
44
"bytes"
55
"fmt"
6+
"reflect"
67
"unicode"
78

89
"github.com/pelletier/go-toml/v2/internal/characters"
@@ -83,10 +84,18 @@ func (p *Parser) rangeOfToken(token, rest []byte) Range {
8384
}
8485

8586
// subsliceOffset returns the byte offset of subslice b within p.data.
86-
// b must be a suffix (tail) of p.data.
87+
// b must share the same backing array as p.data.
8788
func (p *Parser) subsliceOffset(b []byte) int {
88-
// b is a suffix of p.data, so its offset is len(p.data) - len(b)
89-
return len(p.data) - len(b)
89+
if len(b) == 0 {
90+
return len(p.data)
91+
}
92+
dataPtr := reflect.ValueOf(p.data).Pointer()
93+
subPtr := reflect.ValueOf(b).Pointer()
94+
offset := int(subPtr - dataPtr)
95+
if offset < 0 || offset > len(p.data) {
96+
panic("subslice is not within data")
97+
}
98+
return offset
9099
}
91100

92101
// Raw returns the slice corresponding to the bytes in the given range.

‎unstable/parser_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package unstable
22

33
import (
4+
"errors"
45
"fmt"
56
"strconv"
67
"strings"
@@ -695,3 +696,87 @@ func ExampleParser() {
695696
// Expression: KeyValue
696697
// value -> (Integer) 42
697698
}
699+
700+
func TestParserErrorPosition(t *testing.T) {
701+
examples := []struct {
702+
name string
703+
input string
704+
expectedOffset int
705+
expectedLine int
706+
expectedColumn int
707+
}{
708+
{
709+
name: "comment then invalid key start",
710+
input: "# comment\n= \"value\"",
711+
expectedOffset: 10,
712+
expectedLine: 2,
713+
expectedColumn: 1,
714+
},
715+
{
716+
name: "no comment invalid key start",
717+
input: "= \"value\"",
718+
expectedOffset: 0,
719+
expectedLine: 1,
720+
expectedColumn: 1,
721+
},
722+
{
723+
name: "multiple comments then error",
724+
input: "# c1\n# c2\n= \"val\"",
725+
expectedOffset: 10,
726+
expectedLine: 3,
727+
expectedColumn: 1,
728+
},
729+
{
730+
name: "valid line then invalid key start",
731+
input: "a = 1\n= \"val\"",
732+
expectedOffset: 6,
733+
expectedLine: 2,
734+
expectedColumn: 1,
735+
},
736+
{
737+
name: "blank lines then error",
738+
input: "\n\n= \"val\"",
739+
expectedOffset: 2,
740+
expectedLine: 3,
741+
expectedColumn: 1,
742+
},
743+
{
744+
name: "expected newline but got invalid char",
745+
input: "a = 1 b = 2",
746+
expectedOffset: 6,
747+
expectedLine: 1,
748+
expectedColumn: 7,
749+
},
750+
}
751+
752+
for _, e := range examples {
753+
t.Run(e.name, func(t *testing.T) {
754+
p := Parser{}
755+
p.Reset([]byte(e.input))
756+
for p.NextExpression() {
757+
}
758+
err := p.Error()
759+
if err == nil {
760+
t.Fatal("expected an error")
761+
}
762+
763+
var perr *ParserError
764+
if !errors.As(err, &perr) {
765+
t.Fatalf("expected *ParserError, got %T", err)
766+
}
767+
768+
r := p.Range(perr.Highlight)
769+
shape := p.Shape(r)
770+
771+
if int(r.Offset) != e.expectedOffset {
772+
t.Errorf("offset: got %d, want %d", r.Offset, e.expectedOffset)
773+
}
774+
if shape.Start.Line != e.expectedLine {
775+
t.Errorf("line: got %d, want %d", shape.Start.Line, e.expectedLine)
776+
}
777+
if shape.Start.Column != e.expectedColumn {
778+
t.Errorf("column: got %d, want %d", shape.Start.Column, e.expectedColumn)
779+
}
780+
})
781+
}
782+
}

0 commit comments

Comments
 (0)