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

Unify MinimumOSPlatformAttribute and UnsupportedOSPlatformAttribute #39989

Closed
terrajobst opened this issue Jul 28, 2020 · 3 comments
Closed

Unify MinimumOSPlatformAttribute and UnsupportedOSPlatformAttribute #39989

terrajobst opened this issue Jul 28, 2020 · 3 comments
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-Meta area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer
Milestone

Comments

@terrajobst
Copy link
Contributor

terrajobst commented Jul 28, 2020

The original proposal for marking-platform-specific APIs only considered cases where an API was platform-specific, such as the Windows registry. When trying to design a solution for marking APIs as unsupported by Blazor, it became obvious that we need a way to exclude some platforms without treating the API as platform-specific. Hence, a proposal for platform-exclusion was born dotnet/designs#143.

During the design, it emerged that it would be desirable to align these two proposals.

API proposal

  • Rename MinimumOSPlatformAttribute to SupportedOSPlatformAttribute
  • Rename RemovedInOSPlatformAttribute to UnsupportedOSPlatformAttribute
namespace System.Runtime.Versioning
{
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class SupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public SupportedOSPlatformAttribute(string platformName);
    }
  
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class UnsupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public UnsupportedOSPlatformAttribute(string platformName);
    }
}

The semantics of these new attributes are as follows:

  • An API that doesn't have any of these attributes is considered supported by all platforms.
  • If either [SupportedOSPlatform] or [UnsupportedOSPlatform] attributes are present, we group all attributes by OS platform identifier:
    • Allow list. If the lowest version for each OS platform is a [SupportedOSPlatform] attribute, the API is considered to only be supported by the listed platforms and unsupported by all other platforms.
    • Deny list. If the lowest version for each OS platform is a [UnsupportedOSPlatform] attribute, then the API is considered to only be unsupported by the listed platforms and supported by all other platforms.
    • Inconsistent list. If for some platforms the lowest version attribute is [SupportedOSPlatform] while for others it is [UnsupportedOSPlatform], the analyzer will produce a warning on the API definition because the API is attributed inconsistently.
  • Both attributes can be instantiated without version numbers. This means the version number is assumed to be 0.0. This simplifies guard clauses, see examples below for more details.
  • [ObsoletedInOSPlatform] continuous to require a version number.
  • [ObsoletedInOSPlatform] by itself doesn't imply support. However, it doesn't make sense to apply [ObsoletedInOSPlatform] unless that platform is supported.

API Usage

Unsuported platform

In .NET 5:

[UnsupportedOSPlatform("windows")]
public void DoesNotWorkOnWindows();

In .NET 6 we change the code to support the API, but only on Windows 10:

[UnsupportedOSPlatform("windows")]
[SupportedOSPlatform("windows10.0.1903")]
public void DoesNotWorkOnWindows();

Platform-specific API

[SupportedOSPlatform("ios12.0")]
[ObsoletedInOSPlatform("ios13.0")]
[UnsupportedOSPlatform("ios14.0")]
[SupportedOSPlatform("ipados13.0")]
public void OnlyWorksOniOS();

Related

@terrajobst terrajobst added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Meta area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer labels Jul 28, 2020
@terrajobst terrajobst self-assigned this Jul 28, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jul 28, 2020
@terrajobst terrajobst added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work and removed untriaged New issue has not been triaged by the area owner api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 28, 2020
@terrajobst terrajobst added this to the 5.0.0 milestone Jul 28, 2020
@jeffhandley
Copy link
Member

As I commented on in the spec PR, I wonder if we should support params string[] platformNames on these attributes. That would make it easier to annotate the iOS family of platforms.

[SupportedOSPlatform("ios12.0", "ipados13.0")]
[ObsoletedInOSPlatform("ios13.0")]
[UnsupportedOSPlatform("ios14.0")]
public void OnlyWorksOniOS();

@eerhardt
Copy link
Member

Overall, this proposal looks good - it will allow us to model unsupported APIs in Blazor WASM.

namespace System.Runtime.Versioning

RuntimeInformation.IsOSPlatform() and the OSPlatform struct are in the System.Runtime.InteropServices namespace. But these attributes are in a different namespace. Why the difference? They are all about "OS Platforms". The rest of the types in System.Runtime.Versioning seem to be primarily about versioning. But these attributes are primarily about which Operating Systems an API is supported/unsupported in. Versioning seems to be a secondary concern.

It feels like the types OSPlatform and OSPlatformAttribute should be in the same namespace.

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels Jul 28, 2020
@terrajobst
Copy link
Contributor Author

terrajobst commented Jul 28, 2020

Video

  • We'll rename MinimumOSPlatformAttribute and RemovedInOSPlatformAttribute to SupportedOSPlatformAttribute and UnsupportedOSPlatformAttribute respectively
  • We'll move them (plus ObsoletedInOSPlatform) from System.Runtime.Versioning to System.Runtime.InteropServices to co-locate them with them guarding API RuntimeInformation
  • We don't like InteropServices. @terrajobst to follow up if w can have the guard methods on a type/namespace that isn't RuntimeInformation
  • We'll leave both OSPlatformAttribute and TargetPlatformAttribute in System.Runtime.Versioning
  • The difference between net5.0-windows and [SupportedOSPlatform("windows)] is that the TFM is expanded at build time to a concrete version, which for .NET 5 is windows7.0 while the platform name in the attribute means 0.0. The resulting behavior will be intuitive for the user, but it's an important distinction.
  • We generally don't use params on attributes that can be applied multiple times
namespace System.Runtime.Versioning
{
    public abstract class OSPlatformAttribute : Attribute
    {
        private protected OSPlatformAttribute(string platformName);
        public string PlatformName { get; }
    }

    [AttributeUsage(AttributeTargets.Assembly,
                    AllowMultiple=false, Inherited=false)]
    public sealed class TargetPlatformAttribute : OSPlatformAttribute
    {
        public TargetPlatformAttribute(string platformName);
    }
}
namespace System.Runtime.InteropServices
{
    // Renamed from MinimumOSPlatformAttribute
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class SupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public SupportedOSPlatformAttribute(string platformName);
    }

    // RemovedInOSPlatformAttribute
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Enum |
                    AttributeTargets.Event |
                    AttributeTargets.Field |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple = true, Inherited = false)]
    public sealed class UnsupportedOSPlatformAttribute : OSPlatformAttribute
    {
        public UnsupportedOSPlatformAttribute(string platformName);
    }

    // Same shape, but different namespace
    [AttributeUsage(AttributeTargets.Assembly |
                    AttributeTargets.Class |
                    AttributeTargets.Constructor |
                    AttributeTargets.Event |
                    AttributeTargets.Method |
                    AttributeTargets.Module |
                    AttributeTargets.Property |
                    AttributeTargets.Struct,
                    AllowMultiple=true, Inherited=false)]
    public sealed class ObsoletedInOSPlatformAttribute : OSPlatformAttribute
    {
        public ObsoletedInPlatformAttribute(string platformName);
        public ObsoletedInPlatformAttribute(string platformName, string message);
        public string Message { get; }
        public string Url { get; set; }
    }
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-Meta area-System.Runtime code-analyzer Marks an issue that suggests a Roslyn analyzer
Projects
None yet
Development

No branches or pull requests

6 participants