Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] [.NET 8] Use ExperimentalAttribute instead of ObsoleteAttribute to indicate alpha APIs. #1219

Open
philliphoff opened this issue Jan 3, 2024 · 4 comments

Comments

@philliphoff
Copy link
Collaborator

philliphoff commented Jan 3, 2024

Describe the proposal

Currently alpha APIs are decorated with the ObsoleteAttribute which causes the compiler to generate a warning/error when used. While the generated message is not entirely accurate, it does ensure users see and understand the risks of using a potentially unstable API.

.NET 6 introduced a new attribute, RequiresPreviewFeaturesAttribute, which serves a purpose similar to our use of ObsoleteAttribute. This generates a similar message (but from the Rosyln analyzer rather than the compiler itself), and requires a slightly different method to disable (via MSBuild property). The main disadvantage is disabling applies to all uses of such decorated APIs, rather than individual uses.

Still, RequiresPreviewFeaturesAttribute feels like a more appropriate way to indicate alpha APIs.

Another option is to create our own attribute and set of Roslyn analyzers that have our specific desired semantics; this could then be a start to additional work to help ensure best practices when using the SDK.

Update: .NET 8 introduces ExperimentalAttribute, which is even more closely aligned with our use case.

@halspang
Copy link
Contributor

halspang commented Jan 9, 2024

@philliphoff - Would this be a breaking change if we switched them all over? Or is there a way to avoid that?

Or, is this proposal just to use it for new features and the old features will be [Obsolete] until they're no longer preview?

@philliphoff
Copy link
Collaborator Author

@halspang Yes, it would be breaking in that it's a different compiler/analyzer error to disable. That said, I just discovered that .NET 8 includes a new ExperimentalAttribute which seems even closer to what we want. (RequiresPreviewFeaturesAttribute was originally targeted at APIs in .NET itself but would still have been better than ObsoleteAttribute.) I suggest we wait until .NET 6 support is dropped and then switch to ExperimentalAttribute, as a whole, so users needn't have to change how they suppress the warnings more than once.

@philliphoff philliphoff changed the title [Proposal] Use RequiresPreviewFeaturesAttribute instead of ObsoleteAttribute to indicate alpha APIs. [Proposal] [.NET 8] Use ExperimentalAttribute instead of ObsoleteAttribute to indicate alpha APIs. Jan 11, 2024
@Neme12
Copy link

Neme12 commented Mar 28, 2024

@philliphoff Couldn't the analyzer just suggest using ExperimentalAttribute if it exists? That means for users on .NET 8 and others who define it. From what I understand, others can define it in their own code as well even on a lower TFM, just like many other attributes that the compiler understands - they're not required to come from mscorlib.

Also, regardless of whether it supports the atribute being defined manually (which it should IMO, there's no reason not to - it's simple to just check whether a type named System.Diagnostics.CodeAnalysis.ExperimentalAttribute exists), why wait until .NET 6 support is dropped when .NET 8 already exists and the analyzer could already work on .NET 8?

@Neme12
Copy link

Neme12 commented Mar 28, 2024

Oh, this isn't about an analyzer, my bad.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants