Merge pull request #453 from eatdrinksleepcode/support-string-assert

Support classic StringAssert in ConstActualValueUsage analyzer and code fix
nunit · May 12, 2022 · 195337e · 195337e
2 parents 0e75c25 + 1968727
commit 195337e
Show file tree

Hide file tree

Showing 10 changed files with 298 additions and 13 deletions.
diff --git a/src/nunit.analyzers.tests/ConstActualValueUsage/ConstActualValueUsageAnalyzerTests.cs b/src/nunit.analyzers.tests/ConstActualValueUsage/ConstActualValueUsageAnalyzerTests.cs
@@ -311,5 +311,162 @@ public void Test()
 
             RoslynAssert.Valid(analyzer, testCode);
         }
+
+        [Test]
+        public void AnalyzeWhenLiteralArgumentIsProvidedForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    string expected = ""exp"";
+                    StringAssert.Contains(expected, ↓""act"");
+                }");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void AnalyzeWhenLiteralNamedArgumentIsProvidedForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    string expected = ""exp"";
+                    StringAssert.Contains(actual: ↓""act"", expected: expected);
+                }");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void AnalyzeWhenLocalConstArgumentIsProvidedForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    const string actual = ""act"";
+                    string expected = ""exp"";
+                    StringAssert.Contains(expected, ↓actual);
+                }");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void AnalyzeWhenLocalConstNamedArgumentIsProvidedForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    const string actual = ""act"";
+                    string expected = ""exp"";
+                    StringAssert.Contains(actual: ↓actual, expected: expected);
+                }");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void AnalyzeWhenConstFieldArgumentIsProvidedForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+            public class TestFixture
+            {
+                private const string actual = ""act"";
+
+                public void Test()
+                {
+                    string expected = ""exp"";
+                    StringAssert.Contains(expected, ↓actual);
+                }
+            }");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void AnalyzeWhenStringEmptyArgumentIsProvidedForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    string actual = ""act"";
+                    StringAssert.Contains(actual, ↓string.Empty);
+                }");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [Test]
+        public void ValidWhenNonConstValueIsProvidedAsActualArgumentForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+            public class TestFixture
+            {
+                private const string expected = ""exp"";
+
+                public void Test()
+                {
+                    string actual = ""act"";
+                    StringAssert.Contains(expected, actual);
+                }
+            }");
+
+            RoslynAssert.Valid(analyzer, testCode);
+        }
+
+        [Test]
+        public void ValidWhenConstValueIsProvidedAsActualAndExpectedArgumentForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+            public class TestFixture
+            {
+                private const string expected = ""exp"";
+
+                public void Test()
+                {
+                    const string actual = ""act"";
+                    StringAssert.Contains(expected, actual);
+                }
+            }");
+
+            RoslynAssert.Valid(analyzer, testCode);
+        }
+
+        [Test]
+        public void ValidWhenNonConstValueIsProvidedAsActualNamedArgumentForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+            public class TestFixture
+            {
+                private const string expected = ""exp"";
+
+                public void Test()
+                {
+                    string actual = ""act"";
+                    StringAssert.Contains(actual: actual, expected: expected);
+                }
+            }");
+
+            RoslynAssert.Valid(analyzer, testCode);
+        }
+
+        [Test]
+        public void ValidWhenConstValueIsProvidedAsActualAndExpectedNamedArgumentForStringAssertContains()
+        {
+            var testCode = TestUtility.WrapClassInNamespaceAndAddUsing(@"
+            public class TestFixture
+            {
+                private const string expected = ""exp"";
+
+                public void Test()
+                {
+                    const string actual = ""act"";
+                    StringAssert.Contains(actual: actual, expected: expected);
+                }
+            }");
+
+            RoslynAssert.Valid(analyzer, testCode);
+        }
     }
 }
diff --git a/src/nunit.analyzers.tests/ConstActualValueUsage/ConstActualValueUsageCodeFixTests.cs b/src/nunit.analyzers.tests/ConstActualValueUsage/ConstActualValueUsageCodeFixTests.cs
@@ -112,5 +112,56 @@ public void Test()
 
             RoslynAssert.NoFix(analyzer, fix, expectedDiagnostic, code);
         }
+
+        [TestCase(nameof(StringAssert.AreEqualIgnoringCase))]
+        [TestCase(nameof(StringAssert.AreNotEqualIgnoringCase))]
+        [TestCase(nameof(StringAssert.Contains))]
+        [TestCase(nameof(StringAssert.EndsWith))]
+        [TestCase(nameof(StringAssert.IsMatch))]
+        [TestCase(nameof(StringAssert.StartsWith))]
+        [TestCase(nameof(StringAssert.DoesNotContain))]
+        [TestCase(nameof(StringAssert.DoesNotMatch))]
+        [TestCase(nameof(StringAssert.DoesNotEndWith))]
+        [TestCase(nameof(StringAssert.DoesNotStartWith))]
+        public void LiteralArgumentIsProvidedForClassicStringAssertCodeFix(string classicAssertMethod)
+        {
+            var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+                public void Test()
+                {{
+                    string actual = ""act"";
+                    StringAssert.{classicAssertMethod}(actual, ↓""exp"");
+                }}");
+
+            var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+                public void Test()
+                {{
+                    string actual = ""act"";
+                    StringAssert.{classicAssertMethod}(""exp"", actual);
+                }}");
+
+            RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
+                fixTitle: ConstActualValueUsageCodeFix.SwapArgumentsDescription);
+        }
+
+        [Test]
+        public void LiteralNamedArgumentIsProvidedForStringAssertContainsCodeFix()
+        {
+            var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    string actual = ""act"";
+                    StringAssert.Contains(actual: ↓""exp"", expected: actual);
+                }");
+
+            var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings(@"
+                public void Test()
+                {
+                    string actual = ""act"";
+                    StringAssert.Contains(actual: actual, expected: ""exp"");
+                }");
+
+            RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
+                fixTitle: ConstActualValueUsageCodeFix.SwapArgumentsDescription);
+        }
     }
 }
diff --git a/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs b/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs
@@ -93,6 +93,18 @@ public sealed class NUnitFrameworkConstantsTests
             (nameof(NUnitFrameworkConstants.NameOfAssertThrows), nameof(Assert.Throws)),
             (nameof(NUnitFrameworkConstants.NameOfAssertThrowsAsync), nameof(Assert.ThrowsAsync)),
 
+            (nameof(NUnitFrameworkConstants.NameOfStringAssert), nameof(StringAssert)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertAreEqualIgnoringCase), nameof(StringAssert.AreEqualIgnoringCase)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertAreNotEqualIgnoringCase), nameof(StringAssert.AreNotEqualIgnoringCase)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertContains), nameof(StringAssert.Contains)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotContain), nameof(StringAssert.DoesNotContain)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotEndWith), nameof(StringAssert.DoesNotEndWith)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotMatch), nameof(StringAssert.DoesNotMatch)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertDoesNotStartWith), nameof(StringAssert.DoesNotStartWith)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertEndsWith), nameof(StringAssert.EndsWith)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertIsMatch), nameof(StringAssert.IsMatch)),
+            (nameof(NUnitFrameworkConstants.NameOfStringAssertStartsWith), nameof(StringAssert.StartsWith)),
+
             (nameof(NUnitFrameworkConstants.NameOfConstraint), nameof(Constraint)),
 
             (nameof(NUnitFrameworkConstants.NameOfTestCaseAttribute), nameof(TestCaseAttribute)),

diff --git a/src/nunit.analyzers.tests/Extensions/ITypeSymbolExtensionsTests.cs b/src/nunit.analyzers.tests/Extensions/ITypeSymbolExtensionsTests.cs
@@ -200,6 +200,24 @@ public sealed class IsAssertWhenSymbolIsAssertType
             Assert.That(typeSymbol.IsAssert(), Is.True);
         }
 
