feat: add support for special floating-point values (NaN, Infinity)

[thomhurst]

Fixes #3157

[claude]

Claude finished @thomhurst's task —— View job

---

Code Review Results

I've completed my review of the PR that adds support for special floating-point values (NaN, Infinity) in code generation. Here's my comprehensive feedback:

Todo List

• Read repository guidelines (CLAUDE.md)
• Analyze TypedConstantFormatter.cs changes
• Analyze FullyQualifiedWithGlobalPrefixRewriter.cs changes
• Review new SpecialFloatingPointValuesHelper.cs
• Analyze TypedConstantParser.cs changes
• Review test file for bug fix
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Evaluate test coverage
• Provide comprehensive feedback

Code Quality & Best Practices ✅

The implementation follows TUnit's coding standards well:

• Modern C# patterns: Good use of pattern matching in SpecialFloatingPointValuesHelper.cs:52-61
• Proper separation of concerns: New helper class isolates floating-point handling logic
• Consistent naming: Methods and variables follow established conventions
• Code organization: Files are well-structured with appropriate namespaces

Potential Issues & Bugs ⚠️

1. Redundant Null Checks

In TypedConstantFormatter.cs:253-260, there's redundancy with the helper call. The helper already checks for null at line 18-19, making the outer null check unnecessary.

2. Missing Edge Cases in Tests

The test file 3157_InfinitySymbolIssue.cs could be enhanced:

• Lines 40-42: Direct usage of special constants (good)
• Missing test cases for edge scenarios like comparing NaN values
• No negative test cases for malformed input

3. Potential Culture Issues

While the code uses InvariantCulture consistently (good practice), the special floating-point formatting doesn't explicitly handle culture-specific scenarios.

Performance Considerations ✅

The implementation is performance-conscious:

• Early returns: Helper methods return quickly for non-special values
• Minimal allocations: String formatting only when necessary
• Efficient pattern matching: Uses modern C# switch expressions
• No reflection overhead: Direct type checking with is patterns

Security Concerns ✅

No security issues identified:

• No user input parsing that could cause injection
• Proper handling of special values prevents potential numeric edge cases
• No dynamic code generation vulnerabilities

Test Coverage ⚠️

Good Coverage:

• Tests both float and double special values
• Covers NaN, PositiveInfinity, and NegativeInfinity
• Tests integration with the Arguments attribute

Missing Coverage:

• No tests for the syntax tree generation methods in SpecialFloatingPointValuesHelper
• Missing tests for the FullyQualifiedWithGlobalPrefixRewriter special handling
• No tests verifying the generated code compiles correctly

Architecture Alignment ✅

The implementation maintains behavioral parity between source-generated and reflection modes as required by TUnit's critical rules. The changes are isolated to the source generator components, which is appropriate.

Recommendations

1. Add snapshot tests for source generator output since this changes generated code formatting
2. Add unit tests for SpecialFloatingPointValuesHelper.TryCreateSpecialFloatingPointSyntax
3. Consider extracting constants for repeated special value strings ("NaN", "PositiveInfinity", etc.)
4. Verify that snapshot tests pass: dotnet test TUnit.Core.SourceGenerator.Tests

Overall Assessment

This is a solid implementation that properly addresses issue #3157. The code is well-structured, follows TUnit's patterns, and maintains performance standards. The main areas for improvement are around test coverage and potential redundancy cleanup.

Recommendation: Approve with minor suggestions ✅

[slang25]

You should be able to use ToCSharpString() on TypedConstant which should give you a valid C# representation of the value, however it's good that you didn't because it's doesn't work for this scenario 😅

I'm just thinking long term it might make more sense to get these things fixed in Roslyn, then it future proofs things and makes the TUnit code simpler.

I'll raise an issue over at dotnet/roslyn for this one (regardless of if it ends up useful here).