From 1b398f6213c93eab81451a1a8ce580f8ec95e52d Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 12 Aug 2024 09:09:26 +0100 Subject: [PATCH] Replace panics with errors Signed-off-by: Paulo Gomes --- doc.go | 97 +++++++++++++++---------------- errors.go | 31 +++++----- errors_test.go | 17 ++++++ read.go | 84 +++++++++++++++++++++------ read_test.go | 2 +- scanner/example_test.go | 20 +++++-- scanner/scanner.go | 106 ++++++++++++++++++++++++++-------- scanner/scanner_test.go | 124 ++++++++++++++++++++++++++++++---------- set.go | 27 ++++----- token/position.go | 61 +++++++++----------- token/position_test.go | 51 ++++++++++++++--- token/serialize_test.go | 5 +- token/token.go | 4 -- types/int.go | 12 ++-- 14 files changed, 436 insertions(+), 205 deletions(-) create mode 100644 errors_test.go diff --git a/doc.go b/doc.go index 7bdefbf..ce9a9ab 100644 --- a/doc.go +++ b/doc.go @@ -4,29 +4,29 @@ // This package is still a work in progress; see the sections below for planned // changes. // -// Syntax +// # Syntax // // The syntax is based on that used by git config: // http://git-scm.com/docs/git-config#_syntax . // There are some (planned) differences compared to the git config format: -// - improve data portability: -// - must be encoded in UTF-8 (for now) and must not contain the 0 byte -// - include and "path" type is not supported -// (path type may be implementable as a user-defined type) -// - internationalization -// - section and variable names can contain unicode letters, unicode digits -// (as defined in http://golang.org/ref/spec#Characters ) and hyphens -// (U+002D), starting with a unicode letter -// - disallow potentially ambiguous or misleading definitions: -// - `[sec.sub]` format is not allowed (deprecated in gitconfig) -// - `[sec ""]` is not allowed -// - use `[sec]` for section name "sec" and empty subsection name -// - (planned) within a single file, definitions must be contiguous for each: -// - section: '[secA]' -> '[secB]' -> '[secA]' is an error -// - subsection: '[sec "A"]' -> '[sec "B"]' -> '[sec "A"]' is an error -// - multivalued variable: 'multi=a' -> 'other=x' -> 'multi=b' is an error -// -// Data structure +// - improve data portability: +// - must be encoded in UTF-8 (for now) and must not contain the 0 byte +// - include and "path" type is not supported +// (path type may be implementable as a user-defined type) +// - internationalization +// - section and variable names can contain unicode letters, unicode digits +// (as defined in http://golang.org/ref/spec#Characters ) and hyphens +// (U+002D), starting with a unicode letter +// - disallow potentially ambiguous or misleading definitions: +// - `[sec.sub]` format is not allowed (deprecated in gitconfig) +// - `[sec ""]` is not allowed +// - use `[sec]` for section name "sec" and empty subsection name +// - (planned) within a single file, definitions must be contiguous for each: +// - section: '[secA]' -> '[secB]' -> '[secA]' is an error +// - subsection: '[sec "A"]' -> '[sec "B"]' -> '[sec "A"]' is an error +// - multivalued variable: 'multi=a' -> 'other=x' -> 'multi=b' is an error +// +// # Data structure // // The functions in this package read values into a user-defined struct. // Each section corresponds to a struct field in the config struct, and each @@ -52,11 +52,11 @@ // "default-" (or by setting values in the corresponding struct // field "Default_"). // -// The functions in this package panic if config is not a pointer to a struct, +// The functions in this package error if config is not a pointer to a struct, // or when a field is not of a suitable type (either a struct or a map with // string keys and pointer-to-struct values). // -// Parsing of values +// # Parsing of values // // The section structs in the config struct may contain single-valued or // multi-valued variables. Variables of unnamed slice type (that is, a type @@ -98,20 +98,18 @@ // The types subpackage for provides helpers for parsing "enum-like" and integer // types. // -// Error handling +// # Error handling // // There are 3 types of errors: // -// - programmer errors / panics: -// - invalid configuration structure -// - data errors: -// - fatal errors: -// - invalid configuration syntax -// - warnings: -// - data that doesn't belong to any part of the config structure -// -// Programmer errors trigger panics. These are should be fixed by the programmer -// before releasing code that uses gcfg. +// 1. Logic errors: invalid configuration structure. +// 2. Data errors (fatal): invalid configuration syntax. +// 3. Data errors (warning): data that doesn't belong to any part of the config +// structure. + +// All errors are handled via Go's built-in error convention. Warnings regarding +// data errors are wrapped around ErrSyntaxWarning, so that it can be more easily +// identified by consumers. This library do not cause panics. // // Data errors cause gcfg to return a non-nil error value. This includes the // case when there are extra unknown key-value definitions in the configuration @@ -122,24 +120,23 @@ // filtered out programmatically. To ignore extra data warnings, wrap the // gcfg.Read*Into invocation into a call to gcfg.FatalOnly. // -// TODO +// # TODO // // The following is a list of changes under consideration: -// - documentation -// - self-contained syntax documentation -// - more practical examples -// - move TODOs to issue tracker (eventually) -// - syntax -// - reconsider valid escape sequences -// (gitconfig doesn't support \r in value, \t in subsection name, etc.) -// - reading / parsing gcfg files -// - define internal representation structure -// - support multiple inputs (readers, strings, files) -// - support declaring encoding (?) -// - support varying fields sets for subsections (?) -// - writing gcfg files -// - error handling -// - make error context accessible programmatically? -// - limit input size? -// +// - documentation +// - self-contained syntax documentation +// - more practical examples +// - move TODOs to issue tracker (eventually) +// - syntax +// - reconsider valid escape sequences +// (gitconfig doesn't support \r in value, \t in subsection name, etc.) +// - reading / parsing gcfg files +// - define internal representation structure +// - support multiple inputs (readers, strings, files) +// - support declaring encoding (?) +// - support varying fields sets for subsections (?) +// - writing gcfg files +// - error handling +// - make error context accessible programmatically? +// - limit input size? package gcfg // import "github.com/go-git/gcfg" diff --git a/errors.go b/errors.go index a638d3b..b1d444c 100644 --- a/errors.go +++ b/errors.go @@ -6,19 +6,11 @@ import ( ) var ( - ErrCantStoreData = errors.New("can't store data") -) + ErrSyntaxWarning = errors.New("syntax warning") -func newParseError(sec, sub, variable string) error { - msg := fmt.Sprintf("section %q", sec) - if sub != "" { - msg += fmt.Sprintf(", subsection %q", sub) - } - if variable != "" { - msg += fmt.Sprintf(", variable %q", variable) - } - return fmt.Errorf("%w: %s", ErrCantStoreData, msg) -} + ErrMissingEscapeSequence = errors.New("missing escape sequence") + ErrMissingEndQuote = errors.New("missing end quote") +) // FatalOnly filters the results of a Read*Into invocation and returns only // fatal errors. That is, errors (warnings) indicating data for unknown @@ -33,14 +25,25 @@ func FatalOnly(err error) error { return nil } err = errors.Unwrap(err) - if !errors.Is(err, ErrCantStoreData) { + if !errors.Is(err, ErrSyntaxWarning) { return err } } } +func newSyntaxWarning(sec, sub, variable string) error { + msg := fmt.Sprintf("can't store data in section %q", sec) + if sub != "" { + msg += fmt.Sprintf(", subsection %q", sub) + } + if variable != "" { + msg += fmt.Sprintf(", variable %q", variable) + } + return fmt.Errorf("%w: %s", ErrSyntaxWarning, msg) +} + func joinNonFatal(prev, cur error) (error, bool) { - if !errors.Is(cur, ErrCantStoreData) { + if !errors.Is(cur, ErrSyntaxWarning) { return cur, true } return errors.Join(prev, cur), false diff --git a/errors_test.go b/errors_test.go new file mode 100644 index 0000000..10eb192 --- /dev/null +++ b/errors_test.go @@ -0,0 +1,17 @@ +package gcfg_test + +import "testing" + +func TestXxx(t *testing.T) { + tests := []struct { + name string + in error + want error + }{} + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // For + }) + } +} diff --git a/read.go b/read.go index 635287c..4aba311 100644 --- a/read.go +++ b/read.go @@ -13,7 +13,7 @@ import ( var unescape = map[rune]rune{'\\': '\\', '"': '"', 'n': '\n', 't': '\t', 'b': '\b', '\n': '\n'} // no error: invalid literals should be caught by scanner -func unquote(s string) string { +func unquote(s string) (string, error) { u, q, esc := make([]rune, 0, len(s)), false, false for _, c := range s { if esc { @@ -26,7 +26,7 @@ func unquote(s string) string { esc = false continue } - panic("invalid escape sequence") + return "", ErrMissingEscapeSequence } switch c { case '"': @@ -38,12 +38,12 @@ func unquote(s string) string { } } if q { - panic("missing end quote") + return "", ErrMissingEndQuote } if esc { - panic("invalid escape sequence") + return "", ErrMissingEscapeSequence } - return string(u) + return string(u), nil } func read(callback func(string, string, string, string, bool) error, @@ -53,10 +53,13 @@ func read(callback func(string, string, string, string, bool) error, var errs scanner.ErrorList s.Init(file, src, func(p token.Position, m string) { errs.Add(p, m) }, 0) sect, sectsub := "", "" - pos, tok, lit := s.Scan() + pos, tok, lit, err := s.Scan() errfn := func(msg string) error { return fmt.Errorf("%s: %s", fset.Position(pos), msg) } + if err != nil { + return err + } var accErr error for { if errs.Len() > 0 { @@ -68,9 +71,15 @@ func read(callback func(string, string, string, string, bool) error, case token.EOF: return nil case token.EOL, token.COMMENT: - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } case token.LBRACK: - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if errs.Len() > 0 { if err, fatal := joinNonFatal(accErr, errs.Err()); fatal { return err @@ -82,20 +91,31 @@ func read(callback func(string, string, string, string, bool) error, } } sect, sectsub = lit, "" - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if errs.Len() > 0 { if err, fatal := joinNonFatal(accErr, errs.Err()); fatal { return err } } if tok == token.STRING { - sectsub = unquote(lit) + ss, err := unquote(lit) + if err != nil { + return err + } + + sectsub = ss if sectsub == "" { if err, fatal := joinNonFatal(accErr, errfn("empty subsection name")); fatal { return err } } - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if errs.Len() > 0 { if err, fatal := joinNonFatal(accErr, errs.Err()); fatal { return err @@ -112,7 +132,10 @@ func read(callback func(string, string, string, string, bool) error, return err } } - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if tok != token.EOL && tok != token.EOF && tok != token.COMMENT { if err, fatal := joinNonFatal(accErr, errfn("expected EOL, EOF, or comment")); fatal { return err @@ -132,7 +155,10 @@ func read(callback func(string, string, string, string, bool) error, } } n := lit - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if errs.Len() > 0 { return errs.Err() } @@ -143,7 +169,10 @@ func read(callback func(string, string, string, string, bool) error, return err } } - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if errs.Len() > 0 { if err, fatal := joinNonFatal(accErr, errs.Err()); fatal { return err @@ -154,8 +183,16 @@ func read(callback func(string, string, string, string, bool) error, return err } } - v = unquote(lit) - pos, tok, lit = s.Scan() + unq, err := unquote(lit) + if err != nil { + return err + } + + v = unq + pos, tok, lit, err = s.Scan() + if err != nil { + return err + } if errs.Len() > 0 { if err, fatal := joinNonFatal(accErr, errs.Err()); fatal { return err @@ -223,7 +260,10 @@ func ReadWithCallback(reader io.Reader, callback func(string, string, string, st } fset := token.NewFileSet() - file := fset.AddFile("", fset.Base(), len(src)) + file, err := fset.AddFile("", fset.Base(), len(src)) + if err != nil { + return err + } return read(callback, fset, file, src) } @@ -236,7 +276,10 @@ func ReadInto(config interface{}, reader io.Reader) error { return err } fset := token.NewFileSet() - file := fset.AddFile("", fset.Base(), len(src)) + file, err := fset.AddFile("", fset.Base(), len(src)) + if err != nil { + return err + } return readInto(config, fset, file, src) } @@ -260,6 +303,9 @@ func ReadFileInto(config interface{}, filename string) error { return err } fset := token.NewFileSet() - file := fset.AddFile(filename, fset.Base(), len(src)) + file, err := fset.AddFile(filename, fset.Base(), len(src)) + if err != nil { + return err + } return readInto(config, fset, file, src) } diff --git a/read_test.go b/read_test.go index d2caae2..0c2392c 100644 --- a/read_test.go +++ b/read_test.go @@ -379,7 +379,7 @@ func TestReadStringIntoExtraData(t *testing.T) { name2 = value2` err := FatalOnly(ReadStringInto(res, cfg)) if err != nil { - t.Error(err) + t.Errorf("unexpected error: %v", err) } if res.Section.Name != "value" { t.Errorf("res.Section.Name=%q; want %q", res.Section.Name, "value") diff --git a/scanner/example_test.go b/scanner/example_test.go index 3a9b0d5..7e4a2d3 100644 --- a/scanner/example_test.go +++ b/scanner/example_test.go @@ -6,9 +6,8 @@ package scanner_test import ( "fmt" -) + "log" -import ( "github.com/go-git/gcfg/scanner" "github.com/go-git/gcfg/token" ) @@ -20,13 +19,22 @@ color = blue ; Comment`) // Initialize the scanner. var s scanner.Scanner - fset := token.NewFileSet() // positions are relative to fset - file := fset.AddFile("", fset.Base(), len(src)) // register input "file" - s.Init(file, src, nil /* no error handler */, scanner.ScanComments) + fset := token.NewFileSet() // positions are relative to fset + file, err := fset.AddFile("", fset.Base(), len(src)) // register input "file" + if err != nil { + log.Fatalf("failed to add file: %v", err) + } + err = s.Init(file, src, nil /* no error handler */, scanner.ScanComments) + if err != nil { + log.Fatalf("failed to initialize scanner: %v", err) + } // Repeated calls to Scan yield the token sequence found in the input. for { - pos, tok, lit := s.Scan() + pos, tok, lit, err := s.Scan() + if err != nil { + log.Fatalf("failed to scan: %v", err) + } if tok == token.EOF { break } diff --git a/scanner/scanner.go b/scanner/scanner.go index b3da03d..bc9f5c7 100644 --- a/scanner/scanner.go +++ b/scanner/scanner.go @@ -11,6 +11,7 @@ package scanner import ( + "errors" "fmt" "path/filepath" "unicode" @@ -19,6 +20,8 @@ import ( "github.com/go-git/gcfg/token" ) +var ErrSourceLenAndSizeMismatch = errors.New("source length and file size mismatch") + // An ErrorHandler may be provided to Scanner.Init. If a syntax error is // encountered and a handler was installed, the handler is called with a // position and an error message. The position points to the beginning of @@ -49,7 +52,7 @@ type Scanner struct { // Read the next Unicode char into s.ch. // s.ch < 0 means end-of-file. -func (s *Scanner) next() { +func (s *Scanner) next() error { if s.rdOffset < len(s.src) { s.offset = s.rdOffset if s.ch == '\n' { @@ -59,12 +62,18 @@ func (s *Scanner) next() { r, w := rune(s.src[s.rdOffset]), 1 switch { case r == 0: - s.error(s.offset, "illegal character NUL") + err := s.error(s.offset, "illegal character NUL") + if err != nil { + return err + } case r >= 0x80: // not ASCII r, w = utf8.DecodeRune(s.src[s.rdOffset:]) if r == utf8.RuneError && w == 1 { - s.error(s.offset, "illegal UTF-8 encoding") + err := s.error(s.offset, "illegal UTF-8 encoding") + if err != nil { + return err + } } } s.rdOffset += w @@ -77,6 +86,7 @@ func (s *Scanner) next() { } s.ch = -1 // eof } + return nil } // A mode value is a set of flags (or 0). @@ -91,8 +101,9 @@ const ( // scanner at the beginning of src. The scanner uses the file set file // for position information and it adds line information for each line. // It is ok to re-use the same file when re-scanning the same file as -// line information which is already present is ignored. Init causes a -// panic if the file size does not match the src size. +// line information which is already present is ignored. Init returns +// ErrSourceLenAndSizeMismatch if the file size does not match the src +// size. // // Calls to Scan will invoke the error handler err if they encounter a // syntax error and err is not nil. Also, for each error encountered, @@ -101,10 +112,11 @@ const ( // // Note that Init may call err if there is an error in the first character // of the file. -func (s *Scanner) Init(file *token.File, src []byte, err ErrorHandler, mode Mode) { +func (s *Scanner) Init(file *token.File, src []byte, err ErrorHandler, mode Mode) error { // Explicitly initialize all fields since a scanner may be reused. if file.Size() != len(src) { - panic(fmt.Sprintf("file size (%d) does not match src len (%d)", file.Size(), len(src))) + return fmt.Errorf("%w: file size (%d) src len (%d)", + ErrSourceLenAndSizeMismatch, file.Size(), len(src)) } s.file = file s.dir, _ = filepath.Split(file.Name()) @@ -120,13 +132,23 @@ func (s *Scanner) Init(file *token.File, src []byte, err ErrorHandler, mode Mode s.nextVal = false s.next() + return nil } -func (s *Scanner) error(offs int, msg string) { +func (s *Scanner) error(offs int, msg string) error { if s.err != nil { - s.err(s.file.Position(s.file.Pos(offs)), msg) + pos, err := s.file.Pos(offs) + if err != nil { + return err + } + position, err := s.file.Position(pos) + if err != nil { + return err + } + s.err(position, msg) } s.ErrorCount++ + return nil } func (s *Scanner) scanComment() string { @@ -156,7 +178,7 @@ func (s *Scanner) scanIdentifier() string { } // val indicate if we are scanning a value (vs a header) -func (s *Scanner) scanEscape(val bool) { +func (s *Scanner) scanEscape(val bool) error { offs := s.offset ch := s.ch s.next() // always make progress @@ -169,11 +191,15 @@ func (s *Scanner) scanEscape(val bool) { } fallthrough default: - s.error(offs, "unknown escape sequence") + err := s.error(offs, "unknown escape sequence") + if err != nil { + return err + } } + return nil } -func (s *Scanner) scanString() string { +func (s *Scanner) scanString() (string, error) { // '"' opening already consumed offs := s.offset - 1 @@ -181,7 +207,10 @@ func (s *Scanner) scanString() string { ch := s.ch s.next() if ch == '\n' || ch < 0 { - s.error(offs, "string not terminated") + err := s.error(offs, "string not terminated") + if err != nil { + return "", err + } break } if ch == '\\' { @@ -189,9 +218,12 @@ func (s *Scanner) scanString() string { } } - s.next() + err := s.next() + if err != nil { + return "", err + } - return string(s.src[offs:s.offset]) + return string(s.src[offs:s.offset]), nil } func stripCR(b []byte) []byte { @@ -206,7 +238,7 @@ func stripCR(b []byte) []byte { return c[:i] } -func (s *Scanner) scanValString() string { +func (s *Scanner) scanValString() (string, error) { offs := s.offset hasCR := false @@ -234,7 +266,10 @@ loop: case ch == '\r': hasCR = true case ch < 0 || inQuote && ch == '\n': - s.error(offs, "string not terminated") + err := s.error(offs, "string not terminated") + if err != nil { + return "", err + } break loop } if inQuote || !isWhiteSpace(ch) { @@ -247,7 +282,7 @@ loop: lit = stripCR(lit) } - return string(lit) + return string(lit), nil } func isWhiteSpace(ch rune) bool { @@ -282,17 +317,27 @@ func (s *Scanner) skipWhitespace() { // Scan adds line information to the file added to the file // set with Init. Token positions are relative to that file // and thus relative to the file set. -func (s *Scanner) Scan() (pos token.Pos, tok token.Token, lit string) { +func (s *Scanner) Scan() (pos token.Pos, tok token.Token, lit string, err error) { scanAgain: s.skipWhitespace() // current token start - pos = s.file.Pos(s.offset) + p, err2 := s.file.Pos(s.offset) + if err2 != nil { + err = fmt.Errorf("unexpected error at pos %v offset %d: %w", p, s.offset, err2) + return + } + pos = p // determine token value switch ch := s.ch; { case s.nextVal: - lit = s.scanValString() + l, err2 := s.scanValString() + if err2 != nil { + err = fmt.Errorf("unexpected error at ch %v: %w", ch, err2) + return + } + lit = l tok = token.STRING s.nextVal = false case isLetter(ch): @@ -307,7 +352,12 @@ scanAgain: tok = token.EOL case '"': tok = token.STRING - lit = s.scanString() + l, err2 := s.scanString() + if err2 != nil { + err = fmt.Errorf("unexpected error at ch %v: %w", ch, err2) + return + } + lit = l case '[': tok = token.LBRACK case ']': @@ -324,7 +374,17 @@ scanAgain: tok = token.ASSIGN s.nextVal = true default: - s.error(s.file.Offset(pos), fmt.Sprintf("illegal character %#U", ch)) + offset, err2 := s.file.Offset(pos) + if err2 != nil { + err = fmt.Errorf("unexpected error at pos %v: %w", pos, err2) + return + } + + err2 = s.error(offset, fmt.Sprintf("illegal character %#U", ch)) + if err2 != nil { + err = fmt.Errorf("unexpected error at ch %v offset %d: %w", ch, s.offset, err2) + return + } tok = token.ILLEGAL lit = string(ch) } diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index dcb76b4..b7c1834 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -8,9 +8,7 @@ import ( "os" "strings" "testing" -) -import ( "github.com/go-git/gcfg/token" ) @@ -136,7 +134,11 @@ func TestScan(t *testing.T) { // verify scan var s Scanner - s.Init(fset.AddFile("", fset.Base(), len(source)), source, eh, ScanComments) + file, err := fset.AddFile("", fset.Base(), len(source)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + s.Init(file, source, eh, ScanComments) // epos is the expected position epos := token.Position{ Filename: "", @@ -145,7 +147,10 @@ func TestScan(t *testing.T) { Column: 1, } for { - pos, tok, lit := s.Scan() + pos, tok, lit, err := s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if lit == "" { // no literal value for non-literal tokens lit = tok.String() @@ -171,7 +176,10 @@ func TestScan(t *testing.T) { if tok != etok { t.Errorf("bad token for %q: got %q, expected %q", lit, tok, etok) } - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } } epos.Offset += len(e.pre) if tok != token.EOF { @@ -187,7 +195,10 @@ func TestScan(t *testing.T) { epos.Line++ epos.Offset++ epos.Column = 1 - pos, tok, lit = s.Scan() + pos, tok, lit, err = s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } } checkPos(t, lit, pos, epos) if tok != e.tok { @@ -214,19 +225,28 @@ func TestScan(t *testing.T) { break } if e.suf == "value" { - pos, tok, lit = s.Scan() + _, tok, lit, err = s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if tok != token.STRING { t.Errorf("bad token for %q: got %q, expected %q", lit, tok, token.STRING) } } else if strings.ContainsRune(e.suf, ';') || strings.ContainsRune(e.suf, '#') { - pos, tok, lit = s.Scan() + _, tok, lit, err = s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if tok != token.COMMENT { t.Errorf("bad token for %q: got %q, expected %q", lit, tok, token.COMMENT) } } // skip EOLs for i := 0; i < whitespace_linecount+newlineCount(e.suf); i++ { - pos, tok, lit = s.Scan() + _, tok, lit, err = s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } if tok != token.EOL { t.Errorf("bad token for %q: got %q, expected %q", lit, tok, token.EOL) } @@ -240,11 +260,17 @@ func TestScan(t *testing.T) { func TestScanValStringEOF(t *testing.T) { var s Scanner src := "= value" - f := fset.AddFile("src", fset.Base(), len(src)) + f, err := fset.AddFile("src", fset.Base(), len(src)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } s.Init(f, []byte(src), nil, 0) - s.Scan() // = - s.Scan() // value - _, tok, _ := s.Scan() // EOF + s.Scan() // = + s.Scan() // value + _, tok, _, err := s.Scan() // EOF + if err != nil { + t.Errorf("unexpected error: %v", err) + } if tok != token.EOF { t.Errorf("bad token: got %s, expected %s", tok, token.EOF) } @@ -259,26 +285,38 @@ func TestInit(t *testing.T) { // 1st init src1 := "\nname = value" - f1 := fset.AddFile("src1", fset.Base(), len(src1)) + f1, err := fset.AddFile("src1", fset.Base(), len(src1)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } s.Init(f1, []byte(src1), nil, 0) if f1.Size() != len(src1) { t.Errorf("bad file size: got %d, expected %d", f1.Size(), len(src1)) } - s.Scan() // \n - s.Scan() // name - _, tok, _ := s.Scan() // = + s.Scan() // \n + s.Scan() // name + _, tok, _, err := s.Scan() // = + if err != nil { + t.Errorf("unexpected error: %v", err) + } if tok != token.ASSIGN { t.Errorf("bad token: got %s, expected %s", tok, token.ASSIGN) } // 2nd init src2 := "[section]" - f2 := fset.AddFile("src2", fset.Base(), len(src2)) + f2, err := fset.AddFile("src2", fset.Base(), len(src2)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } s.Init(f2, []byte(src2), nil, 0) if f2.Size() != len(src2) { t.Errorf("bad file size: got %d, expected %d", f2.Size(), len(src2)) } - _, tok, _ = s.Scan() // [ + _, tok, _, err = s.Scan() // [ + if err != nil { + t.Errorf("unexpected error: %v", err) + } if tok != token.LBRACK { t.Errorf("bad token: got %s, expected %s", tok, token.LBRACK) } @@ -296,9 +334,17 @@ func TestStdErrorHandler(t *testing.T) { eh := func(pos token.Position, msg string) { list.Add(pos, msg) } var s Scanner - s.Init(fset.AddFile("File1", fset.Base(), len(src)), []byte(src), eh, 0) + file, err := fset.AddFile("File1", fset.Base(), len(src)) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + s.Init(file, []byte(src), eh, 0) for { - if _, tok, _ := s.Scan(); tok == token.EOF { + _, tok, _, err := s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if tok == token.EOF { break } } @@ -339,12 +385,25 @@ func checkError(t *testing.T, src string, tok token.Token, pos int, err string) h.msg = msg h.pos = pos } - s.Init(fset.AddFile("", fset.Base(), len(src)), []byte(src), eh, ScanComments) + file, err2 := fset.AddFile("", fset.Base(), len(src)) + if err2 != nil { + t.Errorf("unexpected error: %v", err2) + } + s.Init(file, []byte(src), eh, ScanComments) if src[0] == '=' { - _, _, _ = s.Scan() + _, _, _, err := s.Scan() + if err != nil { + t.Errorf("unexpected error: %v", err) + } + } + _, tok0, _, err2 := s.Scan() + if err2 != nil { + t.Errorf("unexpected error: %v", err2) + } + _, tok1, _, err2 := s.Scan() + if err2 != nil { + t.Errorf("unexpected error: %v", err2) } - _, tok0, _ := s.Scan() - _, tok1, _ := s.Scan() if tok0 != tok { t.Errorf("%q: got %s, expected %s", src, tok0, tok) } @@ -366,7 +425,7 @@ func checkError(t *testing.T, src string, tok token.Token, pos int, err string) } } -var errors = []struct { +var testErrors = []struct { src string tok token.Token pos int @@ -394,7 +453,7 @@ var errors = []struct { } func TestScanErrors(t *testing.T) { - for _, e := range errors { + for _, e := range testErrors { checkError(t, e.src, e.tok, e.pos, e.err) } } @@ -402,13 +461,20 @@ func TestScanErrors(t *testing.T) { func BenchmarkScan(b *testing.B) { b.StopTimer() fset := token.NewFileSet() - file := fset.AddFile("", fset.Base(), len(source)) + file, err := fset.AddFile("", fset.Base(), len(source)) + if err != nil { + b.Fatalf("unexpected error: %v", err) + } + var s Scanner b.StartTimer() for i := b.N - 1; i >= 0; i-- { s.Init(file, source, nil, ScanComments) for { - _, tok, _ := s.Scan() + _, tok, _, err := s.Scan() + if err != nil { + b.Fatalf("unexpected error: %v", err) + } if tok == token.EOF { break } diff --git a/set.go b/set.go index 685727f..7d49cf1 100644 --- a/set.go +++ b/set.go @@ -16,8 +16,11 @@ import ( ) var ( - ErrUnsupportedType = errors.New("unsupported type") - ErrBlankUnsupported = errors.New("blank value not supported for type") + ErrUnsupportedType = errors.New("unsupported type") + ErrBlankUnsupported = errors.New("blank value not supported for type") + ErrConfigMustBePointerToStruct = errors.New("config must be a pointer to a struct") + ErrInvalidMapFieldForSection = errors.New("map field for section must have string keys and pointer-to-struct values") + ErrInvalidFieldForSection = errors.New("field for section must be a map or a struct") ) type tag struct { @@ -206,13 +209,13 @@ func newValue(sect string, vCfg reflect.Value, b := bytes.NewBuffer(nil) ge := gob.NewEncoder(b) err = ge.EncodeValue(dfltField) - if err != nil && errors.Is(err, ErrCantStoreData) { + if err != nil && errors.Is(err, ErrSyntaxWarning) { return pv, err } gd := gob.NewDecoder(bytes.NewReader(b.Bytes())) err = gd.DecodeValue(pv.Elem()) - if err != nil && errors.Is(err, ErrCantStoreData) { + if err != nil && errors.Is(err, ErrSyntaxWarning) { return pv, err } } @@ -224,12 +227,12 @@ func set(cfg interface{}, sect, sub, name string, // vPCfg := reflect.ValueOf(cfg) if vPCfg.Kind() != reflect.Ptr || vPCfg.Elem().Kind() != reflect.Struct { - panic(fmt.Errorf("config must be a pointer to a struct")) + return ErrConfigMustBePointerToStruct } vCfg := vPCfg.Elem() vSect, _ := fieldFold(vCfg, sect) if !vSect.IsValid() { - return newParseError(sect, "", "") + return newSyntaxWarning(sect, "", "") } isSubsect := vSect.Kind() == reflect.Map if subsectPass != isSubsect { @@ -240,8 +243,7 @@ func set(cfg interface{}, sect, sub, name string, if vst.Key().Kind() != reflect.String || vst.Elem().Kind() != reflect.Ptr || vst.Elem().Elem().Kind() != reflect.Struct { - panic(fmt.Errorf("map field for section must have string keys and "+ - " pointer-to-struct values: section %q", sect)) + return fmt.Errorf("%w: section %q", ErrInvalidMapFieldForSection, sect) } if vSect.IsNil() { vSect.Set(reflect.MakeMap(vst)) @@ -258,10 +260,9 @@ func set(cfg interface{}, sect, sub, name string, } vSect = pv.Elem() } else if vSect.Kind() != reflect.Struct { - panic(fmt.Errorf("field for section must be a map or a struct: "+ - "section %q", sect)) + return fmt.Errorf("%w: section %q", ErrInvalidFieldForSection, sect) } else if sub != "" { - return newParseError(sect, sub, "") + return newSyntaxWarning(sect, sub, "") } // Empty name is a special value, meaning that only the // section/subsection object is to be created, with no values set. @@ -272,9 +273,9 @@ func set(cfg interface{}, sect, sub, name string, if !vVar.IsValid() { var err error if isSubsect { - err = newParseError(sect, sub, name) + err = newSyntaxWarning(sect, sub, name) } else { - err = newParseError(sect, "", name) + err = newSyntaxWarning(sect, "", name) } return err } diff --git a/token/position.go b/token/position.go index fc45c1e..9aa68b5 100644 --- a/token/position.go +++ b/token/position.go @@ -7,18 +7,25 @@ package token import ( + "errors" "fmt" "sort" "sync" ) +var ( + ErrIllegalFileOffset = errors.New("illegal file offset") + ErrIllegalPosValue = errors.New("illegal Pos value") + ErrIllegalBaseOrValue = errors.New("illegal base or value") + ErrPosOffsetOverflow = errors.New("token.Pos offset overflow (> 2G of source code in file set)") +) + // ----------------------------------------------------------------------------- // Positions // Position describes an arbitrary source position // including the file, line, and column location. // A Position is valid if the line number is > 0. -// type Position struct { Filename string // filename, if any Offset int // offset, starting at 0 @@ -35,7 +42,6 @@ func (pos *Position) IsValid() bool { return pos.Line > 0 } // line:column valid position without file name // file invalid position with file name // - invalid position without file name -// func (pos Position) String() string { s := pos.Filename if pos.IsValid() { @@ -69,14 +75,12 @@ func (pos Position) String() string { // equivalent to comparing the respective source file offsets. If p and q // are in different files, p < q is true if the file implied by p was added // to the respective file set before the file implied by q. -// type Pos int // The zero value for Pos is NoPos; there is no file and line information // associated with it, and NoPos().IsValid() is false. NoPos is always // smaller than any other Pos value. The corresponding Position value // for NoPos is the zero value for Position. -// const NoPos Pos = 0 // IsValid returns true if the position is valid. @@ -89,7 +93,6 @@ func (p Pos) IsValid() bool { // A File is a handle for a file belonging to a FileSet. // A File has a name, size, and line offset table. -// type File struct { set *FileSet name string // file name as provided to AddFile @@ -127,7 +130,6 @@ func (f *File) LineCount() int { // AddLine adds the line offset for a new line. // The line offset must be larger than the offset for the previous line // and smaller than the file size; otherwise the line offset is ignored. -// func (f *File) AddLine(offset int) { f.set.mutex.Lock() if i := len(f.lines); (i == 0 || f.lines[i-1] < offset) && offset < f.size { @@ -143,7 +145,6 @@ func (f *File) AddLine(offset int) { // Each line offset must be larger than the offset for the previous line // and smaller than the file size; otherwise SetLines fails and returns // false. -// func (f *File) SetLines(lines []int) bool { // verify validity of lines table size := f.size @@ -197,7 +198,6 @@ type lineInfo struct { // // AddLineInfo is typically used to register alternative position // information for //line filename:line comments in source files. -// func (f *File) AddLineInfo(offset int, filename string, line int) { f.set.mutex.Lock() if i := len(f.infos); i == 0 || f.infos[i-1].Offset < offset && offset < f.size { @@ -209,31 +209,32 @@ func (f *File) AddLineInfo(offset int, filename string, line int) { // Pos returns the Pos value for the given file offset; // the offset must be <= f.Size(). // f.Pos(f.Offset(p)) == p. -// -func (f *File) Pos(offset int) Pos { +func (f *File) Pos(offset int) (Pos, error) { if offset > f.size { - panic("illegal file offset") + return 0, ErrIllegalFileOffset } - return Pos(f.base + offset) + return Pos(f.base + offset), nil } // Offset returns the offset for the given file position p; // p must be a valid Pos value in that file. // f.Offset(f.Pos(offset)) == offset. -// -func (f *File) Offset(p Pos) int { +func (f *File) Offset(p Pos) (int, error) { if int(p) < f.base || int(p) > f.base+f.size { - panic("illegal Pos value") + return 0, ErrIllegalPosValue } - return int(p) - f.base + return int(p) - f.base, nil } // Line returns the line number for the given file position p; // p must be a Pos value in that file or NoPos. -// -func (f *File) Line(p Pos) int { +func (f *File) Line(p Pos) (int, error) { // TODO(gri) this can be implemented much more efficiently - return f.Position(p).Line + position, err := f.Position(p) + if err != nil { + return 0, err + } + return position.Line, nil } func searchLineInfos(a []lineInfo, x int) int { @@ -268,15 +269,14 @@ func (f *File) position(p Pos) (pos Position) { // Position returns the Position value for the given file position p; // p must be a Pos value in that file or NoPos. -// -func (f *File) Position(p Pos) (pos Position) { +func (f *File) Position(p Pos) (Position, error) { if p != NoPos { if int(p) < f.base || int(p) > f.base+f.size { - panic("illegal Pos value") + return Position{}, ErrIllegalPosValue } - pos = f.position(p) + return f.position(p), nil } - return + return Position{}, nil } // ----------------------------------------------------------------------------- @@ -285,7 +285,6 @@ func (f *File) Position(p Pos) (pos Position) { // A FileSet represents a set of source files. // Methods of file sets are synchronized; multiple goroutines // may invoke them concurrently. -// type FileSet struct { mutex sync.RWMutex // protects the file set base int // base offset for the next file @@ -302,7 +301,6 @@ func NewFileSet() *FileSet { // Base returns the minimum base offset that must be provided to // AddFile when adding the next file. -// func (s *FileSet) Base() int { s.mutex.RLock() b := s.base @@ -325,29 +323,27 @@ func (s *FileSet) Base() int { // with offs in the range [0, size] and thus p in the range [base, base+size]. // For convenience, File.Pos may be used to create file-specific position // values from a file offset. -// -func (s *FileSet) AddFile(filename string, base, size int) *File { +func (s *FileSet) AddFile(filename string, base, size int) (*File, error) { s.mutex.Lock() defer s.mutex.Unlock() if base < s.base || size < 0 { - panic("illegal base or size") + return nil, ErrIllegalBaseOrValue } // base >= s.base && size >= 0 f := &File{s, filename, base, size, []int{0}, nil} base += size + 1 // +1 because EOF also has a position if base < 0 { - panic("token.Pos offset overflow (> 2G of source code in file set)") + return nil, ErrPosOffsetOverflow } // add the file to the file set s.base = base s.files = append(s.files, f) s.last = f - return f + return f, nil } // Iterate calls f for the files in the file set in the order they were added // until f returns false. -// func (s *FileSet) Iterate(f func(*File) bool) { for i := 0; ; i++ { var file *File @@ -386,7 +382,6 @@ func (s *FileSet) file(p Pos) *File { // File returns the file that contains the position p. // If no such file is found (for instance for p == NoPos), // the result is nil. -// func (s *FileSet) File(p Pos) (f *File) { if p != NoPos { s.mutex.RLock() diff --git a/token/position_test.go b/token/position_test.go index 160107d..56b7cc0 100644 --- a/token/position_test.go +++ b/token/position_test.go @@ -63,14 +63,28 @@ func linecol(lines []int, offs int) (int, int) { func verifyPositions(t *testing.T, fset *FileSet, f *File, lines []int) { for offs := 0; offs < f.Size(); offs++ { - p := f.Pos(offs) - offs2 := f.Offset(p) + p, err := f.Pos(offs) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + offs2, err := f.Offset(p) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if offs2 != offs { t.Errorf("%s, Offset: expected offset %d; got %d", f.Name(), offs, offs2) } line, col := linecol(lines, offs) msg := fmt.Sprintf("%s (offs = %d, p = %d)", f.Name(), offs, p) - checkPos(t, msg, f.Position(f.Pos(offs)), Position{f.Name(), offs, line, col}) + pos, err := f.Pos(offs) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + position, err := f.Position(pos) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + checkPos(t, msg, position, Position{f.Name(), offs, line, col}) checkPos(t, msg, fset.Position(p), Position{f.Name(), offs, line, col}) } } @@ -95,14 +109,21 @@ func TestPositions(t *testing.T) { } // add file and verify name and size - f := fset.AddFile(test.filename, fset.Base()+delta, test.size) + f, err := fset.AddFile(test.filename, fset.Base()+delta, test.size) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if f.Name() != test.filename { t.Errorf("expected filename %q; got %q", test.filename, f.Name()) } if f.Size() != test.size { t.Errorf("%s: expected file size %d; got %d", f.Name(), test.size, f.Size()) } - if fset.File(f.Pos(0)) != f { + pos, err := f.Pos(0) + if err != nil { + t.Errorf("unexpected error %v", err) + } + if fset.File(pos) != f { t.Errorf("%s: f.Pos(0) was not found in f", f.Name()) } @@ -145,7 +166,10 @@ func TestPositions(t *testing.T) { func TestLineInfo(t *testing.T) { fset := NewFileSet() - f := fset.AddFile("foo", fset.Base(), 500) + f, err := fset.AddFile("foo", fset.Base(), 500) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } lines := []int{0, 42, 77, 100, 210, 220, 277, 300, 333, 401} // add lines individually and provide alternative line information for _, offs := range lines { @@ -154,10 +178,21 @@ func TestLineInfo(t *testing.T) { } // verify positions for all offsets for offs := 0; offs <= f.Size(); offs++ { - p := f.Pos(offs) + p, err := f.Pos(offs) + if err != nil { + t.Errorf("unexpected error: %v", err) + } _, col := linecol(lines, offs) msg := fmt.Sprintf("%s (offs = %d, p = %d)", f.Name(), offs, p) - checkPos(t, msg, f.Position(f.Pos(offs)), Position{"bar", offs, 42, col}) + pos, err := f.Pos(offs) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + position, err := f.Position(pos) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + checkPos(t, msg, position, Position{"bar", offs, 42, col}) checkPos(t, msg, fset.Position(p), Position{"bar", offs, 42, col}) } } diff --git a/token/serialize_test.go b/token/serialize_test.go index 4e925ad..f3b7c8f 100644 --- a/token/serialize_test.go +++ b/token/serialize_test.go @@ -95,7 +95,10 @@ func TestSerialization(t *testing.T) { checkSerialize(t, p) // add some files for i := 0; i < 10; i++ { - f := p.AddFile(fmt.Sprintf("file%d", i), p.Base()+i, i*100) + f, err := p.AddFile(fmt.Sprintf("file%d", i), p.Base()+i, i*100) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } checkSerialize(t, p) // add some lines and alternative file infos line := 1000 diff --git a/token/token.go b/token/token.go index b3c7c83..102df38 100644 --- a/token/token.go +++ b/token/token.go @@ -7,7 +7,6 @@ // // Note that the API for the token package may change to accommodate new // features or implementation changes in gcfg. -// package token import "strconv" @@ -58,7 +57,6 @@ var tokens = [...]string{ // sequence (e.g., for the token ASSIGN, the string is "="). For all other // tokens the string corresponds to the token constant name (e.g. for the // token IDENT, the string is "IDENT"). -// func (tok Token) String() string { s := "" if 0 <= tok && tok < Token(len(tokens)) { @@ -74,10 +72,8 @@ func (tok Token) String() string { // IsLiteral returns true for tokens corresponding to identifiers // and basic type literals; it returns false otherwise. -// func (tok Token) IsLiteral() bool { return literal_beg < tok && tok < literal_end } // IsOperator returns true for tokens corresponding to operators and // delimiters; it returns false otherwise. -// func (tok Token) IsOperator() bool { return operator_beg < tok && tok < operator_end } diff --git a/types/int.go b/types/int.go index af7e75c..61c101c 100644 --- a/types/int.go +++ b/types/int.go @@ -1,10 +1,16 @@ package types import ( + "errors" "fmt" "strings" ) +var ( + ErrAmbiguousInt = fmt.Errorf("ambiguous integer value; must include '0' prefix") + ErrUnsupportedMode = errors.New("unsupported mode") +) + // An IntMode is a mode for parsing integer values, representing a set of // accepted bases. type IntMode uint8 @@ -31,8 +37,6 @@ func (m IntMode) String() string { return "IntMode(" + strings.Join(modes, "|") + ")" } -var errIntAmbig = fmt.Errorf("ambiguous integer value; must include '0' prefix") - func prefix0(val string) bool { return strings.HasPrefix(val, "0") || strings.HasPrefix(val, "-0") } @@ -76,11 +80,11 @@ func ParseInt(intptr interface{}, val string, mode IntMode) error { if prefix0(val) { verb = 'v' } else { - return errIntAmbig + return ErrAmbiguousInt } } if verb == 0 { - panic("unsupported mode") + return ErrUnsupportedMode } return ScanFully(intptr, val, verb) }