Made the panic() calls (which basically indicate parser implementation bugs) more useful by referencing from where illegal calls were made.

This commit is contained in:
Maurice Makaay 2019-05-29 07:24:27 +00:00
parent 2fa5b8d0f4
commit 21f1aa597c
10 changed files with 86 additions and 55 deletions

View File

@ -16,8 +16,11 @@ import (
type Chunks []string type Chunks []string
func (l *Chunks) AddChopped(s string, chunkSize int) *parsekit.Error { func (l *Chunks) AddChopped(s string, chunkSize int) *parsekit.Error {
c, a := parsekit.C, parsekit.A
chunkOfRunes := c.MinMax(1, chunkSize, a.AnyRune)
parser := parsekit.NewParser(func(p *parsekit.ParseAPI) { parser := parsekit.NewParser(func(p *parsekit.ParseAPI) {
for p.On(parsekit.C.MinMax(1, chunkSize, parsekit.A.AnyRune)).Accept() { for p.On(chunkOfRunes).Accept() {
*l = append(*l, p.BufLiteral()) *l = append(*l, p.BufLiteral())
p.BufClear() p.BufClear()
} }

View File

@ -44,19 +44,18 @@ func (p *ParseAPI) panicWhenStoppedOrInError() {
if !p.isStoppedOrInError() { if !p.isStoppedOrInError() {
return return
} }
// No error handling, because it's private known-to-work use only.
pc, _, _, _ := runtime.Caller(1) called, _ := p.getCaller(1)
call := runtime.FuncForPC(pc) parts := strings.Split(called, ".")
pc, _, _, _ = runtime.Caller(2) calledShort := parts[len(parts)-1]
caller := runtime.FuncForPC(pc) caller, filepos := p.getCaller(2)
after := "Error()" after := "Error()"
if p.stopped { if p.stopped {
after = "Stop()" after = "Stop()"
} }
parts := strings.Split(call.Name(), ".")
name := parts[len(parts)-1] panic(fmt.Sprintf("Illegal call to ParseAPI.%s() from %s at %s: no calls allowed after ParseAPI.%s", calledShort, caller, filepos, after))
panic(fmt.Sprintf("Illegal call to ParseAPI.%s() from %s: no calls allowed after ParseAPI.%s", name, caller.Name(), after))
} }
func (p *ParseAPI) isStoppedOrInError() bool { func (p *ParseAPI) isStoppedOrInError() bool {
@ -64,13 +63,11 @@ func (p *ParseAPI) isStoppedOrInError() bool {
} }
func (p *ParseAPI) checkForLoops() { func (p *ParseAPI) checkForLoops() {
pc, file, line, _ := runtime.Caller(2) caller, filepos := p.getCaller(2)
id := fmt.Sprintf("%s:%d", file, line) if _, ok := p.loopCheck[filepos]; ok {
if _, ok := p.loopCheck[id]; ok { panic(fmt.Sprintf("Loop detected in parser in %s at %s", caller, filepos))
caller := runtime.FuncForPC(pc)
panic(fmt.Sprintf("Loop detected in parser in %s at %s", caller.Name(), id))
} }
p.loopCheck[id] = true p.loopCheck[filepos] = true
} }
// peek returns but does not advance the cursor to the next rune in the input. // peek returns but does not advance the cursor to the next rune in the input.
@ -109,3 +106,11 @@ func handleRuneError(r rune, w int) (rune, int, bool) {
} }
return r, w, true return r, w, true
} }
func (p *ParseAPI) getCaller(depth int) (string, string) {
// No error handling, because we call this method ourselves with safe depth values.
pc, file, line, _ := runtime.Caller(depth + 1)
filepos := fmt.Sprintf("%s:%d", file, line)
caller := runtime.FuncForPC(pc)
return caller.Name(), filepos
}

View File

@ -43,7 +43,7 @@ func (p *ParseAPI) On(tokenHandler TokenHandler) *ParseAPIOnAction {
// Perform the matching operation. // Perform the matching operation.
m := &TokenAPI{p: p} m := &TokenAPI{p: p}
if tokenHandler == nil { if tokenHandler == nil {
panic("internal parser error: tokenHandler argument for On() is nil") panic("ParseHandler bug: tokenHandler argument for On() is nil")
} }
ok := tokenHandler(m) ok := tokenHandler(m)

View File

@ -10,10 +10,18 @@ import "fmt"
// or the parser was stopped (using ParseAPI.Stop()). // or the parser was stopped (using ParseAPI.Stop()).
func (p *ParseAPI) Handle(parseHandler ParseHandler) bool { func (p *ParseAPI) Handle(parseHandler ParseHandler) bool {
p.panicWhenStoppedOrInError() p.panicWhenStoppedOrInError()
p.panicWhenParseHandlerNil(parseHandler)
parseHandler(p) parseHandler(p)
return !p.isStoppedOrInError() return !p.isStoppedOrInError()
} }
func (p *ParseAPI) panicWhenParseHandlerNil(parseHandler ParseHandler) {
if parseHandler == nil {
caller, filepos := p.getCaller(2)
panic(fmt.Sprintf("ParseAPI.Handle() called with nil input from %s at %s", caller, filepos))
}
}
// Expects is used to let a ParseHandler function describe what input it is // Expects is used to let a ParseHandler function describe what input it is
// expecting. This expectation is used in error messages to provide some // expecting. This expectation is used in error messages to provide some
// context to them. // context to them.
@ -41,7 +49,6 @@ func (p *ParseAPI) Stop() {
} }
// ExpectEndOfFile can be used to check if the input is at end of file. // ExpectEndOfFile can be used to check if the input is at end of file.
// Intended use:
// //
// When it finds that the end of the file was indeed reached, then the // When it finds that the end of the file was indeed reached, then the
// parser will be stopped through ParseAPI.Stop(). Otherwise unexpected // parser will be stopped through ParseAPI.Stop(). Otherwise unexpected

View File

@ -12,7 +12,7 @@ func TestGivenNilTokenHandler_WhenCallingOn_ParsekitPanics(t *testing.T) {
}) })
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { p.Execute("") }, func() { p.Execute("") },
"internal parser error: tokenHandler argument for On() is nil"}) `ParseHandler bug: tokenHandler argument for On\(\) is nil`})
} }
func TestGivenStoppedParser_WhenCallingHandle_ParsekitPanics(t *testing.T) { func TestGivenStoppedParser_WhenCallingHandle_ParsekitPanics(t *testing.T) {
@ -25,9 +25,8 @@ func TestGivenStoppedParser_WhenCallingHandle_ParsekitPanics(t *testing.T) {
}) })
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { p.Execute("") }, func() { p.Execute("") },
"Illegal call to ParseAPI.Handle() from git.makaay.nl/mauricem/go-parsekit_test." + `Illegal call to ParseAPI.Handle\(\) from .*ParsekitPanics.func.* at ` +
"TestGivenStoppedParser_WhenCallingHandle_ParsekitPanics.func2: " + `.*/parsehandler_test.go:\d+: no calls allowed after ParseAPI.Stop\(\)`})
"no calls allowed after ParseAPI.Stop()"})
} }
func TestGivenParserWithError_WhenCallingHandle_ParsekitPanics(t *testing.T) { func TestGivenParserWithError_WhenCallingHandle_ParsekitPanics(t *testing.T) {
@ -40,9 +39,8 @@ func TestGivenParserWithError_WhenCallingHandle_ParsekitPanics(t *testing.T) {
}) })
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { p.Execute("") }, func() { p.Execute("") },
"Illegal call to ParseAPI.Handle() from git.makaay.nl/mauricem/go-parsekit_test." + `Illegal call to ParseAPI\.Handle\(\) from .*ParsekitPanics\.func2 at ` +
"TestGivenParserWithError_WhenCallingHandle_ParsekitPanics.func2: " + `.*/parsehandler_test\.go:\d+: no calls allowed after ParseAPI\.Error\(\)`})
"no calls allowed after ParseAPI.Error()"})
} }
func TestGivenFilledStringBuffer_BufInterpreted_ReturnsInterpretedString(t *testing.T) { func TestGivenFilledStringBuffer_BufInterpreted_ReturnsInterpretedString(t *testing.T) {
@ -102,9 +100,7 @@ func TestGivenLoopingParserDefinition_ParserPanics(t *testing.T) {
parser := parsekit.NewParser(looper.first) parser := parsekit.NewParser(looper.first)
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { parser.Execute("Het houdt niet op, niet vanzelf") }, func() { parser.Execute("Het houdt niet op, niet vanzelf") },
"Loop detected in parser in git.makaay.nl/mauricem/go-parsekit_test." + `Loop detected in parser in .*\(\*parserWithLoop\).second at .*/parsehandler_test\.go:\d+`})
"(*parserWithLoop).second at /home/ubuntu/Projects/Parsekit/go-parsekit" +
"/parsehandler_test.go:87"})
} }
// This test incorporates an actual loop bug that I dropped on myself and // This test incorporates an actual loop bug that I dropped on myself and
@ -129,7 +125,12 @@ func TestGivenLoopingParserDefinition2_ParserPanics(t *testing.T) {
}) })
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { parser.Execute("This will end soon") }, func() { parser.Execute("This will end soon") },
"Loop detected in parser in git.makaay.nl/mauricem/go-parsekit_test." + `Loop detected in parser in .*ParserPanics.* at .*/parsehandler_test.go:\d+`})
"TestGivenLoopingParserDefinition2_ParserPanics.func1 at " + }
"/home/ubuntu/Projects/Parsekit/go-parsekit/parsehandler_test.go:125"})
func TestGivenNullHandler_HandlePanics(t *testing.T) {
parser := parsekit.NewParser(nil)
RunPanicTest(t, PanicTest{
func() { parser.Execute("") },
`ParseAPI.Handle\(\) called with nil input from .*\(\*Parser\).Execute at .*/parsekit\.go:\d+`})
} }

