[Analyzer]: Enforce Curiously Recurring Template Pattern in Generic Math interfaces

[eerhardt]

Suggested severity: Warning
Suggested category: Usage

Background

With the introduction of Static abstract members in interfaces and Generic Math Support, a common pattern emerging is to use 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:

runtime/src/libraries/System.Private.CoreLib/src/System/IParsable.cs

Lines 10 to 27 in 6ca8c9b

	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>
	/// <exception cref="ArgumentNullException"><paramref name="s" /> is <c>null</c>.</exception>
	/// <exception cref="FormatException"><paramref name="s" /> is not in the correct format.</exception>
	/// <exception cref="OverflowException"><paramref name="s" /> is not representable by <typeparamref name="TSelf" />.</exception>
	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>

This allows for developers to write methods that accept an IParsable<T>, and call Parse on data passed to the method. The method can be written generically without knowing exactly which parsable data type is used to fill the TSelf.

static void ParseAndWrite<T>(string data)
    where T : IParsable<T>
{
    T parsed = T.Parse(data, null);
    Console.WriteLine(parsed);
}

// caller
ParseAndWrite<DateOnly>("2022-05-24");

// prints (in en-US):
// 5/24/2022

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.

Proposal

We create an analyzer that knows about the generic math interfaces using the CRTP:

• IParsable<TSelf>
• ISpanParsable<TSelf>
• IAdditionOperators<TSelf, TOther, TResult>
• IAdditiveIdentity<TSelf, TResult>
• IBinaryFloatingPointIeee754<TSelf>
• IBinaryInteger<TSelf>
• IBinaryNumber<TSelf>
• IBitwiseOperators<TSelf, TOther, TResult>
• IComparisonOperators<TSelf, TOther>
• IDecrementOperators<TSelf>
• IDivisionOperators<TSelf, TOther, TResult>
• IEqualityOperators<TSelf, TOther>
• IExponentialFunctions<TSelf>
• IFloatingPointIeee754<TSelf>
• IFloatingPoint<TSelf>
• IHyperbolicFunctions<TSelf>
• IIncrementOperators<TSelf>
• ILogarithmicFunctions<TSelf>
• IMinMaxValue<TSelf>
• IModulusOperators<TSelf, TOther, TResult>
• IMultiplicativeIdentity<TSelf, TResult>
• IMultiplyOperators<TSelf, TOther, TResult>
• INumberBase<TSelf>
• INumber<TSelf>
• IPowerFunctions<TSelf>
• IRootFunctions<TSelf>
• IShiftOperators<TSelf, TResult>
• ISignedNumber<TSelf>
• ISubtractionOperators<TSelf, TOther, TResult>
• ITrigonometricFunctions<TSelf>
• IUnaryNegationOperators<TSelf, TResult>
• IUnaryPlusOperators<TSelf, TResult>
• IUnsignedNumber<TSelf>

The analyzer will flag any Type that implements one of the above 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 Type.

See Also

https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-04-11.md#self-type-stopgap-attribute

Rejected Alternatives

1. Add an attribute [SelfTypeAttribute] that could be applied to the generic parameter of interfaces such as IParsable<[SelfType] TSelf>. The analyzer wouldn't recognize the recursive pattern itself, but rely on the interface being attributed.

2. Key the analyzer off of the generic type named, literally TSelf.

3. Add an analyzer that flags types that implement a CRTP interface without passing in the same type as the TSelf parameter. The situation where the analyzer flags code is when a Type implements an interface such that:

   1. the interface is generic
   2. one generic type parameter is recursive (i.e. where T : interface<T>)
   3. the interface exposes at least one static abstract member (either directly or through inheritance)

When the above rules are met, the analyzer will flag any Type that implements the interface and fills T with a non-generic Type other than itself. If the T 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 Type.

cc @jeffhandley @tannergooding @buyaa-n

[ghost]

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

Issue Details

---

Suggested severity: Warning
Suggested category: Usage

Background

With the introduction of Static abstract members in interfaces and Generic Math Support, a common pattern emerging is to use 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:

