Fix handling of unknown values for known property names #155
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
When the option to ignore unknown values was turned on, unknown values were not being stored in the parsed properties as UnknownProperty was only being used when an unknown property name was found, not an unknown value for a known property. To handle this, need to split the logic around unknown property into two - firstly, if we seen an unknown property name then we instantiate an UnknownProperty instance, but we if find a known property that we can't set the value on (TrySetValue returns false), we need to convert that property to an UnknownProperty instance and re-set the value on it.
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.
When the option to ignore unknown values was turned on, unknown values were not being stored in the parsed properties as
UnknownPropertywas only being used when an unknown property name was found, not an unknown value for a known property.To handle this, we need to split the logic around unknown properties into two. Firstly, if we see an unknown property name then we instantiate an
UnknownPropertyinstance, but if we find a known property that we can't set the value on (TrySetValuereturnsfalse), we need to convert that property to an UnknownProperty instance and callTrySetValueon it again.This requires a bunch of changes to unit test expectations since there are some tests that use very liberal parsing options for allowing unknown properties, unknown values etc. and due to everything being stored internally as
UnknownPropertybefore (which will accept any value), some of the expected values for things like colours/short-hands etc. were out of date and failing.