feat: add support for enabling/disabling TUnit source generation via project properties

[thomhurst]

Fixes #3619

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Enable/Disable TUnit Source Generation

Reviewed the pull request for adding project property support to control TUnit source generation.

Todo List:

• Read and analyze modified source generator files
• Review .props file for MSBuild best practices
• Check documentation updates
• Identify missing generators in implementation
• Assess test coverage
• Provide comprehensive feedback on code quality, bugs, performance, and security

---

Code Review Findings

✅ Strengths

1. Consistent Implementation Pattern: All modified generators follow the same pattern for checking the EnableTUnitSourceGeneration property
2. Proper MSBuild Integration: The .props file correctly defines the property with appropriate defaults and makes it compiler-visible
3. Good Documentation: The new documentation in engine-modes.md is comprehensive and well-structured
4. Performance Consideration: Early return pattern minimizes overhead when source generation is disabled

🔍 Critical Issue: Incomplete Implementation

Missing Generators: The implementation is incomplete. Based on my analysis, the following generators in the /Generators folder are missing the enable/disable check:

• TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs - Critical: Main test generation
• TUnit.Core.SourceGenerator/Generators/AotConverterGenerator.cs - AOT-specific generation
• TUnit.Core.SourceGenerator/Generators/PropertyInjectionSourceGenerator.cs - Property injection
• TUnit.Core.SourceGenerator/Generators/HookMetadataGenerator.cs - Test hooks

These generators will continue to run even when EnableTUnitSourceGeneration=false, which defeats the purpose of this feature.

Recommended Fix: Add the same enablement check to all missing generators:

public void Initialize(IncrementalGeneratorInitializationContext context)
{
    var enabledProvider = context.AnalyzerConfigOptionsProvider
        .Select((options, _) =>
        {
            options.GlobalOptions.TryGetValue("build_property.EnableTUnitSourceGeneration", out var value);
            return !string.Equals(value, "false", StringComparison.OrdinalIgnoreCase);
        });

    // Combine with existing providers and add early return...
}

📋 Other Issues

Test Coverage:

• No tests verify the property behavior
• Should add integration tests that set EnableTUnitSourceGeneration=false and verify no source is generated
• Consider adding this to TUnit.Core.SourceGenerator.Tests

Minor Code Issues:

• Line 23 in modified files: Consider extracting the property check logic to a shared helper method to reduce duplication
• MSBuild Property: The default value logic Condition="'$(EnableTUnitSourceGeneration)' == ''" is correct

🔒 Security & Performance

• Security: No security concerns identified
• Performance: ✅ Early return pattern is efficient when disabled
• Performance: ⚠️ Missing generators will continue consuming resources when they should be disabled

📝 Code Quality

• Style: Code follows existing patterns and conventions
• Maintainability: Could benefit from shared helper method for the enable check
• Error Handling: Appropriate for source generators

🧪 Test Recommendations

1. Add test case that sets EnableTUnitSourceGeneration=false via AnalyzerConfigOptions
2. Verify no source files are generated when disabled
3. Test the MSBuild property integration in different project configurations
4. Add snapshot tests for the new .props file behavior

---

Summary

While the implementation shows good understanding of the requirements and follows consistent patterns, the PR is incomplete due to missing generators that should also respect the EnableTUnitSourceGeneration property. The most critical missing piece is TestMetadataGenerator.cs, which handles the core test generation.

Recommendation: Please add the enable/disable logic to all remaining generators before merging to ensure the feature works as intended.

---