Records and the ‘with’ operator, redux

In my previous blog post I described some behaviour of C# record types which was unexpected to me, though entirely correct according to the documentation. This is a follow-up post to that one, so if you haven't read that one yet, please do so - I won't go over all the same ground.

Is this a problem or not?

The previous blog post made it to Hacker News, and the comments there, on Bluesky, and on the post itself have been decidedly mixed.

Several people believe this is working as intended, several people believe it's a terrible decision on the part of the C# language team.

Perhaps unsurprisingly, the most insightful comment was from Eric Lippert, who referred to a post on the Programming Language Design and Implementation Stack Exchange site. Eric has answered the question thoroughly, as ever.

I believe the difference in opinions comes from interpretation about what with" is requesting. Eric wrote:

The with makes a copy of the record, changing only the value of the property you identified as wishing to change, so it ought not to be surprising that nothing else changed.

That's not how I've ever thought of with - I haven't expected it to say this object but with these different properties", but instead a new record, with the same parameters as the original one, but with these different parameters. It's a subtle distinction - sufficiently subtle that I hadn't even bothered to think about it until running into this problem - but I suspect it explains how different people think about the same feature in different ways. I wouldn't have thought of setting the property" because I think of records as being immutable to start with: that the only way you can end up with a record where the property returns a different value is by providing that different value as part of construction. (Again, to be crystal clear: I don't think I've found any bits of documentation which are incorrect. It's my mental model that has been wrong.)

I haven't gone back over previous YouTube videos describing the feature - either from the C# team itself or from other developers - to see whether a) it's described in terms of setting properties rather than parameters; b) the videos describe the distinction in order to make it clear which is right".

In my defence, even when you do have a better mental model for how records work, this is a pretty easy mistake to make, and you need to be on the ball to spot it in code review. The language absolutely allows you to write records which aren't just lightweight data records" in the same way that you do for classes - so I don't think it should be surprising that folks are going to do that.

So, after this introductory spiel, this post has two aspects to it:

• How am I going to stop myself from falling into the same trap again?
• What changes have I made within the Election 2029 code base?

Trap avoidance: Roslyn Analyzers

In the previous post, I mentioned writing a Roslyn analyzer as a possible way forward. My initial hope was to have a single analyzer which would just spot the use of with operators targeting any parameter which was used during initialization.

That initial attempt worked to some extent - it would have spotted the dangerous code from the original blog post - but it only worked when the source code for the record and the source code using the with operator were in the same project. I've now got a slightly better solution with two analyzers, which can even work with package references where you may not have access to the source code for the record at all... so long as the package author is using the same analyzers! (This will make more sense when you've seen the analyzers.)

The source code of the analyzers is on GitHub and the analyzers themselves are in the JonSkeet.RoslynAnalyzers NuGet package. To install them in a project, just add this to an item group in your project file:

<PackageReference Include="JonSkeet.RoslynAnalyzers" Version="1.0.0-beta.6" PrivateAssets="all" IncludeAssets="runtime; build; native; contentfiles; analyzers"/>