View File

@ -4,6 +4,7 @@ package parsekit_test
// No actual tests belong in this file. // No actual tests belong in this file.
import ( import (
"regexp"
"testing" "testing"
"git.makaay.nl/mauricem/go-parsekit" "git.makaay.nl/mauricem/go-parsekit"
@ -42,17 +43,17 @@ func RunTokenHandlerTest(t *testing.T, test TokenHandlerTest) {
type PanicTest struct { type PanicTest struct {
function func() function func()
epxected string expected string
} }
func RunPanicTest(t *testing.T, p PanicTest) { func RunPanicTest(t *testing.T, p PanicTest) {
defer func() { defer func() {
if r := recover(); r != nil { if r := recover(); r != nil {
if r != p.epxected { if !regexp.MustCompile(p.expected).MatchString(r.(string)) {
t.Errorf("Function did panic, but unexpected panic message received:\nexpected: %q\nactual: %q\n", p.epxected, r) t.Errorf("Function did panic, but unexpected panic message received:\nexpected: %q\nactual: %q\n", p.expected, r)
} }
} else { } else {
t.Errorf("Function did not panic (expected panic message: %s)", p.epxected) t.Errorf("Function did not panic (expected panic message: %s)", p.expected)
} }
}() }()
p.function() p.function()

View File

@ -86,7 +86,10 @@ type runeInfo struct {
// rune is explicitly accepted or skipped as described above. // rune is explicitly accepted or skipped as described above.
func (t *TokenAPI) NextRune() (rune, bool) { func (t *TokenAPI) NextRune() (rune, bool) {
if t.currRune != nil { if t.currRune != nil {
panic("internal Matcher error: NextRune() was called without accepting or skipping the previously read rune") caller, filepos := t.p.getCaller(1)
panic(fmt.Sprintf(
"TokenHandler bug: NextRune() was called from %s at %s "+
"without accepting or skipping the previously read rune", caller, filepos))
} }
r, w, ok := t.p.peek(t.inputOffset) r, w, ok := t.p.peek(t.inputOffset)
t.currRune = &runeInfo{r, w, ok} t.currRune = &runeInfo{r, w, ok}
@ -152,10 +155,16 @@ func (t *TokenAPI) Skip() {
func (t *TokenAPI) checkAllowedCall(name string) { func (t *TokenAPI) checkAllowedCall(name string) {
if t.currRune == nil { if t.currRune == nil {
panic(fmt.Sprintf("internal Matcher error: %s was called without a prior call to NextRune()", name)) caller, filepos := t.p.getCaller(2)
panic(fmt.Sprintf(
"TokenHandler bug: %s was called from %s at %s without a prior call to NextRune()",
name, caller, filepos))
} }
if !t.currRune.OK { if !t.currRune.OK {
panic(fmt.Sprintf("internal Matcher error: %s was called, but prior call to NextRune() did not return OK (EOF or invalid rune)", name)) caller, filepos := t.p.getCaller(2)
panic(fmt.Sprintf(
"TokenHandler bug: %s was called from %s at %s, but prior call to NextRune() "+
"did not return OK (EOF or invalid rune)", name, caller, filepos))
} }
} }
@ -169,7 +178,7 @@ func (t *TokenAPI) checkAllowedCall(name string) {
// input offset which is kept at its current position). // input offset which is kept at its current position).
func (t *TokenAPI) Merge() bool { func (t *TokenAPI) Merge() bool {
if t.parent == nil { if t.parent == nil {
panic("internal parser error: Cannot call Merge a a non-forked MatchDialog") panic("TokenHandler bug: Cannot call Merge a a non-forked MatchDialog")
} }
t.parent.input = append(t.parent.input, t.input...) t.parent.input = append(t.parent.input, t.input...)
t.parent.output = append(t.parent.output, t.output...) t.parent.output = append(t.parent.output, t.output...)

View File

@ -30,7 +30,8 @@ func TestGivenNextRuneCalled_WithoutAcceptOrSkip_NextCallToNextRunePanics(t *tes
}, "test") }, "test")
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { parser.Execute("input string") }, func() { parser.Execute("input string") },
"internal Matcher error: NextRune() was called without accepting or skipping the previously read rune"}) `TokenHandler bug: NextRune\(\) was called from .*NextCallToNextRunePanics.* ` +
`at .*/tokenhandler_test\.go:\d+ without accepting or skipping the previously read rune`})
} }
func TestGivenNextRuneNotCalled_CallToAcceptPanics(t *testing.T) { func TestGivenNextRuneNotCalled_CallToAcceptPanics(t *testing.T) {
@ -40,7 +41,8 @@ func TestGivenNextRuneNotCalled_CallToAcceptPanics(t *testing.T) {
}, "test") }, "test")
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { parser.Execute("input string") }, func() { parser.Execute("input string") },
"internal Matcher error: Accept() was called without a prior call to NextRune()"}) `TokenHandler bug: Accept\(\) was called from .*CallToAcceptPanics.* ` +
`at .*/tokenhandler_test\.go:\d+ without a prior call to NextRune\(\)`})
} }
func TestGivenNextRuneNotCalled_CallToSkipPanics(t *testing.T) { func TestGivenNextRuneNotCalled_CallToSkipPanics(t *testing.T) {
@ -50,7 +52,8 @@ func TestGivenNextRuneNotCalled_CallToSkipPanics(t *testing.T) {
}, "test") }, "test")
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { parser.Execute("input string") }, func() { parser.Execute("input string") },
"internal Matcher error: Skip() was called without a prior call to NextRune()"}) `TokenHandler bug: Skip\(\) was called from .*CallToSkipPanics.* ` +
`at .*tokenhandler_test\.go:\d+ without a prior call to NextRune\(\)`})
} }
func TestGivenNextRuneReturningNotOk_CallToAcceptPanics(t *testing.T) { func TestGivenNextRuneReturningNotOk_CallToAcceptPanics(t *testing.T) {
@ -61,7 +64,9 @@ func TestGivenNextRuneReturningNotOk_CallToAcceptPanics(t *testing.T) {
}, "test") }, "test")
RunPanicTest(t, PanicTest{ RunPanicTest(t, PanicTest{
func() { parser.Execute("\xcd") }, func() { parser.Execute("\xcd") },
"internal Matcher error: Accept() was called, but prior call to NextRune() did not return OK (EOF or invalid rune)"}) `TokenHandler bug: Accept\(\) was called from .*CallToAcceptPanics.* ` +
`at .*tokenhandler_test\.go:\d+, but prior call to NextRune\(\) did not ` +
`return OK \(EOF or invalid rune\)`})
} }
func TestGivenRootTokenAPI_CallingMergePanics(t *testing.T) { func TestGivenRootTokenAPI_CallingMergePanics(t *testing.T) {
@ -70,6 +75,6 @@ func TestGivenRootTokenAPI_CallingMergePanics(t *testing.T) {
a := parsekit.TokenAPI{} a := parsekit.TokenAPI{}
a.Merge() a.Merge()
}, },
"internal parser error: Cannot call Merge a a non-forked MatchDialog", `TokenHandler bug: Cannot call Merge a a non-forked MatchDialog`,
}) })
} }

