[Serialization] More useful throw when missing parameterless ctor #2098
Conversation
|
should i write a diagnostics analyzer for that case? as i cant handle parameter constrcutors either |
|
Yep, that would be best |
|
Do we have unit tests covering these serialization cases? If so, they might need updating. |
|
i have it halfway finished but i need polysharp for it, which is currently removed, lets see how it goes when the compilerservices are restored |
|
Added a simple test, it should be enough for this feature ... ? |
| } | ||
| catch (Exception ex) | ||
| { | ||
| Assert.True(ex.InnerException is DefaultObjectFactory.InstanceCreationException); |
There was a problem hiding this comment.
cant you use here Assert.Throws?
There was a problem hiding this comment.
I looked at it initially but didn't find an overload for inner exceptions ?
There was a problem hiding this comment.
Assert.That(ex.InnerException, Is.TypeOf<BadException>() ); mybe that one?
There was a problem hiding this comment.
Changed to that one instead Assert.IsType<DefaultObjectFactory.InstanceCreationException>(ex.InnerException);, no API for .That()
| @@ -119,11 +120,12 @@ public object Create(Type type) | |||
| } | |||
| } | |||
|
|
|||
| return null; | |||
| throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not implement a parameterless constructor."); | |||
There was a problem hiding this comment.
Nit: a type has or doesn't have a parameterless constructor. It doesn't really implement it, the same way an interface or abstract method can be.
| throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not implement a parameterless constructor."); | |
| throw new InstanceCreationException($"Failed to create instance of type '{type}', type does not have a parameterless constructor."); |
PR Details
Serialization throws with a random null ref exception when the type being deserialized does not have a parameterless constructor defined, this PR throws with a more specific exception. Users should be able to figure it out from there until @IXLLEGACYIXL work gets integrated.
Types of changes
^ It would seem like this would be a breaking change, but all calling logic to that method actually expected it to never return null.
Checklist