Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -216,4 +216,7 @@
<data name="RemoveDuplicateDataRowFix" xml:space="preserve">
<value>Remove duplicate 'DataRow'</value>
</data>
<data name="UseMSTestDescriptionAttributeInsteadFix" xml:space="preserve">
<value>Use MSTest 'Description' attribute instead</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Immutable;
using System.Composition;

using Analyzer.Utilities;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Simplification;

using MSTest.Analyzers.Helpers;

namespace MSTest.Analyzers;

/// <summary>
/// Code fixer for <see cref="DoNotUseSystemDescriptionAttributeAnalyzer"/>.
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(DoNotUseSystemDescriptionAttributeFixer))]
[Shared]
public sealed class DoNotUseSystemDescriptionAttributeFixer : CodeFixProvider
{
/// <inheritdoc />
public override ImmutableArray<string> FixableDiagnosticIds { get; }
= ImmutableArray.Create(DiagnosticIds.DoNotUseSystemDescriptionAttributeRuleId);

/// <inheritdoc />
public override FixAllProvider GetFixAllProvider()
// See https://github.com/dotnet/roslyn/blob/main/docs/analyzers/FixAllProvider.md for more information on Fix All Providers
=> WellKnownFixAllProviders.BatchFixer;

/// <inheritdoc />
public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

Diagnostic diagnostic = context.Diagnostics[0];
SyntaxToken syntaxToken = root.FindToken(diagnostic.Location.SourceSpan.Start);
if (syntaxToken.Parent is null)
{
return;
}

MethodDeclarationSyntax? methodDeclaration = syntaxToken.Parent.AncestorsAndSelf().OfType<MethodDeclarationSyntax>().FirstOrDefault();
if (methodDeclaration is null)
{
return;
}

context.RegisterCodeFix(
CodeAction.Create(
title: CodeFixResources.UseMSTestDescriptionAttributeInsteadFix,
createChangedDocument: c => ReplaceWithMSTestDescriptionAttributeAsync(context.Document, methodDeclaration, c),
equivalenceKey: nameof(DoNotUseSystemDescriptionAttributeFixer)),
diagnostic);
}

private static async Task<Document> ReplaceWithMSTestDescriptionAttributeAsync(Document document, MethodDeclarationSyntax methodDeclaration, CancellationToken cancellationToken)
{
SemanticModel semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false);

INamedTypeSymbol? systemDescriptionAttributeSymbol = semanticModel.Compilation.GetTypeByMetadataName(WellKnownTypeNames.SystemDescriptionAttribute);

if (systemDescriptionAttributeSymbol is null)
{
return document;
}

AttributeSyntax? systemDescriptionAttribute = null;

foreach (AttributeListSyntax attributeList in methodDeclaration.AttributeLists)
{
foreach (AttributeSyntax attribute in attributeList.Attributes)
{
if (semanticModel.GetSymbolInfo(attribute, cancellationToken).Symbol is IMethodSymbol { ContainingType: { } containingType }
&& SymbolEqualityComparer.Default.Equals(containingType, systemDescriptionAttributeSymbol))
{
systemDescriptionAttribute = attribute;
break;
}
}

if (systemDescriptionAttribute is not null)
{
break;
}
}

if (systemDescriptionAttribute is null)
{
return document;
}

// Replace the System.ComponentModel.Description attribute name with the fully-qualified MSTest Description
// attribute name, annotated for simplification. The Simplifier will reduce it to the simple name if the
// MSTest namespace is already in scope and there is no ambiguity; otherwise it keeps the fully-qualified form.
NameSyntax msTestDescriptionName = SyntaxFactory.ParseName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingDescriptionAttribute)
.WithTriviaFrom(systemDescriptionAttribute.Name)
.WithAdditionalAnnotations(Simplifier.Annotation);

AttributeSyntax newAttribute = systemDescriptionAttribute.WithName(msTestDescriptionName);

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Místo řetězcového argumentu použijte vlastnost DisplayName</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Použít {0}</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Verwenden Sie die Eigenschaft „DisplayName“ anstelle eines Zeichenfolgenarguments.</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">"{0}" verwenden</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Usar la propiedad "DisplayName" en lugar del argumento de cadena</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Usar "{0}"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Utilisez la propriété « DisplayName » au lieu d’un argument de type chaîne</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Utiliser « {0} »</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Usare la proprietà 'DisplayName' invece di un argomento stringa</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Usa '{0}'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">文字列引数の代わりに 'DisplayName' プロパティを使用する</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">'{0}' を使用します</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">문자열 인수 대신 'DisplayName' 속성 사용</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">'{0}' 사용</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Użyj właściwości „DisplayName” zamiast argumentu ciągu</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Użyj „{0}”</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Usar a propriedade "DisplayName" em vez do argumento de cadeia de caracteres</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Usar '{0}'</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Использовать свойство "DisplayName" вместо строкового аргумента</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">Использовать "{0}"</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">Dize bağımsız değişkeni yerine 'DisplayName' özelliğini kullanın</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">'{0}' kullan</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">使用属性‘DisplayName’替代字符串参数</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">使用“{0}”</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,11 @@
<target state="translated">使用 'DisplayName' 屬性取代字串引數</target>
<note />
</trans-unit>
<trans-unit id="UseMSTestDescriptionAttributeInsteadFix">
<source>Use MSTest 'Description' attribute instead</source>
<target state="new">Use MSTest 'Description' attribute instead</target>
<note />
</trans-unit>
<trans-unit id="UseProperAssertMethodsFix">
<source>Use '{0}'</source>
<target state="translated">使用 '{0}'</target>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using VerifyCS = MSTest.Analyzers.Test.CSharpCodeFixVerifier<
MSTest.Analyzers.DoNotUseSystemDescriptionAttributeAnalyzer,
Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>;
MSTest.Analyzers.DoNotUseSystemDescriptionAttributeFixer>;

namespace MSTest.Analyzers.Test;

[TestClass]
public sealed class DoNotUseSystemDescriptionAttributeAnalyzerTests
{
[TestMethod]
public async Task WhenTestMethodHasSystemDescriptionAttribute_Diagnostic()
public async Task WhenTestMethodHasFullyQualifiedSystemDescriptionAttribute_Diagnostic()
{
string code = """
using Microsoft.VisualStudio.TestTools.UnitTesting;
Expand All @@ -27,7 +27,87 @@ public class MyTestClass
}
""";

await VerifyCS.VerifyAnalyzerAsync(code);
string fixedCode = """
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[TestMethod]
[Description("Description")]
public void MyTestMethod()
{
}
}
""";

await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
Comment thread
Evangelink marked this conversation as resolved.
}

[TestMethod]
Comment thread
Evangelink marked this conversation as resolved.
public async Task WhenTestMethodHasSystemDescriptionAttributeWithSystemComponentModelUsing_UsesFullyQualifiedMSTestDescription()
{
string code = """
using System.ComponentModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;

[TestClass]
public class MyTestClass
{
[TestMethod]
[System.ComponentModel.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);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)".

}

[TestMethod]
public async Task WhenTestMethodHasSystemDescriptionAttributeWithoutUnitTestingUsing_UsesFullyQualifiedMSTestDescription()
{
string code = """
[Microsoft.VisualStudio.TestTools.UnitTesting.TestClass]
public class MyTestClass
{
[Microsoft.VisualStudio.TestTools.UnitTesting.TestMethod]
[System.ComponentModel.Description("Description")]
public void [|MyTestMethod|]()
{
}
}
""";

string fixedCode = """
[Microsoft.VisualStudio.TestTools.UnitTesting.TestClass]
public class MyTestClass
{
[Microsoft.VisualStudio.TestTools.UnitTesting.TestMethod]
[Microsoft.VisualStudio.TestTools.UnitTesting.Description("Description")]
public void MyTestMethod()
{
}
}
""";

await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
}

[TestMethod]
Expand Down
Loading