Wrong ctor is chosen on deserialization

[fandrei]

Looks like there are two problems here. The parameterless ctor should be preferred, as much as I know. And in any case, it must not call a ctor with ignored parameters.

	public static void Run()
	{
		var obj = new TestClass("test", null);
		var text = JsonConvert.SerializeObject(obj);
		var res = JsonConvert.DeserializeObject<TestClass>(text);
	}

	class TestClass
	{
		protected TestClass()
		{
		}

		public TestClass(string name, TestClass parent)
		{
			// must not be called for deserialization
			Name = name;
			Parent = parent;
		}

		[JsonIgnore]
		public string Name;

		[JsonIgnore]
		public TestClass Parent;
	}

>	JsonTests.dll!JsonTests.CtorTests.TestClass.TestClass(string name, JsonTests.CtorTests.TestClass parent) Line 34	C#
 	[Lightweight Function]	
 	Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObjectUsingCreatorWithParameters(Newtonsoft.Json.JsonReader reader, Newtonsoft.Json.Serialization.JsonObjectContract contract, Newtonsoft.Json.Serialization.JsonProperty containerProperty, Newtonsoft.Json.Serialization.ObjectConstructor<object> creator, string id)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateNewObject(Newtonsoft.Json.JsonReader reader, Newtonsoft.Json.Serialization.JsonObjectContract objectContract, Newtonsoft.Json.Serialization.JsonProperty containerMember, Newtonsoft.Json.Serialization.JsonProperty containerProperty, string id, out bool createdFromNonDefaultCreator)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(Newtonsoft.Json.JsonReader reader, System.Type objectType, Newtonsoft.Json.Serialization.JsonContract contract, Newtonsoft.Json.Serialization.JsonProperty member, Newtonsoft.Json.Serialization.JsonContainerContract containerContract, Newtonsoft.Json.Serialization.JsonProperty containerMember, object existingValue)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(Newtonsoft.Json.JsonReader reader, System.Type objectType, Newtonsoft.Json.Serialization.JsonContract contract, Newtonsoft.Json.Serialization.JsonProperty member, Newtonsoft.Json.Serialization.JsonContainerContract containerContract, Newtonsoft.Json.Serialization.JsonProperty containerMember, object existingValue)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(Newtonsoft.Json.JsonReader reader, System.Type objectType, bool checkAdditionalContent)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.JsonSerializer.DeserializeInternal(Newtonsoft.Json.JsonReader reader, System.Type objectType)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.JsonSerializer.Deserialize(Newtonsoft.Json.JsonReader reader, System.Type objectType)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.JsonConvert.DeserializeObject(string value, System.Type type, Newtonsoft.Json.JsonSerializerSettings settings)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.JsonConvert.DeserializeObject<JsonTests.CtorTests.TestClass>(string value, Newtonsoft.Json.JsonSerializerSettings settings)	Unknown
 	Newtonsoft.Json.dll!Newtonsoft.Json.JsonConvert.DeserializeObject<JsonTests.CtorTests.TestClass>(string value)	Unknown
 	JsonTests.dll!JsonTests.CtorTests.Run() Line 15	C#
 	JsonTests.dll!JsonTests.Program.Main(string[] args) Line 21	C#

[elgonzo]

The parameterless ctor should be preferred, as much as I know

Wrong. See documentation for ConstructorHandling, and in particular the behavior specified for ConstructorHandling.Default: https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_ConstructorHandling.htm

Either configure ConstructorHandling.AllowNonPublicDefaultConstructor or annotate the ctor the deserializer should use with the [JsonConstructor] attribute, and Bob should be your uncle...

And in any case, it must not call a ctor with ignored parameters.

When parameterized ctors are being used for deserialization, the deserializer has to provide some value to the constructor parameters in any case. There is no way for the deserializer to pass no value to (i.e., ignore) a ctor parameter. The solution to that problem is to either use a parameterless ctor (see above), craft a ctor that fits the needs of the desired deserialization result, or - if that is not feasible - write a bespoke JsonConverter that implements the desired deserialization behavior for the respective type(s).

[fandrei]

@elgonzo I see that you deleted "Note how it explains that this attribute affects serialization (and not deserialization)." So, it should ignore this field during deserialization too?

Wrong. See documentation for ConstructorHandling, and in particular the behavior specified for ConstructorHandling.Default

Interesting. In this case, why does it use the parameterless ctor for this class?

	class TestClass
	{
		protected TestClass()
		{
		}

		public TestClass(string name)
		{
			Name = name;
		}

		public TestClass(string name, TestClass parent)
		{
			Name = name;
			Parent = parent;
		}

		[JsonIgnore]
		public string Name;

		[JsonIgnore]
		public TestClass Parent;
	}

[elgonzo]

@elgonzo I see that you deleted "Note how it explains that this attribute affects serialization (and not deserialization)." So, it should ignore this field during deserialization too?

I deleted it because it was not correct; i made a false statement. There is no point in discussing a false statement...

Interesting. In this case, why does it use the parameterless ctor for this class?

Read the documentation for ConstructorHandling.Default i linked to. It says "First attempt to use the public default constructor, then fall back to a single parameterized constructor, then to the non-public default constructor."

In your latter example class there is no single parameterized ctor, there are two parameterized ctors.
So, according to the documented behavior:

1. Is there a public default ctor? No.
2. Is there a single parameterized ctor? No.
3. Is there a non-public default ctor? Yes!

Side note 1: Based on cursory tests, it seems the single parameterized ctor the deserializer is looking for has to be public, even though the documentation doesn't mention it explicitly.

Side note 2: I don't like the documentation using the term "default constructor" incorrectly also as a synonym for "parameterless constructor". But it shouldn't be an impediment to comprehending the documentation...

[fandrei]

@elgonzo

fall back to a single parameterized constructor

That's pretty confusing way to describe how it works. And really confusing resolution logic, too.

In any case, it must not try to assign the field that it was supposed to ignore. If it cannot choose the ctor that it wants to use, it should throw an exception rather than do something wrong and completely unexpected.

By the way, I had the same result when I had only 1 parameterized ctor, but I couldn't reproduce it in a small repro so far.

[elgonzo]

If it cannot choose the ctor that it wants to use, it should throw an exception rather than do something wrong and completely unexpected.

That would be a sensible behavior. But i don't think this is going to happen for two reasons: There is little development activities going on with Newtonsoft.Json for quite some time -- in my personal view pretty much a library in maintenance mode. But even so, given that Newtonsoft.Json has been around for a long time and enjoys (enjoyed?) wide-spread use, changing the existing behavior would carry a risk of breaking code that is far more substantial than the slight benefit this code change would provide.

[elgonzo]

By the way, I had the same result when I had only 1 parameterized ctor

Perhaps that parameterized ctor was non-public.

[fandrei]

@elgonzo

There is little development activities going on with Newtonsoft.Json for quite some time -- in my personal view pretty much a library in maintenance mode.

Hm. Any info from the author?

changing the existing behavior would carry a risk of breaking code

Only if that code was incorrect, in the first place.
And not changing it adds very real risk to break code in strange ways when you slightly modify your ctors.

Perhaps that parameterized ctor was non-public.

No. I only added another argument to the parameterized ctor.