sonar-dotnet
sonar-dotnet copied to clipboard
SonarSource
Improve S3925 message to be clear about expected action.
This snippet
public class SomeException : Exception { }
produces
S3925 Update this implementation of 'ISerializable' to conform to the recommended serialization pattern.
There are 9 different things that can go wrong with this rule. It's not clear what action is (or what actions are) expected to be done.
I believe this rule is also outdated with .NET Core / .NET 5+? Because IIRC, Exceptions are binary serialized and do not need to implement the ISerializable interface or the [Serializable]
attribute. At least the .NET Team itself does not use the interface and Serializable attribute.
@m-gallesio no, there's no active work on it. For now, it's recorded in the backlog.
@pavel-mikula-sonarsource this is also happening for classes that derive from Dictionary<Tk, Tv>.
This rule should be disabled for .NET Core and .NET 5 and greater. Previously exceptions needed to be serializable so they could be marshalled across AppDomains. But since .NET Core, there are no more AppDomans. Even the new exceptions in the BCL itself are not serializable anymore (see for example HttpRequestException).
It seems that we have secondary locations with more hints but we still need to improve the rule.
We are going to improve the primary message with details from the secondary locations. Regarding the other issues mentioned in the thread (deprecation, dictionary, etc.) please have a look at what we did in #7673 and #8377: Binary serialization is "opt-in" now and requires at least one of the serialization properties to be present (ISerializable, or [Serializable] or serialization constructor).
