Test plan for "field keyword in properties"

[jcouv]

Championed proposal: https://github.com/dotnet/csharplang/issues/140 Spec: proposals/field-keyword.md

public string PropertyConstraint {
    get;
    set => field = value ?? throw new ArgumentNullException();
} = "";

Compiler:

• [ ] Check with local named field in accessor
• [ ] Warning wave warning for binding to real field
• [ ] No warning for binding to real field when using @ prefix
• [ ] Test using field in local function
• [ ] Test with multiple such properties (they each get their own private backing field)
• [ ] Implemented on class, struct, ref struct, interface
• [ ] field in the speculative model does not bind to a backing field if the original location was not a semi-auto property
• [ ] Test with auto-default
  • [ ] expect no auto-default inserted when a field initializer is used
  • [ ] expect an auto-default when assigning in constructor
• [ ] ref field
  • [ ] report error for ref int P => ref field; that property must be non-ref-valued
• [ ] Check warning WRN_SequentialOnPartialClass is produced when needed (see also comment)
• [ ] struct fulfills unmanaged constraint and managed type rules (see pointer types) depending on synthesized fields
• [ ] attributes attached to synthesized field using [field: ...]
• [ ] ref safety:

  struct S
  {
      [UnscopedRef]
      Span<int> Prop1 => M(ref field); // should not error
  
      // Should error
      Span<int> Prop2 => M(ref field); // should error
  
      static Span<int> M(ref Span<int> span) => default;
  }
  

• [ ] emitting synthesized field when used in [Conditional] code only:

  struct S
  {
      object P
      {
          get
          {
              Debug.Assert(field is null);
              return null;
          }
      }
  }
  

• [ ] partial properties where the implementation uses field and one or both parts have field: targeted attribute lists. (See also PartialPropertiesTests.Attributes_Property_BackingField. This issue referenced in source.)
• [ ] initializer on field-backed property
• [ ] initializer on partial property parts
• [ ] comment from partial properties
• [ ] comment from partial properties
• [ ] synthesized field is emitted to ref assembly
• [ ] public API review: https://github.com/dotnet/roslyn/issues/74937

Productivity:

• [ ] Fixer for compat break
• [ ] QuickInfo
• [ ] Completion
• [ ] Colorization
• [ ] Conversion?
• [ ] F1 help on field
• [ ] EnC

[CyrusNajmabadi]

Moving to 17.2. The work for thsi hasn't gotten merged in yet anyways.

[Youssef1313]

Note to self: The following properties for SynthesizedBackingFieldSymbol needs to be revised:

• DeclaringSyntaxReferences (should it remain empty or return the first usage of field keyword?)
• IsImplicitlyDeclared (should it always return true or return false for field-keyword backing fields?)

[Youssef1313]

There are interesting scenarios around overriding virtual properties.

public class Base
{
    public virtual int P1 { get; set; }
    public virtual int P2 { get; set; }
    public virtual int P3 { get; set; }
    public virtual int P4 { get; set; }
    public virtual int P5 { get; set; }

    public virtual int P6 { get => 0; set { } }
}

public class Derived : Base
{
    // no error.
    public override int P1 { get => 0; }
    
    // This produces Auto-implemented properties must have get accessors.
    // However, per the spec updates, a set-only with semicolon is allowed.
    // Should this now produce an error similar to ERR_AutoPropertyMustOverrideSet?
    public override int P2 { set; }

    // This synthesizes a sealed setter. Make sure we have this behavior for semi-auto.
    public sealed override int P3 { get => 0; }

    // Auto-implemented properties must override all accessors of the overridden property.
    public override int P4 { get; }

    // Will this be allowed? It's not allowed for old-style auto properties (see P4).
    public override int P5 { get => field; }

    public override int P6 { get => field; }

    public Derived()
    {
        // P6 looks like a property that's assignable through backing field.
        // But this shouldn't be the case due to the inherited setter.
        // We should use 'GetOwnOrInheritedSetMethod' for the logic of assignability through backing field.
        // Add a test to confirm that.
        P6 = 10;
    }
}

Do these scenarios have clear answer as to what's the expected behavior? Are there any that needs to be discussed in LDM?

cc @AlekseyTs @333fred @CyrusNajmabadi

[AlekseyTs]

per the spec updates, a set-only with semicolon is allowed.

Could you add a quote from the relevant portion of the spec?

Should this now produce an error similar to ERR_AutoPropertyMustOverrideSet?

Please include exact wording of the message.

// Will this be allowed? It's not allowed for old-style auto properties (see P4).

We probably should treat this as a property with a user provided body. Presence of the field keyword probably shouldn't make any difference.

[AlekseyTs]

@Youssef1313 BTW, consider not "polluting" the Test Plan issue with "random" discussions. Open a separate issue for each specific question.

[CyrusNajmabadi]

Note: Fixer for compat break

Productivity does not own this. We've been explicit on compiler semantic breaks that the best people who understand the breaks perfectly, and know exactly how to fix them, are the compiler team (since they put in the new rules/code), and know exactly what change will result in code with the exact same semantics as before.

As such, if we want such a fixer, it should be part of the compiler, potentially driven by known compiler diags (hidden or not) and with exact fixes for all the scenarios enumerated and tested.