Merge pull request #555 from manfred-brands/issue/541

Only report the first statement of the ones that can be combined in an Assert.Multiple
nunit · Jun 10, 2023 · b53185d · b53185d
2 parents 72b8eef + 9e9ddff
commit b53185d
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 23 deletions.
diff --git a/src/nunit.analyzers.tests/UseAssertMultiple/UseAssertMultipleAnalyzerTests.cs b/src/nunit.analyzers.tests/UseAssertMultiple/UseAssertMultipleAnalyzerTests.cs
@@ -51,7 +51,7 @@ public void Test()
             var configuration = new Configuration();
             Assert.That(configuration, Is.Not.Null);
             ↓Assert.That(configuration.Value1, Is.EqualTo(0));
-            ↓Assert.That(configuration.Value2, Is.EqualTo(0.0));
+            Assert.That(configuration.Value2, Is.EqualTo(0.0));
             Assert.That(configuration.Value11, Is.EqualTo(string.Empty));
         }
 

diff --git a/src/nunit.analyzers.tests/UseAssertMultiple/UseAssertMultipleCodeFixTests.cs b/src/nunit.analyzers.tests/UseAssertMultiple/UseAssertMultipleCodeFixTests.cs
@@ -56,7 +56,7 @@ public void Test()
             var configuration = new Configuration();
             Assert.That(configuration, Is.Not.Null);
             ↓Assert.That(configuration.Value1, Is.EqualTo(0));
-            ↓Assert.That(configuration.Value2, Is.EqualTo(0.0));
+            Assert.That(configuration.Value2, Is.EqualTo(0.0));
             Assert.That(configuration.Value11, Is.EqualTo(string.Empty));
             configuration = null;
         }
@@ -99,7 +99,7 @@ public void Test()
             var configuration = new Configuration();
             Assert.That(configuration, Is.Not.Null);
             ↓Assert.That(configuration.Value1, Is.EqualTo(0));
-            ↓Assert.That(configuration.Value2, Is.EqualTo(0.0));
+            Assert.That(configuration.Value2, Is.EqualTo(0.0));
             Assert.That(await configuration.AsStringAsync(), Is.EqualTo(string.Empty));
             configuration = null;
         }

diff --git a/src/nunit.analyzers/UseAssertMultiple/UseAssertMultipleAnalyzer.cs b/src/nunit.analyzers/UseAssertMultiple/UseAssertMultipleAnalyzer.cs
@@ -1,16 +1,12 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
-using System.Diagnostics;
-using System.Linq;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
 using NUnit.Analyzers.Constants;
 using NUnit.Analyzers.Extensions;
 using NUnit.Analyzers.Helpers;
-using NUnit.Analyzers.Operations;
 
 namespace NUnit.Analyzers.UseAssertMultiple
 {
@@ -51,7 +47,7 @@ internal static bool IsIndependent(HashSet<string> previousArguments, string arg
             foreach (var previousArgument in previousArguments)
             {
                 if (argument.StartsWith(previousArgument, StringComparison.Ordinal) &&
-                    !char.IsLetterOrDigit(argument[previousArgument.Length]))
+                   (argument.Length > previousArgument.Length && !char.IsLetterOrDigit(argument[previousArgument.Length])))
                 {
                     return false;
                 }
@@ -86,8 +82,9 @@ protected override void AnalyzeAssertInvocation(OperationAnalysisContext context
                 var previousArguments = new HashSet<string>(StringComparer.Ordinal);
 
                 // No need to check argument count as Assert.That needs at least one argument.
-                Add(previousArguments, assertOperation.Arguments[0].Syntax.ToString());
+                var assertArgument = assertOperation.Arguments[0].Syntax.ToString();
 
+                IOperation? statementBefore = null;
                 int firstAssert = -1;
                 int lastAssert = -1;
 
@@ -98,31 +95,35 @@ protected override void AnalyzeAssertInvocation(OperationAnalysisContext context
                     i++;
                     if (statement == assertExpression)
                     {
+                        if (statementBefore is not null)
+                        {
+                            var beforeArguments = new HashSet<string>(StringComparer.Ordinal);
+                            if (IsIndependentAssert(beforeArguments, statementBefore) &&
+                                IsIndependent(beforeArguments, assertArgument))
+                            {
+                                // This statement can be merged with the previous, hence was reported already.
+                                return;
+                            }
+                        }
+
+                        Add(previousArguments, assertArgument);
                         firstAssert = lastAssert = i;
                     }
                     else if (firstAssert >= 0)
                     {
-                        IInvocationOperation? currentAssertOperation = TryGetAssertThatOperation(statement);
-                        if (currentAssertOperation is not null)
+                        if (IsIndependentAssert(previousArguments, statement))
                         {
-                            // No need to check argument count as Assert.That needs at least one argument.
-                            string currentArgument = currentAssertOperation.Arguments[0].Syntax.ToString();
-
-                            // Check if test is independent
-                            if (IsIndependent(previousArguments, currentArgument))
-                            {
-                                lastAssert = i;
-                            }
-                            else
-                            {
-                                break;
-                            }
+                            lastAssert = i;
                         }
                         else
                         {
                             break;
                         }
                     }
+                    else
+                    {
+                        statementBefore = statement;
+                    }
                 }
 
                 if (lastAssert > firstAssert)
@@ -132,6 +133,22 @@ protected override void AnalyzeAssertInvocation(OperationAnalysisContext context
             }
         }
 
+        private static bool IsIndependentAssert(HashSet<string> previousArguments, IOperation statement)
+        {
+            IInvocationOperation? currentAssertOperation = TryGetAssertThatOperation(statement);
+            if (currentAssertOperation is not null)
+            {
+                // No need to check argument count as Assert.That needs at least one argument.
+                string currentArgument = currentAssertOperation.Arguments[0].Syntax.ToString();
+
+                // Check if test is independent
+                return IsIndependent(previousArguments, currentArgument);
+            }
+
+            // Not even an Assert operation.
+            return false;
+        }
+
         private static IInvocationOperation? TryGetAssertThatOperation(IOperation operation)
         {
             return (operation is IExpressionStatementOperation expressionOperation &&