feat: Add DataRow ordering analyzer with code fixer and generic attribute ordering utilities (PH2152)

Documentation/Diagnostics/PH2152.md

@@ -0,0 +1,69 @@
+# PH2152: Order DataRow attribute above TestMethod for unit tests
+
+| Property | Value  |
+|--|--|
+| Package | [Philips.CodeAnalysis.MsTestAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MsTestAnalyzers) |
+| Diagnostic ID | PH2152 |
+| Category  | [MsTest](../MsTest.md) |
+| Analyzer | [DataRowOrderAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MsTestAnalyzers/DataRowOrderAnalyzer.cs)
+| CodeFix  | Yes |
+| Severity | Warning |
+| Enabled By Default | No |
+
+## Introduction
+
+DataRow attributes should be consistently ordered above TestMethod/DataTestMethod attributes on test methods to improve readability and maintain consistency across the codebase.
+
+## How to solve
+
+Move all DataRow attributes to appear before TestMethod or DataTestMethod attributes on the method.
+
+## Example
+
+Code that triggers a diagnostic:
+``` cs
+[TestClass]
+public class Tests
+{
+    [TestMethod]
+    [DataRow(1, 2)]
+    [DataRow(3, 4)]
+    public void TestMethod1(int x, int y) 
+    { 
+        // Test implementation
+    }
+
+    [DataTestMethod]
+    [DataRow("test")]
+    public void TestMethod2(string value) 
+    { 
+        // Test implementation
+    }
+}
+```
+
+And the replacement code:
+``` cs
+[TestClass]
+public class Tests
+{
+    [DataRow(1, 2)]
+    [DataRow(3, 4)]
+    [TestMethod]
+    public void TestMethod1(int x, int y) 
+    { 
+        // Test implementation
+    }
+
+    [DataRow("test")]
+    [DataTestMethod]
+    public void TestMethod2(string value) 
+    { 
+        // Test implementation
+    }
+}
+```
+
+## Configuration
+
+This analyzer does not offer any special configuration. The general ways of [suppressing](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/suppress-warnings) diagnostics apply.

Philips.CodeAnalysis.Common/AttributeHelper.cs

@@ -1,6 +1,7 @@
 // © 2019 Koninklijke Philips N.V. See License.md in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
@@ -122,6 +123,88 @@
 			return IsAttribute(attribute, context, MsTestFrameworkDefinitions.DataRowAttribute, out _, out _);
 		}
 
