Possible diagnostic improvement in covariant returns with set accessor
#43,794 opened on Apr 29, 2020
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.