Fix source generation for async tests inherited from external assemblies

[thomhurst]

Fixes #2893.

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review Complete ✅

I've analyzed the pull request that fixes source generation for async tests inherited from external assemblies. Here's my comprehensive review:

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze the core source generation changes in AttributeWriter.cs
• Review type extension enhancements in TypeExtensions.cs
• Examine test files for coverage and implementation
• Provide comprehensive feedback on code quality, bugs, performance, security, and test coverage

---

🎯 Summary

This PR addresses issue #2893 by improving source generation handling for attributes on inherited async test methods from external assemblies. The fix introduces proper compiler-generated type detection and enhanced attribute writing capabilities.

📋 Code Quality & Best Practices

Excellent overall quality. The code follows C# conventions and demonstrates good architectural patterns:

✅ Strengths:

• Clear separation of concerns between syntax-based and metadata-based attribute writing
• Comprehensive null safety with proper nullable annotations
• Well-documented logic through meaningful method names and comments
• Consistent error handling patterns

✅ Minor suggestions:

• Consider extracting the compiler-generated type detection logic (lines 252-265 in AttributeWriter.cs) into a separate method for better readability
• The WriteDataSourceGeneratorPropertiesWithoutSyntax method could benefit from extracted constants for the repeated property type handling

🐛 Potential Issues

No critical bugs identified. The implementation appears robust:

✅ Good defensive programming:

• Proper null checks throughout (attributeData.AttributeClass is not null)
• Early returns prevent deep nesting
• Exception handling for IErrorTypeSymbol types (TypeExtensions.cs:44-47)

✅ Edge case handling:

• Compiler-generated types are properly filtered out
• Framework-specific attributes are conditionally included
• Both syntax-available and syntax-unavailable scenarios are handled

⚡ Performance Considerations

Well-optimized for a source generator:

✅ Efficient patterns:

• Uses lazy evaluation with FirstOrDefault() and early returns
• Minimizes allocations with StringBuilder for type name construction (TypeExtensions.cs:216-219)
• Proper use of ImmutableArray for thread safety

✅ Compilation efficiency:

• Filters out unusable attributes early in the pipeline (AttributeWriter.cs:25-28)
• Caches compilation lookups appropriately

🔒 Security Concerns

No security issues identified. This is source generation code with appropriate boundaries:

✅ Safe practices:

• No user input directly processed
• Type resolution uses compiler's semantic model
• No reflection or dynamic code execution at runtime

🧪 Test Coverage

Good test coverage approach:

✅ Comprehensive test scenarios:

• AsyncBaseTests.cs covers multiple async patterns:
  • Basic async methods (BaseAsyncTest)
  • Parameterized tests (BaseAsyncTestWithArguments)
  • Cancellation token handling (BaseAsyncTestWithCancellation)
  • Mixed sync/async methods
• AsyncInheritedTestsRepro.cs demonstrates the inheritance scenario that was failing

✅ Real-world test case:

• The repro class effectively validates the fix for cross-assembly inheritance
• Includes both inherited and local test methods

🎯 Technical Implementation Details

Key Fix Analysis:

1. AttributeWriter.cs:70-81 - New branching logic properly handles attributes without syntax reference (from external assemblies)
2. AttributeWriter.cs:248-304 - WriteAttributeWithoutSyntax method provides metadata-based attribute reconstruction
3. TypeExtensions.cs:231-248 - IsCompilerGeneratedType correctly identifies types that can't be represented in source code

The fix addresses the core issue: When inheriting async test methods from external assemblies, the source generator previously couldn't handle attributes because it relied on syntax references that don't exist for compiled code. The new approach falls back to metadata-based attribute writing.

📊 Overall Assessment

Excellent implementation - This is a well-thought-out fix that:

• Solves the specific problem without breaking existing functionality
• Maintains backward compatibility
• Includes appropriate test coverage
• Follows established patterns in the codebase