+		/// <summary>
+		/// Checks if any attribute of the specified types appears after any attribute of the other specified types.
+		/// </summary>
+		/// <param name="attributeLists">The attribute lists to examine</param>
+		/// <param name="context">The syntax node analysis context</param>
+		/// <param name="attributesToFind">The attributes to look for that should appear first</param>
+		/// <param name="attributesToCheckAfter">The attributes that should not appear before attributesToFind</param>
+		/// <returns>True if any attributesToFind appears after any attributesToCheckAfter</returns>
+		public bool HasAttributeAfterOther(SyntaxList<AttributeListSyntax> attributeLists, SyntaxNodeAnalysisContext context, INamedTypeSymbol[] attributesToFind, INamedTypeSymbol[] attributesToCheckAfter)
+		{
+			var allAttributes = attributeLists
+				.SelectMany((list, listIndex) =>
+					list.Attributes.Select((attr, attrIndex) => new
+					{
+						Attribute = attr,
+						Position = (listIndex, attrIndex),
+						Symbol = context.SemanticModel.GetSymbolInfo(attr).Symbol?.ContainingType
+					}))
+				.Where(x => x.Symbol != null)
+				.ToArray();
+
+			(int listIndex, int attrIndex)? firstCheckAfterPosition = allAttributes
+				.Where(x => attributesToCheckAfter.Any(attr => AttributeMatches(x.Symbol, attr)))
+				.Select(x => x.Position)
+				.FirstOrDefault();
+
+			if (firstCheckAfterPosition == null)
+			{
+				return false;
+			}
+
+			return allAttributes
+				.Where(x => attributesToFind.Any(attr => AttributeMatches(x.Symbol, attr)))
+				.Any(x => x.Position.CompareTo(firstCheckAfterPosition.Value) > 0);
+		}
+
+		/// <summary>
+		/// Categorizes attributes into groups based on the provided type predicates.
+		/// </summary>
+		/// <param name="attributeLists">The attribute lists to categorize</param>
+		/// <param name="context">The syntax node analysis context</param>
+		/// <param name="categorizers">Functions that determine which category an attribute belongs to</param>
+		/// <returns>A dictionary mapping category names to lists of attributes</returns>
+		public Dictionary<string, List<AttributeSyntax>> CategorizeAttributes(SyntaxList<AttributeListSyntax> attributeLists, SyntaxNodeAnalysisContext context, Dictionary<string, Func<INamedTypeSymbol, bool>> categorizers)
+		{
+			var result = new Dictionary<string, List<AttributeSyntax>>();
+			foreach (var category in categorizers.Keys)
+			{
+				result[category] = [];
+			}
+
+			foreach (AttributeListSyntax attributeList in attributeLists)
+			{
+				foreach (AttributeSyntax attribute in attributeList.Attributes)
+				{
+					INamedTypeSymbol attributeSymbol = context.SemanticModel.GetSymbolInfo(attribute).Symbol?.ContainingType;
+					if (attributeSymbol != null)
+					{
+						KeyValuePair<string, Func<INamedTypeSymbol, bool>>? matchingCategorizer = categorizers.Where(c => c.Value(attributeSymbol)).FirstOrDefault();
+						if (matchingCategorizer.HasValue)
+						{
+							result[matchingCategorizer.Value.Key].Add(attribute);
+						}
+					}
+				}
+			}
+
+			return result;
+		}
+
+		/// <summary>
+		/// Checks if an attribute symbol matches a target symbol (including inheritance).
+		/// </summary>
+		/// <param name="attributeSymbol">The attribute symbol to check</param>
+		/// <param name="targetSymbol">The target symbol to match against</param>
+		/// <returns>True if the symbols match or if attributeSymbol is derived from targetSymbol</returns>
+		private static bool AttributeMatches(INamedTypeSymbol attributeSymbol, INamedTypeSymbol targetSymbol)
+		{
+			return SymbolEqualityComparer.Default.Equals(attributeSymbol, targetSymbol) ||
+				   attributeSymbol.IsDerivedFrom(targetSymbol);
+		}
+
 		public bool TryExtractAttributeArgument<T>(AttributeArgumentSyntax argumentSyntax, SyntaxNodeAnalysisContext context, out string argumentString, out T value)
 		{
 			argumentString = argumentSyntax.Expression.ToString();

Philips.CodeAnalysis.Common/DiagnosticId.cs

@@ -141,6 +141,7 @@ public enum DiagnosticId
 		AvoidVariableNamedUnderscore = 2147,
 		AvoidProblematicUsingPatterns = 2149,
 		AvoidTodoComments = 2151,
+		DataRowOrderInTestMethod = 2152,
 		AvoidUnusedToString = 2153,
 		AvoidUnlicensedPackages = 2155,
 		AvoidPkcsPaddingWithRsaEncryption = 2158,

Philips.CodeAnalysis.Common/Helper.cs

@@ -10,5 +10,7 @@ public class Helper(AnalyzerOptions options, Compilation compilation) : CodeFixH
 		public AllowedSymbols ForAllowedSymbols { get; } = new AllowedSymbols(compilation);
 
 		public AdditionalFilesHelper ForAdditionalFiles { get; } = new AdditionalFilesHelper(options, compilation);
+
+		public AttributeHelper AttributeHelper { get; } = new AttributeHelper();
 	}
 }

Philips.CodeAnalysis.MsTestAnalyzers/DataRowOrderAnalyzer.cs