+        [TestCase("Assert")]
+        [TestCase("StringAssert")]
+        public async Task IsAnyAssertWhenSymbolIsAnyAssertType(string assertType)
+        {
+            var testCode = $@"
+using NUnit.Framework;
+
+namespace NUnit.Analyzers.Tests.Targets.Extensions
+{{
+    public sealed class IsAnyAssertWhenSymbolIsAnyAssertType
+    {{
+        public {assertType} x;
+    }}
+}}";
+            var typeSymbol = await GetTypeSymbolFromFieldAsync(testCode, "IsAnyAssertWhenSymbolIsAnyAssertType").ConfigureAwait(false);
+            Assert.That(typeSymbol.IsAnyAssert(), Is.True);
+        }
+
         private static async Task<ImmutableArray<ITypeSymbol>> GetTypeSymbolAsync(string code, string[] typeNames)
         {
             var rootAndModel = await TestHelpers.GetRootAndModel(code).ConfigureAwait(false);

diff --git a/src/nunit.analyzers/BaseAssertionAnalyzer.cs b/src/nunit.analyzers/BaseAssertionAnalyzer.cs
@@ -16,14 +16,7 @@ public override void Initialize(AnalysisContext context)
 
         protected abstract void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation);
 
-        protected static bool IsAssert(IOperation operation)
-        {
-            return operation is IExpressionStatementOperation expressionOperation &&
-                expressionOperation.Operation is IInvocationOperation invocationOperation &&
-                IsAssert(invocationOperation);
-        }
-
-        protected static bool IsAssert(IInvocationOperation invocationOperation)
+        protected virtual bool IsAssert(IInvocationOperation invocationOperation)
         {
             return invocationOperation.TargetMethod.ContainingType.IsAssert();
         }
@@ -33,7 +26,7 @@ private void AnalyzeInvocation(OperationAnalysisContext context)
             if (context.Operation is not IInvocationOperation invocationOperation)
                 return;
 
-            if (!IsAssert(invocationOperation))
+            if (!this.IsAssert(invocationOperation))
                 return;
 
             context.CancellationToken.ThrowIfCancellationRequested();

diff --git a/src/nunit.analyzers/ConstActualValueUsage/ConstActualValueUsageAnalyzer.cs b/src/nunit.analyzers/ConstActualValueUsage/ConstActualValueUsageAnalyzer.cs
@@ -22,6 +22,11 @@ public class ConstActualValueUsageAnalyzer : BaseAssertionAnalyzer
 
         public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(descriptor);
 
+        protected override bool IsAssert(IInvocationOperation invocationOperation)
+        {
+            return invocationOperation.TargetMethod.ContainingType.IsAnyAssert();
+        }
+
         protected override void AnalyzeAssertInvocation(OperationAnalysisContext context, IInvocationOperation assertOperation)
         {
             static bool IsStringEmpty(IOperation operation)

diff --git a/src/nunit.analyzers/ConstActualValueUsage/ConstActualValueUsageCodeFix.cs b/src/nunit.analyzers/ConstActualValueUsage/ConstActualValueUsageCodeFix.cs
@@ -23,7 +23,21 @@ public class ConstActualValueUsageCodeFix : CodeFixProvider
             NUnitFrameworkConstants.NameOfAssertAreEqual,
             NUnitFrameworkConstants.NameOfAssertAreNotEqual,
             NUnitFrameworkConstants.NameOfAssertAreSame,
-            NUnitFrameworkConstants.NameOfAssertAreNotSame
+            NUnitFrameworkConstants.NameOfAssertAreNotSame,
+        };
+
+        private static readonly string[] SupportedStringAsserts = new[]
+        {
+            NUnitFrameworkConstants.NameOfStringAssertAreEqualIgnoringCase,
+            NUnitFrameworkConstants.NameOfStringAssertAreNotEqualIgnoringCase,
+            NUnitFrameworkConstants.NameOfStringAssertContains,
+            NUnitFrameworkConstants.NameOfStringAssertDoesNotContain,
+            NUnitFrameworkConstants.NameOfStringAssertDoesNotEndWith,
+            NUnitFrameworkConstants.NameOfStringAssertDoesNotMatch,
+            NUnitFrameworkConstants.NameOfStringAssertDoesNotStartWith,
+            NUnitFrameworkConstants.NameOfStringAssertEndsWith,
+            NUnitFrameworkConstants.NameOfStringAssertIsMatch,
+            NUnitFrameworkConstants.NameOfStringAssertStartsWith,
         };
 
         private static readonly string[] SupportedIsConstraints = new[]
