avoid some string alloc by using AppendJoin

[SimonCropp]

No description provided.

[claude]

Code Review

Summary

This PR replaces patterns with to avoid intermediate string allocations. The optimization is correct and well-understood.

---

Positive Observations

• The core intent is sound. string.Join allocates an intermediate string before Append writes it; AppendJoin writes tokens directly into the builder.
• The new CodeWriter.AppendJoin<T> implementation correctly preserves the existing indentation/newline behaviour (_isNewLine guard) before delegating to the underlying StringBuilder.AppendJoin.
• The interface doc-comment follows the existing convention.

---

Issue: Redundant .ToArray() calls in AssertionMethodGenerator.cs

The PR's goal is reducing allocations, but the AssertionMethodGenerator.cs changes leave a redundant intermediate array in place:

// current (after this PR)
var paramList = parameters.Select(p => $"_{p.Name}").ToArray();
sourceBuilder.AppendJoin(", ", paramList);

StringBuilder.AppendJoin has an IEnumerable<T> overload, so the ToArray() call creates an array that is iterated once and immediately discarded. The allocation this PR is trying to save is partially offset by that array allocation.

Suggested fix – drop the intermediate variable entirely:

sourceBuilder.AppendJoin(", ", parameters.Select(p => $"_{p.Name}"));

This applies to all four call-sites changed in AssertionMethodGenerator.cs (lines 783, 832, 857, 866 in the post-merge file).

---

Minor: Adding a method to a public interface is a breaking change

ICodeWriter is declared public, so any external code that implements it will fail to compile after this change. If external implementors are a concern, a default interface method would be safer:

ICodeWriter AppendJoin<T>(string separator, IEnumerable<T> values)
    => Append(string.Join(separator, values)); // default keeps existing behaviour

If ICodeWriter is intentionally internal-only (despite the public modifier), this can be ignored.

---

Scope note

The source-generator call sites (compile-time, cold path) will see negligible real-world benefit from this change. The most meaningful win is AspireFixture.cs, which runs at test execution time. That said, consistency across the codebase is valuable and the change does no harm.

---

Overall: good direction. The main ask is removing the now-redundant .ToArray() calls to actually realise the full allocation saving the PR aims for.

[claude]

Code Review

Summary

This PR replaces sb.Append(string.Join(...)) patterns with sb.AppendJoin(...) to avoid intermediate string allocations. The optimization is correct and well-understood.

---

Positive Observations

• The core intent is sound. string.Join allocates an intermediate string before Append writes it; AppendJoin writes tokens directly into the builder.
• The new CodeWriter.AppendJoin<T> implementation correctly preserves the existing indentation/newline behaviour (_isNewLine guard) before delegating to the underlying StringBuilder.AppendJoin.
• The interface doc-comment follows the existing convention.

---

Issue: Redundant .ToArray() calls in AssertionMethodGenerator.cs

The PR's goal is reducing allocations, but the AssertionMethodGenerator.cs changes leave a redundant intermediate array in place:

// current (after this PR)
var paramList = parameters.Select(p => $"_{p.Name}").ToArray();
sourceBuilder.AppendJoin(", ", paramList);

StringBuilder.AppendJoin has an IEnumerable<T> overload, so the .ToArray() call creates an array that is iterated once and immediately discarded. The allocation this PR is trying to save is partially offset by that array allocation.

Suggested fix – drop the intermediate variable entirely:

sourceBuilder.AppendJoin(", ", parameters.Select(p => $"_{p.Name}"));

This applies to all four call-sites changed in AssertionMethodGenerator.cs.

---

Minor: Adding a method to a public interface is a breaking change

ICodeWriter is declared public, so any external code that implements it will fail to compile after this change. If external implementors are a concern, a default interface method would be safer:

ICodeWriter AppendJoin<T>(string separator, IEnumerable<T> values)
    => Append(string.Join(separator, values)); // default keeps existing behaviour

If ICodeWriter is intentionally internal-only (despite the public modifier), this can be ignored.

---

Scope note

The source-generator call sites (compile-time, cold path) will see negligible real-world benefit from this change. The most meaningful win is AspireFixture.cs, which runs at test execution time. That said, consistency across the codebase is valuable and the change does no harm.

---

Overall: good direction. The main ask is removing the now-redundant .ToArray() calls to actually realise the full allocation saving the PR aims for.