diff --git a/src/Analyzers/MSTest.Analyzers.CodeFixes/AssemblyCleanupShouldBeValidFixer.cs b/src/Analyzers/MSTest.Analyzers.CodeFixes/AssemblyCleanupShouldBeValidFixer.cs index 1ceb9cd634..96728f4228 100644 --- a/src/Analyzers/MSTest.Analyzers.CodeFixes/AssemblyCleanupShouldBeValidFixer.cs +++ b/src/Analyzers/MSTest.Analyzers.CodeFixes/AssemblyCleanupShouldBeValidFixer.cs @@ -72,6 +72,9 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context) if (fixesToApply != FixtureMethodSignatureChanges.None) { + // Ensure that the method will be static. + fixesToApply |= FixtureMethodSignatureChanges.MakeStatic; + context.RegisterCodeFix( CodeAction.Create( CodeFixResources.AssemblyCleanupShouldBeValidCodeFix, diff --git a/src/Analyzers/MSTest.Analyzers.CodeFixes/FixtureMethodSignatureChanges.cs b/src/Analyzers/MSTest.Analyzers.CodeFixes/FixtureMethodSignatureChanges.cs index 63a15c0529..478a7d6959 100644 --- a/src/Analyzers/MSTest.Analyzers.CodeFixes/FixtureMethodSignatureChanges.cs +++ b/src/Analyzers/MSTest.Analyzers.CodeFixes/FixtureMethodSignatureChanges.cs @@ -17,4 +17,5 @@ internal enum FixtureMethodSignatureChanges FixAsyncVoid = 32, FixReturnType = 64, RemoveGeneric = 128, + RemoveAbstract = 256, } diff --git a/src/Analyzers/MSTest.Analyzers.CodeFixes/Helpers/FixtureMethodFixer.cs b/src/Analyzers/MSTest.Analyzers.CodeFixes/Helpers/FixtureMethodFixer.cs index 5c6f3c0cbc..45b7913a2f 100644 --- a/src/Analyzers/MSTest.Analyzers.CodeFixes/Helpers/FixtureMethodFixer.cs +++ b/src/Analyzers/MSTest.Analyzers.CodeFixes/Helpers/FixtureMethodFixer.cs @@ -54,13 +54,15 @@ private static IEnumerable GetStatements(SyntaxNode node, SyntaxGene private static DeclarationModifiers GetModifiers(FixtureMethodSignatureChanges fixesToApply, IMethodSymbol methodSymbol) { - var currentModifiers = DeclarationModifiers.From(methodSymbol); + DeclarationModifiers newModifiers = methodSymbol.IsAsync + ? DeclarationModifiers.Async + : DeclarationModifiers.None; return fixesToApply.HasFlag(FixtureMethodSignatureChanges.MakeStatic) - ? currentModifiers.WithIsStatic(true) + ? newModifiers.WithIsStatic(true) : fixesToApply.HasFlag(FixtureMethodSignatureChanges.RemoveStatic) - ? currentModifiers.WithIsStatic(false) - : currentModifiers; + ? newModifiers.WithIsStatic(false) + : newModifiers; } private static Accessibility GetAccessibility(FixtureMethodSignatureChanges fixesToApply, IMethodSymbol methodSymbol) diff --git a/src/Analyzers/MSTest.Analyzers.CodeFixes/TestCleanupShouldBeValidFixer.cs b/src/Analyzers/MSTest.Analyzers.CodeFixes/TestCleanupShouldBeValidFixer.cs new file mode 100644 index 0000000000..1dd8569045 --- /dev/null +++ b/src/Analyzers/MSTest.Analyzers.CodeFixes/TestCleanupShouldBeValidFixer.cs @@ -0,0 +1,88 @@ +// 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 MSTest.Analyzers.Helpers; + +namespace MSTest.Analyzers; + +[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(TestCleanupShouldBeValidFixer))] +[Shared] +public sealed class TestCleanupShouldBeValidFixer : CodeFixProvider +{ + public sealed override ImmutableArray FixableDiagnosticIds { get; } + = ImmutableArray.Create(DiagnosticIds.TestCleanupShouldBeValidRuleId); + + 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; + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + SyntaxNode root = await context.Document.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + SyntaxNode node = root.FindNode(context.Span); + if (node == null) + { + return; + } + + FixtureMethodSignatureChanges fixesToApply = context.Diagnostics.Aggregate(FixtureMethodSignatureChanges.None, (acc, diagnostic) => + { + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.NotStaticRule) + { + return acc | FixtureMethodSignatureChanges.RemoveStatic; + } + + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.PublicRule) + { + return acc | FixtureMethodSignatureChanges.MakePublic; + } + + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) + { + return acc | FixtureMethodSignatureChanges.FixReturnType; + } + + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.NotAsyncVoidRule) + { + return acc | FixtureMethodSignatureChanges.FixAsyncVoid; + } + + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.NoParametersRule) + { + return acc | FixtureMethodSignatureChanges.RemoveParameters; + } + + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.NotGenericRule) + { + return acc | FixtureMethodSignatureChanges.RemoveGeneric; + } + + if (diagnostic.Descriptor == TestCleanupShouldBeValidAnalyzer.NotAbstractRule) + { + return acc | FixtureMethodSignatureChanges.RemoveAbstract; + } + + // return accumulator unchanged, either the action cannot be fixed or it will be fixed by default. + return acc; + }); + + if (fixesToApply != FixtureMethodSignatureChanges.None) + { + context.RegisterCodeFix( + CodeAction.Create( + CodeFixResources.AssemblyCleanupShouldBeValidCodeFix, + ct => FixtureMethodFixer.FixSignatureAsync(context.Document, root, node, fixesToApply, ct), + nameof(TestCleanupShouldBeValidFixer)), + context.Diagnostics); + } + } +} diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/TestCleanupShouldBeValidAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/TestCleanupShouldBeValidAnalyzerTests.cs index 8ecb71c680..8bb25e2236 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/TestCleanupShouldBeValidAnalyzerTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/TestCleanupShouldBeValidAnalyzerTests.cs @@ -6,7 +6,7 @@ using VerifyCS = MSTest.Analyzers.Test.CSharpCodeFixVerifier< MSTest.Analyzers.TestCleanupShouldBeValidAnalyzer, - Microsoft.CodeAnalysis.Testing.EmptyCodeFixProvider>; + MSTest.Analyzers.TestCleanupShouldBeValidFixer>; namespace MSTest.Analyzers.Test; @@ -68,11 +68,27 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [assembly: DiscoverInternals] + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public void TestCleanup() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.PublicRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } [Arguments("protected")] @@ -94,11 +110,25 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public void TestCleanup() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.PublicRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } public async Task WhenTestCleanupIsNotOrdinary_Diagnostic() @@ -116,11 +146,12 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.OrdinaryRule) .WithLocation(0) - .WithArguments("Finalize")); + .WithArguments("Finalize"), + code); } public async Task WhenTestCleanupIsAbstract_Diagnostic() @@ -136,11 +167,25 @@ public abstract class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public abstract class MyTestClass + { + [TestCleanup] + public void TestCleanup() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.NotAbstractRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } public async Task WhenTestCleanupIsGeneric_Diagnostic() @@ -158,11 +203,25 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public void TestCleanup() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.NotGenericRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } public async Task WhenTestCleanupIsStatic_Diagnostic() @@ -180,11 +239,25 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public void TestCleanup() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.NotStaticRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } public async Task WhenTestCleanupHasParameters_Diagnostic() @@ -202,11 +275,25 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public void TestCleanup() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.NoParametersRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } public async Task WhenTestCleanupReturnTypeIsNotValid_Diagnostic() @@ -244,20 +331,53 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + using System.Threading.Tasks; + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public void TestCleanup0() + { + } + + [TestCleanup] + public void TestCleanup1() + { + } + + [TestCleanup] + public Task {|CS0161:TestCleanup2|}() + { + } + + [TestCleanup] + public ValueTask {|CS0161:TestCleanup3|}() + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, - VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) - .WithLocation(0) - .WithArguments("TestCleanup0"), - VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) - .WithLocation(1) - .WithArguments("TestCleanup1"), - VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) - .WithLocation(2) - .WithArguments("TestCleanup2"), - VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) - .WithLocation(3) - .WithArguments("TestCleanup3")); + new[] + { + VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) + .WithLocation(0) + .WithArguments("TestCleanup0"), + VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) + .WithLocation(1) + .WithArguments("TestCleanup1"), + VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) + .WithLocation(2) + .WithArguments("TestCleanup2"), + VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.ReturnTypeRule) + .WithLocation(3) + .WithArguments("TestCleanup3"), + }, + fixedCode); } public async Task WhenTestCleanupReturnTypeIsValid_NoDiagnostic() @@ -308,10 +428,26 @@ public class MyTestClass } """; - await VerifyCS.VerifyAnalyzerAsync( + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + using System.Threading.Tasks; + + [TestClass] + public class MyTestClass + { + [TestCleanup] + public async Task TestCleanup() + { + await Task.Delay(0); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync( code, VerifyCS.Diagnostic(TestCleanupShouldBeValidAnalyzer.NotAsyncVoidRule) .WithLocation(0) - .WithArguments("TestCleanup")); + .WithArguments("TestCleanup"), + fixedCode); } }