Skip to content

Commit

Permalink
Replace panics with errors
Browse files Browse the repository at this point in the history 
Signed-off-by: Paulo Gomes <[email protected]>
  • Loading branch information
@pjbgf
pjbgf committed Aug 12, 2024
1 parent 633fd26 commit 1b398f6
Show file tree
Hide file tree
Showing 14 changed files with 436 additions and 205 deletions.
97 changes: 47 additions & 50 deletions doc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -52,11 +52,11 @@
// "default-<sectionname>" (or by setting values in the corresponding struct
// field "Default_<sectionname>").
//
// 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
Expand Down Expand Up @@ -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
Expand All @@ -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"
31 changes: 17 additions & 14 deletions errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
17 changes: 17 additions & 0 deletions errors_test.go
View file
Original file line number Diff line number Diff line change
@@ -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
})
}
}
84 changes: 65 additions & 19 deletions read.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -26,7 +26,7 @@ func unquote(s string) string {
esc = false
continue
}
panic("invalid escape sequence")
return "", ErrMissingEscapeSequence
}
switch c {
case '"':
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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()
}
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}

Expand All @@ -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)
}
Loading

0 comments on commit 1b398f6

Please sign in to comment.