Recommendation: ✅ Approve - Ready for merge after any minor style improvements if desired.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review Complete ✅

Todo List:

• Read repository guidelines (CLAUDE.md)
• Analyze the core source generation changes in AttributeWriter.cs
• Review type extension enhancements in TypeExtensions.cs
• Examine test files for coverage and implementation
• Provide comprehensive feedback on code quality, bugs, performance, security, and test coverage

---

🎯 Summary

This PR addresses issue #2893 by improving source generation handling for attributes on inherited async test methods from external assemblies. The fix introduces proper compiler-generated type detection and enhanced attribute writing capabilities.

📋 Code Quality & Best Practices

Excellent overall quality. The code follows C# conventions and demonstrates good architectural patterns:

✅ Strengths:

• Clear separation of concerns between syntax-based and metadata-based attribute writing
• Comprehensive null safety with proper nullable annotations
• Well-documented logic through meaningful method names and comments
• Consistent error handling patterns

✅ Minor suggestions:

• Consider extracting the compiler-generated type detection logic (lines 252-265 in AttributeWriter.cs) into a separate method for better readability
• The WriteDataSourceGeneratorPropertiesWithoutSyntax method could benefit from extracted constants for the repeated property type handling

🐛 Potential Issues

No critical bugs identified. The implementation appears robust:

✅ Good defensive programming:

• Proper null checks throughout (attributeData.AttributeClass is not null)
• Early returns prevent deep nesting
• Exception handling for IErrorTypeSymbol types (TypeExtensions.cs:44-47)

✅ Edge case handling:

• Compiler-generated types are properly filtered out
• Framework-specific attributes are conditionally included
• Both syntax-available and syntax-unavailable scenarios are handled

⚡ Performance Considerations

Well-optimized for a source generator:

✅ Efficient patterns:

• Uses lazy evaluation with FirstOrDefault() and early returns
• Minimizes allocations with StringBuilder for type name construction (TypeExtensions.cs:216-219)
• Proper use of ImmutableArray for thread safety

✅ Compilation efficiency:

• Filters out unusable attributes early in the pipeline (AttributeWriter.cs:25-28)
• Caches compilation lookups appropriately

🔒 Security Concerns

No security issues identified. This is source generation code with appropriate boundaries:

✅ Safe practices:

• No user input directly processed
• Type resolution uses compiler's semantic model
• No reflection or dynamic code execution at runtime

🧪 Test Coverage

Good test coverage approach:

✅ Comprehensive test scenarios:

• AsyncBaseTests.cs covers multiple async patterns:
  • Basic async methods (BaseAsyncTest)
  • Parameterized tests (BaseAsyncTestWithArguments)
  • Cancellation token handling (BaseAsyncTestWithCancellation)
  • Mixed sync/async methods
• AsyncInheritedTestsRepro.cs demonstrates the inheritance scenario that was failing

✅ Real-world test case:

• The repro class effectively validates the fix for cross-assembly inheritance
• Includes both inherited and local test methods

🎯 Technical Implementation Details

Key Fix Analysis:

1. AttributeWriter.cs:70-81 - New branching logic properly handles attributes without syntax reference (from external assemblies)
2. AttributeWriter.cs:248-304 - WriteAttributeWithoutSyntax method provides metadata-based attribute reconstruction
3. TypeExtensions.cs:231-248 - IsCompilerGeneratedType correctly identifies types that can't be represented in source code

The fix addresses the core issue: When inheriting async test methods from external assemblies, the source generator previously couldn't handle attributes because it relied on syntax references that don't exist for compiled code. The new approach falls back to metadata-based attribute writing.

📊 Overall Assessment

Excellent implementation - This is a well-thought-out fix that:

• Solves the specific problem without breaking existing functionality
• Maintains backward compatibility
• Includes appropriate test coverage
• Follows established patterns in the codebase

