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

Consider shipping RequiresPreviewFeaturesAttribute as a downlevel assembly #79479

Closed
terrajobst opened this issue Dec 10, 2022 · 3 comments
Closed

Comments

@terrajobst
Copy link
Member

terrajobst commented Dec 10, 2022

Attributes for analyzers and compilers are usually matched by name such that downlevel consumers can carry an internal copy to give their consumers a better experience without having to depend on extra assemblies.

The downside is that internal attribute applications won't show up in documentation. For preview APIs it would be desirable to have information available in the docs. Now, rendering the attribute as part of the syntax is only one way to express that. The alternative design is to add a banner-style warning. At that point, the attribute application being visible seems less important.

The downside of shipping this attribute downlevel is added complexity. The design for this would look as follows:

Design Sketch

Component Name
NuGet Package Microsoft.CodeAnalysis.NetAnalyzers
Assembly System.Runtime.Versioning.RequiresPreviewFeaturesAttribute.dll
Frameworks netstandard2.0, net461, net6.0
#if NET6_0_OR_GREATER

using System.Runtime.CompilerServices;
using System.Runtime.Versioning;
[assembly: TypeForwardedTo(typeof(RequiresPreviewFeaturesAttribute))]

#else

namespace System.Runtime.Versioning;

[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Module | AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Event | AttributeTargets.Interface | AttributeTargets.Delegate, Inherited = false)]
public sealed class RequiresPreviewFeaturesAttribute : Attribute
{
    public RequiresPreviewFeaturesAttribute()
    {
    }

    public RequiresPreviewFeaturesAttribute(string/*?*/ message)
    {
    }

    public string/*?*/ Message { get; }

    public string/*?*/ Url { get; set; }
}

#endif

/cc @geeknoid @stephentoub

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 10, 2022
@ghost
Copy link

ghost commented Dec 10, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Attributes for analyzers and compilers are usually matched by name such that downlevel consumers can carry an internal copy to give their consumers a better experience without having to depend on extra assemblies.

The downside is that internal attribute applications won't show up in documentation. For preview APIs it would be desirable to have information available in the docs. Now, rendering the attribute as part of the syntax is only one way to express that. The alternative design is to add a banner-style warning. At that point, the attribute application being visible seems less important.

The downside of shipping this attribute downlevel is added complexity. The design for this would look as follows:

Design Sketch

Component Name
NuGet Package Microsoft.CodeAnalysis.NetAnalyzers
Assembly System.Runtime.Versioning.RequiresPreviewFeaturesAttribute.dll
Frameworks netstandard2.0, net461, net6.0
#if NET6_0_OR_GREATER

using System.Runtime.CompilerServices;
using System.Runtime.Versioning;
[assembly: TypeForwardedTo(typeof(RequiresPreviewFeaturesAttribute))]

#else

namespace System.Runtime.Versioning;

[AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Module | AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | AttributeTargets.Constructor | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Event | AttributeTargets.Interface | AttributeTargets.Delegate, Inherited = false)]
public sealed class RequiresPreviewFeaturesAttribute : Attribute
{
    public RequiresPreviewFeaturesAttribute()
    {
    }

    public RequiresPreviewFeaturesAttribute(string/*?*/ message)
    {
    }

    public string/*?*/ Message { get; }

    public string/*?*/ Url { get; set; }
}

#endif

/cc @geeknoid @stephentoub

Author: terrajobst
Assignees: -
Labels:

area-System.Runtime, untriaged

Milestone: -

@terrajobst terrajobst removed the untriaged New issue has not been triaged by the area owner label Dec 12, 2022
@dakersnar dakersnar added this to the Future milestone Dec 13, 2022
@terrajobst
Copy link
Member Author

Moving forward, we don't expect third parties to use [RequiresPreviewFeatures] and instead use the upcoming [Experimental] attribute instead, so I don't think this will be necessary.

@terrajobst terrajobst closed this as not planned Won't fix, can't repro, duplicate, stale Aug 18, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants