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

yaml: typeError implements PrettyPrinter interface #280

Merged
merged 2 commits into from May 8, 2022

Conversation

braydonk
Copy link
Contributor

@braydonk braydonk commented Feb 9, 2022

When unmarshalling YAML and encountering an error in decoding that
results in a yaml.typeError, the error can't have the same nice
formatting because it doesn't implement the errors.PrettyPrinter
interface. This adds a field to the yaml.typeError struct that
specifies a token which is used in the implementation of the
PrettyPrinter interface. The tests were updated to instead match on
expected message substrings instead of full equality.

For example, when getting an error like this:
cannot unmarshal uint64 into Go struct field T.B of type time.Duration
This PR allows for output like this:

[1:4] cannot unmarshal uint64 into Go struct field T.B of type time.Duration
>  1 | b: 10
          ^

When unmarshalling YAML and encountering an error in decoded that
results in a `yaml.typeError`, the error can't have the same nice
formatting because it doesn't implement the `errors.PrettyPrinter`
interface. This adds a field to the `yaml.typeError` struct that
specifies a token which is used in the implementation of the
PrettyPrinter interface. The tests were updated to instead match on
expected message substrings instead of full equality.
@@ -67,10 +67,10 @@ type wrapError struct {
frame xerrors.Frame
}

type myprinter struct {
type FormatErrorPrinter struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seemed like a nicer solution than declaring another identical type in decode.go for the printer struct, but I don't have to do it this way if that's preferred.

decode.go Outdated
@@ -417,8 +419,31 @@ func (e *typeError) Error() string {
return fmt.Sprintf("cannot unmarshal %s into Go value of type %s", e.srcType, e.dstType)
}

func errTypeMismatch(dstType, srcType reflect.Type) *typeError {
return &typeError{dstType: dstType, srcType: srcType}
func (e *typeError) PrettyPrint(p xerrors.Printer, colored, inclSource bool) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd also considered moving typeError to the internal/errors package, which is where the other PrettyPrinter errors are. I left it here for now, but am interested if you would prefer this or the alternative.

Copy link
Owner

Choose a reason for hiding this comment

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

As you say, it seems better to move to internal/errors so that the processing can be standardized. Could you please ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I can!

@goccy
Copy link
Owner

goccy commented May 1, 2022

Thank you for the contribution and very nice feature !!

The typeError previously located in decode.go now implements
PrettyPrinter, so it's been moved to internal/errors to be colocated
with the other PrettyPrinter errors.
@goccy
Copy link
Owner

goccy commented May 8, 2022

LGTM !!

@goccy goccy merged commit a353932 into goccy:master May 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants