Avoid conditionals inside assertions analyzer

src/Analyzers/MSTest.Analyzers/AnalyzerReleases.Unshipped.md

@@ -1,2 +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
+--------|----------|----------|-------
+MSTEST0026 | Usage | Info | AssertionArgsShouldAvoidConditionalAccessRuleId, [Documentation](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0026)

src/Analyzers/MSTest.Analyzers/AssertionArgsShouldAvoidConditionalAccessAnalyzer.cs

@@ -0,0 +1,146 @@
+// 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 Analyzer.Utilities.Extensions;
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+using MSTest.Analyzers.Helpers;
+
+namespace MSTest.Analyzers;
+
+[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+public sealed class AssertionArgsShouldAvoidConditionalAccessAnalyzer : DiagnosticAnalyzer
+{
+    private static readonly ImmutableArray<string> AssertSupportedMethodNames = ImmutableArray.Create(new[]
+    {
+        "IsTrue",
+        "IsFalse",
+        "AreEqual",
+        "AreNotEqual",
+        "AreSame",
+        "AreNotSame",
+    });
+
+    private static readonly ImmutableArray<string> CollectionAssertSupportedMethodNames = ImmutableArray.Create(new[]
+    {
+        "IsTrue",
+        "IsFalse",
+        "AreEqual",
+        "AreNotEqual",
+        "AreEquivalent",
+        "AreNotEquivalent",
+        "Contains",
+        "DoesNotContain",
+        "AllItemsAreNotNull",
+        "AllItemsAreUnique",
+        "IsSubsetOf",
+        "IsNotSubsetOf",
+        "AllItemsAreInstancesOfType",
+    });
+
+    private static readonly ImmutableArray<string> StringAssertSupportedMethodNames = ImmutableArray.Create(new[]
+    {
+        "Contains",
+        "StartsWith",
+        "EndsWith",
+        "Matches",
+        "DoesNotMatch",
+    });
+
+    private static readonly LocalizableResourceString Title = new(nameof(Resources.AssertionArgsShouldAvoidConditionalAccessTitle), Resources.ResourceManager, typeof(Resources));
+    private static readonly LocalizableResourceString MessageFormat = new(nameof(Resources.AssertionArgsShouldAvoidConditionalAccessMessageFormat), Resources.ResourceManager, typeof(Resources));
+
+    internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
+        DiagnosticIds.AssertionArgsShouldAvoidConditionalAccessRuleId,
+        Title,
+        MessageFormat,
+        description: null,
+        Category.Usage,
+        DiagnosticSeverity.Info,
+        isEnabledByDefault: true);
+
+    public override ImmutableArray<DiagnosticDescriptor> 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? assertSymbol))
+            {
+                context.RegisterOperationAction(context => AnalyzeOperation(context, assertSymbol, AssertSupportedMethodNames), OperationKind.Invocation);
+            }
+
+            if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingCollectionAssert, out INamedTypeSymbol? collectionAssertSymbol))
+            {
+                context.RegisterOperationAction(context => AnalyzeOperation(context, collectionAssertSymbol, CollectionAssertSupportedMethodNames), OperationKind.Invocation);
+            }
+
+            if (context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.MicrosoftVisualStudioTestToolsUnitTestingStringAssert, out INamedTypeSymbol? stringAssertSymbol))
+            {
+                context.RegisterOperationAction(context => AnalyzeOperation(context, stringAssertSymbol, StringAssertSupportedMethodNames), OperationKind.Invocation);
+            }
+        });
+    }
+
+    private static void AnalyzeOperation(OperationAnalysisContext context, INamedTypeSymbol assertSymbol, ImmutableArray<string> supportedMethodNames)
+    {
+        var invocationOperation = (IInvocationOperation)context.Operation;
+
+        // This is not an invocation of the expected assertion methods.
+        if (!supportedMethodNames.Contains(invocationOperation.TargetMethod.Name)
+            || !SymbolEqualityComparer.Default.Equals(assertSymbol, invocationOperation.TargetMethod.ContainingType)
+            || !HasAnyConditionalAccessOperationChild(invocationOperation))
+        {
+            return;
+        }
+
+        context.ReportDiagnostic(invocationOperation.CreateDiagnostic(Rule));
+    }
+
+    private static bool HasAnyConditionalAccessOperationChild(IInvocationOperation invocationOperation)
+    {
+        foreach (IArgumentOperation argument in invocationOperation.Arguments)
+        {
+            // Check for conditional access
+            //      a?.b
+            //      a?.b?.c
+            //      a.b?.c
+            if (argument.Value is IConditionalAccessOperation conditionalAccessOperation)
+            {
+                if (conditionalAccessOperation.Kind == OperationKind.ConditionalAccess)
+                {
+                    return true;
+                }
+            }
+
+            // Check for binary operations with conditional access => s?.Length > 1.
+            if (argument.Value is IBinaryOperation binaryOperation)
+            {
+                if (binaryOperation.LeftOperand.Kind == OperationKind.ConditionalAccess || binaryOperation.RightOperand.Kind == OperationKind.ConditionalAccess)
+                {
+                    return true;
+                }
+            }
+
+            // Check for conversion operations with conditional access => (s?.Length).
+            if (argument.Value is IConversionOperation conversionOperation)
+            {
+                if (conversionOperation.Operand.Kind == OperationKind.ConditionalAccess)
+                {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
+}

src/Analyzers/MSTest.Analyzers/Helpers/DiagnosticIds.cs

@@ -29,4 +29,5 @@ internal static class DiagnosticIds
     public const string DoNotNegateBooleanAssertionRuleId = "MSTEST0023";
     public const string DoNotStoreStaticTestContextAnalyzerRuleId = "MSTEST0024";
     public const string PreferAssertFailOverAlwaysFalseConditionsRuleId = "MSTEST0025";
+    public const string AssertionArgsShouldAvoidConditionalAccessRuleId = "MSTEST0026";
 }

src/Analyzers/MSTest.Analyzers/Helpers/WellKnownTypeNames.cs

@@ -11,6 +11,7 @@ internal static class WellKnownTypeNames
     public const string MicrosoftVisualStudioTestToolsUnitTestingAssert = "Microsoft.VisualStudio.TestTools.UnitTesting.Assert";
     public const string MicrosoftVisualStudioTestToolsUnitTestingClassCleanupAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.ClassCleanupAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingClassInitializeAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.ClassInitializeAttribute";
+    public const string MicrosoftVisualStudioTestToolsUnitTestingCollectionAssert = "Microsoft.VisualStudio.TestTools.UnitTesting.CollectionAssert";
     public const string MicrosoftVisualStudioTestToolsUnitTestingCssIterationAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.CssIterationAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingCssProjectStructureAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.CssProjectStructureAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingDataRowAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.DataRowAttribute";
@@ -23,6 +24,7 @@ internal static class WellKnownTypeNames
     public const string MicrosoftVisualStudioTestToolsUnitTestingOwnerAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.OwnerAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingParallelizeAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.ParallelizeAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingPriorityAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.PriorityAttribute";
+    public const string MicrosoftVisualStudioTestToolsUnitTestingStringAssert = "Microsoft.VisualStudio.TestTools.UnitTesting.StringAssert";
     public const string MicrosoftVisualStudioTestToolsUnitTestingTestClassAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.TestClassAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingTestCleanupAttribute = "Microsoft.VisualStudio.TestTools.UnitTesting.TestCleanupAttribute";
     public const string MicrosoftVisualStudioTestToolsUnitTestingTestContext = "Microsoft.VisualStudio.TestTools.UnitTesting.TestContext";

src/Analyzers/MSTest.Analyzers/Resources.resx

@@ -191,6 +191,12 @@
   <data name="AssemblyInitializeShouldBeValidTitle" xml:space="preserve">
     <value>AssemblyInitialize methods should have valid layout</value>
   </data>
+  <data name="AssertionArgsShouldAvoidConditionalAccessMessageFormat" xml:space="preserve">
+    <value>Prefer adding an additional assertion that checks for null</value>
+  </data>
+  <data name="AssertionArgsShouldAvoidConditionalAccessTitle" xml:space="preserve">
+    <value>Avoid conditional access in assertions</value>
+  </data>
   <data name="AssertionArgsShouldBePassedInCorrectOrderDescription" xml:space="preserve">
     <value>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</value>
   </data>

src/Analyzers/MSTest.Analyzers/xlf/Resources.cs.xlf

@@ -132,6 +132,16 @@
         <target state="translated">Metody AssemblyInitialize musí mít platné rozložení</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">Assert.AreEqual, Assert.AreNotEqual, Assert.AreSame a Assert.AreNotSame očekává, že se jako první argument předá očekávaná hodnota a jako druhý argument skutečná hodnota.</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.de.xlf

@@ -130,6 +130,16 @@
         <target state="translated">AssemblyInitialize-Methoden müssen über ein gültiges Layout verfügen.</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">“Assert.AreEqual“, „Assert.AreNotEqual“, „Assert.AreSame“ und „Assert.AreNotSame“ erwarten, dass der erwartete Wert zuerst übergeben wird und der tatsächliche Wert als zweites Argument übergeben wird.</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.es.xlf

@@ -130,6 +130,16 @@
         <target state="translated">Los métodos AssemblyInitialize deben tener un diseño válido</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">"Assert.AreEqual", "Assert.AreNotEqual", "Assert.AreSame" y "Assert.AreNotSame" esperan que el valor esperado se pase primero y que el valor real se pase como segundo argumento.</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.fr.xlf

@@ -130,6 +130,16 @@
         <target state="translated">La méthode AssemblyInitialize doit avoir une disposition valide</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">« Assert.AreEqual », « Assert.AreNotEqual », « Assert.AreSame » et « Assert.AreNotSame » supposent que la valeur attendue soit passée en premier et que la valeur réelle soit passée en tant que deuxième argument.</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.it.xlf

@@ -130,6 +130,16 @@
         <target state="translated">Il metodo AssemblyInitialize deve avere un layout valido</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">"Assert.AreEqual", "Assert.AreNotEqual", "Assert.AreSame" e "Assert.AreNotSame" prevedono che il valore previsto venga passato per primo e il valore effettivo come secondo argomento.</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.ja.xlf

@@ -130,6 +130,16 @@
         <target state="translated">AssemblyInitialize メソッドには有効なレイアウトが必要です</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">'Assert.AreEqual'、'Assert.AreNotEqual'、'Assert.AreSame'、および 'Assert.AreNotSame' では、必要な値が最初に渡され、実際の値が 2 番目の引数として渡される必要があります。</target>

src/Analyzers/MSTest.Analyzers/xlf/Resources.ko.xlf

@@ -130,6 +130,16 @@
         <target state="translated">AssemblyInitialize 메서드에는 유효한 레이아웃이 있어야 합니다.</target>
         <note />
       </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessMessageFormat">
+        <source>Prefer adding an additional assertion that checks for null</source>
+        <target state="new">Prefer adding an additional assertion that checks for null</target>
+        <note />
+      </trans-unit>
+      <trans-unit id="AssertionArgsShouldAvoidConditionalAccessTitle">
+        <source>Avoid conditional access in assertions</source>
+        <target state="new">Avoid conditional access in assertions</target>
+        <note />
+      </trans-unit>
       <trans-unit id="AssertionArgsShouldBePassedInCorrectOrderDescription">
         <source>'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' and 'Assert.AreNotSame' expects the expected value to be passed first and the actual value to be passed as second argument.</source>
         <target state="translated">'Assert.AreEqual', 'Assert.AreNotEqual', 'Assert.AreSame' 및 'Assert.AreNotSame'에는 필요한 값이 먼저 전달되고 실제 값이 두 번째 인수로 전달되어야 합니다.</target>