perf: reduce allocations in generated code

[TimothyMakkison]

Modify generated code to avoid allocations.

• Removes string[] allocation, ArrayWhereIterator and string concatenation.
• Does TUnit have tests for generated assertion code? Most of the tests for TUnit usually fail for me so I don't bother using them 😅
  • Does the CI run unit tests or does it only try to run claude?

Note that there is a lot of room for improvement with the additionalParams.Length > 0 case. I can think of a couple of approaches if you'd like to try them

Before

After

Note

[thomhurst]

Summary

This PR modifies the AssertionExtensionGenerator to reduce allocations in generated extension methods by avoiding string concatenation, array allocations, and LINQ operations.

Critical Issues

1. Null handling removed - Breaking Change ⚠️

The PR removes the .Where(e => e != null) filter when building expression strings. While CallerArgumentExpression typically provides non-null values in normal usage, the nullable type (string?) and default value (= null) indicate this is a possibility.

Current behavior:

source.Context.ExpressionBuilder.Append(".IsBetween("
    + string.Join(", ", new[] { lowerBoundExpression, upperBoundExpression }.Where(e => e != null)) + ")");

• If lowerBoundExpression = null, upperBoundExpression = "bar" → .IsBetween(bar)
• If both are null → .IsBetween()

New behavior after PR:

source.Context.ExpressionBuilder.Append(string.Join(", ", new[] { lowerBoundExpression, upperBoundExpression }));

• If lowerBoundExpression = null, upperBoundExpression = "bar" → .IsBetween(, bar)
• If both are null → .IsBetween(, )

This produces malformed expression strings with empty elements. To fix this while maintaining the performance improvement:

For single parameter case - add null coalescing:

source.Context.ExpressionBuilder.Append({additionalParams[0].Name}Expression ?? "");

For multiple parameters case - keep the .Where(e => e != null) or use a null-safe alternative:

source.Context.ExpressionBuilder.Append(string.Join(", ", new[] { ... }.Where(e => e != null)));

2. Missing snapshot test updates 🚫

Changes to source generator output require updating .verified.txt snapshots (per CLAUDE.md rule #2). The PR modifies generated code structure but doesn't include updated snapshot files.

Files that will need updates:

• TUnit.Assertions.SourceGenerator.Tests/AssertionExtensionGeneratorTests.AssertionWithMultipleParameters.*.verified.txt (all framework versions)
• Any other AssertionExtensionGenerator snapshots with parameters

Current snapshot (line 24-25 of AssertionWithMultipleParameters.DotNet9_0.verified.txt):

source.Context.ExpressionBuilder.Append(".IsBetween("
    + string.Join(", ", new[] { lowerBoundExpression, upperBoundExpression }.Where(e => e != null)) + ")");

Expected after PR (fixing the null issue):

source.Context.ExpressionBuilder.Append(".IsBetween(");
source.Context.ExpressionBuilder.Append(string.Join(", ", new[] { lowerBoundExpression, upperBoundExpression }.Where(e => e != null)));
source.Context.ExpressionBuilder.Append(")");

Action required: Run snapshot tests, review changes, then commit all .verified.txt files. See CLAUDE.md for commands.

Suggestions

The performance optimization approach is sound for the single parameter case, which completely eliminates allocations. Nice work on that optimization!

For the multiple parameters case, consider whether the performance gain (avoiding string concatenation) justifies the continued array allocation and string.Join. The Where clause adds minimal overhead but is important for correctness.

Verdict

⚠️ REQUEST CHANGES - Critical issues found:

1. Null handling must be preserved to avoid malformed expression strings
2. Snapshot tests must be updated per TUnit development rules

[TimothyMakkison]

Null handling must be preserved to avoid malformed expression strings

StringBuilder.Append has null handling built in as does string.Join, this is not an issue.

Snapshot tests must be updated per TUnit development rules

Where are these? TUnit.Assertions.SourceGenerator.Tests running them in console yields a bunch of error messages like:
error CS0103: The name 'Sourcy' does not exist in the current context C:\Users\Lenovo ThinkPad\Desktop\coding\C#\TUnit\TUnit.Assertions.SourceGenerator.Tests\MethodAssertionGeneratorTests.cs

[thomhurst]

Hmm... I don't know why that's not working. That should source generate project locations on your machine

[TimothyMakkison]

Okay, I've reverted back to using .Where because I misunderstood how string.Join handled null. From what I can tell most code calls the .Append path so will get large benefits from no longer allocating a single item array and ArrayWhereEnumerator.

I have a lot of ideas for how to improve the path with better code generation, but I suspect it is low hanging fruit.

Hmm... I don't know why that's not working. That should source generate project locations on your machine

• Opening this project in rider I can see all projects but It's never been able to detect or run TUnit tests (yes I've tried all the techniques mentioned in the docs).
• Opening in visual studio let's me detect and run some tests but TUnit.SourceGenerators.Tests is unloaded along with most projects.
  • Log file states LimitedFunctionality Microsoft.VisualStudio.LanguageServices.ProjectSystem.InvalidProjectDataException: Property 'TargetPath' is required to be an absolute path, but the value is ''.
