Add spec for platform exclusion

[terrajobst]

No description provided.

[eerhardt]

Looks like a good start to me. Some early thoughts.

[eerhardt]

Looking at dotnet/sdk#12547, we already have an MSBuild item named SupportedTargetPlatform. Are we intending this to be the same thing as that? If not, these names are too similar to both be used.

cc @dsplaisted @sfoslund

[sfoslund]

Right now we're using SupportedTargetPlatform as an itemgroup containing the supported TargetPlatformVersions that correspond to the specified TargetPlatformIdentifier.

[terrajobst]

So no then 😄 . Your notion of supported is "what can this SDK produce" while mine is "what platforms is the author of this project file intending to support". We probably need a different name then.

[eerhardt]

Your notion of supported is "what can this SDK produce"

Should we rename the SDK itemgroup then? I think the name SupportedTargetPlatform sounds like it is what target platforms the current project supports, not necessarily what the SDK supports.

[terrajobst]

@dsplaisted were should I file a bug to discuss renaming the SDK's SupportedTargetPlatform item group?

[dsplaisted]

In dotnet/sdk.

Note that there was already a SupportedTargetFramework item which we use to populate the TargetFramework dropdown on the VS property page, so we went with the same pattern for SupportedTargetPlatform.

[terrajobst]

Makes sense. In that sense I think we should come up with a new name for the analyzer. Let me chew on that.

[jeffhandley]

I keep thinking about how the crux of the challenge we're having is that marking a specific OSPlatform as supported implies that no other platforms are supported, but we don't always want that behavior.

[MinimumOSPlatform("windows7.0")]
public void WindowsOnlyAPI() {}

[MinimumOSPlatform("ios14")]
public void WorksEverywhereButOnIosVersion14IsRequired() {}

If we set that problem aside for a moment, and we focus purely on excluding an OSPlatform, then I think we're otherwise OK. The following would just work, wouldn't it?

[UnsupportedOSPlatform("browser")]
public void WorksEverywhereExceptBrowser() {}

I believe the ranges were being introduced so that we could use [UnsupportedOSPlatform] to denote a combination of supported and unsupported, specifying which version ranges are unsupported, leaving anything outside the range as supported without implying exclusive platform support. Using [UnsupportedOSPlatform] in that way feels awkward to me, and less expressive than using [MinimumOSPlatform] to indicate support.

I'm really concerned about the range support, regardless of the syntax. It makes the data harder to reason over as a human code reader, and potentially more brittle when reasoning over in the analyzer implementation. I think we'd introduce a higher likelihood that the data will be incomprehensible and then the analyzer will need to introduce more diagnostics for when that's the case.

So what if we did the following?

1. Rename RemovedInOSPlatformAttribute to UnsupportedOSPlatformAttribute, but otherwise leave it as-is
   • This specifies the lower-bound version in which an API became unsupported
2. Add a property to MinimumOSPlatformAttribute that indicates whether or not the platform should be treated as exclusive

I think we could then satisfy our scenarios:

[MinimumOSPlatform("windows7.0")]
public void WindowsOnlyAPI() {}

[MinimumOSPlatform("ios14.0", IsExclusive = false)]
public void WorksEverywhereButOnIosVersion14IsRequired() {}

[UnsupportedOSPlatform("browser")]
public void WorksEverywhereExceptBrowser() {}

The evolution of an API going from unsupported to having a minimum version required for support would involve changing [UnsupportedOSPlatform("ios")] to [MinimumOSPlatform("ios14", IsExclusive = false)]. If there are multiple platforms that add support over time, then they would each need to specify IsExclusive = false.

To show an edge case where something comes in and out of support for a platform repeatedly:

[MinimumOSPlatform("ios12.0", IsExclusive = false)]
[UnsupportedOSPlatform("ios14.0")]
[MinimumOSPlatform("ios12.0", IsExclusive = false)]
[MinimumOSPlatform("ipados16.0", IsExclusive = false)]
public void Worked_In_12_And_13_But_Not_14_Or_15_But_Works_Again_In_16() {}

If this approach has merit, then I'd suggest that we also rename MinimumOSPlatformAttribute to SupportedOSPlatformAttribute. Both SupportedOSPlatform and UnsupportedOSPlatform would accept a platform name with an optional version such that the version is always treated as the lower-bound.

By default, a [SupportedOSPlatform] attribute denotes exclusive support on that platform. But specifying IsExclusive = false on the attribute allows support to be turned on for the platform without affecting support on other platforms. An [UnsupportedOSPlatform] attribute never implies anything about any other platforms.

To address the [concern of introducing another means for obsoleting an API]([MinimumOSPlatform("ios12.0", IsExclusive = false)]), I suggest we remove the ObsoletedInOSPlatformAttribute that we introduced.

To summarize, I propose that we:

1. Rename RemovedInOSPlatformAttribute to UnsupportedOSPlatformAttribute
2. Rename MinimumOSPlatformAttribute to SupportedOSPlatformAttribute
3. Remove ObsoletedInOSPlatformAttribute
4. Add an IsExclusive Boolean property to SupportedOSPlatformAttribute, that defaults to true

[jeffhandley]

Thank you @terrajobst! I left some comments and primary have 2 topics that will need resolution:

1. Should we use a params string[] platformNames constructor with the iOS family of platforms in mind?
2. I think we had problems with the || based guards; I'll need @buyaa-n to weigh in there.

[buyaa-n]

2. I think we had problems with the || based guards; I'll need @buyaa-n to weigh in there.

This is a more meaningful scenario than the other || example we discussed before, for this one we should try to add support