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

Remove internal errors for protolint to parse #47

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
25 changes: 6 additions & 19 deletions parser/enum.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
package parser

import (
"fmt"

"github.com/yoheimuta/go-protoparser/internal/lexer/scanner"
"github.com/yoheimuta/go-protoparser/parser/meta"
)

type parseEnumBodyStatementErr struct {
parseEnumFieldErr error
parseEmptyStatementErr error
}

func (e *parseEnumBodyStatementErr) Error() string {
return fmt.Sprintf(
"%v:%v",
e.parseEnumFieldErr,
e.parseEmptyStatementErr,
)
}

// EnumValueOption is an option of a enumField.
type EnumValueOption struct {
OptionName string
Expand Down Expand Up @@ -211,10 +196,12 @@ func (p *Parser) parseEnumBody() (
break
}

return nil, nil, scanner.Position{}, &parseEnumBodyStatementErr{
parseEnumFieldErr: enumFieldErr,
parseEmptyStatementErr: emptyErr,
}
return nil, nil, scanner.Position{}, emptyErr
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certain internal errors get thrown on parse issues, which makes it impossible to open a switch statement on them.

I understand your need.

This PR removes those, instead returning the latest error, which seems to be the most relevant from the parse tests I ran.
As you can see, both errors are very similar. In my opinion, the second one is slightly more helpful; hence why i return that one.

I thought this composite type was necessary to include enough errors so that end-users(or rather, maintainers) can see what input token is being expected.

In terms of usability, the error message should have looked like below.

func (e *parseEnumBodyStatementErr) Error() string {
	return fmt.Sprintf(
		"expected enumField or emptyStatement and tried parsing each, "+
			"but following two errors cropped up: (1) parseEnumFieldErr`%v` (2) parseEmptyStatementErr`%v`",
		e.parseEnumFieldErr,
		e.parseEmptyStatementErr,
	)
}

expected enumField or emptyStatement and tried parsing each, but following two errors cropped up: (1) parseEnumFieldErrfound "\"option\"(Token=2, Pos=simple.proto:9:5)" but expected [=] at /GOPATH/src/github.com/yoheimuta/go-protoparser/parser/enum.go:242 (2) parseEmptyStatementErrfound "option" but expected [;]

makes it impossible to open a switch statement on them.

Is it not enough to prepare a method/interface for extracting inner errors?

type CompositeError interface {
	Errors() []error
}
func (e *parseEnumBodyStatementErr) Errors() []error {
	return []error{e.parseEnumFieldErr, e.parseEmptyStatementErr}
}


//&parseStatementErr{
// parseFieldErr: enumFieldErr,
// parseEmptyStatementErr: emptyErr,
//}
}

p.MaybeScanInlineComment(stmt)
Expand Down
28 changes: 10 additions & 18 deletions parser/extend.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
package parser

import (
"fmt"

"github.com/yoheimuta/go-protoparser/internal/lexer/scanner"
"github.com/yoheimuta/go-protoparser/parser/meta"
)

type parseExtendBodyStatementErr struct {
parseFieldErr error
parseEmptyStatementErr error
}

func (e *parseExtendBodyStatementErr) Error() string {
return fmt.Sprintf(
"%v:%v",
e.parseFieldErr,
e.parseEmptyStatementErr,
)
}

// Extend consists of a messageType and an extend body.
type Extend struct {
MessageType string
Expand Down Expand Up @@ -172,10 +157,17 @@ func (p *Parser) parseExtendBody() (
break
}

return nil, nil, scanner.Position{}, &parseExtendBodyStatementErr{
parseFieldErr: fieldErr,
parseEmptyStatementErr: emptyErr,
metaFieldErr := fieldErr.(*meta.Error)
return nil, nil, scanner.Position{}, &meta.Error{
Pos: metaFieldErr.Pos,
Expected: "",
Found: "",
}

//&parseStatementErr{
// parseFieldErr: fieldErr,
// parseEmptyStatementErr: emptyErr,
//}
}

p.MaybeScanInlineComment(stmt)
Expand Down
23 changes: 1 addition & 22 deletions parser/message.go
Original file line number Diff line number Diff line change
@@ -1,25 +1,10 @@
package parser

import (
"fmt"

"github.com/yoheimuta/go-protoparser/internal/lexer/scanner"
"github.com/yoheimuta/go-protoparser/parser/meta"
)

type parseMessageBodyStatementErr struct {
parseFieldErr error
parseEmptyStatementErr error
}

func (e *parseMessageBodyStatementErr) Error() string {
return fmt.Sprintf(
"%v:%v",
e.parseFieldErr,
e.parseEmptyStatementErr,
)
}

// Message consists of a message name and a message body.
type Message struct {
MessageName string
Expand Down Expand Up @@ -216,7 +201,6 @@ func (p *Parser) parseMessageBody() (
extensions.Comments = comments
stmt = extensions
default:
var ferr error
isGroup := p.peekIsGroup()
if isGroup {
groupField, groupErr := p.ParseGroupField()
Expand All @@ -225,7 +209,6 @@ func (p *Parser) parseMessageBody() (
stmt = groupField
break
}
ferr = groupErr
p.lex.UnNext()
} else {
field, fieldErr := p.ParseField()
Expand All @@ -234,7 +217,6 @@ func (p *Parser) parseMessageBody() (
stmt = field
break
}
ferr = fieldErr
p.lex.UnNext()
}

Expand All @@ -244,10 +226,7 @@ func (p *Parser) parseMessageBody() (
break
}

return nil, nil, scanner.Position{}, &parseMessageBodyStatementErr{
parseFieldErr: ferr,
parseEmptyStatementErr: emptyErr,
}
return nil, nil, scanner.Position{}, emptyErr
}

p.MaybeScanInlineComment(stmt)
Expand Down
16 changes: 1 addition & 15 deletions parser/reserved.go
Original file line number Diff line number Diff line change
@@ -1,21 +1,10 @@
package parser

import (
"fmt"

"github.com/yoheimuta/go-protoparser/internal/lexer/scanner"
"github.com/yoheimuta/go-protoparser/parser/meta"
)

type parseReservedErr struct {
parseRangesErr error
parseFieldNamesErr error
}

func (e *parseReservedErr) Error() string {
return fmt.Sprintf("%v:%v", e.parseRangesErr, e.parseFieldNamesErr)
}

// Range is a range of field numbers. End is an optional value.
type Range struct {
Begin string
Expand Down Expand Up @@ -77,10 +66,7 @@ func (p *Parser) ParseReserved() (*Reserved, error) {
return nil, fieldNames, nil
}

return nil, nil, &parseReservedErr{
parseRangesErr: err,
parseFieldNamesErr: ferr,
}
return nil, nil, ferr
}

ranges, fieldNames, err := parse()
Expand Down