• Running TUnit.SourceGenerators.Tests from terminal yields a bunch of warnings for conflicts ie "Microsoft.Bcl.AsyncInterfaces" that could not be resolved. There was a conflict between "Microsoft.Bcl.AsyncInterfaces, Version=7.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ff cd2ddd51" and "Microsoft.Bcl.AsyncInterfaces, Version=9.0.0.10,, some warnings about files being unable to be deleted warning MSB3061: Unable to delete file "C:\Users\Lenovo ThinkPad\Desktop\coding\C#\TUnit\TUnit.Assertions.SourceGenerator.Tests\SourceGeneratedViewer\TUnit.Core.SourceGenerator\TUnit.Core.SourceGenerator.Generators.TestMetadataGenerator\TUnit_Assertions_SourceGenerator_Tests_ThreadAssertionGeneratorTests_GeneratesThreadAssertions.g.cs", before finally failing because Sourcy doesn't exist.
  I guess I could manually modify the snapshot tests but that sounds like a nightmare 😅

Running git clean and reset hasn't helped, running this on Windows 11 and I have .NET 10.100 installed.

[thomhurst]

If any snapshots need updating I can pull the branch and do it for ya. I'm not sure the assertions have snapshots though. It's mainly TUnit.Core.SourceGenerator that uses a lot of snapshots

[thomhurst]

We could probably also optimise for parameter counts of 2 and 3. I think that'd cover 99% of scenarios, and avoid the LINQ/enumerable allocations

[TimothyMakkison]

If any snapshots need updating I can pull the branch and do it for ya. I'm not sure the assertions have snapshots though. It's mainly TUnit.Core.SourceGenerator that uses a lot of snapshots

Thanks, 🙏 I've been trying to fix the issue but I might have to ask you to do that.

We could probably also optimise for parameter counts of 2 and 3. I think that'd cover 99% of scenarios, and avoid the LINQ/enumerable allocations

Okay, these were my ideas

Custom method

// collection initialiser uses a custom struct coerced into a span, no allocations
// should work for all versions of .NET, might break depending on C# version iirc not an issue
// Don't know where the extension method should live
// If span is an issue the method could have overloads for each number of parameters with a params fallback
source.Context.ExpressionBuilder.JoinNonNull([boolValueExpression, intValueExpression, stringValueExpression]);


public static void JoinNonNull(this StringBuilder sb, string separator, ReadOnlySpan<string?> vals); // adds non null value with separator

Chain ifs

var added = false;
if(boolValueExpression != null)
{
    source.Context.ExpressionBuilder.Append(added ? ", " : "");
    source.Context.ExpressionBuilder.Append(boolValueExpression);
    added = true;
}
if(intValueExpression!= null)
{
    source.Context.ExpressionBuilder.Append(added ? ", " : "");
    source.Context.ExpressionBuilder.Append(intValueExpression);
    added = true;
}
// etc

Foreach

ReadOnlySpan<string?> expressions = [boolValueExpression, intValueExpression, stringValueExpression];
var added = false;
foreach (var expression in expressions)
{
    if(expression == null)
    {
        continue;
    }
    source.Context.ExpressionBuilder.Append(added ? ", " : "");
    source.Context.ExpressionBuilder.Append(expression);
    addedd = true;
}

All these solutions won't allocate and likely have the same performance

• The first will have the issue of where the extension method lives but will be the cleanest (could simplify the generator and always use this version)
• Second and third may have edge cases with the names of the local variables, I'm not sure if this is a realistic concern as I don't see much room for user generated code breaking this.
• First and third use Spans and rely on modern C#, this might cause an issue with .NET framework? I can't remember what my solution with mapperly was. I guess this depends on where the generated code lives.

[TimothyMakkison]

Okay, opted for if statement generation, as it doesn't need Span support. If we're being pedantic it's probably slightly faster, although this shouldn't be a concern.

Second and third may have edge cases with the names of the local variables,

I don't see how using the local variable added will break anything as we're pretty insulated from user code. It might be an issue if you could somehow have a paramter called added 🤷

If any snapshots need updating I can pull the branch and do it for ya. I'm not sure the assertions have snapshots though. It's mainly TUnit.Core.SourceGenerator that uses a lot of snapshots

Yes please 🙏 I still can't get the tests to run.