Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(parser): improve error handling and reporting #880

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions pkg/parser/attributes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 16,
},
Elements: []interface{}{
&types.ImageBlock{
Location: &types.Location{
Expand All @@ -40,6 +44,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 17,
},
Elements: []interface{}{
&types.ImageBlock{
Location: &types.Location{
Expand All @@ -58,6 +66,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ , , ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 21,
},
Elements: []interface{}{
&types.ImageBlock{
Location: &types.Location{
Expand All @@ -76,6 +88,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ , 200, ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 24,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -97,6 +113,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here"]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 30,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -118,6 +138,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["The Foo\"Bar\" here"]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 37,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -139,6 +163,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png['The Foo\'Bar\' here']`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 37,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -160,6 +188,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["The Foo\Bar here"]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 34,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -181,6 +213,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png['The Foo\Bar here']`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 34,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -202,6 +238,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here", height=100]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 42,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -226,6 +266,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here", 1, 2, height=100]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 48,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -249,6 +293,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png["Quoted, Here", 1, 2, height=100, test1=123 ,test2 = second test ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 81,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand All @@ -274,6 +322,10 @@ var _ = Describe("attributes", func() {
source := "image::foo.png[\"Quoted, Here\", 1, 2, height=100, test1=123 ,test2 = second \"test\" ]"
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 83,
},
Elements: []interface{}{
&types.ImageBlock{
Attributes: types.Attributes{
Expand Down Expand Up @@ -305,6 +357,10 @@ var _ = Describe("attributes", func() {
source := `image::foo.png[ "This \Backslash 2Spaced End Space " ]`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 55,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand Down Expand Up @@ -689,6 +745,9 @@ var _ = DescribeTable("invalid block attributes",

func(source string) {
// given
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
content := strings.NewReader(source + "\n")
// when parsing only (ie, no substitution applied)
_, err := parser.ParseReader("", content, parser.Entrypoint("BlockAttributes"))
Expand Down
39 changes: 36 additions & 3 deletions pkg/parser/blank_line_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ var _ = Describe("blank lines", func() {
second paragraph`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 16,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -27,11 +31,19 @@ second paragraph`
},
},
{
Position: types.Position{
Start: 16,
End: 18,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 18,
End: 34,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -53,6 +65,10 @@ second paragraph
`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 16,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -62,21 +78,37 @@ second paragraph
},
},
{
Position: types.Position{
Start: 16,
End: 20,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 20,
End: 21,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 21,
End: 24,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 24,
End: 41,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand All @@ -95,10 +127,11 @@ second paragraph
`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 11,
},
Elements: []interface{}{
// types.Attributes{
// types.AttrTitle: "ignored",
// },
&types.BlankLine{},
},
},
Expand Down
4 changes: 4 additions & 0 deletions pkg/parser/delimited_block_example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -51,6 +52,9 @@ some *example* content
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `====
.foo
====`
Expand Down
15 changes: 13 additions & 2 deletions pkg/parser/delimited_block_listing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -1161,9 +1162,19 @@ and <more text> on the +
})

It("should fail when substitution is unknown", func() {
logs, reset := ConfigureLogger(log.DebugLevel) //, DiscardStdout)
defer reset()
s := strings.ReplaceAll(source, "$SUBS", "unknown")
_, err := ParseDocument(s)
Expect(err).To(HaveOccurred())
expected := &types.Document{
Elements: []interface{}{
&types.AttributeDeclaration{
Name: "github-url",
Value: string("https://github.com"),
},
},
}
Expect(ParseDocument(s)).To(MatchDocument(expected))
Expect(logs).To(ContainJSONLog(log.ErrorLevel, 33, 183, "unsupported kind of substitution: 'unknown'"))
})
})
})
Expand Down
12 changes: 12 additions & 0 deletions pkg/parser/delimited_block_passthrough_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ _foo_
another paragraph`
expected := []types.DocumentFragment{
{
Position: types.Position{
Start: 0,
End: 19,
},
Elements: []interface{}{
&types.Paragraph{
Attributes: types.Attributes{
Expand All @@ -35,11 +39,19 @@ another paragraph`
},
},
{
Position: types.Position{
Start: 19,
End: 20,
},
Elements: []interface{}{
&types.BlankLine{},
},
},
{
Position: types.Position{
Start: 20,
End: 37,
},
Elements: []interface{}{
&types.Paragraph{
Elements: []interface{}{
Expand Down
4 changes: 4 additions & 0 deletions pkg/parser/delimited_block_quote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package parser_test
import (
"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -154,6 +155,9 @@ ____
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `[quote]
____
.foo
Expand Down
7 changes: 7 additions & 0 deletions pkg/parser/delimited_block_sidebar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package parser_test
import (
"github.com/bytesparadise/libasciidoc/pkg/types"
. "github.com/bytesparadise/libasciidoc/testsupport"
log "github.com/sirupsen/logrus"

. "github.com/onsi/ginkgo" //nolint golint
. "github.com/onsi/gomega" //nolint golint
Expand Down Expand Up @@ -98,6 +99,9 @@ bar
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `
****
.foo
Expand Down Expand Up @@ -198,6 +202,9 @@ bar
})

It("with single line starting with a dot", func() {
// do not show parse errors in the logs for this test
_, reset := ConfigureLogger(log.FatalLevel)
defer reset()
source := `
****
.foo
Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/document_processing_aggregate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ func aggregate(ctx *ParseContext, fragmentStream <-chan types.DocumentFragment)
var toc *types.TableOfContents
for f := range fragmentStream {
if f.Error != nil {
log.Debugf("skipping aggregation because of fragment with error: %v", f.Error)
return nil, nil, f.Error
log.WithField("start_offset", f.Position.Start).WithField("end_offset", f.Position.End).Error(f.Error)
continue
}
for _, element := range f.Elements {
switch e := element.(type) {
Expand Down
Loading