SonarSource
Fix on FP 3925: Classes not having extra properties should not have to extend ISerializable interface
As reported (#3945), classed not having extra properties should not have extend the ISerialziable interface.
@costin-zaharia-sonarsource Currenlty, I exclude any analysis when no field or property is defined on a class, when somewhere in any of its base classes ISerializable is introduced. I'm suppose that not correct, as the [Serializable] attribute is still required?!
Furthermore, this change breaks most scenarios. I'm willing to change them, but before doing so, lets have all constraints crystal clear.
I'm suppose that not correct, as the [Serializable] attribute is still required?!
Yes. The [Serializable] attribute must be present in the child class. From the documentation:
Any class that might be serialized must be marked with the SerializableAttribute. If a class needs to control its serialization process, it can implement the ISerializable interface.
I had a look at the types that implement ISerializable in the framework and that users are likely to derive from:
System.Collections.Generic.Dictionary<TKey,TValue>System.Collections.Generic.HashSet<T>System.Collections.Generic.LinkedList<T>System.Collections.Generic.SortedSet<T>- System.Collections.Hashtable
- System.Collections.Specialized.NameObjectCollectionBase
- System.Collections.Specialized.OrderedDictionary
- System.Data.DataSet
- System.Data.DataTable
- System.Exception
- System.Net.FileWebRequest
- System.Net.FileWebResponse
- System.Net.HttpWebRequest
- System.Net.HttpWebResponse
- System.Net.WebHeaderCollection
- System.Net.WebProxy
- System.Net.WebRequest
- System.Net.WebResponse
- System.Windows.Forms.ListViewGroup
- System.Windows.Forms.ListViewItem
- System.Windows.Forms.OwnerDrawPropertyBag
- System.Windows.Forms.TableLayoutSettings
- System.Windows.Forms.TreeNode
As a user, I would not want to be reminded to implement ISerializable for any of these types. Therefore I would suggest to
not raising an issue if the derived type is not decorated with [Serializable].
We may consider Exception being an exception to this rule. @Corniel please wait with the implementation until I get confirmation from the rest of the team.
Hi @Corniel
I'm closing your PR in favor of #7673. We are only raising issues if there is at least one indication that the user is actually interested in the serializability of the derived type (as described here https://github.com/SonarSource/sonar-dotnet/pull/5709#issuecomment-1317011974). Thank you for bringing the issue up. It would be nice if you could have a look at the new PR as well and leave comments if you think it is misbehaving. The new PR is still linked to your original issue and we will rename it if we think the new PR is going to be merged.