@@ -0,0 +1,68 @@
+// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Linq;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Philips.CodeAnalysis.Common;
+
+namespace Philips.CodeAnalysis.MsTestAnalyzers
+{
+	[DiagnosticAnalyzer(LanguageNames.CSharp)]
+	public class DataRowOrderAnalyzer : TestAttributeDiagnosticAnalyzer
+	{
+		private const string Title = @"Order DataRow attribute above TestMethod for unit tests";
+		public static readonly string MessageFormat = @"DataRow attributes should be placed above TestMethod/DataTestMethod attributes";
+		private const string Description = @"DataRow attributes should be consistently ordered above TestMethod/DataTestMethod attributes on test methods";
+		private const string Category = Categories.MsTest;
+
+		private static readonly DiagnosticDescriptor Rule = new(DiagnosticId.DataRowOrderInTestMethod.ToId(),
+												Title, MessageFormat, Category, DiagnosticSeverity.Warning, isEnabledByDefault: false, description: Description);
+
+
+		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);
+
+		protected override Implementation OnInitializeAnalyzer(AnalyzerOptions options, Compilation compilation, MsTestAttributeDefinitions definitions)
+		{
+			return new DataRowOrderImplementation(definitions, Helper);
+		}
+
+		private sealed class DataRowOrderImplementation : Implementation
+		{
+			private readonly MsTestAttributeDefinitions _definitions;
+
+			public DataRowOrderImplementation(MsTestAttributeDefinitions definitions, Helper helper) : base(helper)
+			{
+				_definitions = definitions;
+			}
+
+			public override void OnTestAttributeMethod(SyntaxNodeAnalysisContext context, MethodDeclarationSyntax methodDeclaration, IMethodSymbol methodSymbol, HashSet<INamedTypeSymbol> presentAttributes)
+			{
+				if (!HasRequiredAttributes(presentAttributes))
+				{
+					return;
+				}
+
+				// Check if DataRow comes after TestMethod using the generic AttributeHelper method
+				INamedTypeSymbol[] dataRowAttributes = { _definitions.DataRowSymbol };
+				INamedTypeSymbol[] testMethodAttributes = { _definitions.TestMethodSymbol, _definitions.DataTestMethodSymbol };
+
+				if (Helper.AttributeHelper.HasAttributeAfterOther(methodDeclaration.AttributeLists, context, dataRowAttributes, testMethodAttributes))
+				{
+					Location location = methodDeclaration.Identifier.GetLocation();
+					context.ReportDiagnostic(Diagnostic.Create(Rule, location, methodDeclaration.Identifier));
+				}
+			}
+
+			private bool HasRequiredAttributes(HashSet<INamedTypeSymbol> presentAttributes)
+			{
+				return presentAttributes.Contains(_definitions.DataRowSymbol) &&
+					presentAttributes.Any(attr =>
+						attr.IsDerivedFrom(_definitions.TestMethodSymbol) ||
+						attr.IsDerivedFrom(_definitions.DataTestMethodSymbol));
+			}
+		}
+	}
+}

Philips.CodeAnalysis.MsTestAnalyzers/DataRowOrderCodeFixProvider.cs

