From 3b4c063baeb8e72a0cf00a891fb2f631fa68f611 Mon Sep 17 00:00:00 2001 From: ionous Date: Fri, 19 Jul 2024 19:22:34 -0700 Subject: [PATCH] package charm: improve error reporting removes "EndpointError" and "Parse" in favor of a Parser object that can be interrogated for status and errors. the package level ParseEof method still exists, but its error reporting is improved. --- charm/charmEnd.go | 37 ++++++---- charm/charmParse.go | 136 ++++++++++++++++++------------------ charm/charmRequires_test.go | 24 ++++--- charmed/charmedNum_test.go | 41 ++++++----- decode/decode.go | 14 +--- token/sig_test.go | 12 +++- token/token_test.go | 17 +++-- 7 files changed, 155 insertions(+), 126 deletions(-) diff --git a/charm/charmEnd.go b/charm/charmEnd.go index f0f0879..0ffcb5d 100644 --- a/charm/charmEnd.go +++ b/charm/charmEnd.go @@ -5,46 +5,59 @@ import ( "fmt" ) -// a next state indicating an error that caused the state machine to exit +// returned from Parser +type UnhandledRune rune + +func (u UnhandledRune) Error() string { + return fmt.Sprintf("unhanded rune %v", rune(u)) +} + +// Used by states to wrap an error in a Terminal state. +// This is the only way for states to return an error. +// To stop processing, but return no error: see Finished() func Error(e error) State { return Terminal{err: e} } -// a next state indicating an expected termination. -// ( for example, to absorb runes.Eof and end a state gracefully. -// | whereas returning nil from a state would consider the Eof unhandled -// | and trigger attempts by chained states to handle the Eof themselves. ) +// A next state indicating an expected termination. +// This is used to absorb runes.Eof and end a state gracefully because +// returning nil on Eof will trigger attempts by chained states to handle the Eof themselves. func Finished() State { - return Terminal{err: errFinished} + return Terminal{err: ErrFinished} } -var errFinished = errors.New("finished") +// this provided an alternative to the terminal state holding nil. +// ( to avoid testing its error when dereferencing ) +var ErrFinished = errors.New("finished") // acts as both an error and a state type Terminal struct { err error } +// true if this was an expected termination. +// see also: the package level Finished(). func (e Terminal) Finished() bool { - return e.err == errFinished + return e.err == ErrFinished } -// returns itself forever +// returns itself forever. func (e Terminal) NewRune(r rune) (ret State) { return e } -// implements string for printing states +// implements string for printing states. func (e Terminal) String() string { return fmt.Sprintf("terminal state: %s", e.Error()) } -// access the underlying error +// access the underlying error. func (e Terminal) Unwrap() error { return e.err } -// terminal implements error +// terminal implements error. +// returns the string of the wrapped error. func (e Terminal) Error() string { return e.err.Error() } diff --git a/charm/charmParse.go b/charm/charmParse.go index b38b44c..ca9ddb2 100644 --- a/charm/charmParse.go +++ b/charm/charmParse.go @@ -1,7 +1,6 @@ package charm import ( - "errors" "fmt" "io" "strings" @@ -9,54 +8,41 @@ import ( const Eof = rune(-1) -// Parse sends each rune of string to the passed state chart, -// Returns the error underlying error states, -// or the last returned state if there was no error. -func Parse(str string, first State) (ret State, err error) { - return innerParse(first, strings.NewReader(str)) +// utility function which creates a string reader, +// a parser, and calls Parser.ParseEof() +func ParseEof(str string, first State) error { + p := MakeParser(strings.NewReader(str)) + return p.ParseEof(first) } -func Read(in io.RuneReader, first State) (err error) { - _, err = innerParse(first, in) - return +type Parser struct { + in io.RuneReader + err error + ofs int } -func innerParse(first State, in io.RuneReader) (ret State, err error) { - try := first - for i := 0; ; i++ { - if r, _, e := in.ReadRune(); e != nil { - if e != io.EOF { - err = errors.Join(e, EndpointError{r, in, i, try}) - } - break - } else { - if next := try.NewRune(r); next == nil { - // no states left to parse remaining input - e := errors.New("unhandled rune") - err = errors.Join(e, EndpointError{r, in, i, try}) - break - } else if es, ok := next.(Terminal); ok { - err = errors.Join(es.err, EndpointError{r, in, i, try}) - break - } else { - try = next - } - } - } - if err == nil { - ret = try - } - return +func MakeParser(in io.RuneReader) Parser { + return Parser{in: in} +} + +func (p *Parser) Error() error { + return p.err } -// on error, provide a bit of the input remaining -// so that the user has an idea of where the error occurred -func errContext(r rune, in io.RuneReader) (ret string) { +// number of runes read from the input +func (p *Parser) Offset() int { + return p.ofs +} + +// consumes ~25 of the remaining runes for error reporting +func (p *Parser) Remaining() string { const size = 25 var b strings.Builder - b.WriteRune(r) + if r, ok := p.err.(UnhandledRune); ok { + b.WriteRune(rune(r)) + } for i := 0; i < size; i++ { - if r, _, e := in.ReadRune(); e != nil { + if r, _, e := p.in.ReadRune(); e != nil { break } else { b.WriteRune(r) @@ -65,36 +51,52 @@ func errContext(r rune, in io.RuneReader) (ret string) { return b.String() } -// ParseEof sends each rune of string to the passed state chart; -// after its done with the string, it sends an eof(-1) to flush any remaining data. -// see also Parse() which does not send the eof. -func ParseEof(str string, first State) (err error) { - if last, e := innerParse(first, strings.NewReader(str)); e != nil { - err = e - } else if last != nil { - if fini := last.NewRune(Eof); fini != nil { - if es, ok := fini.(Terminal); !ok || !es.Finished() { - err = fmt.Errorf("%s at eof for %q", es.err, str) - } +// run Parse() and send the final state an explicit Eof rune. +// unlike parse, only returns an error if there was an error. +// this also unwraps Finished and all terminal errors, +// returning the underlying error ( if any. ) +func (p *Parser) ParseEof(first State) (err error) { + if last, e := p.Parse(first); e != io.EOF { + // return unrecognized errors as is. + if es, ok := e.(Terminal); !ok { + err = e + } else if !es.Finished() { + // honor a state if it finished ( by not returning error ) + // while unwrapping all other reported errors. + err = es.Unwrap() + } + } else if fini := last.NewRune(Eof); fini != nil { + // if there's *still* a state and its not a terminal state... report that + if es, ok := fini.(Terminal); !ok { + err = fmt.Errorf("unfinished states remain after end of file %s", fini) + } else if !es.Finished() { + // otherwise return the wrapped error + err = es.Unwrap() } } return } -// ended before the whole input was parsed. -type EndpointError struct { - r rune - in io.RuneReader - end int - last State -} - -// index of the failure point in the input -func (e EndpointError) End() int { - return e.end -} - -func (e EndpointError) Error() (ret string) { - sink := errContext(e.r, e.in) - return fmt.Sprintf("%q (%q ended at index %d)", sink, StateName(e.last), e.end) +// always returns an error, and the final state. +// ex. if the last rune was unhandled, then this returns an +// UnhandledRune error and the state that failed to handle it. +func (p *Parser) Parse(first State) (ret State, err error) { + try := first + for { + if r, _, e := p.in.ReadRune(); e != nil { + err = e // ex. io.Eof + break + } else if next := try.NewRune(r); next == nil { + err = UnhandledRune(r) + break + } else if es, ok := next.(Terminal); ok { + err = es // a wrapped error: ex. ErrFinished or otherwise. + break + } else { + try = next // keep going. + p.ofs++ + } + } + ret = try + return } diff --git a/charm/charmRequires_test.go b/charm/charmRequires_test.go index b980b6e..2ab0287 100644 --- a/charm/charmRequires_test.go +++ b/charm/charmRequires_test.go @@ -1,38 +1,40 @@ -package charm +package charm_test import ( "errors" "fmt" "strings" "testing" + + "github.com/ionous/tell/charm" ) func TestRequires(t *testing.T) { isSpace := func(r rune) bool { return r == ' ' } // index of the fail point, or -1 if success is expected - count := func(failPoint int, str string, style State) (err error) { - var ep EndpointError - if e := ParseEof(str, style); e == nil && failPoint != -1 { + count := func(failPoint int, str string, style charm.State) (err error) { + p := charm.MakeParser(strings.NewReader(str)) + if e := p.ParseEof(style); e == nil && failPoint != -1 { err = errors.New("unexpected success") - } else if !errors.As(e, &ep) { - err = e - } else if at := ep.End(); at != failPoint { + } else if failPoint == -1 { + err = e // expected success; if err is not nil caller will fail. + } else if at := p.Offset(); at != failPoint { // 0 means okay, -1 incomplete, >0 the one-index of the failure point. err = fmt.Errorf("%s len: %d", str, at) } return } - if e := count(0, "a", AtleastOne(isSpace)); e != nil { + if e := count(0, "a", charm.AtleastOne(isSpace)); e != nil { t.Fatal(e) } - if e := count(0, "a", Optional(isSpace)); e != nil { + if e := count(0, "a", charm.Optional(isSpace)); e != nil { t.Fatal(e) } - if e := count(-1, strings.Repeat(" ", 5), Optional(isSpace)); e != nil { + if e := count(-1, strings.Repeat(" ", 5), charm.Optional(isSpace)); e != nil { t.Fatal(e) } - if e := count(3, strings.Repeat(" ", 3)+"x", Optional(isSpace)); e != nil { + if e := count(3, strings.Repeat(" ", 3)+"x", charm.Optional(isSpace)); e != nil { t.Fatal(e) } } diff --git a/charmed/charmedNum_test.go b/charmed/charmedNum_test.go index 7faf90f..054fde5 100644 --- a/charmed/charmedNum_test.go +++ b/charmed/charmedNum_test.go @@ -1,8 +1,8 @@ package charmed import ( - "errors" "math" + "strings" "testing" "github.com/ionous/tell/charm" @@ -12,24 +12,29 @@ func TestNum(t *testing.T) { var NaN = math.NaN() // returns point of failure run := func(str string) (val float64, err error) { - var p NumParser - if e := charm.ParseEof(str, p.Decode()); e != nil { + var num NumParser + // use the make parser version to gain access to the offset + p := charm.MakeParser(strings.NewReader(str)) + if e := p.ParseEof(num.Decode()); e != nil { val = NaN - err = e - } else if v, e := p.GetFloat(); e != nil { - err = e + err = endpointError{e, p.Offset()} + } else if v, e := num.GetFloat(); e != nil { + err = e // all of the input was okay, but we couldn't make a float of it. } else { val = v } return } tests := []struct { - input string - endpoint int // 0 means okay, -1 incomplete, >0 the one-index of the failure point. + input string + // 0 means success is expected, and the value contains the parsed result + // -1 means we expect the input to run out before parsing is finished. + // >0 the one-index of an expected failure point + endpoint int value float64 }{ // bad decimals - {"0.", -1, NaN}, + {"0.", -1, NaN}, // it parses just fine, but GetFloat() will error with unknown number {".0", 1, NaN}, // floats {"0.0", 0, 0}, @@ -83,15 +88,12 @@ func TestNum(t *testing.T) { } else { // error returned, check the expected error if test.endpoint == 0 { - t.Fatal("expected success", e) + t.Fatal("expected success but received an error", e) break } else if test.endpoint > 0 { - var ep charm.EndpointError - if !errors.As(e, &ep) { - t.Fatal("unexpected error", e) - break - } else if ep.End() != test.endpoint-1 { - t.Fatal("mismatched endpoint at", e) + // ensure our expected fail point is correct + if ep := e.(endpointError); ep.pos != test.endpoint-1 { + t.Fatal("mismatched endpoint at", ep.error) break } } @@ -99,3 +101,10 @@ func TestNum(t *testing.T) { } } } + +// backwards compatibility for tests +// reports the offset of failure ( if any ) when parsing a number +type endpointError struct { + error + pos int +} diff --git a/decode/decode.go b/decode/decode.go index 1143a43..f690cf8 100644 --- a/decode/decode.go +++ b/decode/decode.go @@ -41,19 +41,11 @@ func (d *Decoder) Decode(src io.RuneReader) (ret any, err error) { d.decodeDoc(), // tbd: wrap with charmed.UnhandledError()? why/why not. charmed.DecodePos(&y, &x), ) - if e := charm.Read(src, run); e != nil { + p := charm.MakeParser(src) + if e := p.ParseEof(run); e != nil { err = ErrorAt(y, x, e) } else { - // send eof - if next := charm.RunState(runes.Eof, run); next != nil { - // if there was a next state, and it was an error - // other than "finished okay", generate an error. - if es, ok := next.(charm.Terminal); ok && !es.Finished() { - err = ErrorAt(y, x, es.Unwrap()) - } else { - ret, err = d.out.finalizeAll() - } - } + ret, err = d.out.finalizeAll() } return } diff --git a/token/sig_test.go b/token/sig_test.go index 788ff69..e8d5202 100644 --- a/token/sig_test.go +++ b/token/sig_test.go @@ -3,17 +3,25 @@ package token_test import ( "errors" "fmt" + "io" + "strings" "testing" "github.com/ionous/tell/charm" "github.com/ionous/tell/token" ) +// always returns an error; io.EOF means all the input was consumed. +func parseString(str string, state charm.State) (err error) { + p := charm.MakeParser(strings.NewReader(str)) + _, err = p.Parse(state) + return +} + func TestSig(t *testing.T) { - // returns point of failure test := func(str string) (ret string, err error) { var sig token.Signature - if _, e := charm.Parse(str, sig.Decoder()); e != nil { + if e := parseString(str, sig.Decoder()); e != io.EOF { err = e } else if sig.Pending() { err = errors.New("signature pending") diff --git a/token/token_test.go b/token/token_test.go index 4c9c06f..8156684 100644 --- a/token/token_test.go +++ b/token/token_test.go @@ -3,6 +3,7 @@ package token_test import ( "errors" "fmt" + "io" "log" "reflect" "strings" @@ -55,7 +56,7 @@ func TestArray(t *testing.T) { for i, test := range tests { var pairs results run := token.NewTokenizer(&pairs) - if _, e := charm.Parse(test.str, run); e != nil { + if e := parseString(test.str, run); e != io.EOF { t.Fatal("failed test", i, e) } else if e := pairs.compare(test.expect); e != nil { t.Fatal("failed test", i, e) @@ -118,12 +119,13 @@ line // at the very least it helps to validate tokens must be separated by whitespace. var combined results run := token.NewTokenizer(&combined) + var combinedReader strings.Reader + combinedParser := charm.MakeParser(&combinedReader) - for i := 0; i < len(tests); i += 3 { + for i, whichTest := 0, 1; i < len(tests); i, whichTest = i+3, whichTest+1 { wantType := tests[i+0].(token.Type) testStr := tests[i+1].(string) wantVal := tests[i+2] - whichTest := 1 + i/3 if e := testOne(wantType, testStr, wantVal); e != nil { t.Logf("failed single %d: %s", whichTest, e) t.Fail() @@ -132,7 +134,8 @@ line if wantType == token.Comment { sep = "\n" // comments have to be ended with a newlne } - if next, e := charm.Parse(testStr+sep, run); e != nil { + combinedReader.Reset(testStr + sep) // lets keep the party going + if next, e := combinedParser.Parse(run); e != io.EOF { t.Logf("failed combine parse %d: %s", whichTest, e) t.Fail() } else { @@ -151,7 +154,7 @@ line func testOne(tokenType token.Type, testStr string, tokenValue any) (err error) { var pairs results run := token.NewTokenizer(&pairs) - if _, e := charm.Parse(testStr+"\n", run); e != nil { + if e := parseString(testStr+"\n", run); e != io.EOF { err = compare(e, tokenValue) } else if cnt := len(pairs); cnt == 0 { err = errors.New("didn't collect any tokens") @@ -198,7 +201,7 @@ func (res results) compare(expects results) (err error) { func (p result) compare(wantType token.Type, wantValue any) (err error) { if tt := p.tokenType; tt != wantType { - err = fmt.Errorf("mismatched types want: %s, have: %s", wantType, tt) + err = fmt.Errorf("mismatched types\nwant: %s\nhave: %s", wantType, tt) } else { err = compare(p.tokenValue, wantValue) } @@ -208,7 +211,7 @@ func (p result) compare(wantType token.Type, wantValue any) (err error) { func compare(have any, want any) (err error) { if haveErr, ok := have.(error); !ok { if !reflect.DeepEqual(have, want) { - err = fmt.Errorf("mismatched want: %v(%T) have: %v(%T)", want, want, have, have) + err = fmt.Errorf("mismatched values\nwant: %v(%T)\nhave: %v(%T)", want, want, have, have) } } else { if expectErr, ok := want.(error); !ok {