Recommendation: ✅ Approve - Ready for merge after any minor style improvements if desired.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review Complete ✅

Todo List:

• Read repository guidelines (CLAUDE.md) - None found
• Analyze the core source generation changes in AttributeWriter.cs
• Review type extension enhancements in TypeExtensions.cs
• Examine test files for coverage and implementation
• Provide comprehensive feedback on code quality, bugs, performance, security, and test coverage

---

🎯 Summary

This PR addresses issue #2893 by improving source generation handling for attributes on inherited async test methods from external assemblies. The fix introduces proper compiler-generated type detection and enhanced attribute writing capabilities.

📋 Code Quality & Best Practices

Excellent overall quality. The code follows C# conventions and demonstrates good architectural patterns:

✅ Strengths:

• Clear separation of concerns between syntax-based and metadata-based attribute writing
• Comprehensive null safety with proper nullable annotations
• Well-documented logic through meaningful method names and comments
• Consistent error handling patterns

✅ Minor suggestions:

• Consider extracting the compiler-generated type detection logic (lines 241-247 in AttributeWriter.cs) into a separate method for better readability
• The WriteDataSourceGeneratorPropertiesWithoutSyntax method could benefit from extracted constants for the repeated property type handling

🐛 Potential Issues

No critical bugs identified. The implementation appears robust:

✅ Good defensive programming:

• Proper null checks throughout (attributeData.AttributeClass is not null)
• Early returns prevent deep nesting
• Exception handling for IErrorTypeSymbol types (TypeExtensions.cs:44-47)

✅ Edge case handling:

• Compiler-generated types are properly filtered out
• Framework-specific attributes are conditionally included
• Both syntax-available and syntax-unavailable scenarios are handled

⚡ Performance Considerations

Well-optimized for a source generator:

✅ Efficient patterns:

• Uses lazy evaluation with FirstOrDefault() and early returns
• Minimizes allocations with StringBuilder for type name construction (TypeExtensions.cs:216-219)
• Proper use of ImmutableArray for thread safety

✅ Compilation efficiency:

• Filters out unusable attributes early in the pipeline (AttributeWriter.cs:25-28)
• Caches compilation lookups appropriately

🔒 Security Concerns

No security issues identified. This is source generation code with appropriate boundaries:

✅ Safe practices:

• No user input directly processed
• Type resolution uses compiler's semantic model
• No reflection or dynamic code execution at runtime

🧪 Test Coverage

Good test coverage approach:

✅ Comprehensive test scenarios:

• AsyncBaseTests.cs covers multiple async patterns:
  • Basic async methods (BaseAsyncTest)
  • Parameterized tests (BaseAsyncTestWithArguments)
  • Cancellation token handling (BaseAsyncTestWithCancellation)
  • Mixed sync/async methods
• AsyncInheritedTestsRepro.cs demonstrates the inheritance scenario that was failing

✅ Real-world test case:

• The repro class effectively validates the fix for cross-assembly inheritance
• Includes both inherited and local test methods

🎯 Technical Implementation Details

Key Fix Analysis:

1. AttributeWriter.cs:58-68 - New branching logic properly handles attributes without syntax reference (from external assemblies)
2. AttributeWriter.cs:236-286 - WriteAttributeWithoutSyntax method provides metadata-based attribute reconstruction
3. TypeExtensions.cs:231-248 - IsCompilerGeneratedType correctly identifies types that can't be represented in source code

The fix addresses the core issue: When inheriting async test methods from external assemblies, the source generator previously couldn't handle attributes because it relied on syntax references that don't exist for compiled code. The new approach falls back to metadata-based attribute writing.

📊 Overall Assessment

Excellent implementation - This is a well-thought-out fix that:

• Solves the specific problem without breaking existing functionality
• Maintains backward compatibility
• Includes appropriate test coverage
• Follows established patterns in the codebase

Recommendation: ✅ Approve - Ready for merge after any minor style improvements if desired.

---