fix: don't panic when capturing into struct types that implement Capture - #466
Open
Pastalikek65 wants to merge 1 commit into
Open
fix: don't panic when capturing into struct types that implement Capture#466Pastalikek65 wants to merge 1 commit into
Pastalikek65 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #140.
Problem
A struct that implements
Capture(orencoding.TextUnmarshaler) and is captured with@@panicked at runtime:setFieldbuilt thevalues []stringargument from converted reflect values of the already-parsed struct, sov.Interface().(string)was a hard type assertion on aBoxstruct value.Fix
Captureis now fed from the raw token values, which is what theCapturecontract actually expects (values []string):node.Parsecollects token values in order, sovalues[0]is the captured token text.@@struct that also implementsCapture), it is pre-assigned soPos/other already-parsed fields are kept.encoding.TextUnmarshalerpath was equally broken for struct types; it now unmarshals the joined token text (and keeps the parsed struct value).Example (from the issue)
Two new tests:
TestStructCaptureViaExpression,TestStructCaptureWithTextUnmarshaler. The existingTestBoxedCapture/TestCaptureOnSliceElementscontinue to pass.