@jeffkl @MichaelSimons @mthalman

Taking a step back and looking at the source of the problem. Is there any chance of fixing the underlying issue? I see the type is public so maybe not without a breaking change. If not, would it be better to #pragma warning disable/restore the warning in code around the specific instance? The reason I mention this is to avoid the source-build specialization here.

The ironic thing is that we added these constructor overloads because roslyn CA code analyzers told us to! 🤦

Something else to note is that these constructors & ISerializable implementation is needed for cross-AppDomain support on .NET Framework, which NuGet still supports (largely due to Visual Studio). Having said that, NuGet doesn't use AppDomains, so we don't explicitly need cross-AppDomain support, but it goes to show the challenge of supporting .NET (Core) & .NET Framework at the same time. Depending on which TFM/LangVersion, you get contradictory analyzers 😭

Taking a step back and looking at the source of the problem. Is there any chance of fixing the underlying issue? I see the type is public so maybe not without a breaking change. If not, would it be better to #pragma warning disable/restore the warning in code around the specific instance? The reason I mention this is to avoid the source-build specialization here.

Based on the guidance in the breaking change we should be able to add the following:

#if NET8_0_OR_GREATER
    [Obsolete(DiagnosticId = "SYSLIB0051")] // add this attribute to the serialization ctor
#endif

Taking a step back and looking at the source of the problem. Is there any chance of fixing the underlying issue? I see the type is public so maybe not without a breaking change. If not, would it be better to #pragma warning disable/restore the warning in code around the specific instance? The reason I mention this is to avoid the source-build specialization here.

Based on the guidance in the breaking change we should be able to add the following:

#if NET8_0_OR_GREATER
    [Obsolete(DiagnosticId = "SYSLIB0051")] // add this attribute to the serialization ctor
#endif

Fixed with 49ed7f4

Full source-build validation build is in progress: https://dev.azure.com/dnceng/internal/_build/results?buildId=2308863&view=results

cc @zivkan @MichaelSimons @mthalman

There are more instances, about a dozen. I wonder if we should add a new NoWarn here instead of making all the source changes:

I've updated the PR with conditional obsolete attributes, per guidance in breaking change document: dotnet/docs#34893

This PR adds the nowarn to repo's Directory.Build.props file - conditioned for full product source-build. It does not affect any other build target.

Thank you for the contribution. However, I don't see any changes to the repo's Directory.Build.props file in the proposed changes. Could you please update the PR description to accurately reflect the proposed changes?

This PR adds the nowarn to repo's Directory.Build.props file - conditioned for full product source-build. It does not affect any other build target.

Thank you for the contribution. However, I don't see any changes to the repo's Directory.Build.props file in the proposed changes. Could you please update the PR description to accurately reflect the proposed changes?

Thank you - will update the description. Implementation has changed based on PR discussion.

The ironic thing is that we added these constructor overloads because roslyn CA code analyzers told us to! 🤦

Something else to note is that these constructors & ISerializable implementation is needed for cross-AppDomain support on .NET Framework, which NuGet still supports (largely due to Visual Studio). Having said that, NuGet doesn't use AppDomains, so we don't explicitly need cross-AppDomain support, but it goes to show the challenge of supporting .NET (Core) & .NET Framework at the same time. Depending on which TFM/LangVersion, you get contradictory analyzers 😭

I created a bug report dotnet/roslyn-analyzers#7026. I am also thinking to disable these analyzers from our code analysis ruleset file, remove [Serializable] attribute and constructor from the code as recommended in the dotnet/docs#34893 issue especially Microsoft strongly recommends against serialization libraries supporting the legacy serialization infrastructure ([Serializable] and ISerializable).