Repository: ollama/ollama
Pull request: add line numbers for parser errors

If a Modelfile has an error in it, it's often difficult to debug where the error is located in the Modelfile itself. This change adds the line which the error was detected on.

This changes:
```
Error: command must be one of "from", "license", "template", "system", "adapter", "parameter", or "message"
```
into something like this:
```
Error: (line 3): command must be one of "from", "license", "template", "system", "adapter", "parameter", or "message"
```

Fixes #7251 

Changed files:
filename: parser/parser.go
patch: @@ -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
 
@@ -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
@@ -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()

filename: parser/parser_test.go
patch: @@ -3,6 +3,7 @@ package parser
 import (
 	"bytes"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"io"
 	"strings"
@@ -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) {
@@ -245,7 +253,10 @@ FROM foo
 MESSAGE badguy I'm a bad guy!
 `,
 			nil,
-			errInvalidMessageRole,
+			&ParserError{
+				LineNumber: 3,
+				Msg:        errInvalidMessageRole.Error(),
+			},
 		},
 		{
 			`
@@ -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)
 		})
 	}
 }