diff --git a/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md b/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md
index b1b99aaf26..238002c327 100644
--- a/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md
+++ b/src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md
@@ -1,3 +1,8 @@
; Unshipped analyzer release
; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md
+### New Rules
+
+Rule ID | Category | Severity | Notes
+--------|----------|----------|-------
+MSTEST0037 | `Usage` | Disabled | UseProperAssertMethodsAnalyzer
diff --git a/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs b/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs
index d8ce01e700..79c269bbe8 100644
--- a/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs
+++ b/src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs
@@ -41,4 +41,5 @@ internal static class DiagnosticIds
public const string UseClassCleanupBehaviorEndOfClassRuleId = "MSTEST0034";
public const string UseDeploymentItemWithTestMethodOrTestClassRuleId = "MSTEST0035";
public const string DoNotUseShadowingRuleId = "MSTEST0036";
+ public const string UseProperAssertMethodsRuleId = "MSTEST0037";
}
diff --git a/src/Analyzers/MSTest.Analyzers/Resources.Designer.cs b/src/Analyzers/MSTest.Analyzers/Resources.Designer.cs
index 7855473260..80b2ef9c1c 100644
--- a/src/Analyzers/MSTest.Analyzers/Resources.Designer.cs
+++ b/src/Analyzers/MSTest.Analyzers/Resources.Designer.cs
@@ -1044,5 +1044,23 @@ internal static string UseParallelizeAttributeAnalyzerTitle {
return ResourceManager.GetString("UseParallelizeAttributeAnalyzerTitle", resourceCulture);
}
}
+
+ ///
+ /// Looks up a localized string similar to Use 'Assert.{0}' instead of 'Assert.{1}'.
+ ///
+ internal static string UseProperAssertMethodsMessageFormat {
+ get {
+ return ResourceManager.GetString("UseProperAssertMethodsMessageFormat", resourceCulture);
+ }
+ }
+
+ ///
+ /// Looks up a localized string similar to Use proper 'Assert' methods.
+ ///
+ internal static string UseProperAssertMethodsTitle {
+ get {
+ return ResourceManager.GetString("UseProperAssertMethodsTitle", resourceCulture);
+ }
+ }
}
}
diff --git a/src/Analyzers/MSTest.Analyzers/Resources.resx b/src/Analyzers/MSTest.Analyzers/Resources.resx
index ff98060e2e..7f85479cee 100644
--- a/src/Analyzers/MSTest.Analyzers/Resources.resx
+++ b/src/Analyzers/MSTest.Analyzers/Resources.resx
@@ -529,4 +529,10 @@ The type declaring these methods should also respect the following rules:
Do not use shadowing
+
+ Use proper 'Assert' methods
+
+
+ Use 'Assert.{0}' instead of 'Assert.{1}'
+
\ No newline at end of file
diff --git a/src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/IOperationExtensions.cs b/src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/IOperationExtensions.cs
index d089ad8c19..1584ad6d0f 100644
--- a/src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/IOperationExtensions.cs
+++ b/src/Analyzers/MSTest.Analyzers/RoslynAnalyzerHelpers/IOperationExtensions.cs
@@ -25,4 +25,19 @@ internal static class IOperationExtensions
_ => null,
};
+
+ ///
+ /// Walks down consecutive conversion operations until an operand is reached that isn't a conversion operation.
+ ///
+ /// The starting operation.
+ /// The inner non conversion operation or the starting operation if it wasn't a conversion operation.
+ public static IOperation WalkDownConversion(this IOperation operation)
+ {
+ while (operation is IConversionOperation conversionOperation)
+ {
+ operation = conversionOperation.Operand;
+ }
+
+ return operation;
+ }
}
diff --git a/src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.cs
new file mode 100644
index 0000000000..c32a8e776d
--- /dev/null
+++ b/src/Analyzers/MSTest.Analyzers/UseProperAssertMethodsAnalyzer.cs
@@ -0,0 +1,238 @@
+// 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.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
+
+using Analyzer.Utilities.Extensions;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+using MSTest.Analyzers.Helpers;
+using MSTest.Analyzers.RoslynAnalyzerHelpers;
+
+namespace MSTest.Analyzers;
+
+///
+/// MSTEST0037: Use proper 'Assert' methods.
+///
+///
+/// The analyzer captures the following cases:
+///
+/// -
+///
Assert.[IsTrue|IsFalse](x [==|!=|is|is not] null)
+///
+/// -
+///
Assert.[IsTrue|IsFalse](x [==|!=] y)
+///
+/// -
+///
Assert.AreEqual([true|false], x)
+///
+/// -
+///
Assert.[AreEqual|AreNotEqual](null, x)
+///
+///
+///
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+internal sealed class UseProperAssertMethodsAnalyzer : DiagnosticAnalyzer
+{
+ private enum NullCheckStatus
+ {
+ Unknown,
+ IsNull,
+ IsNotNull,
+ }
+
+ private enum EqualityCheckStatus
+ {
+ Unknown,
+ Equals,
+ NotEquals,
+ }
+
+ private static readonly LocalizableResourceString Title = new(nameof(Resources.UseProperAssertMethodsTitle), Resources.ResourceManager, typeof(Resources));
+ private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.UseProperAssertMethodsMessageFormat), Resources.ResourceManager, typeof(Resources));
+
+ internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
+ DiagnosticIds.UseProperAssertMethodsRuleId,
+ Title,
+ MessageFormat,
+ null,
+ Category.Usage,
+ DiagnosticSeverity.Info,
+ isEnabledByDefault: false);
+
+ public override ImmutableArray SupportedDiagnostics { get; }
+ = ImmutableArray.Create(Rule);
+
+ public override void Initialize(AnalysisContext context)
+ {
+ context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+ context.EnableConcurrentExecution();
+
+ context.RegisterCompilationStartAction(context =>
+ {
+ if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingAssert, out INamedTypeSymbol? assertTypeSymbol))
+ {
+ return;
+ }
+
+ context.RegisterOperationAction(context => AnalyzeInvocationOperation(context, assertTypeSymbol), OperationKind.Invocation);
+ });
+ }
+
+ private static void AnalyzeInvocationOperation(OperationAnalysisContext context, INamedTypeSymbol assertTypeSymbol)
+ {
+ var operation = (IInvocationOperation)context.Operation;
+ IMethodSymbol targetMethod = operation.TargetMethod;
+ if (!SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, assertTypeSymbol))
+ {
+ return;
+ }
+
+ if (!TryGetFirstArgumentValue(operation, out IOperation? firstArgument))
+ {
+ return;
+ }
+
+ switch (targetMethod.Name)
+ {
+ case "IsTrue":
+ AnalyzeIsTrueOrIsFalseInvocation(context, firstArgument, isTrueInvocation: true);
+ break;
+
+ case "IsFalse":
+ AnalyzeIsTrueOrIsFalseInvocation(context, firstArgument, isTrueInvocation: false);
+ break;
+
+ case "AreEqual":
+ AnalyzeAreEqualOrAreNotEqualInvocation(context, firstArgument, isAreEqualInvocation: true);
+ break;
+
+ case "AreNotEqual":
+ AnalyzeAreEqualOrAreNotEqualInvocation(context, firstArgument, isAreEqualInvocation: false);
+ break;
+ }
+ }
+
+ private static bool IsIsNullPattern(IOperation operation)
+ => operation is IIsPatternOperation { Pattern: IConstantPatternOperation { Value: { } constantPatternValue } } &&
+ constantPatternValue.WalkDownConversion() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
+
+ private static bool IsIsNotNullPattern(IOperation operation)
+ => operation is IIsPatternOperation { Pattern: INegatedPatternOperation { Pattern: IConstantPatternOperation { Value: { } constantPatternValue } } } &&
+ constantPatternValue.WalkDownConversion() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
+
+ // TODO: Recognize 'null == something' (i.e, when null is the left operand)
+ private static bool IsEqualsNullBinaryOperator(IOperation operation)
+ => operation is IBinaryOperation { OperatorKind: BinaryOperatorKind.Equals, RightOperand: { } rightOperand } &&
+ rightOperand.WalkDownConversion() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
+
+ // TODO: Recognize 'null != something' (i.e, when null is the left operand)
+ private static bool IsNotEqualsNullBinaryOperator(IOperation operation)
+ => operation is IBinaryOperation { OperatorKind: BinaryOperatorKind.NotEquals, RightOperand: { } rightOperand } &&
+ rightOperand.WalkDownConversion() is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } };
+
+ private static NullCheckStatus RecognizeNullCheck(IOperation operation)
+ {
+ if (IsIsNullPattern(operation) || IsEqualsNullBinaryOperator(operation))
+ {
+ return NullCheckStatus.IsNull;
+ }
+ else if (IsIsNotNullPattern(operation) || IsNotEqualsNullBinaryOperator(operation))
+ {
+ return NullCheckStatus.IsNotNull;
+ }
+
+ return NullCheckStatus.Unknown;
+ }
+
+ private static EqualityCheckStatus RecognizeEqualityCheck(IOperation operation)
+ {
+ if (operation is IIsPatternOperation { Pattern: IConstantPatternOperation } or
+ IBinaryOperation { OperatorKind: BinaryOperatorKind.Equals })
+ {
+ return EqualityCheckStatus.Equals;
+ }
+ else if (operation is IIsPatternOperation { Pattern: INegatedPatternOperation { Pattern: IConstantPatternOperation } } or
+ IBinaryOperation { OperatorKind: BinaryOperatorKind.NotEquals })
+ {
+ return EqualityCheckStatus.NotEquals;
+ }
+
+ return EqualityCheckStatus.Unknown;
+ }
+
+ private static void AnalyzeIsTrueOrIsFalseInvocation(OperationAnalysisContext context, IOperation conditionArgument, bool isTrueInvocation)
+ {
+ NullCheckStatus nullCheckStatus = RecognizeNullCheck(conditionArgument);
+ if (nullCheckStatus != NullCheckStatus.Unknown)
+ {
+ Debug.Assert(nullCheckStatus is NullCheckStatus.IsNull or NullCheckStatus.IsNotNull, "Unexpected NullCheckStatus value.");
+ bool shouldUseIsNull = isTrueInvocation
+ ? nullCheckStatus == NullCheckStatus.IsNull
+ : nullCheckStatus == NullCheckStatus.IsNotNull;
+
+ // The message is: Use 'Assert.{0}' instead of 'Assert.{1}'.
+ context.ReportDiagnostic(context.Operation.CreateDiagnostic(
+ Rule,
+ shouldUseIsNull ? "IsNull" : "IsNotNull",
+ isTrueInvocation ? "IsTrue" : "IsFalse"));
+ return;
+ }
+
+ EqualityCheckStatus equalityCheckStatus = RecognizeEqualityCheck(conditionArgument);
+ if (equalityCheckStatus != EqualityCheckStatus.Unknown)
+ {
+ Debug.Assert(equalityCheckStatus is EqualityCheckStatus.Equals or EqualityCheckStatus.NotEquals, "Unexpected EqualityCheckStatus value.");
+ bool shouldUseAreEqual = isTrueInvocation
+ ? equalityCheckStatus == EqualityCheckStatus.Equals
+ : equalityCheckStatus == EqualityCheckStatus.NotEquals;
+
+ // The message is: Use 'Assert.{0}' instead of 'Assert.{1}'.
+ context.ReportDiagnostic(context.Operation.CreateDiagnostic(
+ Rule,
+ shouldUseAreEqual ? "AreEqual" : "AreNotEqual",
+ isTrueInvocation ? "IsTrue" : "IsFalse"));
+ return;
+ }
+ }
+
+ private static void AnalyzeAreEqualOrAreNotEqualInvocation(OperationAnalysisContext context, IOperation expectedArgument, bool isAreEqualInvocation)
+ {
+ // Don't flag a warning for Assert.AreNotEqual([true|false], x).
+ // This is not the same as Assert.IsFalse(x).
+ if (isAreEqualInvocation && expectedArgument is ILiteralOperation { ConstantValue: { HasValue: true, Value: bool expectedLiteralBoolean } })
+ {
+ bool shouldUseIsTrue = expectedLiteralBoolean;
+
+ // The message is: Use 'Assert.{0}' instead of 'Assert.{1}'.
+ context.ReportDiagnostic(context.Operation.CreateDiagnostic(
+ Rule,
+ shouldUseIsTrue ? "IsTrue" : "IsFalse",
+ isAreEqualInvocation ? "AreEqual" : "AreNotEqual"));
+ }
+ else if (expectedArgument is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } })
+ {
+ bool shouldUseIsNull = isAreEqualInvocation;
+
+ // The message is: Use 'Assert.{0}' instead of 'Assert.{1}'.
+ context.ReportDiagnostic(context.Operation.CreateDiagnostic(
+ Rule,
+ shouldUseIsNull ? "IsNull" : "IsNotNull",
+ isAreEqualInvocation ? "AreEqual" : "AreNotEqual"));
+ }
+ }
+
+ private static bool TryGetFirstArgumentValue(IInvocationOperation operation, [NotNullWhen(true)] out IOperation? argumentValue)
+ => TryGetArgumentValueForParameterOrdinal(operation, 0, out argumentValue);
+
+ private static bool TryGetArgumentValueForParameterOrdinal(IInvocationOperation operation, int ordinal, [NotNullWhen(true)] out IOperation? argumentValue)
+ {
+ argumentValue = operation.Arguments.FirstOrDefault(arg => arg.Parameter?.Ordinal == ordinal)?.Value?.WalkDownConversion();
+ return argumentValue is not null;
+ }
+}
diff --git a/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf b/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
index 4c4c02ca44..9e957be5c3 100644
--- a/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
+++ b/src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf
@@ -726,6 +726,16 @@ Typ deklarující tyto metody by měl také respektovat následující pravidla:
Explicitně povolit nebo zakázat paralelizaci testů
+
+ Use 'Assert.{0}' instead of 'Assert.{1}'
+ Use 'Assert.{0}' instead of 'Assert.{1}'
+
+
+
+ Use proper 'Assert' methods
+ Use proper 'Assert' methods
+
+