diff --git a/src/Analyzers/MSTest.Analyzers.CodeFixes/AvoidOutRefTestMethodParametersFixer.cs b/src/Analyzers/MSTest.Analyzers.CodeFixes/AvoidOutRefTestMethodParametersFixer.cs index 53f7d1005e..e4c99eb992 100644 --- a/src/Analyzers/MSTest.Analyzers.CodeFixes/AvoidOutRefTestMethodParametersFixer.cs +++ b/src/Analyzers/MSTest.Analyzers.CodeFixes/AvoidOutRefTestMethodParametersFixer.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Text; using MSTest.Analyzers.Helpers; @@ -61,20 +62,42 @@ private static async Task RemoveOutRefModifiersAsync(Document document DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); foreach (ParameterSyntax parameter in methodDeclaration.ParameterList.Parameters) { - int indexToRemove = parameter.Modifiers.IndexOf(SyntaxKind.OutKeyword); - if (indexToRemove < 0) + SyntaxToken[] removedModifiers = parameter.Modifiers + .Where(modifier => modifier.IsKind(SyntaxKind.OutKeyword) + || modifier.IsKind(SyntaxKind.RefKeyword) + || modifier.IsKind(SyntaxKind.ReadOnlyKeyword)) + .ToArray(); + if (!removedModifiers.Any(modifier => modifier.IsKind(SyntaxKind.OutKeyword) || modifier.IsKind(SyntaxKind.RefKeyword))) { - indexToRemove = parameter.Modifiers.IndexOf(SyntaxKind.RefKeyword); + continue; } - if (indexToRemove >= 0) - { - editor.ReplaceNode(parameter, (node, _) => - { - var parameter = (ParameterSyntax)node; - return parameter.WithModifiers(parameter.Modifiers.RemoveAt(indexToRemove)).WithLeadingTrivia(parameter.GetLeadingTrivia()); - }); - } + SyntaxTokenList filteredModifiers = SyntaxFactory.TokenList( + parameter.Modifiers.Except(removedModifiers)); + + ParameterSyntax updatedParameter = parameter.WithModifiers(filteredModifiers); + SyntaxTrivia[] removedTrivia = removedModifiers + .SelectMany(modifier => modifier.LeadingTrivia.Concat(modifier.TrailingTrivia)) + .ToArray(); + SyntaxTrivia[] preservedTrivia = removedTrivia + .SkipWhile(trivia => trivia.IsKind(SyntaxKind.WhitespaceTrivia)) + .Reverse() + .SkipWhile(trivia => trivia.IsKind(SyntaxKind.WhitespaceTrivia)) + .Reverse() + .ToArray(); + IEnumerable typeLeadingTrivia = preservedTrivia.Length > 0 + && !preservedTrivia[^1].IsKind(SyntaxKind.EndOfLineTrivia) + ? preservedTrivia.Append(SyntaxFactory.Space) + : preservedTrivia; + + updatedParameter = parameter.Type is { } parameterType + && preservedTrivia.Any(trivia => !trivia.IsKind(SyntaxKind.EndOfLineTrivia)) + ? updatedParameter.WithType(parameterType.WithLeadingTrivia(typeLeadingTrivia.Concat(parameterType.GetLeadingTrivia()))) + : updatedParameter.WithLeadingTrivia(parameter.GetLeadingTrivia()); + + editor.ReplaceNode( + parameter, + updatedParameter.WithAdditionalAnnotations(Formatter.Annotation)); } return editor.GetChangedDocument(); diff --git a/src/Analyzers/MSTest.Analyzers/AvoidOutRefTestMethodParametersAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/AvoidOutRefTestMethodParametersAnalyzer.cs index 2011885971..899a1a333c 100644 --- a/src/Analyzers/MSTest.Analyzers/AvoidOutRefTestMethodParametersAnalyzer.cs +++ b/src/Analyzers/MSTest.Analyzers/AvoidOutRefTestMethodParametersAnalyzer.cs @@ -61,9 +61,18 @@ private static void AnalyzeSymbol(SymbolAnalysisContext context, INamedTypeSymbo } // Check for out/ref parameters - if (methodSymbol.Parameters.Any(p => p.RefKind is RefKind.Out or RefKind.Ref)) + if (methodSymbol.Parameters.Any(parameter => IsOutOrRefParameter(parameter, context.CancellationToken))) { context.ReportDiagnostic(methodSymbol.CreateDiagnostic(AvoidOutRefParametersRule, methodSymbol.Name)); } } + + private static bool IsOutOrRefParameter(IParameterSymbol parameter, CancellationToken cancellationToken) + => parameter.RefKind is RefKind.Out or RefKind.Ref + || parameter.DeclaringSyntaxReferences.Any(reference => + { + SyntaxNode parameterSyntax = reference.GetSyntax(cancellationToken); + return parameterSyntax.ChildTokens().Any(token => token.Text is "ref") + && parameterSyntax.ChildTokens().Any(token => token.Text is "readonly"); + }); } diff --git a/src/Analyzers/MSTest.Analyzers/NonNullableReferenceNotInitializedSuppressor.cs b/src/Analyzers/MSTest.Analyzers/NonNullableReferenceNotInitializedSuppressor.cs index 201c1dd520..c82681fcea 100644 --- a/src/Analyzers/MSTest.Analyzers/NonNullableReferenceNotInitializedSuppressor.cs +++ b/src/Analyzers/MSTest.Analyzers/NonNullableReferenceNotInitializedSuppressor.cs @@ -65,6 +65,8 @@ public override void ReportSuppressions(SuppressionAnalysisContext context) if (declaredSymbol is IPropertySymbol property && string.Equals(property.Name, "TestContext", StringComparison.Ordinal) && SymbolEqualityComparer.Default.Equals(testContextSymbol, property.GetMethod?.ReturnType) + && property.DeclaredAccessibility == Accessibility.Public + && property.SetMethod is not null && property.ContainingType.GetAttributes().Any(attr => attr.AttributeClass.Inherits(testClassAttributeSymbol))) { context.ReportSuppression(Suppression.Create(Rule, diagnostic)); diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/AssertThrowsShouldContainSingleStatementAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/AssertThrowsShouldContainSingleStatementAnalyzerTests.cs index 26c4c0754b..f13fedaa60 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/AssertThrowsShouldContainSingleStatementAnalyzerTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/AssertThrowsShouldContainSingleStatementAnalyzerTests.cs @@ -602,4 +602,68 @@ public void MyTestMethod() await VerifyCS.VerifyAnalyzerAsync(code); } + + [TestMethod] + public async Task WhenAssertThrowsReceivesMethodGroup_CSharp_NoDiagnostic() + { + // When a method group (not a lambda) is passed to Assert.Throws, the delegate creation + // target is an IMethodReferenceOperation, not IAnonymousFunctionOperation. + // The analyzer's early-return guard ('delegateCreation.Target is not IAnonymousFunctionOperation') + // fires, so no diagnostic is reported regardless of how many statements the method contains. + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void MyTestMethod() + { + // Method group passed as Action — not a lambda, so the analyzer cannot inspect + // the body and must not fire. + Assert.Throws(DoSomethingMultiple); + Assert.ThrowsExactly(DoSomethingMultiple); + } + + private static void DoSomethingMultiple() + { + Console.WriteLine("one"); + throw new Exception("two"); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code); + } + + [TestMethod] + public async Task WhenAssertThrowsReceivesNonLambdaDelegate_CSharp_NoDiagnostic() + { + // An existing delegate local is an ILocalReferenceOperation, not an + // IDelegateCreationOperation, so the analyzer's early-return guard fires. + string code = """ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void MyTestMethod() + { + Action multiStep = () => + { + Console.WriteLine("one"); + throw new Exception("two"); + }; + + // The delegate is referenced by name, not created inline — no diagnostic expected. + Assert.Throws(multiStep); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code); + } } diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertAreSameWithValueTypesAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertAreSameWithValueTypesAnalyzerTests.cs index 8598c73d57..d4d55dd9b3 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertAreSameWithValueTypesAnalyzerTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidAssertAreSameWithValueTypesAnalyzerTests.cs @@ -386,4 +386,92 @@ public void TestMethod() await VerifyCS.VerifyCodeFixAsync(code, fixedCode); } + + [TestMethod] + public async Task WhenBothArgsAreNullLiterals_NoDiagnostic() + { + // WalkDownConversion() removes the object conversion, leaving an untyped null literal. + // The null-propagating IsValueType check therefore does not report a diagnostic. + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() + { + Assert.AreSame((object)null, (object)null); + Assert.AreNotSame((object)null, (object)null); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code); + } + + [TestMethod] + public async Task WhenGenericTypeParameterConstrainedToStruct_Diagnostic() + { + // A generic type parameter constrained to 'struct' has IsValueType == true, + // so the analyzer should report a diagnostic and the fixer should replace the method name. + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() where T : struct + { + T a = default; + T b = default; + [|Assert.AreSame(a, b)|]; + [|Assert.AreNotSame(a, b)|]; + } + } + """; + + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod() where T : struct + { + T a = default; + T b = default; + Assert.AreEqual(a, b); + Assert.AreNotEqual(a, b); + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, fixedCode); + } + + [TestMethod] + public async Task WhenGenericTypeParameterWithNoConstraint_NoDiagnostic() + { + // An unconstrained generic type parameter is not known to be a value type at analysis time, + // so the analyzer should not report a diagnostic even though T can be instantiated with one. + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod(T a, T b) + { + Assert.AreSame(a, b); + Assert.AreNotSame(a, b); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code); + } } diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidOutRefTestMethodParametersAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidOutRefTestMethodParametersAnalyzerTests.cs index 1a6d23bd50..de90c091cd 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidOutRefTestMethodParametersAnalyzerTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/AvoidOutRefTestMethodParametersAnalyzerTests.cs @@ -324,4 +324,93 @@ public void TestMethod1(in string s) await VerifyCS.VerifyCodeFixAsync(code, code); } + +#if NET + [TestMethod] + public async Task WhenTestMethodHasRefReadonlyParameter_Diagnostic() + { + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void [|TestMethod1|](ref readonly int value) + { + } + } + """; + + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod1(int value) + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, fixedCode); + } + + [TestMethod] + public async Task WhenRefReadonlyModifiersContainComment_CodeFixPreservesComment() + { + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void [|TestMethod1|](ref /* rationale */ readonly int value) + { + } + } + """; + + string fixedCode = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod1(/* rationale */ int value) + { + } + } + """; + + await VerifyCS.VerifyCodeFixAsync(code, fixedCode); + } +#endif + + [TestMethod] + public async Task WhenParameterTypeAndNameAreEscapedKeywords_NoDiagnostic() + { + string code = """ + using Microsoft.VisualStudio.TestTools.UnitTesting; + + public sealed class @ref + { + } + + [TestClass] + public class MyTestClass + { + [TestMethod] + public void TestMethod1(@ref @readonly) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(code); + } } diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/NonNullableReferenceNotInitializedSuppressorTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/NonNullableReferenceNotInitializedSuppressorTests.cs index ad3daf8bb8..129920c678 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/NonNullableReferenceNotInitializedSuppressorTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/NonNullableReferenceNotInitializedSuppressorTests.cs @@ -143,6 +143,72 @@ public class SomeClass await test.RunAsync(); } + [TestMethod] + public async Task TestContextFieldOnTestClass_DiagnosticIsNotSuppressed() + { + string code = @" +#nullable enable + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +public class SomeClass +{ + public TestContext {|#0:_testContext|}; +} +"; + + var test = new VerifyCS.Test + { + TestCode = code, + }; + + test.ExpectedDiagnostics.Add(DiagnosticResult.CompilerError("CS8618") + .WithLocation(0) + .WithOptions(DiagnosticOptions.IgnoreAdditionalLocations) + .WithArguments("field", "_testContext") + .WithIsSuppressed(false)); + + await test.RunAsync(); + } + + [TestMethod] + public async Task TestContextGetterOnlyPropertyOnTestClass_DiagnosticIsNotSuppressed() + { + // MSTest cannot assign a getter-only property, so CS8618 must remain visible. + string code = @" +#nullable enable + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +public class SomeClass +{ + public TestContext {|#0:TestContext|} { get; } +} +"; + + await VerifySingleSuppressionAsync(code, isSuppressed: false); + } + + [TestMethod] + public async Task PrivateTestContextPropertyOnTestClass_DiagnosticIsNotSuppressed() + { + string code = @" +#nullable enable + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +[TestClass] +public class SomeClass +{ + private TestContext {|#0:TestContext|} { get; set; } +} +"; + + await VerifySingleSuppressionAsync(code, isSuppressed: false); + } + private Task VerifySingleSuppressionAsync(string source, bool isSuppressed) => VerifyDiagnosticsAsync(source, [(0, isSuppressed)]);