dotnet/csharplang

[Proposal]: PrintMembersIgnoreAttribute for records

Youssef1313 opened this issue · 7 comments

PrintMembersIgnoreAttribute for records

  • Proposed
  • Prototype: Not Started
  • Implementation: Not Started
  • Specification: Not Started

Summary

Currently, there is no way for the developer to exclude specific fields or properties from record's PrintMembers.

Motivation

  1. If a record contains an instance member of its same type, PrintMembers will get called recursively forever. Converting the instance member to be a static is one way to fix this, but being able to completely exclude an instance member seems to be a great addition.

  2. If one of the properties doesn't have an overridden ToString() implementation. It's useless to be printed, the developer might want to exclude it, but in that case marking it as static wouldn't be always an available option.

Detailed design

A new attribute is introduced:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public class PrintMembersIgnoreAttribute : Attribute { }

If GenerateMethodBody in SynthesizedRecordPrintMembers found the field/property being decorated with this attribute, it shouldn't include it.

Drawbacks

Alternatives

Unresolved questions

  1. If this attribute is applied to non-record members, should the compiler warn and ignore it, or error it completely?
  2. Should the developer be able to force including statics and constants with a similar attribute (e.g. ForceIncludeInPrintMembersAttribute)?

Design meetings

If we were going to allow a user to force the inclusion of a member, we could just use one attribute. For example:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public class PrintMembersInclusion : Attribute
{
    public PrintMembersInclusion(bool shouldInclude)
    {
        this.ShouldInclude = shouldInclude;
    }

    public bool ShouldInclude { get; }
}

This could then be applied as follows, with the described behaviour:

record TestRecord
{
    [PrintMembersInclusion(shouldInclude: true)] // Included. Overrides default behaviour for constants.
    public const string PublicConstantString = "This is a string.";

    [PrintMembersInclusion(shouldInclude: true)] // Included. Overrides default behaviour for static readonly fields.
    public static readonly int PublicStaticReadonlyInt = 42;

    [PrintMembersInclusion(shouldInclude: false)] // No effect. Field is already ignored.
    private static readonly int PrivateStaticReadonlyInt = 1788;

    [PrintMembersInclusion(shouldInclude: true)] // No effect. Property is already included.
    public string PublicInstanceString { get; }

    [PrintMembersInclusion(shouldInclude: false)] // Not included. Overrides default behaviour for public properties.
    public int PublicInstanceInt { get; }

    [PrintMembersInclusion(shouldInclude: true)] // Included. Overrides default behaviour for private properties.
    private bool PrivateBool { get; }
}

Seems like something that could be handled by a source generator which emits the appropriate PrintMembers method into your record. Having the compiler manage that out-of-the-box would be wading into policy territory that the team has expressed wanting to avoid. I think it's be difficult to argue that the equality should not be influenced based on attributes if something like printing members is.

I would agree with @HaloFour's sentiment here. This, particularly the potential for stack overflows, was explicitly discussed, and we feel that the scenarios you're going to want to have a custom Equals implementation are either the same or very similar to the scenarios that you'll want a custom ToString implementation (and GetHashCode). imo, this is something best addressed in a source generator.

Thanks @HaloFour and @333fred. This is indeed achievable with source generators.

If anyone is interested, I started working on a simple source generator for this purpose.

@HaloFour

Seems like something that could be handled by a source generator which emits the appropriate PrintMembers method into your record. Having the compiler manage that out-of-the-box would be wading into policy territory that the team has expressed wanting to avoid. I think it's be difficult to argue that the equality should not be influenced based on attributes if something like printing members is.

The generated Equals and GetHashCode only read fields, while PrintMembers and ToString read all 'printable members (non-static public field and readable property members)', which in particular includes calculated properties. It's easy to shoot yourself with this. I just did with properties for casting purposes, along the following lines:

record TypeA(int Value) {
  public TypeB TypeB => new TypeB(Value);
}

record TypeB(long Value) {
  public TypeA TypeA => new TypeA((int)Value);
}

An attribute for excluding properties would be a convenient way to fix ToString here. I don't understand the slippery slope argument with regard to equality.

@stephan-tolksdorf

I don't understand the slippery slope argument with regard to equality.

It's not in regard to equality, it is in regard with having policy drive the language's behavior. The compiler offers an escape hatch in that if you provide an implementation of PrintMembers then it will not synthesize one for you, and having a source generator apply the policy you want in order to emit that member automatically is an intentionally supported scenario.