Obviously, it's all very beta - and there are lots of corner cases it probably wouldn't find at the moment. (Pull requests are welcome.) But it scratches my particular itch for now. (If someone else wants to take the idea and run with it in a more professional, supported way, ideally in a package with dozens of other useful analyzers, that's great.)

As I mentioned, there are two analyzers, with IDs of JS0001 and JS0002. Let's look at how they work by going back to the original demo code from the previous post. Here's the complete buggy code:

// Recordpublic sealed record Number(int Value){ public bool Even { get; } = (Value & 1) == 0;}// Use of recordvar n2 = new Number(2);var n3 = n2 with { Value = 3 };Console.WriteLine(n2); // Output: Number { Value = 2, Even = True }Console.WriteLine(n3); // Output: Number { Value = 3, Even = True }

Adding the analyzer package highlights the int Value parameter declaration in Number, with this warning:

JS0001 Record parameter Value' is used during initialization; it should be annotated with [DangerousWithTarget]

Currently, there's no code fix, but we need to do two things:

• Declare an attribute called DangerousWithTargetAttribute
• Apply the attribute to the parameter

Here's the complete attribute and record code with the fix applied:

[AttributeUsage(AttributeTargets.Parameter)]internal sealed class DangerousWithTargetAttribute : Attribute;public sealed record Number([DangerousWithTarget] int Value){ public bool Even { get; } = (Value & 1) == 0;}

The attribute doesn't have to be internal, and indeed in my election code base it's not. But it can be, even if you're using the record from a different assembly. The analyzer doesn't care what namespace it's in or any other details (although it does currently have to be called DangerousWithTargetAttribute rather than just DangerousWithTarget).

At this point:

• The source code makes it clear to humans that we know it would be dangerous to set the Value property in a with operator with Number
• The compiled code makes that clear (to the other analyzer) as well

After applying the above change, we get a different warning - this time on n2 with { Value = 3 }:

JS0002: Record parameter Value' is annotated with [DangerousWithTarget]

(Both of these warnings have more detailed descriptions associated with them as well as the summary.)

Now you know the problem exists, it's up to you to fix it... and there are multiple different ways you could do that. Let's try to get warning-free by replacing our precomputed property with one which is computed on demand. The analyzers don't try to tell you if [DangerousWithTarget] is applied where it doesn't need to be, so this code compiles without any warnings, but it doesn't remove our JS0002 warning:

// No warning here, but the expression 'n2 with { Value = 3 }' still warns.public sealed record Number([DangerousWithTarget] int Value){ public bool Even => (Value & 1) == 0;}

As it happens, this has proved unexpectedly useful within the Election2029 code, where even though a parameter isn't used in initialization, there's an expected consistency between parameters which should discourage the use of the with operator to set one of them.

Once we remove the [DangerousWithTarget] attribute from the parameter though, all the warnings are gone:

public sealed record Number(int Value){ public bool Even => (Value & 1) == 0;}

The analyzer ignores the Even property because it doesn't have an initializer - it's fine to use Value for computed properties after initialization.

A new pattern for Election2029

So, what happened when I enabled the analyzers in my Election2029 project? (Let's leave aside the bits where it didn't work first time... there's a reason the version number is 1.0.0-beta.6 already.)

Predictably enough, a bunch of records were flagged for not specifying the [DangerousWithTarget]... and when I'd applied the attribute, there were just one or two places where I was using the with operator in an unsafe way. (Of course, I checked whether the original bug which had highlighted the issue for me in the first place was caught by the analyzers - and it was.)

For most of the records, the precomputation feels okay to me. They're still fundamentally pretty lightweight records, with a smattering of precomputation which would feel pointlessly inefficient if I made it on-demand. I like the functionality that I'm given automatically by virtue of them being records. I've chosen to leave those as records, knowing that at least if I try to use the with operator in a dangerous operator, I'll be warned about it.

However, there are two types - ElectionCoreContext and ElectionContext, which I [wrote about earlier] - which have a lot of precomputation. They feel more reasonable to be classes. Initially, I converted them into just normal" classes, with a primary constructor and properties. It felt okay, but not quite right somehow. I liked the idea of the record type for just the canonical information for the context... so I've transformed ElectionContext like this (there's something similar for ElectionCoreContext):

public sealed class ElectionContext : IEquatable<ElectionContext>{ public ElectionContextSourceData SourceData { get; } // Bunch of properties proxying access public Instant Timestamp => SourceData.Timestamp; // ... public ElectionContext(ElectionContextSourceData sourceData) { // Initialization and validation } public sealed record ElectionContextSourceData(Instance Timestamp, ...) { // Equals and GetHashCode, but nothing else }}

At this point:

• I've been able to add validation to the constructor. I couldn't do that with a record in its primary constructor.
• It's really clear what's canonical information vs derived data - I could even potentially refactor the storage layer to only construct and consume the ElectionContextSourceData, for example. (I'm now tempted to try that. I suspect it would be somewhat inefficient though, as it uses the derived data to look things up when deserializing.)
• I can still use the with operator with the record, when I need to (which is handy in a few places)
• There's no risk of the derived data being out of sync with the canonical data, because the ordering is very explicit

Ignoring the naming (and possibly the nesting), is this a useful pattern? I wouldn't want to do it for every record, but for these two core, complex types it feels like it's working well so far. It's early days though.

Conclusion

I'm really pleased that I can now use records more safely, even if I'm using them in ways that other folks may not entirely condone. I may well change my mind and go back to using regular classes for all but the most cut-and-dried cases. But for now, the approaches I've got of use records where it feels right, even if that means precomputation" and use classes to wrap records where there's enough behavior to justify it" are working reasonably well.

I don't really expect other developers to use my analyzers (although you're very welcome to do so, of course) - but the fact that they're even feasible points to Roslyn being a bit of a miracle. I'm not recommending either my careful use of records slightly beyond their intended use" or class wrapping a record" approaches yet. I've got plenty of time to refactor if they don't work out for the Election2029 project. But I'd still be interested in getting feedback on whether my decisions at least seem somewhat reasonable to others.