Remove dependency on Type.IsSerializable from OptionSet serialization #54990
Conversation
| @@ -439,11 +398,9 @@ static bool TryDeserializeOptionKey(ObjectReader reader, ILookup<string, IOption | |||
|
|
|||
| private enum OptionValueKind | |||
| { | |||
| Null, | |||
There was a problem hiding this comment.
Null and String and Serializable were merged into a single item called 'Object'
|
|
||
| var kind = OptionValueKind.Null; | ||
| object? valueToWrite = null; |
There was a problem hiding this comment.
no need to specially handle 'null', the default case below handles it.
There was a problem hiding this comment.
also, no need for 'valueToWrite'. we always just write out the literal value.
| @@ -215,57 +215,28 @@ public void Serialize(ObjectWriter writer, CancellationToken cancellationToken) | |||
| Debug.Assert(ShouldSerialize(optionKey)); | |||
There was a problem hiding this comment.
should view with whitespace off.
| if (type.IsEnum) | ||
| { | ||
| kind = OptionValueKind.Enum; | ||
| valueToWrite = (int)value; |
There was a problem hiding this comment.
this cast isn't necessary here due to teh line where we actually write the enum doing this already.
| @@ -386,10 +350,6 @@ public static SerializableOptionSet Deserialize(ObjectReader reader, IOptionServ | |||
| optionValue = Enum.ToObject(optionKey.Option.Type, readValue); | |||
| break; | |||
|
|
|||
| case OptionValueKind.Null: | |||
| optionValue = null; | |||
| break; | |||
There was a problem hiding this comment.
no need for this. already handled by the object reading code.
| } | ||
|
|
||
| case ICodeStyleOption: | ||
| if (optionKey.Option.Type.GenericTypeArguments.Length != 1) | ||
| continue; |
There was a problem hiding this comment.
We just ignore options without single generic type param? Shouldn't this throw or something?
There was a problem hiding this comment.
i am adding this to the list of things to try out in my followup PR :) but yes. seems insane.
ghost
added this to the 
Previous code depended on both sides of the OOP communication channel agreeing on what types are serializable or not. new code does not.