Merge pull request #772 from manfred-brands/Issue770_TestContext.Write

Added TestContext.Write Is Obsolete Analyzer

documentation/NUnit1033.md

@@ -0,0 +1,79 @@
+# NUnit1033
+
+## The Write methods on TestContext will be marked as Obsolete and eventually removed
+
+| Topic    | Value
+| :--      | :--
+| Id       | NUnit1033
+| Severity | Warning
+| Enabled  | True
+| Category | Structure
+| Code     | [TestContextWriteIsObsoleteAnalyzer](https://github.com/nunit/nunit.analyzers/blob/master/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs)
+
+## Description
+
+Direct `Write` calls should be replaced with `Out.Write`.
+
+Future version of NUnit will first mark the `.Write` methods on `TestContext`
+as `Obsolete` and eventually remove them.
+
+This rule allows updating your code before the methods are removed.
+
+## Motivation
+
+The `Write` methods are simple wrappers calling `Out.Write`.
+There is no wrapper for `Error` which always required to use `TestContext.Error.Write`.
+Besides this being inconsistent, later versions of .NET added new overloads,
+ e.g. for `ReadOnlySpan<char>` and `async` methods like `WriteAsync`.
+Instead of adding more and more dummy wrappers, it was decided that user code should use
+ the `Out` property and then can use any `Write` overload available on `TextWriter`.
+
+## How to fix violations
+
+Simply insert `.Out` between `TestContext` and `.Write`.
+
+`TestContext.WriteLine("This isn't right");`
+
+becomes
+
+`TestContext.Out.WriteLine("This isn't right");`
+
+<!-- start generated config severity -->
+## Configure severity
+
+### Via ruleset file
+
+Configure the severity per project, for more info see
+[MSDN](https://learn.microsoft.com/en-us/visualstudio/code-quality/using-rule-sets-to-group-code-analysis-rules?view=vs-2022).
+
+### Via .editorconfig file
+
+```ini
+# NUnit1033: The Write methods on TestContext will be marked as Obsolete and eventually removed
+dotnet_diagnostic.NUnit1033.severity = chosenSeverity
+```
+
+where `chosenSeverity` can be one of `none`, `silent`, `suggestion`, `warning`, or `error`.
+
+### Via #pragma directive
+
+```csharp
+#pragma warning disable NUnit1033 // The Write methods on TestContext will be marked as Obsolete and eventually removed
+Code violating the rule here
+#pragma warning restore NUnit1033 // The Write methods on TestContext will be marked as Obsolete and eventually removed
+```
+
+Or put this at the top of the file to disable all instances.
+
+```csharp
+#pragma warning disable NUnit1033 // The Write methods on TestContext will be marked as Obsolete and eventually removed
+```
+
+### Via attribute `[SuppressMessage]`
+
+```csharp
+[System.Diagnostics.CodeAnalysis.SuppressMessage("Structure",
+    "NUnit1033:The Write methods on TestContext will be marked as Obsolete and eventually removed",
+    Justification = "Reason...")]
+```
+<!-- end generated config severity -->

documentation/index.md

@@ -51,6 +51,7 @@ Rules which enforce structural requirements on the test code.
 | [NUnit1030](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1030.md) | The type of parameter provided by the TestCaseSource does not match the type of the parameter in the Test method | :white_check_mark: | :exclamation: | :x: |
 | [NUnit1031](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1031.md) | The individual arguments provided by a ValuesAttribute must match the type of the corresponding parameter of the method | :white_check_mark: | :exclamation: | :x: |
 | [NUnit1032](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1032.md) | An IDisposable field/property should be Disposed in a TearDown method | :white_check_mark: | :exclamation: | :x: |
+| [NUnit1033](https://github.com/nunit/nunit.analyzers/tree/master/documentation/NUnit1033.md) | The Write methods on TestContext will be marked as Obsolete and eventually removed | :white_check_mark: | :warning: | :white_check_mark: |
 
 ## Assertion Rules (NUnit2001 - )
 

src/nunit.analyzers.sln

@@ -42,6 +42,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "documentation", "documentat
 		..\documentation\NUnit1030.md = ..\documentation\NUnit1030.md
 		..\documentation\NUnit1031.md = ..\documentation\NUnit1031.md
 		..\documentation\NUnit1032.md = ..\documentation\NUnit1032.md
+		..\documentation\NUnit1033.md = ..\documentation\NUnit1033.md
 		..\documentation\NUnit2001.md = ..\documentation\NUnit2001.md
 		..\documentation\NUnit2002.md = ..\documentation\NUnit2002.md
 		..\documentation\NUnit2003.md = ..\documentation\NUnit2003.md

src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs

@@ -76,6 +76,10 @@ public sealed class NUnitFrameworkConstantsTests
             (nameof(NUnitFrameworkConstants.NameOfMultipleAsync), "MultipleAsync"),
 #endif
 
+            (nameof(NUnitFrameworkConstants.NameOfOut), nameof(TestContext.Out)),
+            (nameof(NUnitFrameworkConstants.NameOfWrite), nameof(TestContext.Out.Write)),
+            (nameof(NUnitFrameworkConstants.NameOfWriteLine), nameof(TestContext.Out.WriteLine)),
+
             (nameof(NUnitFrameworkConstants.NameOfThrows), nameof(Throws)),
             (nameof(NUnitFrameworkConstants.NameOfThrowsArgumentException), nameof(Throws.ArgumentException)),
             (nameof(NUnitFrameworkConstants.NameOfThrowsArgumentNullException), nameof(Throws.ArgumentNullException)),
@@ -208,6 +212,8 @@ public sealed class NUnitFrameworkConstantsTests
             (nameof(NUnitFrameworkConstants.FullNameOfCancelAfterAttribute), typeof(CancelAfterAttribute)),
             (nameof(NUnitFrameworkConstants.FullNameOfCancellationToken), typeof(CancellationToken)),
 
+            (nameof(NUnitFrameworkConstants.FullNameOfTypeTestContext), typeof(TestContext)),
+
             (nameof(NUnitFrameworkConstants.FullNameOfSameAsConstraint), typeof(SameAsConstraint)),
             (nameof(NUnitFrameworkConstants.FullNameOfSomeItemsConstraint), typeof(SomeItemsConstraint)),
             (nameof(NUnitFrameworkConstants.FullNameOfEqualToConstraint), typeof(EqualConstraint)),

src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs

@@ -0,0 +1,39 @@
+using Gu.Roslyn.Asserts;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NUnit.Analyzers.Constants;
+using NUnit.Analyzers.TestContextWriteIsObsolete;
+using NUnit.Framework;
+
+namespace NUnit.Analyzers.Tests.TestContextWriteIsObsolete
+{
+    public class TestContextWriteIsObsoleteAnalyzerTests
+    {
+        private static readonly DiagnosticAnalyzer analyzer = new TestContextWriteIsObsoleteAnalyzer();
+        private static readonly ExpectedDiagnostic expectedDiagnostic =
+            ExpectedDiagnostic.Create(AnalyzerIdentifiers.TestContextWriteIsObsolete);
+
+        [TestCaseSource(typeof(TestContextWriteIsObsoleteTestCases), nameof(TestContextWriteIsObsoleteTestCases.WriteInvocations))]
+        public void AnyDirectWriteMethod(string writeMethodAndParameters)
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+                public void Test()
+                {{
+                    ↓TestContext.{writeMethodAndParameters};
+                }}");
+
+            RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode);
+        }
+
+        [TestCaseSource(typeof(TestContextWriteIsObsoleteTestCases), nameof(TestContextWriteIsObsoleteTestCases.WriteInvocations))]
+        public void AnyIndirectWriteMethod(string writeMethodAndParameters)
+        {
+            var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+                public void Test()
+                {{
+                    TestContext.Out.{writeMethodAndParameters};
+                }}");
+
+            RoslynAssert.Valid(analyzer, testCode);
+        }
+    }
+}

src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs

@@ -0,0 +1,36 @@
+using Gu.Roslyn.Asserts;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Diagnostics;
+using NUnit.Analyzers.Constants;
+using NUnit.Analyzers.TestContextWriteIsObsolete;
+using NUnit.Framework;
+
+namespace NUnit.Analyzers.Tests.TestContextWriteIsObsolete
+{
+    public class TestContextWriteIsObsoleteCodeFixTests
+    {
+        private static readonly DiagnosticAnalyzer analyzer = new TestContextWriteIsObsoleteAnalyzer();
+        private static readonly CodeFixProvider fix = new TestContextWriteIsObsoleteCodeFix();
+        private static readonly ExpectedDiagnostic expectedDiagnostic =
+            ExpectedDiagnostic.Create(AnalyzerIdentifiers.TestContextWriteIsObsolete);
+
+        [TestCaseSource(typeof(TestContextWriteIsObsoleteTestCases), nameof(TestContextWriteIsObsoleteTestCases.WriteInvocations))]
+        public void AnyWriteMethod(string writeMethodAndParameters)
+        {
+            var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+                public void Test()
+                {{
+                    ↓TestContext.{writeMethodAndParameters};
+                }}");
+
+            var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@"
+                public void Test()
+                {{
+                    TestContext.Out.{writeMethodAndParameters};
+                }}");
+
+            RoslynAssert.CodeFix(analyzer, fix, expectedDiagnostic, code, fixedCode,
+                fixTitle: TestContextWriteIsObsoleteCodeFix.InsertOutDescription);
+        }
+    }
+}

src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteTestCases.cs

@@ -0,0 +1,46 @@
+namespace NUnit.Analyzers.Tests.TestContextWriteIsObsolete
+{
+    internal static class TestContextWriteIsObsoleteTestCases
+    {
+        public static readonly string[] WriteInvocations =
+        {
+            "Write(true)",
+            "Write('!')",
+            "Write(new char[] { '!', '!' })",
+            "Write(default(char[]))",
+            "Write(1D)",
+            "Write(1)",
+            "Write(1L)",
+            "Write(1M)",
+            "Write(default(object))",
+            "Write(1F)",
+            "Write(\"NUnit\")",
+            "Write(default(string))",
+            "Write(1U)",
+            "Write(1UL)",
+            "Write(\"{0}\", 1)",
+            "Write(\"{0} + {1}\", 1, 2)",
+            "Write(\"{0} + {1} = {2}\", 1, 2, 3)",
+            "Write(\"{0} + {1} = {2} + {3}\", 1, 2, 2, 1)",
+            "WriteLine()",
+            "WriteLine(true)",
+            "WriteLine('!')",
+            "WriteLine(new char[] { '!', '!' })",
+            "WriteLine(default(char[]))",
+            "WriteLine(1D)",
+            "WriteLine(1)",
+            "WriteLine(1L)",
+            "WriteLine(1M)",
+            "WriteLine(default(object))",
+            "WriteLine(1F)",
+            "WriteLine(\"NUnit\")",
+            "Write(default(string))",
+            "WriteLine(1U)",
+            "WriteLine(1UL)",
+            "WriteLine(\"{0}\", 1)",
+            "WriteLine(\"{0} + {1}\", 1, 2)",
+            "WriteLine(\"{0} + {1} = {2}\", 1, 2, 3)",
+            "WriteLine(\"{0} + {1} = {2} + {3}\", 1, 2, 2, 1)",
+        };
+    }
+}

src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs

@@ -36,6 +36,7 @@ internal static class AnalyzerIdentifiers
         internal const string TestCaseSourceMismatchWithTestMethodParameterType = "NUnit1030";
         internal const string ValuesParameterTypeMismatchUsage = "NUnit1031";
         internal const string FieldIsNotDisposedInTearDown = "NUnit1032";
+        internal const string TestContextWriteIsObsolete = "NUnit1033";
 
         #endregion Structure
 

src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs

@@ -52,6 +52,10 @@ public static class NUnitFrameworkConstants
         public const string NameOfMultiple = "Multiple";
         public const string NameOfMultipleAsync = "MultipleAsync";
 
+        public const string NameOfOut = "Out";
+        public const string NameOfWrite = "Write";
+        public const string NameOfWriteLine = "WriteLine";
+
         public const string NameOfThrows = "Throws";
         public const string NameOfThrowsArgumentException = "ArgumentException";
         public const string NameOfThrowsArgumentNullException = "ArgumentNullException";
@@ -153,6 +157,8 @@ public static class NUnitFrameworkConstants
         public const string FullNameOfCancelAfterAttribute = "NUnit.Framework.CancelAfterAttribute";
         public const string FullNameOfCancellationToken = "System.Threading.CancellationToken";
 
+        public const string FullNameOfTypeTestContext = "NUnit.Framework.TestContext";
+
         public const string NameOfConstraint = "Constraint";
 
         public const string FullNameOfSameAsConstraint = "NUnit.Framework.Constraints.SameAsConstraint";

src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs

@@ -0,0 +1,68 @@
+using System.Collections.Immutable;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+using NUnit.Analyzers.Constants;
+
+namespace NUnit.Analyzers.TestContextWriteIsObsolete
+{
+    [DiagnosticAnalyzer(LanguageNames.CSharp)]
+    public class TestContextWriteIsObsoleteAnalyzer : DiagnosticAnalyzer
+    {
+        private static readonly DiagnosticDescriptor descriptor = DiagnosticDescriptorCreator.Create(
+            id: AnalyzerIdentifiers.TestContextWriteIsObsolete,
+            title: TestContextWriteIsObsoleteAnalyzerConstants.Title,
+            messageFormat: TestContextWriteIsObsoleteAnalyzerConstants.Message,
+            category: Categories.Structure,
+            defaultSeverity: DiagnosticSeverity.Warning,
+            description: TestContextWriteIsObsoleteAnalyzerConstants.Description);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(descriptor);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+            context.EnableConcurrentExecution();
+            context.RegisterCompilationStartAction(AnalyzeCompilationStart);
+        }
+
+        private static void AnalyzeCompilationStart(CompilationStartAnalysisContext context)
+        {
+            INamedTypeSymbol? testContextType = context.Compilation.GetTypeByMetadataName(NUnitFrameworkConstants.FullNameOfTypeTestContext);
+            if (testContextType is null)
+            {
+                return;
+            }
+
+            context.RegisterOperationAction(context => AnalyzeInvocation(testContextType, context), OperationKind.Invocation);
+        }
+
+        private static void AnalyzeInvocation(INamedTypeSymbol testContextType, OperationAnalysisContext context)
+        {
+            if (context.Operation is not IInvocationOperation invocationOperation)
+                return;
+
+            // TestContext.Write methods are static methods
+            if (invocationOperation.Instance is not null)
+                return;
+
+            IMethodSymbol targetMethod = invocationOperation.TargetMethod;
+
+            if (!targetMethod.ReturnsVoid)
+                return;
+
+            context.CancellationToken.ThrowIfCancellationRequested();
+
+            if (!SymbolEqualityComparer.Default.Equals(targetMethod.ContainingType, testContextType))
+                return;
+
+            if (targetMethod.Name is NUnitFrameworkConstants.NameOfWrite or
+                                     NUnitFrameworkConstants.NameOfWriteLine)
+            {
+                context.ReportDiagnostic(Diagnostic.Create(
+                    descriptor,
+                    invocationOperation.Syntax.GetLocation()));
+            }
+        }
+    }
+}

src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs

@@ -0,0 +1,9 @@
+namespace NUnit.Analyzers.TestContextWriteIsObsolete
+{
+    internal static class TestContextWriteIsObsoleteAnalyzerConstants
+    {
+        public const string Title = "The Write methods on TestContext will be marked as Obsolete and eventually removed";
+        public const string Message = "The Write methods are wrappers on TestContext.Out";
+        public const string Description = "Direct Write calls should be replaced with Out.Write.";
+    }
+}