New Analyzer: Implement Generic Math Interfaces Correctly

[buyaa-n]

Fixes dotnet/runtime#69775

With Static abstract members in interfaces the Generic Math Support feature uses the Curiously Recurring Template Pattern (CRTP) to enable scenarios where interfaces can declare that a method takes or returns the concrete type that implements the interface. For example: https://github.com/dotnet/runtime/blob/6ca8c9bc0c4a5fc1082c690b6768ab3be8761b11/src/libraries/System.Private.CoreLib/src/System/IParsable.cs#L10-L27

public interface IParsable<TSelf> where TSelf : IParsable<TSelf>
{
    /// <summary>Parses a string into a value.</summary>
    /// <param name="s">The string to parse.</param>
    /// <param name="provider">An object that provides culture-specific formatting information about <paramref name="s" />.</param>
    /// <returns>The result of parsing <paramref name="s" />.</returns>
    static abstract TSelf Parse(string s, IFormatProvider? provider);

    /// <summary>Tries to parses a string into a value.</summary>
    /// <param name="s">The string to parse.</param>
    /// <param name="provider">An object that provides culture-specific formatting information about <paramref name="s" />.</param>
    /// <param name="result">On return, contains the result of succesfully parsing <paramref name="s" /> or an undefined value on failure.</param>
    /// <returns><c>true</c> if <paramref name="s" /> was successfully parsed; otherwise, <c>false</c>.</returns>
    static abstract bool TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out TSelf result);
}

In this example, TSelf will be filled by the deriving type with its own type:

public readonly struct DateOnly : IParsable<DateOnly>

The Issue

An issue is that nothing is enforcing that a type implementing a curiously recurring template interface fills the generic type with its same type. For example:

public readonly struct MyDate : IParsable<DateOnly>

The above definition will compile successfully - it is a valid definition from a language perspective. However, MyDate cannot be passed to the above ParseAndWrite<T>(string data) method. Trying to use MyDate in this way will result in a compiler error:

ParseAndWrite<MyDate>("2022-05-24"); // error CS0315: The type 'MyDate' cannot be used as type parameter 'T' in the generic type or method 'ParseAndWrite<T>(string)'. There is no boxing conversion from 'MyDate' to 'System.IParsable<MyDate>'.

Which is confusing. A similar error CS0311 is raised if MyDate is a class.

Even worse, the MyDate type could have been shipped publicly in a library. And only once consumers of this library try to use it as IParsable<DateOnly>, they get the compiler error. When the real error was that MyDate was not defined correctly.

In the future, we hope to adopt a full-fledged self-type mechanism (such as dotnet/csharplang#5413) in the Generic Math interfaces. We would like to enforce that any implementers of the Generic Math interfaces now would not be broken with the adoption of a self-type C# feature in the future.

The Analyzer Requirement:

The analyzer should be thought of as a stop gap solution for generic math until there is a C# feature.
We will have list of generic math interfaces that analyzer would use, the analyzer will flag any Type that implements one of the interfaces and fills TSelf with a non-generic Type other than itself. If the TSelf is filled with another generic Type, such as:

public readonly struct MyDate<TSelf> : IParsable<TSelf>
    where TSelf : IParsable<TSelf>

The analyzer will not flag the above Type. But should flag:

public readonly struct MyDate : IParsable<DateOnly>

The list of generic math interfaces

* `IParsable`
* `ISpanParsable`
* `IAdditionOperators`
* `IAdditiveIdentity`
* `IBinaryFloatingPointIeee754`
* `IBinaryInteger`
* `IBinaryNumber`
* `IBitwiseOperators`
* `IComparisonOperators`
* `IDecrementOperators`
* `IDivisionOperators`
* `IEqualityOperators`
* `IExponentialFunctions`
* `IFloatingPointIeee754`
* `IFloatingPoint`
* `IHyperbolicFunctions`
* `IIncrementOperators`
* `ILogarithmicFunctions`
* `IMinMaxValue`
* `IModulusOperators`
* `IMultiplicativeIdentity`
* `IMultiplyOperators`
* `INumberBase`
* `INumber`
* `IPowerFunctions`
* `IRootFunctions`
* `IShiftOperators`
* `ISignedNumber`
* `ISubtractionOperators`
* `ITrigonometricFunctions`
* `IUnaryNegationOperators`
* `IUnaryPlusOperators`
* `IUnsignedNumber`

Severity: Warning
Category: Usage

[codecov]

Codecov Report

Merging #6126 (ca74d7b) into main (604771c) will increase coverage by 0.01%.
The diff coverage is 99.07%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6126      +/-   ##
==========================================
+ Coverage   96.02%   96.03%   +0.01%     
==========================================
  Files        1343     1351       +8     
  Lines      309474   310585    +1111     
  Branches     9950     9986      +36     
==========================================
+ Hits       297184   298283    +1099     
- Misses       9890     9901      +11     
- Partials     2400     2401       +1     

[eerhardt]

This LGTM. Thanks for the great work here @buyaa-n.

[tannergooding]

Tests coverage looks to cover the scenarios that I was thinking about and the code generally makes sense to me. Not an expert in reviewing analyzer code, however :)

[buyaa-n]

Thanks for the thorough review @Youssef1313 @eerhardt @tannergooding

@mavasani @jmarolf @JoeRobich this one of the new analyzers we want to merge in 7.0, please take a look and let us know if we need to get any additional approval. The analyzer severity is warning but it is only apply for new APIs added in 7.0 (some of them have been a preview feature in 6.0), therefore I would not expect it will broke people plus if they had this issue they would most likely would like to know/warned as the APIs would not function properly to if not implemented properly

CC @jeffhandley

[jeffhandley]

Looks great!

[buyaa-n]

Ping @jmarolf again, we need this merged in 7.0

[buyaa-n]

I am gonna merge the PR now, feel free to leave feedbacks, I will apply them separately