fix: casted enum array in arguments attribute causing source generator compilation failures #4066
Conversation
…r compilation failures
thomhurst
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
thomhurst
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
thomhurst
temporarily deployed
to
Pull Requests
GitHub Actions
Inactive
Code ReviewSummaryThis PR fixes issue #4065 where casted enum arrays in the Arguments attribute cause source generator compilation failures. The fix enhances the VisitCollectionExpression method to properly infer element types from parent cast expressions and from the collection elements themselves. Strengths
Observations and RecommendationsMinor: Potential Edge Cases
Performance: Hot Path Consideration The VisitCollectionExpression method is in a hot path (source generation). The current implementation is good - checks are ordered by likelihood and early exits when element type is found. The List allocation is acceptable given this is a syntax rewriter already allocating extensively. Code Style: Fully Compliant Modern C# syntax, clear variable names, proper use of braces, and comments explain the why. Testing Verification NeededPer TUnit CLAUDE.md guidelines:
Pre-Commit Checklist
VerdictRecommendation: Approve with minor suggestions The fix is sound and directly addresses the reported issue. Consider: (1) adding a test case for empty enum arrays, (2) optional null/error type check on line 242, (3) verify snapshot tests were run. Great work tracking down this edge case in the cast expression handling! |
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #4065 where casted enum arrays in Arguments attributes (e.g., (MyEnum[])[MyEnum.One, MyEnum.Two]) caused source generator compilation failures. The fix enhances the collection expression visitor in the source generator to handle additional scenarios for determining array element types.
Key Changes
- Added logic to extract element type from parent cast expressions when processing collection expressions
- Added fallback logic to infer element type from the first element when semantic model doesn't provide type information
- Added integration tests to verify casted enum arrays work correctly with the Arguments attribute
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| TUnit.TestProject/Bugs/4065/BugRepro4065.cs | Adds integration tests for casted enum array arguments using both collection expression syntax and traditional array initialization |
| TUnit.Core.SourceGenerator/CodeGenerators/Helpers/FullyQualifiedWithGlobalPrefixRewriter.cs | Enhances VisitCollectionExpression to handle cast expressions and infer element types when semantic model doesn't provide them |
| using TUnit.TestProject.Attributes; | ||
|
|
||
| namespace TUnit.TestProject.Bugs._4065; | ||
|
|
||
| public enum MyEnum | ||
| { | ||
| One, | ||
| Two, | ||
| Three | ||
| } | ||
|
|
||
| [EngineTest(ExpectedResult.Pass)] | ||
| public class BugRepro4065 | ||
| { | ||
| [Test] | ||
| [Arguments((MyEnum[])[MyEnum.One, MyEnum.Two])] | ||
| public async Task EnumArrayWithCollectionExpression(MyEnum[] values) | ||
| { | ||
| await Assert.That(values.Length).IsEqualTo(2); | ||
| await Assert.That(values[0]).IsEqualTo(MyEnum.One); | ||
| await Assert.That(values[1]).IsEqualTo(MyEnum.Two); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments((MyEnum[])[MyEnum.Three])] | ||
| public async Task EnumArraySingleElement(MyEnum[] values) | ||
| { | ||
| await Assert.That(values.Length).IsEqualTo(1); | ||
| await Assert.That(values[0]).IsEqualTo(MyEnum.Three); | ||
| } | ||
|
|
||
| [Test] | ||
| [Arguments(new MyEnum[] { MyEnum.One, MyEnum.Two })] // Old syntax (should still work) | ||
| public async Task EnumArrayWithNewSyntax(MyEnum[] values) | ||
| { | ||
| await Assert.That(values.Length).IsEqualTo(2); | ||
| } | ||
| } |
There was a problem hiding this comment.
This bug fix is missing a snapshot test in TUnit.Core.SourceGenerator.Tests. Based on the project's pattern (as seen in bugs 1538, 2085, etc.), there should be a corresponding test file at TUnit.Core.SourceGenerator.Tests/Bugs/4065/Tests4065.cs that verifies the generated code output matches expectations.
The snapshot test should:
- Extend TestsBase
- Call RunTest with the path to this test file
- Generate a .verified.txt file that captures the generated source code
This is critical because it ensures the source generator produces correct output for casted enum arrays and prevents future regressions. See TUnit.Core.SourceGenerator.Tests/Bugs/2085/Tests2085.cs as an example pattern to follow.
Fixes #4065