reviewed classes that participate in binary serialization

src/Common/src/NativeMethods.cs

@@ -2401,7 +2401,6 @@ public class NONCLIENTMETRICS
         }
 
         [StructLayout(LayoutKind.Sequential)]
-        [Serializable]
         public struct MSG
         {
             public IntPtr hwnd;
@@ -3225,7 +3224,6 @@ public sealed class tagOleMenuGroupWidths
         }
 
         [StructLayout(LayoutKind.Sequential)]
-        [Serializable]
         public class MSOCRINFOSTRUCT
         {
             public int cbSize = Marshal.SizeOf<MSOCRINFOSTRUCT>();              // size of MSOCRINFO structure in bytes.

src/Common/tests/InternalUtilitiesForTests/src/BinarySerialization.cs

@@ -0,0 +1,91 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection;
+using System.Runtime.Serialization.Formatters;
+using System.Runtime.Serialization.Formatters.Binary;
+using Xunit;
+
+namespace System
+{
+    public static class BinarySerialization
+    {
+        public static void EnsureSerializableAttribute(Assembly assemblyUnderTest, Dictionary<string, bool> serializableTypes)
+        {
+            foreach (Type type in assemblyUnderTest.GetTypes())
+            {
+                var serializableAttribute = Attribute.GetCustomAttribute(type, typeof(SerializableAttribute));
+
+                // Ensure that all types required by known serialization scenarions 
+                // and only these types are decorated with the SerializableAttribute.
+                if (serializableTypes.ContainsKey(type.FullName))
+                {
+                    Assert.NotNull(serializableAttribute);
+                    serializableTypes[type.FullName] = true;
+                }
+                else
+                {
+                    Assert.True(null == serializableAttribute, $"Serializable attribute is not expected on {type.FullName}");
+                }
+            }
+
+            foreach (KeyValuePair<string, bool> entry in serializableTypes)
+            {
+                Assert.True(entry.Value, $"Did we remove {entry.Key}?");
+            }
+        }
+
+        public static object EnsureDeserialize(string blob)
+        {
+            var @object = FromBase64String(blob);
+            Assert.NotNull(@object);
+            return @object;
+        }
+
+        public static string ToBase64String(object @object,
+            FormatterAssemblyStyle assemblyStyle = FormatterAssemblyStyle.Simple)
+        {
+            byte[] raw = ToByteArray(@object, assemblyStyle);
+            return Convert.ToBase64String(raw);
+        }
+
+        private static object FromBase64String(string base64String,
+            FormatterAssemblyStyle assemblyStyle = FormatterAssemblyStyle.Simple)
+        {
+            byte[] raw = Convert.FromBase64String(base64String);
+            return FromByteArray(raw, assemblyStyle);
+        }
+
+        private static object FromByteArray(byte[] raw,
+            FormatterAssemblyStyle assemblyStyle = FormatterAssemblyStyle.Simple)
+        {
+            var binaryFormatter = new BinaryFormatter
+            {
+                AssemblyFormat = assemblyStyle
+            };
+
+            using (var serializedStream = new MemoryStream(raw))
+            {
+                return binaryFormatter.Deserialize(serializedStream);
+            }
+        }
+
+        private static byte[] ToByteArray(object obj,
+            FormatterAssemblyStyle assemblyStyle = FormatterAssemblyStyle.Simple)
+        {
+            var binaryFormatter = new BinaryFormatter
+            {
+                AssemblyFormat = assemblyStyle
+            };
+
+            using (MemoryStream ms = new MemoryStream())
+            {
+                binaryFormatter.Serialize(ms, obj);
+                return ms.ToArray();
+            }
+        }
+    }
+}

src/System.Windows.Forms.Design.Editors/tests/UnitTests/SerializableAttributeTest.cs

@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.ComponentModel.Design;
+using Xunit;
+
+namespace System.Windows.Forms.Design.Editors.Tests.Serialization
+{
+    public class SerializableAttributeTests
+    {
+        [Fact]
+        public void EnsureSerializableAttribute()
+        {
+            BinarySerialization.EnsureSerializableAttribute(
+                typeof(ArrayEditor).Assembly,
+                new Dictionary<string, bool>
+                {
+                    // This Assembly does not have any serializable types.
+                });
+        }
+    }
+}

src/System.Windows.Forms.Design.Editors/tests/UnitTests/System.Windows.Forms.Design.Editors.Tests.csproj

@@ -12,6 +12,7 @@
   </ItemGroup>
 
   <ItemGroup>
+    <ProjectReference Include="..\..\..\Common\tests\InternalUtilitiesForTests\InternalUtilitiesForTests.csproj" />
     <ProjectReference Include="..\..\src\System.Windows.Forms.Design.Editors.csproj" />
     <Compile Include="..\..\..\Common\tests\CommonTestHelper.cs" Link="Common\CommonTestHelper.cs" />
     <Compile Include="..\..\..\Common\tests\CommonMemberDataAttribute.cs" Link="Common\CommonMemberDataAttribute.cs" />

src/System.Windows.Forms.Design/src/System/ComponentModel/Design/ExceptionCollection.cs

@@ -7,10 +7,13 @@
 
 namespace System.ComponentModel.Design
 {
-    [Serializable]
+    [Serializable]  // Exceptions should exchange successfully between the classic and Core frameworks.
+    // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+    // It is safe to change member names, but not the serialization key values.
     public sealed class ExceptionCollection : Exception
     {
         private readonly ArrayList _exceptions;
+        private const string SerializationKey = "exceptions";
 
         public ExceptionCollection(ArrayList exceptions)
         {
@@ -19,7 +22,7 @@ public ExceptionCollection(ArrayList exceptions)
 
         private ExceptionCollection(SerializationInfo info, StreamingContext context) : base(info, context)
         {
-            _exceptions = (ArrayList)info.GetValue("exceptions", typeof(ArrayList));
+            _exceptions = (ArrayList)info.GetValue(SerializationKey, typeof(ArrayList));
         }
 
         public ArrayList Exceptions => (ArrayList)_exceptions?.Clone();
@@ -31,7 +34,7 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont
                 throw new ArgumentNullException(nameof(info));
             }
 
-            info.AddValue("exceptions", _exceptions);
+            info.AddValue(SerializationKey, _exceptions);
             base.GetObjectData(info, context);
         }
     }

src/System.Windows.Forms.Design/src/System/ComponentModel/Design/Serialization/CodeDomComponentSerializationService.cs

@@ -226,19 +226,27 @@ public override void DeserializeTo(SerializationStore store, IContainer containe
         }
 
         /// <summary>
-        /// The SerializationStore class is an implementation-specific class that stores  serialization data for the component serialization service.  The  service adds state to this serialization store.  Once the store is  closed it can be saved to a stream.  A serialization store can be  deserialized at a later date by the same type of serialization service.   SerializationStore implements the IDisposable interface such that Dispose  simply calls the Close method.  Dispose is implemented as a private interface to avoid confusion.  The IDisposable pattern is provided  for languages that support a "using" syntax like C# and VB .NET.
+        /// The SerializationStore class is an implementation-specific class that stores serialization data for the component serialization service.  
+        /// The service adds state to this serialization store.  Once the store is closed it can be saved to a stream.  A serialization store can
+        /// be deserialized at a later date by the same type of serialization service. SerializationStore implements the IDisposable interface such
+        /// that Dispose  simply calls the Close method.  Dispose is implemented as a private interface to avoid confusion.
+        /// The <see cref="IDisposable" /> pattern is provided for languages that support a "using" syntax like C# and VB .NET.
         /// </summary>
-        [Serializable]
+        [Serializable] // This class is stored in binary serializad format during CodeDOM serialization scenarios.
+        // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+        // It is safe to change member names, but not the serialization key values.
         private sealed class CodeDomSerializationStore : SerializationStore, ISerializable
         {
 #if DEBUG
             private static readonly TraceSwitch s_trace = new TraceSwitch("ComponentSerializationService", "Trace component serialization");
 #endif
+            // Do not rename, add or delete the following keys (binary serialization)
             private const string StateKey = "State";
             private const string NameKey = "Names";
             private const string AssembliesKey = "Assemblies";
             private const string ResourcesKey = "Resources";
             private const string ShimKey = "Shim";
+
             private const int StateCode = 0;
             private const int StateCtx = 1;
             private const int StateProperties = 2;

src/System.Windows.Forms.Design/src/System/ComponentModel/Design/Serialization/CodeDomSerializerException.cs

@@ -10,7 +10,9 @@ namespace System.ComponentModel.Design.Serialization
     /// <summary>
     /// The exception that is thrown when the code dom serializer experiences an error.
     /// </summary>
-    [Serializable]
+    [Serializable] // Exceptions should exchange successfully between the classic and Core frameworks.
+    // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+    // It is safe to change member names, but not the serialization key values.
     public class CodeDomSerializerException : SystemException
     {
         public CodeDomSerializerException(string message, CodeLinePragma linePragma) : base(message)

src/System.Windows.Forms.Design/src/System/Drawing/Design/ToolboxItem.cs

@@ -17,7 +17,9 @@ namespace System.Drawing.Design
     /// <summary>
     /// Provides a base implementation of a toolbox item.
     /// </summary>
-    [Serializable]
+    [Serializable] //  ToolBox service stores items in binary serialized format
+    // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+    // It is safe to change member names, but not the serialization key values.
     public class ToolboxItem : ISerializable
     {
         private static readonly TraceSwitch s_toolboxItemPersist = new TraceSwitch("ToolboxPersisting", "ToolboxItem: write data");

src/System.Windows.Forms.Design/tests/UnitTests/SerializableAttributeTests.cs

@@ -0,0 +1,31 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.ComponentModel.Design;
+using System.ComponentModel.Design.Serialization;
+using System.Drawing.Design;
+using Xunit;
+
+namespace System.Windows.Forms.Design.Tests.Serialization
+{
+    public class SerializableAttributeTests
+    {
+        [Fact]
+        public void EnsureSerializableAttribute()
+        {
+            BinarySerialization.EnsureSerializableAttribute(
+                typeof(ToolboxItem).Assembly, 
+                new Dictionary<string, bool>
+                {
+                     // Following classes are participating in resx serialization scenarios.
+                    { typeof(ExceptionCollection).FullName,  false },
+                    { "System.ComponentModel.Design.Serialization.CodeDomComponentSerializationService+CodeDomSerializationStore",  false }, // This type is private.
+                    { typeof(CodeDomSerializerException).FullName,  false },
+                    { typeof(ToolboxItem).FullName,  false },
+                    { "System.Windows.Forms.Design.Behavior.DesignerActionKeyboardBehavior+<>c", false }
+                });
+        }
+    }
+}

src/System.Windows.Forms/src/System/Resources/ResXDataNode.cs

@@ -20,7 +20,9 @@
 
 namespace System.Resources
 {
-    [Serializable]
+    [Serializable] // This class is participating in resx serialization scenarios.
+    // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+    // It is safe to change member names, but not the serialization key values.
     public sealed class ResXDataNode : ISerializable
     {
         private static readonly char[] SpecialChars = new char[] { ' ', '\r', '\n' };
@@ -30,7 +32,8 @@ public sealed class ResXDataNode : ISerializable
         private string name;
         private string comment;
 
-        private string typeName; // is only used when we create a resxdatanode manually with an object and contains the FQN
+        // is only used when we create a resxdatanode manually with an object and contains the FQN
+        private string typeName; 
 
         private string fileRefFullPath;
         private string fileRefType;

src/System.Windows.Forms/src/System/Resources/ResXFileRef.cs

@@ -18,13 +18,17 @@ namespace System.Resources
     ///  a link to an external resource. When the resource manager asks
     ///  for the value of the resource item, the external resource is loaded.
     /// </summary>
-    [TypeConverter(typeof(Converter)), Serializable]
+    [TypeConverter(typeof(Converter))]
+    [Serializable] // This class participates in resx serialization.
+    // Note: do NOT change field names.
     public class ResXFileRef
     {
+#pragma warning disable IDE1006
         private string fileName;
         private readonly string typeName;
         [OptionalField(VersionAdded = 2)]
         private Encoding textFileEncoding;
+#pragma warning restore IDE1006
 
         /// <summary>
         ///  Creates a new ResXFileRef that points to the specified file.

src/System.Windows.Forms/src/System/Resources/ResXNullRef.cs

@@ -9,9 +9,7 @@ namespace System.Resources
     ///  It is a placeholder that is written into the file.  On read, it is replaced
     ///  with null.
     /// </summary>
-    [Serializable]
     internal sealed class ResXNullRef
     {
     }
 }
-

src/System.Windows.Forms/src/System/Windows/Forms/AxHost.cs

@@ -7114,10 +7114,10 @@ public override object ConvertTo(ITypeDescriptorContext context, CultureInfo cul
         ///  An instance of this class my be obtained either by calling getOcxState on an
         ///  AxHost object, or by reading in from a stream.
         /// </summary>
-        [
-            TypeConverter(typeof(TypeConverter)),
-            Serializable
-        ]
+        [TypeConverterAttribute(typeof(TypeConverter))]
+        [Serializable] // This class participates in resx serialization.
+        // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+        // It is safe to change member names, but not the serialization key values.
         public class State : ISerializable
         {
             private readonly int VERSION = 1;

src/System.Windows.Forms/src/System/Windows/Forms/Cursor.cs

@@ -18,9 +18,11 @@ namespace System.Windows.Forms
     ///  Different cursor shapes are used to inform the user what operation the mouse will
     ///  have.
     /// </summary>
-    [TypeConverter(typeof(CursorConverter)),
-        Serializable,
-        Editor("System.Drawing.Design.CursorEditor, " + AssemblyRef.SystemDrawingDesign, typeof(UITypeEditor))]
+    [TypeConverter(typeof(CursorConverter))]
+    [Serializable]  // This class participates in resx serialization.
+    [Editor("System.Drawing.Design.CursorEditor, " + AssemblyRef.SystemDrawingDesign, typeof(UITypeEditor))]
+    // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+    // It is safe to change member names, but not the serialization key values.
     public sealed class Cursor : IDisposable, ISerializable
     {
         private static Size cursorSize = System.Drawing.Size.Empty;

src/System.Windows.Forms/src/System/Windows/Forms/ImageListStreamer.cs

@@ -9,7 +9,9 @@
 
 namespace System.Windows.Forms
 {
-    [Serializable]
+    [Serializable] // This class is participating in resx serialization scenarios.
+    // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+    // It is safe to change member names, but not the serialization key values.
     public sealed class ImageListStreamer : ISerializable, IDisposable
     {
         // compressed magic header.  If we see this, the image stream is compressed.

src/System.Windows.Forms/src/System/Windows/Forms/LinkArea.cs

@@ -10,11 +10,12 @@
 namespace System.Windows.Forms
 {
     [TypeConverter(typeof(LinkAreaConverter))]
-    [Serializable]
+    [Serializable] // This class is participating in resx serialization scenarios.
+    // Note: do NOT change property names.
     public struct LinkArea
     {
-        private int start; // Do not rename (binary serialization)
-        private int length; // Do not rename (binary serialization)
+        private int start;
+        private int length;
 
         public LinkArea(int start, int length)
         {

src/System.Windows.Forms/src/System/Windows/Forms/ListViewGroup.cs

@@ -15,7 +15,9 @@ namespace System.Windows.Forms
     [ToolboxItem(false)]
     [DesignTimeVisible(false)]
     [DefaultProperty(nameof(Header))]
-    [Serializable]
+    [Serializable] // This class is participating in resx serialization scenarios.
+     // Note: This class implements ISerializable and uses hardcoded strings to store member data. 
+     // It is safe to change member names, but not the serialization key values.
     public sealed class ListViewGroup : ISerializable
     {
         private string _header;