Repository navigation
Add ThreadStatic correctness analyzers - #1646
Conversation
Report ThreadStatic attributes on instance fields and inline initialization of thread-static state, with C# and VB coverage and analyzer documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds two new analyzers to improve correctness around ThreadStaticAttribute usage (VSTHRD116/VSTHRD117), covering both C# and Visual Basic, and publishes accompanying documentation and tests as part of the existing analyzer suite.
Changes:
- Introduces
VSTHRD116ThreadStaticAnalyzerto flagThreadStaticAttributeon non-static fields (including auto-property backing fields via[field: ...]). - Adds VSTHRD117 diagnostics to flag inline initialization of thread-static fields/backing fields via operation analysis.
- Adds localized strings plus DocFX/GitHub Pages documentation and a new test suite validating both rules for C# and VB.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD116ThreadStaticAnalyzerTests.cs | Adds coverage for VSTHRD116/VSTHRD117 across C# and VB scenarios. |
| src/Microsoft.VisualStudio.Threading.Analyzers/VSTHRD116ThreadStaticAnalyzer.cs | Implements the new analyzer and both diagnostics (non-static + inline initialization). |
| src/Microsoft.VisualStudio.Threading.Analyzers/Types.cs | Adds Types.ThreadStaticAttribute.FullName for reliable metadata lookup. |
| src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx | Adds localized titles/messages for VSTHRD116 and VSTHRD117. |
| docfx/analyzers/VSTHRD116.md | Adds DocFX help page for the new VSTHRD116 rule. |
| docfx/analyzers/VSTHRD117.md | Adds DocFX help page for the new VSTHRD117 rule. |
| docfx/analyzers/toc.yml | Registers the new analyzer docs in the DocFX TOC. |
| docfx/analyzers/index.md | Adds the new rules to the analyzer index table. |
| doc/analyzers/VSTHRD116.md | Adds redirect stub to GitHub Pages for VSTHRD116. |
| doc/analyzers/VSTHRD117.md | Adds redirect stub to GitHub Pages for VSTHRD117. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/ThreadStaticAnalyzerTests.cs:177
- Visual Basic tests cover fields, but there are no tests exercising the analyzer’s support for auto-property backing fields (e.g.,
<Field: ThreadStatic>on an auto-implementedProperty, and thePropertyinitializer case). Since the analyzer explicitly handlesSymbolKind.PropertyandOperationKind.PropertyInitializer, adding VB auto-property tests would help prevent regressions and validate the intended cross-language behavior.
[Fact]
public async Task NonStaticFields_VisualBasic()
{
const string test = """
Imports System
Class Test
<ThreadStatic>
Private {|VSTHRD116:first|} As Object
<Global.System.ThreadStaticAttribute>
Private {|VSTHRD116:second|}, {|VSTHRD116:third|} As Integer
End Class
""";
await VBVerify.VerifyAnalyzerAsync(test);
}
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/Microsoft.VisualStudio.Threading.Analyzers/ThreadStaticAnalyzer.cs:16
- Consider renaming this analyzer type to match the established naming convention in this project where DiagnosticAnalyzer implementations are prefixed with their primary diagnostic ID (e.g. VSTHRD115AvoidJoinableTaskContextCtorWithNullArgsAnalyzer, VSTHRD111UseConfigureAwaitAnalyzer). This makes analyzers easier to discover/grep when triaging diagnostics.
public class ThreadStaticAnalyzer : DiagnosticAnalyzer
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/ThreadStaticAnalyzerTests.cs:7
- To align with the existing test naming pattern in this project (most analyzer test classes start with the diagnostic ID, e.g. VSTHRD115AvoidJoinableTaskContextCtorWithNullArgTests), consider prefixing this test class name with the primary ID it covers.
public class ThreadStaticAnalyzerTests
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
ThreadStaticAttributeon non-static fields and auto-property backing fieldsValidation
analyzers/csandanalyzers/vbCloses #594
Closes #595