Add spec for obsoletions in .NET 5

[jeffhandley]

No description provided.

[Happypig375]

https://aka.ms/dotnet-warnings/MSLIB0004 doesn't work yet?

[jeffhandley]

https://aka.ms/dotnet-warnings/MSLIB0004 doesn't work yet?

@Happypig375 Thanks for checking that. We'll get the URLs working before shipping the GA release in November.

[AustinWise]

For CAS related items, the documented says "all [classes] from the System.Security.Permissions namespace" will be marked obsolete. What about other permissions like System.Net.SocketPermission?

It looks like the whole of the System.Security.Permissions are shims that either do nothing or throw PNSE. Examples include:

• Evidence
• EvidenceBase and subclasses
• HostSecurityManager
• SecurityContext
• A few members of SecurityManager have not already been obsoleted.

There are also some CAS types in CorLib like SecurityTransparentAttribute. Some, like SecurityTreatAsSafeAttribute, are already obsolete but the message does not point the user in the right direction: "SecurityTreatAsSafe is only used for .NET 2.0 transparency compatibility. Please use the SecuritySafeCriticalAttribute instead.".

If not all CAS types are obsoleted, it may be worth giving a rationale in the document.

[GrabYourPitchforks]

What about other permissions like System.Net.SocketPermission?

That's a good point. The obsoletions should probably include everything that subclasses CodeAccessSecurityAttribute or which implements IPermission, regardless of what namespace it's in. I suspect we'll discover some additional types not emcompassed by the above list once we mark the base types / interfaces obsolete and see what compiler failures fall out of the libraries code.

[jeffhandley]

The obsoletions should probably include everything that subclasses CodeAccessSecurityAttribute or which implements IPermission

• I'll add all derivatives of CodeAccessSecurityAttribute and add them to the CAS category
• I can also find the implementations of IPermission; should IPermission itself be in the list?

[GrabYourPitchforks]

@jeffhandley Yeah, I'd also add IPermission and IStackWalk to the list, along with everything implementing them or exposing them. Not sure how much creep that will add though.

[jeffhandley]

This comment is no longer valid. SecurityAction was already in the list. I've updated the PR to keep the attribute on that Enum and remove the attribute from the members that were already marked as Obsolete, as the bigger obsoletion supersedes the old one.

@GrabYourPitchforks I'm considering keeping SecurityAction.PermitOnly in the list but noting that we should use the same format of the [Obsolete] attribute that several other members of SecurityAction already have:

[Obsolete("Assembly level declarative security is obsolete and is no longer enforced by the CLR by default. See https://go.microsoft.com/fwlink/?LinkID=155570 for more information.")]

That is currently applied to Deny, RequestMinimum, RequestOptional, and RequestRefuse.

What do you think? Should we obsolete it that way, assign it into the CAS DiagnosticId, or omit it from the obsoletions list?

[jkotas]

See https://go.microsoft.com/fwlink/?LinkID=155570

This link has information specific to .NET Framework 2.0 -> .NET Framework 4.0 specific CAS breaking change that is not relevant to .NET Core. We should delete it in .NET Core and supersede it with bulk CAS obsoletion.

I think we should:

• Mark all types related to CAS obsolete. Keep it simply and mark everything. I do not think it is worth trying to reason about one-off cases that somebody somewhere may use. It just makes the story more complicated.
• The auto-fixer should be conservative and not remove code that might have been doing something important. Proposed code fixer for obsoleted CAS APIs runtime#39235 is a good start.

[jeffhandley]

@jkotas / @GrabYourPitchforks - I went ahead and combined the PrincipalPermission and PermissionState into the CAS category.

[Joe4evr]

I've also made a list of candidate Obsolete APIs a few months back: dotnet/runtime#33360
I think it would be good to cross-reference with that list and the discussions pointed to, since there's a few on there that I don't see here.

[danmoseley]

LGTM, perhaps it would be worth noting why some things aren't on this list that were in the original discussions eg SecureString. I assume because it's intended to be a small start to prove the mechanism that focuses on the most obvious cases.

[jeffhandley]

@danmosemsft @Joe4evr - I just added an "Other Considerations" section that talks to why we chose the APIs we did for .NET 5.

[jeffhandley]

@bartonjs HashAlgorithm.Create() also throws (and there's a unit test including it in the same list as these others). Should we obsolete it as well?

[bartonjs]

HashAlgorithm.Create() also throws (and there's a unit test including it in the same list as these others). Should we obsolete it as well?

Yep.

[terrajobst]

Review notes:

• Can we turn off all obsoletions? I'll follow up with the I'll file an issue.
• We shouldn't reuse PrincipalPermissionAttribute b/c it's an error (we shouldn't be using the same ID for different severities)
• We should avoid using the same if for where some of the APIs will have fixers and some of them don't

[jeffhandley]

Thanks, @terrajobst.

• We shouldn't reuse PrincipalPermissionAttribute b/c it's an error (we shouldn't be using the same ID for different severities)

I will keep PrincipalPermissionAttribute as SYSLIB0002 (as an error) and then I will add the rest of CAS into SYSLIB0003 (as a warning).

Sorry @GrabYourPitchforks -- this will steal SYSLIB0003 out from under you.

[jeffhandley]

The way it's playing out with PrincipalPermissionAttribute is:

1. I'll leave the existing diagnostic on its constructor (as an error)
2. I'll apply the new diagnostic to the type (as a warning)

We cannot put errors on types because of our type forwarding infrastructure--the type forward files would then fail to build because they are referencing types that have obsoletions as errors, which cannot be suppressed.

We cannot put all of the new diagnostics on constructors since we're obsoleting the interfaces; the types themselves need to then be obsoleted as well.

[bartonjs]

I will keep PrincipalPermissionAttribute as SYSLIB0002 (as an error) and then I will add the rest of CAS into SYSLIB0003 (as a warning).

They don't need to be adjacent. In fact, being not adjacent early on will show that we don't care about such trivialities.

[terrajobst]

Yeah, let's not do unnatural acts for aesthetics. We can't keep it up anyways. We should think of SYSLIBXXXX as an opaque ID.

[jeffhandley]

Good point -- sorry I had already moved forward with that. BinaryFormatter needed to change from MSLIB to SYSLIB, so I needed to touch it either way. In the future, I will intentionally not care about such trvialities. 😃

[Gnbrkm41]

Thread.Suspend seems to be missing (it's unsupported along with Thread.Abort I believe) from the list.

[jeffhandley]

Thread.Suspend seems to be missing (it's unsupported along with Thread.Abort I believe) from the list.

Thread.Suspend and Thread.Resume have already been obsoleted. Although we should consider unifying all of the obsoleted Thread APIs into a single diagnostic ID and message (SYSLIB0006). @terrajobst -- any objections to that?

[terrajobst]

Depends. If we're changing diagnostic IDs for already shipped obsolete messages, we probably want to consider this separately. I'm not opposed to that, but we shouldn't do one-offs without considering the larger picture.

If we only shipped the obsoletions in a preview of .NET 5, feel free to change it.