dotnet/roslyn

Possible diagnostic improvement in covariant returns with set accessor

Open

#43,794 opened on Apr 29, 2020

View on GitHub
 (0 comments) (0 reactions) (0 assignees)C# (4,257 forks)batch import
Area-CompilersConcept-Diagnostic ClarityFeature - Covariant Returnshelp wanted

Repository metrics

Stars
 (20,414 stars)
PR merge metrics
 (Avg merge 6d 17h) (256 merged PRs in 30d)

Description

In the review for https://github.com/dotnet/roslyn/pull/43673 @AlekseyTs commented on the test

        [Fact]
        public void CovariantReturns_14()
        {
            var source = @"
public class Base
{
    public virtual object P { get; set; }
}
public class Derived : Base
{
    public override string P { get => string.Empty; }
}
public class Derived2 : Derived
{
    public override string P { get => string.Empty; set { } }
}
";
            var comp = CreateCompilation(source, parseOptions: TestOptions.WithoutCovariantReturns).VerifyDiagnostics(
                // (8,28): error CS8652: The feature 'covariant returns' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version.
                //     public override string P { get => string.Empty; }
                Diagnostic(ErrorCode.ERR_FeatureInPreview, "P").WithArguments("covariant returns").WithLocation(8, 28),
                // (12,53): error CS0546: 'Derived2.P.set': cannot override because 'Derived.P' does not have an overridable set accessor
                //     public override string P { get => string.Empty; set { } }
                Diagnostic(ErrorCode.ERR_NoSetToOverride, "set").WithArguments("Derived2.P.set", "Derived.P").WithLocation(12, 53)
                );
            verify(comp);
            comp = CreateCompilation(source, parseOptions: TestOptions.WithCovariantReturns).VerifyDiagnostics(
                // (12,53): error CS0546: 'Derived2.P.set': cannot override because 'Derived.P' does not have an overridable set accessor
                //     public override string P { get => string.Empty; set { } }
                Diagnostic(ErrorCode.ERR_NoSetToOverride, "set").WithArguments("Derived2.P.set", "Derived.P").WithLocation(12, 53)
                );
            verify(comp);

            static void verify(CSharpCompilation comp)
            {
                VerifyOverride(comp, "Derived.P", "System.String Derived.P { get; }", "System.Object Base.P { get; set; }");
                VerifyOverride(comp, "Derived.get_P", "System.String Derived.P.get", "System.Object Base.P.get");
                VerifyOverride(comp, "Derived2.P", "System.String Derived2.P { get; set; }", "System.String Derived.P { get; }");
                VerifyOverride(comp, "Derived2.get_P", "System.String Derived2.P.get", "System.String Derived.P.get");
                VerifyNoOverride(comp, "Derived2.set_P");
            }
        }

regarding the diagnostic

error CS0546: 'Derived2.P.set': cannot override because 'Derived.P' does not have an overridable set accessor

I think this diagnostics is confusing because it implies that either there is no setter, or it is not virtual/overridable. I think we should improve that because before one could almost never run into this existing diagnostics and it would most likely mean what it says. For this scenario, the diagnostic should be improved to explain what is wrong in a way the people can understand.

Contributor guide