@@ -0,0 +1,100 @@
+// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Composition;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Philips.CodeAnalysis.Common;
+
+namespace Philips.CodeAnalysis.MsTestAnalyzers
+{
+	[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(DataRowOrderCodeFixProvider)), Shared]
+	public class DataRowOrderCodeFixProvider : SingleDiagnosticCodeFixProvider<MethodDeclarationSyntax>
+	{
+		protected override string Title => "Move DataRow attributes above TestMethod";
+
+		protected override DiagnosticId DiagnosticId => DiagnosticId.DataRowOrderInTestMethod;
+
+		protected override async Task<Document> ApplyFix(Document document, MethodDeclarationSyntax node, ImmutableDictionary<string, string> properties, CancellationToken cancellationToken)
+		{
+			SyntaxNode root = await document.GetSyntaxRootAsync(cancellationToken);
+			SemanticModel semanticModel = await document.GetSemanticModelAsync(cancellationToken);
+
+			// Get the MsTest definitions to identify attributes
+			var definitions = MsTestAttributeDefinitions.FromCompilation(semanticModel.Compilation);
+
+			// Reorder the attributes
+			MethodDeclarationSyntax reorderedMethod = ReorderAttributes(node, semanticModel, definitions);
+
+			SyntaxNode newRoot = root.ReplaceNode(node, reorderedMethod);
+			return document.WithSyntaxRoot(newRoot);
+		}
+
+		private static MethodDeclarationSyntax ReorderAttributes(MethodDeclarationSyntax method, SemanticModel semanticModel, MsTestAttributeDefinitions definitions)
+		{
+			var dataRowAttributes = new List<AttributeSyntax>();
+			var testMethodAttributes = new List<AttributeSyntax>();
+			var otherAttributes = new List<AttributeSyntax>();
+
+			// Categorize all attributes
+			foreach (AttributeListSyntax attributeList in method.AttributeLists)
+			{
+				foreach (AttributeSyntax attribute in attributeList.Attributes)
+				{
+					INamedTypeSymbol attributeSymbol = semanticModel.GetSymbolInfo(attribute).Symbol?.ContainingType;
+					if (attributeSymbol != null)
+					{
+						if (SymbolEqualityComparer.Default.Equals(attributeSymbol, definitions.DataRowSymbol))
+						{
+							dataRowAttributes.Add(attribute);
+						}
+						else if (attributeSymbol.IsDerivedFrom(definitions.TestMethodSymbol) ||
+								 attributeSymbol.IsDerivedFrom(definitions.DataTestMethodSymbol))
+						{
+							testMethodAttributes.Add(attribute);
+						}
+						else
+						{
+							otherAttributes.Add(attribute);
+						}
+					}
+					else
+					{
+						otherAttributes.Add(attribute);
+					}
+				}
+			}
+
+			// Create new attribute lists in the correct order: DataRow, Other, TestMethod
+			SyntaxList<AttributeListSyntax> newAttributeLists = SyntaxFactory.List<AttributeListSyntax>();
+
+			// Add DataRow attributes first
+			foreach (AttributeSyntax attr in dataRowAttributes)
+			{
+				newAttributeLists = newAttributeLists.Add(
+					SyntaxFactory.AttributeList(SyntaxFactory.SingletonSeparatedList(attr)));
+			}
+
+			// Add other attributes (like TestCategory, Timeout, etc.)
+			foreach (AttributeSyntax attr in otherAttributes)
+			{
+				newAttributeLists = newAttributeLists.Add(
+					SyntaxFactory.AttributeList(SyntaxFactory.SingletonSeparatedList(attr)));
+			}
+
+			// Add TestMethod/DataTestMethod attributes last
+			foreach (AttributeSyntax attr in testMethodAttributes)
+			{
+				newAttributeLists = newAttributeLists.Add(
+					SyntaxFactory.AttributeList(SyntaxFactory.SingletonSeparatedList(attr)));
+			}
+
+			return method.WithAttributeLists(newAttributeLists);
+		}
+	}
+}

Philips.CodeAnalysis.MsTestAnalyzers/Philips.CodeAnalysis.MsTestAnalyzers.md

@@ -38,5 +38,5 @@
 | [PH2059](../Documentation/Diagnostics/PH2059.md)  | Public Method should be TestMethod                   | Public methods inside a TestClass should either be a test method or non-public. | ✅ **Use [MSTEST0029](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0029)** |
 | [PH2076](../Documentation/Diagnostics/PH2076.md)  | Assert.Fail alternatives                             | Assert.Fail should not be used if an alternative is more appropriate | ⚠️ **Consider [MSTEST0025](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0025)** |
 | [PH2095](../Documentation/Diagnostics/PH2095.md)  | TestMethods must return void/Task for async methods  | TestMethods must return Task if they are async methods, or void if not | ⚠️ **Consider [MSTEST0003](https://learn.microsoft.com/dotnet/core/testing/mstest-analyzers/mstest0003)** |
-
+| [PH2152](../Documentation/Diagnostics/PH2152.md)  | Order DataRow attribute above TestMethod for unit tests | DataRow attributes should be consistently ordered above TestMethod/DataTestMethod attributes on test methods |