@@ -88,11 +102,12 @@ private static bool TryFindArguments(SemanticModel semanticModel, InvocationExpr
 
             var methodSymbol = semanticModel.GetSymbolInfo(invocationSyntax).Symbol as IMethodSymbol;
 
-            if (methodSymbol is null || !methodSymbol.ContainingType.IsAssert())
+            if (methodSymbol is null || !methodSymbol.ContainingType.IsAnyAssert())
                 return false;
 
             // option 1: Classic assert (e.g. Assert.AreEqual(expected, actual) )
-            if (SupportedClassicAsserts.Contains(methodSymbol.Name) && methodSymbol.Parameters.Length >= 2)
+            if ((IsSupportedAssert(methodSymbol) || IsSupportedStringAssert(methodSymbol))
+                && methodSymbol.Parameters.Length >= 2)
             {
                 expectedArgument = invocationSyntax.ArgumentList.Arguments[0].Expression;
                 actualArgument = invocationSyntax.ArgumentList.Arguments[1].Expression;
@@ -135,5 +150,15 @@ private static bool TryFindArguments(SemanticModel semanticModel, InvocationExpr
 
             return false;
         }
+
+        private static bool IsSupportedAssert(IMethodSymbol methodSymbol)
+        {
+            return methodSymbol.ContainingType.Name == NUnitFrameworkConstants.NameOfAssert && SupportedClassicAsserts.Contains(methodSymbol.Name);
+        }
+
+        private static bool IsSupportedStringAssert(IMethodSymbol methodSymbol)
+        {
+            return methodSymbol.ContainingType.Name == NUnitFrameworkConstants.NameOfStringAssert && SupportedStringAsserts.Contains(methodSymbol.Name);
+        }
     }
 }
diff --git a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs
@@ -82,6 +82,18 @@ public static class NUnitFrameworkConstants
         public const string NameOfAssertThrows = "Throws";
         public const string NameOfAssertThrowsAsync = "ThrowsAsync";
 
+        public const string NameOfStringAssert = "StringAssert";
+        public const string NameOfStringAssertAreEqualIgnoringCase = "AreEqualIgnoringCase";
+        public const string NameOfStringAssertAreNotEqualIgnoringCase = "AreNotEqualIgnoringCase";
+        public const string NameOfStringAssertContains = "Contains";
+        public const string NameOfStringAssertDoesNotContain = "DoesNotContain";
+        public const string NameOfStringAssertDoesNotEndWith = "DoesNotEndWith";
+        public const string NameOfStringAssertDoesNotMatch = "DoesNotMatch";
+        public const string NameOfStringAssertDoesNotStartWith = "DoesNotStartWith";
+        public const string NameOfStringAssertEndsWith = "EndsWith";
+        public const string NameOfStringAssertIsMatch = "IsMatch";
+        public const string NameOfStringAssertStartsWith = "StartsWith";
+
         public const string FullNameOfTypeIs = "NUnit.Framework.Is";
         public const string FullNameOfTypeTestCaseAttribute = "NUnit.Framework.TestCaseAttribute";
         public const string FullNameOfTypeTestCaseSourceAttribute = "NUnit.Framework.TestCaseSourceAttribute";
@@ -136,5 +148,10 @@ public static class NUnitFrameworkConstants
         public const string NameOfUsing = "Using";
 
         public const string NUnitFrameworkAssemblyName = "nunit.framework";
+
+        public static readonly string[] AllAsserts =
+        {
+            NameOfAssert, NameOfStringAssert
+        };
     }
 }
diff --git a/src/nunit.analyzers/Extensions/ITypeSymbolExtensions.cs b/src/nunit.analyzers/Extensions/ITypeSymbolExtensions.cs
@@ -26,6 +26,13 @@ internal static bool IsAssert(this ITypeSymbol? @this)
                 @this.Name == NUnitFrameworkConstants.NameOfAssert;
         }
 
+        internal static bool IsAnyAssert(this ITypeSymbol? @this)
+        {
+            return @this is not null &&
+                   @this.ContainingAssembly.Name == NUnitFrameworkConstants.NUnitFrameworkAssemblyName &&
+                   NUnitFrameworkConstants.AllAsserts.Contains(@this.Name);
+        }
+
         internal static bool IsConstraint(this ITypeSymbol? @this)
         {
             return @this is not null && @this.GetAllBaseTypes()