From b1fa587153b12902ad70a547582867817661e9a2 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Fri, 26 Jul 2024 14:53:51 +0800 Subject: [PATCH 1/4] Added TestContext.Write Is Obsolete Analyzer --- documentation/NUnit1033.md | 65 ++++++++++++++++++ documentation/index.md | 1 + .../Constants/NUnitFrameworkConstantsTests.cs | 6 ++ ...TestContextWriteIsObsoleteAnalyzerTests.cs | 39 +++++++++++ .../TestContextWriteIsObsoleteCodeFixTests.cs | 36 ++++++++++ .../TestContextWriteIsObsoleteTestCases.cs | 46 +++++++++++++ .../Constants/AnalyzerIdentifiers.cs | 1 + .../Constants/NUnitFrameworkConstants.cs | 6 ++ .../TestContextWriteIsObsoleteAnalyzer.cs | 68 +++++++++++++++++++ ...ContextWriteIsObsoleteAnalyzerConstants.cs | 9 +++ .../TestContextWriteIsObsoleteCodeFix.cs | 66 ++++++++++++++++++ 11 files changed, 343 insertions(+) create mode 100644 documentation/NUnit1033.md create mode 100644 src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs create mode 100644 src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs create mode 100644 src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteTestCases.cs create mode 100644 src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs create mode 100644 src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs create mode 100644 src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFix.cs diff --git a/documentation/NUnit1033.md b/documentation/NUnit1033.md new file mode 100644 index 00000000..c174d0b8 --- /dev/null +++ b/documentation/NUnit1033.md @@ -0,0 +1,65 @@ +# NUnit1033 + +## The Write methods on TestContext are Obsolete + +| Topic | Value +| :-- | :-- +| Id | NUnit1033 +| Severity | Error +| 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. + +## 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` 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 + +Simple insert `.Out` between `TestContext` and `.Write`. + + +## 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 are Obsolete +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 are Obsolete +Code violating the rule here +#pragma warning restore NUnit1033 // The Write methods on TestContext are Obsolete +``` + +Or put this at the top of the file to disable all instances. + +```csharp +#pragma warning disable NUnit1033 // The Write methods on TestContext are Obsolete +``` + +### Via attribute `[SuppressMessage]` + +```csharp +[System.Diagnostics.CodeAnalysis.SuppressMessage("Structure", + "NUnit1033:The Write methods on TestContext are Obsolete", + Justification = "Reason...")] +``` + diff --git a/documentation/index.md b/documentation/index.md index 44ba0c2e..2984e88d 100644 --- a/documentation/index.md +++ b/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 are Obsolete | :white_check_mark: | :exclamation: | :white_check_mark: | ## Assertion Rules (NUnit2001 - ) diff --git a/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs b/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs index c0e907ae..4ea50688 100644 --- a/src/nunit.analyzers.tests/Constants/NUnitFrameworkConstantsTests.cs +++ b/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)), diff --git a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs new file mode 100644 index 00000000..2ca84ae0 --- /dev/null +++ b/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); + } + } +} diff --git a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs new file mode 100644 index 00000000..c58194c5 --- /dev/null +++ b/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); + } + } +} diff --git a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteTestCases.cs b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteTestCases.cs new file mode 100644 index 00000000..818040bf --- /dev/null +++ b/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)", + }; + } +} diff --git a/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs b/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs index 8abaaa78..79c92c74 100644 --- a/src/nunit.analyzers/Constants/AnalyzerIdentifiers.cs +++ b/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 diff --git a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs b/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs index 300763e9..7f5f7b2c 100644 --- a/src/nunit.analyzers/Constants/NUnitFrameworkConstants.cs +++ b/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"; diff --git a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs new file mode 100644 index 00000000..d87eaf08 --- /dev/null +++ b/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.Error, + description: TestContextWriteIsObsoleteAnalyzerConstants.Description); + + public override ImmutableArray 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())); + } + } + } +} diff --git a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs new file mode 100644 index 00000000..b3549685 --- /dev/null +++ b/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 are Obsolete"; + 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."; + } +} diff --git a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFix.cs b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFix.cs new file mode 100644 index 00000000..0c98a020 --- /dev/null +++ b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFix.cs @@ -0,0 +1,66 @@ +using System.Collections.Immutable; +using System.Composition; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using NUnit.Analyzers.Constants; + +namespace NUnit.Analyzers.TestContextWriteIsObsolete +{ + [ExportCodeFixProvider(LanguageNames.CSharp)] + [Shared] + public class TestContextWriteIsObsoleteCodeFix : CodeFixProvider + { + internal const string InsertOutDescription = "Replace TestContext.Write with TestContext.Out.Write"; + + public override ImmutableArray FixableDiagnosticIds { get; } + = ImmutableArray.Create(AnalyzerIdentifiers.TestContextWriteIsObsolete); + + public sealed override FixAllProvider GetFixAllProvider() + { + return WellKnownFixAllProviders.BatchFixer; + } + + public override async Task RegisterCodeFixesAsync(CodeFixContext context) + { + var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false); + + if (root is null) + { + return; + } + + context.CancellationToken.ThrowIfCancellationRequested(); + + var node = root.FindNode(context.Span); + var invocationNode = node as InvocationExpressionSyntax; + if (invocationNode is null) + return; + + var memberAccessExpression = invocationNode.Expression as MemberAccessExpressionSyntax; + if (memberAccessExpression is null) + return; + + var updatedMemberAccessExpression = + SyntaxFactory.MemberAccessExpression( + memberAccessExpression.Kind(), + SyntaxFactory.MemberAccessExpression( + SyntaxKind.SimpleMemberAccessExpression, + memberAccessExpression.Expression, + SyntaxFactory.IdentifierName(NUnitFrameworkConstants.NameOfOut)), + memberAccessExpression.Name); + + var newRoot = root.ReplaceNode(memberAccessExpression, updatedMemberAccessExpression); + + var codeAction = CodeAction.Create( + InsertOutDescription, + _ => Task.FromResult(context.Document.WithSyntaxRoot(newRoot)), + InsertOutDescription); + + context.RegisterCodeFix(codeAction, context.Diagnostics); + } + } +} From 2149eca53c06b75d491aae019468c01d010dd0d0 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Fri, 26 Jul 2024 14:59:30 +0800 Subject: [PATCH 2/4] Fix line length in help --- documentation/NUnit1033.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/documentation/NUnit1033.md b/documentation/NUnit1033.md index c174d0b8..a60126e0 100644 --- a/documentation/NUnit1033.md +++ b/documentation/NUnit1033.md @@ -16,14 +16,23 @@ Direct Write calls should be replaced with Out.Write. ## 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` 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`. +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` 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 Simple insert `.Out` between `TestContext` and `.Write`. +`TestContext.WriteLine("This isn't right");` + +becomes + +`TestContext.Out.WriteLine("This isn't right");` + ## Configure severity From 9501ab848c8ec060fa82d97c2abeb24ef7c71382 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Fri, 9 Aug 2024 15:10:11 +0800 Subject: [PATCH 3/4] Code review changes --- documentation/NUnit1033.md | 24 ++++++++++++------- documentation/index.md | 2 +- src/nunit.analyzers.sln | 1 + ...TestContextWriteIsObsoleteAnalyzerTests.cs | 2 +- .../TestContextWriteIsObsoleteCodeFixTests.cs | 2 +- .../TestContextWriteIsObsoleteAnalyzer.cs | 2 +- ...ContextWriteIsObsoleteAnalyzerConstants.cs | 2 +- 7 files changed, 21 insertions(+), 14 deletions(-) diff --git a/documentation/NUnit1033.md b/documentation/NUnit1033.md index a60126e0..9e508fef 100644 --- a/documentation/NUnit1033.md +++ b/documentation/NUnit1033.md @@ -1,18 +1,23 @@ # NUnit1033 -## The Write methods on TestContext are Obsolete +## The Write methods on TestContext will be marked as Obsolete and eventually removed | Topic | Value | :-- | :-- | Id | NUnit1033 -| Severity | Error +| 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. +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 @@ -23,9 +28,10 @@ Besides this being inconsistent, later versions of .NET added new overloads, 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 -Simple insert `.Out` between `TestContext` and `.Write`. +Simply insert `.Out` between `TestContext` and `.Write`. `TestContext.WriteLine("This isn't right");` @@ -44,7 +50,7 @@ Configure the severity per project, for more info see ### Via .editorconfig file ```ini -# NUnit1033: The Write methods on TestContext are Obsolete +# NUnit1033: The Write methods on TestContext will be marked as Obsolete and eventually removed dotnet_diagnostic.NUnit1033.severity = chosenSeverity ``` @@ -53,22 +59,22 @@ where `chosenSeverity` can be one of `none`, `silent`, `suggestion`, `warning`, ### Via #pragma directive ```csharp -#pragma warning disable NUnit1033 // The Write methods on TestContext are Obsolete +#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 are Obsolete +#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 are Obsolete +#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 are Obsolete", + "NUnit1033:The Write methods on TestContext will be marked as Obsolete and eventually removed", Justification = "Reason...")] ``` diff --git a/documentation/index.md b/documentation/index.md index 2984e88d..88e55e3a 100644 --- a/documentation/index.md +++ b/documentation/index.md @@ -51,7 +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 are Obsolete | :white_check_mark: | :exclamation: | :white_check_mark: | +| [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 - ) diff --git a/src/nunit.analyzers.sln b/src/nunit.analyzers.sln index 69bbcafb..8cd7c0ed 100644 --- a/src/nunit.analyzers.sln +++ b/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 diff --git a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs index 2ca84ae0..24515785 100644 --- a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs +++ b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerTests.cs @@ -18,7 +18,7 @@ public void AnyDirectWriteMethod(string writeMethodAndParameters) var testCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" public void Test() {{ - TestContext.{writeMethodAndParameters}; + ↓TestContext.{writeMethodAndParameters}; }}"); RoslynAssert.Diagnostics(analyzer, expectedDiagnostic, testCode); diff --git a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs index c58194c5..48a4e8b1 100644 --- a/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs +++ b/src/nunit.analyzers.tests/TestContextWriteIsObsolete/TestContextWriteIsObsoleteCodeFixTests.cs @@ -20,7 +20,7 @@ public void AnyWriteMethod(string writeMethodAndParameters) var code = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" public void Test() {{ - TestContext.{writeMethodAndParameters}; + ↓TestContext.{writeMethodAndParameters}; }}"); var fixedCode = TestUtility.WrapMethodInClassNamespaceAndAddUsings($@" diff --git a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs index d87eaf08..325c1c8d 100644 --- a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs +++ b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzer.cs @@ -14,7 +14,7 @@ public class TestContextWriteIsObsoleteAnalyzer : DiagnosticAnalyzer title: TestContextWriteIsObsoleteAnalyzerConstants.Title, messageFormat: TestContextWriteIsObsoleteAnalyzerConstants.Message, category: Categories.Structure, - defaultSeverity: DiagnosticSeverity.Error, + defaultSeverity: DiagnosticSeverity.Warning, description: TestContextWriteIsObsoleteAnalyzerConstants.Description); public override ImmutableArray SupportedDiagnostics { get; } = ImmutableArray.Create(descriptor); diff --git a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs index b3549685..88e94ed0 100644 --- a/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs +++ b/src/nunit.analyzers/TestContextWriteIsObsolete/TestContextWriteIsObsoleteAnalyzerConstants.cs @@ -2,7 +2,7 @@ namespace NUnit.Analyzers.TestContextWriteIsObsolete { internal static class TestContextWriteIsObsoleteAnalyzerConstants { - public const string Title = "The Write methods on TestContext are Obsolete"; + 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."; } From bf26b78e0c55eaa7a6d22c1b84a09579ee92e488 Mon Sep 17 00:00:00 2001 From: Manfred Brands Date: Fri, 9 Aug 2024 15:14:28 +0800 Subject: [PATCH 4/4] Fix linting error --- documentation/NUnit1033.md | 1 - 1 file changed, 1 deletion(-) diff --git a/documentation/NUnit1033.md b/documentation/NUnit1033.md index 9e508fef..1e4b8e82 100644 --- a/documentation/NUnit1033.md +++ b/documentation/NUnit1033.md @@ -28,7 +28,6 @@ Besides this being inconsistent, later versions of .NET added new overloads, 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`.