Test plan for "covariant returns"

[jcouv]

Championed issue: dotnet/csharplang#49
Spec: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-9.0/covariant-returns.md
Runtime spec: https://github.com/dotnet/runtime/blob/master/docs/design/features/covariant-return-methods.md

Test plan

• Test the following part of the specification: "The override method must have have a return type that is convertible by an identity or implicit reference conversion to the return type of the overridden base method."

  • Cover error cases when the condition isn't met
  • Cover success cases

• Test the following part of the specification: "The override method must have have a return type that is convertible by an identity or implicit reference conversion to the return type of every override of the overridden base method that is declared in a (direct or indirect) base type of the override method."

  • Cover error cases when the condition isn't met
  • Cover success cases
  • Cover success and error cases for new overrides when the methods in the base type hierarchy do not follow the outlined requirement (could be achieved when bases are imported from metadata and with employing retargeting, both methods should be covered). Example of a scenario:
    Coming from references:

public class C1{}
public class C2 : C1{}
public class C3 : C2{}
public class C4 : C3{}

public class Base1
{
     public virtual C1 M() => throw null; 
}

public class Base2 : Base1
{
     public virtual C3 M() => throw null; // Presumably added in a new version of the library defining Base2
}

public class Base3 : Base2 // Presumable defined in a different library 
{
     public virtual C2 M() => throw null; 
}

Test compile and runtime behavior for the following attempts to override:

public class Test1 : Base3
{
     public virtual C2 M() => throw null; 
}
public class Test2 : Base3
{
     public virtual C3 M() => throw null; 
}
public class Test1 : Base3
{
     public virtual C4 M() => throw null; 
}

• Test the following part of the specification: "the override method's return type must be at least as accessible as the override method." This constraint permits an override method in a private class to have a private return type. However it requires a public override method in a public type to have a public return type.

  • Cover error cases when the condition isn't met
  • Cover success cases

• Test scenarios outlined above for properties.

• Test the following part of the specification: "there shall be an identity conversion or (if the inherited property is read-only) implicit reference conversion from the type of the overriding property to the type of the inherited property."

  • Cover error cases when the condition isn't met
  • Cover success cases
  • Cover scenarios when a read-write property is overridden with a a property that has only getter and an attempt to use covariant return type is made. Including scenarios when there is a getter-only override for the same property in an intermediate base type.

• Test the following part of the specification: "Callers of the method or property would statically receive the more refined return type from an invocation." Cover both runtime and compile time effect of this. Verify IL generated.

• Symbol model

  • Verify the expected metadata information emitted for the overrides (MethodImpl entry, newslot flag, attributes, etc.)
  • Verify behavior of related symbol APIs
  • APIs should have consistent behavior across PE, Source and Retargeting symbols

• Metadata import

  • Verify able to import and properly interpret new metadata.
  • Test scenarios when an "overriding" MethodImpl cannot be resolved (for example, removed in a new version). What happens when we attempt to override an imported method like that? Should any use-site diagnostics be reported?
  • Verify behavior for an "unconventional" use of the metadata (MethodImpl entry, newslot flag, etc.)
    • Combining an "overriding" MethodImpl with interface implementation MethodImpls should be recognized and handled without problems
    • Test a scenario when there is a name mismatch with the method pointed to by the "overriding" MethodImpl.
    • Test scenarios when method explicitly overrides more than one method, including cases when the methods are related (i.e. different overrides of the same method) and are not related.

• Sealing overriding with a covariant return override. Verify that the compiler respects the sealing (i.e. doesn't allow to override the member in a more derived type). Verify that runtime respects the sealing.

• Confirm ref returns are not subject to the covariant return changes.

• Test binary compatibility scenarios when overrides are added or removed in bases in new version of the libraries. Cover compile and runtime behavior.

• Language version should be checked for an attempt to take advantage of the feature

• Runtime capability should checked for an attempt to take advantage of the feature

• Verify cannot use covariant return for an interface implementation

• Test interop with VB

• Test IDE behavior around overriding.

  • Override completion should give the proper return type, i.e. the most specific for the base.
  • Changing return type to most specific after completion doesn't have negative effect on IDE.
  • On override keyword, GoToDefinition should go to the most derived override above.
  • On a virtual method, GoToImplementation/FindAllReferences should find all overrides
  • "Implement abstract method" properly implements an abstract override that uses a covariant return.

• Test that we don't attempt to copy custom modifiers for return type when type changes.

• When the return type changes, make sure warnings about nullability mismatch in the type are reported as appropriate. Some examples of the scenarios:

using System.Collections.Generic;
#nullable enable
abstract class A
{
internal abstract IEnumerable<object> F();
}
class B : A
{
internal override List<object?> F() => default;
}

#nullable enable
abstract class A<T> where T : class
{
internal abstract T F();
}
class B<T, U> : A<T> where T : class where U : class?, T?
{
internal override U F() => default!;
}

Tests not yet implemented

• Test what happens when hidebysig/hidebyname appears in superclass metadata?
• Suggest additional binary compatibility tests (like BinaryCompatibility_*) but with a method newly inserted into the hierarchy (in place of Mid.M) where
  • new virtual method, we assume it will be newSlot
  • new but not virtual, we assume it will be newSlot
  • all of the above including original but when there is no hidebysig flag in metadata
• Verify that the runtime performs unification of virtual slots, for example, in a scenario like the test BinaryCompatibility_03
• Make sure tests such as TestAmbiguousOverridesRefOut in CodeGenOverridingAndHiding are tested on runtimes that support covariant returns (i.e. handle methodimpl correctly).
• Add a test in response to Covariant returns part 4 #44025 (review)
• Add PreserveBaseOverridesAttribute to the spec

Tests believed to already exist

• Test code generated for the use of covariant methods from VB: [TestVBConsumption_01]
  • Calling a method
  • base call
  • Delegate creation (including base call)
  • Any of the above in an expression tree.
  • Attempted override of a covariant method.
• Test the use of covariant override in a delegate creation expression (all forms, including base call) [InDelegateCreation_01]
• Test the use of covariant override inside expression trees (calling, delegate creation) [InExpressionTree_01]
• Verify that the emitted call instruction targets a method with the correct return type. [Most of the new tests]
• Test base.M() calls of a covariant override. [Most of the new tests]
• Test nullability variance (COut<object?>vs.COut<string!> and some permutations). [NestedVariance_01, NestedVariance_02]
• [Please] test the following scenarios:
  • 1. an attempt to override a get/set property with a get-set property [See CovariantReturnTests.CovariantReturns_WritableProperties]
  • 2. an attempt to override a get/set property with a get-only property [See CovariantReturnTests.CovariantReturns_13]
  • 3. similar as the second case only with next level derived type attempting to override with get/set or set-only property [CovariantReturns_14, CovariantReturns_15]
  • 4. similar as the second case only with next level derived type attempting to override with more specific get-only property [CovariantReturns_16]
• Test that the type received at the call site is the type from the most derived method matching the call site. [Most tests in CovariantReturnTests]
• Type for call in consumption from VB [TestVBConsumption_*]

Runtime:

• runtime support (PR Covariant Returns Feature runtime#35308)
• add BCL type (RequireMethodImplToRemainInEffectAttribute)