Skip to content

Remove some redundant immutable array allocation#4988

Merged
thomhurst merged 2 commits intothomhurst:mainfrom
SimonCropp:remove-some-redundant-ImmutableArray-allocation
Feb 25, 2026
Merged

Remove some redundant immutable array allocation#4988
thomhurst merged 2 commits intothomhurst:mainfrom
SimonCropp:remove-some-redundant-ImmutableArray-allocation

Conversation

@SimonCropp
Copy link
Contributor

No description provided.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Summary: This PR removes redundant allocations across 4 files. All changes are correct and beneficial.


TUnit.Analyzers/TestDataAnalyzer.cs

Removing is valid — already returns (per ), so calling on it was a no-op allocation. Good catch.


TUnit.Assertions.SourceGenerator/Generators/AssertionMethodGenerator.cs

Removing before is correct. Roslyn's accepts directly — the intermediate conversion to was purely wasteful.


TUnit.Core.SourceGenerator/Generators/PropertyInjectionSourceGenerator.cs

Three cleanups bundled here:

  1. Return type change ( → ): Valid because immediately unpacks the container, so it's never stored as intermediate state in the generator pipeline. The caching concern for incremental generators applies to the items (individual values), not the container wrapper. Worth noting: if implements , downstream caching works correctly regardless of container type.

  2. Early-exit collection expression ( instead of ): Cleaner.

  3. ** → **: Correct update for .

One minor observation: the variable is now declared after the early-return guard, which actually improves readability — no allocation happens on the hot path when or is null.


TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs

Removing before is correct — accepts (line 13 of ). The concat chain of is now lazily consumed rather than materialized first.


Overall: Clean, targeted micro-optimizations that reduce unnecessary heap allocations in both analyzer and source generator hot paths. All changes are correctness-safe. No issues found.

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Summary: This PR removes redundant ImmutableArray allocations across 4 files. All changes are correct and beneficial.


TUnit.Analyzers/TestDataAnalyzer.cs

Removing .ToImmutableArray() is valid. WithoutCancellationTokenParameter() already returns ImmutableArray<IParameterSymbol> (per ParameterExtensions.cs:9), so calling .ToImmutableArray() on it was a no-op allocation. Good catch.


TUnit.Assertions.SourceGenerator/Generators/AssertionMethodGenerator.cs

Removing .ToImmutableArray() before SelectMany is correct. Roslyn's IncrementalValuesProvider.SelectMany accepts IEnumerable<T> directly, so the intermediate conversion to ImmutableArray was purely wasteful.


TUnit.Core.SourceGenerator/Generators/PropertyInjectionSourceGenerator.cs

Three cleanups bundled here:

  1. Return type change from ImmutableArray to List: Valid because SelectMany immediately unpacks the container, so it is never stored as intermediate pipeline state. The incremental generator caching concern applies to the items themselves (individual ConcreteGenericTypeModel values), not the container wrapper. If ConcreteGenericTypeModel implements IEquatable<T>, downstream caching works correctly regardless of container type.

  2. Early-exit collection expression (return [] instead of return ImmutableArray<ConcreteGenericTypeModel>.Empty): Cleaner modern C# syntax.

  3. x.Length to x.Count: Correct update for List<T>.

Minor positive side effect: moving results declaration after the early-return guard means no allocation happens on the fast path when dataSourceInterface or asyncInitializerInterface is null.


TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs

Removing .ToImmutableArray() before WriteAttributes() is correct. AttributeWriter.WriteAttributes accepts IEnumerable<AttributeData> (line 13 of AttributeWriter.cs), so the concat chain is now lazily consumed rather than eagerly materialized first.


Overall: Clean, targeted micro-optimizations that reduce unnecessary heap allocations in both analyzer and source generator hot paths. All changes are correctness-safe. No issues found.

@thomhurst thomhurst enabled auto-merge (squash) February 25, 2026 18:14
@thomhurst thomhurst merged commit 3733777 into thomhurst:main Feb 25, 2026
12 of 13 checks passed
@claude claude bot mentioned this pull request Feb 25, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants