Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

Only emit ClassMetadata reference setter when there are properties cutting down on unneeded generated code. Might have a tiny performance benefit 🤷 the JIT is hard to understand.

I did think that the loop could be a dedicated helper method inside TUnit.Core

@TimothyMakkison
Copy link
Contributor Author

While I can run some tests, anything that relies on Sourcy won't build. Would it be possible if you update the snapshot tests?

@thomhurst
Copy link
Owner

thomhurst commented Jan 14, 2026

While I can run some tests, anything that relies on Sourcy won't build. Would it be possible if you update the snapshot tests?

This is really annoying :(

Does the compiler give any sort of error or anything?

@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Jan 14, 2026

    if (propertyCount == 0)

👍 added.

Was thinking of looking at the source generators performance as I assume they contribute to build times. Can't imagine it's possible to do any more incremental generator caching due to the nature of TUnit

Does the compiler give any sort of error or anything?

Sourcy does not exist in the current context. This issue persists in terminal, rider and VS2026.I wonder if I need the exact same version of .NET for it to work

Would it be possible if you update the snapshot tests?

Just checking; you do have an extension which lets you mass approve verify changes right? I don't want to waste your evening 😅

@thomhurst
Copy link
Owner

I wonder if it's failing to run on your machine for some reason (it's a source generator)

@thomhurst
Copy link
Owner

Just checking; you do have an extension which lets you mass approve verify changes right? I don't want to waste your evening

Yep!

@thomhurst
Copy link
Owner

Summary

This PR optimizes source generator output by only emitting the foreach loop that sets ClassMetadata and ContainingTypeMetadata references on properties when the class actually has properties, avoiding unnecessary code generation for classes with empty property arrays.

Critical Issues

None found ✅

Suggestions

Minor: Consider extracting loop body to helper method

The PR author mentioned in the description: "I did think that the loop could be a dedicated helper method inside TUnit.Core". This is worth considering as it would:

  • Further reduce generated code size
  • Centralize the property metadata initialization logic
  • Make future changes to property initialization easier to maintain

However, this would require:

  • A public/internal helper method in TUnit.Core
  • Passing the properties array as a parameter
  • Verifying AOT compatibility

This is not blocking and can be done in a follow-up if desired.

Observation: Consistent optimization patterns

The change is applied to both overloads of the property array generation methods (WritePropertyMetadataArray and GeneratePropertyMetadataArray), which is correct and maintains consistency.

Verification Checklist

Snapshot tests updated: All 100+ .verified.txt files updated, no .received.txt files committed
Dual-mode compatibility: Not applicable - this only affects source generator output, not reflection mode
AOT compatibility: No changes to [DynamicallyAccessedMembers] annotations needed
Performance: Positive impact - eliminates unnecessary foreach loops in generated code
No VSTest references: Not applicable to this change

Verdict

APPROVE - Clean optimization that reduces generated code size without affecting functionality.

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