Special case Option intermediary type in extraction. #1717
Merged
Conversation
This file contains 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
This essentially treats Option as a type of collection, which it is in a way, and special-cases the handling of it so that invoking something like `extract[List[Option[String]]]` will work as the user expects.
| @@ -59,6 +59,11 @@ object ExtractionBugs extends Specification { | |||
| json.extract[Response] mustEqual Response(List(Map("one" -> 1, "two" -> 2))) | |||
| } | |||
|
|
|||
| "Extracting List[Option[String]]" in { | |||
There was a problem hiding this comment.
Phrasing here seems wrong (“Extraction should handle…” seems to be the trend above).
|
Looks cool! |
farmdawgnation
added a commit
that referenced
this pull request
Sep 26, 2015
We now special case Option as a type of collection in the generation of the Meta ADT, and handle that case when constructing an Option[_] when invoking extract[...].
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.
This PR resolves a long-standing bug where trying to execute extraction when
Optionwas an intermediary type on aListwould cause fireworks and explosions. So, for example if you tried something like the following:You would get explosions and fireworks because lift-json would try to match up what you've provided to the constructor for
Option. We now special caseOptionas a type of collection in the generation of theMetaADT, and handle that case when constructing the concrete instance.I'm not quite familiar enough with this code to know if this is a kludge or a clever hack. Would love some 👀 from @jonifreeman (if you have time). Either way, I humbly submit this for review.
closes #1071