Running CodeFormatter renames private fields that have [DataMember] which breaks serialization

[jaredmoo]

We were bit by a nasty issue that running CodeFormatter broke serialization of a type that had [DataMember] on a private field. CodeFormatter renamed the field, which caused the serialization format to change, so our pre-CodeFormatter and post-CodeFormatter serialization was incompatible.

Below program demonstrates the issue:

using System;
using System.Runtime.Serialization;
using System.Xml;

namespace DataMemberSample
{
    class Program
    {
        static void Main(string[] args)
        {
            var myData = new MyData(5);

            var dataContractSerializer = new DataContractSerializer(typeof(MyData));
            using (var xw = XmlWriter.Create(Console.Out, new XmlWriterSettings { Indent = true }))
            {
                dataContractSerializer.WriteObject(xw, myData);
            }
        }
    }

    [DataContract]
    class MyData
    {
        [DataMember]
        private int MyInteger;

        public MyData(int myInteger)
        {
            MyInteger = myInteger;
        }
    }
}

Before CodeFormatter, produces the below output:

<?xml version="1.0" encoding="IBM437"?>
<MyData xmlns:i="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://schemas.datacontract.org/2004/07/DataMemberSample">
<MyInteger>5</MyInteger>
</MyData>

Running CodeFormatter renames MyInteger to _myInteger, so now it produces this output:

<?xml version="1.0" encoding="IBM437"?>
<MyData xmlns:i="http://www.w3.org/2001/XMLSchema-instance" xmlns="http://schemas.datacontract.org/2004/07/DataMemberSample">
<_myInteger>5</_myInteger>
</MyData>

Of course we should have had tests that detected this issue, but I should feel confident that CodeFormatter will not affect code functionality.

[AndyGerlicher]

+1 on this. In MSBuild we use binary serialization all over (uhg) and changing private fields on [Serializable] classes changes the contract for serialization. It would be good to detect both of these and be able to disable renaming in those cases (even if it's an extra option).

[ghost]

Another irritant: renaming fields that are part of an interop struct and are intentionally named to match their C++ counterparts. Opened a separate issue for this:

#230