runtime/src/libraries/System.Private.CoreLib/src/System/IParsable.cs

Lines 10 to 27 in 6ca8c9b

	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>
	/// <exception cref="ArgumentNullException"><paramref name="s" /> is <c>null</c>.</exception>
	/// <exception cref="FormatException"><paramref name="s" /> is not in the correct format.</exception>
	/// <exception cref="OverflowException"><paramref name="s" /> is not representable by <typeparamref name="TSelf" />.</exception>
	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>

This allows for developers to write methods that accept an IParsable<T>, and call Parse on data passed to the method. The method can be written generically without knowing exactly which parsable data type is used to fill the TSelf.

static void ParseAndWrite<T>(string data)
    where T : IParsable<T>
{
    T parsed = T.Parse(data, null);
    Console.WriteLine(parsed);
}

// caller
ParseAndWrite<DateOnly>("2022-05-24");

// prints (in en-US):
// 5/24/2022

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.

Proposal

We add an analyzer that flags types that implement a CRTP interface without passing in the same type as the TSelf parameter. The situation where the analyzer flags code is when a Type implements an interface such that:

1. the interface is generic
2. one generic type parameter is recursive (i.e. where T : interface<T>)
3. the interface exposes at least one static abstract member (either directly or through inheritance)

When the above rules are met, the analyzer will flag any Type that implements the interface and fills T with a non-generic Type other than itself. If the T 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 Type.

Rejected Alternatives

1. Add an attribute [SelfTypeAttribute] that could be applied to the generic parameter of interfaces such as IParsable<[SelfType] TSelf>. The analyzer wouldn't recognize the recursive pattern itself, but rely on the interface being attributed.
2. Key the analyzer off of the generic type named, literally TSelf.

cc @jeffhandley @tannergooding @buyaa-n

Author:	eerhardt
Assignees:	-
Labels:	api-suggestion, area-Meta, code-analyzer
Milestone:	7.0.0

[stephentoub]

@tannergooding, wasn't this part of the plan / an outcome from https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-04-11.md#self-type-stopgap-attribute? (but I thought we were going to do it specifically for the exact types in generic math rather than create something more general that might conflict with a future self type mechanism in the language)

[tannergooding]

@stephentoub, could you elaborate on how you think this might conflict?

This is restricted to the "exact" scenario that is impacted and equally applies to our own types as to types an end user might also expose. In both cases, it prevents consumers from doing something that may prevent that type from switching to a future self type because the switch would then be breaking for the declared type.

[stephentoub]

could you elaborate on how you think this might conflict?

Because we don't know yet what the future self type feature will be; it hasn't been designed. We're basically inventing our own, via an analyzer. If it's limited to specific interfaces, then it's just a statement around how those interfaces are to be used. Elevating it to the level of a pattern is much broader reaching, especially with a default level of warning, and isn't about our own types: the proposed heuristic is all about how specific language features are used.

[tannergooding]

I'm fine with limiting this explicitly to our own interfaces.

Users who need the same limitation, such as because they want to have the same future support can just clone our analyzer.

[ghost]

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

Issue Details

---

Suggested severity: Warning
Suggested category: Usage

Background

With the introduction of Static abstract members in interfaces and Generic Math Support, a common pattern emerging is to use 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:

runtime/src/libraries/System.Private.CoreLib/src/System/IParsable.cs

Lines 10 to 27 in 6ca8c9b

	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>
	/// <exception cref="ArgumentNullException"><paramref name="s" /> is <c>null</c>.</exception>
	/// <exception cref="FormatException"><paramref name="s" /> is not in the correct format.</exception>
	/// <exception cref="OverflowException"><paramref name="s" /> is not representable by <typeparamref name="TSelf" />.</exception>
	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>

This allows for developers to write methods that accept an IParsable<T>, and call Parse on data passed to the method. The method can be written generically without knowing exactly which parsable data type is used to fill the TSelf.

static void ParseAndWrite<T>(string data)
    where T : IParsable<T>
{
    T parsed = T.Parse(data, null);
    Console.WriteLine(parsed);
}

// caller
ParseAndWrite<DateOnly>("2022-05-24");

