Audit analyzers for ValueTask support (#414) - #1675
Open
Andrew Arnott (AArnott) wants to merge 1 commit into
Open
Audit analyzers for ValueTask support (#414)#1675Andrew Arnott (AArnott) wants to merge 1 commit into
Andrew Arnott (AArnott) wants to merge 1 commit into
Conversation
Audited all analyzers for issue #414 (ValueTask support). Found: - VSTHRD111 (ConfigureAwait): already handles ValueTask explicitly. - VSTHRD011, VSTHRD109, VSTHRD200: use HasAsyncCompatibleReturnType/IsAsyncCompatibleReturnType, which already recognize ValueTask via its AsyncMethodBuilderAttribute. - VSTHRD110, VSTHRD200 Remove-suffix check: use AwaitableTypeTester, which discovers any type with a GetAwaiter method, including ValueTask. - VSTHRD012, VSTHRD108, VSTHRD001, VSTHRD004, VSTHRD100, VSTHRD106, VSTHRD116/117: not Task-type-specific; no gap. - VSTHRD114 (avoid returning null Task): its doc comment incorrectly claimed ValueTask support, but ValueTask/ValueTask<T> are value types so \ eturn null;\ is a compile error (CS0037) and can never reach this analyzer. Corrected the doc comment to explain this instead of implying non-existent support. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The change is limited to an accurate documentation correction and aligns with the stated PR intent without introducing behavioral risk.
Pull request overview
This PR addresses issue #414 by auditing the Microsoft.VisualStudio.Threading.Analyzers package for ValueTask/ValueTask<T> support, and concludes there were no analyzer behavior gaps. The only code change is a documentation correction for VSTHRD114 to remove misleading claims about ValueTask applicability.
Changes:
- Corrected the XML documentation on
AbstractVSTHRD114AvoidReturningNullTaskAnalyzerto accurately describe what the analyzer checks. - Added remarks explaining why
ValueTask/ValueTask<T>are not applicable to the “return null task” pattern (value types; compiler prevents it).
File summaries
| File | Description |
|---|---|
| src/Microsoft.VisualStudio.Threading.Analyzers/AbstractVSTHRD114AvoidReturningNullTaskAnalyzer.cs | Fixes incorrect/misleading doc comment and clarifies ValueTask non-applicability for VSTHRD114. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #414.
Audited all analyzers in
Microsoft.VisualStudio.Threading.AnalyzersforValueTask/ValueTask<T>support:ValueTask.HasAsyncCompatibleReturnType/IsAsyncCompatibleReturnType, which already recognizeValueTaskvia itsAsyncMethodBuilderAttribute.AwaitableTypeTester, which discovers any type with aGetAwaitermethod (includingValueTask) dynamically from the compilation.Task-type-specific to begin with; no gap applies.Task): its doc comment incorrectly claimedValueTasksupport (likely copy-pasted from VSTHRD111). In reality,ValueTask/ValueTask<T>are value types, soreturn null;is a compile error (CS0037) and this code path can never be reached for them. Corrected the doc comment to explain whyValueTaskisn't applicable here instead of implying non-existent support.No behavioral analyzer gaps were found — only a misleading doc comment was corrected. All existing VSTHRD114 tests (24) continue to pass.