Skip to content

fix: don't panic when capturing into struct types that implement Capture - #466

Open
Pastalikek65 wants to merge 1 commit into
alecthomas:masterfrom
Pastalikek65:fix-struct-capture
Open

fix: don't panic when capturing into struct types that implement Capture#466
Pastalikek65 wants to merge 1 commit into
alecthomas:masterfrom
Pastalikek65:fix-struct-capture

Conversation

@Pastalikek65

Copy link
Copy Markdown

Fixes #140.

Problem

A struct that implements Capture (or encoding.TextUnmarshaler) and is captured with @@ panicked at runtime:

panic: interface conversion: interface {} is parser.Box, not string

setField built the values []string argument from converted reflect values of the already-parsed struct, so v.Interface().(string) was a hard type assertion on a Box struct value.

Fix

  • Capture is now fed from the raw token values, which is what the Capture contract actually expects (values []string): node.Parse collects token values in order, so values[0] is the captured token text.
  • When the parsed field value is assignable to the field (e.g. a @@ struct that also implements Capture), it is pre-assigned so Pos/other already-parsed fields are kept.
  • The encoding.TextUnmarshaler path was equally broken for struct types; it now unmarshals the joined token text (and keeps the parsed struct value).
  • Slice captures keep passing plain string values unchanged.

Example (from the issue)

parser.ParseString("test", "abc::cdef.abc", &Boxes{})
// Boxes.Boxes.Val == "abc::cdef.abc"  (previously: panic)

Two new tests: TestStructCaptureViaExpression, TestStructCaptureWithTextUnmarshaler. The existing TestBoxedCapture/TestCaptureOnSliceElements continue to pass.

Capture and encoding.TextUnmarshaler types used as `@@` fields were fed
reflect values of the already-parsed struct in setField, which then
panicked in the `v.Interface().(string)` assertion (e.g.
'interface conversion: interface {} is Box, not string').

- Feed Capture from the raw token values instead of converted field
  values, which is what the Capture contract expects (`values []string`).
- Pre-assign the parsed struct value when it is assignable to the field,
  so structs that also implement Capture/UnmarshalText keep their parsed
  fields (e.g. Pos).
- Slice captures keep passing plain string values through unchanged.

Fixes alecthomas#140.
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.

bug: using struct-types with Capture interface is broken

1 participant