xunit/xunit#3423: Suppress CS8618 for members set in InitializeAsync#203
Conversation
Add a DiagnosticSuppressor that suppresses CS8618 (non-nullable field/property not initialized) when the member is directly assigned in an IAsyncLifetime.InitializeAsync implementation. Handles both fields and properties, including this-qualified access. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@dotnet-policy-service agree |
When a class has an explicit constructor, CS8618 fires on the constructor rather than the field/property. The suppressor now resolves the target member by checking AdditionalLocations and falling back to parsing the member name from the diagnostic message. Also adds V3 (ValueTask-based IAsyncLifetime) test coverage for all scenarios, bringing total tests from 6 to 16. Inspired by edge cases identified by @ssaporito in #3423.
|
Hi, just checking in — is there anything else needed from my side on this PR? Happy to make adjustments if needed. |
|
Friendly ping — this PR has been open for ~25 days now. Is there anything I can adjust or improve to help move it forward? Happy to rework anything needed. |
|
Marking this as stale on my end — it's been 27 days with no maintainer response. The PR is still mergeable and CI is green, so happy to revisit if this gets picked up. Thanks! |
|
Sorry, other items have been higher priority for me at the moment. I do appreciate the work and will get to it. 🙇🏼♂️ |
of course np. thank you! |
|
There are some warnings causing failures that I don't think are caused by your PR, just that you happen to trigger them. I'm looking at them right now. Example:
|
…618-async-lifetime
|
After fixing the reference issue, it appears that none of the tests that verify the suppression are passing with latest Roslyn: 
|
Newer Roslyn versions emit AdditionalLocations on CS8618 diagnostics (dotnet/roslyn#58096), which causes the test framework to fail when the expected DiagnosticResult doesn't account for them. Add DiagnosticOptions.IgnoreAdditionalLocations to all suppressed diagnostic expectations so tests pass on both old and new Roslyn. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Thanks for looking into this! I dug into it — the issue is that newer Roslyn (4.14+) started emitting The suppressor itself works fine — the diagnostics are correctly suppressed. It's just the test assertions that were too strict. I've pushed a fix that adds pls lmk if this is fine with you |
That seems reasonable to me. |
|
Thanks! |
Summary
Adds a
DiagnosticSuppressorthat suppresses CS8618 ("Non-nullable field/property must contain a non-null value when exiting constructor") when the member is directly assigned in anIAsyncLifetime.InitializeAsyncimplementation. Fixes xunit/xunit#3423.Changes
NonNullableFieldInitializationSuppressor— walks theInitializeAsyncmethod body looking for direct assignments to the flagged memberCS8618_Suppressiondescriptor toDescriptors.Suppressors.csVerifyCompilerWarningSuppressortest helpers toCSharpVerifier.Suppressors.cs(setsCompilerDiagnostics.Warningsso CS8618 is included in analysis)this.-qualified access, and selective suppression with multiple fieldsDesign
The suppressor:
IAsyncLifetimeInitializeAsyncimplementation viaFindImplementationForInterfaceMemberKnown Limitations
InitializeAsyncbody — assignments through helper methods called fromInitializeAsyncare not detected (conservative approach)Testing