Add code fix for MSTEST0031 — DoNotUseSystemDescriptionAttribute#7898
Add code fix for MSTEST0031 — DoNotUseSystemDescriptionAttribute#7898
Conversation
Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/bfc27008-89bf-43d2-8334-80122ee20056 Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
|
@copilot that seems wrong, we should instead use MSTest |
…yName Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/8f6b70e9-bcdc-47c6-83a2-e56a7118878c Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Updated in aace8a7. The fixer now replaces |
There was a problem hiding this comment.
Summary
Workflow: Test Expert Reviewer 🧪
Date: 2026-04-28
Repository: microsoft/testfx
Key Findings
- [Coverage] Missing edge case: shortened attribute form with
using System.ComponentModel;
The added code-fix test covers only the fully-qualified[System.ComponentModel.Description("...")]syntax. The analyzer (which uses semantic analysis) would also trigger on the shortened[Description("...")]form whenusing System.ComponentModel;is in scope. In that scenario, the fixer's current implementation — unconditionally replacing the attribute name with the simple identifierDescription— could produce ambiguous code that won't compile, because bothSystem.ComponentModel.DescriptionAttributeandMicrosoft.VisualStudio.TestTools.UnitTesting.DescriptionAttributewould be in scope. A test case covering this form is needed to surface and protect against that behavior.
What's Good
- The test follows the repo's established pattern (
CSharpCodeFixVerifier<TAnalyzer, TFixer>alias,VerifyCodeFixAsync) perfectly. - Roslyn inline diagnostic markup (
[|MyTestMethod|]) is used correctly. - The no-diagnostic path (
WhenMethodWithoutTestMethodAttribute_HasSystemDescriptionAttribute_NoDiagnostic) is retained and correct. - No isolation or flakiness concerns — all tests run in-memory via the Roslyn testing framework.
Recommendations
- Add a test case for the shortened
[Description("text")]form whenusing System.ComponentModel;is present. This will reveal whether the fixer needs to also remove (or adjust) the conflictingusingdirective to produce valid code.
Generated by Test Expert Reviewer 🧪
🧪 Test quality reviewed by Test Expert Reviewer 🧪
There was a problem hiding this comment.
Pull request overview
Adds a C# code fix for MSTEST0031 (“DoNotUseSystemDescriptionAttribute”) so users can automatically replace System.ComponentModel.DescriptionAttribute on test methods with MSTest’s DescriptionAttribute, along with the necessary localized UI strings and a verifier update.
Changes:
- Introduces
DoNotUseSystemDescriptionAttributeFixercode fix provider. - Adds the new code fix title to
CodeFixResources.resxand propagates it to localized*.xlffiles. - Updates the analyzer unit test to validate the code fix result.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/MSTest.Analyzers.UnitTests/DoNotUseSystemDescriptionAttributeAnalyzerTests.cs | Switches to the real code fix provider and adds a code-fix verification. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/DoNotUseSystemDescriptionAttributeFixer.cs | New fixer that rewrites the attribute name for MSTEST0031. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/CodeFixResources.resx | Adds the code-fix title string. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.cs.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.de.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.es.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.fr.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.it.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.ja.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.ko.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.pl.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.pt-BR.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.ru.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.tr.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.zh-Hans.xlf | Adds localized entry for the new code-fix title. |
| src/Analyzers/MSTest.Analyzers.CodeFixes/xlf/CodeFixResources.zh-Hant.xlf | Adds localized entry for the new code-fix title. |
Copilot's findings
- Files reviewed: 16/16 changed files
- Comments generated: 2
There was a problem hiding this comment.
Summary
Workflow: Expert Code Reviewer 🧠
Date: 2026-04-28
Repository: microsoft/testfx
Key Findings
[Correctness] Ambiguous/no-op fix when System.ComponentModel namespace is imported
File: src/Analyzers/MSTest.Analyzers.CodeFixes/DoNotUseSystemDescriptionAttributeFixer.cs (line 100)
The fix replaces the System attribute name with the bare identifier "Description". This works only in the narrow case where using Microsoft.VisualStudio.TestTools.UnitTesting; is in scope and using System.ComponentModel; is not.
Two broken scenarios the current test suite does not cover:
- Both namespaces imported →
[Description]becomes ambiguous (CS0104), producing non-compiling code. - Only
System.ComponentModelimported (with a fully-qualified[Microsoft.VisualStudio.TestTools.UnitTesting.TestMethod]) → the replacement is syntactically identical to what was there before; the MSTest namespace still isn't in scope forDescription, so the diagnostic would still fire.
Recommended fix: Use the fully-qualified MSTest name annotated with Simplifier.Annotation, letting the workspace simplifier reduce it only when the reference is unambiguous:
// using Microsoft.CodeAnalysis.Simplification;
AttributeSyntax newAttribute = systemDescriptionAttribute.WithName(
SyntaxFactory.ParseName("Microsoft.VisualStudio.TestTools.UnitTesting.Description")
.WithTriviaFrom(systemDescriptionAttribute.Name)
.WithAdditionalAnnotations(Simplifier.Annotation));
SyntaxNode root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
Document updatedDocument = document.WithSyntaxRoot(root.ReplaceNode(systemDescriptionAttribute, newAttribute));
return await Simplifier.ReduceAsync(updatedDocument, cancellationToken: cancellationToken).ConfigureAwait(false);A new test case should be added to cover the scenario where using System.ComponentModel; is also present, verifying that the result is unambiguous.
Positive Observations
- The semantic-model–based attribute discovery (iterating attribute lists and comparing
ContainingTypeviaSymbolEqualityComparer) is correct and robust across fully-qualified and simple-name forms. GetFixAllProvider()→BatchFixeris the right choice for this single-replacement fix.ConfigureAwait(false)is consistently applied on all awaited calls.- The
.xlflocalization files were auto-generated (not hand-edited), matching the repo convention.
Generated by Expert Code Reviewer
🧠 Reviewed by Expert Code Reviewer 🧠
|
@copilot resolve the merge conflicts in this pull request and address review comments |
…ation Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/6b91d4c0-83e5-407b-a2dc-9fe8712e11dd Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Resolved the merge conflict in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot resolve the merge conflicts in this pull request |
Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/9866530f-a95f-4703-8378-d7ae99ad9510 Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
|
@copilot resolve the merge conflicts in this pull request |
…1-description-attribute # Conflicts: # src/Analyzers/MSTest.Analyzers.CodeFixes/CodeFixResources.resx Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
- Test 2: Use fully-qualified [System.ComponentModel.Description] in input to avoid CS0104 ambiguity when both namespaces are imported - Tests 2 & 3: Expect 'Description' instead of 'DescriptionAttribute' since Roslyn Simplifier strips the Attribute suffix
Evangelink
left a comment
There was a problem hiding this comment.
Summary
Workflow: Test Expert Reviewer 🧪
Date: 2026-04-30
Repository: microsoft/testfx
Key Findings
Good: Tests are well-isolated, use strong VerifyCodeFixAsync assertions that verify both the diagnostic location (via [|...|] markers) and the exact post-fix code. No shared mutable state, no timing dependencies, no flakiness patterns.
Coverage gap (1 finding): The PR description explicitly describes a scenario where the user writes the short form [Description("...")] with using System.ComponentModel in scope (where it resolves to System.ComponentModel.DescriptionAttribute). None of the three new code-fix tests cover this form — they all use the fully-qualified [System.ComponentModel.Description("...")] input. The inline comment above details the missing test case.
Recommendations
- Add a test for
[Description("...")](short form) +using System.ComponentModelin scope → fixer should produce[Microsoft.VisualStudio.TestTools.UnitTesting.Description("...")]. See inline comment for suggested test.
Generated by Test Expert Reviewer
🧪 Test quality reviewed by Test Expert Reviewer 🧪
| } | ||
| """; | ||
|
|
||
| await VerifyCS.VerifyCodeFixAsync(code, fixedCode); |
There was a problem hiding this comment.
[Coverage] The PR description explicitly calls out a key scenario that is not tested: the user writes the short form [Description("...")] when using System.ComponentModel is in scope, and that short form resolves to System.ComponentModel.DescriptionAttribute. The current test (WhenTestMethodHasSystemDescriptionAttributeWithSystemComponentModelUsing) only covers the fully-qualified input [System.ComponentModel.Description("...")].
Impact: If the fixer silently fails (returns the original document) or produces incorrect output for the short-form input, no test would catch it.
Suggestion: Add a complementary test case:
[TestMethod]
public async Task WhenTestMethodHasShortFormDescriptionAttributeWithSystemComponentModelUsing_UsesFullyQualifiedMSTestDescription()
{
string code = """
using System.ComponentModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;
[TestClass]
public class MyTestClass
{
[TestMethod]
[Description("Description")]
public void [|MyTestMethod|]()
{
}
}
""";
string fixedCode = """
using System.ComponentModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;
[TestClass]
public class MyTestClass
{
[TestMethod]
[Microsoft.VisualStudio.TestTools.UnitTesting.Description("Description")]
public void MyTestMethod()
{
}
}
""";
await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
}This is the scenario described in the PR body under "Before / After (with using System.ComponentModel also imported)".
Evangelink
left a comment
There was a problem hiding this comment.
Summary
Workflow: Expert Code Reviewer 🧠
Date: 2026-04-30
Repository: microsoft/testfx
Key Findings
No issues found. The implementation is correct and well-structured across all review dimensions.
Correctness ✅
RegisterCodeFixesAsynccorrectly locates theMethodDeclarationSyntaxby walking up from the diagnostic's token (placed on the method identifier bymethodSymbol.CreateDiagnostic).ReplaceWithMSTestDescriptionAttributeAsyncuses the semantic model to identify the exactSystem.ComponentModel.DescriptionAttributeconstructor symbol (viaGetSymbolInfo+SymbolEqualityComparer.Default), which robustly handles aliases and qualified names.- The
Simplifier.Annotation+Simplifier.ReduceAsyncpattern correctly produces the minimal unambiguous form: simplified to[Description]when MSTest namespace is in scope without conflict, kept fully-qualified whenusing System.ComponentModelis also present or when nousingis present. All three cases are tested. - The fully-qualified name
Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttributeis correctly simplified by Roslyn (includingAttributesuffix elision in attribute position) to match expected test outputs.
Threading / Concurrency ✅
Design-time code-fixer code; no static mutable state; ConfigureAwait(false) used consistently throughout all async paths.
Performance ✅
Not on a hot path; Roslyn caches the syntax tree so the double root retrieval (RegisterCodeFixesAsync + ReplaceWithMSTestDescriptionAttributeAsync) has no real cost.
Public API & Binary Compatibility ✅
New public sealed class, pure addition. No existing API changes.
Cross-TFM ✅
Analyzer targets C#+VB; fixer is C#-only (standard practice). No TFM-specific APIs used without guards.
Resources / IDisposable ✅
No disposables introduced without cleanup.
Security ✅
Operates purely on Roslyn syntax/semantic APIs; no injection or path traversal risk.
Defensive Coding ✅
Null guards at all early-exit points; systemDescriptionAttributeSymbol is null and methodDeclaration is null checks both present.
Positive Observations
- Follows all established code-fixer patterns in the codebase (same structure as
UseAttributeOnTestMethodFixer,PreferDisposeOverTestCleanupFixer, etc.). WellKnownTypeNamesconstants reused correctly — no magic strings in the fixer.- Three well-chosen test scenarios cover: typical case, ambiguity case (both usings), and no-using case.
Generated by Expert Code Reviewer
🧠 Reviewed by Expert Code Reviewer 🧠
MSTEST0031 warns when
[System.ComponentModel.Description("...")]is applied to a test method, but provided no automated fix. This adds a code fixer that replaces theSystem.ComponentModel.DescriptionAttributewith MSTest's ownDescriptionAttribute, using Roslyn'sSimplifierto produce the correct name form based on the currentusingdirectives.Fix behavior
[System.ComponentModel.Description("text")](or[Description("text")]bound toSystem.ComponentModel) with MSTest's[Description("text")](Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute)Simplifier.ReduceAsyncwith the fully-qualifiedMicrosoft.VisualStudio.TestTools.UnitTesting.DescriptionAttributename so the output is always correct:[Description("text")]when the MSTest namespace is unambiguously in scope[Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute("text")]whenusing System.ComponentModelis also present (ambiguous) or when no namespace is importedBefore / After (typical case):
Before / After (with
using System.ComponentModelalso imported):Changes
DoNotUseSystemDescriptionAttributeFixer.cs— new fixer using the semantic model to identify theSystem.ComponentModel.DescriptionAttribute, replaces its name with the fully-qualified MSTestDescriptionAttributeannotated withSimplifier.Annotation, then callsSimplifier.ReduceAsyncto produce the minimal unambiguous formCodeFixResources.resx— addsUseMSTestDescriptionAttributeInsteadFixtitle stringDoNotUseSystemDescriptionAttributeAnalyzerTests.cs— switches fromEmptyCodeFixProviderto the new fixer; adds code-fix tests covering the standard case, theusing System.ComponentModelambiguity case, and the fully-qualified (nousing) case