View File

@ -94,7 +94,7 @@ func MatchRunes(expected ...rune) TokenHandler {
// creates a TokenHandler that will match any of 'g', 'h', 'i', 'j' or 'k'. // creates a TokenHandler that will match any of 'g', 'h', 'i', 'j' or 'k'.
func MatchRuneRange(start rune, end rune) TokenHandler { func MatchRuneRange(start rune, end rune) TokenHandler {
if end < start { if end < start {
panic(fmt.Sprintf("internal parser error: MatchRuneRange definition error: start %q must not be < end %q", start, end)) panic(fmt.Sprintf("TokenHandler bug: MatchRuneRange definition error: start %q must not be < end %q", start, end))
} }
return func(t *TokenAPI) bool { return func(t *TokenAPI) bool {
input, ok := t.NextRune() input, ok := t.NextRune()
@ -210,7 +210,7 @@ func MatchRep(times int, handler TokenHandler) TokenHandler {
// When more matches are possible, these will be included in the output. // When more matches are possible, these will be included in the output.
func MatchMin(min int, handler TokenHandler) TokenHandler { func MatchMin(min int, handler TokenHandler) TokenHandler {
if min < 0 { if min < 0 {
panic("internal parser error: MatchMin definition error: min must be >= 0") panic("TokenHandler bug: MatchMin definition error: min must be >= 0")
} }
return matchMinMax(min, -1, handler, "MatchMin") return matchMinMax(min, -1, handler, "MatchMin")
} }
@ -221,7 +221,7 @@ func MatchMin(min int, handler TokenHandler) TokenHandler {
// Zero matches are considered a successful match. // Zero matches are considered a successful match.
func MatchMax(max int, handler TokenHandler) TokenHandler { func MatchMax(max int, handler TokenHandler) TokenHandler {
if max < 0 { if max < 0 {
panic("internal parser error: MatchMax definition error: max must be >= 0") panic("TokenHandler bug: MatchMax definition error: max must be >= 0")
} }
return matchMinMax(0, max, handler, "MatchMax") return matchMinMax(0, max, handler, "MatchMax")
} }
@ -244,17 +244,17 @@ func MatchOneOrMore(handler TokenHandler) TokenHandler {
// inclusive. All matches will be included in the output. // inclusive. All matches will be included in the output.
func MatchMinMax(min int, max int, handler TokenHandler) TokenHandler { func MatchMinMax(min int, max int, handler TokenHandler) TokenHandler {
if max < 0 { if max < 0 {
panic("internal parser error: MatchMinMax definition error: max must be >= 0") panic("TokenHandler bug: MatchMinMax definition error: max must be >= 0")
} }
if min < 0 { if min < 0 {
panic("internal parser error: MatchMinMax definition error: min must be >= 0") panic("TokenHandler bug: MatchMinMax definition error: min must be >= 0")
} }
return matchMinMax(min, max, handler, "MatchMinMax") return matchMinMax(min, max, handler, "MatchMinMax")
} }
func matchMinMax(min int, max int, handler TokenHandler, name string) TokenHandler { func matchMinMax(min int, max int, handler TokenHandler, name string) TokenHandler {
if max >= 0 && min > max { if max >= 0 && min > max {
panic(fmt.Sprintf("internal parser error: %s definition error: max %d must not be < min %d", name, max, min)) panic(fmt.Sprintf("TokenHandler bug: %s definition error: max %d must not be < min %d", name, max, min))
} }
return func(t *TokenAPI) bool { return func(t *TokenAPI) bool {
child := t.Fork() child := t.Fork()

View File

@ -84,17 +84,17 @@ func TestCombinators(t *testing.T) {
func TestCombinatorPanics(t *testing.T) { func TestCombinatorPanics(t *testing.T) {
RunPanicTests(t, []PanicTest{ RunPanicTests(t, []PanicTest{
{func() { parsekit.C.RuneRange('z', 'a') }, {func() { parsekit.C.RuneRange('z', 'a') },
"internal parser error: MatchRuneRange definition error: start 'z' must not be < end 'a'"}, "TokenHandler bug: MatchRuneRange definition error: start 'z' must not be < end 'a'"},
{func() { parsekit.C.MinMax(-1, 1, parsekit.A.Space) }, {func() { parsekit.C.MinMax(-1, 1, parsekit.A.Space) },
"internal parser error: MatchMinMax definition error: min must be >= 0"}, "TokenHandler bug: MatchMinMax definition error: min must be >= 0"},
{func() { parsekit.C.MinMax(1, -1, parsekit.A.Space) }, {func() { parsekit.C.MinMax(1, -1, parsekit.A.Space) },
"internal parser error: MatchMinMax definition error: max must be >= 0"}, "TokenHandler bug: MatchMinMax definition error: max must be >= 0"},
{func() { parsekit.C.MinMax(10, 5, parsekit.A.Space) }, {func() { parsekit.C.MinMax(10, 5, parsekit.A.Space) },
"internal parser error: MatchMinMax definition error: max 5 must not be < min 10"}, "TokenHandler bug: MatchMinMax definition error: max 5 must not be < min 10"},
{func() { parsekit.C.Min(-10, parsekit.A.Space) }, {func() { parsekit.C.Min(-10, parsekit.A.Space) },
"internal parser error: MatchMin definition error: min must be >= 0"}, "TokenHandler bug: MatchMin definition error: min must be >= 0"},
{func() { parsekit.C.Max(-42, parsekit.A.Space) }, {func() { parsekit.C.Max(-42, parsekit.A.Space) },
"internal parser error: MatchMax definition error: max must be >= 0"}, "TokenHandler bug: MatchMax definition error: max must be >= 0"},
}) })
} }