Test plan for "records"

[jcouv]

Relates to championed issue: dotnet/csharplang#39
Spec: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-9.0/records.md
Records board: https://github.com/dotnet/roslyn/projects/57

LDM:

• ToString: should we always print a trailing comma R { Property1 = 42, Property2 = 43, }? (no)
• ToString: can we deal with stack overflow potential? (do nothing special for now)
• ToString: should we try to quote/escape values, for example R { IntProperty = 42, StringProperty = "hello" }? (no)
• ToString: should we omit ToString on abstract records? (no)
• ToString: should PrintMembers call ToString() on values to avoid boxing them in builder.Append((object)value)? (yes)
• ToString: should we try and avoid the double space in empty record R { }? (yes)
• ToString: do we want to economize a call to StringBuilder.Append to print <name> { ? (maybe)
• warn on declaring type named "record" in C# 9? (yes)
• Can we omit EqualityContract if the record type is sealed and derives from System.Object? (no)
• Should we allow base syntax on record without parameter list? record Derived : Base(0) { } (yes)
• nullability expectations for bool Equals(R? other) and Type EqualityContract { get; } and R? <Clone>$() and void Deconstruct(out string? NotNullableStringProperty). (issue Records: Incorrect nullable annotations for generated Equals override #47627)
• Ensure spec forbids an empty parameter list.
• LDM: How to give base ctor arguments if there are no record parameters
• Should records have a base type that isn't object? (no)
• Allow with on non-record types (maybe with user-defined Clone() method)? (out of scope for C# 9)
• Confirm syntax model for data properties (out of scope for C# 9)
• Constraint for with-able? Allows t with { ... } with use of : where T : I, with (like we could do : where T : I, new)? (out of scope for C# 9)
• Should anonymous objects be with-able? (out of scope for C# 9)
• bringing structs closer to ideal value semantics (out of scope for C# 9)
• .ToString() (yes)
• xml docs (issue Figure out XML docs for positional records #44571, support params for now)
• Should we allow record Base { public int P => 0; } record Derived(int P); and how does it work? (maybe Derived gets new int P { get; init; }, or maybe it's an error)
• allow deriving from non-record types? (not in C# 9)
• tagging record types (supports MetadataAsSource scenario, using unspeakable clone method for now)
• confirm data vs. record syntax (record Person(...) for next preview)
• init-only: should _ = new C() { readonlyField = null }; be allowed in a method on type C? (comment) (punted out of C# 9)
• init-only: confirm metadata encoding (modreq?) and rules for dealing with ambiguous IsExternalInit type (confirmed)
• init-only methods ? init void Init() (punted out of C# 9, strong interest, issue with Add methods in BCL immutable collection types)
• with initializer with expressions allowed in other kinds of initializers, such as [0] = 2 or values for IEnumerable/Add collection. (punted out of C# 9)

Record declarations

Compiler:

• Block or handle EnC (Support EnC on records and with expressions #44877)
• xml docs (params should apply to primary constructor parameters) (Figure out XML docs for positional records #44571)
• LangVer diagnostic quality (Record syntax in LangVer=8 should not complain about top-level statement #45900)
• nullability analysis: warn on bad user-provided methods (issue Records: Incorrect nullable annotations for generated Equals override #47627)
• nullability analysis: set property state based on constructor arguments (Record constructor arguments should propagate nullability to properties #44763)
• probably need some public APIs for detecting record, identifying primary constructor and mapping parameters with properties
• operator== (Records should generate operator == and != #46381)
• ToString (Records should generate ToString #46382)
• attributes on record parameters targeted to parameter (default) or field or property
• parsing with different LangVersions (and document breaking change)
• Disallow declaring members with name "Clone"
• parsing of record is conditional on LangVersion ("preview" now and "9" later)
  • abstract record M(int i, int j); (parses differently based on LangVersion)
  • bool record; (remains a field or local, regardless of LangVersion)
• Clone extension method allowed? (no)
• Clone with params or optional parameters? (no)
• record with custom copy constructor
• out vars in with initializer (verifies scoping and execution order)
• two copy constructors, one is correct and the other adds params or optional parameter
• verify sequence points
• default values on positional members (data class Person(string Name = null, decimal height = 0) { })
• test type named "@record" (see TypeNamedRecord_*)
• generic or retargeted records
• can positional partial record reiterate its base without providing arguments Bind base arguments fo records #44876 (comment)
• Inherit from a record containing sealed override <>Clone
• record C(Property int) { public int Property { set { } } } (no getter in user-defined property)
• test IOperation on record base
• test covering user-defined copy ctor without initializer and synthesized copy ctor when System.Object lacks parameter-less constructor.
• test synthesized copy ctor where base copy ctor produces use-site diagnostic (comment) or has unsupported metadata (comment)
• Test caller line number supplied by compiler constructor initialiser based on base arguments (see LocalRewriter.GetCallerLocation, see AttributesOnPrimaryConstructorParameters_09_CallerMemberName)
• Test implementation of SyntaxExtensions.IsInContextWhichNeedsDynamicAttribute in relation to PrimaryConstructorBaseType
• Test implementation of SyntaxFacts.IsInTypeOnlyContext and SyntaxFacts.IsNamedArgumentName in relation to PrimaryConstructorBaseType
• Test nullability analysis within base arguments.
• attributes on record (AttributesOnPrimaryConstructorParameters_*)
• modifiers followed by partial (PartialTypes_04_PartialBeforeModifiers)
• only one partial declaration can have the parameter list (PartialTypes_01, missing from SPEC)
• Can't provide Equals(Base) in source (BaseEquals_05)
• Can't provide Equals(object) in source (ObjectEquals_06)
• scope of positional parameters
• Verify that field initializers on 'partial' records can access primary constructor parameters, even if the lexically containing partial declaration doesn't include a parameter list (see PartialRecord_ParametersInScopeOfBothParts)
• ping F# team for records design
• Deconstruct and deconstruct (see Deconstruct_*)
• record nested in generic type (see RecordInsideGenericType)
• Fix 'error CS8850: A positional record must have both a 'data' modifier and non-empty parameter list' message
• spec: Explaining what is a "record" type (ie. has a clone method)
• record synthesized property can implement interface (see InterfaceImplementation)

Productivity:

• completion for class, data and with (PR Recommenders for data and with #43971)
• display records with record keyword
• classification/colorization
• formatting
• Confirm expected QuickInfo behavior (issue Figure out QuickInfo for positional records #44556)
• Rename Tracking (issue Rename tracking doesn't work on positional members #44560)
• Find All References (issue FindAllReferences should find usages of positional members #44559)
• Change Signature (issue ChangeSignature: should trigger in positional records #44558)
• Add parameter/member
• go to definition on deconstruction
• MetadataAsSource (issue MetadataAsSource: should record types be displayed as records? #44566)
• BCL analyzer "don't use lower-case names for types"
• recommended casing for positional parameters
• FAR on user-defined Clone() method (not applicable since unspeakable)
• completion for in but not ref/out/this in record parameters

with expressions

• ref safe-to-escape
• disallow as expression-statement (Fix parsing of with-expression directly inside expression statement #44688)
• semantic model on with expression (get the Clone() method)
• with on a ref local
• GetSymbolInfo on the properties inside a with (see WithExpr28_WithAwait)
• data flow in with (see TestWithExpression)
• with on TRecord (fixed)
• _ = tRecordAndInterface with { /* try to set interface members */ }; (fixed)
• with expression on dynamic type (disallow, see WithExpr24_Dynamic)
• should Obsolete on record type carry over to synthesized methods? (no)
• test in exception filter (see WithExpr27_InExceptionFilter)
• await inside a with (see WithExpr28_WithAwait)
• test new Person() with { First = null, First = null } (WithExpr_NullableAnalysis_*)
• should we consider going even one step further and parsing with initializers with kind = ObjectInitializerExpression instead of WithInitializerExpression? (done)
• OperationCloner and CFG should support with expressions (added IWithOperation)
• block with in expression trees (TestInExpressionTree)
• IOperation/CFG for with expression (PR)

init only

Speclet: https://github.com/dotnet/csharplang/blob/master/proposals/init.md

• add IsExternalInit (API request, PR)
• Allow setting from attribute (issue Permit init-only initialization in attribute usages csharplang#3425)
• VB
• ping F# team
• HaloFour's comment on modreqs: Added: LDM Notes for March 30, 2020 csharplang#3323 (comment) and Discussion: Init Properties and modreq csharplang#3376 (confirmed metadata encoding with LDM)
• Address remaining PROTOTYPE comments

Productivity:

• recommend the keyword (PR Recommend init keyword #43865)
• generate implementations and overrides with the init accessor (PR Add public IsInitOnly API and use it to fix code generation of 'init'  #44077)
• FAR on the accessor (verified in Recommend init keyword #43865)
• hide the modreq from MetadataAsSource and generated syntax (verified in Add public IsInitOnly API and use it to fix code generation of 'init'  #44077)
• analyzer to upgrade LangVersion when a method returns type named record (exists)

API:

• Confirm behavior of IsInitOnly on static int Property { init { } }
• Confirm name of API (IsInitOnly) and feature in spec

data properties (out of scope for C# 9)

• Test that analyzer events are created/received for the symbol and the initializer pieces
• IOperation test for the initializer
• allow sealed modifier (LDM 7/6/2020)

[PathogenDavid]

• ToString: can we deal with stack overflow potential?

Equals and GetHashCode too. Basically all of the synthesized methods can overflow the stack due to infinite recursion if there's a reference cycle.

Here's a simplified example of where I ran into this while experimenting with using records: (SharpLab)

public record TranslatedDeclaration
{
    public TranslatedDeclaration Original { get; } // Seemingly innocent reference to the original instance
    public string Name { get; init; }
    
    internal TranslatedDeclaration()
        => Original = this;
}

(The idea here is that external consumers can create variants of TranslatedDeclaration but there's always a reference to the original, unmodified version available.)

---

Not sure what (if anything) the compiler can really do about this except maybe emit a warning if it detects the potential for a cycle to form.

Although such a warning wouldn't be especially actionable beyond removing the potential cycle without some ability to customize the synthesized methods beyond implementing them all manually. (At which point records basically become glorified classes-with-with-support.)

It may just be that records-with-cycles becomes an anti-pattern to be avoided, but I do feel like it's significantly easier to bump into this problem compared to similar ones when you're implementing things manually.

[tmat]

@PathogenDavid Filed #48646.

[PathogenDavid]

@tmat Thanks! Gave that issue a follow.

[uxsoft]

Hmm, I'm not sure if this is a good place to mention this, but positional records don't work with System.Text.Json deserialization because they "miss a parameterless constructor". It seems very important to add functionality to System.Text.Json to be able to deserialize positional records as I reckon that will be a pretty regular use-case. At least it is for me.

[PathogenDavid]

@uxsoft
dotnet/runtime is probably a better place for that, but have you tried recently?

I believe the issue you're talking about is dotnet/runtime#38539 which was fixed in dotnet/runtime#38959

It's working fine for me on RC2:

using System;
using System.Text.Json;

Console.WriteLine("Hello World!");
Person david = new("David", "Maas");
Console.WriteLine($"Record: {david}");

string json = JsonSerializer.Serialize(david);
Console.WriteLine($"Serialized: {json}");

Person david2 = JsonSerializer.Deserialize<Person>(json);
Console.WriteLine($"Deserialized: {david2}");

record Person(string FirstName, string LastName);

Output:

Hello World!
Record: Person { FirstName = David, LastName = Maas }
Serialized: {"FirstName":"David","LastName":"Maas"}
Deserialized: Person { FirstName = David, LastName = Maas }

[jcouv]

We have individual issues tracking follow-up work. Closing this umbrella issue