feat: Add analyzer PH2162 and code fix for TestTimeouts class accessibility

Documentation/Diagnostics/PH2159.md

@@ -0,0 +1,139 @@
+# PH2159: TestTimeouts class should be internal
+
+| Property | Value  |
+|--|--|
+| Package | [Philips.CodeAnalysis.MsTestAnalyzers](https://www.nuget.org/packages/Philips.CodeAnalysis.MsTestAnalyzers) |
+| Diagnostic ID | PH2159 |
+| Category  | [MsTest](../MsTest.md) |
+| Analyzer | [TestTimeoutsClassAccessibilityAnalyzer](https://github.com/philips-software/roslyn-analyzers/blob/main/Philips.CodeAnalysis.MsTestAnalyzers/TestTimeoutsClassAccessibilityAnalyzer.cs) |
+| CodeFix  | Yes |
+| Severity | Warning |
+| Enabled By Default | Yes |
+
+## Introduction
+
+Projects commonly define a `TestTimeouts` class that centralizes timeout constants (e.g., `CiAppropriate`, `Integration`). This analyzer enforces that any class named `TestTimeouts` is declared as `internal` and not `static`. Public exposure or use of `static` on this coordination class is discouraged.
+
+## Reason
+
+Keeping `TestTimeouts` internal:
+
+1. Prevents leaking test-only infrastructure into the public API surface of a library.
+2. Avoids external projects taking a compile-time dependency on timeout values that are intended to be adjusted freely.
+3. Encourages each assembly to own and tune its own timeout set without cross-assembly coupling.
+4. Reduces the risk that production code (outside test projects) references test timing constants.
+
+Disallowing `static` (while permitting `sealed`) helps future flexibility (e.g., partial conditional compilation patterns) and keeps the shape consistent with existing guidance in related timeout analyzers. A `sealed` internal class with only `const` members satisfies the same usage needs without requiring `static`.
+
+## What triggers the diagnostic
+
+The diagnostic is reported for any class whose identifier is exactly `TestTimeouts` if either:
+
+* It is declared `public` (with or without `sealed`).
+* It is declared `internal static`.
+
+The class name comparison is exact and case-sensitive.
+
+## How to fix
+
+Change the declaration to be `internal` (optionally `sealed`) and remove `static` if present. The provided Code Fix will:
+
+* Remove `public`.
+* Remove `static` (when paired with `internal`).
+* Insert `internal` if missing.
+* Preserve `sealed` if it exists.
+
+## Examples
+
+Code that triggers the diagnostic (public):
+```cs
+public class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Fix:
+```cs
+internal class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Code that triggers the diagnostic (public sealed):
+```cs
+public sealed class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Fix (Code Fix output):
+```cs
+internal sealed class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Code that triggers the diagnostic (internal static):
+```cs
+internal static class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Fix (Code Fix output):
+```cs
+internal class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Code that does NOT trigger the diagnostic:
+```cs
+internal class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+
+internal sealed class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+Other class names are ignored:
+```cs
+public class SomeOtherClass
+{
+	public const int CiAppropriate = 1000;
+}
+```
+
+## Code Fix behavior
+
+Selecting the Code Fix on a reported diagnostic rewrites the class modifiers as described above. It does not otherwise reorder members or alter whitespace beyond necessary modifier changes.
+
+## Similar analyzers
+
+| ID | Title |
+|--|--|
+| [PH2012](./PH2012.md) | Test has Timeout |
+| [PH2015](./PH2015.md) | Test must have appropriate Test Category |
+
+These analyzers collectively encourage consistent timeout usage patterns.
+
+## Configuration
+
+This analyzer does not currently support custom configuration keys. Standard suppression mechanisms (pragma, GlobalSuppressions.cs, or EditorConfig severity adjustments) apply, but suppression is discouraged—prefer adopting the recommended accessibility.
+
+```
+# Example: adjust severity (not recommended)
+dotnet_diagnostic.PH2159.severity = warning
+```
+
+

Philips.CodeAnalysis.Common/DiagnosticId.cs

@@ -144,5 +144,6 @@ public enum DiagnosticId
 		AvoidUnusedToString = 2153,
 		AvoidUnlicensedPackages = 2155,
 		AvoidPkcsPaddingWithRsaEncryption = 2158,
+		TestTimeoutsClassShouldBeInternal = 2159,
 	}
 }

Philips.CodeAnalysis.MsTestAnalyzers/TestTimeoutsClassAccessibilityAnalyzer.cs

@@ -0,0 +1,61 @@
+// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Philips.CodeAnalysis.Common;
+
+namespace Philips.CodeAnalysis.MsTestAnalyzers
+{
+	[DiagnosticAnalyzer(LanguageNames.CSharp)]
+	public class TestTimeoutsClassAccessibilityAnalyzer : SingleDiagnosticAnalyzer
+	{
+		private const string Title = @"TestTimeouts class should be internal";
+		private const string MessageFormat = @"Class 'TestTimeouts' should be declared as internal";
+		private const string Description = @"TestTimeouts classes should be internal to avoid accessibility issues";
+
+		public TestTimeoutsClassAccessibilityAnalyzer()
+			: base(DiagnosticId.TestTimeoutsClassShouldBeInternal, Title, MessageFormat, Description, Categories.MsTest, isEnabled: true)
+		{ }
+
+		protected override void InitializeCompilation(CompilationStartAnalysisContext context)
+		{
+			context.RegisterSyntaxNodeAction(AnalyzeClassDeclaration, SyntaxKind.ClassDeclaration);
+		}
+
+		private void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context)
+		{
+			if (Helper.ForGeneratedCode.IsGeneratedCode(context))
+			{
+				return;
+			}
+
+			var classDeclaration = (ClassDeclarationSyntax)context.Node;
+
+			// Check if the class is named "TestTimeouts"
+			if (classDeclaration.Identifier.ValueText != "TestTimeouts")
+			{
+				return;
+			}
+
+			SyntaxTokenList modifiers = classDeclaration.Modifiers;
+
+			// Check if the class has problematic modifiers:
+			// 1. public (with or without sealed)
+			// 2. internal static
+			var hasPublic = modifiers.Any(SyntaxKind.PublicKeyword);
+			var hasInternal = modifiers.Any(SyntaxKind.InternalKeyword);
+			var hasStatic = modifiers.Any(SyntaxKind.StaticKeyword);
+
+			var needsFix = hasPublic || (hasInternal && hasStatic);
+
+			if (needsFix)
+			{
+				Location location = classDeclaration.Identifier.GetLocation();
+				var diagnostic = Diagnostic.Create(Rule, location);
+				context.ReportDiagnostic(diagnostic);
+			}
+		}
+	}
+}

Philips.CodeAnalysis.MsTestAnalyzers/TestTimeoutsClassAccessibilityCodeFixProvider.cs

@@ -0,0 +1,54 @@
+// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Collections.Immutable;
+using System.Composition;
+using System.Linq;
+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(TestTimeoutsClassAccessibilityCodeFixProvider)), Shared]
+	public class TestTimeoutsClassAccessibilityCodeFixProvider : SingleDiagnosticCodeFixProvider<ClassDeclarationSyntax>
+	{
+		protected override string Title => "Make TestTimeouts class internal";
+
+		protected override DiagnosticId DiagnosticId => DiagnosticId.TestTimeoutsClassShouldBeInternal;
+
+		protected override async Task<Document> ApplyFix(Document document, ClassDeclarationSyntax node, ImmutableDictionary<string, string> properties, CancellationToken cancellationToken)
+		{
+			SyntaxNode rootNode = await document.GetSyntaxRootAsync(cancellationToken);
+
+			// Remove problematic modifiers and ensure internal is present
+			SyntaxTokenList modifiers = node.Modifiers;
+
+			// Remove public, sealed (when public), and static (when internal) modifiers
+			var newModifiers = modifiers
+				.Where(m => m.Kind() is not SyntaxKind.PublicKeyword and not SyntaxKind.StaticKeyword)
+				.ToList();
+
+			// Check if we need to add internal
+			var hasInternal = newModifiers.Any(m => m.Kind() == SyntaxKind.InternalKeyword);
+
+			if (!hasInternal)
+			{
+				// Add internal modifier at the beginning
+				newModifiers.Insert(0, SyntaxFactory.Token(SyntaxKind.InternalKeyword));
+			}
+
+			// Create new class declaration with updated modifiers
+			ClassDeclarationSyntax newClass = node.WithModifiers(SyntaxFactory.TokenList(newModifiers));
+
+			// Replace the node in the root
+			SyntaxNode root = rootNode.ReplaceNode(node, newClass);
+			Document newDocument = document.WithSyntaxRoot(root);
+
+			return newDocument;
+		}
+	}
+}

Philips.CodeAnalysis.Test/Maintainability/Maintainability/AvoidProblematicUsingPatternsAnalyzerTest.cs

@@ -1,4 +1,4 @@
-// © 2024 Koninklijke Philips N.V. See License.md in the project root for license information.
+// © 2024 Koninklijke Philips N.V. See License.md in the project root for license information.
 
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Diagnostics;
@@ -239,4 +239,4 @@ protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
 			return new AvoidProblematicUsingPatternsAnalyzer();
 		}
 	}
-}
+}

Philips.CodeAnalysis.Test/MsTest/TestTimeoutsClassAccessibilityAnalyzerTest.cs

@@ -0,0 +1,105 @@
+// © 2025 Koninklijke Philips N.V. See License.md in the project root for license information.
+
+using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Philips.CodeAnalysis.Common;
+using Philips.CodeAnalysis.MsTestAnalyzers;
+using Philips.CodeAnalysis.Test.Helpers;
+using Philips.CodeAnalysis.Test.Verifiers;
+
+namespace Philips.CodeAnalysis.Test.MsTest
+{
+	[TestClass]
+	public class TestTimeoutsClassAccessibilityAnalyzerTest : CodeFixVerifier
+	{
+		protected override DiagnosticAnalyzer GetDiagnosticAnalyzer()
+		{
+			return new TestTimeoutsClassAccessibilityAnalyzer();
+		}
+
+		protected override CodeFixProvider GetCodeFixProvider()
+		{
+			return new TestTimeoutsClassAccessibilityCodeFixProvider();
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task TimeoutsClassPublicShouldTriggerDiagnostic()
+		{
+			const string code = @"public class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			const string expectedCode = @"internal class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			await VerifyDiagnostic(code, DiagnosticId.TestTimeoutsClassShouldBeInternal).ConfigureAwait(false);
+			await VerifyFix(code, expectedCode).ConfigureAwait(false);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task TimeoutsClassInternalShouldNotTriggerDiagnostic()
+		{
+			const string code = @"internal class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			await VerifySuccessfulCompilation(code).ConfigureAwait(false);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task TimeoutsClassPublicSealedShouldTriggerDiagnostic()
+		{
+			const string code = @"public sealed class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			const string expectedCode = @"internal sealed class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			await VerifyDiagnostic(code, DiagnosticId.TestTimeoutsClassShouldBeInternal).ConfigureAwait(false);
+			await VerifyFix(code, expectedCode).ConfigureAwait(false);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task TimeoutsClassInternalStaticShouldTriggerDiagnostic()
+		{
+			const string code = @"internal static class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			const string expectedCode = @"internal class TestTimeouts
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			await VerifyDiagnostic(code, DiagnosticId.TestTimeoutsClassShouldBeInternal).ConfigureAwait(false);
+			await VerifyFix(code, expectedCode).ConfigureAwait(false);
+		}
+
+		[TestMethod]
+		[TestCategory(TestDefinitions.UnitTests)]
+		public async Task OtherClassNameShouldNotTriggerDiagnostic()
+		{
+			const string code = @"public class SomeOtherClass
+{
+	public const int CiAppropriate = 1000;
+}";
+
+			await VerifySuccessfulCompilation(code).ConfigureAwait(false);
+		}
+	}
+}