Skip to content

Commit

Permalink
add line numbers for parser errors (ollama#7326)
Browse files Browse the repository at this point in the history
  • Loading branch information
pdevine authored Nov 14, 2024
1 parent 0679d49 commit 4efb98c
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 9 deletions.
32 changes: 29 additions & 3 deletions parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,22 @@ var (
errInvalidCommand = errors.New("command must be one of \"from\", \"license\", \"template\", \"system\", \"adapter\", \"parameter\", or \"message\"")
)

type ParserError struct {
LineNumber int
Msg string
}

func (e *ParserError) Error() string {
if e.LineNumber > 0 {
return fmt.Sprintf("(line %d): %s", e.LineNumber, e.Msg)
}
return e.Msg
}

func ParseFile(r io.Reader) (*File, error) {
var cmd Command
var curr state
var currLine int = 1
var b bytes.Buffer
var role string

Expand All @@ -84,19 +97,29 @@ func ParseFile(r io.Reader) (*File, error) {
return nil, err
}

if isNewline(r) {
currLine++
}

next, r, err := parseRuneForState(r, curr)
if errors.Is(err, io.ErrUnexpectedEOF) {
return nil, fmt.Errorf("%w: %s", err, b.String())
} else if err != nil {
return nil, err
return nil, &ParserError{
LineNumber: currLine,
Msg: err.Error(),
}
}

// process the state transition, some transitions need to be intercepted and redirected
if next != curr {
switch curr {
case stateName:
if !isValidCommand(b.String()) {
return nil, errInvalidCommand
return nil, &ParserError{
LineNumber: currLine,
Msg: errInvalidCommand.Error(),
}
}

// next state sometimes depends on the current buffer value
Expand All @@ -117,7 +140,10 @@ func ParseFile(r io.Reader) (*File, error) {
cmd.Name = b.String()
case stateMessage:
if !isValidMessageRole(b.String()) {
return nil, errInvalidMessageRole
return nil, &ParserError{
LineNumber: currLine,
Msg: errInvalidMessageRole.Error(),
}
}

role = b.String()
Expand Down
45 changes: 39 additions & 6 deletions parser/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package parser
import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"io"
"strings"
Expand Down Expand Up @@ -180,8 +181,15 @@ func TestParseFileBadCommand(t *testing.T) {
FROM foo
BADCOMMAND param1 value1
`
parserError := &ParserError{
LineNumber: 3,
Msg: errInvalidCommand.Error(),
}

_, err := ParseFile(strings.NewReader(input))
require.ErrorIs(t, err, errInvalidCommand)
if !errors.As(err, &parserError) {
t.Errorf("unexpected error: expected: %s, actual: %s", parserError.Error(), err.Error())
}
}

func TestParseFileMessages(t *testing.T) {
Expand Down Expand Up @@ -245,7 +253,10 @@ FROM foo
MESSAGE badguy I'm a bad guy!
`,
nil,
errInvalidMessageRole,
&ParserError{
LineNumber: 3,
Msg: errInvalidMessageRole.Error(),
},
},
{
`
Expand All @@ -264,13 +275,35 @@ MESSAGE system`,
},
}

for _, c := range cases {
for _, tt := range cases {
t.Run("", func(t *testing.T) {
modelfile, err := ParseFile(strings.NewReader(c.input))
require.ErrorIs(t, err, c.err)
modelfile, err := ParseFile(strings.NewReader(tt.input))

if modelfile != nil {
assert.Equal(t, c.expected, modelfile.Commands)
assert.Equal(t, tt.expected, modelfile.Commands)
}

if tt.err == nil {
if err != nil {
t.Fatalf("expected no error, but got %v", err)
}
return
}

switch tt.err.(type) {
case *ParserError:
var pErr *ParserError
if errors.As(err, &pErr) {
// got the correct type of error
return
}
}

if errors.Is(err, tt.err) {
return
}

t.Fatalf("unexpected error: expected: %v, actual: %v", tt.err, err)
})
}
}
Expand Down

0 comments on commit 4efb98c

Please sign in to comment.