Skip to content
Merged
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 numberDiff line numberDiff line change
Expand Up@@ -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;
Expand DownExpand Up@@ -61,20 +62,42 @@ private static async Task<Document> 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<SyntaxTrivia> 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();
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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");
});
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Comment thread
Evangelink marked this conversation as resolved.
&& property.ContainingType.GetAttributes().Any(attr => attr.AttributeClass.Inherits(testClassAttributeSymbol)))
{
context.ReportSuppression(Suppression.Create(Rule, diagnostic));
Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<Exception>(DoSomethingMultiple);
Assert.ThrowsExactly<Exception>(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<Exception>(multiStep);
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(code);
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -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<T>() 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<T>() 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>(T a, T b)
{
Assert.AreSame(a, b);
Assert.AreNotSame(a, b);
}
}
""";

await VerifyCS.VerifyAnalyzerAsync(code);
}
}
Original file line numberDiff line numberDiff line change
Expand Up@@ -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);
}
}
Loading
Loading