// prints (in en-US):
// 5/24/2022

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.

Proposal

We create an analyzer that knows about the generic math interfaces using the CRTP:

• IParsable<TSelf>
• ISpanParsable<TSelf>
• IAdditionOperators<TSelf, TOther, TResult>
• IAdditiveIdentity<TSelf, TResult>
• IBinaryFloatingPointIeee754<TSelf>
• IBinaryInteger<TSelf>
• IBinaryNumber<TSelf>
• IBitwiseOperators<TSelf, TOther, TResult>
• IComparisonOperators<TSelf, TOther>
• IDecrementOperators<TSelf>
• IDivisionOperators<TSelf, TOther, TResult>
• IEqualityOperators<TSelf, TOther>
• IExponentialFunctions<TSelf>
• IFloatingPointIeee754<TSelf>
• IFloatingPoint<TSelf>
• IHyperbolicFunctions<TSelf>
• IIncrementOperators<TSelf>
• ILogarithmicFunctions<TSelf>
• IMinMaxValue<TSelf>
• IModulusOperators<TSelf, TOther, TResult>
• IMultiplicativeIdentity<TSelf, TResult>
• IMultiplyOperators<TSelf, TOther, TResult>
• INumberBase<TSelf>
• INumber<TSelf>
• IPowerFunctions<TSelf>
• IRootFunctions<TSelf>
• IShiftOperators<TSelf, TResult>
• ISignedNumber<TSelf>
• ISubtractionOperators<TSelf, TOther, TResult>
• ITrigonometricFunctions<TSelf>
• IUnaryNegationOperators<TSelf, TResult>
• IUnaryPlusOperators<TSelf, TResult>
• IUnsignedNumber<TSelf>

The analyzer will flag any Type that implements one of the above 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 Type.

See Also

https://github.com/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-04-11.md#self-type-stopgap-attribute

Rejected Alternatives

1. Add an attribute [SelfTypeAttribute] that could be applied to the generic parameter of interfaces such as IParsable<[SelfType] TSelf>. The analyzer wouldn't recognize the recursive pattern itself, but rely on the interface being attributed.

2. Key the analyzer off of the generic type named, literally TSelf.

3. Add an analyzer that flags types that implement a CRTP interface without passing in the same type as the TSelf parameter. The situation where the analyzer flags code is when a Type implements an interface such that:

   1. the interface is generic
   2. one generic type parameter is recursive (i.e. where T : interface<T>)
   3. the interface exposes at least one static abstract member (either directly or through inheritance)

When the above rules are met, the analyzer will flag any Type that implements the interface and fills T with a non-generic Type other than itself. If the T 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 Type.

cc @jeffhandley @tannergooding @buyaa-n

Author:	eerhardt
Assignees:	-
Labels:	api-suggestion, area-System.Numerics, code-analyzer
Milestone:	7.0.0

[eerhardt]

Thanks, @stephentoub and @tannergooding. I have updated the top proposal to follow the recommended approach.

[eerhardt]

Marking as "ready for review".

[tannergooding]

I added blocking as well to ensure this gets review for .NET 7 as part of the generic math reqs

[buyaa-n]

Suggested category: Usage

I would put it in Reliability category as it looks more about correctness

[tannergooding]

This isn't just reliability, its a usage requirement around the APIs so we can make a source breaking change in the future.

[terrajobst]

Video

• We rejected that attribute based approach because there is a chance this will become a general C# feature. Having a BCL attribute and analyzer would sort of create a competing language feature.
• The analyzer should be thought of as a stop gap solution for generic math until there is a C# feature. Whether or not we'll be able to use that is TBD, and largely depends on how the C# feature is expressed in metadata and whether we can update the generic math APIs in a binary compatible way.

Severity: Warning
Category: Usage

[carlossanlop]

@tannergooding this got reviewed and approved. Do we still want it in 7.0?

[tannergooding]

I believe yes and the intent is to get this worked on post code complete.
CC. @jeffhandley

[jeffhandley]

This analyzer work will happen after the RC1 snap, so this issue will carry past our "zero bugs" goal at the snap.