fix: relax TUnit0046 to only warn for mutable reference types#4852
fix: relax TUnit0046 to only warn for mutable reference types#4852
Conversation
TUnit0046 previously warned on all reference types (except string), including immutable records, get-only classes, and types like Encoding and StringComparer. This narrows the diagnostic to only fire when the type shows evidence of mutability (non-init setters, public mutable fields, Lazy<T> members, arrays, interfaces, or type parameters). Fixes #4847 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| && lazyFieldType.ConstructedFrom.ToDisplayString() == "System.Lazy<T>") | ||
| { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The Lazy<T> field check has no accessibility filter, unlike the plain field check directly above it which correctly requires field.DeclaredAccessibility == Accessibility.Public. This causes false positives for the common private readonly Lazy<T> lazy-initialization pattern.
Example false positive:
public class MyService
{
private readonly Lazy<string> _computed = new(() => ExpensiveOp());
public string Value => _computed.Value; // Only getter exposed
}HasVisibleMutability() returns true for MyService despite no visible mutability, because the private Lazy<T> field matches this check. The same issue exists for the lazyProp check a few lines below.
The fix is to add an accessibility guard consistent with the plain-field check above:
| } | |
| // Lazy<T> field or property (any accessibility) — deferred mutation | |
| if (member is IFieldSymbol lazyField | |
| && lazyField.DeclaredAccessibility == Accessibility.Public | |
| && lazyField.Type is INamedTypeSymbol { IsGenericType: true } lazyFieldType | |
| && lazyFieldType.ConstructedFrom.ToDisplayString() == "System.Lazy<T>") | |
| { | |
| return true; | |
| } |
Why this matters: The method is named HasVisibleMutability() — if a Lazy<T> field is private, it's not "visible" to test code and doesn't represent a shared-state risk from the caller's perspective.
Code reviewThis PR makes a solid improvement to TUnit0046 — narrowing it from all reference types to only mutable reference types is the right call, and the One issue found: The The same applies to the |
Summary
string)HasVisibleMutability()extension method that checks for non-init setters, public mutable fields,Lazy<T>members, arrays, interfaces, and type parametersFixes #4847
Test plan
TUnit.AnalyzersandTUnit.Analyzers.Roslyn44build successfully🤖 Generated with Claude Code