Obsolete PrincipalPermissionAttribute ctor as error #37536
Conversation
GrabYourPitchforks
added
area-System.Security
breaking-change
| @@ -1128,6 +1128,9 @@ public sealed partial class PrincipalPermission : System.Security.IPermission, S | |||
| [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method, AllowMultiple=true, Inherited=false)] | |||
| public sealed partial class PrincipalPermissionAttribute : System.Security.Permissions.CodeAccessSecurityAttribute | |||
| { | |||
| #if CAS_OBSOLETIONS | |||
| [System.ObsoleteAttribute("PrincipalPermissionAttribute is not honored by the runtime and must not be used.", true, DiagnosticId = "BCL0002", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] | |||
There was a problem hiding this comment.
Do we have the destination already created? We should ensure this is all in place before the first Preview this is contained in is released.
There was a problem hiding this comment.
Right now it redirects to #36972, pending final location of where the guidance doc will be.
| @@ -1128,6 +1128,9 @@ public sealed partial class PrincipalPermission : System.Security.IPermission, S | |||
| [System.AttributeUsageAttribute(System.AttributeTargets.Class | System.AttributeTargets.Method, AllowMultiple=true, Inherited=false)] | |||
| public sealed partial class PrincipalPermissionAttribute : System.Security.Permissions.CodeAccessSecurityAttribute | |||
| { | |||
| #if CAS_OBSOLETIONS | |||
| [System.ObsoleteAttribute("PrincipalPermissionAttribute is not honored by the runtime and must not be used.", true, DiagnosticId = "BCL0002", UrlFormat = "https://aka.ms/dotnet-warnings/{0}")] | |||
There was a problem hiding this comment.
How/where are we tracking the distribution/assignment of BCL#### IDs?
There was a problem hiding this comment.
I tried cataloging them in a single place as part of #37535. The idea was that this file would be included in every project we compile. However, since the IL linker "trim" step doesn't run over every project, it ended up including the internal class in every library we built. :(
There was a problem hiding this comment.
I added a markdown file to act as a centralized list. Does this help?
| { | ||
| PrincipalPermissionAttribute ppa = new PrincipalPermissionAttribute(new Permissions.SecurityAction()); | ||
| IPermission ip = ppa.CreatePermission(); | ||
| } |
There was a problem hiding this comment.
Since existing binaries will continue to work, seems valuable to continue to validate the behavior. Do we not have any way to write the test given the error obsoletion?
There was a problem hiding this comment.
Could put a test binary in the test data repo (where we put compression test data)?
There was a problem hiding this comment.
These methods quite literally no-op. If you attempt to use them for real you'll get a NullReferenceException. IMO this test (and the tests for FileIOPermissionAttribute, etc.) isn't testing anything useful.
There was a problem hiding this comment.
If there's really a desire to keep this test, we could always Activator.CreateInstance it, since that won't produce the build error. But again, since the implementations are no-ops, I'm not sure what utility such a test has.
I'll defer to your collective judgment if you feel that such a test is useful. :)
src/libraries/System.Security.Permissions/src/System.Security.Permissions.csproj
Outdated
Show resolved
Hide resolved
|
Yep, that looks correct. |
|
Thank you both for the eagle-eyed catch! :) |
ghost
locked as resolved and limited conversation to collaborators
Resolves #36972.
Resolves #31279.
Source breaking change, not a runtime breaking change. (We didn't change the behavior of the APIs.)
/cc @terrajobst for the diagnostic ids and "better obsoletion" stuff.