Consider flag platform specific constructor called implicitly in derived types

[buyaa-n]

Platform attributes not derived by design, this means when A has platform-specific attribute and B derived from A is not counted as platform-specific. But if B's constructor is implicitly calling the base default constructor then it might need to flag that, especially when the base constructor does any platform-specific operation. As shown below example if derived type calls any platform-specific API from the base it would warn, including explicit call to the base constructor, but implicit call will not be caught

    [SupportedOSPlatform("windows")]
    class A
    {
        public A() 
       {
            // some platform specific operation
        } 
        public void M1() { }
    }

    class B : A
    {
        public void Test()
        {
            A a = new A(); // warns as expected
            B b = new B(); // not warn, but could warn as it will call base constructor implicitly

            a.M1(); // warns as expected
            b.M1(); // warns as expected
        }
    }

I could propose 2 way of covering this:

1. If the child constructor has no explicit call to parent platform-specific type check if the parent default constructor is implemented and warn if it is doing any platform-specific operation. This solution will do more computation but might eliminate false positives.
2. If the child constructor has no explicit call to parent platform-specific type warn unconditionally. This solution will be easier but might cause false positives

cc @jeffhandley @terrajobst

When we cover this scenario we also could consider to flag generic type constructors @mavasani suggested such as below:

using System.Runtime.Versioning;

class GenericClass<T>
{
}

[SupportedOSPlatform("windows")]
class WindowsOnlyType { }

class DerivedWindowsOnlyType : GenericClass<WindowsOnlyType> { }
class Test
{
    void M<T>()
    {
        var t = new DerivedWindowsOnlyType();  // Should be flagged
    }
}

Originally posted by @mavasani in #4390 (comment)

[terrajobst]

We should also consider warning on the inverse, i.e. when someone adds platform-specific annotations on derived types because one could argue that polymorphism breaks the analysis.

For example, if I derive from a non-platform specific type (or interface) and add platform-specific functionality callers might not get any warnings when they consume the type through the base. Same applies to overrides.
​
Maybe we should warn when a user adds platform constraints to overrides or interface implementations and instead tell them to mark the constructor of that type as platform specific. This ensures that someone, somewhere will get a warning. And for polymorphic types the constructor/factory method/static singleton property seems to be the appropriate place for that.

[buyaa-n]

We should also consider warning on the inverse, i.e. when someone adds platform-specific annotations on derived types because one could argue that polymorphism breaks the analysis.

For example, if I derive from a non-platform specific type (or interface) and add platform-specific functionality callers might not get any warnings when they consume the type through the base. Same applies to overrides.

Good point

Maybe we should warn when a user adds platform constraints to overrides or interface implementations and instead tell them to mark the constructor of that type as platform specific.

I think if we suggest to mark constructor instead of the overridden method it would be the same as marking the entire type platform-specific, which would bad if the type has other APIs not platform-specific. Isn't it better to suggest to mark the base type method instead of suggesting to mark constructor?

    interface A {
        [SupportedOSPlatform("windows")]
        public void M1();
        public void M2();  // maybe suggest to mark this one? 
    }
    class D : A
    {
        public void M1() { }
        [SupportedOSPlatform("windows")]
        public void M2() { }
        public void Test()
        {
            A a = new D(); // not warn, you mean we could warn here?
            D d = new D(); // not warn, and here?
            a.M1(); // warns as expected
            d.M1(); // not warns as expected
            a.M2(); // not warn, for me, it would be great if we could warn here instead of constructor
            d.M2(); // warns as expected
        }
    }

[terrajobst]

Isn't it better to suggest to mark the base type method instead of suggesting to mark constructor?

In principle yes, but that's not always actionable because I may not own the type. For example, let's say I need to implement a framework provided abstraction like IComparable<T> or TextWriter.

[buyaa-n]

In principle yes, but that's not always actionable because I may not own the type. For example, let's say I need to implement a framework provided abstraction like IComparable or TextWriter.

Makes sense, if we decide to suggest to move the attribute to the constructor that could be covered with the other analyzer for platform attributes consistency

[jeffhandley]

We've not seen many hits of this; I'm tempted to close it out. @buyaa-n -- any objections?

[buyaa-n]

We've not seen many hits of this; I'm tempted to close it out. @buyaa-n -- any objections?

Not sure if we should close it, probably haven't seen many hits because it is an edge case, but I think it is a valid case that need to be covered, maybe move to 7.0?

Tagging @terrajobst for his opinion

[jeffhandley]

Since this is a rare corner case that we haven't seen more hits of, I don't expect we'll make time for it in .NET 7. I'm going to go ahead and close it out. If folks stumble upon this behavior in the future, please file a new issue presenting the scenario in which you encountered it, and we can reconsider.