Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Value objects: ToString override is not inherited #43617

Closed
tnotheis opened this issue Sep 23, 2024 · 3 comments
Closed

Value objects: ToString override is not inherited #43617

tnotheis opened this issue Sep 23, 2024 · 3 comments
Labels
Area-Workloads untriaged Request triage from a team member

Comments

@tnotheis
Copy link

Describe the bug

(I'm not sure whether this is the correct repo for this issue)

Given two records Parent and Child, where Parent overrides the ToString method. When I call ToString on an instance of Child, ToString of Parent is not called. Instead, the one of object seems to be used.

To Reproduce

Given the following code:

public record Parent(string Value)
{
    public override string ToString() => Value;
}

public record Child(string Value) : Parent(Value);

I would expect the following test to succeed:

public void TestToString() 
{
    var child = new Child("value");
    
    var actual = child.ToString();

    Assert.Equal("value", actual);
}

But it fails for the following reason:

Assert.Equal() Failure: Strings differ
           ↓ (pos 0)
Expected: "value"
Actual:   "Child { Value = value }"
           ↑ (pos 0)

When debugging this, I noticed that ToString of Parent is never called.

This can be "fixed" by overriding the ToString method in Child as well:

public record Child(string Value) : Parent(Value)
{
    public override string ToString() => base.ToString();
}

But I refuse to believe that's how it's supposed to work.

Further technical details

.NET SDK:
 Version:           8.0.401
 Commit:            811edcc344
 Workload version:  8.0.400-manifests.7e29de6a
 MSBuild version:   17.11.4+37eb419ad

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\8.0.401\

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
 [ios]
   Installation Source: VS 17.10.35013.160
   Manifest Version:    17.2.8053/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.ios\17.2.8053\WorkloadManifest.json
   Install Type:        FileBased

 [aspire]
   Installation Source: VS 17.10.35013.160
   Manifest Version:    8.0.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.0.0\WorkloadManifest.json
   Install Type:        FileBased

 [android]
   Installation Source: VS 17.10.35013.160
   Manifest Version:    34.0.95/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.android\34.0.95\WorkloadManifest.json
   Install Type:        FileBased

 [maui-windows]
   Installation Source: VS 17.10.35013.160
   Manifest Version:    8.0.40/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maui\8.0.40\WorkloadManifest.json
   Install Type:        FileBased

 [maccatalyst]
   Installation Source: VS 17.10.35013.160
   Manifest Version:    17.2.8053/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.maccatalyst\17.2.8053\WorkloadManifest.json
   Install Type:        FileBased


Host:
  Version:      8.0.8
  Architecture: x64
  Commit:       08338fcaa5

.NET SDKs installed:
  7.0.317 [C:\Program Files\dotnet\sdk]
  8.0.302 [C:\Program Files\dotnet\sdk]
  8.0.401 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.31 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.31 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.8 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.31 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.8 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Workloads untriaged Request triage from a team member labels Sep 23, 2024
@KalleOlaviNiemitalo
Copy link

Based on https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/record#built-in-formatting-for-display and https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-9.0/records, this seems to be by design. Because the source code of record Child does not override ToString(), the compiler generates an override, and that one calls this.PrintMembers rather than base.ToString.

If the C# language were changed so that the compiler does not generate a ToString() override in record Child when its base class is record Parent, then the generated Parent.ToString() method would also have to be changed not to hardcode the "Parent" name in the output. I didn't find any discussion about that in dotnet/csharplang#39 nor in the language design meetings linked from there. At this point, it would be a breaking change.

@tnotheis
Copy link
Author

Hmm, I see. That's very unfortunate for me. But I understand why it was done that way.

It would be nice to have a way to control whether ToString is generated or not. However, I don't see an elegant solution for this.

@tnotheis
Copy link
Author

A colleague of mine just pointed out to me that there is actually a solution for this already. Starting with C# 10, sealing the ToString method in Parent prevents the compiler from generating a ToString method in Child (see https://learn.microsoft.com/en-us/dotnet/csharp/whats-new/csharp-10#record-types-can-seal-tostring).

While this may not be a solution in some cases, it certainly is in my case. So I will close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Workloads untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

2 participants